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

New controller and routing system ignores global request interceptors #345

Closed
dunsunik opened this issue Jan 25, 2017 · 15 comments
Closed

Comments

@dunsunik
Copy link

dunsunik commented Jan 25, 2017

  1. Whenever I annotate my action method with more then 1 route annotation for example @get("") and @put("") only the first one defined is actualy enabled and executed.

  2. I tried to create some basic type of interceptors for example one annotation @justatest that is placed on my controller class and a second one annotation @required that is placed on an action method.
    When I annotade my controller class with @justatest and some of my action method with @required only the @required annotation is executed. Both of them should be executed right ?

  3. My application based filters are not executed anymore after using controllers.
    In my Application I have defined this filter:

// auth user before filter
ALL("/.*", new RouteHandler() {

	@Override
	public void handle(RouteContext routeContext) {
		log.error("WHY IT DOES NOT GET HERE");
	}

});
@dunsunik
Copy link
Author

dunsunik commented Jan 25, 2017

Bug 3) has been fixed. At least it seems so.

Anyways I think there needs to be a better handling of interceptors.

Whenever I annotate my Controller class with an interceptor and some action method as well both of them should be executed. First should be executed action interceptors, then all class interceptors and then base class interceptors.

Thank you

@decebals
Copy link
Member

I tried to create some basic type of interceptors for example one annotation @justatest that is placed on my controller class and a second one annotation @required that is placed on an action method.
When I annotade my controller class with @justatest and some of my action method with @required only the @required annotation is executed. Both of them should be executed right ?

I think that current implementation works well with your scenario. When a controller method is invoked the interceptors on method are executed and AFTER them the interceptors on class are executed.
Your interceptors are not executed because probably you forgot to call routeContext.next() method in Interceptor route handler.
Example:

import ro.pippo.controller.Interceptor;

import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;

@Retention(RetentionPolicy.RUNTIME)
@Target({ElementType.TYPE, ElementType.METHOD})
@Interceptor(LoggingHandler.class)
public @interface Logging {
}
import ro.pippo.core.route.RouteContext;
import ro.pippo.core.route.RouteHandler;

public class LoggingHandler implements RouteHandler {

    @Override
    public void handle(RouteContext routeContext) {
        System.out.println("LoggingHandler.handle");
        routeContext.next(); // <<< IMPORTANT
    }

}

All interceptors are added in a chain and you can specify in the route handler of the interceptor if you want to call next interceptor or not (according the business logic from interceptor).

@dunsunik
Copy link
Author

dunsunik commented Jan 28, 2017

Yes sorry I know I forgot to announce it here :)

Anyways please is it possible to get route attributes either from a controller's action or filter ? I mean for a current request.

I can get list of routes using findRoutes or searchRoutes but it gets list of routes. I would like to get a specified 1 route that translated a request to a specified action method.

@decebals
Copy link
Member

@dunsunik

I think that #347 resolves your request.

@decebals
Copy link
Member

Whenever I annotate my action method with more then 1 route annotation for example @get("") and @put("") only the first one defined is actualy enabled and executed.

For now it's not possible but you have this workaround (use a common method for post and get methods):

class MyController extends Controller {

   @GET
   myGetMethod(...) {
       commonMethod(...);
   }

   @PUT
   myPutMethod(...) {
       commonMethod(...);
   }

   commonMethod(...) {
       // your business
   }

}

@dunsunik
Copy link
Author

dunsunik commented Jan 29, 2017 via email

@dunsunik
Copy link
Author

Hey,

I have tested new method getRoute() that I call in my Authentication interceptor handler.
It returns a route but sadly attributes are no present on a route or better say do not contain my previously set key.
I set an attribute using a transformer (during an application startup). If I print attributes from within that transformer a set attribute is there. But it's not the case when gettting it from my authentication interceptor handler.

Any clue ?
Thank you

@dunsunik
Copy link
Author

dunsunik commented Jan 29, 2017

When I print attributes from a controller's action method they are there and my previously set key is there as well.
So there seems to be a problem that attributes are not accessible from within interceptor handler.

@decebals
Copy link
Member

Can you share the code from your transformer (or only the part that create the returned value)?

@dunsunik
Copy link
Author

public class ActivationNotRequiredTransformer implements RouteTransformer {

	@Override
	public Route transform(Route route) {
		Method method = route.getAttribute("__controllerMethod");
		if (method == null) {
			// it's not a controller route; do nothing
			return route;
		}

		if (method.isAnnotationPresent(ActivationNotRequired.class)) {
			System.out.println("not require activation annot");
			// ActivationNotRequired named = method.getAnnotation(ActivationNotRequired.class);
			route.bind("activationNotRequired", true);
		}

		return route;
	}

}

@decebals
Copy link
Member

Thanks. I found a possible bug at this line. When I create the virtual route for interceptor I doesn't pass the attributes from controller route and probably your espectation is to have these attributes.
Probably it's a better solution to have a single attribute __controllerRoute in interceptor that returns the controller route.

@dunsunik
Copy link
Author

dunsunik commented Jan 29, 2017 via email

@decebals
Copy link
Member

decebals commented Jan 30, 2017

@dunsunik
Commit 84ea840 added controller route attributes to interceptor. I created a new SNAPSHOT which contains the modification.

@decebals
Copy link
Member

@dunsunik I don't know if the interceptor is not enough in your case (without the route transformer). For example you can retrieve in interceptor route handler the controller method (via routeContext.getRoute().getAttribute("__controllerMethod")) and check if ActivationNotRequired annotation is present or not.

@dunsunik
Copy link
Author

dunsunik commented Jan 31, 2017 via email

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

No branches or pull requests

2 participants