Skip to content

Core: Check for valid identifiers in REST catalog #5107

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
danielcweeks merged 10 commits into from
Jun 21, 2022
Merged

Core: Check for valid identifiers in REST catalog #5107

Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
8f836b0
Shutdown refresh token thread during REST catalog client close
bryanck Jun 4, 2022
12b6349
Merge branch 'apache:master' into master
bryanck Jun 7, 2022
4670322
Improved shutdown of token refresh executor during REST catalog close
bryanck Jun 7, 2022
e9a39da
Merge remote-tracking branch 'upstream/master'
bryanck Jun 20, 2022
68b9490
REST: Set table format version for create table transactions
bryanck Jun 20, 2022
5aaf1f3
Merge remote-tracking branch 'upstream/master'
bryanck Jun 20, 2022
d029cc0
add test for creating v2 table via transaction
bryanck Jun 20, 2022
576f7c0
Merge remote-tracking branch 'upstream/master'
bryanck Jun 21, 2022
ec90232
Merge remote-tracking branch 'upstream/master'
bryanck Jun 21, 2022
2ce73b0
Core: Check for valid identifiers in REST catalog
bryanck Jun 21, 2022
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
29 changes: 29 additions & 0 deletions core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,13 +176,17 @@ public void setConf(Configuration newConf) {

@Override
public List<TableIdentifier> listTables(SessionContext context, Namespace ns) {
checkNamespaceIsValid(ns);

ListTablesResponse response = client.get(
paths.tables(ns), ListTablesResponse.class, headers(context), ErrorHandlers.namespaceErrorHandler());
return response.identifiers();
}

@Override
public boolean dropTable(SessionContext context, TableIdentifier identifier) {
checkIdentifierIsValid(identifier);

try {
client.delete(paths.table(identifier), null, headers(context), ErrorHandlers.tableErrorHandler());
return true;
Expand All @@ -198,6 +202,9 @@ public boolean purgeTable(SessionContext context, TableIdentifier ident) {

@Override
public void renameTable(SessionContext context, TableIdentifier from, TableIdentifier to) {
checkIdentifierIsValid(from);
checkIdentifierIsValid(to);

RenameTableRequest request = RenameTableRequest.builder()
.withSource(from)
.withDestination(to)
Expand All @@ -214,6 +221,8 @@ private LoadTableResponse loadInternal(SessionContext context, TableIdentifier i

@Override
public Table loadTable(SessionContext context, TableIdentifier identifier) {
checkIdentifierIsValid(identifier);

MetadataTableType metadataType;
LoadTableResponse response;
try {
Expand Down Expand Up @@ -295,6 +304,8 @@ public List<Namespace> listNamespaces(SessionContext context, Namespace namespac

@Override
public Map<String, String> loadNamespaceMetadata(SessionContext context, Namespace ns) {
checkNamespaceIsValid(ns);

// TODO: rename to LoadNamespaceResponse?
GetNamespaceResponse response = client
.get(paths.namespace(ns), GetNamespaceResponse.class, headers(context), ErrorHandlers.namespaceErrorHandler());
Expand All @@ -303,6 +314,8 @@ public Map<String, String> loadNamespaceMetadata(SessionContext context, Namespa

@Override
public boolean dropNamespace(SessionContext context, Namespace ns) {
checkNamespaceIsValid(ns);

try {
client.delete(paths.namespace(ns), null, headers(context), ErrorHandlers.namespaceErrorHandler());
return true;
Expand All @@ -314,6 +327,8 @@ public boolean dropNamespace(SessionContext context, Namespace ns) {
@Override
public boolean updateNamespaceMetadata(SessionContext context, Namespace ns,
Map<String, String> updates, Set<String> removals) {
checkNamespaceIsValid(ns);

UpdateNamespacePropertiesRequest request = UpdateNamespacePropertiesRequest.builder()
.updateAll(updates)
.removeAll(removals)
Expand Down Expand Up @@ -404,6 +419,8 @@ private class Builder implements Catalog.TableBuilder {
private String location = null;

private Builder(TableIdentifier ident, Schema schema, SessionContext context) {
checkIdentifierIsValid(ident);

this.ident = ident;
this.schema = schema;
this.context = context;
Expand Down Expand Up @@ -678,6 +695,18 @@ private Long expiresInMs(Map<String, String> properties) {
}
}

private void checkIdentifierIsValid(TableIdentifier tableIdentifier) {
if (tableIdentifier.namespace().isEmpty()) {
throw new NoSuchTableException("Invalid table identifier: %s", tableIdentifier);
}
}

private void checkNamespaceIsValid(Namespace namespace) {
if (namespace.isEmpty()) {
throw new NoSuchNamespaceException("Invalid namespace: %s", namespace);
}
}

private static Map<String, String> configHeaders(Map<String, String> properties) {
return RESTUtil.extractPrefixMap(properties, "header.");
}
Expand Down