Delete access control object when destroying a resource

.rubocop_fixme.yml

@@ -114,6 +114,7 @@ RSpec/AnyInstance:
     - 'spec/controllers/hyrax/batch_edits_controller_spec.rb'
     - 'spec/controllers/hyrax/stats_controller_spec.rb'
     - 'spec/controllers/hyrax/users_controller_spec.rb'
+    - 'spec/hyrax/transactions/steps/delete_access_control_spec.rb'
     - 'spec/jobs/content_restored_version_event_job_spec.rb'
     - 'spec/jobs/file_set_attached_event_job_spec.rb'
     - 'spec/jobs/hyrax/grant_edit_to_members_job_spec.rb'

app/services/hyrax/access_control_list.rb

@@ -174,6 +174,19 @@ def save
       true
     end
 
+    ##
+    # @api public
+    #
+    # Deletes the ACL for the resource
+    #
+    # @return [Boolean]
+    def destroy
+      persister.delete(resource: change_set.resource) if change_set.resource.persisted?
+      @change_set = nil
+
+      true
+    end
+
     private
 
     ##

lib/hyrax/transactions/admin_set_destroy.rb

@@ -9,7 +9,8 @@ module Transactions
     # @since 3.4.0
     class AdminSetDestroy < Transaction
       DEFAULT_STEPS = ['admin_set_resource.check_empty',
-                       'admin_set_resource.delete'].freeze
+                       'admin_set_resource.delete',
+                       'admin_set_resource.delete_acl'].freeze
 
       ##
       # @see Hyrax::Transactions::Transaction

lib/hyrax/transactions/container.rb

@@ -38,6 +38,7 @@ class Container # rubocop:disable Metrics/ClassLength
       require 'hyrax/transactions/steps/apply_permission_template'
       require 'hyrax/transactions/steps/apply_visibility'
       require 'hyrax/transactions/steps/check_for_empty_admin_set'
+      require 'hyrax/transactions/steps/delete_access_control'
       require 'hyrax/transactions/steps/delete_resource'
       require 'hyrax/transactions/steps/destroy_work'
       require 'hyrax/transactions/steps/ensure_admin_set'
@@ -159,6 +160,10 @@ class Container # rubocop:disable Metrics/ClassLength
           Steps::ApplyCollectionTypePermissions.new
         end
 
+        ops.register 'delete_acl' do
+          Steps::DeleteAccessControl.new
+        end
+
         ops.register 'save_acl' do
           Steps::SaveAccessControl.new
         end
@@ -169,6 +174,10 @@ class Container # rubocop:disable Metrics/ClassLength
           Steps::ApplyCollectionTypePermissions.new
         end
 
+        ops.register 'delete_acl' do
+          Steps::DeleteAccessControl.new
+        end
+
         ops.register 'save_acl' do
           Steps::SaveAccessControl.new
         end
@@ -191,6 +200,10 @@ class Container # rubocop:disable Metrics/ClassLength
           WorkDestroy.new
         end
 
+        ops.register 'delete_acl' do
+          Steps::DeleteAccessControl.new
+        end
+
         ops.register 'save_acl' do
           Steps::SaveAccessControl.new
         end

lib/hyrax/transactions/steps/delete_access_control.rb

@@ -0,0 +1,32 @@
+# frozen_string_literal: true
+require 'dry/monads'
+
+module Hyrax
+  module Transactions
+    module Steps
+      ##
+      # Deletes the Hyrax::AccessControlList for any resource with a `#permission_manager`.
+      # If `#permission_manager` is undefined, succeeds.
+      #
+      # @see https://dry-rb.org/gems/dry-monads/1.0/result/
+      class DeleteAccessControl
+        include Dry::Monads[:result]
+
+        ##
+        # @param [Valkyrie::Resource] obj
+        #
+        # @return [Dry::Monads::Result]
+        def call(obj)
+          return Success(obj) unless obj.respond_to?(:permission_manager)
+
+          acl = obj.permission_manager&.acl
+          return Success(obj) if acl.nil?
+
+          acl.destroy || (return Failure[:failed_to_delete_acl, acl])
+
+          Success(obj)
+        end
+      end
+    end
+  end
+end

lib/hyrax/transactions/work_destroy.rb

@@ -8,7 +8,8 @@ module Transactions
     #
     # @since 3.0.0
     class WorkDestroy < Transaction
-      DEFAULT_STEPS = ['work_resource.delete'].freeze
+      DEFAULT_STEPS = ['work_resource.delete',
+                       'work_resource.delete_acl'].freeze
 
       ##
       # @see Hyrax::Transactions::Transaction

spec/hyrax/transactions/steps/delete_access_control_spec.rb

@@ -0,0 +1,63 @@
+# frozen_string_literal: true
+require 'spec_helper'
+require 'dry/monads'
+require 'hyrax/transactions'
+
+RSpec.describe Hyrax::Transactions::Steps::DeleteAccessControl, valkyrie_adapter: :test_adapter do
+  subject(:step) { described_class.new }
+  let(:work)     { FactoryBot.valkyrie_create(:hyrax_work) }
+
+  context 'when acl has not been persisted' do
+    it 'gives Success(obj) in basic case' do
+      expect(step.call(work).value!).to eql(work)
+    end
+  end
+
+  context 'when existing permissions exist' do
+    let(:user) { FactoryBot.create(:user) }
+
+    before do
+      work.permission_manager.read_users = [user.user_key]
+      work.permission_manager.acl.save
+    end
+
+    it 'deletes the access control resource' do
+      expect { step.call(work) }
+        .to change { Hyrax::AccessControl.for(resource: work).persisted? }
+        .from(true)
+        .to(false)
+    end
+
+    context 'when it fails to destroy' do
+      before { allow_any_instance_of(Hyrax::AccessControlList).to receive(:destroy).and_return(false) }
+
+      it 'returns a Failure' do
+        result = step.call(work)
+
+        expect(result).to be_failure
+        expect(result.failure).to contain_exactly(Symbol, Hyrax::AccessControlList)
+      end
+    end
+  end
+
+  context 'when the resource has no permission_manager' do
+    before do
+      module Hyrax
+        module Test
+          module DeleteAccessControlStep
+            class SimpleResource < Valkyrie::Resource
+            end
+          end
+        end
+      end
+    end
+
+    after { Hyrax::Test.send(:remove_const, :DeleteAccessControlStep) }
+
+    let(:resource) { Hyrax.persister.save(resource: Hyrax::Test::DeleteAccessControlStep::SimpleResource.new) }
+
+    it 'succeeds happily' do
+      expect(step.call(work).value!).to eql(work)
+    end
+  end
+end

spec/services/hyrax/access_control_list_spec.rb

@@ -164,4 +164,25 @@
       end
     end
   end
+
+  describe "#destroy" do
+    let(:listener) { Hyrax::Specs::SpyListener.new }
+
+    before do
+      acl << permission
+      acl.save
+
+      # Subscribe to events after acl has been persisted
+      Hyrax.publisher.subscribe(listener)
+    end
+
+    after { Hyrax.publisher.unsubscribe(listener) }
+
+    it 'deletes the acl resource' do
+      expect { acl.destroy }
+        .to change { Hyrax::AccessControl.for(resource: resource, query_service: acl.query_service).persisted? }
+        .from(true)
+        .to(false)
+    end
+  end
 end