Add simple Spring DI support #563

geoand · 2019-01-18T11:24:30Z

Support is provided my mapping Spring DI annotations to their CDI
counterparts leveraging the annotation transformation mechanism that
Arc provides

geoand · 2019-01-18T11:59:16Z

I can't see the logs of CI process unfortunately.
Also when I run the build on my machine it works just fine and the CI build seems to fail very quickly which seems fishy :)

mkouba · 2019-01-18T12:50:28Z

...ring-di/deployment/src/main/java/org/jboss/shamrock/spring/deployment/SpringDIProcessor.java

+    private static final DotName VALUE_ANNOTATION
+            = DotName.createSimple("org.springframework.beans.factory.annotation.Value");
+
+    private static final DotName CDI_SINGLETON_ANNOTATION = DotName.createSimple(Singleton.class.getName());


You can use org.jboss.protean.arc.processor.ScopeInfo#getDotName() and org.jboss.protean.arc.processor.DotNames.INJECT and friends instead.

Will do that!

mkouba · 2019-01-18T12:53:01Z

I can see this:

2019-01-18T11:56:52.4616604Z [FATAL] Non-resolvable parent POM for org.jboss.shamrock:shamrock-spring-deployment:[unknown-version]: Could not find artifact org.jboss.shamrock:shamrock-spring:pom:1.0.0.Alpha1-SNAPSHOT in jboss-public-repository-group (http://repository.jboss.org/nexus/content/groups/developer/) and 'parent.relativePath' points at wrong local POM @ line 5, column 13
2019-01-18T11:56:52.4617877Z [WARNING] 'dependencies.dependency.(groupId:artifactId:type:classifier)' must be unique: org.jboss.shamrock:shamrock-arc-deployment:jar -> duplicate declaration of version (?) @ org.jboss.shamrock:shamrock-agroal-deployment:[unknown-version], /home/vsts/work/1/s/extensions/agroal/deployment/pom.xml, line 45, column 21

geoand · 2019-01-18T12:53:45Z

I can see this:

2019-01-18T11:56:52.4616604Z [FATAL] Non-resolvable parent POM for org.jboss.shamrock:shamrock-spring-deployment:[unknown-version]: Could not find artifact org.jboss.shamrock:shamrock-spring:pom:1.0.0.Alpha1-SNAPSHOT in jboss-public-repository-group (http://repository.jboss.org/nexus/content/groups/developer/) and 'parent.relativePath' points at wrong local POM @ line 5, column 13
2019-01-18T11:56:52.4617877Z [WARNING] 'dependencies.dependency.(groupId:artifactId:type:classifier)' must be unique: org.jboss.shamrock:shamrock-arc-deployment:jar -> duplicate declaration of version (?) @ org.jboss.shamrock:shamrock-agroal-deployment:[unknown-version], /home/vsts/work/1/s/extensions/agroal/deployment/pom.xml, line 45, column 21

Very helpful, thanks!

geoand · 2019-01-18T14:08:51Z

@mkouba Would be so kind as to paste the logs of the failed test?
Seems like something else failed this time.

Thanks a lot and sorry for the inconvenience!

mkouba · 2019-01-18T14:12:29Z

@geoand No problem. The log should be visible here: https://dev.azure.com/protean-ci/Shamrock/_build/results?buildId=1267 (click on "1 errors / 0 warnings" link in the azure check detail)

2019-01-18T13:59:07.9251386Z [INFO] Running org.jboss.shamrock.spring.tests.InjectedSpringBeansResourceIT
2019-01-18T13:59:07.9814472Z [ERROR] Tests run: 1, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 0.054 s <<< FAILURE! - in org.jboss.shamrock.spring.tests.InjectedSpringBeansResourceIT
2019-01-18T13:59:07.9815203Z [ERROR] initializationError(org.jboss.shamrock.spring.tests.InjectedSpringBeansResourceIT)  Time elapsed: 0.012 s  <<< ERROR!
2019-01-18T13:59:07.9815529Z java.lang.Exception: No runnable methods

geoand · 2019-01-18T14:15:23Z

@geoand No problem. The log should be visible here: https://dev.azure.com/protean-ci/Shamrock/_build/results?buildId=1267 (click on "1 errors / 0 warnings" link in the azure check detail)

2019-01-18T13:59:07.9251386Z [INFO] Running org.jboss.shamrock.spring.tests.InjectedSpringBeansResourceIT
2019-01-18T13:59:07.9814472Z [ERROR] Tests run: 1, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 0.054 s <<< FAILURE! - in org.jboss.shamrock.spring.tests.InjectedSpringBeansResourceIT
2019-01-18T13:59:07.9815203Z [ERROR] initializationError(org.jboss.shamrock.spring.tests.InjectedSpringBeansResourceIT)  Time elapsed: 0.012 s  <<< ERROR!
2019-01-18T13:59:07.9815529Z java.lang.Exception: No runnable methods

Thanks!
Silly me, didn't write the substrate test correctly.
Should be fixed now

mkouba · 2019-01-18T16:36:19Z

...projects/arc/processor/src/main/java/org/jboss/protean/arc/processor/InjectionPointInfo.java

@@ -39,7 +39,7 @@

    static InjectionPointInfo fromField(FieldInfo field, BeanDeployment beanDeployment) {
        Set<AnnotationInstance> qualifiers = new HashSet<>();
-        for (AnnotationInstance annotation : field.annotations()) {
+        for (AnnotationInstance annotation : beanDeployment.getAnnotations(field)) {


I think we should update fromMethod() as well, in order to support setter/constructor injection. I'd guess it is also supported by Spring DI, or?

In fact, AnnotationsTransformer method params support is not that good. We should improve this.

If I am not mistaken AnnotationTransformer doesn't currently support method parameters at all. Would you like me to address that with this PR?

And to answer you other question, yes Spring DI does allow annotations like (@Qualifier) on method params (and obviously this PR doesn't handle them)

Maybe the transformation of annotations method params should be disabled and only enabled by a config property than extension would set?
If you think we should have method param annotation handling, I will gladly add it with PR and update the Spring processor to handle such annotations

I played around a little more with this and it turned out that adding support for method param qualifiers was fairly easy - so it was added in my latest commit

Support is provided my mapping Spring DI annotations to their CDI counterparts leveraging the annotation transformation mechanism that Arc provides

stuartwdouglas

This extension should also be added to the bom, to make it easier for end users to consumer. Other than these issues I think it looks good, but I would like @mkouba to have the final say.

stuartwdouglas · 2019-01-21T00:30:13Z

integration-tests/spring-di/pom.xml

+        <dependency>
+            <groupId>org.springframework</groupId>
+            <artifactId>spring-context</artifactId>
+            <version>5.1.4.RELEASE</version>


This should be managed in the build parent

stuartwdouglas · 2019-01-21T00:32:03Z

...ring-di/deployment/src/main/java/org/jboss/shamrock/spring/deployment/SpringDIProcessor.java

+
+            private String determineName(AnnotationValue annotationValue) {
+                final String className = annotationValue.getClass().getName();
+                if (className.contains("ArrayValue")) {


This should use AnnotationValue.kind()

geoand · 2019-01-21T09:19:36Z

Should I perhaps create an issue for this and link it to this PR?
I mean just for the release notes

geoand force-pushed the basic-spring-di branch 3 times, most recently from c4426de to 2119ada Compare January 18, 2019 11:55

mkouba reviewed Jan 18, 2019

View reviewed changes

geoand force-pushed the basic-spring-di branch from 54e5317 to 8043c76 Compare January 20, 2019 11:42

Add simple Spring DI support

b842d28

Support is provided my mapping Spring DI annotations to their CDI counterparts leveraging the annotation transformation mechanism that Arc provides

geoand force-pushed the basic-spring-di branch from 8043c76 to b842d28 Compare January 20, 2019 11:44

stuartwdouglas requested changes Jan 21, 2019

View reviewed changes

Incorporate review comments

9379f77

stuartwdouglas approved these changes Jan 21, 2019

View reviewed changes

mkouba approved these changes Jan 21, 2019

View reviewed changes

mkouba merged commit e2b2c88 into quarkusio:master Jan 21, 2019

geoand deleted the basic-spring-di branch January 21, 2019 09:18

stuartwdouglas added this to the 0.7.0 milestone Jan 21, 2019

geoand added the area/spring Issues relating to the Spring integration label Sep 16, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add simple Spring DI support #563

Add simple Spring DI support #563

geoand commented Jan 18, 2019

geoand commented Jan 18, 2019 •

edited

Loading

mkouba Jan 18, 2019

geoand Jan 18, 2019

geoand Jan 18, 2019

mkouba commented Jan 18, 2019

geoand commented Jan 18, 2019

geoand commented Jan 18, 2019

mkouba commented Jan 18, 2019

geoand commented Jan 18, 2019

mkouba Jan 18, 2019

geoand Jan 18, 2019 •

edited

Loading

geoand Jan 18, 2019

geoand Jan 18, 2019 •

edited

Loading

stuartwdouglas left a comment

stuartwdouglas Jan 21, 2019

geoand Jan 21, 2019

stuartwdouglas Jan 21, 2019

geoand Jan 21, 2019

geoand commented Jan 21, 2019

Add simple Spring DI support #563

Add simple Spring DI support #563

Conversation

geoand commented Jan 18, 2019

geoand commented Jan 18, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mkouba commented Jan 18, 2019

geoand commented Jan 18, 2019

geoand commented Jan 18, 2019

mkouba commented Jan 18, 2019

geoand commented Jan 18, 2019

Choose a reason for hiding this comment

geoand Jan 18, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

geoand Jan 18, 2019 • edited Loading

Choose a reason for hiding this comment

stuartwdouglas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

geoand commented Jan 21, 2019

geoand commented Jan 18, 2019 •

edited

Loading

geoand Jan 18, 2019 •

edited

Loading

geoand Jan 18, 2019 •

edited

Loading