From d3e40a7c654a1108f1c0aae10bd72949e797cdf9 Mon Sep 17 00:00:00 2001 From: Justin Coyne Date: Tue, 29 May 2018 21:20:11 -0500 Subject: [PATCH 01/15] Extract solr config dir --- tasks/blacklight.rake | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tasks/blacklight.rake b/tasks/blacklight.rake index 8dc3dd1868..343adf5515 100644 --- a/tasks/blacklight.rake +++ b/tasks/blacklight.rake @@ -10,11 +10,12 @@ require 'rubocop/rake_task' RuboCop::RakeTask.new(:rubocop) EngineCart.fingerprint_proc = EngineCart.rails_fingerprint_proc +SOLR_CONFIG_DIR = File.join(File.expand_path("..", __dir__), '.internal_test_app', 'solr', 'conf').freeze desc "Run test suite" -task :ci => ['blacklight:generate'] do +task ci: ['blacklight:generate'] do SolrWrapper.wrap do |solr| - solr.with_collection(name: 'blacklight-core', dir: File.join(File.expand_path("..", File.dirname(__FILE__)), "solr", "conf")) do + solr.with_collection(name: 'blacklight-core', dir: SOLR_CONFIG_DIR) do within_test_app do system "RAILS_ENV=test rake blacklight:index:seed" end @@ -52,8 +53,8 @@ namespace :blacklight do Rake::Task['engine_cart:generate'].invoke end - SolrWrapper.wrap(port: '8983') do |solr| - solr.with_collection(name: 'blacklight-core', dir: File.join(File.expand_path("..", File.dirname(__FILE__)), "solr", "conf")) do + SolrWrapper.wrap do |solr| + solr.with_collection(name: 'blacklight-core', dir: SOLR_CONFIG_DIR) do Rake::Task['blacklight:internal:seed'].invoke within_test_app do From 963d988bf3cbc9680d47b60d0ae9998fff0282af Mon Sep 17 00:00:00 2001 From: Justin Coyne Date: Wed, 30 May 2018 08:10:24 -0500 Subject: [PATCH 02/15] Use solr_wrapper config file for collection configuration --- tasks/blacklight.rake | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tasks/blacklight.rake b/tasks/blacklight.rake index 343adf5515..eb09e12601 100644 --- a/tasks/blacklight.rake +++ b/tasks/blacklight.rake @@ -10,12 +10,11 @@ require 'rubocop/rake_task' RuboCop::RakeTask.new(:rubocop) EngineCart.fingerprint_proc = EngineCart.rails_fingerprint_proc -SOLR_CONFIG_DIR = File.join(File.expand_path("..", __dir__), '.internal_test_app', 'solr', 'conf').freeze desc "Run test suite" task ci: ['blacklight:generate'] do SolrWrapper.wrap do |solr| - solr.with_collection(name: 'blacklight-core', dir: SOLR_CONFIG_DIR) do + solr.with_collection do within_test_app do system "RAILS_ENV=test rake blacklight:index:seed" end @@ -54,7 +53,7 @@ namespace :blacklight do end SolrWrapper.wrap do |solr| - solr.with_collection(name: 'blacklight-core', dir: SOLR_CONFIG_DIR) do + solr.with_collection do Rake::Task['blacklight:internal:seed'].invoke within_test_app do From 80fe3d328a2244f249f5e44471464f886d7fc844 Mon Sep 17 00:00:00 2001 From: David Kinzer Date: Mon, 14 May 2018 12:11:37 -0400 Subject: [PATCH 03/15] Normalize the bookmark_controller.delete method. Currently the `bookmark_controller.create` method allows the user to pass an array of bookmarks via the parameters with a format of `[{ id: foo, document_type: bar }]`. This is useful because theoretically we can bookmark records that are not of type SolrDocument. Likewise the bookmark model does not make any assumptions about document_type other than providing a default if one is not given. Contrary to these methods, the `bookmark_controller.delete` is not implemented in a consistent fashion to its counterpart. `bookmark_controller.delete` works on only one bookmark at a time and its document type can only be derived from `blacklight_config`. This is unfortunate because `blacklight_config` is copied from the `CatalogController` and the document type is already set to be `SolrDocument`. This commit updates `bookmark_controller.delete` to work in a complimentary manner to `bookmark_controller.create`. The use case of this change is to allow for being able to bookmark non SolrDocuments in an environment where there are multiple backends not just of type Solr. * This change also fixes bookmark creation via bookmarks params not working in Rails when strong parameters is enabled. --- .../concerns/blacklight/bookmarks.rb | 20 +++++++++-- spec/controllers/bookmarks_controller_spec.rb | 35 +++++++++++++++++++ 2 files changed, 52 insertions(+), 3 deletions(-) diff --git a/app/controllers/concerns/blacklight/bookmarks.rb b/app/controllers/concerns/blacklight/bookmarks.rb index e2c89cf360..e88b2fc516 100644 --- a/app/controllers/concerns/blacklight/bookmarks.rb +++ b/app/controllers/concerns/blacklight/bookmarks.rb @@ -71,7 +71,7 @@ def update # is simpler. def create @bookmarks = if params[:bookmarks] - params[:bookmarks] + permit_bookmarks[:bookmarks] else [{ document_id: params[:id], document_type: blacklight_config.document_model.to_s }] end @@ -103,9 +103,19 @@ def create # Beware, :id is the Solr document_id, not the actual Bookmark id. # idempotent, as DELETE is supposed to be. def destroy - bookmark = current_or_guest_user.bookmarks.find_by(document_id: params[:id], document_type: blacklight_config.document_model.to_s) + @bookmarks = + if params[:bookmarks] + permit_bookmarks[:bookmarks] + else + [{ document_id: params[:id], document_type: blacklight_config.document_model.to_s }] + end - if bookmark && bookmark.delete && bookmark.destroyed? + success = @bookmarks.all? do |bookmark| + bookmark = current_or_guest_user.bookmarks.find_by(bookmark) + bookmark && bookmark.delete && bookmark.destroyed? + end + + if success if request.xhr? render(json: { bookmarks: { count: current_or_guest_user.bookmarks.count }}) elsif respond_to? :redirect_back @@ -144,4 +154,8 @@ def verify_user def start_new_search_session? action_name == "index" end + + def permit_bookmarks + params.permit(bookmarks: [:document_id, :document_type]) + end end diff --git a/spec/controllers/bookmarks_controller_spec.rb b/spec/controllers/bookmarks_controller_spec.rb index 3403233405..f201b07fa1 100644 --- a/spec/controllers/bookmarks_controller_spec.rb +++ b/spec/controllers/bookmarks_controller_spec.rb @@ -25,6 +25,25 @@ expect(response.code).to eq "500" end end + + describe "create" do + it "can create bookmarks via params bookmarks attribute" do + @controller.send(:current_or_guest_user).save + put :create, xhr: true, params: { + id: "notused", + bookmarks: [ + { document_id: "2007020969", document_type: "SolrDocument" }, + { document_id: "2007020970", document_type: "SolrDocument" }, + { document_id: "2007020971", document_type: "SolrDocument" } + ], + format: :js + } + + expect(response).to be_success + expect(response.code).to eq "200" + expect(JSON.parse(response.body)["bookmarks"]["count"]).to eq 3 + end + end describe "delete" do before do @@ -39,6 +58,22 @@ expect(JSON.parse(response.body)["bookmarks"]["count"]).to eq 0 end + it "can handle bookmark deletion via :bookmarks param" do + class FooDocument < SolrDocument; end + @controller.send(:current_or_guest_user).bookmarks.create! document_id: '2007020970', document_type: "FooDocument" + delete :destroy, xhr: true, params: { + id: 'notused', + bookmarks: [ + { document_id: '2007020969', document_type: 'SolrDocument' }, + { document_id: '2007020970', document_type: 'FooDocument' } + ], + format: :js + } + expect(response).to be_success + expect(response.code).to eq "200" + expect(JSON.parse(response.body)["bookmarks"]["count"]).to eq 0 + end + it "has a 500 status code when delete is not success" do bm = instance_double(Bookmark) allow(@controller).to receive_message_chain(:current_or_guest_user, :existing_bookmark_for).and_return(bm) From 058b4f552ea9776a8cf0d8c53a25a6e37315eaf0 Mon Sep 17 00:00:00 2001 From: Justin Coyne Date: Mon, 4 Jun 2018 09:47:57 -0500 Subject: [PATCH 04/15] Move asset generating to the assets generator --- lib/generators/blacklight/assets_generator.rb | 9 ++++++++- lib/generators/blacklight/install_generator.rb | 12 ++++++------ 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/lib/generators/blacklight/assets_generator.rb b/lib/generators/blacklight/assets_generator.rb index c167aac43f..c531a3e8bd 100644 --- a/lib/generators/blacklight/assets_generator.rb +++ b/lib/generators/blacklight/assets_generator.rb @@ -2,7 +2,14 @@ module Blacklight class Assets < Rails::Generators::Base source_root File.expand_path('../templates', __FILE__) - + + # This could be skipped if you want to use webpacker + def add_javascript_dependencies + gem 'bootstrap', '~> 4.0' + gem 'popper_js' + gem 'twitter-typeahead-rails', '0.11.1.pre.corejavascript' + end + def assets copy_file "blacklight.scss", "app/assets/stylesheets/blacklight.scss" diff --git a/lib/generators/blacklight/install_generator.rb b/lib/generators/blacklight/install_generator.rb index e7fce89782..5750577143 100644 --- a/lib/generators/blacklight/install_generator.rb +++ b/lib/generators/blacklight/install_generator.rb @@ -40,12 +40,6 @@ def add_solr_wrapper generate solr_generator, generator_options end - def bundle_install - Bundler.with_clean_env do - run "bundle install" - end - end - # Copy all files in templates/public/ directory to public/ # Call external generator in AssetsGenerator, so we can # leave that callable seperately too. @@ -53,6 +47,12 @@ def copy_public_assets generate "blacklight:assets" unless options[:'skip-assets'] end + def bundle_install + Bundler.with_clean_env do + run "bundle install" + end + end + def generate_blacklight_document generate 'blacklight:document', document_name end From 6dfe4b41fb622d92255f53f749f8a654826e7fdd Mon Sep 17 00:00:00 2001 From: Justin Coyne Date: Mon, 4 Jun 2018 09:18:31 -0500 Subject: [PATCH 05/15] Don't configure assets in rails API mode This prevents a method_missing error. See #1886 --- lib/blacklight/engine.rb | 3 ++- lib/generators/blacklight/install_generator.rb | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/blacklight/engine.rb b/lib/blacklight/engine.rb index 2d3308cf69..a27281bbfe 100644 --- a/lib/blacklight/engine.rb +++ b/lib/blacklight/engine.rb @@ -28,7 +28,8 @@ class Engine < Rails::Engine end initializer "blacklight.assets.precompile" do |app| - app.config.assets.precompile += %w(favicon.ico) + # When Rails has been generated in API mode, it does not have sprockets available + app.config.assets.precompile += %w(favicon.ico) if defined? Sprockets end Blacklight::Engine.config.sms_mappings = { diff --git a/lib/generators/blacklight/install_generator.rb b/lib/generators/blacklight/install_generator.rb index 5750577143..c0e63a2f0a 100644 --- a/lib/generators/blacklight/install_generator.rb +++ b/lib/generators/blacklight/install_generator.rb @@ -12,7 +12,7 @@ class Install < Rails::Generators::Base class_option :devise , type: :boolean, default: false, aliases: "-d", desc: "Use Devise as authentication logic." class_option :jettywrapper, type: :boolean, default: false, desc: "Use jettywrapper to download and control Jetty" class_option :marc , type: :boolean, default: false, aliases: "-m", desc: "Generate MARC-based demo ." - class_option :'skip-assets', type: :boolean, default: false, desc: "Skip generating javascript and css assets into the application" + class_option :'skip-assets', type: :boolean, default: !defined?(Sprockets), desc: "Skip generating javascript and css assets into the application" class_option :'skip-solr', type: :boolean, default: false, desc: "Skip generating solr configurations." desc <<-EOS From 5751f81c160f34e0aae4d802569f930c671892ff Mon Sep 17 00:00:00 2001 From: Justin Coyne Date: Mon, 4 Jun 2018 10:45:21 -0500 Subject: [PATCH 06/15] Only use helper_method when the view layer is present In API mode, the view layer is not available and these trigger method_missing errors. This fix is similar to how devise solved this problem: https://github.com/plataformatec/devise/pull/3732 --- .rubocop_todo.yml | 2 +- app/controllers/concerns/blacklight/catalog.rb | 4 +++- app/controllers/concerns/blacklight/controller.rb | 15 +++++++++------ .../concerns/blacklight/search_context.rb | 8 +++++--- .../concerns/blacklight/token_based_user.rb | 4 +++- 5 files changed, 21 insertions(+), 12 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 2a9e283e2a..4e4ec7b83d 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -108,7 +108,7 @@ Metrics/MethodLength: # Offense count: 9 # Configuration parameters: CountComments. Metrics/ModuleLength: - Max: 208 + Max: 211 Exclude: - 'lib/blacklight/solr/search_builder_behavior.rb' diff --git a/app/controllers/concerns/blacklight/catalog.rb b/app/controllers/concerns/blacklight/catalog.rb index 06ade55038..be839dd498 100644 --- a/app/controllers/concerns/blacklight/catalog.rb +++ b/app/controllers/concerns/blacklight/catalog.rb @@ -9,7 +9,9 @@ module Blacklight::Catalog # The following code is executed when someone includes blacklight::catalog in their # own controller. included do - helper_method :sms_mappings, :has_search_parameters? + if respond_to? :helper_method + helper_method :sms_mappings, :has_search_parameters? + end helper Blacklight::Facet diff --git a/app/controllers/concerns/blacklight/controller.rb b/app/controllers/concerns/blacklight/controller.rb index cb9e4f567a..e01e8eca89 100644 --- a/app/controllers/concerns/blacklight/controller.rb +++ b/app/controllers/concerns/blacklight/controller.rb @@ -14,17 +14,20 @@ module Blacklight::Controller # now in application.rb file under config.filter_parameters # filter_parameter_logging :password, :password_confirmation - helper_method :current_user_session, :current_user, :current_or_guest_user after_action :discard_flash_if_xhr # handle basic authorization exception with #access_denied rescue_from Blacklight::Exceptions::AccessDenied, :with => :access_denied - # extra head content - helper_method :has_user_authentication_provider? - helper_method :blacklight_config, :blacklight_configuration_context - helper_method :search_action_url, :search_action_path, :search_facet_url, :search_facet_path - helper_method :search_state + if respond_to? :helper_method + helper_method :current_user_session, :current_user, :current_or_guest_user + + # extra head content + helper_method :has_user_authentication_provider? + helper_method :blacklight_config, :blacklight_configuration_context + helper_method :search_action_url, :search_action_path, :search_facet_path + helper_method :search_state + end # Specify which class to use for the search state. You can subclass SearchState if you # want to override any of the methods (e.g. SearchState#url_for_document) diff --git a/app/controllers/concerns/blacklight/search_context.rb b/app/controllers/concerns/blacklight/search_context.rb index 6d9a4d79f8..9c4ba7dd4a 100644 --- a/app/controllers/concerns/blacklight/search_context.rb +++ b/app/controllers/concerns/blacklight/search_context.rb @@ -2,10 +2,12 @@ module Blacklight::SearchContext extend ActiveSupport::Concern - # The following code is executed when someone includes blacklight::catalog::search_session in their + # The following code is executed when someone includes Blacklight::Catalog::SearchSession in their # own controller. - included do - helper_method :current_search_session, :search_session + included do + if respond_to? :helper_method + helper_method :current_search_session, :search_session + end end module ClassMethods diff --git a/app/controllers/concerns/blacklight/token_based_user.rb b/app/controllers/concerns/blacklight/token_based_user.rb index 7b73edc984..93e757ac3b 100644 --- a/app/controllers/concerns/blacklight/token_based_user.rb +++ b/app/controllers/concerns/blacklight/token_based_user.rb @@ -3,7 +3,9 @@ module Blacklight::TokenBasedUser extend ActiveSupport::Concern included do - helper_method :encrypt_user_id + if respond_to? :helper_method + helper_method :encrypt_user_id + end rescue_from Blacklight::Exceptions::ExpiredSessionToken do head :unauthorized From 00c46263704a69de48e7f0e36aea13b8bf988610 Mon Sep 17 00:00:00 2001 From: Justin Coyne Date: Mon, 4 Jun 2018 10:55:46 -0500 Subject: [PATCH 07/15] Only use helper when the view layer is present --- app/controllers/concerns/blacklight/catalog.rb | 2 +- app/controllers/concerns/blacklight/controller.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/concerns/blacklight/catalog.rb b/app/controllers/concerns/blacklight/catalog.rb index be839dd498..c83cf790c4 100644 --- a/app/controllers/concerns/blacklight/catalog.rb +++ b/app/controllers/concerns/blacklight/catalog.rb @@ -13,7 +13,7 @@ module Blacklight::Catalog helper_method :sms_mappings, :has_search_parameters? end - helper Blacklight::Facet + helper Blacklight::Facet if respond_to? :helper # When an action raises Blacklight::Exceptions::RecordNotFound, handle # the exception appropriately. diff --git a/app/controllers/concerns/blacklight/controller.rb b/app/controllers/concerns/blacklight/controller.rb index e01e8eca89..59d9e644d9 100644 --- a/app/controllers/concerns/blacklight/controller.rb +++ b/app/controllers/concerns/blacklight/controller.rb @@ -8,7 +8,7 @@ module Blacklight::Controller included do include Blacklight::SearchFields - helper Blacklight::SearchFields + helper Blacklight::SearchFields if respond_to? :helper include ActiveSupport::Callbacks From 6b6b6020410cc495f01a2f4fc9b8cddec1e451c5 Mon Sep 17 00:00:00 2001 From: Justin Coyne Date: Mon, 4 Jun 2018 11:39:22 -0500 Subject: [PATCH 08/15] Require globalid explicitly Typically globalid gets required by activejob, but a rails installation may have deactivated that gem. This ensures the `GlobalID` constant can be found --- app/models/concerns/blacklight/document.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/models/concerns/blacklight/document.rb b/app/models/concerns/blacklight/document.rb index dde66894b0..23b68df81f 100644 --- a/app/models/concerns/blacklight/document.rb +++ b/app/models/concerns/blacklight/document.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true -## + +require 'globalid' + ## # = Introduction # Blacklight::Document is the module with logic for a class representing From cb388c5db60d657c7224f322aaf64960c7de4cd5 Mon Sep 17 00:00:00 2001 From: Justin Coyne Date: Wed, 6 Jun 2018 09:58:01 -0700 Subject: [PATCH 09/15] Require jbuilder --- blacklight.gemspec | 2 +- lib/blacklight.rb | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/blacklight.gemspec b/blacklight.gemspec index e1f89c562f..46bfcd65e0 100644 --- a/blacklight.gemspec +++ b/blacklight.gemspec @@ -27,7 +27,7 @@ Gem::Specification.new do |s| s.add_dependency "rails", ">= 4.2", "< 6" s.add_dependency "globalid" - s.add_dependency "jbuilder" + s.add_dependency "jbuilder", '~> 2.7' s.add_dependency "nokogiri", "~>1.6" # XML Parser s.add_dependency "kaminari", ">= 0.15" # the pagination (page 1,2,3, etc..) of our search results s.add_dependency "rsolr", ">= 1.0.6", "< 3" # Library for interacting with rSolr. diff --git a/lib/blacklight.rb b/lib/blacklight.rb index b491a0872e..f20345f2f2 100644 --- a/lib/blacklight.rb +++ b/lib/blacklight.rb @@ -3,6 +3,7 @@ require 'deprecation' require 'blacklight/utils' require 'active_support/hash_with_indifferent_access' +require 'jbuilder' module Blacklight autoload :AbstractRepository, 'blacklight/abstract_repository' @@ -105,17 +106,16 @@ def self.logger= logger @logger = logger end - ############# + ############# # Methods for figuring out path to BL plugin, and then locate various files # either in the app itself or defaults in the plugin -- whether you are running # from the plugin itself or from an actual app using te plugin. # In a seperate module so it can be used by both Blacklight class, and - # by rake tasks without loading the whole Rails environment. + # by rake tasks without loading the whole Rails environment. ############# - + # returns the full path the the blacklight plugin installation def self.root @root ||= File.expand_path(File.dirname(File.dirname(__FILE__))) end - end From eaaa1e5bde88759d653ac1bc7cf4d6a4f8222ec4 Mon Sep 17 00:00:00 2001 From: Justin Coyne Date: Wed, 27 Jun 2018 07:58:25 -0500 Subject: [PATCH 10/15] Move suggest handling into the repository This allows the repository to abstract how suggestion handling happens --- app/models/blacklight/suggest_search.rb | 17 +++------------- lib/blacklight/solr/repository.rb | 20 ++++++++++++++++--- spec/models/blacklight/suggest_search_spec.rb | 19 ++++++------------ 3 files changed, 26 insertions(+), 30 deletions(-) diff --git a/app/models/blacklight/suggest_search.rb b/app/models/blacklight/suggest_search.rb index 4b17976837..4efcf18875 100644 --- a/app/models/blacklight/suggest_search.rb +++ b/app/models/blacklight/suggest_search.rb @@ -1,10 +1,12 @@ # frozen_string_literal: true + module Blacklight class SuggestSearch attr_reader :request_params, :repository ## # @param [Hash] params + # @param [Blacklight::AbstractRepository] repository def initialize(params, repository) @request_params = { q: params[:q] } @repository = repository @@ -15,20 +17,7 @@ def initialize(params, repository) # Blacklight::Suggest::Response # @return [Blacklight::Suggest::Response] def suggestions - Blacklight::Suggest::Response.new suggest_results, request_params, suggest_handler_path - end - - ## - # Query the suggest handler using RSolr::Client::send_and_receive - # @return [RSolr::HashWithResponse] - def suggest_results - repository.connection.send_and_receive(suggest_handler_path, params: request_params) - end - - ## - # @return [String] - def suggest_handler_path - repository.blacklight_config.autocomplete_path + repository.suggestions(request_params) end end end diff --git a/lib/blacklight/solr/repository.rb b/lib/blacklight/solr/repository.rb index be8c7d90e2..72e518c7d5 100644 --- a/lib/blacklight/solr/repository.rb +++ b/lib/blacklight/solr/repository.rb @@ -22,8 +22,16 @@ def search params = {} send_and_receive blacklight_config.solr_path, params.reverse_merge(qt: blacklight_config.qt) end + # @param [Hash] params + # @return [Blacklight::Suggest::Response] + def suggestions(request_params) + suggest_results = connection.send_and_receive(suggest_handler_path, params: request_params) + Blacklight::Suggest::Response.new suggest_results, request_params, suggest_handler_path + end + ## # Execute a solr query + # TODO: Make this private after we have a way to abstract admin/luke and ping # @see [RSolr::Client#send_and_receive] # @overload find(solr_path, params) # Execute a solr query at the given path with the parameters @@ -51,8 +59,14 @@ def send_and_receive(path, solr_params = {}) protected - def build_connection - RSolr.connect(connection_config.merge(adapter: connection_config[:http_adapter])) - end + ## + # @return [String] + def suggest_handler_path + blacklight_config.autocomplete_path + end + + def build_connection + RSolr.connect(connection_config.merge(adapter: connection_config[:http_adapter])) + end end end diff --git a/spec/models/blacklight/suggest_search_spec.rb b/spec/models/blacklight/suggest_search_spec.rb index 50d2478979..b904b49119 100644 --- a/spec/models/blacklight/suggest_search_spec.rb +++ b/spec/models/blacklight/suggest_search_spec.rb @@ -2,21 +2,14 @@ describe Blacklight::SuggestSearch do let(:params) { {q: 'test'} } - let(:suggest_path) { 'suggest' } - let(:connection) { instance_double(RSolr::Client, send_and_receive: 'sent')} - let(:repository) { instance_double(Blacklight::Solr::Repository, connection: connection) } + let(:response) { instance_double(Blacklight::Suggest::Response)} + let(:repository) { instance_double(Blacklight::Solr::Repository, suggestions: response) } let(:suggest_search) { described_class.new(params, repository)} + describe '#suggestions' do - it 'returns a Blacklight::Suggest::Response' do - expect(suggest_search).to receive(:suggest_results).and_return([]) - expect(suggest_search).to receive(:suggest_handler_path).and_return(suggest_path) - expect(suggest_search.suggestions).to be_an Blacklight::Suggest::Response - end - end - describe '#suggest_results' do - it 'calls send_and_recieve from a repository connection' do - expect(suggest_search).to receive(:suggest_handler_path).and_return(suggest_path) - expect(suggest_search.suggest_results).to eq 'sent' + it 'delegates to the repository' do + expect(repository).to receive(:suggestions).with(q: 'test').and_return(response) + expect(suggest_search.suggestions).to eq response end end end From ef95f256776cddee09c8089e05bbca7e63f50f1f Mon Sep 17 00:00:00 2001 From: Justin Coyne Date: Wed, 27 Jun 2018 10:40:05 -0500 Subject: [PATCH 11/15] Extract reflect_fields method to the repository This allows the repository to implement reflect_fields and allows us to remove a conditional. --- lib/blacklight/abstract_repository.rb | 9 +- lib/blacklight/configuration/fields.rb | 87 +++++----- lib/blacklight/solr/repository.rb | 7 + spec/models/blacklight/configuration_spec.rb | 160 ++++++++++--------- 4 files changed, 142 insertions(+), 121 deletions(-) diff --git a/lib/blacklight/abstract_repository.rb b/lib/blacklight/abstract_repository.rb index 8c76cc2b17..5c2a48af72 100644 --- a/lib/blacklight/abstract_repository.rb +++ b/lib/blacklight/abstract_repository.rb @@ -32,7 +32,14 @@ def search(params = {}) raise NotImplementedError end - protected + ## + # Query the fields that exist from the index + # @return [Hash] + def reflect_fields + raise NotImplementedError + end + + private def connection_config blacklight_config.connection_config diff --git a/lib/blacklight/configuration/fields.rb b/lib/blacklight/configuration/fields.rb index b6022b547c..11186d8efd 100644 --- a/lib/blacklight/configuration/fields.rb +++ b/lib/blacklight/configuration/fields.rb @@ -5,6 +5,8 @@ class Configuration # solr fields configuration module Fields extend ActiveSupport::Concern + extend Deprecation + self.deprecation_horizon = 'blacklight version 8.0.0' module ClassMethods # Add a configuration block for a collection of solr fields @@ -20,7 +22,7 @@ def define_field_access(key, options = {}) class #{key.camelcase} < #{base_class_name}; end END_EVAL end - + class_eval <<-END_EVAL, __FILE__, __LINE__ + 1 def add_#{key}(*args, &block) add_blacklight_field("#{key}", *args, &block) @@ -32,7 +34,7 @@ def add_#{key}(*args, &block) # Add a solr field configuration to the given configuration key # # The recommended and strongly encouraged format is a field name, configuration pair, e.g.: - # add_blacklight_field :index_field, 'format', :label => 'Format' + # add_blacklight_field :index_field, 'format', :label => 'Format' # # Alternative formats include: # @@ -56,7 +58,7 @@ def add_#{key}(*args, &block) # field.field = 'format' # field.label = 'Format' # end - # + # # * a configuration hash: # # @overload add_blacklight_field(config_key, options) @@ -64,8 +66,8 @@ def add_#{key}(*args, &block) # @param [Hash] options # # add_blacklight_field :index_field, :field => 'format', :label => 'Format' - # - # * a Field instance: + # + # * a Field instance: # # @overload add_blacklight_field(config_key, field) # @param [Symbol] config_key @@ -74,7 +76,7 @@ def add_#{key}(*args, &block) # # add_blacklight_field :index_field, IndexField.new(:field => 'format', :label => 'Format') # - # * an array of hashes: + # * an array of hashes: # # @overload add_blacklight_field(config_key, fields) # @param [Symbol] config_key @@ -102,31 +104,14 @@ def add_blacklight_field config_key, *args, &block # look up any dynamic fields if field_config.match - - salient_fields = luke_fields.select do |k,v| - k =~ field_config.match - end - - salient_fields.each do |field, luke_config| - config = field_config.dup - config.match = nil - config.field = field - config.key = field - - if self[config_key.pluralize][ config.key ] - self[config_key.pluralize][ config.key ] = config.merge(self[config_key.pluralize][ config.key ]) - else - add_blacklight_field(config_key, config, &block) - end - end - + handle_matching_fields(config_key, field_config, &block) return end - + if block_given? yield field_config end - + field_config.normalize!(self) field_config.validate! @@ -137,27 +122,47 @@ def add_blacklight_field config_key, *args, &block protected - def luke_fields - if @table[:luke_fields] == false + ## + # Using reflection into the index, add any fields in the index that match the field_config + def handle_matching_fields(config_key, field_config, &block) + salient_fields = reflected_fields.select do |k, _v| + k =~ field_config.match + end + + salient_fields.each_key do |field| + config = field_config.dup + config.match = nil + config.field = field + config.key = field + if self[config_key.pluralize][config.key] + self[config_key.pluralize][config.key] = config.merge(self[config_key.pluralize][config.key]) + else + add_blacklight_field(config_key, config, &block) + end + end + end + + def reflected_fields + if @table[:reflected_fields] == false return nil end - @table[:luke_fields] ||= Rails.cache.fetch("blacklight_configuration/admin/luke", expires_in: 1.hour) do + @table[:reflected_fields] ||= Rails.cache.fetch("blacklight_configuration/admin/reflected_fields", expires_in: 1.hour) do begin - if repository_class <= Blacklight::Solr::Repository - repository = repository_class.new(self) - repository.send_and_receive('admin/luke', params: { fl: '*', 'json.nl' => 'map' })['fields'] - end + repository = repository_class.new(self) + repository.reflect_fields rescue => e Blacklight.logger.warn "Error retrieving field metadata: #{e}" false end end - @table[:luke_fields] || {} + @table[:reflected_fields] || {} end + alias luke_fields reflected_fields + deprecation_deprecate luke_fields: 'use reflected_fields instead' - # Add a solr field by a solr field name and hash + # Add a solr field by a solr field name and hash def field_config_from_key_and_hash config_key, field_name, field_or_hash = {} field_config = field_config_from_field_or_hash(config_key, field_or_hash) field_config.key = field_name @@ -166,7 +171,7 @@ def field_config_from_key_and_hash config_key, field_name, field_or_hash = {} # Add multiple solr fields using a hash or Field instance def field_config_from_array config_key, array_of_fields_or_hashes, &block - array_of_fields_or_hashes.map do |field_or_hash| + array_of_fields_or_hashes.map do |field_or_hash| add_blacklight_field(config_key, field_or_hash, &block) end end @@ -180,16 +185,16 @@ def field_config_from_field_or_hash config_key, field_or_hash = {} # and makes it into a specific config OpenStruct, like # FacetField or SearchField. Or if the param already was # one, that's cool. Or if the param is nil, make - # an empty one. Second argument is an actual class object. + # an empty one. Second argument is an actual class object. def hash_arg_to_config(hash_arg, klass) case hash_arg - when Hash + when Hash klass.new(hash_arg) - when NilClass + when NilClass klass.new - else + else # this assumes it already is an element of klass, or acts like one, - # if not something bad will happen later, that's your problem. + # if not something bad will happen later, that's your problem. hash_arg end end diff --git a/lib/blacklight/solr/repository.rb b/lib/blacklight/solr/repository.rb index 72e518c7d5..6720fddfda 100644 --- a/lib/blacklight/solr/repository.rb +++ b/lib/blacklight/solr/repository.rb @@ -29,6 +29,13 @@ def suggestions(request_params) Blacklight::Suggest::Response.new suggest_results, request_params, suggest_handler_path end + ## + # Gets a list of available fields + # @return [Hash] + def reflect_fields + send_and_receive('admin/luke', params: { fl: '*', 'json.nl' => 'map' })['fields'] + end + ## # Execute a solr query # TODO: Make this private after we have a way to abstract admin/luke and ping diff --git a/spec/models/blacklight/configuration_spec.rb b/spec/models/blacklight/configuration_spec.rb index 3ee022ddb3..96e7d87dc4 100644 --- a/spec/models/blacklight/configuration_spec.rb +++ b/spec/models/blacklight/configuration_spec.rb @@ -1,11 +1,13 @@ # frozen_string_literal: true describe "Blacklight::Configuration" do - + before(:each) do - @config = Blacklight::Configuration.new + @config = config end + let(:config) { Blacklight::Configuration.new } + it "supports arbitrary configuration values" do @config.a = 1 @@ -78,7 +80,7 @@ it "defaults to 5" do expect(Blacklight::Configuration.new.spell_max).to eq 5 end - + it "accepts config'd value" do expect(Blacklight::Configuration.new(:spell_max => 10).spell_max).to eq 10 end @@ -148,7 +150,7 @@ config.add_my_custom_field 'qwerty', :label => "asdf" end - + expect(config.my_custom_fields.keys).to include('qwerty') end @@ -161,17 +163,17 @@ config.add_my_custom_facet_field 'qwerty', :label => "asdf" end - + expect(config.my_custom_facet_fields['qwerty']).to be_a_kind_of(Blacklight::Configuration::FacetField) end end - + describe "add_facet_field" do it "accepts field name and hash form arg" do @config.add_facet_field('format', :label => "Format", :limit => true) - + expect(@config.facet_fields["format"]).to_not be_nil expect(@config.facet_fields["format"]["label"]).to eq "Format" expect(@config.facet_fields["format"]["limit"]).to be true @@ -179,17 +181,17 @@ it "accepts FacetField obj arg" do @config.add_facet_field("format", Blacklight::Configuration::FacetField.new( :label => "Format")) - + expect(@config.facet_fields["format"]).to_not be_nil expect(@config.facet_fields["format"]["label"]).to eq "Format" end - + it "accepts field name and block form" do - @config.add_facet_field("format") do |facet| + @config.add_facet_field("format") do |facet| facet.label = "Format" facet.limit = true end - + expect(@config.facet_fields["format"]).to_not be_nil expect(@config.facet_fields["format"].limit).to be true end @@ -223,38 +225,38 @@ it "creates default label from titleized solr field" do @config.add_facet_field("publication_date") - + expect(@config.facet_fields["publication_date"].label).to eq "Publication Date" end it "allows you to not show the facet in the facet bar" do @config.add_facet_field("publication_date", :show=>false) - + expect(@config.facet_fields["publication_date"]['show']).to be false end - + it "raises on nil solr field name" do expect { @config.add_facet_field(nil) }.to raise_error ArgumentError end it "looks up and match field names" do - allow(@config).to receive_messages(luke_fields: { - "some_field_facet" => {}, + allow(config).to receive_messages(reflected_fields: { + "some_field_facet" => {}, "another_field_facet" => {}, "a_facet_field" => {}, }) - expect { |b| @config.add_facet_field match: /_facet$/, &b }.to yield_control.twice + expect { |b| config.add_facet_field match: /_facet$/, &b }.to yield_control.twice expect(@config.facet_fields.keys).to eq ["some_field_facet", "another_field_facet"] end it "takes wild-carded field names and dereference them to solr fields" do - allow(@config).to receive_messages(luke_fields: { + allow(config).to receive(:reflected_fields).and_return( "some_field_facet" => {}, "another_field_facet" => {}, - "a_facet_field" => {}, - }) - expect { |b| @config.add_facet_field "*_facet", &b }.to yield_control.twice + "a_facet_field" => {} + ) + expect { |b| config.add_facet_field "*_facet", &b }.to yield_control.twice expect(@config.facet_fields.keys).to eq ["some_field_facet", "another_field_facet"] end @@ -275,45 +277,45 @@ end end end - + describe "add_index_field" do it "takes hash form" do @config.add_index_field("title_display", :label => "Title") - + expect(@config.index_fields["title_display"]).to_not be_nil expect(@config.index_fields["title_display"].label).to eq "Title" end it "takes IndexField param" do @config.add_index_field("title_display", Blacklight::Configuration::IndexField.new(:field => "title_display", :label => "Title")) - + expect(@config.index_fields["title_display"]).to_not be_nil expect(@config.index_fields["title_display"].label).to eq "Title" end it "takes block form" do - @config.add_index_field("title_display") do |field| + @config.add_index_field("title_display") do |field| field.label = "Title" end expect(@config.index_fields["title_display"]).to_not be_nil expect(@config.index_fields["title_display"].label).to eq "Title" end - + it "creates default label from titleized field" do @config.add_index_field("title_display") - + expect(@config.index_fields["title_display"].label).to eq "Title Display" end - + it "raises on nil solr field name" do expect { @config.add_index_field(nil) }.to raise_error ArgumentError end it "takes wild-carded field names and dereference them to solr fields" do - allow(@config).to receive_messages(luke_fields: { - "some_field_display" => {}, + allow(config).to receive(:reflected_fields).and_return( + "some_field_display" => {}, "another_field_display" => {}, - "a_facet_field" => {}, - }) - @config.add_index_field "*_display" + "a_facet_field" => {} + ) + config.add_index_field "*_display" expect(@config.index_fields.keys).to eq ["some_field_display", "another_field_display"] end @@ -323,53 +325,53 @@ expect(@config.index_fields.keys).to include "subtitle_display", "subtitle_vern_display", "title_display", "title_vern_display" end end - + describe "add_show_field" do it "takes hash form" do @config.add_show_field("title_display", :label => "Title") - + expect(@config.show_fields["title_display"]).to_not be_nil expect(@config.show_fields["title_display"].label).to eq "Title" end it "takes ShowField argument" do @config.add_show_field("title_display", Blacklight::Configuration::ShowField.new(:field => "title_display", :label => "Title")) - + expect(@config.show_fields["title_display"]).to_not be_nil expect(@config.show_fields["title_display"].label).to eq "Title" end it "takes block form" do - @config.add_show_field("title_display") do |f| + @config.add_show_field("title_display") do |f| f.label = "Title" end - + expect(@config.show_fields["title_display"]).to_not be_nil expect(@config.show_fields["title_display"].label).to eq "Title" end - + it "creates default label humanized from field" do @config.add_show_field("my_custom_field") - + expect(@config.show_fields["my_custom_field"].label).to eq "My Custom Field" end - + it "raises on nil solr field name" do expect { @config.add_show_field(nil) }.to raise_error ArgumentError end - + it "takes wild-carded field names and dereference them to solr fields" do - allow(@config).to receive_messages(luke_fields: { - "some_field_display" => {}, + allow(config).to receive(:reflected_fields).and_return( + "some_field_display" => {}, "another_field_display" => {}, - "a_facet_field" => {}, - }) - @config.add_show_field "*_display" + "a_facet_field" => {} + ) + config.add_show_field "*_display" expect(@config.show_fields.keys).to eq ["some_field_display", "another_field_display"] end end - - + + describe "add_search_field" do it "accepts hash form" do c = Blacklight::Configuration.new @@ -379,55 +381,55 @@ it "accepts two-arg hash form" do c = Blacklight::Configuration.new - + c.add_search_field("my_search_type", :key => "my_search_type", - :solr_parameters => { :qf => "my_field_qf^10" }, + :solr_parameters => { :qf => "my_field_qf^10" }, :solr_local_parameters => { :pf=>"$my_field_pf"}) - + field = c.search_fields["my_search_type"] - + expect(field).to_not be_nil - - + + expect(field.solr_parameters).to_not be_nil - expect(field.solr_local_parameters).to_not be_nil - - + expect(field.solr_local_parameters).to_not be_nil + + end - + it "accepts block form" do c = Blacklight::Configuration.new - - c.add_search_field("some_field") do |field| + + c.add_search_field("some_field") do |field| field.solr_parameters = {:qf => "solr_field^10"} field.solr_local_parameters = {:pf => "$some_field_pf"} end - + f = c.search_fields["some_field"] - + expect(f).to_not be_nil expect(f.solr_parameters).to_not be_nil - expect(f.solr_local_parameters).to_not be_nil + expect(f.solr_local_parameters).to_not be_nil end - + it "accepts SearchField object" do c = Blacklight::Configuration.new - + f = Blacklight::Configuration::SearchField.new( :foo => "bar") - + c.add_search_field("foo", f) - + expect(c.search_fields["foo"]).to_not be_nil end - + it "raises on nil key" do expect {@config.add_search_field(nil, :foo => "bar")}.to raise_error ArgumentError end - + it "creates default label from titleized field key" do @config.add_search_field("author_name") - + expect(@config.search_fields["author_name"].label).to eq "Author Name" end @@ -447,7 +449,7 @@ end end end - + describe "add_sort_field" do it "takes a hash" do c = Blacklight::Configuration.new @@ -456,29 +458,29 @@ end it "takes a two-arg form with a hash" do - @config.add_sort_field("score desc, pub_date_sort desc, title_sort asc", :label => "relevance") + @config.add_sort_field("score desc, pub_date_sort desc, title_sort asc", :label => "relevance") + - expect(@config.sort_fields.values.find{|f| f.label == "relevance"}).to_not be_nil end - + it "takes a SortField object" do @config.add_sort_field(Blacklight::Configuration::SortField.new(:label => "relevance", :sort => "score desc, pub_date_sort desc, title_sort asc" -)) +)) expect(@config.sort_fields.values.find{|f| f.label == "relevance"}).to_not be_nil end - + it "takes block form" do @config.add_sort_field do |field| field.label = "relevance" field.sort = "score desc, pub_date_sort desc, title_sort asc" end - + expect(@config.sort_fields.values.find{|f| f.label == "relevance"}).to_not be_nil end end - + describe "#default_search_field" do it "uses the field with a :default key" do @config.add_search_field('search_field_1') From f0649a061cb10632840a8e0943adee3369105b65 Mon Sep 17 00:00:00 2001 From: Justin Coyne Date: Wed, 27 Jun 2018 11:34:59 -0500 Subject: [PATCH 12/15] Move the ping method into the repository This will allow different types of repositories to add their own ping handler --- lib/blacklight/abstract_repository.rb | 7 ++++- lib/blacklight/solr/repository.rb | 8 ++++++ lib/railties/blacklight.rake | 27 ++++++++----------- .../models/blacklight/solr/repository_spec.rb | 15 ++++++++--- 4 files changed, 36 insertions(+), 21 deletions(-) diff --git a/lib/blacklight/abstract_repository.rb b/lib/blacklight/abstract_repository.rb index 5c2a48af72..6efaf5cfef 100644 --- a/lib/blacklight/abstract_repository.rb +++ b/lib/blacklight/abstract_repository.rb @@ -32,13 +32,18 @@ def search(params = {}) raise NotImplementedError end - ## # Query the fields that exist from the index # @return [Hash] def reflect_fields raise NotImplementedError end + ## + # Is the repository in a working state? + def ping + raise NotImplementedError + end + private def connection_config diff --git a/lib/blacklight/solr/repository.rb b/lib/blacklight/solr/repository.rb index 6720fddfda..aa2a3c7165 100644 --- a/lib/blacklight/solr/repository.rb +++ b/lib/blacklight/solr/repository.rb @@ -36,6 +36,14 @@ def reflect_fields send_and_receive('admin/luke', params: { fl: '*', 'json.nl' => 'map' })['fields'] end + ## + # @return [boolean] true if the repository is reachable + def ping + response = connection.send_and_receive 'admin/ping', {} + Blacklight.logger.info("Ping [#{connection.uri}] returned: '#{response['status']}'") + response['status'] == "OK" + end + ## # Execute a solr query # TODO: Make this private after we have a way to abstract admin/luke and ping diff --git a/lib/railties/blacklight.rake b/lib/railties/blacklight.rake index db777c8c1c..878f5575ba 100644 --- a/lib/railties/blacklight.rake +++ b/lib/railties/blacklight.rake @@ -2,11 +2,11 @@ namespace :blacklight do # task to clean out old, unsaved searches # rake blacklight:delete_old_searches[days_old] - # example cron entry to delete searches older than 7 days at 2:00 AM every day: + # example cron entry to delete searches older than 7 days at 2:00 AM every day: # 0 2 * * * cd /path/to/your/app && /path/to/rake blacklight:delete_old_searches[7] RAILS_ENV=your_env desc "Removes entries in the searches table that are older than the number of days given." task :delete_old_searches, [:days_old] => [:environment] do |t, args| - args.with_defaults(:days_old => 7) + args.with_defaults(:days_old => 7) Search.delete_old_searches(args[:days_old].to_i) end @@ -24,24 +24,19 @@ namespace :blacklight do namespace :check do desc "Check the Solr connection and controller configuration" - task :solr, [:controller_name] => [:environment] do |_, args| - errors = 0 - verbose = ENV.fetch('VERBOSE', false).present? - - conn = Blacklight.default_index.connection - puts "[#{conn.uri}]" - - print " - admin/ping: " + task :solr, [:controller_name] => [:environment] do begin - response = conn.send_and_receive 'admin/ping', {} - puts response['status'] - errors += 1 unless response['status'] == "OK" + conn = Blacklight.default_index + if conn.ping + puts "OK" + else + puts "Unable to reach: #{conn.uri}" + exit 1 + end rescue => e - errors += 1 puts e.to_s + exit 1 end - - exit 1 if errors > 0 end task :controller, [:controller_name] => [:environment] do |_, args| diff --git a/spec/models/blacklight/solr/repository_spec.rb b/spec/models/blacklight/solr/repository_spec.rb index 4389ec9fe8..f801168338 100644 --- a/spec/models/blacklight/solr/repository_spec.rb +++ b/spec/models/blacklight/solr/repository_spec.rb @@ -1,6 +1,9 @@ # frozen_string_literal: true -describe Blacklight::Solr::Repository do +RSpec.describe Blacklight::Solr::Repository, api: true do + subject(:repository) do + described_class.new blacklight_config + end let :blacklight_config do CatalogController.blacklight_config.deep_copy @@ -42,7 +45,7 @@ allow(subject.connection).to receive(:send_and_receive).with('select', hash_including(params: { id: '123', qt: 'abc'})).and_return(mock_response) expect(subject.find("123", {qt: 'abc'})).to be_a_kind_of Blacklight::Solr::Response end - + it "should use the :qt parameter from the default_document_solr_params" do blacklight_config.default_document_solr_params[:qt] = 'abc' blacklight_config.document_solr_request_handler = 'xyz' @@ -82,12 +85,12 @@ allow(subject.connection).to receive(:send_and_receive).with('select', hash_including(params: { qt: 'abc'})).and_return(mock_response) expect(subject.search({qt: 'abc'})).to be_a_kind_of Blacklight::Solr::Response end - + it "should preserve the class of the incoming params" do search_params = ActiveSupport::HashWithIndifferentAccess.new search_params[:q] = "query" allow(subject.connection).to receive(:send_and_receive).with('select', anything).and_return(mock_response) - + response = subject.search(search_params) expect(response).to be_a_kind_of Blacklight::Solr::Response expect(response.params).to be_a_kind_of ActiveSupport::HashWithIndifferentAccess @@ -134,5 +137,9 @@ end end + describe '#ping' do + subject { repository.ping } + it { is_expected.to be true } + end end From 4107b4ca7af4e881d15160fb00d74676eb6e7194 Mon Sep 17 00:00:00 2001 From: cajahnke Date: Tue, 31 Jul 2018 09:23:13 -0500 Subject: [PATCH 13/15] Update catalog_controller.rb added optional autocomplete_controller parameter Update response.rb pass blacklight_config.suggester_name from repository Update repository.rb pass autocomplete_suggester to suggest response move uniq call to outside because response can be nil if all else fails add comment describing new param move default suggester to Blacklight::Config per Justin's suggestion add default for autocomplete_suggester adding suggester_name to test cases --- app/models/concerns/blacklight/suggest/response.rb | 8 +++++--- lib/blacklight/configuration.rb | 3 ++- lib/blacklight/solr/repository.rb | 6 +++++- .../blacklight/templates/catalog_controller.rb | 3 +++ spec/models/blacklight/suggest/response_spec.rb | 9 +++++---- 5 files changed, 20 insertions(+), 9 deletions(-) diff --git a/app/models/concerns/blacklight/suggest/response.rb b/app/models/concerns/blacklight/suggest/response.rb index 8aab43dbdd..cf320f1dbf 100644 --- a/app/models/concerns/blacklight/suggest/response.rb +++ b/app/models/concerns/blacklight/suggest/response.rb @@ -2,17 +2,19 @@ module Blacklight module Suggest class Response - attr_reader :response, :request_params, :suggest_path + attr_reader :response, :request_params, :suggest_path, :suggester_name ## # Creates a suggest response # @param [RSolr::HashWithResponse] response # @param [Hash] request_params # @param [String] suggest_path - def initialize(response, request_params, suggest_path) + # @param [String] suggester_name + def initialize(response, request_params, suggest_path, suggester_name) @response = response @request_params = request_params @suggest_path = suggest_path + @suggester_name = suggester_name end ## @@ -20,7 +22,7 @@ def initialize(response, request_params, suggest_path) # present # @return [Array] def suggestions - response.try(:[], suggest_path).try(:[], 'mySuggester').try(:[], request_params[:q]).try(:[], 'suggestions') || [] + (response.try(:[], suggest_path).try(:[], suggester_name).try(:[], request_params[:q]).try(:[], 'suggestions') || []).uniq end end end diff --git a/lib/blacklight/configuration.rb b/lib/blacklight/configuration.rb index f3d0cb8d98..5871dd1f2d 100644 --- a/lib/blacklight/configuration.rb +++ b/lib/blacklight/configuration.rb @@ -142,7 +142,8 @@ def default_values default_more_limit: 20, # proc for determining whether the session is a crawler/bot # ex.: crawler_detector: lambda { |req| req.env['HTTP_USER_AGENT'] =~ /bot/ } - crawler_detector: nil + crawler_detector: nil, + autocomplete_suggester: 'mySuggester' } end end diff --git a/lib/blacklight/solr/repository.rb b/lib/blacklight/solr/repository.rb index aa2a3c7165..56c2364a01 100644 --- a/lib/blacklight/solr/repository.rb +++ b/lib/blacklight/solr/repository.rb @@ -26,7 +26,7 @@ def search params = {} # @return [Blacklight::Suggest::Response] def suggestions(request_params) suggest_results = connection.send_and_receive(suggest_handler_path, params: request_params) - Blacklight::Suggest::Response.new suggest_results, request_params, suggest_handler_path + Blacklight::Suggest::Response.new suggest_results, request_params, suggest_handler_path, suggester_name end ## @@ -80,6 +80,10 @@ def suggest_handler_path blacklight_config.autocomplete_path end + def suggester_name + blacklight_config.autocomplete_suggester + end + def build_connection RSolr.connect(connection_config.merge(adapter: connection_config[:http_adapter])) end diff --git a/lib/generators/blacklight/templates/catalog_controller.rb b/lib/generators/blacklight/templates/catalog_controller.rb index 6f725a1515..836b98980f 100644 --- a/lib/generators/blacklight/templates/catalog_controller.rb +++ b/lib/generators/blacklight/templates/catalog_controller.rb @@ -190,5 +190,8 @@ class <%= controller_name.classify %>Controller < ApplicationController # Configuration for autocomplete suggestor config.autocomplete_enabled = true config.autocomplete_path = 'suggest' + # if the name of the solr.SuggestComponent provided in your solrcongig.xml is not the + # default 'mySuggester', uncomment and provide it below + # config.autocomplete_suggester = 'mySuggester' end end diff --git a/spec/models/blacklight/suggest/response_spec.rb b/spec/models/blacklight/suggest/response_spec.rb index 43ccc53ac2..c02e367062 100644 --- a/spec/models/blacklight/suggest/response_spec.rb +++ b/spec/models/blacklight/suggest/response_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true -describe Blacklight::Suggest::Response do - let(:empty_response) { described_class.new({}, { q: 'hello' }, 'suggest') } +RSpec.describe Blacklight::Suggest::Response, api: true do + let(:empty_response) { described_class.new({}, { q: 'hello' }, 'suggest', 'mySuggester') } let(:full_response) do described_class.new( { @@ -36,7 +36,8 @@ { q: 'new' }, - 'suggest' + 'suggest', + 'mySuggester' ) end @@ -52,4 +53,4 @@ expect(full_response.suggestions.first['term']).to eq 'new jersey' end end -end \ No newline at end of file +end From b9bd88417815878240803175d1f3bcd6919404a7 Mon Sep 17 00:00:00 2001 From: Chris Beer Date: Sun, 6 Aug 2017 08:39:34 -0700 Subject: [PATCH 14/15] Swap poltergeist for headless chrome --- .travis.yml | 4 ++++ blacklight.gemspec | 3 ++- spec/spec_helper.rb | 16 ++++++++++------ 3 files changed, 16 insertions(+), 7 deletions(-) diff --git a/.travis.yml b/.travis.yml index 341d447172..910129fd63 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,3 +1,6 @@ +dist: trusty +addons: + chrome: stable language: ruby sudo: false @@ -26,6 +29,7 @@ matrix: before_install: - gem update --system - gem install bundler + - google-chrome-stable --headless --disable-gpu --remote-debugging-port=9222 http://localhost & before_script: - if [[ "${RAILS_VERSION}" =~ ^4.2.* ]]; then perl -pi -e "s/ActiveRecord::Migration\[[\d\.]+\]/ActiveRecord::Migration/" db/migrate/*; fi diff --git a/blacklight.gemspec b/blacklight.gemspec index 46bfcd65e0..c86da90e85 100644 --- a/blacklight.gemspec +++ b/blacklight.gemspec @@ -40,7 +40,8 @@ Gem::Specification.new do |s| s.add_development_dependency "rspec-its" s.add_development_dependency "rspec-collection_matchers", ">= 1.0" s.add_development_dependency "capybara", '~> 2.6' - s.add_development_dependency "poltergeist" + s.add_development_dependency "chromedriver-helper" + s.add_development_dependency "selenium-webdriver" s.add_development_dependency 'engine_cart', '~> 1.0' s.add_development_dependency "equivalent-xml" s.add_development_dependency "coveralls" diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index ba2762be0f..a223049c33 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -25,15 +25,19 @@ require 'rspec/its' require 'rspec/collection_matchers' require 'capybara/rspec' -require 'capybara/poltergeist' +require 'selenium-webdriver' require 'equivalent-xml' -Capybara.javascript_driver = :poltergeist +Capybara.javascript_driver = :headless_chrome -Capybara.register_driver :poltergeist do |app| - options = {} - options[:timeout] = 120 if RUBY_PLATFORM == "java" - Capybara::Poltergeist::Driver.new(app, options) +Capybara.register_driver :headless_chrome do |app| + capabilities = Selenium::WebDriver::Remote::Capabilities.chrome( + chromeOptions: { args: %w[headless disable-gpu] } + ) + + Capybara::Selenium::Driver.new(app, + browser: :chrome, + desired_capabilities: capabilities) end # Requires supporting ruby files with custom matchers and macros, etc, From 949540c8bf06a5201013268079aa14965229d90d Mon Sep 17 00:00:00 2001 From: Chris Beer Date: Mon, 9 Apr 2018 13:55:56 -0700 Subject: [PATCH 15/15] Test with capybara 3 --- .travis.yml | 1 + blacklight.gemspec | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 910129fd63..6d1622c3e9 100644 --- a/.travis.yml +++ b/.travis.yml @@ -44,5 +44,6 @@ notifications: global_env: - NOKOGIRI_USE_SYSTEM_LIBRARIES=true + - ENGINE_CART_RAILS_OPTIONS='--skip-git --skip-bundle --skip-listen --skip-spring --skip-yarn --skip-keeps --skip-action-cable --skip-coffee --skip-test' jdk: oraclejdk8 diff --git a/blacklight.gemspec b/blacklight.gemspec index c86da90e85..ee12b89286 100644 --- a/blacklight.gemspec +++ b/blacklight.gemspec @@ -39,7 +39,7 @@ Gem::Specification.new do |s| s.add_development_dependency "rspec-rails", "~> 3.5" s.add_development_dependency "rspec-its" s.add_development_dependency "rspec-collection_matchers", ">= 1.0" - s.add_development_dependency "capybara", '~> 2.6' + s.add_development_dependency "capybara", '~> 3' s.add_development_dependency "chromedriver-helper" s.add_development_dependency "selenium-webdriver" s.add_development_dependency 'engine_cart', '~> 1.0'