diff --git a/org.fortiss.tooling.kernel/trunk/src/org/fortiss/tooling/kernel/internal/storage/eclipse/EMFTransactionalCommand.java b/org.fortiss.tooling.kernel/trunk/src/org/fortiss/tooling/kernel/internal/storage/eclipse/EMFTransactionalCommand.java index af10119a04d0399504f8c5bf5fd060b698e1d3fc..a90355f8a7e459b93736628438f6921531f293d0 100644 --- a/org.fortiss.tooling.kernel/trunk/src/org/fortiss/tooling/kernel/internal/storage/eclipse/EMFTransactionalCommand.java +++ b/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); diff --git a/org.fortiss.tooling.kernel/trunk/src/org/fortiss/tooling/kernel/internal/storage/eclipse/EclipseResourceStorageProvider.java b/org.fortiss.tooling.kernel/trunk/src/org/fortiss/tooling/kernel/internal/storage/eclipse/EclipseResourceStorageProvider.java index acff14300ed678cda8c7afe168ac2de188f462b8..70d3dcfbe5c9520949fbec7902891b120e72014b 100644 --- a/org.fortiss.tooling.kernel/trunk/src/org/fortiss/tooling/kernel/internal/storage/eclipse/EclipseResourceStorageProvider.java +++ b/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. */ diff --git a/org.fortiss.tooling.kernel/trunk/src/org/fortiss/tooling/kernel/internal/storage/eclipse/ModelContext.java b/org.fortiss.tooling.kernel/trunk/src/org/fortiss/tooling/kernel/internal/storage/eclipse/ModelContext.java index c3debee5771c3b08091f18e38b46097211dbaca0..c830abfb4f6cdd4c6837769ab8e328f21c7e1b72 100644 --- a/org.fortiss.tooling.kernel/trunk/src/org/fortiss/tooling/kernel/internal/storage/eclipse/ModelContext.java +++ b/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 längerer Name als ein Buchstabe wäre 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 dafür - // keine Ergebnisse. Also wäre eine Erklärung, 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