WIP: Play routers and services #378
Conversation
|
|
||
| /** Extract the longest common package prefix for a list of packages. */ | ||
| private[gen] def commonPackage(packages: Seq[String]): String = | ||
| packages.reduce(commonPackage(_, _)) |
There was a problem hiding this comment.
The old code had an optimisation to check everything for equality. I've moved that optimisation out of this method and into the commonPackage(String,String) method. That means we can use reduce instead of writing our own fold.
| @@ -50,7 +50,7 @@ class PlayJavaRouterSpec extends WordSpec with Matchers with BeforeAndAfterAll w | |||
| implicit def fromSourceMarshaller[T](implicit serializer: ProtobufSerializer[T], mat: Materializer, codec: Codec): ToResponseMarshaller[Source[T, NotUsed]] = | |||
| Marshaller.opaque((response: Source[T, NotUsed]) ⇒ GrpcMarshalling.marshalStream(response)(serializer, mat, codec)) | |||
|
|
|||
| val router = new GreeterServiceImpl(mat) | |||
| val router = new GreeterServiceRouter(new GreeterServiceImpl(), mat) | |||
There was a problem hiding this comment.
Here you can see how a new Java router and service are composed together.
| @@ -29,7 +30,7 @@ class PlayScalaRouterSpec extends WordSpec with Matchers with BeforeAndAfterAll | |||
| implicit val ec = sys.dispatcher | |||
| implicit val patience = PatienceConfig(timeout = 3.seconds, interval = 15.milliseconds) | |||
|
|
|||
| val router = new GreeterServiceImpl | |||
| val router = new GreeterServiceRouter(new GreeterServiceImpl) | |||
There was a problem hiding this comment.
Here you can see how a new Scala router and service are composed together.
There was a problem hiding this comment.
Ah, this looks nice than val router = new GreeterServiceImpl! 👍🏼
| */ | ||
| @InternalApi | ||
| protected def createHandler(serviceName: String, mat: Materializer): HttpRequest => Future[HttpResponse] | ||
| @InternalApi abstract class PlayRouter(prefix: String, underlyingHandler: HttpRequest => Future[HttpResponse]) extends play.api.routing.Router { |
There was a problem hiding this comment.
I removed a lot of the logic from this class and just pass things in as constructor arguments.
| @varargs // Helper for Java subclasses | ||
| final protected def classSeq(classes: Class[_]*): Seq[Class[_]] = classes | ||
|
|
||
| protected def bindingsForService[T](serviceClass: Class[T], environment: Environment, configuration: Configuration): Seq[Binding[_]] = { |
There was a problem hiding this comment.
This is how a service is bound. I try to be helpful in my error messages.
TODO: Write tests to check this works.
|
Apologies that I didn't get a chance to finish this. Here are some changes that are still needed. TODO:
|
|
@raboof / @johanandren, given your comments in #364, WDYT about this proposal? |
|
@richdougherty I can't push to this PR. I'm looking into it more carefully and wanted to do a couple of small changes (unregarding the outcome of the discussion in #364) |
|
Discussed internally, and the vote was to reject it. Some notes:
|
Fixes #364.
This changes the way that Play gRPC services are generated so that the service implementation and router are in separate classes. I feel this is more idiomatic for Play. The service implementation isn't generated - the user has to do that - so I've worked to make the log and error messages informative and helpful.
At the moment this PR is in a rough state. I've made the main changes and got the tests passing. I still need to go through the code and remove duplication, moving files around and add scaladoc and javadoc.