From 731cc832661d87601b1a3e7d7a381f2ec6116f1f Mon Sep 17 00:00:00 2001 From: "Michael J. Giarlo" Date: Wed, 1 May 2024 17:07:56 -0700 Subject: [PATCH] Remove the event_factory dependency injection It is never used and adds noise. --- app/controllers/objects_controller.rb | 2 +- app/jobs/create_virtual_objects_job.rb | 4 +-- app/services/constituent_service.rb | 11 +++--- app/services/create_object_service.rb | 12 +++---- app/services/delete_service.rb | 11 +++--- app/services/update_object_service.rb | 16 ++++----- app/services/version_service.rb | 29 +++++++--------- db/structure.sql | 7 ++++ spec/jobs/create_virtual_objects_job_spec.rb | 4 +-- spec/requests/start_accession_spec.rb | 2 +- spec/services/constituent_service_spec.rb | 8 ++--- spec/services/delete_service_spec.rb | 16 +++------ spec/services/version_service_spec.rb | 35 ++++++++++---------- 13 files changed, 70 insertions(+), 87 deletions(-) diff --git a/app/controllers/objects_controller.rb b/app/controllers/objects_controller.rb index af280e968..968b27e6f 100644 --- a/app/controllers/objects_controller.rb +++ b/app/controllers/objects_controller.rb @@ -91,7 +91,7 @@ def accession message: 'This object cannot be opened for versioning.') end - updated_cocina_object = version_service.open(cocina_object: @cocina_object, assume_accessioned: false, event_factory: EventFactory, **version_open_params) + updated_cocina_object = version_service.open(cocina_object: @cocina_object, assume_accessioned: false, **version_open_params) end # initialize workflow diff --git a/app/jobs/create_virtual_objects_job.rb b/app/jobs/create_virtual_objects_job.rb index ef60966a0..141e21b32 100644 --- a/app/jobs/create_virtual_objects_job.rb +++ b/app/jobs/create_virtual_objects_job.rb @@ -13,8 +13,8 @@ def perform(virtual_objects:, background_job_result:) virtual_objects.each do |virtual_object| virtual_object_id, constituent_ids = virtual_object.values_at(:virtual_object_id, :constituent_ids) # Update the constituent relationship between the virtual_object and constituent druids - result = ConstituentService.new(virtual_object_druid: virtual_object_id, - event_factory: EventFactory).add(constituent_druids: constituent_ids) + result = ConstituentService.new(virtual_object_druid: virtual_object_id) + .add(constituent_druids: constituent_ids) # Do not add `nil`s to the errors array as they signify successful # creation of the virtual object errors << result if result.present? diff --git a/app/services/constituent_service.rb b/app/services/constituent_service.rb index 68e455066..67836b8f4 100644 --- a/app/services/constituent_service.rb +++ b/app/services/constituent_service.rb @@ -8,9 +8,8 @@ class ConstituentService VERSION_DESCRIPTION = 'Virtual object created' # @param [String] virtual_object_druid the identifier of the virtual object - def initialize(virtual_object_druid:, event_factory:) + def initialize(virtual_object_druid:) @virtual_object_druid = virtual_object_druid - @event_factory = event_factory end # This resets the structural metadata of the virtual object and then adds the constituent resources. @@ -32,16 +31,14 @@ def add(constituent_druids:) virtual_object else VersionService.open(cocina_object: virtual_object, - description: VERSION_DESCRIPTION, - event_factory:) + description: VERSION_DESCRIPTION) end updated_virtual_object = ResetContentMetadataService.reset(cocina_item: updated_virtual_object, constituent_druids:) UpdateObjectService.update(updated_virtual_object) - VersionService.close(druid: updated_virtual_object.externalIdentifier, version: updated_virtual_object.version, - event_factory:) + VersionService.close(druid: updated_virtual_object.externalIdentifier, version: updated_virtual_object.version) Indexer.reindex(cocina_object: updated_virtual_object) @@ -52,7 +49,7 @@ def add(constituent_druids:) private - attr_reader :virtual_object_druid, :event_factory + attr_reader :virtual_object_druid def virtual_object @virtual_object ||= ItemQueryService.find_combinable_item(virtual_object_druid) diff --git a/app/services/create_object_service.rb b/app/services/create_object_service.rb index 9f2ed1769..4e7759cb4 100644 --- a/app/services/create_object_service.rb +++ b/app/services/create_object_service.rb @@ -11,18 +11,16 @@ class CreateObjectService # @param [Cocina::Models::RequestDRO,Cocina::Models::RequestCollection,Cocina::Models::RequestAdminPolicy] cocina_object # @param [boolean] assign_doi - # @param [#create] event_factory creates events # @param [#call] id_minter assigns identifiers. You can provide your own minter if you want to use a specific druid for an item. # @return [Cocina::Models::DROWithMetadata,Cocina::Models::CollectionWithMetadata,Cocina::Models::AdminPolicyWithMetadata] # @raise [Catalog::MarcService::MarcServiceError::CatalogRecordNotFoundError] if catalog identifer not found when refreshing descMetadata # @raise [Catalog::MarcService::MarcServiceError::CatalogResponseError] if other error occurred refreshing descMetadata from catalog source # @raise [Cocina::ValidationError] raised when validation of the Cocina object fails. - def self.create(cocina_request_object, assign_doi: false, event_factory: EventFactory, id_minter: -> { SuriService.mint_id }) - new(event_factory:, id_minter:).create(cocina_request_object, assign_doi:) + def self.create(cocina_request_object, assign_doi: false, id_minter: -> { SuriService.mint_id }) + new(id_minter:).create(cocina_request_object, assign_doi:) end - def initialize(event_factory: EventFactory, id_minter: -> { SuriService.mint_id }) - @event_factory = event_factory + def initialize(id_minter: -> { SuriService.mint_id }) @id_minter = id_minter end @@ -43,7 +41,7 @@ def create(cocina_request_object, assign_doi: false) # This creates version 1 (Initial Version) ObjectVersion.initial_version(druid:) - event_factory.create(druid:, event_type: 'registration', data: cocina_object.to_h) + EventFactory.create(druid:, event_type: 'registration', data: cocina_object.to_h) # Broadcast this to a topic Notifications::ObjectCreated.publish(model: cocina_object_with_metadata) @@ -53,7 +51,7 @@ def create(cocina_request_object, assign_doi: false) private - attr_reader :event_factory, :id_minter + attr_reader :id_minter # If an object references the Ur-AdminPolicy, it has to exist first. # This is particularly important in testing, where the repository may be empty. diff --git a/app/services/delete_service.rb b/app/services/delete_service.rb index 5a2afe610..8cda80f5c 100644 --- a/app/services/delete_service.rb +++ b/app/services/delete_service.rb @@ -11,14 +11,13 @@ class DeleteService # - Removes content from purl # - Removes active workflows # @param [Cocina::Models::DRO|AdminPolicy||Collection] cocina object wish to remove - def self.destroy(cocina_object, user_name:, event_factory: EventFactory) - new(cocina_object, user_name, event_factory).destroy + def self.destroy(cocina_object, user_name:) + new(cocina_object, user_name).destroy end - def initialize(cocina_object, user_name, event_factory) + def initialize(cocina_object, user_name) @cocina_object = Cocina::Models.without_metadata(cocina_object) @user_name = user_name - @event_factory = event_factory end def destroy @@ -27,12 +26,12 @@ def destroy cleanup_purl_doc_cache remove_active_workflows delete_from_dor - event_factory.create(druid:, event_type: 'delete', data: { request: cocina_object.to_h, source_id: cocina_object&.identification&.sourceId, user_name: }) + EventFactory.create(druid:, event_type: 'delete', data: { request: cocina_object.to_h, source_id: cocina_object&.identification&.sourceId, user_name: }) end private - attr_reader :cocina_object, :event_factory, :user_name + attr_reader :cocina_object, :user_name def cleanup_stacks stacks_druid = DruidTools::StacksDruid.new(druid, Settings.stacks.local_stacks_root) diff --git a/app/services/update_object_service.rb b/app/services/update_object_service.rb index c8afeac63..db8b19471 100644 --- a/app/services/update_object_service.rb +++ b/app/services/update_object_service.rb @@ -8,22 +8,20 @@ class UpdateObjectService # Normalizes, validates, and updates a Cocina object in the datastore. # Since normalization is performed, the Cocina object that is returned may differ from the Cocina object that is provided. # @param [Cocina::Models::DRO|Collection|AdminPolicy|DROWithMetadata|CollectionWithMetadata|AdminPolicyWithMetadata] cocina_object - # @param [#create] event_factory creates events # @param [boolean] skip_lock do not perform an optimistic lock check # @param [boolean] skip_open_check do not check that the object has an open version # @raise [Cocina::ValidationError] raised when validation of the Cocina object fails. # @raise [CocinaObjectNotFoundError] raised if the cocina object does not already exist in the datastore. # @raise [StateLockError] raised if optimistic lock failed. # @return [Cocina::Models::DRO, Cocina::Models::Collection, Cocina::Models::AdminPolicy] normalized cocina object - def self.update(cocina_object, event_factory: EventFactory, skip_lock: false, skip_open_check: false) - new(cocina_object:, skip_lock:, event_factory:, skip_open_check:).update + def self.update(cocina_object, skip_lock: false, skip_open_check: false) + new(cocina_object:, skip_lock:, skip_open_check:).update end - def initialize(cocina_object:, skip_lock:, skip_open_check:, event_factory: EventFactory) + def initialize(cocina_object:, skip_lock:, skip_open_check:) @cocina_object = cocina_object @skip_lock = skip_lock @skip_open_check = skip_open_check - @event_factory = event_factory end def update @@ -50,7 +48,7 @@ def update compare_legacy - event_factory.create(druid:, event_type: 'update', data: { success: true, request: cocina_object_without_metadata.to_h }) + EventFactory.create(druid:, event_type: 'update', data: { success: true, request: cocina_object_without_metadata.to_h }) Indexer.reindex_later(cocina_object: cocina_object_with_metadata) @@ -58,14 +56,14 @@ def update PublishItemsModifiedJob.perform_later(druid) if update_items cocina_object_with_metadata rescue Cocina::ValidationError => e - event_factory.create(druid:, event_type: 'update', - data: { success: false, error: e.message, request: Cocina::Models.without_metadata(cocina_object).to_h }) + EventFactory.create(druid:, event_type: 'update', + data: { success: false, error: e.message, request: Cocina::Models.without_metadata(cocina_object).to_h }) raise end private - attr_reader :cocina_object, :skip_lock, :event_factory, :skip_open_check + attr_reader :cocina_object, :skip_lock, :skip_open_check delegate :version, to: :cocina_object diff --git a/app/services/version_service.rb b/app/services/version_service.rb index 2450d4435..5989c86ae 100644 --- a/app/services/version_service.rb +++ b/app/services/version_service.rb @@ -14,13 +14,12 @@ def self.open?(...) # @param [String] description set description of version change # @param [String] opening_user_name add opening username to the events datastream # @param [Boolean] assume_accessioned If true, does not check whether object has been accessioned. - # @param [Class] event_factory (EventFactory) the factory for creating events - def self.open(cocina_object:, description:, event_factory: EventFactory, opening_user_name: nil, assume_accessioned: false) - new(druid: cocina_object.externalIdentifier, version: cocina_object.version).open(description:, - opening_user_name:, - assume_accessioned:, - event_factory:, - cocina_object:) + def self.open(cocina_object:, description:, opening_user_name: nil, assume_accessioned: false) + new(druid: cocina_object.externalIdentifier, version: cocina_object.version) + .open(description:, + opening_user_name:, + assume_accessioned:, + cocina_object:) end # @param [String] druid of the item @@ -35,12 +34,10 @@ def self.can_open?(druid:, version:, assume_accessioned: false) # @param [String] description describes the version change # @param [String] user_name add username to the events datastream # @param [Boolean] start_accession (true) set to true if you want accessioning to start, false otherwise - # @param [Class] event_factory (EventFactory) the factory for creating events - def self.close(druid:, version:, event_factory: EventFactory, description: nil, user_name: nil, start_accession: true) + def self.close(druid:, version:, description: nil, user_name: nil, start_accession: true) new(druid:, version:).close(description:, user_name:, - start_accession:, - event_factory:) + start_accession:) end # @param [String] druid of the item @@ -61,11 +58,10 @@ def initialize(druid:, version:) # @param [String] description set description of version change # @param [String] opening_user_name add opening username to the events datastream # @param [Boolean] assume_accessioned If true, does not check whether object has been accessioned. - # @param [Class] event_factory (EventFactory) the factory for creating events # @return [Cocina::Models::DRO, Cocina::Models::AdminPolicy, Cocina::Models::Collection] updated cocina object # @raise [VersionService::VersioningError] if the object hasn't been accessioned, or if a version is already opened # @raise [Preservation::Client::Error] if bad response from preservation catalog. - def open(cocina_object:, description:, opening_user_name:, assume_accessioned:, event_factory:) + def open(cocina_object:, description:, opening_user_name:, assume_accessioned:) raise ArgumentError, 'description is required to open a new version' if description.blank? ensure_openable!(assume_accessioned:) @@ -91,7 +87,7 @@ def open(cocina_object:, description:, opening_user_name:, assume_accessioned:, workflow_client.create_workflow_by_name(druid, 'versioningWF', version: new_object_version.version.to_s) - event_factory.create(druid:, event_type: 'version_open', data: { who: opening_user_name, version: new_object_version.version.to_s }) + EventFactory.create(druid:, event_type: 'version_open', data: { who: opening_user_name, version: new_object_version.version.to_s }) update_cocina_object end @@ -110,11 +106,10 @@ def can_open?(assume_accessioned: false) # Sets versioningWF:submit-version to completed and initiates accessionWF for the object # @param [String] :description describes the version change # @param [String] :user_name add username to the events datastream - # @param [Class] event_factory (EventFactory) the factory for creating events # @param [Boolean] :start_accession set to true if you want accessioning to start (default), false otherwise # @raise [VersionService::VersioningError] if the object hasn't been opened for versioning, or if accessionWF has # already been instantiated or the current version is missing a description - def close(description:, user_name:, event_factory:, start_accession: true) + def close(description:, user_name:, start_accession: true) ObjectVersion.update_current_version(druid:, description:) if description ensure_closeable! @@ -128,7 +123,7 @@ def close(description:, user_name:, event_factory:, start_accession: true) # RepositoryObject.find_by!(external_identifier: druid).close_version! RepositoryObject.find_by(external_identifier: druid)&.close_version!(description:) - event_factory.create(druid:, event_type: 'version_close', data: { who: user_name, version: version.to_s }) + EventFactory.create(druid:, event_type: 'version_close', data: { who: user_name, version: version.to_s }) end # Determines whether a version can be closed for an object. diff --git a/db/structure.sql b/db/structure.sql index 31071efb1..bf777b28b 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -9,6 +9,13 @@ SET xmloption = content; SET client_min_messages = warning; SET row_security = off; +-- +-- Name: public; Type: SCHEMA; Schema: -; Owner: - +-- + +-- *not* creating schema, since initdb creates it + + -- -- Name: background_job_result_status; Type: TYPE; Schema: public; Owner: - -- diff --git a/spec/jobs/create_virtual_objects_job_spec.rb b/spec/jobs/create_virtual_objects_job_spec.rb index a44b26a3c..c3bf914b5 100644 --- a/spec/jobs/create_virtual_objects_job_spec.rb +++ b/spec/jobs/create_virtual_objects_job_spec.rb @@ -12,7 +12,7 @@ before do allow(ConstituentService).to receive(:new) - .with(virtual_object_druid: virtual_object_id, event_factory: EventFactory).and_return(service) + .with(virtual_object_druid: virtual_object_id).and_return(service) allow(BackgroundJobResult).to receive(:find).and_return(result) allow(result).to receive(:processing!) end @@ -77,7 +77,7 @@ before do allow(ConstituentService).to receive(:new) - .with(virtual_object_druid: other_virtual_object_id, event_factory: EventFactory).and_return(service) + .with(virtual_object_druid: other_virtual_object_id).and_return(service) allow(service).to receive(:add).and_return(nil, virtual_object_id => ['One thing was not combinable', 'And another']) described_class.perform_now(virtual_objects:, background_job_result: result) diff --git a/spec/requests/start_accession_spec.rb b/spec/requests/start_accession_spec.rb index 80a2f6341..da18a6d62 100644 --- a/spec/requests/start_accession_spec.rb +++ b/spec/requests/start_accession_spec.rb @@ -63,7 +63,7 @@ expect(version_service).to have_received(:open).with( assume_accessioned: false, cocina_object:, - event_factory: EventFactory, **params + **params ) end diff --git a/spec/services/constituent_service_spec.rb b/spec/services/constituent_service_spec.rb index 5dce38c8c..711322c44 100644 --- a/spec/services/constituent_service_spec.rb +++ b/spec/services/constituent_service_spec.rb @@ -5,11 +5,10 @@ RSpec.describe ConstituentService do describe '#add' do let(:constituent_druids) { ['druid:xh235dd9059'] } - let(:event_factory) { class_double(EventFactory) } let(:item_errors) { {} } let(:mock_item) { build(:dro) } let(:open_for_versioning) { true } - let(:service) { described_class.new(virtual_object_druid:, event_factory:) } + let(:service) { described_class.new(virtual_object_druid:) } let(:virtual_object) { build(:dro_with_metadata, id: virtual_object_druid) } let(:virtual_object_druid) { 'druid:bc123df4567' } @@ -45,8 +44,7 @@ it 'opens virtual object for versioning' do service.add(constituent_druids:) expect(VersionService).to have_received(:open).with(cocina_object: virtual_object, - description: ConstituentService::VERSION_DESCRIPTION, - event_factory:) + description: ConstituentService::VERSION_DESCRIPTION) end end @@ -57,7 +55,7 @@ it 'closes open version' do service.add(constituent_druids:) - expect(VersionService).to have_received(:close).with(druid: virtual_object.externalIdentifier, version: virtual_object.version, event_factory:) + expect(VersionService).to have_received(:close).with(druid: virtual_object.externalIdentifier, version: virtual_object.version) end it 'indexes virtual object' do diff --git a/spec/services/delete_service_spec.rb b/spec/services/delete_service_spec.rb index 8faa7c9db..3064789f7 100644 --- a/spec/services/delete_service_spec.rb +++ b/spec/services/delete_service_spec.rb @@ -3,28 +3,20 @@ require 'rails_helper' RSpec.describe DeleteService do - let(:service) { described_class.new(cocina_object, user_name, event_factory) } - + let(:service) { described_class.new(cocina_object, user_name) } let(:cocina_object) { build(:dro, id: druid, source_id:) } - let(:druid) { 'druid:bb408qn5061' } - let(:source_id) { 'hydrus:object-63-sdr-client' } - - let(:event_factory) { class_double(EventFactory, create: nil) } - let(:user_name) { 'some_person' } - let(:client) { instance_double(Dor::Workflow::Client, delete_all_workflows: nil) } before do allow(WorkflowClientFactory).to receive(:build).and_return(client) + allow(EventFactory).to receive(:create).and_return(nil) end describe '#destroy' do - # let(:dro) { create(:ar_dro, external_identifier: druid) } - - subject(:destroy) { described_class.destroy(cocina_object, user_name:, event_factory:) } + subject(:destroy) { described_class.destroy(cocina_object, user_name:) } before do Dro.upsert_cocina(cocina_object) @@ -34,7 +26,7 @@ it 'destroys the object' do expect { destroy }.to change(Dro, :count).by(-1) - expect(event_factory).to have_received(:create).with(druid:, event_type: 'delete', data: { request: cocina_object.to_h, source_id:, user_name: }) + expect(EventFactory).to have_received(:create).with(druid:, event_type: 'delete', data: { request: cocina_object.to_h, source_id:, user_name: }) expect(Indexer).to have_received(:delete) end diff --git a/spec/services/version_service_spec.rb b/spec/services/version_service_spec.rb index 443914d6a..154ab1767 100644 --- a/spec/services/version_service_spec.rb +++ b/spec/services/version_service_spec.rb @@ -6,16 +6,16 @@ let(:druid) { 'druid:xz456jk0987' } let(:cocina_object) { create(:ar_dro, external_identifier: druid).to_cocina_with_metadata } let(:version) { 1 } - let(:event_factory) { class_double(EventFactory, create: true) } let(:workflow_state_service) { instance_double(WorkflowStateService) } before do allow(WorkflowStateService).to receive(:new).and_return(workflow_state_service) allow(Indexer).to receive(:reindex_later) + allow(EventFactory).to receive(:create).and_return(true) end describe '.open' do - subject(:open) { described_class.open(cocina_object:, description: 'same as it ever was', opening_user_name: 'sunetid', event_factory:) } + subject(:open) { described_class.open(cocina_object:, description: 'same as it ever was', opening_user_name: 'sunetid') } let(:workflow_client) do instance_double(Dor::Workflow::Client, create_workflow_by_name: true) @@ -54,9 +54,9 @@ expect(current_version.version).to eq(2) expect(current_version.description).to eq('same as it ever was') - expect(event_factory).to have_received(:create).with(data: { version: '2', who: 'sunetid' }, - druid:, - event_type: 'version_open') + expect(EventFactory).to have_received(:create).with(data: { version: '2', who: 'sunetid' }, + druid:, + event_type: 'version_open') expect(repository_object.reload.opened_version.version).to eq 2 expect(repository_object.opened_version.version_description).to eq 'same as it ever was' end @@ -79,9 +79,9 @@ expect(workflow_state_service).to have_received(:accessioning?) expect(workflow_client).to have_received(:create_workflow_by_name).with(druid, 'versioningWF', version: '2') - expect(event_factory).to have_received(:create).with(data: { version: '2', who: 'sunetid' }, - druid:, - event_type: 'version_open') + expect(EventFactory).to have_received(:create).with(data: { version: '2', who: 'sunetid' }, + druid:, + event_type: 'version_open') expect(repository_object.reload.opened_version).to be_present end end @@ -132,9 +132,9 @@ expect(Dro.find_by(external_identifier: druid).version).to eq 2 expect(workflow_client).to have_received(:create_workflow_by_name).with(druid, 'versioningWF', version: '2') - expect(event_factory).to have_received(:create).with(data: { version: '2', who: 'sunetid' }, - druid:, - event_type: 'version_open') + expect(EventFactory).to have_received(:create).with(data: { version: '2', who: 'sunetid' }, + druid:, + event_type: 'version_open') end end @@ -156,9 +156,9 @@ expect(Dro.find_by(external_identifier: druid).version).to eq 2 expect(workflow_client).to have_received(:create_workflow_by_name).with(druid, 'versioningWF', version: '2') - expect(event_factory).to have_received(:create).with(data: { version: '2', who: 'sunetid' }, - druid:, - event_type: 'version_open') + expect(EventFactory).to have_received(:create).with(data: { version: '2', who: 'sunetid' }, + druid:, + event_type: 'version_open') expect(RepositoryObject.last).to be_open end end @@ -239,7 +239,6 @@ described_class.close(druid:, version:, description:, user_name: 'jcoyne', - event_factory:, start_accession:) end @@ -274,9 +273,9 @@ object_version = ObjectVersion.find_by(druid:, version: 2) expect(object_version.description).to eq('closing text') - expect(event_factory).to have_received(:create).with(data: { version: '2', who: 'jcoyne' }, - druid:, - event_type: 'version_close') + expect(EventFactory).to have_received(:create).with(data: { version: '2', who: 'jcoyne' }, + druid:, + event_type: 'version_close') expect(workflow_client).to have_received(:close_version) .with(druid:, version: '2', create_accession_wf: true)