Feat/1213

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`.
 
 

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) {

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() {
 

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<MigrationVersion, Element> buildChain0(MigrationContext context, Lis
 		}
 
 		final String incompleteMigrationsMessage = "More migrations have been applied to the database than locally resolved.";
-		Map<MigrationVersion, Element> fullMigrationChain = new LinkedHashMap<>(
-			discoveredMigrations.size() + appliedMigrations.size());
+		Map<MigrationVersion, Element> fullMigrationChain = new TreeMap<>(context.getConfig().getVersionComparator());
+		boolean outOfOrderAllowed = context.getConfig().isOutOfOrder();
 		int i = 0;
 		for (Map.Entry<MigrationVersion, Element> entry : appliedMigrations.entrySet()) {
 			MigrationVersion expectedVersion = entry.getKey();
 			Optional<String> 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

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() {
 	}
 }

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;
@@ -51,7 +52,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;
 
@@ -80,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;
@@ -721,34 +725,37 @@ private void apply0(List<Migration> migrations) {
 
 		StopWatch stopWatch = new StopWatch();
 		for (Migration migration : new IterableMigrations(config, migrations)) {
+			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<String> 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();
 			}
@@ -766,40 +773,66 @@ private MigrationVersion recordApplication(String neo4jUser, MigrationVersion pr
 		parameters.put("executionTime", executionTime);
 		parameters.put(PROPERTY_MIGRATION_TARGET, migrationTarget.orElse(null));
 
-		TransactionCallback<ResultSummary> uow;
+		record ReplacedMigration(long oldRelId, long newMigrationNodeId) {
+		}
+
+		TransactionCallback<Optional<ReplacedMigration>> uow;
 		if (repeated) {
-			uow = t -> t.run(
-				"MATCH (l:__Neo4jMigration) WHERE l.version = $appliedMigration['version'] AND coalesce(l.migrationTarget,'<default>') = coalesce($migrationTarget,'<default>') WITH l "
+			uow = t -> {
+				t.run(
+					"MATCH (l:__Neo4jMigration) WHERE l.version = $appliedMigration['version'] AND coalesce(l.migrationTarget,'<default>') = coalesce($migrationTarget,'<default>') 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(OLD_REL_ID).isNull()) {
+					return Optional.of(new ReplacedMigration(result.get(OLD_REL_ID).asLong(), result.get(INSERTED_ID).asLong()));
+				} else {
+					return Optional.empty();
+				}
 			};
 		}
 
 		try (Session session = context.getSchemaSession()) {
-			session.executeWrite(uow);
+			var optionalReplacedMigration = session.executeWrite(uow);
+			Consumer<ReplacedMigration> 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.<String, Object>of(OLD_REL_ID, replacedMigration.oldRelId(), INSERTED_ID, replacedMigration.newMigrationNodeId());
+				t.run(query, parameter).consume();
+			});
+			optionalReplacedMigration.ifPresent(rewire);
 		}
 
 		return appliedMigration.getVersion();