sungwy commented on code in PR #4409:
URL: https://github.com/apache/polaris/pull/4409#discussion_r3334988367


##########
polaris-core/src/main/java/org/apache/polaris/core/auth/AuthorizationRequest.java:
##########
@@ -18,111 +18,53 @@
  */
 package org.apache.polaris.core.auth;
 
+import com.google.common.base.Preconditions;
 import java.util.List;
-import java.util.Objects;
-import java.util.stream.Collectors;
 import org.apache.polaris.core.entity.PolarisEntityType;
-import org.apache.polaris.immutables.PolarisImmutable;
-import org.immutables.value.Value;
 import org.jspecify.annotations.NonNull;
 
-/**
- * Authorization request inputs for pre-authorization and core authorization.
- *
- * <p>This wrapper keeps authorization inputs together and conveys the intent 
to be authorized via
- * {@link AuthorizationTargetBinding} target bindings.
- */
-@PolarisImmutable
-public interface AuthorizationRequest {
-  static AuthorizationRequest of(
-      @NonNull PolarisPrincipal principal,
-      @NonNull PolarisAuthorizableOperation operation,
-      @NonNull List<AuthorizationTargetBinding> targetBindings) {
-    return ImmutableAuthorizationRequest.builder()
-        .principal(principal)
-        .operation(operation)
-        .targetBindings(targetBindings)
-        .build();
+/** Full authorization request containing the subject and one or more 
authorization intents. */
+public record AuthorizationRequest(
+    @NonNull PolarisPrincipal principal, @NonNull List<AuthorizationIntent> 
intents) {
+  public AuthorizationRequest {
+    Preconditions.checkNotNull(principal, "principal must be non-null");
+    Preconditions.checkNotNull(intents, "intents must be non-null");
+    intents = List.copyOf(intents);
+    Preconditions.checkArgument(
+        !intents.isEmpty(), "Authorization request must contain at least one 
intent");
   }
 
-  /** Returns the principal requesting authorization. */
-  @NonNull PolarisPrincipal getPrincipal();
-
-  /** Returns the operation being authorized. */
-  @NonNull PolarisAuthorizableOperation getOperation();
-
-  /** Returns the target/secondary target bindings. */
-  @NonNull List<AuthorizationTargetBinding> getTargetBindings();
-
-  /**
-   * Returns the primary target securables, if any.
-   *
-   * <p>Compatibility accessor derived from {@link #getTargetBindings()}.
-   */
-  @NonNull
-  @Value.Derived
-  default List<PolarisSecurable> getTargets() {
-    return getTargetBindings().stream()
-        .map(AuthorizationTargetBinding::getTarget)
-        .filter(Objects::nonNull)
-        .toList();
+  public static AuthorizationRequest of(
+      @NonNull PolarisPrincipal principal, @NonNull AuthorizationIntent 
intent) {
+    return new AuthorizationRequest(principal, List.of(intent));
   }
 
-  /**
-   * Returns secondary securables, if any.
-   *
-   * <p>Compatibility accessor derived from {@link #getTargetBindings()}.
-   */
-  @NonNull
-  @Value.Derived
-  default List<PolarisSecurable> getSecondaries() {
-    return getTargetBindings().stream()
-        .map(AuthorizationTargetBinding::getSecondary)
-        .filter(Objects::nonNull)
-        .toList();
+  public static AuthorizationRequest of(
+      @NonNull PolarisPrincipal principal, @NonNull List<AuthorizationIntent> 
intents) {
+    return new AuthorizationRequest(principal, intents);
   }
 
-  /**
-   * Returns a stable debug string for authorization messages.
-   *
-   * <p>Includes the operation, principal name, formatted targets, and 
formatted secondaries.
-   */
-  @NonNull
-  default String formatForAuthorizationMessage() {
-    return String.format(
-        "operation=%s principal=%s targets=%s secondaries=%s",
-        getOperation(),
-        getPrincipal().getName(),
-        formatSecurables(getTargets()),
-        formatSecurables(getSecondaries()));
+  public static AuthorizationRequest of(
+      @NonNull PolarisPrincipal principal, @NonNull 
PolarisAuthorizableOperation operation) {
+    return of(principal, AuthorizationIntent.of(operation));
   }
 
-  private static String formatSecurables(List<PolarisSecurable> securables) {
-    return securables.stream()
-        .map(PolarisSecurable::formatForAuthorizationMessage)
-        .collect(Collectors.joining(", ", "[", "]"));
+  public static AuthorizationRequest of(
+      @NonNull PolarisPrincipal principal,
+      @NonNull PolarisAuthorizableOperation operation,
+      @NonNull PolarisSecurable target) {
+    return of(principal, AuthorizationIntent.of(operation, target));
   }
 
-  default boolean hasSecurableType(PolarisEntityType... types) {
-    for (AuthorizationTargetBinding targetBinding : getTargetBindings()) {
-      if (targetBinding.getTarget() != null && 
containsType(targetBinding.getTarget(), types)) {
-        return true;
-      }
-      if (targetBinding.getSecondary() != null
-          && containsType(targetBinding.getSecondary(), types)) {
-        return true;
-      }
-    }
-    return false;
+  public static AuthorizationRequest of(
+      @NonNull PolarisPrincipal principal,
+      @NonNull PolarisAuthorizableOperation operation,
+      PolarisSecurable target,
+      PolarisSecurable secondary) {
+    return of(principal, AuthorizationIntent.of(operation, target, secondary));
   }
 
-  static boolean containsType(PolarisSecurable securable, PolarisEntityType... 
types) {
-    PolarisEntityType entityType = securable.getLeaf().entityType();
-    for (PolarisEntityType type : types) {
-      if (entityType == type) {
-        return true;
-      }
-    }
-    return false;
+  public boolean hasSecurableType(PolarisEntityType type) {

Review Comment:
   Sounds good 👍 - I was thinking it would make sense to filter based on 
`AuthorizationIntent` subtype as well, depending on how we land on the subtypes



-- 
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]

Reply via email to