fix: display an accurate number of total hits for search (#1716)

* fix: track total hits when doing an elastic search query to get an accurate number of total hits, cleanup

* add null check due to mocks in unit tests

* move maxResultWindow initialization to application event listener

Author: netomi

server/src/main/java/org/eclipse/openvsx/search/ElasticSearchService.java

@@ -41,7 +41,7 @@
 import org.springframework.data.elasticsearch.core.query.IndexQueryBuilder;
 import org.springframework.retry.annotation.Retryable;
 import org.springframework.scheduling.annotation.Async;
-import org.springframework.stereotype.Component;
+import org.springframework.stereotype.Service;
 import org.springframework.util.StopWatch;
 
 import java.time.ZoneId;
@@ -52,7 +52,7 @@
 
 import static org.eclipse.openvsx.cache.CacheService.CACHE_AVERAGE_REVIEW_RATING;
 
-@Component
+@Service
 public class ElasticSearchService implements ISearchService {
 
     protected final ReadWriteLock rwLock = new ReentrantReadWriteLock();
@@ -68,7 +68,7 @@ public class ElasticSearchService implements ISearchService {
     @Value("${ovsx.elasticsearch.clear-on-start:false}")
     boolean clearOnStart;
 
-    private Long maxResultWindow;
+    private long maxResultWindow;
 
     public ElasticSearchService(
             RepositoryService repositories,
@@ -81,7 +81,7 @@ public ElasticSearchService(
         this.relevanceService = relevanceService;
         this.scheduler = scheduler;
     }
-    
+
     public boolean isEnabled() {
         return enableSearch;
     }
@@ -90,29 +90,43 @@ public boolean isEnabled() {
      * Application start listener that initializes the search index. If the application property
      * {@code ovsx.elasticsearch.clear-on-start} is set to {@code true}, the index is cleared
      * and rebuilt from scratch. If the property is {@code false} and the search index does
-     * not exist yet, it is created and initialized. Otherwise nothing happens.
+     * not exist yet, it is created and initialized. Otherwise, nothing happens.
      */
     @EventListener
-    @Retryable(DataAccessResourceFailureException.class)
+    @Retryable(retryFor = DataAccessResourceFailureException.class)
     @CacheEvict(value = CACHE_AVERAGE_REVIEW_RATING, allEntries = true)
     public void initSearchIndex(ApplicationStartedEvent event) {
-        scheduler.scheduleRecurrently("ElasticSearchUpdateIndex", Cron.daily(4), ZoneId.of("UTC"), new HandlerJobRequest<>(ElasticSearchUpdateIndexJobRequestHandler.class));
-        if (!isEnabled() || !clearOnStart && searchOperations.indexOps(ExtensionSearch.class).exists()) {
+        if (!isEnabled()) {
+            scheduler.deleteRecurringJob("ElasticSearchUpdateIndex");
             return;
         }
-        var stopWatch = new StopWatch();
-        stopWatch.start();
-        updateSearchIndex(clearOnStart);
-        stopWatch.stop();
-        logger.info("Initialized search index in {} ms", stopWatch.getTotalTimeMillis());
+
+        // schedule recurring job to update the search index
+        scheduler.scheduleRecurrently(
+                "ElasticSearchUpdateIndex",
+                Cron.daily(4),
+                ZoneId.of("UTC"),
+                new HandlerJobRequest<>(ElasticSearchUpdateIndexJobRequestHandler.class)
+        );
+
+        if (clearOnStart || !searchOperations.indexOps(ExtensionSearch.class).exists()) {
+            var stopWatch = new StopWatch();
+            stopWatch.start();
+            updateSearchIndex(clearOnStart);
+            stopWatch.stop();
+            logger.info("Initialized search index in {} ms", stopWatch.getTotalTimeMillis());
+        }
+
+        var settings = searchOperations.indexOps(ExtensionSearch.class).getSettings(true);
+        maxResultWindow = Long.parseLong(settings.getOrDefault("index.max_result_window", "10000").toString());
     }
 
     /**
      * Soft-update the search index, because the relevance of index entries
      * consider the extension publishing timestamps in relation to the current
      * time or the extension rating.
      */
-    @Retryable(DataAccessResourceFailureException.class)
+    @Retryable(retryFor = DataAccessResourceFailureException.class)
     @CacheEvict(value = CACHE_AVERAGE_REVIEW_RATING, allEntries = true)
     public void updateSearchIndex() {
         if (!isEnabled()) {
@@ -134,7 +148,7 @@ public void updateSearchIndex() {
      * In any case, this method scans all extensions in the database and indexes their
      * relevant metadata.
      */
-    @Retryable(DataAccessResourceFailureException.class)
+    @Retryable(retryFor = DataAccessResourceFailureException.class)
     public void updateSearchIndex(boolean clear) {
         var locked = false;
         try {
@@ -180,12 +194,12 @@ public void updateSearchIndex(boolean clear) {
     }
 
     @Async
-    @Retryable(DataAccessResourceFailureException.class)
+    @Retryable(retryFor = DataAccessResourceFailureException.class)
     public void updateSearchEntriesAsync(List<Extension> extensions) {
         updateSearchEntries(extensions);
     }
 
-    @Retryable(DataAccessResourceFailureException.class)
+    @Retryable(retryFor = DataAccessResourceFailureException.class)
     public void updateSearchEntries(List<Extension> extensions) {
         if (!isEnabled() || extensions.isEmpty()) {
             return;
@@ -205,7 +219,7 @@ public void updateSearchEntries(List<Extension> extensions) {
         }
     }
 
-    @Retryable(DataAccessResourceFailureException.class)
+    @Retryable(retryFor = DataAccessResourceFailureException.class)
     public void updateSearchEntry(Extension extension) {
         if (!isEnabled()) {
             return;
@@ -223,7 +237,7 @@ public void updateSearchEntry(Extension extension) {
         }
     }
 
-    @Retryable(DataAccessResourceFailureException.class)
+    @Retryable(retryFor = DataAccessResourceFailureException.class)
     public void removeSearchEntries(Collection<Long> ids) {
         if (!isEnabled()) {
             return;
@@ -235,7 +249,7 @@ public void removeSearchEntries(Collection<Long> ids) {
     }
 
 
-    @Retryable(DataAccessResourceFailureException.class)
+    @Retryable(retryFor = DataAccessResourceFailureException.class)
     public void removeSearchEntry(Extension extension) {
         if (!isEnabled()) {
             return;
@@ -251,7 +265,7 @@ public void removeSearchEntry(Extension extension) {
 
     public SearchResult search(Options options) {
         var resultWindow = options.requestedOffset() + options.requestedSize();
-        if(resultWindow > getMaxResultWindow()) {
+        if (resultWindow > maxResultWindow) {
             return new SearchResult(0L, Collections.emptyList());
         }
 
@@ -263,15 +277,16 @@ public SearchResult search(Options options) {
 
         var pages = new ArrayList<Pageable>();
         pages.add(PageRequest.of(options.requestedOffset() / options.requestedSize(), options.requestedSize()));
-        if(options.requestedOffset() % options.requestedSize() > 0) {
+        if (options.requestedOffset() % options.requestedSize() > 0) {
             // size is not exact multiple of offset; this means we need to get two pages
             // e.g. when offset is 20 and size is 50, you want results 20 to 70 which span pages 0 and 1 of a 50 item page
-            pages.add(pages.get(0).next());
+            pages.add(pages.getFirst().next());
         }
 
         var searchHitsList = new ArrayList<SearchHits<ExtensionSearch>>(pages.size());
-        for(var page : pages) {
+        for (var page : pages) {
             queryBuilder.withPageable(page);
+            queryBuilder.withTrackTotalHits(true);
             try {
                 rwLock.readLock().lock();
                 var searchHits = searchOperations.search(queryBuilder.build(), ExtensionSearch.class, searchOperations.indexOps(ExtensionSearch.class).getIndexCoordinates());
@@ -283,7 +298,7 @@ public SearchResult search(Options options) {
 
         var firstSearchHitsPage = searchHitsList.get(0);
         List<SearchHit<ExtensionSearch>> searchHits = new ArrayList<>(firstSearchHitsPage.getSearchHits());
-        if(searchHitsList.size() == 2) {
+        if (searchHitsList.size() == 2) {
             var secondSearchHitsPage = searchHitsList.get(1);
 
             searchHits.addAll(secondSearchHitsPage.getSearchHits());
@@ -383,7 +398,7 @@ private void sortResults(NativeQueryBuilder queryBuilder, String sortOrder, Stri
         );
 
         var type = types.get(sortBy);
-        if(type == null) {
+        if (type == null) {
             throw new ErrorResultException("sortBy parameter must be " + SortBy.OPTIONS + ".");
         }
 
@@ -392,13 +407,4 @@ private void sortResults(NativeQueryBuilder queryBuilder, String sortOrder, Stri
         var sortOptions = sortBy.equals(SortBy.RELEVANCE) ? List.of(scoreSort, fieldSort) : List.of(fieldSort, scoreSort);
         queryBuilder.withSort(sortOptions);
     }
-
-    private long getMaxResultWindow() {
-        if(maxResultWindow == null) {
-            var settings = searchOperations.indexOps(ExtensionSearch.class).getSettings(true);
-            maxResultWindow = Long.parseLong(settings.getOrDefault("index.max_result_window", "10000").toString());
-        }
-
-        return maxResultWindow;
-    }
 }