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

fix: consider current package when generating bean imports #10236

Open
wants to merge 2 commits into
base: 4.3.x
Choose a base branch
from

Conversation

auke-
Copy link
Contributor

@auke- auke- commented Dec 8, 2023

The internal bean element implementation didn't implement the methods for the determination of the visibility of the class, which resulted in the use of reflection for the bean definitions generated for the bean import.

By implementing the visibility methods the generated code for the imported bean definitions don't use reflection when the fields are visible.

I've tried to add a test to inject-java-test/src/test/groovy/io/micronaut/inject/beanimport/BeanImportSpec.groovy but i'm unable to determine if reflection is used, this information isn't available in the BeanDefinition:

    void "test bean import for current package"() {
        given:
        ApplicationContext context = buildContext('''
package io.micronaut.inject.beanimport;

import io.micronaut.context.annotation.*;
import jakarta.inject.Named;
import java.nio.charset.StandardCharsets;

@Import(classes=io.micronaut.inject.beanimport.BeanWithFields.class)
class Application {}
''')
        def bean = context.getBeanDefinition(BeanWithFields)

        expect:
        bean.getInjectedFields()[0].getName() == 'publicField' // How to determine that reflection isn't used?
        bean.getInjectedFields()[1].getName() == 'packageField' // How to determine that reflection isn't used?
        bean.getInjectedFields()[2].getName() == 'protectedField' // How to determine that reflection is used?
        bean.getInjectedFields()[3].getName() == 'privateField'  // How to determine that reflection is used?
    }

@Singleton
class BeanWithFields {
    public @Named("public") String publicField;
    @Named("package") String packageField;
    protected @Named("protected") String protectedField;
    private @Named("private") String privateField;
}

@graemerocher
Copy link
Contributor

could you add a test?

@graemerocher
Copy link
Contributor

Seems like the change broke something, so the logic is not quite correct

@auke- auke- changed the base branch from 4.2.x to 4.3.x December 14, 2023 10:43
@auke- auke- changed the base branch from 4.3.x to 4.2.x December 14, 2023 11:47
@auke-
Copy link
Contributor Author

auke- commented Dec 14, 2023

I think my change exposed a bug in core-processor/src/main/java/io/micronaut/inject/ast/utils/EnclosedElementsQuery.java which didn't filter the package private elements in case the OnlyAccessibleFromType was set.

I've noticed that the 4.3.x branch contains a major change to EnclosedElementsQuery.java and does seem to work with my change to AbstractBeanDefinitionBuilder but also doesn't consider OnlyAccessibleFromType.

I would like to add a test, but i'm not sure where to start since the result of the changes are a change in the generated bytecode and i'm unable to assert that the generated bean definition isn't using reflection for the package private fields.

(and i was triggering the wrong shortcuts which closed and re-openend this merge request, sorry about that)

@auke- auke- closed this Dec 14, 2023
@auke- auke- reopened this Dec 14, 2023
@CLAassistant
Copy link

CLAassistant commented Feb 7, 2024

CLA assistant check
All committers have signed the CLA.

The internal bean element implementation didn't implement the methods
for the determination of the visibility of the class, which resulted in
the use of reflection for the bean definitions generated for the bean
import.

By implementing the visibility methods the generated code for the
imported bean definitions don't use reflection when the fields are
visible.
The enclosed element query didn't consider the type from which the
element is accessible, which made package private elements being
filtered out of the result even though they where visible for the type.
@auke- auke- changed the base branch from 4.2.x to 4.3.x February 8, 2024 13:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants