Add a generic interface for ACL keys to Group and User

app/models/concerns/hyrax/user.rb

@@ -53,6 +53,13 @@ def user_key
     public_send(self.class.user_key_field)
   end
 
+  ##
+  # @return [String] a local identifier for this user; for use (e.g.) in ACL
+  #   data
+  def agent_key
+    user_key
+  end
+
   # Look for, in order:
   #   A cached version of the agent
   #   A non-cached version (direct read of the database)
@@ -177,6 +184,10 @@ def find_or_create_system_user(user_key)
       User.find_by_user_key(user_key) || User.create!(user_key_field => user_key, password: Devise.friendly_token[0, 20])
     end
 
+    def from_agent_key(key)
+      User.find_by_user_key(key)
+    end
+
     def from_url_component(component)
       User.find_by_user_key(component.gsub(/-dot-/, '.'))
     end

app/models/hyrax/group.rb

@@ -7,12 +7,31 @@ def self.name_prefix
       DEFAULT_NAME_PREFIX
     end
 
+    ##
+    # @return [Hyrax::Group]
+    def self.from_agent_key(key)
+      new(key.delete_prefix(name_prefix))
+    end
+
     def initialize(name)
       @name = name
     end
 
     attr_reader :name
 
+    ##
+    # @return [Boolean]
+    def ==(other)
+      other.class == self.class && other.name == name
+    end
+
+    ##
+    # @return [String] a local identifier for this group; for use (e.g.) in ACL
+    #   data
+    def agent_key
+      self.class.name_prefix + name
+    end
+
     def to_sipity_agent
       sipity_agent || create_sipity_agent!
     end

app/services/hyrax/access_control_list.rb

@@ -200,13 +200,14 @@ def initialize(acl, mode)
 
       private
 
+      ##
+      # Returns the identifier used by ACLs to identify agents.
+      #
+      # This defaults to the `:agent_key`, but if that method doesn’t exist,
+      # `:user_key` will be used as a fallback.
       def id_for(agent:)
-        case agent
-        when Hyrax::Group
-          "#{Hyrax::Group.name_prefix}#{agent.name}"
-        else
-          agent.user_key.to_s
-        end
+        key = agent.try(:agent_key) || agent.user_key
+        key.to_s
       end
     end
 

spec/models/hyrax/group_spec.rb

@@ -0,0 +1,50 @@
+# frozen_string_literal: true
+
+RSpec.describe Hyrax::Group, type: :model do
+  let(:name) { 'etaoin' }
+  let(:group) { described_class.new(name) }
+
+  describe '.from_agent_key' do
+    it 'returns an equivalent group' do
+      expect(described_class.from_agent_key(group.agent_key)).to eq group
+    end
+  end
+
+  describe '#==' do
+    let(:other_group) { described_class.new(group.name) }
+
+    it 'correctly determines equality for equivalent groups' do
+      expect(other_group).to eq group
+    end
+  end
+
+  describe '#name' do
+    it 'returns the name' do
+      expect(group.name).to eq name
+    end
+  end
+
+  describe '#agent_key' do
+    it 'returns the name prefixed with the name prefix' do
+      expect(group.agent_key).to eq described_class.name_prefix + group.name
+    end
+  end
+
+  describe '#to_sipity_agent' do
+    subject { group.to_sipity_agent }
+
+    it 'will find or create a Sipity::Agent' do
+      expect { subject }.to change { Sipity::Agent.count }.by(1)
+    end
+
+    context "when another process makes the agent" do
+      before do
+        group.to_sipity_agent # create the agent ahead of time
+      end
+
+      it "returns the existing agent" do
+        expect { subject }.not_to change { Sipity::Agent.count }
+      end
+    end
+  end
+end

spec/models/user_spec.rb

@@ -61,6 +61,20 @@
     end
   end
 
+  describe '#agent_key' do
+    let(:key) { user.agent_key }
+
+    it 'is the same as the user key' do
+      expect(key).to eq user.user_key
+    end
+
+    it 'is findable by agent_key' do
+      user.save!
+
+      expect(described_class.from_agent_key(key)).to eq user
+    end
+  end
+
   it "has an email" do
     expect(user.email).to be_kind_of String
   end