Merge pull request #29 from toolsplus/features/allow-multiple-host-re…

…cord-with-same-base-url

Allow saving new host records with same base URL but different client keys

.github/workflows/ci.yml

@@ -12,6 +12,6 @@ jobs:
           java-version: '11'
           cache: 'sbt'
       - name: Compile, test
-        run: sbt clean coverage test coverageReport
+        run: sbt clean coverage test IntegrationTest/test coverageReport
       - name: Upload coverage to Codecov
         uses: codecov/codecov-action@v3

build.sbt

@@ -5,6 +5,13 @@ val commonSettings = Seq(
   scalaVersion := "2.13.2"
 )
 
+val integrationTestSettings = Defaults.itSettings ++ Seq(
+  IntegrationTest / fork := true,
+  IntegrationTest / testOptions += Tests.Argument(TestFrameworks.ScalaTest,
+    "-u",
+    "target/test-reports")
+)
+
 lazy val publishSettings = Seq(
   releasePublishArtifactsAction := PgpKeys.publishSigned.value,
   homepage := Some(
@@ -72,5 +79,7 @@ lazy val `atlassian-connect-play-slick` = project
   .in(file("."))
   .settings(libraryDependencies ++= Dependencies.root)
   .settings(commonSettings: _*)
+  .settings(integrationTestSettings: _*)
   .settings(publishSettings)
   .settings(moduleSettings(project))
+  .configs(IntegrationTest)

project/Dependencies.scala

@@ -4,11 +4,14 @@ object Dependencies {
   val root = Seq(
     Library.playSlick,
     Library.atlassianConnectApi,
-    Library.playSlickEvolutions % "test",
-    Library.scalaTest % "test",
-    Library.scalaCheck % "test",
-    Library.scalaTestPlusScalaCheck % "test",
-    Library.h2 % "test"
+    Library.playSlickEvolutions % "test, it",
+    Library.scalaTest % "test, it",
+    Library.scalaCheck % "test, it",
+    Library.scalaTestPlusScalaCheck % "test, it",
+    Library.h2 % "test",
+    Library.postgres % "it",
+    Library.testcontainersScala % "it",
+    Library.testcontainersScalaPostgresql % "it"
   )
 }
 
@@ -19,6 +22,8 @@ object Version {
   val scalaCheck = "1.14.3"
   val scalaTestPlusScalaCheck = "3.1.2.0"
   val h2 = "1.4.197"
+  val postgres = "42.6.0"
+  val testcontainersScala = "0.41.0"
 }
 
 object Library {
@@ -29,4 +34,7 @@ object Library {
   val scalaCheck = "org.scalacheck" %% "scalacheck" % Version.scalaCheck
   val scalaTestPlusScalaCheck = "org.scalatestplus" %% "scalacheck-1-14" % Version.scalaTestPlusScalaCheck
   val h2 = "com.h2database" % "h2" % Version.h2
+  val postgres = "org.postgresql" % "postgresql" % Version.postgres
+  val testcontainersScala = "com.dimafeng" %% "testcontainers-scala-scalatest" % Version.testcontainersScala
+  val testcontainersScalaPostgresql = "com.dimafeng" %% "testcontainers-scala-postgresql" % Version.testcontainersScala
 }

src/it/scala/io/toolsplus/atlassian/connect/play/slick/ContainerDbConfiguration.scala

@@ -0,0 +1,17 @@
+package io.toolsplus.atlassian.connect.play.slick
+
+import com.dimafeng.testcontainers.PostgreSQLContainer
+
+object ContainerDbConfiguration {
+
+  def configuration(container: PostgreSQLContainer): Map[String, Any] =
+    Map(
+      "slick.dbs.default.profile" -> "slick.jdbc.PostgresProfile$",
+      "slick.dbs.default.db.driver" -> "org.postgresql.Driver",
+      "slick.dbs.default.db.url" -> container.jdbcUrl,
+      "slick.dbs.default.db.user" -> container.username,
+      "slick.dbs.default.db.password" -> container.password,
+      "play.evolutions.db.default.enabled" -> true
+    )
+
+}

src/it/scala/io/toolsplus/atlassian/connect/play/slick/SlickAtlassianHostRepositoryIt.scala

@@ -0,0 +1,191 @@
+package io.toolsplus.atlassian.connect.play.slick
+
+import com.dimafeng.testcontainers.PostgreSQLContainer
+import com.dimafeng.testcontainers.scalatest.TestContainerForAll
+import io.toolsplus.atlassian.connect.play.slick.fixtures.AtlassianHostFixture
+import org.scalatest.TestData
+import org.scalatest.concurrent.Eventually
+import org.scalatestplus.play.PlaySpec
+import org.scalatestplus.play.guice.GuiceOneAppPerTest
+import org.testcontainers.utility.DockerImageName
+import play.api.Application
+import play.api.db.DBApi
+import play.api.db.evolutions.{ClassLoaderEvolutionsReader, Evolutions}
+import play.api.inject.guice.GuiceApplicationBuilder
+import play.api.test.{DefaultAwaitTimeout, FutureAwaits}
+
+class SlickAtlassianHostRepositoryIt
+    extends PlaySpec
+    with GuiceOneAppPerTest
+    with FutureAwaits
+    with Eventually
+    with DefaultAwaitTimeout
+    with TestContainerForAll {
+
+  val postgresVersion = "15.5"
+
+  override val containerDef: PostgreSQLContainer.Def =
+    PostgreSQLContainer.Def(
+      DockerImageName.parse(s"postgres:$postgresVersion"),
+      databaseName = "intercom",
+      username = "test",
+      password = "test",
+    )
+
+  override def newAppForTest(td: TestData): Application = withContainers {
+    container =>
+      GuiceApplicationBuilder()
+        .configure(ContainerDbConfiguration.configuration(container))
+        .build()
+  }
+
+  def dbApi(implicit app: Application): DBApi =
+    Application.instanceCache[DBApi].apply(app)
+
+  def withEvolutions[T](block: => T): T =
+    Evolutions.withEvolutions(
+      dbApi.database("default"),
+      ClassLoaderEvolutionsReader.forPrefix("evolutions/")) {
+      block
+    }
+
+  def hostRepo(implicit app: Application): SlickAtlassianHostRepository =
+    Application.instanceCache[SlickAtlassianHostRepository].apply(app)
+
+  "Using a Slick host repository" when {
+
+    "repository is empty" should {
+
+      "not find any hosts when fetching all" in {
+        await {
+          hostRepo.all()
+        } mustEqual Seq.empty
+      }
+
+      "return None when trying to find a non existent host by client key" in {
+        await {
+          hostRepo.findByClientKey("fake-client-key")
+        } mustBe None
+      }
+
+      "return None when trying to find a non existent host by baseUrl" in {
+        await {
+          hostRepo.findByBaseUrl("fake-base-url")
+        } mustBe None
+      }
+
+    }
+
+    "saving a Atlassian hosts to the repository" should {
+
+      "successfully save the host" in new AtlassianHostFixture {
+        withEvolutions {
+          await {
+            hostRepo.save(host)
+          } mustEqual host
+
+          await {
+            hostRepo.all()
+          } mustEqual Seq(host)
+        }
+      }
+
+      "find the inserted host by client key" in new AtlassianHostFixture {
+        withEvolutions {
+          await {
+            hostRepo.save(host)
+          }
+
+          await {
+            hostRepo.findByClientKey(host.clientKey)
+          } mustBe Some(host)
+        }
+      }
+
+      "find the inserted host by base URL" in new AtlassianHostFixture {
+        withEvolutions {
+          await {
+            hostRepo.save(host)
+          } mustBe host
+
+          await {
+            hostRepo.findByBaseUrl(host.baseUrl)
+          } mustBe Some(host)
+        }
+      }
+
+    }
+
+    "saving the same Atlassian hosts twice" should {
+
+      "not duplicate the host" in new AtlassianHostFixture {
+        withEvolutions {
+          await {
+            hostRepo.save(host)
+          } mustBe host
+
+          await {
+            hostRepo.save(host)
+          } mustBe host
+
+          await {
+            hostRepo.all()
+          } mustBe Seq(host)
+        }
+      }
+
+    }
+
+    "updating an Atlassian host" should {
+
+      "successfully store the updated version" in new AtlassianHostFixture {
+        withEvolutions {
+          val updated = host.copy(installed = !host.installed)
+          await {
+            hostRepo.save(host)
+          } mustBe host
+
+          await {
+            hostRepo.save(updated)
+          } mustBe updated
+
+          await {
+            hostRepo.all()
+          } mustBe Seq(updated)
+        }
+      }
+
+    }
+
+    "saving the same Atlassian base URL twice" should {
+      /*
+       * This test case checks that an installation record can be saved even if a record with the same base URL
+       * but different client key already exists.
+       *
+       * This case appears in the following scenarios:
+       * - someone migrates to a new Cloud instance and tries to re-install the app again
+       * - sandbox instances which have been installed before
+       */
+      "duplicate the host" in new AtlassianHostFixture {
+        withEvolutions {
+          await {
+            hostRepo.save(host)
+          } mustBe host
+
+          val updated = host.copy(clientKey = "some-other-client-key")
+
+          await {
+            hostRepo.save(updated)
+          } mustBe updated
+
+          await {
+            hostRepo.all()
+          } mustBe Seq(host, updated)
+        }
+      }
+
+    }
+
+  }
+
+}

src/it/scala/io/toolsplus/atlassian/connect/play/slick/fixtures/AtlassianHostFixture.scala

@@ -0,0 +1,22 @@
+package io.toolsplus.atlassian.connect.play.slick.fixtures
+
+import io.toolsplus.atlassian.connect.play.api.models.DefaultAtlassianHost
+import io.toolsplus.atlassian.connect.play.slick.generators.AtlassianHostGen
+
+trait AtlassianHostFixture extends AtlassianHostGen {
+  val defaultHost = DefaultAtlassianHost(
+    "a890cfe7-3518-3920-b0b5-6fa412a7f3d4",
+    "io.toolsplus.atlassian.connect.play.scala.seed",
+    "MIGfMA0GCSqGDc10pQ4Xo+l/BaWhmiHXDDQ/tOjgfqaDxiXuIi/Jhk4D73aHbL9FwIDAQAB",
+    None,
+    "LkbauUXN71J8jxRi9Nbf+8dwGtXxqta+Fu6k86aF+0IIzxkZ/GlggElYVoCqQg",
+    "100035",
+    "1.2.35",
+    "https://example.atlassian.net",
+    "jira",
+    "Atlassian JIRA at https://example.atlassian.net",
+    None,
+    installed = true
+  )
+  val host = atlassianHostGen.sample.getOrElse(defaultHost)
+}

src/it/scala/io/toolsplus/atlassian/connect/play/slick/generators/AtlassianHostGen.scala

@@ -0,0 +1,45 @@
+package io.toolsplus.atlassian.connect.play.slick.generators
+
+import io.toolsplus.atlassian.connect.play.api.models.Predefined.ClientKey
+import io.toolsplus.atlassian.connect.play.api.models.DefaultAtlassianHost
+import org.scalacheck.Gen
+import org.scalacheck.Gen.{alphaStr, numStr, option, _}
+
+trait AtlassianHostGen {
+
+  def clientKeyGen: Gen[ClientKey] = alphaNumStr
+
+  def pluginVersionGen: Gen[String] =
+    listOfN(3, posNum[Int]).map(n => n.mkString("."))
+
+  def productTypeGen: Gen[String] = oneOf("jira", "confluence")
+
+  def atlassianHostGen: Gen[DefaultAtlassianHost] =
+    for {
+      key <- alphaStr
+      clientKey <- clientKeyGen
+      publicKey <- alphaNumStr
+      oauthClientId <- option(alphaNumStr)
+      sharedSecret <- alphaNumStr.suchThat(s => s.length >= 32 && !s.isEmpty)
+      serverVersion <- numStr
+      pluginsVersion <- pluginVersionGen
+      baseUrl <- alphaStr
+      productType <- productTypeGen
+      description <- alphaStr
+      serviceEntitlementNumber <- option(numStr)
+      installed <- oneOf(true, false)
+    } yield
+      DefaultAtlassianHost(clientKey,
+                            key,
+                            publicKey,
+                            oauthClientId,
+                            sharedSecret,
+                            serverVersion,
+                            pluginsVersion,
+                            baseUrl,
+                            productType,
+                            description,
+                            serviceEntitlementNumber,
+                            installed)
+
+}

src/main/resources/evolutions/default/1.sql

@@ -15,7 +15,7 @@ CREATE TABLE atlassian_host (
 );
 CREATE UNIQUE INDEX uq_ac_host_client_key
   ON atlassian_host (client_key);
-CREATE UNIQUE INDEX uq_ac_host_base_url
+CREATE INDEX uq_ac_host_base_url
   ON atlassian_host (base_url);
 
 # --- !Downs

src/main/scala/io/toolsplus/atlassian/connect/play/slick/SlickAtlassianHostRepository.scala

@@ -72,7 +72,7 @@ private[slick] trait AtlassianHostTable {
 
     val clientKeyIndex =
       index("uq_ac_host_client_key", clientKey, unique = true)
-    val baseUrlIndex = index("uq_ac_host_base_url", baseUrl, unique = true)
+    val baseUrlIndex = index("uq_ac_host_base_url", baseUrl)
 
     def * =
       (clientKey,