From a29f88413a00e8adcc738433aee1a0e5434b02dd Mon Sep 17 00:00:00 2001 From: "Ariel Shaqed (Scolnicov)" Date: Sun, 24 Sep 2023 16:07:15 +0300 Subject: [PATCH] Pass a bunch more tests --- clients/hadoopfs/pom.xml | 14 + .../main/java/io/lakefs/LakeFSFileSystem.java | 1 + .../io/lakefs/LakeFSFileSystemServerTest.java | 692 +++++++++++++++--- .../java/io/lakefs/LakeFSFileSystemTest.java | 2 +- 4 files changed, 599 insertions(+), 110 deletions(-) diff --git a/clients/hadoopfs/pom.xml b/clients/hadoopfs/pom.xml index 63fc3f0f1a2..93625de864c 100644 --- a/clients/hadoopfs/pom.xml +++ b/clients/hadoopfs/pom.xml @@ -100,6 +100,20 @@ To export to S3: + + org.apache.maven.plugins + maven-jar-plugin + 3.3 + + + + test-jar + + test-jar + + + + diff --git a/clients/hadoopfs/src/main/java/io/lakefs/LakeFSFileSystem.java b/clients/hadoopfs/src/main/java/io/lakefs/LakeFSFileSystem.java index 0fc5fd02ac5..5f257e30dc9 100644 --- a/clients/hadoopfs/src/main/java/io/lakefs/LakeFSFileSystem.java +++ b/clients/hadoopfs/src/main/java/io/lakefs/LakeFSFileSystem.java @@ -421,6 +421,7 @@ private boolean renameFile(LakeFSFileStatus srcStatus, Path dst) throws IOExcept LOG.debug("renameFile: dst {} exists and is a {}", dst, dstFileStatus.isDirectory() ? "directory" : "file"); if (dstFileStatus.isDirectory()) { dst = buildObjPathOnExistingDestinationDir(srcStatus.getPath(), dst); + LOG.debug("renameFile: use {} to create dst {}", srcStatus.getPath(), dst); } } catch (FileNotFoundException e) { LOG.debug("renameFile: dst does not exist, renaming src {} to a file called dst {}", diff --git a/clients/hadoopfs/src/test/java/io/lakefs/LakeFSFileSystemServerTest.java b/clients/hadoopfs/src/test/java/io/lakefs/LakeFSFileSystemServerTest.java index 7d67c13672b..fc3b42a17ee 100644 --- a/clients/hadoopfs/src/test/java/io/lakefs/LakeFSFileSystemServerTest.java +++ b/clients/hadoopfs/src/test/java/io/lakefs/LakeFSFileSystemServerTest.java @@ -1,5 +1,8 @@ package io.lakefs; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import io.lakefs.clients.api.*; import io.lakefs.clients.api.model.*; import io.lakefs.clients.api.model.ObjectStats.PathTypeEnum; @@ -69,12 +72,14 @@ import java.util.Map; public class LakeFSFileSystemServerTest { + static private final Logger LOG = LoggerFactory.getLogger(LakeFSFileSystemServerTest.class); + static final Long UNUSED_FILE_SIZE = 1L; static final Long UNUSED_MTIME = 0L; static final String UNUSED_CHECKSUM = "unused"; static final Long STATUS_FILE_SIZE = 2L; - static final Long STATUS_MTIME = 0L; + static final Long STATUS_MTIME = 123456789L; static final String STATUS_CHECKSUM = "status"; protected Configuration conf; @@ -100,7 +105,6 @@ public class LakeFSFileSystemServerTest { @Value.Parameter Optional amount(); @Value.Parameter Optional after(); @Value.Parameter Optional prefix(); - @Value.Default default boolean hasMore() { return false; } } @Rule @@ -137,6 +141,11 @@ protected HttpRequest request() { // abstract StagingLocation mockGetPhysicalAddress(String repo, String branch, String key, String physicalKey) throws ApiException; + // TODO(ariels): Override and make abstract! + protected String createPhysicalAddress(String key) { + return s3Url(key); + } + protected static String makeS3BucketName() { String slug = NanoIdUtils.randomNanoId(NanoIdUtils.DEFAULT_NUMBER_GENERATOR, "abcdefghijklmnopqrstuvwxyz-0123456789".toCharArray(), 14); @@ -149,15 +158,15 @@ protected String s3Url(String s3Path) { } protected String getS3Key(StagingLocation stagingLocation) { - return removeStart(stagingLocation.getPhysicalAddress(), s3Base + "/"); + return removeStart(stagingLocation.getPhysicalAddress(), s3Base); } protected void assertS3Object(StagingLocation stagingLocation, String contents) { String s3Key = getS3Key(stagingLocation); - List actualFiles = ImmutableList.of(); + List actualFiles = ImmutableList.of(""); try (S3Object obj = s3Client.getObject(new GetObjectRequest(s3Bucket, "/" + s3Key))) { - actualFiles = getS3FilesByPrefix("/"); + actualFiles = getS3FilesByPrefix(""); String actual = IOUtils.toString(obj.getObjectContent()); Assert.assertEquals(contents, actual); @@ -197,7 +206,19 @@ public void setUp() throws Exception { s3Client.setEndpoint(s3Endpoint); s3Bucket = makeS3BucketName(); - s3Base = String.format("s3://%s", s3Bucket); + s3Base = String.format("s3://%s/", s3Bucket); + LOG.info("S3: bucket {} => base URL {}", s3Bucket, s3Base); + + // Always expect repo "repo" to be found, it's used in all tests. + mockServerClient.when(request() + .withMethod("GET") + .withPath("/repositories/repo")) + .respond(response().withStatusCode(200) + .withBody(gson.toJson(new Repository().id("repo") + .creationDate(1234L) + .defaultBranch("main") + .storageNamespace(s3Base)))); + CreateBucketRequest cbr = new CreateBucketRequest(s3Bucket); s3Client.createBucket(cbr); @@ -247,18 +268,19 @@ public void setUp() throws Exception { * @return all pathnames under s3Prefix that start with prefix. (Obvious not scalable!) */ protected List getS3FilesByPrefix(String prefix) { - final int maxKeys = 1500; ListObjectsRequest req = new ListObjectsRequest() .withBucketName(s3Bucket) .withPrefix(prefix) - .withMaxKeys(maxKeys); + .withDelimiter(null); + ObjectListing listing = s3Client.listObjects(req); + List summaries = listing.getObjectSummaries(); if (listing.isTruncated()) { - Assert.fail(String.format("[internal] no support for test that creates >%d S3 objects", maxKeys)); + Assert.fail(String.format("[internal] no support for test that creates >%d S3 objects", listing.getMaxKeys())); } - return Lists.transform(listing.getObjectSummaries(), S3ObjectSummary::getKey); + return Lists.transform(summaries, S3ObjectSummary::getKey); } @Test @@ -287,17 +309,44 @@ protected void expectStatObject(String repo, String ref, String path, ObjectStat .withBody(gson.toJson(stats))); } - // Expect this lakeFSFS path not to exist + // Expect this lakeFSFS path not to exist. You may still need to + // expectListing for the directory that will not contain this pagth. protected void expectFileDoesNotExist(String repo, String ref, String path) { expectStatObjectNotFound(repo, ref, path); expectStatObjectNotFound(repo, ref, path + Constants.SEPARATOR); - expectListing(repo, ref, ImmutablePagination.builder().prefix(path + Constants.SEPARATOR).build(), true); + } + + protected void expectFilesInDir(String repo, String main, String dir, String... files) { + ObjectStats[] allStats; + if (files.length == 0) { + // Fake a directory marker + Path dirPath = new Path(String.format("lakefs://%s/%s/%s", repo, main, dir)); + ObjectLocation dirLoc = ObjectLocation.pathToObjectLocation(dirPath); + ObjectStats dirStats = expectDirectoryMarker(dirLoc); + allStats = new ObjectStats[1]; + allStats[0] = dirStats; + } else { + expectStatObjectNotFound(repo, main, dir); + expectStatObjectNotFound(repo, main, dir + Constants.SEPARATOR); + + allStats = new ObjectStats[files.length]; + for (int i = 0; i < files.length; i++) { + allStats[i] = new ObjectStats() + .pathType(PathTypeEnum.OBJECT) + .path(dir + Constants.SEPARATOR + files[i]); + } + } + + // Directory can be listed! + expectListing("repo", "main", + ImmutablePagination.builder().prefix(dir + Constants.SEPARATOR).build(), + allStats); } protected void expectUploadObject(String repo, String branch, String path) { StagingLocation stagingLocation = new StagingLocation() .token("token:foo:" + sessionId()) - .physicalAddress(s3Url(String.format("/repo-base/dir-marker/%s/%s/%s/%s", + .physicalAddress(s3Url(String.format("repo-base/dir-marker/%s/%s/%s/%s", sessionId(), repo, branch, path))); mockServerClient.when(request() .withMethod("POST") @@ -307,13 +356,21 @@ protected void expectUploadObject(String repo, String branch, String path) { .withBody(gson.toJson(stagingLocation))); } - // Return a location under namespace for this getPhysicalAddress call. + protected void expectGetBranch(String repo, String branch) { + mockServerClient.when(request() + .withMethod("GET") + .withPath(String.format("/repositories/%s/branches/%s", repo, branch))) + .respond(response().withStatusCode(200) + .withBody(gson.toJson(new Ref().id("123").commitId("456")))); + } + // Return a location under namespace for this getPhysicalAddress call. + // // TODO(ariels): abstract, overload separately for direct and pre-signed. protected StagingLocation expectGetPhysicalAddress(String repo, String branch, String path, String namespace) { StagingLocation stagingLocation = new StagingLocation() .token("token:foo:" + sessionId()) - .physicalAddress(s3Url(String.format("/%s/%s/%s/%s/%s-object", + .physicalAddress(s3Url(String.format("%s/%s/%s/%s/%s-object", sessionId(), namespace, repo, branch, path))); mockServerClient.when(request() .withMethod("GET") @@ -358,34 +415,45 @@ protected void expectDeleteObjects(String repo, String branch, PathList pathList .errors(Arrays.asList(errors))))); } - protected void expectDirectoryMarker(ObjectLocation objectLoc) { + protected ObjectStats expectDirectoryMarker(ObjectLocation objectLoc) { // Mock parent directory to show the directory marker exists. - ObjectStats markerStats = new ObjectStats().path(objectLoc.getPath()).pathType(PathTypeEnum.OBJECT); + ObjectStats markerStats = new ObjectStats() + .path(objectLoc.getPath()) + .pathType(PathTypeEnum.OBJECT); mockServerClient.when(request() .withMethod("GET") .withPath(String.format("/repositories/%s/refs/%s/objects/stat", objectLoc.getRepository(), objectLoc.getRef())) .withQueryStringParameter("path", objectLoc.getPath())) .respond(response().withStatusCode(200) .withBody(gson.toJson(markerStats))); + return markerStats; } // Expect this listing and return these stats. - protected void expectListing(String repo, String ref, ImmutablePagination pagination, boolean hasMore, ObjectStats... stats) { + protected void expectListing(String repo, String ref, ImmutablePagination pagination, ObjectStats... stats) { + expectListingWithHasMore(repo, ref, pagination, false, stats); + } + + protected void expectListingWithHasMore(String repo, String ref, ImmutablePagination pagination, boolean hasMore, ObjectStats... stats) { HttpRequest req = request() .withMethod("GET") - .withPath(String.format("/repositories/%s/refs/%s/objects/ls", repo, ref)) - // Validate prefix, it matters! - .withQueryStringParameter("prefix", pagination.prefix().or("")) - .withQueryStringParameter("after", pagination.after().or("")); + .withPath(String.format("/repositories/%s/refs/%s/objects/ls", repo, ref)); + // Validate elements of pagination only if present. + if (pagination.after().isPresent()) { + req = req.withQueryStringParameter("after", pagination.after().or("")); + } if (pagination.amount().isPresent()) { - // Validate amount only if requested. req = req.withQueryStringParameter("amount", pagination.amount().get().toString()); } + if (pagination.prefix().isPresent()) { + req = req.withQueryStringParameter("prefix", pagination.prefix().or("")); + } mockServerClient.when(req) .respond(response() .withStatusCode(200) .withBody(gson.toJson(ImmutableMap.of("results", Arrays.asList(stats), - "pagination", ImmutablePagination.builder().hasMore(hasMore).build())))); + "pagination", + new io.lakefs.clients.api.model.Pagination().hasMore(hasMore))))); } @Test @@ -431,7 +499,7 @@ public void testGetFileStatus_NoFile() { expectStatObjectNotFound("repo", "main", "no.file"); expectStatObjectNotFound("repo", "main", "no.file/"); - expectListing("repo", "main", ImmutablePagination.builder().prefix("no.file/").amount(1).build(), false); + expectListing("repo", "main", ImmutablePagination.builder().prefix("no.file/").amount(1).build()); Assert.assertThrows(FileNotFoundException.class, () -> fs.getFileStatus(noFilePath)); } @@ -442,7 +510,7 @@ public void testGetFileStatus_DirectoryMarker() throws IOException { ObjectStats stats = new ObjectStats() .path("dir1/dir2/") - .physicalAddress(s3Url("/repo-base/dir12")); + .physicalAddress(s3Url("repo-base/dir12")); expectStatObject("repo", "main", "dir1/dir2/", stats); LakeFSFileStatus dirStatus = fs.getFileStatus(dirPath); @@ -455,8 +523,8 @@ public void testExists_ExistsAsObject() throws IOException { Path path = new Path("lakefs://repo/main/exis.ts"); ObjectStats stats = new ObjectStats() .path("exis.ts") - .physicalAddress(s3Url("/repo-base/o12")); - expectListing("repo", "main", ImmutablePagination.builder().prefix("exis.ts").build(), true, stats); + .physicalAddress(s3Url("repo-base/o12")); + expectListing("repo", "main", ImmutablePagination.builder().prefix("exis.ts").build(), stats); Assert.assertTrue(fs.exists(path)); } @@ -465,7 +533,7 @@ public void testExists_ExistsAsDirectoryMarker() throws IOException { Path path = new Path("lakefs://repo/main/exis.ts"); ObjectStats stats = new ObjectStats().path("exis.ts"); - expectListing("repo", "main", ImmutablePagination.builder().prefix("exis.ts").build(), false, + expectListing("repo", "main", ImmutablePagination.builder().prefix("exis.ts").build(), stats); Assert.assertTrue(fs.exists(path)); @@ -476,7 +544,7 @@ public void testExists_ExistsAsDirectoryContents() throws IOException { Path path = new Path("lakefs://repo/main/exis.ts"); ObjectStats stats = new ObjectStats().path("exis.ts/object-inside-the-path"); - expectListing("repo", "main", ImmutablePagination.builder().prefix("exis.ts").build(), false, + expectListing("repo", "main", ImmutablePagination.builder().prefix("exis.ts").build(), stats); Assert.assertTrue(fs.exists(path)); } @@ -489,10 +557,12 @@ public void testExists_ExistsAsDirectoryInSecondList() throws IOException { ObjectStats indirStats = new ObjectStats().path("exis.ts/object-inside-the-path"); // First listing returns irrelevant objects, _before_ "exis.ts/" - expectListing("repo", "main", ImmutablePagination.builder().prefix("exis.ts").build(), true, - beforeStats1, beforeStats2); + expectListingWithHasMore("repo", "main", + ImmutablePagination.builder().prefix("exis.ts").build(), + false, + beforeStats1, beforeStats2); // Second listing tries to find an object inside "exis.ts/". - expectListing("repo", "main", ImmutablePagination.builder().prefix("exis.ts/").build(), false, + expectListing("repo", "main", ImmutablePagination.builder().prefix("exis.ts/").build(), indirStats); Assert.assertTrue(fs.exists(path)); } @@ -522,16 +592,16 @@ public void testExists_NotExistsPrefixWithNoSlashTwoLists() { } @Test - public void testDelete_FileExists() throws ApiException, IOException { + public void testDelete_FileExists() throws IOException { expectStatObject("repo", "main", "no/place/file.txt", new ObjectStats() .path("delete/sample/file.txt") .pathType(PathTypeEnum.OBJECT) - .physicalAddress(s3Url("/repo-base/delete"))); + .physicalAddress(s3Url("repo-base/delete"))); String[] arrDirs = {"no/place", "no"}; for (String dir: arrDirs) { expectStatObjectNotFound("repo", "main", dir); expectStatObjectNotFound("repo", "main", dir + "/"); - expectListing("repo", "main", ImmutablePagination.builder().build(), false); + expectListing("repo", "main", ImmutablePagination.builder().build()); } expectDeleteObject("repo", "main", "no/place/file.txt"); expectUploadObject("repo", "main", "no/place/"); @@ -544,12 +614,12 @@ public void testDelete_FileExists() throws ApiException, IOException { } @Test - public void testDelete_FileNotExists() throws ApiException, IOException { + public void testDelete_FileNotExists() throws IOException { expectDeleteObjectNotFound("repo", "main", "no/place/file.txt"); expectStatObjectNotFound("repo", "main", "no/place/file.txt"); expectStatObjectNotFound("repo", "main", "no/place/file.txt/"); expectListing("repo", "main", - ImmutablePagination.builder().prefix("no/place/file.txt/").build(), false); + ImmutablePagination.builder().prefix("no/place/file.txt/").build()); // Should still create a directory marker! expectUploadObject("repo", "main", "no/place/"); @@ -559,7 +629,7 @@ public void testDelete_FileNotExists() throws ApiException, IOException { } @Test - public void testDelete_EmptyDirectoryExists() throws ApiException, IOException { + public void testDelete_EmptyDirectoryExists() throws IOException { ObjectLocation dirObjLoc = new ObjectLocation("lakefs", "repo", "main", "delete/me"); String key = objectLocToS3ObjKey(dirObjLoc); @@ -575,7 +645,7 @@ public void testDelete_EmptyDirectoryExists() throws ApiException, IOException { // Just a directory marker delete/me/, so nothing to delete. expectListing("repo", "main", - ImmutablePagination.builder().prefix("delete/me/").build(), false, + ImmutablePagination.builder().prefix("delete/me/").build(), srcStats); expectDirectoryMarker(dirObjLoc.getParent()); @@ -590,7 +660,7 @@ public void testDelete_EmptyDirectoryExists() throws ApiException, IOException { } @Test - public void testDelete_DirectoryWithFile() throws ApiException, IOException { + public void testDelete_DirectoryWithFile() throws IOException { String directoryPath = "delete/sample"; String existingPath = "delete/sample/file.txt"; String directoryToDelete = "lakefs://repo/main/delete/sample"; @@ -609,7 +679,6 @@ public void testDelete_DirectoryWithFile() throws ApiException, IOException { ImmutablePagination.builder() .prefix(directoryPath + Constants.SEPARATOR) .build(), - false, srcStats); // No deletes! @@ -628,7 +697,7 @@ public void testDelete_DirectoryWithFile() throws ApiException, IOException { } @Test - public void testDelete_NotExistsRecursive() throws ApiException, IOException { + public void testDelete_NotExistsRecursive() throws IOException { // No objects to stat. mockServerClient.when(request() .withMethod("GET") @@ -636,12 +705,12 @@ public void testDelete_NotExistsRecursive() throws ApiException, IOException { .respond(response().withStatusCode(404)); // No objects to list, either -- in directory. expectListing("repo", "main", - ImmutablePagination.builder().prefix("no/place/file.txt/").build(), false); + ImmutablePagination.builder().prefix("no/place/file.txt/").build()); Assert.assertFalse(fs.delete(new Path("lakefs://repo/main/no/place/file.txt"), true)); } @Test - public void testDelete_DirectoryWithFileRecursive() throws ApiException, IOException { + public void testDelete_DirectoryWithFileRecursive() throws IOException { expectStatObjectNotFound("repo", "main", "delete/sample"); expectStatObjectNotFound("repo", "main", "delete/sample/"); ObjectStats stats = new ObjectStats(). @@ -652,7 +721,7 @@ public void testDelete_DirectoryWithFileRecursive() throws ApiException, IOExcep mtime(UNUSED_MTIME). sizeBytes(UNUSED_FILE_SIZE); expectListing("repo", "main", - ImmutablePagination.builder().prefix("delete/sample/").build(), false, + ImmutablePagination.builder().prefix("delete/sample/").build(), stats); expectDeleteObjects("repo", "main", "delete/sample/file.txt"); @@ -669,7 +738,7 @@ public void testDelete_DirectoryWithFileRecursive() throws ApiException, IOExcep Assert.assertTrue(delete); } - protected void caseDeleteDirectoryRecursive(int bulkSize, int numObjects) throws ApiException, IOException { + protected void caseDeleteDirectoryRecursive(int bulkSize, int numObjects) throws IOException { conf.setInt(LakeFSFileSystem.LAKEFS_DELETE_BULK_SIZE, bulkSize); expectStatObjectNotFound("repo", "main", "delete/sample"); expectStatObjectNotFound("repo", "main", "delete/sample/"); @@ -685,7 +754,7 @@ protected void caseDeleteDirectoryRecursive(int bulkSize, int numObjects) throws sizeBytes(UNUSED_FILE_SIZE); } expectListing("repo", "main", - ImmutablePagination.builder().prefix("delete/sample/").build(), false, + ImmutablePagination.builder().prefix("delete/sample/").build(), objects); // Set up multiple deleteObjects expectations of bulkSize deletes @@ -705,34 +774,34 @@ protected void caseDeleteDirectoryRecursive(int bulkSize, int numObjects) throws } @Test - public void testDeleteDirectoryRecursiveBatch1() throws ApiException, IOException { + public void testDeleteDirectoryRecursiveBatch1() throws IOException { caseDeleteDirectoryRecursive(1, 123); } @Test - public void testDeleteDirectoryRecursiveBatch2() throws ApiException, IOException { + public void testDeleteDirectoryRecursiveBatch2() throws IOException { caseDeleteDirectoryRecursive(2, 123); } @Test - public void testDeleteDirectoryRecursiveBatch3() throws ApiException, IOException { + public void testDeleteDirectoryRecursiveBatch3() throws IOException { caseDeleteDirectoryRecursive(3, 123); } @Test - public void testDeleteDirectoryRecursiveBatch5() throws ApiException, IOException { + public void testDeleteDirectoryRecursiveBatch5() throws IOException { caseDeleteDirectoryRecursive(5, 123); } @Test - public void testDeleteDirectoryRecursiveBatch120() throws ApiException, IOException { + public void testDeleteDirectoryRecursiveBatch120() throws IOException { caseDeleteDirectoryRecursive(120, 123); } @Test - public void testDeleteDirectoryRecursiveBatch123() throws ApiException, IOException { + public void testDeleteDirectoryRecursiveBatch123() throws IOException { caseDeleteDirectoryRecursive(123, 123); } @Test - public void testCreate() throws ApiException, IOException { + public void testCreate() throws IOException { String contents = "The quick brown fox jumps over the lazy dog."; long contentsLength = (long) contents.getBytes().length; Path path = new Path("lakefs://repo/main/sub1/sub2/create.me"); @@ -774,19 +843,18 @@ public void testCreate() throws ApiException, IOException { out.close(); // Write succeeded, verify physical file on S3. - String s3Key = getS3Key(stagingLocation); assertS3Object(stagingLocation, contents); } @Test - public void testCreateExistingDirectory() throws ApiException, IOException { + public void testCreateExistingDirectory() throws IOException { Path path = new Path("lakefs://repo/main/sub1/sub2/create.me"); // path is a directory -- so cannot be created as a file. expectStatObjectNotFound("repo", "main", "sub1/sub2/create.me"); ObjectStats stats = new ObjectStats() .path("sub1/sub2/create.me/") - .physicalAddress(s3Url("/repo-base/sub1/sub2/create.me")); + .physicalAddress(s3Url("repo-base/sub1/sub2/create.me")); expectStatObject("repo", "main", "sub1/sub2/create.me/", stats); Exception e = @@ -795,7 +863,7 @@ public void testCreateExistingDirectory() throws ApiException, IOException { } @Test - public void testCreateExistingFile() throws ApiException, IOException { + public void testCreateExistingFile() throws IOException { Path path = new Path("lakefs://repo/main/sub1/sub2/create.me"); ObjectLocation dir = new ObjectLocation("lakefs", "repo", "main", "sub1/sub2"); @@ -807,13 +875,13 @@ public void testCreateExistingFile() throws ApiException, IOException { } @Test - public void testMkdirs() throws ApiException, IOException { + public void testMkdirs() throws IOException { // setup empty folder checks Path path = new Path("dir1/dir2/dir3"); for (Path p = new Path(path.toString()); p != null && !p.isRoot(); p = p.getParent()) { expectStatObjectNotFound("repo", "main", p.toString()); expectStatObjectNotFound("repo", "main", p+"/"); - expectListing("repo", "main", ImmutablePagination.builder().prefix(p+"/").build(), false); + expectListing("repo", "main", ImmutablePagination.builder().prefix(p+"/").build()); } // physical address to directory marker object @@ -822,7 +890,7 @@ public void testMkdirs() throws ApiException, IOException { ObjectStats newStats = new ObjectStats() .path("dir1/dir2/dir3/") - .physicalAddress(s3Url("/repo-base/dir12")); + .physicalAddress(s3Url("repo-base/dir12")); expectStatObject("repo", "main", "dir1/dir2/dir3/", newStats); mockServerClient.when(request() @@ -844,58 +912,464 @@ public void testMkdirs() throws ApiException, IOException { assertS3Object(stagingLocation, ""); } + @Test + public void testOpen() throws IOException, ApiException { + String contents = "The quick brown fox jumps over the lazy dog."; + byte[] contentsBytes = contents.getBytes(); + String physicalPath = sessionId() + "/repo-base/open"; + String physicalKey = createPhysicalAddress(physicalPath); + int readBufferSize = 5; + Path path = new Path("lakefs://repo/main/read.me"); + + expectStatObject("repo", "main", "read.me", + new ObjectStats() + .physicalAddress(physicalKey) + .checksum(UNUSED_CHECKSUM) + .mtime(UNUSED_MTIME) + .sizeBytes((long) contentsBytes.length)); + + // Write physical file to S3. + ObjectMetadata s3Metadata = new ObjectMetadata(); + s3Metadata.setContentLength(contentsBytes.length); + s3Client.putObject(s3Bucket, + physicalPath, + new ByteArrayInputStream(contentsBytes), + s3Metadata); + + try (InputStream in = fs.open(path, readBufferSize)) { + String actual = IOUtils.toString(in); + Assert.assertEquals(contents, actual); + } catch (Exception e) { + String actualFiles = String.join(", ", getS3FilesByPrefix("")); + throw new RuntimeException("Files " + actualFiles + "; read " + path.toString() + " from " + physicalKey, e); + } + } + + // TODO(ariels): Rename test to "testOpenWithNonAsciiUriChars". + @Test + public void testOpenWithInvalidUriChars() throws IOException, ApiException { + String contents = "The quick brown fox jumps over the lazy dog."; + byte[] contentsBytes = contents.getBytes(); + int readBufferSize = 5; + + String[] suffixes = { + "with space/open", + "wi:th$cha&rs#/%op;e?n", + "עכשיו/בעברית/open", + "\uD83E\uDD2F/imoji/open", + }; + for (String suffix : suffixes) { + String key = "/repo-base/" + suffix; + + // Write physical file to S3. + ObjectMetadata s3Metadata = new ObjectMetadata(); + s3Metadata.setContentLength(contentsBytes.length); + s3Client.putObject(new PutObjectRequest(s3Bucket, key, new ByteArrayInputStream(contentsBytes), s3Metadata)); + + String path = String.format("lakefs://repo/main/%s-x", suffix); + ObjectStats stats = new ObjectStats() + .physicalAddress(s3Url(key)) + .sizeBytes((long) contentsBytes.length); + expectStatObject("repo", "main", suffix + "-x", stats); + + try (InputStream in = fs.open(new Path(path), readBufferSize)) { + String actual = IOUtils.toString(in); + Assert.assertEquals(contents, actual); + } + } + } + + @Test + public void testOpen_NotExists() throws IOException, ApiException { + Path path = new Path("lakefs://repo/main/doesNotExi.st"); + expectStatObjectNotFound("repo", "main", "doesNotExi.st"); + Assert.assertThrows(FileNotFoundException.class, + () -> fs.open(path)); + } + + @Test + public void testListStatusFile() throws IOException { + ObjectStats objectStats = new ObjectStats(). + path("status/file"). + pathType(PathTypeEnum.OBJECT). + physicalAddress(s3Url("/repo-base/status")). + checksum(STATUS_CHECKSUM). + mtime(STATUS_MTIME). + sizeBytes(STATUS_FILE_SIZE); + expectStatObject("repo", "main", "status/file", objectStats); + Path path = new Path("lakefs://repo/main/status/file"); + FileStatus[] fileStatuses = fs.listStatus(path); + LakeFSFileStatus expectedFileStatus = new LakeFSFileStatus.Builder(path) + .length(STATUS_FILE_SIZE) + .checksum(STATUS_CHECKSUM) + .mTime(STATUS_MTIME) + .physicalAddress(s3Url("/repo-base/status")) + .blockSize(Constants.DEFAULT_BLOCK_SIZE) + .build(); + LakeFSFileStatus[] expectedFileStatuses = new LakeFSFileStatus[]{expectedFileStatus}; + Assert.assertArrayEquals(expectedFileStatuses, fileStatuses); + } + + @Test + public void testListStatusNotFound() throws ApiException { + expectStatObjectNotFound("repo", "main", "status/file"); + expectStatObjectNotFound("repo", "main", "status/file/"); + expectListing("repo", "main", + ImmutablePagination.builder().prefix("status/file/").build()); + Path path = new Path("lakefs://repo/main/status/file"); + Assert.assertThrows(FileNotFoundException.class, () -> fs.listStatus(path)); + } + + @Test + public void testListStatusDirectory() throws IOException { + int totalObjectsCount = 3; + ObjectStats[] objects = new ObjectStats[3]; + for (int i = 0; i < totalObjectsCount; i++) { + objects[i] = new ObjectStats(). + path("status/file" + i). + pathType(PathTypeEnum.OBJECT). + physicalAddress(s3Url("/repo-base/status" + i)). + checksum(STATUS_CHECKSUM). + mtime(STATUS_MTIME). + sizeBytes(STATUS_FILE_SIZE); + } + expectListing("repo", "main", + ImmutablePagination.builder().prefix("status/").build(), + objects); + expectStatObjectNotFound("repo", "main", "status"); + + Path dir = new Path("lakefs://repo/main/status"); + FileStatus[] fileStatuses = fs.listStatus(dir); + + FileStatus[] expectedFileStatuses = new LocatedFileStatus[totalObjectsCount]; + for (int i = 0; i < totalObjectsCount; i++) { + Path p = new Path(dir + "/file" + i); + LakeFSFileStatus fileStatus = new LakeFSFileStatus.Builder(p) + .length(STATUS_FILE_SIZE) + .checksum(STATUS_CHECKSUM) + .mTime(STATUS_MTIME) + .blockSize(Constants.DEFAULT_BLOCK_SIZE) + .physicalAddress(s3Url("/repo-base/status" + i)) + .build(); + expectedFileStatuses[i] = new LocatedFileStatus(fileStatus, null); + } + Assert.assertArrayEquals(expectedFileStatuses, fileStatuses); + } + + @Test(expected = UnsupportedOperationException.class) + public void testAppend() throws IOException { + fs.append(null, 0, null); + } + + /** + * rename(src.txt, non-existing-dst) -> non-existing/new - unsupported, should fail with false. (Test was buggy, FIX!) + */ + @Test + public void testRename_existingFileToNonExistingDst() throws IOException, ApiException { + Path src = new Path("lakefs://repo/main/existing.src"); + + ObjectStats stats = new ObjectStats() + .path("existing.src") + .sizeBytes(STATUS_FILE_SIZE) + .mtime(STATUS_MTIME) + .pathType(PathTypeEnum.OBJECT) + .physicalAddress(s3Url("existing.src")) + .checksum(STATUS_CHECKSUM); + + expectStatObject("repo", "main", "existing.src", stats); + + Path dst = new Path("lakefs://repo/main/non-existing/new"); + + expectListing("repo", "main", + ImmutablePagination.builder().prefix("non-existing/").build()); + expectStatObjectNotFound("repo", "main", "non-existing/new"); + expectStatObjectNotFound("repo", "main", "non-existing/new/"); + expectListing("repo", "main", + ImmutablePagination.builder().prefix("non-existing/new/").build()); + expectStatObjectNotFound("repo", "main", "non-existing"); + expectStatObjectNotFound("repo", "main", "non-existing/"); + + boolean renamed = fs.rename(src, dst); + Assert.assertFalse(renamed); + } + + @Test + public void testRename_existingFileToExistingFileName() throws IOException { + String contents = "The quick brown fox jumps over the lazy dog."; + byte[] contentsBytes = contents.getBytes(); + + Path src = new Path("lakefs://repo/main/existing.src"); + ObjectStats srcStats = new ObjectStats() + .path("existing.src") + .pathType(PathTypeEnum.OBJECT) + .physicalAddress("base/existing.src"); + expectStatObject("repo", "main", "existing.src", srcStats); + + Path dst = new Path("lakefs://repo/main/existing.dst"); + ObjectStats dstStats = new ObjectStats() + .pathType(PathTypeEnum.OBJECT) + .path("existing.dst") + .physicalAddress(s3Url("existing.dst")); + expectStatObject("repo", "main", "existing.dst", dstStats); + + mockServerClient.when(request() + .withMethod("POST") + .withPath("/repositories/repo/branches/main/objects/copy") + .withQueryStringParameter("dest_path", "existing.dst") + .withBody(json(gson.toJson(new ObjectCopyCreation() + .srcRef("main") + .srcPath("existing.src"))))) + .respond(response() + .withStatusCode(201) + // Actual new dstStats will be different... but lakeFSFS doesn't care. + .withBody(json(gson.toJson(dstStats)))); + + expectDeleteObject("repo", "main", "existing.src"); + + Assert.assertTrue(fs.rename(src, dst)); + } + + @Test + public void testRename_existingDirToExistingFileName() throws IOException { + Path fileInSrcDir = new Path("lakefs://repo/main/existing-dir/existing.src"); + ObjectStats srcStats = new ObjectStats() + .pathType(PathTypeEnum.OBJECT) + .path("existing-dir/existing.src"); + Path srcDir = new Path("lakefs://repo/main/existing-dir"); + expectStatObjectNotFound("repo", "main", "existing-dir"); + expectStatObjectNotFound("repo", "main", "existing-dir/"); + expectListing("repo", "main", + ImmutablePagination.builder().prefix("existing-dir/").build(), + srcStats); + + Path dst = new Path("lakefs://repo/main/existingdst.file"); + ObjectStats dstStats = new ObjectStats() + .pathType(PathTypeEnum.OBJECT) + .path("existingdst.file"); + expectStatObject("repo", "main", "existingdst.file", dstStats); + + Assert.assertFalse(fs.rename(srcDir, dst)); + } + + /** + * file -> existing-directory-name: rename(src.txt, existing-dstdir) -> existing-dstdir/src.txt + */ + @Test + public void testRename_existingFileToExistingDirName() throws ApiException, IOException { + Path src = new Path("lakefs://repo/main/existing-dir1/existing.src"); + ObjectStats srcStats = new ObjectStats() + .pathType(PathTypeEnum.OBJECT) + .path("existing-dir1/existing.src"); + expectStatObject("repo", "main", "existing-dir1/existing.src", srcStats); + + ObjectStats dstStats = new ObjectStats() + .pathType(PathTypeEnum.OBJECT) + .path("existing-dir2/existing.src"); + expectFileDoesNotExist("repo", "main", "existing-dir2"); + expectFileDoesNotExist("repo", "main", "existing-dir2/"); + expectListing("repo", "main", + ImmutablePagination.builder().prefix("existing-dir2/").build(), + dstStats); + + Path dst = new Path("lakefs://repo/main/existing-dir2/"); + + mockServerClient.when(request() + .withMethod("POST") + .withPath("/repositories/repo/branches/main/objects/copy") + .withQueryStringParameter("dest_path", "existing-dir2/existing.src") + .withBody(json(gson.toJson(new ObjectCopyCreation() + .srcRef("main") + .srcPath("existing-dir1/existing.src"))))) + .respond(response() + .withStatusCode(201) + // Actual new dstStats will be different... but lakeFSFS doesn't care. + .withBody(json(gson.toJson(dstStats)))); + expectGetBranch("repo", "main"); + expectDeleteObject("repo", "main", "existing-dir1/existing.src"); + + // Need a directory marker at the source because it's now empty! + expectUploadObject("repo", "main", "existing-dir1/"); + + Assert.assertTrue(fs.rename(src, dst)); + } + + /** + * rename(srcDir(containing srcDir/a.txt, srcDir/b.txt), non-existing-dir/new) -> unsupported, rename should fail by returning false + */ + @Test + public void testRename_existingDirToNonExistingDirWithoutParent() throws IOException { + Path fileInSrcDir = new Path("lakefs://repo/main/existing-dir/existing.src"); + Path srcDir = new Path("lakefs://repo/main/existing-dir"); + + expectFilesInDir("repo", "main", "existing-dir", "existing.src"); + + expectFileDoesNotExist("repo", "main", "x/non-existing-dir"); + expectFileDoesNotExist("repo", "main", "x/non-existing-dir/new"); + // Will also check if parent of destination is a directory (it isn't). + expectListing("repo", "main", + ImmutablePagination.builder().prefix("x/non-existing-dir/").build()); + expectListing("repo", "main", + ImmutablePagination.builder().prefix("x/non-existing-dir/new/").build()); + + // Keep a directory marker, or rename will try to create one because + // it emptied the existing directory. + expectStatObject("repo", "main", "x", + new ObjectStats().pathType(PathTypeEnum.OBJECT).path("x")); + + Path dst = new Path("lakefs://repo/main/x/non-existing-dir/new"); + + Assert.assertFalse(fs.rename(srcDir, dst)); + } + + /** + * rename(srcDir(containing srcDir/a.txt, srcDir/b.txt), non-existing-dir/new) -> unsupported, rename should fail by returning false + */ + @Test + public void testRename_existingDirToNonExistingDirWithParent() throws ApiException, IOException { + Path fileInSrcDir = new Path("lakefs://repo/main/existing-dir/existing.src"); + Path srcDir = new Path("lakefs://repo/main/existing-dir"); + Path dst = new Path("lakefs://repo/main/existing-dir2/new"); + + ObjectStats srcStats = new ObjectStats() + .pathType(PathTypeEnum.OBJECT) + .path("existing-dir/existing.src"); + + expectStatObjectNotFound("repo", "main", "existing-dir"); + expectStatObjectNotFound("repo", "main", "existing-dir/"); + expectListing("repo", "main", ImmutablePagination.builder().prefix("existing-dir/").build(), + srcStats); + + expectStatObjectNotFound("repo", "main", "existing-dir2"); + expectStatObject("repo", "main", "existing-dir2/", + new ObjectStats().pathType(PathTypeEnum.OBJECT).path("existing-dir2/")); + + expectStatObjectNotFound("repo", "main", "existing-dir2/new"); + expectStatObjectNotFound("repo", "main", "existing-dir2/new/"); + expectListing("repo", "main", ImmutablePagination.builder().prefix("existing-dir2/new/").build()); + + ObjectStats dstStats = new ObjectStats() + .pathType(PathTypeEnum.OBJECT) + .path("existing-dir2/new/existing.src"); + + mockServerClient.when(request() + .withMethod("POST") + .withPath("/repositories/repo/branches/main/objects/copy") + .withQueryStringParameter("dest_path", "existing-dir2/new/existing.src") + .withBody(json(gson.toJson(new ObjectCopyCreation() + .srcRef("main") + .srcPath("existing-dir/existing.src"))))) + .respond(response() + .withStatusCode(201) + .withBody(json(gson.toJson(dstStats)))); + expectDeleteObject("repo", "main", "existing-dir/existing.src"); + // Directory marker no longer required. + expectDeleteObject("repo", "main", "existing-dir2/"); + + boolean renamed = fs.rename(srcDir, dst); + Assert.assertTrue(renamed); + } + + // /** + // * rename(srcDir(containing srcDir/a.txt), existing-nonempty-dstdir) -> unsupported, rename should fail by returning false. + // */ + // @Test + // public void testRename_existingDirToExistingNonEmptyDirName() throws ApiException, IOException { + // Path firstSrcFile = new Path("lakefs://repo/main/existing-dir1/a.src"); + // ObjectLocation firstObjLoc = fs.pathToObjectLocation(firstSrcFile); + // Path secSrcFile = new Path("lakefs://repo/main/existing-dir1/b.src"); + // ObjectLocation secObjLoc = fs.pathToObjectLocation(secSrcFile); + + // Path srcDir = new Path("lakefs://repo/main/existing-dir1"); + // ObjectLocation srcDirObjLoc = fs.pathToObjectLocation(srcDir); + // mockExistingDirPath(srcDirObjLoc, ImmutableList.of(firstObjLoc, secObjLoc)); + + // Path fileInDstDir = new Path("lakefs://repo/main/existing-dir2/file.dst"); + // ObjectLocation dstFileObjLoc = fs.pathToObjectLocation(fileInDstDir); + // Path dstDir = new Path("lakefs://repo/main/existing-dir2"); + // ObjectLocation dstDirObjLoc = fs.pathToObjectLocation(dstDir); + // mockExistingDirPath(dstDirObjLoc, ImmutableList.of(dstFileObjLoc)); + + // boolean renamed = fs.rename(srcDir, dstDir); + // Assert.assertFalse(renamed); + // } + + // /** + // * Check that a file is renamed when working against a lakeFS version + // * where CopyObject API doesn't exist + // */ + // @Test + // public void testRename_fallbackStageAPI() throws ApiException, IOException { + // Path src = new Path("lakefs://repo/main/existing-dir1/existing.src"); + // ObjectLocation srcObjLoc = fs.pathToObjectLocation(src); + // mockExistingFilePath(srcObjLoc); + + // Path fileInDstDir = new Path("lakefs://repo/main/existing-dir2/existing.src"); + // ObjectLocation fileObjLoc = fs.pathToObjectLocation(fileInDstDir); + // Path dst = new Path("lakefs://repo/main/existing-dir2"); + // ObjectLocation dstObjLoc = fs.pathToObjectLocation(dst); + + // mockExistingDirPath(dstObjLoc, ImmutableList.of(fileObjLoc)); + // mockDirectoryMarker(fs.pathToObjectLocation(src.getParent())); + // mockMissingCopyAPI(); + + // boolean renamed = fs.rename(src, dst); + // Assert.assertTrue(renamed); + // Path expectedDstPath = new Path("lakefs://repo/main/existing-dir2/existing.src"); + // Assert.assertTrue(dstPathLinkedToSrcPhysicalAddress(srcObjLoc, fs.pathToObjectLocation(expectedDstPath))); + // verifyObjDeletion(srcObjLoc); + // } + + // @Test + // public void testRename_srcAndDstOnDifferentBranch() throws IOException, ApiException { + // Path src = new Path("lakefs://repo/branch/existing.src"); + // Path dst = new Path("lakefs://repo/another-branch/existing.dst"); + // boolean renamed = fs.rename(src, dst); + // Assert.assertFalse(renamed); + // Mockito.verify(objectsApi, never()).statObject(any(), any(), any(), any(), any()); + // Mockito.verify(objectsApi, never()).copyObject(any(), any(), any(), any()); + // Mockito.verify(objectsApi, never()).deleteObject(any(), any(), any()); + // } + + // /** + // * no-op. rename is expected to succeed. + // */ // @Test - // public void testOpen() throws IOException, ApiException { - // String contents = "The quick brown fox jumps over the lazy dog."; - // byte[] contentsBytes = contents.getBytes(); - // String physicalKey = "/repo-base/open"; - // int readBufferSize = 5; - - // // Write physical file to S3. - // ObjectMetadata s3Metadata = new ObjectMetadata(); - // s3Metadata.setContentLength(contentsBytes.length); - // s3Client.putObject(new PutObjectRequest(s3Bucket, physicalKey, new ByteArrayInputStream(contentsBytes), s3Metadata)); - - // Path p = new Path("lakefs://repo/main/read.me"); - // mockStatObject("repo", "main", "read.me", physicalKey, (long)contentsBytes.length); - // try (InputStream in = fs.open(p, readBufferSize)) { - // String actual = IOUtils.toString(in); - // Assert.assertEquals(contents, actual); - // } + // public void testRename_srcEqualsDst() throws IOException, ApiException { + // Path src = new Path("lakefs://repo/main/existing.src"); + // Path dst = new Path("lakefs://repo/main/existing.src"); + // boolean renamed = fs.rename(src, dst); + // Assert.assertTrue(renamed); + // Mockito.verify(objectsApi, never()).statObject(any(), any(), any(), any(), any()); + // Mockito.verify(objectsApi, never()).copyObject(any(), any(), any(), any()); + // Mockito.verify(objectsApi, never()).deleteObject(any(), any(), any()); // } // @Test - // public void testOpenWithInvalidUriChars() throws IOException, ApiException { - // String contents = "The quick brown fox jumps over the lazy dog."; - // byte[] contentsBytes = contents.getBytes(); - // int readBufferSize = 5; - - // String[] keys = { - // "/repo-base/with space/open", - // "/repo-base/wi:th$cha&rs#/%op;e?n", - // "/repo-base/עכשיו/בעברית/open", - // "/repo-base/\uD83E\uDD2F/imoji/open", - // }; - // for (String key : keys) { - // // Write physical file to S3. - // ObjectMetadata s3Metadata = new ObjectMetadata(); - // s3Metadata.setContentLength(contentsBytes.length); - // s3Client.putObject(new PutObjectRequest(s3Bucket, key, new ByteArrayInputStream(contentsBytes), s3Metadata)); - - // Path p = new Path("lakefs://repo/main/read.me"); - // mockStatObject("repo", "main", "read.me", key, (long) contentsBytes.length); - // try (InputStream in = fs.open(p, readBufferSize)) { - // String actual = IOUtils.toString(in); - // Assert.assertEquals(contents, actual); - // } - // } + // public void testRename_nonExistingSrcFile() throws ApiException, IOException { + // Path src = new Path("lakefs://repo/main/non-existing.src"); + // ObjectLocation srcObjLoc = fs.pathToObjectLocation(src); + // mockNonExistingPath(srcObjLoc); + + // Path dst = new Path("lakefs://repo/main/existing.dst"); + // ObjectLocation dstObjLoc = fs.pathToObjectLocation(dst); + // mockExistingFilePath(dstObjLoc); + + // boolean success = fs.rename(src, dst); + // Assert.assertFalse(success); // } - // @Test(expected = FileNotFoundException.class) - // public void testOpen_NotExists() throws IOException, ApiException { - // Path p = new Path("lakefs://repo/main/doesNotExi.st"); - // when(objectsApi.statObject(any(), any(), any(), any(), any())) - // .thenThrow(noSuchFile); - // fs.open(p); + // /** + // * globStatus is used only by the Hadoop CLI where the pattern is always the exact file. + // */ + // @Test + // public void testGlobStatus_SingleFile() throws ApiException, IOException { + // Path path = new Path("lakefs://repo/main/existing.dst"); + // ObjectLocation dstObjLoc = fs.pathToObjectLocation(path); + // mockExistingFilePath(dstObjLoc); + + // FileStatus[] statuses = fs.globStatus(path); + // Assert.assertArrayEquals(new FileStatus[]{ + // new LakeFSFileStatus.Builder(path).build() + // }, statuses); // } } diff --git a/clients/hadoopfs/src/test/java/io/lakefs/LakeFSFileSystemTest.java b/clients/hadoopfs/src/test/java/io/lakefs/LakeFSFileSystemTest.java index 1a64cfb07b3..412e9eaa5e4 100644 --- a/clients/hadoopfs/src/test/java/io/lakefs/LakeFSFileSystemTest.java +++ b/clients/hadoopfs/src/test/java/io/lakefs/LakeFSFileSystemTest.java @@ -925,7 +925,7 @@ public void testRename_existingFileToExistingDirName() throws ApiException, IOEx ObjectLocation fileObjLoc = fs.pathToObjectLocation(fileInDstDir); Path dst = new Path("lakefs://repo/main/existing-dir2"); ObjectLocation dstObjLoc = fs.pathToObjectLocation(dst); - mockExistingDirPath(dstObjLoc, ImmutableList.of(fileObjLoc)); + // mockExistingDirPath(dstObjLoc, ImmutableList.of(fileObjLoc)); mockDirectoryMarker(fs.pathToObjectLocation(src.getParent()));