From 881a335c908d3891c959c5b772eb0e7f52f3d804 Mon Sep 17 00:00:00 2001
From: Fabrice Lecomte <fabrice.lecomte.be@gmail.com>
Date: Sat, 3 Aug 2019 00:53:44 +0200
Subject: [PATCH] fix: migrations

---
 .../fr/postgresjson/definition/Function.kt     | 11 ++++++++---
 .../fr/postgresjson/migration/Function.kt      |  8 ++++++--
 .../fr/postgresjson/migration/Migrations.kt    | 18 ++++++------------
 .../resources/sql/migration/deleteFunction.sql |  2 +-
 .../resources/sql/migration/insertFunction.sql |  6 ++++++
 .../resources/sql/migration/insertHistory.sql  |  4 ++--
 6 files changed, 29 insertions(+), 20 deletions(-)

diff --git a/src/main/kotlin/fr/postgresjson/definition/Function.kt b/src/main/kotlin/fr/postgresjson/definition/Function.kt
index c144066..1afdc7b 100644
--- a/src/main/kotlin/fr/postgresjson/definition/Function.kt
+++ b/src/main/kotlin/fr/postgresjson/definition/Function.kt
@@ -51,7 +51,12 @@ open class Function(
     class FunctionNotFound(cause: Throwable? = null): ParseException("Function not found in script", cause)
 
     fun getDefinition(): String {
-        return "$name (" + parameters.joinToString(", ") + ") $returns"
+        return parameters
+            .filter { it.direction == Parameter.Direction.IN }
+            .joinToString(", ") { "${it.name} ${it.type}" }.let {
+                "$name ($it) $returns"
+            }
+
     }
 
     fun getParametersIndexedByName(): Map<String, Parameter> {
@@ -64,8 +69,8 @@ open class Function(
         return other.getDefinition() == this.getDefinition()
     }
 
-    infix fun `is same`(other: Function): Boolean {
-        return other.script == this.script
+    infix fun `is different from`(other: Function): Boolean {
+        return other.script != this.script
     }
 
     companion object {
diff --git a/src/main/kotlin/fr/postgresjson/migration/Function.kt b/src/main/kotlin/fr/postgresjson/migration/Function.kt
index 8fe1193..2078521 100644
--- a/src/main/kotlin/fr/postgresjson/migration/Function.kt
+++ b/src/main/kotlin/fr/postgresjson/migration/Function.kt
@@ -16,8 +16,8 @@ data class Function(
     override var doExecute: Action? = null
 
     init {
-        if (up.name !== down.name) {
-            throw Exception("UP and DOWN migration must be the same")
+        if (up.name != down.name) {
+            throw Exception("UP and DOWN migration must have the same name [${up.name} !== ${down.name}]")
         }
     }
 
@@ -79,4 +79,8 @@ data class Function(
             it.doExecute = this.doExecute
         }
     }
+
+    infix fun `is different from`(other: DefinitionFunction): Boolean {
+        return other.script != this.up.script
+    }
 }
\ No newline at end of file
diff --git a/src/main/kotlin/fr/postgresjson/migration/Migrations.kt b/src/main/kotlin/fr/postgresjson/migration/Migrations.kt
index a799136..72d065a 100644
--- a/src/main/kotlin/fr/postgresjson/migration/Migrations.kt
+++ b/src/main/kotlin/fr/postgresjson/migration/Migrations.kt
@@ -122,23 +122,17 @@ data class Migrations private constructor(
     internal class DownMigrationNotDefined(path: String, cause: FileNotFoundException):
         Throwable("The file $path whas not found", cause)
 
-    fun addFunction(definition: DefinitionFunction, callback: (Function) -> Unit = {}): Migrations {
-        if (functions[definition.name] === null) {
-            // TODO define down migration
-            functions[definition.name] = Function(definition, definition, connection).apply {
+    fun addFunction(newDefinition: DefinitionFunction, callback: (Function) -> Unit = {}): Migrations {
+        val currentFunction = functions[newDefinition.name]
+        if (currentFunction === null || currentFunction `is different from` newDefinition) {
+            functions[newDefinition.name] = Function(newDefinition, newDefinition, connection).apply {
                 doExecute = Action.UP
             }
         } else {
-            functions[definition.name]!!.apply {
-                if (up `is same` definition) {
-                    doExecute = Action.OK
-                } else {
-                    doExecute = Action.UP
-                }
-            }
+            functions[newDefinition.name]?.doExecute = Action.OK
         }
 
-        callback(functions[definition.name]!!)
+        callback(functions[newDefinition.name]!!)
 
         return this
     }
diff --git a/src/main/resources/sql/migration/deleteFunction.sql b/src/main/resources/sql/migration/deleteFunction.sql
index 73c2786..e1a2f0f 100644
--- a/src/main/resources/sql/migration/deleteFunction.sql
+++ b/src/main/resources/sql/migration/deleteFunction.sql
@@ -1,3 +1,3 @@
 DELETE
 FROM migration.functions
-WHERE filename = :filename;
\ No newline at end of file
+WHERE filename = ?;
\ No newline at end of file
diff --git a/src/main/resources/sql/migration/insertFunction.sql b/src/main/resources/sql/migration/insertFunction.sql
index 6fa31d5..9e260e2 100644
--- a/src/main/resources/sql/migration/insertFunction.sql
+++ b/src/main/resources/sql/migration/insertFunction.sql
@@ -4,4 +4,10 @@ VALUES (?, ?, now(), ?, ?, (
     from migration.functions f2
     where filename = f2.filename
 ))
+ON CONFLICT (filename) DO UPDATE SET
+    definition = excluded.definition,
+    up = excluded.up,
+    down = excluded.down,
+    version = excluded.version,
+    executed_at = now()
 RETURNING to_json(f);
\ No newline at end of file
diff --git a/src/main/resources/sql/migration/insertHistory.sql b/src/main/resources/sql/migration/insertHistory.sql
index 0cb1d47..97353dc 100644
--- a/src/main/resources/sql/migration/insertHistory.sql
+++ b/src/main/resources/sql/migration/insertHistory.sql
@@ -1,7 +1,7 @@
 INSERT INTO migration.history as h (filename, executed_at, up, down, version)
 VALUES (?, now(), ?, ?, (
     select coalesce(max(version), 0)+1
-    from migration.history f2
-    where filename = f2.filename
+    from migration.history h2
+    where h2.filename = filename
 ))
 RETURNING to_json(h);