refine Hyrax::LeaseService behavior to find only enforced leases

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 }