huaxingao commented on code in PR #4659:
URL: https://github.com/apache/polaris/pull/4659#discussion_r3392656418
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java:
##########
@@ -81,17 +82,38 @@ public class IcebergCatalogAdapter
private final CatalogPrefixParser prefixParser;
private final ReservedProperties reservedProperties;
private final IcebergCatalogHandlerFactory handlerFactory;
+ private final IdempotencyHandlerSupport idempotencySupport;
@Inject
public IcebergCatalogAdapter(
CallContext callContext,
CatalogPrefixParser prefixParser,
ReservedProperties reservedProperties,
- IcebergCatalogHandlerFactory handlerFactory) {
+ IcebergCatalogHandlerFactory handlerFactory,
+ IdempotencyHandlerSupport idempotencySupport) {
this.realmConfig = callContext.getRealmConfig();
this.prefixParser = prefixParser;
this.reservedProperties = reservedProperties;
this.handlerFactory = handlerFactory;
+ this.idempotencySupport = idempotencySupport;
+ }
+
+ /**
+ * Validates the {@code Idempotency-Key} parameter auto-bound from the
request header by the
+ * OpenAPI-generated stub. Returns the normalised key string when present
and idempotency is
+ * enabled; otherwise {@link Optional#empty()}.
+ *
+ * @throws BadRequestException if the header is present but not a valid
UUIDv7
+ */
+ private Optional<String> validatedIdempotencyKey(UUID idempotencyKey) {
+ if (idempotencyKey == null) {
+ return Optional.empty();
+ }
+ try {
+ return idempotencySupport.validatedKey(idempotencyKey.toString());
+ } catch (IllegalArgumentException e) {
+ throw new BadRequestException("%s", e.getMessage());
Review Comment:
You are right, `IcebergExceptionMapper` already maps
I`llegalArgumentException to 400, so the rewrap was redundant. Removed the
try/catch.
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java:
##########
@@ -416,30 +431,43 @@ public ListTablesResponse listTables(Namespace namespace)
{
}
/**
- * Create a table.
+ * Convenience entry point used by tests that exercise the authorization
wiring without going
+ * through the REST adapter. Production callers go through the four-arg
overload via the adapter.
*
* @param namespace the namespace to create the table in
* @param request the table creation request
* @return ETagged {@link LoadTableResponse} to uniquely identify the table
metadata
*/
+ @VisibleForTesting
public LoadTableResponse createTableDirect(Namespace namespace,
CreateTableRequest request) {
return createTableDirect(
- namespace, request, EnumSet.noneOf(AccessDelegationMode.class),
Optional.empty());
+ namespace,
+ request,
+ EnumSet.noneOf(AccessDelegationMode.class),
+ Optional.empty(),
+ Optional.empty());
}
/**
- * Create a table.
+ * Convenience entry point used by tests for the write-delegation variant.
Production callers go
+ * through the adapter, which resolves the delegation modes and
refresh-credentials endpoint from
+ * the request.
*
* @param namespace the namespace to create the table in
* @param request the table creation request
* @return ETagged {@link LoadTableResponse} to uniquely identify the table
metadata
*/
+ @VisibleForTesting
public LoadTableResponse createTableDirectWithWriteDelegation(
Review Comment:
Since this is pre-existing test-only scaffolding, is it OK to postpone it to
a separate PR to keep this one focused?
--
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]
