Skip to content

Commit 7075d13

Browse files
XN137XN137
committed
Add PolarisAdminService.loadEntities helper
`PolarisAdminService` has multiple spots where it is working around the sub-optimal `PolarisMetaStoreManager` APIs. This results in multiple fixes like: PR-1949 PR-2258 While eventually the underlying APIs should be improved, for now we can make a single central workaround and clean up some redundant code. Also we can improve the return types as callers are not interested in details of the entity layer.
1 parent d6ae2c8 commit 7075d13

File tree

3 files changed

+71
-98
lines changed

3 files changed

+71
-98
lines changed

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

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,6 @@
4242
import org.apache.polaris.core.auth.PolarisAuthorizerImpl;
4343
import org.apache.polaris.core.context.RealmContext;
4444
import org.apache.polaris.core.entity.PolarisBaseEntity;
45-
import org.apache.polaris.core.entity.PolarisEntity;
4645
import org.apache.polaris.core.entity.PolarisEntityConstants;
4746
import org.apache.polaris.core.entity.PolarisEntitySubType;
4847
import org.apache.polaris.core.entity.PolarisEntityType;
@@ -311,8 +310,8 @@ public void testCatalogNotReturnedWhenDeletedAfterListBeforeGet() {
311310
.when(metaStoreManager)
312311
.loadEntity(Mockito.any(), Mockito.anyLong(), Mockito.anyLong(), Mockito.any());
313312

314-
List<PolarisEntity> catalogs = polarisAdminService.listCatalogs();
313+
List<Catalog> catalogs = polarisAdminService.listCatalogs();
315314
assertThat(catalogs.size()).isEqualTo(1);
316-
assertThat(catalogs.getFirst().getId()).isEqualTo(catalog2.getCatalog().getId());
315+
assertThat(catalogs.getFirst().getName()).isEqualTo(catalog2.getCatalog().getName());
317316
}
318317
}

‎service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java‎

Lines changed: 65 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
import java.util.Optional;
3737
import java.util.Set;
3838
import java.util.function.Function;
39+
import java.util.stream.Stream;
3940
import org.apache.commons.lang3.StringUtils;
4041
import org.apache.iceberg.catalog.Namespace;
4142
import org.apache.iceberg.catalog.TableIdentifier;
@@ -51,6 +52,7 @@
5152
import org.apache.polaris.core.admin.model.Catalog;
5253
import org.apache.polaris.core.admin.model.CatalogGrant;
5354
import org.apache.polaris.core.admin.model.CatalogPrivilege;
55+
import org.apache.polaris.core.admin.model.CatalogRole;
5456
import org.apache.polaris.core.admin.model.ConnectionConfigInfo;
5557
import org.apache.polaris.core.admin.model.CreateCatalogRequest;
5658
import org.apache.polaris.core.admin.model.ExternalCatalog;
@@ -60,6 +62,8 @@
6062
import org.apache.polaris.core.admin.model.OAuthClientCredentialsParameters;
6163
import org.apache.polaris.core.admin.model.PolicyGrant;
6264
import org.apache.polaris.core.admin.model.PolicyPrivilege;
65+
import org.apache.polaris.core.admin.model.Principal;
66+
import org.apache.polaris.core.admin.model.PrincipalRole;
6367
import org.apache.polaris.core.admin.model.PrincipalWithCredentials;
6468
import org.apache.polaris.core.admin.model.PrincipalWithCredentialsCredentials;
6569
import org.apache.polaris.core.admin.model.TableGrant;
@@ -82,6 +86,7 @@
8286
import org.apache.polaris.core.entity.NamespaceEntity;
8387
import org.apache.polaris.core.entity.PolarisBaseEntity;
8488
import org.apache.polaris.core.entity.PolarisEntity;
89+
import org.apache.polaris.core.entity.PolarisEntityCore;
8590
import org.apache.polaris.core.entity.PolarisEntitySubType;
8691
import org.apache.polaris.core.entity.PolarisEntityType;
8792
import org.apache.polaris.core.entity.PolarisGrantRecord;
@@ -200,6 +205,43 @@ private Optional<CatalogRoleEntity> findCatalogRoleByName(String catalogName, St
200205
.map(path -> CatalogRoleEntity.of(path.getRawLeafEntity()));
201206
}
202207

208+
private <T> Stream<T> loadEntities(
209+
@Nonnull PolarisEntityType entityType,
210+
@Nonnull PolarisEntitySubType entitySubType,
211+
@Nullable PolarisEntity catalogEntity,
212+
@Nonnull Function<PolarisBaseEntity, T> transformer) {
213+
List<PolarisEntityCore> catalogPath;
214+
long catalogId;
215+
if (catalogEntity == null) {
216+
catalogPath = null;
217+
catalogId = 0;
218+
} else {
219+
catalogPath = PolarisEntity.toCoreList(List.of(catalogEntity));
220+
catalogId = catalogEntity.getId();
221+
}
222+
// TODO: add loadEntities method to PolarisMetaStoreManager
223+
// loadEntity may return null due to multiple non-atomic API calls to the persistence layer.
224+
// Specifically, this can happen when a PolarisEntity is returned by listEntities, but cannot be
225+
// loaded afterward because it was purged by another process before it could be loaded.
226+
return metaStoreManager
227+
.listEntities(
228+
getCurrentPolarisContext(),
229+
catalogPath,
230+
entityType,
231+
entitySubType,
232+
PageToken.readEverything())
233+
.getEntities()
234+
.stream()
235+
.map(
236+
nameAndId ->
237+
metaStoreManager.loadEntity(
238+
getCurrentPolarisContext(), catalogId, nameAndId.getId(), nameAndId.getType()))
239+
.map(PolarisEntity::of)
240+
.filter(Objects::nonNull)
241+
.map(transformer)
242+
.filter(Objects::nonNull);
243+
}
244+
203245
private void authorizeBasicRootOperationOrThrow(PolarisAuthorizableOperation op) {
204246
resolutionManifest =
205247
resolutionManifestFactory.createResolutionManifest(
@@ -618,8 +660,7 @@ private boolean catalogOverlapsWithExistingCatalog(CatalogEntity catalogEntity)
618660
}
619661

620662
Set<String> newCatalogLocations = getCatalogLocations(catalogEntity);
621-
return listCatalogsUnsafe().stream()
622-
.map(CatalogEntity::new)
663+
return listCatalogsUnsafe()
623664
.anyMatch(
624665
existingCatalog -> {
625666
if (existingCatalog.getName().equals(catalogEntity.getName())) {
@@ -944,40 +985,16 @@ private void validateUpdateCatalogDiffOrThrow(
944985
return returnedEntity;
945986
}
946987

947-
/**
948-
* List all catalogs after checking for permission. Nulls due to non-atomic list-then-get are
949-
* filtered out.
950-
*/
951-
public List<PolarisEntity> listCatalogs() {
988+
/** List all catalogs after checking for permission. */
989+
public List<Catalog> listCatalogs() {
952990
authorizeBasicRootOperationOrThrow(PolarisAuthorizableOperation.LIST_CATALOGS);
953-
return listCatalogsUnsafe();
991+
return listCatalogsUnsafe().map(CatalogEntity::asCatalog).toList();
954992
}
955993

956-
/**
957-
* List all catalogs without checking for permission. Nulls due to non-atomic list-then-get are
958-
* filtered out.
959-
*/
960-
private List<PolarisEntity> listCatalogsUnsafe() {
961-
// loadEntity may return null due to multiple non-atomic
962-
// API calls to the persistence layer. Specifically, this can happen when a PolarisEntity is
963-
// returned by listCatalogs, but cannot be loaded afterward because it was purged by another
964-
// process before it could be loaded.
965-
return metaStoreManager
966-
.listEntities(
967-
getCurrentPolarisContext(),
968-
null,
969-
PolarisEntityType.CATALOG,
970-
PolarisEntitySubType.ANY_SUBTYPE,
971-
PageToken.readEverything())
972-
.getEntities()
973-
.stream()
974-
.map(
975-
nameAndId ->
976-
PolarisEntity.of(
977-
metaStoreManager.loadEntity(
978-
getCurrentPolarisContext(), 0, nameAndId.getId(), nameAndId.getType())))
979-
.filter(Objects::nonNull)
980-
.toList();
994+
/** List all catalogs without checking for permission. */
995+
private Stream<CatalogEntity> listCatalogsUnsafe() {
996+
return loadEntities(
997+
PolarisEntityType.CATALOG, PolarisEntitySubType.ANY_SUBTYPE, null, CatalogEntity::of);
981998
}
982999

9831000
public PrincipalWithCredentials createPrincipal(PolarisEntity entity) {
@@ -1141,24 +1158,16 @@ public void deletePrincipal(String name) {
11411158
return rotateOrResetCredentialsHelper(principalName, true);
11421159
}
11431160

1144-
public List<PolarisEntity> listPrincipals() {
1161+
public List<Principal> listPrincipals() {
11451162
PolarisAuthorizableOperation op = PolarisAuthorizableOperation.LIST_PRINCIPALS;
11461163
authorizeBasicRootOperationOrThrow(op);
11471164

1148-
return metaStoreManager
1149-
.listEntities(
1150-
getCurrentPolarisContext(),
1151-
null,
1165+
return loadEntities(
11521166
PolarisEntityType.PRINCIPAL,
11531167
PolarisEntitySubType.NULL_SUBTYPE,
1154-
PageToken.readEverything())
1155-
.getEntities()
1156-
.stream()
1157-
.map(
1158-
nameAndId ->
1159-
PolarisEntity.of(
1160-
metaStoreManager.loadEntity(
1161-
getCurrentPolarisContext(), 0, nameAndId.getId(), nameAndId.getType())))
1168+
null,
1169+
PrincipalEntity::of)
1170+
.map(PrincipalEntity::asPrincipal)
11621171
.toList();
11631172
}
11641173

@@ -1254,24 +1263,16 @@ public void deletePrincipalRole(String name) {
12541263
return returnedEntity;
12551264
}
12561265

1257-
public List<PolarisEntity> listPrincipalRoles() {
1266+
public List<PrincipalRole> listPrincipalRoles() {
12581267
PolarisAuthorizableOperation op = PolarisAuthorizableOperation.LIST_PRINCIPAL_ROLES;
12591268
authorizeBasicRootOperationOrThrow(op);
12601269

1261-
return metaStoreManager
1262-
.listEntities(
1263-
getCurrentPolarisContext(),
1264-
null,
1270+
return loadEntities(
12651271
PolarisEntityType.PRINCIPAL_ROLE,
12661272
PolarisEntitySubType.NULL_SUBTYPE,
1267-
PageToken.readEverything())
1268-
.getEntities()
1269-
.stream()
1270-
.map(
1271-
nameAndId ->
1272-
PolarisEntity.of(
1273-
metaStoreManager.loadEntity(
1274-
getCurrentPolarisContext(), 0, nameAndId.getId(), nameAndId.getType())))
1273+
null,
1274+
PrincipalRoleEntity::of)
1275+
.map(PrincipalRoleEntity::asPrincipalRole)
12751276
.toList();
12761277
}
12771278

@@ -1383,30 +1384,19 @@ public void deleteCatalogRole(String catalogName, String name) {
13831384
return returnedEntity;
13841385
}
13851386

1386-
public List<PolarisEntity> listCatalogRoles(String catalogName) {
1387+
public List<CatalogRole> listCatalogRoles(String catalogName) {
13871388
PolarisAuthorizableOperation op = PolarisAuthorizableOperation.LIST_CATALOG_ROLES;
13881389
authorizeBasicTopLevelEntityOperationOrThrow(op, catalogName, PolarisEntityType.CATALOG);
13891390

13901391
PolarisEntity catalogEntity =
13911392
findCatalogByName(catalogName)
13921393
.orElseThrow(() -> new NotFoundException("Parent catalog %s not found", catalogName));
1393-
return metaStoreManager
1394-
.listEntities(
1395-
getCurrentPolarisContext(),
1396-
PolarisEntity.toCoreList(List.of(catalogEntity)),
1394+
return loadEntities(
13971395
PolarisEntityType.CATALOG_ROLE,
13981396
PolarisEntitySubType.NULL_SUBTYPE,
1399-
PageToken.readEverything())
1400-
.getEntities()
1401-
.stream()
1402-
.map(
1403-
nameAndId ->
1404-
PolarisEntity.of(
1405-
metaStoreManager.loadEntity(
1406-
getCurrentPolarisContext(),
1407-
catalogEntity.getId(),
1408-
nameAndId.getId(),
1409-
nameAndId.getType())))
1397+
catalogEntity,
1398+
CatalogRoleEntity::of)
1399+
.map(CatalogRoleEntity::asCatalogRole)
14101400
.toList();
14111401
}
14121402

‎service/common/src/main/java/org/apache/polaris/service/admin/PolarisServiceImpl.java‎

Lines changed: 4 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -239,11 +239,7 @@ public Response updateCatalog(
239239
@Override
240240
public Response listCatalogs(RealmContext realmContext, SecurityContext securityContext) {
241241
PolarisAdminService adminService = newAdminService(realmContext, securityContext);
242-
List<Catalog> catalogList =
243-
adminService.listCatalogs().stream()
244-
.map(CatalogEntity::new)
245-
.map(CatalogEntity::asCatalog)
246-
.toList();
242+
List<Catalog> catalogList = adminService.listCatalogs();
247243
Catalogs catalogs = new Catalogs(catalogList);
248244
LOGGER.debug("listCatalogs returning: {}", catalogs);
249245
return Response.ok(catalogs).build();
@@ -311,11 +307,7 @@ public Response rotateCredentials(
311307
@Override
312308
public Response listPrincipals(RealmContext realmContext, SecurityContext securityContext) {
313309
PolarisAdminService adminService = newAdminService(realmContext, securityContext);
314-
List<Principal> principalList =
315-
adminService.listPrincipals().stream()
316-
.map(PrincipalEntity::new)
317-
.map(PrincipalEntity::asPrincipal)
318-
.toList();
310+
List<Principal> principalList = adminService.listPrincipals();
319311
Principals principals = new Principals(principalList);
320312
LOGGER.debug("listPrincipals returning: {}", principals);
321313
return Response.ok(principals).build();
@@ -375,11 +367,7 @@ public Response updatePrincipalRole(
375367
@Override
376368
public Response listPrincipalRoles(RealmContext realmContext, SecurityContext securityContext) {
377369
PolarisAdminService adminService = newAdminService(realmContext, securityContext);
378-
List<PrincipalRole> principalRoleList =
379-
adminService.listPrincipalRoles().stream()
380-
.map(PrincipalRoleEntity::new)
381-
.map(PrincipalRoleEntity::asPrincipalRole)
382-
.toList();
370+
List<PrincipalRole> principalRoleList = adminService.listPrincipalRoles();
383371
PrincipalRoles principalRoles = new PrincipalRoles(principalRoleList);
384372
LOGGER.debug("listPrincipalRoles returning: {}", principalRoles);
385373
return Response.ok(principalRoles).build();
@@ -451,11 +439,7 @@ public Response updateCatalogRole(
451439
public Response listCatalogRoles(
452440
String catalogName, RealmContext realmContext, SecurityContext securityContext) {
453441
PolarisAdminService adminService = newAdminService(realmContext, securityContext);
454-
List<CatalogRole> catalogRoleList =
455-
adminService.listCatalogRoles(catalogName).stream()
456-
.map(CatalogRoleEntity::new)
457-
.map(CatalogRoleEntity::asCatalogRole)
458-
.toList();
442+
List<CatalogRole> catalogRoleList = adminService.listCatalogRoles(catalogName);
459443
CatalogRoles catalogRoles = new CatalogRoles(catalogRoleList);
460444
LOGGER.debug("listCatalogRoles returning: {}", catalogRoles);
461445
return Response.ok(catalogRoles).build();

0 commit comments

Comments
 (0)