speed improvements for files page

there's was a lot of unnecessary database calls going
on, should be a little faster now.

fixes #3939

Change-Id: I9a43e0d801bc632d16248ad92b7c9ff16d1673eb
Reviewed-on: https://gerrit.instructure.com/2484
Reviewed-by: Brian Palmer <brianp@instructure.com>
Tested-by: Hudson <hudson@instructure.com>
This commit is contained in:
Brian Whitmer 2011-03-01 12:17:42 -07:00 committed by Zach Wily
parent 62f22d9df7
commit 98c6d03cc9
8 changed files with 52 additions and 92 deletions

View File

@ -69,34 +69,30 @@ class FilesController < ApplicationController
protected :retrieve_folders
def index
return full_index if !params[:old_style] && request.format != :json
add_crumb("Files", named_context_url(@context, :context_files_url))
if authorized_action(@context.attachments.new, @current_user, :read)
@current_folder = Folder.find_folder(@context, params[:folder_id])
get_quota
retrieve_folders if !request.xhr? || !@current_folder
if !@current_folder || authorized_action(@current_folder, @current_user, :read)
@current_sub_folders = @current_folder.active_sub_folders
if @context.grants_right?(@current_user, session, :manage_files)
@current_attachments = @current_folder.active_file_attachments
else
@current_attachments = @current_folder.visible_file_attachments
@root_folders = @root_folders.select{|f| f.visible? } rescue []
@folders = @folders.select{|f| f.visible? } rescue []
@current_folder = @root_folders[0] if !@current_folder.visible?
end
@current_attachments = @current_attachments.scoped(:include => [:thumbnail, :media_object])
log_asset_access("files:#{@context.asset_string}", "files", 'other')
respond_to do |format|
format.html
format.xml { render :xml => @current_attachments.to_xml }
if request.format == :json
if authorized_action(@context.attachments.new, @current_user, :read)
@current_folder = Folder.find_folder(@context, params[:folder_id])
get_quota
retrieve_folders if !@current_folder
if !@current_folder || authorized_action(@current_folder, @current_user, :read)
if params[:folder_id]
format.json { render :json => @current_attachments.to_json(:methods => [:readable_size, :currently_locked], :permissions => {:user => @current_user, :session => session}) }
if @context.grants_right?(@current_user, session, :manage_files)
@current_attachments = @current_folder.active_file_attachments
else
@current_attachments = @current_folder.visible_file_attachments
@root_folders = @root_folders.select{|f| f.visible? } rescue []
@folders = @folders.select{|f| f.visible? } rescue []
@current_folder = @root_folders[0] if !@current_folder.visible?
end
@current_attachments = @current_attachments.scoped(:include => [:thumbnail, :media_object])
render :json => @current_attachments.to_json(:methods => [:readable_size, :currently_locked], :permissions => {:user => @current_user, :session => session})
else
format.json { render :json => @context.file_structure_for(@current_user).to_json(:permissions => {:user => @current_user}, :methods => [:readable_size, :mime_class, :currently_locked, :collaborator_ids]) }
end
render :json => @context.file_structure_for(@current_user).to_json(:permissions => {:user => @current_user}, :methods => [:readable_size, :mime_class, :currently_locked, :collaborator_ids])
end
end
end
else
full_index
end
end
@ -129,20 +125,14 @@ class FilesController < ApplicationController
return
end
return unless tab_enabled?(@context.class::TAB_FILES)
@file_structures = []
@contexts.each do |context|
Folder.root_folders(context)
context.users rescue nil
@file_structures << [context, context.file_structure_for(@current_user)]
end
@context = UserProfile.new(@context) if @context == @current_user
log_asset_access("files:#{@context.asset_string}", "files", 'other') if @context
respond_to do |format|
if @contexts.empty? #authorized_action(@context, @current_user, :read)
if @contexts.empty?
format.html { redirect_to !@context || @context == @current_user ? dashboard_url : named_context_url(@context, :context_url) }
else
format.html { render :action => 'full_index' }
end
format.xml { render :xml => @file_structures.to_xml }
format.json { render :json => @file_structures.to_json }
end
end
@ -212,7 +202,6 @@ class FilesController < ApplicationController
format.html { render :action => 'show' }
end
@attachment.scribd_doc = nil unless @attachment.grants_right?(@current_user, session, :download)
format.xml { render :xml => @attachment.to_xml }
format.json { render :json => @attachment.to_json(:permissions => {:user => @current_user}) }
end
end
@ -232,7 +221,6 @@ class FilesController < ApplicationController
format.html # show.rhtml
end
@attachment.scribd_doc = nil unless @attachment.grants_right?(@current_user, session, :download)
format.xml { render :xml => @attachment.to_xml }
format.json { render :json => @attachment.to_json(:permissions => {:user => @current_user}) }
end
end
@ -565,12 +553,10 @@ class FilesController < ApplicationController
@attachment.move_to_bottom
flash[:notice] = 'File was successfully uploaded.'
format.html { return_to(params[:return_to], named_context_url(@context, :context_files_url)) }
format.xml { head :created, :location => named_context_url(@context, :context_files_url) }
format.json { render_for_text @attachment.to_json(:allow => :uuid, :methods => [:uuid,:readable_size,:mime_class,:currently_locked,:scribdable?], :permissions => {:user => @current_user, :session => session}) }
format.text { render_for_text @attachment.to_json(:allow => :uuid, :methods => [:uuid,:readable_size,:mime_class,:currently_locked,:scribdable?], :permissions => {:user => @current_user, :session => session}) }
else
format.html { render :action => "new" }
format.xml { render :xml => @attachment.errors.to_xml }
format.json { render :json => @attachment.errors.to_json }
format.text { render :json => @attachment.errors.to_json }
end
@ -604,11 +590,9 @@ class FilesController < ApplicationController
@attachment.move_to_bottom if @folder_id_changed
flash[:notice] = 'File was successfully updated.'
format.html { redirect_to named_context_url(@context, :context_files_url) }
format.xml { head :ok }
format.json { render :json => @attachment.to_json(:methods => [:readable_size, :mime_class, :currently_locked], :permissions => {:user => @current_user, :session => session}), :status => :ok }
else
format.html { render :action => "edit" }
format.xml { render :xml => @attachment.errors.to_xml }
format.json { render :json => @attachment.errors.to_json, :status => :bad_request }
end
end
@ -632,7 +616,6 @@ class FilesController < ApplicationController
@attachment.destroy
respond_to do |format|
format.html { redirect_to named_context_url(@context, :context_files_url) }# show.rhtml
format.xml { render :xml => @attachment.to_xml }
format.json { render :json => @attachment.to_json }
end
end

View File

@ -57,6 +57,7 @@ class Account < ActiveRecord::Base
has_many :active_assignments, :as => :context, :class_name => 'Assignment', :conditions => ['assignments.workflow_state != ?', 'deleted']
has_many :folders, :as => :context, :dependent => :destroy, :order => 'folders.name'
has_many :active_folders, :class_name => 'Folder', :as => :context, :conditions => ['folders.workflow_state != ?', 'deleted'], :order => 'folders.name'
has_many :active_folders_with_sub_folders, :class_name => 'Folder', :as => :context, :include => [:active_sub_folders], :conditions => ['folders.workflow_state != ?', 'deleted'], :order => 'folders.name'
has_many :active_folders_detailed, :class_name => 'Folder', :as => :context, :include => [:active_sub_folders, :active_file_attachments], :conditions => ['folders.workflow_state != ?', 'deleted'], :order => 'folders.name'
has_one :account_authorization_config
has_many :account_reports

View File

@ -76,6 +76,7 @@ class Course < ActiveRecord::Base
has_many :active_assignments, :as => :context, :class_name => 'Assignment', :conditions => ['assignments.workflow_state != ?', 'deleted']
has_many :folders, :as => :context, :dependent => :destroy, :order => 'folders.name'
has_many :active_folders, :class_name => 'Folder', :as => :context, :conditions => ['folders.workflow_state != ?', 'deleted'], :order => 'folders.name'
has_many :active_folders_with_sub_folders, :class_name => 'Folder', :as => :context, :include => [:active_sub_folders], :conditions => ['folders.workflow_state != ?', 'deleted'], :order => 'folders.name'
has_many :active_folders_detailed, :class_name => 'Folder', :as => :context, :include => [:active_sub_folders, :active_file_attachments], :conditions => ['folders.workflow_state != ?', 'deleted'], :order => 'folders.name'
has_many :messages, :as => :context, :dependent => :destroy
belongs_to :wiki

View File

@ -43,6 +43,7 @@ class Group < ActiveRecord::Base
has_many :all_attachments, :as => 'context', :class_name => 'Attachment'
has_many :folders, :as => :context, :dependent => :destroy, :order => 'folders.name'
has_many :active_folders, :class_name => 'Folder', :as => :context, :conditions => ['folders.workflow_state != ?', 'deleted'], :order => 'folders.name'
has_many :active_folders_with_sub_folders, :class_name => 'Folder', :as => :context, :include => [:active_sub_folders], :conditions => ['folders.workflow_state != ?', 'deleted'], :order => 'folders.name'
has_many :active_folders_detailed, :class_name => 'Folder', :as => :context, :include => [:active_sub_folders, :active_file_attachments], :conditions => ['folders.workflow_state != ?', 'deleted'], :order => 'folders.name'
has_many :external_feeds, :as => :context, :dependent => :destroy
has_many :messages, :as => :context, :dependent => :destroy

View File

@ -61,6 +61,7 @@ class User < ActiveRecord::Base
has_many :all_attachments, :as => 'context', :class_name => 'Attachment'
has_many :folders, :as => 'context', :order => 'folders.name'
has_many :active_folders, :class_name => 'Folder', :as => :context, :conditions => ['folders.workflow_state != ?', 'deleted'], :order => 'folders.name'
has_many :active_folders_with_sub_folders, :class_name => 'Folder', :as => :context, :include => [:active_sub_folders], :conditions => ['folders.workflow_state != ?', 'deleted'], :order => 'folders.name'
has_many :active_folders_detailed, :class_name => 'Folder', :as => :context, :include => [:active_sub_folders, :active_file_attachments], :conditions => ['folders.workflow_state != ?', 'deleted'], :order => 'folders.name'
has_many :calendar_events, :as => 'context', :dependent => :destroy
has_many :eportfolios, :dependent => :destroy
@ -1070,25 +1071,18 @@ class User < ActiveRecord::Base
def self.file_structure_for(context, user)
res = {
:contexts => [context],
:groups => [],
:collaborations => [],
:folders => [],
:folders_with_subcontent => [],
:files => []
}
visible_groups = []
if context.respond_to?(:groups) && !context.is_a?(User) && context.grants_right?(user, nil, :manage)
visible_groups = context.groups.active.select{|g| g.grants_right?(user, nil, :read) }
end
res[:contexts] += visible_groups
res[:groups] += visible_groups
context_codes = res[:contexts].map{|c| c.asset_string }
if !context.is_a?(User) && user
res[:collaborations] = user.collaborations.active.find(:all, :include => [:user, :users]).select{|c| c.context_id && c.context_type && context_codes.include?("#{c.context_type.underscore}_#{c.context_id}") }
res[:collaborations] = res[:collaborations].sort_by{|c| c.created_at}.reverse
end
res[:contexts].each do |context|
res[:folders] += context.active_folders_detailed
res[:folders] += context.active_folders_with_sub_folders
end
res[:folders] = res[:folders].sort_by{|f| [f.parent_folder_id || 0, f.position || 0, f.name || "", f.created_at]}
res

View File

@ -29,43 +29,12 @@
<li id="swfupload_holder">
<div id="file_swf"></div>
</li>
<% @file_structures.each do |context, structure| %>
<% root_folder = structure[:folders].select{|f| f.root_folder? && f.context_id == context.id && f.context_type == context.class.base_ar_class.to_s }.first %>
<% @contexts.each do |context| %>
<% root_folder = Folder.root_folders(context).first %>
<li class="context folder folder_<%= root_folder.id %> <%= context.asset_string %>" style="display: none;">
<span class="text name"><%= context.name %></span>
<span class="text id" style="display: none;"><%= root_folder.id %></span>
<ul>
<%= render :partial => 'files/nested_content', :object => structure[:folders].select{|f| f.parent_folder_id == root_folder.id }, :locals => {:folders => structure[:folders], :skip_ul => true, :exclude_files => true} %>
<% if !context.is_a?(User) && (!structure[:collaborations].empty? || can_do((context.collaborations.new rescue nil), @current_user, :create)) %>
<li class="collaborations">
<span class="text name">collaborations</span>
<ul>
<% structure[:collaborations].select{|c| c.context == context}.each do |collaboration| %>
<li class="collaboration <%= collaboration.collaboration_type %> collaboration_<%= collaboration.id %>">
<span class="text name"><%= collaboration.title %></span>
<span class="text id" style="display: none;"><%= collaboration.id %></span>
</li>
<% end %>
</ul>
</li>
<% end %>
<% if !structure[:groups].empty? %>
<% structure[:groups].map{|g| g.category}.uniq.each do |category| %>
<li class="groups">
<span class="text name"><%= category %></span>
<ul>
<% structure[:groups].select{|g| g.category == category }.each do |group| %>
<% group_root_folder = Folder.root_folders(group).first %>
<li class="group context folder folder_<%= group_root_folder.id %> <%= group.asset_string %>">
<span class="text name"><%= group.name %></span>
<span class="text id" style="display: none;"><%= group_root_folder.id %></span>
<%= render :partial => 'files/nested_content', :object => group_root_folder.subcontent, :locals => {:folders => structure[:folders]} %>
</li>
<% end %>
</ul>
</li>
<% end %>
<% end %>
</ul>
</li>
<% end %>
@ -330,9 +299,7 @@
<div style="display: none;" id="file_context_links">
<input type="text" id="rename_entry_field" style="width: 200px;"/>
<%= image_tag "ajax-loader.gif", :class => "preview_loading_image" %>
<% contexts = @file_structures.map{|s| s[0]} %>
<% contexts += @file_structures.map{|s| s[1][:groups]}.flatten %>
<% contexts.each do |context| %>
<% @contexts.each do |context| %>
<% root_url = context_url(context, :context_url) rescue '' %>
<a href="<%= root_url %>/files" class="<%= context.asset_string %>_attachments_url">&nbsp;</a>
<a href="<%= root_url %>/folders" class="<%= context.asset_string %>_folders_url">&nbsp;</a>
@ -528,9 +495,7 @@
</div>
<% js_block do %>
<script>
<% @file_structures.each{|c, s| s.delete(:folders) } %>
var fileStructureData = <%= raw @file_structures.to_json(:permissions => {:user => @current_user, :policies => [:create_collaborations,:manage_files,:update,:read_contents,:manage_grades,:read_roster,:download]}, :methods => [:readable_size,:mime_class,:currently_locked,:collaborator_ids]) %>;
var contexts = <%= raw @contexts.to_json(:permissions => {:user => @current_user, :policies => [:create_collaborations,:manage_files,:update,:manage_grades,:read_roster]}, :methods => :asset_string, :include_root => false) %>;
var INST = INST || {};
INST.downloadFolderFiles = function(url) {

View File

@ -17,6 +17,7 @@
*/
var files = {};
var fileStructureData = [];
(function() {
var $files_content = $("#files_content"),
$swfupload_holder = $("#swfupload_holder"),
@ -34,6 +35,20 @@ var files = {};
$files_structure_list = $("#files_structure_list");
$files_content.prepend($swfupload_holder);
files.clearDataCache.cacheIndex = 0;
for(var idx in contexts) {
var obj = {
context: contexts[idx],
context_name: contexts[idx].name,
context_string: contexts[idx].asset_string
};
if(contexts[idx].asset_string) {
var context_type = contexts[idx].asset_string.replace(/_\d+$/, '');
obj[context_type] = contexts[idx];
}
fileStructureData.push([
obj, {}
]);
}
for(var idx in fileStructureData) {
if(fileStructureData[idx]) {
var context = fileStructureData[idx][0];

View File

@ -80,8 +80,8 @@ describe FilesController do
course_with_teacher_logged_in(:active_all => true)
get 'index', :course_id => @course.id
response.should be_success
assigns[:file_structures].should_not be_nil
assigns[:file_structures][0][0].should eql(@course)
assigns[:contexts].should_not be_nil
assigns[:contexts][0].should eql(@course)
end
it "should select a default folder" do
@ -98,10 +98,10 @@ describe FilesController do
a1 = folder_file
get 'index', :course_id => @course.id, :format => 'json'
response.should be_success
assigns[:current_folder].should eql(f1)
assigns[:current_attachments].should_not be_nil
assigns[:current_attachments].should_not be_empty
assigns[:current_attachments][0].should eql(a1)
data = JSON.parse(response.body) rescue nil
data.should_not be_nil
data['contexts'].length.should eql(1)
data['contexts'][0]['course']['id'].should eql(@course.id)
f2 = course_folder
a2 = folder_file