diff --git a/core/src/main/java/org/apache/iceberg/rest/RESTSerializers.java b/core/src/main/java/org/apache/iceberg/rest/RESTSerializers.java index 4311b9aa7701..341dda0e3f6c 100644 --- a/core/src/main/java/org/apache/iceberg/rest/RESTSerializers.java +++ b/core/src/main/java/org/apache/iceberg/rest/RESTSerializers.java @@ -55,6 +55,8 @@ import org.apache.iceberg.rest.requests.ReportMetricsRequestParser; import org.apache.iceberg.rest.requests.UpdateTableRequest; import org.apache.iceberg.rest.requests.UpdateTableRequestParser; +import org.apache.iceberg.rest.responses.ConfigResponse; +import org.apache.iceberg.rest.responses.ConfigResponseParser; import org.apache.iceberg.rest.responses.ErrorResponse; import org.apache.iceberg.rest.responses.ErrorResponseParser; import org.apache.iceberg.rest.responses.ImmutableLoadViewResponse; @@ -111,7 +113,9 @@ public static void registerAll(ObjectMapper mapper) { .addSerializer(LoadViewResponse.class, new LoadViewResponseSerializer<>()) .addSerializer(ImmutableLoadViewResponse.class, new LoadViewResponseSerializer<>()) .addDeserializer(LoadViewResponse.class, new LoadViewResponseDeserializer<>()) - .addDeserializer(ImmutableLoadViewResponse.class, new LoadViewResponseDeserializer<>()); + .addDeserializer(ImmutableLoadViewResponse.class, new LoadViewResponseDeserializer<>()) + .addSerializer(ConfigResponse.class, new ConfigResponseSerializer<>()) + .addDeserializer(ConfigResponse.class, new ConfigResponseDeserializer<>()); mapper.registerModule(module); } @@ -402,4 +406,20 @@ public T deserialize(JsonParser p, DeserializationContext context) throws IOExce return (T) LoadViewResponseParser.fromJson(jsonNode); } } + + static class ConfigResponseSerializer extends JsonSerializer { + @Override + public void serialize(T request, JsonGenerator gen, SerializerProvider serializers) + throws IOException { + ConfigResponseParser.toJson(request, gen); + } + } + + static class ConfigResponseDeserializer extends JsonDeserializer { + @Override + public T deserialize(JsonParser p, DeserializationContext context) throws IOException { + JsonNode jsonNode = p.getCodec().readTree(p); + return (T) ConfigResponseParser.fromJson(jsonNode); + } + } } diff --git a/core/src/main/java/org/apache/iceberg/rest/responses/ConfigResponseParser.java b/core/src/main/java/org/apache/iceberg/rest/responses/ConfigResponseParser.java new file mode 100644 index 000000000000..3240840e3e93 --- /dev/null +++ b/core/src/main/java/org/apache/iceberg/rest/responses/ConfigResponseParser.java @@ -0,0 +1,72 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.iceberg.rest.responses; + +import com.fasterxml.jackson.core.JsonGenerator; +import com.fasterxml.jackson.databind.JsonNode; +import java.io.IOException; +import org.apache.iceberg.relocated.com.google.common.base.Preconditions; +import org.apache.iceberg.util.JsonUtil; + +public class ConfigResponseParser { + + private static final String DEFAULTS = "defaults"; + private static final String OVERRIDES = "overrides"; + + private ConfigResponseParser() {} + + public static String toJson(ConfigResponse response) { + return toJson(response, false); + } + + public static String toJson(ConfigResponse response, boolean pretty) { + return JsonUtil.generate(gen -> toJson(response, gen), pretty); + } + + public static void toJson(ConfigResponse response, JsonGenerator gen) throws IOException { + Preconditions.checkArgument(null != response, "Invalid config response: null"); + + gen.writeStartObject(); + + JsonUtil.writeStringMap(DEFAULTS, response.defaults(), gen); + JsonUtil.writeStringMap(OVERRIDES, response.overrides(), gen); + + gen.writeEndObject(); + } + + public static ConfigResponse fromJson(String json) { + return JsonUtil.parse(json, ConfigResponseParser::fromJson); + } + + public static ConfigResponse fromJson(JsonNode json) { + Preconditions.checkArgument(null != json, "Cannot parse config response from null object"); + + ConfigResponse.Builder builder = ConfigResponse.builder(); + + if (json.hasNonNull(DEFAULTS)) { + builder.withDefaults(JsonUtil.getStringMapNullableValues(DEFAULTS, json)); + } + + if (json.hasNonNull(OVERRIDES)) { + builder.withOverrides(JsonUtil.getStringMapNullableValues(OVERRIDES, json)); + } + + return builder.build(); + } +} diff --git a/core/src/main/java/org/apache/iceberg/util/JsonUtil.java b/core/src/main/java/org/apache/iceberg/util/JsonUtil.java index aa90c63f80da..2810ff5f23c0 100644 --- a/core/src/main/java/org/apache/iceberg/util/JsonUtil.java +++ b/core/src/main/java/org/apache/iceberg/util/JsonUtil.java @@ -36,6 +36,7 @@ import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableSet; +import org.apache.iceberg.relocated.com.google.common.collect.Maps; import org.apache.iceberg.relocated.com.google.common.io.BaseEncoding; public class JsonUtil { @@ -206,6 +207,25 @@ public static Map getStringMap(String property, JsonNode node) { return builder.build(); } + public static Map getStringMapNullableValues(String property, JsonNode node) { + Preconditions.checkArgument(node.has(property), "Cannot parse missing map: %s", property); + JsonNode pNode = node.get(property); + Preconditions.checkArgument( + pNode != null && !pNode.isNull() && pNode.isObject(), + "Cannot parse string map from non-object value: %s: %s", + property, + pNode); + + Map map = Maps.newHashMap(); + Iterator fields = pNode.fieldNames(); + while (fields.hasNext()) { + String field = fields.next(); + map.put(field, getStringOrNull(field, pNode)); + } + + return map; + } + public static String[] getStringArray(JsonNode node) { Preconditions.checkArgument( node != null && !node.isNull() && node.isArray(), diff --git a/core/src/test/java/org/apache/iceberg/rest/responses/TestConfigResponse.java b/core/src/test/java/org/apache/iceberg/rest/responses/TestConfigResponse.java index 298ebc3cf5bb..273fe48e2dcb 100644 --- a/core/src/test/java/org/apache/iceberg/rest/responses/TestConfigResponse.java +++ b/core/src/test/java/org/apache/iceberg/rest/responses/TestConfigResponse.java @@ -145,15 +145,16 @@ public void testDeserializeInvalidResponse() { String jsonDefaultsHasWrongType = "{\"defaults\":[\"warehouse\",\"s3://bucket/warehouse\"],\"overrides\":{\"clients\":\"5\"}}"; Assertions.assertThatThrownBy(() -> deserialize(jsonDefaultsHasWrongType)) - .isInstanceOf(JsonProcessingException.class) + .isInstanceOf(IllegalArgumentException.class) .hasMessageContaining( - "Cannot deserialize value of type `java.util.LinkedHashMap`"); + "Cannot parse string map from non-object value: defaults: [\"warehouse\",\"s3://bucket/warehouse\"]"); String jsonOverridesHasWrongType = "{\"defaults\":{\"warehouse\":\"s3://bucket/warehouse\"},\"overrides\":\"clients\"}"; Assertions.assertThatThrownBy(() -> deserialize(jsonOverridesHasWrongType)) - .isInstanceOf(JsonProcessingException.class) - .hasMessageContaining("Cannot construct instance of `java.util.LinkedHashMap`"); + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining( + "Cannot parse string map from non-object value: overrides: \"clients\""); Assertions.assertThatThrownBy(() -> deserialize(null)) .isInstanceOf(IllegalArgumentException.class) diff --git a/core/src/test/java/org/apache/iceberg/rest/responses/TestConfigResponseParser.java b/core/src/test/java/org/apache/iceberg/rest/responses/TestConfigResponseParser.java new file mode 100644 index 000000000000..ec4c793c279f --- /dev/null +++ b/core/src/test/java/org/apache/iceberg/rest/responses/TestConfigResponseParser.java @@ -0,0 +1,138 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.iceberg.rest.responses; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import com.fasterxml.jackson.databind.JsonNode; +import java.util.Map; +import org.apache.iceberg.relocated.com.google.common.collect.Maps; +import org.junit.jupiter.api.Test; + +public class TestConfigResponseParser { + + @Test + public void nullAndEmptyCheck() { + assertThatThrownBy(() -> ConfigResponseParser.toJson(null)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Invalid config response: null"); + + assertThatThrownBy(() -> ConfigResponseParser.fromJson((JsonNode) null)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot parse config response from null object"); + + ConfigResponse actual = ConfigResponseParser.fromJson("{}"); + ConfigResponse expected = ConfigResponse.builder().build(); + // ConfigResponse doesn't implement hashCode/equals + assertThat(actual.defaults()).isEqualTo(expected.defaults()).isEmpty(); + assertThat(actual.overrides()).isEqualTo(expected.overrides()).isEmpty(); + } + + @Test + public void unknownFields() { + ConfigResponse actual = ConfigResponseParser.fromJson("{\"x\": \"val\", \"y\": \"val2\"}"); + ConfigResponse expected = ConfigResponse.builder().build(); + // ConfigResponse doesn't implement hashCode/equals + assertThat(actual.defaults()).isEqualTo(expected.defaults()).isEmpty(); + assertThat(actual.overrides()).isEqualTo(expected.overrides()).isEmpty(); + } + + @Test + public void defaultsOnly() { + Map defaults = Maps.newHashMap(); + defaults.put("a", "1"); + defaults.put("b", null); + defaults.put("c", "2"); + defaults.put("d", null); + + ConfigResponse response = ConfigResponse.builder().withDefaults(defaults).build(); + String expectedJson = + "{\n" + + " \"defaults\" : {\n" + + " \"a\" : \"1\",\n" + + " \"b\" : null,\n" + + " \"c\" : \"2\",\n" + + " \"d\" : null\n" + + " },\n" + + " \"overrides\" : { }\n" + + "}"; + + String json = ConfigResponseParser.toJson(response, true); + assertThat(json).isEqualTo(expectedJson); + assertThat(ConfigResponseParser.toJson(ConfigResponseParser.fromJson(json), true)) + .isEqualTo(expectedJson); + } + + @Test + public void overridesOnly() { + Map overrides = Maps.newHashMap(); + overrides.put("a", "1"); + overrides.put("b", null); + overrides.put("c", "2"); + overrides.put("d", null); + + ConfigResponse response = ConfigResponse.builder().withOverrides(overrides).build(); + String expectedJson = + "{\n" + + " \"defaults\" : { },\n" + + " \"overrides\" : {\n" + + " \"a\" : \"1\",\n" + + " \"b\" : null,\n" + + " \"c\" : \"2\",\n" + + " \"d\" : null\n" + + " }\n" + + "}"; + + String json = ConfigResponseParser.toJson(response, true); + assertThat(json).isEqualTo(expectedJson); + assertThat(ConfigResponseParser.toJson(ConfigResponseParser.fromJson(json), true)) + .isEqualTo(expectedJson); + } + + @Test + public void roundTripSerde() { + Map defaults = Maps.newHashMap(); + defaults.put("key1", "1"); + defaults.put("key2", null); + + Map overrides = Maps.newHashMap(); + overrides.put("key3", "23"); + overrides.put("key4", null); + + ConfigResponse response = + ConfigResponse.builder().withDefaults(defaults).withOverrides(overrides).build(); + String expectedJson = + "{\n" + + " \"defaults\" : {\n" + + " \"key1\" : \"1\",\n" + + " \"key2\" : null\n" + + " },\n" + + " \"overrides\" : {\n" + + " \"key3\" : \"23\",\n" + + " \"key4\" : null\n" + + " }\n" + + "}"; + + String json = ConfigResponseParser.toJson(response, true); + assertThat(json).isEqualTo(expectedJson); + assertThat(ConfigResponseParser.toJson(ConfigResponseParser.fromJson(json), true)) + .isEqualTo(expectedJson); + } +} diff --git a/core/src/test/java/org/apache/iceberg/util/TestJsonUtil.java b/core/src/test/java/org/apache/iceberg/util/TestJsonUtil.java index 6b558599ac46..f5d92129fb3d 100644 --- a/core/src/test/java/org/apache/iceberg/util/TestJsonUtil.java +++ b/core/src/test/java/org/apache/iceberg/util/TestJsonUtil.java @@ -24,6 +24,7 @@ import java.util.List; import java.util.Map; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; +import org.apache.iceberg.relocated.com.google.common.collect.Maps; import org.apache.iceberg.relocated.com.google.common.io.BaseEncoding; import org.assertj.core.api.Assertions; import org.junit.jupiter.api.Test; @@ -543,4 +544,50 @@ public void getStringMap() throws JsonProcessingException { Assertions.assertThat(JsonUtil.getStringMap("items", JsonUtil.mapper().readTree(json))) .isEqualTo(items); } + + @Test + public void getStringMapNullableValues() throws JsonProcessingException { + Assertions.assertThatThrownBy( + () -> JsonUtil.getStringMapNullableValues("items", JsonUtil.mapper().readTree("{}"))) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot parse missing map: items"); + + Assertions.assertThatThrownBy( + () -> + JsonUtil.getStringMapNullableValues( + "items", JsonUtil.mapper().readTree("{\"items\": null}"))) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot parse string map from non-object value: items: null"); + + Assertions.assertThatThrownBy( + () -> + JsonUtil.getStringMapNullableValues( + "items", JsonUtil.mapper().readTree("{\"items\": {\"a\":\"23\", \"b\":45}}"))) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot parse to a string value: b: 45"); + + Map itemsWithNullableValues = Maps.newHashMap(); + itemsWithNullableValues.put("a", null); + itemsWithNullableValues.put("b", null); + itemsWithNullableValues.put("c", "23"); + Assertions.assertThat( + JsonUtil.getStringMapNullableValues( + "items", + JsonUtil.mapper() + .readTree("{\"items\": {\"a\": null, \"b\": null, \"c\": \"23\"}}"))) + .isEqualTo(itemsWithNullableValues); + + String json = + JsonUtil.generate( + gen -> { + gen.writeStartObject(); + JsonUtil.writeStringMap("items", itemsWithNullableValues, gen); + gen.writeEndObject(); + }, + false); + + Assertions.assertThat( + JsonUtil.getStringMapNullableValues("items", JsonUtil.mapper().readTree(json))) + .isEqualTo(itemsWithNullableValues); + } }