Skip to content

Commit d69d7ef

Browse files
XN137XN137
authored
Reduce getOrCreateMetaStoreManager callers (#2532)
we can inject `PolarisMetaStoreManager` directly into request-scoped beans or build it only once in tests that operate in a single realm.
1 parent be4175c commit d69d7ef

File tree

8 files changed

+20
-42
lines changed

8 files changed

+20
-42
lines changed

‎runtime/service/src/main/java/org/apache/polaris/service/auth/DefaultActiveRolesProvider.java‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@
3535
import org.apache.polaris.core.entity.PolarisEntity;
3636
import org.apache.polaris.core.entity.PolarisEntityType;
3737
import org.apache.polaris.core.entity.PrincipalRoleEntity;
38-
import org.apache.polaris.core.persistence.MetaStoreManagerFactory;
3938
import org.apache.polaris.core.persistence.PolarisMetaStoreManager;
4039
import org.apache.polaris.core.persistence.dao.entity.EntityResult;
4140
import org.apache.polaris.core.persistence.dao.entity.LoadGrantsResult;
@@ -55,7 +54,7 @@ public class DefaultActiveRolesProvider implements ActiveRolesProvider {
5554

5655
@Inject PolarisDiagnostics diagnostics;
5756
@Inject CallContext callContext;
58-
@Inject MetaStoreManagerFactory metaStoreManagerFactory;
57+
@Inject PolarisMetaStoreManager metaStoreManager;
5958

6059
@Override
6160
public Set<String> getActiveRoles(PolarisPrincipal principal) {
@@ -68,9 +67,7 @@ public Set<String> getActiveRoles(PolarisPrincipal principal) {
6867
}
6968
List<PrincipalRoleEntity> activeRoles =
7069
loadActivePrincipalRoles(
71-
principal.getRoles(),
72-
persistedPolarisPrincipal.getEntity(),
73-
metaStoreManagerFactory.getOrCreateMetaStoreManager(callContext.getRealmContext()));
70+
principal.getRoles(), persistedPolarisPrincipal.getEntity(), metaStoreManager);
7471
return activeRoles.stream().map(PrincipalRoleEntity::getName).collect(Collectors.toSet());
7572
}
7673

‎runtime/service/src/main/java/org/apache/polaris/service/auth/DefaultAuthenticator.java‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@
3131
import org.apache.polaris.core.entity.PolarisEntity;
3232
import org.apache.polaris.core.entity.PolarisEntityType;
3333
import org.apache.polaris.core.entity.PrincipalEntity;
34-
import org.apache.polaris.core.persistence.MetaStoreManagerFactory;
3534
import org.apache.polaris.core.persistence.PolarisMetaStoreManager;
3635
import org.slf4j.Logger;
3736
import org.slf4j.LoggerFactory;
@@ -50,14 +49,12 @@ public class DefaultAuthenticator implements Authenticator {
5049

5150
private static final Logger LOGGER = LoggerFactory.getLogger(DefaultAuthenticator.class);
5251

53-
@Inject MetaStoreManagerFactory metaStoreManagerFactory;
5452
@Inject CallContext callContext;
53+
@Inject PolarisMetaStoreManager metaStoreManager;
5554

5655
@Override
5756
public PolarisPrincipal authenticate(PolarisCredential credentials) {
5857
LOGGER.debug("Resolving principal for credentials={}", credentials);
59-
PolarisMetaStoreManager metaStoreManager =
60-
metaStoreManagerFactory.getOrCreateMetaStoreManager(callContext.getRealmContext());
6158
PolarisEntity principal = null;
6259
try {
6360
// If the principal id is present, prefer to use it to load the principal entity,

‎runtime/service/src/test/java/org/apache/polaris/service/admin/ManagementServiceTest.java‎

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -41,14 +41,12 @@
4141
import org.apache.polaris.core.admin.model.UpdateCatalogRequest;
4242
import org.apache.polaris.core.auth.PolarisAuthorizerImpl;
4343
import org.apache.polaris.core.auth.PolarisPrincipal;
44-
import org.apache.polaris.core.context.RealmContext;
4544
import org.apache.polaris.core.entity.PolarisBaseEntity;
4645
import org.apache.polaris.core.entity.PolarisEntityConstants;
4746
import org.apache.polaris.core.entity.PolarisEntitySubType;
4847
import org.apache.polaris.core.entity.PolarisEntityType;
4948
import org.apache.polaris.core.entity.PrincipalEntity;
5049
import org.apache.polaris.core.entity.PrincipalRoleEntity;
51-
import org.apache.polaris.core.persistence.MetaStoreManagerFactory;
5250
import org.apache.polaris.core.persistence.PolarisMetaStoreManager;
5351
import org.apache.polaris.core.persistence.dao.entity.BaseResult;
5452
import org.apache.polaris.core.persistence.dao.entity.CreateCatalogResult;
@@ -228,12 +226,6 @@ public void testUpdateCatalogWithDisallowedStorageConfig() {
228226
.hasMessage("Explicitly setting S3 endpoints is not allowed.");
229227
}
230228

231-
private PolarisMetaStoreManager setupMetaStoreManager() {
232-
MetaStoreManagerFactory metaStoreManagerFactory = services.metaStoreManagerFactory();
233-
RealmContext realmContext = services.realmContext();
234-
return metaStoreManagerFactory.getOrCreateMetaStoreManager(realmContext);
235-
}
236-
237229
private PolarisAdminService setupPolarisAdminService(
238230
PolarisMetaStoreManager metaStoreManager, PolarisCallContext callContext) {
239231
return new PolarisAdminService(
@@ -297,7 +289,7 @@ private PrincipalRoleEntity createRole(
297289

298290
@Test
299291
public void testCannotAssignFederatedEntities() {
300-
PolarisMetaStoreManager metaStoreManager = setupMetaStoreManager();
292+
PolarisMetaStoreManager metaStoreManager = services.metaStoreManager();
301293
PolarisCallContext callContext = services.newCallContext();
302294
PolarisAdminService polarisAdminService =
303295
setupPolarisAdminService(metaStoreManager, callContext);
@@ -316,7 +308,7 @@ public void testCannotAssignFederatedEntities() {
316308

317309
@Test
318310
public void testCanListCatalogs() {
319-
PolarisMetaStoreManager metaStoreManager = setupMetaStoreManager();
311+
PolarisMetaStoreManager metaStoreManager = services.metaStoreManager();
320312
PolarisCallContext callContext = services.newCallContext();
321313
PolarisAdminService polarisAdminService =
322314
setupPolarisAdminService(metaStoreManager, callContext);
@@ -356,7 +348,7 @@ public void testCanListCatalogs() {
356348

357349
@Test
358350
public void testCreateCatalogReturnErrorOnFailure() {
359-
PolarisMetaStoreManager metaStoreManager = Mockito.spy(setupMetaStoreManager());
351+
PolarisMetaStoreManager metaStoreManager = Mockito.spy(services.metaStoreManager());
360352
PolarisCallContext callContext = services.newCallContext();
361353
PolarisAdminService polarisAdminService =
362354
setupPolarisAdminService(metaStoreManager, callContext);

‎runtime/service/src/test/java/org/apache/polaris/service/auth/DefaultAuthenticatorTest.java‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525
import org.apache.polaris.core.PolarisCallContext;
2626
import org.apache.polaris.core.context.RealmContext;
2727
import org.apache.polaris.core.entity.PolarisEntityType;
28-
import org.apache.polaris.core.persistence.MetaStoreManagerFactory;
2928
import org.apache.polaris.core.persistence.PolarisMetaStoreManager;
3029
import org.apache.polaris.core.persistence.dao.entity.BaseResult;
3130
import org.apache.polaris.core.persistence.dao.entity.EntityResult;
@@ -46,11 +45,8 @@ public void setUp() {
4645
polarisCallContext = Mockito.mock(PolarisCallContext.class);
4746
when(polarisCallContext.getRealmContext()).thenReturn(realmContext);
4847
metaStoreManager = Mockito.mock(PolarisMetaStoreManager.class);
49-
MetaStoreManagerFactory metaStoreManagerFactory = Mockito.mock(MetaStoreManagerFactory.class);
50-
when(metaStoreManagerFactory.getOrCreateMetaStoreManager(realmContext))
51-
.thenReturn(metaStoreManager);
5248
authenticator = new DefaultAuthenticator();
53-
authenticator.metaStoreManagerFactory = metaStoreManagerFactory;
49+
authenticator.metaStoreManager = metaStoreManager;
5450
authenticator.callContext = polarisCallContext;
5551
}
5652

‎runtime/service/src/test/java/org/apache/polaris/service/catalog/io/FileIOFactoryTest.java‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -169,8 +169,7 @@ public void testLoadFileIOForCleanupTask(String scheme) {
169169

170170
List<PolarisBaseEntity> tasks =
171171
testServices
172-
.metaStoreManagerFactory()
173-
.getOrCreateMetaStoreManager(realmContext)
172+
.metaStoreManager()
174173
.loadTasks(callContext.getPolarisCallContext(), "testExecutor", PageToken.fromLimit(1))
175174
.getEntities();
176175
Assertions.assertThat(tasks).hasSize(1);
@@ -228,7 +227,7 @@ IcebergCatalog createCatalog(TestServices services, String scheme) {
228227
services.polarisDiagnostics(),
229228
services.storageCredentialCache(),
230229
services.resolverFactory(),
231-
services.metaStoreManagerFactory().getOrCreateMetaStoreManager(realmContext),
230+
services.metaStoreManager(),
232231
callContext,
233232
passthroughView,
234233
services.securityContext(),

‎runtime/service/src/test/java/org/apache/polaris/service/task/TableCleanupTaskHandlerTest.java‎

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@
4848
import org.apache.polaris.core.entity.TaskEntity;
4949
import org.apache.polaris.core.entity.table.IcebergTableLikeEntity;
5050
import org.apache.polaris.core.persistence.MetaStoreManagerFactory;
51+
import org.apache.polaris.core.persistence.PolarisMetaStoreManager;
5152
import org.apache.polaris.core.persistence.pagination.PageToken;
5253
import org.apache.polaris.service.TestFileIOFactory;
5354
import org.assertj.core.api.Assertions;
@@ -62,6 +63,7 @@ class TableCleanupTaskHandlerTest {
6263
@Inject MetaStoreManagerFactory metaStoreManagerFactory;
6364
@Inject PolarisConfigurationStore configurationStore;
6465

66+
private PolarisMetaStoreManager metaStoreManager;
6567
private CallContext callContext;
6668

6769
private final RealmContext realmContext = () -> "realmName";
@@ -76,6 +78,7 @@ private TableCleanupTaskHandler newTableCleanupTaskHandler(FileIO fileIO) {
7678
void setup() {
7779
QuarkusMock.installMockForType(realmContext, RealmContext.class);
7880

81+
metaStoreManager = metaStoreManagerFactory.getOrCreateMetaStoreManager(realmContext);
7982
callContext =
8083
new PolarisCallContext(
8184
realmContext,
@@ -121,8 +124,7 @@ public void testTableCleanup() throws IOException {
121124
handler.handleTask(task, callContext);
122125

123126
assertThat(
124-
metaStoreManagerFactory
125-
.getOrCreateMetaStoreManager(realmContext)
127+
metaStoreManager
126128
.loadTasks(callContext.getPolarisCallContext(), "test", PageToken.fromLimit(2))
127129
.getEntities())
128130
.hasSize(2)
@@ -196,8 +198,7 @@ public void close() {
196198

197199
// both tasks successfully executed, but only one should queue subtasks
198200
assertThat(
199-
metaStoreManagerFactory
200-
.getOrCreateMetaStoreManager(realmContext)
201+
metaStoreManager
201202
.loadTasks(callContext.getPolarisCallContext(), "test", PageToken.fromLimit(5))
202203
.getEntities())
203204
.hasSize(2);
@@ -254,8 +255,7 @@ public void close() {
254255

255256
// both tasks successfully executed, but only one should queue subtasks
256257
assertThat(
257-
metaStoreManagerFactory
258-
.getOrCreateMetaStoreManager(realmContext)
258+
metaStoreManager
259259
.loadTasks(callContext.getPolarisCallContext(), "test", PageToken.fromLimit(5))
260260
.getEntities())
261261
.hasSize(4)
@@ -367,8 +367,7 @@ public void testTableCleanupMultipleSnapshots() throws IOException {
367367
handler.handleTask(task, callContext);
368368

369369
List<PolarisBaseEntity> entities =
370-
metaStoreManagerFactory
371-
.getOrCreateMetaStoreManager(realmContext)
370+
metaStoreManager
372371
.loadTasks(callContext.getPolarisCallContext(), "test", PageToken.fromLimit(5))
373372
.getEntities();
374373

@@ -535,8 +534,7 @@ public void testTableCleanupMultipleMetadata() throws IOException {
535534
handler.handleTask(task, callContext);
536535

537536
List<PolarisBaseEntity> entities =
538-
metaStoreManagerFactory
539-
.getOrCreateMetaStoreManager(callContext.getRealmContext())
537+
metaStoreManager
540538
.loadTasks(callContext.getPolarisCallContext(), "test", PageToken.fromLimit(6))
541539
.getEntities();
542540

‎runtime/service/src/test/java/org/apache/polaris/service/task/TaskExecutorImplTest.java‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
import org.apache.polaris.core.context.CallContext;
2323
import org.apache.polaris.core.context.RealmContext;
2424
import org.apache.polaris.core.entity.TaskEntity;
25-
import org.apache.polaris.core.persistence.MetaStoreManagerFactory;
2625
import org.apache.polaris.core.persistence.PolarisMetaStoreManager;
2726
import org.apache.polaris.service.TestServices;
2827
import org.apache.polaris.service.events.AfterTaskAttemptedEvent;
@@ -43,9 +42,7 @@ void testEventsAreEmitted() {
4342
TestPolarisEventListener testPolarisEventListener =
4443
(TestPolarisEventListener) testServices.polarisEventListener();
4544

46-
MetaStoreManagerFactory metaStoreManagerFactory = testServices.metaStoreManagerFactory();
47-
PolarisMetaStoreManager metaStoreManager =
48-
metaStoreManagerFactory.getOrCreateMetaStoreManager(realmContext);
45+
PolarisMetaStoreManager metaStoreManager = testServices.metaStoreManager();
4946

5047
PolarisCallContext polarisCallCtx = testServices.newCallContext();
5148

‎runtime/service/src/testFixtures/java/org/apache/polaris/service/TestServices.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@ public record TestServices(
9393
RealmContext realmContext,
9494
RealmConfig realmConfig,
9595
SecurityContext securityContext,
96+
PolarisMetaStoreManager metaStoreManager,
9697
FileIOFactory fileIOFactory,
9798
TaskExecutor taskExecutor,
9899
PolarisEventListener polarisEventListener) {
@@ -309,6 +310,7 @@ public String getAuthenticationScheme() {
309310
realmContext,
310311
realmConfig,
311312
securityContext,
313+
metaStoreManager,
312314
fileIOFactory,
313315
taskExecutor,
314316
polarisEventListener);

0 commit comments

Comments
 (0)