Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom

🐛 Restore qt: 'standard' to SolrService #6677

Merged
merged 4 commits into from
Feb 8, 2024
Merged

🐛 Restore qt: 'standard' to SolrService #6677

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
78abeaa
🐛 Restore `qt: 'standard'` to SolrService
jeremyf Feb 7, 2024
67b872e
♻️ Attempt to squash a flaky test
jeremyf Feb 6, 2024
9f41ea2
Merge branch 'main' into fix-hyrax-solr-service
jeremyf Feb 7, 2024
0fed6db
Adding configurability to Hyrax::SolrService
jeremyf Feb 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 18 additions & 4 deletions app/services/hyrax/solr_service.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,22 @@ module Hyrax
class SolrService
COMMIT_PARAMS = { softCommit: true }.freeze

##
# @!group Class Attributes
#
# @!attribute additional_solr_get_and_post_params
#
# ActiveFedora::SolrService always assumed that it would pass `qt: 'standard'`. However, as
# Valkyrie arrived, we have a case where we would not pass the `qt: standard`. We're favoring
# a default value to preserve prior expectations that ActiveFedora::SolrService set and to not
# suddenly break those that switch to Valkyrie and Hyrax::SolrService.
#
# @return [Hash<Symbol, String>]
# @see https://github.com/samvera/hyrax/pull/6677
class_attribute :additional_solr_get_and_post_params, default: { qt: 'standard' }
# @!endgroup Class Attributes
##

##
# @!attribute [r] use_valkyrie
# @private
Expand Down Expand Up @@ -46,9 +62,8 @@ def select_path
def get(query = nil, **args)
# Make Hyrax.config.solr_select_path the default SOLR path
solr_path = args.delete(:path) || Hyrax.config.solr_select_path
args = args.merge(q: query) if query.present?
args = args.merge(additional_solr_get_and_post_params.merge(q: query)) if query.present?

args = args.merge(qt: 'standard') unless query.blank? || use_valkyrie
connection.get(solr_path, params: args)
end

Expand All @@ -66,9 +81,8 @@ def ping
def post(query = nil, **args)
# Make Hyrax.config.solr_select_path the default SOLR path
solr_path = args.delete(:path) || Hyrax.config.solr_select_path
args = args.merge(q: query) if query.present?
args = args.merge(additional_solr_get_and_post_params.merge(q: query)) if query.present?

args = args.merge(qt: 'standard') unless query.blank? || use_valkyrie
connection.post(solr_path, data: args)
end

Expand Down
6 changes: 2 additions & 4 deletions spec/features/default_workflow_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
let(:attributes) { :LEGACY_UNUSED_ARGUMENT_WITH_NO_KNOWN_USE_CASE_SHOULD_NEVER_BE_REQUIRED }
let(:workflow_factory) { Hyrax::Workflow::WorkflowFactory }

before { Hyrax::EnsureWellFormedAdminSetService.call }

it 'sets state to "deposited"' do
workflow_factory.create(work, attributes, depositor)

Expand All @@ -34,9 +36,5 @@
.to include(have_attributes(mode: :edit, agent: depositor.user_key))
end
end

context 'with members' do
it 'grants edit to depositor on members in background'
end
end
end
36 changes: 13 additions & 23 deletions spec/services/hyrax/solr_service_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,29 +15,19 @@
describe "#get" do
it "calls solr" do
stub_result = double("Result")
params = use_valkyrie ? { q: 'querytext' } : { q: 'querytext', qt: 'standard' }
params = { q: 'querytext', qt: 'standard' }
expect(mock_conn).to receive(:get).with('select', params: params).and_return(stub_result)
allow(described_class).to receive(:instance).and_return(double("instance", conn: mock_conn))
expect(described_class.get('querytext')).to eq stub_result
end

it "uses args as params" do
stub_result = double("Result")
params = use_valkyrie ? { fq: ["id:\"1234\""], q: 'querytext' } : { fq: ["id:\"1234\""], q: 'querytext', qt: 'standard' }
params = { fq: ["id:\"1234\""], q: 'querytext', qt: 'standard' }
expect(mock_conn).to receive(:get).with('select', params: params).and_return(stub_result)
allow(described_class).to receive(:instance).and_return(double("instance", conn: mock_conn))
expect(described_class.get('querytext', fq: ["id:\"1234\""])).to eq stub_result
end

context "when use_valkyrie: true" do
subject(:service) { described_class.new(use_valkyrie: true) }

it "uses valkyrie solr based on config query_index_from_valkyrie" do
stub_result = double("Valkyrie Result")
expect(mock_conn).to receive(:get).with('select', params: { q: 'querytext' }).and_return(stub_result)
expect(service.get('querytext')).to eq stub_result
end
end
end

describe '#ping' do
Expand All @@ -64,15 +54,15 @@
describe "#post" do
it "calls solr" do
stub_result = double("Result")
data = use_valkyrie ? { q: 'querytext' } : { q: 'querytext', qt: 'standard' }
data = { q: 'querytext', qt: 'standard' }
expect(mock_conn).to receive(:post).with('select', data: data).and_return(stub_result)
allow(described_class).to receive(:instance).and_return(double("instance", conn: mock_conn))
expect(described_class.post('querytext')).to eq stub_result
end

it "uses args as data" do
stub_result = double("Result")
data = use_valkyrie ? { fq: ["id:\"1234\""], q: 'querytext' } : { fq: ["id:\"1234\""], q: 'querytext', qt: 'standard' }
data = { fq: ["id:\"1234\""], q: 'querytext', qt: 'standard' }
expect(mock_conn).to receive(:post).with('select', data: data).and_return(stub_result)
allow(described_class).to receive(:instance).and_return(double("instance", conn: mock_conn))
expect(described_class.post('querytext', fq: ["id:\"1234\""])).to eq stub_result
Expand All @@ -84,7 +74,7 @@
it "uses valkyrie solr based on config query_index_from_valkyrie" do
stub_result = double("Valkyrie Result")

expect(mock_conn).to receive(:post).with('select', data: { q: 'querytext' }).and_return(stub_result)
expect(mock_conn).to receive(:post).with('select', data: { q: 'querytext', qt: 'standard' }).and_return(stub_result)

expect(service.post('querytext')).to eq stub_result
end
Expand All @@ -101,33 +91,33 @@
end

it "defaults to HTTP POST method" do
data = use_valkyrie ? { rows: 2, q: 'querytext' } : { rows: 2, q: 'querytext', qt: 'standard' }
data = { rows: 2, q: 'querytext', qt: 'standard' }
expect(mock_conn).to receive(:post).with('select', data: data).and_return(stub_result)
described_class.query('querytext', rows: 2)
end

it "allows callers to specify HTTP GET method" do
params = use_valkyrie ? { rows: 2, q: 'querytext' } : { rows: 2, q: 'querytext', qt: 'standard' }
params = { rows: 2, q: 'querytext', qt: 'standard' }
expect(mock_conn).to receive(:get).with('select', params: params).and_return(stub_result)
described_class.query('querytext', rows: 2, method: :get)
end

it "allows callers to specify HTTP POST method" do
data = use_valkyrie ? { rows: 2, q: 'querytext' } : { rows: 2, q: 'querytext', qt: 'standard' }
data = { rows: 2, q: 'querytext', qt: 'standard' }
expect(mock_conn).to receive(:post).with('select', data: data).and_return(stub_result)
described_class.query('querytext', rows: 2, method: :post)
end

it "raises if method not GET or POST" do
data = use_valkyrie ? { rows: 2, q: 'querytext' } : { rows: 2, q: 'querytext', qt: 'standard' }
data = { rows: 2, q: 'querytext', qt: 'standard' }
expect(mock_conn).not_to receive(:head).with('select', data: data)
expect do
described_class.query('querytext', rows: 2, method: :head)
end.to raise_error(RuntimeError, "Unsupported HTTP method for querying SolrService (:head)")
end

it "wraps the solr response documents in Solr hits" do
data = use_valkyrie ? { rows: 2, q: 'querytext' } : { rows: 2, q: 'querytext', qt: 'standard' }
data = { rows: 2, q: 'querytext', qt: 'standard' }
expect(mock_conn).to receive(:post).with('select', data: data).and_return(stub_result)
result = described_class.query('querytext', rows: 2)
expect(result.size).to eq 1
Expand All @@ -146,7 +136,7 @@
let(:doc) { { 'id' => 'valkyrie-x' } }

it "uses valkyrie solr based on config query_index_from_valkyrie" do
expect(mock_conn).to receive(:post).with('select', data: { q: 'querytext' }).and_return(stub_result)
expect(mock_conn).to receive(:post).with('select', data: { q: 'querytext', qt: 'standard' }).and_return(stub_result)

result = service.query('querytext')
expect(result.first.id).to eq 'valkyrie-x'
Expand Down Expand Up @@ -268,7 +258,7 @@
let(:stub_result) { { 'response' => { 'numFound' => '2' } } }

it "calls solr" do
data = use_valkyrie ? { rows: 0, q: 'querytext' } : { rows: 0, q: 'querytext', qt: 'standard' }
data = { rows: 0, q: 'querytext', qt: 'standard' }
expect(mock_conn).to receive(:post).with('select', data: data).and_return(stub_result)
allow(described_class).to receive(:instance).and_return(double("instance", conn: mock_conn))
expect(described_class.count('querytext')).to eq 2
Expand All @@ -278,7 +268,7 @@
subject(:service) { described_class.new(use_valkyrie: true) }

it "uses valkyrie solr based on config query_index_from_valkyrie" do
expect(mock_conn).to receive(:post).with('select', data: { rows: 0, q: 'querytext' }).and_return(stub_result)
expect(mock_conn).to receive(:post).with('select', data: { rows: 0, q: 'querytext', qt: 'standard' }).and_return(stub_result)
expect(service.count('querytext')).to eq 2
end
end
Expand Down