Skip to content

Commit

Permalink
Consistently use Response#aggregation
Browse files Browse the repository at this point in the history
  • Loading branch information
cbeer committed Mar 23, 2015
1 parent 05f309c commit af00c88
Show file tree
Hide file tree
Showing 8 changed files with 63 additions and 39 deletions.
2 changes: 1 addition & 1 deletion lib/blacklight/catalog.rb
Expand Up @@ -78,7 +78,7 @@ def track
def facet
@facet = blacklight_config.facet_fields[params[:id]]
@response = get_facet_field_response(@facet.key, params)
@display_facet = @response.facets.first
@display_facet = @response.aggregations[@facet.key]

@pagination = facet_paginator(@facet, @display_facet)

Expand Down
4 changes: 2 additions & 2 deletions lib/blacklight/request_builders.rb
Expand Up @@ -184,8 +184,8 @@ def facet_limit_for(facet_field)
facet = blacklight_config.facet_fields[facet_field]
return if facet.blank?

if facet.limit and @response and @response.facet_by_field_name(facet_field)
limit = @response.facet_by_field_name(facet_field).limit
if facet.limit and @response and @response.aggregations[facet_field]
limit = @response.aggregations[facet_field].limit

if limit.nil? # we didn't get or a set a limit, so infer one.
facet.limit if facet.limit != true
Expand Down
2 changes: 1 addition & 1 deletion lib/blacklight/search_helper.rb
Expand Up @@ -176,7 +176,7 @@ def get_facet_pagination(facet_field, user_params=params || {}, extra_controller
# NOTE: The sniffing of the proper sort from the solr response is not
# currently tested for, tricky to figure out how to test, since the
# default setup we test against doesn't use this feature.
Blacklight::Solr::FacetPaginator.new(response.facets.first.items,
Blacklight::Solr::FacetPaginator.new(response.aggregations[facet_field].items,
:offset => response.params[:"f.#{facet_field}.facet.offset"],
:limit => limit,
:sort => response.params[:"f.#{facet_field}.facet.sort"] || response.params["facet.sort"]
Expand Down
2 changes: 1 addition & 1 deletion lib/blacklight/solr_response/group_response.rb
Expand Up @@ -25,7 +25,7 @@ def group_limit
def total
# ngroups is only available in Solr 4.1+
# fall back on the number of facet items for that field?
(group["ngroups"] || (response.facet_by_field_name(key) || []).length).to_s.to_i
(group["ngroups"] || (response.aggregations[key] || []).length).to_s.to_i
end

def start
Expand Down
18 changes: 9 additions & 9 deletions spec/controllers/catalog_controller_spec.rb
Expand Up @@ -39,33 +39,33 @@ def assigns_response
it "should have docs and facets for query with results", :integration => true do
get :index, q: user_query
expect(assigns_response.docs).to_not be_empty
assert_facets_have_values(assigns_response.facets)
assert_facets_have_values(assigns_response.aggregations)
end
it "should have docs and facets for existing facet value", :integration => true do
get :index, f: {"format" => 'Book'}
expect(assigns_response.docs).to_not be_empty
assert_facets_have_values(assigns_response.facets)
assert_facets_have_values(assigns_response.aggregations)
end
it "should have docs and facets for non-default results per page", :integration => true do
num_per_page = 7
get :index, :per_page => num_per_page
expect(assigns_response.docs).to have(num_per_page).items
assert_facets_have_values(assigns_response.facets)
assert_facets_have_values(assigns_response.aggregations)
end

it "should have docs and facets for second page", :integration => true do
page = 2
get :index, :page => page
expect(assigns_response.docs).to_not be_empty
expect(assigns_response.params[:start].to_i).to eq (page-1) * @controller.blacklight_config[:default_solr_params][:rows]
assert_facets_have_values(assigns_response.facets)
assert_facets_have_values(assigns_response.aggregations)
end

it "should have no docs or facet values for query without results", :integration => true do
get :index, q: 'sadfdsafasdfsadfsadfsadf' # query for no results

expect(assigns_response.docs).to be_empty
assigns_response.facets.each do |facet|
assigns_response.aggregations.each do |key, facet|
expect(facet.items).to be_empty
end
end
Expand Down Expand Up @@ -97,7 +97,7 @@ def assigns_response
end
it "should get facets when no query", :integration => true do
get :index
assert_facets_have_values(assigns_response.facets)
assert_facets_have_values(assigns_response.aggregations)
end
end

Expand Down Expand Up @@ -737,10 +737,10 @@ def export_as_mock


# there must be at least one facet, and each facet must have at least one value
def assert_facets_have_values(facets)
expect(facets).to_not be_empty
def assert_facets_have_values(aggregations)
expect(aggregations).to_not be_empty
# should have at least one value for each facet
facets.each do |facet|
aggregations.each do |key, facet|
expect(facet.items).to have_at_least(1).item
end
end
19 changes: 9 additions & 10 deletions spec/lib/blacklight/search_helper_spec.rb
Expand Up @@ -138,6 +138,7 @@ def params

describe "get_facet_pagination", :integration => true do
before do
@facet_field = 'format'
Deprecation.silence(Blacklight::SearchHelper) do
@facet_paginator = subject.get_facet_pagination(@facet_field)
end
Expand Down Expand Up @@ -357,27 +358,25 @@ def params

before do
(solr_response, document_list) = subject.search_results({ q: @all_docs_query}, default_method_chain)
@facets = solr_response.facets
@facets = solr_response.aggregations
end

it 'should have more than one facet' do
expect(@facets).to have_at_least(1).facet
end
it 'should have all facets specified in initializer' do
fields = blacklight_config.facet_fields.map { |k,v| v.field }

expect(@facets.map(&:name)).to match_array fields
expect(@facets.none? { |v| v.nil? }).to eq true
expect(@facets.keys).to include *blacklight_config.facet_fields.keys
expect(@facets.none? { |k, v| v.nil? }).to eq true
end

it 'should have at least one value for each facet' do
@facets.each do |facet|
@facets.each do |key, facet|
expect(facet.items).to have_at_least(1).hit
end
end
it 'should have multiple values for at least one facet' do
has_mult_values = false
@facets.each do |facet|
@facets.each do |key, facet|
if facet.items.size > 1
has_mult_values = true
break
Expand All @@ -386,7 +385,7 @@ def params
expect(has_mult_values).to eq true
end
it 'should have all value counts > 0' do
@facets.each do |facet|
@facets.each do |key, facet|
facet.items.each do |facet_vals|
expect(facet_vals.hits).to be > 0
end
Expand Down Expand Up @@ -676,14 +675,14 @@ def params
end
it "should get from @response facet.limit if available" do
@response = double()
allow(@response).to receive(:facet_by_field_name).with("language_facet").and_return(double(limit: nil))
allow(@response).to receive(:aggregations).and_return("language_facet" => double(limit: nil))
subject.instance_variable_set(:@response, @response)
blacklight_config.facet_fields['language_facet'].limit = 10
expect(subject.facet_limit_for("language_facet")).to eq 10
end
it "should get the limit from the facet field in @response" do
@response = double()
allow(@response).to receive(:facet_by_field_name).with("language_facet").and_return(double(limit: 16))
allow(@response).to receive(:aggregations).and_return("language_facet" => double(limit: 16))
subject.instance_variable_set(:@response, @response)
expect(subject.facet_limit_for("language_facet")).to eq 15
end
Expand Down
45 changes: 35 additions & 10 deletions spec/lib/blacklight/solr_response/facets_spec.rb
Expand Up @@ -22,7 +22,32 @@
end
end

describe "#facets" do
subject { Blacklight::SolrResponse.new({}, {}) }
let(:aggregations) { { x: 1, y: 2 } }
it "should get the aggregation values" do
allow(subject).to receive(:aggregations).and_return aggregations

Deprecation.silence(described_class) do
expect(subject.facets).to eq aggregations.values
end
end
end

describe "#facet_by_field_name" do
subject { Blacklight::SolrResponse.new({}, {}) }
let(:aggregations) { { x: double } }

it "should pull facets out of the aggregations" do
allow(subject).to receive(:aggregations).and_return aggregations

Deprecation.silence(described_class) do
expect(subject.facet_by_field_name(:x)).to eq aggregations[:x]
end
end
end

describe "#aggregations" do
let(:facet_field) { ['my_field', []] }
let(:response_header) { { params: request_params }}
let(:request_params) { Hash.new }
Expand All @@ -32,55 +57,55 @@
it "should extract a field-specific limit value" do
request_params['f.my_field.facet.limit'] = "10"
request_params['facet.limit'] = "15"
expect(subject.facet_by_field_name('my_field').limit).to eq 10
expect(subject.aggregations['my_field'].limit).to eq 10
end

it "should extract a global limit value" do
request_params['facet.limit'] = "15"
expect(subject.facet_by_field_name('my_field').limit).to eq 15
expect(subject.aggregations['my_field'].limit).to eq 15
end

it "should be the solr default limit if no value is found" do
expect(subject.facet_by_field_name('my_field').limit).to eq 100
expect(subject.aggregations['my_field'].limit).to eq 100
end
end

describe "#offset" do
it "should extract a field-specific offset value" do
request_params['f.my_field.facet.offset'] = "10"
request_params['facet.offset'] = "15"
expect(subject.facet_by_field_name('my_field').offset).to eq 10
expect(subject.aggregations['my_field'].offset).to eq 10
end

it "should extract a global offset value" do
request_params['facet.offset'] = "15"
expect(subject.facet_by_field_name('my_field').offset).to eq 15
expect(subject.aggregations['my_field'].offset).to eq 15
end

it "should be nil if no value is found" do
expect(subject.facet_by_field_name('my_field').offset).to eq 0
expect(subject.aggregations['my_field'].offset).to eq 0
end
end

describe "#sort" do
it "should extract a field-specific sort value" do
request_params['f.my_field.facet.sort'] = "alpha"
request_params['facet.sort'] = "index"
expect(subject.facet_by_field_name('my_field').sort).to eq 'alpha'
expect(subject.aggregations['my_field'].sort).to eq 'alpha'
end

it "should extract a global sort value" do
request_params['facet.sort'] = "alpha"
expect(subject.facet_by_field_name('my_field').sort).to eq 'alpha'
expect(subject.aggregations['my_field'].sort).to eq 'alpha'
end

it "should default to count if no value is found and the default limit is used" do
expect(subject.facet_by_field_name('my_field').sort).to eq 'count'
expect(subject.aggregations['my_field'].sort).to eq 'count'
end

it "should default to index if no value is found and the limit is unlimited" do
request_params['facet.limit'] = -1
expect(subject.facet_by_field_name('my_field').sort).to eq 'index'
expect(subject.aggregations['my_field'].sort).to eq 'index'
end
end
end
Expand Down
10 changes: 5 additions & 5 deletions spec/lib/blacklight/solr_response_spec.rb
Expand Up @@ -28,13 +28,13 @@ def create_response
end

it 'should provide facet helpers' do
expect(r.facets.size).to eq 2
expect(r.aggregations.size).to eq 2

field_names = r.facets.collect{|facet|facet.name}
field_names = r.aggregations.collect{|key, facet|facet.name}
expect(field_names.include?('cat')).to be true
expect(field_names.include?('manu')).to be true

first_facet = r.facets.select { |x| x.name == 'cat'}.first
first_facet = r.aggregations['cat']
expect(first_facet.name).to eq 'cat'

expect(first_facet.items.size).to eq 10
Expand All @@ -46,7 +46,7 @@ def create_response

expect(received).to eq expected

r.facets.each do |facet|
r.aggregations.each do |key, facet|
expect(facet).to respond_to :name
expect(facet).to respond_to :sort
expect(facet).to respond_to :offset
Expand Down Expand Up @@ -107,7 +107,7 @@ def create_response

it 'should return the correct value when calling facet_by_field_name' do
r = create_response
facet = r.facet_by_field_name('cat')
facet = r.aggregations['cat']
expect(facet.name).to eq 'cat'
end

Expand Down

0 comments on commit af00c88

Please sign in to comment.