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.

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

Change query limit settings format to nested mappings #4189

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
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
6 changes: 3 additions & 3 deletions app/controllers/api/changesets_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -425,13 +425,13 @@ def conditions_ids(changesets, ids)
# Get the maximum number of results to return
def result_limit
if params[:limit]
if params[:limit].to_i.positive? && params[:limit].to_i <= Settings.max_changeset_query_limit
if params[:limit].to_i.positive? && params[:limit].to_i <= Settings.changesets.max_query_limit
params[:limit].to_i
else
raise OSM::APIBadUserInput, "Changeset limit must be between 1 and #{Settings.max_changeset_query_limit}"
raise OSM::APIBadUserInput, "Changeset limit must be between 1 and #{Settings.changesets.max_query_limit}"
end
else
Settings.default_changeset_query_limit
Settings.changesets.default_query_limit
end
end
end
Expand Down
6 changes: 3 additions & 3 deletions app/controllers/api/notes_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -332,13 +332,13 @@ def search
# Get the maximum number of results to return
def result_limit
if params[:limit]
if params[:limit].to_i.positive? && params[:limit].to_i <= Settings.max_note_query_limit
if params[:limit].to_i.positive? && params[:limit].to_i <= Settings.notes.max_query_limit
params[:limit].to_i
else
raise OSM::APIBadUserInput, "Note limit must be between 1 and #{Settings.max_note_query_limit}"
raise OSM::APIBadUserInput, "Note limit must be between 1 and #{Settings.notes.max_query_limit}"
end
else
Settings.default_note_query_limit
Settings.notes.default_query_limit
end
end

Expand Down
8 changes: 4 additions & 4 deletions app/views/api/capabilities/show.builder
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@ xml.osm(OSM::API.new.xml_root_attributes) do |osm|
api.waynodes(:maximum => Settings.max_number_of_way_nodes)
api.relationmembers(:maximum => Settings.max_number_of_relation_members)
api.changesets(:maximum_elements => Changeset::MAX_ELEMENTS,
:default_query_limit => Settings.default_changeset_query_limit,
:maximum_query_limit => Settings.max_changeset_query_limit)
api.notes(:default_query_limit => Settings.default_note_query_limit,
:maximum_query_limit => Settings.max_note_query_limit)
:default_query_limit => Settings.changesets.default_query_limit,
:maximum_query_limit => Settings.changesets.max_query_limit)
api.notes(:default_query_limit => Settings.notes.default_query_limit,
:maximum_query_limit => Settings.notes.max_query_limit)
api.timeout(:seconds => Settings.api_timeout)
api.status(:database => @database_status,
:api => @api_status,
Expand Down
8 changes: 4 additions & 4 deletions app/views/api/capabilities/show.json.jbuilder
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,12 @@ json.api do
end
json.changesets do
json.maximum_elements Changeset::MAX_ELEMENTS
json.default_query_limit Settings.default_changeset_query_limit
json.maximum_query_limit Settings.max_changeset_query_limit
json.default_query_limit Settings.changesets.default_query_limit
json.maximum_query_limit Settings.changesets.max_query_limit
end
json.notes do
json.default_query_limit Settings.default_note_query_limit
json.maximum_query_limit Settings.max_note_query_limit
json.default_query_limit Settings.notes.default_query_limit
json.maximum_query_limit Settings.notes.max_query_limit
end
json.timeout do
json.seconds Settings.api_timeout
Expand Down
8 changes: 8 additions & 0 deletions config/initializers/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -85,5 +85,13 @@
required(:trace_file_storage).filled(:str?)
required(:trace_image_storage).filled(:str?)
required(:trace_icon_storage).filled(:str?)
required(:changesets).schema do
required(:default_query_limit).filled(:int?)
required(:max_query_limit).filled(:int?)
end
required(:notes).schema do
required(:default_query_limit).filled(:int?)
required(:max_query_limit).filled(:int?)
end
end
end
18 changes: 10 additions & 8 deletions config/settings.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,6 @@ status: "online"
max_request_area: 0.25
# Number of GPS trace/trackpoints returned per-page
tracepoints_per_page: 5000
# Default limit on the number of changesets returned by the changeset query api method
default_changeset_query_limit: 100
# Maximum limit on the number of changesets returned by the changeset query api method
max_changeset_query_limit: 100
# Maximum number of nodes that will be returned by the api in a map request
max_number_of_nodes: 50000
# Maximum number of nodes that can be in a way (checked on save)
Expand All @@ -39,12 +35,18 @@ max_number_of_way_nodes: 2000
max_number_of_relation_members: 32000
# The maximum area you're allowed to request notes from, in square degrees
max_note_request_area: 25
# Default limit on the number of notes returned by the note search api method
default_note_query_limit: 100
# Maximum limit on the number of notes returned by the note search api method
max_note_query_limit: 10000
# Maximum value of open issues counter for moderators, anything equal or greater to this value "n" is shown as "n+"
max_issues_count: 99
changesets:
# Default limit on the number of changesets returned by the changeset query api method
default_query_limit: 100
# Maximum limit on the number of changesets returned by the changeset query api method
max_query_limit: 100
notes:
# Default limit on the number of notes returned by the note search api method
default_query_limit: 100
# Maximum limit on the number of notes returned by the note search api method
max_query_limit: 10000
# Zoom level to use for postcode results from the geocoder
postcode_zoom: 15
# Timeout for API calls in seconds
Expand Down
16 changes: 8 additions & 8 deletions test/controllers/api/capabilities_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,12 @@ def test_capabilities
assert_select "tracepoints[per_page='#{Settings.tracepoints_per_page}']", :count => 1
assert_select "changesets" \
"[maximum_elements='#{Changeset::MAX_ELEMENTS}']" \
"[default_query_limit='#{Settings.default_changeset_query_limit}']" \
"[maximum_query_limit='#{Settings.max_changeset_query_limit}']", :count => 1
"[default_query_limit='#{Settings.changesets.default_query_limit}']" \
"[maximum_query_limit='#{Settings.changesets.max_query_limit}']", :count => 1
assert_select "relationmembers[maximum='#{Settings.max_number_of_relation_members}']", :count => 1
assert_select "notes" \
"[default_query_limit='#{Settings.default_note_query_limit}']" \
"[maximum_query_limit='#{Settings.max_note_query_limit}']", :count => 1
"[default_query_limit='#{Settings.notes.default_query_limit}']" \
"[maximum_query_limit='#{Settings.notes.max_query_limit}']", :count => 1
assert_select "status[database='online']", :count => 1
assert_select "status[api='online']", :count => 1
assert_select "status[gpx='online']", :count => 1
Expand All @@ -61,11 +61,11 @@ def test_capabilities_json
assert_equal Settings.max_note_request_area, js["api"]["note_area"]["maximum"]
assert_equal Settings.tracepoints_per_page, js["api"]["tracepoints"]["per_page"]
assert_equal Changeset::MAX_ELEMENTS, js["api"]["changesets"]["maximum_elements"]
assert_equal Settings.default_changeset_query_limit, js["api"]["changesets"]["default_query_limit"]
assert_equal Settings.max_changeset_query_limit, js["api"]["changesets"]["maximum_query_limit"]
assert_equal Settings.changesets.default_query_limit, js["api"]["changesets"]["default_query_limit"]
assert_equal Settings.changesets.max_query_limit, js["api"]["changesets"]["maximum_query_limit"]
assert_equal Settings.max_number_of_relation_members, js["api"]["relationmembers"]["maximum"]
assert_equal Settings.default_note_query_limit, js["api"]["notes"]["default_query_limit"]
assert_equal Settings.max_note_query_limit, js["api"]["notes"]["maximum_query_limit"]
assert_equal Settings.notes.default_query_limit, js["api"]["notes"]["default_query_limit"]
assert_equal Settings.notes.max_query_limit, js["api"]["notes"]["maximum_query_limit"]
assert_equal "online", js["api"]["status"]["database"]
assert_equal "online", js["api"]["status"]["api"]
assert_equal "online", js["api"]["status"]["gpx"]
Expand Down
15 changes: 13 additions & 2 deletions test/controllers/api/changesets_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1983,14 +1983,25 @@ def test_query_limit
get changesets_path(:limit => "0")
assert_response :bad_request

get changesets_path(:limit => Settings.max_changeset_query_limit)
get changesets_path(:limit => Settings.changesets.max_query_limit)
assert_response :success
assert_changesets_in_order [changeset5, changeset4, changeset3, changeset2, changeset1]

get changesets_path(:limit => Settings.max_changeset_query_limit + 1)
get changesets_path(:limit => Settings.changesets.max_query_limit + 1)
assert_response :bad_request
end

##
# test the query functionality of changesets with the default limit
def test_query_default_limit
user = create(:user)
create_list(:changeset, Settings.changesets.default_query_limit + 1, :user => user)

get changesets_path
assert_response :success
assert_select "osm>changeset", Settings.changesets.default_query_limit
end

##
# test the query functionality of sequential changesets with order and time parameters
def test_query_order
Expand Down
20 changes: 14 additions & 6 deletions test/controllers/api/notes_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -696,7 +696,7 @@ def test_index_limit
assert_select "wpt", :count => 1
end

get api_notes_path(:bbox => "1,1,1.2,1.2", :limit => Settings.max_note_query_limit, :format => "rss")
get api_notes_path(:bbox => "1,1,1.2,1.2", :limit => Settings.notes.max_query_limit, :format => "rss")
assert_response :success
end

Expand Down Expand Up @@ -811,7 +811,7 @@ def test_index_bad_params
get api_notes_path(:bbox => "1,1,1.7,1.7", :limit => "0", :format => "json")
assert_response :bad_request

get api_notes_path(:bbox => "1,1,1.7,1.7", :limit => Settings.max_note_query_limit + 1, :format => "json")
get api_notes_path(:bbox => "1,1,1.7,1.7", :limit => Settings.notes.max_query_limit + 1, :format => "json")
assert_response :bad_request
end

Expand Down Expand Up @@ -849,10 +849,18 @@ def test_search_success
assert_select "wpt", :count => 1
end

get search_api_notes_path(:q => "note comment", :limit => Settings.max_note_query_limit, :format => "xml")
get search_api_notes_path(:q => "note comment", :limit => Settings.notes.max_query_limit, :format => "xml")
assert_response :success
end

def test_search_default_limit
create_list(:note, Settings.notes.default_query_limit + 1)

get search_api_notes_path
assert_response :success
assert_select "osm>note", Settings.changesets.default_query_limit
end

def test_search_by_display_name_success
user = create(:user)

Expand Down Expand Up @@ -1027,7 +1035,7 @@ def test_search_bad_params
get search_api_notes_path(:q => "no match", :limit => "0", :format => "json")
assert_response :bad_request

get search_api_notes_path(:q => "no match", :limit => Settings.max_note_query_limit + 1, :format => "json")
get search_api_notes_path(:q => "no match", :limit => Settings.notes.max_query_limit + 1, :format => "json")
assert_response :bad_request

get search_api_notes_path(:display_name => "non-existent")
Expand Down Expand Up @@ -1070,7 +1078,7 @@ def test_feed_success
end
end

get feed_api_notes_path(:bbox => "1,1,1.2,1.2", :limit => Settings.max_note_query_limit, :format => "rss")
get feed_api_notes_path(:bbox => "1,1,1.2,1.2", :limit => Settings.notes.max_query_limit, :format => "rss")
assert_response :success
end

Expand All @@ -1084,7 +1092,7 @@ def test_feed_fail
get feed_api_notes_path(:bbox => "1,1,1.2,1.2", :limit => "0", :format => "rss")
assert_response :bad_request

get feed_api_notes_path(:bbox => "1,1,1.2,1.2", :limit => Settings.max_note_query_limit + 1, :format => "rss")
get feed_api_notes_path(:bbox => "1,1,1.2,1.2", :limit => Settings.notes.max_query_limit + 1, :format => "rss")
assert_response :bad_request
end

Expand Down