adutra commented on code in PR #2925:
URL: https://github.com/apache/polaris/pull/2925#discussion_r2470291502
##########
polaris-core/src/main/java/org/apache/polaris/core/persistence/resolver/Resolver.java:
##########
@@ -791,22 +788,22 @@ private List<ResolvedPolarisEntity>
resolveAllPrincipalRoles(
PolarisEntityType.PRINCIPAL_ROLE,
PolarisEntityConstants.getRootEntityId(),
gr.getSecurableId()))
+ .filter(Objects::nonNull)
.collect(Collectors.toList());
}
/**
- * Resolve the specified list of principal roles. The SecurityContext is
used to determine whether
- * the principal actually has the roles specified.
+ * Resolve the specified list of principal roles. The PolarisPrincipal is
used to determine
+ * whether the principal actually has the roles specified.
*
- * @param toValidate
- * @param roleNames
* @return the filtered list of resolved principal roles
*/
private List<ResolvedPolarisEntity> resolvePrincipalRolesByName(
Review Comment:
Sure that works for me.
> This probably means that the roles entities used by the authenticator
should be made available to the Resolver in order to take their version numbers
into account when traversing grant records.
Maybe, but this needs more investigation imho.
I still think it should possible to avoid that. Looking at the
`Resolver#resolveByName()` method, we see that it ultimately calls
`PolarisMetaStoreManager#loadResolvedEntityByName()` which returns the grant
records for the entity. That creates a "snapshot" of [principal name +
principal roles + grant records] that is consistent for a given pass, afaict
(at least as consistent as it can be, given that the operation implies several
reads that are not in the same transaction).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
