pre/post triggers: define via `def` instead of `val`

dpoetzsch commented 6 years ago

In BaseRepository one can find the following code:

val postLoad: (T => T) = identity
val prePersist: (T => T) = identity
val postPersist: (T => T) = identity
val preUpdate: (T => T) = identity
val postUpdate: (T => T) = identity
val preDelete: (T => T) = identity
val postDelete: (T => T) = identity

I figure these are meant to be override by library users to perform custom pre and post action tasks such as setting createdAt timestamps.

However, the definition via val causes problems if this is used in a nested fashion because it cannot be overriden twice because super.prePersist is not allowed for val.

An easy fix would be to define all of them via def:

def postLoad(t: T): T = t
// ...

However, this would break compatibility. A work around would be:

protected def performPostLoad(t: T): T = t
// ...
val postLoad: (T => T) = performPostLoad

Then, one could simply override performPostLoad.

If this is something you're interested in I'd be happy to create a pull request.

gonmarques commented 6 years ago

Hi,

You are more than welcome to contribute, of course. I'm definitely more into the solution that changes the definition from val to def even if it breaks compatibility - it can be solved by bumping the minor version when the breaking change ends up being released.

Having to perpetuate the performPostLoad workaround is something that we should avoid, IMO.

gonmarques commented 6 years ago

Hi,

I have been doing some experiments and I couldn't reproduce the initially described problem. I tried to override val prePersist at multiple inheritance levels and the callbacks are called as expected. I also had no problems in accessing a super class prePersist by using super.prePersist.

Can you please share a concrete example?

dpoetzsch commented 6 years ago

Sure:

import com.byteslounge.slickrepo.meta.{Entity, Keyed}
import javax.inject.{Inject, Singleton}
import play.api.db.slick.{DatabaseConfigProvider, HasDatabaseConfigProvider}
import slick.ast.BaseTypedType

import scala.concurrent.ExecutionContext

trait TimestampedEntity[T <: TimestampedEntity[T]] extends Entity[T, Int] {
  def createdAt: Option[Long]
  def withTimestamps(createdAt: Long): T
}

case class Coffee(override val id: Option[Int], brand: String, createdAt: Option[Long] = None) extends TimestampedEntity[Coffee] {
  def withId(id: Int): Coffee = this.copy(id = Some(id))
  def withTimestamps(createdAt: Long) = this.copy(createdAt = Some(createdAt))
}

abstract class TimestampedRepository[T <: TimestampedEntity[T]]
  extends com.byteslounge.slickrepo.repository.BaseRepository[T, Int] with HasDatabaseConfigProvider[MyPostgresProfile] {

  override val prePersist: T => T = e => e.withTimestamps(System.currentTimeMillis())
}

@Singleton
class CoffeeRepository @Inject()(val dbConfigProvider: DatabaseConfigProvider)(implicit ec: ExecutionContext)
  extends TimestampedRepository[Coffee] {

  import profile.api._
  val pkType = implicitly[BaseTypedType[Int]]
  val tableQuery = TableQuery[Coffees]
  type TableType = Coffees

  class Coffees(tag: slick.lifted.Tag) extends Table[Coffee](tag, "coffee") with Keyed[Int] {
    def id = column[Int]("id", O.PrimaryKey, O.AutoInc)
    def brand = column[String]("brand")
    def createdAt = column[Long]("createdat")

    def * = (id.?, brand, createdAt.?) <> ((Coffee.apply _).tupled, Coffee.unapply)
  }

  override val prePersist: Coffee => Coffee = c => c.copy(brand = c.brand + " (limited)")
}

I want a repository that automatically adds timestamps to the entities. I use prePersist for this. Then, in the coffe repository, I also want to make modifications in prePersist. The current code does not work as the coffee trigger overrides the timestamp trigger.

The following does not work in CoffeeRepository:

override val prePersist: Coffee => Coffee = c => super.prePersist(c).copy(brand = c.brand + " (limited)")

(leads to super may not be used on value prePersist).

Am I missing something?

gonmarques commented 6 years ago

Ok, thanks. Now I see the problem.

By looking at the provided example there is something that caught my attention though, that is the necessity of calling super.prePersist in the sub class in order to trigger prePersist in the parent class.

Shouldn't this be done transparently to the end user? It seems that it could be done in the following way:

For every entity that is being persisted, automatically call every prePersist that exists in the repository hierarchy, starting in the BaseRepository until the latest sub class, always passing the resulting entity of calling prePersist at a given hierarchy level to the next one.

What are your thoughts about this approach? It eliminates the burden of having to call super.prePersist on sub classes - this intention is already expressed by extending the parent class. The sub class only has to focus on its own modifications in the event listener, not the parent ones.

The same applies to every event listener, of course:

  val postLoad: (T => T) = identity
  val prePersist: (T => T) = identity
  val postPersist: (T => T) = identity
  val preUpdate: (T => T) = identity
  val postUpdate: (T => T) = identity
  val preDelete: (T => T) = identity
  val postDelete: (T => T) = identity

dpoetzsch commented 6 years ago

I feel you want to do this via reflection?

In any case, I think this approach is less transparent. My usual expectation would be that if I override a method the original is gone (as it is intended by the language designers, hence the term override). If I want the original behavior as well I call the super method. I've never seen it another way in any library.

Also, sometimes when extending a class I do want to replace functionality which is no longer possible in your approach.

Also, reflection is usually slow.

The only other intuitive solution I see is to not use methods at all but implement an event pattern where each subclass could register any number of transformers for certain events.

gonmarques commented 6 years ago

What I had in my mind was to get something similar to what we have with JPA entity lifecycle listeners. Having to explicitly call a super listener is something that I would like to avoid.

On the other hand I agree with your statement that the term override does not play well with the approach of calling all hierarchy handlers since we end up adding new functionality instead of overriding the existing one.

I'm seeing this moving towards an annotation approach, similar to JPA:

@PrePersist
def prePersist(t: T): T = {
  t
}

It seems simpler than having to override a value or method. We just declare a method and annotate it.

It's true that this way it wont possible to exclude event listeners that are defined in a super class that we extend. As an analogy, I see this in the same way that it's not possible to have a sub class constructor that does not call a super class constructor. Constructors ensure correct class initialization, so they should be called. Entity lifecycle listeners ensure correct handling of entity data, so they should be called.

It's probably not that much different than the event registering pattern you suggested, but having it done by annotating methods.

I see this in two phases:

A discovery phase, where reflection is used to inspect a given repository hierarchy and build the handlers composition and cache it. Validation also occurs in this step, i.e. if a handler does not conform with (t: T): T signature, then it must error out stating the reason very clearly. If a repository class defines more than a single handler of the same type, it must also error out. This phase runs only once per repository type
An execution phase, where for every entity being handled we fetch the respective listener composition from the cache and pass in the entity, returning the final result. The listener calls are also done via reflection.

About reflection being slow, the reflective calls that shall be executed are most likely a very tiny fraction of the overall operation that we are executing - which includes the entire call to the underlying database via JDBC. The listener call via reflection is most likely irrelevant.

gonmarques commented 6 years ago

Other option may consist in taking advantage of macros and do the listener composition at compile time.

gonmarques commented 6 years ago

I wrote a proof of concept that uses reflection in order to call the lifecycle event listeners.

This is the result of profiling the application with 20 threads constantly updating some records in the database for 30 seconds. There is a preUpdate callback that changes a column value before the update is executed. I'm only highlighting a single database statement execution thread and a single application thread from each of the thread pools:

screen shot 2018-08-25 at 15 39 37

For the approximately 30 seconds that the application was left under profiling, it spent (almost) the entire time executing JDBC calls. The reflection listener calls spent only 249 milliseconds, which corresponds to 0.8% of the overall time.

These metrics were taken with VisualVM Profiling, which instrumentation introduces its own overhead. When we use Sampling instead of Profiling - which periodically dumps thread stack traces in order to calculate where the time is being spent - the reflection calls don't even show up (the calls are so fast that no dump occurs in a time frame where the calls are being executed).

For the time being the multiple event listener calls will be implemented with reflection.

dpoetzsch commented 6 years ago

Works for me.

I updated the readme to reflect your changes: #18

Btw, it is not 100% clear to me which order is now used for the lifecycle handlers when using traits. Maybe a section in the readme would help?

dpoetzsch commented 6 years ago

Ok, I had the time to test the changes a bit more and they actually break my play 2 dependency injection (no idea why, but the life cycle handler is in the stack trace):

39) Error injecting constructor, scala.ScalaReflectionException: class models.dao.WebsiteRepository in JavaMirror with DependencyClassLoader{(...)] not found.
    at models.dao.WebsiteRepository.<init>(WebsiteRepository.scala:59)
    at models.dao.WebsiteRepository.class(WebsiteRepository.scala:59)
    while locating models.dao.WebsiteRepository
        for the 5th parameter of controllers.v1.WebhookController.<init>(WebhookController.scala:32)
    while locating controllers.v1.WebhookController
        for the 4th parameter of router.Routes.<init>(Routes.scala:81)
    while locating router.Routes
    while locating play.api.inject.RoutesProvider
    while locating play.api.routing.Router
        for the 1st parameter of play.api.http.JavaCompatibleHttpRequestHandler.<init>(HttpRequestHandler.scala:222)
    while locating play.api.http.JavaCompatibleHttpRequestHandler
    while locating play.api.http.HttpRequestHandler
        for the 6th parameter of play.api.DefaultApplication.<init>(Application.scala:236)
    at play.api.DefaultApplication.class(Application.scala:235)
    while locating play.api.DefaultApplication
    while locating play.api.Application
        for the 3rd parameter of play.modules.swagger.SwaggerPluginImpl.<init>(SwaggerPlugin.scala:35)
    while locating play.modules.swagger.SwaggerPluginImpl
    at play.modules.swagger.SwaggerModule.bindings(SwaggerModule.scala:11):
Binding(interface play.modules.swagger.SwaggerPlugin to ConstructionTarget(class play.modules.swagger.SwaggerPluginImpl) eagerly) (via modules: com.google.inject.util.Modules$OverrideModule -> play.api.inject.guice.GuiceableModuleConversions$$anon$1)
    while locating play.modules.swagger.SwaggerPlugin
Caused by: scala.ScalaReflectionException: class models.dao.WebsiteRepository in JavaMirror with DependencyClassLoader{(...)] and parent being primordial classloader with boot classpath [(...)] not found.
    at scala.reflect.internal.Mirrors$RootsBase.staticClass(Mirrors.scala:122)
    at scala.reflect.internal.Mirrors$RootsBase.staticClass(Mirrors.scala:22)
    at com.byteslounge.slickrepo.repository.LifecycleHelper.$anonfun$getHandlerMethods$1(LifecycleHelper.scala:107)
    at scala.collection.TraversableLike.$anonfun$map$1(TraversableLike.scala:234)
    at scala.collection.IndexedSeqOptimized.foreach(IndexedSeqOptimized.scala:32)
    at scala.collection.IndexedSeqOptimized.foreach$(IndexedSeqOptimized.scala:29)
    at scala.collection.mutable.ArrayOps$ofRef.foreach(ArrayOps.scala:191)
    at scala.collection.TraversableLike.map(TraversableLike.scala:234)
    at scala.collection.TraversableLike.map$(TraversableLike.scala:227)
    at scala.collection.mutable.ArrayOps$ofRef.map(ArrayOps.scala:191)
    at com.byteslounge.slickrepo.repository.LifecycleHelper.getHandlerMethods(LifecycleHelper.scala:107)
    at com.byteslounge.slickrepo.repository.LifecycleHelper.createLifecycleHandler(LifecycleHelper.scala:65)
    at com.byteslounge.slickrepo.repository.BaseRepository.createHandler(BaseRepository.scala:246)
    at com.byteslounge.slickrepo.repository.BaseRepository.<init>(BaseRepository.scala:235)
    at models.dao.BaseRepository.<init>(BaseRepository.scala:10)
    at models.dao.NestedRepository.<init>(NestedRepository.scala:13)
    at models.dao.WebsiteRepository.<init>(WebsiteRepository.scala:60)
    at models.dao.WebsiteRepository$$FastClassByGuice$$81b11e45.newInstance(<generated>)
    at com.google.inject.internal.DefaultConstructionProxyFactory$FastClassProxy.newInstance(DefaultConstructionProxyFactory.java:89)
    at com.google.inject.internal.ConstructorInjector.provision(ConstructorInjector.java:111)
    at com.google.inject.internal.ConstructorInjector.construct(ConstructorInjector.java:90)
    at com.google.inject.internal.ConstructorBindingImpl$Factory.get(ConstructorBindingImpl.java:268)
    at com.google.inject.internal.ProviderToInternalFactoryAdapter$1.call(ProviderToInternalFactoryAdapter.java:46)
    at com.google.inject.internal.InjectorImpl.callInContext(InjectorImpl.java:1092)
    at com.google.inject.internal.ProviderToInternalFactoryAdapter.get(ProviderToInternalFactoryAdapter.java:40)
    at com.google.inject.internal.SingletonScope$1.get(SingletonScope.java:194)
    at com.google.inject.internal.InternalFactoryToProviderAdapter.get(InternalFactoryToProviderAdapter.java:41)
    at com.google.inject.internal.SingleParameterInjector.inject(SingleParameterInjector.java:38)
    at com.google.inject.internal.SingleParameterInjector.getAll(SingleParameterInjector.java:62)
    at com.google.inject.internal.ConstructorInjector.provision(ConstructorInjector.java:110)
    at com.google.inject.internal.ConstructorInjector.construct(ConstructorInjector.java:90)
    at com.google.inject.internal.ConstructorBindingImpl$Factory.get(ConstructorBindingImpl.java:268)
    at com.google.inject.internal.SingleParameterInjector.inject(SingleParameterInjector.java:38)
    at com.google.inject.internal.SingleParameterInjector.getAll(SingleParameterInjector.java:62)
    at com.google.inject.internal.ConstructorInjector.provision(ConstructorInjector.java:110)
    at com.google.inject.internal.ConstructorInjector.construct(ConstructorInjector.java:90)
    at com.google.inject.internal.ConstructorBindingImpl$Factory.get(ConstructorBindingImpl.java:268)
    at com.google.inject.internal.InjectorImpl$2$1.call(InjectorImpl.java:1019)
    at com.google.inject.internal.InjectorImpl.callInContext(InjectorImpl.java:1092)
    at com.google.inject.internal.InjectorImpl$2.get(InjectorImpl.java:1015)
    at com.google.inject.internal.InjectorImpl.getInstance(InjectorImpl.java:1054)
    at play.api.inject.guice.GuiceInjector.instanceOf(GuiceInjectorBuilder.scala:409)
    at play.api.inject.ContextClassLoaderInjector.$anonfun$instanceOf$3(Injector.scala:118)
    at play.api.inject.ContextClassLoaderInjector.withContext(Injector.scala:126)
    at play.api.inject.ContextClassLoaderInjector.instanceOf(Injector.scala:118)
    at play.api.inject.RoutesProvider.$anonfun$get$2(BuiltinModule.scala:104)
    at scala.Option.fold(Option.scala:158)
    at play.api.inject.RoutesProvider.get$lzycompute(BuiltinModule.scala:104)
    at play.api.inject.RoutesProvider.get(BuiltinModule.scala:100)
    at play.api.inject.RoutesProvider.get(BuiltinModule.scala:99)
    at com.google.inject.internal.ProviderInternalFactory.provision(ProviderInternalFactory.java:81)
    at com.google.inject.internal.BoundProviderFactory.provision(BoundProviderFactory.java:72)
    at com.google.inject.internal.ProviderInternalFactory.circularGet(ProviderInternalFactory.java:61)
    at com.google.inject.internal.BoundProviderFactory.get(BoundProviderFactory.java:62)
    at com.google.inject.internal.SingleParameterInjector.inject(SingleParameterInjector.java:38)
    at com.google.inject.internal.SingleParameterInjector.getAll(SingleParameterInjector.java:62)
    at com.google.inject.internal.ConstructorInjector.provision(ConstructorInjector.java:110)
    at com.google.inject.internal.ConstructorInjector.construct(ConstructorInjector.java:90)
    at com.google.inject.internal.ConstructorBindingImpl$Factory.get(ConstructorBindingImpl.java:268)
    at com.google.inject.internal.FactoryProxy.get(FactoryProxy.java:56)
    at com.google.inject.internal.SingleParameterInjector.inject(SingleParameterInjector.java:38)
    at com.google.inject.internal.SingleParameterInjector.getAll(SingleParameterInjector.java:62)
    at com.google.inject.internal.ConstructorInjector.provision(ConstructorInjector.java:110)
    at com.google.inject.internal.ConstructorInjector.construct(ConstructorInjector.java:90)
    at com.google.inject.internal.ConstructorBindingImpl$Factory.get(ConstructorBindingImpl.java:268)
    at com.google.inject.internal.ProviderToInternalFactoryAdapter$1.call(ProviderToInternalFactoryAdapter.java:46)
    at com.google.inject.internal.InjectorImpl.callInContext(InjectorImpl.java:1092)
    at com.google.inject.internal.ProviderToInternalFactoryAdapter.get(ProviderToInternalFactoryAdapter.java:40)
    at com.google.inject.internal.SingletonScope$1.get(SingletonScope.java:194)
    at com.google.inject.internal.InternalFactoryToProviderAdapter.get(InternalFactoryToProviderAdapter.java:41)
    at com.google.inject.internal.FactoryProxy.get(FactoryProxy.java:56)
    at com.google.inject.internal.SingleParameterInjector.inject(SingleParameterInjector.java:38)
    at com.google.inject.internal.SingleParameterInjector.getAll(SingleParameterInjector.java:62)
    at com.google.inject.internal.ConstructorInjector.provision(ConstructorInjector.java:110)
    at com.google.inject.internal.ConstructorInjector.construct(ConstructorInjector.java:90)
    at com.google.inject.internal.ConstructorBindingImpl$Factory.get(ConstructorBindingImpl.java:268)
    at com.google.inject.internal.FactoryProxy.get(FactoryProxy.java:56)
    at com.google.inject.internal.ProviderToInternalFactoryAdapter$1.call(ProviderToInternalFactoryAdapter.java:46)
    at com.google.inject.internal.InjectorImpl.callInContext(InjectorImpl.java:1092)
    at com.google.inject.internal.ProviderToInternalFactoryAdapter.get(ProviderToInternalFactoryAdapter.java:40)
    at com.google.inject.internal.SingletonScope$1.get(SingletonScope.java:194)
    at com.google.inject.internal.InternalFactoryToProviderAdapter.get(InternalFactoryToProviderAdapter.java:41)
    at com.google.inject.internal.InternalInjectorCreator$1.call(InternalInjectorCreator.java:205)
    at com.google.inject.internal.InternalInjectorCreator$1.call(InternalInjectorCreator.java:199)
    at com.google.inject.internal.InjectorImpl.callInContext(InjectorImpl.java:1085)
    at com.google.inject.internal.InternalInjectorCreator.loadEagerSingletons(InternalInjectorCreator.java:199)
    at com.google.inject.internal.InternalInjectorCreator.injectDynamically(InternalInjectorCreator.java:180)
    at com.google.inject.internal.InternalInjectorCreator.build(InternalInjectorCreator.java:110)
    at com.google.inject.Guice.createInjector(Guice.java:99)
    at com.google.inject.Guice.createInjector(Guice.java:84)
    at play.api.inject.guice.GuiceBuilder.injector(GuiceInjectorBuilder.scala:185)
    at play.api.inject.guice.GuiceApplicationBuilder.build(GuiceApplicationBuilder.scala:137)
    at play.api.inject.guice.GuiceApplicationLoader.load(GuiceApplicationLoader.scala:21)
    at play.core.server.DevServerStart$$anon$1.$anonfun$reload$3(DevServerStart.scala:174)
    at play.utils.Threads$.withContextClassLoader(Threads.scala:21)
    at play.core.server.DevServerStart$$anon$1.reload(DevServerStart.scala:171)
    at play.core.server.DevServerStart$$anon$1.get(DevServerStart.scala:124)
    at play.core.server.AkkaHttpServer.handleRequest(AkkaHttpServer.scala:202)
    at play.core.server.AkkaHttpServer.$anonfun$createServerBinding$1(AkkaHttpServer.scala:117)
    at akka.stream.impl.fusing.MapAsync$$anon$25.onPush(Ops.scala:1194)
    at akka.stream.impl.fusing.GraphInterpreter.processPush(GraphInterpreter.scala:519)
    at akka.stream.impl.fusing.GraphInterpreter.processEvent(GraphInterpreter.scala:482)
    at akka.stream.impl.fusing.GraphInterpreter.execute(GraphInterpreter.scala:378)
    at akka.stream.impl.fusing.GraphInterpreterShell.runBatch(ActorGraphInterpreter.scala:585)
    at akka.stream.impl.fusing.GraphInterpreterShell$AsyncInput.execute(ActorGraphInterpreter.scala:469)
    at akka.stream.impl.fusing.GraphInterpreterShell.processEvent(ActorGraphInterpreter.scala:560)
    at akka.stream.impl.fusing.ActorGraphInterpreter.akka$stream$impl$fusing$ActorGraphInterpreter$$processEvent(ActorGraphInterpreter.scala:742)
    at akka.stream.impl.fusing.ActorGraphInterpreter$$anonfun$receive$1.applyOrElse(ActorGraphInterpreter.scala:757)
    at akka.actor.Actor.aroundReceive(Actor.scala:517)
    at akka.actor.Actor.aroundReceive$(Actor.scala:515)
    at akka.stream.impl.fusing.ActorGraphInterpreter.aroundReceive(ActorGraphInterpreter.scala:667)
    at akka.actor.ActorCell.receiveMessage(ActorCell.scala:590)
    at akka.actor.ActorCell.invoke(ActorCell.scala:559)
    at akka.dispatch.Mailbox.processMailbox(Mailbox.scala:257)
    at akka.dispatch.Mailbox.run(Mailbox.scala:224)
    at akka.dispatch.Mailbox.exec(Mailbox.scala:234)
    at akka.dispatch.forkjoin.ForkJoinTask.doExec(ForkJoinTask.java:260)
    at akka.dispatch.forkjoin.ForkJoinPool$WorkQueue.runTask(ForkJoinPool.java:1339)
    at akka.dispatch.forkjoin.ForkJoinPool.runWorker(ForkJoinPool.java:1979)
    at akka.dispatch.forkjoin.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:107)

gonmarques commented 6 years ago

Hi. Can you please attach a sample that reproduces the behaviour you are observing? Also, when you try to instantiate WebsiteRepository by hand does it result in the same exception being thrown?

gonmarques commented 6 years ago

Hi. I had another time window to play a little bit more with the issue and I still can't reproduce the error you reported.

Another issue has raised though, which is some existing duplication that is still not being avoided. We avoided having to duplicate the call to super class handlers, but we can't avoid duplicating the code that eventually returns an altered entity copy (assuming a scenario for handlers that alter entities).

If we refer to the sample code in this comment: https://github.com/gonmarques/slick-repo/issues/15#issuecomment-413172980

You have defined def withTimestamps(createdAt: Long) = this.copy(createdAt = Some(createdAt)) inside the Coffee case class. I tried to avoid this duplication by playing with case class inheritance and traits because - ideally - this code would be written in a single place and be reused by every entity that requires a createdAt field, but it does not seem possible.

Given that there is still some duplication, I am once again uncomfortable with this approach. If we could avoid all of it, then I would be happy. But if we still have to deal with a certain duplication degree, I don't think that the complexity introduced by the multiple handlers approach worths it.

With this in mind, let's use the solution that changes handlers from val to def instead. I will reopen your PR and merge it to master as soon as I have time.

gonmarques commented 6 years ago

I gave this subject yet another thought - not an easy call on this one.

I decided to write the entire entity+repository definition of what could be an illustrative implementation of the following requirements, using the multiple handlers definition with annotations approach:

having a pre-persist event that sets the created timestamp
having a pre-update event that sets the updated timestamp
having a pre-persist/pre-update event that changes a given entity field

I liked the end result, even if the entity changing method has to be defined for every entity that wants to use a common handler - the compiler will help here since as soon as one throws in a new trait it will have to provide the entity changing implementation (this was already happening with the id and version anyway).

I have added an example with such a scenario in the README file, just after your latest changes.

Version 1.5.1 will be released right next.

Thanks for your contribution!

gonmarques commented 6 years ago

1.5.1 released.

Closing.

dpoetzsch commented 6 years ago

Hi, thanks for all the effort you put into this and sorry for my inactivity during the last days.

The error I mentioned still exists for me. Interstingly only when I start the application, not when running the tests, even though in both cases dependency injection is used.. I will try to create an example app when I find the time to reproduce it.

gonmarques commented 6 years ago

Hi. Back to this. I've reproduced the problem - one just has to inject the repository as a dependency in Play.

Creating a separate issue for this.

gonmarques commented 6 years ago

The issue should be fixed by now in 1.5.2.

gonmarques / slick-repo

pre/post triggers: define via `def` instead of `val` #15