YELLOW

refs 343

org.fortiss.tooling.kernel/trunk/src/org/fortiss/tooling/kernel/internal/storage/eclipse/EMFTransactionalCommand.java

@@ -30,17 +30,16 @@ import org.fortiss.tooling.kernel.ToolingKernelActivator;
 import org.fortiss.tooling.kernel.utils.LoggingUtils;
 
 /**
- * This class wraps another EMF command and makes sure that all execute methods
- * ({@link #execute()}, {@link #undo()}, {@link #redo()}) are executed inside of
+ * This class wraps an EMF command and makes sure that all execute methods (
+ * {@link #execute()}, {@link #undo()}, {@link #redo()}) are executed inside of
  * a transaction. As a bonus the undo and redo methods are based on the
  * transaction of the execute call (i.e. are coming for free).
  * 
  * @author hummel
  * @author $Author$
  * @version $Rev$
- * @ConQAT.Rating GREEN Hash: 3A80DC630AEA6971552C44802D7533A3
+ * @ConQAT.Rating YELLOW Hash: FD089CCD7B06BD0A671A36B07A8F6E9C
  */
-// TODO: wie/warum ANOTHER EMF command? Was war denn der erste?
 public class EMFTransactionalCommand implements Command {
 
 	/** The wrapped command. */
@@ -164,7 +163,6 @@ public class EMFTransactionalCommand implements Command {
 		if (command == null) {
 			return this;
 		}
-
 		CompoundCommand result = new CompoundCommand();
 		result.append(this);
 		result.append(command);

org.fortiss.tooling.kernel/trunk/src/org/fortiss/tooling/kernel/internal/storage/eclipse/EclipseResourceStorageProvider.java

@@ -17,6 +17,9 @@ $Id$
 +--------------------------------------------------------------------------*/
 package org.fortiss.tooling.kernel.internal.storage.eclipse;
 
+import static org.eclipse.core.resources.IResourceDelta.ADDED;
+import static org.eclipse.core.resources.IResourceDelta.CHANGED;
+import static org.eclipse.core.resources.IResourceDelta.REMOVED;
 import static org.fortiss.tooling.kernel.utils.ExtensionPointUtils.getBundle;
 import static org.fortiss.tooling.kernel.utils.ExtensionPointUtils.getConfigurationElements;
 import static org.fortiss.tooling.kernel.utils.ExtensionPointUtils.loadClass;
@@ -59,7 +62,7 @@ import org.osgi.framework.Bundle;
  * @author hoelzlf
  * @author $Author$
  * @version $Rev$
- * @ConQAT.Rating YELLOW Hash: 3DCBF0F656CBD0DAE87FDCAA7CCBA86A
+ * @ConQAT.Rating YELLOW Hash: 97A421BF172008E2FC3E5255ED918D14
  */
 public class EclipseResourceStorageProvider implements
 		IEclipseResourceStorageService, IResourceChangeListener,
@@ -102,13 +105,7 @@ public class EclipseResourceStorageProvider implements
 					if (res instanceof IFile) {
 						IFile file = (IFile) res;
 						if (checkLocationProvider(file)) {
-							try {
-								loadContext(file);
-							} catch (IOException ioex) {
-								error(ToolingKernelActivator.getDefault(),
-										"IO Exception while loading model file: "
-												+ file.getName(), ioex);
-							}
+							loadContext(file);
 						}
 					}
 				}
@@ -164,40 +161,21 @@ public class EclipseResourceStorageProvider implements
 	/** Runs resource change job as workspace job. */
 	private void runWorkspaceChangeJob(final IFile file, final int changeKind) {
 		new WorkspaceJob("ModelFileChangedJob") {
-
 			@Override
-			// TODO: muss man das so schreiben?
 			public IStatus runInWorkspace(IProgressMonitor monitor) {
 				synchronized (EclipseResourceStorageProvider.this) {
-					if (changeKind == IResourceDelta.ADDED) {
-						// TODO: copy&paste 1
-						try {
-							loadContext(file);
-						} catch (IOException ioex) {
-							error(ToolingKernelActivator.getDefault(),
-									"Had an error during reloading the file!",
-									ioex);
-						}
-					} else if (changeKind == IResourceDelta.REMOVED
-							&& isLoaded(file)) {
+					if (changeKind == ADDED) {
+						loadContext(file);
+					} else if (changeKind == REMOVED && isLoaded(file)) {
 						unloadContext(file);
-					} else if (changeKind == IResourceDelta.CHANGED) {
+					} else if (changeKind == CHANGED) {
 						if (isLoaded(file)) {
 							handleChange(file);
 						} else {
-							// TODO: copy&paste 2
-							try {
-								loadContext(file);
-							} catch (IOException ioex) {
-								error(ToolingKernelActivator.getDefault(),
-										"Had an error during reloading the file!",
-										ioex);
-							}
+							loadContext(file);
 						}
 					}
-
 					// Refresh the top-level elements
-					// TODO: Dan wollte sowas als import static..
 					IPersistencyService.INSTANCE
 							.refreshTopLevelElements(EclipseResourceStorageProvider.this);
 				}
@@ -209,15 +187,10 @@ public class EclipseResourceStorageProvider implements
 	/** Handles a change of the given file (which is actually managed). */
 	private void handleChange(IFile file) {
 		ModelContext context = loadedContexts.get(file);
-		if (!context.getLastChangeWasIntended()) {
+		// this avoids to reload the context when itself caused the disk change
+		if (!context.getAndResetLastChangeWasCausedByModelContext()) {
 			unloadContext(file);
-			// TODO: copy&paste 3
-			try {
-				loadContext(file);
-			} catch (final Exception e) {
-				error(ToolingKernelActivator.getDefault(),
-						"Had an error during reloading the file!", e);
-			}
+			loadContext(file);
 		}
 	}
 
@@ -227,11 +200,16 @@ public class EclipseResourceStorageProvider implements
 	}
 
 	/** Loads a file and creates a new context from its contents. */
-	private ModelContext loadContext(IFile file) throws IOException {
-		ModelContext mc = new ModelContext(file);
-		loadedContexts.put(file, mc);
-		rootElementContexts.put(mc.getRootModelElement(), mc);
-		return mc;
+	private void loadContext(IFile file) {
+		try {
+			ModelContext mc = new ModelContext(file);
+			loadedContexts.put(file, mc);
+			rootElementContexts.put(mc.getRootModelElement(), mc);
+		} catch (IOException ioex) {
+			error(ToolingKernelActivator.getDefault(),
+					"IO Exception while loading model file: " + file.getName(),
+					ioex);
+		}
 	}
 
 	/** Unloads the context and destroys it. */

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

@@ -54,7 +54,7 @@ import org.fortiss.tooling.kernel.utils.UniqueIDUtils;
  * @author hummel
  * @author $Author$
  * @version $Rev$
- * @ConQAT.Rating YELLOW Hash: 2A3C09281619DEC4EBEDBA048E8E4046
+ * @ConQAT.Rating YELLOW Hash: 2D5596401C627A81D4E6F3504AD66CF2
  */
 class ModelContext implements ITopLevelElement, CommandStackListener {
 
@@ -65,8 +65,7 @@ class ModelContext implements ITopLevelElement, CommandStackListener {
 	private final IFile file;
 
 	/** The resource containing the model. */
-	// TODO: etwas lngerer Name als ein Buchstabe wre gut.
-	private final Resource r;
+	private final Resource resource;
 
 	/** The editing domain used for this model. */
 	private final TransactionalEditingDomain editingDomain;
@@ -78,25 +77,22 @@ class ModelContext implements ITopLevelElement, CommandStackListener {
 	private final List<CommandStackListener> listeners = new LinkedList<CommandStackListener>();
 
 	/** Flag for remembering whether the last change of the file was intended. */
-	// TODO: was ist ein unintended change? Und was ist ein intended change?
 	private boolean lastChangeWasIntended = false;
 
 	/** The maximal ID used in this model (used to generate new IDs). */
 	private int maxId = -1;
 
 	/** Constructor. */
-	// TODO: Kommentar?
 	/* package */ModelContext(IFile file) throws IOException {
 		this.file = file;
 
-		// TODO: Dan wollte sowas ausgelagert, oder?
 		editingDomain = TransactionalEditingDomain.Factory.INSTANCE
 				.createEditingDomain();
 		rset = editingDomain.getResourceSet();
 
-		r = rset.createResource(URI.createPlatformResourceURI(file
+		resource = rset.createResource(URI.createPlatformResourceURI(file
 				.getFullPath().toString(), true));
-		r.load(buildOptionsMap());
+		resource.load(buildOptionsMap());
 
 		transactionalCommandStack = new AutoUndoCommandStack(editingDomain);
 		transactionalCommandStack.addCommandStackListener(this);
@@ -107,7 +103,7 @@ class ModelContext implements ITopLevelElement, CommandStackListener {
 	/** {@inheritDoc} */
 	@Override
 	public EObject getRootModelElement() {
-		return r.getContents().get(0);
+		return resource.getContents().get(0);
 	}
 
 	/** {@inheritDoc} */
@@ -186,26 +182,31 @@ class ModelContext implements ITopLevelElement, CommandStackListener {
 		// perform ID checking before safe to not produce inconsistent models
 		checkIDs();
 
-		// we use a two-state save process, as otherwise inconsistencies in the
+		// we use a two-phase save process, as otherwise inconsistencies in the
 		// model can lead to partially written files which are unreadable and
 		// thus cause data loss.
-		// TODO: was ist denn ein two-state save process? Google findet dafr
-		// keine Ergebnisse. Also wre eine Erklrung, was das ist, nett, da ich
-		// sonst trotzdem nicht wei, was der Code unten tolles macht.
-		final ByteArrayOutputStream bytes = new ByteArrayOutputStream();
-		r.save(bytes, buildOptionsMap());
+
+		// first we write the content of the resource to a byte array
+		// this will abort the save process if something is wrong with
+		// the EMF model (a typical example of that is an EObject that
+		// is referenced from somewhere, but is not aggregated in the
+		// model tree (it s eContainer() is null
+		ByteArrayOutputStream bytes = new ByteArrayOutputStream();
+		resource.save(bytes, buildOptionsMap());
 		monitor.worked(1);
 
-		setLastChangeWasIntended();
+		// avoid reloading due to disk content change
+		setLastChangeWasCausedByModelContext();
+
+		// write file content to disk
 		file.setContents(new ByteArrayInputStream(bytes.toByteArray()), false,
 				true, monitor);
-
 		monitor.worked(1);
 
+		// set save state and notify listeners
 		((BasicCommandStack) editingDomain.getCommandStack()).saveIsDone();
 		performNotifyListeners(editingDomain.getCommandStack());
 
-		// TODO: Import static..
 		IPersistencyService.INSTANCE
 				.notifyTopLevelElementChanged(ModelContext.this);
 
@@ -217,16 +218,14 @@ class ModelContext implements ITopLevelElement, CommandStackListener {
 	 * side effect! The {@link #lastChangeWasIntended} flag will be reset, so
 	 * this method should only be called once per change event.
 	 */
-	// TODO: aber warum intended? und warum ndert ein "get" den Wert der
-	// Variable?
-	public synchronized boolean getLastChangeWasIntended() {
+	/* package */synchronized boolean getAndResetLastChangeWasCausedByModelContext() {
 		final boolean result = lastChangeWasIntended;
 		lastChangeWasIntended = false;
 		return result;
 	}
 
 	/** Sets the {@link #lastChangeWasIntended} flag to true. */
-	public synchronized void setLastChangeWasIntended() {
+	/* package */synchronized void setLastChangeWasCausedByModelContext() {
 		lastChangeWasIntended = true;
 	}
 
@@ -253,7 +252,6 @@ class ModelContext implements ITopLevelElement, CommandStackListener {
 	}
 
 	/** Helper method for calling notifyListeners on the given stack. */
-	/* TODO: Kommentar unten notwendig? */
 	/* package */void performNotifyListeners(CommandStack commandStack) {
 		try {
 			Method notifyListenersMethod = BasicCommandStack.class