Skip to content

Commit

Permalink
First optimization pass to change create to new whenever possible
Browse files Browse the repository at this point in the history
Partial fix for issue #156 - optimize tests

Also includes fix for issue #151 - remove usage of AddObjectToObject
  • Loading branch information
elrayle committed Jul 20, 2015
1 parent 6479355 commit 6ea9f79
Show file tree
Hide file tree
Showing 36 changed files with 288 additions and 1,008 deletions.
4 changes: 2 additions & 2 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ source 'https://rubygems.org'
# Specify your gem's dependencies in hydra-works.gemspec
gemspec

gem 'activefedora-aggregation', github: 'projecthydra-labs/activefedora-aggregation', ref: '0dfe4f4'
gem 'hydra-pcdm', github: 'projecthydra-labs/hydra-pcdm', ref: '11f46014'
gem 'activefedora-aggregation', github: 'projecthydra-labs/activefedora-aggregation', ref: 'master'
gem 'hydra-pcdm', github: 'projecthydra-labs/hydra-pcdm', ref: 'master'
gem 'hydra-derivatives', github: 'projecthydra/hydra-derivatives', ref: '7a8377c'
gem 'slop', '~> 3.6' # For byebug

Expand Down
3 changes: 1 addition & 2 deletions lib/hydra/works/services/generic_file/add_generic_file.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,7 @@ class AddGenericFileToGenericFile
def self.call( parent_generic_file, child_generic_file )
raise ArgumentError, 'parent_generic_file must be a hydra-works generic file' unless Hydra::Works.generic_file? parent_generic_file
raise ArgumentError, 'child_generic_file must be a hydra-works generic file' unless Hydra::Works.generic_file? child_generic_file
Hydra::PCDM::AddObjectToObject.call( parent_generic_file, child_generic_file )
parent_generic_file.members << child_generic_file
end

end
end
3 changes: 1 addition & 2 deletions lib/hydra/works/services/generic_work/add_generic_file.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,7 @@ class AddGenericFileToGenericWork
def self.call( parent_generic_work, child_generic_file )
raise ArgumentError, 'parent_generic_work must be a hydra-works generic work' unless Hydra::Works.generic_work? parent_generic_work
raise ArgumentError, 'child_generic_file must be a hydra-works generic file' unless Hydra::Works.generic_file? child_generic_file
Hydra::PCDM::AddObjectToObject.call( parent_generic_work, child_generic_file )
parent_generic_work.members << child_generic_file
end

end
end
3 changes: 1 addition & 2 deletions lib/hydra/works/services/generic_work/add_generic_work.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,7 @@ class AddGenericWorkToGenericWork
def self.call( parent_generic_work, child_generic_work )
raise ArgumentError, 'parent_generic_work must be a hydra-works generic work' unless Hydra::Works.generic_work? parent_generic_work
raise ArgumentError, 'child_generic_work must be a hydra-works generic work' unless Hydra::Works.generic_work? child_generic_work
Hydra::PCDM::AddObjectToObject.call( parent_generic_work, child_generic_work )
parent_generic_work.members << child_generic_work
end

end
end
19 changes: 8 additions & 11 deletions spec/hydra/works/models/collection_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,40 +2,37 @@

describe Hydra::Works::Collection do

let(:collection1) { Hydra::Works::Collection.create }
let(:collection2) { Hydra::Works::Collection.create }
let(:collection3) { Hydra::Works::Collection.create }
let(:collection1) { Hydra::Works::Collection.new }
let(:collection2) { Hydra::Works::Collection.new }
let(:collection3) { Hydra::Works::Collection.new }

let(:generic_work1) { Hydra::Works::GenericWork::Base.create }
let(:generic_work2) { Hydra::Works::GenericWork::Base.create }
let(:generic_work1) { Hydra::Works::GenericWork::Base.new }
let(:generic_work2) { Hydra::Works::GenericWork::Base.new }

describe '#collections=' do
it 'should aggregate collections' do
collection1.collections = [collection2, collection3]
collection1.save
expect(collection1.collections).to eq [collection2, collection3]
end
end

describe '#generic_works=' do
it 'should aggregate generic_works' do
collection1.generic_works = [generic_work1,generic_work2]
collection1.save
expect(collection1.generic_works).to eq [generic_work1,generic_work2]
end
end

describe 'Related objects' do
let(:object) { Hydra::PCDM::Object.create }
let(:collection) { Hydra::Works::Collection.create }
let(:object) { Hydra::PCDM::Object.new }
let(:collection) { Hydra::Works::Collection.new }

before do
collection.related_objects = [object]
collection.save
end

it 'persists' do
expect(collection.reload.related_objects).to eq [object]
expect(collection.related_objects).to eq [object]
end
end
end
12 changes: 5 additions & 7 deletions spec/hydra/works/models/generic_file_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,28 +2,26 @@

describe Hydra::Works::GenericFile::Base do

let(:generic_file1) { Hydra::Works::GenericFile::Base.create }
let(:generic_file2) { Hydra::Works::GenericFile::Base.create }
let(:generic_file3) { Hydra::Works::GenericFile::Base.create }
let(:generic_file1) { Hydra::Works::GenericFile::Base.new }
let(:generic_file2) { Hydra::Works::GenericFile::Base.new }
let(:generic_file3) { Hydra::Works::GenericFile::Base.new }

describe '#generic_files=' do
it 'should aggregate generic_files' do
generic_file1.generic_files = [generic_file2, generic_file3]
generic_file1.save
expect(generic_file1.generic_files).to eq [generic_file2, generic_file3]
end
end

describe 'Related objects' do
let(:object1) { Hydra::PCDM::Object.create }
let(:object1) { Hydra::PCDM::Object.new }

before do
generic_file1.related_objects = [object1]
generic_file1.save
end

it 'persists' do
expect(generic_file1.reload.related_objects).to eq [object1]
expect(generic_file1.related_objects).to eq [object1]
end
end

Expand Down
21 changes: 9 additions & 12 deletions spec/hydra/works/models/generic_work_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,27 +2,25 @@

describe Hydra::Works::GenericWork do

let(:generic_work1) { Hydra::Works::GenericWork::Base.create }
let(:generic_work2) { Hydra::Works::GenericWork::Base.create }
let(:generic_work3) { Hydra::Works::GenericWork::Base.create }
let(:generic_work1) { Hydra::Works::GenericWork::Base.new }
let(:generic_work2) { Hydra::Works::GenericWork::Base.new }
let(:generic_work3) { Hydra::Works::GenericWork::Base.new }

let(:generic_file1) { Hydra::Works::GenericFile::Base.create }
let(:generic_file2) { Hydra::Works::GenericFile::Base.create }
let(:generic_file1) { Hydra::Works::GenericFile::Base.new }
let(:generic_file2) { Hydra::Works::GenericFile::Base.new }

let(:pcdm_file1) { Hydra::PCDM::File.new }

describe '#generic_works=' do
it 'should aggregate generic_works' do
generic_work1.generic_works = [generic_work2, generic_work3]
generic_work1.save
expect(generic_work1.generic_works).to eq [generic_work2, generic_work3]
end
end

describe '#generic_files=' do
it 'should aggregate generic_files' do
generic_work1.generic_files = [generic_file1, generic_file2]
generic_work1.save
expect(generic_work1.generic_files).to eq [generic_file1, generic_file2]
end
end
Expand All @@ -33,7 +31,7 @@ class TestWork < Hydra::Works::GenericWork::Base
end
end

subject { TestWork.create(generic_files: [generic_file1]) }
subject { TestWork.new(generic_files: [generic_file1]) }

it "should have many generic files" do
expect(subject.generic_files).to eq [generic_file1]
Expand All @@ -47,16 +45,15 @@ class TestWork < Hydra::Works::GenericWork::Base
end

describe 'Related objects' do
let(:generic_work1) { Hydra::Works::GenericWork::Base.create }
let(:object1) { Hydra::PCDM::Object.create }
let(:generic_work1) { Hydra::Works::GenericWork::Base.new }
let(:object1) { Hydra::PCDM::Object.new }

before do
generic_work1.related_objects = [object1]
generic_work1.save
end

it 'persists' do
expect(generic_work1.reload.related_objects).to eq [object1]
expect(generic_work1.related_objects).to eq [object1]
end
end

Expand Down
74 changes: 13 additions & 61 deletions spec/hydra/works/services/collection/add_collection_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,23 +2,22 @@

describe Hydra::Works::AddCollectionToCollection do

subject { Hydra::Works::Collection.create }
subject { Hydra::Works::Collection.new }

describe '#call' do
context 'with acceptable collections' do
let(:collection1) { Hydra::Works::Collection.create }
let(:collection2) { Hydra::Works::Collection.create }
let(:collection3) { Hydra::Works::Collection.create }
let(:generic_work1) { Hydra::Works::GenericWork::Base.create }
let(:generic_work2) { Hydra::Works::GenericWork::Base.create }
let(:collection1) { Hydra::Works::Collection.new }
let(:collection2) { Hydra::Works::Collection.new }
let(:collection3) { Hydra::Works::Collection.new }
let(:generic_work1) { Hydra::Works::GenericWork::Base.new }
let(:generic_work2) { Hydra::Works::GenericWork::Base.new }

context 'with collections and generic_works' do
before do
Hydra::Works::AddCollectionToCollection.call( subject, collection1 )
Hydra::Works::AddCollectionToCollection.call( subject, collection2 )
Hydra::Works::AddGenericWorkToCollection.call( subject, generic_work1 )
Hydra::Works::AddGenericWorkToCollection.call( subject, generic_work2 )
subject.save
end

it 'should add an generic_work to collection with collections and generic_works' do
Expand All @@ -43,17 +42,15 @@ class Kollection < ActiveFedora::Base
end
end
after { Object.send(:remove_const, :Kollection) }
let(:kollection1) { Kollection.create }
let(:kollection1) { Kollection.new }

it 'should accept implementing collection as a child' do
Hydra::Works::AddCollectionToCollection.call( subject, kollection1 )
subject.save
expect( Hydra::Works::GetCollectionsFromCollection.call( subject ) ).to eq [kollection1]
end

it 'should accept implementing collection as a parent' do
Hydra::Works::AddCollectionToCollection.call( kollection1, collection1 )
subject.save
expect( Hydra::Works::GetCollectionsFromCollection.call( kollection1 ) ).to eq [collection1]
end
end
Expand All @@ -64,30 +61,28 @@ class Cullection < Hydra::Works::Collection
end
end
after { Object.send(:remove_const, :Cullection) }
let(:cullection1) { Cullection.create }
let(:cullection1) { Cullection.new }

it 'should accept extending collection as a child' do
Hydra::Works::AddCollectionToCollection.call( subject, cullection1 )
subject.save
expect( Hydra::Works::GetCollectionsFromCollection.call( subject ) ).to eq [cullection1]
end

it 'should accept extending collection as a parent' do
Hydra::Works::AddCollectionToCollection.call( cullection1, collection1 )
subject.save
expect( Hydra::Works::GetCollectionsFromCollection.call( cullection1 ) ).to eq [collection1]
end
end
end

context 'with unacceptable child collections' do
let(:generic_work1) { Hydra::Works::GenericWork::Base.create }
let(:generic_file1) { Hydra::Works::GenericFile::Base.create }
let(:pcdm_collection1) { Hydra::PCDM::Collection.create }
let(:pcdm_object1) { Hydra::PCDM::Object.create }
let(:generic_work1) { Hydra::Works::GenericWork::Base.new }
let(:generic_file1) { Hydra::Works::GenericFile::Base.new }
let(:pcdm_collection1) { Hydra::PCDM::Collection.new }
let(:pcdm_object1) { Hydra::PCDM::Object.new }
let(:pcdm_file1) { Hydra::PCDM::File.new }
let(:non_PCDM_object) { "I'm not a PCDM object" }
let(:af_base_object) { ActiveFedora::Base.create }
let(:af_base_object) { ActiveFedora::Base.new }

let(:error_message) { 'child_collection must be a hydra-works collection' }

Expand Down Expand Up @@ -119,48 +114,5 @@ class Cullection < Hydra::Works::Collection
expect{ Hydra::Works::AddCollectionToCollection.call( subject, af_base_object ) }.to raise_error(ArgumentError,error_message)
end
end

context 'with unacceptable parent collections' do
let(:collection1) { Hydra::Works::Collection.create }
let(:generic_work1) { Hydra::Works::GenericWork::Base.create }
let(:generic_file1) { Hydra::Works::GenericFile::Base.create }
let(:pcdm_collection1) { Hydra::PCDM::Collection.create }
let(:pcdm_object1) { Hydra::PCDM::Object.create }
let(:pcdm_file1) { Hydra::PCDM::File.new }
let(:non_PCDM_object) { "I'm not a PCDM object" }
let(:af_base_object) { ActiveFedora::Base.create }

let(:error_message) { 'parent_collection must be a hydra-works collection' }

it 'should NOT accept Hydra::Works::GenericWork as parent collection' do
expect{ Hydra::Works::AddCollectionToCollection.call( generic_work1, collection1 ) }.to raise_error(ArgumentError,error_message)
end

it 'should NOT accept Hydra::Works::GenericFile as parent collection' do
expect{ Hydra::Works::AddCollectionToCollection.call( generic_file1, collection1 ) }.to raise_error(ArgumentError,error_message)
end

it 'should NOT accept Hydra::PCDM::Collections as parent collection' do
expect{ Hydra::Works::AddCollectionToCollection.call( pcdm_collection1, collection1 ) }.to raise_error(ArgumentError,error_message)
end

it 'should NOT accept Hydra::PCDM::Objects as parent collection' do
expect{ Hydra::Works::AddCollectionToCollection.call( pcdm_object1, collection1 ) }.to raise_error(ArgumentError,error_message)
end

it 'should NOT accept Hydra::PCDM::Files as parent collection' do
expect{ Hydra::Works::AddCollectionToCollection.call( pcdm_file1, collection1 ) }.to raise_error(ArgumentError,error_message)
end

it 'should NOT accept non-PCDM objects as parent collection' do
expect{ Hydra::Works::AddCollectionToCollection.call( non_PCDM_object, collection1 ) }.to raise_error(ArgumentError,error_message)
end

it 'should NOT accept AF::Base objects as parent collection' do
expect{ Hydra::Works::AddCollectionToCollection.call( af_base_object, collection1 ) }.to raise_error(ArgumentError,error_message)
end
end

end

end
Loading

0 comments on commit 6ea9f79

Please sign in to comment.