From 064ba7df9108b572b1a657420b9c813ea81207c9 Mon Sep 17 00:00:00 2001
From: Fabrice Drouin <sstone@users.noreply.github.com>
Date: Tue, 23 Apr 2019 17:11:53 +0200
Subject: [PATCH] Backup: explicitely specify move options (#960)

* Backup: explicitely specify move options

We now specify that we want to atomically overwrite the existing backup file with the new one (fixes
a potential issue on Windows).
We also publish a specific notification when the backup process has been completed.
---
 eclair-core/src/main/resources/reference.conf    |  4 ++--
 .../scala/fr/acinq/eclair/db/BackupHandler.scala | 16 ++++++++++++++--
 .../fr/acinq/eclair/db/BackupHandlerSpec.scala   |  9 +++++----
 3 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/eclair-core/src/main/resources/reference.conf b/eclair-core/src/main/resources/reference.conf
index f56a78f6f..b9755d1fb 100644
--- a/eclair-core/src/main/resources/reference.conf
+++ b/eclair-core/src/main/resources/reference.conf
@@ -17,7 +17,7 @@ eclair {
   }
 
   // override this with a script/exe that will be called everytime a new database backup has been created
-  backup-notify-script = ""
+  # backup-notify-script = "/path/to/script.sh"
 
   watcher-type = "bitcoind" // other *experimental* values include "electrum"
 
@@ -149,4 +149,4 @@ eclair {
     executor = "thread-pool-executor"
     type = PinnedDispatcher
   }
-}
\ No newline at end of file
+}
diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/db/BackupHandler.scala b/eclair-core/src/main/scala/fr/acinq/eclair/db/BackupHandler.scala
index c607c268a..8ac444984 100644
--- a/eclair-core/src/main/scala/fr/acinq/eclair/db/BackupHandler.scala
+++ b/eclair-core/src/main/scala/fr/acinq/eclair/db/BackupHandler.scala
@@ -1,6 +1,7 @@
 package fr.acinq.eclair.db
 
 import java.io.File
+import java.nio.file.{Files, StandardCopyOption}
 
 import akka.actor.{Actor, ActorLogging, Props}
 import akka.dispatch.{BoundedMessageQueueSemantics, RequiresMessageQueue}
@@ -39,10 +40,16 @@ class BackupHandler private(databases: Databases, backupFile: File, backupScript
       val start = System.currentTimeMillis()
       val tmpFile = new File(backupFile.getAbsolutePath.concat(".tmp"))
       databases.backup(tmpFile)
-      val result = tmpFile.renameTo(backupFile)
-      require(result, s"cannot rename $tmpFile to $backupFile")
+      // this will throw an exception if it fails, which is possible if the backup file is not on the same filesystem
+      // as the temporary file
+      Files.move(tmpFile.toPath, backupFile.toPath, StandardCopyOption.REPLACE_EXISTING, StandardCopyOption.ATOMIC_MOVE)
       val end = System.currentTimeMillis()
+
+      // publish a notification that we have updated our backup
+      context.system.eventStream.publish(BackupCompleted)
+
       log.info(s"database backup triggered by channelId=${persisted.channelId} took ${end - start}ms")
+
       backupScript_opt.foreach(backupScript => {
         Try {
           // run the script in the current thread and wait until it terminates
@@ -55,6 +62,11 @@ class BackupHandler private(databases: Databases, backupFile: File, backupScript
   }
 }
 
+sealed trait BackupEvent
+
+// this notification is sent when we have completed our backup process (our backup file is ready to be used)
+case object BackupCompleted extends BackupEvent
+
 object BackupHandler {
   // using this method is the only way to create a BackupHandler actor
   // we make sure that it uses a custom bounded mailbox, and a custom pinned dispatcher (i.e our actor will have its own thread pool with 1 single thread)
diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/db/BackupHandlerSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/db/BackupHandlerSpec.scala
index d57d9a072..3eefaa894 100644
--- a/eclair-core/src/test/scala/fr/acinq/eclair/db/BackupHandlerSpec.scala
+++ b/eclair-core/src/test/scala/fr/acinq/eclair/db/BackupHandlerSpec.scala
@@ -5,14 +5,12 @@ import java.sql.DriverManager
 import java.util.UUID
 
 import akka.actor.{ActorSystem, Props}
-import akka.testkit.TestKit
+import akka.testkit.{TestKit, TestProbe}
 import fr.acinq.eclair.channel.ChannelPersisted
 import fr.acinq.eclair.db.sqlite.SqliteChannelsDb
 import fr.acinq.eclair.{TestConstants, TestUtils, randomBytes32}
 import org.scalatest.FunSuiteLike
 
-import scala.concurrent.duration._
-
 class BackupHandlerSpec extends TestKit(ActorSystem("test")) with FunSuiteLike {
 
   test("process backups") {
@@ -26,10 +24,13 @@ class BackupHandlerSpec extends TestKit(ActorSystem("test")) with FunSuiteLike {
     assert(db.channels.listLocalChannels() == Seq(channel))
 
     val handler = system.actorOf(BackupHandler.props(db, dest, None))
+    val probe = TestProbe()
+    system.eventStream.subscribe(probe.ref, classOf[BackupEvent])
+
     handler ! ChannelPersisted(null, TestConstants.Alice.nodeParams.nodeId, randomBytes32, null)
     handler ! ChannelPersisted(null, TestConstants.Alice.nodeParams.nodeId, randomBytes32, null)
     handler ! ChannelPersisted(null, TestConstants.Alice.nodeParams.nodeId, randomBytes32, null)
-    awaitCond(dest.exists(), 5 seconds)
+    probe.expectMsg(BackupCompleted)
 
     val db1 = new SqliteChannelsDb(DriverManager.getConnection(s"jdbc:sqlite:$dest"))
     val check = db1.listLocalChannels()