Merge pull request #790 from karussell/config_max_results

Make maximum results configurable
komoot · Apr 18, 2024 · 6732ccf · 6732ccf
2 parents 829e8cc + d97682e
commit 6732ccf
Show file tree

Hide file tree

Showing 9 changed files with 55 additions and 31 deletions.
diff --git a/src/main/java/de/komoot/photon/App.java b/src/main/java/de/komoot/photon/App.java
@@ -191,12 +191,14 @@ private static void startApi(CommandLineArgs args, Server server) {
         String[] langs = dbProperties.getLanguages();
 
         SearchHandler searchHandler = server.createSearchHandler(langs, args.getQueryTimeout());
-        get("api", new SearchRequestHandler("api", searchHandler, langs, args.getDefaultLanguage()));
-        get("api/", new SearchRequestHandler("api/", searchHandler, langs, args.getDefaultLanguage()));
+        get("api", new SearchRequestHandler("api", searchHandler, langs, args.getDefaultLanguage(), args.getMaxResults()));
+        get("api/", new SearchRequestHandler("api/", searchHandler, langs, args.getDefaultLanguage(), args.getMaxResults()));
 
         ReverseHandler reverseHandler = server.createReverseHandler(args.getQueryTimeout());
-        get("reverse", new ReverseSearchRequestHandler("reverse", reverseHandler, dbProperties.getLanguages(), args.getDefaultLanguage()));
-        get("reverse/", new ReverseSearchRequestHandler("reverse/", reverseHandler, dbProperties.getLanguages(), args.getDefaultLanguage()));
+        get("reverse", new ReverseSearchRequestHandler("reverse", reverseHandler, dbProperties.getLanguages(),
+                args.getDefaultLanguage(), args.getMaxReverseResults()));
+        get("reverse/", new ReverseSearchRequestHandler("reverse/", reverseHandler, dbProperties.getLanguages(),
+                args.getDefaultLanguage(), args.getMaxReverseResults()));
 
         get("status", new StatusRequestHandler("status", server));
         get("status/", new StatusRequestHandler("status/", server));

diff --git a/src/main/java/de/komoot/photon/CommandLineArgs.java b/src/main/java/de/komoot/photon/CommandLineArgs.java
@@ -82,6 +82,12 @@ public class CommandLineArgs {
     @Parameter(names = "-h", description = "Show help / usage")
     private boolean usage = false;
 
+    @Parameter(names = "-max-results", description = "The maximum possible 'limit' parameter for geocoding searches")
+    private int maxResults = 50;
+
+    @Parameter(names = "-max-reverse-results", description = "The maximum possible 'limit' parameter for reverse geocoding searches")
+    private int maxReverseResults = 50;
+
     public String[] getLanguages(boolean useDefaultIfEmpty) {
         if (useDefaultIfEmpty && languages.length == 0) {
             return new String[]{"en", "de", "fr", "it"};
@@ -185,5 +191,13 @@ public boolean isEnableUpdateApi() {
     public boolean isUsage() {
         return this.usage;
     }
+
+    public int getMaxReverseResults() {
+        return maxReverseResults;
+    }
+
+    public int getMaxResults() {
+        return maxResults;
+    }
 }
 
diff --git a/src/main/java/de/komoot/photon/ReverseSearchRequestHandler.java b/src/main/java/de/komoot/photon/ReverseSearchRequestHandler.java
@@ -23,10 +23,10 @@ public class ReverseSearchRequestHandler extends RouteImpl {
     private final ReverseRequestFactory reverseRequestFactory;
     private final ReverseHandler requestHandler;
 
-    ReverseSearchRequestHandler(String path, ReverseHandler dbHandler, String[] languages, String defaultLanguage) {
+    ReverseSearchRequestHandler(String path, ReverseHandler dbHandler, String[] languages, String defaultLanguage, int maxResults) {
         super(path);
         List<String> supportedLanguages = Arrays.asList(languages);
-        this.reverseRequestFactory = new ReverseRequestFactory(supportedLanguages, defaultLanguage);
+        this.reverseRequestFactory = new ReverseRequestFactory(supportedLanguages, defaultLanguage, maxResults);
         this.requestHandler = dbHandler;
     }
 

diff --git a/src/main/java/de/komoot/photon/SearchRequestHandler.java b/src/main/java/de/komoot/photon/SearchRequestHandler.java
@@ -20,12 +20,14 @@
 public class SearchRequestHandler extends RouteImpl {
     private final PhotonRequestFactory photonRequestFactory;
     private final SearchHandler requestHandler;
+    private final int maxResults;
 
-    SearchRequestHandler(String path, SearchHandler dbHandler, String[] languages, String defaultLanguage) {
+    SearchRequestHandler(String path, SearchHandler dbHandler, String[] languages, String defaultLanguage, int maxResults) {
         super(path);
         List<String> supportedLanguages = Arrays.asList(languages);
-        this.photonRequestFactory = new PhotonRequestFactory(supportedLanguages, defaultLanguage);
+        this.photonRequestFactory = new PhotonRequestFactory(supportedLanguages, defaultLanguage, maxResults);
         this.requestHandler = dbHandler;
+        this.maxResults = maxResults;
     }
 
     @Override

diff --git a/src/main/java/de/komoot/photon/query/PhotonRequest.java b/src/main/java/de/komoot/photon/query/PhotonRequest.java
@@ -77,9 +77,7 @@ PhotonRequest setLayerFilter(Set<String> filters) {
     }
 
     PhotonRequest setLimit(Integer limit) {
-        if (limit != null) {
-            this.limit = Integer.max(Integer.min(limit, 50), 1);
-        }
+        this.limit = limit;
         return this;
     }
 

diff --git a/src/main/java/de/komoot/photon/query/PhotonRequestFactory.java b/src/main/java/de/komoot/photon/query/PhotonRequestFactory.java
@@ -16,14 +16,16 @@ public class PhotonRequestFactory {
     private static final LocationParamConverter optionalLocationParamConverter = new LocationParamConverter(false);
     private final BoundingBoxParamConverter bboxParamConverter;
     private final LayerParamValidator layerParamValidator;
+    private final int maxResults;
 
     private static final HashSet<String> REQUEST_QUERY_PARAMS = new HashSet<>(Arrays.asList("lang", "q", "lon", "lat",
             "limit", "osm_tag", "location_bias_scale", "bbox", "debug", "zoom", "layer"));
 
-    public PhotonRequestFactory(List<String> supportedLanguages, String defaultLanguage) {
+    public PhotonRequestFactory(List<String> supportedLanguages, String defaultLanguage, int maxResults) {
         this.languageResolver = new RequestLanguageResolver(supportedLanguages, defaultLanguage);
         this.bboxParamConverter = new BoundingBoxParamConverter();
         this.layerParamValidator = new LayerParamValidator();
+        this.maxResults = maxResults;
     }
 
     public PhotonRequest create(Request webRequest) throws BadRequestException {
@@ -38,7 +40,11 @@ public PhotonRequest create(Request webRequest) throws BadRequestException {
 
         PhotonRequest request = new PhotonRequest(query, languageResolver.resolveRequestedLanguage(webRequest));
 
-        request.setLimit(parseInt(webRequest, "limit"));
+        Integer limit = parseInt(webRequest, "limit");
+        if (limit != null) {
+            request.setLimit(Integer.max(Integer.min(limit, maxResults), 1));
+        }
+
         request.setLocationForBias(optionalLocationParamConverter.apply(webRequest));
         request.setBbox(bboxParamConverter.apply(webRequest));
         request.setScale(parseDouble(webRequest, "location_bias_scale"));

diff --git a/src/main/java/de/komoot/photon/query/ReverseRequestFactory.java b/src/main/java/de/komoot/photon/query/ReverseRequestFactory.java
@@ -1,7 +1,6 @@
 package de.komoot.photon.query;
 
 import com.vividsolutions.jts.geom.Point;
-
 import de.komoot.photon.searcher.TagFilter;
 import spark.QueryParamsMap;
 import spark.Request;
@@ -15,16 +14,19 @@
  * Factory that creates a {@link ReverseRequest} from a {@link Request web request}
  */
 public class ReverseRequestFactory {
-    private final RequestLanguageResolver languageResolver;
-    private static final LocationParamConverter mandatoryLocationParamConverter = new LocationParamConverter(true);
-    private final LayerParamValidator layerParamValidator;
 
     private static final HashSet<String> REQUEST_QUERY_PARAMS = new HashSet<>(Arrays.asList("lang", "lon", "lat", "radius",
             "query_string_filter", "distance_sort", "limit", "layer", "osm_tag", "debug"));
+    private static final LocationParamConverter mandatoryLocationParamConverter = new LocationParamConverter(true);
 
-    public ReverseRequestFactory(List<String> supportedLanguages, String defaultLanguage) {
+    private final RequestLanguageResolver languageResolver;
+    private final LayerParamValidator layerParamValidator;
+    private final int maxResults;
+
+    public ReverseRequestFactory(List<String> supportedLanguages, String defaultLanguage, int maxResults) {
         this.languageResolver = new RequestLanguageResolver(supportedLanguages, defaultLanguage);
         this.layerParamValidator = new LayerParamValidator();
+        this.maxResults = maxResults;
     }
 
     public ReverseRequest create(Request webRequest) throws BadRequestException {
@@ -70,9 +72,9 @@ public ReverseRequest create(Request webRequest) throws BadRequestException {
             }
             if (limit <= 0) {
                 throw new BadRequestException(400, "Invalid search term 'limit', expected a strictly positive integer.");
-            } else {
-                limit = Math.min(limit, 50);
             }
+
+            limit = Math.min(limit, maxResults);
         }
 
         boolean enableDebug = webRequest.queryParams("debug") != null;

diff --git a/src/test/java/de/komoot/photon/query/PhotonRequestFactoryTest.java b/src/test/java/de/komoot/photon/query/PhotonRequestFactoryTest.java
@@ -56,7 +56,7 @@ private void requestWithLayers(Request mockRequest, String... layers) {
     }
 
     private PhotonRequest createPhotonRequest(Request mockRequest) throws BadRequestException {
-        PhotonRequestFactory factory = new PhotonRequestFactory(Collections.singletonList("en"), "en");
+        PhotonRequestFactory factory = new PhotonRequestFactory(Collections.singletonList("en"), "en", 10);
         return factory.create(mockRequest);
     }
 

diff --git a/src/test/java/de/komoot/photon/query/ReverseRequestFactoryTest.java b/src/test/java/de/komoot/photon/query/ReverseRequestFactoryTest.java
@@ -56,7 +56,7 @@ private void requestWithLayers(Request mockRequest, String... layers) {
     @Test
     void testWithLocation() throws Exception {
         Request mockRequest = createRequestWithLongitudeLatitude(-87d, 41d);
-        ReverseRequestFactory reverseRequestFactory = new ReverseRequestFactory(Collections.singletonList("en"), "en");
+        ReverseRequestFactory reverseRequestFactory = new ReverseRequestFactory(Collections.singletonList("en"), "en", 10);
         reverseRequest = reverseRequestFactory.create(mockRequest);
         assertEquals(-87, reverseRequest.getLocation().getX(), 0);
         assertEquals(41, reverseRequest.getLocation().getY(), 0);
@@ -66,7 +66,7 @@ void testWithLocation() throws Exception {
 
     private void assertBadRequest(Request mockRequest, String expectedMessage) {
         try {
-            ReverseRequestFactory reverseRequestFactory = new ReverseRequestFactory(Collections.singletonList("en"), "en");
+            ReverseRequestFactory reverseRequestFactory = new ReverseRequestFactory(Collections.singletonList("en"), "en", 10);
             reverseRequest = reverseRequestFactory.create(mockRequest);
             fail();
         } catch (BadRequestException e) {
@@ -155,7 +155,7 @@ void testWithBadRadius() throws Exception {
     void testHighRadius() throws Exception {
         Request mockRequest = createRequestWithLongitudeLatitude(-87d, 41d);
         Mockito.when(mockRequest.queryParams("radius")).thenReturn("5.1");
-        ReverseRequestFactory reverseRequestFactory = new ReverseRequestFactory(Collections.singletonList("en"), "en");
+        ReverseRequestFactory reverseRequestFactory = new ReverseRequestFactory(Collections.singletonList("en"), "en", 10);
         reverseRequest = reverseRequestFactory.create(mockRequest);
         assertEquals(5.1d, reverseRequest.getRadius(), 0);
         Mockito.verify(mockRequest, Mockito.times(1)).queryParams("radius");
@@ -180,7 +180,7 @@ void testWithBadLimit() throws Exception {
     void testHighLimit() throws Exception {
         Request mockRequest = createRequestWithLongitudeLatitude(-87d, 41d);
         Mockito.when(mockRequest.queryParams("limit")).thenReturn("51");
-        ReverseRequestFactory reverseRequestFactory = new ReverseRequestFactory(Collections.singletonList("en"), "en");
+        ReverseRequestFactory reverseRequestFactory = new ReverseRequestFactory(Collections.singletonList("en"), "en", 50);
         reverseRequest = reverseRequestFactory.create(mockRequest);
         assertEquals(50, reverseRequest.getLimit());
         Mockito.verify(mockRequest, Mockito.times(1)).queryParams("limit");
@@ -189,7 +189,7 @@ void testHighLimit() throws Exception {
     @Test
     void testDistanceSortDefault() throws Exception {
         Request mockRequest = createRequestWithLongitudeLatitude(-87d, 41d);
-        ReverseRequestFactory reverseRequestFactory = new ReverseRequestFactory(Collections.singletonList("en"), "en");
+        ReverseRequestFactory reverseRequestFactory = new ReverseRequestFactory(Collections.singletonList("en"), "en", 10);
         reverseRequest = reverseRequestFactory.create(mockRequest);
         Mockito.verify(mockRequest, Mockito.times(1)).queryParamOrDefault("distance_sort", "true");
         assertEquals(true, reverseRequest.getLocationDistanceSort());
@@ -199,7 +199,7 @@ void testDistanceSortDefault() throws Exception {
     void testWithLayersFilters() throws Exception {
         Request mockRequest = createRequestWithLongitudeLatitude(-87d, 41d);
         requestWithLayers(mockRequest, "city", "locality");
-        ReverseRequestFactory reverseRequestFactory = new ReverseRequestFactory(Collections.singletonList("en"), "en");
+        ReverseRequestFactory reverseRequestFactory = new ReverseRequestFactory(Collections.singletonList("en"), "en", 10);
         reverseRequest = reverseRequestFactory.create(mockRequest);
         assertEquals(new HashSet<>(Arrays.asList("city", "locality")), reverseRequest.getLayerFilters());
     }
@@ -208,7 +208,7 @@ void testWithLayersFilters() throws Exception {
     void testWithDuplicatedLayerFilters() throws Exception {
         Request mockRequest = createRequestWithLongitudeLatitude(-87d, 41d);
         requestWithLayers(mockRequest, "city", "locality", "city");
-        ReverseRequestFactory reverseRequestFactory = new ReverseRequestFactory(Collections.singletonList("en"), "en");
+        ReverseRequestFactory reverseRequestFactory = new ReverseRequestFactory(Collections.singletonList("en"), "en", 10);
         reverseRequest = reverseRequestFactory.create(mockRequest);
         assertEquals(new HashSet<>(Arrays.asList("city", "locality")), reverseRequest.getLayerFilters());
     }
@@ -225,7 +225,7 @@ void testWithBadLayerFilters() {
     void testTagFilters() throws Exception {
         Request mockRequest = createRequestWithLongitudeLatitude(-87d, 41d);
         requestWithOsmFilters(mockRequest, "foo", ":!bar");
-        ReverseRequestFactory reverseRequestFactory = new ReverseRequestFactory(Collections.singletonList("en"), "en");
+        ReverseRequestFactory reverseRequestFactory = new ReverseRequestFactory(Collections.singletonList("en"), "en", 10);
         reverseRequest = reverseRequestFactory.create(mockRequest);
 
         List<TagFilter> result = reverseRequest.getOsmTagFilters();
@@ -242,7 +242,7 @@ void testTagFilters() throws Exception {
     void testBadTagFilters() {
         Request mockRequest = createRequestWithLongitudeLatitude(-87d, 41d);
         requestWithOsmFilters(mockRequest, "good", "bad:bad:bad");
-        ReverseRequestFactory reverseRequestFactory = new ReverseRequestFactory(Collections.singletonList("en"), "en");
+        ReverseRequestFactory reverseRequestFactory = new ReverseRequestFactory(Collections.singletonList("en"), "en", 10);
 
         assertThrows(BadRequestException.class, () -> reverseRequestFactory.create(mockRequest));
     }
@@ -252,7 +252,7 @@ void testWithDebug() throws Exception {
         Request mockRequest = createRequestWithLongitudeLatitude(-87d, 41d);
         Mockito.when(mockRequest.queryParams("debug")).thenReturn("1");
 
-        ReverseRequestFactory reverseRequestFactory = new ReverseRequestFactory(Collections.singletonList("en"), "en");
+        ReverseRequestFactory reverseRequestFactory = new ReverseRequestFactory(Collections.singletonList("en"), "en", 10);
         reverseRequest = reverseRequestFactory.create(mockRequest);
 
         assertTrue(reverseRequest.getDebug());