SUP-17264: Don't jump over several schema versions during migration (#…

…1615)

* SUP-17264: Don't jump over several schema versions during migration

* Changelog

* Simplified solution

* Fix

* UT. Fixes.

* UT fix

* Conflicts resolving fix

* Stability fixes pt1

* Simplification

* Minor fix

* Minor fix

* Minor fix

LTS-CHANGELOG.adoc

@@ -17,6 +17,11 @@ include::content/docs/variables.adoc-include[]
 The LTS changelog lists releases which are only accessible via a commercial subscription.
 All fixes and changes in LTS releases will be released the next minor release. Changes from LTS 1.4.x will be included in release 1.5.0.
 
+[[v1.10.33]]
+== 1.10.33 (TBD)
+
+icon:check[] Core: A crash has been fixed on an attempt of (micro)node migration over non-adjacent (micro)schema versions.
+
 [[v1.10.32]]
 == 1.10.32 (07.08.2024)
 

core/src/main/java/com/gentics/mesh/core/migration/AbstractMigrationHandler.java

@@ -2,13 +2,14 @@
 
 import java.util.ArrayList;
 import java.util.List;
-import java.util.Map;
 import java.util.Queue;
 import java.util.Set;
 
 import javax.annotation.ParametersAreNonnullByDefault;
 import javax.inject.Provider;
 
+import org.apache.commons.lang3.tuple.Pair;
+
 import com.gentics.mesh.context.NodeMigrationActionContext;
 import com.gentics.mesh.core.data.HibFieldContainer;
 import com.gentics.mesh.core.data.HibNodeFieldContainer;
@@ -34,13 +35,12 @@
 import com.gentics.mesh.core.rest.event.EventCauseInfo;
 import com.gentics.mesh.core.rest.node.FieldMap;
 import com.gentics.mesh.core.rest.node.FieldMapImpl;
-import com.gentics.mesh.core.rest.node.field.Field;
+import com.gentics.mesh.core.rest.schema.FieldSchemaContainerVersion;
 import com.gentics.mesh.distributed.RequestDelegator;
 import com.gentics.mesh.etc.config.MeshOptions;
 import com.gentics.mesh.event.EventQueueBatch;
 import com.gentics.mesh.metric.MetricsService;
 import com.gentics.mesh.util.CollectionUtil;
-import com.gentics.mesh.util.StreamUtil;
 
 import io.vertx.core.logging.Logger;
 import io.vertx.core.logging.LoggerFactory;
@@ -112,15 +112,29 @@ protected void prepareMigration(HibFieldSchemaVersionElement<?, ?, ?, ?, ?> from
 	 */
 	protected void migrate(NodeMigrationActionContext ac, HibFieldContainer newContainer, FieldContainer newContent,
 						   HibFieldSchemaVersionElement<?, ?, ?, ?, ?> fromVersion) throws Exception {
-		FieldMap fields = new FieldMapImpl();
-		Map<String, Field> newFields = fromVersion.getChanges()
-			.filter(change -> !(change instanceof HibRemoveFieldChange)) // nothing to do for removed fields, they were never added
-			.map(change -> change.createFields(fromVersion.getSchema(), newContent))
-			.collect(StreamUtil.mergeMaps());
-
-		fields.putAll(newFields);
+		ArrayList<HibFieldSchemaVersionElement<?, ?, ?, ?, ?>> versionChain = new ArrayList<>(1);
+		do {
+			versionChain.add(fromVersion);
+			fromVersion = fromVersion.getNextVersion();
+		} while (fromVersion != null && !newContainer.getSchemaContainerVersion().getVersion().equals(fromVersion.getVersion()));
 
-		newContainer.updateFieldsFromRest(ac, fields);
+		versionChain.stream()
+			.flatMap(version -> version.getChanges().map(change -> Pair.of(change, version)))
+			.filter(pair -> !(pair.getKey() instanceof HibRemoveFieldChange)) // nothing to do for removed fields, they were never added
+			.map(pair -> Pair.of(pair.getValue(), pair.getKey().createFields(pair.getValue().getSchema(), newContent)))
+			.forEach(pair -> {
+				FieldMap fm = new FieldMapImpl();
+				fm.putAll(pair.getValue());
+
+				// If a migration has been started over non-adjacent from/to versions, the intermediate changes need no actual storage, but a validation..
+				FieldSchemaContainerVersion schema = pair.getKey().getNextVersion().getSchema();
+				if (schema instanceof FieldSchemaContainerVersion && !((FieldSchemaContainerVersion)schema).getVersion().equals(newContainer.getSchemaContainerVersion().getVersion())) {
+					log.info("Update skipped for container [{}] of schema [{}] version [{}] from the intermediate version [{}]", newContainer.getUuid(), newContainer.getSchemaContainerVersion().getSchema().getName(), newContainer.getSchemaContainerVersion().getVersion(), ((FieldSchemaContainerVersion)schema).getVersion());
+					schema.assertForUnhandledFields(fm);
+				} else {
+					newContainer.updateFieldsFromRest(ac, fm);
+				}
+			});
 	}
 
 	/**

core/src/main/java/com/gentics/mesh/core/migration/impl/MicronodeMigrationImpl.java

@@ -47,6 +47,7 @@
 import com.gentics.mesh.event.EventQueueBatch;
 import com.gentics.mesh.metric.MetricsService;
 import com.gentics.mesh.util.VersionNumber;
+
 import io.reactivex.Completable;
 import io.reactivex.exceptions.CompositeException;
 import io.vertx.core.logging.Logger;
@@ -79,13 +80,18 @@ public Completable migrateMicronodes(MicronodeMigrationContext context) {
 			HibMicroschemaVersion toVersion = context.getToVersion();
 			MigrationStatusHandler status = context.getStatus();
 			MicroschemaMigrationCause cause = context.getCause();
+			String toUuid = db.tx(() -> toVersion.getUuid());
 
 			// Collect the migration scripts
 			NodeMigrationActionContextImpl ac = new NodeMigrationActionContextImpl();
 			Set<String> touchedFields = new HashSet<>();
 			try {
 				db.tx(() -> {
-					prepareMigration(reloadVersion(fromVersion), touchedFields);
+					HibMicroschemaVersion currentVersion = reloadVersion(fromVersion);
+					do {
+						prepareMigration(currentVersion, touchedFields);
+						currentVersion = currentVersion.getNextVersion();
+					} while (currentVersion != null && !currentVersion.getUuid().equals(toUuid));
 					ac.setProject(branch.getProject());
 					ac.setBranch(branch);
 

core/src/main/java/com/gentics/mesh/core/migration/impl/NodeMigrationImpl.java

@@ -25,6 +25,8 @@
 import javax.inject.Provider;
 import javax.inject.Singleton;
 
+import org.apache.commons.lang3.tuple.Pair;
+
 import com.gentics.mesh.context.NodeMigrationActionContext;
 import com.gentics.mesh.context.impl.NodeMigrationActionContextImpl;
 import com.gentics.mesh.core.data.HibField;
@@ -61,7 +63,6 @@
 import io.reactivex.exceptions.CompositeException;
 import io.vertx.core.logging.Logger;
 import io.vertx.core.logging.LoggerFactory;
-import org.apache.commons.lang3.tuple.Pair;
 
 /**
  * Handler for node migrations after schema updates.
@@ -155,18 +156,24 @@ public Completable migrateNodes(NodeMigrationActionContext context) {
 		context.validate();
 		return Completable.defer(() -> {
 			HibSchemaVersion fromVersion = context.getFromVersion();
+			HibSchemaVersion toVersion = context.getToVersion();
 			SchemaMigrationCause cause = context.getCause();
 			HibBranch branch = context.getBranch();
 			MigrationStatusHandler status = context.getStatus();
 			String branchUuid = db.tx(() -> branch.getUuid());
 			String fromUuud = db.tx(() -> fromVersion.getUuid());
-			String toUuid = db.tx(() -> context.getToVersion().getUuid());
+			String toUuid = db.tx(() -> toVersion.getUuid());
 
 			// Prepare the migration - Collect the migration scripts
 			Set<String> touchedFields = new HashSet<>();
 			try {
 				db.tx(() -> {
-					prepareMigration(reloadVersion(fromVersion), touchedFields);
+					HibSchemaVersion currentVersion = reloadVersion(fromVersion);
+					do {
+						prepareMigration(currentVersion, touchedFields);
+						currentVersion = currentVersion.getNextVersion();
+					} while (currentVersion != null && !currentVersion.getUuid().equals(toUuid));
+
 					if (status != null) {
 						status.setStatus(RUNNING);
 						status.commit();

mdm/api/src/main/java/com/gentics/mesh/core/data/HibFieldContainer.java

@@ -47,7 +47,6 @@
 import com.gentics.mesh.core.rest.schema.FieldSchemaContainer;
 import com.gentics.mesh.core.rest.schema.FieldSchemaContainerVersion;
 import com.gentics.mesh.core.rest.schema.ListFieldSchema;
-import com.gentics.mesh.core.rest.schema.SchemaModel;
 
 public interface HibFieldContainer extends HibBasicFieldContainer {
 

mdm/common/src/main/java/com/gentics/mesh/core/migration/impl/MigrationStatusHandlerImpl.java

@@ -18,8 +18,8 @@
 import com.gentics.mesh.core.endpoint.migration.MigrationStatusHandler;
 import com.gentics.mesh.core.migration.MigrationAbortedException;
 import com.gentics.mesh.core.rest.job.JobStatus;
-
 import com.gentics.mesh.core.rest.job.JobWarningList;
+
 import io.vertx.core.logging.Logger;
 import io.vertx.core.logging.LoggerFactory;
 
@@ -126,9 +126,11 @@ public MigrationStatusHandler error(Throwable error, String failureMessage) {
 			setStatus(FAILED);
 			log.error("Error handling migration", error);
 
-			job.setStopTimestamp();
-			job.setError(error);
-			commit(job);
+			if (job != null) {
+				job.setStopTimestamp();
+				job.setError(error);
+				commit(job);
+			}
 		}
 		return this;
 	}

mdm/orientdb-wrapper/src/main/java/com/gentics/mesh/core/data/container/impl/AbstractGraphFieldContainerImpl.java

@@ -9,11 +9,6 @@
 
 import java.util.List;
 
-import com.gentics.mesh.core.data.node.field.nesting.HibMicronodeField;
-import com.gentics.mesh.core.rest.node.FieldMapImpl;
-import com.gentics.mesh.core.rest.schema.SchemaModel;
-import io.vertx.core.logging.Logger;
-import io.vertx.core.logging.LoggerFactory;
 import org.apache.commons.lang3.StringUtils;
 
 import com.gentics.mesh.context.BulkActionContext;
@@ -57,6 +52,7 @@
 import com.gentics.mesh.core.data.node.field.list.impl.NodeGraphFieldListImpl;
 import com.gentics.mesh.core.data.node.field.list.impl.NumberGraphFieldListImpl;
 import com.gentics.mesh.core.data.node.field.list.impl.StringGraphFieldListImpl;
+import com.gentics.mesh.core.data.node.field.nesting.HibMicronodeField;
 import com.gentics.mesh.core.data.node.field.nesting.MicronodeGraphField;
 import com.gentics.mesh.core.data.node.field.nesting.NodeGraphField;
 import com.gentics.mesh.core.data.node.impl.MicronodeImpl;
@@ -69,6 +65,9 @@
 import com.gentics.mesh.core.rest.schema.FieldSchemaContainer;
 import com.syncleus.ferma.traversals.EdgeTraversal;
 
+import io.vertx.core.logging.Logger;
+import io.vertx.core.logging.LoggerFactory;
+
 /**
  * Abstract implementation for a field container. A {@link GraphFieldContainer} is used to store {@link GraphField} instances.
  */

mdm/orientdb-wrapper/src/main/java/com/gentics/mesh/core/data/container/impl/NodeGraphFieldContainerImpl.java

@@ -8,7 +8,6 @@
 import static com.gentics.mesh.core.data.relationship.GraphRelationships.HAS_FIELD_CONTAINER;
 import static com.gentics.mesh.core.data.relationship.GraphRelationships.HAS_LIST;
 import static com.gentics.mesh.core.data.relationship.GraphRelationships.HAS_VERSION;
-import static com.gentics.mesh.core.data.relationship.GraphRelationships.MICROSCHEMA_VERSION_KEY_PROPERTY;
 import static com.gentics.mesh.core.data.relationship.GraphRelationships.SCHEMA_CONTAINER_VERSION_KEY_PROPERTY;
 import static com.gentics.mesh.core.data.util.HibClassConverter.toGraph;
 import static com.gentics.mesh.core.rest.common.ContainerType.DRAFT;
@@ -47,13 +46,11 @@
 import com.gentics.mesh.core.data.node.field.impl.MicronodeGraphFieldImpl;
 import com.gentics.mesh.core.data.node.field.impl.S3BinaryGraphFieldImpl;
 import com.gentics.mesh.core.data.node.field.list.HibMicronodeFieldList;
-import com.gentics.mesh.core.data.node.field.list.MicronodeGraphFieldList;
 import com.gentics.mesh.core.data.node.field.list.impl.MicronodeGraphFieldListImpl;
 import com.gentics.mesh.core.data.node.field.nesting.HibMicronodeField;
 import com.gentics.mesh.core.data.node.field.nesting.MicronodeGraphField;
 import com.gentics.mesh.core.data.node.impl.NodeImpl;
 import com.gentics.mesh.core.data.schema.HibFieldSchemaVersionElement;
-import com.gentics.mesh.core.data.schema.HibMicroschemaVersion;
 import com.gentics.mesh.core.data.schema.HibSchemaVersion;
 import com.gentics.mesh.core.data.schema.impl.SchemaContainerVersionImpl;
 import com.gentics.mesh.core.data.search.BucketableElementHelper;

mdm/orientdb-wrapper/src/main/java/com/gentics/mesh/core/data/impl/GraphFieldTypes.java

@@ -164,8 +164,7 @@ public Field getRestFieldFromGraph(GraphFieldContainer container, InternalAction
 	 *            Field schema to be used to identify the type of the field
 	 * @param schema
 	 */
-	public void updateField(HibFieldContainer container, InternalActionContext ac, FieldMap fieldMap, String fieldKey,
-		FieldSchema fieldSchema, FieldSchemaContainer schema) {
+	public void updateField(HibFieldContainer container, InternalActionContext ac, FieldMap fieldMap, String fieldKey, FieldSchema fieldSchema, FieldSchemaContainer schema) {
 		updater.update(container, ac, fieldMap, fieldKey, fieldSchema, schema);
 	}
 

tests/tests-core/src/main/java/com/gentics/mesh/core/schema/NodeMigrationEndpointTest.java

@@ -19,7 +19,10 @@
 import static org.junit.Assert.assertTrue;
 
 import java.util.Comparator;
+import java.util.List;
 import java.util.UUID;
+import java.util.stream.Collectors;
+import java.util.stream.IntStream;
 
 import org.junit.Test;
 
@@ -75,6 +78,7 @@
 import com.gentics.mesh.event.EventQueueBatch;
 import com.gentics.mesh.json.JsonUtil;
 import com.gentics.mesh.parameter.impl.PublishParametersImpl;
+import com.gentics.mesh.parameter.impl.SchemaUpdateParametersImpl;
 import com.gentics.mesh.parameter.impl.VersioningParametersImpl;
 import com.gentics.mesh.test.MeshTestSetting;
 import com.gentics.mesh.test.context.AbstractMeshTest;
@@ -598,6 +602,76 @@ public void testMigrateAgain() throws Throwable {
 		assertThat(status).containsJobs(jobAUuid);
 	}
 
+	@Test
+	public void testMigrateSkippingVersions() throws Throwable {
+		String oldFieldName = "oldname";
+		String fieldName = "changedfield";
+		HibSchema container;
+		HibSchemaVersion versionA;
+		HibSchemaVersion versionB;
+		HibNode node;
+		String schemaUuid;
+
+		try (Tx tx = tx()) {
+			NodeDao nodeDao = tx.nodeDao();
+			BranchDao branchDao = tx.branchDao();
+			container = createDummySchemaWithChanges(oldFieldName, fieldName, false);
+			versionB = container.getLatestVersion();
+			versionA = versionB.getPreviousVersion();
+			schemaUuid = container.getUuid();
+
+			EventQueueBatch batch = createBatch();
+			branchDao.assignSchemaVersion(project().getLatestBranch(), user(), versionA, batch);
+			Tx.get().commit();
+
+			// create a node and publish
+			node = nodeDao.create(folder("2015"), user(), versionA, project());
+			HibNodeFieldContainer englishContainer = tx.contentDao().createFieldContainer(node, english(), project().getLatestBranch(),
+				user());
+			englishContainer.createString(oldFieldName).setString("content");
+			englishContainer.createString("name").setString("someName");
+			InternalActionContext ac = new InternalRoutingActionContextImpl(mockRoutingContext());
+			nodeDao.publish(node, ac, createBulkContext(), "en");
+			tx.success();
+		}
+
+		// 1. Drop random fields
+		SchemaUpdateRequest request = tx(() -> JsonUtil.readValue(node.getSchemaContainer().getLatestVersion().getJson(),
+			SchemaUpdateRequest.class));
+		List<FieldSchema> someFields = IntStream.range(0, request.getFields().size()).filter(i -> i % 2 == 0).mapToObj(i -> request.getFields().get(i)).collect(Collectors.toList());
+		request.getFields().removeAll(someFields);
+		adminCall(() -> client().updateSchema(schemaUuid, request, new SchemaUpdateParametersImpl().setUpdateAssignedBranches(false)));
+
+		// 2. Add random fields
+		IntStream.range(0, someFields.size()).forEach(i -> {
+			if (i % 2 == 0) {
+				someFields.get(i).setName("bogus_" + i);
+			}
+		});
+		request.getFields().addAll(someFields);
+		adminCall(() -> client().updateSchema(schemaUuid, request, new SchemaUpdateParametersImpl().setUpdateAssignedBranches(false)));
+
+		try (Tx tx = tx()) {
+			container = tx.schemaDao().findByUuid(schemaUuid);
+			versionB = container.getLatestVersion();
+			EventQueueBatch batch = createBatch();
+			tx.branchDao().assignSchemaVersion(project().getLatestBranch(), user(), versionB, batch);
+			tx.success();
+		}
+		doSchemaMigration(versionA, versionB);
+
+		try (Tx tx = tx()) {
+			ContentDao contentDao = tx.contentDao();
+			assertThat(tx.contentDao().getFieldContainer(node, "en")).as("Migrated skipping draft").isOf(versionB).hasVersion("2.0");
+			assertThat(contentDao.getFieldContainer(node, "en", project().getLatestBranch().getUuid(), ContainerType.PUBLISHED))
+				.as("Migrated skipping published")
+				.isOf(versionB).hasVersion("2.0");
+		}
+
+		JobListResponse status = adminCall(() -> client().findJobs());
+		assertThat(status).listsAll(COMPLETED);
+	}
+
 	@Test
 	public void testMigratePublished() throws Throwable {
 		String oldFieldName = "oldname";