This is an automated email from the ASF dual-hosted git repository.
etudenhoefner pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/iceberg.git
The following commit(s) were added to refs/heads/main by this push:
new 00f46ac096 Core: Introduce ConfigResponseParser (#9952)
00f46ac096 is described below
commit 00f46ac0960237324e69401e98caa79c70407da5
Author: Eduard Tudenhoefner <[email protected]>
AuthorDate: Fri Apr 5 18:14:05 2024 +0200
Core: Introduce ConfigResponseParser (#9952)
---
.../org/apache/iceberg/rest/RESTSerializers.java | 22 +++-
.../rest/responses/ConfigResponseParser.java | 72 +++++++++++
.../java/org/apache/iceberg/util/JsonUtil.java | 20 +++
.../iceberg/rest/responses/TestConfigResponse.java | 9 +-
.../rest/responses/TestConfigResponseParser.java | 138 +++++++++++++++++++++
.../java/org/apache/iceberg/util/TestJsonUtil.java | 47 +++++++
6 files changed, 303 insertions(+), 5 deletions(-)
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 4311b9aa77..341dda0e3f 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.ReportMetricsRequest;
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 class RESTSerializers {
.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 class RESTSerializers {
return (T) LoadViewResponseParser.fromJson(jsonNode);
}
}
+
+ static class ConfigResponseSerializer<T extends ConfigResponse> extends
JsonSerializer<T> {
+ @Override
+ public void serialize(T request, JsonGenerator gen, SerializerProvider
serializers)
+ throws IOException {
+ ConfigResponseParser.toJson(request, gen);
+ }
+ }
+
+ static class ConfigResponseDeserializer<T extends ConfigResponse> extends
JsonDeserializer<T> {
+ @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 0000000000..3240840e3e
--- /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 aa90c63f80..2810ff5f23 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.base.Preconditions;
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 class JsonUtil {
return builder.build();
}
+ public static Map<String, String> 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<String, String> map = Maps.newHashMap();
+ Iterator<String> 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 298ebc3cf5..273fe48e2d 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 class TestConfigResponse extends
RequestResponseTestBase<ConfigResponse>
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<java.lang.String,java.lang.String>`");
+ "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 0000000000..ec4c793c27
--- /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<String, String> 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<String, String> 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<String, String> defaults = Maps.newHashMap();
+ defaults.put("key1", "1");
+ defaults.put("key2", null);
+
+ Map<String, String> 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 6b558599ac..f5d92129fb 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.Arrays;
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 class TestJsonUtil {
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<String, String> 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);
+ }
}
