Create minimal sbt plugin for Play services #8011

gmethvin · 2017-11-15T05:50:36Z

Splits the current Play plugin into a PlayService and PlayWeb plugin.

PlayService uses the standard maven layout and doesn't automatically add template support, sbt-web, or routes file support. Support for routes can be easily added using the RoutesCompiler sbt plugin. The old Play layout can be added by adding PlayLayoutPlugin.

Eventually the PlayService plugin could also be the base plugin for Lagom services.

richdougherty · 2017-11-15T21:44:51Z

framework/src/sbt-plugin/src/main/scala/play/sbt/Play.scala

-      )
+  override def projectSettings = PlaySettings.defaultSettings ++ Seq(
+    scalacOptions ++= Seq("-deprecation", "-unchecked", "-encoding", "utf8"),
+    javacOptions in Compile ++= Seq("-encoding", "utf8", "-g")


Not related to this change, but perhaps we could make these values overridable by users instead of hardcoded.

E.g.

playScalacOptions := Seq("-deprecation", "-unchecked", "-encoding", "utf8") scalacOptions ++= playScalacOptions

richdougherty · 2017-11-15T21:46:22Z

framework/src/sbt-plugin/src/main/scala/play/sbt/Play.scala

+ *
+ * Declares common settings for both Java and Scala based web projects, as well as sbt-web and assets settings.
+ */
+object PlayWeb extends AutoPlugin {


Does this provide the same behaviour as the old Play plugin? What's the impact of changing the name to PlayWeb?

@gmethvin has told me on another channel that most people will be using the PlayJava and PlayScala plugins, not the Play plugin.

richdougherty · 2017-11-15T21:46:40Z

framework/src/sbt-plugin/src/main/scala/play/sbt/Play.scala

@@ -79,7 +95,7 @@ object PlayScala extends AutoPlugin {
 * This plugin enables the Play netty http server
 */
 object PlayNettyServer extends AutoPlugin {
-  override def requires = Play
+  override def requires = PlayService


richdougherty · 2017-11-15T21:48:57Z

framework/src/sbt-plugin/src/main/scala/play/sbt/PlayLayoutPlugin.scala

+/**
+ * Play layout plugin to switch to the classic Play layout instead of the standard Maven layout.
+ *
+ * This is enabled automatically with the PlayWeb plugin but not with the PlayService plugin


This comment reads to me like PlayWeb depends on PlayLayoutPlugin, but the requres = PlayWeb below seems to show that PlayLayoutPlugin depends on PlayWeb. Or am I just confused? :)

It's an AutoPlugin, whose trigger is allDependencies. So it's automatically applied when all dependencies are satisfied. Perhaps it would be better to just make it a dependency of PlayWeb, but I was just going along with the way it was already done.

Actually, it probably would make more sense for it to be a dependency of PlayWeb. That also means you could apply it to a non-Play project which would make sense.

richdougherty · 2017-11-16T01:25:43Z

framework/src/sbt-plugin/src/sbt-test/play-sbt-plugin/basic-service/project/Build.scala

+  val ReadTimeout = 10000
+
+  @tailrec
+  def verifyResourceContains(path: String, status: Int, assertions: Seq[String], attempts: Int): Unit = {


Not really part of this PR, but it feels like this common code should be in a library somewhere…

+1.

This could be side by side with MediatorWorkaroundPlugin.

marcospereira · 2017-11-16T17:59:23Z

framework/src/sbt-plugin/src/main/scala/play/sbt/PlayFilters.scala

@@ -16,7 +16,7 @@ import sbt._
 * }}}
 */
 object PlayFilters extends AutoPlugin {
-  override def requires = Play
+  override def requires = PlayWeb


I understand we need to do this so that we can have better compatibility but to me, this plugin (and also PlayAkkaHttpServer and PlayNettyServer) should just add some settings and requires only sbt.plugins.JvmPlugin. Moreover, I expect people to be doing something like .enablePlugins(PlayScala).enablePlugins(PlayFilters) instead of just doing enablePlugins(PlayFilters) so that they will get PlayWeb through PlayScala or PlayJava.

Is there any reason these plugins needs PlayWeb or PlayService?

The same for PlayLogback.

The dependency is there because this is an AutoPlugin that's triggered on AllRequirements.

I do think we should have it added by default for PlayWeb though, to provide the secure-by-default experience. I guess that's sort of the point of this plugin.

I do think we should have it added by default for PlayWeb though, to provide the secure-by-default experience. I guess that's sort of the point of this plugin.

I like that we do this by using a plugin:

It is enabled by default (since it is an AutoPlugin)

Being a plugin make it easy to disable it if necessary.

My main point here is that if it needs to requires PlayWeb.

Most of the filters are designed to help with web-oriented issues, or at least things that generally affect public-facing apps. With a service you don't often need those things, and I think PlayService should by default give you a minimal service.

The only point of requires = PlayWeb is for the AllRequirements trigger.

marcospereira · 2017-11-16T18:04:53Z

framework/src/sbt-plugin/src/sbt-test/play-sbt-plugin/basic-service-with-routes-file/test

+# Build the distribution and ensure that the files we expect are indeed there
+> stage
+$ exists target/universal/stage/README
+$ exists target/universal/stage/bin/root


Should test routes generation too.

Or maybe you can run and have a task to make a request to /.

By the way, you already have verifyResourceContains. It just needs to be added to the tests.

On a second thought, and based on the basic-service tests, it could be good to check for the generated/compiled route files. That is because we can have routers without the router compiler (like in basic-service).

marcospereira · 2017-11-16T18:09:39Z

framework/src/sbt-plugin/src/sbt-test/play-sbt-plugin/basic-service/test

+# Build the distribution and ensure that the files we expect are indeed there
+> stage
+$ exists target/universal/stage/README
+$ exists target/universal/stage/bin/root


Needs tests to add verifyResourceContains test.

I added the verifyResourceContains test.

marcospereira · 2017-11-17T13:02:53Z

framework/src/sbt-plugin/src/sbt-test/play-sbt-plugin/basic-service-with-routes-file/test

+
+> run
+
+> verifyResourceContains / 200 Hello


Missing playStop as the last line.

gmethvin · 2017-11-19T01:09:49Z

I think this is good to merge. I'd like to add some examples as well, so I'll do that and then update the documentation in another PR.

godenji · 2017-11-19T14:15:53Z

Fantastic, thanks for getting this in place @gmethvin

Looking forward to working with a leaner version of Play 😄

marcospereira

LGTM.

@gmethvin we can add Highlights27 for this change.

…playframework#8011)

gmethvin force-pushed the play-minimal branch 2 times, most recently from 3c3b67d to 0b44a09 Compare November 15, 2017 08:44

richdougherty reviewed Nov 15, 2017

View reviewed changes

richdougherty approved these changes Nov 16, 2017

View reviewed changes

gmethvin force-pushed the play-minimal branch 3 times, most recently from 9e6a48a to 2380783 Compare November 16, 2017 04:04

marcospereira reviewed Nov 16, 2017

View reviewed changes

gmethvin force-pushed the play-minimal branch 4 times, most recently from 7d057f6 to 5d8dda6 Compare November 16, 2017 21:57

marcospereira reviewed Nov 17, 2017

View reviewed changes

gmethvin force-pushed the play-minimal branch 3 times, most recently from 7250d1d to a6da4d4 Compare November 19, 2017 00:26

gmethvin changed the title ~~[wip] Create minimal sbt plugin for Play services~~ Create minimal sbt plugin for Play services Nov 19, 2017

Create sbt plugin for play services that does not include web plugins

1067236

gmethvin force-pushed the play-minimal branch from a6da4d4 to 1067236 Compare November 19, 2017 20:54

marcospereira approved these changes Nov 21, 2017

View reviewed changes

gmethvin merged commit 44db3c2 into playframework:master Nov 22, 2017

gmethvin deleted the play-minimal branch November 22, 2017 21:42

gmethvin mentioned this pull request Nov 23, 2017

Fix defaultSettings to refer to both webSettings and serviceSettings #8034

Merged

gmethvin added a commit to gmethvin/playframework that referenced this pull request Nov 23, 2017

Create sbt plugin for play services that does not include web plugins (…

e861bfe

…playframework#8011)

gmethvin mentioned this pull request Nov 23, 2017

[2.6.x] Create sbt plugin for Play services #8035

Merged

marcospereira mentioned this pull request Nov 23, 2017

Add documentation for new PlayServices/PlayWeb sbt plugins #8039

Open

marcospereira mentioned this pull request Dec 18, 2017

Move Twirl support into an AutoPlugin that is enabled by default and can be disabled #5823

Closed

TimMoore added this to the Play 2.7.0 milestone Dec 10, 2018

eed3si9n mentioned this pull request Feb 14, 2019

Endpoints are not detected for Play 2.7.0 lightbend/sbt-reactive-app#183

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create minimal sbt plugin for Play services #8011

Create minimal sbt plugin for Play services #8011

gmethvin commented Nov 15, 2017 •

edited

richdougherty Nov 15, 2017

richdougherty Nov 15, 2017

richdougherty Nov 16, 2017

richdougherty Nov 15, 2017

richdougherty Nov 15, 2017

gmethvin Nov 16, 2017

gmethvin Nov 16, 2017

richdougherty Nov 16, 2017

marcospereira Nov 16, 2017

marcospereira Nov 16, 2017

marcospereira Nov 16, 2017

gmethvin Nov 16, 2017 •

edited

gmethvin Nov 16, 2017 •

edited

marcospereira Nov 16, 2017

gmethvin Nov 16, 2017 •

edited

marcospereira Nov 16, 2017

marcospereira Nov 16, 2017

marcospereira Nov 16, 2017

marcospereira Nov 16, 2017

marcospereira Nov 16, 2017

gmethvin Nov 19, 2017

marcospereira Nov 17, 2017

gmethvin commented Nov 19, 2017

godenji commented Nov 19, 2017

marcospereira left a comment

Create minimal sbt plugin for Play services #8011

Create minimal sbt plugin for Play services #8011

Conversation

gmethvin commented Nov 15, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gmethvin Nov 16, 2017 • edited

Choose a reason for hiding this comment

gmethvin Nov 16, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gmethvin Nov 16, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gmethvin commented Nov 19, 2017

godenji commented Nov 19, 2017

marcospereira left a comment

Choose a reason for hiding this comment

gmethvin commented Nov 15, 2017 •

edited

gmethvin Nov 16, 2017 •

edited

gmethvin Nov 16, 2017 •

edited

gmethvin Nov 16, 2017 •

edited