🐛 fix(search-api): stable result order and configurable per-file hit limit

apiary.apib

@@ -643,10 +643,12 @@ The repository path is relative to source root.
   + repository - repository path with native path separators (of the machine
   running the service) starting with path separator for which to return type
 
-## Search [/search{?full,def,symbol,path,hist,type,projects,maxresults,start,sort}]
+## Search [/search{?full,def,symbol,path,hist,type,projects,maxresults,start,sort,maxhitsperfile}]
 
 ## return search results [GET]
 
+The `results` map preserves the order of the `sort` parameter (Lucene scoring order for `relevancy`).
+
 + Parameters
   + full (optional, string) - full search field value to search for
   + def (optional, string) - definition field value to search for
@@ -657,6 +659,7 @@ The repository path is relative to source root.
   + projects (optional, string) - projects to search in
   + maxresults (optional, string) - maximum number of documents whose hits will be returned (default 1000)
   + start (optional, string) - start index from which to return results
+  + maxhitsperfile (optional, number) - maximum number of matching lines returned per file. 0 (default) returns all matching lines. Set to a positive integer to bound response size when a symbol appears many times per file.
   + sort: relevancy (optional, enum[string])
      + Enum
          + relevancy

opengrok-indexer/src/main/java/org/opengrok/indexer/search/SearchEngine.java

@@ -137,6 +137,11 @@ public class SearchEngine {
     private final char[] content = new char[1024 * 8];
     private String source;
     private String data;
+    /**
+     * Holds value of property maxHitsPerFile.
+     * 0 means unlimited (default).
+     */
+    private int maxHitsPerFile;
     int hitsPerPage = RuntimeEnvironment.getInstance().getHitsPerPage();
     int cachePages = RuntimeEnvironment.getInstance().getCachePages();
     int totalHits = 0;
@@ -505,8 +510,8 @@ public void results(int start, int end, List<Hit> ret) {
                             hasContext = sourceContext.getContext(
                                 new InputStreamReader(new FileInputStream(
                                 source + filename), StandardCharsets.UTF_8),
-                                null, null, null, filename, tags, nhits > 100,
-                                getDefinition() != null, ret, scopes);
+                                null, null, null, filename, tags, false,
+                                getDefinition() != null, ret, scopes, maxHitsPerFile);
                         } else if (AbstractAnalyzer.Genre.XREFABLE == genre && data != null && summarizer != null) {
                             int l;
                             /*
@@ -624,6 +629,26 @@ public void setFreetext(String freetext) {
         this.freetext = freetext;
     }
 
+    /**
+     * Getter for property maxHitsPerFile.
+     *
+     * @return Value of property maxHitsPerFile. 0 means unlimited.
+     */
+    public int getMaxHitsPerFile() {
+        return this.maxHitsPerFile;
+    }
+
+    /**
+     * Sets the maximum number of matching lines returned per file in {@link #results}.
+     * 0 (the default) returns all matching lines, consistent with the HTML search page.
+     * Callers that need to bound response size can pass a positive value to cap per-file hits.
+     *
+     * @param maxHitsPerFile maximum hits per file, or 0 for unlimited.
+     */
+    public void setMaxHitsPerFile(int maxHitsPerFile) {
+        this.maxHitsPerFile = maxHitsPerFile;
+    }
+
     /**
      * Getter for property history.
      *

opengrok-indexer/src/main/java/org/opengrok/indexer/search/context/Context.java

@@ -273,6 +273,29 @@ public boolean getContext(Reader in, Writer out, String urlPrefix,
     public boolean getContext(Reader in, Writer out, String urlPrefix,
             String morePrefix, String path, Definitions tags,
             boolean limit, boolean isDefSearch, List<Hit> hits, Scopes scopes) {
+        return getContext(in, out, urlPrefix, morePrefix, path, tags, limit, isDefSearch, hits, scopes, 0);
+    }
+
+    /**
+     * Get the context for a source file.
+     * Closes the given <var>in</var> reader on return.
+     *
+     * @param in File to be matched
+     * @param out to write the context
+     * @param urlPrefix URL prefix
+     * @param morePrefix to link to more... page
+     * @param path path of the file
+     * @param tags format to highlight defs.
+     * @param limit should the number of matching lines be limited?
+     * @param isDefSearch is definition search
+     * @param hits list of hits
+     * @param scopes scopes object
+     * @param maxHits maximum number of hits to return for this file (0 = unlimited)
+     * @return Did it get any matching context?
+     */
+    public boolean getContext(Reader in, Writer out, String urlPrefix,
+            String morePrefix, String path, Definitions tags,
+            boolean limit, boolean isDefSearch, List<Hit> hits, Scopes scopes, int maxHits) {
         if (m == null) {
             IOUtils.close(in);
             return false;
@@ -415,7 +438,8 @@ public boolean getContext(Reader in, Writer out, String urlPrefix,
             int matchState;
             int matchedLines = 0;
             while ((token = tokens.yylex()) != null && (!lim ||
-                    matchedLines < limitMaxLines)) {
+                    matchedLines < limitMaxLines) &&
+                    (maxHits <= 0 || matchedLines < maxHits)) {
                 for (LineMatcher lineMatcher : m) {
                     matchState = lineMatcher.match(token);
                     if (matchState == LineMatcher.MATCHED) {

opengrok-indexer/src/test/java/org/opengrok/indexer/search/SearchEngineTest.java

@@ -29,6 +29,7 @@
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
+import java.util.Map;
 import java.util.TreeSet;
 
 import org.junit.jupiter.api.AfterAll;
@@ -225,6 +226,62 @@ void testDefaultSortOrder() {
         assertNull(instance.getSortOrder(), "Default sort should be relevancy (null implies Lucene score ordering)");
     }
 
+    @Test
+    void testMaxHitsPerFileDefault() {
+        SearchEngine instance = new SearchEngine();
+        assertEquals(0, instance.getMaxHitsPerFile());
+    }
+
+    @Test
+    void testMaxHitsPerFileSetterGetter() {
+        SearchEngine instance = new SearchEngine();
+        instance.setMaxHitsPerFile(20);
+        assertEquals(20, instance.getMaxHitsPerFile());
+        instance.setMaxHitsPerFile(0);
+        assertEquals(0, instance.getMaxHitsPerFile());
+    }
+
+    @Test
+    void testUnlimitedHitsPerFileContainLineNumbers() {
+        SearchEngine instance = new SearchEngine();
+        instance.setFile("main.c");
+        instance.setFreetext("arguments");
+        instance.setMaxHitsPerFile(0);
+        int hitsCount = instance.search();
+        List<Hit> hits = new ArrayList<>();
+        instance.results(0, hitsCount, hits);
+        assertFalse(hits.isEmpty());
+        assertTrue(hits.stream().allMatch(h -> !h.getLineno().isEmpty()));
+        instance.destroy();
+    }
+
+    @Test
+    void testMaxHitsPerFileLimitsHitsPerFile() {
+        SearchEngine unlimited = new SearchEngine();
+        unlimited.setFile("main.c");
+        unlimited.setFreetext("printf");
+        unlimited.setMaxHitsPerFile(0);
+        int hitsCount = unlimited.search();
+        List<Hit> allHits = new ArrayList<>();
+        unlimited.results(0, hitsCount, allHits);
+        unlimited.destroy();
+
+        int maxPerFile = 1;
+        SearchEngine limited = new SearchEngine();
+        limited.setFile("main.c");
+        limited.setFreetext("printf");
+        limited.setMaxHitsPerFile(maxPerFile);
+        hitsCount = limited.search();
+        List<Hit> cappedHits = new ArrayList<>();
+        limited.results(0, hitsCount, cappedHits);
+        limited.destroy();
+
+        assertTrue(allHits.size() > cappedHits.size());
+        Map<String, Long> hitsPerFile = cappedHits.stream()
+                .collect(java.util.stream.Collectors.groupingBy(Hit::getPath, java.util.stream.Collectors.counting()));
+        hitsPerFile.values().forEach(count -> assertTrue(count <= maxPerFile));
+    }
+
     /* see https://github.com/oracle/opengrok/issues/2030
     @Test
     void testSearch() {

opengrok-web/src/main/java/org/opengrok/web/api/v1/controller/SearchController.java

@@ -48,6 +48,7 @@
 import java.time.Instant;
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -84,9 +85,10 @@ public SearchResult search(
             @QueryParam("maxresults") // Akin to QueryParameters.COUNT_PARAM
             @DefaultValue(MAX_RESULTS + "") final int maxResults,
             @QueryParam(QueryParameters.START_PARAM) @DefaultValue(0 + "") final int startDocIndex,
-            @QueryParam(QueryParameters.SORT_PARAM) @DefaultValue(DEFAULT_SORT_ORDER) final String sort
+            @QueryParam(QueryParameters.SORT_PARAM) @DefaultValue(DEFAULT_SORT_ORDER) final String sort,
+            @QueryParam("maxhitsperfile") @DefaultValue("0") final int maxHitsPerFile
     ) {
-        try (SearchEngineWrapper engine = new SearchEngineWrapper(full, def, symbol, path, hist, type, SortOrder.get(sort))) {
+        try (SearchEngineWrapper engine = new SearchEngineWrapper(full, def, symbol, path, hist, type, SortOrder.get(sort), maxHitsPerFile)) {
 
             if (!engine.isValid()) {
                 throw new WebApplicationException("Invalid request", Response.Status.BAD_REQUEST);
@@ -99,6 +101,7 @@ public SearchResult search(
             Map<String, List<SearchHit>> hits = engine.search(req, projects, startDocIndex, maxResults)
                     .stream()
                     .collect(Collectors.groupingBy(Hit::getPath,
+                            LinkedHashMap::new,
                             Collectors.mapping(h -> new SearchHit(h.getLine(), h.getLineno(), h.getTag()),
                                     Collectors.toList())));
 
@@ -123,7 +126,8 @@ private SearchEngineWrapper(
                 final String path,
                 final String hist,
                 final String type,
-                final SortOrder sortOrder
+                final SortOrder sortOrder,
+                final int maxHitsPerFile
         ) {
             engine.setFreetext(full);
             engine.setDefinition(def);
@@ -132,6 +136,7 @@ private SearchEngineWrapper(
             engine.setHistory(hist);
             engine.setType(type);
             engine.setSortOrder(sortOrder);
+            engine.setMaxHitsPerFile(maxHitsPerFile);
         }
 
         public List<Hit> search(

opengrok-web/src/main/java/org/opengrok/web/api/v1/filter/IncomingFilter.java

@@ -66,7 +66,7 @@ public class IncomingFilter implements ContainerRequestFilter, ConfigurationChan
     /**
      * Endpoint paths that are exempted from this filter.
      * @see SearchController#search(HttpServletRequest, String, String, String, String, String, String,
-     * java.util.List, int, int, String)
+     * java.util.List, int, int, String, int)
      * @see SuggesterController#getSuggestions(org.opengrok.web.api.v1.suggester.model.SuggesterQueryData)
      * @see SuggesterController#getConfig()
      */

opengrok-web/src/test/java/org/opengrok/web/api/v1/controller/SearchControllerTest.java

@@ -23,6 +23,7 @@
  */
 package org.opengrok.web.api.v1.controller;
 
+import jakarta.ws.rs.core.GenericType;
 import jakarta.ws.rs.core.Response;
 import org.glassfish.jersey.servlet.ServletContainer;
 import org.glassfish.jersey.test.DeploymentContext;
@@ -34,12 +35,20 @@
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.Test;
 import org.opengrok.indexer.configuration.RuntimeEnvironment;
+import org.opengrok.indexer.history.RepositoryFactory;
+import org.opengrok.indexer.index.Indexer;
 import org.opengrok.indexer.util.TestRepository;
+import org.opengrok.indexer.web.QueryParameters;
 import org.opengrok.web.api.v1.RestApp;
 
+import java.net.URL;
 import java.util.Collections;
+import java.util.List;
+import java.util.Map;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.opengrok.web.api.v1.filter.CorsFilter.ALLOW_CORS_HEADER;
 import static org.opengrok.web.api.v1.filter.CorsFilter.CORS_REQUEST_HEADER;
 
@@ -63,14 +72,20 @@ protected TestContainerFactory getTestContainerFactory() throws TestContainerExc
     public static void setUpClass() throws Exception {
         System.setProperty("sun.net.http.allowRestrictedHeaders", "true"); // necessary to test CORS from controllers
         repository = new TestRepository();
+        URL url = SearchControllerTest.class.getClassLoader().getResource("sources");
+        assertNotNull(url);
+        repository.create(url);
 
-        repository.create(SearchControllerTest.class.getClassLoader().getResource("sources"));
-
+        env.setSourceRoot(repository.getSourceRoot());
+        env.setDataRoot(repository.getDataRoot());
         env.setHistoryEnabled(false);
         env.setProjectsEnabled(true);
         env.setDefaultProjectsFromNames(Collections.singleton("__all__"));
-
         env.getSuggesterConfig().setRebuildCronConfig(null);
+        RepositoryFactory.initializeIgnoredNames(env);
+
+        Indexer.getInstance().prepareIndexer(env, true, true, null, null);
+        Indexer.getInstance().doIndexerExecution(null, null);
     }
 
     @AfterAll
@@ -86,4 +101,73 @@ void testSearchCors() {
                 .get();
         assertEquals("*", response.getHeaderString(ALLOW_CORS_HEADER));
     }
+
+    @Test
+    void testSearchReturnsResults() {
+        // "dump" is a method name defined in java/Main.java — verifies the index is live
+        // and the API returns a well-formed response.
+        GenericType<Map<String, Object>> type = new GenericType<>() { };
+        Response response = target(SearchController.PATH)
+                .queryParam(QueryParameters.FULL_SEARCH_PARAM, "dump")
+                .request()
+                .get();
+        assertEquals(Response.Status.OK.getStatusCode(), response.getStatus());
+        Map<String, Object> body = response.readEntity(type);
+        Object resultsObj = body.get("results");
+        assertNotNull(resultsObj);
+        @SuppressWarnings("unchecked")
+        Map<String, List<Map<String, Object>>> results =
+                (Map<String, List<Map<String, Object>>>) resultsObj;
+        assertFalse(results.isEmpty());
+    }
+
+    @Test
+    void testSearchResultsPreserveHitOrder() {
+        // Results presented by the search API must be stable w.r.t. file ordering across
+        // repeated calls, while preserving Lucene scoring order.
+        GenericType<Map<String, Object>> type = new GenericType<>() { };
+        String firstKey1 = getFirstResultKey(type);
+        String firstKey2 = getFirstResultKey(type);
+        assertNotNull(firstKey1);
+        assertEquals(firstKey1, firstKey2);
+    }
+
+    private String getFirstResultKey(GenericType<Map<String, Object>> type) {
+        Response response = target(SearchController.PATH)
+                .queryParam(QueryParameters.FULL_SEARCH_PARAM, "dump")
+                .request()
+                .get();
+        Map<String, Object> body = response.readEntity(type);
+        @SuppressWarnings("unchecked")
+        Map<String, ?> results = (Map<String, ?>) body.get("results");
+        if (results == null || results.isEmpty()) {
+            return null;
+        }
+        return results.keySet().iterator().next();
+    }
+
+    @Test
+    void testSearchResultsHaveLineNumbers() {
+        // maxhitsperfile defaults to 0 (unlimited) so all per-file hits carry a lineNumber.
+        // Restrict to Java source type to exclude XREFABLE binary files (jar, class, elf)
+        // whose hits go through the Summarizer path and never carry a lineNumber.
+        GenericType<Map<String, Object>> type = new GenericType<>() { };
+        Response response = target(SearchController.PATH)
+                .queryParam(QueryParameters.FULL_SEARCH_PARAM, "dump")
+                .queryParam(QueryParameters.TYPE_SEARCH_PARAM, "java")
+                .request()
+                .get();
+        Map<String, Object> body = response.readEntity(type);
+        @SuppressWarnings("unchecked")
+        Map<String, List<Map<String, Object>>> results =
+                (Map<String, List<Map<String, Object>>>) body.get("results");
+        assertNotNull(results);
+        assertFalse(results.isEmpty());
+        results.values().forEach(hits ->
+                hits.forEach(hit -> {
+                    assertNotNull(hit.get("lineNumber"));
+                    assertFalse(hit.get("lineNumber").toString().isEmpty());
+                })
+        );
+    }
 }