Routes are not found when they contain parameters (19.11.0)

[cernerae-avlino]

Reporting issue where routes are not being resolved when they contains parameters, as in the example below.

I migrated from Finatra v19.7.0 to v19.11.0. The Expected behavior below works in v19.7.0, but fails in v19.11.0.

Test

prefix(value = "/hello") { post(route = "/:status/?") { request: Request =>... } }

Results in a 404 for /hello/on, /hello/off/, /hello/anything/. Changing the route from /hello/:status to /hello/on explicitly, will resolve the route when a request to POST /hello/on/ is made.

Expected behavior

POST /hello/on/ will call post(route = "/:status/?") POST /hello/off/ will call post(route = "/:status/?")

Actual behavior

The endpoint cannot be found 15:21:24.881 [finagle/netty4-1-4] INFO c.t.f.h.filters.AccessLoggingFilter - 0:0:0:0:0:0:0:1 - - [27/Nov/2019:20:21:24 +0000] "POST /hello/on/ HTTP/1.1" 404 - 26 "PostmanRuntime/7.19.0"

Reproduce

Make a POST request to the route /hello/:status/, where the route is inside one or more prefixes.

[cernerae-avlino]

Here are additional details. Removed sensitive details.

Router

override protected def configureHttp(router: HttpRouter) {

  router
    .filter[CommonFilters]
    .filter[LoggingMDCFilter[Request, Response]]
    .filter[TraceIdMDCFilter[Request, Response]]

    ...

    .add[MyController]

}

 override def configureHttpServer(server: Http.Server): Http.Server = {

  server.withMaxRequestSize(StorageUnit.fromMegabytes(5))
    .withRequestTimeout(twitterDuration.fromMinutes(1))
}

Controller

class MyController extends Controller ... with Logging {

  private val fetchInterval = Duration.fromSeconds(seconds = 30)

  private val sseHeaders = Map(
    "Connection" -> "keep-alive",
    "Content-Type" -> MediaTypes.EventStream,
    "Cache-Control" -> "no-cache"
   )

  prefix(value = s"/api/v1") {

   prefix(value = "/hello") {

      post(route = "/:status/?") { request: Request =>

         ...

      }

  }
}

Removing the prefixes causes the same behavior.

Using com.twitter:: finagle-*-19.11.0 finatra-*-19.11.0 inject-*-19.11.0 twitter-*-19.11.0 util-*-19.11.0

[jyanJing]

Hi @cernerae-avlino ,

I ran a test on the latest version of Finatra HTTP routing (no behavior change since our 19.9.0 release) and I was unable to reproduce the issue you reported, prefixed routes with route parameters is working as expected. Is there anything I am missing?

There are the test details: Controller

prefix(value = "/api/v1") {
    prefix(value = "/hello") {
      post(route = "/:status/?") { request: Request =>
        "Done!"
      }
    }
}

Test passed

test("prefixed routes with parameters") {
    server.httpPost(
      "/api/v1/hello/on/",
      "",
      andExpect = Ok,
      withBody = "Done!"
    )
}

[cernerae-avlino]

Figured it out (related to https://github.com/twitter/finatra/issues/514#issuecomment-559235331). I was defining a "root" controller at the top of the stack to catch and handle endpoint requests that are not found. I moved the controller to the bottom of the stack and it stopped blocking, but was still catching correctly.

Stripped down from original.

     class TestRootController extends Controller with .... {

         private val apiVersion = appConfig.getString("app.apiVersion")

          options("/:*") {
                _: Request => response.ok
          }

         /**
            * GET: Endpoint not found
            */
         any("/:*") { _: Request => response.notFound }

         prefix(s"/api/") {

              options("/:*") {
                   _: Request => response.ok
               }

          /**
             * GET: Endpoint not found
             */
             any("/:*") { _: Request => response.notFound }

              prefix(s"/$apiVersion") {

                 options("/:*") {
                     _: Request => response.ok
                  }

                /**
                   * GET: Endpoint not found
                   */
                any("/:*") { _: Request =>
                    info("NOT FOUND!")
                    response.notFound
                }

           }

       }

    }

Do you know why was this working in v19.7 but not in v19.11?

[jyanJing]

Hi @cernerae-avlino ,

Thanks for the details! And yes, the way you define the controller happens to reflect one major change of routing in v19.9. Before v19.9, Finatra stores routes per method verb. When a route comes in, we will check its method verb, and will only match routes that were defined for that method verb. But in v19.9, we will store the method verb with a route, and we match the route path first then decide if there is a matching method verb. This helps us distinguish a 404 not found response vs. a 405 method not allowed response.

In your case, since you defined 2 routes: Option("/:") Any("/:")

Before v19.9, if you call Option("/abc") from your app, it will only look for the routes that defined for the Option keyword, and you got a perfect match. However, after v19.9, it will find 2 routes that matches /abc, and whichever defines first will be matched. So if you define Any first, it will return the Any route.

In fact, the reason your controller works before is because it happens to catch a grey area in routing pre v19.9. As Chris mentioned in another ticket, Finatra will match routes in order. So please keep that in mind when defining the controllers :-)

Hope that answers your question!

Best, Jing