Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
48 changes: 27 additions & 21 deletions stamina-core/src/main/scala/stamina/migrations.scala
Original file line number Diff line number Diff line change
Expand Up @@ -11,22 +11,12 @@ package object migrations {
/** The identity Migration will always return its input as its output. */
def identityMigration[T]: Migration[T] = identity[T]

/**
* Adds support for combining two instances of Migration[T] into a new
* Migration[T] that will apply the first one and then the second one.
*/
implicit class MigrationWithComposition[T](val firstMigration: Migration[T]) extends AnyVal {
def &&(secondMigration: Migration[T]): Migration[T] = {
(value: T) ⇒ secondMigration(firstMigration(value))
}
}

/**
* Creates a Migrator[T, V1] that can function as a builder for
* creating Migrator[T, V2], etc. Its migration will be the identity
* function so calling its migrate function will not have any effect.
*/
def from[T, V <: V1: VersionInfo]: Migrator[T, V] = new Migrator[T, V](Map(Version.numberFor[V] → identityMigration[T]))
def from[T, V <: V1: VersionInfo]: Migrator[T, V] = new Migrator[T, V](Map(Version.numberFor[V] → identityMigration[T]), Map.empty)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not make the empty map a default argument of the Migrator class?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because backward migrations are empty when we start defining migration - so empty map is natural initial value.

}

package migrations {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why was this removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because it is not used anymore.

Expand All @@ -35,8 +25,8 @@ package migrations {
extends RuntimeException(s"No migration defined from version ${fromVersion} to version ${toVersion}.")

/**
* A `Migrator[R, V]` can migrate raw values of type R from older
* versions to version `V` by applying a specific `Migration[R]` to it.
* A `Migrator[R, V]` can migrate raw values of type R between
* two versions by applying all `Migration[R]`s between these versions.
*
* You can create instances of `Migrator[R, V]` by using
* a small type-safe DSL consisting of two parts: the
Expand All @@ -55,20 +45,36 @@ package migrations {
* }}}
*
* @tparam R The type of raw data being migrated. In the JSON implementation this would be `JsValue`.
* @tparam V The "current" version of this Migrator, i.e. it can migrate values from V1 to this version or any version in between.
* @tparam V The "current" version of this Migrator.
*/
class Migrator[R, V <: Version: VersionInfo] private[stamina] (migrations: Map[Int, Migration[R]] = Map.empty) {
def canMigrate(fromVersion: Int): Boolean = migrations.contains(fromVersion)
class Migrator[R, V <: Version: VersionInfo] private[stamina] (forwardMigrations: Map[Int, Migration[R]] = Map.empty, backwardMigrations: Map[Int, Migration[R]] = Map.empty) {
def canMigrate(fromVersion: Int, toVersion: Int): Boolean = forwardMigrations.contains(fromVersion) && forwardMigrations.contains(toVersion)
Copy link
Member

Choose a reason for hiding this comment

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

Is this correct? It's not obvious to me that we now also need forwardMigrations.contains(toVersion), and shouldn't this also take into account that we can support backwards migrations?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is a simple code from tests:

val migrator = from[String, V1]
   .to[V2](_ + "V2")
   .backTo[V1](_.replace("V2", ""))
   .to[V3](_ + "V3")
   .backTo[V2](_.replace("V3", ""))
   .to[V4](_ + "V4")
   .backTo[V3](_.replace("V4", ""))
   .to[V5](_ + "V5")

The assumption is to define continuous forward migrations, so you cannot define migrations only for version 1 and version 3. You have to define migration also for version 2. So in case when we are going to migrate from version 2 to version 5, we need migrations from 2 to 3, from 3 to 4 and from 4 to 5. Migration from 4 to 5 is under 5 key, it is toVersion, so it is needed as well. canMigrate function checks in this case existence of 2 -> 3 and 4 -> 5 migrations, because forward migrations are defined continuously. Backward migrations are optional, so they doesn't affect canMigrate function at all. If backward migration is not defined for particular version, identity function is used.


def migrate(value: R, fromVersion: Int): R = {
migrations.get(fromVersion).map(_.apply(value)).getOrElse(
throw UndefinedMigrationException(fromVersion, Version.numberFor[V])
)
def migrate(value: R, fromVersion: Int, toVersion: Int): R = {
def applyMigrationOn(value: R, migration: Migration[R]) = migration(value)

if (fromVersion <= toVersion) {
(fromVersion to toVersion).drop(1)
.map(version ⇒ forwardMigrations.getOrElse(version, throw UndefinedMigrationException(fromVersion, toVersion)))
.foldLeft(value)(applyMigrationOn)
} else {
(toVersion to fromVersion).drop(1).reverse
.flatMap(backwardMigrations.get)
.foldLeft(value)(applyMigrationOn)
}
}

def to[NextV <: Version: VersionInfo](migration: Migration[R])(implicit isNextAfter: IsNextVersionAfter[NextV, V]) = {
new Migrator[R, NextV](
migrations.mapValues(_ && migration) + (Version.numberFor[NextV] → identityMigration[R])
forwardMigrations + (Version.numberFor[NextV] → migration),
backwardMigrations
)
}

def backTo[PrevV <: Version: VersionInfo](migration: Migration[R])(implicit e: IsNextVersionAfter[V, PrevV]) = {
new Migrator[R, V](
forwardMigrations,
backwardMigrations + (Version.numberFor[V] → migration)
)
}
}
Expand Down
85 changes: 85 additions & 0 deletions stamina-core/src/test/scala/stamina/migrations/MigratorSpec.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
package stamina.migrations

import stamina._

class MigratorSpec extends StaminaSpec {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This test suits to scalacheck, do you think I can add scalacheck as dependency and rewrite this test?


val migrator = from[String, V1]
.to[V2](_ + "V2")
.backTo[V1](_.replace("V2", ""))
.to[V3](_ + "V3")
.backTo[V2](_.replace("V3", ""))
.to[V4](_ + "V4")
.backTo[V3](_.replace("V4", ""))
.to[V5](_ + "V5")

"Migrator V5" should {
"be able to migrate" when {
"migration is from V1 to V5" in {
migrator.canMigrate(1, 5) shouldBe true
}

"migration is from V1 to V1" in {
migrator.canMigrate(1, 1) shouldBe true
}

"migration is from V5 to V2" in {
migrator.canMigrate(5, 2) shouldBe true
}
}

"not be able to migrate" when {
"migration is from V6 to V2" in {
migrator.canMigrate(6, 2) shouldBe false
}

"migration is from V6 to V7" in {
migrator.canMigrate(6, 7) shouldBe false
}

"migration is from V1 to V7" in {
migrator.canMigrate(1, 7) shouldBe false
}
}

"migrate forward" when {
"migration is from V1 to V5" in {
migrator.migrate("V1", 1, 5) shouldBe "V1V2V3V4V5"
}

"migration is from V2 to V5" in {
migrator.migrate("V1V2", 2, 5) shouldBe "V1V2V3V4V5"
}

"migration is from V1 to V4" in {
migrator.migrate("V1", 1, 4) shouldBe "V1V2V3V4"
}

"migration is from V1 to V1" in {
migrator.migrate("V1", 1, 1) shouldBe "V1"
}

"migration is from V2 to V2" in {
migrator.migrate("V1V2", 2, 2) shouldBe "V1V2"
}
}

"migrate backward" when {
"migration is from V2 to V1" in {
migrator.migrate("V1V2", 2, 1) shouldBe "V1"
}

"migration is from V5 to V1" in {
migrator.migrate("V1V2V3V4V5", 5, 1) shouldBe "V1V5"
}

"migration is from V5 to V2" in {
migrator.migrate("V1V2V3V4V5", 5, 2) shouldBe "V1V2V5"
}

"migration is from V5 to V3" in {
migrator.migrate("V1V2V3V4V5", 5, 3) shouldBe "V1V2V3V5"
}
}
}
}
6 changes: 4 additions & 2 deletions stamina-json/src/main/scala/stamina/json/json.scala
Original file line number Diff line number Diff line change
Expand Up @@ -78,11 +78,13 @@ package json {
}

private[json] class VnJsonPersister[T: RootJsonFormat: ClassTag, V <: Version: VersionInfo: MigratableVersion](key: String, migrator: JsonMigrator[V]) extends JsonPersister[T, V](key) {
override def canUnpersist(p: Persisted): Boolean = p.key == key && migrator.canMigrate(p.version)
private val toVersionNumber = implicitly[VersionInfo[V]].versionNumber

override def canUnpersist(p: Persisted): Boolean = p.key == key && migrator.canMigrate(p.version, toVersionNumber)

def persist(t: T): Persisted = Persisted(key, currentVersion, toJsonBytes(t))
def unpersist(p: Persisted): T = {
if (canUnpersist(p)) migrator.migrate(parseJson(p.bytes), p.version).convertTo[T]
if (canUnpersist(p)) migrator.migrate(parseJson(p.bytes), p.version, toVersionNumber).convertTo[T]
else throw new IllegalArgumentException(cannotUnpersist(p))
}
}
Expand Down