Skip to content

Remove duplicate MetaStoreManagerFactory mocks #2023

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,8 @@
import java.util.Set;
import java.util.UUID;
import java.util.function.Function;
import java.util.function.Supplier;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import org.apache.commons.lang3.NotImplementedException;
import org.apache.iceberg.BaseTable;
import org.apache.iceberg.CatalogProperties;
import org.apache.iceberg.ContentFile;
Expand Down Expand Up @@ -114,13 +112,10 @@
import org.apache.polaris.core.persistence.PolarisEntityManager;
import org.apache.polaris.core.persistence.PolarisMetaStoreManager;
import org.apache.polaris.core.persistence.PolarisResolvedPathWrapper;
import org.apache.polaris.core.persistence.bootstrap.RootCredentialsSet;
import org.apache.polaris.core.persistence.cache.InMemoryEntityCache;
import org.apache.polaris.core.persistence.dao.entity.BaseResult;
import org.apache.polaris.core.persistence.dao.entity.EntityResult;
import org.apache.polaris.core.persistence.dao.entity.PrincipalSecretsResult;
import org.apache.polaris.core.persistence.pagination.PageToken;
import org.apache.polaris.core.persistence.transactional.TransactionalPersistence;
import org.apache.polaris.core.secrets.UserSecretsManager;
import org.apache.polaris.core.secrets.UserSecretsManagerFactory;
import org.apache.polaris.core.storage.PolarisStorageActions;
Expand Down Expand Up @@ -239,7 +234,7 @@ public Map<String, String> getConfigOverrides() {
CatalogProperties.TABLE_OVERRIDE_PREFIX + "override-key4",
"catalog-override-key4");

@Inject MetaStoreManagerFactory managerFactory;
@Inject MetaStoreManagerFactory metaStoreManagerFactory;
@Inject PolarisConfigurationStore configurationStore;
@Inject PolarisStorageIntegrationProvider storageIntegrationProvider;
@Inject UserSecretsManagerFactory userSecretsManagerFactory;
Expand Down Expand Up @@ -284,12 +279,12 @@ public void before(TestInfo testInfo) {
testInfo.getTestMethod().map(Method::getName).orElse("test"), System.nanoTime());
RealmContext realmContext = () -> realmName;
QuarkusMock.installMockForType(realmContext, RealmContext.class);
metaStoreManager = managerFactory.getOrCreateMetaStoreManager(realmContext);
metaStoreManager = metaStoreManagerFactory.getOrCreateMetaStoreManager(realmContext);
userSecretsManager = userSecretsManagerFactory.getOrCreateUserSecretsManager(realmContext);
polarisContext =
new PolarisCallContext(
realmContext,
managerFactory.getOrCreateSessionSupplier(realmContext).get(),
metaStoreManagerFactory.getOrCreateSessionSupplier(realmContext).get(),
diagServices,
configurationStore,
Clock.systemDefaultZone());
Expand Down Expand Up @@ -360,9 +355,10 @@ public void before(TestInfo testInfo) {
.asCatalog()));

RealmEntityManagerFactory realmEntityManagerFactory =
new RealmEntityManagerFactory(createMockMetaStoreManagerFactory());
new RealmEntityManagerFactory(metaStoreManagerFactory);
this.fileIOFactory =
new DefaultFileIOFactory(realmEntityManagerFactory, managerFactory, configurationStore);
new DefaultFileIOFactory(
realmEntityManagerFactory, metaStoreManagerFactory, configurationStore);

StsClient stsClient = Mockito.mock(StsClient.class);
when(stsClient.assumeRole(isA(AssumeRoleRequest.class)))
Expand Down Expand Up @@ -450,42 +446,6 @@ protected boolean supportsNotifications() {
return true;
}

private MetaStoreManagerFactory createMockMetaStoreManagerFactory() {
return new MetaStoreManagerFactory() {
@Override
public PolarisMetaStoreManager getOrCreateMetaStoreManager(RealmContext realmContext) {
return metaStoreManager;
}

@Override
public Supplier<TransactionalPersistence> getOrCreateSessionSupplier(
RealmContext realmContext) {
return () -> ((TransactionalPersistence) polarisContext.getMetaStore());
}

@Override
public StorageCredentialCache getOrCreateStorageCredentialCache(RealmContext realmContext) {
return new StorageCredentialCache(realmContext, configurationStore);
}

@Override
public InMemoryEntityCache getOrCreateEntityCache(RealmContext realmContext) {
return new InMemoryEntityCache(realmContext, configurationStore, metaStoreManager);
}

@Override
public Map<String, PrincipalSecretsResult> bootstrapRealms(
Iterable<String> realms, RootCredentialsSet rootCredentialsSet) {
throw new NotImplementedException("Bootstrapping realms is not supported");
Copy link
Contributor Author

@XN137 XN137 Jul 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

was throwing this exception the reason why some tests where using createMockMetaStoreManagerFactory() instead of the Mock in the field?

or just left-over code without a particular reason?
sorry in case i am missing something obvious

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not see any test relying on this exception, so removing this piece of code LGTM.

}

@Override
public Map<String, BaseResult> purgeRealms(Iterable<String> realms) {
throw new NotImplementedException("Purging realms is not supported");
}
};
}

@Test
public void testEmptyNamespace() {
IcebergCatalog catalog = catalog();
Expand Down Expand Up @@ -1030,8 +990,8 @@ public void testValidateNotificationFailToCreateFileIO() {
FileIOFactory fileIOFactory =
spy(
new DefaultFileIOFactory(
new RealmEntityManagerFactory(createMockMetaStoreManagerFactory()),
managerFactory,
new RealmEntityManagerFactory(metaStoreManagerFactory),
metaStoreManagerFactory,
configurationStore));
IcebergCatalog catalog =
new IcebergCatalog(
Expand Down Expand Up @@ -1922,7 +1882,6 @@ public void testDropTableWithPurge() {
.containsEntry(StorageAccessProperty.AWS_KEY_ID, TEST_ACCESS_KEY)
.containsEntry(StorageAccessProperty.AWS_SECRET_KEY, SECRET_ACCESS_KEY)
.containsEntry(StorageAccessProperty.AWS_TOKEN, SESSION_TOKEN);
MetaStoreManagerFactory metaStoreManagerFactory = createMockMetaStoreManagerFactory();
FileIO fileIO =
new TaskFileIOSupplier(
new DefaultFileIOFactory(
Expand Down Expand Up @@ -2071,8 +2030,8 @@ public void testFileIOWrapper() {

MeasuredFileIOFactory measured =
new MeasuredFileIOFactory(
new RealmEntityManagerFactory(createMockMetaStoreManagerFactory()),
managerFactory,
new RealmEntityManagerFactory(metaStoreManagerFactory),
metaStoreManagerFactory,
configurationStore);
IcebergCatalog catalog =
new IcebergCatalog(
Expand Down Expand Up @@ -2144,8 +2103,7 @@ public FileIO loadFileIO(
});

TableCleanupTaskHandler handler =
new TableCleanupTaskHandler(
Mockito.mock(), createMockMetaStoreManagerFactory(), taskFileIOSupplier);
new TableCleanupTaskHandler(Mockito.mock(), metaStoreManagerFactory, taskFileIOSupplier);
handler.handleTask(taskEntity, polarisContext);
Assertions.assertThat(measured.getNumDeletedFiles()).as("A table was deleted").isGreaterThan(0);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ public Map<String, String> getConfigOverrides() {
CatalogProperties.VIEW_OVERRIDE_PREFIX + "key3", "catalog-override-key3",
CatalogProperties.VIEW_OVERRIDE_PREFIX + "key4", "catalog-override-key4");

@Inject MetaStoreManagerFactory managerFactory;
@Inject MetaStoreManagerFactory metaStoreManagerFactory;
@Inject UserSecretsManagerFactory userSecretsManagerFactory;
@Inject PolarisConfigurationStore configurationStore;
@Inject PolarisDiagnostics diagServices;
Expand Down Expand Up @@ -166,12 +166,12 @@ public void before(TestInfo testInfo) {
RealmContext realmContext = () -> realmName;
QuarkusMock.installMockForType(realmContext, RealmContext.class);

metaStoreManager = managerFactory.getOrCreateMetaStoreManager(realmContext);
metaStoreManager = metaStoreManagerFactory.getOrCreateMetaStoreManager(realmContext);
userSecretsManager = userSecretsManagerFactory.getOrCreateUserSecretsManager(realmContext);
polarisContext =
new PolarisCallContext(
realmContext,
managerFactory.getOrCreateSessionSupplier(realmContext).get(),
metaStoreManagerFactory.getOrCreateSessionSupplier(realmContext).get(),
diagServices,
configurationStore,
Clock.systemDefaultZone());
Expand Down Expand Up @@ -236,7 +236,9 @@ public void before(TestInfo testInfo) {
polarisContext, entityManager, securityContext, CATALOG_NAME);
FileIOFactory fileIOFactory =
new DefaultFileIOFactory(
new RealmEntityManagerFactory(managerFactory), managerFactory, configurationStore);
new RealmEntityManagerFactory(metaStoreManagerFactory),
metaStoreManagerFactory,
configurationStore);

testPolarisEventListener = (TestPolarisEventListener) polarisEventListener;
this.catalog =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,6 @@
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.function.Supplier;
import org.apache.commons.lang3.NotImplementedException;
import org.apache.iceberg.CatalogProperties;
import org.apache.iceberg.Schema;
import org.apache.iceberg.catalog.Namespace;
Expand All @@ -61,11 +59,7 @@
import org.apache.polaris.core.persistence.MetaStoreManagerFactory;
import org.apache.polaris.core.persistence.PolarisEntityManager;
import org.apache.polaris.core.persistence.PolarisMetaStoreManager;
import org.apache.polaris.core.persistence.bootstrap.RootCredentialsSet;
import org.apache.polaris.core.persistence.cache.InMemoryEntityCache;
import org.apache.polaris.core.persistence.dao.entity.BaseResult;
import org.apache.polaris.core.persistence.dao.entity.PrincipalSecretsResult;
import org.apache.polaris.core.persistence.transactional.TransactionalPersistence;
import org.apache.polaris.core.secrets.UserSecretsManager;
import org.apache.polaris.core.secrets.UserSecretsManagerFactory;
import org.apache.polaris.core.storage.PolarisStorageIntegration;
Expand Down Expand Up @@ -123,7 +117,7 @@ public Map<String, String> getConfigOverrides() {
public static final String SECRET_ACCESS_KEY = "secret_access_key";
public static final String SESSION_TOKEN = "session_token";

@Inject MetaStoreManagerFactory managerFactory;
@Inject MetaStoreManagerFactory metaStoreManagerFactory;
@Inject UserSecretsManagerFactory userSecretsManagerFactory;
@Inject PolarisConfigurationStore configurationStore;
@Inject PolarisStorageIntegrationProvider storageIntegrationProvider;
Expand Down Expand Up @@ -165,12 +159,12 @@ public void before(TestInfo testInfo) {
testInfo.getTestMethod().map(Method::getName).orElse("test"), System.nanoTime());
RealmContext realmContext = () -> realmName;
QuarkusMock.installMockForType(realmContext, RealmContext.class);
metaStoreManager = managerFactory.getOrCreateMetaStoreManager(realmContext);
metaStoreManager = metaStoreManagerFactory.getOrCreateMetaStoreManager(realmContext);
userSecretsManager = userSecretsManagerFactory.getOrCreateUserSecretsManager(realmContext);
polarisContext =
new PolarisCallContext(
realmContext,
managerFactory.getOrCreateSessionSupplier(realmContext).get(),
metaStoreManagerFactory.getOrCreateSessionSupplier(realmContext).get(),
diagServices,
configurationStore,
Clock.systemDefaultZone());
Expand Down Expand Up @@ -243,9 +237,10 @@ public void before(TestInfo testInfo) {
polarisContext, entityManager, securityContext, CATALOG_NAME);
TaskExecutor taskExecutor = Mockito.mock();
RealmEntityManagerFactory realmEntityManagerFactory =
new RealmEntityManagerFactory(createMockMetaStoreManagerFactory());
new RealmEntityManagerFactory(metaStoreManagerFactory);
this.fileIOFactory =
new DefaultFileIOFactory(realmEntityManagerFactory, managerFactory, configurationStore);
new DefaultFileIOFactory(
realmEntityManagerFactory, metaStoreManagerFactory, configurationStore);

StsClient stsClient = Mockito.mock(StsClient.class);
when(stsClient.assumeRole(isA(AssumeRoleRequest.class)))
Expand Down Expand Up @@ -288,42 +283,6 @@ public void after() throws IOException {
metaStoreManager.purge(polarisContext);
}

private MetaStoreManagerFactory createMockMetaStoreManagerFactory() {
return new MetaStoreManagerFactory() {
@Override
public PolarisMetaStoreManager getOrCreateMetaStoreManager(RealmContext realmContext) {
return metaStoreManager;
}

@Override
public Supplier<TransactionalPersistence> getOrCreateSessionSupplier(
RealmContext realmContext) {
return () -> ((TransactionalPersistence) polarisContext.getMetaStore());
}

@Override
public StorageCredentialCache getOrCreateStorageCredentialCache(RealmContext realmContext) {
return new StorageCredentialCache(realmContext, configurationStore);
}

@Override
public InMemoryEntityCache getOrCreateEntityCache(RealmContext realmContext) {
return new InMemoryEntityCache(realmContext, configurationStore, metaStoreManager);
}

@Override
public Map<String, PrincipalSecretsResult> bootstrapRealms(
Iterable<String> realms, RootCredentialsSet rootCredentialsSet) {
throw new NotImplementedException("Bootstrapping realms is not supported");
}

@Override
public Map<String, BaseResult> purgeRealms(Iterable<String> realms) {
throw new NotImplementedException("Purging realms is not supported");
}
};
}

@Test
public void testCreateGenericTableDoesNotThrow() {
Namespace namespace = Namespace.of("ns");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,6 @@
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.function.Supplier;
import org.apache.commons.lang3.NotImplementedException;
import org.apache.iceberg.CatalogProperties;
import org.apache.iceberg.Schema;
import org.apache.iceberg.catalog.Namespace;
Expand All @@ -70,11 +68,7 @@
import org.apache.polaris.core.persistence.PolarisEntityManager;
import org.apache.polaris.core.persistence.PolarisMetaStoreManager;
import org.apache.polaris.core.persistence.PolicyMappingAlreadyExistsException;
import org.apache.polaris.core.persistence.bootstrap.RootCredentialsSet;
import org.apache.polaris.core.persistence.cache.InMemoryEntityCache;
import org.apache.polaris.core.persistence.dao.entity.BaseResult;
import org.apache.polaris.core.persistence.dao.entity.PrincipalSecretsResult;
import org.apache.polaris.core.persistence.transactional.TransactionalPersistence;
import org.apache.polaris.core.policy.PredefinedPolicyTypes;
import org.apache.polaris.core.policy.exceptions.NoSuchPolicyException;
import org.apache.polaris.core.policy.exceptions.PolicyInUseException;
Expand Down Expand Up @@ -153,7 +147,7 @@ public Map<String, String> getConfigOverrides() {
new PolicyAttachmentTarget(
PolicyAttachmentTarget.TypeEnum.TABLE_LIKE, List.of(TABLE.toString().split("\\.")));

@Inject MetaStoreManagerFactory managerFactory;
@Inject MetaStoreManagerFactory metaStoreManagerFactory;
@Inject UserSecretsManagerFactory userSecretsManagerFactory;
@Inject PolarisConfigurationStore configurationStore;
@Inject PolarisStorageIntegrationProvider storageIntegrationProvider;
Expand Down Expand Up @@ -191,12 +185,12 @@ public void before(TestInfo testInfo) {
testInfo.getTestMethod().map(Method::getName).orElse("test"), System.nanoTime());
RealmContext realmContext = () -> realmName;
QuarkusMock.installMockForType(realmContext, RealmContext.class);
metaStoreManager = managerFactory.getOrCreateMetaStoreManager(realmContext);
metaStoreManager = metaStoreManagerFactory.getOrCreateMetaStoreManager(realmContext);
userSecretsManager = userSecretsManagerFactory.getOrCreateUserSecretsManager(realmContext);
polarisContext =
new PolarisCallContext(
realmContext,
managerFactory.getOrCreateSessionSupplier(realmContext).get(),
metaStoreManagerFactory.getOrCreateSessionSupplier(realmContext).get(),
diagServices,
configurationStore,
Clock.systemDefaultZone());
Expand Down Expand Up @@ -269,9 +263,10 @@ public void before(TestInfo testInfo) {
callContext, entityManager, securityContext, CATALOG_NAME);
TaskExecutor taskExecutor = Mockito.mock();
RealmEntityManagerFactory realmEntityManagerFactory =
new RealmEntityManagerFactory(createMockMetaStoreManagerFactory());
new RealmEntityManagerFactory(metaStoreManagerFactory);
this.fileIOFactory =
new DefaultFileIOFactory(realmEntityManagerFactory, managerFactory, configurationStore);
new DefaultFileIOFactory(
realmEntityManagerFactory, metaStoreManagerFactory, configurationStore);

StsClient stsClient = Mockito.mock(StsClient.class);
when(stsClient.assumeRole(isA(AssumeRoleRequest.class)))
Expand Down Expand Up @@ -312,42 +307,6 @@ public void after() throws IOException {
metaStoreManager.purge(polarisContext);
}

private MetaStoreManagerFactory createMockMetaStoreManagerFactory() {
return new MetaStoreManagerFactory() {
@Override
public PolarisMetaStoreManager getOrCreateMetaStoreManager(RealmContext realmContext) {
return metaStoreManager;
}

@Override
public Supplier<TransactionalPersistence> getOrCreateSessionSupplier(
RealmContext realmContext) {
return () -> ((TransactionalPersistence) polarisContext.getMetaStore());
}

@Override
public StorageCredentialCache getOrCreateStorageCredentialCache(RealmContext realmContext) {
return new StorageCredentialCache(realmContext, configurationStore);
}

@Override
public InMemoryEntityCache getOrCreateEntityCache(RealmContext realmContext) {
return new InMemoryEntityCache(realmContext, configurationStore, metaStoreManager);
}

@Override
public Map<String, PrincipalSecretsResult> bootstrapRealms(
Iterable<String> realms, RootCredentialsSet rootCredentialsSet) {
throw new NotImplementedException("Bootstrapping realms is not supported");
}

@Override
public Map<String, BaseResult> purgeRealms(Iterable<String> realms) {
throw new NotImplementedException("Purging realms is not supported");
}
};
}

@Test
public void testCreatePolicyDoesNotThrow() {
icebergCatalog.createNamespace(NS);
Expand Down
Loading