diff --git a/runtime/service/src/test/java/org/apache/polaris/service/quarkus/admin/ManagementServiceTest.java b/runtime/service/src/test/java/org/apache/polaris/service/quarkus/admin/ManagementServiceTest.java index 5178cd9fc2..4ef214c0ad 100644 --- a/runtime/service/src/test/java/org/apache/polaris/service/quarkus/admin/ManagementServiceTest.java +++ b/runtime/service/src/test/java/org/apache/polaris/service/quarkus/admin/ManagementServiceTest.java @@ -315,4 +315,81 @@ public void testCatalogNotReturnedWhenDeletedAfterListBeforeGet() { assertThat(catalogs.size()).isEqualTo(1); assertThat(catalogs.getFirst().getId()).isEqualTo(catalog2.getCatalog().getId()); } + + /** + * Simulates the case when a principal is dropped after being found while listing all principals. + */ + @Test + public void testPrincipalNotReturnedWhenDeletedAfterListBeforeGet() { + PolarisMetaStoreManager metaStoreManager = Mockito.spy(setupMetaStoreManager()); + PolarisCallContext callContext = setupCallContext(); + PolarisAdminService polarisAdminService = + setupPolarisAdminService(metaStoreManager, callContext); + + int initialPrincipalCount = polarisAdminService.listPrincipals().size(); + + PrincipalEntity principal1 = createPrincipal(metaStoreManager, callContext, "my-principal-1"); + metaStoreManager.createPrincipal(callContext, principal1); + + PrincipalEntity principal2 = createPrincipal(metaStoreManager, callContext, "my-principal-2"); + metaStoreManager.createPrincipal(callContext, principal2); + + Mockito.doAnswer( + invocation -> { + Object entityId = invocation.getArgument(2); + if (entityId.equals(principal1.getId())) { + return new EntityResult(BaseResult.ReturnStatus.ENTITY_NOT_FOUND, ""); + } + return invocation.callRealMethod(); + }) + .when(metaStoreManager) + .loadEntity(Mockito.any(), Mockito.anyLong(), Mockito.anyLong(), Mockito.any()); + + List principals = polarisAdminService.listPrincipals(); + + assertThat(principals.size()) + .isEqualTo(initialPrincipalCount + 1); // initial + 2 created - 1 filtered + assertThat(principals.stream().anyMatch(p -> p.getId() == principal2.getId())) + .isTrue(); // principal2 survived + assertThat(principals.stream().noneMatch(p -> p.getId() == principal1.getId())) + .isTrue(); // principal1 filtered out + } + + /** + * Simulates the case when a principal role is dropped after being found while listing all + * principal roles. + */ + @Test + public void testPrincipalRoleNotReturnedWhenDeletedAfterListBeforeGet() { + PolarisMetaStoreManager metaStoreManager = Mockito.spy(setupMetaStoreManager()); + PolarisCallContext callContext = setupCallContext(); + PolarisAdminService polarisAdminService = + setupPolarisAdminService(metaStoreManager, callContext); + + int initialRoleCount = polarisAdminService.listPrincipalRoles().size(); + + PrincipalRoleEntity role1 = createRole(metaStoreManager, callContext, "my-role-1", false); + EntityResult createResult1 = metaStoreManager.createEntityIfNotExists(callContext, null, role1); + assertThat(createResult1.isSuccess()).isTrue(); + + PrincipalRoleEntity role2 = createRole(metaStoreManager, callContext, "my-role-2", false); + EntityResult createResult2 = metaStoreManager.createEntityIfNotExists(callContext, null, role2); + assertThat(createResult2.isSuccess()).isTrue(); + + Mockito.doAnswer( + invocation -> { + Object entityId = invocation.getArgument(2); + if (entityId.equals(role1.getId())) { + return new EntityResult(BaseResult.ReturnStatus.ENTITY_NOT_FOUND, ""); + } + return invocation.callRealMethod(); + }) + .when(metaStoreManager) + .loadEntity(Mockito.any(), Mockito.anyLong(), Mockito.anyLong(), Mockito.any()); + + List principalRoles = polarisAdminService.listPrincipalRoles(); + assertThat(principalRoles.size()).isEqualTo(initialRoleCount + 1); + assertThat(principalRoles.stream().anyMatch(r -> r.getId() == role2.getId())).isTrue(); + assertThat(principalRoles.stream().noneMatch(r -> r.getId() == role1.getId())).isTrue(); + } } diff --git a/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java b/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java index 816fc67986..ce2f9fafac 100644 --- a/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java +++ b/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java @@ -1141,10 +1141,18 @@ public void deletePrincipal(String name) { return rotateOrResetCredentialsHelper(principalName, true); } + /** + * List all principals after checking for permission. Nulls due to non-atomic list-then-get are + * filtered out. + */ public List listPrincipals() { PolarisAuthorizableOperation op = PolarisAuthorizableOperation.LIST_PRINCIPALS; authorizeBasicRootOperationOrThrow(op); + // loadEntity may return null due to multiple non-atomic + // API calls to the persistence layer. Specifically, this can happen when a PolarisEntity is + // returned by listEntities, but cannot be loaded afterward because it was purged by another + // process before it could be loaded. return metaStoreManager .listEntities( getCurrentPolarisContext(), @@ -1159,6 +1167,7 @@ public List listPrincipals() { PolarisEntity.of( metaStoreManager.loadEntity( getCurrentPolarisContext(), 0, nameAndId.getId(), nameAndId.getType()))) + .filter(Objects::nonNull) .toList(); } @@ -1254,10 +1263,18 @@ public void deletePrincipalRole(String name) { return returnedEntity; } + /** + * List all principal roles after checking for permission. Nulls due to non-atomic list-then-get + * are filtered out. + */ public List listPrincipalRoles() { PolarisAuthorizableOperation op = PolarisAuthorizableOperation.LIST_PRINCIPAL_ROLES; authorizeBasicRootOperationOrThrow(op); + // loadEntity may return null due to multiple non-atomic + // API calls to the persistence layer. Specifically, this can happen when a PolarisEntity is + // returned by listEntities, but cannot be loaded afterward because it was purged by another + // process before it could be loaded. return metaStoreManager .listEntities( getCurrentPolarisContext(), @@ -1272,6 +1289,7 @@ public List listPrincipalRoles() { PolarisEntity.of( metaStoreManager.loadEntity( getCurrentPolarisContext(), 0, nameAndId.getId(), nameAndId.getType()))) + .filter(Objects::nonNull) .toList(); }