Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove collection size feature #4207

Merged
merged 2 commits into from
Jan 24, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions app/indexers/hyrax/collection_indexer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@ def generate_solr_document
super.tap do |solr_doc|
# Makes Collections show under the "Collections" tab
solr_doc['generic_type_sim'] = ["Collection"]
# Index the size of the collection in bytes
solr_doc['bytes_lts'] = object.bytes
solr_doc['visibility_ssi'] = object.visibility

object.in_collections.each do |col|
Expand Down
1 change: 0 additions & 1 deletion app/jobs/characterize_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ def characterize(file_set, _file_id, filepath)
file_set.characterization_proxy.alpha_channels = channels(filepath) if file_set.image? && Hyrax.config.iiif_image_server?
file_set.characterization_proxy.save!
file_set.update_index
file_set.parent&.in_collections&.each(&:update_index)
end

def channels(filepath)
Expand Down
35 changes: 8 additions & 27 deletions app/models/concerns/hyrax/collection_behavior.rb
Original file line number Diff line number Diff line change
Expand Up @@ -111,18 +111,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
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
Expand Down Expand Up @@ -169,22 +166,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
"file_size_lts"
end

# Solr field name works use to index member ids
def member_ids_field
"member_ids_ssim"
Expand Down
13 changes: 12 additions & 1 deletion app/presenters/hyrax/collection_presenter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
2 changes: 0 additions & 2 deletions spec/indexers/hyrax/collection_indexer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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],
Expand All @@ -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
Expand Down
14 changes: 0 additions & 14 deletions spec/jobs/characterize_job_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
7 changes: 7 additions & 0 deletions spec/models/collection_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
18 changes: 5 additions & 13 deletions spec/presenters/hyrax/collection_presenter_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 }

Expand Down Expand Up @@ -126,10 +123,11 @@
it { is_expected.to eq ['adc12v'] }
end

describe "#size", :clean_repo do
mjgiarlo marked this conversation as resolved.
Show resolved Hide resolved
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
Expand Down Expand Up @@ -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 }

Expand Down
Original file line number Diff line number Diff line change
@@ -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',
Expand All @@ -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

Expand All @@ -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
Original file line number Diff line number Diff line change
@@ -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',
Expand All @@ -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

Expand All @@ -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