Merge branch '3459-checkIDs-avoid-duplicate-save' into 'master'

3459 check i ds avoid duplicate save

See merge request !8

org.fortiss.tooling.kernel/src/org/fortiss/tooling/kernel/internal/storage/eclipse/.ratings

 AutoUndoCommandStack.java 6aa645a9ed6e6547539c376fda97284928c4f9d4 GREEN
 EMFTransactionalCommand.java ba4b5bead9768b6ce6c955b9238cd96cb722533c GREEN
 EclipseResourceStorageService.java 1b9722e31a5ec33e4c3f7bb171fc2ce587729bf8 GREEN
-ModelContext.java 85ede185d4b232221a96d0bf37a4c676b105dc00 GREEN
+ModelContext.java 220944231c0e036f29d4911d2658c33b2427b09f GREEN
 NonDirtyingEMFTransactionalCommand.java ec5f282603891096b09f2628155dd27e3a21c588 GREEN

org.fortiss.tooling.kernel/src/org/fortiss/tooling/kernel/internal/storage/eclipse/ModelContext.java

@@ -34,6 +34,7 @@ import java.util.Iterator;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
+import java.util.Map.Entry;
 import java.util.Set;
 
 import org.eclipse.core.resources.IFile;
@@ -151,7 +152,14 @@ class ModelContext implements ITopLevelElement, CommandStackListener {
 			}
 		});
 
-		checkIDs();
+		if(checkIDs()) {
+			try {
+				doSave(new NullProgressMonitor());
+			} catch(IOException | CoreException e) {
+				error(ToolingKernelActivator.getDefault(),
+						"Error saving model file when fixing missing/duplicate IDs.", e);
+			}
+		}
 	}
 
 	/** {@inheritDoc} */
@@ -182,29 +190,35 @@ class ModelContext implements ITopLevelElement, CommandStackListener {
 	}
 
 	/**
-	 * Checks whether all IDs are present and unique and updates {@link #maxId}. In case duplicate
-	 * IDs have been detected (which could only be caused by a bug in AF3), these are removed and an
-	 * error is logged.
+	 * Checks whether all IDs are present and unique and updates {@link #maxId}.
+	 * <ul>
+	 * <li>Duplicate IDs indicate a programming error. They are removed and an error is logged</li>
+	 * <li>Missing IDs are silently added.</li>
+	 * </ul>
+	 * 
+	 * @return {@code true} iff the model was modified because of missing/duplicated IDs (and hence
+	 *         must
+	 *         be saved).
 	 */
-	private void checkIDs() {
+	private boolean checkIDs() {
 		Set<Integer> ids = new HashSet<Integer>();
 
-		boolean hadMissing = false;
-		boolean hadDuplicates = false;
+		// Store String representation of element with duplicated IDs before fixing it
+		Map<String, Integer> objWithIdProblem = new HashMap<String, Integer>();
 		for(Iterator<EObject> i = getRootModelElement().eAllContents(); i.hasNext();) {
 			EObject eo = i.next();
 			if(eo instanceof IIdLabeled) {
 				final IIdLabeled element = (IIdLabeled)eo;
 				int id = element.getId();
 				if(id <= 0) {
-					hadMissing = true;
+					objWithIdProblem.put(element.toString(), null);
 				} else {
 					// Reset duplicate IDs. They will be regenerated together with missing IDs. This
 					// is a safe operation since at this stage, references are still based on Java
 					// references. Only in the persisted resource, references will be based on IDs,
 					// which is why special care must be taken to ensure uniqueness.
 					if(ids.contains(id)) {
-						hadDuplicates = true;
+						objWithIdProblem.put(element.toString(), element.getId());
 						runAsNonDirtyingCommand(() -> {
 							element.setId(0);
 						});
@@ -218,24 +232,31 @@ class ModelContext implements ITopLevelElement, CommandStackListener {
 
 		maxId = max(0, maxId);
 
-		if(hadMissing || hadDuplicates) {
+		if(!objWithIdProblem.isEmpty()) {
 			runAsNonDirtyingCommand(() -> {
 				maxId = 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) {
-			String msg = "Duplicate IDs have been removed from \"";
-			msg += resource.getURI().lastSegment() + "\". ";
-			msg += "Please report this incident since it could result in corrupted model files.";
+			String msg = "The following missing/duplicate IDs have been fixed in \"";
+			msg += resource.getURI().lastSegment() + "\".\n";
+			msg += "Please report this incident since it indicates a programming error ";
+			msg += "that could result in corrupted model files.\n";
+			for(Entry<String, Integer> entry : objWithIdProblem.entrySet()) {
+				Integer id = entry.getValue();
+				if(id == null) {
+					msg += "  Missing ID added to ";
+				} else {
+					msg += "  Duplicate ID " + id + " disambiguated for ";
+				}
+				String name = entry.getKey();
+				msg += "element \"" + name + "\"\n";
+			}
 			error(ToolingKernelActivator.getDefault(), msg);
+
+			return true;
 		}
+
+		return false;
 	}
 
 	/** {@inheritDoc} */