diff --git a/src/main/java/org/codelibs/fess/ingest/IngestFactory.java b/src/main/java/org/codelibs/fess/ingest/IngestFactory.java index 34010e777..9601429af 100644 --- a/src/main/java/org/codelibs/fess/ingest/IngestFactory.java +++ b/src/main/java/org/codelibs/fess/ingest/IngestFactory.java @@ -15,24 +15,34 @@ */ package org.codelibs.fess.ingest; +import java.util.ArrayList; import java.util.Arrays; +import java.util.Comparator; +import java.util.List; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; /** * Factory class for managing and organizing document ingesters. - * The factory maintains a sorted collection of ingesters based on their priority - * and provides methods to add new ingesters and retrieve the current collection. + * The factory maintains a sorted array of ingesters based on their priority. * - * Ingesters are automatically sorted by priority, with lower numbers having higher priority. + *

Ingesters are automatically sorted by priority, with lower numbers having higher priority. + * This class is designed for initialization-time registration only and is not thread-safe.

+ * + *

IMPORTANT: The {@code add()} method should only be called during + * the initialization phase (typically via DI container) before the factory is accessed + * by multiple threads. Runtime modification is not supported.

*/ public class IngestFactory { /** Logger instance for this class */ private static final Logger logger = LogManager.getLogger(IngestFactory.class); + /** Comparator for sorting ingesters by priority */ + private static final Comparator PRIORITY_COMPARATOR = Comparator.comparingInt(Ingester::getPriority); + /** Array of registered ingesters, sorted by priority */ - private Ingester[] ingesters = {}; + private Ingester[] ingesters = new Ingester[0]; /** * Default constructor. @@ -43,19 +53,39 @@ public IngestFactory() { /** * Adds an ingester to the factory. - * The ingester is inserted into the collection and the array is re-sorted by priority. - * This method is thread-safe. + * If an ingester with the same class already exists, it will be replaced. * - * @param ingester the ingester to add + *

IMPORTANT: This method is NOT thread-safe. It should only + * be called during the initialization phase before the factory is used.

+ * + * @param ingester the ingester to add (must not be null) + * @throws IllegalArgumentException if ingester is null */ - public synchronized void add(final Ingester ingester) { + public void add(final Ingester ingester) { + if (ingester == null) { + throw new IllegalArgumentException("Ingester cannot be null"); + } + if (logger.isDebugEnabled()) { - logger.debug("Loaded {}", ingester.getClass().getSimpleName()); + logger.debug("Loading {}", ingester.getClass().getSimpleName()); + } + + // Convert to list for manipulation + final List list = new ArrayList<>(Arrays.asList(ingesters)); + + // Remove existing ingester of the same class if present + list.removeIf(existing -> existing.getClass().equals(ingester.getClass())); + + // Add new ingester and sort by priority + list.add(ingester); + list.sort(PRIORITY_COMPARATOR); + + // Update array + ingesters = list.toArray(new Ingester[0]); + + if (logger.isDebugEnabled()) { + logger.debug("Loaded {} with priority {}", ingester.getClass().getSimpleName(), ingester.getPriority()); } - final Ingester[] newIngesters = Arrays.copyOf(ingesters, ingesters.length + 1); - newIngesters[ingesters.length] = ingester; - Arrays.sort(newIngesters, (o1, o2) -> o1.priority - o2.priority); - ingesters = newIngesters; } /** @@ -63,7 +93,11 @@ public synchronized void add(final Ingester ingester) { * The returned array contains all ingesters in priority order * (lower priority numbers first). * - * @return the sorted array of ingesters + *

The array is returned directly for read-only access. Modifications + * to the array will not affect future calls to this method, but callers + * should treat the array as immutable.

+ * + * @return the array of ingesters */ public Ingester[] getIngesters() { return ingesters; diff --git a/src/main/java/org/codelibs/fess/ingest/Ingester.java b/src/main/java/org/codelibs/fess/ingest/Ingester.java index 0474e9704..b6536b3c0 100644 --- a/src/main/java/org/codelibs/fess/ingest/Ingester.java +++ b/src/main/java/org/codelibs/fess/ingest/Ingester.java @@ -29,12 +29,25 @@ * extract additional metadata, or perform other transformations during the * indexing process. * - * Ingesters are processed in priority order, with lower numbers having higher priority. + *

Ingesters are processed in priority order, with lower numbers having higher priority. + * Valid priority values range from 0 to 999, with a default value of 99.

+ * + *

Implementations should override the appropriate {@code process()} methods + * to implement their specific document transformation logic.

*/ public abstract class Ingester { + /** Minimum allowed priority value */ + public static final int MIN_PRIORITY = 0; + + /** Maximum allowed priority value */ + public static final int MAX_PRIORITY = 999; + + /** Default priority value */ + public static final int DEFAULT_PRIORITY = 99; + /** Priority of this ingester (lower numbers = higher priority) */ - protected int priority = 99; + protected int priority = DEFAULT_PRIORITY; /** * Default constructor. @@ -57,9 +70,14 @@ public int getPriority() { * Sets the priority of this ingester. * Lower numbers indicate higher priority. * - * @param priority the priority value to set + * @param priority the priority value to set (must be between 0 and 999) + * @throws IllegalArgumentException if priority is outside valid range */ public void setPriority(final int priority) { + if (priority < MIN_PRIORITY || priority > MAX_PRIORITY) { + throw new IllegalArgumentException( + "Priority must be between " + MIN_PRIORITY + " and " + MAX_PRIORITY + ", but was: " + priority); + } this.priority = priority; } @@ -128,4 +146,43 @@ protected Map process(final Map target) { return target; } + /** + * Indicates whether some other object is "equal to" this one. + * Two ingesters are considered equal if they are of the same class. + * + * @param obj the reference object with which to compare + * @return true if this object is the same as the obj argument; false otherwise + */ + @Override + public boolean equals(final Object obj) { + if (this == obj) { + return true; + } + if (obj == null || getClass() != obj.getClass()) { + return false; + } + return true; + } + + /** + * Returns a hash code value for the object. + * The hash code is based on the class type. + * + * @return a hash code value for this object + */ + @Override + public int hashCode() { + return getClass().hashCode(); + } + + /** + * Returns a string representation of this ingester. + * + * @return a string representation including class name and priority + */ + @Override + public String toString() { + return getClass().getSimpleName() + "{priority=" + priority + "}"; + } + } diff --git a/src/test/java/org/codelibs/fess/ingest/IngestFactoryTest.java b/src/test/java/org/codelibs/fess/ingest/IngestFactoryTest.java index 5a7834e9a..2eea08646 100644 --- a/src/test/java/org/codelibs/fess/ingest/IngestFactoryTest.java +++ b/src/test/java/org/codelibs/fess/ingest/IngestFactoryTest.java @@ -15,35 +15,162 @@ */ package org.codelibs.fess.ingest; +import java.util.Map; + import org.codelibs.fess.unit.UnitFessTestCase; +/** + * Test class for IngestFactory. + * Tests basic functionality of ingester registration and sorting. + */ public class IngestFactoryTest extends UnitFessTestCase { - public void test_add_1() { + public void test_add_sortedOrder() { IngestFactory factory = new IngestFactory(); factory.add(new TestIngester(1)); factory.add(new TestIngester(2)); factory.add(new TestIngester(3)); Ingester[] ingesters = factory.getIngesters(); + assertEquals(3, ingesters.length); assertEquals(1, ingesters[0].getPriority()); assertEquals(2, ingesters[1].getPriority()); assertEquals(3, ingesters[2].getPriority()); } - public void test_add_2() { + public void test_add_reversedOrder() { IngestFactory factory = new IngestFactory(); factory.add(new TestIngester(3)); factory.add(new TestIngester(2)); factory.add(new TestIngester(1)); Ingester[] ingesters = factory.getIngesters(); + assertEquals(3, ingesters.length); assertEquals(1, ingesters[0].getPriority()); assertEquals(2, ingesters[1].getPriority()); assertEquals(3, ingesters[2].getPriority()); } + public void test_add_duplicateClass() { + IngestFactory factory = new IngestFactory(); + factory.add(new TestIngester(1)); + factory.add(new TestIngester(2)); // Same class, should replace + Ingester[] ingesters = factory.getIngesters(); + assertEquals(1, ingesters.length); // Only one ingester should remain + assertEquals(2, ingesters[0].getPriority()); // Latest one should be kept + } + + public void test_add_null() { + IngestFactory factory = new IngestFactory(); + try { + factory.add(null); + fail("Expected IllegalArgumentException"); + } catch (IllegalArgumentException e) { + assertEquals("Ingester cannot be null", e.getMessage()); + } + } + + public void test_addSameClassDifferentPriorities() { + IngestFactory factory = new IngestFactory(); + factory.add(new TestIngester(10)); + assertEquals(1, factory.getIngesters().length); + assertEquals(10, factory.getIngesters()[0].getPriority()); + + // Add same class with different priority - should replace + factory.add(new TestIngester(5)); + assertEquals(1, factory.getIngesters().length); + assertEquals(5, factory.getIngesters()[0].getPriority()); + + // Add again with even different priority - should replace again + factory.add(new TestIngester(20)); + assertEquals(1, factory.getIngesters().length); + assertEquals(20, factory.getIngesters()[0].getPriority()); + } + + public void test_multipleDifferentClassesSamePriorityStability() { + IngestFactory factory = new IngestFactory(); + + // Add multiple ingesters with same priority + TestIngester t1 = new TestIngester(5); + AnotherTestIngester t2 = new AnotherTestIngester(5); + ThirdTestIngester t3 = new ThirdTestIngester(5); + + factory.add(t1); + factory.add(t2); + factory.add(t3); + + Ingester[] ingesters = factory.getIngesters(); + assertEquals(3, ingesters.length); + + // All should have same priority + assertEquals(5, ingesters[0].getPriority()); + assertEquals(5, ingesters[1].getPriority()); + assertEquals(5, ingesters[2].getPriority()); + } + + public void test_largeNumberOfIngesters() { + IngestFactory factory = new IngestFactory(); + + // Add 100 ingesters with different priorities + for (int i = 0; i < 100; i++) { + if (i % 3 == 0) { + factory.add(new TestIngester(i)); + } else if (i % 3 == 1) { + factory.add(new AnotherTestIngester(i)); + } else { + factory.add(new ThirdTestIngester(i)); + } + } + + // Verify sorting and that ingesters were added + Ingester[] ingesters = factory.getIngesters(); + assertTrue(ingesters.length > 0); + assertTrue(ingesters.length <= 100); + + for (int i = 1; i < ingesters.length; i++) { + assertTrue("Ingesters not sorted at index " + i, ingesters[i - 1].getPriority() <= ingesters[i].getPriority()); + } + } + + public void test_addMultiple_samePriority() { + IngestFactory factory = new IngestFactory(); + factory.add(new TestIngester(1)); + factory.add(new AnotherTestIngester(1)); + factory.add(new ThirdTestIngester(1)); + + Ingester[] ingesters = factory.getIngesters(); + assertEquals(3, ingesters.length); + // All should have same priority + assertEquals(1, ingesters[0].getPriority()); + assertEquals(1, ingesters[1].getPriority()); + assertEquals(1, ingesters[2].getPriority()); + } + private static class TestIngester extends Ingester { public TestIngester(int priority) { this.priority = priority; } + + @Override + protected Map process(Map target) { + target.put("test", "value"); + return target; + } + } + + private static class AnotherTestIngester extends Ingester { + public AnotherTestIngester(int priority) { + this.priority = priority; + } + + @Override + protected Map process(Map target) { + target.put("another", "value"); + return target; + } + } + + private static class ThirdTestIngester extends Ingester { + public ThirdTestIngester(int priority) { + this.priority = priority; + } } } diff --git a/src/test/java/org/codelibs/fess/ingest/IngesterTest.java b/src/test/java/org/codelibs/fess/ingest/IngesterTest.java index e7087c5d4..b76fca619 100644 --- a/src/test/java/org/codelibs/fess/ingest/IngesterTest.java +++ b/src/test/java/org/codelibs/fess/ingest/IngesterTest.java @@ -15,6 +15,13 @@ */ package org.codelibs.fess.ingest; +import java.util.HashMap; +import java.util.Map; + +import org.codelibs.fess.crawler.entity.AccessResult; +import org.codelibs.fess.crawler.entity.ResponseData; +import org.codelibs.fess.crawler.entity.ResultData; +import org.codelibs.fess.entity.DataStoreParams; import org.codelibs.fess.unit.UnitFessTestCase; public class IngesterTest extends UnitFessTestCase { @@ -29,32 +36,418 @@ public void tearDown() throws Exception { super.tearDown(); } - // Basic test to verify test framework is working - public void test_basicAssertion() { - assertTrue(true); - assertFalse(false); - assertNotNull("test"); - assertEquals(1, 1); + public void test_defaultPriority() { + TestIngester ingester = new TestIngester(); + assertEquals(Ingester.DEFAULT_PRIORITY, ingester.getPriority()); + assertEquals(99, ingester.getPriority()); + } + + public void test_setPriority_valid() { + TestIngester ingester = new TestIngester(); + ingester.setPriority(50); + assertEquals(50, ingester.getPriority()); + + ingester.setPriority(Ingester.MIN_PRIORITY); + assertEquals(0, ingester.getPriority()); + + ingester.setPriority(Ingester.MAX_PRIORITY); + assertEquals(999, ingester.getPriority()); + } + + public void test_setPriority_invalid_tooLow() { + TestIngester ingester = new TestIngester(); + try { + ingester.setPriority(-1); + fail("Expected IllegalArgumentException"); + } catch (IllegalArgumentException e) { + assertTrue(e.getMessage().contains("Priority must be between")); + } + } + + public void test_setPriority_invalid_tooHigh() { + TestIngester ingester = new TestIngester(); + try { + ingester.setPriority(1000); + fail("Expected IllegalArgumentException"); + } catch (IllegalArgumentException e) { + assertTrue(e.getMessage().contains("Priority must be between")); + } + } + + public void test_process_resultData() { + TestIngester ingester = new TestIngester(); + ResultData resultData = new ResultData(); + ResponseData responseData = new ResponseData(); + + ResultData processed = ingester.process(resultData, responseData); + assertNotNull(processed); + assertSame(resultData, processed); // Default implementation returns same object + } + + public void test_process_mapWithAccessResult() { + TestIngester ingester = new TestIngester(); + Map target = new HashMap<>(); + target.put("url", "http://example.com"); + + // AccessResult is abstract, so we create a simple concrete implementation for testing + @SuppressWarnings("unchecked") + AccessResult accessResult = new AccessResult() { + @Override + public void setLastModified(Long lastModified) { + // Stub implementation for testing + } + + @Override + public Long getLastModified() { + // Stub implementation for testing + return null; + } + + @Override + public void setContentLength(Long contentLength) { + // Stub implementation for testing + } + + @Override + public Long getContentLength() { + // Stub implementation for testing + return null; + } + + @Override + public void setAccessResultData(org.codelibs.fess.crawler.entity.AccessResultData data) { + // Stub implementation for testing + } + + @Override + public org.codelibs.fess.crawler.entity.AccessResultData getAccessResultData() { + // Stub implementation for testing + return null; + } + + @Override + public void setExecutionTime(Integer executionTime) { + // Stub implementation for testing + } + + @Override + public Integer getExecutionTime() { + // Stub implementation for testing + return null; + } + }; + + Map processed = ingester.process(target, accessResult); + assertNotNull(processed); + assertSame(target, processed); // Default delegates to basic process + } + + public void test_process_mapWithDataStoreParams() { + TestIngester ingester = new TestIngester(); + Map target = new HashMap<>(); + target.put("id", "123"); + + DataStoreParams params = new DataStoreParams(); + + Map processed = ingester.process(target, params); + assertNotNull(processed); + assertSame(target, processed); // Default delegates to basic process + } + + public void test_equals_sameInstance() { + TestIngester ingester = new TestIngester(); + assertTrue(ingester.equals(ingester)); + } + + public void test_equals_sameClass() { + TestIngester ingester1 = new TestIngester(); + TestIngester ingester2 = new TestIngester(); + assertTrue(ingester1.equals(ingester2)); + assertTrue(ingester2.equals(ingester1)); + } + + public void test_equals_differentClass() { + TestIngester ingester1 = new TestIngester(); + AnotherTestIngester ingester2 = new AnotherTestIngester(); + assertFalse(ingester1.equals(ingester2)); + } + + public void test_equals_null() { + TestIngester ingester = new TestIngester(); + assertFalse(ingester.equals(null)); + } + + public void test_hashCode_differentClass() { + TestIngester ingester1 = new TestIngester(); + AnotherTestIngester ingester2 = new AnotherTestIngester(); + // Different classes should have different hash codes + assertFalse(ingester1.hashCode() == ingester2.hashCode()); + } + + public void test_toString() { + TestIngester ingester = new TestIngester(); + ingester.setPriority(42); + String str = ingester.toString(); + assertNotNull(str); + assertTrue(str.contains("TestIngester")); + assertTrue(str.contains("42")); + } + + public void test_customProcess() { + CustomProcessIngester ingester = new CustomProcessIngester(); + Map target = new HashMap<>(); + + Map processed = ingester.process(target, (DataStoreParams) null); + assertNotNull(processed); + assertTrue(processed.containsKey("processed")); + assertEquals("true", processed.get("processed")); + } + + public void test_setPriority_boundaryValues() { + TestIngester ingester = new TestIngester(); + + // Test minimum boundary + ingester.setPriority(Ingester.MIN_PRIORITY); + assertEquals(Ingester.MIN_PRIORITY, ingester.getPriority()); + + // Test maximum boundary + ingester.setPriority(Ingester.MAX_PRIORITY); + assertEquals(Ingester.MAX_PRIORITY, ingester.getPriority()); + + // Test middle value + ingester.setPriority(500); + assertEquals(500, ingester.getPriority()); + } + + public void test_setPriority_extremeInvalidValues() { + TestIngester ingester = new TestIngester(); + + // Test very large negative value + try { + ingester.setPriority(Integer.MIN_VALUE); + fail("Expected IllegalArgumentException"); + } catch (IllegalArgumentException e) { + assertTrue(e.getMessage().contains("Priority must be between")); + } + + // Test very large positive value + try { + ingester.setPriority(Integer.MAX_VALUE); + fail("Expected IllegalArgumentException"); + } catch (IllegalArgumentException e) { + assertTrue(e.getMessage().contains("Priority must be between")); + } + } + + public void test_equals_symmetry() { + TestIngester ingester1 = new TestIngester(); + TestIngester ingester2 = new TestIngester(); + + // Test symmetry: if a.equals(b), then b.equals(a) + assertTrue(ingester1.equals(ingester2)); + assertTrue(ingester2.equals(ingester1)); + } + + public void test_equals_transitivity() { + TestIngester ingester1 = new TestIngester(); + TestIngester ingester2 = new TestIngester(); + TestIngester ingester3 = new TestIngester(); + + // Test transitivity: if a.equals(b) and b.equals(c), then a.equals(c) + assertTrue(ingester1.equals(ingester2)); + assertTrue(ingester2.equals(ingester3)); + assertTrue(ingester1.equals(ingester3)); + } + + public void test_equals_consistency() { + TestIngester ingester1 = new TestIngester(); + TestIngester ingester2 = new TestIngester(); + + // Multiple invocations should return same result + assertTrue(ingester1.equals(ingester2)); + assertTrue(ingester1.equals(ingester2)); + assertTrue(ingester1.equals(ingester2)); + } + + public void test_equals_differentPrioritiesSameClass() { + TestIngester ingester1 = new TestIngester(); + ingester1.setPriority(10); + + TestIngester ingester2 = new TestIngester(); + ingester2.setPriority(20); + + // Should still be equal since same class + assertTrue(ingester1.equals(ingester2)); + } + + public void test_hashCode_equalObjectsSameHashCode() { + TestIngester ingester1 = new TestIngester(); + ingester1.setPriority(10); + + TestIngester ingester2 = new TestIngester(); + ingester2.setPriority(20); + + // Equal objects must have same hash code + assertTrue(ingester1.equals(ingester2)); + assertEquals(ingester1.hashCode(), ingester2.hashCode()); + } + + public void test_hashCode_consistency() { + TestIngester ingester = new TestIngester(); + + int hash1 = ingester.hashCode(); + int hash2 = ingester.hashCode(); + int hash3 = ingester.hashCode(); + + // hashCode should be consistent across invocations + assertEquals(hash1, hash2); + assertEquals(hash2, hash3); + } + + public void test_hashCode_notAffectedByPriority() { + TestIngester ingester = new TestIngester(); + ingester.setPriority(10); + int hash1 = ingester.hashCode(); + + ingester.setPriority(20); + int hash2 = ingester.hashCode(); + + // Hash code should not change when priority changes + assertEquals(hash1, hash2); + } + + public void test_toString_includesPriorityAndClass() { + TestIngester ingester = new TestIngester(); + ingester.setPriority(123); + + String str = ingester.toString(); + + assertNotNull(str); + assertTrue(str.contains("TestIngester")); + assertTrue(str.contains("123")); + assertTrue(str.contains("priority")); } - // Test placeholder for future implementation - public void test_placeholder() { - // This test verifies the test class can be instantiated and run - String testValue = "test"; - assertNotNull(testValue); - assertEquals("test", testValue); + public void test_toString_differentPriorities() { + TestIngester ingester1 = new TestIngester(); + ingester1.setPriority(10); + + TestIngester ingester2 = new TestIngester(); + ingester2.setPriority(20); + + String str1 = ingester1.toString(); + String str2 = ingester2.toString(); + + assertNotNull(str1); + assertNotNull(str2); + assertFalse(str1.equals(str2)); // Should be different due to different priorities } - // Additional test for coverage - public void test_additionalCoverage() { - int a = 5; - int b = 10; - int sum = a + b; - assertEquals(15, sum); + public void test_process_chainedCalls() { + ChainedProcessIngester ingester = new ChainedProcessIngester(); + Map target = new HashMap<>(); + target.put("initial", "value"); + + // Process multiple times with different params + Map result1 = ingester.process(target, (DataStoreParams) null); + Map result2 = ingester.process(result1, (AccessResult) null); + + assertNotNull(result2); + assertTrue(result2.containsKey("initial")); + assertTrue(result2.containsKey("chained")); + assertEquals(2, result2.get("count")); + } + + public void test_process_nullTarget() { + NullSafeIngester ingester = new NullSafeIngester(); + + // Process with null target - implementation should handle gracefully + Map result = ingester.process(null, (DataStoreParams) null); + // Implementation might return null or empty map + // This tests that no exception is thrown + } + + public void test_process_emptyTarget() { + CustomProcessIngester ingester = new CustomProcessIngester(); + Map target = new HashMap<>(); + + Map result = ingester.process(target, (DataStoreParams) null); + assertNotNull(result); + assertTrue(result.containsKey("processed")); + } + + public void test_process_largeTarget() { + CustomProcessIngester ingester = new CustomProcessIngester(); + Map target = new HashMap<>(); + + // Add many fields + for (int i = 0; i < 1000; i++) { + target.put("field" + i, "value" + i); + } + + Map result = ingester.process(target, (DataStoreParams) null); + assertNotNull(result); + assertTrue(result.size() >= 1000); + assertTrue(result.containsKey("processed")); + } + + public void test_defaultConstructor() { + TestIngester ingester = new TestIngester(); + assertNotNull(ingester); + assertEquals(Ingester.DEFAULT_PRIORITY, ingester.getPriority()); + } + + public void test_priorityConstants() { + assertEquals(0, Ingester.MIN_PRIORITY); + assertEquals(999, Ingester.MAX_PRIORITY); + assertEquals(99, Ingester.DEFAULT_PRIORITY); + + // Verify logical ordering + assertTrue(Ingester.MIN_PRIORITY < Ingester.DEFAULT_PRIORITY); + assertTrue(Ingester.DEFAULT_PRIORITY < Ingester.MAX_PRIORITY); + } + + // Test helper classes + private static class TestIngester extends Ingester { + public TestIngester() { + super(); + } + } + + private static class AnotherTestIngester extends Ingester { + public AnotherTestIngester() { + super(); + } + } + + private static class CustomProcessIngester extends Ingester { + @Override + protected Map process(Map target) { + target.put("processed", "true"); + return target; + } + } + + private static class ChainedProcessIngester extends Ingester { + private int processCount = 0; + + @Override + protected Map process(Map target) { + processCount++; + target.put("chained", "true"); + target.put("count", processCount); + return target; + } + } - String str = "Hello"; - assertTrue(str.startsWith("H")); - assertTrue(str.endsWith("o")); - assertEquals(5, str.length()); + private static class NullSafeIngester extends Ingester { + @Override + protected Map process(Map target) { + if (target == null) { + return new HashMap<>(); + } + target.put("nullsafe", "true"); + return target; + } } } \ No newline at end of file