You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@johnzon.apache.org by "rmannibucau (via GitHub)" <gi...@apache.org> on 2023/04/17 07:40:40 UTC
[GitHub] [johnzon] rmannibucau commented on a diff in pull request #100: Implement JSON-B 3 Polymorphism
rmannibucau commented on code in PR #100:
URL: https://github.com/apache/johnzon/pull/100#discussion_r1168282944
##########
johnzon-jsonb/src/main/java/org/apache/johnzon/jsonb/JsonbPolymorphismHandler.java:
##########
@@ -0,0 +1,155 @@
+/*
+ * 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.johnzon.jsonb;
+
+import org.apache.johnzon.mapper.access.Meta;
+
+import jakarta.json.JsonObject;
+import jakarta.json.JsonString;
+import jakarta.json.JsonValue;
+import jakarta.json.bind.JsonbException;
+import jakarta.json.bind.annotation.JsonbSubtype;
+import jakarta.json.bind.annotation.JsonbTypeInfo;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+
+public class JsonbPolymorphismHandler {
+ public boolean hasPolymorphism(Class<?> clazz) {
+ return clazz.isAnnotationPresent(JsonbTypeInfo.class) || !getParentClassesWithTypeInfo(clazz).isEmpty();
+ }
+
+ public List<Map.Entry<String, String>> getPolymorphismPropertiesToSerialize(Class<?> clazz, Collection<String> otherProperties) {
Review Comment:
looks like the returned type should be an array of pairs (or `Map.Entry` this part is a detail) instead of a list to make the iteration faster at runtime, wdyt?
##########
johnzon-mapper/src/main/java/org/apache/johnzon/mapper/access/Meta.java:
##########
@@ -78,21 +78,28 @@ private static <T extends Annotation> T getIndirectAnnotation(final Class<T> api
return null;
}
+ public static <T extends Annotation> T getDirectAnnotation(final Class<?> clazz, final Class<T> api) {
Review Comment:
not sure why there is a diff in this class, it is already on master and should stay private while the usage should be `getAnnotation` in all the rest of the diff :thinking:
##########
johnzon-jsonb/src/main/java/org/apache/johnzon/jsonb/JohnzonBuilder.java:
##########
@@ -31,6 +31,7 @@
import org.apache.johnzon.mapper.Mapper;
Review Comment:
guess except the setMappingsFactory the rest of the diff is no more needed?
##########
johnzon-jsonb/src/main/java/org/apache/johnzon/jsonb/JsonbPolymorphismHandler.java:
##########
@@ -0,0 +1,155 @@
+/*
+ * 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.johnzon.jsonb;
+
+import org.apache.johnzon.mapper.access.Meta;
+
+import jakarta.json.JsonObject;
+import jakarta.json.JsonString;
+import jakarta.json.JsonValue;
+import jakarta.json.bind.JsonbException;
+import jakarta.json.bind.annotation.JsonbSubtype;
+import jakarta.json.bind.annotation.JsonbTypeInfo;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+
+public class JsonbPolymorphismHandler {
+ public boolean hasPolymorphism(Class<?> clazz) {
+ return clazz.isAnnotationPresent(JsonbTypeInfo.class) || !getParentClassesWithTypeInfo(clazz).isEmpty();
+ }
+
+ public List<Map.Entry<String, String>> getPolymorphismPropertiesToSerialize(Class<?> clazz, Collection<String> otherProperties) {
+ List<Map.Entry<String, String>> entries = new ArrayList<>();
Review Comment:
why not a plain `LinkedHashMap` which will enable to simplify the "found more than once" check and keep the ordering (will just need to `sorted(reverse)` when converting to an array), wdyt? Overall idea is to make the intent to provide a partial object (generator usage) more explicit.
##########
johnzon-jsonb/src/main/java/org/apache/johnzon/jsonb/JsonbPolymorphismHandler.java:
##########
@@ -0,0 +1,155 @@
+/*
+ * 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.johnzon.jsonb;
+
+import org.apache.johnzon.mapper.access.Meta;
+
+import jakarta.json.JsonObject;
+import jakarta.json.JsonString;
+import jakarta.json.JsonValue;
+import jakarta.json.bind.JsonbException;
+import jakarta.json.bind.annotation.JsonbSubtype;
+import jakarta.json.bind.annotation.JsonbTypeInfo;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+
+public class JsonbPolymorphismHandler {
+ public boolean hasPolymorphism(Class<?> clazz) {
+ return clazz.isAnnotationPresent(JsonbTypeInfo.class) || !getParentClassesWithTypeInfo(clazz).isEmpty();
+ }
+
+ public List<Map.Entry<String, String>> getPolymorphismPropertiesToSerialize(Class<?> clazz, Collection<String> otherProperties) {
+ List<Map.Entry<String, String>> entries = new ArrayList<>();
+
+ Class<?> current = clazz;
+ while (current != null) {
+ validateJsonbTypeInfo(current);
+ validateOnlyOneParentWithTypeInfo(current);
+
+ JsonbTypeInfo typeInfo = Meta.getDirectAnnotation(current, JsonbTypeInfo.class);
+ if (typeInfo != null) {
+ if (otherProperties.contains(typeInfo.key())) {
+ throw new JsonbException("JsonbTypeInfo key '" + typeInfo.key() + "' collides with other properties in json");
+ }
+
+ if (entries.stream().anyMatch(entry -> Objects.equals(entry.getKey(), typeInfo.key()))) {
+ throw new JsonbException("JsonbTypeInfo key '" + typeInfo.key() + "' found more than once in type hierarchy of " + clazz.getName());
+ }
+
+ String bestMatchingAlias = null;
+ for (JsonbSubtype subtype : typeInfo.value()) {
+ if (subtype.type().isAssignableFrom(clazz)) {
+ bestMatchingAlias = subtype.alias();
+
+ if (clazz == subtype.type()) { // Exact match found, no need to continue further
+ break;
+ }
+ }
+ }
+
+ if (bestMatchingAlias != null) {
+ entries.add(0, Map.entry(typeInfo.key(), bestMatchingAlias));
+ }
+ }
+
+ List<Class<?>> parentClassesWithTypeInfo = getParentClassesWithTypeInfo(current);
+ current = parentClassesWithTypeInfo.isEmpty() ? null : parentClassesWithTypeInfo.get(0);
+ }
+
+ return entries;
+ }
+
+ public Class<?> getTypeToDeserialize(JsonObject jsonObject, Class<?> clazz) {
+ validateJsonbTypeInfo(clazz);
+ validateOnlyOneParentWithTypeInfo(clazz);
+
+ JsonbTypeInfo typeInfo = Meta.getDirectAnnotation(clazz, JsonbTypeInfo.class);
+ if (typeInfo == null || !jsonObject.containsKey(typeInfo.key())) {
+ return clazz;
+ }
+
+ JsonValue typeValue = jsonObject.get(typeInfo.key());
+ if (!(typeValue instanceof JsonString)) {
+ throw new JsonbException("Property '" + typeInfo.key() + "' isn't a String, resolving "
+ + "JsonbSubtype is impossible");
+ }
+
+ String typeValueString = ((JsonString) typeValue).getString();
+ for (JsonbSubtype subtype : typeInfo.value()) {
+ if (subtype.alias().equals(typeValueString)) {
+ return subtype.type();
+ }
+ }
+
+ throw new JsonbException("No JsonbSubtype found for alias '" + typeValueString + "' on " + clazz.getName());
+ }
+
+ /**
+ * Validates that only one parent class (superclass + interfaces) has {@link JsonbTypeInfo} annotation
+ *
+ * @param classToValidate class to validate
+ * @throws JsonbException validation failed
+ */
+ private void validateOnlyOneParentWithTypeInfo(Class<?> classToValidate) {
+ if (getParentClassesWithTypeInfo(classToValidate).size() > 1) {
Review Comment:
this looks called too often (at least twice per type if I'm not mistaken, maybe reuse the same instance for both needs?)
##########
johnzon-mapper/src/test/java/org/superbiz/ExtendMappingTest.java:
##########
@@ -62,9 +62,9 @@ public MyMappings() {
-1, true, true, true, false, false, false,
new FieldAccessMode(false, false),
StandardCharsets.UTF_8, String::compareTo, false, null, false, false,
- emptyMap(), true, false, true,
- null, null, null, null, null,
- type -> new EnumConverter(type)));
+ emptyMap(), true, false, true, null,
Review Comment:
see previous comment but overall this test is a contract so any diff is likely a bug
##########
johnzon-mapper/src/main/java/org/apache/johnzon/mapper/MapperConfig.java:
##########
@@ -98,6 +98,8 @@ public Object fromJson(JsonValue jsonObject, Type targetType, MappingParser pars
private final SnippetFactory snippet;
+ private final Function<MapperConfig, Mappings> mappingsFactory;
+
//CHECKSTYLE:OFF
@Deprecated
public MapperConfig(final LazyConverterMap adapters,
Review Comment:
please keep the compatibilty, we guarantee that, this is illustrated by `ExtendMappingTest` test (you broke this contract modifying the test). Same trick than for mappings is more than sufficient to me.
##########
johnzon-mapper/src/main/java/org/apache/johnzon/mapper/MappingGeneratorImpl.java:
##########
@@ -179,23 +179,26 @@ public void doWriteObject(Object object, JsonGenerator generator, boolean writeB
}
} else {
if (classMapping == null) { // will be created anyway now so force it and if it has an adapter respect it
- final Mappings.ClassMapping mapping = mappings.findOrCreateClassMapping(objectClass);
- if (mapping != null && mapping.adapter != null) {
- final Object result = mapping.adapter.from(object);
- doWriteObject(result, generator, writeBody, ignoredProperties, jsonPointer);
- return;
- }
+ classMapping = mappings.findOrCreateClassMapping(objectClass);
+ }
+
+ if (classMapping.adapter != null) {
+ final Object result = classMapping.adapter.from(object);
+ doWriteObject(result, generator, writeBody, ignoredProperties, jsonPointer);
+ return;
}
+
if (writeBody) {
generator.writeStartObject();
}
- final boolean writeEnd;
- if (config.getSerializationPredicate() != null && config.getSerializationPredicate().test(objectClass)) {
Review Comment:
:+1:
##########
johnzon-jsonb/src/main/java/org/apache/johnzon/jsonb/JsonbPolymorphismHandler.java:
##########
@@ -0,0 +1,155 @@
+/*
+ * 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.johnzon.jsonb;
+
+import org.apache.johnzon.mapper.access.Meta;
+
+import jakarta.json.JsonObject;
+import jakarta.json.JsonString;
+import jakarta.json.JsonValue;
+import jakarta.json.bind.JsonbException;
+import jakarta.json.bind.annotation.JsonbSubtype;
+import jakarta.json.bind.annotation.JsonbTypeInfo;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+
+public class JsonbPolymorphismHandler {
+ public boolean hasPolymorphism(Class<?> clazz) {
+ return clazz.isAnnotationPresent(JsonbTypeInfo.class) || !getParentClassesWithTypeInfo(clazz).isEmpty();
+ }
+
+ public List<Map.Entry<String, String>> getPolymorphismPropertiesToSerialize(Class<?> clazz, Collection<String> otherProperties) {
+ List<Map.Entry<String, String>> entries = new ArrayList<>();
+
+ Class<?> current = clazz;
+ while (current != null) {
+ validateJsonbTypeInfo(current);
+ validateOnlyOneParentWithTypeInfo(current);
+
+ JsonbTypeInfo typeInfo = Meta.getDirectAnnotation(current, JsonbTypeInfo.class);
+ if (typeInfo != null) {
+ if (otherProperties.contains(typeInfo.key())) {
+ throw new JsonbException("JsonbTypeInfo key '" + typeInfo.key() + "' collides with other properties in json");
+ }
+
+ if (entries.stream().anyMatch(entry -> Objects.equals(entry.getKey(), typeInfo.key()))) {
+ throw new JsonbException("JsonbTypeInfo key '" + typeInfo.key() + "' found more than once in type hierarchy of " + clazz.getName());
+ }
+
+ String bestMatchingAlias = null;
+ for (JsonbSubtype subtype : typeInfo.value()) {
+ if (subtype.type().isAssignableFrom(clazz)) {
+ bestMatchingAlias = subtype.alias();
+
+ if (clazz == subtype.type()) { // Exact match found, no need to continue further
+ break;
+ }
+ }
+ }
+
+ if (bestMatchingAlias != null) {
+ entries.add(0, Map.entry(typeInfo.key(), bestMatchingAlias));
+ }
+ }
+
+ List<Class<?>> parentClassesWithTypeInfo = getParentClassesWithTypeInfo(current);
+ current = parentClassesWithTypeInfo.isEmpty() ? null : parentClassesWithTypeInfo.get(0);
+ }
+
+ return entries;
+ }
+
+ public Class<?> getTypeToDeserialize(JsonObject jsonObject, Class<?> clazz) {
+ validateJsonbTypeInfo(clazz);
Review Comment:
this should be done one per class somehow since it is a runtime class (compared to getPolymorphismPropertiesToSerialize which is a classmapping init computation only) so maybe use ClassMapping to do the validation only one, can likely become a `Supplier<Class<?>>` using the supplier "pre" part to do the validation and `supplier#get` to impl the runtime only (current line 91+).
##########
src/site/markdown/index.md:
##########
@@ -497,57 +497,6 @@ in `MessageDecoder`.
}
-
-### JSON-B Extra
Review Comment:
maybe keep it and explain:
1. very high level what is in the spec
2. how to migrate from extras module to the standard impl
##########
johnzon-jsonb/src/main/java/org/apache/johnzon/jsonb/JsonbPolymorphismHandler.java:
##########
@@ -0,0 +1,155 @@
+/*
+ * 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.johnzon.jsonb;
+
+import org.apache.johnzon.mapper.access.Meta;
+
+import jakarta.json.JsonObject;
+import jakarta.json.JsonString;
+import jakarta.json.JsonValue;
+import jakarta.json.bind.JsonbException;
+import jakarta.json.bind.annotation.JsonbSubtype;
+import jakarta.json.bind.annotation.JsonbTypeInfo;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+
+public class JsonbPolymorphismHandler {
+ public boolean hasPolymorphism(Class<?> clazz) {
+ return clazz.isAnnotationPresent(JsonbTypeInfo.class) || !getParentClassesWithTypeInfo(clazz).isEmpty();
+ }
+
+ public List<Map.Entry<String, String>> getPolymorphismPropertiesToSerialize(Class<?> clazz, Collection<String> otherProperties) {
+ List<Map.Entry<String, String>> entries = new ArrayList<>();
+
+ Class<?> current = clazz;
+ while (current != null) {
+ validateJsonbTypeInfo(current);
+ validateOnlyOneParentWithTypeInfo(current);
+
+ JsonbTypeInfo typeInfo = Meta.getDirectAnnotation(current, JsonbTypeInfo.class);
+ if (typeInfo != null) {
+ if (otherProperties.contains(typeInfo.key())) {
+ throw new JsonbException("JsonbTypeInfo key '" + typeInfo.key() + "' collides with other properties in json");
+ }
+
+ if (entries.stream().anyMatch(entry -> Objects.equals(entry.getKey(), typeInfo.key()))) {
+ throw new JsonbException("JsonbTypeInfo key '" + typeInfo.key() + "' found more than once in type hierarchy of " + clazz.getName());
+ }
+
+ String bestMatchingAlias = null;
+ for (JsonbSubtype subtype : typeInfo.value()) {
+ if (subtype.type().isAssignableFrom(clazz)) {
+ bestMatchingAlias = subtype.alias();
+
+ if (clazz == subtype.type()) { // Exact match found, no need to continue further
+ break;
+ }
+ }
+ }
+
+ if (bestMatchingAlias != null) {
+ entries.add(0, Map.entry(typeInfo.key(), bestMatchingAlias));
+ }
+ }
+
+ List<Class<?>> parentClassesWithTypeInfo = getParentClassesWithTypeInfo(current);
+ current = parentClassesWithTypeInfo.isEmpty() ? null : parentClassesWithTypeInfo.get(0);
+ }
+
+ return entries;
+ }
+
+ public Class<?> getTypeToDeserialize(JsonObject jsonObject, Class<?> clazz) {
+ validateJsonbTypeInfo(clazz);
+ validateOnlyOneParentWithTypeInfo(clazz);
+
+ JsonbTypeInfo typeInfo = Meta.getDirectAnnotation(clazz, JsonbTypeInfo.class);
+ if (typeInfo == null || !jsonObject.containsKey(typeInfo.key())) {
+ return clazz;
+ }
+
+ JsonValue typeValue = jsonObject.get(typeInfo.key());
+ if (!(typeValue instanceof JsonString)) {
Review Comment:
`typeValue != null && typeValue.getValueType() != STRING` is faster due to JVM `instanceof` impl (to avoid when possible)
##########
johnzon-jsonb/src/main/java/org/apache/johnzon/jsonb/JsonbPolymorphismHandler.java:
##########
@@ -0,0 +1,155 @@
+/*
+ * 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.johnzon.jsonb;
+
+import org.apache.johnzon.mapper.access.Meta;
+
+import jakarta.json.JsonObject;
+import jakarta.json.JsonString;
+import jakarta.json.JsonValue;
+import jakarta.json.bind.JsonbException;
+import jakarta.json.bind.annotation.JsonbSubtype;
+import jakarta.json.bind.annotation.JsonbTypeInfo;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+
+public class JsonbPolymorphismHandler {
+ public boolean hasPolymorphism(Class<?> clazz) {
+ return clazz.isAnnotationPresent(JsonbTypeInfo.class) || !getParentClassesWithTypeInfo(clazz).isEmpty();
+ }
+
+ public List<Map.Entry<String, String>> getPolymorphismPropertiesToSerialize(Class<?> clazz, Collection<String> otherProperties) {
+ List<Map.Entry<String, String>> entries = new ArrayList<>();
+
+ Class<?> current = clazz;
+ while (current != null) {
+ validateJsonbTypeInfo(current);
+ validateOnlyOneParentWithTypeInfo(current);
+
+ JsonbTypeInfo typeInfo = Meta.getDirectAnnotation(current, JsonbTypeInfo.class);
+ if (typeInfo != null) {
+ if (otherProperties.contains(typeInfo.key())) {
+ throw new JsonbException("JsonbTypeInfo key '" + typeInfo.key() + "' collides with other properties in json");
+ }
+
+ if (entries.stream().anyMatch(entry -> Objects.equals(entry.getKey(), typeInfo.key()))) {
+ throw new JsonbException("JsonbTypeInfo key '" + typeInfo.key() + "' found more than once in type hierarchy of " + clazz.getName());
+ }
+
+ String bestMatchingAlias = null;
+ for (JsonbSubtype subtype : typeInfo.value()) {
+ if (subtype.type().isAssignableFrom(clazz)) {
+ bestMatchingAlias = subtype.alias();
+
+ if (clazz == subtype.type()) { // Exact match found, no need to continue further
+ break;
+ }
+ }
+ }
+
+ if (bestMatchingAlias != null) {
+ entries.add(0, Map.entry(typeInfo.key(), bestMatchingAlias));
+ }
+ }
+
+ List<Class<?>> parentClassesWithTypeInfo = getParentClassesWithTypeInfo(current);
+ current = parentClassesWithTypeInfo.isEmpty() ? null : parentClassesWithTypeInfo.get(0);
+ }
+
+ return entries;
+ }
+
+ public Class<?> getTypeToDeserialize(JsonObject jsonObject, Class<?> clazz) {
+ validateJsonbTypeInfo(clazz);
+ validateOnlyOneParentWithTypeInfo(clazz);
+
+ JsonbTypeInfo typeInfo = Meta.getDirectAnnotation(clazz, JsonbTypeInfo.class);
+ if (typeInfo == null || !jsonObject.containsKey(typeInfo.key())) {
+ return clazz;
+ }
+
+ JsonValue typeValue = jsonObject.get(typeInfo.key());
+ if (!(typeValue instanceof JsonString)) {
+ throw new JsonbException("Property '" + typeInfo.key() + "' isn't a String, resolving "
+ + "JsonbSubtype is impossible");
+ }
+
+ String typeValueString = ((JsonString) typeValue).getString();
+ for (JsonbSubtype subtype : typeInfo.value()) {
+ if (subtype.alias().equals(typeValueString)) {
+ return subtype.type();
+ }
+ }
+
+ throw new JsonbException("No JsonbSubtype found for alias '" + typeValueString + "' on " + clazz.getName());
+ }
+
+ /**
+ * Validates that only one parent class (superclass + interfaces) has {@link JsonbTypeInfo} annotation
+ *
+ * @param classToValidate class to validate
+ * @throws JsonbException validation failed
+ */
+ private void validateOnlyOneParentWithTypeInfo(Class<?> classToValidate) {
+ if (getParentClassesWithTypeInfo(classToValidate).size() > 1) {
+ throw new JsonbException("More than one interface/superclass of " + classToValidate.getName() +
+ " has JsonbTypeInfo Annotation");
+ }
+ }
+
+ /**
+ * Validates {@link JsonbTypeInfo} on clazz.
+ * Validation fails either if any {@link JsonbSubtype#type()} is the same as clazz
+ * or if any clazz and {@link JsonbSubtype#type()} aren't compatible.
+ *
+ * @param classToValidate Class to validate
+ * @throws JsonbException validation failed
+ */
+ private void validateJsonbTypeInfo(Class<?> classToValidate) {
+ if (!classToValidate.isAnnotationPresent(JsonbTypeInfo.class)) {
+ return;
+ }
+
+ JsonbTypeInfo typeInfo = Meta.getDirectAnnotation(classToValidate, JsonbTypeInfo.class);
+ for (JsonbSubtype subtype : typeInfo.value()) {
+ if (!classToValidate.isAssignableFrom(subtype.type())) {
+ throw new JsonbException("JsonbSubtype '" + subtype.alias() + "'" +
+ " (" + subtype.type().getName() + ") is not a subclass of " + classToValidate);
+ }
+ }
+ }
+
+ /**
+ * Collects all parent classes (superclass + interfaces) that have the JsonbTypeInfo annotation
+ * @param clazz base class
+ * @return List of classes with JsonbTypeInfo annotation
+ */
+ private List<Class<?>> getParentClassesWithTypeInfo(Class<?> clazz) {
Review Comment:
wonder if the following pattern isnt more relevant (pseudo code):
```
final var superClass = clazz.getSuperclass();
if (superClass != null && hasTypeInfo(superClass)) {
return superClass;
}
return Stream.of(clazz.getInterfaces()).filter(it -> hasTypeInfo(it)).findFirst().orElse(null);
```
Overall idea is to avoid to create a list to keep only the first element. For interface case it will not change a lot but for parent case it will make it way more efficient. wdyt?
##########
johnzon-jsonb/src/main/java/org/apache/johnzon/jsonb/JsonbPolymorphismHandler.java:
##########
@@ -0,0 +1,155 @@
+/*
+ * 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.johnzon.jsonb;
+
+import org.apache.johnzon.mapper.access.Meta;
+
+import jakarta.json.JsonObject;
+import jakarta.json.JsonString;
+import jakarta.json.JsonValue;
+import jakarta.json.bind.JsonbException;
+import jakarta.json.bind.annotation.JsonbSubtype;
+import jakarta.json.bind.annotation.JsonbTypeInfo;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+
+public class JsonbPolymorphismHandler {
+ public boolean hasPolymorphism(Class<?> clazz) {
+ return clazz.isAnnotationPresent(JsonbTypeInfo.class) || !getParentClassesWithTypeInfo(clazz).isEmpty();
+ }
+
+ public List<Map.Entry<String, String>> getPolymorphismPropertiesToSerialize(Class<?> clazz, Collection<String> otherProperties) {
+ List<Map.Entry<String, String>> entries = new ArrayList<>();
+
+ Class<?> current = clazz;
+ while (current != null) {
+ validateJsonbTypeInfo(current);
+ validateOnlyOneParentWithTypeInfo(current);
+
+ JsonbTypeInfo typeInfo = Meta.getDirectAnnotation(current, JsonbTypeInfo.class);
+ if (typeInfo != null) {
+ if (otherProperties.contains(typeInfo.key())) {
+ throw new JsonbException("JsonbTypeInfo key '" + typeInfo.key() + "' collides with other properties in json");
+ }
+
+ if (entries.stream().anyMatch(entry -> Objects.equals(entry.getKey(), typeInfo.key()))) {
+ throw new JsonbException("JsonbTypeInfo key '" + typeInfo.key() + "' found more than once in type hierarchy of " + clazz.getName());
+ }
+
+ String bestMatchingAlias = null;
+ for (JsonbSubtype subtype : typeInfo.value()) {
+ if (subtype.type().isAssignableFrom(clazz)) {
+ bestMatchingAlias = subtype.alias();
+
+ if (clazz == subtype.type()) { // Exact match found, no need to continue further
+ break;
+ }
+ }
+ }
+
+ if (bestMatchingAlias != null) {
+ entries.add(0, Map.entry(typeInfo.key(), bestMatchingAlias));
+ }
+ }
+
+ List<Class<?>> parentClassesWithTypeInfo = getParentClassesWithTypeInfo(current);
+ current = parentClassesWithTypeInfo.isEmpty() ? null : parentClassesWithTypeInfo.get(0);
+ }
+
+ return entries;
+ }
+
+ public Class<?> getTypeToDeserialize(JsonObject jsonObject, Class<?> clazz) {
+ validateJsonbTypeInfo(clazz);
+ validateOnlyOneParentWithTypeInfo(clazz);
+
+ JsonbTypeInfo typeInfo = Meta.getDirectAnnotation(clazz, JsonbTypeInfo.class);
+ if (typeInfo == null || !jsonObject.containsKey(typeInfo.key())) {
+ return clazz;
+ }
+
+ JsonValue typeValue = jsonObject.get(typeInfo.key());
+ if (!(typeValue instanceof JsonString)) {
+ throw new JsonbException("Property '" + typeInfo.key() + "' isn't a String, resolving "
+ + "JsonbSubtype is impossible");
+ }
+
+ String typeValueString = ((JsonString) typeValue).getString();
+ for (JsonbSubtype subtype : typeInfo.value()) {
Review Comment:
should be a map for runtime probably
##########
johnzon-mapper/src/main/java/org/apache/johnzon/mapper/MapperBuilder.java:
##########
@@ -18,17 +18,12 @@
*/
package org.apache.johnzon.mapper;
-import static java.util.Arrays.asList;
Review Comment:
if possible maybe try to reduce the diff at the min :pray:
--
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: dev-unsubscribe@johnzon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org