roryqi commented on code in PR #12964:
URL: https://github.com/apache/gravitino/pull/12964#discussion_r4026634699
##########
core/src/main/java/org/apache/gravitino/policy/PolicyManager.java:
##########
@@ -300,7 +304,12 @@ public PolicyEntity[] listPolicyInfosForMetadataObject(
MetadataObjectUtil.checkMetadataObject(metalake, metadataObject);
checkMetalake(NameIdentifier.of(metalake), entityStore);
- return listDirectPoliciesForMetadataObject(entityIdent, entityType,
metadataObject);
+ Map<Long, PolicyEntity> policiesById = new LinkedHashMap<>();
+ Arrays.stream(listDirectPoliciesForMetadataObject(entityIdent, entityType,
metadataObject))
+ .forEach(policy -> policiesById.putIfAbsent(policy.id(), policy));
+ Arrays.stream(objectPolicyResolver.resolve(metalake, metadataObject))
Review Comment:
Thanks for catching this. Fixed in the latest revision. The REST list path
now calls `listPolicyInfosForMetadataObject` only for the requested object,
applies `LOAD_POLICY_AUTHORIZATION_EXPRESSION` to the complete resolved result,
and only then builds the names or details response. It no longer appends
unfiltered ancestor results. I also added
`testListPoliciesFiltersResolvedResult`, which covers both `details=true` and
`details=false`.
##########
core/src/main/java/org/apache/gravitino/policy/PolicyManager.java:
##########
@@ -300,7 +304,12 @@ public PolicyEntity[] listPolicyInfosForMetadataObject(
MetadataObjectUtil.checkMetadataObject(metalake, metadataObject);
checkMetalake(NameIdentifier.of(metalake), entityStore);
- return listDirectPoliciesForMetadataObject(entityIdent, entityType,
metadataObject);
+ Map<Long, PolicyEntity> policiesById = new LinkedHashMap<>();
+ Arrays.stream(listDirectPoliciesForMetadataObject(entityIdent, entityType,
metadataObject))
+ .forEach(policy -> policiesById.putIfAbsent(policy.id(), policy));
+ Arrays.stream(objectPolicyResolver.resolve(metalake, metadataObject))
+ .forEach(policy -> policiesById.putIfAbsent(policy.id(), policy));
Review Comment:
Thanks. Fixed by resolving policies exactly once for the requested object.
In the policy-on-tag target model, object policies are derived exclusively from
the object’s effective tags, so legacy direct object-policy relations are
intentionally not read and the REST layer no longer traverses ancestors.
`testChildTagValueOverridesParentPolicySelector` and the REST regression verify
that a parent `domain=finance` selector is not returned when the child
overrides it with `domain=risk`. The resolver now also preserves tag
provenance, so `inherited` is true only when the policy is selected solely
through inherited tags.
--
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]
