Skip to content

Commit 904fd30

Browse files
sfc-gh-xiwensfc-gh-xiwen
committed
Move ETag computation into IcebergCatalogHandler
1 parent 9d4f34f commit 904fd30

File tree

4 files changed

+413
-65
lines changed

4 files changed

+413
-65
lines changed

‎runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/ETaggedLoadTableResponse.java‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
package org.apache.polaris.service.catalog.iceberg;
20+
21+
import java.util.Optional;
22+
import org.apache.iceberg.rest.responses.LoadTableResponse;
23+
24+
/**
25+
* Pairs a {@link LoadTableResponse} with an optional entity tag derived from the response's
26+
* metadata location. The handler is responsible for computing the etag (when possible); transport
27+
* layers such as the REST adapter translate the etag into an HTTP {@code ETag} header.
28+
*/
29+
public record ETaggedLoadTableResponse(LoadTableResponse response, Optional<String> etag) {}

‎runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java‎

Lines changed: 11 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@
4343
import org.apache.iceberg.rest.requests.ReportMetricsRequest;
4444
import org.apache.iceberg.rest.requests.UpdateNamespacePropertiesRequest;
4545
import org.apache.iceberg.rest.requests.UpdateTableRequest;
46-
import org.apache.iceberg.rest.responses.LoadTableResponse;
4746
import org.apache.polaris.core.auth.PolarisPrincipal;
4847
import org.apache.polaris.core.config.RealmConfig;
4948
import org.apache.polaris.core.context.CallContext;
@@ -56,7 +55,6 @@
5655
import org.apache.polaris.service.catalog.api.IcebergRestConfigurationApiService;
5756
import org.apache.polaris.service.catalog.common.CatalogAdapter;
5857
import org.apache.polaris.service.config.ReservedProperties;
59-
import org.apache.polaris.service.http.IcebergHttpUtil;
6058
import org.apache.polaris.service.http.IfNoneMatch;
6159
import org.apache.polaris.service.types.CommitTableRequest;
6260
import org.apache.polaris.service.types.CommitViewRequest;
@@ -171,27 +169,12 @@ public Response loadNamespaceMetadata(
171169
}
172170

173171
/**
174-
* For situations where we typically expect a metadataLocation to be present in the response and
175-
* so expect to insert an etag header, this helper gracefully falls back to omitting the header if
176-
* unable to get metadata location and logs a warning.
172+
* Translate an {@link ETaggedLoadTableResponse} produced by the handler into a JAX-RS response
173+
* builder, attaching the handler-computed etag as an {@code ETag} HTTP header when present.
177174
*/
178-
private Response.ResponseBuilder tryInsertETagHeader(
179-
Response.ResponseBuilder builder,
180-
LoadTableResponse response,
181-
String namespace,
182-
String tableName) {
183-
if (response.metadataLocation() != null) {
184-
builder =
185-
builder.header(
186-
HttpHeaders.ETAG,
187-
IcebergHttpUtil.generateETagForMetadataFileLocation(response.metadataLocation()));
188-
} else {
189-
LOGGER
190-
.atWarn()
191-
.addKeyValue("namespace", namespace)
192-
.addKeyValue("tableName", tableName)
193-
.log("Response has null metadataLocation; omitting etag");
194-
}
175+
private Response.ResponseBuilder toResponseBuilder(ETaggedLoadTableResponse tagged) {
176+
Response.ResponseBuilder builder = Response.ok(tagged.response());
177+
tagged.etag().ifPresent(etag -> builder.header(HttpHeaders.ETAG, etag));
195178
return builder;
196179
}
197180

@@ -282,11 +265,9 @@ public Response createTable(
282265
ns, createTableRequest, delegationModes, refreshCredentialsEndpoint))
283266
.build();
284267
} else {
285-
LoadTableResponse response =
286-
catalog.createTableDirect(
287-
ns, createTableRequest, delegationModes, refreshCredentialsEndpoint);
288-
return tryInsertETagHeader(
289-
Response.ok(response), response, namespace, createTableRequest.name())
268+
return toResponseBuilder(
269+
catalog.createTableDirect(
270+
ns, createTableRequest, delegationModes, refreshCredentialsEndpoint))
290271
.build();
291272
}
292273
});
@@ -334,7 +315,7 @@ public Response loadTable(
334315
securityContext,
335316
prefix,
336317
catalog -> {
337-
Optional<LoadTableResponse> response =
318+
Optional<ETaggedLoadTableResponse> response =
338319
catalog.loadTable(
339320
tableIdentifier,
340321
snapshots,
@@ -346,8 +327,7 @@ public Response loadTable(
346327
return Response.notModified().build();
347328
}
348329

349-
return tryInsertETagHeader(Response.ok(response.get()), response.get(), namespace, table)
350-
.build();
330+
return toResponseBuilder(response.get()).build();
351331
});
352332
}
353333

@@ -415,12 +395,7 @@ public Response registerTable(
415395
return withCatalog(
416396
securityContext,
417397
prefix,
418-
catalog -> {
419-
LoadTableResponse response = catalog.registerTable(ns, registerTableRequest);
420-
return tryInsertETagHeader(
421-
Response.ok(response), response, namespace, registerTableRequest.name())
422-
.build();
423-
});
398+
catalog -> toResponseBuilder(catalog.registerTable(ns, registerTableRequest)).build());
424399
}
425400

426401
@Override

‎runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java‎

Lines changed: 57 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -420,7 +420,8 @@ public ListTablesResponse listTables(Namespace namespace) {
420420
*/
421421
public LoadTableResponse createTableDirect(Namespace namespace, CreateTableRequest request) {
422422
return createTableDirect(
423-
namespace, request, EnumSet.noneOf(AccessDelegationMode.class), Optional.empty());
423+
namespace, request, EnumSet.noneOf(AccessDelegationMode.class), Optional.empty())
424+
.response();
424425
}
425426

426427
/**
@@ -435,7 +436,8 @@ public LoadTableResponse createTableDirectWithWriteDelegation(
435436
CreateTableRequest request,
436437
Optional<String> refreshCredentialsEndpoint) {
437438
return createTableDirect(
438-
namespace, request, EnumSet.of(VENDED_CREDENTIALS), refreshCredentialsEndpoint);
439+
namespace, request, EnumSet.of(VENDED_CREDENTIALS), refreshCredentialsEndpoint)
440+
.response();
439441
}
440442

441443
public void authorizeCreateTableDirect(
@@ -456,7 +458,7 @@ public void authorizeCreateTableDirect(
456458
}
457459
}
458460

459-
public LoadTableResponse createTableDirect(
461+
public ETaggedLoadTableResponse createTableDirect(
460462
Namespace namespace,
461463
CreateTableRequest request,
462464
EnumSet<AccessDelegationMode> delegationModes,
@@ -488,16 +490,18 @@ public LoadTableResponse createTableDirect(
488490

489491
if (table instanceof BaseTable baseTable) {
490492
TableMetadata tableMetadata = baseTable.operations().current();
491-
return buildLoadTableResponseWithDelegationCredentials(
492-
tableIdentifier,
493-
tableMetadata,
494-
resolvedMode,
495-
Set.of(
496-
PolarisStorageActions.READ,
497-
PolarisStorageActions.WRITE,
498-
PolarisStorageActions.LIST),
499-
refreshCredentialsEndpoint)
500-
.build();
493+
return withETag(
494+
buildLoadTableResponseWithDelegationCredentials(
495+
tableIdentifier,
496+
tableMetadata,
497+
resolvedMode,
498+
Set.of(
499+
PolarisStorageActions.READ,
500+
PolarisStorageActions.WRITE,
501+
PolarisStorageActions.LIST),
502+
refreshCredentialsEndpoint)
503+
.build(),
504+
tableIdentifier);
501505
} else if (table instanceof BaseMetadataTable) {
502506
// metadata tables are loaded on the client side, return NoSuchTableException for now
503507
throw notFoundExceptionForTableLikeEntity(
@@ -610,12 +614,13 @@ public LoadTableResponse createTableStaged(
610614
* @param request the register table request
611615
* @return ETagged {@link LoadTableResponse} to uniquely identify the table metadata
612616
*/
613-
public LoadTableResponse registerTable(Namespace namespace, RegisterTableRequest request) {
617+
public ETaggedLoadTableResponse registerTable(Namespace namespace, RegisterTableRequest request) {
614618
PolarisAuthorizableOperation op = PolarisAuthorizableOperation.REGISTER_TABLE;
615-
authorizeCreateTableLikeUnderNamespaceOperationOrThrow(
616-
op, TableIdentifier.of(namespace, request.name()));
619+
TableIdentifier tableIdentifier = TableIdentifier.of(namespace, request.name());
620+
authorizeCreateTableLikeUnderNamespaceOperationOrThrow(op, tableIdentifier);
617621

618-
return catalogHandlerUtils().registerTable(baseCatalog, namespace, request);
622+
return withETag(
623+
catalogHandlerUtils().registerTable(baseCatalog, namespace, request), tableIdentifier);
619624
}
620625

621626
public boolean sendNotification(TableIdentifier identifier, NotificationRequest request) {
@@ -708,11 +713,12 @@ public LoadTableResponse loadTable(TableIdentifier tableIdentifier, String snaps
708713
public Optional<LoadTableResponse> loadTableIfStale(
709714
TableIdentifier tableIdentifier, IfNoneMatch ifNoneMatch, String snapshots) {
710715
return loadTable(
711-
tableIdentifier,
712-
snapshots,
713-
ifNoneMatch,
714-
EnumSet.noneOf(AccessDelegationMode.class),
715-
Optional.empty());
716+
tableIdentifier,
717+
snapshots,
718+
ifNoneMatch,
719+
EnumSet.noneOf(AccessDelegationMode.class),
720+
Optional.empty())
721+
.map(ETaggedLoadTableResponse::response);
716722
}
717723

718724
public LoadTableResponse loadTableWithAccessDelegation(
@@ -740,11 +746,12 @@ public Optional<LoadTableResponse> loadTableWithAccessDelegationIfStale(
740746
String snapshots,
741747
Optional<String> refreshCredentialsEndpoint) {
742748
return loadTable(
743-
tableIdentifier,
744-
snapshots,
745-
ifNoneMatch,
746-
EnumSet.of(VENDED_CREDENTIALS),
747-
refreshCredentialsEndpoint);
749+
tableIdentifier,
750+
snapshots,
751+
ifNoneMatch,
752+
EnumSet.of(VENDED_CREDENTIALS),
753+
refreshCredentialsEndpoint)
754+
.map(ETaggedLoadTableResponse::response);
748755
}
749756

750757
/**
@@ -852,7 +859,7 @@ private Set<PolarisStorageActions> authorizeLoadTable(
852859
return actionsRequested;
853860
}
854861

855-
public Optional<LoadTableResponse> loadTable(
862+
public Optional<ETaggedLoadTableResponse> loadTable(
856863
TableIdentifier tableIdentifier,
857864
String snapshots,
858865
IfNoneMatch ifNoneMatch,
@@ -897,7 +904,7 @@ public Optional<LoadTableResponse> loadTable(
897904
actionsRequested,
898905
refreshCredentialsEndpoint)
899906
.build();
900-
return Optional.of(filterResponseToSnapshots(response, snapshots));
907+
return Optional.of(withETag(filterResponseToSnapshots(response, snapshots), tableIdentifier));
901908
} else if (table instanceof BaseMetadataTable) {
902909
// metadata tables are loaded on the client side, return NoSuchTableException for now
903910
throw notFoundExceptionForTableLikeEntity(
@@ -907,6 +914,27 @@ public Optional<LoadTableResponse> loadTable(
907914
throw new IllegalStateException("Cannot wrap catalog that does not produce BaseTable");
908915
}
909916

917+
/**
918+
* Pair a {@link LoadTableResponse} with an entity tag derived from its metadata location. When
919+
* the response has no metadata location (e.g. staged create or external catalogs that don't
920+
* expose one) the etag is omitted and a warning is logged.
921+
*/
922+
private ETaggedLoadTableResponse withETag(
923+
LoadTableResponse response, TableIdentifier tableIdentifier) {
924+
if (response.metadataLocation() != null) {
925+
return new ETaggedLoadTableResponse(
926+
response,
927+
Optional.of(
928+
IcebergHttpUtil.generateETagForMetadataFileLocation(response.metadataLocation())));
929+
}
930+
LOGGER
931+
.atWarn()
932+
.addKeyValue("namespace", tableIdentifier.namespace())
933+
.addKeyValue("tableName", tableIdentifier.name())
934+
.log("Response has null metadataLocation; omitting etag");
935+
return new ETaggedLoadTableResponse(response, Optional.empty());
936+
}
937+
910938
private LoadTableResponse.Builder buildLoadTableResponseWithDelegationCredentials(
911939
TableIdentifier tableIdentifier,
912940
TableMetadata tableMetadata,

0 commit comments

Comments
 (0)