Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow chaining of #where with combination of string and hash arguments #372

Merged
merged 1 commit into from
Mar 10, 2014
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/active_fedora/associations/association_scope.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ def add_constraints(scope)
if reflection.source_macro == :belongs_to
elsif reflection.source_macro == :has_and_belongs_to_many
else
scope = scope.where( association.send(:find_predicate) => owner.internal_uri)
scope = scope.where( ActiveFedora::SolrService.construct_query_for_rel(association.send(:find_predicate) => owner.internal_uri))
end


Expand Down
33 changes: 16 additions & 17 deletions lib/active_fedora/relation/finder_methods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -151,9 +151,8 @@ def find_one(pid, cast=nil)
if where_values.empty?
load_from_fedora(pid, cast)
else
query = [ActiveFedora::SolrService.construct_query_for_rel(where_values),
ActiveFedora::SolrService.raw_query(SOLR_DOCUMENT_ID, pid)].
join(" AND ".freeze)
conditions = where_values + [ActiveFedora::SolrService.raw_query(SOLR_DOCUMENT_ID, pid)]
query = conditions.join(" AND ".freeze)
to_enum(:find_each, query, {}).to_a.first
end
end
Expand Down Expand Up @@ -196,18 +195,27 @@ def find_some(ids, cast)
# Returns a solr query for the supplied conditions
# @param[Hash] conditions solr conditions to match
def create_query(conditions)
conditions.kind_of?(Hash) ? create_query_from_hash(conditions) : create_query_from_string(conditions)
case conditions
when Hash
build_query([create_query_from_hash(conditions)])
when String
build_query([conditions])
else
build_query(conditions)
end
end

def create_query_from_hash(conditions)
def build_query(conditions)
clauses = search_model_clause ? [search_model_clause] : []
conditions.each_pair do |key,value|
clauses << condition_to_clauses(key, value)
end
clauses += conditions.reject{|c| c.blank?}
return "*:*" if clauses.empty?
clauses.compact.join(" AND ")
end

def create_query_from_hash(conditions)
conditions.map {|key,value| condition_to_clauses(key, value)}.compact.join(" AND ")
end

def condition_to_clauses(key, value)
unless value.nil?
# if the key is a property name, turn it into a solr field
Expand All @@ -227,15 +235,6 @@ def condition_to_clauses(key, value)
end
end

def create_query_from_string(conditions)
model_clause = search_model_clause
if model_clause
conditions ? "#{model_clause} AND (#{conditions})" : model_clause
else
conditions
end
end

# Return the solr clause that queries for this type of class
def search_model_clause
unless @klass == ActiveFedora::Base
Expand Down
2 changes: 1 addition & 1 deletion lib/active_fedora/relation/merger.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ def initialize(relation, other)

def merge
# TODO merge order
relation.where_values = relation.where_values.merge(other.where_values)
relation.where_values += other.where_values
relation
end
end
Expand Down
16 changes: 11 additions & 5 deletions lib/active_fedora/relation/query_methods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ def extending_values=(values)
end

def where_values
@values[:where] ||= {}
@values[:where] ||= []
end

def where_values=(values)
Expand Down Expand Up @@ -49,12 +49,18 @@ def where(values)
end

def where!(values)
if values.is_a?(String)
self.where_values = values
self.where_values += build_where(values)
self
end

def build_where(values)
return [] if values.blank?
case values
when Hash
[create_query_from_hash(values)]
else
self.where_values = self.where_values.merge(values)
[values]
end
self
end

# Order the returned records by the field and direction provided
Expand Down
2 changes: 1 addition & 1 deletion spec/integration/scoped_query_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ def to_solr(doc = {})
end

it "should chain where queries" do
ModelIntegrationSpec::Basic.where(ActiveFedora::SolrService.solr_name('bar', type: :string) => 'Peanuts').where(ActiveFedora::SolrService.solr_name('foo', type: :string) => 'bar').where_values.should == {ActiveFedora::SolrService.solr_name('bar', type: :string) => 'Peanuts', ActiveFedora::SolrService.solr_name('foo', type: :string) => 'bar'}
ModelIntegrationSpec::Basic.where(ActiveFedora::SolrService.solr_name('bar', type: :string) => 'Peanuts').where("#{ActiveFedora::SolrService.solr_name('foo', type: :string)}:bar").where_values.should == ["#{ActiveFedora::SolrService.solr_name('bar', type: :string)}:Peanuts", "#{ActiveFedora::SolrService.solr_name('foo', type: :string)}:bar"]
end

it "should chain count" do
Expand Down
2 changes: 1 addition & 1 deletion spec/unit/query_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ class Basic < ActiveFedora::Base
end
it "should allow conditions" do
mock_result = {'response'=>{'numFound'=>7}}
ActiveFedora::SolrService.should_receive(:query).with("#{@model_query} AND (foo:bar)", :rows=>0, :raw=>true).and_return(mock_result)
ActiveFedora::SolrService.should_receive(:query).with("#{@model_query} AND foo:bar", :rows=>0, :raw=>true).and_return(mock_result)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test needs to be restored. The parentheses are needed for any where condition that has an OR including access control conditions. I'll update this PR once I've fixed this.

SpecModel::Basic.count(:conditions=>'foo:bar').should == 7
end

Expand Down