From 7854b349fcc7a711031e00eed6f08c4bacdf9c18 Mon Sep 17 00:00:00 2001 From: Justin Littman Date: Fri, 12 Apr 2024 07:12:52 -0400 Subject: [PATCH] Support skipping open check in UpdateObjectService. --- app/services/update_object_service.rb | 12 +++++---- app/services/version_service.rb | 3 ++- bin/migrate-cocina | 2 +- spec/services/update_object_service_spec.rb | 30 ++++++++++++++++++--- spec/services/version_service_spec.rb | 4 +-- 5 files changed, 38 insertions(+), 13 deletions(-) diff --git a/app/services/update_object_service.rb b/app/services/update_object_service.rb index fcace2902..76183ee71 100644 --- a/app/services/update_object_service.rb +++ b/app/services/update_object_service.rb @@ -10,17 +10,19 @@ class UpdateObjectService # @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) - new(cocina_object:, skip_lock:, event_factory:).update + 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 end - def initialize(cocina_object:, skip_lock:, event_factory: EventFactory) + def initialize(cocina_object:, skip_lock:, skip_open_check:, event_factory: EventFactory) @cocina_object = cocina_object @skip_lock = skip_lock + @skip_open_check = skip_open_check @event_factory = event_factory end @@ -55,7 +57,7 @@ def update private - attr_reader :cocina_object, :skip_lock, :event_factory + attr_reader :cocina_object, :skip_lock, :event_factory, :skip_open_check delegate :version, to: :cocina_object @@ -64,7 +66,7 @@ def druid end def notify_unless_open_version - return if VersionService.open?(druid:, version:) + return if skip_open_check || VersionService.open?(druid:, version:) Honeybadger.notify('Updating repository item without an open version', context: { druid:, version: }) end diff --git a/app/services/version_service.rb b/app/services/version_service.rb index ed2f74e7f..63978812a 100644 --- a/app/services/version_service.rb +++ b/app/services/version_service.rb @@ -85,7 +85,8 @@ def open(cocina_object:, description:, opening_user_name:, assume_accessioned:, update_cocina_object = cocina_object - update_cocina_object = UpdateObjectService.update(cocina_object.new(version: new_object_version.version)) if cocina_object.version != new_object_version.version + # Skipping open check since not yet opened. + update_cocina_object = UpdateObjectService.update(cocina_object.new(version: new_object_version.version), skip_open_check: true) if cocina_object.version != new_object_version.version workflow_client.create_workflow_by_name(druid, 'versioningWF', version: new_object_version.version.to_s) diff --git a/bin/migrate-cocina b/bin/migrate-cocina index 84d93b2ba..5b44f4f5e 100755 --- a/bin/migrate-cocina +++ b/bin/migrate-cocina @@ -90,7 +90,7 @@ def perform_migrate(migrator_class:, obj:, mode:) ## Note: because we are now storing the cocina for every version we cannot remediate cocina breaking changes for a particular cocina version here. if mode == :migrate - updated_cocina_object = UpdateObjectService.update(updated_cocina_object) + updated_cocina_object = UpdateObjectService.update(updated_cocina_object, skip_open_check: !migrator.version?) Publish::MetadataTransferService.publish(updated_cocina_object) if migrator.publish? close_version(cocina_object: updated_cocina_object, version_description: migrator.version_description) unless updated_cocina_object.version == current_object_version end diff --git a/spec/services/update_object_service_spec.rb b/spec/services/update_object_service_spec.rb index dacab9281..eb50545ff 100644 --- a/spec/services/update_object_service_spec.rb +++ b/spec/services/update_object_service_spec.rb @@ -4,7 +4,7 @@ RSpec.describe UpdateObjectService do include Dry::Monads[:result] - let(:store) { described_class.new(cocina_object:, skip_lock: true) } + let(:store) { described_class.new(cocina_object:, skip_lock: true, skip_open_check: false) } let(:open) { true } describe '#update' do @@ -41,7 +41,7 @@ end context 'when checking lock succeeds' do - let(:store) { described_class.new(cocina_object:, skip_lock: false) } + let(:store) { described_class.new(cocina_object:, skip_lock: false, skip_open_check: false) } let(:ar_cocina_object) { create(:ar_dro) } let(:lock) { "#{ar_cocina_object.external_identifier}=0" } @@ -72,7 +72,7 @@ end context 'when checking lock fails' do - let(:store) { described_class.new(cocina_object:, skip_lock: false) } + let(:store) { described_class.new(cocina_object:, skip_lock: false, skip_open_check: false) } let!(:ar_cocina_object) { create(:ar_dro) } let(:lock) { '64e8320d19d62ddb73c501276c5655cf' } @@ -90,7 +90,7 @@ context 'when version is not open' do let(:open) { false } - let(:store) { described_class.new(cocina_object:, skip_lock: false) } + let(:store) { described_class.new(cocina_object:, skip_lock: false, skip_open_check: false) } let(:ar_cocina_object) { create(:ar_dro) } let(:lock) { "#{ar_cocina_object.external_identifier}=0" } @@ -110,6 +110,28 @@ context: { druid: ar_cocina_object.external_identifier, version: 1 }) end end + + context 'when version is not open but skipping open check' do + let(:open) { false } + let(:store) { described_class.new(cocina_object:, skip_lock: false, skip_open_check: true) } + + let(:ar_cocina_object) { create(:ar_dro) } + let(:lock) { "#{ar_cocina_object.external_identifier}=0" } + + let(:cocina_object) do + Cocina::Models.with_metadata(ar_cocina_object.to_cocina, lock, created: ar_cocina_object.created_at.utc, modified: ar_cocina_object.updated_at.utc) + .new(label: 'new label') + end + + before do + allow(Honeybadger).to receive(:notify) + end + + it 'does not notify honeybadger' do + expect(store.update).to be_a Cocina::Models::DROWithMetadata + expect(Honeybadger).not_to have_received(:notify) + end + end end context 'when object is an AdminPolicy' do diff --git a/spec/services/version_service_spec.rb b/spec/services/version_service_spec.rb index c54088b05..6bc231ad8 100644 --- a/spec/services/version_service_spec.rb +++ b/spec/services/version_service_spec.rb @@ -43,7 +43,7 @@ expect(Dro.find_by(external_identifier: druid).version).to eq 2 expect(ObjectVersion.current_version(druid).version).to eq(2) expect(workflow_state_service).to have_received(:accessioned?) - expect(workflow_state_service).to have_received(:open?).twice + expect(workflow_state_service).to have_received(:open?).once expect(workflow_state_service).to have_received(:accessioning?) expect(workflow_client).to have_received(:create_workflow_by_name).with(druid, 'versioningWF', version: '2') @@ -71,7 +71,7 @@ expect(ObjectVersion.current_version(druid).version).to eq(2) expect(ObjectVersion).not_to have_received(:sync_then_increment_version) expect(workflow_state_service).to have_received(:accessioned?) - expect(workflow_state_service).to have_received(:open?).twice + expect(workflow_state_service).to have_received(:open?).once expect(workflow_state_service).to have_received(:accessioning?) expect(workflow_client).to have_received(:create_workflow_by_name).with(druid, 'versioningWF', version: '2')