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

Fixed wrong route prefix for inherit generic method #673

Closed
wants to merge 2 commits into from

Conversation

dtelaroli
Copy link
Contributor

When I use generic controller and inherited controller have no @Path annotation all routes are generated without controller name prefix.

class MyExtendedController extends GenericController {
   @Get("/method") 
   public void method() {}
}
// The route translated for method is '/method', but should be /myExtended/method

Other problem is when I try to override the component PathAnnotationRoutesParser to change the default prefix it not execute the methods extractControllerNameFrom and defaultUriFor, turning impossible to override the behavior.

@renanigt
Copy link
Contributor

renanigt commented Jul 8, 2014

When you don't define a @Path to your Controller but define a @Path to your method, the correct path is only the method path. At least with me it's the behavior.

@renanigt
Copy link
Contributor

renanigt commented Jul 8, 2014

For me it's the correct behavior.

@dtelaroli
Copy link
Contributor Author

Look, both are without @Path.
@Get is equal to @Path?

@renanigt
Copy link
Contributor

renanigt commented Jul 8, 2014

Yes. You can define the path with @Path, @Get, @Post and the others...

@garcia-jj
Copy link
Member

Path, Get and others have more priority than convention. So if you have @Get("/method") annotation declared in method, the right URL will /method.

@dtelaroli
Copy link
Contributor Author

For me it's wrong, because the relative path not work and the API not allow override methods as I reported.

@garcia-jj
Copy link
Member

If you want the URL /myExtended/method you need to put @Path("/myExtended") in the class MyExtendedController.

@dtelaroli
Copy link
Contributor Author

Sorry, it's specific to me.

@dtelaroli dtelaroli closed this Jul 8, 2014
@rponte
Copy link
Contributor

rponte commented Jul 8, 2014

As Otávio said, you need to annotate your generic controller with @path.

On Tuesday, July 8, 2014, Denilson Telaroli notifications@github.com
wrote:

Sorry, it's specific for me.


Reply to this email directly or view it on GitHub
#673 (comment).

Rafael Ponte
TriadWorks | Formação Java
http://cursos.triadworks.com.br

@dtelaroli
Copy link
Contributor Author

Ok, thanks

@dtelaroli
Copy link
Contributor Author

@garcia-jj The method is not invoked for generic controller.

//apply only for non generic
@Override
    protected String extractControllerNameFrom(Class<?> type) {
        return "/prefix" + super.extractControllerNameFrom(type);
    }

//apply only for generic
@Override
    protected String extractPrefix(Class<?> type) {
        return "/prefix" + super.extractPrefix(type);
    }

If I overrided both methods the non generic add twice prefix.
What I'm doing wrong?

@dtelaroli
Copy link
Contributor Author

If I override both methods like the example and I not use convention work, so the @path is required and override both methods is required if override class (extractControllerNameFrom and extractPrefix).

It's a workaround, but I like to use the convention.
Anyway, thanks for the support.

@garcia-jj
Copy link
Member

Have you annotated the class with @Specializes and define a scope?

@Turini
Copy link
Member

Turini commented Jul 9, 2014

@dtelaroli
Copy link
Contributor Author

@lucascs
Copy link
Member

lucascs commented Jul 9, 2014

Seems like a bug to me... Can you open an issue, and perhaps a PR with tests that break because of this case?

@dtelaroli
Copy link
Contributor Author

👍

@garcia-jj
Copy link
Member

Why you consider a bug Lucas?

@renanigt
Copy link
Contributor

renanigt commented Jul 9, 2014

If I understand the problem is because isn't generated routes for generic methods. Is it @dtelaroli ?

@garcia-jj
Copy link
Member

If I understand the problem is because isn't generated routes for generic methods. Is it @dtelaroli ?

But this is the right behavior, as I know. @dtelaroli have annotation in the method, but not in the class. So vraptor will consider only annotation in the method. The controller name only is considered with have no annotation in both class and method.

If you want a prefix in the route, you need to annotate the class with @Path, or leave both method and class without annotation.

Because of this I ask Lucas, because may I can have a wrong knowledge about this feature.

@dtelaroli
Copy link
Contributor Author

The PR yes, but was my wrong understanding about the @get (and others) annotations.

Now, I can't to override the default route component because the methods are not being invoked.

To night I will verify, I don't think that it's a bug.

@renanigt
Copy link
Contributor

renanigt commented Jul 9, 2014

I agree with you @garcia-jj .
I thought the problem was because @dtelaroli would like generate routes for the superclass methods...

Sorry, so I didn't understand either.

@garcia-jj
Copy link
Member

Oh, now I see. My mistake :)

@lucascs
Copy link
Member

lucascs commented Jul 9, 2014

Yes, the bug is that we are not passing properly the subclass methods the overridable protected methods that are meant to be extended.

@garcia-jj
Copy link
Member

Indeed, @lucascs

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.

None yet

6 participants