search

perwendel / spark

A simple expressive web framework for java. Spark has a kotlin DSL https://github.com/perwendel/spark-kotlin
Apache License 2.0
9.64k stars 1.56k forks source link

When routes are declared across multiple classes, and a common before {...} method is declared in an abstract base class, the before filter runs multiple times #1047

Open v79 opened 5 years ago

v79 commented 5 years ago

Using spark-kotlin, but I imagine the same is true of sparkjava:

I've declared several "Controller" classes, each with their own routes defined (HomeController for "/", PageController for "/pages", etc). These controllers all inherit from an AbstractController class. The AbstractController class defines the before route for authentication, logging, etc.

I now have 5 such controllers. And it seems the before route is evaluated 5 times for every request.

abstract class AbstractController() {
  init {
    before {
      logger.info("- before AbstractController - ${Instant.now()}")
    }
  }
}

Then logging for every request shows:

qtp1205109442-17] INFO org.liamjd.bascule.controllers.AbstractController - - before AbstractController - 2018-08-22T20:17:10.229Z [qtp1205109442-17] INFO org.liamjd.bascule.controllers.AbstractController - - before AbstractController - 2018-08-22T20:17:10.229Z [qtp1205109442-17] INFO org.liamjd.bascule.controllers.AbstractController - - before AbstractController - 2018-08-22T20:17:10.229Z [qtp1205109442-17] INFO org.liamjd.bascule.controllers.AbstractController - - before AbstractController - 2018-08-22T20:17:10.230Z [qtp1205109442-17] INFO org.liamjd.bascule.controllers.AbstractController - - before AbstractController - 2018-08-22T20:17:10.230Z

Is this expected behaviour? Should the before route be evaluated 5 times, once per Controller? Maybe I'm trying to force an MVC approach onto Spark where it doesn't really fit...

tipsy commented 5 years ago

Yes, this is expected. Every time you call before {} you create an independent filter which executes on all paths.

v79 commented 5 years ago

I can kinda see why. I'll have a rethink about my design, unless you're planning on adding a beforeBefore{} method which only runs once :). Thanks for confirming.