Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Database patches: #6345

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 3 additions & 7 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -40,21 +40,18 @@ env:
- >-
BUILD_TYPE=centaurHoricromtalPapiV2beta
BUILD_MYSQL=5.7
- >-
BUILD_TYPE=centaurHoricromtalPapiV2beta
BUILD_MARIADB=10.3
- >-
BUILD_TYPE=centaurHoricromtalEngineUpgradePapiV2alpha1
BUILD_MYSQL=5.7
- >-
BUILD_TYPE=centaurHoricromtalEngineUpgradePapiV2alpha1
BUILD_MARIADB=10.3
- >-
BUILD_TYPE=centaurPapiUpgradePapiV2alpha1
BUILD_MYSQL=5.7
- >-
BUILD_TYPE=centaurPapiUpgradeNewWorkflowsPapiV2alpha1
BUILD_MYSQL=5.7
- >-
BUILD_TYPE=centaurLocal
BUILD_MARIADB=10.3
- >-
BUILD_TYPE=centaurLocal
BUILD_MYSQL=5.7
Expand All @@ -78,7 +75,6 @@ env:
BUILD_MYSQL=5.7
- >-
BUILD_TYPE=checkPublish
BUILD_MYSQL=5.7
- >-
BUILD_TYPE=conformanceLocal
BUILD_MYSQL=5.7
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ abstract class SlickDatabase(override val originalDatabaseConfig: Config) extend
Instead resorting to reflection.
*/
val message = pSQLException.getServerErrorMessage
val field = classOf[ServerErrorMessage].getDeclaredField("m_mesgParts")
val field = classOf[ServerErrorMessage].getDeclaredField("mesgParts")
field.setAccessible(true)
val parts = field.get(message).asInstanceOf[java.util.Map[Character, String]]
parts.remove('D')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,12 @@ object SqlConverters {
// https://github.com/slick/slick/issues/1026

implicit class TimestampToSystemOffsetDateTime(val timestamp: Timestamp) extends AnyVal {
def toSystemOffsetDateTime = timestamp.toLocalDateTime.atZone(ZoneId.systemDefault).toOffsetDateTime
def toSystemOffsetDateTime: OffsetDateTime = timestamp.toLocalDateTime.atZone(ZoneId.systemDefault).toOffsetDateTime
}

implicit class OffsetDateTimeToSystemTimestamp(val offsetDateTime: OffsetDateTime) extends AnyVal {
def toSystemTimestamp = Timestamp.valueOf(offsetDateTime.atZoneSameInstant(ZoneId.systemDefault).toLocalDateTime)
def toSystemTimestamp: Timestamp =
Timestamp.valueOf(offsetDateTime.atZoneSameInstant(ZoneId.systemDefault).toLocalDateTime)
}

implicit class ClobOptionToRawString(val clobOption: Option[Clob]) extends AnyVal {
Expand Down Expand Up @@ -56,10 +57,11 @@ object SqlConverters {
import eu.timepit.refined.api.Refined
import eu.timepit.refined.collection.NonEmpty

def toClobOption: Option[SerialClob] = if (str.isEmpty) None else Option(new SerialClob(str.toCharArray))
def toClobOption: Option[SerialClob] =
if (str == null || str.isEmpty) None else Option(new SerialClob(str.toCharArray))

def toClob(default: String Refined NonEmpty): SerialClob = {
val nonEmpty = if (str.isEmpty) default.value else str
val nonEmpty = if (str == null || str.isEmpty) default.value else str
new SerialClob(nonEmpty.toCharArray)
}
}
Expand Down Expand Up @@ -91,7 +93,7 @@ object SqlConverters {
}

implicit class BytesToBlobOption(val bytes: Array[Byte]) extends AnyVal {
def toBlobOption: Option[SerialBlob] = if (bytes.isEmpty) None else Option(new SerialBlob(bytes))
def toBlobOption: Option[SerialBlob] = if (bytes == null || bytes.isEmpty) None else Option(new SerialBlob(bytes))
}

implicit class EnhancedFiniteDuration(val duration: FiniteDuration) extends AnyVal {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
# if not present, FileSystems.newFileSystem throw NotProviderFoundException
com.upplication.s3fs.S3FileSystemProvider
org.lerch.s3fs.S3FileSystemProvider
18 changes: 4 additions & 14 deletions project/Dependencies.scala
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ object Dependencies {
private val hsqldbV = "2.5.1"
private val http4sVersion = "0.21.7" // scala-steward:off (CROM-6678)
private val jacksonV = "2.12.2"
private val jacksonJqV = "1.0.0-preview.20201123"
private val janinoV = "3.1.3"
private val javaxActivationV = "1.2.0"
// jaxb-impl 2.3.3 depends on com.sun.activation:jakarta.activation and jakarta.xml.bind:jakarta.xml.bind-api,
Expand All @@ -72,12 +71,7 @@ object Dependencies {
private val kindProjectorV = "0.9.9"
private val kittensV = "2.2.1"
private val liquibaseSlf4jV = "4.0.0"
// Scala Steward wanted to upgrade liquibase-core to 3.10.2 but that version does not find some uniqueness
// constraints and models datatypes in ways that are incompatible with our test expectations.
// liquibase-core 4.0.0 did not have either of those problems but produced tons of strange warnings at runtime
// similar in form to this: https://github.com/liquibase/liquibase/issues/1294
// Pinning Liquibase version for the time being.
private val liquibaseV = "3.6.3" // scala-steward:off
private val liquibaseV = "4.3.5"
private val logbackV = "1.2.3"
private val lz4JavaV = "1.7.1"
private val mariadbV = "2.7.2"
Expand All @@ -91,9 +85,7 @@ object Dependencies {
private val owlApiV = "5.1.16" // scala-steward:off (CROM-6677)
private val paradiseV = "2.1.1"
private val pegdownV = "1.6.0"
// For org.postgresql:postgresql 42.2.6 - 42.2.14:
// java.lang.NoSuchFieldException: m_mesgParts in KeyValueSpec "fail if one of the inserts fails"
private val postgresV = "42.2.5" // scala-steward:off
private val postgresV = "42.2.20"
private val rdf4jV = "2.4.2"
private val refinedV = "0.9.22"
private val rhinoV = "1.7.13"
Expand Down Expand Up @@ -243,10 +235,8 @@ object Dependencies {
)

private val liquibaseDependencies = List(
"org.liquibase" % "liquibase-core" % liquibaseV,
// The exclusion below will be needed if / when liquibase-core is upgraded to 3.10+
// Avert collision with jakarta.xml.bind-api
// exclude("javax.xml.bind", "jaxb-api"),
"org.liquibase" % "liquibase-core" % liquibaseV
exclude("javax.xml.bind", "jaxb-api"),
// This is to stop liquibase from being so noisy by default
// See: http://stackoverflow.com/questions/20880783/how-to-get-liquibase-to-log-using-slf4j
"com.mattbertolini" % "liquibase-slf4j" % liquibaseSlf4jV
Expand Down
2 changes: 1 addition & 1 deletion server/src/test/scala/cromwell/CromwellTestKitSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ object CromwellTestKitSpec {
| # Some of our tests fire off a message, then expect a particular event message within 3s (the default).
| # Especially on CI, the metadata test does not seem to be returning in time. So, overriding the timeouts
| # with slightly higher values. Alternatively, could also adjust the akka.test.timefactor only in CI.
| filter-leeway = 10s
| filter-leeway = 20s
| single-expect-default = 5s
| default-timeout = 10s
| }
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
package cromwell.services.database

import java.sql.Connection

import better.files._
import com.dimafeng.testcontainers.{Container, JdbcDatabaseContainer, MariaDBContainer, MySQLContainer, PostgreSQLContainer}
import com.dimafeng.testcontainers._
import com.typesafe.config.{Config, ConfigFactory}
import com.typesafe.scalalogging.StrictLogging
import cromwell.database.migration.liquibase.LiquibaseUtils
Expand All @@ -12,12 +10,14 @@ import cromwell.services.ServicesStore.EnhancedSqlDatabase
import cromwell.services.{EngineServicesStore, MetadataServicesStore}
import liquibase.snapshot.DatabaseSnapshot
import liquibase.structure.core.Index
import org.testcontainers.containers.{JdbcDatabaseContainer => JavaJdbcDatabaseContainer}
import org.testcontainers.utility.DockerImageName
import slick.jdbc.JdbcProfile
import slick.jdbc.meta.{MIndexInfo, MPrimaryKey}

import java.sql.Connection
import scala.concurrent.Await
import scala.concurrent.duration.Duration
import scala.concurrent.duration._

object DatabaseTestKit extends StrictLogging {

Expand Down Expand Up @@ -109,7 +109,7 @@ object DatabaseTestKit extends StrictLogging {
}

def getDatabaseTestContainer(databaseSystem: DatabaseSystem): Option[Container] = {
databaseSystem match {
val containerOption: Option[SingleContainer[_ <: JavaJdbcDatabaseContainer[_]]] = databaseSystem match {
case HsqldbDatabaseSystem => None
case networkDbSystem: NetworkDatabaseSystem =>
networkDbSystem.platform match {
Expand All @@ -135,6 +135,12 @@ object DatabaseTestKit extends StrictLogging {
case _ => None
}
}

// Give slow CI like TravisCI a bit more patience to pull and then start the container
containerOption.map(_.configure { container =>
container.withStartupTimeoutSeconds(5.minutes.toSeconds.toInt)
()
})
}

def initializeDatabaseByContainerOptTypeAndSystem[A <: SlickDatabase](containerOpt: Option[Container],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,18 @@ import scala.reflect._
*/
class LiquibaseComparisonSpec extends AnyFlatSpec with CromwellTimeoutSpec with Matchers with ScalaFutures {

implicit val executionContext = ExecutionContext.global
implicit val executionContext: ExecutionContext = ExecutionContext.global

implicit val defaultPatience = PatienceConfig(timeout = scaled(5.seconds), interval = scaled(100.millis))
implicit val defaultPatience: PatienceConfig =
PatienceConfig(timeout = scaled(5.seconds), interval = scaled(100.millis))

CromwellDatabaseType.All foreach { databaseType =>

lazy val expectedSnapshot = DatabaseTestKit.inMemorySnapshot(databaseType, SlickSchemaManager)
lazy val expectedColumns = get[Column](expectedSnapshot)
lazy val expectedPrimaryKeys = get[PrimaryKey](expectedSnapshot)
lazy val expectedForeignKeys = get[ForeignKey](expectedSnapshot)
lazy val expectedUniqueConstraints = get[UniqueConstraint](expectedSnapshot)
lazy val expectedColumns = get[Column](expectedSnapshot).sorted
lazy val expectedPrimaryKeys = get[PrimaryKey](expectedSnapshot).sorted
lazy val expectedForeignKeys = get[ForeignKey](expectedSnapshot).sorted
lazy val expectedUniqueConstraints = get[UniqueConstraint](expectedSnapshot).sorted
lazy val expectedIndexes = get[Index](expectedSnapshot) filterNot DatabaseTestKit.isGenerated

DatabaseSystem.All foreach { databaseSystem =>
Expand Down Expand Up @@ -80,7 +82,8 @@ class LiquibaseComparisonSpec extends AnyFlatSpec with CromwellTimeoutSpec with
// Auto increment columns may have different types, such as SERIAL/BIGSERIAL
// https://www.postgresql.org/docs/11/datatype-numeric.html#DATATYPE-SERIAL
val actualColumnDefault = ColumnDefault(actualColumnType, actualColumn.getDefaultValue)
val autoIncrementDefault = getAutoIncrementDefault(databaseSystem, columnMapping, expectedColumn)
val autoIncrementDefault =
getAutoIncrementDefault(expectedColumn, columnMapping, databaseSystem, connectionMetadata)
actualColumnDefault should be(autoIncrementDefault)
} else {

Expand Down Expand Up @@ -162,6 +165,8 @@ class LiquibaseComparisonSpec extends AnyFlatSpec with CromwellTimeoutSpec with
}
val actualForeignKey = actualForeignKeyOption getOrElse fail(s"Did not find $description")

actualForeignKey.getPrimaryKeyTable.getName should be(expectedForeignKey.getPrimaryKeyTable.getName)
actualForeignKey.getForeignKeyTable.getName should be(expectedForeignKey.getForeignKeyTable.getName)
actualForeignKey.getPrimaryKeyColumns.asScala.map(ColumnDescription.from) should
contain theSameElementsAs expectedForeignKey.getPrimaryKeyColumns.asScala.map(ColumnDescription.from)
actualForeignKey.getForeignKeyColumns.asScala.map(ColumnDescription.from) should
Expand Down Expand Up @@ -194,6 +199,7 @@ class LiquibaseComparisonSpec extends AnyFlatSpec with CromwellTimeoutSpec with
val actualUniqueConstraint = actualUniqueConstraintOption getOrElse
fail(s"Did not find $description")

actualUniqueConstraint.getRelation.getName should be(expectedUniqueConstraint.getRelation.getName)
actualUniqueConstraint.getColumns.asScala.map(ColumnDescription.from) should
contain theSameElementsAs expectedUniqueConstraint.getColumns.asScala.map(ColumnDescription.from)
}
Expand Down Expand Up @@ -228,7 +234,7 @@ class LiquibaseComparisonSpec extends AnyFlatSpec with CromwellTimeoutSpec with
object LiquibaseComparisonSpec {
private def get[T <: DatabaseObject : ClassTag : Ordering](databaseSnapshot: DatabaseSnapshot): Seq[T] = {
val databaseObjectClass = classTag[T].runtimeClass.asInstanceOf[Class[T]]
databaseSnapshot.get(databaseObjectClass).asScala.toSeq.sorted
databaseSnapshot.get(databaseObjectClass).asScala.toSeq
}

private val DefaultNullBoolean = Boolean.box(false)
Expand Down Expand Up @@ -276,8 +282,8 @@ object LiquibaseComparisonSpec {

case class ColumnMapping
(
typeMapping: Map[ColumnType, ColumnType] = Map.empty,
defaultMapping: Map[ColumnDefault, ColumnDefault] = Map.empty,
typeMapping: PartialFunction[ColumnType, ColumnType] = PartialFunction.empty,
defaultMapping: Map[ColumnDefault, AnyRef] = Map.empty,
)

/** Generate the expected PostgreSQL sequence name for a column. */
Expand Down Expand Up @@ -315,6 +321,9 @@ object LiquibaseComparisonSpec {
private val HsqldbTypeInteger = ColumnType("INTEGER", Option(32))
private val HsqldbTypeTimestamp = ColumnType("TIMESTAMP")

// Defaults as they are represented in HSQLDB that will have different representations in other DBMS.
private val HsqldbDefaultBooleanTrue = ColumnDefault(HsqldbTypeBoolean, Boolean.box(true))

// Nothing to map as the original is also HSQLDB
private val HsqldbColumnMapping = ColumnMapping()

Expand All @@ -331,13 +340,25 @@ object LiquibaseComparisonSpec {
HsqldbTypeTimestamp -> ColumnType("DATETIME"),
),
defaultMapping = Map(
ColumnDefault(HsqldbTypeBoolean, Boolean.box(true)) ->
ColumnDefault(ColumnType("TINYINT", Option(3)), Int.box(1)),
HsqldbDefaultBooleanTrue -> Int.box(1)
),
)

// MariaDB should behave exactly the same as MySQL
private val MariadbColumnMapping = MysqldbColumnMapping
// MariaDB should behave similar to MySQL except that only LOBs have sizes
private val MariadbColumnMapping =
ColumnMapping(
typeMapping = Map(
HsqldbTypeBigInt -> ColumnType("BIGINT"),
HsqldbTypeBlob -> ColumnType("LONGBLOB", Option(2147483647)),
HsqldbTypeBoolean -> ColumnType("TINYINT"),
HsqldbTypeClob -> ColumnType("LONGTEXT", Option(2147483647)),
HsqldbTypeInteger -> ColumnType("INT"),
HsqldbTypeTimestamp -> ColumnType("DATETIME"),
),
defaultMapping = Map(
HsqldbDefaultBooleanTrue -> Int.box(1),
),
)

private val PostgresqlColumnMapping =
ColumnMapping(
Expand Down Expand Up @@ -367,24 +388,26 @@ object LiquibaseComparisonSpec {
*/
private def getColumnType(column: Column, columnMapping: ColumnMapping): ColumnType = {
val columnType = ColumnType.from(column)
columnMapping.typeMapping.getOrElse(columnType, columnType)
columnMapping.typeMapping.applyOrElse[ColumnType, ColumnType](columnType, _ => columnType)
}

/**
* Returns the default for the column, either from ColumnMapping or the column itself.
*/
private def getColumnDefault(column: Column, columnMapping: ColumnMapping): AnyRef = {
columnMapping.defaultMapping get ColumnDefault.from(column) map (_.defaultValue) getOrElse column.getDefaultValue
columnMapping.defaultMapping.getOrElse(ColumnDefault.from(column), column.getDefaultValue)
}

/**
* Return the default for the auto increment column.
*/
private def getAutoIncrementDefault(databaseSystem: DatabaseSystem,
private def getAutoIncrementDefault(column: Column,
columnMapping: ColumnMapping,
column: Column): ColumnDefault = {
databaseSystem: DatabaseSystem,
connectionMetadata: ConnectionMetadata,
): ColumnDefault = {
databaseSystem.platform match {
case PostgresqlDatabasePlatform =>
case PostgresqlDatabasePlatform if connectionMetadata.databaseMajorVersion <= 9 =>
val columnType = column.getType.getTypeName match {
case "BIGINT" => ColumnType("BIGSERIAL", None)
case "INTEGER" => ColumnType("SERIAL", None)
Expand Down Expand Up @@ -441,8 +464,8 @@ object LiquibaseComparisonSpec {
/**
* Returns an optional extra check to ensure that sequences have the same types as their auto increment columns.
*
* This is because PostgreSQL requires two statements to modify SERIAL columns to BIGSERIAL, one to widen the column,
* and another to widen the sequence.
* This is because PostgreSQL <= 9 requires two statements to modify SERIAL columns to BIGSERIAL, one to widen the
* column, and another to widen the sequence.
*
* https://stackoverflow.com/questions/52195303/postgresql-primary-key-id-datatype-from-serial-to-bigserial#answer-52195920
* https://www.postgresql.org/docs/11/datatype-numeric.html#DATATYPE-SERIAL
Expand All @@ -455,8 +478,6 @@ object LiquibaseComparisonSpec {
case PostgresqlDatabasePlatform if column.isAutoIncrement && connectionMetadata.databaseMajorVersion <= 9 =>
// "this is currently always bigint" --> https://www.postgresql.org/docs/9.6/infoschema-sequences.html
Option("bigint")
case PostgresqlDatabasePlatform if column.isAutoIncrement =>
Option(column.getType.getTypeName.toLowerCase)
case _ => None
}
}
Expand All @@ -467,8 +488,8 @@ object LiquibaseComparisonSpec {
import database.dataAccess.driver.api._
databaseSystem.platform match {
case PostgresqlDatabasePlatform if column.isAutoIncrement =>
//noinspection SqlDialectInspection
sql"""select data_type
//noinspection SqlDialectInspection
sql"""select data_type
from INFORMATION_SCHEMA.sequences
where sequence_name = '#${postgresqlSeqName(column)}'
""".as[String].head
Expand Down
Loading