From 3ec253707dcfba4055e99d6af97d0d3877d0999a Mon Sep 17 00:00:00 2001 From: ian Date: Fri, 15 Aug 2014 19:28:51 -0400 Subject: [PATCH 1/2] keep bundle annotations and prevent simple cycles I don't see any downsides and this makes it easier to manipulate custom annotations, introspectors, serializers, and other parts that interact with annotated objects. The cycle detection is a bonus. --- .../databind/introspect/AnnotatedClass.java | 12 ++-- .../databind/introspect/AnnotatedMember.java | 8 +-- .../databind/introspect/AnnotationMap.java | 13 +++-- .../introspect/TestAnnotionBundles.java | 58 ++++++++++++++++++- 4 files changed, 73 insertions(+), 18 deletions(-) diff --git a/src/main/java/com/fasterxml/jackson/databind/introspect/AnnotatedClass.java b/src/main/java/com/fasterxml/jackson/databind/introspect/AnnotatedClass.java index fa379b104d..8265d25661 100644 --- a/src/main/java/com/fasterxml/jackson/databind/introspect/AnnotatedClass.java +++ b/src/main/java/com/fasterxml/jackson/databind/introspect/AnnotatedClass.java @@ -908,13 +908,13 @@ private void _addAnnotationsIfNotPresent(AnnotationMap result, Annotation[] anns if (anns != null) { List bundles = null; for (Annotation ann : anns) { // first: direct annotations - if (_isAnnotationBundle(ann)) { + // note: we will NOT filter out non-Jackson anns any more + boolean wasNotPresent = result.addIfNotPresent(ann); + if (wasNotPresent && _isAnnotationBundle(ann)) { if (bundles == null) { bundles = new LinkedList(); } bundles.add(ann.annotationType().getDeclaredAnnotations()); - } else { // note: we will NOT filter out non-Jackson anns any more - result.addIfNotPresent(ann); } } if (bundles != null) { // and secondarily handle bundles, if any found: precedence important @@ -930,13 +930,13 @@ private void _addAnnotationsIfNotPresent(AnnotatedMember target, Annotation[] an if (anns != null) { List bundles = null; for (Annotation ann : anns) { // first: direct annotations - if (_isAnnotationBundle(ann)) { + // note: we will NOT filter out non-Jackson anns any more + boolean wasNotPresent = target.addIfNotPresent(ann); + if (wasNotPresent && _isAnnotationBundle(ann)) { if (bundles == null) { bundles = new LinkedList(); } bundles.add(ann.annotationType().getDeclaredAnnotations()); - } else { // note: we will NOT filter out non-Jackson anns any more - target.addIfNotPresent(ann); } } if (bundles != null) { // and secondarily handle bundles, if any found: precedence important diff --git a/src/main/java/com/fasterxml/jackson/databind/introspect/AnnotatedMember.java b/src/main/java/com/fasterxml/jackson/databind/introspect/AnnotatedMember.java index 871a65c58d..550b26329d 100644 --- a/src/main/java/com/fasterxml/jackson/databind/introspect/AnnotatedMember.java +++ b/src/main/java/com/fasterxml/jackson/databind/introspect/AnnotatedMember.java @@ -51,8 +51,8 @@ protected AnnotationMap getAllAnnotations() { * annotation masking or overriding an annotation 'real' constructor * has. */ - public final void addOrOverride(Annotation a) { - _annotations.add(a); + public final boolean addOrOverride(Annotation a) { + return _annotations.add(a); } /** @@ -60,8 +60,8 @@ public final void addOrOverride(Annotation a) { * annotation if and only if it is not yet present in the * annotation map we have. */ - public final void addIfNotPresent(Annotation a) { - _annotations.addIfNotPresent(a); + public final boolean addIfNotPresent(Annotation a) { + return _annotations.addIfNotPresent(a); } /** diff --git a/src/main/java/com/fasterxml/jackson/databind/introspect/AnnotationMap.java b/src/main/java/com/fasterxml/jackson/databind/introspect/AnnotationMap.java index 1b350fe0a2..0b514ffb51 100644 --- a/src/main/java/com/fasterxml/jackson/databind/introspect/AnnotationMap.java +++ b/src/main/java/com/fasterxml/jackson/databind/introspect/AnnotationMap.java @@ -71,18 +71,20 @@ public int size() { * Method called to add specified annotation in the Map, but * only if it didn't yet exist. */ - public void addIfNotPresent(Annotation ann) + public boolean addIfNotPresent(Annotation ann) { if (_annotations == null || !_annotations.containsKey(ann.annotationType())) { _add(ann); + return true; } + return false; } /** * Method called to add specified annotation in the Map. */ - public void add(Annotation ann) { - _add(ann); + public boolean add(Annotation ann) { + return _add(ann); } @Override @@ -99,11 +101,12 @@ public String toString() { /********************************************************** */ - protected final void _add(Annotation ann) { + protected final boolean _add(Annotation ann) { if (_annotations == null) { _annotations = new HashMap,Annotation>(); } - _annotations.put(ann.annotationType(), ann); + Annotation previous = _annotations.put(ann.annotationType(), ann); + return (previous != null) && previous.equals(ann); } } diff --git a/src/test/java/com/fasterxml/jackson/databind/introspect/TestAnnotionBundles.java b/src/test/java/com/fasterxml/jackson/databind/introspect/TestAnnotionBundles.java index ddb77360da..d968cc5bf1 100644 --- a/src/test/java/com/fasterxml/jackson/databind/introspect/TestAnnotionBundles.java +++ b/src/test/java/com/fasterxml/jackson/databind/introspect/TestAnnotionBundles.java @@ -3,9 +3,13 @@ import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; -import com.fasterxml.jackson.annotation.*; +import com.fasterxml.jackson.annotation.JacksonAnnotationsInside; +import com.fasterxml.jackson.annotation.JsonAutoDetect; import com.fasterxml.jackson.annotation.JsonAutoDetect.Visibility; +import com.fasterxml.jackson.annotation.JsonIgnore; +import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.PropertyName; /* Tests mostly for [JACKSON-754]: ability to create "annotation bundles" */ @@ -51,7 +55,44 @@ public class NoAutoDetect { public class Bean92 { @Bundle92 protected String id = "abc"; - } + } + + @HolderB + @JacksonAnnotationsInside + @Retention(RetentionPolicy.RUNTIME) + static @interface HolderA {} + + @HolderA + @JacksonAnnotationsInside + @Retention(RetentionPolicy.RUNTIME) + static @interface HolderB {} + + static class HolderHolder { + @HolderA + @InformativeHolder + public int unimportant = 42; + } + + @JsonProperty + @JacksonAnnotationsInside + @Retention(RetentionPolicy.RUNTIME) + static @interface InformativeHolder { + // doesn't really contribute to the test, but would be impossible without this feature + boolean important() default true; + } + + static class BundleAnnotationIntrospector extends JacksonAnnotationIntrospector { + @Override + public PropertyName findNameForSerialization(Annotated a) + { + InformativeHolder informativeHolder = a.getAnnotation(InformativeHolder.class); + if ((informativeHolder != null) && informativeHolder.important()) { + return new PropertyName("important"); + } + return super.findNameForSerialization(a); + } + } + /* /********************************************************** /* Test methods @@ -59,7 +100,18 @@ public class Bean92 { */ private final ObjectMapper MAPPER = new ObjectMapper(); - + + public void testKeepAnnotationBundle() throws Exception + { + MAPPER.setAnnotationIntrospector(new BundleAnnotationIntrospector()); + assertEquals("{\"important\":42}", MAPPER.writeValueAsString(new HolderHolder())); + } + + public void testRecursiveBundles() throws Exception + { + assertEquals("{\"unimportant\":42}", MAPPER.writeValueAsString(new HolderHolder())); + } + public void testBundledIgnore() throws Exception { assertEquals("{\"foobar\":13}", MAPPER.writeValueAsString(new Bean())); From 21df384f15ea3a456b38e43b0dd613fa11025383 Mon Sep 17 00:00:00 2001 From: ian Date: Fri, 15 Aug 2014 19:37:35 -0400 Subject: [PATCH 2/2] finish override logic and split test logic up --- .../databind/introspect/AnnotatedClass.java | 6 +++--- .../databind/introspect/TestAnnotionBundles.java | 14 ++++++++------ 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/src/main/java/com/fasterxml/jackson/databind/introspect/AnnotatedClass.java b/src/main/java/com/fasterxml/jackson/databind/introspect/AnnotatedClass.java index 8265d25661..2a9c458d5e 100644 --- a/src/main/java/com/fasterxml/jackson/databind/introspect/AnnotatedClass.java +++ b/src/main/java/com/fasterxml/jackson/databind/introspect/AnnotatedClass.java @@ -952,13 +952,13 @@ private void _addOrOverrideAnnotations(AnnotatedMember target, Annotation[] anns if (anns != null) { List bundles = null; for (Annotation ann : anns) { // first: direct annotations - if (_isAnnotationBundle(ann)) { + // note: we will NOT filter out non-Jackson anns any more + boolean wasModified = target.addOrOverride(ann); + if (wasModified && _isAnnotationBundle(ann)) { if (bundles == null) { bundles = new LinkedList(); } bundles.add(ann.annotationType().getDeclaredAnnotations()); - } else { // note: no filtering by jackson-annotations - target.addOrOverride(ann); } } if (bundles != null) { // and then bundles, if any: important for precedence diff --git a/src/test/java/com/fasterxml/jackson/databind/introspect/TestAnnotionBundles.java b/src/test/java/com/fasterxml/jackson/databind/introspect/TestAnnotionBundles.java index d968cc5bf1..e8d1af606e 100644 --- a/src/test/java/com/fasterxml/jackson/databind/introspect/TestAnnotionBundles.java +++ b/src/test/java/com/fasterxml/jackson/databind/introspect/TestAnnotionBundles.java @@ -67,10 +67,8 @@ public class Bean92 { @Retention(RetentionPolicy.RUNTIME) static @interface HolderB {} - static class HolderHolder { - @HolderA - @InformativeHolder - public int unimportant = 42; + static class RecursiveHolder { + @HolderA public int unimportant = 42; } @JsonProperty @@ -81,6 +79,10 @@ static class HolderHolder { boolean important() default true; } + static class InformingHolder { + @InformativeHolder public int unimportant = 42; + } + static class BundleAnnotationIntrospector extends JacksonAnnotationIntrospector { @Override public PropertyName findNameForSerialization(Annotated a) @@ -104,12 +106,12 @@ public PropertyName findNameForSerialization(Annotated a) public void testKeepAnnotationBundle() throws Exception { MAPPER.setAnnotationIntrospector(new BundleAnnotationIntrospector()); - assertEquals("{\"important\":42}", MAPPER.writeValueAsString(new HolderHolder())); + assertEquals("{\"important\":42}", MAPPER.writeValueAsString(new InformingHolder())); } public void testRecursiveBundles() throws Exception { - assertEquals("{\"unimportant\":42}", MAPPER.writeValueAsString(new HolderHolder())); + assertEquals("{\"unimportant\":42}", MAPPER.writeValueAsString(new RecursiveHolder())); } public void testBundledIgnore() throws Exception