OwnedTransientEStore::isSet for multiple different container references

[cdpm]

OwnedTransientEStore::isSet is currently implemented like this:

@Override
public boolean isSet(final InternalEObject object, final EStructuralFeature feature) {
    this.assertIsOwner(object);
    checkNotNull(feature, "Precondition violation - argument 'feature' must not be NULL!");
    // special case: a feature is always "set" if it is the container feature and eContainer != null
    if (feature instanceof EReference) {
        EReference eReference = (EReference) feature;
        if (eReference.isContainer()) {
            return this.getContainer(object) != null;
        }
    }
    if (feature.isMany()) {
        // for many-valued features, "being set" is defined as "not being empty"
        return this.getListOfValuesFor(feature).isEmpty() == false;
    }
    return this.contents.containsKey(feature);
}

@Override
public InternalEObject getContainer(final InternalEObject object) {
    this.assertIsOwner(object);
    return this.eContainer;
}

I think the special treatment of container references in OwnedTransientEStore::isSet misses the case where feature is a containment reference, but not the only one. A simple example would be two EClasses Order and OrderItem, with Order having two many-valued containment references to OrderItem, items (for items that have been added to the order) and consideredItems (for items that haven't been added to the order yet). If an OrderItem was contained by an Order via consideredItems, that would erroneously cause the modeled inverse of items to be considered set. (This example is taken from EMF: Eclipse Modeling Framework, p. 265.)

Failing unit test for reproducing:

package org.chronos.chronosphere.test.emf.estore.impl;

import org.chronos.chronosphere.test.emf.estore.base.EStoreTest;
import org.eclipse.emf.ecore.*;
import org.junit.Test;

import java.util.List;

import static org.junit.Assert.*;

public class IssueBasicEStoreTest extends EStoreTest {

    @Test
    @SuppressWarnings("unchecked")
    public void multipleMultiplicityManyContainmentsWithMultiplicityOneOppositeRefWorks() {

        this.createEPackageMultipleMultiplicityManyContainmentsWithMultiplicityOneOpposite();
        EPackage ePackage = this.getEPackageByNsURI("http://www.example.com/model");

        EClass myClass = (EClass) ePackage.getEClassifier("MyEClass");
        EClass yourClass = (EClass) ePackage.getEClassifier("YourEClass");

        assertNotNull(myClass);
        assertNotNull(yourClass);

        EReference eRefChildren = (EReference) myClass.getEStructuralFeature("children");
        EReference eRefOtherChildren = (EReference) myClass.getEStructuralFeature("otherChildren");
        EReference eRefParent = (EReference) yourClass.getEStructuralFeature("parent");
        EReference eRefOtherParent = (EReference) yourClass.getEStructuralFeature("otherParent");

        assertNotNull(eRefChildren);
        assertNotNull(eRefOtherChildren);
        assertNotNull(eRefParent);
        assertNotNull(eRefOtherParent);

        assertEquals(eRefParent, eRefChildren.getEOpposite());
        assertEquals(eRefChildren, eRefParent.getEOpposite());
        assertEquals(eRefOtherChildren, eRefOtherParent.getEOpposite());

        assertTrue(eRefChildren.isContainment());
        assertTrue(eRefOtherChildren.isContainment());
        assertTrue(eRefParent.isContainer());
        assertTrue(eRefOtherParent.isContainer());

        EObject container = this.createEObject(myClass);
        EObject child = this.createEObject(yourClass);

        // add a child to the container by adding it to the children reference
        ((List<EObject>) container.eGet(eRefChildren)).add(child);

        // add a child to the container by setting the container as parent in the child
        child.eSet(eRefParent, container);

        assertTrue(child.eIsSet(eRefParent));
        assertFalse(child.eIsSet(eRefOtherParent));
    }

    private void createEPackageMultipleMultiplicityManyContainmentsWithMultiplicityOneOpposite() {
        EPackage ePackage = this.createNewEPackage("MyEPackage", "http://www.example.com/model", "model");
        ePackage.setName("MyPackage");

        EClass eClass = EcoreFactory.eINSTANCE.createEClass();
        eClass.setName("MyEClass");

        EClass eOtherClass = EcoreFactory.eINSTANCE.createEClass();
        eOtherClass.setName("YourEClass");

        EReference eRefChildren = EcoreFactory.eINSTANCE.createEReference();
        eRefChildren.setName("children");
        eRefChildren.setEType(eOtherClass);
        eRefChildren.setLowerBound(0);
        eRefChildren.setUpperBound(-1);
        eRefChildren.setContainment(true);
        eClass.getEStructuralFeatures().add(eRefChildren);

        EReference eRefOtherChildren = EcoreFactory.eINSTANCE.createEReference();
        eRefOtherChildren.setName("otherChildren");
        eRefOtherChildren.setEType(eOtherClass);
        eRefOtherChildren.setLowerBound(0);
        eRefOtherChildren.setUpperBound(-1);
        eRefOtherChildren.setContainment(true);
        eClass.getEStructuralFeatures().add(eRefOtherChildren);

        EReference eRefParent = EcoreFactory.eINSTANCE.createEReference();
        eRefParent.setName("parent");
        eRefParent.setLowerBound(0);
        eRefParent.setUpperBound(1);
        eRefParent.setEType(eClass);
        eRefParent.setEOpposite(eRefChildren);
        eRefChildren.setEOpposite(eRefParent);
        eOtherClass.getEStructuralFeatures().add(eRefParent);

        EReference eRefOtherParent = EcoreFactory.eINSTANCE.createEReference();
        eRefOtherParent.setName("otherParent");
        eRefOtherParent.setLowerBound(0);
        eRefOtherParent.setUpperBound(1);
        eRefOtherParent.setEType(eClass);
        eRefOtherParent.setEOpposite(eRefOtherChildren);
        eRefOtherChildren.setEOpposite(eRefOtherParent);
        eOtherClass.getEStructuralFeatures().add(eRefOtherParent);

        ePackage.getEClassifiers().add(eClass);
        ePackage.getEClassifiers().add(eOtherClass);
        this.registerEPackages(ePackage);
    }

}

[MartinHaeusler]

Hello,

thank you for your efforts and the detailed report!

As you can tell from the commit history, this project has been inactive for quite some time. Wile it remains close to my heart, I currently don't have the time to work on it. However, I would be willing to review pull requests.

[cdpm]

Hi, thank you for your quick reply, I opened a pull request: #5 .

[MartinHaeusler]

@cdpm thank you for your contribution, I just merged your Pull Request!