Improve rank fusion implementation: thread safety, error handling, and code quality (#2955)

* Improve Rank Fusion implementation

This commit enhances the rank fusion implementation with several improvements:

1. Fix typo: Rename setSeacher() to setSearcher()
   - Updated method name in RankFusionProcessor
   - Updated DI configuration in fess_rankfusion.xml
   - Updated all test usages in RankFusionProcessorTest

2. Improve thread safety
   - Replace array-based searchers with CopyOnWriteArrayList
   - Eliminates race conditions during searcher registration
   - Removes inefficient Arrays.copyOf() pattern

3. Enhance error handling and logging
   - Separate InterruptedException and ExecutionException handling
   - Restore interrupt status when interrupted
   - Log actual exception causes for better debugging
   - Add more descriptive debug messages throughout

4. Improve code readability
   - Replace ambiguous lambda parameters (n, i, j) with descriptive names
   - Add inline comments explaining RRF score calculation
   - Improve variable names (scoreDocMap -> documentsByIdMap, etc.)
   - Clarify window size validation logic
   - Use method references instead of lambda expressions where applicable

These changes maintain backward compatibility while improving code quality,
maintainability, and thread safety of the rank fusion system.

* Improve log message clarity in RankFusionProcessor

- Change "minimum recommended size" to "required minimum size" in warning
  message, as the code enforces this as a hard requirement
- Change "using first searcher" to "falling back to default searcher" for
  better clarity about which searcher is being used

* Add comprehensive test suite for rank fusion improvements

Add extensive test coverage for the rank fusion implementation improvements:

1. RankFusionProcessorConcurrencyTest.java
   - Test concurrent searcher registration from multiple threads
   - Test search operations during searcher registration
   - Test concurrent setSearcher and register operations
   - Test multiple concurrent search operations
   - Verifies CopyOnWriteArrayList thread safety improvements

2. RankFusionProcessorEdgeCaseTest.java
   - Test empty searcher list handling
   - Test setSearcher on empty and non-empty lists
   - Test zero and very large page sizes
   - Test search beyond available documents
   - Test negative start positions
   - Test searchers returning no/single/mixed results
   - Test null and empty query strings
   - Test duplicate document IDs across searchers
   - Test multiple close() calls

3. RankFusionProcessorErrorHandlingTest.java
   - Test handling of searchers throwing exceptions
   - Test documents with missing/null/non-string ID fields
   - Test all searchers failing gracefully
   - Test slow searcher handling (timeout scenarios)
   - Verifies improved error handling with separate exception types

4. SearchResultTest.java
   - Test SearchResult builder pattern
   - Test default values and various field combinations
   - Test document list operations
   - Test immutability and builder reuse

5. RankFusionSearcherTest.java
   - Test getName() derivation from class name
   - Test name caching and inheritance
   - Test abstract search method implementation

These tests provide comprehensive coverage of:
- Thread safety improvements (CopyOnWriteArrayList)
- Error handling improvements (InterruptedException, ExecutionException)
- Edge cases and boundary conditions
- Builder pattern correctness
- API contract verification

Total test methods added: 40+

* Fix NullPointerException in rank fusion tests

1. RankFusionProcessor.java (line 309)
   - Add null check before accessing document fields
   - Prevents NPE when searcher returns null documents
   - Change: if (doc.get(idField)) -> if (doc != null && doc.get(idField))
   - Fixes: RankFusionProcessorErrorHandlingTest.test_searcherReturnsNullDocuments

2. FacetResponse.java (line 55)
   - Add null check for aggregations parameter in constructor
   - Allows FacetResponse to be created with null aggregations
   - Update JavaDoc to document null parameter support
   - Fixes: SearchResultTest.test_searchResultWithFacetResponse
   - Fixes: SearchResultTest.test_allFieldsPopulated

These defensive checks improve robustness when handling edge cases
in error scenarios and test conditions.

* Fix ArithmeticException and RuntimeException in rank fusion tests

This commit addresses test failures in RankFusionProcessorEdgeCaseTest
and RankFusionProcessorErrorHandlingTest:

1. Fix ArithmeticException: / by zero in test_emptySearcherList
   - Add check in search() method for empty searcher array
   - Return empty result list when no searchers are available
   - Add defensive check in searchWithMultipleSearchers() to prevent
     division by zero: windowSize / searchers.length
   - Fixes: RankFusionProcessorEdgeCaseTest.test_emptySearcherList

2. Fix RuntimeException in test_allSearchersFail
   - Wrap searcher.search() call in try-catch in searchWithMainSearcher()
   - Return empty result when main searcher throws exception
   - Log error with full exception details for debugging
   - Fixes: RankFusionProcessorErrorHandlingTest.test_allSearchersFail

These defensive programming improvements ensure graceful degradation:
- Empty searcher lists return empty results instead of crashing
- Failed searchers are logged and handled without propagating exceptions
- Consistent empty result format across all failure scenarios

All fixes maintain backward compatibility and improve system robustness
when dealing with edge cases and error conditions.

* Change log level from error to warn for search failures

Adjust logging severity for non-critical search operation failures:

1. ExecutionException in concurrent search operations (line 303)
   - Change: logger.error -> logger.warn
   - Reason: Individual searcher failures don't stop the system

2. Main searcher exceptions in searchWithMainSearcher (line 421)
   - Change: logger.error -> logger.warn
   - Reason: Search failures are recoverable, return empty results

Rationale:
- logger.error should be reserved for critical system failures
- Search operation failures are recoverable and don't require system shutdown
- These scenarios gracefully degrade by returning empty results
- warn level is appropriate for operational issues that don't prevent
  the system from continuing to function

This change improves log clarity by distinguishing between recoverable
search failures (warn) and critical system errors (error).

---------

Co-authored-by: Claude <[email protected]>

Author: marevol

src/main/java/org/codelibs/fess/rank/fusion/DefaultSearcher.java

@@ -117,7 +117,7 @@ protected SearchResult processResponse(final OptionalEntity<SearchResponse> sear
                                     docMap.put(fessConfig.getQueryCollapseInnerHitsName() + "_hash", bitsField.getValues().get(0));
                                 }
                                 docMap.put(fessConfig.getQueryCollapseInnerHitsName(), StreamUtil.stream(innerSearchHits.getHits())
-                                        .get(stream -> stream.map(v -> parseSearchHit(fessConfig, hlPrefix, v)).toArray(n -> new Map[n])));
+                                        .get(stream -> stream.map(hit -> parseSearchHit(fessConfig, hlPrefix, hit)).toArray(Map[]::new)));
                             }
                         }
                     }
@@ -227,7 +227,7 @@ protected Map<String, Object> parseSearchHit(final FessConfig fessConfig, final
                 });
                 if (Constants.TEXT_FRAGMENT_TYPE_HIGHLIGHT.equals(fessConfig.getQueryHighlightTextFragmentType())) {
                     docMap.put(Constants.TEXT_FRAGMENTS,
-                            viewHelper.createTextFragmentsByHighlight(highlightFields.values().toArray(n -> new HighlightField[n])));
+                            viewHelper.createTextFragmentsByHighlight(highlightFields.values().toArray(HighlightField[]::new)));
                 }
             }
         } catch (final Exception e) {

src/main/java/org/codelibs/fess/rank/fusion/RankFusionProcessor.java

@@ -49,20 +49,22 @@ public class FacetResponse {
      * Constructs a FacetResponse from OpenSearch aggregations.
      * Processes both field facets and query facets from the aggregation results.
      *
-     * @param aggregations the OpenSearch aggregations containing facet data
+     * @param aggregations the OpenSearch aggregations containing facet data, may be null
      */
     public FacetResponse(final Aggregations aggregations) {
-        aggregations.forEach(aggregation -> {
-            if (aggregation.getName().startsWith(Constants.FACET_FIELD_PREFIX)) {
-                final Terms termFacet = (Terms) aggregation;
-                fieldList.add(new Field(termFacet));
-            } else if (aggregation.getName().startsWith(Constants.FACET_QUERY_PREFIX)) {
-                final Filter queryFacet = (Filter) aggregation;
-                final String encodedQuery = queryFacet.getName().substring(Constants.FACET_QUERY_PREFIX.length());
-                queryCountMap.put(new String(BaseEncoding.base64().decode(encodedQuery), StandardCharsets.UTF_8), queryFacet.getDocCount());
-            }
-
-        });
+        if (aggregations != null) {
+            aggregations.forEach(aggregation -> {
+                if (aggregation.getName().startsWith(Constants.FACET_FIELD_PREFIX)) {
+                    final Terms termFacet = (Terms) aggregation;
+                    fieldList.add(new Field(termFacet));
+                } else if (aggregation.getName().startsWith(Constants.FACET_QUERY_PREFIX)) {
+                    final Filter queryFacet = (Filter) aggregation;
+                    final String encodedQuery = queryFacet.getName().substring(Constants.FACET_QUERY_PREFIX.length());
+                    queryCountMap.put(new String(BaseEncoding.base64().decode(encodedQuery), StandardCharsets.UTF_8), queryFacet.getDocCount());
+                }
+
+            });
+        }
     }
 
     /**

src/main/java/org/codelibs/fess/util/FacetResponse.java

@@ -4,7 +4,7 @@
 <components>
 	<component name="rankFusionProcessor"
 		class="org.codelibs.fess.rank.fusion.RankFusionProcessor">
-		<postConstruct name="setSeacher">
+		<postConstruct name="setSearcher">
 			<arg>
 				<component
 					class="org.codelibs.fess.rank.fusion.DefaultSearcher">