diff --git a/core/src/main/java/org/apache/iceberg/rest/RESTCatalog.java b/core/src/main/java/org/apache/iceberg/rest/RESTCatalog.java index e30db22ef43d..d2e7cad3e9d4 100644 --- a/core/src/main/java/org/apache/iceberg/rest/RESTCatalog.java +++ b/core/src/main/java/org/apache/iceberg/rest/RESTCatalog.java @@ -54,12 +54,12 @@ import org.apache.iceberg.rest.requests.CreateNamespaceRequest; import org.apache.iceberg.rest.requests.CreateTableRequest; import org.apache.iceberg.rest.requests.UpdateNamespacePropertiesRequest; +import org.apache.iceberg.rest.responses.ConfigResponse; import org.apache.iceberg.rest.responses.CreateNamespaceResponse; import org.apache.iceberg.rest.responses.GetNamespaceResponse; import org.apache.iceberg.rest.responses.ListNamespacesResponse; import org.apache.iceberg.rest.responses.ListTablesResponse; import org.apache.iceberg.rest.responses.LoadTableResponse; -import org.apache.iceberg.rest.responses.RESTCatalogConfigResponse; import org.apache.iceberg.rest.responses.UpdateNamespacePropertiesResponse; import org.apache.iceberg.util.Pair; import org.slf4j.Logger; @@ -85,7 +85,7 @@ public RESTCatalog() { @Override public void initialize(String name, Map props) { - RESTCatalogConfigResponse config = fetchConfig(props); + ConfigResponse config = fetchConfig(props); Map mergedProps = config.merge(props); this.client = clientBuilder.apply(mergedProps); this.catalogName = name; @@ -429,14 +429,14 @@ private Pair tableClients(Map config) { return Pair.of(tableClient, tableIO); } - private RESTCatalogConfigResponse fetchConfig(Map props) { + private ConfigResponse fetchConfig(Map props) { // Create a client for one time use, as we will reconfigure the client using the merged server and application // defined configuration. RESTClient singleUseClient = clientBuilder.apply(props); try { - RESTCatalogConfigResponse configResponse = singleUseClient - .get(ResourcePaths.config(), RESTCatalogConfigResponse.class, ErrorHandlers.defaultErrorHandler()); + ConfigResponse configResponse = singleUseClient + .get(ResourcePaths.config(), ConfigResponse.class, ErrorHandlers.defaultErrorHandler()); configResponse.validate(); return configResponse; } finally { diff --git a/core/src/main/java/org/apache/iceberg/rest/responses/RESTCatalogConfigResponse.java b/core/src/main/java/org/apache/iceberg/rest/responses/ConfigResponse.java similarity index 94% rename from core/src/main/java/org/apache/iceberg/rest/responses/RESTCatalogConfigResponse.java rename to core/src/main/java/org/apache/iceberg/rest/responses/ConfigResponse.java index 10e3e8228297..0029770b67bd 100644 --- a/core/src/main/java/org/apache/iceberg/rest/responses/RESTCatalogConfigResponse.java +++ b/core/src/main/java/org/apache/iceberg/rest/responses/ConfigResponse.java @@ -40,16 +40,16 @@ *
  • overrides - properties that should be used to override client configuration
    • * */ -public class RESTCatalogConfigResponse implements RESTResponse { +public class ConfigResponse implements RESTResponse { private Map defaults; private Map overrides; - public RESTCatalogConfigResponse() { + public ConfigResponse() { // Required for Jackson deserialization } - private RESTCatalogConfigResponse(Map defaults, Map overrides) { + private ConfigResponse(Map defaults, Map overrides) { this.defaults = defaults; this.overrides = overrides; validate(); @@ -152,8 +152,8 @@ public Builder withOverrides(Map overridesToAdd) { return this; } - public RESTCatalogConfigResponse build() { - return new RESTCatalogConfigResponse(defaults, overrides); + public ConfigResponse build() { + return new ConfigResponse(defaults, overrides); } } } diff --git a/core/src/test/java/org/apache/iceberg/rest/RESTCatalogAdapter.java b/core/src/test/java/org/apache/iceberg/rest/RESTCatalogAdapter.java index 536ea2bbdc07..0a12305aa001 100644 --- a/core/src/test/java/org/apache/iceberg/rest/RESTCatalogAdapter.java +++ b/core/src/test/java/org/apache/iceberg/rest/RESTCatalogAdapter.java @@ -45,8 +45,8 @@ import org.apache.iceberg.rest.requests.CreateTableRequest; import org.apache.iceberg.rest.requests.UpdateNamespacePropertiesRequest; import org.apache.iceberg.rest.requests.UpdateTableRequest; +import org.apache.iceberg.rest.responses.ConfigResponse; import org.apache.iceberg.rest.responses.ErrorResponse; -import org.apache.iceberg.rest.responses.RESTCatalogConfigResponse; import org.apache.iceberg.util.Pair; /** @@ -154,7 +154,7 @@ public T handleRequest(Route route, Map Object body, Class responseType) { switch (route) { case CONFIG: - return castResponse(responseType, RESTCatalogConfigResponse.builder().build()); + return castResponse(responseType, ConfigResponse.builder().build()); case LIST_NAMESPACES: if (asNamespaceCatalog != null) { diff --git a/core/src/test/java/org/apache/iceberg/rest/TestRESTCatalog.java b/core/src/test/java/org/apache/iceberg/rest/TestRESTCatalog.java index 5c8d3169a089..5c95ab5eebdd 100644 --- a/core/src/test/java/org/apache/iceberg/rest/TestRESTCatalog.java +++ b/core/src/test/java/org/apache/iceberg/rest/TestRESTCatalog.java @@ -31,8 +31,8 @@ import org.apache.iceberg.catalog.CatalogTests; import org.apache.iceberg.jdbc.JdbcCatalog; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; +import org.apache.iceberg.rest.responses.ConfigResponse; import org.apache.iceberg.rest.responses.ErrorResponse; -import org.apache.iceberg.rest.responses.RESTCatalogConfigResponse; import org.junit.Assert; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; @@ -112,7 +112,7 @@ public T delete(String path, Class responseType, @Override public T get(String path, Class responseType, Consumer errorHandler) { - return (T) RESTCatalogConfigResponse + return (T) ConfigResponse .builder() .withDefaults(ImmutableMap.of(CatalogProperties.CLIENT_POOL_SIZE, "1")) .withOverrides(ImmutableMap.of(CatalogProperties.CACHE_ENABLED, "false")) diff --git a/core/src/test/java/org/apache/iceberg/rest/responses/TestRESTCatalogConfigResponse.java b/core/src/test/java/org/apache/iceberg/rest/responses/TestRESTCatalogConfigResponse.java index 82d411d9d497..be6ed80f5d96 100644 --- a/core/src/test/java/org/apache/iceberg/rest/responses/TestRESTCatalogConfigResponse.java +++ b/core/src/test/java/org/apache/iceberg/rest/responses/TestRESTCatalogConfigResponse.java @@ -29,7 +29,7 @@ import org.junit.BeforeClass; import org.junit.Test; -public class TestRESTCatalogConfigResponse extends RequestResponseTestBase { +public class TestRESTCatalogConfigResponse extends RequestResponseTestBase { private static final Map DEFAULTS = ImmutableMap.of("warehouse", "s3://bucket/warehouse"); private static final Map OVERRIDES = ImmutableMap.of("clients", "5"); @@ -50,63 +50,63 @@ public void testRoundTripSerDe() throws JsonProcessingException { String fullJson = "{\"defaults\":{\"warehouse\":\"s3://bucket/warehouse\"},\"overrides\":{\"clients\":\"5\"}}"; assertRoundTripSerializesEquallyFrom( fullJson, - RESTCatalogConfigResponse.builder() + ConfigResponse.builder() .withOverrides(OVERRIDES).withDefaults(DEFAULTS).build()); assertRoundTripSerializesEquallyFrom( fullJson, - RESTCatalogConfigResponse.builder() + ConfigResponse.builder() .withOverride("clients", "5").withDefault("warehouse", "s3://bucket/warehouse").build()); // `defaults` is empty String jsonEmptyDefaults = "{\"defaults\":{},\"overrides\":{\"clients\":\"5\"}}"; assertRoundTripSerializesEquallyFrom( jsonEmptyDefaults, - RESTCatalogConfigResponse.builder().withOverrides(OVERRIDES).build()); + ConfigResponse.builder().withOverrides(OVERRIDES).build()); assertRoundTripSerializesEquallyFrom( jsonEmptyDefaults, - RESTCatalogConfigResponse.builder().withOverrides(OVERRIDES).withDefaults(ImmutableMap.of()).build()); + ConfigResponse.builder().withOverrides(OVERRIDES).withDefaults(ImmutableMap.of()).build()); assertRoundTripSerializesEquallyFrom( jsonEmptyDefaults, - RESTCatalogConfigResponse.builder().withOverride("clients", "5").build()); + ConfigResponse.builder().withOverride("clients", "5").build()); // `overrides` is empty String jsonEmptyOverrides = "{\"defaults\":{\"warehouse\":\"s3://bucket/warehouse\"},\"overrides\":{}}"; assertRoundTripSerializesEquallyFrom( jsonEmptyOverrides, - RESTCatalogConfigResponse.builder().withDefaults(DEFAULTS).build()); + ConfigResponse.builder().withDefaults(DEFAULTS).build()); assertRoundTripSerializesEquallyFrom( jsonEmptyOverrides, - RESTCatalogConfigResponse.builder().withDefault("warehouse", "s3://bucket/warehouse").build()); + ConfigResponse.builder().withDefault("warehouse", "s3://bucket/warehouse").build()); assertRoundTripSerializesEquallyFrom( jsonEmptyOverrides, - RESTCatalogConfigResponse.builder().withDefaults(DEFAULTS).withOverrides(ImmutableMap.of()).build()); + ConfigResponse.builder().withDefaults(DEFAULTS).withOverrides(ImmutableMap.of()).build()); // Both are empty String emptyJson = "{\"defaults\":{},\"overrides\":{}}"; assertRoundTripSerializesEquallyFrom( emptyJson, - RESTCatalogConfigResponse.builder().build()); + ConfigResponse.builder().build()); assertRoundTripSerializesEquallyFrom( emptyJson, - RESTCatalogConfigResponse.builder().withOverrides(ImmutableMap.of()).withDefaults(ImmutableMap.of()).build()); + ConfigResponse.builder().withOverrides(ImmutableMap.of()).withDefaults(ImmutableMap.of()).build()); } @Test // Test cases that cannot be built with our builder, but that are accepted when parsed public void testCanDeserializeWithoutDefaultValues() throws JsonProcessingException { - RESTCatalogConfigResponse noOverrides = RESTCatalogConfigResponse.builder().withDefaults(DEFAULTS).build(); + ConfigResponse noOverrides = ConfigResponse.builder().withDefaults(DEFAULTS).build(); String jsonMissingOverrides = "{\"defaults\":{\"warehouse\":\"s3://bucket/warehouse\"}}"; assertEquals(deserialize(jsonMissingOverrides), noOverrides); String jsonNullOverrides = "{\"defaults\":{\"warehouse\":\"s3://bucket/warehouse\"},\"overrides\":null}"; assertEquals(deserialize(jsonNullOverrides), noOverrides); - RESTCatalogConfigResponse noDefaults = RESTCatalogConfigResponse.builder().withOverrides(OVERRIDES).build(); + ConfigResponse noDefaults = ConfigResponse.builder().withOverrides(OVERRIDES).build(); String jsonMissingDefaults = "{\"overrides\":{\"clients\":\"5\"}}"; assertEquals(deserialize(jsonMissingDefaults), noDefaults); String jsonNullDefaults = "{\"defaults\":null,\"overrides\":{\"clients\":\"5\"}}"; assertEquals(deserialize(jsonNullDefaults), noDefaults); - RESTCatalogConfigResponse noValues = RESTCatalogConfigResponse.builder().build(); + ConfigResponse noValues = ConfigResponse.builder().build(); String jsonEmptyObject = "{}"; assertEquals(deserialize(jsonEmptyObject), noValues); String jsonNullForAllFields = "{\"defaults\":null,\"overrides\":null}"; @@ -119,22 +119,22 @@ public void testCanUseNullAsPropertyValue() throws JsonProcessingException { "{\"defaults\":{\"warehouse\":null},\"overrides\":{\"clients\":\"5\"}}"; assertRoundTripSerializesEquallyFrom( jsonNullValueInDefaults, - RESTCatalogConfigResponse.builder() + ConfigResponse.builder() .withDefaults(DEFAULTS_WITH_NULL_VALUE).withOverrides(OVERRIDES).build()); assertRoundTripSerializesEquallyFrom( jsonNullValueInDefaults, - RESTCatalogConfigResponse.builder() + ConfigResponse.builder() .withDefault("warehouse", null).withOverrides(OVERRIDES).build()); String jsonNullValueInOverrides = "{\"defaults\":{\"warehouse\":\"s3://bucket/warehouse\"},\"overrides\":{\"clients\":null}}"; assertRoundTripSerializesEquallyFrom( jsonNullValueInOverrides, - RESTCatalogConfigResponse.builder() + ConfigResponse.builder() .withDefaults(DEFAULTS).withOverrides(OVERRIDES_WITH_NULL_VALUE).build()); assertRoundTripSerializesEquallyFrom( jsonNullValueInOverrides, - RESTCatalogConfigResponse.builder() + ConfigResponse.builder() .withDefaults(DEFAULTS).withOverride("clients", null).build()); } @@ -177,28 +177,28 @@ public void testBuilderDoesNotCreateInvalidObjects() { "The builder should not allow using null as a key in the properties to override", NullPointerException.class, "Invalid override property: null", - () -> RESTCatalogConfigResponse.builder().withOverride(null, "100").build() + () -> ConfigResponse.builder().withOverride(null, "100").build() ); AssertHelpers.assertThrows( "The builder should not allow using null as a key in the default properties", NullPointerException.class, "Invalid default property: null", - () -> RESTCatalogConfigResponse.builder().withDefault(null, "100").build() + () -> ConfigResponse.builder().withDefault(null, "100").build() ); AssertHelpers.assertThrows( "The builder should not allow passing a null map of config properties to override", NullPointerException.class, "Invalid override properties map: null", - () -> RESTCatalogConfigResponse.builder().withOverrides(null).build() + () -> ConfigResponse.builder().withOverrides(null).build() ); AssertHelpers.assertThrows( "The builder should not allow passing a null map of default config properties", NullPointerException.class, "Invalid default properties map: null", - () -> RESTCatalogConfigResponse.builder().withDefaults(null).build() + () -> ConfigResponse.builder().withDefaults(null).build() ); Map mapWithNullKey = Maps.newHashMap(); @@ -208,14 +208,14 @@ public void testBuilderDoesNotCreateInvalidObjects() { "The builder should not allow passing a map of default config properties with a null key", IllegalArgumentException.class, "Invalid default property: null", - () -> RESTCatalogConfigResponse.builder().withDefaults(mapWithNullKey).build() + () -> ConfigResponse.builder().withDefaults(mapWithNullKey).build() ); AssertHelpers.assertThrows( "The builder should not allow passing a map of properties to override with a null key", IllegalArgumentException.class, "Invalid override property: null", - () -> RESTCatalogConfigResponse.builder().withOverrides(mapWithNullKey).build() + () -> ConfigResponse.builder().withOverrides(mapWithNullKey).build() ); } @@ -229,7 +229,7 @@ public void testMergeStripsNullValuedEntries() { Map defaults = ImmutableMap.of("a", "from_defaults"); Map clientConfig = ImmutableMap.of("a", "from_client", "c", "from_client"); - RESTCatalogConfigResponse resp = RESTCatalogConfigResponse.builder() + ConfigResponse resp = ConfigResponse.builder() .withOverrides(overrides).withDefaults(defaults).build(); // "a" isn't present as it was marked as `null` in the overrides, so the provided client configuration is discarded @@ -251,15 +251,15 @@ public String[] allFieldsFromSpec() { } @Override - public RESTCatalogConfigResponse createExampleInstance() { - return RESTCatalogConfigResponse.builder() + public ConfigResponse createExampleInstance() { + return ConfigResponse.builder() .withDefaults(DEFAULTS) .withOverrides(OVERRIDES) .build(); } @Override - public void assertEquals(RESTCatalogConfigResponse actual, RESTCatalogConfigResponse expected) { + public void assertEquals(ConfigResponse actual, ConfigResponse expected) { Assert.assertEquals("Config properties to use as defaults should be equal", actual.defaults(), expected.defaults()); Assert.assertEquals("Config properties to use as overrides should be equal", @@ -267,8 +267,8 @@ public void assertEquals(RESTCatalogConfigResponse actual, RESTCatalogConfigResp } @Override - public RESTCatalogConfigResponse deserialize(String json) throws JsonProcessingException { - RESTCatalogConfigResponse resp = mapper().readValue(json, RESTCatalogConfigResponse.class); + public ConfigResponse deserialize(String json) throws JsonProcessingException { + ConfigResponse resp = mapper().readValue(json, ConfigResponse.class); resp.validate(); return resp; } diff --git a/open-api/rest-catalog-open-api.yaml b/open-api/rest-catalog-open-api.yaml index 8f431b45277c..3790a874f3a6 100644 --- a/open-api/rest-catalog-open-api.yaml +++ b/open-api/rest-catalog-open-api.yaml @@ -98,15 +98,13 @@ paths: content: application/json: schema: - $ref: '#/components/schemas/CatalogConfiguration' + $ref: '#/components/schemas/CatalogConfig' example: { - "data": { - "overrides": { - "warehouse": "s3://bucket/warehouse/" - }, - "defaults": { - "clients": "4" - } + "overrides": { + "warehouse": "s3://bucket/warehouse/" + }, + "defaults": { + "clients": "4" } } 400: @@ -757,7 +755,7 @@ components: items: type: string - CatalogConfiguration: + CatalogConfig: type: object description: Server-provided configuration for the catalog. required: