refine Hyrax::LeaseService behavior to find only enforced leases (#6330)

* refine Hyrax::LeaseService behavior to find only enforced leases

repeats the work in #6241, but with the lease infrastructure.

it would still be really nice to flatten these into one concept, but the data
layer is still an issue (stored data has different names for functionally
identical Embargo and Lease abstractions).

* fin

---------

Co-authored-by: Daniel Pierce <dlpierce@indiana.edu>

app/actors/hyrax/actors/lease_actor.rb

@@ -14,12 +14,9 @@ def initialize(work)
       def destroy
         case work
         when Valkyrie::Resource
-          lease_manager = Hyrax::LeaseManager.new(resource: work)
-          return if lease_manager.lease.lease_expiration_date.blank?
-
-          lease_manager.deactivate!
-          work.lease = Hyrax.persister.save(resource: lease_manager.lease)
-          Hyrax::AccessControlList(work).save
+          Hyrax::LeaseManager.deactivate_lease_for(resource: work) &&
+            Hyrax.persister.save(resource: work.lease) &&
+            Hyrax::AccessControlList(work).save
         else
           work.lease_visibility! # If the lease has lapsed, update the current visibility.
           work.deactivate_lease!

app/models/concerns/hyrax/solr_document_behavior.rb

@@ -125,7 +125,16 @@ def embargo_enforced?
     end
 
     def lease_enforced?
-      lease_expiration_date.present?
+      return false if lease_expiration_date.blank?
+
+      indexed_lease_visibility = first('visibility_during_lease_ssim')
+      # if we didn't index an embargo visibility, assume the release date means
+      # it's enforced
+      return true if indexed_lease_visibility.blank?
+
+      # if the visibility and the visibility during lease are the same, we're
+      # enforcing the lease
+      self['visibility_ssi'] == indexed_lease_visibility
     end
 
     private

app/presenters/hyrax/lease_presenter.rb

@@ -27,5 +27,9 @@ def visibility_after_lease
     def lease_history
       solr_document['lease_history_ssim']
     end
+
+    def enforced?
+      solr_document.lease_enforced?
+    end
   end
 end

app/services/hyrax/lease_manager.rb

@@ -12,12 +12,13 @@ module Hyrax
   #       date is today or later.
   #    - "Applied" means the lease's pre-expiration visibility has been set on
   #      the resource.
-  #    - "Released" means the lease's post-expiration visibility has been set on
-  #      the resource.
   #    - "Enforced" means the object's visibility matches the pre-expiration
   #      visibility of the lease; i.e. the lease has been applied,
   #      but not released.
-  #    - "Deactivate" means that the existing lease will be removed
+  #    - "Released" means the leases's post-release visibility has been set on
+  #      the resource.
+  #    - "Deactivate" means that the existing lease will be removed, even if it
+  #      is active
   #
   class LeaseManager # rubocop:disable Metrics/ClassLength
     ##
@@ -53,11 +54,25 @@ def lease_for(resource:, query_service: Hyrax.query_service)
           .lease
       end
 
+      # @return [Boolean]
+      def deactivate_lease_for(resource:, query_service: Hyrax.query_service)
+        new(resource: resource, query_service: query_service)
+          .deactivate
+      end
+
+      # @return [Boolean]
+      def deactivate_lease_for!(resource:, query_service: Hyrax.query_service)
+        new(resource: resource, query_service: query_service)
+          .deactivate!
+      end
+
+      # @return [Boolean]
       def release_lease_for(resource:, query_service: Hyrax.query_service)
         new(resource: resource, query_service: query_service)
           .release
       end
 
+      # @return [Boolean]
       def release_lease_for!(resource:, query_service: Hyrax.query_service)
         new(resource: resource, query_service: query_service)
           .release!
@@ -93,20 +108,15 @@ def create_or_update_lease_on_members(members, work)
       # rubocop:enable Metrics/AbcSize, Metrics/MethodLength
     end
 
+    def deactivate
+      release(force: true) &&
+        nullify(force: true)
+    end
+
     # Deactivates the lease and logs a message to the lease_history property
     def deactivate!
-      lease_state = lease.active? ? 'active' : 'expired'
-      lease_record = lease_history_message(
-        lease_state,
-        Time.zone.today,
-        lease.lease_expiration_date,
-        lease.visibility_during_lease,
-        lease.visibility_after_lease
-      )
-
-      release(force: true)
+      release!(force: true)
       nullify(force: true)
-      lease.lease_history += [lease_record]
     end
 
     ##
@@ -153,14 +163,15 @@ def lease
     ##
     # Drop the lease by setting its release date and visibility settings to `nil`.
     #
-    # @param force [boolean] force the nullify even when the lease period is current
-    # @return [void]
+    # @param force [Boolean] force the nullify even when the lease period is current
+    # @return [Boolean]
     def nullify(force: false)
       return false if !force && under_lease?
 
       lease.lease_expiration_date = nil
       lease.visibility_during_lease = nil
       lease.visibility_after_lease = nil
+      true
     end
 
     ##
@@ -172,6 +183,18 @@ def nullify(force: false)
     # @return [Boolean]
     def release(force: false)
       return false if !force && under_lease?
+
+      lease_state = lease.active? ? 'active' : 'expired'
+      history_record = lease_history_message(
+        lease_state,
+        Hyrax::TimeService.time_in_utc,
+        lease.lease_expiration_date,
+        lease.visibility_during_lease,
+        lease.visibility_after_lease
+      )
+
+      lease.lease_history += [history_record]
+
       return true if lease.visibility_after_lease.nil?
 
       resource.visibility = lease.visibility_after_lease
@@ -202,7 +225,7 @@ def core_attribute_keys
       [:visibility_after_lease, :visibility_during_lease, :lease_expiration_date]
     end
 
-    # Create the log message used when deactivating a lease
+    # Create the log message used when releasing a lease
     def lease_history_message(state, deactivate_date, expiration_date, visibility_during, visibility_after)
       I18n.t 'hydra.lease.history_message',
               state: state,

app/services/hyrax/lease_service.rb

@@ -1,20 +1,26 @@
 # frozen_string_literal: true
 module Hyrax
+  ##
+  # Methods for Querying Repository to find Objects with leases
   class LeaseService < RestrictionService
     class << self
       # Returns all assets with lease expiration date set to a date in the past
-      def assets_with_expired_leases
+      def assets_with_expired_enforced_leases
         builder = Hyrax::ExpiredLeaseSearchBuilder.new(self)
         presenters(builder)
       end
+      alias assets_with_expired_leases assets_with_expired_enforced_leases
 
-      # Returns all assets with lease expiration date set
-      #   (assumes that when lease visibility is applied to assets
-      #    whose leases have expired, the lease expiration date will be removed from its metadata)
-      def assets_under_lease
+      ##
+      # Returns all assets with leases that are currently enforced,
+      # regardless of whether the leases are active or expired.
+      #
+      # @see Hyrax::LeaseManager
+      def assets_with_enforced_leases
         builder = Hyrax::LeaseSearchBuilder.new(self)
-        presenters(builder)
+        presenters(builder).select(&:enforced?)
       end
+      alias assets_under_lease assets_with_enforced_leases
 
       # Returns all assets that have had embargoes deactivated in the past.
       def assets_with_deactivated_leases

lib/tasks/embargo_lease.rake

@@ -21,6 +21,7 @@ namespace :hyrax do
 
       Hyrax.query_service.find_many_by_ids(ids: ids).each do |resource|
         Hyrax::LeaseManager.release_lease_for(resource: resource) &&
+          Hyrax.persister.save(resource: resource.lease) &&
           Hyrax::AccessControlList(resource).save
       end
     end

spec/factories/hyrax_work.rb

@@ -28,6 +28,25 @@
 
     trait :under_lease do
       association :lease, factory: :hyrax_lease
+
+      after(:create) do |work, _e|
+        Hyrax::LeaseManager.new(resource: work).apply
+        work.permission_manager.acl.save
+      end
+    end
+
+    trait :with_expired_enforced_lease do
+      after(:build) do |work, _evaluator|
+        work.lease = FactoryBot.valkyrie_create(:hyrax_lease, :expired)
+      end
+
+      after(:create) do |work, _evaluator|
+        allow(Hyrax::TimeService).to receive(:time_in_utc).and_return(10.days.ago)
+        Hyrax::LeaseManager.new(resource: work).apply
+        allow(Hyrax::TimeService).to receive(:time_in_utc).and_call_original
+
+        work.permission_manager.acl.save
+      end
     end
 
     transient do

spec/models/concerns/hyrax/solr_document_behavior_spec.rb

@@ -213,5 +213,45 @@
         expect(solr_document.visibility).to eq "embargo"
       end
     end
+
+    context 'when an lease is enforced' do
+      let(:solr_hash) do
+        { "lease_expiration_date_dtsi" => "2023-08-30T00:00:00Z",
+          "visibility_during_lease_ssim" => "open",
+          "visibility_after_lease_ssim" => "restricted",
+          "visibility_ssi" => "open" }
+      end
+
+      it 'is "lease"' do
+        expect(solr_document.visibility).to eq "lease"
+      end
+    end
+
+    context 'when an lease is released' do
+      let(:solr_hash) do
+        { "lease_expiration_date_dtsi" => "2023-08-30T00:00:00Z",
+          "visibility_during_lease_ssim" => "authenticated",
+          "visibility_after_lease_ssim" => "restricted",
+          "visibility_ssi" => "restricted", # expect this to be ignored
+          "read_access_group_ssim" => ["public"] }
+      end
+
+      it 'is based on the read groups and Ability behavior' do
+        expect(solr_document.visibility).to eq "open"
+      end
+    end
+
+    # this is a special case because some of the older Hyrax tests
+    # expected this situation to work. Both ActiveFedora and Valkyrie
+    # actually index the whole embargo structure.
+    context 'when only an lease date is indexed' do
+      let(:solr_hash) do
+        { "lease_expiration_date_dtsi" => "2023-08-30T00:00:00Z" }
+      end
+
+      it 'is "lease"' do
+        expect(solr_document.visibility).to eq "lease"
+      end
+    end
   end
 end

spec/presenters/hyrax/lease_presenter_spec.rb

@@ -1,52 +1,63 @@
 # frozen_string_literal: true
 RSpec.describe Hyrax::LeasePresenter do
+  subject(:presenter) { described_class.new(document) }
   let(:document) { SolrDocument.new(attributes) }
-  let(:presenter) { described_class.new(document) }
   let(:attributes) { {} }
 
-  describe "visibility" do
+  describe "#visibility" do
     subject { presenter.visibility }
 
     it { is_expected.to eq 'restricted' }
   end
 
-  describe "to_s" do
+  describe "#to_s" do
     let(:attributes) { { 'title_tesim' => ['Hey guys!'] } }
 
     subject { presenter.to_s }
 
     it { is_expected.to eq 'Hey guys!' }
   end
 
-  describe "human_readable_type" do
+  describe "#human_readable_type" do
     let(:attributes) { { 'human_readable_type_tesim' => ['File'] } }
 
     subject { presenter.human_readable_type }
 
     it { is_expected.to eq 'File' }
   end
 
-  describe "lease_expiration_date" do
+  describe "#lease_expiration_date" do
     let(:attributes) { { 'lease_expiration_date_dtsi' => '2015-10-15T00:00:00Z' } }
 
     subject { presenter.lease_expiration_date }
 
     it { is_expected.to eq '15 Oct 2015' }
   end
 
-  describe "visibility_after_lease" do
+  describe "#visibility_after_lease" do
     let(:attributes) { { 'visibility_after_lease_ssim' => ['restricted'] } }
 
     subject { presenter.visibility_after_lease }
 
     it { is_expected.to eq 'restricted' }
   end
 
-  describe "lease_history" do
+  describe "#lease_history" do
     let(:attributes) { { 'lease_history_ssim' => ['This is in the past'] } }
 
     subject { presenter.lease_history }
 
     it { is_expected.to eq ['This is in the past'] }
   end
+
+  describe "#enforced?" do
+    let(:attributes) do
+      { "lease_expiration_date_dtsi" => "2023-08-30T00:00:00Z",
+        "visibility_during_lease_ssim" => "open",
+        "visibility_after_lease_ssim" => "restricted",
+        "visibility_ssi" => "open" }
+    end
+
+    it { is_expected.to be_enforced }
+  end
 end

spec/services/hyrax/lease_service_spec.rb

@@ -1,5 +1,5 @@
 # frozen_string_literal: true
-RSpec.describe Hyrax::LeaseService do
+RSpec.describe Hyrax::LeaseService, :clean_repo do
   subject { described_class }
 
   let(:future_date) { 2.days.from_now }