SebastianGruza commented on code in PR #3209: URL: https://github.com/apache/hugegraph/pull/3209#discussion_r4183133522
########## hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/graph/PropertiesDeserializer.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.hugegraph.api.graph; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; + +import com.fasterxml.jackson.core.JsonFactory; +import com.fasterxml.jackson.core.JsonParser; +import com.fasterxml.jackson.core.JsonToken; +import com.fasterxml.jackson.databind.DeserializationContext; +import com.fasterxml.jackson.databind.JsonDeserializer; +import com.fasterxml.jackson.databind.JsonMappingException; + +/** + * Reads the "properties" object of a vertex or edge body so that a JSON + * fraction keeps every digit: it becomes a BigDecimal instead of a double, + * which is what a DECIMAL property key needs and what every numeric key + * narrows through DataType.valueToNumber as before. Only property values + * are read this way; the rest of the request body (job parameters, schema + * userdata, query options) keeps Jackson's default number types. + */ +public class PropertiesDeserializer extends JsonDeserializer<Map<String, Object>> { + + private static final JsonFactory FACTORY = new JsonFactory(); + + @Override + public Map<String, Object> deserialize(JsonParser parser, + DeserializationContext ctxt) + throws IOException { + JsonToken token = parser.currentToken(); + if (token == JsonToken.VALUE_NULL) { + return null; + } + if (token != JsonToken.START_OBJECT) { + throw JsonMappingException.from(parser, + "Expected an object for 'properties', but got " + token); + } + return readObject(parser, true); + } + + /** + * The properties filter of the list APIs, read by the same rule as a + * request body: exact fractions for the values of the object and the + * members of a top-level array, Jackson's number types inside an OBJECT + * value, so a filter on an OBJECT key equals the stored map. + */ + public static Map<String, Object> parse(String json) throws IOException { + try (JsonParser parser = FACTORY.createParser(json)) { + if (parser.nextToken() != JsonToken.START_OBJECT) { + throw JsonMappingException.from(parser, + "Expected an object for 'properties', but got " + parser.currentToken()); + } + Map<String, Object> properties = readObject(parser, true); + if (parser.nextToken() != null) { + throw JsonMappingException.from(parser, "Trailing content after 'properties'"); + } + return properties; + } + } + + /** + * `exact` holds for the values of the properties object itself and the + * elements of a top-level array (a DECIMAL value or a DECIMAL LIST/SET); + * anything nested deeper is the content of an OBJECT property and keeps + * Jackson's default number types. + */ + private static Map<String, Object> readObject(JsonParser parser, boolean exact) + throws IOException { + Map<String, Object> object = new LinkedHashMap<>(); + while (parser.nextToken() != JsonToken.END_OBJECT) { + String name = parser.currentName(); + parser.nextToken(); + object.put(name, readValue(parser, exact)); + } + return object; + } + + private static List<Object> readArray(JsonParser parser, boolean exact) + throws IOException { + List<Object> array = new ArrayList<>(); + while (parser.nextToken() != JsonToken.END_ARRAY) { + array.add(readValue(parser, exact)); + } + return array; + } + + /** One value at the parser's current token: exact fractions, nested objects and arrays. */ + public static Object readValue(JsonParser parser) throws IOException { + return readValue(parser, true); + } + + private static Object readValue(JsonParser parser, boolean exact) throws IOException { + JsonToken token = parser.currentToken(); + switch (token) { + case START_OBJECT: + return readObject(parser, false); + case START_ARRAY: + // a top-level array is a LIST/SET value: its members are exact; + // an array inside an array or an object is OBJECT content + return readArray(parser, exact && !parser.getParsingContext().getParent().inArray()); + case VALUE_STRING: + return parser.getText(); + case VALUE_NUMBER_INT: + return parser.getNumberValue(); + case VALUE_NUMBER_FLOAT: + // Exact for a property value: the literal's digits, not the nearest double + return exact ? parser.getDecimalValue() : (Object) parser.getDoubleValue(); Review Comment: Right. The conversion to the key's type handles it: `PropertyKey.convValue()` passes an OBJECT value through `undoExact()` (BigDecimal to Double, recursively over collections), so an OBJECT scalar `0.25` and an OBJECT list member go back to the type the default parser produced and serialize as numbers, on the TextSerializer too. Same in the struct copy. Tests: `testObjectValuesKeepTheirNumberTypes` (core), a case in the struct `PropertyKeyTest`, and in `VertexApiTest` the round trip of an OBJECT scalar `0.25` and a list `[0.25,1]` through POST and GET. ########## hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/schema/PropertyKey.java: ########## @@ -368,6 +454,10 @@ private <V> V convSingleValue(V value) { @SuppressWarnings("unchecked") V blob = (V) this.dataType().valueToBlob(value); return blob; + } else if (this.dataType().isDecimal()) { + @SuppressWarnings("unchecked") + V decimal = (V) this.dataType().valueToDecimal(value); Review Comment: Right. A DECIMAL SET is distinct by value (`stripTrailingZeros()` as the key, the first form kept) in core and struct; LIST unchanged. Tests `testDecimalSetIsDistinctByValue` in both copies: `{1.0, 1.00, 2, 2.000, 1}` gives `{1.0, 2}`. ########## hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/type/define/DataType.java: ########## @@ -43,7 +45,14 @@ public enum DataType implements SerialEnum { TEXT(8, "text", String.class), BLOB(9, "blob", Blob.class), DATE(10, "date", Date.class), - UUID(11, "uuid", UUID.class); + UUID(11, "uuid", UUID.class), + /* + * Arbitrary-precision decimal (java.math.BigDecimal). Stored exactly; not a + * "number" in the isNumber() sense because it has no fixed-width, sortable + * encoding, so it can't be a sort key, a range/secondary index field or an + * OLAP range property. + */ + DECIMAL(12, "decimal", BigDecimal.class); Review Comment: Done: DECIMAL in the `data_type` description of `PropertyKeyAPI`. ########## hugegraph-store/hg-store-core/src/main/java/org/apache/hugegraph/store/business/GraphStoreIterator.java: ########## @@ -255,6 +257,12 @@ private <P extends BaseProperty<?>> List<Graphpb.Property> buildProperties( variant.setType(VariantType.VT_DOUBLE) .setValueDouble((Double) v); break; + case DECIMAL: Review Comment: Done. The encoding is factored into `decimalVariant()` / `decimalString()` (package-private) and a new `GraphStoreIteratorDecimalTest` in hg-store-test pins it: scalar `1.50` as VT_STRING `"1.50"`, 38 digits, no exponent (`1E-18`, `1E+3`), LIST/SET as `["1.5","2","1000"]`, an empty collection as `[]`. ########## hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/io/HugeGraphSONModule.java: ########## @@ -755,18 +760,14 @@ public void writePropertiesField(Collection<HugeProperty<?>> properties, String key = property.key(); Object val = property.value(); try { - generator.writeFieldName(key); - if (val != null) { - JsonSerializer<Object> serializer = - provider.findValueSerializer(val.getClass()); - serializer.serialize(val, generator, provider); - } else { - generator.writeNull(); - } + // The provider contextualizes the serializer: a bare + // findValueSerializer() returns a Map serializer without + // its key serializer, so an OBJECT map value failed here + provider.defaultSerializeField(key, val, generator); Review Comment: Right, thanks for the probe; I had not checked the typed mappers. The contextualized serializer without the wrapper: `findValueSerializer(val.getClass(), null)`, null through `writeNull()`. Test `testScalarPropertiesAreNotTypeWrappedInGraphSONV2`: a vertex with INT, DOUBLE and TEXT properties through V2 and V3 with HugeGraphIoRegistry gives `"age":29`, no `@type`; the OBJECT map case of the previous test still passes. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
