Skip to content

Instantly share code, notes, and snippets.

@jeremyf
Last active January 23, 2024 18:16
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save jeremyf/90f6c6ae81f9c4596b666237d5da2931 to your computer and use it in GitHub Desktop.
Save jeremyf/90f6c6ae81f9c4596b666237d5da2931 to your computer and use it in GitHub Desktop.

tl;dr It appears that the memory adapter has a bug in the double_combo branch of Hyrax.

Note: I have run the below in double_combo and have run the code in main. In double_combo I'm seeing duplicate resources. In main, I'm not. Which highlights that perhaps the persister is not doing it's job correctly.

Reviewing spec/services/hyrax/access_control_list_spec.rb#L156 (see below), I added a debug in the change { } to see the values before and after calling acl.save.

it 'deletes the permission policy' do
  delete_me = acl.permissions.first
  acl.delete(delete_me)
  rest = acl.permissions.clone

  expect { acl.save }
    .to change {
          require 'byebug'; byebug; 
          Hyrax::AccessControl.for(resource: resource, query_service: acl.query_service).permissions
    }.to contain_exactly(*rest)
end

Reviewing the Hyrax::CustomQueries::FindAccessControl.find_access_control_for function:

:CAPTURED_AT: 2024-01-23 11:18 :REMOTE_URL: Hyrax::CustomQueries::FindAccessControl :LOCAL_FILE: file:///Users/jeremy/git/hyrax/app/services/hyrax/custom_queries/find_access_control.rb :FUNCTION_NAME: Hyrax::CustomQueries::FindAccessControl

def find_access_control_for(resource:)
  query_service
    .find_inverse_references_by(resource: resource, property: :access_to)
    .find { |r| r.is_a?(Hyrax::AccessControl) } ||
    raise(Valkyrie::Persistence::ObjectNotFoundError)
rescue ArgumentError # some adapters raise ArgumentError for missing resources
  raise(Valkyrie::Persistence::ObjectNotFoundError)
end

The to change {} block calls the above Hyrax::CustomQueries::FindAccessControl.find_access_control_for function. The first time, before we call { acl.save } the query_service.find_inverse_references_by(resource: resource, property: :access_to) returns an array with one element; call it Object 1.

The second time, after the { acl.save } the query_service.find_inverse_references_by(resource: resource, property: :access_to) returns an array with two elements. The first elmenent remains Object 1 with the original permissions. The second element, Object 2, has the modified permissions. Both are Hyrax::AccessControl objects. When we then call find on the array, we fetch the first element; (e.g. Object 1).

Below is the debug of the code after the { acl.save } block.

[129, 138] in /app/bundle/ruby/3.2.0/bundler/gems/valkyrie-3037490be75d/lib/valkyrie/persistence/memory/query_service.rb
   129:       ensure_persisted(resource) if resource
   130:       id ||= resource.id
   131:       result = find_all.select do |obj|
   132:         Array.wrap(obj[property]).include?(id)
   133:       end
=> 134:       return result unless model
   135:       result.select { |obj| obj.is_a?(model) }
   136:     end
   137:
   138:     # Find all parents of a given resource.

   (byebug) find_all.map(&:access_to)
   [#<Valkyrie::ID:0x0000ffffaa966860 @id="42bf3f7f-e594-4b20-8bf7-3554ba9c7da4">, #<Valkyrie::ID:0x0000ffff9f8fd668 @id="42bf3f7f-e594-4b20-8bf7-3554ba9c7da4">]
   (byebug) pp find_all.map(&:permissions)
   [[#<Hyrax::Permission access_to=#<Valkyrie::ID:0x0000ffff9f764310 @id="42bf3f7f-e594-4b20-8bf7-3554ba9c7da4"> agent="user1@example.com" mode=:read>,
     #<Hyrax::Permission access_to=#<Valkyrie::ID:0x0000ffffaa96ff00 @id="42bf3f7f-e594-4b20-8bf7-3554ba9c7da4"> agent="user2@example.com" mode=:edit>],
    [#<Hyrax::Permission access_to=#<Valkyrie::ID:0x0000ffffaa96ff00 @id="42bf3f7f-e594-4b20-8bf7-3554ba9c7da4"> agent="user2@example.com" mode=:edit>]]

The above demonstrates that we have two different Hyrax::AccessControl objects that point access to the same Valkyrie::Resource.

@jeremyf
Copy link
Author

jeremyf commented Jan 23, 2024

Delving further:

In double_combo we have:

# Hyrax::AccessControlList#save
def save
  return true unless pending_changes?

  change_set.sync
  persister.save(resource: change_set.resource)
  Hyrax.publisher.publish('object.acl.updated', acl: self, result: :success)

  true
end

The 166ba72dbab2fc48215cb4ec46041f6a8155da52 removes @change_set = nil after the call to Hyrax.publisher.

The impact is that when we save the change_set.resource the returning value has an ID. However, the memoized change_set retains an ID of nil. Thus, when we again call save our memoized change_set does not have an ID and we create a new AccessControl object.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment