From 36583995ce0622ea013805a05379681d9a611a5b Mon Sep 17 00:00:00 2001 From: Barak Amar Date: Wed, 29 Jan 2025 23:53:59 +0200 Subject: [PATCH 1/6] test: Add ImmutablePagination to remove the use of Value.Immutable - Added ImmutablePagination class to handle pagination parameters in tests. - Updated test setup to use ImmutablePagination class for pagination parameters. - This change removes the use of @Value.Immutable in the test and works with latest Java compilers. --- .../src/test/java/io/lakefs/FSTestBase.java | 24 ++------ .../java/io/lakefs/ImmutablePagination.java | 56 +++++++++++++++++++ .../io/lakefs/LakeFSFileSystemServerTest.java | 18 ------ 3 files changed, 60 insertions(+), 38 deletions(-) create mode 100644 clients/hadoopfs/src/test/java/io/lakefs/ImmutablePagination.java diff --git a/clients/hadoopfs/src/test/java/io/lakefs/FSTestBase.java b/clients/hadoopfs/src/test/java/io/lakefs/FSTestBase.java index 6b41dd1da4b..3e996a28b27 100644 --- a/clients/hadoopfs/src/test/java/io/lakefs/FSTestBase.java +++ b/clients/hadoopfs/src/test/java/io/lakefs/FSTestBase.java @@ -1,15 +1,7 @@ package io.lakefs; -import com.amazonaws.ClientConfiguration; -import com.amazonaws.auth.AWSCredentials; -import com.amazonaws.auth.BasicAWSCredentials; -import com.amazonaws.services.s3.AmazonS3; -import com.amazonaws.services.s3.AmazonS3Client; -import com.amazonaws.services.s3.S3ClientOptions; -import com.amazonaws.services.s3.model.*; import com.aventrix.jnanoid.jnanoid.NanoIdUtils; import com.google.common.base.Optional; -import com.google.common.collect.ImmutableMap; import com.google.gson.FieldNamingPolicy; import com.google.gson.Gson; import com.google.gson.GsonBuilder; @@ -20,27 +12,20 @@ import io.lakefs.utils.ObjectLocation; import org.apache.hadoop.conf.Configuration; -import org.apache.hadoop.fs.FileAlreadyExistsException; -import org.apache.hadoop.fs.FileStatus; -import org.apache.hadoop.fs.LocatedFileStatus; import org.apache.hadoop.fs.Path; import org.apache.http.HttpStatus; import org.immutables.value.Value; import org.junit.Before; import org.junit.Rule; -import org.junit.Test; import org.junit.rules.TestName; import org.mockserver.client.MockServerClient; import org.mockserver.junit.MockServerRule; -import org.mockserver.matchers.MatchType; import org.mockserver.matchers.TimeToLive; import org.mockserver.matchers.Times; import org.mockserver.model.Cookie; import org.mockserver.model.HttpRequest; -import org.mockserver.model.HttpResponse; -import org.mockserver.model.Parameter; import static org.apache.commons.lang3.StringUtils.removeStart; @@ -51,7 +36,6 @@ import java.net.URI; import java.net.URISyntaxException; import java.util.Arrays; -import java.util.List; /** * Base for all LakeFSFilesystem tests. Helps set common components up but @@ -85,10 +69,10 @@ public abstract class FSTestBase { .setFieldNamingPolicy(FieldNamingPolicy.LOWER_CASE_WITH_UNDERSCORES) .create(); - @Value.Immutable static public interface Pagination { - @Value.Parameter Optional amount(); - @Value.Parameter Optional after(); - @Value.Parameter Optional prefix(); + static public interface Pagination { + Optional amount(); + Optional after(); + Optional prefix(); } @Rule diff --git a/clients/hadoopfs/src/test/java/io/lakefs/ImmutablePagination.java b/clients/hadoopfs/src/test/java/io/lakefs/ImmutablePagination.java new file mode 100644 index 00000000000..ed29fa6fd50 --- /dev/null +++ b/clients/hadoopfs/src/test/java/io/lakefs/ImmutablePagination.java @@ -0,0 +1,56 @@ +package io.lakefs; + +import com.google.common.base.Optional; + +public final class ImmutablePagination implements FSTestBase.Pagination { + private final Optional amount; + private final Optional after; + private final Optional prefix; + + private ImmutablePagination(Optional amount, Optional after, Optional prefix) { + this.amount = amount; + this.after = after; + this.prefix = prefix; + } + + public Optional amount() { + return this.amount; + } + + public Optional after() { + return this.after; + } + + public Optional prefix() { + return this.prefix; + } + + public static Builder builder() { + return new Builder(); + } + + public static class Builder { + private Optional amount = Optional.absent(); + private Optional after = Optional.absent(); + private Optional prefix = Optional.absent(); + + public Builder amount(Integer amount) { + this.amount = Optional.fromNullable(amount); + return this; + } + + public Builder after(String after) { + this.after = Optional.fromNullable(after); + return this; + } + + public Builder prefix(String prefix) { + this.prefix = Optional.fromNullable(prefix); + return this; + } + + public ImmutablePagination build() { + return new ImmutablePagination(amount, after, prefix); + } + } +} diff --git a/clients/hadoopfs/src/test/java/io/lakefs/LakeFSFileSystemServerTest.java b/clients/hadoopfs/src/test/java/io/lakefs/LakeFSFileSystemServerTest.java index e6f394d3d8d..e9e34f61e57 100644 --- a/clients/hadoopfs/src/test/java/io/lakefs/LakeFSFileSystemServerTest.java +++ b/clients/hadoopfs/src/test/java/io/lakefs/LakeFSFileSystemServerTest.java @@ -5,40 +5,22 @@ import io.lakefs.clients.sdk.*; import io.lakefs.clients.sdk.model.*; -import io.lakefs.clients.sdk.model.ObjectStats.PathTypeEnum; import io.lakefs.utils.ObjectLocation; import com.google.common.collect.ImmutableMap; -import com.google.common.collect.ImmutableList; -import com.google.common.collect.Lists; -import org.apache.commons.io.IOUtils; import org.apache.hadoop.fs.FileStatus; import org.apache.hadoop.fs.LocatedFileStatus; import org.apache.hadoop.fs.Path; -import org.apache.http.HttpStatus; import org.junit.Assert; import org.junit.Test; import org.hamcrest.core.StringContains; -import org.mockserver.client.MockServerClient; -import org.mockserver.matchers.TimeToLive; -import org.mockserver.matchers.Times; -import org.mockserver.model.Cookie; -import org.mockserver.model.HttpRequest; -import org.mockserver.model.HttpResponse; -import org.mockserver.model.Parameter; - import static org.mockserver.model.HttpResponse.response; import static org.mockserver.model.JsonBody.json; import java.io.*; import java.net.URI; -import java.net.URISyntaxException; -import java.util.ArrayList; -import java.util.Collections; -import java.util.HashMap; -import java.util.List; import java.util.Map; public class LakeFSFileSystemServerTest extends FSTestBase { From 987294ebaadd419dccfff93c97635b76a005d49d Mon Sep 17 00:00:00 2001 From: Barak Amar Date: Thu, 30 Jan 2025 00:40:49 +0200 Subject: [PATCH 2/6] Remove immutable package and unused import --- clients/hadoopfs/pom.xml | 6 ------ clients/hadoopfs/src/test/java/io/lakefs/FSTestBase.java | 1 - 2 files changed, 7 deletions(-) diff --git a/clients/hadoopfs/pom.xml b/clients/hadoopfs/pom.xml index ff31651e65a..1bfa78f276b 100644 --- a/clients/hadoopfs/pom.xml +++ b/clients/hadoopfs/pom.xml @@ -391,12 +391,6 @@ To export to S3: 5.15.0 test - - org.immutables - value - 2.9.3 - test - com.google.guava guava diff --git a/clients/hadoopfs/src/test/java/io/lakefs/FSTestBase.java b/clients/hadoopfs/src/test/java/io/lakefs/FSTestBase.java index 3e996a28b27..15a78bc6e6a 100644 --- a/clients/hadoopfs/src/test/java/io/lakefs/FSTestBase.java +++ b/clients/hadoopfs/src/test/java/io/lakefs/FSTestBase.java @@ -15,7 +15,6 @@ import org.apache.hadoop.fs.Path; import org.apache.http.HttpStatus; -import org.immutables.value.Value; import org.junit.Before; import org.junit.Rule; import org.junit.rules.TestName; From 939d812d68664e1e9b47bdd1ba5d359c89cb8dcf Mon Sep 17 00:00:00 2001 From: Barak Amar Date: Sat, 1 Feb 2025 10:55:41 +0200 Subject: [PATCH 3/6] Reduce dep switch from Google's Options to Java's Options --- .../src/test/java/io/lakefs/FSTestBase.java | 6 +++--- .../test/java/io/lakefs/ImmutablePagination.java | 14 +++++++------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/clients/hadoopfs/src/test/java/io/lakefs/FSTestBase.java b/clients/hadoopfs/src/test/java/io/lakefs/FSTestBase.java index 15a78bc6e6a..e2a3cb412f1 100644 --- a/clients/hadoopfs/src/test/java/io/lakefs/FSTestBase.java +++ b/clients/hadoopfs/src/test/java/io/lakefs/FSTestBase.java @@ -1,7 +1,6 @@ package io.lakefs; import com.aventrix.jnanoid.jnanoid.NanoIdUtils; -import com.google.common.base.Optional; import com.google.gson.FieldNamingPolicy; import com.google.gson.Gson; import com.google.gson.GsonBuilder; @@ -35,6 +34,7 @@ import java.net.URI; import java.net.URISyntaxException; import java.util.Arrays; +import java.util.Optional; /** * Base for all LakeFSFilesystem tests. Helps set common components up but @@ -310,13 +310,13 @@ protected void mockListingWithHasMore(String repo, String ref, ImmutablePaginati .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("")); + req = req.withQueryStringParameter("after", pagination.after().orElse("")); } if (pagination.amount().isPresent()) { req = req.withQueryStringParameter("amount", pagination.amount().get().toString()); } if (pagination.prefix().isPresent()) { - req = req.withQueryStringParameter("prefix", pagination.prefix().or("")); + req = req.withQueryStringParameter("prefix", pagination.prefix().orElse("")); } ObjectStatsList resp = new ObjectStatsList() .results(Arrays.asList(stats)) diff --git a/clients/hadoopfs/src/test/java/io/lakefs/ImmutablePagination.java b/clients/hadoopfs/src/test/java/io/lakefs/ImmutablePagination.java index ed29fa6fd50..7850e7fd9e4 100644 --- a/clients/hadoopfs/src/test/java/io/lakefs/ImmutablePagination.java +++ b/clients/hadoopfs/src/test/java/io/lakefs/ImmutablePagination.java @@ -1,6 +1,6 @@ package io.lakefs; -import com.google.common.base.Optional; +import java.util.Optional; public final class ImmutablePagination implements FSTestBase.Pagination { private final Optional amount; @@ -30,22 +30,22 @@ public static Builder builder() { } public static class Builder { - private Optional amount = Optional.absent(); - private Optional after = Optional.absent(); - private Optional prefix = Optional.absent(); + private Optional amount = Optional.empty(); + private Optional after = Optional.empty(); + private Optional prefix = Optional.empty(); public Builder amount(Integer amount) { - this.amount = Optional.fromNullable(amount); + this.amount = Optional.of(amount); return this; } public Builder after(String after) { - this.after = Optional.fromNullable(after); + this.after = Optional.of(after); return this; } public Builder prefix(String prefix) { - this.prefix = Optional.fromNullable(prefix); + this.prefix = Optional.of(prefix); return this; } From c958ede219713d37184279d7a9bfd3fa774eb681 Mon Sep 17 00:00:00 2001 From: Barak Amar Date: Sat, 1 Feb 2025 18:24:17 +0200 Subject: [PATCH 4/6] Use the pagination class directly and drop the interface --- .../src/test/java/io/lakefs/FSTestBase.java | 25 +++----- .../java/io/lakefs/ImmutablePagination.java | 56 ----------------- .../lakefs/LakeFSFileSystemServerS3Test.java | 2 +- .../io/lakefs/LakeFSFileSystemServerTest.java | 62 +++++++++---------- .../src/test/java/io/lakefs/Pagination.java | 54 ++++++++++++++++ 5 files changed, 95 insertions(+), 104 deletions(-) delete mode 100644 clients/hadoopfs/src/test/java/io/lakefs/ImmutablePagination.java create mode 100644 clients/hadoopfs/src/test/java/io/lakefs/Pagination.java diff --git a/clients/hadoopfs/src/test/java/io/lakefs/FSTestBase.java b/clients/hadoopfs/src/test/java/io/lakefs/FSTestBase.java index e2a3cb412f1..3f28527c50e 100644 --- a/clients/hadoopfs/src/test/java/io/lakefs/FSTestBase.java +++ b/clients/hadoopfs/src/test/java/io/lakefs/FSTestBase.java @@ -34,7 +34,6 @@ import java.net.URI; import java.net.URISyntaxException; import java.util.Arrays; -import java.util.Optional; /** * Base for all LakeFSFilesystem tests. Helps set common components up but @@ -68,12 +67,6 @@ public abstract class FSTestBase { .setFieldNamingPolicy(FieldNamingPolicy.LOWER_CASE_WITH_UNDERSCORES) .create(); - static public interface Pagination { - Optional amount(); - Optional after(); - Optional prefix(); - } - @Rule public MockServerRule mockServerRule = new MockServerRule(this); protected MockServerClient mockServerClient; @@ -228,7 +221,7 @@ protected void mockFilesInDir(String repo, String main, String dir, String... fi // Directory can be listed! mockListing("repo", "main", - ImmutablePagination.builder().prefix(dir + Constants.SEPARATOR).build(), + Pagination.builder().prefix(dir + Constants.SEPARATOR).build(), allStats); } @@ -300,23 +293,23 @@ protected ObjectStats mockDirectoryMarker(ObjectLocation objectLoc) { } // Mock this listing and return these stats. - protected void mockListing(String repo, String ref, ImmutablePagination pagination, ObjectStats... stats) { + protected void mockListing(String repo, String ref, Pagination pagination, ObjectStats... stats) { mockListingWithHasMore(repo, ref, pagination, false, stats); } - protected void mockListingWithHasMore(String repo, String ref, ImmutablePagination pagination, boolean hasMore, ObjectStats... stats) { + protected void mockListingWithHasMore(String repo, String ref, Pagination pagination, boolean hasMore, ObjectStats... stats) { HttpRequest req = request() .withMethod("GET") .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().orElse("")); + if (pagination.after() != null) { + req = req.withQueryStringParameter("after", pagination.after()); } - if (pagination.amount().isPresent()) { - req = req.withQueryStringParameter("amount", pagination.amount().get().toString()); + if (pagination.amount() != null) { + req = req.withQueryStringParameter("amount", pagination.amount().toString()); } - if (pagination.prefix().isPresent()) { - req = req.withQueryStringParameter("prefix", pagination.prefix().orElse("")); + if (pagination.prefix() != null) { + req = req.withQueryStringParameter("prefix", pagination.prefix()); } ObjectStatsList resp = new ObjectStatsList() .results(Arrays.asList(stats)) diff --git a/clients/hadoopfs/src/test/java/io/lakefs/ImmutablePagination.java b/clients/hadoopfs/src/test/java/io/lakefs/ImmutablePagination.java deleted file mode 100644 index 7850e7fd9e4..00000000000 --- a/clients/hadoopfs/src/test/java/io/lakefs/ImmutablePagination.java +++ /dev/null @@ -1,56 +0,0 @@ -package io.lakefs; - -import java.util.Optional; - -public final class ImmutablePagination implements FSTestBase.Pagination { - private final Optional amount; - private final Optional after; - private final Optional prefix; - - private ImmutablePagination(Optional amount, Optional after, Optional prefix) { - this.amount = amount; - this.after = after; - this.prefix = prefix; - } - - public Optional amount() { - return this.amount; - } - - public Optional after() { - return this.after; - } - - public Optional prefix() { - return this.prefix; - } - - public static Builder builder() { - return new Builder(); - } - - public static class Builder { - private Optional amount = Optional.empty(); - private Optional after = Optional.empty(); - private Optional prefix = Optional.empty(); - - public Builder amount(Integer amount) { - this.amount = Optional.of(amount); - return this; - } - - public Builder after(String after) { - this.after = Optional.of(after); - return this; - } - - public Builder prefix(String prefix) { - this.prefix = Optional.of(prefix); - return this; - } - - public ImmutablePagination build() { - return new ImmutablePagination(amount, after, prefix); - } - } -} diff --git a/clients/hadoopfs/src/test/java/io/lakefs/LakeFSFileSystemServerS3Test.java b/clients/hadoopfs/src/test/java/io/lakefs/LakeFSFileSystemServerS3Test.java index ef70cc74ac4..3589a5d3e99 100644 --- a/clients/hadoopfs/src/test/java/io/lakefs/LakeFSFileSystemServerS3Test.java +++ b/clients/hadoopfs/src/test/java/io/lakefs/LakeFSFileSystemServerS3Test.java @@ -172,7 +172,7 @@ public void testMkdirs() throws IOException { for (Path p = new Path(path.toString()); p != null && !p.isRoot(); p = p.getParent()) { mockStatObjectNotFound("repo", "main", p.toString()); mockStatObjectNotFound("repo", "main", p+"/"); - mockListing("repo", "main", ImmutablePagination.builder().prefix(p+"/").build()); + mockListing("repo", "main", Pagination.builder().prefix(p+"/").build()); } // physical address to directory marker object diff --git a/clients/hadoopfs/src/test/java/io/lakefs/LakeFSFileSystemServerTest.java b/clients/hadoopfs/src/test/java/io/lakefs/LakeFSFileSystemServerTest.java index e9e34f61e57..b2702536f5e 100644 --- a/clients/hadoopfs/src/test/java/io/lakefs/LakeFSFileSystemServerTest.java +++ b/clients/hadoopfs/src/test/java/io/lakefs/LakeFSFileSystemServerTest.java @@ -81,7 +81,7 @@ public void testGetFileStatus_NoFile() { mockStatObjectNotFound("repo", "main", "no.file"); mockStatObjectNotFound("repo", "main", "no.file/"); - mockListing("repo", "main", ImmutablePagination.builder().prefix("no.file/").amount(1).build()); + mockListing("repo", "main", Pagination.builder().prefix("no.file/").amount(1).build()); Assert.assertThrows(FileNotFoundException.class, () -> fs.getFileStatus(noFilePath)); } @@ -102,7 +102,7 @@ public void testGetFileStatus_DirectoryMarker() throws IOException { public void testExists_ExistsAsObject() throws IOException { Path path = new Path("lakefs://repo/main/exis.ts"); ObjectStats stats = makeObjectStats("exis.ts"); - mockListing("repo", "main", ImmutablePagination.builder().prefix("exis.ts").build(), stats); + mockListing("repo", "main", Pagination.builder().prefix("exis.ts").build(), stats); Assert.assertTrue(fs.exists(path)); } @@ -111,7 +111,7 @@ public void testExists_ExistsAsDirectoryMarker() throws IOException { Path path = new Path("lakefs://repo/main/exis.ts"); ObjectStats stats = makeObjectStats("exis.ts"); - mockListing("repo", "main", ImmutablePagination.builder().prefix("exis.ts").build(), + mockListing("repo", "main", Pagination.builder().prefix("exis.ts").build(), stats); Assert.assertTrue(fs.exists(path)); @@ -122,7 +122,7 @@ public void testExists_ExistsAsDirectoryContents() throws IOException { Path path = new Path("lakefs://repo/main/exis.ts"); ObjectStats stats = makeObjectStats("exis.ts/object-inside-the-path"); - mockListing("repo", "main", ImmutablePagination.builder().prefix("exis.ts").build(), + mockListing("repo", "main", Pagination.builder().prefix("exis.ts").build(), stats); Assert.assertTrue(fs.exists(path)); } @@ -136,11 +136,11 @@ public void testExists_ExistsAsDirectoryInSecondList() throws IOException { // First listing returns irrelevant objects, _before_ "exis.ts/" mockListingWithHasMore("repo", "main", - ImmutablePagination.builder().prefix("exis.ts").build(), + Pagination.builder().prefix("exis.ts").build(), true, beforeStats1, beforeStats2); // Second listing tries to find an object inside "exis.ts/". - mockListing("repo", "main", ImmutablePagination.builder().prefix("exis.ts/").build(), + mockListing("repo", "main", Pagination.builder().prefix("exis.ts/").build(), indirStats); Assert.assertTrue(fs.exists(path)); } @@ -148,7 +148,7 @@ public void testExists_ExistsAsDirectoryInSecondList() throws IOException { @Test public void testExists_NotExistsNoPrefix() throws IOException { Path path = new Path("lakefs://repo/main/doesNotExi.st"); - mockListing("repo", "main", ImmutablePagination.builder().prefix("doesNotExi.st").build()); + mockListing("repo", "main", Pagination.builder().prefix("doesNotExi.st").build()); Assert.assertFalse(fs.exists(path)); } @@ -171,7 +171,7 @@ public void testDelete_FileExists() throws IOException { for (String dir: arrDirs) { mockStatObjectNotFound("repo", "main", dir); mockStatObjectNotFound("repo", "main", dir + "/"); - mockListing("repo", "main", ImmutablePagination.builder().build()); + mockListing("repo", "main", Pagination.builder().build()); } mockDeleteObject("repo", "main", "no/place/file.txt"); mockUploadObject("repo", "main", "no/place/"); @@ -189,7 +189,7 @@ public void testDelete_FileNotExists() throws IOException { mockStatObjectNotFound("repo", "main", "no/place/file.txt"); mockStatObjectNotFound("repo", "main", "no/place/file.txt/"); mockListing("repo", "main", - ImmutablePagination.builder().prefix("no/place/file.txt/").build()); + Pagination.builder().prefix("no/place/file.txt/").build()); // Should still create a directory marker! mockUploadObject("repo", "main", "no/place/"); @@ -208,11 +208,11 @@ public void testDelete_EmptyDirectoryExists() throws IOException { // Just a directory marker delete/me/, so nothing to delete. mockListing("repo", "main", - ImmutablePagination.builder().prefix("delete/me/").build(), + Pagination.builder().prefix("delete/me/").build(), srcStats); // Mock listing in createDirectoryMarkerIfNotExists to return listing - mockListing("repo", "main", ImmutablePagination.builder().prefix("delete/").build()); + mockListing("repo", "main", Pagination.builder().prefix("delete/").build()); mockDirectoryMarker(dirObjLoc.getParent()); mockStatObject(dirObjLoc.getRepository(), dirObjLoc.getRef(), dirObjLoc.getPath(), srcStats); mockDeleteObject("repo", "main", "delete/me/"); @@ -235,7 +235,7 @@ public void testDelete_DirectoryWithFile() throws IOException { // marker for delete/sample/. ObjectStats srcStats = makeObjectStats(existingPath); mockListing("repo", "main", - ImmutablePagination.builder() + Pagination.builder() .prefix(directoryPath + Constants.SEPARATOR) .build(), srcStats); @@ -264,7 +264,7 @@ public void testDelete_NotExistsRecursive() throws IOException { .respond(response().withStatusCode(404)); // No objects to list, either -- in directory. mockListing("repo", "main", - ImmutablePagination.builder().prefix("no/place/file.txt/").build()); + Pagination.builder().prefix("no/place/file.txt/").build()); Assert.assertFalse(fs.delete(new Path("lakefs://repo/main/no/place/file.txt"), true)); } @@ -274,7 +274,7 @@ public void testDelete_DirectoryWithFileRecursive() throws IOException { mockStatObjectNotFound("repo", "main", "delete/sample/"); ObjectStats stats = makeObjectStats("delete/sample/file.txt"); mockListing("repo", "main", - ImmutablePagination.builder().prefix("delete/sample/").build(), + Pagination.builder().prefix("delete/sample/").build(), stats); mockDeleteObjects("repo", "main", "delete/sample/file.txt"); @@ -284,7 +284,7 @@ public void testDelete_DirectoryWithFileRecursive() throws IOException { // Mock listing in createDirectoryMarkerIfNotExists to return empty path mockListing("repo", "main", - ImmutablePagination.builder().prefix("delete/").build()); + Pagination.builder().prefix("delete/").build()); mockDirectoryMarker(ObjectLocation.pathToObjectLocation(null, path.getParent())); // Must create a parent directory marker: it wasn't deleted, and now @@ -305,7 +305,7 @@ protected void caseDeleteDirectoryRecursive(int bulkSize, int numObjects) throws objects[i] = makeObjectStats(String.format("delete/sample/file%04d.txt", i)); } mockListing("repo", "main", - ImmutablePagination.builder().prefix("delete/sample/").build(), + Pagination.builder().prefix("delete/sample/").build(), objects); // Set up multiple deleteObjects expectations of bulkSize deletes @@ -319,7 +319,7 @@ protected void caseDeleteDirectoryRecursive(int bulkSize, int numObjects) throws } // Mock listing in createDirectoryMarkerIfNotExists to return empty path mockListing("repo", "main", - ImmutablePagination.builder().prefix("delete/").build()); + Pagination.builder().prefix("delete/").build()); // Mock parent directory marker creation at end of fs.delete to show // the directory marker exists. mockUploadObject("repo", "main", "delete/"); @@ -376,7 +376,7 @@ public void testListStatusNotFound() throws ApiException { mockStatObjectNotFound("repo", "main", "status/file"); mockStatObjectNotFound("repo", "main", "status/file/"); mockListing("repo", "main", - ImmutablePagination.builder().prefix("status/file/").build()); + Pagination.builder().prefix("status/file/").build()); Path path = new Path("lakefs://repo/main/status/file"); Assert.assertThrows(FileNotFoundException.class, () -> fs.listStatus(path)); } @@ -389,7 +389,7 @@ public void testListStatusDirectory() throws IOException { objects[i] = makeObjectStats("status/file" + i); } mockListing("repo", "main", - ImmutablePagination.builder().prefix("status/").build(), + Pagination.builder().prefix("status/").build(), objects); mockStatObjectNotFound("repo", "main", "status"); @@ -429,11 +429,11 @@ public void testRename_existingFileToNonExistingDst() throws IOException, ApiExc Path dst = new Path("lakefs://repo/main/non-existing/new"); mockListing("repo", "main", - ImmutablePagination.builder().prefix("non-existing/").build()); + Pagination.builder().prefix("non-existing/").build()); mockStatObjectNotFound("repo", "main", "non-existing/new"); mockStatObjectNotFound("repo", "main", "non-existing/new/"); mockListing("repo", "main", - ImmutablePagination.builder().prefix("non-existing/new/").build()); + Pagination.builder().prefix("non-existing/new/").build()); mockStatObjectNotFound("repo", "main", "non-existing"); mockStatObjectNotFound("repo", "main", "non-existing/"); @@ -476,7 +476,7 @@ public void testRename_existingDirToExistingFileName() throws IOException { mockStatObjectNotFound("repo", "main", "existing-dir"); mockStatObjectNotFound("repo", "main", "existing-dir/"); mockListing("repo", "main", - ImmutablePagination.builder().prefix("existing-dir/").build(), + Pagination.builder().prefix("existing-dir/").build(), srcStats); Path dst = new Path("lakefs://repo/main/existingdst.file"); @@ -499,7 +499,7 @@ public void testRename_existingFileToExistingDirName() throws IOException { mockFileDoesNotExist("repo", "main", "existing-dir2"); mockFileDoesNotExist("repo", "main", "existing-dir2/"); mockListing("repo", "main", - ImmutablePagination.builder().prefix("existing-dir2/").build(), + Pagination.builder().prefix("existing-dir2/").build(), dstStats); Path dst = new Path("lakefs://repo/main/existing-dir2/"); @@ -520,7 +520,7 @@ public void testRename_existingFileToExistingDirName() throws IOException { // Mock listing in createDirectoryMarkerIfNotExists to return empty path mockListing("repo", "main", - ImmutablePagination.builder().prefix("existing-dir1/").build()); + Pagination.builder().prefix("existing-dir1/").build()); // Need a directory marker at the source because it's now empty! mockUploadObject("repo", "main", "existing-dir1/"); @@ -542,9 +542,9 @@ public void testRename_existingDirToNonExistingDirWithoutParent() throws IOExcep mockFileDoesNotExist("repo", "main", "x/non-existing-dir/new"); // Will also check if parent of destination is a directory (it isn't). mockListing("repo", "main", - ImmutablePagination.builder().prefix("x/non-existing-dir/").build()); + Pagination.builder().prefix("x/non-existing-dir/").build()); mockListing("repo", "main", - ImmutablePagination.builder().prefix("x/non-existing-dir/new/").build()); + Pagination.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. @@ -568,7 +568,7 @@ public void testRename_existingDirToNonExistingDirWithParent() throws ApiExcepti mockStatObjectNotFound("repo", "main", "existing-dir"); mockStatObjectNotFound("repo", "main", "existing-dir/"); - mockListing("repo", "main", ImmutablePagination.builder().prefix("existing-dir/").build(), + mockListing("repo", "main", Pagination.builder().prefix("existing-dir/").build(), srcStats); mockStatObjectNotFound("repo", "main", "existing-dir2"); @@ -576,7 +576,7 @@ public void testRename_existingDirToNonExistingDirWithParent() throws ApiExcepti mockStatObjectNotFound("repo", "main", "existing-dir2/new"); mockStatObjectNotFound("repo", "main", "existing-dir2/new/"); - mockListing("repo", "main", ImmutablePagination.builder().prefix("existing-dir2/new/").build()); + mockListing("repo", "main", Pagination.builder().prefix("existing-dir2/new/").build()); ObjectStats dstStats = makeObjectStats("existing-dir2/new/existing.src"); @@ -612,7 +612,7 @@ public void testRename_existingDirToExistingNonEmptyDirName() throws IOException mockStatObjectNotFound("repo", "main", "existing-dir1"); mockStatObjectNotFound("repo", "main", "existing-dir1/"); - mockListing("repo", "main", ImmutablePagination.builder().prefix("existing-dir1/").build(), + mockListing("repo", "main", Pagination.builder().prefix("existing-dir1/").build(), firstSrcFileStats, secSrcFileStats); Path fileInDstDir = new Path("lakefs://repo/main/existing-dir2/file.dst"); @@ -620,7 +620,7 @@ public void testRename_existingDirToExistingNonEmptyDirName() throws IOException Path dstDir = new Path("lakefs://repo/main/existing-dir2"); mockStatObjectNotFound("repo", "main", "existing-dir2"); mockStatObjectNotFound("repo", "main", "existing-dir2/"); - mockListing("repo", "main", ImmutablePagination.builder().prefix("existing-dir2/").build(), + mockListing("repo", "main", Pagination.builder().prefix("existing-dir2/").build(), fileInDstDirStats); boolean renamed = fs.rename(srcDir, dstDir); @@ -655,7 +655,7 @@ public void testRename_nonExistingSrcFile() throws IOException { Path src = new Path("lakefs://repo/main/non-existing.src"); mockStatObjectNotFound("repo", "main", "non-existing.src"); mockStatObjectNotFound("repo", "main", "non-existing.src/"); - mockListing("repo", "main", ImmutablePagination.builder().prefix("non-existing.src/").build()); + mockListing("repo", "main", Pagination.builder().prefix("non-existing.src/").build()); Path dst = new Path("lakefs://repo/main/existing.dst"); mockStatObject("repo", "main", "existing.dst", makeObjectStats("existing.dst")); diff --git a/clients/hadoopfs/src/test/java/io/lakefs/Pagination.java b/clients/hadoopfs/src/test/java/io/lakefs/Pagination.java new file mode 100644 index 00000000000..e585fa0d718 --- /dev/null +++ b/clients/hadoopfs/src/test/java/io/lakefs/Pagination.java @@ -0,0 +1,54 @@ +package io.lakefs; + +public final class Pagination { + private final Integer amount; + private final String after; + private final String prefix; + + private Pagination(Builder builder) { + this.amount = builder.amount; + this.after = builder.after; + this.prefix = builder.prefix; + } + + public Integer amount() { + return this.amount; + } + + public String after() { + return this.after; + } + + public String prefix() { + return this.prefix; + } + + public static Builder builder() { + return new Builder(); + } + + public static class Builder { + private Integer amount; + private String after; + private String prefix; + + public Builder amount(Integer amount) { + this.amount = amount; + return this; + } + + public Builder after(String after) { + this.after = after; + return this; + } + + public Builder prefix(String prefix) { + this.prefix = after; + return this; + } + + public Pagination build() { + return new Pagination(this); + } + } +} From f113246238aa7f7428767ec4d46aa57c0059df01 Mon Sep 17 00:00:00 2001 From: Barak Amar Date: Sat, 1 Feb 2025 18:49:34 +0200 Subject: [PATCH 5/6] remove dup dependency --- clients/hadoopfs/pom.xml | 6 ------ 1 file changed, 6 deletions(-) diff --git a/clients/hadoopfs/pom.xml b/clients/hadoopfs/pom.xml index 1bfa78f276b..3b4a7e5b215 100644 --- a/clients/hadoopfs/pom.xml +++ b/clients/hadoopfs/pom.xml @@ -313,12 +313,6 @@ To export to S3: ${hadoop.version} provided - - - org.apache.commons - commons-lang3 - 3.12.0 - javax.xml.bind jaxb-api From fcaab56381e896d63faee4e8c371e18517beb614 Mon Sep 17 00:00:00 2001 From: Barak Amar Date: Sun, 2 Feb 2025 12:03:38 +0200 Subject: [PATCH 6/6] fix prefix value in pagination --- clients/hadoopfs/src/test/java/io/lakefs/Pagination.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clients/hadoopfs/src/test/java/io/lakefs/Pagination.java b/clients/hadoopfs/src/test/java/io/lakefs/Pagination.java index e585fa0d718..523b019dda6 100644 --- a/clients/hadoopfs/src/test/java/io/lakefs/Pagination.java +++ b/clients/hadoopfs/src/test/java/io/lakefs/Pagination.java @@ -43,7 +43,7 @@ public Builder after(String after) { } public Builder prefix(String prefix) { - this.prefix = after; + this.prefix = prefix; return this; }