From 105c658c815e8f0f354b1683349f95f554e05b66 Mon Sep 17 00:00:00 2001
From: Simon Barner <barner@fortiss.org>
Date: Tue, 24 Apr 2018 09:16:51 +0200
Subject: [PATCH] Rework handling of missing and duplicated IDs

- UniqueIdUtils:
    - Remove fixMissingIDs(final ITopLevelElement modelElement)
      (only used in one place)
    - fixMissingIDs(EObject object, EObject existingModel): Do not wrap
      model modification into a command. This is up to the caller of
      this method (e.g., the command stack is not available in tests)
- Remove IDMigrationProvider: Fixup of missing / duplicated is handled
  in ModelContext
- ModelContext: Use nonDirtyingCommand, and automatically save model
  in case IDs have been fixed

Issue-Ref: 3369
Signed-off-by: Simon Barner <barner@fortiss.org>
---
 org.fortiss.tooling.base/plugin.xml           |  3 --
 .../fortiss/tooling/base/migration/.ratings   |  1 -
 .../base/migration/IDMigrationProvider.java   | 52 -------------------
 .../kernel/internal/storage/eclipse/.ratings  |  2 +-
 .../storage/eclipse/ModelContext.java         | 11 +++-
 .../org/fortiss/tooling/kernel/utils/.ratings |  2 +-
 .../tooling/kernel/utils/UniqueIDUtils.java   | 28 ++--------
 7 files changed, 16 insertions(+), 83 deletions(-)
 delete mode 100644 org.fortiss.tooling.base/src/org/fortiss/tooling/base/migration/IDMigrationProvider.java

diff --git a/org.fortiss.tooling.base/plugin.xml b/org.fortiss.tooling.base/plugin.xml
index b647acd9b..5ed524a48 100644
--- a/org.fortiss.tooling.base/plugin.xml
+++ b/org.fortiss.tooling.base/plugin.xml
@@ -34,9 +34,6 @@
    </extension>
    
    <extension point="org.fortiss.tooling.kernel.migrationProvider">
-      <migrationProvider migrationProvider="org.fortiss.tooling.base.migration.IDMigrationProvider">
-         <objectClass objectClass="org.fortiss.tooling.kernel.extension.data.ITopLevelElement"/>
-      </migrationProvider>
       <migrationProvider migrationProvider="org.fortiss.tooling.base.migration.RemoveDuplicatedAnnotationsMigrationProvider">
          <objectClass objectClass="org.fortiss.tooling.kernel.extension.data.ITopLevelElement"/>
       </migrationProvider>
diff --git a/org.fortiss.tooling.base/src/org/fortiss/tooling/base/migration/.ratings b/org.fortiss.tooling.base/src/org/fortiss/tooling/base/migration/.ratings
index 5a96ad0bc..f6b064d04 100644
--- a/org.fortiss.tooling.base/src/org/fortiss/tooling/base/migration/.ratings
+++ b/org.fortiss.tooling.base/src/org/fortiss/tooling/base/migration/.ratings
@@ -1,3 +1,2 @@
-IDMigrationProvider.java fa145250ce031ab8e635db290b12cd6be2bda75c GREEN
 RemoveDuplicatedAnnotationsMigrationProvider.java 5d7689066a577110dbdf84b5c81372b1df2a51c6 GREEN
 RemoveOutdatedAnnotationInstanceMigrationProvider.java 29c29f2bb7515cad1de45a30ffc185001b47a016 GREEN
diff --git a/org.fortiss.tooling.base/src/org/fortiss/tooling/base/migration/IDMigrationProvider.java b/org.fortiss.tooling.base/src/org/fortiss/tooling/base/migration/IDMigrationProvider.java
deleted file mode 100644
index fa145250c..000000000
--- a/org.fortiss.tooling.base/src/org/fortiss/tooling/base/migration/IDMigrationProvider.java
+++ /dev/null
@@ -1,52 +0,0 @@
-/*-------------------------------------------------------------------------+
-| Copyright 2013 fortiss GmbH                                              |
-|                                                                          |
-| Licensed under the Apache License, Version 2.0 (the "License");          |
-| you may not use this file except in compliance with the License.         |
-| You may obtain a copy of the License at                                  |
-|                                                                          |
-|    http://www.apache.org/licenses/LICENSE-2.0                            |
-|                                                                          |
-| Unless required by applicable law or agreed to in writing, software      |
-| distributed under the License is distributed on an "AS IS" BASIS,        |
-| WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. |
-| See the License for the specific language governing permissions and      |
-| limitations under the License.                                           |
-+--------------------------------------------------------------------------*/
-package org.fortiss.tooling.base.migration;
-
-import static org.fortiss.tooling.kernel.utils.UniqueIDUtils.fixMissingIDs;
-import static org.fortiss.tooling.kernel.utils.UniqueIDUtils.hasMissingIDs;
-
-import java.util.Map;
-
-import org.eclipse.emf.ecore.EObject;
-import org.eclipse.emf.ecore.xml.type.AnyType;
-import org.fortiss.tooling.kernel.extension.IMigrationProvider;
-import org.fortiss.tooling.kernel.extension.data.ITopLevelElement;
-
-/**
- * {@link IDMigrationProvider} that - if required - adds missing IDs to a model.
- * 
- * @author Shaka
- */
-public class IDMigrationProvider implements IMigrationProvider {
-
-	/** {@inheritDoc} */
-	@Override
-	public boolean needMigration(ITopLevelElement modelElement,
-			Map<EObject, AnyType> unknownFeatures) {
-		// if the id of model element is 0
-		if(modelElement != null) {
-			EObject rootElement = modelElement.getRootModelElement();
-			return (rootElement != null) && hasMissingIDs(rootElement);
-		}
-		return false;
-	}
-
-	/** {@inheritDoc} */
-	@Override
-	public void migrate(ITopLevelElement modelElement, Map<EObject, AnyType> unknownFeatures) {
-		fixMissingIDs(modelElement);
-	}
-}
diff --git a/org.fortiss.tooling.kernel/src/org/fortiss/tooling/kernel/internal/storage/eclipse/.ratings b/org.fortiss.tooling.kernel/src/org/fortiss/tooling/kernel/internal/storage/eclipse/.ratings
index a01c117e8..185da99aa 100644
--- a/org.fortiss.tooling.kernel/src/org/fortiss/tooling/kernel/internal/storage/eclipse/.ratings
+++ b/org.fortiss.tooling.kernel/src/org/fortiss/tooling/kernel/internal/storage/eclipse/.ratings
@@ -1,5 +1,5 @@
 AutoUndoCommandStack.java 6aa645a9ed6e6547539c376fda97284928c4f9d4 GREEN
 EMFTransactionalCommand.java ba4b5bead9768b6ce6c955b9238cd96cb722533c GREEN
 EclipseResourceStorageService.java 1b9722e31a5ec33e4c3f7bb171fc2ce587729bf8 GREEN
-ModelContext.java 58e41785101a92624e60329b5bf8046466a49d33 GREEN
+ModelContext.java 9b0ae3e9ffbe2604be2631b44d94dcba9bd9ff99 YELLOW
 NonDirtyingEMFTransactionalCommand.java ec5f282603891096b09f2628155dd27e3a21c588 GREEN
diff --git a/org.fortiss.tooling.kernel/src/org/fortiss/tooling/kernel/internal/storage/eclipse/ModelContext.java b/org.fortiss.tooling.kernel/src/org/fortiss/tooling/kernel/internal/storage/eclipse/ModelContext.java
index 58e417851..9b0ae3e9f 100644
--- a/org.fortiss.tooling.kernel/src/org/fortiss/tooling/kernel/internal/storage/eclipse/ModelContext.java
+++ b/org.fortiss.tooling.kernel/src/org/fortiss/tooling/kernel/internal/storage/eclipse/ModelContext.java
@@ -36,6 +36,7 @@ import java.util.Set;
 import org.eclipse.core.resources.IFile;
 import org.eclipse.core.runtime.CoreException;
 import org.eclipse.core.runtime.IProgressMonitor;
+import org.eclipse.core.runtime.NullProgressMonitor;
 import org.eclipse.emf.common.command.AbstractCommand;
 import org.eclipse.emf.common.command.BasicCommandStack;
 import org.eclipse.emf.common.command.CommandStack;
@@ -201,7 +202,7 @@ class ModelContext implements ITopLevelElement, CommandStackListener {
 					// which is why special care must be taken to ensure uniqueness.
 					if(ids.contains(id)) {
 						hadDuplicates = true;
-						runAsCommand(new Runnable() {
+						runAsNonDirtyingCommand(new Runnable() {
 							@Override
 							public void run() {
 								element.setId(0);
@@ -218,12 +219,18 @@ class ModelContext implements ITopLevelElement, CommandStackListener {
 		maxId = Math.max(0, maxId);
 
 		if(hadMissing || hadDuplicates) {
-			runAsCommand(new Runnable() {
+			runAsNonDirtyingCommand(new Runnable() {
 				@Override
 				public void run() {
 					maxId = UniqueIDUtils.generateMissingIDs(getRootModelElement(), maxId);
 				}
 			});
+			try {
+				doSave(new NullProgressMonitor());
+			} catch(IOException | CoreException e) {
+				error(ToolingKernelActivator.getDefault(),
+						"Error saving model file when fixing missing/duplicate IDs.", e);
+			}
 		}
 
 		if(hadDuplicates) {
diff --git a/org.fortiss.tooling.kernel/src/org/fortiss/tooling/kernel/utils/.ratings b/org.fortiss.tooling.kernel/src/org/fortiss/tooling/kernel/utils/.ratings
index 16f03fb58..75d81368c 100644
--- a/org.fortiss.tooling.kernel/src/org/fortiss/tooling/kernel/utils/.ratings
+++ b/org.fortiss.tooling.kernel/src/org/fortiss/tooling/kernel/utils/.ratings
@@ -11,4 +11,4 @@ LoggingUtils.java a982f7c3371e72feb8658510b5b0358876281a12 GREEN
 PrototypesUtils.java ec75bed75cfc5103f1f38e3a29df86f729428775 GREEN
 ResourceUtils.java 5d8f55b3b22a8d963a26ae4a83b4228e2a03d963 GREEN
 TransformationUtils.java ef1f2346a7e31059fe6a48dff49d247809d38dcd GREEN
-UniqueIDUtils.java 99cf0934ee27a95f4b6edbc251e6ff24e5d1d7f7 GREEN
+UniqueIDUtils.java c520d053a6530a490431f7acc3cda1a2f8907281 YELLOW
diff --git a/org.fortiss.tooling.kernel/src/org/fortiss/tooling/kernel/utils/UniqueIDUtils.java b/org.fortiss.tooling.kernel/src/org/fortiss/tooling/kernel/utils/UniqueIDUtils.java
index 99cf0934e..c520d053a 100644
--- a/org.fortiss.tooling.kernel/src/org/fortiss/tooling/kernel/utils/UniqueIDUtils.java
+++ b/org.fortiss.tooling.kernel/src/org/fortiss/tooling/kernel/utils/UniqueIDUtils.java
@@ -259,42 +259,24 @@ public class UniqueIDUtils {
 		return false;
 	}
 
-	/**
-	 * Generates all missing IDs of the given model.
-	 * 
-	 * @param modelElement
-	 *            top level element
-	 */
-	public static void fixMissingIDs(final ITopLevelElement modelElement) {
-		final int maxID = getLargestID(modelElement.getRootModelElement());
-		if(maxID > 0) {
-			modelElement.runAsCommand(() -> {
-				generateMissingIDs(modelElement.getRootModelElement(), maxID);
-			});
-		}
-	}
-
 	/**
 	 * Generates all missing IDs of the given sub-model.
 	 * 
 	 * @param object
-	 *            The sub-model element for which missing IDs should be prepared
+	 *            Sub-model element for which missing IDs should be prepared
 	 * @param existingModel
-	 *            the existing model to be considered for used IDs if object
-	 *            already contains IDs > 0
+	 *            Any element in an existing model to be considered for used IDs.
+	 *            If this model does not contain any IDs <> 0, this method has no effect.
 	 */
 	public static void fixMissingIDs(EObject object, EObject existingModel) {
 		EObject root = existingModel;
 		while(root.eContainer() != null) {
 			root = root.eContainer();
 		}
-		ITopLevelElement topLevelElement =
-				IPersistencyService.getInstance().getTopLevelElementFor(existingModel);
 		final int maxID = getLargestID(root);
+
 		if(maxID > 0) {
-			topLevelElement.runAsCommand(() -> {
-				generateMissingIDs(object, maxID);
-			});
+			generateMissingIDs(object, maxID);
 		}
 	}
 }
-- 
GitLab