From af48abae9e9782b06419b9fea861cfb5914830de Mon Sep 17 00:00:00 2001 From: Christopher Lambert <1204398+XN137@users.noreply.github.com> Date: Tue, 15 Jul 2025 18:28:46 +0200 Subject: [PATCH 1/2] Add FileIOFactory.wrapExisting helper --- .../task/BatchFileCleanupTaskHandlerTest.java | 19 +--------- .../ManifestFileCleanupTaskHandlerTest.java | 19 +--------- .../task/TableCleanupTaskHandlerTest.java | 36 ++----------------- .../service/catalog/io/FileIOFactory.java | 10 ++++++ 4 files changed, 15 insertions(+), 69 deletions(-) diff --git a/runtime/service/src/test/java/org/apache/polaris/service/quarkus/task/BatchFileCleanupTaskHandlerTest.java b/runtime/service/src/test/java/org/apache/polaris/service/quarkus/task/BatchFileCleanupTaskHandlerTest.java index eaaf649b62..1f7e59f28b 100644 --- a/runtime/service/src/test/java/org/apache/polaris/service/quarkus/task/BatchFileCleanupTaskHandlerTest.java +++ b/runtime/service/src/test/java/org/apache/polaris/service/quarkus/task/BatchFileCleanupTaskHandlerTest.java @@ -23,13 +23,11 @@ import static org.assertj.core.api.Assertions.assertThatPredicate; import io.quarkus.test.junit.QuarkusTest; -import jakarta.annotation.Nonnull; import jakarta.inject.Inject; import java.io.IOException; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Set; import java.util.UUID; import java.util.concurrent.CompletableFuture; import java.util.concurrent.Executors; @@ -51,8 +49,6 @@ import org.apache.polaris.core.entity.AsyncTaskType; import org.apache.polaris.core.entity.TaskEntity; import org.apache.polaris.core.persistence.MetaStoreManagerFactory; -import org.apache.polaris.core.persistence.PolarisResolvedPathWrapper; -import org.apache.polaris.core.storage.PolarisStorageActions; import org.apache.polaris.service.catalog.io.FileIOFactory; import org.apache.polaris.service.task.BatchFileCleanupTaskHandler; import org.apache.polaris.service.task.TaskFileIOSupplier; @@ -65,20 +61,7 @@ public class BatchFileCleanupTaskHandlerTest { private final RealmContext realmContext = () -> "realmName"; private TaskFileIOSupplier buildTaskFileIOSupplier(FileIO fileIO) { - return new TaskFileIOSupplier( - new FileIOFactory() { - @Override - public FileIO loadFileIO( - @Nonnull CallContext callContext, - @Nonnull String ioImplClassName, - @Nonnull Map properties, - @Nonnull TableIdentifier identifier, - @Nonnull Set tableLocations, - @Nonnull Set storageActions, - @Nonnull PolarisResolvedPathWrapper resolvedEntityPath) { - return fileIO; - } - }); + return new TaskFileIOSupplier(FileIOFactory.wrapExisting(fileIO)); } @Test diff --git a/runtime/service/src/test/java/org/apache/polaris/service/quarkus/task/ManifestFileCleanupTaskHandlerTest.java b/runtime/service/src/test/java/org/apache/polaris/service/quarkus/task/ManifestFileCleanupTaskHandlerTest.java index 8c370dc9f1..98d13a5086 100644 --- a/runtime/service/src/test/java/org/apache/polaris/service/quarkus/task/ManifestFileCleanupTaskHandlerTest.java +++ b/runtime/service/src/test/java/org/apache/polaris/service/quarkus/task/ManifestFileCleanupTaskHandlerTest.java @@ -24,12 +24,10 @@ import static org.assertj.core.api.Assertions.assertThatPredicate; import io.quarkus.test.junit.QuarkusTest; -import jakarta.annotation.Nonnull; import jakarta.inject.Inject; import java.io.IOException; import java.util.HashMap; import java.util.Map; -import java.util.Set; import java.util.UUID; import java.util.concurrent.Executors; import java.util.concurrent.atomic.AtomicInteger; @@ -49,8 +47,6 @@ import org.apache.polaris.core.entity.AsyncTaskType; import org.apache.polaris.core.entity.TaskEntity; import org.apache.polaris.core.persistence.MetaStoreManagerFactory; -import org.apache.polaris.core.persistence.PolarisResolvedPathWrapper; -import org.apache.polaris.core.storage.PolarisStorageActions; import org.apache.polaris.service.catalog.io.FileIOFactory; import org.apache.polaris.service.task.ManifestFileCleanupTaskHandler; import org.apache.polaris.service.task.TaskFileIOSupplier; @@ -64,20 +60,7 @@ class ManifestFileCleanupTaskHandlerTest { private final RealmContext realmContext = () -> "realmName"; private TaskFileIOSupplier buildTaskFileIOSupplier(FileIO fileIO) { - return new TaskFileIOSupplier( - new FileIOFactory() { - @Override - public FileIO loadFileIO( - @Nonnull CallContext callContext, - @Nonnull String ioImplClassName, - @Nonnull Map properties, - @Nonnull TableIdentifier identifier, - @Nonnull Set tableLocations, - @Nonnull Set storageActions, - @Nonnull PolarisResolvedPathWrapper resolvedEntityPath) { - return fileIO; - } - }); + return new TaskFileIOSupplier(FileIOFactory.wrapExisting(fileIO)); } @Test diff --git a/runtime/service/src/test/java/org/apache/polaris/service/quarkus/task/TableCleanupTaskHandlerTest.java b/runtime/service/src/test/java/org/apache/polaris/service/quarkus/task/TableCleanupTaskHandlerTest.java index 9b03feee5b..5a6b76e3d5 100644 --- a/runtime/service/src/test/java/org/apache/polaris/service/quarkus/task/TableCleanupTaskHandlerTest.java +++ b/runtime/service/src/test/java/org/apache/polaris/service/quarkus/task/TableCleanupTaskHandlerTest.java @@ -23,13 +23,10 @@ import io.quarkus.test.junit.QuarkusMock; import io.quarkus.test.junit.QuarkusTest; -import jakarta.annotation.Nonnull; import jakarta.inject.Inject; import java.io.IOException; import java.time.Clock; import java.util.List; -import java.util.Map; -import java.util.Set; import java.util.UUID; import org.apache.commons.codec.binary.Base64; import org.apache.iceberg.ManifestFile; @@ -52,11 +49,8 @@ import org.apache.polaris.core.entity.PolarisEntityType; import org.apache.polaris.core.entity.TaskEntity; import org.apache.polaris.core.entity.table.IcebergTableLikeEntity; -import org.apache.polaris.core.persistence.BasePersistence; import org.apache.polaris.core.persistence.MetaStoreManagerFactory; -import org.apache.polaris.core.persistence.PolarisResolvedPathWrapper; import org.apache.polaris.core.persistence.pagination.PageToken; -import org.apache.polaris.core.storage.PolarisStorageActions; import org.apache.polaris.service.catalog.io.FileIOFactory; import org.apache.polaris.service.task.BatchFileCleanupTaskHandler; import org.apache.polaris.service.task.ManifestFileCleanupTaskHandler; @@ -80,20 +74,7 @@ class TableCleanupTaskHandlerTest { private final RealmContext realmContext = () -> "realmName"; private TaskFileIOSupplier buildTaskFileIOSupplier(FileIO fileIO) { - return new TaskFileIOSupplier( - new FileIOFactory() { - @Override - public FileIO loadFileIO( - @Nonnull CallContext callContext, - @Nonnull String ioImplClassName, - @Nonnull Map properties, - @Nonnull TableIdentifier identifier, - @Nonnull Set tableLocations, - @Nonnull Set storageActions, - @Nonnull PolarisResolvedPathWrapper resolvedEntityPath) { - return fileIO; - } - }); + return new TaskFileIOSupplier(FileIOFactory.wrapExisting(fileIO)); } @BeforeEach @@ -170,8 +151,7 @@ public void testTableCleanup() throws IOException { assertThat(taskEntity) .returns(PolarisEntityType.TASK.getCode(), PolarisBaseEntity::getTypeCode) .extracting(TaskEntity::of) - .returns( - AsyncTaskType.BATCH_FILE_CLEANUP, taskEntity2 -> taskEntity2.getTaskType()) + .returns(AsyncTaskType.BATCH_FILE_CLEANUP, TaskEntity::getTaskType) .returns( new BatchFileCleanupTaskHandler.BatchFileCleanupTask( tableIdentifier, @@ -183,8 +163,6 @@ public void testTableCleanup() throws IOException { @Test public void testTableCleanupHandlesAlreadyDeletedMetadata() throws IOException { - BasePersistence metaStoreSession = - metaStoreManagerFactory.getOrCreateSessionSupplier(realmContext).get(); FileIO fileIO = new InMemoryFileIO() { @Override @@ -237,8 +215,6 @@ public void close() { @Test public void testTableCleanupDuplicatesTasksIfFileStillExists() throws IOException { - BasePersistence metaStoreSession = - metaStoreManagerFactory.getOrCreateSessionSupplier(realmContext).get(); FileIO fileIO = new InMemoryFileIO() { @Override @@ -333,9 +309,7 @@ public void close() { assertThat(taskEntity) .returns(PolarisEntityType.TASK.getCode(), PolarisBaseEntity::getTypeCode) .extracting(TaskEntity::of) - .returns( - AsyncTaskType.MANIFEST_FILE_CLEANUP, - taskEntity4 -> taskEntity4.getTaskType()) + .returns(AsyncTaskType.MANIFEST_FILE_CLEANUP, TaskEntity::getTaskType) .returns( new ManifestFileCleanupTaskHandler.ManifestCleanupTask( tableIdentifier, @@ -347,8 +321,6 @@ public void close() { @Test public void testTableCleanupMultipleSnapshots() throws IOException { - BasePersistence metaStoreSession = - metaStoreManagerFactory.getOrCreateSessionSupplier(realmContext).get(); FileIO fileIO = new InMemoryFileIO(); TableIdentifier tableIdentifier = TableIdentifier.of(Namespace.of("db1", "schema1"), "table1"); TableCleanupTaskHandler handler = @@ -492,8 +464,6 @@ public void testTableCleanupMultipleSnapshots() throws IOException { @Test public void testTableCleanupMultipleMetadata() throws IOException { - BasePersistence metaStoreSession = - metaStoreManagerFactory.getOrCreateSessionSupplier(realmContext).get(); FileIO fileIO = new InMemoryFileIO(); TableIdentifier tableIdentifier = TableIdentifier.of(Namespace.of("db1", "schema1"), "table1"); TableCleanupTaskHandler handler = diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/io/FileIOFactory.java b/service/common/src/main/java/org/apache/polaris/service/catalog/io/FileIOFactory.java index f3e0d6b98b..50cb813684 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/io/FileIOFactory.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/io/FileIOFactory.java @@ -58,4 +58,14 @@ FileIO loadFileIO( @Nonnull Set tableLocations, @Nonnull Set storageActions, @Nonnull PolarisResolvedPathWrapper resolvedEntityPath); + + static FileIOFactory wrapExisting(FileIO fileIO) { + return (callContext, + ioImplClassName, + properties, + identifier, + tableLocations, + storageActions, + resolvedEntityPath) -> fileIO; + } } From 9d96a5e6573c7c324b6dd50cdd4af0d7927d6368 Mon Sep 17 00:00:00 2001 From: Christopher Lambert <1204398+XN137@users.noreply.github.com> Date: Wed, 16 Jul 2025 19:00:15 +0200 Subject: [PATCH 2/2] review: move to TestFileIOFactory --- .../task/BatchFileCleanupTaskHandlerTest.java | 4 +- .../ManifestFileCleanupTaskHandlerTest.java | 4 +- .../task/TableCleanupTaskHandlerTest.java | 4 +- .../service/catalog/io/FileIOFactory.java | 10 ---- .../polaris/service/TestFileIOFactory.java | 52 +++++++++++++++++++ 5 files changed, 58 insertions(+), 16 deletions(-) create mode 100644 service/common/src/testFixtures/java/org/apache/polaris/service/TestFileIOFactory.java diff --git a/runtime/service/src/test/java/org/apache/polaris/service/quarkus/task/BatchFileCleanupTaskHandlerTest.java b/runtime/service/src/test/java/org/apache/polaris/service/quarkus/task/BatchFileCleanupTaskHandlerTest.java index 1f7e59f28b..37162b5df6 100644 --- a/runtime/service/src/test/java/org/apache/polaris/service/quarkus/task/BatchFileCleanupTaskHandlerTest.java +++ b/runtime/service/src/test/java/org/apache/polaris/service/quarkus/task/BatchFileCleanupTaskHandlerTest.java @@ -49,7 +49,7 @@ import org.apache.polaris.core.entity.AsyncTaskType; import org.apache.polaris.core.entity.TaskEntity; import org.apache.polaris.core.persistence.MetaStoreManagerFactory; -import org.apache.polaris.service.catalog.io.FileIOFactory; +import org.apache.polaris.service.TestFileIOFactory; import org.apache.polaris.service.task.BatchFileCleanupTaskHandler; import org.apache.polaris.service.task.TaskFileIOSupplier; import org.apache.polaris.service.task.TaskUtils; @@ -61,7 +61,7 @@ public class BatchFileCleanupTaskHandlerTest { private final RealmContext realmContext = () -> "realmName"; private TaskFileIOSupplier buildTaskFileIOSupplier(FileIO fileIO) { - return new TaskFileIOSupplier(FileIOFactory.wrapExisting(fileIO)); + return new TaskFileIOSupplier(new TestFileIOFactory(fileIO)); } @Test diff --git a/runtime/service/src/test/java/org/apache/polaris/service/quarkus/task/ManifestFileCleanupTaskHandlerTest.java b/runtime/service/src/test/java/org/apache/polaris/service/quarkus/task/ManifestFileCleanupTaskHandlerTest.java index 98d13a5086..381dfa8a81 100644 --- a/runtime/service/src/test/java/org/apache/polaris/service/quarkus/task/ManifestFileCleanupTaskHandlerTest.java +++ b/runtime/service/src/test/java/org/apache/polaris/service/quarkus/task/ManifestFileCleanupTaskHandlerTest.java @@ -47,7 +47,7 @@ import org.apache.polaris.core.entity.AsyncTaskType; import org.apache.polaris.core.entity.TaskEntity; import org.apache.polaris.core.persistence.MetaStoreManagerFactory; -import org.apache.polaris.service.catalog.io.FileIOFactory; +import org.apache.polaris.service.TestFileIOFactory; import org.apache.polaris.service.task.ManifestFileCleanupTaskHandler; import org.apache.polaris.service.task.TaskFileIOSupplier; import org.apache.polaris.service.task.TaskUtils; @@ -60,7 +60,7 @@ class ManifestFileCleanupTaskHandlerTest { private final RealmContext realmContext = () -> "realmName"; private TaskFileIOSupplier buildTaskFileIOSupplier(FileIO fileIO) { - return new TaskFileIOSupplier(FileIOFactory.wrapExisting(fileIO)); + return new TaskFileIOSupplier(new TestFileIOFactory(fileIO)); } @Test diff --git a/runtime/service/src/test/java/org/apache/polaris/service/quarkus/task/TableCleanupTaskHandlerTest.java b/runtime/service/src/test/java/org/apache/polaris/service/quarkus/task/TableCleanupTaskHandlerTest.java index 5a6b76e3d5..69304956b0 100644 --- a/runtime/service/src/test/java/org/apache/polaris/service/quarkus/task/TableCleanupTaskHandlerTest.java +++ b/runtime/service/src/test/java/org/apache/polaris/service/quarkus/task/TableCleanupTaskHandlerTest.java @@ -51,7 +51,7 @@ import org.apache.polaris.core.entity.table.IcebergTableLikeEntity; import org.apache.polaris.core.persistence.MetaStoreManagerFactory; import org.apache.polaris.core.persistence.pagination.PageToken; -import org.apache.polaris.service.catalog.io.FileIOFactory; +import org.apache.polaris.service.TestFileIOFactory; import org.apache.polaris.service.task.BatchFileCleanupTaskHandler; import org.apache.polaris.service.task.ManifestFileCleanupTaskHandler; import org.apache.polaris.service.task.TableCleanupTaskHandler; @@ -74,7 +74,7 @@ class TableCleanupTaskHandlerTest { private final RealmContext realmContext = () -> "realmName"; private TaskFileIOSupplier buildTaskFileIOSupplier(FileIO fileIO) { - return new TaskFileIOSupplier(FileIOFactory.wrapExisting(fileIO)); + return new TaskFileIOSupplier(new TestFileIOFactory(fileIO)); } @BeforeEach diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/io/FileIOFactory.java b/service/common/src/main/java/org/apache/polaris/service/catalog/io/FileIOFactory.java index 50cb813684..f3e0d6b98b 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/io/FileIOFactory.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/io/FileIOFactory.java @@ -58,14 +58,4 @@ FileIO loadFileIO( @Nonnull Set tableLocations, @Nonnull Set storageActions, @Nonnull PolarisResolvedPathWrapper resolvedEntityPath); - - static FileIOFactory wrapExisting(FileIO fileIO) { - return (callContext, - ioImplClassName, - properties, - identifier, - tableLocations, - storageActions, - resolvedEntityPath) -> fileIO; - } } diff --git a/service/common/src/testFixtures/java/org/apache/polaris/service/TestFileIOFactory.java b/service/common/src/testFixtures/java/org/apache/polaris/service/TestFileIOFactory.java new file mode 100644 index 0000000000..67d15a94ca --- /dev/null +++ b/service/common/src/testFixtures/java/org/apache/polaris/service/TestFileIOFactory.java @@ -0,0 +1,52 @@ +/* + * 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.polaris.service; + +import jakarta.annotation.Nonnull; +import java.util.Map; +import java.util.Set; +import org.apache.iceberg.catalog.TableIdentifier; +import org.apache.iceberg.io.FileIO; +import org.apache.polaris.core.context.CallContext; +import org.apache.polaris.core.persistence.PolarisResolvedPathWrapper; +import org.apache.polaris.core.storage.PolarisStorageActions; +import org.apache.polaris.service.catalog.io.FileIOFactory; + +/** A FileIOFactory that always returns the same FileIO instance. */ +public class TestFileIOFactory implements FileIOFactory { + + private final FileIO fileIO; + + public TestFileIOFactory(@Nonnull FileIO fileIO) { + this.fileIO = fileIO; + } + + @Override + public FileIO loadFileIO( + @Nonnull CallContext callContext, + @Nonnull String ioImplClassName, + @Nonnull Map properties, + @Nonnull TableIdentifier identifier, + @Nonnull Set tableLocations, + @Nonnull Set storageActions, + @Nonnull PolarisResolvedPathWrapper resolvedEntityPath) { + return fileIO; + } +}