diff --git a/app/helpers/blacklight/catalog_helper_behavior.rb b/app/helpers/blacklight/catalog_helper_behavior.rb index 6e5c81fac6..bdcefa3ef1 100644 --- a/app/helpers/blacklight/catalog_helper_behavior.rb +++ b/app/helpers/blacklight/catalog_helper_behavior.rb @@ -14,11 +14,9 @@ def page_entries_info(collection, options = {}) entry_name = if options[:entry_name] options[:entry_name] elsif collection.respond_to? :model # DataMapper - collection.model.model_name.human.downcase + collection.model.model_name.human.downcase elsif collection.respond_to? :model_name and !collection.model_name.nil? # AR, Blacklight::PaginationMethods - collection.model_name.human.downcase - elsif collection.is_a?(::Kaminari::PaginatableArray) - 'entry' + collection.model_name.human.downcase else t('blacklight.entry_name.default') end diff --git a/app/helpers/blacklight/url_helper_behavior.rb b/app/helpers/blacklight/url_helper_behavior.rb index 4c815701f5..7b4be741db 100644 --- a/app/helpers/blacklight/url_helper_behavior.rb +++ b/app/helpers/blacklight/url_helper_behavior.rb @@ -235,7 +235,7 @@ def add_facet_params(field, item, source_params = params) p[:f][url_field].push(value) if item and item.respond_to?(:fq) and item.fq - item.fq.each do |f,v| + Array(item.fq).each do |f,v| p = add_facet_params(f, v, p) end end diff --git a/blacklight.gemspec b/blacklight.gemspec index 1c505159c0..526a5749cb 100644 --- a/blacklight.gemspec +++ b/blacklight.gemspec @@ -19,7 +19,7 @@ Gem::Specification.new do |s| s.add_dependency "rails", ">= 3.2.6", "< 5" s.add_dependency "nokogiri", "~>1.6" # XML Parser - s.add_dependency "kaminari", "~> 0.13" # the pagination (page 1,2,3, etc..) of our search results + s.add_dependency "kaminari", "~> 0.15" # the pagination (page 1,2,3, etc..) of our search results s.add_dependency "rsolr", "~> 1.0.11" # Library for interacting with rSolr. s.add_dependency "bootstrap-sass", "~> 3.2" s.add_dependency "deprecation" diff --git a/config/locales/blacklight.en.yml b/config/locales/blacklight.en.yml index deece2f7e7..d87cba780a 100644 --- a/config/locales/blacklight.en.yml +++ b/config/locales/blacklight.en.yml @@ -188,6 +188,7 @@ en: more_html: 'more %{field_name} »' selected: remove: '[remove]' + missing: "[Missing]" group: more: 'more »' filters: diff --git a/lib/blacklight/facet.rb b/lib/blacklight/facet.rb index c5b9e9f8d2..1158f93be5 100644 --- a/lib/blacklight/facet.rb +++ b/lib/blacklight/facet.rb @@ -32,61 +32,13 @@ def facet_configuration_for_field(field) # Get a FacetField object from the @response def facet_by_field_name field_or_field_name case field_or_field_name - when String, Symbol - facet_field = facet_configuration_for_field(field_or_field_name) - extract_facet_by_field(facet_field) - when Blacklight::Configuration::FacetField - extract_facet_by_field(field_or_field_name) - else - field_or_field_name - end - end - - private - - # Get the solr response for the field :field - def extract_facet_by_field facet_field - case - when (facet_field.respond_to?(:query) and facet_field.query) - create_facet_field_response_for_query_facet_field facet_field.key, facet_field - when (facet_field.respond_to?(:pivot) and facet_field.pivot) - create_facet_field_response_for_pivot_facet_field facet_field.key, facet_field - else - @response.facet_by_field_name(facet_field.field) - end - end - - def create_facet_field_response_for_query_facet_field facet_name, facet_field - salient_facet_queries = facet_field.query.map { |k, x| x[:fq] } - items = [] - @response.facet_queries.select { |k,v| salient_facet_queries.include?(k) }.reject { |value, hits| hits == 0 }.map do |value,hits| - salient_fields = facet_field.query.select { |key, val| val[:fq] == value } - key = ((salient_fields.keys if salient_fields.respond_to? :keys) || salient_fields.first).first - items << Blacklight::SolrResponse::Facets::FacetItem.new(:value => key, :hits => hits, :label => facet_field.query[key][:label]) + when String, Symbol, Blacklight::Configuration::FacetField + facet_field = facet_configuration_for_field(field_or_field_name) + @response.aggregations[facet_field.key] + else + # is this really a useful case? + field_or_field_name end - - Blacklight::SolrResponse::Facets::FacetField.new facet_name, items - end - - - def create_facet_field_response_for_pivot_facet_field facet_name, facet_field - items = [] - (@response.facet_pivot[facet_field.pivot.join(",")] || []).map do |lst| - items << construct_pivot_field(lst) - end - - Blacklight::SolrResponse::Facets::FacetField.new facet_name, items - end - - def construct_pivot_field lst, parent_fq = {} - items = [] - - lst[:pivot].each do |i| - items << construct_pivot_field(i, parent_fq.merge({ lst[:field] => lst[:value] })) - end if lst[:pivot] - - Blacklight::SolrResponse::Facets::FacetItem.new(:value => lst[:value], :hits => lst[:count], :field => lst[:field], :items => items, :fq => parent_fq) - end end end diff --git a/lib/blacklight/solr_repository.rb b/lib/blacklight/solr_repository.rb index 067f78ba77..293cda345c 100644 --- a/lib/blacklight/solr_repository.rb +++ b/lib/blacklight/solr_repository.rb @@ -43,7 +43,7 @@ def send_and_receive(path, solr_params = {}) key = blacklight_config.http_method == :post ? :data : :params res = connection.send_and_receive(path, {key=>solr_params.to_hash, method:blacklight_config.http_method}) - solr_response = blacklight_config.response_model.new(res, solr_params, document_model: blacklight_config.document_model) + solr_response = blacklight_config.response_model.new(res, solr_params, document_model: blacklight_config.document_model, blacklight_config: blacklight_config) Blacklight.logger.debug("Solr query: #{solr_params.inspect}") Blacklight.logger.debug("Solr response: #{solr_response.inspect}") if defined?(::BLACKLIGHT_VERBOSE_LOGGING) and ::BLACKLIGHT_VERBOSE_LOGGING diff --git a/lib/blacklight/solr_response.rb b/lib/blacklight/solr_response.rb index 2790703a1d..34decbe269 100644 --- a/lib/blacklight/solr_response.rb +++ b/lib/blacklight/solr_response.rb @@ -1,4 +1,5 @@ class Blacklight::SolrResponse < HashWithIndifferentAccess + extend Deprecation require 'blacklight/solr_response/pagination_methods' @@ -16,44 +17,35 @@ class Blacklight::SolrResponse < HashWithIndifferentAccess include MoreLikeThis attr_reader :request_params - attr_accessor :document_model + attr_accessor :document_model, :blacklight_config def initialize(data, request_params, options = {}) super(force_to_utf8(data)) @request_params = request_params self.document_model = options[:solr_document_model] || options[:document_model] || SolrDocument + self.blacklight_config = options[:blacklight_config] end def header - self['responseHeader'] + self['responseHeader'] || {} end - - def update(other_hash) - other_hash.each_pair { |key, value| self[key] = value } - self - end def params - (header and header['params']) ? header['params'] : request_params + header['params'] || request_params end def rows - params[:rows].to_i + params[:rows].to_i end def sort params[:sort] end - def docs - @docs ||= begin - response['docs'] || [] - end - end - def documents - docs.collect{|doc| document_model.new(doc, self) } + @documents ||= (response['docs'] || []).collect{|doc| document_model.new(doc, self) } end + alias_method :docs, :documents def grouped @groups ||= self["grouped"].map do |field, group| diff --git a/lib/blacklight/solr_response/facets.rb b/lib/blacklight/solr_response/facets.rb index 0723591ee1..bac1655466 100644 --- a/lib/blacklight/solr_response/facets.rb +++ b/lib/blacklight/solr_response/facets.rb @@ -1,6 +1,7 @@ require 'ostruct' module Blacklight::SolrResponse::Facets + extend Deprecation # represents a facet value; which is a field value and its hit count class FacetItem < OpenStruct @@ -67,40 +68,29 @@ def solr_default_offset end end - + + ## + # Get all the Solr facet data (fields, queries, pivots) as a hash keyed by + # both the Solr field name and/or by the blacklight field name + def aggregations + @aggregations ||= {}.merge(facet_field_aggregations).merge(facet_query_aggregations).merge(facet_pivot_aggregations) + end + # @response.facets.each do |facet| # facet.name # facet.items # end # "caches" the result in the @facets instance var def facets - @facets ||= begin - facet_fields.map do |(facet_field_name,values_and_hits)| - items = [] - options = {} - values_and_hits.each_slice(2) do |k,v| - items << FacetItem.new(:value => k, :hits => v) - end - options[:sort] = (params[:"f.#{facet_field_name}.facet.sort"] || params[:'facet.sort']) - if params[:"f.#{facet_field_name}.facet.limit"] || params[:"facet.limit"] - options[:limit] = (params[:"f.#{facet_field_name}.facet.limit"] || params[:"facet.limit"]).to_i - end - - if params[:"f.#{facet_field_name}.facet.offset"] || params[:'facet.offset'] - options[:offset] = (params[:"f.#{facet_field_name}.facet.offset"] || params[:'facet.offset']).to_i - end - FacetField.new(facet_field_name, items, options) - end - end + aggregations.values end - + deprecation_deprecate facets: :aggregations + # pass in a facet field name and get back a Facet instance def facet_by_field_name(name) - @facets_by_field_name ||= {} - @facets_by_field_name[name] ||= ( - facets.detect{|facet|facet.name.to_s == name.to_s} - ) + aggregations[name] end + deprecation_deprecate facet_by_field_name: :aggregations def facet_counts @facet_counts ||= self['facet_counts'] || {} @@ -108,7 +98,16 @@ def facet_counts # Returns the hash of all the facet_fields (ie: {'instock_b' => ['true', 123, 'false', 20]} def facet_fields - @facet_fields ||= facet_counts['facet_fields'] || {} + @facet_fields ||= begin + val = facet_counts['facet_fields'] || {} + + # this is some old solr (1.4? earlier?) serialization of facet fields + if val.is_a? Array + Hash[val] + else + val + end + end end # Returns all of the facet queries @@ -120,5 +119,114 @@ def facet_queries def facet_pivot @facet_pivot ||= facet_counts['facet_pivot'] || {} end + + private + ## + # Convert Solr responses of various json.nl flavors to + def list_as_hash solr_list + # map + @facet_fields_as_hash ||= if solr_list.values.first.is_a? Hash + solr_list + else + solr_list.each_with_object({}) do |(key, values), hash| + hash[key] = if values.first.is_a? Array + # arrarr + Hash[values] + else + # flat + Hash[values.each_slice(2).to_a] + end + end + end + end + + ## + # Convert Solr's facet_field response into + # a hash of Blacklight::SolrResponse::Facet::FacetField objects + def facet_field_aggregations + list_as_hash(facet_fields).each_with_object({}) do |(facet_field_name, values), hash| + items = [] + options = {} + values.each do |value, hits| + i = FacetItem.new(value: value, hits: hits) + + # solr facet.missing serialization + if value.nil? + i.label = I18n.t(:"blacklight.search.fields.facet.missing.#{facet_field_name}", default: [:"blacklight.search.facets.missing"]) + i.fq = "-#{facet_field_name}:[* TO *]" + end + + items << i + end + options[:sort] = (params[:"f.#{facet_field_name}.facet.sort"] || params[:'facet.sort']) + if params[:"f.#{facet_field_name}.facet.limit"] || params[:"facet.limit"] + options[:limit] = (params[:"f.#{facet_field_name}.facet.limit"] || params[:"facet.limit"]).to_i + end + + if params[:"f.#{facet_field_name}.facet.offset"] || params[:'facet.offset'] + options[:offset] = (params[:"f.#{facet_field_name}.facet.offset"] || params[:'facet.offset']).to_i + end + + hash[facet_field_name] = FacetField.new(facet_field_name, items, options) + + if blacklight_config and !blacklight_config.facet_fields[facet_field_name] + # alias all the possible blacklight config names.. + blacklight_config.facet_fields.select { |k,v| v.field == facet_field_name }.each do |key,_| + hash[key] = hash[facet_field_name] + end + end + end + end + + ## + # Aggregate Solr's facet_query response into the virtual facet fields defined + # in the blacklight configuration + def facet_query_aggregations + return {} unless blacklight_config + + blacklight_config.facet_fields.select { |k,v| v.query }.each_with_object({}) do |(field_name, facet_field), hash| + salient_facet_queries = facet_field.query.map { |k, x| x[:fq] } + items = [] + facet_queries.select { |k,v| salient_facet_queries.include?(k) }.reject { |value, hits| hits == 0 }.map do |value,hits| + salient_fields = facet_field.query.select { |key, val| val[:fq] == value } + key = ((salient_fields.keys if salient_fields.respond_to? :keys) || salient_fields.first).first + items << Blacklight::SolrResponse::Facets::FacetItem.new(value: key, hits: hits, label: facet_field.query[key][:label]) + end + + hash[field_name] = Blacklight::SolrResponse::Facets::FacetField.new field_name, items + end + end + + ## + # Convert Solr's facet_pivot response into + # a hash of Blacklight::SolrResponse::Facet::FacetField objects + def facet_pivot_aggregations + facet_pivot.each_with_object({}) do |(field_name, values), hash| + items = [] + values.map do |lst| + items << construct_pivot_field(lst) + end + + if blacklight_config and !blacklight_config.facet_fields[field_name] + # alias all the possible blacklight config names.. + blacklight_config.facet_fields.select { |k,v| v.pivot and v.pivot.join(",") == field_name }.each do |key, _| + hash[key] = Blacklight::SolrResponse::Facets::FacetField.new key, items + end + end + end + end + + ## + # Recursively parse the pivot facet response to build up the full pivot tree + def construct_pivot_field lst, parent_fq = {} + items = [] + + lst[:pivot].each do |i| + items << construct_pivot_field(i, parent_fq.merge({ lst[:field] => lst[:value] })) + end if lst[:pivot] + + Blacklight::SolrResponse::Facets::FacetItem.new(value: lst[:value], hits: lst[:count], field: lst[:field], items: items, fq: parent_fq) + end + end # end Facets diff --git a/lib/blacklight/solr_response/pagination_methods.rb b/lib/blacklight/solr_response/pagination_methods.rb index 93d7c04c37..519219fae9 100644 --- a/lib/blacklight/solr_response/pagination_methods.rb +++ b/lib/blacklight/solr_response/pagination_methods.rb @@ -14,21 +14,4 @@ def offset_value #:nodoc: def total_count #:nodoc: total end - - def model_name - if !docs.empty? and docs.first.respond_to? :model_name - docs.first.model_name - end - end - - ## Methods in kaminari master that we'd like to use today. - # Next page number in the collection - def next_page - current_page + 1 unless last_page? - end - - # Previous page number in the collection - def prev_page - current_page - 1 unless first_page? - end end \ No newline at end of file diff --git a/lib/railties/blacklight.rake b/lib/railties/blacklight.rake index 3b931d4868..028eb7ad71 100644 --- a/lib/railties/blacklight.rake +++ b/lib/railties/blacklight.rake @@ -103,7 +103,7 @@ namespace :blacklight do print " - fetch: " begin - doc_id = response.docs.first[SolrDocument.unique_key] + doc_id = response.documents.first.id response, doc = controller.fetch doc_id if response.header['status'] == 0 and doc diff --git a/spec/helpers/facets_helper_spec.rb b/spec/helpers/facets_helper_spec.rb index e157b248c8..6a4f4dff51 100644 --- a/spec/helpers/facets_helper_spec.rb +++ b/spec/helpers/facets_helper_spec.rb @@ -107,88 +107,15 @@ describe "facet_by_field_name" do it "should retrieve the facet from the response given a string" do - facet_config = double(:query => nil, field: 'a') + facet_config = double(:query => nil, field: 'a', key: 'a') facet_field = double() allow(helper).to receive(:facet_configuration_for_field).with(anything()).and_return(facet_config) @response = double() - allow(@response).to receive(:facet_by_field_name).with('a').and_return(facet_field) + allow(@response).to receive(:aggregations).and_return('a' => facet_field) expect(helper.facet_by_field_name('a')).to eq facet_field end - - it "should also work for facet query fields" do - facet_config = double(:query => {}, key: 'a_query_facet_field') - allow(helper).to receive(:facet_configuration_for_field).with('a_query_facet_field').and_return(facet_config) - allow(helper).to receive(:create_facet_field_response_for_query_facet_field).with('a_query_facet_field', facet_config) - - helper.facet_by_field_name 'a_query_facet_field' - end - - describe "query facets" do - let(:facet_config) { - double( - key: 'my_query_facet_field', - :query => { - 'a_simple_query' => { :fq => 'field:search', :label => 'A Human Readable label'}, - 'another_query' => { :fq => 'field:different_search', :label => 'Label'}, - 'without_results' => { :fq => 'field:without_results', :label => 'No results for this facet'} - } - ) - } - - before(:each) do - allow(helper).to receive(:facet_configuration_for_field).with(anything()).and_return(facet_config) - - @response = double(:facet_queries => { - 'field:search' => 10, - 'field:different_search' => 2, - 'field:not_appearing_in_the_config' => 50, - 'field:without_results' => 0 - }) - end - - it"should convert the query facets into a double RSolr FacetField" do - field = helper.facet_by_field_name('my_query_facet_field') - expect(field).to be_a_kind_of Blacklight::SolrResponse::Facets::FacetField - - expect(field.name).to eq'my_query_facet_field' - expect(field.items.size).to eq 2 - expect(field.items.map { |x| x.value }).to_not include 'field:not_appearing_in_the_config' - - facet_item = field.items.select { |x| x.value == 'a_simple_query' }.first - - expect(facet_item.value).to eq 'a_simple_query' - expect(facet_item.hits).to eq 10 - expect(facet_item.label).to eq 'A Human Readable label' - end - end - - describe "pivot facets" do - let(:facet_config) { - double(key: 'my_pivot_facet_field', pivot: ['field_a', 'field_b']) - } - - before(:each) do - allow(helper).to receive(:facet_configuration_for_field).with(anything()).and_return(facet_config) - - @response = double(:facet_pivot => { 'field_a,field_b' => [{:field => 'field_a', :value => 'a', :count => 10, :pivot => [{:field => 'field_b', :value => 'b', :count => 2}]}]}) - end - - it "should convert the pivot facet into a double RSolr FacetField" do - field = helper.facet_by_field_name('my_pivot_facet_field') - expect(field).to be_a_kind_of Blacklight::SolrResponse::Facets::FacetField - - expect(field.name).to eq 'my_pivot_facet_field' - - expect(field.items.size).to eq 1 - - expect(field.items.first).to respond_to(:items) - - expect(field.items.first.items.size).to eq 1 - expect(field.items.first.items.first.fq).to eq({ 'field_a' => 'a' }) - end - end end diff --git a/spec/lib/blacklight/search_helper_spec.rb b/spec/lib/blacklight/search_helper_spec.rb index 2690b73f50..3b859f17f3 100644 --- a/spec/lib/blacklight/search_helper_spec.rb +++ b/spec/lib/blacklight/search_helper_spec.rb @@ -364,12 +364,12 @@ def params expect(@facets).to have_at_least(1).facet end it 'should have all facets specified in initializer' do - fields = blacklight_config.facet_fields.reject { |k,v| v.query || v.pivot } - expect(@facets.map { |f| f.name }).to match_array fields.map { |k, v| v.field } - fields.each do |key, field| - expect(@facets.find {|f| f.name == field.field}).not_to be_nil - end + 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 end + it 'should have at least one value for each facet' do @facets.each do |facet| expect(facet.items).to have_at_least(1).hit diff --git a/spec/lib/blacklight/solr_response/facets_spec.rb b/spec/lib/blacklight/solr_response/facets_spec.rb index 9eeaf96109..685ad488ea 100644 --- a/spec/lib/blacklight/solr_response/facets_spec.rb +++ b/spec/lib/blacklight/solr_response/facets_spec.rb @@ -84,4 +84,113 @@ end end end + + context "facet.missing" do + let(:response) do + { + facet_counts: { + facet_fields: { + some_field: ['a', 1, nil, 2] + } + } + } + end + + subject { Blacklight::SolrResponse.new(response, {}) } + + it "should mark the facet.missing field with a human-readable label and fq" do + missing = subject.aggregations["some_field"].items.select { |i| i.value.nil? }.first + + expect(missing.label).to eq "[Missing]" + expect(missing.fq).to eq "-some_field:[* TO *]" + end + end + + describe "query facets" do + let(:facet_config) { + double( + key: 'my_query_facet_field', + :query => { + 'a_simple_query' => { :fq => 'field:search', :label => 'A Human Readable label'}, + 'another_query' => { :fq => 'field:different_search', :label => 'Label'}, + 'without_results' => { :fq => 'field:without_results', :label => 'No results for this facet'} + } + ) + } + + let(:blacklight_config) { double(facet_fields: { 'my_query_facet_field' => facet_config } )} + + let(:response) do + { + facet_counts: { + facet_queries: { + 'field:search' => 10, + 'field:different_search' => 2, + 'field:not_appearing_in_the_config' => 50, + 'field:without_results' => 0 + } + } + } + end + + subject { Blacklight::SolrResponse.new(response, {}, blacklight_config: blacklight_config) } + + it"should convert the query facets into a double RSolr FacetField" do + field = subject.aggregations['my_query_facet_field'] + + expect(field).to be_a_kind_of Blacklight::SolrResponse::Facets::FacetField + + expect(field.name).to eq'my_query_facet_field' + expect(field.items.size).to eq 2 + expect(field.items.map { |x| x.value }).to_not include 'field:not_appearing_in_the_config' + + facet_item = field.items.select { |x| x.value == 'a_simple_query' }.first + + expect(facet_item.value).to eq 'a_simple_query' + expect(facet_item.hits).to eq 10 + expect(facet_item.label).to eq 'A Human Readable label' + end + end + + describe "pivot facets" do + let(:facet_config) { + double(key: 'my_pivot_facet_field', query: nil, pivot: ['field_a', 'field_b']) + } + + let(:blacklight_config) { double(facet_fields: { 'my_pivot_facet_field' => facet_config } )} + + let(:response) do + { + facet_counts: { + facet_pivot: { + 'field_a,field_b' => [ + { + field: 'field_a', + value: 'a', + count: 10, + pivot: [{field: 'field_b', value: 'b', count: 2}] + }] + } + } + } + end + + subject { Blacklight::SolrResponse.new(response, {}, blacklight_config: blacklight_config) } + + it "should convert the pivot facet into a double RSolr FacetField" do + field = subject.aggregations['my_pivot_facet_field'] + + expect(field).to be_a_kind_of Blacklight::SolrResponse::Facets::FacetField + + expect(field.name).to eq 'my_pivot_facet_field' + + expect(field.items.size).to eq 1 + + expect(field.items.first).to respond_to(:items) + + expect(field.items.first.items.size).to eq 1 + expect(field.items.first.items.first.fq).to eq({ 'field_a' => 'a' }) + end + end + end diff --git a/spec/lib/blacklight/solr_response_spec.rb b/spec/lib/blacklight/solr_response_spec.rb index 4bdafcff58..8ba20b1005 100644 --- a/spec/lib/blacklight/solr_response_spec.rb +++ b/spec/lib/blacklight/solr_response_spec.rb @@ -70,14 +70,6 @@ def create_response expect(r).to be_a_kind_of Kaminari::PageScopeMethods end - it "should provide a model name helper" do - first_doc_model_name = double(:human => 'xyz') - - allow(r.docs.first).to receive(:model_name).and_return first_doc_model_name - - expect(r.model_name).to eq first_doc_model_name - end - describe "FacetItem" do it "should work with a field,value tuple" do item = Blacklight::SolrResponse::Facets::FacetItem.new('value', 15) diff --git a/spec/views/catalog/_facets.html.erb_spec.rb b/spec/views/catalog/_facets.html.erb_spec.rb index 9d74f9f694..ca13745d81 100644 --- a/spec/views/catalog/_facets.html.erb_spec.rb +++ b/spec/views/catalog/_facets.html.erb_spec.rb @@ -31,7 +31,7 @@ :facet_limit_for => 10 ) @response = double() - allow(@response).to receive(:facet_by_field_name).with("facet_field_1") { @mock_display_facet_1 } + allow(@response).to receive(:aggregations).and_return("facet_field_1" => @mock_display_facet_1) end it "should have a header" do