From 0d9d9e06ba1f07d45b68709412cdd4ccce1c3485 Mon Sep 17 00:00:00 2001 From: Graeme Rocher Date: Fri, 17 Feb 2023 08:43:11 +0100 Subject: [PATCH] Fix regression where AliasFor is no longer recursive (#8746) --------- Co-authored-by: Denis Stepanov --- .../AbstractAnnotationMetadataBuilder.java | 26 ++++++---- .../MultiValuesConverterFactory.java | 14 ++--- http-client/src/test/resources/logback.xml | 4 +- .../GroovyAnnotationMetadataBuilder.java | 19 +++++-- ...eritedConfigurationReaderPrefixSpec.groovy | 4 +- .../inject/visitor/ClassElementSpec.groovy | 51 ++++++++++++++++++ .../visitors/ClassElementSpec.groovy | 52 +++++++++++++++++++ .../micronaut/context/DefaultBeanContext.java | 1 - .../context/annotation/Replaces.java | 1 + .../replacesbug/MathInnerServiceSpec.groovy | 46 ++++++++++++++++ 10 files changed, 193 insertions(+), 25 deletions(-) create mode 100644 test-suite/src/test/groovy/io/micronaut/test/replacesbug/MathInnerServiceSpec.groovy diff --git a/core-processor/src/main/java/io/micronaut/inject/annotation/AbstractAnnotationMetadataBuilder.java b/core-processor/src/main/java/io/micronaut/inject/annotation/AbstractAnnotationMetadataBuilder.java index 5744d000586..6c134a8cd34 100644 --- a/core-processor/src/main/java/io/micronaut/inject/annotation/AbstractAnnotationMetadataBuilder.java +++ b/core-processor/src/main/java/io/micronaut/inject/annotation/AbstractAnnotationMetadataBuilder.java @@ -38,7 +38,6 @@ import io.micronaut.inject.qualifiers.InterceptorBindingQualifier; import io.micronaut.inject.visitor.VisitorContext; import jakarta.inject.Qualifier; -import org.jetbrains.annotations.NotNull; import java.lang.annotation.Annotation; import java.lang.annotation.RetentionPolicy; @@ -670,13 +669,16 @@ private void processAnnotationAlias(Map annotationValues, aliasedAnnotation = aliasAnnotation.orElseGet(aliasAnnotationName::get); String aliasedMemberName = aliasMember.get(); if (annotationValue != null) { - introducedAnnotations.add( - toProcessedAnnotation( + ProcessedAnnotation newAnnotation = toProcessedAnnotation( AnnotationValue.builder(aliasedAnnotation, getRetentionPolicy(aliasedAnnotation)) - .members(Collections.singletonMap(aliasedMemberName, annotationValue)) - .build() - ) + .members(Collections.singletonMap(aliasedMemberName, annotationValue)) + .build() ); + introducedAnnotations.add(newAnnotation); + ProcessedAnnotation newNewAnnotation = processAliases(newAnnotation, introducedAnnotations); + if (newNewAnnotation != newAnnotation) { + introducedAnnotations.set(introducedAnnotations.indexOf(newAnnotation), newNewAnnotation); + } } } } else if (aliasMember.isPresent()) { @@ -771,7 +773,7 @@ private void addAnnotations(MutableAnnotationMetadata annotationMetadata, addAnnotations(annotationMetadata, annotationValues, isDeclared, parentAnnotations); } - @NotNull + @NonNull private Stream annotationMirrorToAnnotationValue(Stream stream, T element, boolean originatingElementIsSameParent, @@ -868,8 +870,14 @@ private ProcessedAnnotation createAnnotationValue(T originatingElement, ); } - @NotNull - private Map getCachedAnnotationDefaults(String annotationName, T annotationType) { + /** + * Get the cached annotation defaults. + * @param annotationName The annotation name + * @param annotationType The annotation type + * @return The defaults + */ + @NonNull + protected Map getCachedAnnotationDefaults(String annotationName, T annotationType) { Map defaultValues; final Map defaults = ANNOTATION_DEFAULTS.get(annotationName); if (defaults != null) { diff --git a/core/src/main/java/io/micronaut/core/convert/converters/MultiValuesConverterFactory.java b/core/src/main/java/io/micronaut/core/convert/converters/MultiValuesConverterFactory.java index 59305ccd3ae..c011a4a4e5d 100644 --- a/core/src/main/java/io/micronaut/core/convert/converters/MultiValuesConverterFactory.java +++ b/core/src/main/java/io/micronaut/core/convert/converters/MultiValuesConverterFactory.java @@ -257,15 +257,15 @@ public Optional convert( ArgumentConversionContext context = (ArgumentConversionContext) conversionContext; String format = conversionContext.getAnnotationMetadata() - .getValue(Format.class, String.class).orElse(null); + .stringValue(Format.class).orElse(null); if (format == null) { return Optional.empty(); } - String name = conversionContext.getAnnotationMetadata().getValue(Bindable.class, String.class) + String name = conversionContext.getAnnotationMetadata().stringValue(Bindable.class) .orElse(context.getArgument().getName()); String defaultValue = conversionContext.getAnnotationMetadata() - .getValue(Bindable.class, "defaultValue", String.class) + .stringValue(Bindable.class, "defaultValue") .orElse(null); switch (normalizeFormatName(format)) { @@ -524,7 +524,7 @@ private Optional convertValues(ArgumentConversionContext context Object[] constructorParameters = new Object[constructorArguments.length]; for (int i = 0; i < constructorArguments.length; ++i) { Argument argument = constructorArguments[i]; - String name = argument.getAnnotationMetadata().getValue(Bindable.class, String.class) + String name = argument.getAnnotationMetadata().stringValue(Bindable.class) .orElse(argument.getName()); constructorParameters[i] = conversionService.convert(values.get(name), ConversionContext.of(argument)) .orElse(null); @@ -575,12 +575,12 @@ public Optional convert( // noinspection unchecked ArgumentConversionContext context = (ArgumentConversionContext) conversionContext; - String format = conversionContext.getAnnotationMetadata().getValue(Format.class, String.class).orElse(null); + String format = conversionContext.getAnnotationMetadata().stringValue(Format.class).orElse(null); if (format == null) { return Optional.empty(); } - String name = conversionContext.getAnnotationMetadata().getValue(Bindable.class, String.class) + String name = conversionContext.getAnnotationMetadata().stringValue(Bindable.class) .orElse(context.getArgument().getName()); MutableConvertibleMultiValuesMap parameters = new MutableConvertibleMultiValuesMap<>(); @@ -775,7 +775,7 @@ private void processValues(ArgumentConversionContext context, } for (BeanProperty property: beanWrapper.getBeanProperties()) { - String key = property.getValue(Bindable.class, String.class).orElse(property.getName()); + String key = property.stringValue(Bindable.class).orElse(property.getName()); ArgumentConversionContext conversionContext = ConversionContext.STRING.with(property.getAnnotationMetadata()); conversionService.convert(property.get(object), conversionContext).ifPresent(value -> { diff --git a/http-client/src/test/resources/logback.xml b/http-client/src/test/resources/logback.xml index d15761dd727..f0acf1096e5 100644 --- a/http-client/src/test/resources/logback.xml +++ b/http-client/src/test/resources/logback.xml @@ -12,4 +12,6 @@ - \ No newline at end of file + + + diff --git a/inject-groovy/src/main/groovy/io/micronaut/ast/groovy/annotation/GroovyAnnotationMetadataBuilder.java b/inject-groovy/src/main/groovy/io/micronaut/ast/groovy/annotation/GroovyAnnotationMetadataBuilder.java index 914a5d089e4..c69c7979073 100644 --- a/inject-groovy/src/main/groovy/io/micronaut/ast/groovy/annotation/GroovyAnnotationMetadataBuilder.java +++ b/inject-groovy/src/main/groovy/io/micronaut/ast/groovy/annotation/GroovyAnnotationMetadataBuilder.java @@ -31,7 +31,6 @@ import io.micronaut.core.reflect.ClassUtils; import io.micronaut.core.reflect.ReflectionUtils; import io.micronaut.core.util.CollectionUtils; -import io.micronaut.core.util.StringUtils; import io.micronaut.inject.annotation.AbstractAnnotationMetadataBuilder; import io.micronaut.inject.annotation.AnnotatedElementValidator; import io.micronaut.inject.visitor.VisitorContext; @@ -376,9 +375,7 @@ protected void readAnnotationRawValues( if (expression instanceof ConstantExpression constantExpression) { final Object v = constantExpression.getValue(); if (v instanceof String s) { - if (StringUtils.isNotEmpty(s)) { - defaultValues.put(method, new ConstantExpression(v)); - } + defaultValues.put(method, new ConstantExpression(s)); } else if (v != null) { defaultValues.put(method, expression); } @@ -597,7 +594,8 @@ private Object convertConstantValue(Object value) { @Override protected Optional> getAnnotationValues(AnnotatedNode originatingElement, AnnotatedNode member, Class annotationType) { if (member != null) { - final List anns = member.getAnnotations(ClassHelper.make(annotationType)); + ClassNode annotationTypeNode = ClassHelper.make(annotationType); + final List anns = member.getAnnotations(annotationTypeNode); if (CollectionUtils.isNotEmpty(anns)) { AnnotationNode ann = anns.get(0); Map converted = new LinkedHashMap<>(); @@ -608,6 +606,17 @@ protected Optional> getAnnotationValue AnnotatedNode annotationMember = annotationNode.getMethod(key, new Parameter[0]); readAnnotationRawValues(originatingElement, annotationType.getName(), annotationMember, key, value, converted); } + Map annotationDefaults = getCachedAnnotationDefaults(annotationType.getName(), annotationTypeNode); + if (!annotationDefaults.isEmpty()) { + Iterator> i = converted.entrySet().iterator(); + while (i.hasNext()) { + Map.Entry next = i.next(); + Object v = annotationDefaults.get(next.getKey()); + if (v != null && v.equals(next.getValue())) { + i.remove(); + } + } + } return Optional.of(AnnotationValue.builder(annotationType).members(converted).build()); } } diff --git a/inject-groovy/src/test/groovy/io/micronaut/inject/configproperties/InheritedConfigurationReaderPrefixSpec.groovy b/inject-groovy/src/test/groovy/io/micronaut/inject/configproperties/InheritedConfigurationReaderPrefixSpec.groovy index e2b7209837e..66ea10f7753 100644 --- a/inject-groovy/src/test/groovy/io/micronaut/inject/configproperties/InheritedConfigurationReaderPrefixSpec.groovy +++ b/inject-groovy/src/test/groovy/io/micronaut/inject/configproperties/InheritedConfigurationReaderPrefixSpec.groovy @@ -37,7 +37,7 @@ class MyBean { beanDefinition.getInjectedMethods()[0].name == 'setMyValue' def metadata = beanDefinition.getInjectedMethods()[0].getAnnotationMetadata() metadata.hasAnnotation(Property) - metadata.getValue(Property, "name", String).get() == 'endpoints.my-value' + metadata.getValue(Property, "name", String).get() == 'simple.my-value' } void "property path is overriding the existing one without base prefix"() { @@ -75,7 +75,7 @@ class MyBean { beanDefinition.getInjectedMethods()[0].name == 'setMyValue' def metadata = beanDefinition.getInjectedMethods()[0].getAnnotationMetadata() metadata.hasAnnotation(Property) - metadata.getValue(Property, "name", String).get() == 'endpoints.my-value' + metadata.getValue(Property, "name", String).get() == 'endpoints.simple.my-value' } void "property path is overriding the existing one"() { diff --git a/inject-groovy/src/test/groovy/io/micronaut/inject/visitor/ClassElementSpec.groovy b/inject-groovy/src/test/groovy/io/micronaut/inject/visitor/ClassElementSpec.groovy index 4955b5e23f7..995f8a0a7c3 100644 --- a/inject-groovy/src/test/groovy/io/micronaut/inject/visitor/ClassElementSpec.groovy +++ b/inject-groovy/src/test/groovy/io/micronaut/inject/visitor/ClassElementSpec.groovy @@ -17,6 +17,7 @@ package io.micronaut.inject.visitor import io.micronaut.ast.groovy.TypeElementVisitorStart import io.micronaut.ast.transform.test.AbstractBeanDefinitionSpec +import io.micronaut.context.annotation.Replaces import io.micronaut.context.exceptions.BeanContextException import io.micronaut.core.annotation.Introspected import io.micronaut.inject.ast.ClassElement @@ -1996,6 +1997,56 @@ class MyBean { returnType.hasAnnotation(Introspected.class) } + void "test alias for recursion"() { + given: + ClassElement ce = buildClassElement('''\ +package test; + +import io.micronaut.inject.annotation.*; +import io.micronaut.context.annotation.* +import io.micronaut.test.annotation.MockBean; +import jakarta.inject.Singleton; +import jakarta.inject.Inject; +import java.util.List; +import java.lang.Integer; + +interface MathService { + + Integer compute(Integer num); +} + +@Singleton +class MathServiceImpl implements MathService { + + @Override + Integer compute(Integer num) { + return num * 4 // should never be called + } +} + +@Singleton +class MathInnerServiceSpec { + + @Inject + MathService mathService + + @MockBean(MathService) + static class MyMock implements MathService { + + @Override + Integer compute(Integer num) { + return 50 + } + } +} + +''') + when: + def replaces = ce.getAnnotation(Replaces) + then: + replaces.stringValue("bean").get() == "test.MathService" + } + void "test how the type annotations from the type are propagated"() { given: ClassElement ce = buildClassElement('''\ diff --git a/inject-java/src/test/groovy/io/micronaut/visitors/ClassElementSpec.groovy b/inject-java/src/test/groovy/io/micronaut/visitors/ClassElementSpec.groovy index 61fa2b68b18..56a0fae819f 100644 --- a/inject-java/src/test/groovy/io/micronaut/visitors/ClassElementSpec.groovy +++ b/inject-java/src/test/groovy/io/micronaut/visitors/ClassElementSpec.groovy @@ -17,6 +17,7 @@ package io.micronaut.visitors import io.micronaut.annotation.processing.test.AbstractTypeElementSpec import io.micronaut.annotation.processing.visitor.JavaClassElement +import io.micronaut.context.annotation.Replaces import io.micronaut.context.exceptions.BeanContextException import io.micronaut.core.annotation.AnnotationUtil import io.micronaut.core.annotation.Introspected @@ -2302,6 +2303,57 @@ class MyBean { returnType.hasAnnotation(Introspected.class) } + void "test alias for recursion"() { + given: + ClassElement ce = buildClassElement('''\ +package test; + +import io.micronaut.inject.annotation.*; +import io.micronaut.context.annotation.*; +import io.micronaut.test.annotation.MockBean; +import jakarta.inject.Singleton; +import jakarta.inject.Inject; +import java.util.List; +import java.lang.Integer; + +@Singleton +class MathInnerServiceSpec { + + @Inject + MathService mathService; + + @MockBean(MathService.class) + static class MyMock implements MathService { + + @Override + public Integer compute(Integer num) { + return 50; + } + } +} + +interface MathService { + + Integer compute(Integer num); +} + + +@Singleton +class MathServiceImpl implements MathService { + + @Override + public Integer compute(Integer num) { + return num * 4; // should never be called + } +} + +''') + when: + def replaces = ce.getEnclosedElements(ElementQuery.ALL_INNER_CLASSES).get(0).getAnnotation(Replaces) + then: + replaces.stringValue("bean").get() == "test.MathService" + } + void "test how the type annotations from the type are propagated"() { given: ClassElement ce = buildClassElement('''\ diff --git a/inject/src/main/java/io/micronaut/context/DefaultBeanContext.java b/inject/src/main/java/io/micronaut/context/DefaultBeanContext.java index 2173c0fd3ac..58826ef7943 100644 --- a/inject/src/main/java/io/micronaut/context/DefaultBeanContext.java +++ b/inject/src/main/java/io/micronaut/context/DefaultBeanContext.java @@ -110,7 +110,6 @@ import io.micronaut.inject.QualifiedBeanType; import io.micronaut.inject.ValidatedBeanDefinition; import io.micronaut.inject.provider.AbstractProviderDefinition; -import io.micronaut.inject.provider.BeanProviderDefinition; import io.micronaut.inject.proxy.InterceptedBeanProxy; import io.micronaut.inject.qualifiers.AnyQualifier; import io.micronaut.inject.qualifiers.Qualified; diff --git a/inject/src/main/java/io/micronaut/context/annotation/Replaces.java b/inject/src/main/java/io/micronaut/context/annotation/Replaces.java index e4fca881933..ad30072fef5 100644 --- a/inject/src/main/java/io/micronaut/context/annotation/Replaces.java +++ b/inject/src/main/java/io/micronaut/context/annotation/Replaces.java @@ -46,6 +46,7 @@ /** * @return The bean type that this bean replaces */ + @AliasFor(member = "value") Class bean() default void.class; /** diff --git a/test-suite/src/test/groovy/io/micronaut/test/replacesbug/MathInnerServiceSpec.groovy b/test-suite/src/test/groovy/io/micronaut/test/replacesbug/MathInnerServiceSpec.groovy new file mode 100644 index 00000000000..93c5b1c2d66 --- /dev/null +++ b/test-suite/src/test/groovy/io/micronaut/test/replacesbug/MathInnerServiceSpec.groovy @@ -0,0 +1,46 @@ +package io.micronaut.test.replacesbug + +import io.micronaut.test.extensions.spock.annotation.MicronautTest +import io.micronaut.test.annotation.MockBean +import jakarta.inject.Singleton +import spock.lang.Specification + +import jakarta.inject.Inject + +@MicronautTest +class MathInnerServiceSpec extends Specification { + + @Inject + MathService mathService + + void "should compute use inner mock"() { + when: + def result = mathService.compute(10) + + then: + result == 50 + } + + @MockBean(MathService) + static class MyMock implements MathService { + + @Override + Integer compute(Integer num) { + return 50 + } + } +} + +interface MathService { + + Integer compute(Integer num); +} + +@Singleton +class MathServiceImpl implements MathService { + + @Override + Integer compute(Integer num) { + return num * 4 // should never be called + } +}