fabriziofortino commented on code in PR #826:
URL: https://github.com/apache/jackrabbit-oak/pull/826#discussion_r1082205408


##########
oak-run-commons/src/main/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/FlatFileNodeStoreBuilder.java:
##########
@@ -31,13 +32,16 @@
 import org.apache.jackrabbit.oak.query.NodeStateNodeTypeInfoProvider;
 import org.apache.jackrabbit.oak.spi.blob.BlobStore;
 import org.apache.jackrabbit.oak.spi.state.NodeStore;
+import org.jetbrains.annotations.Nullable;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import java.io.File;
+import java.io.FilenameFilter;

Review Comment:
   not used



##########
oak-run-commons/src/main/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/FlatFileSplitter.java:
##########
@@ -73,7 +74,9 @@ public class FlatFileSplitter {
     private Set<String> splitNodeTypeNames;
     private boolean useCompression = 
Boolean.parseBoolean(System.getProperty(OAK_INDEXER_USE_ZIP, "true"));
     private boolean useLZ4 = 
Boolean.parseBoolean(System.getProperty(OAK_INDEXER_USE_LZ4, "false"));
-
+    
+    static Predicate IS_SPLIT = (Predicate<File>) path -> 
path.getParent().endsWith(SPLIT_DIR_NAME);

Review Comment:
   I would change this with the following
   ```suggestion
       static Predicate<File> IS_SPLIT = path -> 
path.getParent().endsWith(SPLIT_DIR_NAME);
   ```



##########
oak-run-commons/src/test/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/FlatFileNodeStoreBuilderTest.java:
##########
@@ -60,60 +87,162 @@ public void defaultSortStrategy() throws Exception {
 
     @Test
     public void sortStrategyBasedOnSystemProperty() throws Exception {
-        try {
-            System.setProperty(OAK_INDEXER_SORT_STRATEGY_TYPE, 
FlatFileNodeStoreBuilder.SortStrategyType.TRAVERSE_WITH_SORT.toString());
-            FlatFileNodeStoreBuilder builder = new 
FlatFileNodeStoreBuilder(folder.getRoot())
-                    
.withNodeStateEntryTraverserFactory(nodeStateEntryTraverserFactory);
-            SortStrategy sortStrategy = 
builder.createSortStrategy(builder.createStoreDir());
-            assertTrue(sortStrategy instanceof TraverseWithSortStrategy);
-        } finally {
-            System.clearProperty(OAK_INDEXER_SORT_STRATEGY_TYPE);
-        }
+        System.setProperty(OAK_INDEXER_SORT_STRATEGY_TYPE, 
FlatFileNodeStoreBuilder.SortStrategyType.TRAVERSE_WITH_SORT.toString());
+        FlatFileNodeStoreBuilder builder = new 
FlatFileNodeStoreBuilder(folder.getRoot())
+                
.withNodeStateEntryTraverserFactory(nodeStateEntryTraverserFactory);
+        SortStrategy sortStrategy = 
builder.createSortStrategy(builder.createStoreDir());
+        assertTrue(sortStrategy instanceof TraverseWithSortStrategy);
     }
 
     @Test
     public void disableTraverseAndSortStrategyUsingSystemProperty() throws 
Exception {
-        try {
-            System.setProperty(OAK_INDEXER_TRAVERSE_WITH_SORT, "false");
-            FlatFileNodeStoreBuilder builder = new 
FlatFileNodeStoreBuilder(folder.getRoot())
-                    
.withNodeStateEntryTraverserFactory(nodeStateEntryTraverserFactory);
-            SortStrategy sortStrategy = 
builder.createSortStrategy(builder.createStoreDir());
-            assertTrue(sortStrategy instanceof StoreAndSortStrategy);
-        } finally {
-            System.clearProperty(OAK_INDEXER_TRAVERSE_WITH_SORT);
-        }
+        System.setProperty(OAK_INDEXER_TRAVERSE_WITH_SORT, "false");
+        FlatFileNodeStoreBuilder builder = new 
FlatFileNodeStoreBuilder(folder.getRoot())
+                
.withNodeStateEntryTraverserFactory(nodeStateEntryTraverserFactory);
+        SortStrategy sortStrategy = 
builder.createSortStrategy(builder.createStoreDir());
+        assertTrue(sortStrategy instanceof StoreAndSortStrategy);
     }
 
     @Test
     public void sortStrategySystemPropertyPrecedence() throws Exception {
-        try {
-            System.setProperty(OAK_INDEXER_TRAVERSE_WITH_SORT, "false");
-            System.setProperty(OAK_INDEXER_SORT_STRATEGY_TYPE, 
FlatFileNodeStoreBuilder.SortStrategyType.TRAVERSE_WITH_SORT.toString());
-            FlatFileNodeStoreBuilder builder = new 
FlatFileNodeStoreBuilder(folder.getRoot())
-                    
.withNodeStateEntryTraverserFactory(nodeStateEntryTraverserFactory);
-            SortStrategy sortStrategy = 
builder.createSortStrategy(builder.createStoreDir());
-            assertTrue(sortStrategy instanceof TraverseWithSortStrategy);
-        } finally {
-            System.clearProperty(OAK_INDEXER_SORT_STRATEGY_TYPE);
-            System.clearProperty(OAK_INDEXER_TRAVERSE_WITH_SORT);
-        }
+        System.setProperty(OAK_INDEXER_TRAVERSE_WITH_SORT, "false");
+        System.setProperty(OAK_INDEXER_SORT_STRATEGY_TYPE, 
FlatFileNodeStoreBuilder.SortStrategyType.TRAVERSE_WITH_SORT.toString());
+        FlatFileNodeStoreBuilder builder = new 
FlatFileNodeStoreBuilder(folder.getRoot())
+                
.withNodeStateEntryTraverserFactory(nodeStateEntryTraverserFactory);
+        SortStrategy sortStrategy = 
builder.createSortStrategy(builder.createStoreDir());
+        assertTrue(sortStrategy instanceof TraverseWithSortStrategy);
+    }
+
+    @Test
+    public void testBuild() throws CompositeException, IOException {
+        System.setProperty(OAK_INDEXER_USE_ZIP, "false");
+        File newFlatFile = getFile("simple-split.json", Compression.NONE);
+
+        System.setProperty(OAK_INDEXER_SORTED_FILE_PATH, 
newFlatFile.getParentFile().getAbsolutePath());
+        assertBuild(newFlatFile.getParentFile().getAbsolutePath());
+    }
+
+    @Test
+    public void testBuildGZIP() throws CompositeException, IOException {
+        System.setProperty(OAK_INDEXER_USE_ZIP, "true");
+        File newFlatFile = getFile("simple-split.json", Compression.GZIP);
+        System.setProperty(OAK_INDEXER_SORTED_FILE_PATH, 
newFlatFile.getParentFile().getAbsolutePath());
+        
+        assertBuild(newFlatFile.getParentFile().getAbsolutePath());
+    }
+
+    @Test
+    public void testBuildLZ4() throws CompositeException, IOException {
+        System.setProperty(OAK_INDEXER_USE_ZIP, "true");
+        System.setProperty(OAK_INDEXER_USE_LZ4, "true");
+        LZ4Compression compression = new LZ4Compression();
+
+        File newFlatFile = getFile("simple-split.json", compression);
+        System.setProperty(OAK_INDEXER_SORTED_FILE_PATH, 
newFlatFile.getParentFile().getAbsolutePath());
+        
+        assertBuild(newFlatFile.getParentFile().getAbsolutePath());
+    }
+
+    @Test
+    public void testBuildListNoSplit() throws CompositeException, IOException {
+        System.setProperty(OAK_INDEXER_USE_ZIP, "false");
+
+        File newFlatFile = getFile("complex-split.json", Compression.NONE);
+        System.setProperty(OAK_INDEXER_SORTED_FILE_PATH, 
newFlatFile.getParentFile().getAbsolutePath());
+        
+        assertBuildList(newFlatFile.getParentFile().getAbsolutePath(), false);
+    }
+    
+    @Test
+    public void testBuildListSplit() throws CompositeException, IOException {
+        System.setProperty(OAK_INDEXER_USE_ZIP, "false");
+        
System.setProperty(IndexerConfiguration.PROP_OAK_INDEXER_MIN_SPLIT_THRESHOLD, 
"0");
+
+        File newFlatFile = getFile("complex-split.json", Compression.NONE);
+        System.setProperty(OAK_INDEXER_SORTED_FILE_PATH, 
newFlatFile.getParentFile().getAbsolutePath());
+        
+        assertBuildList(newFlatFile.getParentFile().getAbsolutePath(), true);
+    }
+
+    @Test
+    public void testBuildListSplitGZIP() throws CompositeException, 
IOException {
+        System.setProperty(OAK_INDEXER_USE_ZIP, "true");
+        
System.setProperty(IndexerConfiguration.PROP_OAK_INDEXER_MIN_SPLIT_THRESHOLD, 
"0");
+        
+        File newFlatFile = getFile("complex-split.json", Compression.GZIP);
+        System.setProperty(OAK_INDEXER_SORTED_FILE_PATH, 
newFlatFile.getParentFile().getAbsolutePath());
+                
+        assertBuildList(newFlatFile.getParentFile().getAbsolutePath(), true);
     }
 
     @Test
-    public void testBuildList() throws CompositeException, IOException {
-        try {
-            File flatFile = new 
File(getClass().getClassLoader().getResource("simple-split.json").getFile());
-            System.setProperty(OAK_INDEXER_SORTED_FILE_PATH, 
flatFile.getAbsolutePath());
-            FlatFileNodeStoreBuilder builder = new 
FlatFileNodeStoreBuilder(folder.getRoot()).withNodeStateEntryTraverserFactory(
-                    nodeStateEntryTraverserFactory);
-            IndexHelper indexHelper = mock(IndexHelper.class);
-            IndexerSupport indexerSupport = mock(IndexerSupport.class);
-            NodeState rootState = mock(NodeState.class);
-            
when(indexerSupport.retrieveNodeStateForCheckpoint()).thenReturn(rootState);
-            List<FlatFileStore> storeList = builder.buildList(indexHelper, 
indexerSupport, null);
-            assertEquals(1, storeList.size());
-        } finally {
-            System.clearProperty(OAK_INDEXER_SORTED_FILE_PATH);
+    public void testBuildListSplitLZ4() throws CompositeException, IOException 
{
+        System.setProperty(OAK_INDEXER_USE_ZIP, "true");
+        System.setProperty(OAK_INDEXER_USE_LZ4, "true");
+        
System.setProperty(IndexerConfiguration.PROP_OAK_INDEXER_MIN_SPLIT_THRESHOLD, 
"0");
+        LZ4Compression compression = new LZ4Compression();
+
+        File newFlatFile = getFile("complex-split.json", compression);
+        System.setProperty(OAK_INDEXER_SORTED_FILE_PATH, 
newFlatFile.getParentFile().getAbsolutePath());
+        
+        assertBuildList(newFlatFile.getParentFile().getAbsolutePath(), true);
+    }
+
+    public void assertBuild(String dir) throws CompositeException, IOException 
{
+        FlatFileNodeStoreBuilder builder = new 
FlatFileNodeStoreBuilder(folder.getRoot()).withNodeStateEntryTraverserFactory(
+            nodeStateEntryTraverserFactory);
+        FlatFileStore store = builder.build();
+        assertEquals(dir, store.getFlatFileStorePath());
+    }
+    
+    private File getFile(String dataFile, Compression compression) throws 
IOException {
+        File flatFile = new 
File(getClass().getClassLoader().getResource(dataFile).getFile());
+        File newFlatFile = new File(folder.getRoot(), 
FlatFileStoreUtils.getSortedStoreFileName(compression));
+        try (BufferedReader reader = FlatFileStoreUtils.createReader(flatFile, 
false);
+            BufferedWriter writer = 
FlatFileStoreUtils.createWriter(newFlatFile, compression)) {
+            IOUtils.copy(reader, writer);
+        }
+        return newFlatFile;
+    }
+
+    public void assertBuildList(String dir, boolean split) throws 
CompositeException, IOException {
+        FlatFileNodeStoreBuilder builder = new 
FlatFileNodeStoreBuilder(folder.getRoot()).withNodeStateEntryTraverserFactory(
+            nodeStateEntryTraverserFactory);
+        IndexHelper indexHelper = mock(IndexHelper.class);
+        when(indexHelper.getWorkDir()).thenReturn(new File(dir));
+        IndexerSupport indexerSupport = mock(IndexerSupport.class);
+        NodeState rootState = mock(NodeState.class);
+        
when(indexerSupport.retrieveNodeStateForCheckpoint()).thenReturn(rootState);
+        
+        List<FlatFileStore> storeList = builder.buildList(indexHelper, 
indexerSupport, mockIndexDefns());
+        
+        if (split) {
+            assertEquals(new File(dir, "split").getAbsolutePath(), 
storeList.get(0).getFlatFileStorePath());
+            assertTrue(storeList.size() > 1);
+        } else {
+            assertEquals(dir, storeList.get(0).getFlatFileStorePath());
+            assertTrue(storeList.size() == 1);

Review Comment:
   it can be simplified
   ```suggestion
               assertEquals(1, storeList.size());
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@jackrabbit.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to