Merge pull request #4237 from samvera/backport-to-2-6-remove-collecti…

…on-size-feature-hyrax-4207

Backport to 2 6 remove collection size feature hyrax 4207

app/indexers/hyrax/collection_indexer.rb

@@ -11,9 +11,7 @@ class CollectionIndexer < Hydra::PCDM::CollectionIndexer
     def generate_solr_document
       super.tap do |solr_doc|
         # Makes Collections show under the "Collections" tab
-        Solrizer.set_field(solr_doc, 'generic_type', 'Collection', :facetable)
-        # Index the size of the collection in bytes
-        solr_doc[Solrizer.solr_name(:bytes, STORED_LONG)] = object.bytes
+        solr_doc['generic_type_sim'] = ["Collection"]
         solr_doc['visibility_ssi'] = object.visibility
 
         object.in_collections.each do |col|

app/models/concerns/hyrax/collection_behavior.rb

@@ -93,25 +93,15 @@ def collection_type_gid_document_field_name
       end
     end
 
-    # Compute the sum of each file in the collection using Solr to
-    # avoid having to access Fedora
-    #
-    # @return [Fixnum] size of collection in bytes
-    # @raise [RuntimeError] unsaved record does not exist in solr
+    # @deprecated to be removed in 4.0.0; this feature was replaced with a
+    #   hard-coded null implementation
+    # @return [Fixnum] 0
     def bytes
-      return 0 if member_object_ids.empty?
-
-      raise "Collection must be saved to query for bytes" if new_record?
-
-      # One query per member_id because Solr is not a relational database
-      member_object_ids.collect { |work_id| size_for_work(work_id) }.sum
-    end
-
-    # Use this query to get the ids of the member objects (since the containment
-    # association has been flipped)
-    def member_object_ids
-      return [] unless id
-      ActiveFedora::Base.search_with_conditions("member_of_collection_ids_ssim:#{id}").map(&:id)
+      Deprecation.warn('#bytes has been deprecated for removal in Hyrax 4.0.0; ' \
+                       'The implementation of the indexed Collection size ' \
+                       'feature is extremely inefficient, so it has been removed. ' \
+                       'This method now returns a hard-coded `0` for compatibility.')
+      0
     end
 
     # @api public
@@ -158,22 +148,6 @@ def visibility_group
         []
       end
 
-      # Calculate the size of all the files in the work
-      # @param work_id [String] identifer for a work
-      # @return [Integer] the size in bytes
-      def size_for_work(work_id)
-        argz = { fl: "id, #{file_size_field}",
-                 fq: "{!join from=#{member_ids_field} to=id}id:#{work_id}" }
-        files = ::FileSet.search_with_conditions({}, argz)
-        files.reduce(0) { |sum, f| sum + f[file_size_field].to_i }
-      end
-
-      # Field name to look up when locating the size of each file in Solr.
-      # Override for your own installation if using something different
-      def file_size_field
-        Solrizer.solr_name(:file_size, Hyrax::FileSetIndexer::STORED_LONG)
-      end
-
       # Solr field name works use to index member ids
       def member_ids_field
         Solrizer.solr_name('member_ids', :symbol)

app/presenters/hyrax/collection_presenter.rb

@@ -49,6 +49,9 @@ def terms_with_values
       self.class.terms.select { |t| self[t].present? }
     end
 
+    ##
+    # @param [Symbol] key
+    # @return [Object]
     def [](key)
       case key
       when :size
@@ -60,8 +63,16 @@ def [](key)
       end
     end
 
+    # @deprecated to be removed in 4.0.0; this feature was replaced with a
+    #   hard-coded null implementation
+    # @return [String] 'unknown'
     def size
-      number_to_human_size(@solr_document['bytes_lts'])
+      Deprecation.warn('#size has been deprecated for removal in Hyrax 4.0.0; ' \
+                       'The implementation of the indexed Collection size ' \
+                       'feature is extremely inefficient, so it has been removed. ' \
+                       'This method now returns a hard-coded `"unknown"` for ' \
+                       'compatibility.')
+      'unknown'
     end
 
     def total_items

spec/indexers/hyrax/collection_indexer_spec.rb

@@ -10,7 +10,6 @@
   let(:doc) do
     {
       'generic_type_sim' => ['Collection'],
-      'bytes_lts' => 1000,
       'thumbnail_path_ss' => '/downloads/1234?file=thumbnail',
       'member_of_collection_ids_ssim' => [col1id, col2id],
       'member_of_collections_ssim' => [col1title, col2title],
@@ -20,7 +19,6 @@
 
   describe "#generate_solr_document" do
     before do
-      allow(collection).to receive(:bytes).and_return(1000)
       allow(collection).to receive(:in_collections).and_return([col1, col2])
       allow(Hyrax::ThumbnailPathService).to receive(:call).and_return("/downloads/1234?file=thumbnail")
     end

spec/jobs/characterize_job_spec.rb

@@ -49,18 +49,4 @@
       expect { described_class.perform_now(file_set, file.id) }.to raise_error(StandardError, /original_file was not found/)
     end
   end
-
-  context "when the file set's work is in a collection" do
-    let(:work)       { build(:generic_work) }
-    let(:collection) { build(:collection_lw) }
-
-    before do
-      allow(file_set).to receive(:parent).and_return(work)
-      allow(work).to receive(:in_collections).and_return([collection])
-    end
-    it "reindexes the collection" do
-      expect(collection).to receive(:update_index)
-      described_class.perform_now(file_set, file.id)
-    end
-  end
 end

spec/models/collection_spec.rb

@@ -5,6 +5,13 @@
     expect(collection.read_groups).to eq ['public']
   end
 
+  describe '#bytes' do
+    it 'returns a hard-coded integer and issues a deprecation warning' do
+      expect(Deprecation).to receive(:warn).once
+      expect(collection.bytes).to eq(0)
+    end
+  end
+
   describe "#validates_with" do
     before { collection.title = nil }
     it "ensures the collection has a title" do

spec/presenters/hyrax/collection_presenter_spec.rb

@@ -25,9 +25,6 @@
   let(:presenter) { described_class.new(solr_doc, ability) }
   let(:solr_doc) { SolrDocument.new(collection.to_solr) }
 
-  # Mock bytes so collection does not have to be saved.
-  before { allow(collection).to receive(:bytes).and_return(0) }
-
   describe "collection type methods" do
     subject { presenter }
 
@@ -126,10 +123,11 @@
     it { is_expected.to eq ['adc12v'] }
   end
 
-  describe "#size", :clean_repo do
-    subject { presenter.size }
-
-    it { is_expected.to eq '0 Bytes' }
+  describe '#size' do
+    it 'returns a hard-coded string and issues a deprecation warning' do
+      expect(Deprecation).to receive(:warn).once
+      expect(presenter.size).to eq('unknown')
+    end
   end
 
   describe "#total_items", :clean_repo do
@@ -284,12 +282,6 @@
     end
   end
 
-  describe "#size", :clean_repo do
-    subject { presenter.size }
-
-    it { is_expected.to eq '0 Bytes' }
-  end
-
   describe "#parent_collection_count" do
     subject { presenter.parent_collection_count }
 

spec/views/hyrax/collections/_show_descriptions.html.erb_spec.rb

@@ -1,6 +1,5 @@
 RSpec.describe 'hyrax/collections/_show_descriptions.html.erb', type: :view do
   context 'displaying a custom collection' do
-    let(:collection_size) { 123_456_678 }
     let(:collection) do
       {
         id: '999',
@@ -15,7 +14,6 @@
 
     before do
       allow(presenter).to receive(:total_items).and_return(2)
-      allow(presenter).to receive(:size).and_return("118 MB")
       assign(:presenter, presenter)
     end
 
@@ -25,8 +23,6 @@
       expect(rendered).to include('itemprop="dateCreated"')
       expect(rendered).to have_content 'Total items'
       expect(rendered).to have_content '2'
-      expect(rendered).to have_content 'Size'
-      expect(rendered).to have_content '118 MB'
     end
   end
 end

spec/views/hyrax/dashboard/collections/_show_descriptions.html.erb_spec.rb

@@ -1,6 +1,5 @@
 RSpec.describe 'hyrax/dashboard/collections/_show_descriptions.html.erb', type: :view do
   context 'displaying a custom collection' do
-    let(:collection_size) { 123_456_678 }
     let(:collection) do
       {
         id: '999',
@@ -15,7 +14,6 @@
 
     before do
       allow(presenter).to receive(:total_items).and_return(2)
-      allow(presenter).to receive(:size).and_return("118 MB")
       assign(:presenter, presenter)
     end
 
@@ -25,8 +23,6 @@
       expect(rendered).to include('itemprop="dateCreated"')
       expect(rendered).to have_content 'Total items'
       expect(rendered).to have_content '2'
-      expect(rendered).to have_content 'Size'
-      expect(rendered).to have_content '118 MB'
     end
   end
 end