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