Versioning w/ Valkyrie (#6316)

* Get versioning_service working.

* Get VersionListPresenter passing.

* Add TODO

* Get file sets deleting.

* Gettin' there

* Get some versioning happening.

* Start working on restore.

* Display versions & fix download controller.

* Fix some tests.

* Get more tests passing.

* TESTS PASSING.

* Really passing tests this time.

* Fix specs

* Wings storage passes specs.

* Fix some Dassie tests.

* Fix downloads controller.

* Fix ValkyrieUpload

* Rubocop.

* More rubocop.

* Fix.

* Use released Valkyrie.

* Remove shared specs.

They're in Valkyrie now.

.koppie/config/initializers/1_valkyrie.rb

@@ -47,8 +47,8 @@
 #
 # Valkyrie.config.storage_adapter = :repository_s3
 Valkyrie::StorageAdapter.register(
-  Valkyrie::Storage::Disk.new(base_path: Rails.root.join("storage", "files"),
-                              file_mover: FileUtils.method(:cp)),
+  Valkyrie::Storage::VersionedDisk.new(base_path: Rails.root.join("storage", "files"),
+                                       file_mover: FileUtils.method(:cp)),
   :disk
 )
 Valkyrie.config.storage_adapter  = :disk

app/controllers/concerns/hyrax/valkyrie_downloads_controller_behavior.rb

@@ -17,7 +17,7 @@ def send_file_contents_valkyrie(file_set)
       file_metadata = find_file_metadata(file_set: file_set, use: use, mime_type: mime_type)
       return unless stale?(last_modified: file_metadata.updated_at, template: false)
 
-      file = Hyrax.storage_adapter.find_by(id: file_metadata.file_identifier)
+      file = Valkyrie::StorageAdapter.find_by(id: file_metadata.file_identifier)
       prepare_file_headers_valkyrie(metadata: file_metadata, file: file)
 
       # Warning - using the range header will load the range selection in to memory

app/controllers/hyrax/file_sets_controller.rb

@@ -113,31 +113,38 @@ def delete(file_set:)
     def update_metadata
       case file_set
       when Hyrax::Resource
-        change_set = Hyrax::Forms::ResourceForm.for(file_set)
-
-        change_set.validate(attributes) &&
-          transactions['change_set.update_file_set']
-            .with_step_args(
-              'file_set.save_acl' => { permissions_params: change_set.input_params["permissions"] }
-            )
-            .call(change_set).value_or { false }
+        valkyrie_update_metadata
       else
         file_attributes = form_class.model_attributes(attributes)
         actor.update_metadata(file_attributes)
       end
     end
 
+    def valkyrie_update_metadata
+      change_set = Hyrax::Forms::ResourceForm.for(file_set)
+
+      result =
+        change_set.validate(attributes) &&
+        transactions['change_set.update_file_set']
+        .with_step_args(
+          'file_set.save_acl' => { permissions_params: change_set.input_params["permissions"] }
+        )
+        .call(change_set).value_or { false }
+      @file_set = result if result
+    end
+
     def parent(file_set: curation_concern)
       @parent ||=
         case file_set
-        when Hyrax::Resource
+        when Hyrax::FileSet
           Hyrax.query_service.find_parents(resource: file_set).first
         else
           file_set.parent
         end
     end
 
     def attempt_update
+      return attempt_update_valkyrie if ::FileSet < Hyrax::Resource
       if wants_to_revert?
         actor.revert_content(params[:revision])
       elsif params.key?(:file_set)
@@ -153,9 +160,31 @@ def attempt_update
       end
     end
 
+    def attempt_update_valkyrie
+      return revert_valkyrie if wants_to_revert_valkyrie?
+      if params.key?(:file_set)
+        if params[:file_set].key?(:files)
+          ValkyrieIngestJob.perform_later(uploaded_file_from_path)
+        else
+          update_metadata
+        end
+      elsif params.key?(:files_files) # version file already uploaded with ref id in :files_files array
+        uploaded_files = Array(Hyrax::UploadedFile.find(params[:files_files]))
+        uploaded_files.first.file_set_uri = file_set.id.to_s
+        uploaded_files.first.save
+        ValkyrieIngestJob.perform_later(uploaded_files.first)
+        update_metadata
+      end
+    end
+
+    def revert_valkyrie
+      Hyrax::VersioningService.create(file_metadata, current_user, Hyrax.storage_adapter.find_by(id: params[:revision]))
+      true
+    end
+
     def uploaded_file_from_path
       uploaded_file = CarrierWave::SanitizedFile.new(params[:file_set][:files].first)
-      Hyrax::UploadedFile.create(user_id: current_user.id, file: uploaded_file)
+      Hyrax::UploadedFile.create(user_id: current_user.id, file: uploaded_file, file_set_uri: @file_set.id.to_s)
     end
 
     def after_update_response
@@ -255,6 +284,14 @@ def wants_to_revert?
       params.key?(:revision) && params[:revision] != curation_concern.latest_content_version.label
     end
 
+    def wants_to_revert_valkyrie?
+      params.key?(:revision) && params[:revision] != Hyrax::VersioningService.new(resource: file_metadata).latest_version.version_id.to_s
+    end
+
+    def file_metadata
+      @file_metadata ||= Hyrax.query_service.custom_queries.find_file_metadata_by(id: curation_concern.original_file_id)
+    end
+
     # Override this method to add additional response formats to your local app
     def additional_response_formats(_); end
 

app/models/concerns/hyrax/ability.rb

@@ -370,6 +370,7 @@ def admin_permissions
       can :manage, curation_concerns_models
       can :manage, Sipity::WorkflowResponsibility
       can :manage, :collection_types
+      can :manage, ::FileSet
     end
 
     ##
@@ -418,7 +419,7 @@ def user_is_depositor?(document_id)
     end
 
     def curation_concerns_models
-      [::FileSet, Hyrax.config.collection_class] + Hyrax.config.curation_concerns
+      [::FileSet, ::Hyrax::FileSet, Hyrax.config.collection_class] + Hyrax.config.curation_concerns
     end
 
     def can_review_submissions?

app/models/hyrax/file_metadata.rb

@@ -59,7 +59,7 @@ def uri_for(use:)
       module_function :uri_for
     end
 
-    attribute :file_identifier, Valkyrie::Types::ID # id of the file stored by the storage adapter
+    attribute :file_identifier, ::Valkyrie::Types::ID # id of the file stored by the storage adapter
     attribute :alternate_ids, Valkyrie::Types::Set.of(Valkyrie::Types::ID) # id of the file, populated for queryability
     attribute :file_set_id, ::Valkyrie::Types::ID # id of parent file set resource
 

app/presenters/hyrax/version_list_presenter.rb

@@ -38,9 +38,15 @@ def self.for(file_set:)
 
     def wrapped_list
       @wrapped_list ||=
-        @versioning_service.versions.map { |v| Hyrax::VersionPresenter.new(v) } # wrap each item in a presenter
-                           .sort { |a, b| b.version.created <=> a.version.created } # sort list of versions based on creation date
-                           .tap { |l| l.first.try(:current!) } # set the first version to the current version
+        begin
+          presenters = @versioning_service.versions.map { |v| Hyrax::VersionPresenter.new(v) } # wrap each item in a presenter
+          if presenters.first&.version&.respond_to?(:created)
+            presenters.sort! { |a, b| b.version.created <=> a.version.created } if presenters.first&.version.respond_to?(:created) # sort list of versions based on creation date
+          else
+            presenters.reverse!
+          end
+          presenters.tap { |l| l.first.try(:current!) } # set the first version to the current version
+        end
     end
   end
 end

app/presenters/hyrax/version_presenter.rb

@@ -15,14 +15,29 @@ def current!
       @current = true
     end
 
+    def label
+      version.try(:label) || version.version_id.to_s
+    end
+
+    def uri
+      version.try(:uri) || version.version_id.to_s
+    end
+
     def created
-      @created ||= version.created.in_time_zone.to_formatted_s(:long_ordinal)
+      @created ||= created_time&.in_time_zone&.to_formatted_s(:long_ordinal) || "Unknown"
     end
 
-    def committer
+    def created_time
+      version.try(:created) || version_committer.try(:created_at)
+    end
+
+    def version_committer
       Hyrax::VersionCommitter
-        .find_by(version_id: @version.uri)
-        &.committer_login
+        .find_by(version_id: @version.try(:uri) || @version.try(:version_id))
+    end
+
+    def committer
+      version_committer&.committer_login
     end
   end
 end

app/services/hyrax/valkyrie_upload.rb

@@ -34,13 +34,15 @@ def initialize(storage_adapter: Hyrax.storage_adapter)
     @storage_adapter = storage_adapter
   end
 
-  def upload(filename:, file_set:, io:, use: Hyrax::FileMetadata::Use::ORIGINAL_FILE, user: nil, mime_type: nil)
+  def upload(filename:, file_set:, io:, use: Hyrax::FileMetadata::Use::ORIGINAL_FILE, user: nil, mime_type: nil) # rubocop:disable Metrics/AbcSize
+    return version_upload(file_set: file_set, io: io, user: user) if use == Hyrax::FileMetadata::Use::ORIGINAL_FILE && file_set.original_file_id && storage_adapter.supports?(:versions)
     streamfile = storage_adapter.upload(file: io, original_filename: filename, resource: file_set)
     file_metadata = Hyrax::FileMetadata(streamfile)
     file_metadata.file_set_id = file_set.id
     file_metadata.pcdm_use = [use]
     file_metadata.recorded_size = [io.size]
     file_metadata.mime_type = mime_type if mime_type
+    file_metadata.original_filename = File.basename(filename).to_s || File.basename(io)
 
     if use == Hyrax::FileMetadata::Use::ORIGINAL_FILE
       # Set file set label.
@@ -64,6 +66,13 @@ def upload(filename:, file_set:, io:, use: Hyrax::FileMetadata::Use::ORIGINAL_FI
     saved_metadata
   end
 
+  def version_upload(file_set:, io:, user:)
+    file_metadata = Hyrax.query_service.custom_queries.find_file_metadata_by(id: file_set.original_file_id)
+    Hyrax::VersioningService.create(file_metadata, user, io)
+    Hyrax.publisher.publish("file.uploaded", metadata: file_metadata)
+    ContentNewVersionEventJob.perform_later(file_set, user)
+  end
+
   # @param [Hyrax::FileSet] file_set the file set to add to
   # @param [Hyrax::FileMetadata] file_metadata the metadata object representing
   #   the file to add

app/services/hyrax/versioning_service.rb

@@ -35,7 +35,9 @@ def versions
       if !supports_multiple_versions?
         []
       elsif resource.is_a?(Hyrax::FileMetadata)
-        storage_adapter.find_versions(id: resource.file_identifier).to_a
+        # Reverse - Valkyrie puts these most recent first, we assume most recent
+        # last.
+        storage_adapter.find_versions(id: resource.file_identifier).to_a.reverse
       else
         return resource.versions if resource.versions.is_a?(Array)
         resource.versions.all.to_a
@@ -67,7 +69,7 @@ def supports_multiple_versions?
     # If the resource is nil, this method returns an empty string.
     def versioned_file_id
       latest = latest_version
-      if latest
+      if latest && !resource.is_a?(Hyrax::FileMetadata)
         if latest.respond_to?(:id)
           latest.id
         else
@@ -76,7 +78,7 @@ def versioned_file_id
       elsif resource.nil?
         ""
       elsif resource.is_a?(Hyrax::FileMetadata)
-        resource.file_identifier
+        latest_version&.version_id || resource.file_identifier
       else
         resource.id
       end
@@ -86,9 +88,9 @@ class << self
       # Make a version and record the version committer
       # @param [ActiveFedora::File | Hyrax::FileMetadata] content
       # @param [User, String] user
-      def create(content, user = nil)
+      def create(content, user = nil, file = nil)
         use_valkyrie = content.is_a? Hyrax::FileMetadata
-        perform_create(content, user, use_valkyrie)
+        perform_create(content, user, file, use_valkyrie)
       end
 
       # @param [ActiveFedora::File | Hyrax::FileMetadata] file
@@ -108,14 +110,14 @@ def record_committer(content, user_key)
         user_key = user_key.user_key if user_key.respond_to?(:user_key)
         version = latest_version_of(content)
         return if version.nil?
-        version_id = content.is_a?(Hyrax::FileMetadata) ? version.id.to_s : version.uri
+        version_id = content.is_a?(Hyrax::FileMetadata) ? version.version_id.to_s : version.uri
         Hyrax::VersionCommitter.create(version_id: version_id, committer_login: user_key)
       end
 
       private
 
-      def perform_create(content, user, use_valkyrie)
-        use_valkyrie ? perform_create_through_valkyrie(content, user) : perform_create_through_active_fedora(content, user)
+      def perform_create(content, user, file, use_valkyrie)
+        use_valkyrie ? perform_create_through_valkyrie(content, file, user) : perform_create_through_active_fedora(content, user)
       rescue NotImplementedError
         Hyrax.logger.warn "Declining to create a Version for #{content}; #{self} doesn't support versioning with use_valkyrie: #{use_valkyrie}"
       end
@@ -125,8 +127,14 @@ def perform_create_through_active_fedora(content, user)
         record_committer(content, user) if user
       end
 
-      def perform_create_through_valkyrie(content, user) # no-op
-        raise NotImplementedError
+      def perform_create_through_valkyrie(content, file, user)
+        raise NotImplementedError unless Hyrax.storage_adapter.supports?(:versions)
+        Hyrax.storage_adapter.upload_version(id: content.file_identifier, file: file)
+        record_committer(content, user) if user
+      end
+
+      def storage_adapter
+        Hyrax.storage_adapter
       end
     end
   end

hyrax.gemspec

@@ -82,7 +82,7 @@ SUMMARY
   spec.add_dependency 'retriable', '>= 2.9', '< 4.0'
   spec.add_dependency 'signet'
   spec.add_dependency 'tinymce-rails', '~> 5.10'
-  spec.add_dependency 'valkyrie', '~> 3.0.1'
+  spec.add_dependency 'valkyrie', '~> 3.1.0'
   spec.add_dependency 'view_component', '~> 2.74.1' # Pin until blacklight is updated with workaround for https://github.com/ViewComponent/view_component/issues/1565
   spec.add_dependency 'sprockets', '~> 3.7'
   spec.add_dependency 'sass-rails', '~> 6.0'

lib/hyrax/transactions/container.rb

@@ -53,6 +53,9 @@ class Container # rubocop:disable Metrics/ClassLength
       require 'hyrax/transactions/steps/set_user_as_depositor'
       require 'hyrax/transactions/steps/update_work_members'
       require 'hyrax/transactions/steps/validate'
+      require 'hyrax/transactions/steps/delete_all_file_metadata'
+      require 'hyrax/transactions/steps/file_metadata_delete'
+      require 'hyrax/transactions/file_metadata_destroy'
 
       extend Dry::Container::Mixin
 
@@ -128,11 +131,25 @@ class Container # rubocop:disable Metrics/ClassLength
         end
       end
 
+      namespace "file_metadata" do |ops| # Hyrax::FileMetadata
+        ops.register 'destroy' do
+          FileMetadataDestroy.new
+        end
+
+        ops.register 'delete' do
+          Steps::FileMetadataDelete.new
+        end
+      end
+
       namespace 'file_set' do |ops| # Hyrax::FileSet resource
         ops.register 'delete' do
           Steps::DeleteResource.new
         end
 
+        ops.register 'delete_all_file_metadata' do
+          Steps::DeleteAllFileMetadata.new(property: :file_ids)
+        end
+
         ops.register 'destroy' do
           FileSetDestroy.new
         end

lib/hyrax/transactions/file_metadata_destroy.rb

@@ -0,0 +1,20 @@
+# frozen_string_literal: true
+require 'hyrax/transactions/transaction'
+
+module Hyrax
+  module Transactions
+    ##
+    # destroys a FileSet resource.
+    #
+    # @since 3.1.0
+    class FileMetadataDestroy < Transaction
+      DEFAULT_STEPS = ['file_metadata.delete'].freeze
+
+      ##
+      # @see Hyrax::Transactions::Transaction
+      def initialize(container: Container, steps: DEFAULT_STEPS)
+        super
+      end
+    end
+  end
+end

lib/hyrax/transactions/file_set_destroy.rb

@@ -9,7 +9,8 @@ module Transactions
     # @since 3.1.0
     class FileSetDestroy < Transaction
       DEFAULT_STEPS = ['file_set.remove_from_work',
-                       'file_set.delete'].freeze
+                       'file_set.delete',
+                       'file_set.delete_all_file_metadata'].freeze
 
       ##
       # @see Hyrax::Transactions::Transaction