>From Murtadha Hubail <[email protected]>: Murtadha Hubail has uploaded this change for review. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/21959?usp=email )
Change subject: [NO ISSUE][EXT] Fix the library listing when databases are in use ...................................................................... [NO ISSUE][EXT] Fix the library listing when databases are in use With databases, library storage is laid out as db/dv/lib. The listing walk returned TERMINATE at the db/dv directory, so the whole listing came back empty, and the branch that was meant to handle databases also indexed past the end of the path and ignored the database name. Ext-ref: MB-67950 Generated-by: Claude Opus 5.5 <[email protected]> Change-Id: Ic0300108f9ee051415b68b05932dab6effc1dbaf --- M asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/library/ExternalLibraryManager.java A asterixdb/asterix-external-data/src/test/java/org/apache/asterix/external/library/ExternalLibraryListingTest.java 2 files changed, 113 insertions(+), 20 deletions(-) git pull ssh://asterix-gerrit.ics.uci.edu:29418/asterixdb refs/changes/59/21959/1 diff --git a/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/library/ExternalLibraryManager.java b/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/library/ExternalLibraryManager.java index 3841146..b8a91f7 100755 --- a/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/library/ExternalLibraryManager.java +++ b/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/library/ExternalLibraryManager.java @@ -275,18 +275,24 @@ @Override public List<Pair<Namespace, String>> getLibraryListing() throws IOException { + return listLibraries(storageDirPath, namespacePathResolver.usingDatabase()); + } + + static List<Pair<Namespace, String>> listLibraries(Path storageDirPath, boolean usingDatabase) throws IOException { List<Pair<Namespace, String>> libs = new ArrayList<>(); + // with databases, the layout is db/dv/^dv2/.../lib rather than dv/^dv2/.../lib + int dvStartToken = usingDatabase ? 1 : 0; Files.walkFileTree(storageDirPath, new SimpleFileVisitor<Path>() { @Override public FileVisitResult visitFile(Path currPath, BasicFileAttributes attrs) { - //never want to see any files - return FileVisitResult.TERMINATE; + // a stray file must not cut the listing short + return FileVisitResult.CONTINUE; } @Override public FileVisitResult preVisitDirectory(Path currPath, BasicFileAttributes attrs) throws HyracksDataException { - if (currPath.equals(storageDirPath) || currPath.getParent().equals(storageDirPath)) { + if (currPath.equals(storageDirPath)) { return FileVisitResult.CONTINUE; } if (currPath.getFileName().toString().codePointAt(0) == StoragePathUtil.DATAVERSE_CONTINUATION_MARKER) { @@ -296,24 +302,15 @@ .relativize(currPath.toAbsolutePath().normalize()).toString(); List<String> dvParts = new ArrayList<>(); final String[] tokens = StringUtils.split(candidateDvAndLib, File.separatorChar); - if (tokens == null || tokens.length < 2) { - //? shouldn't happen - return FileVisitResult.TERMINATE; + if (tokens.length < dvStartToken + 2) { + // a database or dataverse directory; libraries are further down + return FileVisitResult.CONTINUE; } try { - String candidateDb = MetadataConstants.DEFAULT_DATABASE; - if (namespacePathResolver.usingDatabase()) { - if (tokens.length < 3) { - return FileVisitResult.TERMINATE; - } - libs.add(Pair.of(new Namespace(candidateDb, DataverseName.create(List.of(tokens[1]))), - tokens[3])); - return FileVisitResult.SKIP_SUBTREE; - - } + String candidateDb = usingDatabase ? tokens[0] : MetadataConstants.DEFAULT_DATABASE; //add first part, then look for multiparts - dvParts.add(tokens[0]); - int currToken = 1; + dvParts.add(tokens[dvStartToken]); + int currToken = dvStartToken + 1; for (; currToken < tokens.length && tokens[currToken] .codePointAt(0) == StoragePathUtil.DATAVERSE_CONTINUATION_MARKER; currToken++) { dvParts.add(tokens[currToken].substring(1)); @@ -327,8 +324,8 @@ DataverseName candidateDv; candidateDv = DataverseName.create(dvParts); Namespace candidateNs = new Namespace(candidateDb, candidateDv); - FileReference candidateLibPath = findLibraryRevDir(candidateNs, candidateLib); - if (candidateLibPath != null) { + if (Files.isDirectory(currPath.resolve(REV_1_DIR_NAME)) + || Files.isDirectory(currPath.resolve(REV_0_DIR_NAME))) { libs.add(Pair.of(candidateNs, candidateLib)); } } catch (AsterixException e) { diff --git a/asterixdb/asterix-external-data/src/test/java/org/apache/asterix/external/library/ExternalLibraryListingTest.java b/asterixdb/asterix-external-data/src/test/java/org/apache/asterix/external/library/ExternalLibraryListingTest.java new file mode 100644 index 0000000..daf5622 --- /dev/null +++ b/asterixdb/asterix-external-data/src/test/java/org/apache/asterix/external/library/ExternalLibraryListingTest.java @@ -0,0 +1,96 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.asterix.external.library; + +import static org.apache.asterix.external.library.ExternalLibraryManager.REV_0_DIR_NAME; +import static org.apache.asterix.external.library.ExternalLibraryManager.REV_1_DIR_NAME; +import static org.junit.Assert.assertEquals; + +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.HashSet; +import java.util.List; +import java.util.Set; + +import org.apache.asterix.common.metadata.DataverseName; +import org.apache.asterix.common.metadata.MetadataConstants; +import org.apache.asterix.common.metadata.Namespace; +import org.apache.commons.lang3.tuple.Pair; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; + +/** + * Covers {@link ExternalLibraryManager#listLibraries} over the on-disk library storage layout, with and + * without databases. + */ +public class ExternalLibraryListingTest { + + @Rule + public TemporaryFolder tmp = new TemporaryFolder(); + + /** + * With databases, the database directory sits above the dataverse. The listing used to stop the whole walk + * at the first dataverse directory and so always came back empty. + */ + @Test + public void testListingWithDatabases() throws Exception { + Path storage = tmp.newFolder("storage").toPath(); + mkLib(storage, "Default/Default/lib1", REV_1_DIR_NAME); + mkLib(storage, "Default/Default/lib2", REV_0_DIR_NAME); + mkLib(storage, "db2/dv/lib3", REV_1_DIR_NAME); + mkLib(storage, "db2/a/^b/lib4", REV_1_DIR_NAME); + // a library directory with no revision is not a library yet + Files.createDirectories(storage.resolve("db2/dv/notalib")); + Files.createFile(storage.resolve("db2/stray")); + + assertEquals( + Set.of(lib("Default", "lib1", "Default"), lib("Default", "lib2", "Default"), lib("db2", "lib3", "dv"), + lib("db2", "lib4", "a", "b")), + new HashSet<>(ExternalLibraryManager.listLibraries(storage, true))); + } + + @Test + public void testListingWithoutDatabases() throws Exception { + Path storage = tmp.newFolder("storage").toPath(); + mkLib(storage, "dv/lib1", REV_1_DIR_NAME); + mkLib(storage, "a/^b/^c/lib2", REV_0_DIR_NAME); + Files.createDirectories(storage.resolve("dv/notalib")); + Files.createFile(storage.resolve("stray")); + + String db = MetadataConstants.DEFAULT_DATABASE; + assertEquals(Set.of(lib(db, "lib1", "dv"), lib(db, "lib2", "a", "b", "c")), + new HashSet<>(ExternalLibraryManager.listLibraries(storage, false))); + } + + @Test + public void testEmptyListing() throws Exception { + Path storage = tmp.newFolder("storage").toPath(); + assertEquals(List.of(), ExternalLibraryManager.listLibraries(storage, true)); + assertEquals(List.of(), ExternalLibraryManager.listLibraries(storage, false)); + } + + private static void mkLib(Path storage, String libPath, String revDir) throws Exception { + Files.createDirectories(storage.resolve(libPath).resolve(revDir)); + } + + private static Pair<Namespace, String> lib(String db, String lib, String... dvParts) throws Exception { + return Pair.of(new Namespace(db, DataverseName.create(List.of(dvParts))), lib); + } +} -- To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/21959?usp=email To unsubscribe, or for help writing mail filters, visit https://asterix-gerrit.ics.uci.edu/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Change-Id: Ic0300108f9ee051415b68b05932dab6effc1dbaf Gerrit-Change-Number: 21959 Gerrit-PatchSet: 1 Gerrit-Owner: Murtadha Hubail <[email protected]>
