Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 6 additions & 3 deletions app/graphql/types/query_type.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ def record_id(id:, index:)
argument :title, String, required: false, default_value: nil, description: 'Search by title'
argument :from, String, required: false, default_value: '0',
description: 'Search result number to begin with (the first result is 0)'
argument :per_page, Integer, required: false, default_value: 20,
description: 'Number of results per page. Defaults to 20.'
argument :fulltext, Boolean, required: false, default_value: false,
description: 'Include fulltext field in search? Defaults to false.'
argument :index, String, required: false, default_value: nil,
Expand Down Expand Up @@ -101,9 +103,9 @@ def record_id(id:, index:)
end

def search(searchterm:, citation:, contributors:, funding_information:, geodistance:, geobox:, identifiers:,
locations:, subjects:, title:, index:, source:, from:, boolean_type:, fulltext:, **filters)
locations:, subjects:, title:, index:, source:, from:, boolean_type:, fulltext:, per_page: 20, **filters)
query = construct_query(searchterm, citation, contributors, funding_information, geodistance, geobox, identifiers,
locations, subjects, title, source, boolean_type, filters)
locations, subjects, title, source, boolean_type, filters, per_page)

results = Opensearch.new.search(from, query, Timdex::OSClient, highlight_requested?, index, fulltext)

Expand Down Expand Up @@ -133,7 +135,7 @@ def inject_hits_fields_into_source(hits)
end

def construct_query(searchterm, citation, contributors, funding_information, geodistance, geobox, identifiers,
locations, subjects, title, source, boolean_type, filters)
locations, subjects, title, source, boolean_type, filters, per_page)
query = {}
query[:q] = searchterm
query[:boolean_type] = boolean_type
Expand All @@ -144,6 +146,7 @@ def construct_query(searchterm, citation, contributors, funding_information, geo
query[:geobox] = geobox
query[:identifiers] = identifiers
query[:locations] = locations
query[:per_page] = per_page
query[:subjects] = subjects
query[:title] = title
query[:access_to_files_filter] = filters[:access_to_files_filter]
Expand Down
13 changes: 11 additions & 2 deletions app/models/opensearch.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
# rubocop:disable Metrics/MethodLength
class Opensearch
SIZE = 20
MAX_PAGE = 200
MAX_SIZE = 200

def search(from, params, client, highlight = false, index = nil, fulltext = false)
@params = params
Expand All @@ -24,9 +24,18 @@ def default_index

# Construct the json query to send to elasticsearch
def build_query(from)
# allow overriding the OpenSearch `size` via params (per_page), capped by MAX_PAGE
calculate_size = if @params && @params[:per_page]
per_page = @params[:per_page].to_i
per_page = SIZE if per_page <= 0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ruby treating strings it doesn't know how to convert to an integer as zero is either brilliant or super weird. In this case it works well so I guess I should appreciate it.

[per_page, MAX_SIZE].min
else
SIZE
end

query_hash = {
from:,
size: SIZE,
size: calculate_size,
query:,
aggregations: Aggregations.all,
sort:
Expand Down
19 changes: 19 additions & 0 deletions test/controllers/graphql_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -937,4 +937,23 @@ class GraphqlControllerTest < ActionDispatch::IntegrationTest
end
end
end

test 'graphql search respects perPage argument' do
VCR.use_cassette('opensearch_init') do
VCR.use_cassette('graphql_search_per_page_5', match_requests_on: [:method, :uri]) do
post '/graphql', params: { query: '{
search(perPage:5) {
hits
records {
title
}
}
}' }
assert_equal(200, response.status)
json = JSON.parse(response.body)
assert_equal(5, json['data']['search']['records'].count)
assert_equal(100, json['data']['search']['hits'])
end
end
end
end
28 changes: 28 additions & 0 deletions test/models/opensearch_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -359,4 +359,32 @@ class OpensearchTest < ActiveSupport::TestCase
os.query.to_json.include?('{"locations.geoshape":{"top":"42.886","bottom":"41.239","left":"-69.507","right":"-73.928"}}')
)
end

test 'build_query uses default size' do
os = Opensearch.new
os.instance_variable_set(:@params, {})
json = JSON.parse(os.build_query(0))
assert_equal Opensearch::SIZE, json['size']
end

test 'build_query respects per_page' do
os = Opensearch.new
os.instance_variable_set(:@params, { per_page: 5 })
json = JSON.parse(os.build_query(0))
assert_equal 5, json['size']
end

test 'build_query falls back for nonpositive per_page' do
os = Opensearch.new
os.instance_variable_set(:@params, { per_page: 0 })
json = JSON.parse(os.build_query(0))
assert_equal Opensearch::SIZE, json['size']
end

test 'build_query caps per_page at MAX_SIZE' do
os = Opensearch.new
os.instance_variable_set(:@params, { per_page: Opensearch::MAX_SIZE + 100 })
json = JSON.parse(os.build_query(0))
assert_equal Opensearch::MAX_SIZE, json['size']
end
end
70 changes: 70 additions & 0 deletions test/vcr_cassettes/graphql_search_per_page_5.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.