Skip to content

Commit

Permalink
🐛 Restore qt: 'standard' to SolrService
Browse files Browse the repository at this point in the history
In `ActiveFedora::SolrService` we always assign the `qt: 'standard'`.[1]

However, with [this old commit][1] we dropped passing the `qt:
'standard'` when we had `use_valkyrie` of true.

We discovered this bug when upgrading Hyku and Bulkrax to Hyrax v3.x and
some.

Closes #6676

Related to:

- #3857

[1]:https://github.com/samvera/active_fedora/blob/8e7d365a087974b4ff9b9217f792c0c049789de6/lib/active_fedora/solr_service.rb#L40-L48
[2]:be6104f
  • Loading branch information
jeremyf committed Feb 7, 2024
1 parent dcb655d commit 8c3f32d
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 27 deletions.
6 changes: 2 additions & 4 deletions app/services/hyrax/solr_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,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(q: query, qt: 'standard') if query.present?

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

Expand All @@ -66,9 +65,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(q: query, qt: 'standard') if query.present?

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

Expand Down
36 changes: 13 additions & 23 deletions spec/services/hyrax/solr_service_spec.rb
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

0 comments on commit 8c3f32d

Please sign in to comment.