Skip to content

Commit 179da6d

Browse files
dimas-bdimas-b
authored
Simplify PolarisGrantManager (#1171)
* Simplify PolarisGrantManager Previously strongly typed methods redirected to typeless lookup methods, while implementations had only the typeless variant. This change reverts the redirects from strongly typed methods to existing typeless methods in implementations. As a result it is possible to simplify the interface by removing typeless lookup methods. Existing call sites all have strongly typed parameters available and use the typed lookup methods now. For context: This refactoring seems valuable by itself, but it is also needed for the upcoming NoSQL implementations for reasons similar to #1112
1 parent ff6440a commit 179da6d

File tree

7 files changed

+46
-84
lines changed

7 files changed

+46
-84
lines changed

‎polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisGrantManager.java‎

Lines changed: 2 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
import jakarta.annotation.Nullable;
2323
import java.util.List;
2424
import org.apache.polaris.core.PolarisCallContext;
25-
import org.apache.polaris.core.entity.PolarisBaseEntity;
2625
import org.apache.polaris.core.entity.PolarisEntityCore;
2726
import org.apache.polaris.core.entity.PolarisPrivilege;
2827
import org.apache.polaris.core.persistence.dao.entity.LoadGrantsResult;
@@ -119,23 +118,8 @@ PrivilegeResult revokePrivilegeOnSecurableFromRole(
119118
* ENTITY_NOT_FOUND if the securable cannot be found
120119
*/
121120
@Nonnull
122-
default LoadGrantsResult loadGrantsOnSecurable(
123-
@Nonnull PolarisCallContext callCtx, PolarisBaseEntity securable) {
124-
return loadGrantsOnSecurable(callCtx, securable.getCatalogId(), securable.getId());
125-
}
126-
127-
/**
128-
* This method should be used by the Polaris app to cache all grant records on a securable.
129-
*
130-
* @param callCtx call context
131-
* @param securableCatalogId id of the catalog this securable belongs to
132-
* @param securableId id of the securable
133-
* @return the list of grants and the version of the grant records. We will return
134-
* ENTITY_NOT_FOUND if the securable cannot be found
135-
*/
136-
@Nonnull
137121
LoadGrantsResult loadGrantsOnSecurable(
138-
@Nonnull PolarisCallContext callCtx, long securableCatalogId, long securableId);
122+
@Nonnull PolarisCallContext callCtx, PolarisEntityCore securable);
139123

140124
/**
141125
* This method should be used by the Polaris app to load all grants made to a grantee, either a
@@ -147,22 +131,6 @@ LoadGrantsResult loadGrantsOnSecurable(
147131
* grantee does not exist
148132
*/
149133
@Nonnull
150-
default LoadGrantsResult loadGrantsToGrantee(
151-
@Nonnull PolarisCallContext callCtx, PolarisBaseEntity grantee) {
152-
return loadGrantsToGrantee(callCtx, grantee.getCatalogId(), grantee.getId());
153-
}
154-
155-
/**
156-
* This method should be used by the Polaris app to load all grants made to a grantee, either a
157-
* role or a principal.
158-
*
159-
* @param callCtx call context
160-
* @param granteeCatalogId id of the catalog this grantee belongs to
161-
* @param granteeId id of the grantee
162-
* @return the list of grants and the version of the grant records. We will return NULL if the
163-
* grantee does not exist
164-
*/
165-
@Nonnull
166134
LoadGrantsResult loadGrantsToGrantee(
167-
PolarisCallContext callCtx, long granteeCatalogId, long granteeId);
135+
@Nonnull PolarisCallContext callCtx, PolarisEntityCore grantee);
168136
}

‎polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1330,6 +1330,11 @@ private void revokeGrantRecord(
13301330

13311331
/** {@inheritDoc} */
13321332
@Override
1333+
public @Nonnull LoadGrantsResult loadGrantsOnSecurable(
1334+
@Nonnull PolarisCallContext callCtx, PolarisEntityCore securable) {
1335+
return loadGrantsOnSecurable(callCtx, securable.getCatalogId(), securable.getId());
1336+
}
1337+
13331338
public @Nonnull LoadGrantsResult loadGrantsOnSecurable(
13341339
@Nonnull PolarisCallContext callCtx, long securableCatalogId, long securableId) {
13351340
// get metastore we should be using
@@ -1371,6 +1376,11 @@ private void revokeGrantRecord(
13711376

13721377
/** {@inheritDoc} */
13731378
@Override
1379+
public @Nonnull LoadGrantsResult loadGrantsToGrantee(
1380+
@Nonnull PolarisCallContext callCtx, PolarisEntityCore grantee) {
1381+
return loadGrantsToGrantee(callCtx, grantee.getCatalogId(), grantee.getId());
1382+
}
1383+
13741384
public @Nonnull LoadGrantsResult loadGrantsToGrantee(
13751385
@Nonnull PolarisCallContext callCtx, long granteeCatalogId, long granteeId) {
13761386
// get metastore we should be using

‎polaris-core/src/main/java/org/apache/polaris/core/persistence/TransactionWorkspaceMetaStoreManager.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -280,17 +280,17 @@ public PrivilegeResult revokePrivilegeOnSecurableFromRole(
280280
}
281281

282282
@Override
283-
public LoadGrantsResult loadGrantsOnSecurable(
284-
@Nonnull PolarisCallContext callCtx, long securableCatalogId, long securableId) {
283+
public @Nonnull LoadGrantsResult loadGrantsOnSecurable(
284+
@Nonnull PolarisCallContext callCtx, PolarisEntityCore securable) {
285285
callCtx
286286
.getDiagServices()
287287
.fail("illegal_method_in_transaction_workspace", "loadGrantsOnSecurable");
288288
return null;
289289
}
290290

291291
@Override
292-
public LoadGrantsResult loadGrantsToGrantee(
293-
PolarisCallContext callCtx, long granteeCatalogId, long granteeId) {
292+
public @Nonnull LoadGrantsResult loadGrantsToGrantee(
293+
@Nonnull PolarisCallContext callCtx, PolarisEntityCore grantee) {
294294
callCtx
295295
.getDiagServices()
296296
.fail("illegal_method_in_transaction_workspace", "loadGrantsToGrantee");

‎polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/PolarisMetaStoreManagerImpl.java‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1757,6 +1757,11 @@ private PolarisEntityResolver resolveSecurableToRoleGrant(
17571757

17581758
/** {@inheritDoc} */
17591759
@Override
1760+
public @Nonnull LoadGrantsResult loadGrantsOnSecurable(
1761+
@Nonnull PolarisCallContext callCtx, PolarisEntityCore securable) {
1762+
return loadGrantsOnSecurable(callCtx, securable.getCatalogId(), securable.getId());
1763+
}
1764+
17601765
public @Nonnull LoadGrantsResult loadGrantsOnSecurable(
17611766
@Nonnull PolarisCallContext callCtx, long securableCatalogId, long securableId) {
17621767
// get metastore we should be using
@@ -1807,6 +1812,11 @@ private PolarisEntityResolver resolveSecurableToRoleGrant(
18071812

18081813
/** {@inheritDoc} */
18091814
@Override
1815+
public @Nonnull LoadGrantsResult loadGrantsToGrantee(
1816+
@Nonnull PolarisCallContext callCtx, PolarisEntityCore grantee) {
1817+
return loadGrantsToGrantee(callCtx, grantee.getCatalogId(), grantee.getId());
1818+
}
1819+
18101820
public @Nonnull LoadGrantsResult loadGrantsToGrantee(
18111821
@Nonnull PolarisCallContext callCtx, long granteeCatalogId, long granteeId) {
18121822
// get metastore we should be using

‎polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/PolarisTestMetaStoreManager.java‎

Lines changed: 14 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -268,8 +268,7 @@ void ensureGrantRecordExists(
268268

269269
// load all grant records on that securable, should not fail
270270
LoadGrantsResult loadGrantsOnSecurable =
271-
polarisMetaStoreManager.loadGrantsOnSecurable(
272-
this.polarisCallContext, securable.getCatalogId(), securable.getId());
271+
polarisMetaStoreManager.loadGrantsOnSecurable(this.polarisCallContext, securable);
273272
// ensure entities for these grant records have been properly loaded
274273
this.validateLoadedGrants(loadGrantsOnSecurable, false);
275274

@@ -278,8 +277,7 @@ void ensureGrantRecordExists(
278277

279278
// load all grant records on that grantee, should not fail
280279
LoadGrantsResult loadGrantsOnGrantee =
281-
polarisMetaStoreManager.loadGrantsToGrantee(
282-
this.polarisCallContext, grantee.getCatalogId(), grantee.getId());
280+
polarisMetaStoreManager.loadGrantsToGrantee(this.polarisCallContext, grantee);
283281
// ensure entities for these grant records have been properly loaded
284282
this.validateLoadedGrants(loadGrantsOnGrantee, true);
285283

@@ -355,8 +353,7 @@ void ensureGrantRecordRemoved(
355353

356354
// load all grant records on that securable, should not fail
357355
LoadGrantsResult loadGrantsOnSecurable =
358-
polarisMetaStoreManager.loadGrantsOnSecurable(
359-
this.polarisCallContext, securable.getCatalogId(), securable.getId());
356+
polarisMetaStoreManager.loadGrantsOnSecurable(this.polarisCallContext, securable);
360357
// ensure entities for these grant records have been properly loaded
361358
this.validateLoadedGrants(loadGrantsOnSecurable, false);
362359

@@ -365,8 +362,7 @@ void ensureGrantRecordRemoved(
365362

366363
// load all grant records on that grantee, should not fail
367364
LoadGrantsResult loadGrantsOnGrantee =
368-
polarisMetaStoreManager.loadGrantsToGrantee(
369-
this.polarisCallContext, grantee.getCatalogId(), grantee.getId());
365+
polarisMetaStoreManager.loadGrantsToGrantee(this.polarisCallContext, grantee);
370366
this.validateLoadedGrants(loadGrantsOnGrantee, true);
371367

372368
// check that the grant record has been removed
@@ -734,14 +730,12 @@ void dropEntity(List<PolarisEntityCore> catalogPath, PolarisBaseEntity entityToD
734730
granteeEntities =
735731
new ArrayList<>(
736732
polarisMetaStoreManager
737-
.loadGrantsOnSecurable(
738-
this.polarisCallContext, entity.getCatalogId(), entity.getId())
733+
.loadGrantsOnSecurable(this.polarisCallContext, entity)
739734
.getEntities());
740735
securableEntities =
741736
new ArrayList<>(
742737
polarisMetaStoreManager
743-
.loadGrantsToGrantee(
744-
this.polarisCallContext, entity.getCatalogId(), entity.getId())
738+
.loadGrantsToGrantee(this.polarisCallContext, entity)
745739
.getEntities());
746740
} else {
747741
granteeEntities = List.of();
@@ -831,8 +825,7 @@ void dropEntity(List<PolarisEntityCore> catalogPath, PolarisBaseEntity entityToD
831825
// of the entity it was connected with before being dropped
832826
for (PolarisBaseEntity connectedEntity : granteeEntities) {
833827
LoadGrantsResult grantResult =
834-
polarisMetaStoreManager.loadGrantsToGrantee(
835-
this.polarisCallContext, connectedEntity.getCatalogId(), connectedEntity.getId());
828+
polarisMetaStoreManager.loadGrantsToGrantee(this.polarisCallContext, connectedEntity);
836829
if (grantResult.isSuccess()) {
837830
long cnt =
838831
grantResult.getGrantRecords().stream()
@@ -853,8 +846,7 @@ void dropEntity(List<PolarisEntityCore> catalogPath, PolarisBaseEntity entityToD
853846
}
854847
for (PolarisBaseEntity connectedEntity : securableEntities) {
855848
LoadGrantsResult grantResult =
856-
polarisMetaStoreManager.loadGrantsOnSecurable(
857-
this.polarisCallContext, connectedEntity.getCatalogId(), connectedEntity.getId());
849+
polarisMetaStoreManager.loadGrantsOnSecurable(this.polarisCallContext, connectedEntity);
858850
long cnt =
859851
grantResult.getGrantRecords().stream()
860852
.filter(gr -> gr.getGranteeId() == entityToDrop.getId())
@@ -1294,8 +1286,7 @@ private void validateCacheEntryLoad(ResolvedEntityResult cacheEntry) {
12941286
List<PolarisGrantRecord> refGrantRecords = new ArrayList<>();
12951287
if (refEntity.getType().isGrantee()) {
12961288
LoadGrantsResult loadGrantResult =
1297-
this.polarisMetaStoreManager.loadGrantsToGrantee(
1298-
this.polarisCallContext, refEntity.getCatalogId(), refEntity.getId());
1289+
this.polarisMetaStoreManager.loadGrantsToGrantee(this.polarisCallContext, refEntity);
12991290
this.validateLoadedGrants(loadGrantResult, true);
13001291

13011292
// same version
@@ -1306,8 +1297,7 @@ private void validateCacheEntryLoad(ResolvedEntityResult cacheEntry) {
13061297
}
13071298

13081299
LoadGrantsResult loadGrantResult =
1309-
this.polarisMetaStoreManager.loadGrantsOnSecurable(
1310-
this.polarisCallContext, refEntity.getCatalogId(), refEntity.getId());
1300+
this.polarisMetaStoreManager.loadGrantsOnSecurable(this.polarisCallContext, refEntity);
13111301
this.validateLoadedGrants(loadGrantResult, false);
13121302

13131303
// same version
@@ -1347,10 +1337,9 @@ private void validateCacheEntryRefresh(
13471337
// reload the grants
13481338
LoadGrantsResult loadGrantResult =
13491339
refEntity.getType().isGrantee()
1350-
? this.polarisMetaStoreManager.loadGrantsToGrantee(
1351-
this.polarisCallContext, catalogId, entityId)
1340+
? this.polarisMetaStoreManager.loadGrantsToGrantee(this.polarisCallContext, refEntity)
13521341
: this.polarisMetaStoreManager.loadGrantsOnSecurable(
1353-
this.polarisCallContext, catalogId, entityId);
1342+
this.polarisCallContext, refEntity);
13541343
this.validateLoadedGrants(loadGrantResult, refEntity.getType().isGrantee());
13551344
Assertions.assertThat(cacheEntry.getGrantRecordsVersion())
13561345
.isEqualTo(loadGrantResult.getGrantsVersion());
@@ -2099,16 +2088,15 @@ public void testPrivileges() {
20992088
grantToGrantee(catalog, R1, PR9000, PolarisPrivilege.CATALOG_ROLE_USAGE);
21002089

21012090
LoadGrantsResult loadGrantsResult =
2102-
polarisMetaStoreManager.loadGrantsToGrantee(this.polarisCallContext, 0L, PR9000.getId());
2091+
polarisMetaStoreManager.loadGrantsToGrantee(this.polarisCallContext, PR9000);
21032092
this.validateLoadedGrants(loadGrantsResult, true);
21042093
Assertions.assertThat(loadGrantsResult.getGrantRecords()).hasSize(1);
21052094
Assertions.assertThat(loadGrantsResult.getGrantRecords().get(0).getSecurableCatalogId())
21062095
.isEqualTo(R1.getCatalogId());
21072096
Assertions.assertThat(loadGrantsResult.getGrantRecords().get(0).getSecurableId())
21082097
.isEqualTo(R1.getId());
21092098

2110-
loadGrantsResult =
2111-
polarisMetaStoreManager.loadGrantsToGrantee(this.polarisCallContext, 0L, PR900.getId());
2099+
loadGrantsResult = polarisMetaStoreManager.loadGrantsToGrantee(this.polarisCallContext, PR900);
21122100
Assertions.assertThat(loadGrantsResult).isNotNull();
21132101
Assertions.assertThat(loadGrantsResult.getGrantRecords()).hasSize(0);
21142102
}

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -362,8 +362,7 @@ public void after() {
362362

363363
protected @Nonnull Set<String> loadPrincipalRolesNames(AuthenticatedPolarisPrincipal p) {
364364
return metaStoreManager
365-
.loadGrantsToGrantee(
366-
callContext.getPolarisCallContext(), 0L, p.getPrincipalEntity().getId())
365+
.loadGrantsToGrantee(callContext.getPolarisCallContext(), p.getPrincipalEntity())
367366
.getGrantRecords()
368367
.stream()
369368
.filter(gr -> gr.getPrivilegeCode() == PolarisPrivilege.PRINCIPAL_ROLE_USAGE.getCode())

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

Lines changed: 5 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1217,8 +1217,7 @@ public List<PolarisEntity> listPrincipalRolesAssigned(String principalName) {
12171217
findPrincipalByName(principalName)
12181218
.orElseThrow(() -> new NotFoundException("Principal %s not found", principalName));
12191219
LoadGrantsResult grantList =
1220-
metaStoreManager.loadGrantsToGrantee(
1221-
getCurrentPolarisContext(), principalEntity.getCatalogId(), principalEntity.getId());
1220+
metaStoreManager.loadGrantsToGrantee(getCurrentPolarisContext(), principalEntity);
12221221
return buildEntitiesFromGrantResults(grantList, false, PolarisEntityType.PRINCIPAL_ROLE, null);
12231222
}
12241223

@@ -1281,10 +1280,7 @@ public List<PolarisEntity> listAssigneePrincipalsForPrincipalRole(String princip
12811280
.orElseThrow(
12821281
() -> new NotFoundException("PrincipalRole %s not found", principalRoleName));
12831282
LoadGrantsResult grantList =
1284-
metaStoreManager.loadGrantsOnSecurable(
1285-
getCurrentPolarisContext(),
1286-
principalRoleEntity.getCatalogId(),
1287-
principalRoleEntity.getId());
1283+
metaStoreManager.loadGrantsOnSecurable(getCurrentPolarisContext(), principalRoleEntity);
12881284
return buildEntitiesFromGrantResults(grantList, true, PolarisEntityType.PRINCIPAL, null);
12891285
}
12901286

@@ -1336,10 +1332,7 @@ public List<PolarisEntity> listCatalogRolesForPrincipalRole(
13361332
.orElseThrow(
13371333
() -> new NotFoundException("PrincipalRole %s not found", principalRoleName));
13381334
LoadGrantsResult grantList =
1339-
metaStoreManager.loadGrantsToGrantee(
1340-
getCurrentPolarisContext(),
1341-
principalRoleEntity.getCatalogId(),
1342-
principalRoleEntity.getId());
1335+
metaStoreManager.loadGrantsToGrantee(getCurrentPolarisContext(), principalRoleEntity);
13431336
return buildEntitiesFromGrantResults(
13441337
grantList,
13451338
false,
@@ -1565,10 +1558,7 @@ public List<PolarisEntity> listAssigneePrincipalRolesForCatalogRole(
15651558
findCatalogRoleByName(catalogName, catalogRoleName)
15661559
.orElseThrow(() -> new NotFoundException("CatalogRole %s not found", catalogRoleName));
15671560
LoadGrantsResult grantList =
1568-
metaStoreManager.loadGrantsOnSecurable(
1569-
getCurrentPolarisContext(),
1570-
catalogRoleEntity.getCatalogId(),
1571-
catalogRoleEntity.getId());
1561+
metaStoreManager.loadGrantsOnSecurable(getCurrentPolarisContext(), catalogRoleEntity);
15721562
return buildEntitiesFromGrantResults(grantList, true, PolarisEntityType.PRINCIPAL_ROLE, null);
15731563
}
15741564

@@ -1584,10 +1574,7 @@ public List<GrantResource> listGrantsForCatalogRole(String catalogName, String c
15841574
findCatalogRoleByName(catalogName, catalogRoleName)
15851575
.orElseThrow(() -> new NotFoundException("CatalogRole %s not found", catalogRoleName));
15861576
LoadGrantsResult grantList =
1587-
metaStoreManager.loadGrantsToGrantee(
1588-
getCurrentPolarisContext(),
1589-
catalogRoleEntity.getCatalogId(),
1590-
catalogRoleEntity.getId());
1577+
metaStoreManager.loadGrantsToGrantee(getCurrentPolarisContext(), catalogRoleEntity);
15911578
List<CatalogGrant> catalogGrants = new ArrayList<>();
15921579
List<NamespaceGrant> namespaceGrants = new ArrayList<>();
15931580
List<TableGrant> tableGrants = new ArrayList<>();

0 commit comments

Comments
 (0)