Add DependencyInjector to inject dependencies in annotations #4202

minwoox · 2022-04-11T14:47:51Z

Motivation:
Users cannot inject an instance, which has constructor parameters, through @Decorator and other annotations.
It would be nice if we provide a way to inject dependencies.

Modifications:

Add DependencyInjector to inject the instances that are specified in an annotated service.
- If a DependencyInjector is not set, ReflectiveDependencyInjector is used to keep the backward-compatibility
- If a DependencyInjector is set, BuiltInDependencyInjector is applied at first to support built-in classes.
Add SpringDependencyInjector that injects using BeanFactory.
- You can set armeria.enable-auto-injection to true to apply SpringDependencyInjector.
- You can also create a bean for DependencyInjector.

Result:

Close Provide a way to inject an instance for @Decorator #4006

You can now inject dependencies using DependencyInjector to annotated services.

// Inject authClient that is needed to create the AuthDecorator.
WebClient authClient = ...
DependencyInjector injector = DependencyInjector.ofSingletons(new AuthDecorator(authClient));
serverBuilder.dependencyInjector(dependencyInjector, true);

// An annotated service that uses AuthDecorator.
@Get("/foo")
@Decorator(AuthDecorator.class)
public FooResponse foo(FooRequest req) {
    // Authrorized request.
    ...
}

// authClient is injected.
class AuthDecorator implements DecoratingHttpServiceFunction {
    AuthDecorator(WebClient authClient) { ... }

    @Override
    public HttpResponse serve(HttpService delegate, ServiceRequestContext ctx, HttpRequest req)
            throws Exception {
        // Authorize the request.
        ...
    }
}

Motivation: Users cannot inject an instance, which has constructor parameters, through `@Decorator` and other anntations. It would be nice if we provide a way to inject dependencies. Modifications: - Add `DependencyInjector` to inject the instances that are specified in an annotated service. - Spring integration module automatically adds `SpringDependencyInjector` to inject the depdendencies defined via bean definition. Result: - Close line#4006 - You can now inject dependencies that have constructor parameters to an annotated service.

codecov · 2022-04-11T15:56:22Z

Codecov Report

Merging #4202 (a35ca48) into master (554d596) will increase coverage by 0.00%.
The diff coverage is 74.58%.

@@            Coverage Diff             @@
##             master    #4202    +/-   ##
==========================================
  Coverage     73.44%   73.44%            
- Complexity    17395    17443    +48     
==========================================
  Files          1482     1488     +6     
  Lines         65296    65448   +152     
  Branches       8207     8221    +14     
==========================================
+ Hits          47954    48067   +113     
- Misses        13169    13201    +32     
- Partials       4173     4180     +7

Impacted Files	Coverage Δ
...ria/internal/server/annotation/AnnotationUtil.java	`89.05% <ø> (ø)`
...java/com/linecorp/armeria/server/ServerConfig.java	`0.00% <ø> (ø)`
...ver/VirtualHostAnnotatedServiceBindingBuilder.java	`53.01% <ø> (ø)`
...ia/server/annotation/ExceptionHandlerFunction.java	`100.00% <ø> (ø)`
...ia/server/annotation/RequestConverterFunction.java	`100.00% <ø> (ø)`
...a/server/annotation/ResponseConverterFunction.java	`100.00% <ø> (ø)`
...rnal/server/annotation/AnnotatedObjectFactory.java	`33.33% <33.33%> (-18.85%)`	⬇️
...ecorp/armeria/common/OrElseDependencyInjector.java	`41.17% <41.17%> (ø)`
...ria/internal/common/BuiltInDependencyInjector.java	`41.66% <41.66%> (ø)`
...corp/armeria/common/DefaultDependencyInjector.java	`50.00% <50.00%> (ø)`
... and 37 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 554d596...a35ca48. Read the comment docs.

ikhoon · 2022-04-12T06:40:15Z

...toconfigure/src/main/java/com/linecorp/armeria/internal/spring/SpringDependencyInjector.java

+    @Override
+    public <T> T getInstance(Class<T> type) {
+        try {
+            return beanFactory.getBean(type);


Would be useful to inject a decorator with a name?

@Decorator(FooDecorator.class, "foo1Decorator") public String foo(); // Will be chosen @Bean(name = "foo1Decorator") FooDecorator foo1() { ... } @Bean(name = "foo2Decorator") FooDecorator foo2() { ... }

Thanks, that's a good suggestion. 👍
However, can I just leave it as a TODO and implement it later because:

We haven't had such demands so far

It increases the complexity of the implementation

Users can easily apply the different instances using @DecoratorFactory

Makes sense. We can add the feature later without hurting the original API.

Would naming a bean work for other DI frameworks by the way? (Guice, Dagger, etc)

Guice and Dagger supports @Named bean injection as well.
https://github.com/google/guice/wiki/BindingAnnotations#named

core/src/main/java/com/linecorp/armeria/server/DependencyInjector.java

core/src/main/java/com/linecorp/armeria/server/DefaultDependencyInjector.java

core/src/main/java/com/linecorp/armeria/server/OrElseDependencyInjector.java

ikhoon

Overall, looking good. Left minor comments.

core/src/main/java/com/linecorp/armeria/server/DependencyInjectorBuilder.java

core/src/main/java/com/linecorp/armeria/server/FallbackDependencyInjector.java

core/src/main/java/com/linecorp/armeria/server/DefaultDependencyInjector.java

core/src/main/java/com/linecorp/armeria/server/ServerBuilder.java

ikhoon · 2022-04-15T07:09:58Z

...toconfigure/src/main/java/com/linecorp/armeria/internal/spring/SpringDependencyInjector.java

+    @Override
+    public <T> T getInstance(Class<T> type) {
+        try {
+            return beanFactory.getBean(type);


Makes sense. We can add the feature later without hurting the original API.

core/src/main/java/com/linecorp/armeria/server/DefaultDependencyInjector.java

core/src/main/java/com/linecorp/armeria/server/FallbackDependencyInjector.java

core/src/main/java/com/linecorp/armeria/server/Server.java

core/src/main/java/com/linecorp/armeria/server/ServerBuilder.java

jrhee17

Left a nit comment, but happy with the changes once @ikhoon , @trustin 's comments are addressed 👍 Thanks @minwoox ! 🙇 👍 🙇

...toconfigure/src/main/java/com/linecorp/armeria/internal/spring/SpringDependencyInjector.java

Motivation Currently, depedencies for decorators are injected when the class is set to the `GrpcServiceBuilder`. However, we need to use the `DependencyInjector` that will be introduced in line#4202. The `DependencyInjector` is set to the `Server` so we need to delay to inject dependencies until the `serviceAdded` method is called which means we can get the injector. Modifications: - Refactor `GrpcServiceBuilder` for adding decorators - Use `lookupMethodFromAttribute` where applicable - Fix a bug that decorator is not applied when `grpcBuilder.addService(path, service, method)` is called. - Inject dependencies in `GrpcDecoratingService.serviceAdded()` Result: - Ready to apply line#4202 - Decorator is correctly applied when a method is added via `GrpcServiceBuilder.addService(String,BindableService,MethodDescriptor)`.

…ratingService (#4228) Motivation Currently, dependencies for decorators are injected when the class is set to the `GrpcServiceBuilder`. However, we need to use the `DependencyInjector` that will be introduced in #4202. The `DependencyInjector` is set to the `Server` so we need to delay injecting dependencies until the `serviceAdded` method is called which means we can get the injector. Modifications: - Refactor `GrpcServiceBuilder` for adding decorators - Fix a bug that decorator is not applied when `grpcBuilder.addService(path, service, method)` is called. - The path, which is used to set a service, is now correctly normalized. - Inject dependencies in `GrpcDecoratingService.serviceAdded()` Result: - Ready to apply #4202 - Decorator is correctly applied when a method is added via `GrpcServiceBuilder.addService(String,BindableService,MethodDescriptor)`.

core/src/main/java/com/linecorp/armeria/server/DependencyInjector.java

core/src/main/java/com/linecorp/armeria/server/DependencyInjectorEntry.java

core/src/main/java/com/linecorp/armeria/internal/server/annotation/AnnotatedObjectFactory.java

core/src/main/java/com/linecorp/armeria/server/DependencyInjectorEntry.java

minwoox · 2022-06-16T07:28:15Z

Updated, PTAL. 🙇‍♂️

core/src/main/java/com/linecorp/armeria/server/BuiltInDependencyInjector.java

core/src/main/java/com/linecorp/armeria/server/DefaultDependencyInjector.java

core/src/main/java/com/linecorp/armeria/server/ReflectiveDependencyInjector.java

core/src/main/java/com/linecorp/armeria/server/ShutdownSupport.java

...toconfigure/src/main/java/com/linecorp/armeria/internal/spring/ArmeriaConfigurationUtil.java

jrhee17

Looks good! Only left nits and a question 🙇

core/src/main/java/com/linecorp/armeria/server/ServerBuilder.java

core/src/main/java/com/linecorp/armeria/server/DefaultDependencyInjector.java

core/src/main/java/com/linecorp/armeria/server/ServerBuilder.java

core/src/main/java/com/linecorp/armeria/server/BuiltInDependencyInjector.java

ikhoon

Awesome!! 🍃🎉
Spring users will love this feature.

core/src/main/java/com/linecorp/armeria/server/DefaultServerConfig.java

core/src/main/java/com/linecorp/armeria/server/DependencyInjector.java

ikhoon · 2022-07-01T08:21:17Z

core/src/main/java/com/linecorp/armeria/server/ServerConfig.java

@@ -289,4 +289,9 @@ default boolean shutdownBlockingTaskExecutorOnStop() {
     * supports case sensitive HTTP/1 headers.
     */
    Http1HeaderNaming http1HeaderNaming();
+
+    /**
+     * Returns the {@link DependencyInjector} that injects dependencies in annotated services.


Should the DependencyInjector be used in a gRPC service?

That's a good point. 😉 We might support adding annotations to a Thrift service later so let me change the sentence to injects dependencies in annotations

trustin · 2022-07-01T13:05:44Z

core/src/main/java/com/linecorp/armeria/server/DependencyInjector.java

+ * constructor, which does not have a parameter, of the classes.
+ */
+@UnstableApi
+public interface DependencyInjector extends SafeCloseable {


Would it make sense to move this interface to common so we can use this for both client and server side? For example, in the future, we could introduce a client-side (or common) @Decorator annotation and let our users use it with Retrofit or our future Retrofit alternative.

That's a good suggestion. 👍 Moved the package. 😉

core/src/main/java/com/linecorp/armeria/server/OrElseDependencyInjector.java

core/src/main/java/com/linecorp/armeria/server/ServerBuilder.java

jrhee17

Should be a really useful feature! Thanks @minwoox 🙇 👍 🙇

ikhoon

Still LGTM with new changes.

trustin

Thanks for this awesome feature, @minwoox! 👍

minwoox · 2022-07-05T06:04:33Z

Thanks, reviewers! 😉

minwoox added the new feature label Apr 11, 2022

minwoox added this to the 1.16.0 milestone Apr 11, 2022

minwoox requested review from ikhoon, jrhee17 and trustin as code owners April 11, 2022 14:47

Add UnstableApi

6af8a24

ikhoon reviewed Apr 12, 2022

View reviewed changes

minwoox added 2 commits April 13, 2022 00:14

Address comments by @ikhoon

26009b0

Fix

571fd5a

ikhoon reviewed Apr 15, 2022

View reviewed changes

trustin reviewed Apr 16, 2022

View reviewed changes

jrhee17 approved these changes Apr 17, 2022

View reviewed changes

...toconfigure/src/main/java/com/linecorp/armeria/internal/spring/SpringDependencyInjector.java Outdated Show resolved Hide resolved

ikhoon modified the milestones: 1.16.0, 1.17.0 Apr 18, 2022

Address comments by @ikhoon, @trustin and @jrhee17

33b05cb

minwoox mentioned this pull request Apr 26, 2022

Delay injecting dependencies until serviceAdded is called in GrpcDecoratingService #4228

Merged

Merge branch 'master' into DI

73282df

minwoox added 2 commits May 25, 2022 10:32

Merge branch 'master' into DI

035cb2f

Fix indent

a38280e

ikhoon reviewed May 27, 2022

View reviewed changes

ikhoon reviewed May 31, 2022

View reviewed changes

core/src/main/java/com/linecorp/armeria/internal/server/annotation/AnnotatedObjectFactory.java Outdated Show resolved Hide resolved

core/src/main/java/com/linecorp/armeria/server/DependencyInjectorEntry.java Outdated Show resolved Hide resolved

minwoox mentioned this pull request Jun 16, 2022

Chain the AccessLogWriter if another one is previously set #4280

Merged

minwoox added 2 commits June 16, 2022 10:15

Merge branch 'master' into DI

85e2f71

Apply ShutdownSupport

4dd986e

minwoox added 2 commits June 16, 2022 16:37

Fix Javadoc

d73914b

Add BuiltInDependencyInjector

b37d96b

ta7uw mentioned this pull request Jun 27, 2022

Provide a way to decorate a gRPC service using a builder method #4316

Merged

ikhoon reviewed Jun 28, 2022

View reviewed changes

jrhee17 self-requested a review June 30, 2022 01:26

Address comments by @ikhoon

0eb885d

jrhee17 reviewed Jul 1, 2022

View reviewed changes

ikhoon approved these changes Jul 1, 2022

View reviewed changes

Address comments by @ikhoon and @jrhee17

1261318

minwoox force-pushed the DI branch from 5861a2c to 1261318 Compare July 1, 2022 08:44

minwoox changed the title ~~Add DependencyInjector to inject dependencies for an annotated service~~ Add DependencyInjector to inject dependencies in annotations Jul 1, 2022

trustin reviewed Jul 1, 2022

View reviewed changes

minwoox added 2 commits July 4, 2022 10:59

Move to common package

828d85b

Merge branch 'master' into DI

a35ca48

minwoox force-pushed the DI branch from c493e23 to a35ca48 Compare July 4, 2022 01:59

jrhee17 approved these changes Jul 4, 2022

View reviewed changes

ikhoon reviewed Jul 4, 2022

View reviewed changes

trustin approved these changes Jul 5, 2022

View reviewed changes

minwoox merged commit 91c787f into line:master Jul 5, 2022

minwoox deleted the DI branch July 5, 2022 06:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add DependencyInjector to inject dependencies in annotations #4202

Add DependencyInjector to inject dependencies in annotations #4202

minwoox commented Apr 11, 2022 •

edited

Loading

codecov bot commented Apr 11, 2022 •

edited

Loading

ikhoon Apr 12, 2022

minwoox Apr 12, 2022

ikhoon Apr 15, 2022

trustin Apr 16, 2022

ikhoon Apr 18, 2022

ikhoon left a comment

ikhoon Apr 15, 2022

jrhee17 left a comment

minwoox commented Jun 16, 2022

jrhee17 left a comment

ikhoon left a comment

ikhoon Jul 1, 2022

minwoox Jul 1, 2022

trustin Jul 1, 2022 •

edited

Loading

minwoox Jul 4, 2022

jrhee17 left a comment

ikhoon left a comment

trustin left a comment

minwoox commented Jul 5, 2022

Add DependencyInjector to inject dependencies in annotations #4202

Add DependencyInjector to inject dependencies in annotations #4202

Conversation

minwoox commented Apr 11, 2022 • edited Loading

codecov bot commented Apr 11, 2022 • edited Loading

Codecov Report

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

ikhoon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jrhee17 left a comment

Choose a reason for hiding this comment

minwoox commented Jun 16, 2022

jrhee17 left a comment

Choose a reason for hiding this comment

ikhoon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

trustin Jul 1, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jrhee17 left a comment

Choose a reason for hiding this comment

ikhoon left a comment

Choose a reason for hiding this comment

trustin left a comment

Choose a reason for hiding this comment

minwoox commented Jul 5, 2022

minwoox commented Apr 11, 2022 •

edited

Loading

codecov bot commented Apr 11, 2022 •

edited

Loading

trustin Jul 1, 2022 •

edited

Loading