From 1eb4aa4da1ce685875dd9785f720d85dc2fc2fd8 Mon Sep 17 00:00:00 2001 From: Michael Simons Date: Thu, 28 Nov 2024 11:58:55 +0100 Subject: [PATCH 1/3] add config flag. --- .../neo4j/migrations/cli/MigrationsCli.java | 8 ++++ .../migrations/cli/MigrationsCliTest.java | 20 +++++++++ .../neo4j/migrations/core/Defaults.java | 15 ++++++- .../migrations/core/MigrationsConfig.java | 29 +++++++++++++ .../migrations/core/MigrationsConfigTest.java | 12 ++++++ .../maven/AbstractConnectedMojo.java | 10 ++++- .../neo4j/migrations/maven/CleanMojo.java | 6 +++ .../neo4j/migrations/maven/InfoMojo.java | 6 +++ .../neo4j/migrations/maven/MigrateMojo.java | 6 +++ .../neo4j/migrations/maven/RepairMojo.java | 6 +++ .../neo4j/migrations/maven/ValidateMojo.java | 6 +++ .../maven/AbstractConnectedMojoTest.java | 26 +++++++++++ .../src/test/resources/out-of-order/pom.xml | 43 +++++++++++++++++++ .../quarkus/runtime/MigrationsProperties.java | 12 ++++++ .../quarkus/runtime/MigrationsRecorder.java | 1 + .../runtime/MigrationsRecorderTest.java | 27 ++++++++++++ .../MigrationsAutoConfiguration.java | 3 +- .../autoconfigure/MigrationsProperties.java | 24 +++++++++++ .../MigrationsAutoConfigurationTest.java | 21 +++++++++ 19 files changed, 278 insertions(+), 3 deletions(-) create mode 100644 neo4j-migrations-maven-plugin/src/test/resources/out-of-order/pom.xml diff --git a/neo4j-migrations-cli/src/main/java/ac/simons/neo4j/migrations/cli/MigrationsCli.java b/neo4j-migrations-cli/src/main/java/ac/simons/neo4j/migrations/cli/MigrationsCli.java index e3855d00ce..40ec76398c 100644 --- a/neo4j-migrations-cli/src/main/java/ac/simons/neo4j/migrations/cli/MigrationsCli.java +++ b/neo4j-migrations-cli/src/main/java/ac/simons/neo4j/migrations/cli/MigrationsCli.java @@ -244,6 +244,13 @@ public static void main(String... args) { ) private VersionSortOrder versionSortOrder; + @Option( + names = {"--out-of-order"}, + description = "Use this flag to enable migrations to be discovered out-of-order and integrated into the migration chain.", + defaultValue = Defaults.OUT_OF_ORDER_VALUE + ) + private boolean outOfOrder; + @Spec private CommandSpec commandSpec; @@ -298,6 +305,7 @@ MigrationsConfig getConfig(boolean forceSilence) { .withAutocrlf(autocrlf) .withDelayBetweenMigrations(delayBetweenMigrations) .withVersionSortOrder(versionSortOrder) + .withOutOfOrderAllowed(outOfOrder) .build(); if (!forceSilence) { diff --git a/neo4j-migrations-cli/src/test/java/ac/simons/neo4j/migrations/cli/MigrationsCliTest.java b/neo4j-migrations-cli/src/test/java/ac/simons/neo4j/migrations/cli/MigrationsCliTest.java index 0826b074ad..0475557cbf 100644 --- a/neo4j-migrations-cli/src/test/java/ac/simons/neo4j/migrations/cli/MigrationsCliTest.java +++ b/neo4j-migrations-cli/src/test/java/ac/simons/neo4j/migrations/cli/MigrationsCliTest.java @@ -141,6 +141,26 @@ void shouldParseVersionSortOrder() { assertThat(cli.getConfig().getVersionSortOrder()).isEqualTo(MigrationsConfig.VersionSortOrder.SEMANTIC); } + @Test // GH-1213 + void outOfOrderShouldBeDisallowedByDefault() { + + MigrationsCli cli = new MigrationsCli(); + CommandLine commandLine = new CommandLine(cli); + commandLine.parseArgs(); + + assertThat(cli.getConfig().isOutOfOrder()).isFalse(); + } + + @Test // GH-1213 + void outOfOrderShouldBeApplied() { + + MigrationsCli cli = new MigrationsCli(); + CommandLine commandLine = new CommandLine(cli); + commandLine.parseArgs("--out-of-order"); + + assertThat(cli.getConfig().isOutOfOrder()).isTrue(); + } + @Test void shouldRequire2Connections() { diff --git a/neo4j-migrations-core/src/main/java/ac/simons/neo4j/migrations/core/Defaults.java b/neo4j-migrations-core/src/main/java/ac/simons/neo4j/migrations/core/Defaults.java index 86d166f6b8..c8836ff821 100644 --- a/neo4j-migrations-core/src/main/java/ac/simons/neo4j/migrations/core/Defaults.java +++ b/neo4j-migrations-core/src/main/java/ac/simons/neo4j/migrations/core/Defaults.java @@ -99,7 +99,8 @@ public final class Defaults { public static final String AUTOCRLF_VALUE = "false"; /** - * Same as {@link #VERSION_SORT_ORDER} but as {@link String string value} to be used in configuration that requires defaults given as string. + * Same as {@link #VERSION_SORT_ORDER} but as {@link String string value} to be used in configuration that requires + * defaults given as string. */ public static final String VERSION_SORT_ORDER_VALUE = "LEXICOGRAPHIC"; @@ -110,6 +111,18 @@ public final class Defaults { */ public static final VersionSortOrder VERSION_SORT_ORDER = VersionSortOrder.LEXICOGRAPHIC; + /** + * Default setting for {@code outOfOrder}. + * @since 2.14.0 + */ + public static final boolean OUT_OF_ORDER = false; + + /** + * Same as {@link #OUT_OF_ORDER} but as a {@link String string value} to be used in configuration that requires + * defaults given as string. + */ + public static final String OUT_OF_ORDER_VALUE = "false"; + private Defaults() { } } diff --git a/neo4j-migrations-core/src/main/java/ac/simons/neo4j/migrations/core/MigrationsConfig.java b/neo4j-migrations-core/src/main/java/ac/simons/neo4j/migrations/core/MigrationsConfig.java index 970d6fd9e7..d192666fd6 100644 --- a/neo4j-migrations-core/src/main/java/ac/simons/neo4j/migrations/core/MigrationsConfig.java +++ b/neo4j-migrations-core/src/main/java/ac/simons/neo4j/migrations/core/MigrationsConfig.java @@ -137,6 +137,8 @@ public static MigrationsConfig defaultConfig() { private final Duration transactionTimeout; + private final boolean outOfOrder; + private MigrationsConfig(Builder builder) { this.packagesToScan = @@ -158,6 +160,7 @@ private MigrationsConfig(Builder builder) { this.constraintOptions = builder.constraintOptions; this.versionSortOrder = builder.versionSortOrder; this.transactionTimeout = builder.transactionTimeout; + this.outOfOrder = builder.outOfOrder; } /** @@ -267,6 +270,18 @@ public Duration getTransactionTimeout() { return transactionTimeout; } + /** + * When this flag is set to {@literal true}, new migrations discovered that are "out of order", such as a version 15 + * is to be found between 10 and 20, it will be accepted, integrated into the chain and then move on instead of throwing + * an error. + * + * @return true if migrations shall be allowed to be out of order + * @since 2.14.0 + */ + public boolean isOutOfOrder() { + return outOfOrder; + } + /** * Helper method to pretty print this configuration into a logger (on level {@literal INFO} respectively {@literal WARNING}. * @@ -381,6 +396,8 @@ public static class Builder { private Duration transactionTimeout; + private boolean outOfOrder = Defaults.OUT_OF_ORDER; + private Builder() { // The explicit constructor has been added to avoid warnings when Neo4j-Migrations // is used on the module path. JMS will complain about Builder being exported with @@ -591,6 +608,18 @@ public Builder withTransactionTimeout(Duration newTransactionTimeout) { return this; } + /** + * Allows or disallows migrations discovered to be out of order. + * + * @param allowed use {@literal true} to allow out-of-order discovery of migrations + * @return The builder for further customization + * @since 2.14.0 + */ + public Builder withOutOfOrderAllowed(boolean allowed) { + this.outOfOrder = allowed; + return this; + } + /** * @return The immutable configuration */ diff --git a/neo4j-migrations-core/src/test/java/ac/simons/neo4j/migrations/core/MigrationsConfigTest.java b/neo4j-migrations-core/src/test/java/ac/simons/neo4j/migrations/core/MigrationsConfigTest.java index a33afb2f37..e11e81eeb8 100644 --- a/neo4j-migrations-core/src/test/java/ac/simons/neo4j/migrations/core/MigrationsConfigTest.java +++ b/neo4j-migrations-core/src/test/java/ac/simons/neo4j/migrations/core/MigrationsConfigTest.java @@ -65,6 +65,18 @@ void autocrlfShouldBeChangeable() { assertThat(MigrationsConfig.builder().withAutocrlf(true).build().isAutocrlf()).isTrue(); } + @Test // GH-1213 + void outOfOrderShouldBeFalseByDefault() { + + assertThat(MigrationsConfig.builder().build().isOutOfOrder()).isFalse(); + } + + @Test // GH-1213 + void outOfOrderShouldBeChangeable() { + + assertThat(MigrationsConfig.builder().withOutOfOrderAllowed(true).build().isOutOfOrder()).isTrue(); + } + @Test void logToShouldWork() { diff --git a/neo4j-migrations-maven-plugin/src/main/java/ac/simons/neo4j/migrations/maven/AbstractConnectedMojo.java b/neo4j-migrations-maven-plugin/src/main/java/ac/simons/neo4j/migrations/maven/AbstractConnectedMojo.java index 91cb3bf5a4..845e7dc2e3 100644 --- a/neo4j-migrations-maven-plugin/src/main/java/ac/simons/neo4j/migrations/maven/AbstractConnectedMojo.java +++ b/neo4j-migrations-maven-plugin/src/main/java/ac/simons/neo4j/migrations/maven/AbstractConnectedMojo.java @@ -129,11 +129,18 @@ abstract class AbstractConnectedMojo extends AbstractMojo { private boolean verbose; /** - * The sort order to use. + * The sort order to use. */ @Parameter(defaultValue = Defaults.VERSION_SORT_ORDER_VALUE) private VersionSortOrder versionSortOrder; + /** + * Whether to allow out-of-order migrations or not. + * @since 2.14.0 + */ + @Parameter(defaultValue = Defaults.OUT_OF_ORDER_VALUE) + private boolean outOfOrder; + @Override public void execute() throws MojoExecutionException, MojoFailureException { @@ -169,6 +176,7 @@ MigrationsConfig getConfig() { .withSchemaDatabase(schemaDatabase) .withImpersonatedUser(impersonatedUser) .withVersionSortOrder(versionSortOrder) + .withOutOfOrderAllowed(outOfOrder) .build(); config.logTo(LOGGER, verbose); diff --git a/neo4j-migrations-maven-plugin/src/main/java/ac/simons/neo4j/migrations/maven/CleanMojo.java b/neo4j-migrations-maven-plugin/src/main/java/ac/simons/neo4j/migrations/maven/CleanMojo.java index 278cd743eb..c17f575a2b 100644 --- a/neo4j-migrations-maven-plugin/src/main/java/ac/simons/neo4j/migrations/maven/CleanMojo.java +++ b/neo4j-migrations-maven-plugin/src/main/java/ac/simons/neo4j/migrations/maven/CleanMojo.java @@ -36,6 +36,12 @@ threadSafe = true) public class CleanMojo extends AbstractConnectedMojo { + /** + * The default constructor is primarily used by the Maven machinery. + */ + public CleanMojo() { + } + /** * Set to true to delete all migration chains as well as all Neo4j-Migration * constraints and not only the chain for the target database. diff --git a/neo4j-migrations-maven-plugin/src/main/java/ac/simons/neo4j/migrations/maven/InfoMojo.java b/neo4j-migrations-maven-plugin/src/main/java/ac/simons/neo4j/migrations/maven/InfoMojo.java index b76a526343..8cb7181a1c 100644 --- a/neo4j-migrations-maven-plugin/src/main/java/ac/simons/neo4j/migrations/maven/InfoMojo.java +++ b/neo4j-migrations-maven-plugin/src/main/java/ac/simons/neo4j/migrations/maven/InfoMojo.java @@ -34,6 +34,12 @@ threadSafe = true) public class InfoMojo extends AbstractConnectedMojo { + /** + * The default constructor is primarily used by the Maven machinery. + */ + public InfoMojo() { + } + @Override void withMigrations(Migrations migrations) { diff --git a/neo4j-migrations-maven-plugin/src/main/java/ac/simons/neo4j/migrations/maven/MigrateMojo.java b/neo4j-migrations-maven-plugin/src/main/java/ac/simons/neo4j/migrations/maven/MigrateMojo.java index 1279dfec05..d200ae05b3 100644 --- a/neo4j-migrations-maven-plugin/src/main/java/ac/simons/neo4j/migrations/maven/MigrateMojo.java +++ b/neo4j-migrations-maven-plugin/src/main/java/ac/simons/neo4j/migrations/maven/MigrateMojo.java @@ -37,6 +37,12 @@ threadSafe = true) public class MigrateMojo extends AbstractConnectedMojo { + /** + * The default constructor is primarily used by the Maven machinery. + */ + public MigrateMojo() { + } + @Override void withMigrations(Migrations migrations) { diff --git a/neo4j-migrations-maven-plugin/src/main/java/ac/simons/neo4j/migrations/maven/RepairMojo.java b/neo4j-migrations-maven-plugin/src/main/java/ac/simons/neo4j/migrations/maven/RepairMojo.java index c01d1daadf..ef6a7955a6 100644 --- a/neo4j-migrations-maven-plugin/src/main/java/ac/simons/neo4j/migrations/maven/RepairMojo.java +++ b/neo4j-migrations-maven-plugin/src/main/java/ac/simons/neo4j/migrations/maven/RepairMojo.java @@ -33,6 +33,12 @@ threadSafe = true) public class RepairMojo extends AbstractConnectedMojo { + /** + * The default constructor is primarily used by the Maven machinery. + */ + public RepairMojo() { + } + @Override void withMigrations(Migrations migrations) { diff --git a/neo4j-migrations-maven-plugin/src/main/java/ac/simons/neo4j/migrations/maven/ValidateMojo.java b/neo4j-migrations-maven-plugin/src/main/java/ac/simons/neo4j/migrations/maven/ValidateMojo.java index 6c3cf7adb6..6633d06e71 100644 --- a/neo4j-migrations-maven-plugin/src/main/java/ac/simons/neo4j/migrations/maven/ValidateMojo.java +++ b/neo4j-migrations-maven-plugin/src/main/java/ac/simons/neo4j/migrations/maven/ValidateMojo.java @@ -40,6 +40,12 @@ threadSafe = true) public class ValidateMojo extends AbstractConnectedMojo { + /** + * The default constructor is primarily used by the Maven machinery. + */ + public ValidateMojo() { + } + /** * Set this to {@literal false} if the database can be brought into a valid state by just applying the configuration */ diff --git a/neo4j-migrations-maven-plugin/src/test/java/ac/simons/neo4j/migrations/maven/AbstractConnectedMojoTest.java b/neo4j-migrations-maven-plugin/src/test/java/ac/simons/neo4j/migrations/maven/AbstractConnectedMojoTest.java index 9e0d9be724..b64c090ead 100644 --- a/neo4j-migrations-maven-plugin/src/test/java/ac/simons/neo4j/migrations/maven/AbstractConnectedMojoTest.java +++ b/neo4j-migrations-maven-plugin/src/test/java/ac/simons/neo4j/migrations/maven/AbstractConnectedMojoTest.java @@ -151,5 +151,31 @@ public void createDriverConfigShouldSetCorrectValues() { assertThat(config.logging()).isInstanceOf(ConsoleLogging.class); assertThat(config.userAgent()).startsWith("neo4j-migrations/"); } + + @Test // GH-1213 + public void outOfOrderShouldNotBeAllowedByDefault() throws Exception { + + File pom = new File("target/test-classes/with-imp-and-schema/"); + assertThat(pom) + .isNotNull() + .exists(); + + InfoMojo infoMojo = (InfoMojo) rule.lookupConfiguredMojo(pom, "info"); + assertThat(infoMojo).isNotNull(); + assertThat(infoMojo.getConfig().isOutOfOrder()).isFalse(); + } + + @Test // GH-1213 + public void outOfOrderShouldBeConfigurable() throws Exception { + + File pom = new File("target/test-classes/out-of-order/"); + assertThat(pom) + .isNotNull() + .exists(); + + InfoMojo infoMojo = (InfoMojo) rule.lookupConfiguredMojo(pom, "info"); + assertThat(infoMojo).isNotNull(); + assertThat(infoMojo.getConfig().isOutOfOrder()).isTrue(); + } } diff --git a/neo4j-migrations-maven-plugin/src/test/resources/out-of-order/pom.xml b/neo4j-migrations-maven-plugin/src/test/resources/out-of-order/pom.xml new file mode 100644 index 0000000000..0964c7c27e --- /dev/null +++ b/neo4j-migrations-maven-plugin/src/test/resources/out-of-order/pom.xml @@ -0,0 +1,43 @@ + + + + 4.0.0 + + eu.michael-simons.neo4j + neo4j-migrations-maven-plugin-test-project + 1.0-SNAPSHOT + jar + Test MyMojo + + + + + eu.michael-simons.neo4j + neo4j-migrations-maven-plugin + + secret + classpath:/wontwork + true + + + + + diff --git a/neo4j-migrations-quarkus-parent/runtime/src/main/java/ac/simons/neo4j/migrations/quarkus/runtime/MigrationsProperties.java b/neo4j-migrations-quarkus-parent/runtime/src/main/java/ac/simons/neo4j/migrations/quarkus/runtime/MigrationsProperties.java index 7d205a831c..c876878ee5 100644 --- a/neo4j-migrations-quarkus-parent/runtime/src/main/java/ac/simons/neo4j/migrations/quarkus/runtime/MigrationsProperties.java +++ b/neo4j-migrations-quarkus-parent/runtime/src/main/java/ac/simons/neo4j/migrations/quarkus/runtime/MigrationsProperties.java @@ -117,7 +117,19 @@ public interface MigrationsProperties { * on {@link #transactionMode}). {@literal null} is a valid value and make the driver apply the default timeout for * the database. * + * @return an optional transaction timeout * @since 2.13.0 */ Optional transactionTimeout(); + + /** + * When this flag is set to {@literal true}, new migrations discovered that are "out of order", such as a version 15 + * is to be found between 10 and 20, it will be accepted, integrated into the chain and then move on instead of throwing + * an error. + * + * @return whether to allow out-of-order migrations or not + * @since 2.14.0 + */ + @WithDefault(Defaults.OUT_OF_ORDER_VALUE) + boolean outOfOrder(); } diff --git a/neo4j-migrations-quarkus-parent/runtime/src/main/java/ac/simons/neo4j/migrations/quarkus/runtime/MigrationsRecorder.java b/neo4j-migrations-quarkus-parent/runtime/src/main/java/ac/simons/neo4j/migrations/quarkus/runtime/MigrationsRecorder.java index 99a989a149..c73b332fbb 100644 --- a/neo4j-migrations-quarkus-parent/runtime/src/main/java/ac/simons/neo4j/migrations/quarkus/runtime/MigrationsRecorder.java +++ b/neo4j-migrations-quarkus-parent/runtime/src/main/java/ac/simons/neo4j/migrations/quarkus/runtime/MigrationsRecorder.java @@ -78,6 +78,7 @@ public RuntimeValue recordConfig( .withResourceScanner(resourceScanner) .withDelayBetweenMigrations(runtimeProperties.delayBetweenMigrations().orElse(null)) .withVersionSortOrder(runtimeProperties.versionSortOrder()) + .withOutOfOrderAllowed(runtimeProperties.outOfOrder()) .build(); return new RuntimeValue<>(config); diff --git a/neo4j-migrations-quarkus-parent/runtime/src/test/java/ac/simons/neo4j/migrations/quarkus/runtime/MigrationsRecorderTest.java b/neo4j-migrations-quarkus-parent/runtime/src/test/java/ac/simons/neo4j/migrations/quarkus/runtime/MigrationsRecorderTest.java index 923651ba4c..52bc2bdebd 100644 --- a/neo4j-migrations-quarkus-parent/runtime/src/test/java/ac/simons/neo4j/migrations/quarkus/runtime/MigrationsRecorderTest.java +++ b/neo4j-migrations-quarkus-parent/runtime/src/test/java/ac/simons/neo4j/migrations/quarkus/runtime/MigrationsRecorderTest.java @@ -87,4 +87,31 @@ void delayShallBeConfigurable() { var config = new MigrationsRecorder().recordConfig(buildTimeProperties, properties, null, null).getValue(); assertThat(config.getOptionalDelayBetweenMigrations()).hasValue(Duration.ofSeconds(1)); } + + @Test // GH-1213 + void outOfOrderShouldBeDisallowedByDefault() { + + var properties = mock(MigrationsProperties.class); + + var buildTimeProperties = mock(MigrationsBuildTimeProperties.class); + when(buildTimeProperties.packagesToScan()).thenReturn(Optional.empty()); + when(buildTimeProperties.locationsToScan()).thenReturn(List.of("bar")); + + var config = new MigrationsRecorder().recordConfig(buildTimeProperties, properties, null, null).getValue(); + assertThat(config.isOutOfOrder()).isFalse(); + } + + @Test // GH-1213 + void outOfOrderShouldBeApplied() { + + var properties = mock(MigrationsProperties.class); + when(properties.outOfOrder()).thenReturn(true); + + var buildTimeProperties = mock(MigrationsBuildTimeProperties.class); + when(buildTimeProperties.packagesToScan()).thenReturn(Optional.empty()); + when(buildTimeProperties.locationsToScan()).thenReturn(List.of("bar")); + + var config = new MigrationsRecorder().recordConfig(buildTimeProperties, properties, null, null).getValue(); + assertThat(config.isOutOfOrder()).isTrue(); + } } diff --git a/neo4j-migrations-spring-boot-starter-parent/neo4j-migrations-spring-boot-autoconfigure/src/main/java/ac/simons/neo4j/migrations/springframework/boot/autoconfigure/MigrationsAutoConfiguration.java b/neo4j-migrations-spring-boot-starter-parent/neo4j-migrations-spring-boot-autoconfigure/src/main/java/ac/simons/neo4j/migrations/springframework/boot/autoconfigure/MigrationsAutoConfiguration.java index 19cbdf8078..d15ef21c6c 100644 --- a/neo4j-migrations-spring-boot-starter-parent/neo4j-migrations-spring-boot-autoconfigure/src/main/java/ac/simons/neo4j/migrations/springframework/boot/autoconfigure/MigrationsAutoConfiguration.java +++ b/neo4j-migrations-spring-boot-starter-parent/neo4j-migrations-spring-boot-autoconfigure/src/main/java/ac/simons/neo4j/migrations/springframework/boot/autoconfigure/MigrationsAutoConfiguration.java @@ -80,7 +80,8 @@ MigrationsConfig neo4jMigrationsConfig(ResourceLoader resourceLoader, Migrations .withAutocrlf(migrationsProperties.isAutocrlf()) .withDelayBetweenMigrations(migrationsProperties.getDelayBetweenMigrations()) .withVersionSortOrder(migrationsProperties.getVersionSortOrder()) - .withMigrationClassesDiscoverer(applicationContextAwareDiscoverer.getIfAvailable()); + .withMigrationClassesDiscoverer(applicationContextAwareDiscoverer.getIfAvailable()) + .withOutOfOrderAllowed(migrationsProperties.isOutOfOrder()); configBuilderCustomizers.orderedStream().forEach(customizer -> customizer.customize(builder)); return builder.build(); } diff --git a/neo4j-migrations-spring-boot-starter-parent/neo4j-migrations-spring-boot-autoconfigure/src/main/java/ac/simons/neo4j/migrations/springframework/boot/autoconfigure/MigrationsProperties.java b/neo4j-migrations-spring-boot-starter-parent/neo4j-migrations-spring-boot-autoconfigure/src/main/java/ac/simons/neo4j/migrations/springframework/boot/autoconfigure/MigrationsProperties.java index 354ab0513b..4d3a905dd6 100644 --- a/neo4j-migrations-spring-boot-starter-parent/neo4j-migrations-spring-boot-autoconfigure/src/main/java/ac/simons/neo4j/migrations/springframework/boot/autoconfigure/MigrationsProperties.java +++ b/neo4j-migrations-spring-boot-starter-parent/neo4j-migrations-spring-boot-autoconfigure/src/main/java/ac/simons/neo4j/migrations/springframework/boot/autoconfigure/MigrationsProperties.java @@ -124,6 +124,15 @@ public class MigrationsProperties { */ private Duration transactionTimeout; + /** + * When this flag is set to {@literal true}, new migrations discovered that are "out of order", such as a version 15 + * is to be found between 10 and 20, it will be accepted, integrated into the chain and then move on instead of throwing + * an error. + * + * @since 2.14.0 + */ + private boolean outOfOrder = Defaults.OUT_OF_ORDER; + /** * @return see {@link #enabled} */ @@ -333,4 +342,19 @@ public Duration getTransactionTimeout() { public void setTransactionTimeout(Duration transactionTimeout) { this.transactionTimeout = transactionTimeout; } + + /** + * {@return whether out-of-order migrations are allowed and integrated into the chain or not} + */ + public boolean isOutOfOrder() { + return outOfOrder; + } + + /** + * Configures whether migrations are allowed to be out-of-order or not + * @param outOfOrder the new value for the out-of-order flag + */ + public void setOutOfOrder(boolean outOfOrder) { + this.outOfOrder = outOfOrder; + } } diff --git a/neo4j-migrations-spring-boot-starter-parent/neo4j-migrations-spring-boot-autoconfigure/src/test/java/ac/simons/neo4j/migrations/springframework/boot/autoconfigure/MigrationsAutoConfigurationTest.java b/neo4j-migrations-spring-boot-starter-parent/neo4j-migrations-spring-boot-autoconfigure/src/test/java/ac/simons/neo4j/migrations/springframework/boot/autoconfigure/MigrationsAutoConfigurationTest.java index 3cc18eba62..3a9492806a 100644 --- a/neo4j-migrations-spring-boot-starter-parent/neo4j-migrations-spring-boot-autoconfigure/src/test/java/ac/simons/neo4j/migrations/springframework/boot/autoconfigure/MigrationsAutoConfigurationTest.java +++ b/neo4j-migrations-spring-boot-starter-parent/neo4j-migrations-spring-boot-autoconfigure/src/test/java/ac/simons/neo4j/migrations/springframework/boot/autoconfigure/MigrationsAutoConfigurationTest.java @@ -358,6 +358,27 @@ public boolean useExplicitPropertyIndexType() { assertThat(config.getOptionalImpersonatedUser()).hasValue("blah"); assertThat(config.getConstraintRenderingOptions()).hasSize(1); } + + @Test // GH-1213 + void outOfOrderShouldBeDisallowedByDefault() { + + MigrationsProperties properties = new MigrationsProperties(); + properties.setPackagesToScan(new String[] { "na" }); + + MigrationsConfig config = new MigrationsAutoConfiguration().neo4jMigrationsConfig(resourceLoader, properties, noCustomizer(), noSpringDiscoverer()); + assertThat(config.isOutOfOrder()).isFalse(); + } + + @Test // GH-1213 + void outOfOrderShouldBeApplied() { + + MigrationsProperties properties = new MigrationsProperties(); + properties.setPackagesToScan(new String[] { "na" }); + properties.setOutOfOrder(true); + + MigrationsConfig config = new MigrationsAutoConfiguration().neo4jMigrationsConfig(resourceLoader, properties, noCustomizer(), noSpringDiscoverer()); + assertThat(config.isOutOfOrder()).isTrue(); + } } private static class LoggingAppender extends ListAppender { From a815bb96ee3489cb495dcfa18722b928ccab3e97 Mon Sep 17 00:00:00 2001 From: Michael Simons Date: Thu, 28 Nov 2024 17:03:01 +0100 Subject: [PATCH 2/3] implement out of order migrations. Signed-off-by: Michael Simons --- docs/modules/ROOT/pages/usage.adoc | 13 ++ .../neo4j/migrations/core/ChainBuilder.java | 58 +++-- .../neo4j/migrations/core/Migrations.java | 60 +++-- .../neo4j/migrations/core/MigrationsIT.java | 209 ++++++++++++++++++ .../additional/second/V014__NewSecond.cypher | 1 + .../additional/second/V019__NewSecond.cypher | 1 + .../additional/third/V001__NewFirst.cypher | 1 + .../additional/third/V009__NewFirst.cypher | 1 + .../ooo/base/first/V010__First.cypher | 1 + .../ooo/base/first/V020__InitialSecond.cypher | 1 + .../ooo/base/second/V015__NewSecond.cypher | 1 + .../ooo/base/third/V005__NewFirst.cypher | 1 + .../ooo/repeatable/modified/R002__na.cypher | 1 + .../ooo/repeatable/modified/R016__na.cypher | 1 + .../ooo/repeatable/orig/R002__na.cypher | 1 + .../ooo/repeatable/orig/R016__na.cypher | 1 + 16 files changed, 311 insertions(+), 41 deletions(-) create mode 100644 neo4j-migrations-test-resources/src/main/resources/ooo/additional/second/V014__NewSecond.cypher create mode 100644 neo4j-migrations-test-resources/src/main/resources/ooo/additional/second/V019__NewSecond.cypher create mode 100644 neo4j-migrations-test-resources/src/main/resources/ooo/additional/third/V001__NewFirst.cypher create mode 100644 neo4j-migrations-test-resources/src/main/resources/ooo/additional/third/V009__NewFirst.cypher create mode 100644 neo4j-migrations-test-resources/src/main/resources/ooo/base/first/V010__First.cypher create mode 100644 neo4j-migrations-test-resources/src/main/resources/ooo/base/first/V020__InitialSecond.cypher create mode 100644 neo4j-migrations-test-resources/src/main/resources/ooo/base/second/V015__NewSecond.cypher create mode 100644 neo4j-migrations-test-resources/src/main/resources/ooo/base/third/V005__NewFirst.cypher create mode 100644 neo4j-migrations-test-resources/src/main/resources/ooo/repeatable/modified/R002__na.cypher create mode 100644 neo4j-migrations-test-resources/src/main/resources/ooo/repeatable/modified/R016__na.cypher create mode 100644 neo4j-migrations-test-resources/src/main/resources/ooo/repeatable/orig/R002__na.cypher create mode 100644 neo4j-migrations-test-resources/src/main/resources/ooo/repeatable/orig/R016__na.cypher diff --git a/docs/modules/ROOT/pages/usage.adoc b/docs/modules/ROOT/pages/usage.adoc index 2d92884316..0bfa7fded4 100644 --- a/docs/modules/ROOT/pages/usage.adoc +++ b/docs/modules/ROOT/pages/usage.adoc @@ -46,6 +46,12 @@ It applies all locally resolved migrations to the target database and stores the It returns the last applied version. +That operation does not allow migrations out-of-order by default. +That means if you add version 2 after you already migrated to 5, it will fail with an appropriate error. +You can spot these cases beforehand by validating your chain of migrations (see below). +If you absolutely must however, you can use `withOutOfOrderAllowed` on the config object or the corresponding property in either the Spring Boot starter or the Quarkus extension. +Setting this to true will integrate out-of-order migrations into the chain. + [[usage_common_repair]] === Repair @@ -677,6 +683,13 @@ A configurable delay that will be applied in between applying two migrations. - Type: `java.time.Duration` - Default: `false` +`org.neo4j.migrations.out-of-order`:: +A flag to enable out-of-order migrations. + +- Type: `java.lang.Boolean` +- Default: `false` + + NOTE: Migrations can be disabled by setting `org.neo4j.migrations.enabled` to `false`. diff --git a/neo4j-migrations-core/src/main/java/ac/simons/neo4j/migrations/core/ChainBuilder.java b/neo4j-migrations-core/src/main/java/ac/simons/neo4j/migrations/core/ChainBuilder.java index b8bffbc59d..f7256c3aa6 100644 --- a/neo4j-migrations-core/src/main/java/ac/simons/neo4j/migrations/core/ChainBuilder.java +++ b/neo4j-migrations-core/src/main/java/ac/simons/neo4j/migrations/core/ChainBuilder.java @@ -20,6 +20,7 @@ import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.TreeMap; import org.neo4j.driver.Record; import org.neo4j.driver.Result; @@ -87,41 +88,50 @@ private Map buildChain0(MigrationContext context, Lis } final String incompleteMigrationsMessage = "More migrations have been applied to the database than locally resolved."; - Map fullMigrationChain = new LinkedHashMap<>( - discoveredMigrations.size() + appliedMigrations.size()); + Map fullMigrationChain = new TreeMap<>(context.getConfig().getVersionComparator()); + boolean outOfOrderAllowed = context.getConfig().isOutOfOrder(); int i = 0; for (Map.Entry entry : appliedMigrations.entrySet()) { MigrationVersion expectedVersion = entry.getKey(); Optional expectedChecksum = entry.getValue().getChecksum(); - Migration newMigration; - try { - newMigration = discoveredMigrations.get(i); - } catch (IndexOutOfBoundsException e) { - if (detailedCauses) { - throw new MigrationsException(incompleteMigrationsMessage, e); + boolean checkNext = true; + while (checkNext) { + checkNext = false; + Migration newMigration; + try { + newMigration = discoveredMigrations.get(i++); + } catch (IndexOutOfBoundsException e) { + if (detailedCauses) { + throw new MigrationsException(incompleteMigrationsMessage, e); + } + throw new MigrationsException(incompleteMigrationsMessage); } - throw new MigrationsException(incompleteMigrationsMessage); - } - if (!newMigration.getVersion().equals(expectedVersion)) { - if (getNumberOfAppliedMigrations(context) > discoveredMigrations.size()) { - throw new MigrationsException(incompleteMigrationsMessage, new IndexOutOfBoundsException()); + if (!newMigration.getVersion().equals(expectedVersion)) { + if (getNumberOfAppliedMigrations(context) > discoveredMigrations.size()) { + throw new MigrationsException(incompleteMigrationsMessage, new IndexOutOfBoundsException()); + } + if (outOfOrderAllowed) { + fullMigrationChain.put(newMigration.getVersion(), DefaultMigrationChainElement.pendingElement(newMigration)); + checkNext = true; + continue; + } else { + throw new MigrationsException("Unexpected migration at index " + (i - 1) + ": " + Migrations.toString(newMigration) + "."); + } } - throw new MigrationsException("Unexpected migration at index " + i + ": " + Migrations.toString(newMigration) + "."); - } - if (newMigration.isRepeatable() != expectedVersion.isRepeatable()) { - throw new MigrationsException("State of " + Migrations.toString(newMigration) + " changed from " + (expectedVersion.isRepeatable() ? "repeatable to non-repeatable" : "non-repeatable to repeatable")); - } + if (newMigration.isRepeatable() != expectedVersion.isRepeatable()) { + throw new MigrationsException("State of " + Migrations.toString(newMigration) + " changed from " + (expectedVersion.isRepeatable() ? "repeatable to non-repeatable" : "non-repeatable to repeatable")); + } - if ((context.getConfig().isValidateOnMigrate() || alwaysVerify) && !(matches(expectedChecksum, newMigration) || expectedVersion.isRepeatable())) { - throw new MigrationsException("Checksum of " + Migrations.toString(newMigration) + " changed!"); - } + if ((context.getConfig().isValidateOnMigrate() || alwaysVerify) && !(matches(expectedChecksum, newMigration) || expectedVersion.isRepeatable())) { + throw new MigrationsException("Checksum of " + Migrations.toString(newMigration) + " changed!"); + } - // This is not a pending migration anymore - fullMigrationChain.put(expectedVersion, entry.getValue()); - ++i; + // This is not a pending migration anymore + fullMigrationChain.put(expectedVersion, entry.getValue()); + } } // All remaining migrations are pending diff --git a/neo4j-migrations-core/src/main/java/ac/simons/neo4j/migrations/core/Migrations.java b/neo4j-migrations-core/src/main/java/ac/simons/neo4j/migrations/core/Migrations.java index 735dbabcc2..f568f2b0a8 100644 --- a/neo4j-migrations-core/src/main/java/ac/simons/neo4j/migrations/core/Migrations.java +++ b/neo4j-migrations-core/src/main/java/ac/simons/neo4j/migrations/core/Migrations.java @@ -51,7 +51,6 @@ import org.neo4j.driver.Values; import org.neo4j.driver.exceptions.NoSuchRecordException; import org.neo4j.driver.exceptions.ServiceUnavailableException; -import org.neo4j.driver.summary.ResultSummary; import org.neo4j.driver.summary.SummaryCounters; import org.neo4j.driver.types.Node; @@ -721,7 +720,9 @@ private void apply0(List migrations) { StopWatch stopWatch = new StopWatch(); for (Migration migration : new IterableMigrations(config, migrations)) { - + if (!chain.isApplied(migration.getVersion().getValue()) && config.getVersionComparator().compare(migration.getVersion(), previousVersion) < 0) { + previousVersion = MigrationVersion.baseline(); + } boolean repeated = false; Supplier logMessage = () -> String.format("Applied migration %s.", toString(migration)); if (previousVersion != MigrationVersion.baseline() && chain.isApplied(migration.getVersion().getValue())) { @@ -766,40 +767,65 @@ private MigrationVersion recordApplication(String neo4jUser, MigrationVersion pr parameters.put("executionTime", executionTime); parameters.put(PROPERTY_MIGRATION_TARGET, migrationTarget.orElse(null)); - TransactionCallback uow; + record ReplacedMigration(long oldRelId, long newMigrationNodeId) { + } + + TransactionCallback> uow; if (repeated) { - uow = t -> t.run( - "MATCH (l:__Neo4jMigration) WHERE l.version = $appliedMigration['version'] AND coalesce(l.migrationTarget,'') = coalesce($migrationTarget,'') WITH l " + uow = t -> { + t.run( + "MATCH (l:__Neo4jMigration) WHERE l.version = $appliedMigration['version'] AND coalesce(l.migrationTarget,'') = coalesce($migrationTarget,'') WITH l " + "CREATE (l) - [:REPEATED {checksum: $appliedMigration['checksum'], at: datetime({timezone: 'UTC'}), in: duration( {milliseconds: $executionTime} ), by: $installedBy, connectedAs: $neo4jUser}] -> (l)", - parameters).consume(); + parameters).consume(); + return Optional.empty(); + }; } else { uow = t -> { - String mergeOrMatchAndMaybeCreate; + String mergePreviousMigration; if (migrationTarget.isPresent()) { - mergeOrMatchAndMaybeCreate = "MERGE (p:__Neo4jMigration {version: $previousVersion, migrationTarget: $migrationTarget}) "; + mergePreviousMigration = "MERGE (p:__Neo4jMigration {version: $previousVersion, migrationTarget: $migrationTarget}) WITH p "; } else { Result result = t.run( "MATCH (p:__Neo4jMigration {version: $previousVersion}) WHERE p.migrationTarget IS NULL RETURN id(p) AS id", Values.parameters("previousVersion", previousVersion.getValue())); if (result.hasNext()) { parameters.put("id", result.single().get("id").asLong()); - mergeOrMatchAndMaybeCreate = "MATCH (p) WHERE id(p) = $id WITH p "; + mergePreviousMigration = "MATCH (p) WHERE id(p) = $id WITH p "; } else { - mergeOrMatchAndMaybeCreate = "CREATE (p:__Neo4jMigration {version: $previousVersion}) "; + mergePreviousMigration = "CREATE (p:__Neo4jMigration {version: $previousVersion}) WITH p "; } } - return t.run( - mergeOrMatchAndMaybeCreate - + "CREATE (c:__Neo4jMigration) SET c = $appliedMigration, c.migrationTarget = $migrationTarget " - + "MERGE (p) - [:MIGRATED_TO {at: datetime({timezone: 'UTC'}), in: duration( {milliseconds: $executionTime} ), by: $installedBy, connectedAs: $neo4jUser}] -> (c)", - parameters) - .consume(); + var createNewMigrationAndPath = """ + OPTIONAL MATCH (p) -[om:MIGRATED_TO]-> (ot) + CREATE (c:__Neo4jMigration) SET c = $appliedMigration, c.migrationTarget = $migrationTarget + MERGE (p) - [:MIGRATED_TO {at: datetime({timezone: 'UTC'}), in: duration( {milliseconds: $executionTime} ), by: $installedBy, connectedAs: $neo4jUser}] -> (c) + RETURN id(c) AS insertedId, id(om) AS oldRelId, properties(om), id(ot) AS oldEndId + """; + + var result = t.run(mergePreviousMigration + " " + createNewMigrationAndPath, parameters).single(); + if (!result.get("oldRelId").isNull()) { + return Optional.of(new ReplacedMigration(result.get("oldRelId").asLong(), result.get("insertedId").asLong())); + } else { + return Optional.empty(); + } }; } try (Session session = context.getSchemaSession()) { - session.executeWrite(uow); + var f = session.executeWrite(uow); + f.ifPresent(replacedMigration -> { + session.executeWriteWithoutResult(t -> { + t.run(""" + MATCH ()-[oldRel]->(oldEnd) + WHERE id(oldRel) = $oldRelId + MATCH (inserted) WHERE id(inserted) = $insertedId + MERGE (inserted) -[r:MIGRATED_TO]-> (oldEnd) + SET r = properties(oldRel) + DELETE (oldRel) + """, Map.of("oldRelId", replacedMigration.oldRelId(), "insertedId", replacedMigration.newMigrationNodeId())).consume(); + }); + }); } return appliedMigration.getVersion(); diff --git a/neo4j-migrations-core/src/test/java/ac/simons/neo4j/migrations/core/MigrationsIT.java b/neo4j-migrations-core/src/test/java/ac/simons/neo4j/migrations/core/MigrationsIT.java index 225d429d9a..dc0b7b5de6 100644 --- a/neo4j-migrations-core/src/test/java/ac/simons/neo4j/migrations/core/MigrationsIT.java +++ b/neo4j-migrations-core/src/test/java/ac/simons/neo4j/migrations/core/MigrationsIT.java @@ -33,7 +33,9 @@ import java.util.Objects; import java.util.Optional; import java.util.UUID; +import java.util.function.Predicate; import java.util.stream.IntStream; +import java.util.stream.StreamSupport; import org.assertj.core.api.Assertions; import org.junit.jupiter.api.Nested; @@ -47,6 +49,7 @@ import org.neo4j.driver.GraphDatabase; import org.neo4j.driver.Session; import org.neo4j.driver.exceptions.ClientException; +import org.neo4j.driver.types.Node; import org.testcontainers.containers.Neo4jContainer; import ac.simons.neo4j.migrations.core.MigrationChain.ChainBuilderMode; @@ -902,6 +905,212 @@ void shouldCreatedRelationshipIndex() { } } + @Nested + class Ordering { + + static final String FIND_NODES_QUERY = "MATCH (n:OOO) RETURN n ORDER BY n.created_on"; + + @Test + // GH-1213 + void shouldDefaultToOrderAndOrderShouldBeRight() { + + var migrations = new Migrations(defaultConfigPart().withLocationsToScan("classpath:ooo/base").build(), driver); + migrations.apply(); + + assertChainOrder(migrations, "005", "010", "015", "020"); + assertCreationOrder("N4", "N1", "N3", "N2"); + } + + @Test + // GH-1213 + void shouldFailOnOutOfOrderByDefault() { + + var migrations = new Migrations(defaultConfigPart().withLocationsToScan("classpath:ooo/base/first").build(), driver); + migrations.apply(); + + assertChainOrder(migrations, "010", "020"); + assertCreationOrder("N1", "N2"); + + migrations = new Migrations(defaultConfigPart() + .withLocationsToScan("classpath:ooo/base/first", "classpath:ooo/base/second").build(), driver); + assertThatExceptionOfType(MigrationsException.class).isThrownBy(migrations::apply) + .withMessage("Unexpected migration at index 1: 015 (\"NewSecond\")."); + } + + @Test + // GH-1213 + void shouldNotFailOnOutOfOrderInTheBeginningOne() { + + var migrations = new Migrations(defaultConfigPart().withLocationsToScan("classpath:ooo/base/first").build(), driver); + migrations.apply(); + + assertChainOrder(migrations, "010", "020"); + assertCreationOrder("N1", "N2"); + + migrations = new Migrations(defaultConfigPart() + .withLocationsToScan("classpath:ooo/base/first", "classpath:ooo/base/third") + .withOutOfOrderAllowed(true) + .build(), driver); + migrations.apply(); + + assertChainOrder(migrations, "005", "010", "020"); + assertCreationOrder("N1", "N2", "N4"); + } + + @Test + // GH-1213 + void shouldNotFailOnOutOfOrderInTheBeginningN() { + + var migrations = new Migrations(defaultConfigPart().withLocationsToScan("classpath:ooo/base/first").build(), driver); + migrations.apply(); + + assertChainOrder(migrations, "010", "020"); + assertCreationOrder("N1", "N2"); + + migrations = new Migrations(defaultConfigPart() + .withLocationsToScan("classpath:ooo/base/first", "classpath:ooo/base/third", "classpath:ooo/additional/third") + .withOutOfOrderAllowed(true) + .build(), driver); + migrations.apply(); + + assertChainOrder(migrations, "001", "005", "009", "010", "020"); + assertCreationOrder("N1", "N2", "N7", "N4", "N8"); + } + + @Test + // GH-1213 + void shouldNotFailOnOutOfOrderInTheMiddleOne() { + + var migrations = new Migrations(defaultConfigPart().withLocationsToScan("classpath:ooo/base/first").build(), driver); + migrations.apply(); + + assertChainOrder(migrations, "010", "020"); + assertCreationOrder("N1", "N2"); + + migrations = new Migrations(defaultConfigPart() + .withLocationsToScan("classpath:ooo/base/first", "classpath:ooo/base/second") + .withOutOfOrderAllowed(true) + .build(), driver); + migrations.apply(); + + assertChainOrder(migrations, "010", "015", "020"); + assertCreationOrder("N1", "N2", "N3"); + } + + @Test + // GH-1213 + void shouldNotFailOnOutOfOrderInTheMiddleN() { + + var migrations = new Migrations(defaultConfigPart().withLocationsToScan("classpath:ooo/base/first").build(), driver); + migrations.apply(); + + assertChainOrder(migrations, "010", "020"); + assertCreationOrder("N1", "N2"); + + migrations = new Migrations(defaultConfigPart() + .withLocationsToScan("classpath:ooo/base/first", "classpath:ooo/base/second", "classpath:ooo/additional/second") + .withOutOfOrderAllowed(true) + .build(), driver); + migrations.apply(); + + assertChainOrder(migrations, "010", "014", "015", "019", "020"); + assertCreationOrder("N1", "N2", "N5", "N3", "N6"); + } + + @Test + // GH-1213 + void shouldNotFailOnOutOfOrderCombined() { + + var migrations = new Migrations(defaultConfigPart().withLocationsToScan("classpath:ooo/base/first").build(), driver); + migrations.apply(); + + assertChainOrder(migrations, "010", "020"); + assertCreationOrder("N1", "N2"); + + migrations = new Migrations(defaultConfigPart() + .withLocationsToScan("classpath:ooo/base/first", "classpath:ooo/base/second", "classpath:ooo/base/third") + .withOutOfOrderAllowed(true) + .build(), driver); + migrations.apply(); + + assertChainOrder(migrations, "005", "010", "015", "020"); + assertCreationOrder("N1", "N2", "N4", "N3"); + } + + @Test + // GH-1213 + void shouldNotFailOnOutOfOrderCombinedAll() { + + var migrations = new Migrations(defaultConfigPart().withLocationsToScan("classpath:ooo/base/first").build(), driver); + migrations.apply(); + + assertChainOrder(migrations, "010", "020"); + assertCreationOrder("N1", "N2"); + + migrations = new Migrations(defaultConfigPart() + .withLocationsToScan("classpath:ooo/base", "classpath:ooo/additional", "classpath:ooo/repeatable/orig") + .withOutOfOrderAllowed(true) + .build(), driver); + migrations.apply(); + + assertChainOrder(migrations, "001", "002", "005", "009", "010", "014", "015", "016", "019", "020"); + assertCreationOrder("N1", "N2", "N7", "N4", "N8", "N5", "N3", "N6"); + assertRepeats(2, 0); + + migrations = new Migrations(defaultConfigPart() + .withLocationsToScan("classpath:ooo/base", "classpath:ooo/additional", "classpath:ooo/repeatable/modified") + .withOutOfOrderAllowed(true) + .build(), driver); + migrations.apply(); + + assertChainOrder(migrations, "001", "002", "005", "009", "010", "014", "015", "016", "019", "020"); + assertCreationOrder("N1", "N2", "N7", "N4", "N8", "N5", "N3", "N6"); + assertRepeats(4, 2); + } + + private static MigrationsConfig.Builder defaultConfigPart() { + return MigrationsConfig.builder() + .withTransactionMode(MigrationsConfig.TransactionMode.PER_MIGRATION); + } + + private static void assertChainOrder(Migrations migrations, String... expectedVersions) { + var chain = migrations.info(); + assertThat(chain.getElements()) + .allSatisfy(c -> { + assertThat(c.getInstalledOn()).isNotEmpty(); + }) + .map(MigrationChain.Element::getVersion) + .containsExactly(expectedVersions); + } + + private void assertCreationOrder(String... labels) { + try (var session = driver.session()) { + var nodes = session.executeRead(tx -> tx.run(FIND_NODES_QUERY).list(r -> r.get("n").asNode())); + assertThat(nodes) + .map(n -> StreamSupport.stream(n.labels().spliterator(), false).filter(Predicate.not("OOO"::equals)).findFirst().orElseThrow()) + .containsExactly(labels); + } + } + + private void assertRepeats(int expectedTotal, int expectedModified) { + try (var session = driver.session()) { + var nodes = session.executeRead(tx -> tx.run("MATCH (n:FromRepeatable) RETURN n").list(r -> r.get("n").asNode())); + int cnt = 0; + int mod = 0; + for (Node node : nodes) { + ++cnt; + for (String l : node.labels()) { + if (l.equals("Mod")) { + ++mod; + } + } + } + assertThat(cnt).isEqualTo(expectedTotal); + assertThat(mod).isEqualTo(expectedModified); + } + } + } + @Nested class Repairing { diff --git a/neo4j-migrations-test-resources/src/main/resources/ooo/additional/second/V014__NewSecond.cypher b/neo4j-migrations-test-resources/src/main/resources/ooo/additional/second/V014__NewSecond.cypher new file mode 100644 index 0000000000..b7fdd20786 --- /dev/null +++ b/neo4j-migrations-test-resources/src/main/resources/ooo/additional/second/V014__NewSecond.cypher @@ -0,0 +1 @@ +CREATE (m:OOO:N5 {created_on: datetime.transaction()}) RETURN m; \ No newline at end of file diff --git a/neo4j-migrations-test-resources/src/main/resources/ooo/additional/second/V019__NewSecond.cypher b/neo4j-migrations-test-resources/src/main/resources/ooo/additional/second/V019__NewSecond.cypher new file mode 100644 index 0000000000..40f4e3a8f1 --- /dev/null +++ b/neo4j-migrations-test-resources/src/main/resources/ooo/additional/second/V019__NewSecond.cypher @@ -0,0 +1 @@ +CREATE (m:OOO:N6 {created_on: datetime.transaction()}) RETURN m; \ No newline at end of file diff --git a/neo4j-migrations-test-resources/src/main/resources/ooo/additional/third/V001__NewFirst.cypher b/neo4j-migrations-test-resources/src/main/resources/ooo/additional/third/V001__NewFirst.cypher new file mode 100644 index 0000000000..e93489033c --- /dev/null +++ b/neo4j-migrations-test-resources/src/main/resources/ooo/additional/third/V001__NewFirst.cypher @@ -0,0 +1 @@ +CREATE (m:OOO:N7 {created_on: datetime.transaction()}) RETURN m; \ No newline at end of file diff --git a/neo4j-migrations-test-resources/src/main/resources/ooo/additional/third/V009__NewFirst.cypher b/neo4j-migrations-test-resources/src/main/resources/ooo/additional/third/V009__NewFirst.cypher new file mode 100644 index 0000000000..9614c702f6 --- /dev/null +++ b/neo4j-migrations-test-resources/src/main/resources/ooo/additional/third/V009__NewFirst.cypher @@ -0,0 +1 @@ +CREATE (m:OOO:N8 {created_on: datetime.transaction()}) RETURN m; \ No newline at end of file diff --git a/neo4j-migrations-test-resources/src/main/resources/ooo/base/first/V010__First.cypher b/neo4j-migrations-test-resources/src/main/resources/ooo/base/first/V010__First.cypher new file mode 100644 index 0000000000..2ae568ef57 --- /dev/null +++ b/neo4j-migrations-test-resources/src/main/resources/ooo/base/first/V010__First.cypher @@ -0,0 +1 @@ +CREATE (m:OOO:N1 {created_on: datetime.transaction()}) RETURN m; diff --git a/neo4j-migrations-test-resources/src/main/resources/ooo/base/first/V020__InitialSecond.cypher b/neo4j-migrations-test-resources/src/main/resources/ooo/base/first/V020__InitialSecond.cypher new file mode 100644 index 0000000000..b4700c9956 --- /dev/null +++ b/neo4j-migrations-test-resources/src/main/resources/ooo/base/first/V020__InitialSecond.cypher @@ -0,0 +1 @@ +CREATE (m:OOO:N2 {created_on: datetime.transaction()}) RETURN m; \ No newline at end of file diff --git a/neo4j-migrations-test-resources/src/main/resources/ooo/base/second/V015__NewSecond.cypher b/neo4j-migrations-test-resources/src/main/resources/ooo/base/second/V015__NewSecond.cypher new file mode 100644 index 0000000000..46f0971bb8 --- /dev/null +++ b/neo4j-migrations-test-resources/src/main/resources/ooo/base/second/V015__NewSecond.cypher @@ -0,0 +1 @@ +CREATE (m:OOO:N3 {created_on: datetime.transaction()}) RETURN m; \ No newline at end of file diff --git a/neo4j-migrations-test-resources/src/main/resources/ooo/base/third/V005__NewFirst.cypher b/neo4j-migrations-test-resources/src/main/resources/ooo/base/third/V005__NewFirst.cypher new file mode 100644 index 0000000000..8107f7d5c6 --- /dev/null +++ b/neo4j-migrations-test-resources/src/main/resources/ooo/base/third/V005__NewFirst.cypher @@ -0,0 +1 @@ +CREATE (m:OOO:N4 {created_on: datetime.transaction()}) RETURN m; \ No newline at end of file diff --git a/neo4j-migrations-test-resources/src/main/resources/ooo/repeatable/modified/R002__na.cypher b/neo4j-migrations-test-resources/src/main/resources/ooo/repeatable/modified/R002__na.cypher new file mode 100644 index 0000000000..d16743e10a --- /dev/null +++ b/neo4j-migrations-test-resources/src/main/resources/ooo/repeatable/modified/R002__na.cypher @@ -0,0 +1 @@ +CREATE (n:FromRepeatable:Mod); \ No newline at end of file diff --git a/neo4j-migrations-test-resources/src/main/resources/ooo/repeatable/modified/R016__na.cypher b/neo4j-migrations-test-resources/src/main/resources/ooo/repeatable/modified/R016__na.cypher new file mode 100644 index 0000000000..d16743e10a --- /dev/null +++ b/neo4j-migrations-test-resources/src/main/resources/ooo/repeatable/modified/R016__na.cypher @@ -0,0 +1 @@ +CREATE (n:FromRepeatable:Mod); \ No newline at end of file diff --git a/neo4j-migrations-test-resources/src/main/resources/ooo/repeatable/orig/R002__na.cypher b/neo4j-migrations-test-resources/src/main/resources/ooo/repeatable/orig/R002__na.cypher new file mode 100644 index 0000000000..b28765efbe --- /dev/null +++ b/neo4j-migrations-test-resources/src/main/resources/ooo/repeatable/orig/R002__na.cypher @@ -0,0 +1 @@ +CREATE (n:FromRepeatable); \ No newline at end of file diff --git a/neo4j-migrations-test-resources/src/main/resources/ooo/repeatable/orig/R016__na.cypher b/neo4j-migrations-test-resources/src/main/resources/ooo/repeatable/orig/R016__na.cypher new file mode 100644 index 0000000000..b28765efbe --- /dev/null +++ b/neo4j-migrations-test-resources/src/main/resources/ooo/repeatable/orig/R016__na.cypher @@ -0,0 +1 @@ +CREATE (n:FromRepeatable); \ No newline at end of file From e2d2014f97f737f1eb98bced7422c08dea202a22 Mon Sep 17 00:00:00 2001 From: Michael Simons Date: Thu, 28 Nov 2024 22:08:08 +0100 Subject: [PATCH 3/3] Polish. Signed-off-by: Michael Simons --- .../neo4j/migrations/core/Migrations.java | 57 +++++++++++-------- .../migrations/core/MigrationsException.java | 21 ++++++- .../neo4j/migrations/maven/CleanMojo.java | 1 + .../neo4j/migrations/maven/InfoMojo.java | 1 + .../neo4j/migrations/maven/MigrateMojo.java | 1 + .../neo4j/migrations/maven/RepairMojo.java | 1 + .../neo4j/migrations/maven/ValidateMojo.java | 1 + 7 files changed, 57 insertions(+), 26 deletions(-) diff --git a/neo4j-migrations-core/src/main/java/ac/simons/neo4j/migrations/core/Migrations.java b/neo4j-migrations-core/src/main/java/ac/simons/neo4j/migrations/core/Migrations.java index f568f2b0a8..176bb9228a 100644 --- a/neo4j-migrations-core/src/main/java/ac/simons/neo4j/migrations/core/Migrations.java +++ b/neo4j-migrations-core/src/main/java/ac/simons/neo4j/migrations/core/Migrations.java @@ -35,6 +35,7 @@ import java.util.Objects; import java.util.Optional; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.function.Consumer; import java.util.function.Function; import java.util.function.Supplier; import java.util.logging.Level; @@ -79,6 +80,10 @@ public final class Migrations { .named("repeated_at__Neo4jMigration") .onProperties("at"); + // Used when rewiring relationships during out-of-order migrations + private static final String OLD_REL_ID = "oldRelId"; + private static final String INSERTED_ID = "insertedId"; + private final MigrationsConfig config; private final Driver driver; private final MigrationContext context; @@ -720,36 +725,37 @@ private void apply0(List migrations) { StopWatch stopWatch = new StopWatch(); for (Migration migration : new IterableMigrations(config, migrations)) { - if (!chain.isApplied(migration.getVersion().getValue()) && config.getVersionComparator().compare(migration.getVersion(), previousVersion) < 0) { + var isApplied = chain.isApplied(migration.getVersion().getValue()); + var isRepeated = false; + + if (!isApplied && config.getVersionComparator().compare(migration.getVersion(), previousVersion) < 0) { previousVersion = MigrationVersion.baseline(); } - boolean repeated = false; + Supplier logMessage = () -> String.format("Applied migration %s.", toString(migration)); - if (previousVersion != MigrationVersion.baseline() && chain.isApplied(migration.getVersion().getValue())) { - if (checksumOfRepeatableChanged(chain, migration)) { - logMessage = () -> String.format("Reapplied changed repeatable migration %s", toString(migration)); - repeated = true; - } else { + if (isApplied && previousVersion != MigrationVersion.baseline()) { + if (!checksumOfRepeatableChanged(chain, migration)) { LOGGER.log(Level.INFO, "Skipping already applied migration {0}", toString(migration)); previousVersion = migration.getVersion(); continue; } + + logMessage = () -> String.format("Reapplied changed repeatable migration %s", toString(migration)); + isRepeated = true; } try { stopWatch.start(); migration.apply(context); long executionTime = stopWatch.stop(); - previousVersion = recordApplication(chain.getUsername(), previousVersion, migration, executionTime, repeated); + previousVersion = recordApplication(chain.getUsername(), previousVersion, migration, executionTime, isRepeated); LOGGER.log(Level.INFO, logMessage); } catch (Exception e) { if (HBD.constraintProbablyRequiredEnterpriseEdition(e, getConnectionDetails())) { throw new MigrationsException(Messages.INSTANCE.format("errors.edition_mismatch", toString(migration), getConnectionDetails().getServerEdition())); - } else if (e instanceof MigrationsException) { - throw e; } - throw new MigrationsException("Could not apply migration: " + toString(migration) + ".", e); + throw MigrationsException.of(e, () -> "Could not apply migration: " + toString(migration) + "."); } finally { stopWatch.reset(); } @@ -804,8 +810,8 @@ RETURN id(c) AS insertedId, id(om) AS oldRelId, properties(om), id(ot) AS oldEnd """; var result = t.run(mergePreviousMigration + " " + createNewMigrationAndPath, parameters).single(); - if (!result.get("oldRelId").isNull()) { - return Optional.of(new ReplacedMigration(result.get("oldRelId").asLong(), result.get("insertedId").asLong())); + if (!result.get(OLD_REL_ID).isNull()) { + return Optional.of(new ReplacedMigration(result.get(OLD_REL_ID).asLong(), result.get(INSERTED_ID).asLong())); } else { return Optional.empty(); } @@ -813,19 +819,20 @@ RETURN id(c) AS insertedId, id(om) AS oldRelId, properties(om), id(ot) AS oldEnd } try (Session session = context.getSchemaSession()) { - var f = session.executeWrite(uow); - f.ifPresent(replacedMigration -> { - session.executeWriteWithoutResult(t -> { - t.run(""" - MATCH ()-[oldRel]->(oldEnd) - WHERE id(oldRel) = $oldRelId - MATCH (inserted) WHERE id(inserted) = $insertedId - MERGE (inserted) -[r:MIGRATED_TO]-> (oldEnd) - SET r = properties(oldRel) - DELETE (oldRel) - """, Map.of("oldRelId", replacedMigration.oldRelId(), "insertedId", replacedMigration.newMigrationNodeId())).consume(); - }); + var optionalReplacedMigration = session.executeWrite(uow); + Consumer rewire = replacedMigration -> session.executeWriteWithoutResult(t -> { + var query = """ + MATCH ()-[oldRel]->(oldEnd) + WHERE id(oldRel) = $oldRelId + MATCH (inserted) WHERE id(inserted) = $insertedId + MERGE (inserted) -[r:MIGRATED_TO]-> (oldEnd) + SET r = properties(oldRel) + DELETE (oldRel) + """; + var parameter = Map.of(OLD_REL_ID, replacedMigration.oldRelId(), INSERTED_ID, replacedMigration.newMigrationNodeId()); + t.run(query, parameter).consume(); }); + optionalReplacedMigration.ifPresent(rewire); } return appliedMigration.getVersion(); diff --git a/neo4j-migrations-core/src/main/java/ac/simons/neo4j/migrations/core/MigrationsException.java b/neo4j-migrations-core/src/main/java/ac/simons/neo4j/migrations/core/MigrationsException.java index 2548e15bba..2378e31c09 100644 --- a/neo4j-migrations-core/src/main/java/ac/simons/neo4j/migrations/core/MigrationsException.java +++ b/neo4j-migrations-core/src/main/java/ac/simons/neo4j/migrations/core/MigrationsException.java @@ -15,6 +15,9 @@ */ package ac.simons.neo4j.migrations.core; +import java.io.Serial; +import java.util.function.Supplier; + /** * An unchecked exception that is thrown when something didn't work as expected. Most of the time, mitigation from the * call won't be possible. @@ -24,7 +27,23 @@ */ public final class MigrationsException extends RuntimeException { - static final long serialVersionUID = 1L; + @Serial + private static final long serialVersionUID = 1L; + + /** + * If {@code cause} is already a {@link MigrationsException}, {@code cause} will be returned, otherwise + * a new {@link MigrationsException} will be created + * + * @param cause original cause + * @param messageSupplier message supplier for the new exception to be created + * @return a {@link MigrationsException} + */ + static MigrationsException of(Exception cause, Supplier messageSupplier) { + if (cause instanceof MigrationsException me) { + return me; + } + return new MigrationsException(messageSupplier.get(), cause); + } /** * Constructs a new exception with the given message. diff --git a/neo4j-migrations-maven-plugin/src/main/java/ac/simons/neo4j/migrations/maven/CleanMojo.java b/neo4j-migrations-maven-plugin/src/main/java/ac/simons/neo4j/migrations/maven/CleanMojo.java index c17f575a2b..418ed15980 100644 --- a/neo4j-migrations-maven-plugin/src/main/java/ac/simons/neo4j/migrations/maven/CleanMojo.java +++ b/neo4j-migrations-maven-plugin/src/main/java/ac/simons/neo4j/migrations/maven/CleanMojo.java @@ -40,6 +40,7 @@ public class CleanMojo extends AbstractConnectedMojo { * The default constructor is primarily used by the Maven machinery. */ public CleanMojo() { + // Make both JDK 21 JavaDoc and Maven happy } /** diff --git a/neo4j-migrations-maven-plugin/src/main/java/ac/simons/neo4j/migrations/maven/InfoMojo.java b/neo4j-migrations-maven-plugin/src/main/java/ac/simons/neo4j/migrations/maven/InfoMojo.java index 8cb7181a1c..e96960aefc 100644 --- a/neo4j-migrations-maven-plugin/src/main/java/ac/simons/neo4j/migrations/maven/InfoMojo.java +++ b/neo4j-migrations-maven-plugin/src/main/java/ac/simons/neo4j/migrations/maven/InfoMojo.java @@ -38,6 +38,7 @@ public class InfoMojo extends AbstractConnectedMojo { * The default constructor is primarily used by the Maven machinery. */ public InfoMojo() { + // Make both JDK 21 JavaDoc and Maven happy } @Override diff --git a/neo4j-migrations-maven-plugin/src/main/java/ac/simons/neo4j/migrations/maven/MigrateMojo.java b/neo4j-migrations-maven-plugin/src/main/java/ac/simons/neo4j/migrations/maven/MigrateMojo.java index d200ae05b3..77745950ea 100644 --- a/neo4j-migrations-maven-plugin/src/main/java/ac/simons/neo4j/migrations/maven/MigrateMojo.java +++ b/neo4j-migrations-maven-plugin/src/main/java/ac/simons/neo4j/migrations/maven/MigrateMojo.java @@ -41,6 +41,7 @@ public class MigrateMojo extends AbstractConnectedMojo { * The default constructor is primarily used by the Maven machinery. */ public MigrateMojo() { + // Make both JDK 21 JavaDoc and Maven happy } @Override diff --git a/neo4j-migrations-maven-plugin/src/main/java/ac/simons/neo4j/migrations/maven/RepairMojo.java b/neo4j-migrations-maven-plugin/src/main/java/ac/simons/neo4j/migrations/maven/RepairMojo.java index ef6a7955a6..3640cfc7e2 100644 --- a/neo4j-migrations-maven-plugin/src/main/java/ac/simons/neo4j/migrations/maven/RepairMojo.java +++ b/neo4j-migrations-maven-plugin/src/main/java/ac/simons/neo4j/migrations/maven/RepairMojo.java @@ -37,6 +37,7 @@ public class RepairMojo extends AbstractConnectedMojo { * The default constructor is primarily used by the Maven machinery. */ public RepairMojo() { + // Make both JDK 21 JavaDoc and Maven happy } @Override diff --git a/neo4j-migrations-maven-plugin/src/main/java/ac/simons/neo4j/migrations/maven/ValidateMojo.java b/neo4j-migrations-maven-plugin/src/main/java/ac/simons/neo4j/migrations/maven/ValidateMojo.java index 6633d06e71..a3e1ccf8cf 100644 --- a/neo4j-migrations-maven-plugin/src/main/java/ac/simons/neo4j/migrations/maven/ValidateMojo.java +++ b/neo4j-migrations-maven-plugin/src/main/java/ac/simons/neo4j/migrations/maven/ValidateMojo.java @@ -44,6 +44,7 @@ public class ValidateMojo extends AbstractConnectedMojo { * The default constructor is primarily used by the Maven machinery. */ public ValidateMojo() { + // Make both JDK 21 JavaDoc and Maven happy } /**