From 6803e6e302a1b5fd8f2f0e9b8c9a926782f8ad25 Mon Sep 17 00:00:00 2001 From: Justin Coyne Date: Mon, 1 Apr 2019 10:33:32 -0500 Subject: [PATCH] Upgrade to rsolr 2 --- .rubocop_todo.yml | 63 +++++++++++------------ Gemfile | 2 +- Gemfile.lock | 5 +- app/models/hydrus/collection.rb | 4 +- app/models/hydrus/responsible.rb | 2 +- app/models/hydrus/solr_queryable.rb | 12 ++--- config/environment.rb | 3 -- config/rsolr_certificate.rb | 52 ------------------- config/rsolr_no_certificate.rb | 38 -------------- spec/models/hydrus/collection_spec.rb | 13 +++-- spec/models/hydrus/solr_queryable_spec.rb | 16 +++--- 11 files changed, 61 insertions(+), 149 deletions(-) delete mode 100644 config/rsolr_certificate.rb delete mode 100644 config/rsolr_no_certificate.rb diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index fc0f97a74..7c79b597e 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1,6 +1,6 @@ # This configuration was generated by # `rubocop --auto-gen-config` -# on 2019-04-08 11:23:59 -0500 using RuboCop version 0.58.2. +# on 2019-04-08 12:25:57 -0500 using RuboCop version 0.58.2. # The point is for the user to remove these configuration records # one by one as the offenses are removed from the code base. # Note that changes in the inspected code, or installation of new @@ -72,15 +72,14 @@ Lint/ScriptPermission: - 'Rakefile' - 'devel/experiment.rb' -# Offense count: 4 +# Offense count: 3 Lint/ShadowingOuterLocalVariable: Exclude: - 'app/models/hydrus/collection.rb' - 'app/models/hydrus/licenseable.rb' - - 'config/rsolr_certificate.rb' - 'lib/tasks/fixtures.rake' -# Offense count: 20 +# Offense count: 17 # Cop supports --auto-correct. # Configuration parameters: IgnoreEmptyBlocks, AllowUnusedKeywordArguments. Lint/UnusedBlockArgument: @@ -92,13 +91,12 @@ Lint/UnusedBlockArgument: - 'app/models/hydrus/generic_object.rb' - 'app/models/hydrus/remediation_runner.rb' - 'app/models/hydrus/responsible.rb' - - 'config/rsolr_certificate.rb' - 'lib/tasks/fixtures.rake' - 'lib/tasks/hydrus.rake' - 'spec/features/collection_edit_spec.rb' - 'spec/models/hydrus/collection_spec.rb' -# Offense count: 11 +# Offense count: 9 # Cop supports --auto-correct. # Configuration parameters: AllowUnusedKeywordArguments, IgnoreEmptyMethods. Lint/UnusedMethodArgument: @@ -108,14 +106,27 @@ Lint/UnusedMethodArgument: - 'app/models/hydrus/collection.rb' - 'app/models/hydrus/solr_queryable.rb' - 'app/uploaders/file_uploader.rb' - - 'config/rsolr_certificate.rb' - - 'config/rsolr_no_certificate.rb' - 'devel/dashboard_experiment.rb' - 'spec/helpers/application_helper_spec.rb' -# Offense count: 30 +# Offense count: 29 Lint/UselessAssignment: - Enabled: false + Exclude: + - 'app/models/hydrus/collection.rb' + - 'app/models/hydrus/contributor.rb' + - 'app/models/hydrus/responsible.rb' + - 'app/models/hydrus/solr_queryable.rb' + - 'config/compass.rb' + - 'devel/dashboard_experiment.rb' + - 'devel/experiment.rb' + - 'spec/features/collection_create_spec.rb' + - 'spec/features/collection_edit_spec.rb' + - 'spec/features/item_create_spec.rb' + - 'spec/features/item_edit_spec.rb' + - 'spec/features/models/item_spec.rb' + - 'spec/helpers/application_helper_spec.rb' + - 'spec/models/hydrus/solr_queryable_spec.rb' + - 'spec/support/helpers.rb' # Offense count: 1 # Configuration parameters: CheckForMethodsWithNoSideEffects. @@ -123,7 +134,7 @@ Lint/Void: Exclude: - 'spec/features/item_create_spec.rb' -# Offense count: 45 +# Offense count: 43 Metrics/AbcSize: Max: 119 @@ -142,7 +153,7 @@ Metrics/ClassLength: Metrics/CyclomaticComplexity: Max: 22 -# Offense count: 61 +# Offense count: 59 # Configuration parameters: CountComments. Metrics/MethodLength: Max: 82 @@ -307,7 +318,7 @@ Style/BarePercentLiterals: - 'spec/models/hydrus/role_metadata_ds_spec.rb' - 'spec/models/hydrus/solr_queryable_spec.rb' -# Offense count: 69 +# Offense count: 68 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle, ProceduralMethods, FunctionalMethods, IgnoredMethods. # SupportedStyles: line_count_based, semantic, braces_for_chaining @@ -331,7 +342,7 @@ Style/BracesAroundHashParameters: - 'spec/lib/is_druid_validator_spec.rb' - 'spec/models/hydrus/item_spec.rb' -# Offense count: 34 +# Offense count: 31 # Cop supports --auto-correct. # Configuration parameters: AutoCorrect, EnforcedStyle. # SupportedStyles: nested, compact @@ -396,7 +407,7 @@ Style/DefWithParentheses: - 'app/models/hy_time.rb' - 'devel/list_all_hydrus_objects.rb' -# Offense count: 72 +# Offense count: 70 Style/Documentation: Enabled: false @@ -444,7 +455,7 @@ Style/ExpandPathArguments: - 'script/rails' - 'spec/spec_helper.rb' -# Offense count: 171 +# Offense count: 169 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle. # SupportedStyles: when_needed, always, never @@ -531,7 +542,7 @@ Style/LineEndConcatenation: Style/MethodCallWithoutArgsParentheses: Enabled: false -# Offense count: 22 +# Offense count: 20 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle. # SupportedStyles: require_parentheses, require_no_parentheses, require_no_parentheses_except_multiline @@ -542,8 +553,6 @@ Style/MethodDefParentheses: - 'app/models/hydrus/generic_object.rb' - 'app/models/hydrus/item.rb' - 'app/models/hydrus/responsible.rb' - - 'config/rsolr_certificate.rb' - - 'config/rsolr_no_certificate.rb' # Offense count: 1 # Cop supports --auto-correct. @@ -551,14 +560,13 @@ Style/MultilineIfModifier: Exclude: - 'devel/create_test_item.rb' -# Offense count: 6 +# Offense count: 5 Style/MultilineTernaryOperator: Exclude: - 'app/models/hydrus/cant.rb' - 'app/models/hydrus/generic_ds.rb' - 'app/models/hydrus/item.rb' - 'app/models/hydrus/responsible.rb' - - 'config/rsolr_no_certificate.rb' # Offense count: 1 Style/MultipleComparison: @@ -659,7 +667,7 @@ Style/PreferredHashMethods: - 'app/helpers/hydrus_form_helper.rb' - 'devel/create_test_item.rb' -# Offense count: 5 +# Offense count: 3 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle. # SupportedStyles: compact, exploded @@ -668,7 +676,6 @@ Style/RaiseArgs: - 'app/controllers/datastreams_controller.rb' - 'app/controllers/events_controller.rb' - 'app/controllers/hydrus_items_controller.rb' - - 'config/rsolr_no_certificate.rb' # Offense count: 1 # Cop supports --auto-correct. @@ -734,14 +741,6 @@ Style/SafeNavigation: - 'app/models/hydrus/responsible.rb' - 'devel/create_test_item.rb' -# Offense count: 2 -# Cop supports --auto-correct. -# Configuration parameters: EnforcedStyle. -# SupportedStyles: use_perl_names, use_english_names -Style/SpecialGlobalVars: - Exclude: - - 'config/rsolr_no_certificate.rb' - # Offense count: 2 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle. @@ -855,7 +854,7 @@ Style/WordArray: Style/ZeroLengthPredicate: Enabled: false -# Offense count: 944 +# Offense count: 935 # Configuration parameters: AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, IgnoredPatterns. # URISchemes: http, https Metrics/LineLength: diff --git a/Gemfile b/Gemfile index afc3939c7..71bc22869 100644 --- a/Gemfile +++ b/Gemfile @@ -65,4 +65,4 @@ end gem 'honeybadger' gem 'rsolr-ext' -gem 'rsolr', '~> 1.1' +gem 'rsolr', '~> 2.2' diff --git a/Gemfile.lock b/Gemfile.lock index 3c0ec9c1f..80523de85 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -403,8 +403,9 @@ GEM mime-types (>= 1.16, < 4.0) netrc (~> 0.8) retries (0.0.5) - rsolr (1.1.2) + rsolr (2.2.1) builder (>= 2.1.2) + faraday (>= 0.9.0) rsolr-ext (1.0.3) rsolr (>= 1.0.2) rspec-core (3.8.0) @@ -555,7 +556,7 @@ DEPENDENCIES okcomputer pry rails (~> 4.2.9) - rsolr (~> 1.1) + rsolr (~> 2.2) rsolr-ext rspec-rails (~> 3.1) rubocop (~> 0.58.1) diff --git a/app/models/hydrus/collection.rb b/app/models/hydrus/collection.rb index 898233021..b98578e81 100644 --- a/app/models/hydrus/collection.rb +++ b/app/models/hydrus/collection.rb @@ -686,11 +686,11 @@ def self.item_counts_of_collections(coll_pids) counts end - # Takes a SOLR response. + # @param [RSolr::HashWithResponse] # Returns an array of hashes containing the needed facet counts. # Written as a separate method for testing purposes. def self.get_facet_counts_from_response(resp) - resp.facet_counts['facet_pivot'].values.first + resp.fetch('facet_counts').fetch('facet_pivot').values.first end # Returns an array-of-arrays containing the collection's @item_counts diff --git a/app/models/hydrus/responsible.rb b/app/models/hydrus/responsible.rb index 3afddad29..55afebe51 100644 --- a/app/models/hydrus/responsible.rb +++ b/app/models/hydrus/responsible.rb @@ -65,7 +65,7 @@ def self.roles_of_person person_id, apo_id h[:fl] = '*' resp, sdocs = Hydrus::SolrQueryable.issue_solr_query(h) # only 1 doc - doc = resp.docs.first + doc = resp.fetch('response').fetch('docs').first roles.keys.each do |key| # the solr field is based on the role name, but doesnt match it precisely field_name = key.gsub('hydrus-', '').gsub('-', '_') + '_person_identifier_t' diff --git a/app/models/hydrus/solr_queryable.rb b/app/models/hydrus/solr_queryable.rb index 2dfa9bcf2..25935506d 100644 --- a/app/models/hydrus/solr_queryable.rb +++ b/app/models/hydrus/solr_queryable.rb @@ -70,8 +70,8 @@ def issue_solr_query(*args) # This static version was added specifically to deal with loading the dashboard without instantiating an object. def self.issue_solr_query(h) - solr_response = solr.find(h) - document_list = solr_response.docs.map { |doc| SolrDocument.new(doc, solr_response) } + solr_response = solr.post('select', data: h) + document_list = solr_response.fetch('response').fetch('docs').map { |doc| SolrDocument.new(doc, solr_response) } [solr_response, document_list] end @@ -187,11 +187,11 @@ def all_hydrus_objects(opts = {}) data end - # Takes a SOLR response. - # Returns an array of druids corresponding to the documents. + # @param [RSolr::HashWithResponse] + # @return [Array] an array of druids corresponding to the documents. def get_druids_from_response(resp) k = 'objectId_ssim' - resp.docs.map { |doc| doc[k].first } + resp.fetch('response').fetch('docs').map { |doc| doc[k].first } end # Takes a SOLR response and a hash of field remappings. @@ -203,7 +203,7 @@ def get_druids_from_response(resp) # When retrieving values from the SOLR documents, only the first # values for each key is retained. def get_fields_from_response(resp, fields) - resp.docs.map { |doc| + resp.fetch('response').fetch('docs').map { |doc| h = {} fields.each { |solr_doc_key, remapped_key| d = doc[solr_doc_key] diff --git a/config/environment.rb b/config/environment.rb index 90c8c1767..d5a766dcd 100644 --- a/config/environment.rb +++ b/config/environment.rb @@ -3,9 +3,6 @@ current_path = File.dirname(__FILE__) -# Override make_solr_connection() so that we use POST for solr queries -require File.expand_path(File.join(current_path, 'rsolr_no_certificate')) - Hydrus::Application.configure do # this is the path from the root of the public folder into which file uploads will be stored config.file_upload_path = 'uploads' diff --git a/config/rsolr_certificate.rb b/config/rsolr_certificate.rb deleted file mode 100644 index b83164f4a..000000000 --- a/config/rsolr_certificate.rb +++ /dev/null @@ -1,52 +0,0 @@ -require 'rsolr/client_cert' -require 'solrizer-fedora' - -# Force Blacklight to use RSolr::ClientCert::Connection -Rails.logger.warn 'Monkey-patching Blacklight.solr to use RSolr::ClientCert::Connection' -begin - Blacklight.instance_variable_set(:@solr, Dor::Config.make_solr_connection(Blacklight.solr_config)) -end - -# Monkey patch Solrizer::Fedora::Indexer to use RSolr::ClientCert::Connection -# Rails.logger.warn "Monkey-patching Solrizer::Fedora::Indexer to use RSolr::ClientCert::Connection" -class Solrizer::Fedora::Indexer - def connect - @solr = Dor::Config.make_solr_connection - end -end -class RSolr::ClientCert::Connection - def execute client, request_context - old_proxy = RestClient.proxy - begin - RestClient.proxy = request_context[:proxy] - resource = RestClient::Resource.new( - request_context[:uri].to_s, - open_timeout: request_context[:open_timeout], - timeout: request_context[:read_timeout], - ssl_client_cert: ssl_client_cert, - ssl_client_key: ssl_client_key - ) - result = {} - if request_context[:method] == :get - response = resource.post request_context[:data], {} - result = { - status: response.net_http_res.code.to_i, - headers: response.net_http_res.to_hash, - body: response.net_http_res.body - } - else - signature = [request_context[:method], request_context[:data], request_context[:headers]].compact - resource.send(*signature) { |response, request, http_result, &block| - result = { - status: response.net_http_res.code.to_i, - headers: response.net_http_res.to_hash, - body: response.net_http_res.body - } - } - end - result - ensure - RestClient.proxy = old_proxy - end - end -end diff --git a/config/rsolr_no_certificate.rb b/config/rsolr_no_certificate.rb deleted file mode 100644 index 0de34899f..000000000 --- a/config/rsolr_no_certificate.rb +++ /dev/null @@ -1,38 +0,0 @@ -class RSolr::Connection - def execute client, request_context - if request_context[:method] != :post - # if this is not a POST, take all of the parameters in the query string and put them into a POST body to avoid queryies with the query string being too long - uri = URI.parse(request_context[:uri].to_s) - body = uri.query - uri.query = nil - request_context[:uri] = uri - body = Rack::Utils.parse_query(body) - # move qt to post body - - h = http request_context[:uri], request_context[:proxy], request_context[:read_timeout], request_context[:open_timeout] - - request_context[:method] = :post - request = setup_raw_request request_context - request.set_form_data(body) - else - - h = http request_context[:uri], request_context[:proxy], request_context[:read_timeout], request_context[:open_timeout] - - request = setup_raw_request request_context - - request.body = request_context[:data] if request_context[:method] == :post && request_context[:data] - end - begin - response = h.request request - charset = response.type_params['charset'] - { status: response.code.to_i, headers: response.to_hash, body: force_charset(response.body, charset) } - rescue Errno::ECONNREFUSED => e - raise(Errno::ECONNREFUSED.new(request_context.inspect)) - # catch the undefined closed? exception -- this is a confirmed ruby bug - rescue NoMethodError - $!.message == "undefined method `closed?' for nil:NilClass" ? - raise(Errno::ECONNREFUSED.new) : - raise($!) - end - end -end diff --git a/spec/models/hydrus/collection_spec.rb b/spec/models/hydrus/collection_spec.rb index fd21564b0..5b6899feb 100644 --- a/spec/models/hydrus/collection_spec.rb +++ b/spec/models/hydrus/collection_spec.rb @@ -472,11 +472,14 @@ expect(@HC.all_hydrus_collections).to eq(exp) end - it 'can exercise get_facet_counts_from_response()' do - exp = 1234 - fcs = { 'facet_pivot' => { a: exp } } - resp = double('resp', facet_counts: fcs) - expect(@HC.get_facet_counts_from_response(resp)).to eq(exp) + describe '#get_facet_counts_from_response' do + let(:resp_hash) { { 'facet_pivot' => { a: exp } } } + let(:resp) { instance_double(RSolr::HashWithResponse, fetch: resp_hash) } + let(:exp) { 1234 } + + it 'returns the document identifiers' do + expect(@HC.get_facet_counts_from_response(resp)).to eq(exp) + end end it 'can exercise item_counts_with_labels()' do diff --git a/spec/models/hydrus/solr_queryable_spec.rb b/spec/models/hydrus/solr_queryable_spec.rb index 08c3406fe..826861b0c 100644 --- a/spec/models/hydrus/solr_queryable_spec.rb +++ b/spec/models/hydrus/solr_queryable_spec.rb @@ -102,12 +102,14 @@ expect(Set.new(h.keys)).to eq(Set.new([:rows, :fl, :q, :fq])) end - it 'can exercise get_druids_from_response()' do - k = 'objectId_ssim' - exp = [12, 34, 56] - docs = exp.map { |n| { k => [n] } } - resp = double('resp', docs: docs) - expect(instance.get_druids_from_response(resp)).to eq(exp) + describe '#get_druids_from_response' do + let(:resp_hash) { { 'docs' => exp.map { |n| { 'objectId_ssim' => [n] } } } } + let(:resp) { instance_double(RSolr::HashWithResponse, fetch: resp_hash) } + let(:exp) { [12, 34, 56] } + + it 'returns the document identifiers' do + expect(instance.get_druids_from_response(resp)).to eq(exp) + end end # Note: These are integration tests. @@ -156,7 +158,7 @@ fake_pids << 'fake_pid' end h = instance.squery_item_counts_of_collections(fake_pids) - # this raises an exception due to receiving a 413 error from solr unless the parameters are posted + # this raises a RSolr::Error::Http exception due to receiving a 414 error from solr unless the parameters are posted expect { resp, sdocs = instance.issue_solr_query(h) }.not_to raise_error end end