RPC based REST #78
RPC based REST #78
Conversation
commons-jetty: implemented handler for new rest
…vlet and RestHandler commons-jetty: added unit tests for RestHandler
…methods the same way as returning failed futures
# Conflicts: # commons-jetty/src/main/scala/com/avsystem/commons/jetty/rpc/JettyRPCFramework.scala
Starzu
left a comment
Starzu
left a comment
There was a problem hiding this comment.
As we discussed before:
- It would be great to provide sensible defaults for all REST annotations in order to make it possible to use a clean RPC trait as a REST interface. I think we should also add a test to ensure that is possible.
- We should provide some kind of
DefaultRpcAndRestApiCompanionto simplify usage of interface in both RPC and REST.
| case multiple => | ||
| val pathStr = headers.path.iterator.map(_.value).mkString("/") | ||
| val callsRepr = multiple.iterator.map(p => s" ${p.rpcChainRepr}").mkString("\n", "\n", "") | ||
| throw new RestException(s"path $pathStr is ambiguous, it could map to following calls:$callsRepr") |
There was a problem hiding this comment.
You ensure that paths are not ambiguous in line 49, so resolvePath can return Option[ResolvedPath]. Probably you can make ensureUnambiguousPaths and ensureUniqueParams boolean lazy vals and require them to be true inside resolvePath.
There was a problem hiding this comment.
I agree that it makes sense to refactor resolvePath to return Opt/Option
I don't understand why you want validation to be done lazily. I explicitly wanted it to be done eagerly so that it fails fast and can be put into a unit test. Ideally I'd like this validation to be done in compile time but that's impossible in this case.
|
|
||
| def empty: HttpBody = Empty | ||
|
|
||
| def apply(content: String, mimeType: String): HttpBody = |
There was a problem hiding this comment.
Could you create a wrapper for mimeType: String?
There was a problem hiding this comment.
It seems a bit of overkill to me as it's already wrapped into HttpBody so it's meaning is clear and it's intended to be always used when handling bodies.
| final val GET, PUT, POST, PATCH, DELETE: Value = new HttpMethod | ||
| } | ||
|
|
||
| case class RestHeaders( |
There was a problem hiding this comment.
This name is a little misleading - it contains not only headers, but also other arguments. What about RestParameters?
There was a problem hiding this comment.
RestParameters is not perfect either because there are also body parameters which are not part of this class. I'm open to other naming suggestions but I couldn't find better.
There was a problem hiding this comment.
I agree, RestParameters is not perfect, but it's a little better.
There was a problem hiding this comment.
So I finally decided that RestParameters is better
|
Documentation