Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue with Requires condition with missingClasses as string values #11087

Merged
merged 7 commits into from
Aug 16, 2024

Conversation

radovanradic
Copy link
Contributor

No description provided.

package test;

import io.micronaut.context.annotation.*;

Copy link
Contributor Author

@radovanradic radovanradic Aug 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like there is an issue in micronaut-core 4.6.x when declaring something like this
@Requires(missingClasses = "org.hibernate.reactive.provider.ReactiveServiceRegistryBuilder") because this code in MatchesConditionUtils

if (requirement.contains(RequiresCondition.MEMBER_MISSING_CLASSES)) {
            AnnotationClassValue<?>[] classes = requirement.annotationClassValues(RequiresCondition.MEMBER_MISSING_CLASSES);
            if (classes.length > 0) {
                preConditions.add(new MatchesAbsenceOfClassesCondition(classes));
            }
        }

expects to get classes in annotation value

public AnnotationClassValue<?>[] annotationClassValues(@NonNull String member) {
        if (StringUtils.isEmpty(member)) {
            return AnnotationClassValue.ZERO_ANNOTATION_CLASS_VALUES;
        }
        Object o = values.get(member);
        if (o instanceof AnnotationClassValue<?> annotationClassValue) {
            return new AnnotationClassValue[]{annotationClassValue};
        }
        if (o instanceof AnnotationClassValue<?>[] annotationClassValues) {
            return annotationClassValues;
        }
        return AnnotationClassValue.ZERO_ANNOTATION_CLASS_VALUES;
    }

in this case, instanceof returns AnnotationClassValue.ZERO_ANNOTATION_CLASS_VALUES since value is string and then fails to match condition, actually condition will not be evaluated.

@dstepanov
Copy link
Contributor

The "problem" is actually with:

if (requirement.contains(RequiresCondition.MEMBER_MISSING_CLASSES)) {
            AnnotationClassValue<?>[] classes = requirement.annotationClassValues(RequiresCondition.MEMBER_MISSING_CLASSES);
            if (classes.length > 0) {
                preConditions.add(new MatchesAbsenceOfClassesCondition(classes));
            }
        }

Previous version would use stringValues there.

But I think your fix is also correct, you might want to fix and test the singular method annotationClassValue below. Please extract a private method to create AnnotationClassValue from a class name.

@radovanradic
Copy link
Contributor Author

The "problem" is actually with:

if (requirement.contains(RequiresCondition.MEMBER_MISSING_CLASSES)) {
            AnnotationClassValue<?>[] classes = requirement.annotationClassValues(RequiresCondition.MEMBER_MISSING_CLASSES);
            if (classes.length > 0) {
                preConditions.add(new MatchesAbsenceOfClassesCondition(classes));
            }
        }

Previous version would use stringValues there.

But I think your fix is also correct, you might want to fix and test the singular method annotationClassValue below. Please extract a private method to create AnnotationClassValue from a class name.

Added util method, will improve coverage as well.
Singular method to create AnnotationClassValue is used for @Requires(bean = Class) or @Requires(condition = Class) and can't get class names. Only @Requires field possibly taking strings is classNames:

@AliasFor(member = "missing")
String[] missingClasses() default {};

* @return AnnotationClassValue for given class name
*/
private static AnnotationClassValue<?> getAnnotationClassValue(String className) {
Optional<Class<?>> theClass = ClassUtils.forName(className, null);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The difference from previous version is that it was using classloader from beanContext.getClassLoader() and this will fallback to current thread classloader.

if (classLoader == null) {
                classLoader = Thread.currentThread().getContextClassLoader();
            }
            if (classLoader == null) {
                classLoader = ClassLoader.getSystemClassLoader();
            }

Not sure if this could cause other issues.

@radovanradic radovanradic marked this pull request as ready for review August 15, 2024 09:06
gradle.properties Outdated Show resolved Hide resolved
@radovanradic radovanradic added the type: bug Something isn't working label Aug 15, 2024
Copy link

sonarcloud bot commented Aug 15, 2024

@sdelamo sdelamo merged commit cfa80a6 into 4.6.x Aug 16, 2024
17 checks passed
@sdelamo sdelamo deleted the missing-classes-issue branch August 16, 2024 07:22
@rsalbergaria
Copy link

@sdelamo and @radovanradic, could the issue reported be related to the problem described in micronaut-projects/micronaut-platform/discussions/1704?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Something isn't working
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants