Merge pull request #90 from psu-stewardship/hit_fedora_less_for_bette…

…r_performance

Replaces #load() with #find_instance_from_solr() for performance reasons.

app/controllers/single_use_link_controller.rb

@@ -1,90 +1,94 @@
 require 'sufia/single_use_error'
 
 class SingleUseLinkController < DownloadsController
-  before_filter :authenticate_user!, :except => [:show, :download]
+  before_filter :authenticate_user!, :except => [:download, :show]
+  before_filter :find_file, :only => [:generate_download, :generate_show]
+  before_filter :authorize_user!, :only => [:generate_download, :generate_show]
   skip_filter :normalize_identifier
   prepend_before_filter :normalize_identifier, :except => [:download, :show]
   rescue_from Sufia::SingleUseError, :with => :render_single_use_error
-  
+
   def generate_download
-    @generic_file = GenericFile.find(params[:id])
-    authorize! :read, @generic_file   
     @su =  SingleUseLink.create_download(params[:id])
     @link =  sufia.download_single_use_link_path(@su.downloadKey)
     respond_to do |format|
       format.html
       format.js  {render :js => @link}
     end
-
   end
 
   def generate_show
-    @generic_file = GenericFile.find(params[:id])
-    authorize! :read, @generic_file   
     @su = SingleUseLink.create_show(params[:id])
     @link = sufia.show_single_use_link_path(@su.downloadKey)
     respond_to do |format|
       format.html
       format.js  {render :js => @link}
     end
-
   end
 
   def download
     #look up the item
     link = lookup_hash
 
     #grab the item id
-    id = link.itemId 
-    
+    id = link.itemId
+
     #check to make sure the path matches
     not_found if link.path != sufia.download_path(id)
-    
+
     # send the data content
-    asset = ActiveFedora::Base.find(id, :cast=>true)
+    asset = GenericFile.find(id)
     send_content(asset)
   end
 
   def show
     link = lookup_hash
 
     #grab the item id
-    id = link.itemId 
-    
+    id = link.itemId
+
     #check to make sure the path matches
     not_found if link.path != sufia.generic_file_path(id)
-    
+
     #show the file
-    @generic_file = GenericFile.find(id)
+    @generic_file = GenericFile.load_instance_from_solr(id)
     @terms = @generic_file.terms_for_display
-    
+
     # create a dowload link that is single use for the user since we do not just want to show metadata we want to access it too
     @su =  SingleUseLink.create_download(id)
     @download_link =  sufia.download_single_use_link_path(@su.downloadKey)
   end
-  
+
   protected
-
-  def lookup_hash  
+
+  def authorize_user!
+    authorize! :read, @generic_file
+  end
+
+  def find_file
+    @generic_file = GenericFile.load_instance_from_solr(params[:id])
+  end
+
+  def lookup_hash
     id = params[:id]
     # invalid hash send not found
     link = SingleUseLink.find_by_downloadKey(id) ||  not_found
-     
+
     # expired hash send not found
     now = DateTime.now
-    not_found if link.expires <= now 
-        
+    not_found if link.expires <= now
+
     # delete the link since it has been used
     link.destroy
-    
+
     return link
   end
- 
+
   def not_found
-    raise Sufia::SingleUseError.new('Single Use Link Not Found')
+    raise Sufia::SingleUseError.new('Single-Use Link Not Found')
   end
+
   def expired
-    raise Sufia::SingleUseError.new('Single Use Link Expired')
+    raise Sufia::SingleUseError.new('Single-Use Link Expired')
   end
-
 end

app/helpers/sufia_helper.rb

@@ -39,6 +39,10 @@ def display_user_name(recent_document)
     return User.find_by_user_key(recent_document.depositor).name rescue recent_document.depositor
   end
 
+  def number_of_deposits(user)
+    ActiveFedora::SolrService.query("#{Solrizer.solr_name('depositor', :stored_searchable, :type => :string)}:#{user.user_key}").count
+  end
+
   def link_to_facet(field, field_string)
     link_to(field, add_facet_params(field_string, field).merge!({"controller" => "catalog", :action=> "index"}))
   end

app/models/checksum_audit_log.rb

@@ -26,10 +26,10 @@ def ChecksumAuditLog.prune_history(version)
     ## Check to see if there are previous passing logs that we can delete
     # we want to keep the first passing event after a failure, the most current passing event, and all failures so that this table doesn't grow too large
     # Simple way (a little naieve): if the last 2 were passing, delete the first one
-    logs = GenericFile.find(version.pid).logs(version.dsid)
+    logs = GenericFile.load_instance_from_solr(version.pid).logs(version.dsid)
     list = logs.limit(2)
     if list.size > 1 && (list[0].pass == 1) && (list[1].pass == 1)
-      list[0].delete 
+      list[0].delete
     end
   end
-end  
+end

app/views/dashboard/_index_partials/_list_files.html.erb

@@ -14,22 +14,20 @@ See the License for the specific language governing permissions and
 limitations under the License.
 %>
 
-<!-- scholarsphere over ride -->
-<!-- the sub on ':' is b.c. jQuery has trouble with '.' or ':'
-on element ids -->
 <% noid = document.noid %>
-<tr id="document_<%= noid.gsub(":", "_") %>" class="<%= cycle("","zebra") %>">
-  <% gf = GenericFile.find(document.id) %>
+<tr id="document_<%= noid %>" class="<%= cycle("","zebra") %>">
+  <% gf = GenericFile.load_instance_from_solr(document.id) %>
   <td width="6%"><%= batch_edit_select(document) %>&nbsp;</td>
   <td width="1%">
+  <%# This block is for adding/removing the magic wand while batch updates are processing %>
   <% if gf.processing? %>
      <i class="icon-magic icon-large <%= 'ss-'+gf.batch.noid%>"/>
   <% elsif gf.depositor != @user.user_key %>
      <i class="icon-gift icon-large"/>
   <% end %>
   </td>
   <td width="37%">
-    <a href="" title="Click for more details"><i id="expand_<%= noid.gsub(":", "_") %>" class="icon-plus icon-large fleft"></i></a>&nbsp;
+    <a href="" title="Click for more details"><i id="expand_<%= noid %>" class="icon-plus icon-large fleft"></i></a>&nbsp;
     <%= render :partial => 'dashboard/_index_partials/thumbnail_display', :locals => {:document=>document} %>
     <span class=center><%= link_to document.title_or_label, sufia.generic_file_path(noid), :id => "src_copy_link#{noid}" %> <br /></span>
   </td>
@@ -67,7 +65,7 @@ on element ids -->
     </div>
   </td>
 </tr>
-<tr class="hide" id="detail_<%= noid.gsub(":", "_") %>"> <!--  document detail"> -->
+<tr class="hide" id="detail_<%= noid %>"> <!--  document detail"> -->
   <td colspan="6">
     <table class="expanded-details">
       <tr>

app/views/users/index.html.erb

@@ -36,10 +36,9 @@ limitations under the License.
                    <td ><a href="<%= sufia.profile_path(URI.escape(user.user_key,'@.')) %>"  ><%= user.name %><a></td> 
                    <td><a href="<%= sufia.profile_path(URI.escape(user.user_key,'@.')) %>"  ><%= user.user_key %><a></td>
                    <td><%= user.department %> </td> 
-                   <td><%= GenericFile.find(:depositor => user.to_s).count %> </td> 
+                   <td><%= number_of_deposits(user) %> </td>
                 </tr> 
-
-            <% end %> 
+             <% end %> 
         </tbody> 
     </table> 
     <div class="pager">

lib/sufia/batch_edits_controller_behavior.rb

@@ -3,7 +3,7 @@ module BatchEditsControllerBehavior
     extend ActiveSupport::Concern
 
     def edit
-       super 
+       super
        @generic_file = ::GenericFile.new
        @generic_file.depositor = current_user.user_key
        @terms = @generic_file.terms_for_editing - [:title, :format, :resource_type]
@@ -17,13 +17,13 @@ def edit
 
        # For each of the files in the batch, set the attributes to be the concatination of all the attributes
        batch.each do |doc_id|
-          gf = ::GenericFile.find(doc_id)
+          gf = ::GenericFile.load_instance_from_solr(doc_id)
           gf.terms_for_editing.each do |key|
             h[key] ||= []
-            h[key] = (h[key] + gf.send(key)).uniq 
+            h[key] = (h[key] + gf.send(key)).uniq
           end
-          @names << display_title(gf)    
-          permissions =  (permissions+gf.permissions).uniq
+          @names << display_title(gf)
+          permissions = (permissions + gf.permissions).uniq
        end
 
        initialize_fields(h, @show_file)
@@ -32,58 +32,56 @@ def edit
        # todo sort the access level some how...
        perm_param ={'user'=>{},'group'=>{"public"=>"1"}}
        permissions.each{ |perm| perm_param[perm[:type]][perm[:name]] = perm[:access]}
-       @show_file.permissions = HashWithIndifferentAccess.new(perm_param)       
+       @show_file.permissions = HashWithIndifferentAccess.new(perm_param)
     end
 
-    def after_update 
-     redirect_to sufia.dashboard_index_path unless request.xhr?
+    def after_update
+      redirect_to sufia.dashboard_index_path unless request.xhr?
     end
-   
+
     def update_document(obj)
       super
       obj.date_modified = Time.now.ctime
       obj.set_visibility(params[:visibility])
     end
-    
+
     def update
       # keep the batch around if we are doing ajax calls
-      batch_sav = batch.dup if request.xhr?        
+      batch_sav = batch.dup if request.xhr?
       catalog_index_path = sufia.dashboard_index_path
       type = params["update_type"]
-      if (type == "update")
-        #params["generic_file"].reject! {|k,v| (v.blank? || (v.respond_to?(:length) && v.length==1 && v.first.blank?))}
-        super        
-      elsif (type == "delete_all")
+      if type == "update"
+        super
+      elsif type == "delete_all"
         batch.each do |doc_id|
-          gf = ::GenericFile.find(doc_id)
+          gf = ::GenericFile.load_instance_from_solr(doc_id)
           gf.delete
         end
-        clear_batch! 
+        clear_batch!
         after_update
       end
 
       # reset the batch around if we are doing ajax calls
       if request.xhr?
-        self.batch = batch_sav.dup 
+        self.batch = batch_sav.dup
         @key = params["key"]
-        if (@key != "permissions")
-            @vals = params["generic_file"][@key]
+        if @key != "permissions"
+          @vals = params["generic_file"][@key]
         else
-            @vals = [""]
-        end            
+          @vals = [""]
+        end
         render :update_edit
-      end        
+      end
     end
 
     protected
 
     # override this method if you need to initialize more complex RDF assertions (b-nodes)
     def initialize_fields(attributes, file)
        file.terms_for_editing.each do |key|
-         # if value is empty, we create an one element array to loop over for output 
+         # if value is empty, we create an one element array to loop over for output
          file[key] = attributes[key].empty? ? [''] : attributes[key]
        end
     end
-
   end
 end

lib/sufia/files_controller_behavior.rb

@@ -57,12 +57,6 @@ def edit
       @groups = current_user.groups
     end
 
-    # routed to /files/:id
-    def index
-      @generic_files = ::GenericFile.find(:all, :rows => ::GenericFile.count)
-      render :json => @generic_files.map(&:to_jq_upload).to_json
-    end
-
     # routed to /files/:id (DELETE)
     def destroy
       pid = @generic_file.noid
@@ -81,15 +75,15 @@ def create
         if !file
           json_error "Error! No file for upload", 'unknown file', :status => :unprocessable_entity
         elsif (empty_file?(file))
-          json_error "Error! Zero Length File!", file.original_filename 
+          json_error "Error! Zero Length File!", file.original_filename
         elsif (!terms_accepted?)
           json_error "You must accept the terms of service!", file.original_filename
         else
           process_file(file)
         end
       rescue => error
         logger.error "GenericFilesController::create rescued #{error.class}\n\t#{error.to_s}\n #{error.backtrace.join("\n")}\n\n"
-        json_error "Error occurred while creating generic file." 
+        json_error "Error occurred while creating generic file."
       ensure
         # remove the tempfile (only if it is a temp file)
         file.tempfile.delete if file.respond_to?(:tempfile)