This is an automated email from the ASF dual-hosted git repository. jlmonteiro pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/johnzon.git
commit 2553ab22f38a40e0f26f699ea8bb0e76ea4aa896 Author: Jean-Louis Monteiro <jlmonte...@tomitribe.com> AuthorDate: Wed Oct 11 09:48:43 2023 +0200 feat: field visibilty buggy for booleans and not compliant with specification Signed-off-by: Jean-Louis Monteiro <jlmonte...@tomitribe.com> --- .../jsonb/DefaultPropertyVisibilityStrategy.java | 15 ++- .../DefaultPropertyVisibilityStrategyTest.java | 114 +++++++++++++++++++++ 2 files changed, 124 insertions(+), 5 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 919a895c..67ea85c5 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 @@ -50,6 +50,11 @@ class DefaultPropertyVisibilityStrategy implements jakarta.json.bind.config.Prop return strategy == this ? isFieldVisible(field, root, useGetter) : strategy.isVisible(field); } + /** + * If the field is not public then it's of course not visible. If the field is public then we need to look at the + * accessors. If there is a private/protected/default accessor for it then it overrides and the field is not visible + * But if there is no accessor for it, then it's visible. + */ private boolean isFieldVisible(final Field field, final Class<?> root, final boolean useGetter) { if (!Modifier.isPublic(field.getModifiers())) { return false; @@ -57,16 +62,16 @@ class DefaultPropertyVisibilityStrategy implements jakarta.json.bind.config.Prop // 3.7.1. Scope and Field access strategy // note: we should bind the class since a field of a parent class can have a getter in a child if (!useGetter) { - return !hasMethod(root, BeanUtil.setterName(field.getName())); + return !hasMethod(root, BeanUtil.setterName(field.getName()), field.getType()); } - final String capitalizedName = BeanUtil.capitalize(field.getName()); - return !hasMethod(root, "get" + capitalizedName) || hasMethod(root, "is" + capitalizedName); + return !hasMethod(root, BeanUtil.getterName(field.getName(), field.getType())); } private boolean hasMethod(Class<?> clazz, String methodName, Class<?>... paramTypes) { try { - clazz.getDeclaredMethod(methodName, paramTypes); - return true; + final Method declaredMethod = clazz.getDeclaredMethod(methodName, paramTypes); + return !Modifier.isPublic(declaredMethod.getModifiers()); + } catch (NoSuchMethodException e) { final Class<?> superclass = clazz.getSuperclass(); if (superclass == Object.class) { diff --git a/johnzon-jsonb/src/test/java/org/apache/johnzon/jsonb/DefaultPropertyVisibilityStrategyTest.java b/johnzon-jsonb/src/test/java/org/apache/johnzon/jsonb/DefaultPropertyVisibilityStrategyTest.java index 25e9b5f4..ac32834b 100644 --- a/johnzon-jsonb/src/test/java/org/apache/johnzon/jsonb/DefaultPropertyVisibilityStrategyTest.java +++ b/johnzon-jsonb/src/test/java/org/apache/johnzon/jsonb/DefaultPropertyVisibilityStrategyTest.java @@ -18,6 +18,7 @@ */ package org.apache.johnzon.jsonb; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; @@ -57,6 +58,61 @@ public class DefaultPropertyVisibilityStrategyTest { } } + @Test + public void matchingPrivateAccessors() throws Exception { + try (final Jsonb jsonb = JsonbBuilder.create()) { + assertEquals("{}", jsonb.toJson(new PublicFieldWithPrivateAccessors())); + assertEquals("{}", jsonb.toJson(new PublicBooleanFieldWithPrivateAccessors())); + + { + final PublicFieldWithPrivateAccessors unmarshalledObject = + jsonb.fromJson("{\"foo\":\"bad\"}", PublicFieldWithPrivateAccessors.class); + assertEquals("bar", unmarshalledObject.foo); + } + { + final PublicBooleanFieldWithPrivateAccessors unmarshalledObject = + jsonb.fromJson("{\"foo\":\"false\"}", PublicBooleanFieldWithPrivateAccessors.class); + assertEquals(true, unmarshalledObject.foo); + } + } + } + + @Test + public void noMatchingPrivateAccessors() throws Exception { + try (final Jsonb jsonb = JsonbBuilder.create()) { + assertEquals("{\"foo\":\"bar\"}", jsonb.toJson(new PublicFieldWithoutMatchingPrivateAccessors())); + + final PublicFieldWithoutMatchingPrivateAccessors unmarshalledObject = + jsonb.fromJson("{\"foo\":\"good\"}", PublicFieldWithoutMatchingPrivateAccessors.class); + assertEquals("good", unmarshalledObject.foo); + } + } + + @Test + public void oneAccessorOnly() throws Exception { + try (final Jsonb jsonb = JsonbBuilder.create()) { + assertEquals("{}", jsonb.toJson(new PublicFieldWithMatchingPrivateGetterAccessors())); + assertEquals("{\"foo\":\"bar\"}", jsonb.toJson(new PublicFieldWithMatchingPrivateSetterAccessors())); + assertEquals("{\"foo\":\"bar\"}", jsonb.toJson(new PublicFieldWithPublicAccessors())); + + { + final PublicFieldWithMatchingPrivateGetterAccessors unmarshalledObject = + jsonb.fromJson("{\"foo\":\"good\"}", PublicFieldWithMatchingPrivateGetterAccessors.class); + assertEquals("good", unmarshalledObject.foo); + } + { + final PublicFieldWithMatchingPrivateSetterAccessors unmarshalledObject = + jsonb.fromJson("{\"foo\":\"bad\"}", PublicFieldWithMatchingPrivateSetterAccessors.class); + assertEquals("bar", unmarshalledObject.foo); + } + { + final PublicFieldWithPublicAccessors unmarshalledObject = + jsonb.fromJson("{\"foo\":\"good\"}", PublicFieldWithPublicAccessors.class); + assertEquals("good", unmarshalledObject.foo); + } + } + } + public static class HideAll extends DefaultPropertyVisibilityStrategy { @Override public boolean isVisible(final Field field) { @@ -81,6 +137,64 @@ public class DefaultPropertyVisibilityStrategyTest { } } + // if there is a matching private getter/setter, the field even if it's public must be ignored + public static final class PublicFieldWithPrivateAccessors { + public String foo = "bar"; + + private String getFoo() { + return foo; + } + + private void setFoo(final String foo) { + this.foo = foo; + } + } + + public static final class PublicFieldWithPublicAccessors { + public String foo = "bar"; + + public String getFoo() { + return foo; + } + + public void setFoo(final String foo) { + this.foo = foo; + } + } + + public static final class PublicBooleanFieldWithPrivateAccessors { + public boolean foo = true; + + private boolean isFoo() { + return foo; + } + + private void setFoo(final boolean foo) { + this.foo = foo; + } + } + + // if there is not matching getter/setter for a public field, then the field is used directly + public static final class PublicFieldWithoutMatchingPrivateAccessors { + public String foo = "bar"; + } + + public static final class PublicFieldWithMatchingPrivateGetterAccessors { + public String foo = "bar"; + + private String getFoo() { + return foo; + } + } + + public static final class PublicFieldWithMatchingPrivateSetterAccessors { + public String foo = "bar"; + + private void setFoo(final String foo) { + this.foo = foo; + } + } + @JsonbVisibility(MyVisibility.class) public static final class TheClass { @JsonbProperty