Attachment 54674 Details for Bug 161169 – Proposed fix

Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

[patch] Proposed fix

Clipboard-attachment (text/plain), 15.84 KB, created by Christian Damus

on 2006-11-28 16:06:37 EST

(hide)

Description:

Filename:

MIME Type:

Creator: Christian Damus

Created: 2006-11-28 16:06:37 EST

Size: 15.84 KB

patch

obsolete

>### Eclipse Workspace Patch 1.0
>#P org.eclipse.emf.transaction.tests
>Index: src/org/eclipse/emf/transaction/tests/TransactionChangeRecorderTest.java
>===================================================================
>RCS file: /cvsroot/technology/org.eclipse.emft/transaction/tests/org.eclipse.emf.transaction.tests/src/org/eclipse/emf/transaction/tests/TransactionChangeRecorderTest.java,v
>retrieving revision 1.2
>diff -u -r1.2 TransactionChangeRecorderTest.java
>--- src/org/eclipse/emf/transaction/tests/TransactionChangeRecorderTest.java	21 Apr 2006 18:03:41 -0000	1.2
>+++ src/org/eclipse/emf/transaction/tests/TransactionChangeRecorderTest.java	28 Nov 2006 21:01:22 -0000
>@@ -31,8 +31,10 @@
> import org.eclipse.emf.ecore.EPackage;
> import org.eclipse.emf.ecore.resource.Resource;
> import org.eclipse.emf.ecore.resource.ResourceSet;
>+import org.eclipse.emf.ecore.util.EcoreUtil;
> import org.eclipse.emf.ecore.util.InternalEList;
> import org.eclipse.emf.transaction.impl.TransactionChangeRecorder;
>+import org.eclipse.emf.transaction.util.TransactionUtil;
> 
> 
> /**
>@@ -151,7 +153,7 @@
> 	
> 	/**
> 	 * Tests that the change recorder is correctly propagated to the objects in
>-	 * a resource when it is loadedwhen loading in a read-write transaction
>+	 * a resource when it is loaded when loading in a read-write transaction
> 	 * (creating change descriptions).
> 	 */
> 	public void test_changeRecorderPropagatedOnLoad_writeTX() {
>@@ -183,6 +185,216 @@
> 		assertFalse(nestedResource2.isLoaded());
> 	}
> 	
>+	/**
>+	 * Tests that disposing an editing domain (and its change recorder) removes
>+	 * the change recorder from all of the current contents of the resource
>+	 * set.
>+	 */
>+	public void test_changeRecorderDispose_161169() {
>+		startWriting();
>+		loadRoot();
>+		
>+		TransactionChangeRecorder recorder = getRecorder(rootResource);
>+		
>+		EClass eclass = findClass("root/A", true); //$NON-NLS-1$
>+		
>+		assertSame(recorder, getRecorder(eclass));
>+		
>+		commit();
>+		
>+		domain.dispose();
>+		
>+		try {
>+			// attempt to change it without a transaction. Should be allowed
>+			eclass.setName("NewName"); //$NON-NLS-1$
>+		} catch (Exception e) {
>+			e.printStackTrace();
>+			fail("Should not have asserted the transaction protocol: " //$NON-NLS-1$
>+					+ e.getLocalizedMessage());
>+		}
>+	}
>+	
>+	/**
>+	 * Tests that the change recorder is implicitly removed even from model
>+	 * elements that were not attached to the resource set at the time when
>+	 * the editing domain was disposed.
>+	 */
>+	public void test_changeRecorderDispose_detachedElements_161169() {
>+		startWriting();
>+		loadRoot();
>+		
>+		TransactionChangeRecorder recorder = getRecorder(rootResource);
>+		
>+		EClass eclass = findClass("root/A", true); //$NON-NLS-1$
>+		
>+		assertSame(recorder, getRecorder(eclass));
>+		
>+		// detach the EClass
>+		eclass.getEPackage().getEClassifiers().remove(eclass);
>+		
>+		commit();
>+		
>+		domain.dispose();
>+		
>+		try {
>+			// attempt to change it without a transaction. Should be allowed
>+			eclass.setName("NewName"); //$NON-NLS-1$
>+		} catch (Exception e) {
>+			e.printStackTrace();
>+			fail("Should not have asserted the transaction protocol: " //$NON-NLS-1$
>+					+ e.getLocalizedMessage());
>+		}
>+	}
>+	
>+	/**
>+	 * Tests the new utility for "freeing" resources from the transactional
>+	 * protocol of an editing domain.
>+	 */
>+	public void test_freeDetachedResources_161169() {
>+		startWriting();
>+		loadRoot();
>+		
>+		TransactionChangeRecorder recorder = getRecorder(rootResource);
>+		
>+		EClass eclass = findClass("root/A", true); //$NON-NLS-1$
>+		
>+		assertSame(recorder, getRecorder(eclass));
>+		
>+		commit();
>+		
>+		// no transaction protocol on this
>+		domain.getResourceSet().getResources().remove(rootResource);
>+		
>+		// set the resource free
>+		TransactionUtil.disconnectFromEditingDomain(rootResource);
>+		
>+		try {
>+			// attempt to change it without a transaction. Should be allowed
>+			eclass.setName("NewName"); //$NON-NLS-1$
>+		} catch (Exception e) {
>+			e.printStackTrace();
>+			fail("Should not have asserted the transaction protocol: " //$NON-NLS-1$
>+					+ e.getLocalizedMessage());
>+		}
>+	}
>+	
>+	/**
>+	 * Tests that the new utility frees only the proper contents of the resource.
>+	 */
>+	public void test_freeDetachedResources_properContents_161169() {
>+		startWriting();
>+		loadRoot();
>+		
>+		TransactionChangeRecorder recorder = getRecorder(rootResource);
>+		
>+		EClass eclass = findClass("root/A", true); //$NON-NLS-1$
>+		EClass nested = findClass("root/nested1/nested2/nested3/D", true); //$NON-NLS-1$
>+		
>+		assertSame(recorder, getRecorder(eclass));
>+		
>+		commit();
>+		
>+		// no transaction protocol on this
>+		domain.getResourceSet().getResources().remove(rootResource);
>+		
>+		// set the root resource free
>+		TransactionUtil.disconnectFromEditingDomain(rootResource);
>+		
>+		try {
>+			// set the nested resource free
>+			TransactionUtil.disconnectFromEditingDomain(nestedResource1);
>+			
>+			fail("Should have thrown IllegalArgumentException"); //$NON-NLS-1$
>+		} catch (IllegalArgumentException e) {
>+			// pass
>+			System.out.println("Got expected exception: " + e.getLocalizedMessage()); //$NON-NLS-1$
>+		}
>+		
>+		try {
>+			// modify the nested element
>+			nested.setName("NewName"); //$NON-NLS-1$
>+			
>+			fail("Should have thrown IllegalStateException"); //$NON-NLS-1$
>+		} catch (IllegalStateException e) {
>+			// pass
>+			System.out.println("Got expected exception: " + e.getLocalizedMessage()); //$NON-NLS-1$
>+		}
>+	}
>+	
>+	/**
>+	 * Tests the new utility for "freeing" elements from the transactional
>+	 * protocol of an editing domain.
>+	 */
>+	public void test_freeDetachedElements_161169() {
>+		startWriting();
>+		loadRoot();
>+		
>+		TransactionChangeRecorder recorder = getRecorder(rootResource);
>+		
>+		EClass eclass = findClass("root/A", true); //$NON-NLS-1$
>+		
>+		assertSame(recorder, getRecorder(eclass));
>+		
>+		// detach some ancestor, just for fun
>+		EObject container = eclass.eContainer();
>+		EcoreUtil.remove(container);
>+		
>+		commit();
>+		
>+		// set the element free
>+		TransactionUtil.disconnectFromEditingDomain(container);
>+		
>+		try {
>+			// attempt to change it without a transaction. Should be allowed
>+			eclass.setName("NewName"); //$NON-NLS-1$
>+		} catch (Exception e) {
>+			e.printStackTrace();
>+			fail("Should not have asserted the transaction protocol: " //$NON-NLS-1$
>+					+ e.getLocalizedMessage());
>+		}
>+	}
>+	
>+	/**
>+	 * Tests that the new utility frees only the proper contents of an element.
>+	 */
>+	public void test_freeDetachedElements_properContents_161169() {
>+		startWriting();
>+		loadRoot();
>+		
>+		TransactionChangeRecorder recorder = getRecorder(rootResource);
>+		
>+		EClass eclass = findClass("root/A", true); //$NON-NLS-1$
>+		EClass nested = findClass("root/nested1/nested2/nested3/D", true); //$NON-NLS-1$
>+		
>+		assertSame(recorder, getRecorder(eclass));
>+		
>+		// detach some ancestor, just for fun
>+		EObject container = eclass.eContainer();
>+		EcoreUtil.remove(container);
>+ 		
>+		commit();
>+		
>+		try {
>+			// set the nested element free
>+			TransactionUtil.disconnectFromEditingDomain(nested);
>+			
>+			fail("Should have thrown IllegalArgumentException"); //$NON-NLS-1$
>+		} catch (IllegalArgumentException e) {
>+			// pass
>+			System.out.println("Got expected exception: " + e.getLocalizedMessage()); //$NON-NLS-1$
>+		}
>+		
>+		try {
>+			// modify the nested element
>+			nested.setName("NewName"); //$NON-NLS-1$
>+			
>+			fail("Should have thrown IllegalStateException"); //$NON-NLS-1$
>+		} catch (IllegalStateException e) {
>+			// pass
>+			System.out.println("Got expected exception: " + e.getLocalizedMessage()); //$NON-NLS-1$
>+		}
>+	}
>+	
> 	//
> 	// Framework methods
> 	//
>#P org.eclipse.emf.transaction
>Index: src/org/eclipse/emf/transaction/util/TransactionUtil.java
>===================================================================
>RCS file: /cvsroot/technology/org.eclipse.emft/transaction/plugins/org.eclipse.emf.transaction/src/org/eclipse/emf/transaction/util/TransactionUtil.java,v
>retrieving revision 1.3
>diff -u -r1.3 TransactionUtil.java
>--- src/org/eclipse/emf/transaction/util/TransactionUtil.java	21 Apr 2006 18:03:38 -0000	1.3
>+++ src/org/eclipse/emf/transaction/util/TransactionUtil.java	28 Nov 2006 21:01:22 -0000
>@@ -16,13 +16,20 @@
> */
> package org.eclipse.emf.transaction.util;
> 
>+import java.util.Collections;
>+import java.util.Iterator;
>+
>+import org.eclipse.emf.common.notify.Notifier;
> import org.eclipse.emf.ecore.EObject;
> import org.eclipse.emf.ecore.resource.Resource;
> import org.eclipse.emf.ecore.resource.ResourceSet;
>+import org.eclipse.emf.ecore.util.EcoreUtil;
> import org.eclipse.emf.edit.domain.EditingDomain;
> import org.eclipse.emf.edit.domain.IEditingDomainProvider;
> import org.eclipse.emf.transaction.Transaction;
> import org.eclipse.emf.transaction.TransactionalEditingDomain;
>+import org.eclipse.emf.transaction.impl.InternalTransactionalEditingDomain;
>+import org.eclipse.emf.transaction.impl.TransactionChangeRecorder;
> 
> /**
> * Static utilities for dealing with EMF elements and resources in a
>@@ -125,4 +132,110 @@
> 		
> 		return result;
> 	}
>+	
>+	/**
>+	 * 
>+	 * Disconnects the specified resource from its editing domain, so that it is
>+	 * released from the constraints of the transactional environment. Note
>+	 * that this is only permitted if the resource is not currently attached to
>+	 * the resource set of the editing domain in question.
>+	 * 
>+	 * This should only be done with extreme caution. If any other resources
>+	 * that are still being managed by the transactional editing domain have
>+	 * dependencies on this <tt>resource</tt>, then existing undo/redo
>+	 * information for these may be corrupted and future undo recording may not
>+	 * be complete.
>+	 * 
>+	 * 
>+	 * @param resource a resource to disconnect from its current editing domain,
>+	 * if any
>+	 * 
>+	 * @throws IllegalStateException if the specified <tt>resource</tt> is
>+	 * still in the {@link ResourceSet} managed by its current editing
>+	 * domain
>+	 * 
>+	 * @since 1.1
>+	 */
>+	public static void disconnectFromEditingDomain(Resource resource) {
>+		disconnectFromEditingDomain0(resource);
>+	}
>+	
>+	/**
>+	 * 
>+	 * Disconnects the specified element from its editing domain, so that it is
>+	 * released from the constraints of the transactional environment. Note
>+	 * that this is only permitted if the element is not currently attached to
>+	 * the resource set of the editing domain in question.
>+	 * 
>+	 * This should only be done with extreme caution. If any other elements
>+	 * that are still being managed by the transactional editing domain have
>+	 * dependencies on this <tt>eobject</tt>, then existing undo/redo
>+	 * information for these may be corrupted and future undo recording may not
>+	 * be complete.
>+	 * 
>+	 * It is probably more useful to
>+	 * {@linkplain #disconnectFromEditingDomain(Resource) disconnect} an entire
>+	 * {@link Resource} from the editing domain instead of just an object,
>+	 * unless that object is being moved from one editing domain to another.
>+	 * 
>+	 * 
>+	 * @param eobject a model element to disconnect from its current editing
>+	 * domain, if any
>+	 * 
>+	 * @throws IllegalStateException if the specified <tt>eobject</tt> is
>+	 * still in the {@link ResourceSet} managed by its current editing
>+	 * domain
>+	 * 
>+	 * @since 1.1
>+	 * 
>+	 * @see #disconnectFromEditingDomain(Resource)i
>+	 */
>+	public static void disconnectFromEditingDomain(EObject eobject) {
>+		disconnectFromEditingDomain0(eobject);
>+	}
>+	
>+	/**
>+	 * Implements the disconnection of any notifier from the transactional
>+	 * editing domain.
>+	 * 
>+	 * @param notifier the notifier to disconnect
>+	 */
>+	private static void disconnectFromEditingDomain0(Notifier notifier) {
>+		TransactionChangeRecorder recorder = getExistingChangeRecorder(notifier);
>+		
>+		if (recorder != null) {
>+			// this resource is managed by a transactional editing domain
>+			InternalTransactionalEditingDomain domain =
>+				(InternalTransactionalEditingDomain) getEditingDomain(notifier);
>+			
>+			if ((domain != null) && (domain.getChangeRecorder() == recorder)) {
>+				throw new IllegalArgumentException("resource is still in the domain's resource set"); //$NON-NLS-1$
>+			}
>+			
>+			Iterator iter = EcoreUtil.getAllProperContents(Collections.singleton(
>+					notifier), false);
>+			while (iter.hasNext()) {
>+				((Notifier) iter.next()).eAdapters().remove(recorder);
>+			}
>+		}
>+	}
>+	
>+	/**
>+	 * Obtains the transaction change recorder currently attached to a notifier.
>+	 * 
>+	 * @param notifier a notifier
>+	 * @return the currently attached change recorder, or <code>null</code> if none
>+	 */
>+	private static TransactionChangeRecorder getExistingChangeRecorder(Notifier notifier) {
>+		TransactionChangeRecorder result = null;
>+		
>+		Object[] adapters = notifier.eAdapters().toArray();
>+		for (int i = 0; (i < adapters.length) && (result == null); i++) {
>+			if (adapters[i] instanceof TransactionChangeRecorder) {
>+				result = (TransactionChangeRecorder) adapters[i];
>+			}
>+		}
>+		
>+		return result;
>+	}
> }
>Index: src/org/eclipse/emf/transaction/impl/TransactionChangeRecorder.java
>===================================================================
>RCS file: /cvsroot/technology/org.eclipse.emft/transaction/plugins/org.eclipse.emf.transaction/src/org/eclipse/emf/transaction/impl/TransactionChangeRecorder.java,v
>retrieving revision 1.4
>diff -u -r1.4 TransactionChangeRecorder.java
>--- src/org/eclipse/emf/transaction/impl/TransactionChangeRecorder.java	10 Oct 2006 14:31:47 -0000	1.4
>+++ src/org/eclipse/emf/transaction/impl/TransactionChangeRecorder.java	28 Nov 2006 21:01:22 -0000
>@@ -28,6 +28,7 @@
> import org.eclipse.emf.ecore.change.util.ChangeRecorder;
> import org.eclipse.emf.ecore.resource.Resource;
> import org.eclipse.emf.ecore.resource.ResourceSet;
>+import org.eclipse.emf.ecore.util.EcoreUtil;
> import org.eclipse.emf.ecore.util.InternalEList;
> import org.eclipse.emf.transaction.NotificationFilter;
> import org.eclipse.emf.transaction.internal.EMFTransactionPlugin;
>@@ -53,6 +54,8 @@
> 	
> 	private boolean paused;
> 	
>+	private boolean disposed;
>+	
> 	/**
> 	 * Initializes me with the editing domain that I assist and the resource
> 	 * set in which I will record changes. Note that I do not begin recording
>@@ -68,8 +71,7 @@
> 		// super already started recording
> 		endRecording();
> 		
>-		// TODO: Restore when API available
>-		//setResolveProxies(false);
>+		setResolveProxies(false);
> 		
> 		// tell the resource set manager about any resources that already exist
> 		ResourceSetManager.getInstance().observe(rset);
>@@ -119,8 +121,7 @@
> 	 * </ul>
> 	 */
> 	public void setTarget(Notifier target) {
>-		// TODO: Restore when API available
>-		Iterator contents = target instanceof EObject ? /*resolveProxies*/false ? ((EObject) target).eContents().iterator()
>+		Iterator contents = target instanceof EObject ? isResolveProxies() ? ((EObject) target).eContents().iterator()
> 				: ((InternalEList) ((EObject) target).eContents()).basicIterator()
> 				: target instanceof ResourceSet ? ((ResourceSet) target)
> 						.getResources().iterator()
>@@ -143,6 +144,18 @@
> 	 * transaction (if any).
> 	 */
> 	public void notifyChanged(Notification notification) {
>+		if (disposed) {
>+			// I am disposed, so I should no longer be responding to or even
>+			// receiving events. Moreover, I should not propagate myself
>+			// to any elements that are later added to me!
>+			Object notifier = notification.getNotifier();
>+			if (notifier instanceof Notifier) {
>+				removeAdapter((Notifier) notifier);
>+			}
>+			
>+			return;
>+		}
>+		
> 		boolean record = true;
> 		
> 		switch (notification.getEventType()) {
>@@ -341,4 +354,25 @@
> 		
> 		paused = false;
> 	}
>+	
>+	/**
>+	 * Extends the inherited implementation to remove myself from all adapters
>+	 * that I can find in my editing domain.
>+	 * 
>+	 * @since 1.1
>+	 */
>+	public void dispose() {
>+		super.dispose();
>+		
>+		this.disposed = true;
>+		
>+		ResourceSet rset = domain.getResourceSet();
>+		if (rset != null) {
>+			removeAdapter(rset);
>+			
>+			for (Iterator iter = EcoreUtil.getAllProperContents(rset, false); iter.hasNext();) {
>+				removeAdapter((Notifier) iter.next());
>+			}
>+		}
>+	}
> }

Flags:

ahunter.eclipse: iplog+

Actions: View | Diff

Attachments on bug 161169: 54674