Skip to content

Commit

Permalink
Refactoring .find_target for HasAndBelongsToMany
Browse files Browse the repository at this point in the history
Removes the unnecessary calls to :solr_page_size and takes advantage of
ActiveFedora::Base.find accepting an array. The method is also made
private.
  • Loading branch information
awead committed Jun 24, 2015
1 parent e79b12b commit c8f0dcd
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 23 deletions.
@@ -1,11 +1,13 @@
module ActiveFedora::Associations::Builder
class HasAndBelongsToMany < CollectionAssociation #:nodoc:
extend Deprecation
self.macro = :has_and_belongs_to_many

self.valid_options += [:inverse_of, :solr_page_size]

def validate_options
super
Deprecation.warn HasAndBelongsToMany, ":solr_page_size doesn't do anything anymore and will be removed in ActiveFedora 10" if options.key?(:solr_page_size)
if !options[:predicate]
raise "You must specify a predicate for #{name}"
elsif !options[:predicate].kind_of?(RDF::URI)
Expand Down
Expand Up @@ -46,20 +46,6 @@ def concat_records(*records)
result && records
end


def find_target
page_size = @reflection.options[:solr_page_size]
page_size ||= 200
ids = owner[reflection.foreign_key]
return [] if ids.blank?
solr_result = []
0.step(ids.size,page_size) do |startIdx|
query = ActiveFedora::SolrQueryBuilder.construct_query_for_ids(ids.slice(startIdx,page_size))
solr_result += ActiveFedora::SolrService.query(query, rows: page_size)
end
return ActiveFedora::QueryResultBuilder.reify_solr_results(solr_result)
end

# In a HABTM, just look in the RDF, no need to run a count query from solr.
def count(options = {})
owner[reflection.foreign_key].size
Expand Down Expand Up @@ -96,6 +82,12 @@ def stale_state
owner[reflection.foreign_key]
end

def find_target
ids = owner[reflection.foreign_key]
return [] if ids.blank?
ActiveFedora::Base.find(ids)
end

end
end
end
20 changes: 11 additions & 9 deletions spec/unit/has_and_belongs_to_many_association_spec.rb
Expand Up @@ -42,17 +42,19 @@ class Page < ActiveFedora::Base

describe "finding member" do
let(:ids) { (0..15).map(&:to_s) }
let(:query1) { ids.slice(0,10).map {|i| "_query_:\"{!raw f=id}#{i}\""}.join(" OR ") }
let(:query2) { ids.slice(10,10).map {|i| "_query_:\"{!raw f=id}#{i}\""}.join(" OR ") }

it "should call solr query multiple times" do
reflection = Book.create_reflection(:has_and_belongs_to_many, :pages, { predicate: ActiveFedora::RDF::Fcrepo::RelsExt.isMemberOfCollection, solr_page_size: 10}, Book)
let(:reflection) { Book.create_reflection(:has_and_belongs_to_many, :pages, { predicate: ActiveFedora::RDF::Fcrepo::RelsExt.isMemberOfCollection }, Book) }
let(:association) { ActiveFedora::Associations::HasAndBelongsToManyAssociation.new(subject, reflection) }
it "calls ActiveFedora::Base.find" do
expect(subject).to receive(:[]).with('page_ids').and_return(ids)
expect(ActiveFedora::SolrService).to receive(:query).with(query1, rows: 10).and_return([])
expect(ActiveFedora::SolrService).to receive(:query).with(query2, rows: 10).and_return([])
expect(ActiveFedora::Base).to receive(:find).with(ids)
association.send(:find_target)
end
end

ac = ActiveFedora::Associations::HasAndBelongsToManyAssociation.new(subject, reflection)
ac.find_target
describe "solr page size option" do
it "sends a deprecation warning" do
expect(Deprecation).to receive(:warn)
Book.has_and_belongs_to_many(:pages, predicate: ActiveFedora::RDF::Fcrepo::RelsExt.isMemberOfCollection, solr_page_size: 123)
end
end
end
Expand Down

0 comments on commit c8f0dcd

Please sign in to comment.