398712: [Core] Multiple issues with the PageManager

https://bugs.eclipse.org/bugs/show_bug.cgi?id=398712

401434: [Core] The CoreEditor shall be covered with integration tests
https://bugs.eclipse.org/bugs/show_bug.cgi?id=401434

2 files changed, 161 insertions, 76 deletions

diff --git a/plugins/infra/core/org.eclipse.papyrus.infra.core.sasheditor.di/src/org/eclipse/papyrus/infra/core/sashwindows/di/impl/PageRefImpl.java b/plugins/infra/core/org.eclipse.papyrus.infra.core.sasheditor.di/src/org/eclipse/papyrus/infra/core/sashwindows/di/impl/PageRefImpl.java
index fc4530eb71a..b7b9ea3f30a 100644
--- a/plugins/infra/core/org.eclipse.papyrus.infra.core.sasheditor.di/src/org/eclipse/papyrus/infra/core/sashwindows/di/impl/PageRefImpl.java
+++ b/plugins/infra/core/org.eclipse.papyrus.infra.core.sasheditor.di/src/org/eclipse/papyrus/infra/core/sashwindows/di/impl/PageRefImpl.java
@@ -25,13 +25,13 @@ import org.eclipse.papyrus.infra.core.sashwindows.di.TabFolder;
  * <p>

  * The following features are implemented:

  * <ul>

- *   <li>{@link org.eclipse.papyrus.infra.core.sashwindows.di.impl.PageRefImpl#getEmfPageIdentifier <em>Emf Page Identifier</em>}</li>

- *   <li>{@link org.eclipse.papyrus.infra.core.sashwindows.di.impl.PageRefImpl#getPageIdentifier <em>Page Identifier</em>}</li>

- *   <li>{@link org.eclipse.papyrus.infra.core.sashwindows.di.impl.PageRefImpl#getObjectPageIdentifier <em>Object Page Identifier</em>}</li>

- *   <li>{@link org.eclipse.papyrus.infra.core.sashwindows.di.impl.PageRefImpl#getParent <em>Parent</em>}</li>

+ * <li>{@link org.eclipse.papyrus.infra.core.sashwindows.di.impl.PageRefImpl#getEmfPageIdentifier <em>Emf Page Identifier</em>}</li>

+ * <li>{@link org.eclipse.papyrus.infra.core.sashwindows.di.impl.PageRefImpl#getPageIdentifier <em>Page Identifier</em>}</li>

+ * <li>{@link org.eclipse.papyrus.infra.core.sashwindows.di.impl.PageRefImpl#getObjectPageIdentifier <em>Object Page Identifier</em>}</li>

+ * <li>{@link org.eclipse.papyrus.infra.core.sashwindows.di.impl.PageRefImpl#getParent <em>Parent</em>}</li>

  * </ul>

  * </p>

- *

+ * 

  * @generated

  */

 public class PageRefImpl extends EObjectImpl implements PageRef {

@@ -40,6 +40,7 @@ public class PageRefImpl extends EObjectImpl implements PageRef {
 	 * The cached value of the '{@link #getEmfPageIdentifier() <em>Emf Page Identifier</em>}' reference.

 	 * <!-- begin-user-doc -->

 	 * <!-- end-user-doc -->

+	 * 

 	 * @see #getEmfPageIdentifier()

 	 * @generated

 	 * @ordered

@@ -50,6 +51,7 @@ public class PageRefImpl extends EObjectImpl implements PageRef {
 	 * The default value of the '{@link #getPageIdentifier() <em>Page Identifier</em>}' attribute.

 	 * <!-- begin-user-doc -->

 	 * <!-- end-user-doc -->

+	 * 

 	 * @see #getPageIdentifier()

 	 * @generated

 	 * @ordered

@@ -60,6 +62,7 @@ public class PageRefImpl extends EObjectImpl implements PageRef {
 	 * The default value of the '{@link #getObjectPageIdentifier() <em>Object Page Identifier</em>}' attribute.

 	 * <!-- begin-user-doc -->

 	 * <!-- end-user-doc -->

+	 * 

 	 * @see #getObjectPageIdentifier()

 	 * @generated

 	 * @ordered

@@ -70,6 +73,7 @@ public class PageRefImpl extends EObjectImpl implements PageRef {
 	 * The cached value of the '{@link #getObjectPageIdentifier() <em>Object Page Identifier</em>}' attribute.

 	 * <!-- begin-user-doc -->

 	 * <!-- end-user-doc -->

+	 * 

 	 * @see #getObjectPageIdentifier()

 	 * @generated

 	 * @ordered

@@ -79,6 +83,7 @@ public class PageRefImpl extends EObjectImpl implements PageRef {
 	/**

 	 * <!-- begin-user-doc -->

 	 * <!-- end-user-doc -->

+	 * 

 	 * @generated

 	 */

 	protected PageRefImpl() {

@@ -88,6 +93,7 @@ public class PageRefImpl extends EObjectImpl implements PageRef {
 	/**

 	 * <!-- begin-user-doc -->

 	 * <!-- end-user-doc -->

+	 * 

 	 * @generated

 	 */

 	@Override

@@ -98,15 +104,17 @@ public class PageRefImpl extends EObjectImpl implements PageRef {
 	/**

 	 * <!-- begin-user-doc -->

 	 * <!-- end-user-doc -->

+	 * 

 	 * @generated

 	 */

 	public EObject getEmfPageIdentifier() {

-		if (emfPageIdentifier != null && emfPageIdentifier.eIsProxy()) {

+		if(emfPageIdentifier != null && emfPageIdentifier.eIsProxy()) {

 			InternalEObject oldEmfPageIdentifier = (InternalEObject)emfPageIdentifier;

 			emfPageIdentifier = eResolveProxy(oldEmfPageIdentifier);

-			if (emfPageIdentifier != oldEmfPageIdentifier) {

-				if (eNotificationRequired())

+			if(emfPageIdentifier != oldEmfPageIdentifier) {

+				if(eNotificationRequired()) {

 					eNotify(new ENotificationImpl(this, Notification.RESOLVE, DiPackage.PAGE_REF__EMF_PAGE_IDENTIFIER, oldEmfPageIdentifier, emfPageIdentifier));

+				}

 			}

 		}

 		return emfPageIdentifier;

@@ -115,6 +123,7 @@ public class PageRefImpl extends EObjectImpl implements PageRef {
 	/**

 	 * <!-- begin-user-doc -->

 	 * <!-- end-user-doc -->

+	 * 

 	 * @generated

 	 */

 	public EObject basicGetEmfPageIdentifier() {

@@ -124,13 +133,15 @@ public class PageRefImpl extends EObjectImpl implements PageRef {
 	/**

 	 * <!-- begin-user-doc -->

 	 * <!-- end-user-doc -->

+	 * 

 	 * @generated

 	 */

 	public void setEmfPageIdentifier(EObject newEmfPageIdentifier) {

 		EObject oldEmfPageIdentifier = emfPageIdentifier;

 		emfPageIdentifier = newEmfPageIdentifier;

-		if (eNotificationRequired())

+		if(eNotificationRequired()) {

 			eNotify(new ENotificationImpl(this, Notification.SET, DiPackage.PAGE_REF__EMF_PAGE_IDENTIFIER, oldEmfPageIdentifier, emfPageIdentifier));

+		}

 	}

 

 	/**

@@ -144,8 +155,9 @@ public class PageRefImpl extends EObjectImpl implements PageRef {
 

 		if(res != null) {

 			return res;

-		} else

+		} else {

 			return getObjectPageIdentifier();

+		}

 	}

 

 	/**

@@ -155,21 +167,27 @@ public class PageRefImpl extends EObjectImpl implements PageRef {
 	 * @generated NOT

 	 */

 	public boolean isForIdentifier(Object pageIdentifier) {

+		//Avoid npe when there is an invalid "availablePage". See Bug 398712: Multiple issues with the PageManager

+		if(getPageIdentifier() == null) {

+			return false;

+		}

 		return getPageIdentifier().equals(pageIdentifier);

 	}

 

 	/**

 	 * <!-- begin-user-doc -->

 	 * <!-- end-user-doc -->

+	 * 

 	 * @generated

 	 */

 	@Override

 	public NotificationChain eInverseAdd(InternalEObject otherEnd, int featureID, NotificationChain msgs) {

-		switch (featureID) {

-			case DiPackage.PAGE_REF__PARENT:

-				if (eInternalContainer() != null)

-					msgs = eBasicRemoveFromContainer(msgs);

-				return basicSetParent((TabFolder)otherEnd, msgs);

+		switch(featureID) {

+		case DiPackage.PAGE_REF__PARENT:

+			if(eInternalContainer() != null) {

+				msgs = eBasicRemoveFromContainer(msgs);

+			}

+			return basicSetParent((TabFolder)otherEnd, msgs);

 		}

 		return super.eInverseAdd(otherEnd, featureID, msgs);

 	}

@@ -177,13 +195,14 @@ public class PageRefImpl extends EObjectImpl implements PageRef {
 	/**

 	 * <!-- begin-user-doc -->

 	 * <!-- end-user-doc -->

+	 * 

 	 * @generated

 	 */

 	@Override

 	public NotificationChain eInverseRemove(InternalEObject otherEnd, int featureID, NotificationChain msgs) {

-		switch (featureID) {

-			case DiPackage.PAGE_REF__PARENT:

-				return basicSetParent(null, msgs);

+		switch(featureID) {

+		case DiPackage.PAGE_REF__PARENT:

+			return basicSetParent(null, msgs);

 		}

 		return super.eInverseRemove(otherEnd, featureID, msgs);

 	}

@@ -191,13 +210,14 @@ public class PageRefImpl extends EObjectImpl implements PageRef {
 	/**

 	 * <!-- begin-user-doc -->

 	 * <!-- end-user-doc -->

+	 * 

 	 * @generated

 	 */

 	@Override

 	public NotificationChain eBasicRemoveFromContainerFeature(NotificationChain msgs) {

-		switch (eContainerFeatureID()) {

-			case DiPackage.PAGE_REF__PARENT:

-				return eInternalContainer().eInverseRemove(this, DiPackage.TAB_FOLDER__CHILDREN, TabFolder.class, msgs);

+		switch(eContainerFeatureID()) {

+		case DiPackage.PAGE_REF__PARENT:

+			return eInternalContainer().eInverseRemove(this, DiPackage.TAB_FOLDER__CHILDREN, TabFolder.class, msgs);

 		}

 		return super.eBasicRemoveFromContainerFeature(msgs);

 	}

@@ -205,20 +225,23 @@ public class PageRefImpl extends EObjectImpl implements PageRef {
 	/**

 	 * <!-- begin-user-doc -->

 	 * <!-- end-user-doc -->

+	 * 

 	 * @generated

 	 */

 	@Override

 	public Object eGet(int featureID, boolean resolve, boolean coreType) {

-		switch (featureID) {

-			case DiPackage.PAGE_REF__EMF_PAGE_IDENTIFIER:

-				if (resolve) return getEmfPageIdentifier();

-				return basicGetEmfPageIdentifier();

-			case DiPackage.PAGE_REF__PAGE_IDENTIFIER:

-				return getPageIdentifier();

-			case DiPackage.PAGE_REF__OBJECT_PAGE_IDENTIFIER:

-				return getObjectPageIdentifier();

-			case DiPackage.PAGE_REF__PARENT:

-				return getParent();

+		switch(featureID) {

+		case DiPackage.PAGE_REF__EMF_PAGE_IDENTIFIER:

+			if(resolve) {

+				return getEmfPageIdentifier();

+			}

+			return basicGetEmfPageIdentifier();

+		case DiPackage.PAGE_REF__PAGE_IDENTIFIER:

+			return getPageIdentifier();

+		case DiPackage.PAGE_REF__OBJECT_PAGE_IDENTIFIER:

+			return getObjectPageIdentifier();

+		case DiPackage.PAGE_REF__PARENT:

+			return getParent();

 		}

 		return super.eGet(featureID, resolve, coreType);

 	}

@@ -226,23 +249,24 @@ public class PageRefImpl extends EObjectImpl implements PageRef {
 	/**

 	 * <!-- begin-user-doc -->

 	 * <!-- end-user-doc -->

+	 * 

 	 * @generated

 	 */

 	@Override

 	public void eSet(int featureID, Object newValue) {

-		switch (featureID) {

-			case DiPackage.PAGE_REF__EMF_PAGE_IDENTIFIER:

-				setEmfPageIdentifier((EObject)newValue);

-				return;

-			case DiPackage.PAGE_REF__PAGE_IDENTIFIER:

-				setPageIdentifier(newValue);

-				return;

-			case DiPackage.PAGE_REF__OBJECT_PAGE_IDENTIFIER:

-				setObjectPageIdentifier(newValue);

-				return;

-			case DiPackage.PAGE_REF__PARENT:

-				setParent((TabFolder)newValue);

-				return;

+		switch(featureID) {

+		case DiPackage.PAGE_REF__EMF_PAGE_IDENTIFIER:

+			setEmfPageIdentifier((EObject)newValue);

+			return;

+		case DiPackage.PAGE_REF__PAGE_IDENTIFIER:

+			setPageIdentifier(newValue);

+			return;

+		case DiPackage.PAGE_REF__OBJECT_PAGE_IDENTIFIER:

+			setObjectPageIdentifier(newValue);

+			return;

+		case DiPackage.PAGE_REF__PARENT:

+			setParent((TabFolder)newValue);

+			return;

 		}

 		super.eSet(featureID, newValue);

 	}

@@ -250,23 +274,24 @@ public class PageRefImpl extends EObjectImpl implements PageRef {
 	/**

 	 * <!-- begin-user-doc -->

 	 * <!-- end-user-doc -->

+	 * 

 	 * @generated

 	 */

 	@Override

 	public void eUnset(int featureID) {

-		switch (featureID) {

-			case DiPackage.PAGE_REF__EMF_PAGE_IDENTIFIER:

-				setEmfPageIdentifier((EObject)null);

-				return;

-			case DiPackage.PAGE_REF__PAGE_IDENTIFIER:

-				setPageIdentifier(PAGE_IDENTIFIER_EDEFAULT);

-				return;

-			case DiPackage.PAGE_REF__OBJECT_PAGE_IDENTIFIER:

-				setObjectPageIdentifier(OBJECT_PAGE_IDENTIFIER_EDEFAULT);

-				return;

-			case DiPackage.PAGE_REF__PARENT:

-				setParent((TabFolder)null);

-				return;

+		switch(featureID) {

+		case DiPackage.PAGE_REF__EMF_PAGE_IDENTIFIER:

+			setEmfPageIdentifier((EObject)null);

+			return;

+		case DiPackage.PAGE_REF__PAGE_IDENTIFIER:

+			setPageIdentifier(PAGE_IDENTIFIER_EDEFAULT);

+			return;

+		case DiPackage.PAGE_REF__OBJECT_PAGE_IDENTIFIER:

+			setObjectPageIdentifier(OBJECT_PAGE_IDENTIFIER_EDEFAULT);

+			return;

+		case DiPackage.PAGE_REF__PARENT:

+			setParent((TabFolder)null);

+			return;

 		}

 		super.eUnset(featureID);

 	}

@@ -274,19 +299,20 @@ public class PageRefImpl extends EObjectImpl implements PageRef {
 	/**

 	 * <!-- begin-user-doc -->

 	 * <!-- end-user-doc -->

+	 * 

 	 * @generated

 	 */

 	@Override

 	public boolean eIsSet(int featureID) {

-		switch (featureID) {

-			case DiPackage.PAGE_REF__EMF_PAGE_IDENTIFIER:

-				return emfPageIdentifier != null;

-			case DiPackage.PAGE_REF__PAGE_IDENTIFIER:

-				return PAGE_IDENTIFIER_EDEFAULT == null ? getPageIdentifier() != null : !PAGE_IDENTIFIER_EDEFAULT.equals(getPageIdentifier());

-			case DiPackage.PAGE_REF__OBJECT_PAGE_IDENTIFIER:

-				return OBJECT_PAGE_IDENTIFIER_EDEFAULT == null ? objectPageIdentifier != null : !OBJECT_PAGE_IDENTIFIER_EDEFAULT.equals(objectPageIdentifier);

-			case DiPackage.PAGE_REF__PARENT:

-				return getParent() != null;

+		switch(featureID) {

+		case DiPackage.PAGE_REF__EMF_PAGE_IDENTIFIER:

+			return emfPageIdentifier != null;

+		case DiPackage.PAGE_REF__PAGE_IDENTIFIER:

+			return PAGE_IDENTIFIER_EDEFAULT == null ? getPageIdentifier() != null : !PAGE_IDENTIFIER_EDEFAULT.equals(getPageIdentifier());

+		case DiPackage.PAGE_REF__OBJECT_PAGE_IDENTIFIER:

+			return OBJECT_PAGE_IDENTIFIER_EDEFAULT == null ? objectPageIdentifier != null : !OBJECT_PAGE_IDENTIFIER_EDEFAULT.equals(objectPageIdentifier);

+		case DiPackage.PAGE_REF__PARENT:

+			return getParent() != null;

 		}

 		return super.eIsSet(featureID);

 	}

@@ -294,11 +320,14 @@ public class PageRefImpl extends EObjectImpl implements PageRef {
 	/**

 	 * <!-- begin-user-doc -->

 	 * <!-- end-user-doc -->

+	 * 

 	 * @generated

 	 */

 	@Override

 	public String toString() {

-		if (eIsProxy()) return super.toString();

+		if(eIsProxy()) {

+			return super.toString();

+		}

 

 		StringBuffer result = new StringBuffer(super.toString());

 		result.append(" (objectPageIdentifier: ");

@@ -333,6 +362,7 @@ public class PageRefImpl extends EObjectImpl implements PageRef {
 	/**

 	 * <!-- begin-user-doc -->

 	 * <!-- end-user-doc -->

+	 * 

 	 * @generated

 	 */

 	public Object getObjectPageIdentifier() {

@@ -342,28 +372,34 @@ public class PageRefImpl extends EObjectImpl implements PageRef {
 	/**

 	 * <!-- begin-user-doc -->

 	 * <!-- end-user-doc -->

+	 * 

 	 * @generated

 	 */

 	public void setObjectPageIdentifier(Object newObjectPageIdentifier) {

 		Object oldObjectPageIdentifier = objectPageIdentifier;

 		objectPageIdentifier = newObjectPageIdentifier;

-		if (eNotificationRequired())

+		if(eNotificationRequired()) {

 			eNotify(new ENotificationImpl(this, Notification.SET, DiPackage.PAGE_REF__OBJECT_PAGE_IDENTIFIER, oldObjectPageIdentifier, objectPageIdentifier));

+		}

 	}

 

 	/**

 	 * <!-- begin-user-doc -->

 	 * <!-- end-user-doc -->

+	 * 

 	 * @generated

 	 */

 	public TabFolder getParent() {

-		if (eContainerFeatureID() != DiPackage.PAGE_REF__PARENT) return null;

+		if(eContainerFeatureID() != DiPackage.PAGE_REF__PARENT) {

+			return null;

+		}

 		return (TabFolder)eInternalContainer();

 	}

 

 	/**

 	 * <!-- begin-user-doc -->

 	 * <!-- end-user-doc -->

+	 * 

 	 * @generated

 	 */

 	public NotificationChain basicSetParent(TabFolder newParent, NotificationChain msgs) {

@@ -374,22 +410,28 @@ public class PageRefImpl extends EObjectImpl implements PageRef {
 	/**

 	 * <!-- begin-user-doc -->

 	 * <!-- end-user-doc -->

+	 * 

 	 * @generated

 	 */

 	public void setParent(TabFolder newParent) {

-		if (newParent != eInternalContainer() || (eContainerFeatureID() != DiPackage.PAGE_REF__PARENT && newParent != null)) {

-			if (EcoreUtil.isAncestor(this, newParent))

+		if(newParent != eInternalContainer() || (eContainerFeatureID() != DiPackage.PAGE_REF__PARENT && newParent != null)) {

+			if(EcoreUtil.isAncestor(this, newParent)) {

 				throw new IllegalArgumentException("Recursive containment not allowed for " + toString());

+			}

 			NotificationChain msgs = null;

-			if (eInternalContainer() != null)

+			if(eInternalContainer() != null) {

 				msgs = eBasicRemoveFromContainer(msgs);

-			if (newParent != null)

+			}

+			if(newParent != null) {

 				msgs = ((InternalEObject)newParent).eInverseAdd(this, DiPackage.TAB_FOLDER__CHILDREN, TabFolder.class, msgs);

+			}

 			msgs = basicSetParent(newParent, msgs);

-			if (msgs != null) msgs.dispatch();

-		}

-		else if (eNotificationRequired())

+			if(msgs != null) {

+				msgs.dispatch();

+			}

+		} else if(eNotificationRequired()) {

 			eNotify(new ENotificationImpl(this, Notification.SET, DiPackage.PAGE_REF__PARENT, newParent, newParent));

+		}

 	}

 

 } //PageRefImpl

diff --git a/tests/junit/plugins/core/org.eclipse.papyrus.editor.integration.tests/src/org/eclipse/papyrus/editor/integration/tests/tests/RecoveryTest.java b/tests/junit/plugins/core/org.eclipse.papyrus.editor.integration.tests/src/org/eclipse/papyrus/editor/integration/tests/tests/RecoveryTest.java
index a7236186573..afb5be44731 100644
--- a/tests/junit/plugins/core/org.eclipse.papyrus.editor.integration.tests/src/org/eclipse/papyrus/editor/integration/tests/tests/RecoveryTest.java
+++ b/tests/junit/plugins/core/org.eclipse.papyrus.editor.integration.tests/src/org/eclipse/papyrus/editor/integration/tests/tests/RecoveryTest.java
@@ -11,12 +11,18 @@
  *****************************************************************************/

 package org.eclipse.papyrus.editor.integration.tests.tests;

 

+import org.eclipse.emf.common.command.Command;

+import org.eclipse.emf.common.command.CompoundCommand;

 import org.eclipse.emf.transaction.RecordingCommand;

 import org.eclipse.emf.transaction.TransactionalEditingDomain;

+import org.eclipse.gmf.runtime.emf.type.core.requests.DestroyElementRequest;

 import org.eclipse.gmf.runtime.notation.Diagram;

+import org.eclipse.papyrus.commands.DestroyElementPapyrusCommand;

+import org.eclipse.papyrus.commands.wrappers.GMFtoEMFCommandWrapper;

 import org.eclipse.papyrus.infra.core.resource.ModelSet;

 import org.eclipse.papyrus.infra.core.sasheditor.contentprovider.IPageManager;

 import org.eclipse.papyrus.infra.gmfdiag.common.model.NotationModel;

+import org.eclipse.papyrus.infra.gmfdiag.common.model.NotationUtils;

 import org.eclipse.papyrus.uml.diagram.clazz.UmlClassDiagramForMultiEditor;

 import org.junit.Assert;

 import org.junit.Ignore;

@@ -122,6 +128,43 @@ public class RecoveryTest extends AbstractEditorIntegrationTest {
 		Assert.assertEquals(3, pageManager.allPages().size());

 	}

 

+	//Not fixed yet

+	//We should be able to delete a diagram, even when there is an invalid page in the model

+	@Test

+	public void testDeleteDiagramWithInvalidAvailablePage() throws Exception {

+		initModel("deleteDiagramWithInvalidPage", "invalidAvailablePage");

+

+		//Di layout: Diagram 0 - Invalid page - Diagram 1

+		//We take the diagram at index 1 (i.e. after the invalid page) and try to delete it

+		final Diagram diagram = (Diagram)NotationUtils.getNotationModel(editor.getServicesRegistry().getService(ModelSet.class)).getResource().getContents().get(1);

+

+		final IPageManager pageManager = editor.getServicesRegistry().getService(IPageManager.class);

+

+		//Check initial state

+		int availablePages = pageManager.allPages().size();

+

+		TransactionalEditingDomain editingDomain = editor.getServicesRegistry().getService(TransactionalEditingDomain.class);

+

+		CompoundCommand command = new CompoundCommand("Delete diagram");

+

+		Command sashRemoveComd = new RecordingCommand(editingDomain, "Remove page") {

+

+			@Override

+			protected void doExecute() {

+				pageManager.removePage(diagram);

+			}

+

+		};

+

+		command.append(sashRemoveComd);

+		command.append(new GMFtoEMFCommandWrapper(new DestroyElementPapyrusCommand(new DestroyElementRequest(diagram, false))));

+

+		editingDomain.getCommandStack().execute(command);

+

+		Assert.assertEquals("The page has not been correctly removed", availablePages - 1, pageManager.allPages().size());

+		Assert.assertNull("The diagram should not be contained in the resource anymore", diagram.eResource());

+	}

+

 	@Override

 	protected String getSourcePath() {

 		return SOURCE_PATH;