Database nits (#1755)

* proper formatting

* prefix sqlite-specific tests

* fix double init tests

Due to how the `TestDatabases` is instantiated, calling `dbs.dbName`
twice was a no-op.

* add jdbcurl files to gitignore

Author: pm47

.gitignore

@@ -25,5 +25,6 @@ target/
 project/target
 DeleteMe*.*
 *~
+jdbcUrlFile_*.tmp
 
 .DS_Store

eclair-core/src/main/scala/fr/acinq/eclair/db/Databases.scala

@@ -54,13 +54,13 @@ object Databases extends Logging {
     def obtainExclusiveLock(): Unit
   }
 
-  case class SqliteDatabases private (network: SqliteNetworkDb,
-                             audit: SqliteAuditDb,
-                             channels: SqliteChannelsDb,
-                             peers: SqlitePeersDb,
-                             payments: SqlitePaymentsDb,
-                             pendingRelay: SqlitePendingRelayDb,
-                             private val backupConnection: Connection) extends Databases with FileBackup {
+  case class SqliteDatabases private(network: SqliteNetworkDb,
+                                     audit: SqliteAuditDb,
+                                     channels: SqliteChannelsDb,
+                                     peers: SqlitePeersDb,
+                                     payments: SqlitePaymentsDb,
+                                     pendingRelay: SqlitePendingRelayDb,
+                                     private val backupConnection: Connection) extends Databases with FileBackup {
     override def backup(backupFile: File): Unit = SqliteUtils.using(backupConnection.createStatement()) {
       statement => {
         statement.executeUpdate(s"backup to ${backupFile.getAbsolutePath}")
@@ -80,14 +80,14 @@ object Databases extends Logging {
     )
   }
 
-  case class PostgresDatabases private (network: PgNetworkDb,
-                               audit: PgAuditDb,
-                               channels: PgChannelsDb,
-                               peers: PgPeersDb,
-                               payments: PgPaymentsDb,
-                               pendingRelay: PgPendingRelayDb,
-                               dataSource: HikariDataSource,
-                               lock: PgLock) extends Databases with ExclusiveLock {
+  case class PostgresDatabases private(network: PgNetworkDb,
+                                       audit: PgAuditDb,
+                                       channels: PgChannelsDb,
+                                       peers: PgPeersDb,
+                                       payments: PgPaymentsDb,
+                                       pendingRelay: PgPendingRelayDb,
+                                       dataSource: HikariDataSource,
+                                       lock: PgLock) extends Databases with ExclusiveLock {
     override def obtainExclusiveLock(): Unit = lock.obtainExclusiveLock(dataSource)
   }
 

eclair-core/src/test/scala/fr/acinq/eclair/TestDatabases.scala

@@ -2,6 +2,7 @@ package fr.acinq.eclair
 
 import akka.actor.ActorSystem
 import com.opentable.db.postgres.embedded.EmbeddedPostgres
+import com.zaxxer.hikari.HikariConfig
 import fr.acinq.eclair.db.pg.PgUtils
 import fr.acinq.eclair.db.pg.PgUtils.PgLock
 import fr.acinq.eclair.db.sqlite.SqliteUtils
@@ -11,6 +12,7 @@ import fr.acinq.eclair.db.pg.PgUtils.PgLock.LockFailureHandler
 import java.io.File
 import java.sql.{Connection, DriverManager, Statement}
 import java.util.UUID
+import javax.sql.DataSource
 import scala.concurrent.duration._
 
 
@@ -49,12 +51,9 @@ object TestDatabases {
 
   case class TestPgDatabases() extends TestDatabases {
     private val pg = EmbeddedPostgres.start()
-
-    import com.zaxxer.hikari.HikariConfig
-
+    val datasource: DataSource = pg.getPostgresDatabase
     val hikariConfig = new HikariConfig
-    hikariConfig.setDataSource(pg.getPostgresDatabase)
-
+    hikariConfig.setDataSource(datasource)
     val lock: PgLock.LeaseLock = PgLock.LeaseLock(UUID.randomUUID(), 10 minutes, 8 minute, LockFailureHandler.logAndThrow)
 
     val jdbcUrlFile: File = new File(sys.props("tmp.dir"), s"jdbcUrlFile_${UUID.randomUUID()}.tmp")
@@ -71,11 +70,11 @@ object TestDatabases {
   }
 
   def forAllDbs(f: TestDatabases => Unit): Unit = {
+    def using(dbs: TestDatabases)(g: TestDatabases => Unit): Unit = try g(dbs) finally dbs.close()
     // @formatter:off
-  def using(dbs: TestDatabases)(g: TestDatabases => Unit): Unit = try g(dbs) finally dbs.close()
-  using(TestSqliteDatabases())(f)
-  using(TestPgDatabases())(f)
-  // @formatter:on
+    using(TestSqliteDatabases())(f)
+    using(TestPgDatabases())(f)
+    // @fodmatter:on
   }
 
 }

eclair-core/src/test/scala/fr/acinq/eclair/db/AuditDbSpec.scala

@@ -18,13 +18,14 @@ package fr.acinq.eclair.db
 
 import fr.acinq.bitcoin.Crypto.PrivateKey
 import fr.acinq.bitcoin.{ByteVector32, SatoshiLong, Transaction}
-import fr.acinq.eclair.TestDatabases.{TestPgDatabases, TestSqliteDatabases, forAllDbs}
+import fr.acinq.eclair.TestDatabases.{TestPgDatabases, TestSqliteDatabases}
 import fr.acinq.eclair._
 import fr.acinq.eclair.channel.Helpers.Closing.MutualClose
 import fr.acinq.eclair.channel.{ChannelErrorOccurred, LocalError, NetworkFeePaid, RemoteError}
 import fr.acinq.eclair.db.AuditDb.Stats
 import fr.acinq.eclair.db.DbEventHandler.ChannelEvent
 import fr.acinq.eclair.db.jdbc.JdbcUtils.using
+import fr.acinq.eclair.db.pg.PgAuditDb
 import fr.acinq.eclair.db.sqlite.SqliteAuditDb
 import fr.acinq.eclair.payment._
 import fr.acinq.eclair.wire.protocol.Error
@@ -37,12 +38,18 @@ import scala.util.Random
 
 class AuditDbSpec extends AnyFunSuite {
 
+  import fr.acinq.eclair.TestDatabases.forAllDbs
+
   val ZERO_UUID: UUID = UUID.fromString("00000000-0000-0000-0000-000000000000")
 
-  test("init database 2 times in a row") {
-    forAllDbs { dbs =>
-      val db1 = dbs.audit
-      val db2 = dbs.audit
+  test("init database two times in a row") {
+    forAllDbs {
+      case sqlite: TestSqliteDatabases =>
+        new SqliteAuditDb(sqlite.connection)
+        new SqliteAuditDb(sqlite.connection)
+      case pg: TestPgDatabases =>
+        new PgAuditDb()(pg.datasource)
+        new PgAuditDb()(pg.datasource)
     }
   }
 

eclair-core/src/test/scala/fr/acinq/eclair/db/ChannelsDbSpec.scala

@@ -18,9 +18,10 @@ package fr.acinq.eclair.db
 
 import com.softwaremill.quicklens._
 import fr.acinq.bitcoin.ByteVector32
-import fr.acinq.eclair.TestDatabases.{TestPgDatabases, TestSqliteDatabases, forAllDbs}
+import fr.acinq.eclair.TestDatabases.{TestPgDatabases, TestSqliteDatabases}
 import fr.acinq.eclair.db.DbEventHandler.ChannelEvent
 import fr.acinq.eclair.db.jdbc.JdbcUtils.using
+import fr.acinq.eclair.db.pg.PgChannelsDb
 import fr.acinq.eclair.db.sqlite.SqliteChannelsDb
 import fr.acinq.eclair.db.sqlite.SqliteUtils.ExtendedResultSet._
 import fr.acinq.eclair.wire.internal.channel.ChannelCodecs.stateDataCodec
@@ -33,10 +34,16 @@ import java.sql.SQLException
 
 class ChannelsDbSpec extends AnyFunSuite {
 
-  test("init database 2 times in a row") {
-    forAllDbs { dbs =>
-      val db1 = dbs.channels
-      val db2 = dbs.channels
+  import fr.acinq.eclair.TestDatabases.forAllDbs
+
+  test("init database two times in a row") {
+    forAllDbs {
+      case sqlite: TestSqliteDatabases =>
+        new SqliteChannelsDb(sqlite.connection)
+        new SqliteChannelsDb(sqlite.connection)
+      case pg: TestPgDatabases =>
+        new PgChannelsDb()(pg.datasource, pg.lock)
+        new PgChannelsDb()(pg.datasource, pg.lock)
     }
   }
 

eclair-core/src/test/scala/fr/acinq/eclair/db/NetworkDbSpec.scala

@@ -21,6 +21,8 @@ import fr.acinq.bitcoin.{Block, ByteVector32, ByteVector64, Crypto, Satoshi, Sat
 import fr.acinq.eclair.FeatureSupport.Optional
 import fr.acinq.eclair.Features.VariableLengthOnion
 import fr.acinq.eclair.TestDatabases._
+import fr.acinq.eclair.db.pg.PgNetworkDb
+import fr.acinq.eclair.db.sqlite.SqliteNetworkDb
 import fr.acinq.eclair.db.sqlite.SqliteUtils._
 import fr.acinq.eclair.router.Announcements
 import fr.acinq.eclair.router.Router.PublicChannel
@@ -32,12 +34,18 @@ import scala.collection.{SortedMap, mutable}
 
 class NetworkDbSpec extends AnyFunSuite {
 
-  val shortChannelIds = (42 to (5000 + 42)).map(i => ShortChannelId(i))
+  import fr.acinq.eclair.TestDatabases.forAllDbs
 
-  test("init database 2 times in a row") {
-    forAllDbs { dbs =>
-      val db1 = dbs.network
-      val db2 = dbs.network
+  val shortChannelIds: Seq[ShortChannelId] = (42 to (5000 + 42)).map(i => ShortChannelId(i))
+
+  test("init database two times in a row") {
+    forAllDbs {
+      case sqlite: TestSqliteDatabases =>
+        new SqliteNetworkDb(sqlite.connection)
+        new SqliteNetworkDb(sqlite.connection)
+      case pg: TestPgDatabases =>
+        new PgNetworkDb()(pg.datasource)
+        new PgNetworkDb()(pg.datasource)
     }
   }
 

eclair-core/src/test/scala/fr/acinq/eclair/db/PaymentsDbSpec.scala

@@ -20,6 +20,7 @@ import fr.acinq.bitcoin.Crypto.PrivateKey
 import fr.acinq.bitcoin.{Block, ByteVector32, Crypto}
 import fr.acinq.eclair.TestDatabases.{TestPgDatabases, TestSqliteDatabases, forAllDbs}
 import fr.acinq.eclair.crypto.Sphinx
+import fr.acinq.eclair.db.pg.PgPaymentsDb
 import fr.acinq.eclair.db.sqlite.SqlitePaymentsDb
 import fr.acinq.eclair.payment._
 import fr.acinq.eclair.router.Router.{ChannelHop, NodeHop}
@@ -34,10 +35,14 @@ class PaymentsDbSpec extends AnyFunSuite {
 
   import PaymentsDbSpec._
 
-  test("init database 2 times in a row") {
-    forAllDbs { dbs =>
-      val db1 = dbs.payments
-      val db2 = dbs.payments
+  test("init database two times in a row") {
+    forAllDbs {
+      case sqlite: TestSqliteDatabases =>
+        new SqlitePaymentsDb(sqlite.connection)
+        new SqlitePaymentsDb(sqlite.connection)
+      case pg: TestPgDatabases =>
+        new PgPaymentsDb()(pg.datasource, pg.lock)
+        new PgPaymentsDb()(pg.datasource, pg.lock)
     }
   }
 

eclair-core/src/test/scala/fr/acinq/eclair/db/PeersDbSpec.scala

@@ -17,6 +17,9 @@
 package fr.acinq.eclair.db
 
 import fr.acinq.bitcoin.Crypto.PublicKey
+import fr.acinq.eclair.TestDatabases.{TestPgDatabases, TestSqliteDatabases}
+import fr.acinq.eclair.db.pg.PgPeersDb
+import fr.acinq.eclair.db.sqlite.SqlitePeersDb
 import fr.acinq.eclair.randomKey
 import fr.acinq.eclair.wire.protocol.{NodeAddress, Tor2, Tor3}
 import org.scalatest.funsuite.AnyFunSuite
@@ -26,10 +29,14 @@ class PeersDbSpec extends AnyFunSuite {
 
   import fr.acinq.eclair.TestDatabases.forAllDbs
 
-  test("init database 2 times in a row") {
-    forAllDbs { dbs =>
-      val db1 = dbs.peers
-      val db2 = dbs.peers
+  test("init database two times in a row") {
+    forAllDbs {
+      case sqlite: TestSqliteDatabases =>
+        new SqlitePeersDb(sqlite.connection)
+        new SqlitePeersDb(sqlite.connection)
+      case pg: TestPgDatabases =>
+        new PgPeersDb()(pg.datasource, pg.lock)
+        new PgPeersDb()(pg.datasource, pg.lock)
     }
   }
 

eclair-core/src/test/scala/fr/acinq/eclair/db/PendingRelayDbSpec.scala

@@ -16,7 +16,10 @@
 
 package fr.acinq.eclair.db
 
+import fr.acinq.eclair.TestDatabases.{TestPgDatabases, TestSqliteDatabases}
 import fr.acinq.eclair.channel.{CMD_FAIL_HTLC, CMD_FAIL_MALFORMED_HTLC, CMD_FULFILL_HTLC}
+import fr.acinq.eclair.db.pg.PgPendingRelayDb
+import fr.acinq.eclair.db.sqlite.SqlitePendingRelayDb
 import fr.acinq.eclair.randomBytes32
 import fr.acinq.eclair.wire.protocol.FailureMessageCodecs
 import org.scalatest.funsuite.AnyFunSuite
@@ -26,10 +29,14 @@ class PendingRelayDbSpec extends AnyFunSuite {
 
   import fr.acinq.eclair.TestDatabases.forAllDbs
 
-  test("init database 2 times in a row") {
-    forAllDbs { dbs =>
-      val db1 = dbs.pendingRelay
-      val db2 = dbs.pendingRelay
+  test("init database two times in a row") {
+    forAllDbs {
+      case sqlite: TestSqliteDatabases =>
+        new SqlitePendingRelayDb(sqlite.connection)
+        new SqlitePendingRelayDb(sqlite.connection)
+      case pg: TestPgDatabases =>
+        new PgPendingRelayDb()(pg.datasource, pg.lock)
+        new PgPendingRelayDb()(pg.datasource, pg.lock)
     }
   }
 

eclair-core/src/test/scala/fr/acinq/eclair/db/FeeratesDbSpec.scala eclair-core/src/test/scala/fr/acinq/eclair/db/SqliteFeeratesDbSpec.scala

@@ -23,9 +23,9 @@ import fr.acinq.eclair.db.sqlite.SqliteFeeratesDb
 import fr.acinq.eclair.db.sqlite.SqliteUtils.{getVersion, using}
 import org.scalatest.funsuite.AnyFunSuite
 
-class FeeratesDbSpec extends AnyFunSuite {
+class SqliteFeeratesDbSpec extends AnyFunSuite {
 
-  val feerate = FeeratesPerKB(
+  val feerate: FeeratesPerKB = FeeratesPerKB(
     mempoolMinFee = FeeratePerKB(10000 sat),
     block_1 = FeeratePerKB(150000 sat),
     blocks_2 = FeeratePerKB(120000 sat),

eclair-core/src/test/scala/fr/acinq/eclair/db/FileBackupHandlerSpec.scala eclair-core/src/test/scala/fr/acinq/eclair/db/SqliteFileBackupHandlerSpec.scala

@@ -28,7 +28,7 @@ import java.io.File
 import java.sql.DriverManager
 import java.util.UUID
 
-class FileBackupHandlerSpec extends TestKitBaseClass with AnyFunSuiteLike {
+class SqliteFileBackupHandlerSpec extends TestKitBaseClass with AnyFunSuiteLike {
 
   test("process backups") {
     val db = TestDatabases.inMemoryDb()