This is an automated email from the ASF dual-hosted git repository.
struberg pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/johnzon.git
The following commit(s) were added to refs/heads/master by this push:
new a38e0c3 JOHNZON-364 JsonbVisibility always wins over default rules
a38e0c3 is described below
commit a38e0c3daef597bae13e0cc68548f8386272c570
Author: Mark Struberg <[email protected]>
AuthorDate: Fri Feb 25 15:11:03 2022 +0100
JOHNZON-364 JsonbVisibility always wins over default rules
This partially reverts JOHNZON-250 which did remove fields as readers if a
getter exists.
Sadly this also did remove the option to keep those fields via
JsonbVisibility.
---
.../jsonb/DefaultPropertyVisibilityStrategy.java | 36 +++++++++++++++++++++-
.../org/apache/johnzon/jsonb/JohnzonBuilder.java | 2 +-
...icFieldTest.java => HidingPublicFieldTest.java} | 11 ++++++-
.../apache/johnzon/jsonb/JsonbVisitilityTest.java | 15 ++++++++-
.../jsonb/api/experimental/JsonbExtensionTest.java | 4 +++
.../org/apache/johnzon/mapper/MapperBuilder.java | 2 +-
.../mapper/access/FieldAndMethodAccessMode.java | 32 +++----------------
7 files changed, 69 insertions(+), 33 deletions(-)
diff --git
a/johnzon-jsonb/src/main/java/org/apache/johnzon/jsonb/DefaultPropertyVisibilityStrategy.java
b/johnzon-jsonb/src/main/java/org/apache/johnzon/jsonb/DefaultPropertyVisibilityStrategy.java
index 5f59471..141b5a0 100644
---
a/johnzon-jsonb/src/main/java/org/apache/johnzon/jsonb/DefaultPropertyVisibilityStrategy.java
+++
b/johnzon-jsonb/src/main/java/org/apache/johnzon/jsonb/DefaultPropertyVisibilityStrategy.java
@@ -20,9 +20,13 @@ package org.apache.johnzon.jsonb;
import static java.util.Optional.ofNullable;
+import java.beans.Introspector;
import java.lang.reflect.Field;
import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
@@ -32,6 +36,8 @@ import javax.json.bind.config.PropertyVisibilityStrategy;
class DefaultPropertyVisibilityStrategy implements
javax.json.bind.config.PropertyVisibilityStrategy {
private final ConcurrentMap<Class<?>, PropertyVisibilityStrategy>
strategies = new ConcurrentHashMap<>();
+ private final ConcurrentMap<Class<?>, List<String>> getters = new
ConcurrentHashMap<>();
+
private volatile boolean skipGetpackage;
@Override
@@ -41,7 +47,35 @@ class DefaultPropertyVisibilityStrategy implements
javax.json.bind.config.Proper
}
final PropertyVisibilityStrategy strategy = strategies.computeIfAbsent(
field.getDeclaringClass(), this::visibilityStrategy);
- return strategy == this ? Modifier.isPublic(field.getModifiers()) :
strategy.isVisible(field);
+ return strategy == this ? isFieldVisible(field) :
strategy.isVisible(field);
+ }
+
+ private boolean isFieldVisible(Field field) {
+ if (!Modifier.isPublic(field.getModifiers())) {
+ return false;
+ }
+ // also check if there is any setter, in which case the field should
be treated as non-visible as well.
+ return !getters.computeIfAbsent(field.getDeclaringClass(),
this::calculateGetters).contains(field.getName());
+ }
+
+ /**
+ * Calculate all the getters of the given class.
+ */
+ private List<String> calculateGetters(Class<?> clazz) {
+ List<String> getters = new ArrayList<>();
+ for (Method m : clazz.getDeclaredMethods()) {
+ if (m.getParameterCount() == 0) {
+ if (m.getName().startsWith("get")) {
+
getters.add(Introspector.decapitalize(m.getName().substring(3)));
+ } else if (m.getName().startsWith("is")) {
+
getters.add(Introspector.decapitalize(m.getName().substring(2)));
+ }
+ }
+ }
+ if (clazz.getSuperclass() != Object.class) {
+ getters.addAll(calculateGetters(clazz.getSuperclass()));
+ }
+ return getters.isEmpty() ? Collections.emptyList() : getters;
}
@Override
diff --git
a/johnzon-jsonb/src/main/java/org/apache/johnzon/jsonb/JohnzonBuilder.java
b/johnzon-jsonb/src/main/java/org/apache/johnzon/jsonb/JohnzonBuilder.java
index 24966cc..b304bd1 100644
--- a/johnzon-jsonb/src/main/java/org/apache/johnzon/jsonb/JohnzonBuilder.java
+++ b/johnzon-jsonb/src/main/java/org/apache/johnzon/jsonb/JohnzonBuilder.java
@@ -221,7 +221,7 @@ public class JohnzonBuilder implements JsonbBuilder {
factory, jsonp, builderFactorySupplier,
parserFactoryProvider,
config.getProperty("johnzon.accessModeDelegate")
.map(this::toAccessMode)
- .orElseGet(() -> new
FieldAndMethodAccessMode(true, true, false, true)),
+ .orElseGet(() -> new
FieldAndMethodAccessMode(true, true, false)),
// this changes in v3 of the spec so let's use this
behavior which makes everyone happy by default
config.getProperty("johnzon.failOnMissingCreatorValues")
.map(this::toBool)
diff --git
a/johnzon-jsonb/src/test/java/org/apache/johnzon/jsonb/HiddingPublicFieldTest.java
b/johnzon-jsonb/src/test/java/org/apache/johnzon/jsonb/HidingPublicFieldTest.java
similarity index 77%
rename from
johnzon-jsonb/src/test/java/org/apache/johnzon/jsonb/HiddingPublicFieldTest.java
rename to
johnzon-jsonb/src/test/java/org/apache/johnzon/jsonb/HidingPublicFieldTest.java
index 2f6a314..a81dba4 100644
---
a/johnzon-jsonb/src/test/java/org/apache/johnzon/jsonb/HiddingPublicFieldTest.java
+++
b/johnzon-jsonb/src/test/java/org/apache/johnzon/jsonb/HidingPublicFieldTest.java
@@ -24,7 +24,7 @@ import org.apache.johnzon.jsonb.test.JsonbRule;
import org.junit.Rule;
import org.junit.Test;
-public class HiddingPublicFieldTest {
+public class HidingPublicFieldTest {
@Rule
public final JsonbRule jsonb = new JsonbRule();
@@ -38,6 +38,15 @@ public class HiddingPublicFieldTest {
public static class Model {
public String value;
+ /**
+ * As of section 3.7.1 of the JSON-B spec a private method 'hides'
fields
+ * The rules are as following:
+ * <ul>
+ * <li>if public setter/getter exists -> take that</li>
+ * <li>if non public setter/getter exists -> ignore</li>
+ * <li>OTHERWISE (no setter/getter at all) -> use fields</li>
+ * </ul>
+ */
private String getValue() {
return value;
}
diff --git
a/johnzon-jsonb/src/test/java/org/apache/johnzon/jsonb/JsonbVisitilityTest.java
b/johnzon-jsonb/src/test/java/org/apache/johnzon/jsonb/JsonbVisitilityTest.java
index 9fe45aa..20abeb6 100644
---
a/johnzon-jsonb/src/test/java/org/apache/johnzon/jsonb/JsonbVisitilityTest.java
+++
b/johnzon-jsonb/src/test/java/org/apache/johnzon/jsonb/JsonbVisitilityTest.java
@@ -35,12 +35,13 @@ public class JsonbVisitilityTest {
@Test
public void testJsonVisibilityAllFields() {
MyDataVisibility data = new MyDataVisibility();
+ data.setTestKey("yolo");
data.put("x", "a");
data.put("y", "b");
Jsonb jsonb = JsonbProvider.provider().create().build();
String json = jsonb.toJson(data);
- Assert.assertEquals("{\"attribs\":{\"x\":\"a\",\"y\":\"b\"}}", json);
+
Assert.assertEquals("{\"attribs\":{\"x\":\"a\",\"y\":\"b\"},\"testKey\":\"yolo\"}",
json);
MyDataVisibility dataBack = jsonb.fromJson(json,
MyDataVisibility.class);
Assert.assertEquals("a", dataBack.get("x"));
@@ -67,6 +68,8 @@ public class JsonbVisitilityTest {
public static class MyDataVisibility {
private Map<String, String> attribs = new HashMap<>();
+ private String testKey;
+
public void put(String key, String value) {
attribs.put(key, value);
}
@@ -74,6 +77,16 @@ public class JsonbVisitilityTest {
public String get(String key) {
return attribs.get(key);
}
+
+ // intentionally protected!
+ protected String getTestKey() {
+ return testKey;
+ }
+
+ // intentionally protected!
+ protected void setTestKey(String testKey) {
+ this.testKey = testKey;
+ }
}
public static class MyDataJsonField {
diff --git
a/johnzon-jsonb/src/test/java/org/apache/johnzon/jsonb/api/experimental/JsonbExtensionTest.java
b/johnzon-jsonb/src/test/java/org/apache/johnzon/jsonb/api/experimental/JsonbExtensionTest.java
index a2d612c..fc8eacd 100644
---
a/johnzon-jsonb/src/test/java/org/apache/johnzon/jsonb/api/experimental/JsonbExtensionTest.java
+++
b/johnzon-jsonb/src/test/java/org/apache/johnzon/jsonb/api/experimental/JsonbExtensionTest.java
@@ -218,6 +218,10 @@ public class JsonbExtensionTest {
return value;
}
+ public void setValue(Object value) {
+ this.value = value;
+ }
+
@Override
public boolean equals(final Object obj) { // for test
return Wrapper.class.isInstance(obj) &&
diff --git
a/johnzon-mapper/src/main/java/org/apache/johnzon/mapper/MapperBuilder.java
b/johnzon-mapper/src/main/java/org/apache/johnzon/mapper/MapperBuilder.java
index cf6de3d..b02455f 100644
--- a/johnzon-mapper/src/main/java/org/apache/johnzon/mapper/MapperBuilder.java
+++ b/johnzon-mapper/src/main/java/org/apache/johnzon/mapper/MapperBuilder.java
@@ -169,7 +169,7 @@ public class MapperBuilder {
} else if ("strict-method".equalsIgnoreCase(accessModeName)) {
accessMode = new MethodAccessMode(supportConstructors,
supportHiddenAccess, false);
} else if ("both".equalsIgnoreCase(accessModeName) ||
accessModeName == null) {
- accessMode = new FieldAndMethodAccessMode(supportConstructors,
supportHiddenAccess, useGetterForCollections, false);
+ accessMode = new FieldAndMethodAccessMode(supportConstructors,
supportHiddenAccess, useGetterForCollections);
} else {
throw new IllegalArgumentException("Unsupported access mode: "
+ accessModeName);
}
diff --git
a/johnzon-mapper/src/main/java/org/apache/johnzon/mapper/access/FieldAndMethodAccessMode.java
b/johnzon-mapper/src/main/java/org/apache/johnzon/mapper/access/FieldAndMethodAccessMode.java
index 4fb0be4..a651d6d 100644
---
a/johnzon-mapper/src/main/java/org/apache/johnzon/mapper/access/FieldAndMethodAccessMode.java
+++
b/johnzon-mapper/src/main/java/org/apache/johnzon/mapper/access/FieldAndMethodAccessMode.java
@@ -29,7 +29,6 @@ import java.beans.Introspector;
import java.lang.annotation.Annotation;
import java.lang.reflect.Field;
import java.lang.reflect.Method;
-import java.lang.reflect.Modifier;
import java.lang.reflect.ParameterizedType;
import java.lang.reflect.Type;
import java.util.HashMap;
@@ -39,23 +38,14 @@ import java.util.Map;
public class FieldAndMethodAccessMode extends BaseAccessMode {
private final FieldAccessMode fields;
private final MethodAccessMode methods;
- private final boolean alwaysPreferMethodVisibility;
public FieldAndMethodAccessMode(final boolean useConstructor, final
boolean acceptHiddenConstructor,
- final boolean useGettersAsWriter, final
boolean alwaysPreferMethodVisibility) {
+ final boolean useGettersAsWriter) {
super(useConstructor, acceptHiddenConstructor);
this.fields = new FieldAccessMode(useConstructor,
acceptHiddenConstructor);
- this.methods = new MethodAccessMode(
- useConstructor, acceptHiddenConstructor, useGettersAsWriter);
- this.alwaysPreferMethodVisibility = alwaysPreferMethodVisibility;
+ this.methods = new MethodAccessMode(useConstructor,
acceptHiddenConstructor, useGettersAsWriter);
}
- // backward compatibility, don't delete since it can be used from user
code in jsonb delegate access mode property
- @Deprecated
- public FieldAndMethodAccessMode(final boolean useConstructor, final
boolean acceptHiddenConstructor,
- final boolean useGettersAsWriter) {
- this(useConstructor, acceptHiddenConstructor, useGettersAsWriter,
true);
- }
@Override
public Map<String, Reader> doFindReaders(final Class<?> clazz) {
@@ -77,7 +67,7 @@ public class FieldAndMethodAccessMode extends BaseAccessMode {
m = getMethod("is" + Character.toUpperCase(key.charAt(0)) +
(key.length() > 1 ? key.substring(1) : ""), clazz);
}
boolean skip = false;
- if (m != null && Modifier.isPublic(m.getModifiers())) {
+ if (m != null) {
for (final Reader w : methodReaders.values()) {
if
(MethodAccessMode.MethodDecoratedType.class.cast(w).getMethod().equals(m)) {
if (w.getAnnotation(JohnzonProperty.class) != null ||
w.getAnnotation(JohnzonIgnore.class) != null) {
@@ -86,8 +76,6 @@ public class FieldAndMethodAccessMode extends BaseAccessMode {
break;
}
}
- } else if (m != null) { // skip even field
- continue;
}
if (skip) {
continue;
@@ -135,18 +123,8 @@ public class FieldAndMethodAccessMode extends
BaseAccessMode {
private Method getMethod(final String methodName, final Class<?> type,
final Class<?>... args) {
try {
- if (alwaysPreferMethodVisibility) {
- return type.getDeclaredMethod(methodName, args);
- }
return type.getMethod(methodName, args);
} catch (final NoSuchMethodException e) {
- if (alwaysPreferMethodVisibility) {
- try {
- return type.getMethod(methodName, args);
- } catch (final NoSuchMethodException e2) {
- // no-op
- }
- }
return null;
}
}
@@ -175,7 +153,7 @@ public class FieldAndMethodAccessMode extends
BaseAccessMode {
final String key = entry.getKey();
final Method m = getMethod("set" +
Character.toUpperCase(key.charAt(0)) + (key.length() > 1 ? key.substring(1) :
""), clazz, toType(entry.getValue().getType()));
boolean skip = false;
- if (m != null && Modifier.isPublic(m.getModifiers())) {
+ if (m != null) {
for (final Writer w : metodWriters.values()) {
if
(MethodAccessMode.MethodDecoratedType.class.cast(w).getMethod().equals(m)) {
if (w.getAnnotation(JohnzonProperty.class) != null) {
@@ -184,8 +162,6 @@ public class FieldAndMethodAccessMode extends
BaseAccessMode {
break;
}
}
- } else if (m != null) {
- continue;
}
if (skip) {
continue;
