1.2-RC1, route that worked with nightlies now being eaten (404)

[aaron gregg]

Hi All,

I have been updating my rest API to the recently release 1.2-RC1 today and have bump into a strange issue with my routes "90" % of the time not being found and spitting back 404's.

I just rolled my project back to the most recent nightly to see if anything I had been working on (authorize directive within the route structure) caused the issue, but all cool there, the older libraries work as advertised.

Could someone cast a look over the following and tell me what I am doing wrong here. I will include a snippet that works, and one that does not.

Cheers, Aaron

Wrapping Code (makes the calls to the other routers):
  def receive = runRoute {
    respondWithHeader(RawHeader("Strict-Transport-Security", "max-age=7776000; includeSubdomains")) {
      logRequestResponse(showRequestResponse _) {
        pathPrefix("api") {
          pathPrefix("security") { ctx => context.actorSelection(apiSecurityRoutingActor.path) ! ctx } ~
          pathPrefix("meta") { ctx => context.actorSelection(apiMetaRoutingActor.path) ! ctx } ~
          pathPrefix("content") { ctx => context.actorSelection(apiContentRoutingActor.path) ! ctx } ~
          complete { http404 }
        } ~
        path("ping") { complete { "pong" }} ~
        complete { http404 }
      }
    }
  }

From the metaRoutingActor:

  def getSecurityActor: ActorSelection = { context.actorSelection("/user/securityDbActor") }
  def getMetaActor: ActorSelection = { context.actorSelection("/user/metaDbActor") }
  def receive = runRoute {
    clientIP { ip =>
      authenticate(liwAuthenticate(getSecurityActor, ip)) { loggedInAccount =>

/* this section does not work, no logging will be written to the console and works using the previous nightly libraries. */

        pathPrefix("languages") {
          authorize(liwAuthorize(loggedInAccount.user, PermissionProtect.language, _)) {
            pathPrefix(Segment) { languageCode =>
              path("") {
                put { handleWith { model: api.model.Language => (getMetaActor ? SaveLanguageReq(model)).mapTo[Either[Failure, Success]] } }
              }
            } ~
            path("") {
              get { parameters('filter, 'sort ?, 'skip ?, 'take ?).as(Search) { model => complete { (getMetaActor ? LanguageSearchReq(model)).mapTo[SearchResult[api.model.Language]]  } } } ~
              post { handleWith { model: api.model.Language => (getMetaActor ? SaveLanguageReq(model)).mapTo[Either[Failure, Success]] } }
            }
          }
        } ~
        pathPrefix("categories") {
          authorize(liwAuthorize(loggedInAccount.user, PermissionProtect.category, _)) {
            pathPrefix(Segment) { permissionId =>
              path("") {
                put { handleWith { model: Category => (getMetaActor ? SaveCategoryReq(model)).mapTo[Either[Failure, Success]] } }
              }
            } ~
            path("") {
              get { parameters('filter, 'sort ?, 'skip ?, 'take ?).as(Search) { model => complete { (getMetaActor ? CategorySearchReq(model)).mapTo[SearchResult[Category]]  } } } ~
              post { handleWith { model: Category => (getMetaActor ? SaveCategoryReq(model)).mapTo[Either[Failure, Success]] } }
            }
          }
        } ~

/* and for a reason unknown to me, the following call works with both RC and nightly libraries */

        pathPrefix("statistics") {
          authorize(liwAuthorize(loggedInAccount.user, PermissionProtect.statistic, _)) {
            pathPrefix("prices") {
              path("call") {
                get { parameters('category, 'location).as(CategoryLocationQuery) { model => complete { (getMetaActor ? CallPriceForCategoryLocationReq(model)).mapTo[DecimalResult]  } } }
              }
            }
          }
        }
      }
    }
  }

[Vasily Shiyan]

I'm having similar issues. Base route structure is

        pathPrefix("a") {

          path("") {

            complete("")

          } ~

          path("b") {

            complete("")

          }

        }

After upgrade to 1.2-RC1 it stopped matching "/a" (it still matches "/a/", "/a/b", "/a/b/"). Docs to "path" matcher say 

Tries to consume a leading slash from the unmatched path of the spray.routing.RequestContext before applying the given matcher. The matcher has to match the remaining path completely or leave only a single trailing slash.

So this should match "/a"

-- 
Vasily Shiyan

--
You received this message because you are subscribed to the Google Groups "spray-user" group.
To unsubscribe from this group and stop receiving emails from it, send an email to spray-user+...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

[aaron gregg]

Hi Vasily,

Thanks for your post, I tried putting a trailing slash on the path and that seems to work, at least we know what the issue is now!!
I might fish around for a directive or approach that will knock off that trailing slash before the internal routes are run and see if that can 'hack' a temporary solution until a better one is available.

Cheers, Aaron

[Johannes Rudolph]

Hi Aaron and Vasily,

yes, we made a change to the PathMatcher infrastructure that has this
effect. `path("")` before matched more than probably intended: it
matched an empty rest path, a trailing slash, or even two trailing
slashes. Therefore, we introduced the change that makes `path("")`
behave as all other instances of `path(str)` as well, which always
matches a leading slash, then `str`, and then an optional slash at the
end.

To get back the original behavior you can use the directive from this new issue:

https://github.com/spray/spray/issues/628

or override the path-directive in your service with

override def path[L <: shapeless.HList](pm: PathMatcher[L]):
Directive[L] = rawPathPrefix((Slash?) ~ pm ~ (Slash?) ~ PathEnd)

As, this is a breaking change for almost all users we are not yet
sure, how we will fix it. There are two things to consider:
* what to expect from `path("")` which always seemed like a strange
way to match the path end
* if `path` should match an optional trailing slash in general (WDYT?
Would you say that '/x' and '/x/' should be handled equally?)

Sorry for the confusion,

Johannes

--
Johannes

-----------------------------------------------
Johannes Rudolph
http://virtual-void.net

[aaron gregg]

Hi Johannes,

Thanks for your reply, then I would prefer to re-structure my routes to accommodate the new changes than to hack around them. 

Could you assist then, because I was always under the impression run the most specific first, down to the least specific. And following that thought I structure my routes like the following (pseudo as I am on a train right now).

Here you can see deeper structures first, and when the nested user-role or user-space structure is completed, I then handle the less specific paths until I've 'walked' the way back up to /user. At that point the next 'root' path will begin for that domain object, perhaps roles or something else which will follow a similar structure.

  pathPrefix("users") {

    pathPrefix(Segment) { userId =>

      pathPrefix("roles") {

        pathPrefix(Segment) { roleId =>

          put { complete { (getSecurityActor ? CreateUserRoleLink(Join(userId, roleId))).mapTo[Either[Failure, Success]] } } ~ 

          delete { complete { (getSecurityActor ? RemoveUserRoleLink(Join(userId, roleId))).mapTo[Either[Failure, Success]] } }

        }

      } ~

      pathPrefix("spaces") {

        pathPrefix(Segment) { spaceId =>

          put { complete { (getSecurityActor ? CreateUserSpaceLink(Join(userId, spaceId))).mapTo[Either[Failure, Success]] } } ~ 

          delete { complete { (getSecurityActor ? RemoveUserSpaceLink(Join(userId, spaceId))).mapTo[Either[Failure, Success]] } }

        }

      } ~

/* here you can see how I handle a less specific path, as the Segment userId has already been declared and it's sub-paths handled

 * I wanted to handle just that segment here, how would I then capture paths that 'terminate' at this level? */

      path("") {

        get { complete { (getSecurityActor ? GetUser(userId)).mapTo[Option[User]] } }

        put { handleWith { model: User => (getSecurityActor ? UpdateUser(userId, model)).mapTo[Either[Failure, Success]] } }

      }

    } ~

/* The same goes for the following situation, I have already finished with the userId segment, and now want to 

 * handle searches and posts at the root /users path, how can I now handle these paths */

    path("") {

      get { parameters('filter, 'sort ?, 'skip ?, 'take ?).as(Search) { 

        model => complete { (getSecurityActor ? UserSearch(model)).mapTo[SearchResult[User]] }

      } } ~

      post { handleWith { model: User => (getSecurityActor ? CreateUser(model)).mapTo[Either[Failure, Success]] } }

    }

  }

I also want to thank you for your teams support, as usual top-class fantastic effort. Congratulations on such an excellent library and on joining the typesafe team. Looking forward to the future!!

Cheers, Aaron

[Johannes Rudolph]

Hi Aaron,

On Thu, Oct 24, 2013 at 9:05 AM, aaron gregg <atom....@gmail.com> wrote:
> Thanks for your reply, then I would prefer to re-structure my routes to
> accommodate the new changes than to hack around them.

Discussing this more with Mathias, we think the changes are maybe not
yet finished and we should make PathMatchers and directives behave
more straight-forwardly and remove even more automatic behavior we had
in before. In the end, it means that part of the functionality is
missing in the current version and that's why I suggested to add a
work-around until we've figured out how to do it exactly.

So, what you will have to do in future in any case is deciding if
paths with trailing slash should be treated the same as the same path
without.

So, once you have decided on that topic you can define an alias for
what you now have as `path("")` and define it either as
`rawPathPrefix(Slash.? ~ PathEnd)`, `rawPathPrefix(Slash ~ PathEnd)`,
or just `rawPathPrefix(PathEnd)` depending on which behavior you want
and use the alias instead of `path("")`.

We'll probably introduce these aliases in spray as well so you will be
able to use them once they are in but for the time being you will have
to define them yourself.

> Could you assist then, because I was always under the impression run the
> most specific first, down to the least specific.

Yes, and that didn't change.

[Timothy Perrett]

Guys,

I also now see this and just wasted a whole bunch of time debugging it - these RC's are not very stable: I expect this kind of change in a milestone, but RCs should be about locking down features and functionality into a solid state, not altering the semantics of widely used APIs with no deprecation, no warning, and note in the docs (1.2RC2 docs still use path("")).... this is somewhat difficult to understand.

Can the RC3 please get back into a stable, usable state? I appreciate the want to keep improving and striving for perfection, but there has to be a sensible limit to make spray viable for company use.

Thanks, Tim 

[Timothy Perrett]

Just for anyone else finding this thread, RC2 appears to have added pathEnd, pathSingleSlash and pathEndOrSingleSlash directives, so I guess we should use those instead.

That being said, this issue was introduced just before the release:

https://github.com/spray/spray/issues/652

Tim

[Henry Saputra]

Seems like the tests are passing, looks like missing tests for the RC2
to get those regressions?

- Henry

[Mathias Doenitz]

Tim,

we completely agree with you on this, understand your frustration and are sorry about the trouble caused.
The change to the path directive semantics was not planned, it resulted from an accidental change that was introduced with RC1 thereby breaking code that was working with the nightlies before.

RC2 rectifies this problem by solving it properly, unfortunately this required an addition to the public API and a change to the `path` directive semantics.
Because we assumed that this might trip people up we had dedicated a prominent heads up and explanation in the RC2 announcement: https://groups.google.com/forum/#!searchin/spray-user/RC2/spray-user/W6D6DlL2MvU/d0dfcX-BaKgJ

Also we have now updated all RC2 docs to show the new path directive solution and included a special section on this in the migration guide:
http://spray.io/project-info/migration-from-M8/

> Can the RC3 please get back into a stable, usable state?

The purpose of the RCs is to flush out bugs and increase stability for a proper final release.
So far the two RCs have been very effective in doing this.
We try our best to keep all "collateral damage" minimal in the process.
Sorry again, that this hasn't quite worked in your case...

Cheers,
Mathias

---
mat...@spray.io
http://spray.io

On 05.11.2013, at 04:22, Timothy Perrett <tper...@gmail.com> wrote:

> Guys,
>
> I also now see this and just wasted a whole bunch of time debugging it -
> these RC's are not very stable: I expect this kind of change in a
> milestone, but RCs should be about locking down features and functionality
> into a solid state, not altering the semantics of widely used APIs with no
> deprecation, no warning, and note in the docs (1.2RC2 docs still use
> path("")).... this is somewhat difficult to understand.
>
> Can the RC3 please get back into a stable, usable state? I appreciate the
> want to keep improving and striving for perfection, but there has to be a
> sensible limit to make spray viable for company use.
>
> Thanks, Tim
>
> On Thursday, 24 October 2013 01:49:36 UTC-7, Johannes Rudolph wrote:
>>
>> Hi Aaron,
>>

>> On Thu, Oct 24, 2013 at 9:05 AM, aaron gregg <atom....@gmail.com<javascript:>>