Armeria: Per-service setting is not possible for annotated service

Created on 23 Aug 2019  路  6Comments  路  Source: line/armeria

A user can configure per-service settings using fluent Route API.
However, this is only for the services which implement HttpService, not for the annotated service.
We should provide a way for annotated service as well.

defect

Most helpful comment

Hello, @sivaalli! Thanks for the suggestion. However, I think adding more annotations is the last thing I want to do. 馃槃

Had a chat with @trustin and we thought adding a builder would be enough for this.

ServerBuilder sb = Server.builder()          // Returns ServerBuilder
                         .annotatedService() // Returns AnnotatedServiceBindingBuilder
                                             // Not sure this is the best name.
                         .requestTimeoutMillis(5000)
                         .contentPreview(...)
                         ...
                         .build(object);     // Returns to the ServerBuilder

All the properties that a service might want to set separately are gathered in AbstractServiceBindingBuilder thanks to @ikhoon.
However, the builder for annotated service just cannot inherit from it because of AbstractBindingBuilder which AbstractServiceBindingBuilder is inherited from.

- AbstractBindingBuilder
  - AbstractServiceBindingBuilder
    - ServiceBindingBuilder

AbstractBindingBuilder has setters related to paths and HTTP methods which an annotated service does not need.
So I think we have to tear off AbstractServiceBindingBuilder from AbstractBindingBuilder first. We can make an interface for the service configuration binding, so ServiceBindingBuilder implements the interface while extending AbstractBindingBuilder, and AnnotatedServiceBindingBuilder(?) will implements the interface only.
What do you think?

All 6 comments

Hi @trustin @minwoox @ikhoon ,

I think what we could do is, add an annotation called @Configuration that has properties/fields that reflect all the settings that Route api supports. This annotation will be used on each service method. For example a user can declare @Configuration(maxRequestLength = 1024, requestTimeoutMillis=5000) like this on a service method. Instead of creating a new Annotation(in this example @Configuration), we can also add route specific configuration properties on @Path. What do you think?

If you guys have anything else in mind please feel free to share it so I can implement it.

How about just adding buildAnnotated(Object) to ServiceBindingBuilder?

Hello, @sivaalli! Thanks for the suggestion. However, I think adding more annotations is the last thing I want to do. 馃槃

Had a chat with @trustin and we thought adding a builder would be enough for this.

ServerBuilder sb = Server.builder()          // Returns ServerBuilder
                         .annotatedService() // Returns AnnotatedServiceBindingBuilder
                                             // Not sure this is the best name.
                         .requestTimeoutMillis(5000)
                         .contentPreview(...)
                         ...
                         .build(object);     // Returns to the ServerBuilder

All the properties that a service might want to set separately are gathered in AbstractServiceBindingBuilder thanks to @ikhoon.
However, the builder for annotated service just cannot inherit from it because of AbstractBindingBuilder which AbstractServiceBindingBuilder is inherited from.

- AbstractBindingBuilder
  - AbstractServiceBindingBuilder
    - ServiceBindingBuilder

AbstractBindingBuilder has setters related to paths and HTTP methods which an annotated service does not need.
So I think we have to tear off AbstractServiceBindingBuilder from AbstractBindingBuilder first. We can make an interface for the service configuration binding, so ServiceBindingBuilder implements the interface while extending AbstractBindingBuilder, and AnnotatedServiceBindingBuilder(?) will implements the interface only.
What do you think?

Hi @trustin and @minwoox, I think I may have misunderstood this issue. So just want to reiterate the idea. For example, if this is my service:

class MyService{

    @Get("/foo")
    public HttpResponse foo(){
        return HttpResponse.of(200);
    }

    @Post("/bar")
    public HttpResponse bar(){
        return HttpResponse.of(200);
    }
}

and annotated service is registered using

Server server = new ServerBuilder().port(8080, SessionProtocol.HTTP).annotatedService("/hello", new MyService()).build();

Then, we should provide ability for user to

  1. Configure different http settings for /hello/foo(for ex: requestTimeoutMillis =1000) and /hello/bar(for ex: requestTimeoutMillis =5000)
  2. Configure http settings for anything under /hello

If I'm understanding this correct the point# 2 is what we are trying to achieve right?

But your idea sounds good. Sorry If I'm saying things again and again but just wanted to make sure I understand this correctly before I start writing any code.

If I'm understanding this correct the point# 2 is what we are trying to achieve right?

Yes, we are going to provide #2 because this is per-service configuration not per-method configuration. :-)

Configure different http settings for /hello/foo(for ex: requestTimeoutMillis =1000) and /hello/bar(for ex: requestTimeoutMillis =5000)

In this case, we are guiding like:

class MyService{
    @Get("/foo")
    public HttpResponse foo(ServiceRequestContext ctx) { // or RequestContext.current()
        ctx.setRequestTimeoutMillis(1000);
        return HttpResponse.of(200);
    }
}

Sorry If I'm saying things again and again but just wanted to make sure I understand this correctly before I start writing any code.

You are always welcomed to ask us anything. :-)

Thank you @minwoox. I will start looking into this.

Was this page helpful?
0 / 5 - 0 ratings