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

Allow registering DriverInitializer for VerifyMigrationTask with ServiceLoader mechanism #3986

Merged
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
399ba49
Allow passing properties to a migration driver
C2H6O Mar 21, 2023
2d449c6
spotlessApply
C2H6O Mar 21, 2023
f48896e
Merge branch 'master' into lx/migration_driver_connection_properties
C2H6O Mar 23, 2023
3e782d9
Merge branch 'master' into lx/migration_driver_connection_properties
C2H6O Mar 23, 2023
a5c4b49
Merge branch 'master' into lx/migration_driver_connection_properties
C2H6O Mar 24, 2023
bd3ef8e
Add findFirst() to actually load the Driver class
C2H6O Mar 24, 2023
693b3a2
Merge branch 'master' into lx/migration_driver_connection_properties
C2H6O Mar 25, 2023
a879695
Merge branch 'master' into lx/migration_driver_connection_properties
C2H6O Mar 27, 2023
7991af0
Modify VerifyMigrationTask to accept connectionProperties and loading…
C2H6O Mar 27, 2023
bf6ac56
remove connectionProperties from SqlDelightDatabase
C2H6O Mar 27, 2023
d97e81a
different strategy with DriverInitializer
C2H6O Mar 28, 2023
037a269
fixups
C2H6O Mar 28, 2023
1617857
Merge branch 'master' into lx/migration_driver_connection_properties
C2H6O Mar 28, 2023
aeb66c4
try nullable DriverInitializer to see if it passing CI
C2H6O Mar 28, 2023
0a954cb
Test DriverInitializerImpl
C2H6O Apr 3, 2023
e75a3bf
ClassNotFoundException
C2H6O Apr 3, 2023
a8824c1
forgotten rename
C2H6O Apr 3, 2023
cb9df45
Use service loader mechanism
hfhbd Apr 4, 2023
c8dd79f
Expose the configuration name
hfhbd Apr 5, 2023
6c5a53f
Merge pull request #2 from cashapp/hfhbd/migration_driver_connection_…
C2H6O Apr 5, 2023
4e14be9
connectionProperties
C2H6O Apr 5, 2023
cf7a0d3
pass the Properties to DriverInitializer#execute
C2H6O Apr 5, 2023
f028b6b
change execute of impl to include Properties
C2H6O Apr 6, 2023
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
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ import org.gradle.api.tasks.TaskAction
import org.gradle.workers.WorkAction
import org.gradle.workers.WorkParameters
import java.io.File
import java.io.Serializable
import java.sql.DriverManager
import java.util.ServiceLoader

@CacheableTask
Expand All @@ -51,6 +53,8 @@ abstract class VerifyMigrationTask : SqlDelightWorkerTask() {

@Input var verifyDefinitions: Boolean = true

@get:Internal var driverInitializer: DriverInitializer? = null

/* Tasks without an output are never considered UP-TO-DATE by Gradle. Adding an output file that's created when the
* task completes successfully works around the lack of an output for this task. There may be a better solution once
* https://github.com/gradle/gradle/issues/14223 is resolved. */
Expand All @@ -68,6 +72,7 @@ abstract class VerifyMigrationTask : SqlDelightWorkerTask() {
it.verifyMigrations.set(verifyMigrations)
it.compilationUnit.set(compilationUnit)
it.verifyDefinitions.set(verifyDefinitions)
it.driverInitializer.set(driverInitializer)
}
workQueue.await()
}.onSuccess {
Expand All @@ -92,6 +97,7 @@ abstract class VerifyMigrationTask : SqlDelightWorkerTask() {
val compilationUnit: Property<SqlDelightCompilationUnit>
val verifyMigrations: Property<Boolean>
val verifyDefinitions: Property<Boolean>
val driverInitializer: Property<DriverInitializer>
}

abstract class VerifyMigrationAction : WorkAction<VerifyMigrationWorkParameters> {
Expand All @@ -116,6 +122,7 @@ abstract class VerifyMigrationTask : SqlDelightWorkerTask() {
if (!environment.dialect.isSqlite) return
parameters.workingDirectory.get().asFile.deleteRecursively()

parameters.driverInitializer.orNull?.execute()
val catalog = createCurrentDb()

val databaseFiles = sourceFolders.asSequence()
Expand Down Expand Up @@ -199,3 +206,17 @@ abstract class VerifyMigrationTask : SqlDelightWorkerTask() {
}
}
}

/**
* Allows consumers to configure and register (with [DriverManager]) their custom drivers prior to
* running migration verification task.
*/
interface DriverInitializer : Serializable {
fun execute()
hfhbd marked this conversation as resolved.
Show resolved Hide resolved
}

class DriverInitializerImpl: DriverInitializer {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where do I place this class for testing purposes so that it is accessible during a test run?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could create a gradle project in sqldelight-gradle-plugin/src/test, just copy an existing test project and add the test executor here too: https://github.com/cashapp/sqldelight/blob/68a4d849d234b0018c2c2b314915952564ecfe2b/sqldelight-gradle-plugin/src/test/kotlin/app/cash/sqldelight/tests/MigrationTest.kt

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh okay, you already added it the project. This depends on your Gradle setup, for tests you could simple move this class into build.gradle. This should work, I think.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yea, just having it live in the build.gradle file should work

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like VerifyMigrationTask needs the DriverInitializerImpl on its classpath, since now I'm getting ClassNotFoundException

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do I add the classpath of the build.gradle file to the VerifyMigrationTask? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hfhbd any Gradle tips for getting this working? I'm kind of running out of ideas

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I took a look and played with different implementations. The only one I got to run is using service loaders. Other implementations failed with serialization errors.

Copy link
Collaborator

@hfhbd hfhbd Apr 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I created a PR for this PR, because I can't push to this branch: https://github.com/plangrid/sqldelight/pull/2/files

override fun execute() {
println("DriverInitializerImpl executed!")
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,16 @@ class MigrationTest {
assertThat(output.output).contains("BUILD SUCCESSFUL")
}

@Test fun `driver initializer is executed`() {
val output = GradleRunner.create()
.withCommonConfiguration(File("src/test/migration-driver-initializer"))
.withArguments("clean", "verifyMainDatabaseMigration", "--stacktrace")
.build()

assertThat(output.output).contains("DriverInitializerImpl executed!")
assertThat(output.output).contains("BUILD SUCCESSFUL")
}

@Test fun `failing migration errors properly`() {
val output = GradleRunner.create()
.withCommonConfiguration(File("src/test/migration-failure"))
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import app.cash.sqldelight.gradle.VerifyMigrationTask
import app.cash.sqldelight.gradle.DriverInitializerImpl

buildscript {
apply from: "${projectDir.absolutePath}/../buildscript.gradle"
}

apply plugin: 'org.jetbrains.kotlin.jvm'
apply plugin: 'app.cash.sqldelight'

sqldelight {
databases {
Database {
packageName = "com.example"
verifyMigrations = true
}
}
}

dependencies {
implementation libs.junit
}

tasks.whenTaskAdded {
if (it.name == "verifyMainDatabaseMigration") {
def task = it as VerifyMigrationTask
task.configure {
it.driverInitializer = new DriverInitializerImpl()
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
apply from: "../settings.gradle"

rootProject.name = 'sqldelight-migrations'
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
CREATE TABLE test(
value TEXT NOT NULL,
value2 TEXT
);

CREATE INDEX testIndex
ON test(value);

CREATE TRIGGER testTrigger
AFTER DELETE ON test
BEGIN
INSERT INTO test VALUES ("1", "2");
END;

CREATE VIEW testView AS
SELECT *
FROM test;

select:
SELECT *
FROM test;
Binary file not shown.
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
ALTER TABLE test ADD COLUMN value2 TEXT;

CREATE INDEX testIndex ON test(value);

CREATE TRIGGER testTrigger
AFTER DELETE ON test
BEGIN
INSERT INTO test VALUES ("1", "2");
END;

CREATE VIEW testView AS
SELECT *
FROM test;