🤖 Re-arrange CleanupAccountJob specs

The `CleanupAccountJob` was stubbing very nosily; needing to know too
much about implementation details of the end-points.  Instead this
preserves the over-view spec (e.g. what all the cleanup spec actually
cleans up) while moving that nosy logic to the constituent endpoint.

Most of these specs are testing that the method chains work; which is
perhaps adequate as the other option is far more expensive tests (e.g.
make a new Fedora node only to then immediately destroy it)

I'm also leveraging the new `Redis::Namespace#clear` method.

Related to:

- resque/redis-namespace#202

Gemfile

@@ -71,6 +71,7 @@ gem 'rack-test', '0.7.0', group: %i[test] # rack-test >= 0.71 does not work with
 gem 'rails-controller-testing', group: %i[test]
 gem 'rdf', '~> 3.2'
 gem 'redlock', '>= 0.1.2', '< 2.0' # lock redlock per https://github.com/samvera/hyrax/pull/5961
+gem 'redis-namespace', '~> 1.10' # Hyrax v5 relies on 1.5; but we'd like to have the #clear method so we need 1.10 or greater.
 gem 'riiif', '~> 2.0'
 gem 'rolify'
 gem 'rsolr', '~> 2.0'

Gemfile.lock

@@ -1437,6 +1437,7 @@ DEPENDENCIES
   rails (~> 6.0)
   rails-controller-testing
   rdf (~> 3.2)
+  redis-namespace (~> 1.10)
   redlock (>= 0.1.2, < 2.0)
   riiif (~> 2.0)
   rolify

app/models/fcrepo_endpoint.rb

@@ -27,7 +27,7 @@ def ping
   def remove!
     switch!
     # Preceding slash must be removed from base_path when calling delete()
-    path = base_path.sub!(%r{^/}, '')
+    path = base_path.sub(%r{^/}, '')
     ActiveFedora.fedora.connection.delete(path)
     destroy
   end

app/models/redis_endpoint.rb

@@ -21,14 +21,8 @@ def ping
   # Remove all the keys in Redis in this namespace, then destroy the record
   def remove!
     switch!
-    # Redis::Namespace currently doesn't support flushall or flushdb.
-    # See https://github.com/resque/redis-namespace/issues/56
-    # So, instead we select all keys in current namespace and delete
-    keys = redis_instance.keys '*'
-    return if keys.empty?
-    # Delete in slices to avoid "stack level too deep" errors for large numbers of keys
-    # See https://github.com/redis/redis-rb/issues/122
-    keys.each_slice(1000) { |key_slice| redis_instance.del(*key_slice) }
+    # redis-namespace v1.10.0 introduced clear https://github.com/resque/redis-namespace/pull/202
+    redis_instance.clear
     destroy
   end
 

app/models/solr_endpoint.rb

@@ -29,6 +29,9 @@ def switch!
 
   # Remove the solr collection then destroy this record
   def remove!
+    # NOTE: Other end points first call switch!; is that an oversight?  Perhaps not as we're relying
+    # on a scheduled job to do the destructive work.
+
     # Spin off as a job, so that it can fail and be retried separately from the other logic.
     if account.search_only?
       RemoveSolrCollectionJob.perform_later(collection, connection_options, 'cross_search_tenant')

spec/jobs/cleanup_account_job_spec.rb

@@ -10,39 +10,29 @@
   end
 
   before do
-    allow(RemoveSolrCollectionJob).to receive(:perform_later)
-    allow(account.fcrepo_endpoint).to receive(:switch!)
-    allow(ActiveFedora.fedora.connection).to receive(:delete)
+    allow(account.solr_endpoint).to receive(:remove!)
+    allow(account.fcrepo_endpoint).to receive(:remove!)
+    allow(account.redis_endpoint).to receive(:remove!)
     allow(Apartment::Tenant).to receive(:drop).with(account.tenant)
   end
 
   it 'destroys the solr collection' do
-    expect(RemoveSolrCollectionJob).to receive(:perform_later).with('x', hash_including('url'))
-    expect(account.solr_endpoint).to receive(:destroy)
+    expect(account.solr_endpoint).to receive(:remove!)
     described_class.perform_now(account)
   end
 
   it 'destroys the fcrepo collection' do
-    expect(ActiveFedora.fedora.connection).to receive(:delete).with('x')
-    expect(account.fcrepo_endpoint).to receive(:destroy)
+    expect(account.fcrepo_endpoint).to receive(:remove!)
     described_class.perform_now(account)
   end
 
   it 'deletes all entries in the redis namespace' do
-    allow(Redis.current).to receive(:keys).and_return(["x:events:x1", "x:events:x2"])
-    allow(Hyrax::RedisEventStore).to receive(:instance).and_return(
-      Redis::Namespace.new(account.redis_endpoint.namespace, redis: Redis.current)
-    )
-    expect(Hyrax::RedisEventStore.instance.namespace).to eq('x')
-    expect(Hyrax::RedisEventStore.instance.keys).to eq(["events:x1", "events:x2"])
-    expect(Hyrax::RedisEventStore.instance).to receive(:del).with('events:x1', 'events:x2')
-    expect(account.redis_endpoint).to receive(:destroy)
+    expect(account.redis_endpoint).to receive(:remove!)
     described_class.perform_now(account)
   end
 
   it 'destroys the tenant database' do
     expect(Apartment::Tenant).to receive(:drop).with(account.tenant)
-
     described_class.perform_now(account)
   end
 

spec/models/fcrepo_endpoint_spec.rb

@@ -2,10 +2,9 @@
 
 RSpec.describe FcrepoEndpoint do
   let(:base_path) { 'foobar' }
+  subject { described_class.new base_path: }
 
   describe '.options' do
-    subject { described_class.new base_path: }
-
     it 'uses the configured application settings' do
       expect(subject.options[:base_path]).to eq base_path
     end
@@ -28,4 +27,22 @@
       expect(subject.ping).to be_falsey
     end
   end
+
+  describe '#remove!' do
+    it 'removes the base node in fedora and deletes this endpoint' do
+      subject.save!
+      # All of this stubbing doesn't tell us much; except that the method chain is valid.  Which is perhaps better than the two options:
+      #
+      # 1. Creating the Fedora node then tearing it down.
+      # 2. Not testing this at all.
+      #
+      # What I found is that I could not stub the last receiver in the chain; as it would still
+      # attempt a HEAD request.  So here is the "test".
+      connection = double(ActiveFedora::CachingConnection, delete: true)
+      fedora = double(ActiveFedora::Fedora, connection: connection)
+      expect(ActiveFedora).to receive(:fedora).and_return(fedora)
+      expect(connection).to receive(:delete).with(base_path)
+      expect { subject.remove! }.to change(described_class, :count).by(-1)
+    end
+  end
 end

spec/models/redis_endpoint_spec.rb

@@ -2,26 +2,36 @@
 
 RSpec.describe RedisEndpoint do
   let(:namespace) { 'foobar' }
+  let(:faux_redis_instance) { double(Hyrax::RedisEventStore, ping: 'PONG', clear: true) }
+  before { allow(subject).to receive(:redis_instance).and_return(faux_redis_instance) }
+  subject { described_class.new(namespace:) }
 
   describe '.options' do
-    subject { described_class.new(namespace:) }
-
     it 'uses the configured application settings' do
       expect(subject.options[:namespace]).to eq namespace
     end
   end
 
   describe '#ping' do
-    let(:faux_redis_instance) { double(Hyrax::RedisEventStore, ping: 'PONG') }
     it 'checks if the service is up' do
-      allow(subject).to receive(:redis_instance).and_return(faux_redis_instance)
+      allow(faux_redis_instance).to receive(:ping).and_return("PONG")
       expect(subject.ping).to be_truthy
     end
 
     it 'is false if the service is down' do
       allow(faux_redis_instance).to receive(:ping).and_raise(RuntimeError)
-      allow(subject).to receive(:redis_instance).and_return(faux_redis_instance)
       expect(subject.ping).to eq false
     end
   end
+
+  describe '#remove!' do
+    subject { described_class.create! }
+
+    it 'clears the namespace and deletes itself' do
+      expect(faux_redis_instance).to receive(:clear)
+      expect do
+        subject.remove!
+      end.to change(described_class, :count).by(-1)
+    end
+  end
 end

spec/models/solr_endpoint_spec.rb

@@ -61,4 +61,13 @@
       expect(subject.ping).to eq false
     end
   end
+
+  describe '#remove!' do
+    it 'schedules the removal and deletes the end point' do
+      instance = described_class.create!
+      allow(instance).to receive(:account).and_return(double(Account, search_only?: true))
+      expect(RemoveSolrCollectionJob).to receive(:perform_later)
+      expect { instance.remove! }.to change(described_class, :count).by(-1)
+    end
+  end
 end