Skip to content

Commit e1adfe2

Browse files
authored
Merge pull request #187 from mlibrary/LIBSEARCH-1172-query-parser-handles-all-inputs
Libsearch 1172 query parser handles all inputs
2 parents 0ed088c + ce0a887 commit e1adfe2

File tree

6 files changed

+228
-25
lines changed

6 files changed

+228
-25
lines changed

.github/workflows/tests.yml

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,13 @@
11
name: Run Tests
2-
32
on: push
4-
53
jobs:
64
# Run tests
75
test-ruby:
86
runs-on: ubuntu-latest
9-
env:
7+
env:
108
BUNDLE_GEMFILE: ${{ github.workspace }}/umich_catalog_indexing/Gemfile
119
steps:
12-
- uses: actions/checkout@v3
10+
- uses: actions/checkout@v6
1311
- name: Set up ruby 3.2
1412
uses: ruby/setup-ruby@v1
1513
with:
@@ -29,11 +27,10 @@ jobs:
2927
- name: Run tests
3028
working-directory: ./umich_catalog_indexing
3129
run: bundle exec rspec
32-
3330
test-python:
3431
runs-on: ubuntu-latest
3532
steps:
36-
- uses: actions/checkout@v5
33+
- uses: actions/checkout@v6
3734
- name: Create .env file
3835
working-directory: ./api
3936
run: cat env.* > .env
@@ -48,7 +45,7 @@ jobs:
4845
with:
4946
python-version: '3.11'
5047
cache: 'poetry'
51-
- name: install poetry packages
48+
- name: install poetry packages
5249
working-directory: ./api
5350
run: poetry install --no-root
5451
- name: Linting
@@ -58,4 +55,18 @@ jobs:
5855
env:
5956
CI: "true"
6057
working-directory: ./api
61-
run: poetry run pytest
58+
run: poetry run pytest
59+
test-parser:
60+
runs-on: ubuntu-latest
61+
env:
62+
BUNDLE_GEMFILE: ${{ github.workspace }}/search_parser/Gemfile
63+
steps:
64+
- uses: actions/checkout@v6
65+
- name: Set up ruby 4.0
66+
uses: ruby/setup-ruby@v1
67+
with:
68+
ruby-version: "4.0"
69+
bundler-cache: true
70+
- name: Run tests
71+
working-directory: ./search_parser
72+
run: bundle exec rspec

search_parser/.rspec

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
--require spec_helper

search_parser/app.rb

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2,31 +2,28 @@
22
require "sinatra/namespace"
33
require "puma"
44
require "mlibrary_search_parser"
5-
require_relative "lib/services"
65
require "yaml"
7-
require "debug" if S.app_env == "development"
86
require "active_support"
97
require "active_support/core_ext/hash/indifferent_access"
8+
require_relative "lib/services"
9+
require "debug" if S.app_env == "development"
1010

1111
module SearchParser
12-
CATALOG_CONFIG = YAML.safe_load_file("./config/catalog.yaml", aliases: true)
12+
CATALOG_CONFIG = YAML.safe_load_file("./config/catalog.yaml", aliases: true).freeze
1313
CATALOG_BUILDER = MLibrarySearchParser::SearchBuilder.new(CATALOG_CONFIG)
14-
FACETS = CATALOG_CONFIG["facets"]
15-
16-
def self.catalog_config
17-
CATALOG_CONFIG
18-
end
14+
FACETS = CATALOG_CONFIG["facets"].freeze
1915

2016
def self.build(query)
2117
CATALOG_BUILDER.build(query)
2218
end
2319

24-
def self.misc
25-
{}
20+
def self.facets
21+
FACETS
2622
end
2723

2824
def self.facet_params
2925
result = {}
26+
3027
FACETS.map do |field|
3128
result["f.#{field}.facet.limit"] = "50"
3229
result["f.#{field}.facet.mincount"] = "1"
@@ -53,19 +50,21 @@ def self.solr_escape(str)
5350
str.gsub(/([+\-&|!(){}\[\]\^"~*?:\\\/])/, '\\\\\1')
5451
end
5552

56-
def self.solr_query(query:, rows: 10, start: 0)
53+
def self.solr_query(query:, rows:, start:, sort:, fq:)
5754
# how to handle fq:
5855
# fq":["topicStr:(Motion\\ pictures)","institution:(UM\\ Ann\\ Arbor\\ Libraries)","+(new_availability:physical OR new_availability:hathi_trust_full_text_or_electronic_holding)"]
5956
# sort comes from config/sorts.yml
6057
lp = MLibrarySearchParser::Transformer::Solr::LocalParams.new(build(query))
58+
6159
result = {
6260
rows: rows,
63-
start: start
61+
start: start,
62+
sort: sort,
63+
fq: fq
6464
}.merge(facet_params).merge(lp.params).with_indifferent_access
65+
6566
result["qt"] = "standard" unless ["edismax", "dismax"].include?(result["qt"]) # code from spectrum so I don't forget. Don't know if we need this.
6667
result["qq"] = '"' + solr_escape(result["q"]) + '"'
67-
result["sort"] = "score desc"
68-
result["fq"] = ["institution:(UM\\ Ann\\ Arbor\\ Libraries)", "+(new_availability:physical OR new_availability:hathi_trust_full_text_or_electronic_holding)"]
6968

7069
result
7170
end
@@ -76,12 +75,13 @@ class SearchParser::Application < Sinatra::Base
7675
set :host_authorization, {permitted_hosts: []}
7776
namespace "/catalog" do
7877
get "/search" do
79-
# headers "Access-Control-Allow-Origin" => "*"
8078
content_type :json
8179
query_params = {
8280
query: params["query"] || "",
8381
rows: params["rows"] || 10,
84-
start: params["start"] || 0
82+
start: params["start"] || 0,
83+
sort: params["sort"] || "score desc",
84+
fq: params["fq"] || ["institution:(UM\\ Ann\\ Arbor\\ Libraries)", "+(availability:physical OR availability:hathi_trust_full_text_or_electronic_holding)"]
8585
}
8686
S.solr_conn.get("solr/#{S.solr_core}/select", SearchParser.solr_query(**query_params)).body.to_json
8787
end

search_parser/lib/services.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
# Solr
1414
######
1515

16-
S.register(:solr_url) { ENV.fetch("CATALOG_SOLR_URL") }
16+
S.register(:solr_url) { ENV["CATALOG_SOLR_URL"] || "http://solr:8983" }
1717
S.register(:solr_core) { ENV["CATALOG_SOLR_CORE"] || "biblio" }
1818
S.register(:solr_conn) do
1919
Faraday.new(
Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
RSpec.describe "requests" do
2+
context "get /catalog/search" do
3+
let(:key) { "" }
4+
let(:value) { "" }
5+
def gen_solr_stub(key, value)
6+
# this should do proper matching.
7+
# (?=&|\z) means the string is followed by either the "&" character or
8+
# the end of the string
9+
stub_request(:get, /#{S.solr_url}\/solr\/biblio\/select.*[?&]#{key}=#{value}(?=&|\z)/)
10+
end
11+
let(:params) do
12+
{query: "blah"}
13+
end
14+
15+
def expect_has_param(key, value)
16+
solr_stub = gen_solr_stub(key, value)
17+
get "/catalog/search", params
18+
expect(solr_stub).to have_been_requested
19+
end
20+
21+
subject do
22+
get "/catalog/search", params
23+
end
24+
25+
context "standard params" do
26+
it "has a q1 param" do
27+
expect_has_param("q1", "blah")
28+
end
29+
it "has fl of *,score" do
30+
expect_has_param("fl", '\*,score')
31+
end
32+
it "has a default start of 0" do
33+
expect_has_param("start", "0")
34+
end
35+
it "uses start from query parameters" do
36+
params[:start] = "22"
37+
expect_has_param("start", "22")
38+
end
39+
it "has a default rows of 10" do
40+
expect_has_param("rows", "10")
41+
end
42+
it "uses the rows param for rows" do
43+
params[:rows] = "20"
44+
expect_has_param("rows", "20")
45+
end
46+
it "has a default for sort" do
47+
expect_has_param("sort", "score desc")
48+
end
49+
it "uses the sort param for sort" do
50+
params["sort"] = "created asc"
51+
expect_has_param("sort", "created asc")
52+
end
53+
it "includes solr quoted string for qq" do
54+
expect_has_param("qq", '"_query_.*')
55+
end
56+
end
57+
58+
SearchParser.facets.each do |facet|
59+
context "#{facet} parameters" do
60+
it "has the default facet limit" do
61+
expect_has_param("f.#{facet}.facet.limit", "50")
62+
end
63+
it "has the default facet mincount" do
64+
expect_has_param("f.#{facet}.facet.mincount", "1")
65+
end
66+
it "has the default facet offset" do
67+
expect_has_param("f.#{facet}.facet.offset", "0")
68+
end
69+
it "has the default facet sort" do
70+
expect_has_param("f.#{facet}.facet.sort", "count")
71+
end
72+
it "has the facet field" do
73+
expect_has_param("f.#{facet}.facet.sort", "count")
74+
end
75+
end
76+
end
77+
78+
context "filter query" do
79+
it "passes the filter query from fq param" do
80+
params["fq"] = ["first", "second"]
81+
expect_has_param("fq", "first")
82+
end
83+
it "includes the second parameter too" do
84+
params["fq"] = ["first", "second"]
85+
expect_has_param("fq", "second")
86+
end
87+
88+
# these two won't last for ever. The api should always send a fq
89+
it "defaults to um library" do
90+
expect_has_param("fq", "institution.*")
91+
end
92+
it "defaults to not-search-only" do
93+
expect_has_param("fq", '%2B\(availability:physical.*')
94+
end
95+
end
96+
end
97+
end

search_parser/spec/spec_helper.rb

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
require "rack/test"
2+
require "debug"
3+
require "webmock/rspec"
4+
ENV["APP_ENV"] = "test"
5+
6+
require_relative "../app"
7+
8+
# enabling this breaks hash_include which really is needed for this testing
9+
WebMock::Config.instance.query_values_notation = :flat_array
10+
module RSpecMixin
11+
include Rack::Test::Methods
12+
13+
def app = SearchParser::Application
14+
end
15+
16+
RSpec.configure do |config|
17+
config.include RSpecMixin
18+
config.expect_with :rspec do |expectations|
19+
# This option will default to `true` in RSpec 4. It makes the `description`
20+
# and `failure_message` of custom matchers include text for helper methods
21+
# defined using `chain`, e.g.:
22+
# be_bigger_than(2).and_smaller_than(4).description
23+
# # => "be bigger than 2 and smaller than 4"
24+
# ...rather than:
25+
# # => "be bigger than 2"
26+
expectations.include_chain_clauses_in_custom_matcher_descriptions = true
27+
end
28+
29+
# rspec-mocks config goes here. You can use an alternate test double
30+
# library (such as bogus or mocha) by changing the `mock_with` option here.
31+
config.mock_with :rspec do |mocks|
32+
# Prevents you from mocking or stubbing a method that does not exist on
33+
# a real object. This is generally recommended, and will default to
34+
# `true` in RSpec 4.
35+
mocks.verify_partial_doubles = true
36+
end
37+
38+
# This option will default to `:apply_to_host_groups` in RSpec 4 (and will
39+
# have no way to turn it off -- the option exists only for backwards
40+
# compatibility in RSpec 3). It causes shared context metadata to be
41+
# inherited by the metadata hash of host groups and examples, rather than
42+
# triggering implicit auto-inclusion in groups with matching metadata.
43+
config.shared_context_metadata_behavior = :apply_to_host_groups
44+
45+
# The settings below are suggested to provide a good initial experience
46+
# with RSpec, but feel free to customize to your heart's content.
47+
# # This allows you to limit a spec run to individual examples or groups
48+
# # you care about by tagging them with `:focus` metadata. When nothing
49+
# # is tagged with `:focus`, all examples get run. RSpec also provides
50+
# # aliases for `it`, `describe`, and `context` that include `:focus`
51+
# # metadata: `fit`, `fdescribe` and `fcontext`, respectively.
52+
# config.filter_run_when_matching :focus
53+
#
54+
# # Allows RSpec to persist some state between runs in order to support
55+
# # the `--only-failures` and `--next-failure` CLI options. We recommend
56+
# # you configure your source control system to ignore this file.
57+
# config.example_status_persistence_file_path = "spec/examples.txt"
58+
#
59+
# # Limits the available syntax to the non-monkey patched syntax that is
60+
# # recommended. For more details, see:
61+
# # https://rspec.info/features/3-12/rspec-core/configuration/zero-monkey-patching-mode/
62+
# config.disable_monkey_patching!
63+
#
64+
# # This setting enables warnings. It's recommended, but in some cases may
65+
# # be too noisy due to issues in dependencies.
66+
# config.warnings = true
67+
#
68+
# # Many RSpec users commonly either run the entire suite or an individual
69+
# # file, and it's useful to allow more verbose output when running an
70+
# # individual spec file.
71+
# if config.files_to_run.one?
72+
# # Use the documentation formatter for detailed output,
73+
# # unless a formatter has already been configured
74+
# # (e.g. via a command-line flag).
75+
# config.default_formatter = "doc"
76+
# end
77+
#
78+
# # Print the 10 slowest examples and example groups at the
79+
# # end of the spec run, to help surface which specs are running
80+
# # particularly slow.
81+
# config.profile_examples = 10
82+
#
83+
# # Run specs in random order to surface order dependencies. If you find an
84+
# # order dependency and want to debug it, you can fix the order by providing
85+
# # the seed, which is printed after each run.
86+
# # --seed 1234
87+
# config.order = :random
88+
#
89+
# # Seed global randomization in this process using the `--seed` CLI option.
90+
# # Setting this allows you to use `--seed` to deterministically reproduce
91+
# # test failures related to randomization by passing the same `--seed` value
92+
# # as the one that triggered the failure.
93+
# Kernel.srand config.seed
94+
end

0 commit comments

Comments
 (0)