Skip to content

Commit

Permalink
♻️ Favor Hyrax.query_service over method chaining
Browse files Browse the repository at this point in the history
Prior to this commit we relied on the access_control object to "self
convert" via the `#valkyrie_resource` method.  However, that method
circumvents the `Hyrax.query_service` which provides a more up to date
version of the permissions.

This is subtle bug is most notable when we have composite adapters;
those modeled in the `double_combo` branch.

How it manifests is as follows:

- I have a Work and ACL in Fedora.
- I update the ACL to Postgres.
  - When I fetch the Work, it's in Fedora and grabs the ACL from Fedora
    which is not the most current.

This relates to:

- 0de7df6
- #6670
  • Loading branch information
jeremyf committed Feb 6, 2024
1 parent 0de7df6 commit f0e99c5
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 3 deletions.
21 changes: 19 additions & 2 deletions lib/wings/model_transformer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,25 @@ def ensure_current_permissions(resource)
# otherwise, we can just rely on the `access_control_ids`.
return unless resource.respond_to?(:permission_manager)

resource.permission_manager.acl.permissions =
pcdm_object.access_control.valkyrie_resource.permissions
# When the pcdm_object has an access_control (see above) but there's no access_control_id, we
# need to rely on the computed access_control object. Why? There are tests that fail.
acl = if pcdm_object.access_control_id.nil?
pcdm_object.access_control.valkyrie_resource
else
begin
# Given that we have an access_control AND an access_control_id, we want to ensure
# that we fetch the access_control from persistence. Why? Because when we update
# an ACL and are using those adapters, we will write the ACL to the Valkyrie adapter
# without writing the work to the Valkyrie adapter. This might be a failing, but
# migrations in place are hard.
acl_id = pcdm_object.access_control_id
acl_id = ::Valkyrie::ID.new(acl_id) unless acl_id.is_a?(::Valkyrie::ID)
Hyrax.query_service.find_by(id: acl_id)
rescue ::Valkyrie::Persistence::ObjectNotFoundError
pcdm_object.access_control.valkyrie_resource
end
end
resource.permission_manager.acl.permissions = acl.permissions
end

##
Expand Down
2 changes: 1 addition & 1 deletion spec/services/hyrax/admin_set_create_service_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
.and_raise(Valkyrie::Persistence::ObjectNotFoundError)
end
expect(described_class).to receive(:create_default_admin_set!).and_call_original
expect(query_service).to receive(:find_by).with(id: anything).and_call_original # permission template
expect(query_service).to receive(:find_by).with(id: anything).at_least(1).times.and_call_original # permission template
admin_set = described_class.find_or_create_default_admin_set
expect(admin_set.title).to eq described_class::DEFAULT_TITLE
end
Expand Down

0 comments on commit f0e99c5

Please sign in to comment.