From cbbe5fc4b3acdae4ea716ab85f98e7e9a59e84fc Mon Sep 17 00:00:00 2001 From: Daniel Pierce Date: Wed, 4 Dec 2024 15:17:48 -0500 Subject: [PATCH 1/5] Run tests on Fedora 6.5.1 --- .circleci/config.yml | 2 +- .lando.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 47634cdb..a0aab832 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -28,7 +28,7 @@ jobs: environment: CATALINA_OPTS: "-Djava.awt.headless=true -Dfile.encoding=UTF-8 -server -Xms512m -Xmx1024m -XX:NewSize=256m -XX:MaxNewSize=256m -XX:PermSize=256m -XX:MaxPermSize=256m -XX:+DisableExplicitGC" JAVA_OPTIONS: "-Djetty.http.port=8998 -Dfcrepo.dynamic.jms.port=61618 -Dfcrepo.dynamic.stomp.port=61614" - - image: fcrepo/fcrepo:6.4.0 + - image: fcrepo/fcrepo:6.5.1-RC3-tomcat9 environment: CATALINA_OPTS: "-Djava.awt.headless=true -Dfile.encoding=UTF-8 -server -Xms512m -Xmx1024m -XX:NewSize=256m -XX:MaxNewSize=256m -XX:PermSize=256m -XX:MaxPermSize=256m -XX:+DisableExplicitGC -Dorg.apache.tomcat.util.buf.UDecoder.ALLOW_ENCODED_SLASH=true" JAVA_OPTS: "-Djetty.http.port=8978 -Dfcrepo.dynamic.jms.port=61619 -Dfcrepo.dynamic.stomp.port=61615 -Dorg.apache.tomcat.util.buf.UDecoder.ALLOW_ENCODED_SLASH=true" diff --git a/.lando.yml b/.lando.yml index 0ab25dab..ba9fff82 100644 --- a/.lando.yml +++ b/.lando.yml @@ -51,7 +51,7 @@ services: volumes: fedora6: services: - image: fcrepo/fcrepo:6.4.0 + image: fcrepo/fcrepo:6.5.1-RC3-tomcat9 command: - "catalina.sh" - "run" From 5c5fc719d82dfe4c2ef9d031304ba11063af896a Mon Sep 17 00:00:00 2001 From: Daniel Pierce Date: Thu, 5 Dec 2024 11:55:20 -0500 Subject: [PATCH 2/5] Add `supports? :list_deleted_versions` for Fedora 6.5 Changed upstream: https://github.com/fcrepo/fcrepo/pull/2049 --- lib/valkyrie/specs/shared_specs/storage_adapter.rb | 8 +++++++- lib/valkyrie/storage/fedora.rb | 6 +++++- spec/valkyrie/storage/fedora_spec.rb | 2 +- 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/lib/valkyrie/specs/shared_specs/storage_adapter.rb b/lib/valkyrie/specs/shared_specs/storage_adapter.rb index f7717666..94b48f50 100644 --- a/lib/valkyrie/specs/shared_specs/storage_adapter.rb +++ b/lib/valkyrie/specs/shared_specs/storage_adapter.rb @@ -154,7 +154,13 @@ def open_files # instead make deleting delete everything. storage_adapter.delete(id: new_version.id) expect { storage_adapter.find_by(id: new_version.id) }.to raise_error Valkyrie::StorageAdapter::FileNotFound - expect(storage_adapter.find_versions(id: new_version.id).length).to eq 0 + + if storage_adapter.supports?(:list_deleted_versions) + expect(storage_adapter.find_versions(id: new_version.id).length).to eq current_length + else + expect(storage_adapter.find_versions(id: new_version.id).length).to eq 0 + end + ensure f&.close end diff --git a/lib/valkyrie/storage/fedora.rb b/lib/valkyrie/storage/fedora.rb index 2bcaf9f6..ec5a5269 100644 --- a/lib/valkyrie/storage/fedora.rb +++ b/lib/valkyrie/storage/fedora.rb @@ -36,6 +36,8 @@ def supports?(feature) return true if feature == :versions # Fedora 6 auto versions and you can't delete versions. return true if feature == :version_deletion && fedora_version < 6 + # Fedora 6.5+ lists versions for deleted objects + return true if feature == :list_deleted_versions && fedora_version >= 6.5 false end @@ -88,7 +90,9 @@ def find_versions(id:) version_list.map do |version| id = valkyrie_identifier(uri: version["@id"]) perform_find(id: id, version_id: id) - end + rescue Valkyrie::StorageAdapter::FileNotFound + nil + end.compact end # Delete the file in Fedora associated with the given identifier. diff --git a/spec/valkyrie/storage/fedora_spec.rb b/spec/valkyrie/storage/fedora_spec.rb index 763bfa7d..e3624dc5 100644 --- a/spec/valkyrie/storage/fedora_spec.rb +++ b/spec/valkyrie/storage/fedora_spec.rb @@ -166,7 +166,7 @@ class Valkyrie::Specs::FedoraCustomResource < Valkyrie::Resource wipe_fedora!(base_path: "test", fedora_version: 6) end - let(:storage_adapter) { described_class.new(**fedora_adapter_config(base_path: 'test', fedora_version: 6)) } + let(:storage_adapter) { described_class.new(**fedora_adapter_config(base_path: 'test', fedora_version: 6.5)) } let(:file) { fixture_file_upload('files/example.tif', 'image/tiff') } it_behaves_like "a Valkyrie::StorageAdapter" From fabae9570b556f08c5bb76e280891fb061219d12 Mon Sep 17 00:00:00 2001 From: Daniel Pierce Date: Thu, 5 Dec 2024 11:56:21 -0500 Subject: [PATCH 3/5] Report failure details --- lib/valkyrie/storage/fedora.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/valkyrie/storage/fedora.rb b/lib/valkyrie/storage/fedora.rb index ec5a5269..bb1c7009 100644 --- a/lib/valkyrie/storage/fedora.rb +++ b/lib/valkyrie/storage/fedora.rb @@ -215,7 +215,7 @@ def valkyrie_identifier(uri:) # @return [IOProxy] def response(id:) response = connection.http.get(fedora_identifier(id: id)) - raise Valkyrie::StorageAdapter::FileNotFound unless response.success? + raise Valkyrie::StorageAdapter::FileNotFound, "HTTP #{response.status} #{response.body}" unless response.success? IOProxy.new(response.body) end From 540415eb3cff17ffd65039d8d3c8fb42d8769a4c Mon Sep 17 00:00:00 2001 From: Daniel Pierce Date: Thu, 5 Dec 2024 11:57:36 -0500 Subject: [PATCH 4/5] Refactor versioning shared spec --- lib/valkyrie/specs/shared_specs/storage_adapter.rb | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/lib/valkyrie/specs/shared_specs/storage_adapter.rb b/lib/valkyrie/specs/shared_specs/storage_adapter.rb index 94b48f50..13d43a58 100644 --- a/lib/valkyrie/specs/shared_specs/storage_adapter.rb +++ b/lib/valkyrie/specs/shared_specs/storage_adapter.rb @@ -74,8 +74,9 @@ def open_files resource = Valkyrie::Specs::CustomResource.new(id: "test#{SecureRandom.uuid}") sha1 = Digest::SHA1.file(file).to_s size = file.size - expect(uploaded_file = storage_adapter.upload(file: file, original_filename: 'foo.jpg', resource: resource, fake_upload_argument: true)).to be_kind_of Valkyrie::StorageAdapter::File + uploaded_file = storage_adapter.upload(file: file, original_filename: 'foo.jpg', resource: resource, fake_upload_argument: true) + expect(uploaded_file).to be_kind_of Valkyrie::StorageAdapter::File expect(uploaded_file).to respond_to(:checksum).with_keywords(:digests) expect(uploaded_file).to respond_to(:valid?).with_keywords(:size, :digests) expect(uploaded_file.checksum(digests: [Digest::SHA1.new])).to eq([sha1]) @@ -133,21 +134,25 @@ def open_files # Deleting a version should leave the current versions if storage_adapter.supports?(:version_deletion) storage_adapter.delete(id: uploaded_file.version_id) - expect(storage_adapter.find_versions(id: uploaded_file.id).length).to eq 1 + remnants = storage_adapter.find_versions(id: uploaded_file.id) + expect(remnants.length).to eq 1 + expect(remnants.first.version_id).to eq new_version.version_id expect { storage_adapter.find_by(id: uploaded_file.version_id) }.to raise_error Valkyrie::StorageAdapter::FileNotFound end current_length = storage_adapter.find_versions(id: new_version.id).length # Restoring a previous version is just pumping its file into upload_version newest_version = storage_adapter.upload_version(file: new_version, id: new_version.id) + current_length += 1 expect(newest_version.version_id).not_to eq new_version.id expect(storage_adapter.find_by(id: newest_version.id).version_id).to eq newest_version.version_id # I can restore a version twice newest_version = storage_adapter.upload_version(file: new_version, id: new_version.id) + current_length += 1 expect(newest_version.version_id).not_to eq new_version.id expect(storage_adapter.find_by(id: newest_version.id).version_id).to eq newest_version.version_id - expect(storage_adapter.find_versions(id: newest_version.id).length).to eq current_length + 2 + expect(storage_adapter.find_versions(id: newest_version.id).length).to eq current_length # NOTE: We originally wanted deleting the current record to push it into the # versions history, but FCRepo 4/5/6 doesn't work that way, so we changed to From d69f4a1b9df2ebaf2626f217dbbb57893882a1d4 Mon Sep 17 00:00:00 2001 From: Daniel Pierce Date: Thu, 5 Dec 2024 11:59:38 -0500 Subject: [PATCH 5/5] Wait a sec before deleting --- lib/valkyrie/specs/shared_specs/storage_adapter.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/valkyrie/specs/shared_specs/storage_adapter.rb b/lib/valkyrie/specs/shared_specs/storage_adapter.rb index 13d43a58..4a2e9867 100644 --- a/lib/valkyrie/specs/shared_specs/storage_adapter.rb +++ b/lib/valkyrie/specs/shared_specs/storage_adapter.rb @@ -154,6 +154,10 @@ def open_files expect(storage_adapter.find_by(id: newest_version.id).version_id).to eq newest_version.version_id expect(storage_adapter.find_versions(id: newest_version.id).length).to eq current_length + # Fedora 6.5 may not create versions when the timestamp is the same? + # See: https://fedora-repository.atlassian.net/browse/FCREPO-3958 + sleep 1 if storage_adapter.supports?(:list_deleted_versions) + # NOTE: We originally wanted deleting the current record to push it into the # versions history, but FCRepo 4/5/6 doesn't work that way, so we changed to # instead make deleting delete everything.