From 5a218719be586a74dfc566e9c2032471e8ad3d68 Mon Sep 17 00:00:00 2001 From: Tobias Binna Date: Wed, 3 Jan 2024 12:44:32 +0800 Subject: [PATCH] Allow saving new host records with same base URL but different client keys - Update `SlickAtlassianHostRepository` and evolution `1.sql` to remove unique constraint for base URL index - Add testcontainer-based integration tests to verify the new behavior and test against a real postgres instance fixes #28 --- .github/workflows/ci.yml | 2 +- build.sbt | 9 + project/Dependencies.scala | 18 +- .../play/slick/ContainerDbConfiguration.scala | 17 ++ .../SlickAtlassianHostRepositoryIt.scala | 191 ++++++++++++++++++ .../slick/fixtures/AtlassianHostFixture.scala | 22 ++ .../slick/generators/AtlassianHostGen.scala | 45 +++++ src/main/resources/evolutions/default/1.sql | 2 +- .../slick/SlickAtlassianHostRepository.scala | 2 +- 9 files changed, 300 insertions(+), 8 deletions(-) create mode 100644 src/it/scala/io/toolsplus/atlassian/connect/play/slick/ContainerDbConfiguration.scala create mode 100644 src/it/scala/io/toolsplus/atlassian/connect/play/slick/SlickAtlassianHostRepositoryIt.scala create mode 100644 src/it/scala/io/toolsplus/atlassian/connect/play/slick/fixtures/AtlassianHostFixture.scala create mode 100644 src/it/scala/io/toolsplus/atlassian/connect/play/slick/generators/AtlassianHostGen.scala diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9e35330..1ed8680 100644 --- a/.github/workflows/ci.yml +++ b/.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 diff --git a/build.sbt b/build.sbt index b9afa79..0c49bc5 100644 --- a/build.sbt +++ b/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) diff --git a/project/Dependencies.scala b/project/Dependencies.scala index a8cfe55..be76f11 100644 --- a/project/Dependencies.scala +++ b/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 } diff --git a/src/it/scala/io/toolsplus/atlassian/connect/play/slick/ContainerDbConfiguration.scala b/src/it/scala/io/toolsplus/atlassian/connect/play/slick/ContainerDbConfiguration.scala new file mode 100644 index 0000000..ee9f661 --- /dev/null +++ b/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 + ) + +} diff --git a/src/it/scala/io/toolsplus/atlassian/connect/play/slick/SlickAtlassianHostRepositoryIt.scala b/src/it/scala/io/toolsplus/atlassian/connect/play/slick/SlickAtlassianHostRepositoryIt.scala new file mode 100644 index 0000000..59bd358 --- /dev/null +++ b/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) + } + } + + } + + } + +} diff --git a/src/it/scala/io/toolsplus/atlassian/connect/play/slick/fixtures/AtlassianHostFixture.scala b/src/it/scala/io/toolsplus/atlassian/connect/play/slick/fixtures/AtlassianHostFixture.scala new file mode 100644 index 0000000..bdc03ab --- /dev/null +++ b/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) +} diff --git a/src/it/scala/io/toolsplus/atlassian/connect/play/slick/generators/AtlassianHostGen.scala b/src/it/scala/io/toolsplus/atlassian/connect/play/slick/generators/AtlassianHostGen.scala new file mode 100644 index 0000000..666d72b --- /dev/null +++ b/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) + +} diff --git a/src/main/resources/evolutions/default/1.sql b/src/main/resources/evolutions/default/1.sql index 0c23e5c..387c6e1 100644 --- a/src/main/resources/evolutions/default/1.sql +++ b/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 diff --git a/src/main/scala/io/toolsplus/atlassian/connect/play/slick/SlickAtlassianHostRepository.scala b/src/main/scala/io/toolsplus/atlassian/connect/play/slick/SlickAtlassianHostRepository.scala index c2646f5..0427f42 100644 --- a/src/main/scala/io/toolsplus/atlassian/connect/play/slick/SlickAtlassianHostRepository.scala +++ b/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,