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

AnnotatedHttpService: Allow @PathPrefix annotations in class scope #2031

Closed
heowc opened this issue Aug 28, 2019 · 7 comments
Closed

AnnotatedHttpService: Allow @PathPrefix annotations in class scope #2031

heowc opened this issue Aug 28, 2019 · 7 comments
Milestone

Comments

@heowc
Copy link
Contributor

heowc commented Aug 28, 2019

I think it would be nice to be able to specify a prefix as an annotation without using pathPrefix.

For example.

ServerBuilder sb = new ServerBuilder();
sb.annotatedService(new ForBarService());
//...
@PathPrefix("/foo")
public class FooBarService {
    @Get
    @Path("/bar")
    public Object bar() { ... }

    @Get
    public Object fooGet() { ... }

    @Post
    public Object fooPost() { ... }
} 

I think there is an advantage to see the specified path at once.

@trustin
Copy link
Member

trustin commented Aug 28, 2019

That's a good idea. We currently use @Path for other type of path mappings such as glob and regex, so we have two options:

  • Restrict the format of @Path when it's used at class level.
  • Define a different annotation such as @PathPrefix.

I prefer the second option, but what do you think? /cc @ikhoon @minwoox

@ikhoon
Copy link
Contributor

ikhoon commented Aug 28, 2019

I think @Path at class level could make a user confused because of the glob. I also lean to @PathPrefix, furthermore we can use it method level too. 😀

@minwoox
Copy link
Member

minwoox commented Aug 29, 2019

+1 for @PathPrefix :)

@sivaalli
Copy link
Contributor

sivaalli commented Sep 19, 2019

Hi, Is this something a first timer can work on? If yes, I'm more that happy to look at it. I think I could read value inside @PathPrefix("/pathPrefix") in here

and use the read value if the exiting pathPrefix value is /.

@minwoox
Copy link
Member

minwoox commented Sep 19, 2019

@sivaalli Thanks for your interest!
Yes, I think and I hope it's not that difficult. But I think we need to ask @heowc that he is going to deal with this or not before you take this. 😉

@heowc
Copy link
Contributor Author

heowc commented Sep 19, 2019

@sivaalli Thanks for your opinion!
I will another PRs in progress and someday I will proceed this.
but, if you need to this, I think it's not bad for you to deal with it. 👍

@sivaalli
Copy link
Contributor

Thanks you @minwoox and @heowc for your thoughts. I will work on this. BTW excellent work creating armeria and open sourcing it.

@trustin trustin added this to the 0.93.0 milestone Sep 25, 2019
@trustin trustin changed the title AnnotatedHttpService: Allow @Path annotations in class scope AnnotatedHttpService: Allow @PathPrefix annotations in class scope Sep 25, 2019
eugene70 pushed a commit to eugene70/armeria that referenced this issue Oct 16, 2019
Adding @PathPrefix annotation for a user to declare a prefix path that a service object handles.

Motivation:

line#2031

This PR allows a user to declare a 

```
ServerBuilder sb = new ServerBuilder();
sb.annotatedService(new ForBarService());
```

and 

```
@PathPrefix("/foo")
public class FooBarService {
    @get("/bar")
    public Object barGet() { ... }
}
```

Result:

Fixes line#2031
fmguerreiro pushed a commit to fmguerreiro/armeria that referenced this issue Sep 19, 2020
Adding @PathPrefix annotation for a user to declare a prefix path that a service object handles.

Motivation:

line#2031

This PR allows a user to declare a 

```
ServerBuilder sb = new ServerBuilder();
sb.annotatedService(new ForBarService());
```

and 

```
@PathPrefix("/foo")
public class FooBarService {
    @get("/bar")
    public Object barGet() { ... }
}
```

Result:

Fixes line#2031
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants