huaxingao commented on code in PR #4659:
URL: https://github.com/apache/polaris/pull/4659#discussion_r3399213737


##########
runtime/service/src/main/java/org/apache/polaris/service/idempotency/IdempotencyHandlerSupport.java:
##########
@@ -0,0 +1,313 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.service.idempotency;
+
+import jakarta.annotation.Nullable;
+import jakarta.enterprise.context.RequestScoped;
+import jakarta.inject.Inject;
+import jakarta.ws.rs.core.HttpHeaders;
+import java.time.Clock;
+import java.time.Instant;
+import java.util.Locale;
+import java.util.Optional;
+import java.util.TreeSet;
+import java.util.UUID;
+import java.util.regex.Pattern;
+import org.apache.polaris.core.DigestUtils;
+import org.apache.polaris.core.auth.PolarisPrincipal;
+import org.apache.polaris.core.context.RealmContext;
+import org.apache.polaris.core.entity.IdempotencyRecord;
+import org.apache.polaris.core.persistence.IdempotencyStore;
+import org.apache.polaris.core.persistence.IdempotencyStoreFactory;
+
+/**
+ * Handler-side helper for the single-transaction ("optimistic commit") 
idempotency model.
+ *
+ * <p>Responsibilities:
+ *
+ * <ul>
+ *   <li>Read and validate the {@code Idempotency-Key} request header (UUIDv7 
only).
+ *   <li>Compute the principal/resource hashes that form the binding stored 
alongside each record.
+ *   <li>Pre-flight: look up an existing record for the same {@code (realm, 
key)} and dispatch into
+ *       {@link Outcome#owned()} or {@link 
Outcome#duplicate(IdempotencyRecord)} for the handler.
+ *   <li>Record the terminal outcome after a successful operation, returning 
{@link Outcome#owned()}
+ *       on win and {@link Outcome#duplicate(IdempotencyRecord)} on a 
race-driven duplicate.
+ * </ul>
+ *
+ * <p>This bean is {@link RequestScoped}: a single request operates within one 
realm, so the
+ * realm-scoped {@link IdempotencyStore} is resolved once (lazily) from {@link
+ * IdempotencyStoreFactory} for the request's {@link RealmContext}. When 
idempotency is disabled the
+ * store is never resolved — the bean is an inert shell. No response body is 
stored; duplicate
+ * responses are rebuilt from authoritative catalog state by the handler 
itself.
+ */
+@RequestScoped
+public class IdempotencyHandlerSupport {
+
+  // RFC 9562 UUID v7 has version nibble 7 in time_hi_and_version.
+  private static final Pattern UUID_V7_PATTERN =
+      Pattern.compile(
+          
"^[0-9a-f]{8}-[0-9a-f]{4}-7[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$",
+          Pattern.CASE_INSENSITIVE);
+
+  @Inject IdempotencyConfiguration configuration;
+  @Inject IdempotencyStoreFactory storeFactory;
+  @Inject RealmContext realmContext;
+  @Inject Clock clock;
+
+  // Resolved lazily on first use within the request; never resolved when 
idempotency is disabled.
+  private IdempotencyStore store;
+
+  /**
+   * Returns an instance with idempotency permanently disabled. Useful for 
test fixtures that need a
+   * non-null {@code IdempotencyHandlerSupport} but exercise non-idempotent 
code paths.
+   */
+  public static IdempotencyHandlerSupport disabled() {
+    IdempotencyHandlerSupport instance = new IdempotencyHandlerSupport();
+    instance.configuration = DisabledConfiguration.INSTANCE;
+    return instance;
+  }
+
+  /** Returns {@code true} if handler-level idempotency is enabled. */
+  public boolean isEnabled() {
+    return configuration != null && configuration.enabled();
+  }
+
+  /**
+   * Reads and validates the idempotency key from the request headers using 
the deploy-time
+   * configured header name from {@link IdempotencyConfiguration#keyHeader()}.
+   *
+   * @return validated key, or {@link Optional#empty()} if {@code httpHeaders} 
is null, the header
+   *     is absent / blank, or idempotency is disabled
+   * @throws IllegalArgumentException if the header is present but not a valid 
UUIDv7 (callers
+   *     translate this into a 400 Bad Request)
+   */
+  public Optional<String> validatedKey(@Nullable HttpHeaders httpHeaders) {

Review Comment:
   Removed it. Also dropped the now-unused `keyHeader()` config it was the only 
reader of.



##########
runtime/service/src/main/java/org/apache/polaris/service/idempotency/IdempotencyHandlerSupport.java:
##########
@@ -0,0 +1,313 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.service.idempotency;
+
+import jakarta.annotation.Nullable;
+import jakarta.enterprise.context.RequestScoped;
+import jakarta.inject.Inject;
+import jakarta.ws.rs.core.HttpHeaders;
+import java.time.Clock;
+import java.time.Instant;
+import java.util.Locale;
+import java.util.Optional;
+import java.util.TreeSet;
+import java.util.UUID;
+import java.util.regex.Pattern;
+import org.apache.polaris.core.DigestUtils;
+import org.apache.polaris.core.auth.PolarisPrincipal;
+import org.apache.polaris.core.context.RealmContext;
+import org.apache.polaris.core.entity.IdempotencyRecord;
+import org.apache.polaris.core.persistence.IdempotencyStore;
+import org.apache.polaris.core.persistence.IdempotencyStoreFactory;
+
+/**
+ * Handler-side helper for the single-transaction ("optimistic commit") 
idempotency model.
+ *
+ * <p>Responsibilities:
+ *
+ * <ul>
+ *   <li>Read and validate the {@code Idempotency-Key} request header (UUIDv7 
only).
+ *   <li>Compute the principal/resource hashes that form the binding stored 
alongside each record.
+ *   <li>Pre-flight: look up an existing record for the same {@code (realm, 
key)} and dispatch into
+ *       {@link Outcome#owned()} or {@link 
Outcome#duplicate(IdempotencyRecord)} for the handler.
+ *   <li>Record the terminal outcome after a successful operation, returning 
{@link Outcome#owned()}
+ *       on win and {@link Outcome#duplicate(IdempotencyRecord)} on a 
race-driven duplicate.
+ * </ul>
+ *
+ * <p>This bean is {@link RequestScoped}: a single request operates within one 
realm, so the
+ * realm-scoped {@link IdempotencyStore} is resolved once (lazily) from {@link
+ * IdempotencyStoreFactory} for the request's {@link RealmContext}. When 
idempotency is disabled the
+ * store is never resolved — the bean is an inert shell. No response body is 
stored; duplicate
+ * responses are rebuilt from authoritative catalog state by the handler 
itself.
+ */
+@RequestScoped
+public class IdempotencyHandlerSupport {
+
+  // RFC 9562 UUID v7 has version nibble 7 in time_hi_and_version.
+  private static final Pattern UUID_V7_PATTERN =
+      Pattern.compile(
+          
"^[0-9a-f]{8}-[0-9a-f]{4}-7[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$",
+          Pattern.CASE_INSENSITIVE);
+
+  @Inject IdempotencyConfiguration configuration;
+  @Inject IdempotencyStoreFactory storeFactory;
+  @Inject RealmContext realmContext;
+  @Inject Clock clock;
+
+  // Resolved lazily on first use within the request; never resolved when 
idempotency is disabled.
+  private IdempotencyStore store;
+
+  /**
+   * Returns an instance with idempotency permanently disabled. Useful for 
test fixtures that need a
+   * non-null {@code IdempotencyHandlerSupport} but exercise non-idempotent 
code paths.
+   */
+  public static IdempotencyHandlerSupport disabled() {
+    IdempotencyHandlerSupport instance = new IdempotencyHandlerSupport();
+    instance.configuration = DisabledConfiguration.INSTANCE;
+    return instance;
+  }
+
+  /** Returns {@code true} if handler-level idempotency is enabled. */
+  public boolean isEnabled() {
+    return configuration != null && configuration.enabled();
+  }
+
+  /**
+   * Reads and validates the idempotency key from the request headers using 
the deploy-time
+   * configured header name from {@link IdempotencyConfiguration#keyHeader()}.
+   *
+   * @return validated key, or {@link Optional#empty()} if {@code httpHeaders} 
is null, the header
+   *     is absent / blank, or idempotency is disabled
+   * @throws IllegalArgumentException if the header is present but not a valid 
UUIDv7 (callers
+   *     translate this into a 400 Bad Request)
+   */
+  public Optional<String> validatedKey(@Nullable HttpHeaders httpHeaders) {
+    if (httpHeaders == null) {
+      return Optional.empty();
+    }
+    return 
validatedKey(httpHeaders.getHeaderString(configuration.keyHeader()));
+  }
+
+  /**
+   * Validates a raw header value. UUIDv7 is required so that the key has 
enough entropy to be a
+   * meaningful idempotency boundary.
+   */
+  public Optional<String> validatedKey(@Nullable String headerValue) {
+    if (!isEnabled() || headerValue == null) {
+      return Optional.empty();
+    }
+    String trimmed = headerValue.trim();
+    if (trimmed.isEmpty()) {
+      return Optional.empty();
+    }
+    if (!UUID_V7_PATTERN.matcher(trimmed).matches()) {

Review Comment:
   Good point. Switched to checking version()/variant() on the UUID.



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