Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add query bean support #64

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Add query bean support #64

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
43887fb
Add query bean support
andrewl102 Nov 18, 2015
f7fe853
Fix test
andrewl102 Nov 18, 2015
a5fbc72
Fix documentation
andrewl102 Nov 18, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@

docs/conf

/.idea

# sbt specific
dist/*
target/
Expand Down
14 changes: 10 additions & 4 deletions build.sbt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ val PlayEnhancerVersion = "1.1.0"
lazy val root = project
.in(file("."))
.enablePlugins(PlayRootProject)
.aggregate(core)
.aggregate(core,plugin)
.settings(
name := "play-ebean-root",
releaseCrossBuild := false
Expand Down Expand Up @@ -45,9 +45,9 @@ lazy val plugin = project
)

playBuildRepoName in ThisBuild := "play-ebean"
// playBuildExtraTests := {
// (scripted in plugin).toTask("").value
// }
playBuildExtraTests := {
(scripted in plugin).toTask("").value
}
playBuildExtraPublish := {
(PgpKeys.publishSigned in plugin).value
}
Expand All @@ -59,15 +59,21 @@ def playEbeanDeps = Seq(
"com.typesafe.play" %% "play-jdbc-evolutions" % PlayVersion,
"org.avaje.ebeanorm" % "avaje-ebeanorm" % "6.8.1",
avajeEbeanormAgent,
avajeEbeanormQueryAgent,
avajeGenerator,
"com.typesafe.play" %% "play-test" % PlayVersion % Test
)

def sbtPlayEbeanDeps = Seq(
avajeEbeanormAgent,
avajeEbeanormQueryAgent,
avajeGenerator,
"com.typesafe" % "config" % "1.3.0"
)

def avajeEbeanormAgent = "org.avaje.ebeanorm" % "avaje-ebeanorm-agent" % "4.7.1"
def avajeEbeanormQueryAgent = "org.avaje.ebeanorm" % "avaje-ebeanorm-typequery-agent" % "1.5.1"
def avajeGenerator = "org.avaje.ebeanorm" % "avaje-ebeanorm-typequery-generator" % "1.5.1"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the latest is 1.5.3


// Ebean enhancement

Expand Down
67 changes: 61 additions & 6 deletions sbt-play-ebean/src/main/scala/play/ebean/sbt/PlayEbean.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@ package play.ebean.sbt
import java.net.URLClassLoader

import com.typesafe.play.sbt.enhancer.PlayEnhancer
import org.avaje.ebean.typequery.generator.{Generator, GeneratorConfig}
import sbt.Keys._
import sbt._
import sbt.inc._
import scala.collection.JavaConverters._

import scala.util.control.NonFatal

Expand All @@ -16,6 +18,19 @@ object PlayEbean extends AutoPlugin {
val playEbeanVersion = settingKey[String]("The version of Play ebean that should be added to the library dependencies.")
val playEbeanDebugLevel = settingKey[Int]("The debug level to use for the ebean agent. The higher, the more debug is output, with 9 being the most. -1 turns debugging off.")
val playEbeanAgentArgs = taskKey[Map[String, String]]("The arguments to pass to the agent.")

val playEbeanQueryGenerate = settingKey[Boolean]("Generate Query Beans from model classes. Default false.")
val playEbeanQueryEnhance = settingKey[Boolean]("Enhance Query Beans from model classes. Defaults to false")
val playEbeanQueryDestDirectory = settingKey[String]("Target directory for generated classes. Defaults to app ")
val playEbeanQueryResourceDirectory = settingKey[String]("Resource directory to read configuration. Defaults to conf")
val playEbeanQueryModelsPackage = settingKey[String]("Directory of models to scan to build query beans")
val playEbeanQueryModelsQueryModificationPackage = settingKey[Set[String]]("Directories of matching query objects to rewrite field access to use getters. Defaults to [model/query]")
val playEbeanQueryGenerateFinder = settingKey[Boolean]("Generate finder objects")
val playEbeanQueryGenerateFinderField = settingKey[Boolean]("Modify models to add finder field")
val playEbeanQueryGeneratePublicWhereField = settingKey[Boolean]("Public finder field")
val playEbeanQueryGenerateAopStyle = settingKey[Boolean]("Use AOP style generation. Default true")
val playEbeanQueryArgs = settingKey[String]("Args for generation, useful for logging / debugging generation ")
val playEbeanQueryProcessPackages = settingKey[String]("Change to alter the initial package for scanning for model classes. By default views all")
}

import autoImport._
Expand All @@ -25,7 +40,24 @@ object PlayEbean extends AutoPlugin {

override def trigger = noTrigger

override def projectSettings = inConfig(Compile)(scopedSettings) ++ unscopedSettings
override def projectSettings = {
val querySettings = Seq(
playEbeanQueryGenerate := false,
playEbeanQueryEnhance := false,
playEbeanQueryDestDirectory := "app",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if this should go to SBT's classes_managed directory instead?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it needs to go into a managed source directory, assuming it's a Java file. There's actually a better way to configure directories. Take a look at the Twirl sbt plugin to see how it does it:
https://github.com/playframework/twirl/blob/master/sbt-twirl/src/main/scala/play/twirl/sbt/SbtTwirl.scala.

Concretely here are some steps:

  1. Create a new task called playEbeanQuery. Call this task from ebeanEnhance if playEbeanQueryGenerate is created.
  2. The task should read from sources in playEbeanQuery and output to target in playEbeanQuery.
  3. Add playEbeanQuery task configuration into scopedSettings and define values like sources in playEbeanQuery := resourceDirectory.value and target in playEbeanQuery := crossTarget.value / "ebean-query" / Defaults.nameForSrc(configuration.value.name).
  4. Bind these settings using inConfig(Compile)(scopedSettings) ++ inConfig(Test) ++ ... so it will pick up the different directory values when compiling and testing (see raw and configured settings).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well I'm not sure about that.
Twirl and Query bean generation are two separate things.

When you're writing a twirl template, you only have one canonical source file: the .scala.html that the user actively works with to fulfil their tasks. This is compiled in to a mostly unreadable scala file that is used internally by Play that is then compiled down to an actual class file by the compiler.
In general the user has no interest in the intermediate Scala file so it should be not managed by source control and should be stored in the target folder as it is fine to discard and rebuild the contents of this folder.

This use case is different : if you peruse the example project, you will see that they have simply checked the generated source code into source control and more importantly they have modified the query beans after the initial generation to add additional functionality to them.

If we place it within the managed source directory like this (by default), we are essentially limiting users to only using the autogenerated functionality which is the opposite intent of what I was trying to achieve (based upon the Ebean examples).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andrewl102 can you share a link to the example project you're talking about?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/ebean-orm/avaje-ebeanorm-examples .

The a-basic directory contains an example of usage of query beans.
This is only an example obviously, however it seems quite useful to modify the auto generated finder classes to provide richer functionality.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, sounds like the thing to do then is put the query beans into a managed source directory as Rich suggested. That's how SBT plugins would typically handle this from what I've seen.

We could potentially invoke the code generator twice. Once to generate the never-modified query beans into the managed source directory and once to generate the usually modified Finders into the regular source directory. For simplicity though maybe it's easiest to just generate the query beans and forget about the Finders. The Finders seem rather simple compared to the query beans, so most of the value in the code generation is in generating the query beans, so that seems good enough for a v1 of this feature.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might be useful, after reading the previous comments I thought perhaps the way to go was finders -> source control and regular beans -> managed sources.
Ideally this could be done in the same step however it probably requires modification of the existing generation code.

If I ran it twice I'd also have to purge the generated query classes.
I think it's probably worth either making a modification to the other project or just ignoring it as you suggested. The user can always manually copy it to their source control although they'd have to disable the generation afterwards possibly.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think for simplicity we just don't give the user the ability to generate Finders for now

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've spent some time on this and hit some severe problems.

The query generation code requires the model classes already be compiled. By moving it to a generated source, we now have the condition where we need the sources for the models to be compiled before we trigger the query generation phase.
Even if I manage to bind this at the correct point after the model compilation has taken, it now sets up a scenario where in the person has written code that is dependent on the generated query beans, which will now not compile, preventing the original compilation of the model classes from taking place either if they do a clean compile.

It might be possible to solve this but it's way beyond the difficulty I had originally intended.
I would somehow have to trigger only the compilation of the model classes, without any dependent code, for the purposes of completing this phase with the proposed setup.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for being MIA and not reviewing over the holidays.

That's a good point you bring up. Forget that idea then. Can you add a comment to the code explaining why we're not using managed sources? Don't want someone else trying to go down that path in the future without knowing what they're getting into.

I think if we're not going to use managed sources then this is probably better as a task so that you have to type the task name on the console to trigger it. That way we're not automatically overwriting code you have checked in. Hopefully that will be a much easier change to make.

playEbeanQueryResourceDirectory := "conf",
playEbeanQueryModelsPackage := "models",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think users should be able to configure this or the generator must respect models packages even if it is not using the default models package.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be configurable.
If the user specifies playEbeanQueryModelsPackage := "notmodels" in their build.sbt it will override the default models definition unless I'm not understanding your comment correctly.

playEbeanQueryModelsQueryModificationPackage := Set("models/query"),
playEbeanQueryGenerateFinder := true,
playEbeanQueryGenerateFinderField := true,
playEbeanQueryGeneratePublicWhereField := true,
playEbeanQueryGenerateAopStyle := true,
playEbeanQueryArgs := "",
playEbeanQueryProcessPackages := null
)

inConfig(Compile)(scopedSettings) ++ unscopedSettings ++ querySettings
}

def scopedSettings = Seq(
playEbeanModels <<= configuredEbeanModels,
Expand Down Expand Up @@ -61,13 +93,36 @@ object PlayEbean extends AutoPlugin {

import com.avaje.ebean.enhance.agent._
import com.avaje.ebean.enhance.ant._

val transformer = new Transformer(classpath, agentArgsString)

val fileTransform = new OfflineFileTransform(transformer, classLoader, classes.getAbsolutePath)

try {
if(playEbeanQueryGenerate.value) {
val config = new GeneratorConfig()
config.setClassesDirectory(classes.getAbsolutePath)
config.setDestDirectory(playEbeanQueryDestDirectory.value)
config.setDestResourceDirectory(playEbeanQueryResourceDirectory.value)

config.setEntityBeanPackage(playEbeanQueryModelsPackage.value)
config.setAddFinderWherePublic(playEbeanQueryGeneratePublicWhereField.value)
config.setAopStyle(playEbeanQueryGenerateAopStyle.value)

val generator: Generator = new Generator(config)
generator.generateQueryBeans()
if (playEbeanQueryGenerateFinder.value) {
generator.generateFinders()
}
if (playEbeanQueryGenerateFinderField.value) {
generator.modifyEntityBeansAddFinderField()
}
}

val transformer = new Transformer(classpath, agentArgsString)
val fileTransform = new OfflineFileTransform(transformer, classLoader, classes.getAbsolutePath)
fileTransform.process(playEbeanModels.value.mkString(","))
if(playEbeanQueryEnhance.value) {
val queryTransform = new org.avaje.ebean.typequery.agent.Transformer(playEbeanQueryArgs.value, classLoader, playEbeanQueryModelsQueryModificationPackage.value.asJava)
val fileQueryTransform = new org.avaje.ebean.typequery.agent.offline.OfflineFileTransform(queryTransform, classLoader, classes.getAbsolutePath)
//Defaults to null, like the Maven plugin
fileQueryTransform.process(playEbeanQueryProcessPackages.value)
}
} catch {
case NonFatal(_) =>
}
Expand Down