From 73b6a67f7f670da14bd42df15c84b70e3f371e35 Mon Sep 17 00:00:00 2001 From: "Serhii Plyhun (commercial)" Date: Wed, 28 Aug 2024 14:50:43 +0200 Subject: [PATCH] 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 | 5 ++ .../migration/AbstractMigrationHandler.java | 36 ++++++--- .../impl/MicronodeMigrationImpl.java | 8 +- .../migration/impl/NodeMigrationImpl.java | 13 +++- .../mesh/core/data/HibFieldContainer.java | 1 - .../impl/MigrationStatusHandlerImpl.java | 10 ++- .../impl/AbstractGraphFieldContainerImpl.java | 9 +-- .../impl/NodeGraphFieldContainerImpl.java | 3 - .../mesh/core/data/impl/GraphFieldTypes.java | 3 +- .../schema/NodeMigrationEndpointTest.java | 74 +++++++++++++++++++ 10 files changed, 132 insertions(+), 30 deletions(-) diff --git a/LTS-CHANGELOG.adoc b/LTS-CHANGELOG.adoc index 94a3c96137..1d36ef3e20 100644 --- a/LTS-CHANGELOG.adoc +++ b/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) diff --git a/core/src/main/java/com/gentics/mesh/core/migration/AbstractMigrationHandler.java b/core/src/main/java/com/gentics/mesh/core/migration/AbstractMigrationHandler.java index d395480f87..93b2baec36 100644 --- a/core/src/main/java/com/gentics/mesh/core/migration/AbstractMigrationHandler.java +++ b/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 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> 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); + } + }); } /** diff --git a/core/src/main/java/com/gentics/mesh/core/migration/impl/MicronodeMigrationImpl.java b/core/src/main/java/com/gentics/mesh/core/migration/impl/MicronodeMigrationImpl.java index 7b87a5c186..c68eed85f2 100644 --- a/core/src/main/java/com/gentics/mesh/core/migration/impl/MicronodeMigrationImpl.java +++ b/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 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); diff --git a/core/src/main/java/com/gentics/mesh/core/migration/impl/NodeMigrationImpl.java b/core/src/main/java/com/gentics/mesh/core/migration/impl/NodeMigrationImpl.java index e885f314f3..a6c773686f 100644 --- a/core/src/main/java/com/gentics/mesh/core/migration/impl/NodeMigrationImpl.java +++ b/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 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(); diff --git a/mdm/api/src/main/java/com/gentics/mesh/core/data/HibFieldContainer.java b/mdm/api/src/main/java/com/gentics/mesh/core/data/HibFieldContainer.java index cba23e7505..c19ee1d4af 100644 --- a/mdm/api/src/main/java/com/gentics/mesh/core/data/HibFieldContainer.java +++ b/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 { diff --git a/mdm/common/src/main/java/com/gentics/mesh/core/migration/impl/MigrationStatusHandlerImpl.java b/mdm/common/src/main/java/com/gentics/mesh/core/migration/impl/MigrationStatusHandlerImpl.java index 1cfbd5b49b..7031fa0dac 100644 --- a/mdm/common/src/main/java/com/gentics/mesh/core/migration/impl/MigrationStatusHandlerImpl.java +++ b/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; } diff --git a/mdm/orientdb-wrapper/src/main/java/com/gentics/mesh/core/data/container/impl/AbstractGraphFieldContainerImpl.java b/mdm/orientdb-wrapper/src/main/java/com/gentics/mesh/core/data/container/impl/AbstractGraphFieldContainerImpl.java index cee0b25255..3cfc5c9ffa 100644 --- a/mdm/orientdb-wrapper/src/main/java/com/gentics/mesh/core/data/container/impl/AbstractGraphFieldContainerImpl.java +++ b/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. */ diff --git a/mdm/orientdb-wrapper/src/main/java/com/gentics/mesh/core/data/container/impl/NodeGraphFieldContainerImpl.java b/mdm/orientdb-wrapper/src/main/java/com/gentics/mesh/core/data/container/impl/NodeGraphFieldContainerImpl.java index 5e53c7bd61..69731305e0 100644 --- a/mdm/orientdb-wrapper/src/main/java/com/gentics/mesh/core/data/container/impl/NodeGraphFieldContainerImpl.java +++ b/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; diff --git a/mdm/orientdb-wrapper/src/main/java/com/gentics/mesh/core/data/impl/GraphFieldTypes.java b/mdm/orientdb-wrapper/src/main/java/com/gentics/mesh/core/data/impl/GraphFieldTypes.java index 04d725a591..a0baa0b7d3 100644 --- a/mdm/orientdb-wrapper/src/main/java/com/gentics/mesh/core/data/impl/GraphFieldTypes.java +++ b/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); } diff --git a/tests/tests-core/src/main/java/com/gentics/mesh/core/schema/NodeMigrationEndpointTest.java b/tests/tests-core/src/main/java/com/gentics/mesh/core/schema/NodeMigrationEndpointTest.java index 876da0d2f1..2fbefc8801 100644 --- a/tests/tests-core/src/main/java/com/gentics/mesh/core/schema/NodeMigrationEndpointTest.java +++ b/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 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";