Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,6 @@
import com.netflix.metacat.common.server.connectors.exception.TableAlreadyExistsException;
import com.netflix.metacat.common.server.connectors.exception.TablePreconditionFailedException;
import com.netflix.metacat.common.server.monitoring.Metrics;
import com.netflix.metacat.common.server.properties.Config;
import com.netflix.metacat.common.server.usermetadata.AliasService;
import com.netflix.metacat.common.server.usermetadata.UserMetadataServiceException;
import com.netflix.metacat.common.server.util.MetacatContextManager;
import com.netflix.spectator.api.Id;
Expand Down Expand Up @@ -68,8 +66,6 @@
@Component
public final class RequestWrapper {
private final Registry registry;
private final Config config;
private final AliasService aliasService;
private final RequestGateway requestGateway;

//Metrics
Expand All @@ -82,18 +78,12 @@ public final class RequestWrapper {
* Wrapper class for processing the request.
*
* @param registry registry
* @param config Config
* @param aliasService AliasService
* @param requestGateway RequestGateway
*/
@Autowired
public RequestWrapper(@NotNull @NonNull final Registry registry,
@NotNull @NonNull final Config config,
@NotNull @NonNull final AliasService aliasService,
@NotNull @NonNull final RequestGateway requestGateway) {
this.registry = registry;
this.config = config;
this.aliasService = aliasService;
this.requestGateway = requestGateway;

requestCounterId = registry.createId(Metrics.CounterRequestCount.getMetricName());
Expand All @@ -103,33 +93,19 @@ public RequestWrapper(@NotNull @NonNull final Registry registry,
}

/**
* Creates the qualified name, resolving a table alias to the table it points at.
* Creates the qualified name.
*
* @param nameSupplier supplier
* @return name
*/
public QualifiedName qualifyName(final Supplier<QualifiedName> nameSupplier) {
final QualifiedName name = unresolvedQualifiedName(nameSupplier);
if (config.isTableAliasEnabled() && name.getType() == QualifiedName.Type.TABLE) {
return aliasService.getTableName(name);
}
return name;
}

/**
* Creates the qualified name without resolving a table alias. Table endpoints use this so that the
* table service sees the name as it was requested, and can resolve or reject the alias itself.
*
* @param nameSupplier supplier
* @return name
*/
public QualifiedName unresolvedQualifiedName(final Supplier<QualifiedName> nameSupplier) {
try {
return nameSupplier.get();
} catch (Exception e) {
log.error("Invalid qualified name", e);
throw new MetacatBadRequestException(e.getMessage());
}

}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ public TableDto createTable(
@Parameter(description = "The table information", required = true)
@Valid @RequestBody final TableDto table
) {
final QualifiedName name = this.requestWrapper.unresolvedQualifiedName(
final QualifiedName name = this.requestWrapper.qualifyName(
() -> QualifiedName.ofTable(catalogName, databaseName, tableName)
);
if (MetacatServiceHelper.isIcebergTable(table)) {
Expand Down Expand Up @@ -387,7 +387,7 @@ public TableDto deleteTable(
@Parameter(description = "The name of the table", required = true)
@PathVariable("table-name") final String tableName
) {
final QualifiedName name = this.requestWrapper.unresolvedQualifiedName(
final QualifiedName name = this.requestWrapper.qualifyName(
() -> QualifiedName.ofTable(catalogName, databaseName, tableName)
);
return this.requestWrapper.processRequest(
Expand Down Expand Up @@ -642,7 +642,7 @@ public TableDto getTable(
name = "includeMetadataLocationOnly",
defaultValue = "false") final boolean includeMetadataLocationOnly
) {
final QualifiedName name = this.requestWrapper.unresolvedQualifiedName(
final QualifiedName name = this.requestWrapper.qualifyName(
() -> QualifiedName.ofTable(catalogName, databaseName, tableName)
);
return this.requestWrapper.processRequest(
Expand Down Expand Up @@ -997,10 +997,10 @@ public void renameTable(
@Parameter(description = "The name of the table", required = true)
@RequestParam("newTableName") final String newTableName
) {
final QualifiedName oldName = this.requestWrapper.unresolvedQualifiedName(
final QualifiedName oldName = this.requestWrapper.qualifyName(
() -> QualifiedName.ofTable(catalogName, databaseName, tableName)
);
final QualifiedName newName = this.requestWrapper.unresolvedQualifiedName(
final QualifiedName newName = this.requestWrapper.qualifyName(
() -> QualifiedName.ofTable(catalogName, databaseName, newTableName)
);
this.requestWrapper.processRequest(
Expand Down Expand Up @@ -1208,7 +1208,7 @@ public TableDto updateTable(
name = "shouldThrowExceptionOnMetadataSaveFailure",
defaultValue = "false") final boolean shouldThrowExceptionOnMetadataSaveFailure
) {
final QualifiedName name = this.requestWrapper.unresolvedQualifiedName(
final QualifiedName name = this.requestWrapper.qualifyName(
() -> QualifiedName.ofTable(catalogName, databaseName, tableName)
);
return this.requestWrapper.processRequest(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ public void deletePartitions(
@Parameter(description = "partitionId of the partitions to be deleted from this table", required = true)
@RequestBody final List<String> partitionIds
) {
final QualifiedName name = this.requestWrapper.unresolvedQualifiedName(
final QualifiedName name = this.requestWrapper.qualifyName(
() -> QualifiedName.ofTable(catalogName, databaseName, tableName)
);
this.requestWrapper.processRequest(
Expand Down Expand Up @@ -179,7 +179,7 @@ public void deletePartitions(
@Parameter(description = "partitionId of the partitions to be deleted from this table", required = true)
@RequestBody final List<String> partitionIds
) {
final QualifiedName name = this.requestWrapper.unresolvedQualifiedName(
final QualifiedName name = this.requestWrapper.qualifyName(
() -> QualifiedName.ofView(catalogName, databaseName, tableName, viewName)
);
this.requestWrapper.processRequest(
Expand Down Expand Up @@ -251,7 +251,7 @@ public List<PartitionDto> getPartitions(
@Parameter(description = "Whether to include user metadata information to the response")
@RequestParam(name = "includeUserMetadata", defaultValue = "false") final boolean includeUserMetadata
) {
final QualifiedName name = this.requestWrapper.unresolvedQualifiedName(
final QualifiedName name = this.requestWrapper.qualifyName(
() -> QualifiedName.ofTable(catalogName, databaseName, tableName)
);
return this.requestWrapper.processRequest(
Expand Down Expand Up @@ -327,7 +327,7 @@ public List<PartitionDto> getPartitions(
@Parameter(description = "Whether to include user metadata information to the response")
@RequestParam(name = "includeUserMetadata", defaultValue = "false") final boolean includeUserMetadata
) {
final QualifiedName name = this.requestWrapper.unresolvedQualifiedName(
final QualifiedName name = this.requestWrapper.qualifyName(
() -> QualifiedName.ofView(catalogName, databaseName, tableName, viewName)
);
return this.requestWrapper.processRequest(
Expand Down Expand Up @@ -1123,7 +1123,7 @@ public PartitionsSaveResponseDto savePartitions(
@Parameter(description = "Request containing the list of partitions", required = true)
@RequestBody final PartitionsSaveRequestDto partitionsSaveRequestDto
) {
final QualifiedName name = this.requestWrapper.unresolvedQualifiedName(
final QualifiedName name = this.requestWrapper.qualifyName(
() -> QualifiedName.ofView(catalogName, databaseName, tableName, viewName)
);
return this.requestWrapper.processRequest(
Expand Down Expand Up @@ -1188,7 +1188,7 @@ public Integer getPartitionCount(
@Parameter(description = "The name of the table", required = true)
@PathVariable("table-name") final String tableName
) {
final QualifiedName name = this.requestWrapper.unresolvedQualifiedName(
final QualifiedName name = this.requestWrapper.qualifyName(
() -> QualifiedName.ofTable(catalogName, databaseName, tableName)
);
return this.requestWrapper.processRequest(
Expand Down Expand Up @@ -1237,7 +1237,7 @@ public Integer getPartitionCount(
@Parameter(description = "The name of the view", required = true)
@PathVariable("view-name") final String viewName
) {
final QualifiedName name = this.requestWrapper.unresolvedQualifiedName(
final QualifiedName name = this.requestWrapper.qualifyName(
() -> QualifiedName.ofView(catalogName, databaseName, tableName, viewName)
);
return this.requestWrapper.processRequest(
Expand All @@ -1258,7 +1258,7 @@ private List<PartitionDto> getPartitions(
final boolean includeUserMetadata,
@Nullable final GetPartitionsRequestDto getPartitionsRequestDto
) {
final QualifiedName name = this.requestWrapper.unresolvedQualifiedName(
final QualifiedName name = this.requestWrapper.qualifyName(
() -> QualifiedName.ofTable(catalogName, databaseName, tableName)
);

Expand Down Expand Up @@ -1291,7 +1291,7 @@ private List<PartitionDto> getPartitions(
final boolean includeUserMetadata,
@Nullable final GetPartitionsRequestDto getPartitionsRequestDto
) {
final QualifiedName name = this.requestWrapper.unresolvedQualifiedName(
final QualifiedName name = this.requestWrapper.qualifyName(
() -> QualifiedName.ofView(catalogName, databaseName, tableName, viewName)
);
return this.requestWrapper.processRequest(
Expand Down Expand Up @@ -1321,7 +1321,7 @@ private List<String> _getPartitionUris(
@Nullable final Integer limit,
@Nullable final GetPartitionsRequestDto getPartitionsRequestDto
) {
final QualifiedName name = this.requestWrapper.unresolvedQualifiedName(
final QualifiedName name = this.requestWrapper.qualifyName(
() -> QualifiedName.ofTable(catalogName, databaseName, tableName)
);
return this.requestWrapper.processRequest(
Expand Down Expand Up @@ -1352,7 +1352,7 @@ private List<String> _getMViewPartitionKeys(
@Nullable final GetPartitionsRequestDto getPartitionsRequestDto
) {

final QualifiedName name = this.requestWrapper.unresolvedQualifiedName(
final QualifiedName name = this.requestWrapper.qualifyName(
() -> QualifiedName.ofView(catalogName, databaseName, tableName, viewName)
);
return this.requestWrapper.processRequest(
Expand Down Expand Up @@ -1382,7 +1382,7 @@ private List<String> _getMViewPartitionUris(
@Nullable final Integer limit,
@Nullable final GetPartitionsRequestDto getPartitionsRequestDto
) {
final QualifiedName name = this.requestWrapper.unresolvedQualifiedName(
final QualifiedName name = this.requestWrapper.qualifyName(
() -> QualifiedName.ofView(catalogName, databaseName, tableName, viewName)
);
return this.requestWrapper.processRequest(
Expand Down Expand Up @@ -1412,7 +1412,7 @@ private List<String> _getPartitionKeys(
@Nullable final GetPartitionsRequestDto getPartitionsRequestDto
) {

final QualifiedName name = this.requestWrapper.unresolvedQualifiedName(
final QualifiedName name = this.requestWrapper.qualifyName(
() -> QualifiedName.ofTable(catalogName, databaseName, tableName)
);
return this.requestWrapper.processRequest(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import com.netflix.metacat.common.QualifiedName
import com.netflix.metacat.common.exception.MetacatBadRequestException
import com.netflix.metacat.common.server.api.traffic_control.RequestGateway
import com.netflix.metacat.common.server.properties.Config
import com.netflix.metacat.common.server.usermetadata.AliasService
import com.netflix.metacat.common.server.util.MetacatContextManager
import com.netflix.spectator.api.*
import spock.lang.Specification
Expand All @@ -32,15 +31,14 @@ class RequestWrapperSpec extends Specification {

def registry = new NoopRegistry()
def config = Mock(Config)
def aliasService = Mock(AliasService)
def requestGateway = Mock(RequestGateway)
def counter = Mock(Counter)
def supplier = Mock(Supplier)
def requestWrapper

def setup() {
this.supplier.get() >> null
requestWrapper = new RequestWrapper(registry, config, aliasService, requestGateway)
requestWrapper = new RequestWrapper(registry, requestGateway)
}

def "gateway is invoked for each request"() {
Expand Down Expand Up @@ -72,71 +70,11 @@ class RequestWrapperSpec extends Specification {
MetacatContextManager.getContext().getRequestName() == "getTable"
}

def "qualifyName resolves a table alias to its source table"() {
given:
def alias = QualifiedName.ofTable("c", "d", "the_alias")
def source = QualifiedName.ofTable("c", "d", "the_source")

when:
def result = requestWrapper.qualifyName({ alias })

then:
1 * config.isTableAliasEnabled() >> true
1 * aliasService.getTableName(alias) >> source
result == source
}

def "qualifyName does not consult the alias service when aliasing is disabled"() {
given:
def alias = QualifiedName.ofTable("c", "d", "the_alias")

when:
def result = requestWrapper.qualifyName({ alias })

then:
1 * config.isTableAliasEnabled() >> false
0 * aliasService.getTableName(_)
result == alias
}

def "qualifyName only resolves aliases for table names"() {
given:
def dbName = QualifiedName.ofDatabase("c", "d")

when:
def result = requestWrapper.qualifyName({ dbName })

then:
0 * aliasService.getTableName(_)
result == dbName
}

def "qualifyName converts a failing supplier into a bad request"() {
when:
requestWrapper.qualifyName({ throw new IllegalArgumentException("bad name") })

then:
thrown(MetacatBadRequestException)
}

def "unresolvedQualifiedName leaves an alias alone, so the table service can decide what to do with it"() {
given:
def alias = QualifiedName.ofTable("c", "d", "the_alias")

when:
def result = requestWrapper.unresolvedQualifiedName({ alias })

then:
0 * aliasService.getTableName(_)
0 * aliasService.isAlias(_)
result == alias
}

def "unresolvedQualifiedName converts a failing supplier into a bad request"() {
when:
requestWrapper.unresolvedQualifiedName({ throw new IllegalArgumentException("bad name") })

then:
thrown(MetacatBadRequestException)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ class MetacatControllerAliasSpec extends Specification {
def aliasService = Mock(AliasService)
def requestGateway = Mock(RequestGateway)
def tableService = Mock(TableService)
def requestWrapper = new RequestWrapper(new NoopRegistry(), config, aliasService, requestGateway)
def requestWrapper = new RequestWrapper(new NoopRegistry(), requestGateway)
def controller = new MetacatController(
Mock(CatalogService), Mock(DatabaseService), Mock(MViewService),
tableService, requestWrapper, config)
Expand Down
Loading