Skip to content

Commit

Permalink
Update nested reindexing
Browse files Browse the repository at this point in the history
Resolves #1775
Resolves #1785

Also removes ActiveFedora caching from the nesting index adapter.
Nested reindexing occurs midway through the transaction, so AF was
attempting to use the cache. However, this resulted in the updated solr
document not including all of the AF updates.

Reindexing changes have been separated into this one PR to isolate the
indexing changes from other changes.

In addition, unpins the solr version since incompatibilities have been
previously resolved.
  • Loading branch information
LaRita Robinson authored and elrayle committed Oct 11, 2017
1 parent 7e17f2f commit adc7d02
Show file tree
Hide file tree
Showing 7 changed files with 69 additions and 24 deletions.
1 change: 1 addition & 0 deletions .rubocop_fixme.yml
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ Metrics/ModuleLength:
- 'app/models/concerns/hyrax/ability.rb'
- 'app/services/hyrax/workflow/permission_query.rb'
- 'spec/services/hyrax/workflow/permission_query_spec.rb'
- 'app/models/concerns/hyrax/collection_behavior.rb'

Metrics/MethodLength:
Exclude:
Expand Down
1 change: 0 additions & 1 deletion .solr_wrapper
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
# Place any default configuration for solr_wrapper here
# port: 8983
version: 6.6.1
collection:
persist: true
dir: solr/config/
Expand Down
16 changes: 12 additions & 4 deletions app/models/concerns/hyrax/collection_behavior.rb
Original file line number Diff line number Diff line change
Expand Up @@ -126,10 +126,18 @@ def permission_template
# Calculate and update who should have edit access based on who
# has "manage" access in the PermissionTemplateAccess
def update_access_controls!
update!(edit_users: permission_template.agent_ids_for(access: 'manage', agent_type: 'user'),
edit_groups: permission_template.agent_ids_for(access: 'manage', agent_type: 'group'),
read_users: permission_template.agent_ids_for(access: 'view', agent_type: 'user'),
read_groups: (permission_template.agent_ids_for(access: 'view', agent_type: 'group') + visibility_group).uniq)
edit_users = permission_template.agent_ids_for(access: 'manage', agent_type: 'user')
edit_groups = permission_template.agent_ids_for(access: 'manage', agent_type: 'group')
read_users = permission_template.agent_ids_for(access: 'view', agent_type: 'user')
read_groups = (permission_template.agent_ids_for(access: 'view', agent_type: 'group') + visibility_group).uniq
update!(edit_users: edit_users,
edit_groups: edit_groups,
read_users: read_users,
read_groups: read_groups)
# added because the collection indexing happens before Hydra::AccessControls::Permission is
# saved, and the after_update_index callback in the nested_relationship_reindexer removes the
# permissions from the Solr document.
update_index
end

private
Expand Down
31 changes: 18 additions & 13 deletions app/services/hyrax/adapters/nesting_index_adapter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,14 @@ def self.find_preservation_document_by(id:)
def self.find_preservation_parent_ids_for(id:)
# Not everything is guaranteed to have library_collection_ids
# If it doesn't have it, what do we do?
fedora_object = ActiveFedora::Base.find(id)
if fedora_object.respond_to?(:member_of_collection_ids)
fedora_object.member_of_collection_ids
else
[]
ActiveFedora::Base.uncached do
fedora_object = ActiveFedora::Base.find(id)

if fedora_object.respond_to?(:member_of_collection_ids)
fedora_object.member_of_collection_ids
else
[]
end
end
end

Expand Down Expand Up @@ -65,14 +68,16 @@ def self.each_perservation_document_id_and_parent_ids(&block)
# @param pathnames [Array<String>]
# @return Hash - the attributes written to the indexing layer
def self.write_document_attributes_to_index_layer(id:, parent_ids:, ancestors:, pathnames:)
solr_doc = ActiveFedora::Base.find(id).to_solr # What is the current state of the solr document

# Now add the details from the nesting indexor to the document
solr_doc[solr_field_name_for_storing_ancestors] = ancestors
solr_doc[solr_field_name_for_storing_parent_ids] = parent_ids
solr_doc[solr_field_name_for_storing_pathnames] = pathnames
ActiveFedora::SolrService.add(solr_doc, commit: true)
solr_doc
ActiveFedora::Base.uncached do
solr_doc = ActiveFedora::Base.find(id).to_solr # What is the current state of the solr document

# Now add the details from the nesting indexor to the document
solr_doc[solr_field_name_for_storing_ancestors] = ancestors
solr_doc[solr_field_name_for_storing_parent_ids] = parent_ids
solr_doc[solr_field_name_for_storing_pathnames] = pathnames
ActiveFedora::SolrService.add(solr_doc, commit: true)
solr_doc
end
end

# @api public
Expand Down
16 changes: 15 additions & 1 deletion spec/factories/collections_factory.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,25 @@
end

after(:create) do |collection, evaluator|
if evaluator.with_permission_template
# create the permission template if it was requested, OR if
# nested reindexing is included (so we can apply the user's
# permissions).
if evaluator.with_permission_template || RSpec.current_example.metadata[:with_nested_reindexing]
attributes = { source_id: collection.id, source_type: 'collection' }
attributes = evaluator.with_permission_template.merge(attributes) if evaluator.with_permission_template.respond_to?(:merge)
create(:permission_template, attributes) unless Hyrax::PermissionTemplate.find_by(source_id: collection.id)
end
# Nested indexing requires that the user's permissions be saved
# on the Fedora object... if simply in local memory, they are
# lost when the adapter pulls the object from Fedora to reindex.
if RSpec.current_example.metadata[:with_nested_reindexing]
create(:permission_template_access,
:manage,
permission_template: collection.permission_template,
agent_type: 'user',
agent_id: evaluator.user.user_key)
collection.update_access_controls!
end
end

factory :public_collection, traits: [:public]
Expand Down
4 changes: 4 additions & 0 deletions spec/factories/generic_works.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@
end
end

after(:create) do |work, _evaluator|
work.save! if work.member_of_collections.present?
end

title ["Test title"]
visibility Hydra::AccessControls::AccessRight::VISIBILITY_TEXT_VALUE_PRIVATE

Expand Down
24 changes: 19 additions & 5 deletions spec/models/collection_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -243,13 +243,27 @@ class Member < ActiveFedora::Base
end
end

describe 'factories' do
it 'will create a permission_template when one is requested' do
expect { create(:collection, with_permission_template: true) }.to change { Hyrax::PermissionTemplate.count }.by(1)
context 'collection factory' do
describe 'when creating permission templates' do
it 'will create a permission_template when one is requested' do
expect { create(:collection, with_permission_template: true) }.to change { Hyrax::PermissionTemplate.count }.by(1)
end

it 'will not create a permission_template by default' do
expect { create(:collection) }.not_to change { Hyrax::PermissionTemplate.count }
end
end

it 'will not create a permission_template by default' do
expect { create(:collection) }.not_to change { Hyrax::PermissionTemplate.count }
describe 'when including nesting indexing', with_nested_indexing: true do
# Nested indexing requires that the user's permissions be saved
# on the Fedora object... if simply in local memory, they are
# lost when the adapter pulls the object from Fedora to reindex.
let(:user) { create(:user) }
let(:collection) { create(:collection, user: user) }

it 'will authorize the creating user' do
expect(user.can?(:edit, collection)).to be true
end
end
end

Expand Down

0 comments on commit adc7d02

Please sign in to comment.