SimpleModelManipulations.remove(ModelObject object) does not work as intended

[eclipse-viatra-bot]

| --- | --- | | Bugzilla Link | 567920 | | Status | NEW | | Importance | P3 normal | | Reported | Oct 16, 2020 04:17 EDT | | Modified | Nov 10, 2020 18:00 EDT | | Version | 2.3.2 | | Reporter | Hans van der Laan |

Description

Hey,

Perhaps I’m reading the documentation wrong, however it seems as the SimpleModelManipulations.remove(ModelObject object) method does not work as intended.

The IModelManipulations interface describes the method as follows:

/**

• Removes an object from the model, along with all contained objects, and any incoming or outgoing references.\ */\ void remove(ModelObject object) throws ModelManipulationException;

With all other remove methods, it is indicated that “(it is assumed that no dangling cross-references point to it).”. Since this is not indicated for this method, one would assume that this method can handle cross-references.\ However, consider that I have a simple model with user, roles and permissions. users are assign roles and roles are assigned permissions. users, roles and permissions are contained in a “policy” object.

This gives us the following model:\ [U] <-UR -> [R] <- RP -> [P]

All relations, UR and RP also have an inverse called RU and PR and U, R and P are contained in a policy object (omitted in the visualization).

Now consider that we remove a role. You would think, given the method documentation, that references to this role in UR, RU, RP and PR would be removed. This is not the case. The role is removed from the containment relation, but still “lives” in the other references UR and PR.

I always thought this was intended, however I’m currently trying to optimize my queries results and on a reread of the method documents I spotted this.

[eclipse-viatra-bot]

By Hans van der Laan on Nov 09, 2020 10:35

Hey,

Could anyone confirm is this is a bug or intended behavior?

Kind regards,

Hans

[eclipse-viatra-bot]

By Abel Hegedus on Nov 09, 2020 12:28

SimpleModelManipulations.remove(ModelObject object) delgates to EcoreUtil.remove ( https://download.eclipse.org/modeling/emf/emf/javadoc/2.5.0/org/eclipse/emf/ecore/util/EcoreUtil.html#remove(org.eclipse.emf.ecore.EObject) ) that specifically says it "Removes the object from its containing resource and/or its containing object" which will behave as you described.

I think the interface documentation is incorrect, though may be just incomplete: \ "Removes an object from the model, along with all contained objects, and any incoming or outgoing references [between contained objects]."

[eclipse-viatra-bot]

By Gabor Bergmann on Nov 09, 2020 12:39

I think the interface documentation is correct in the sense that it documents the intended semantics. Or at least what I believe was intended.

The implementation, though, has a bug indeed: it should have called EcoreUtil.delete(object, true) to (a) remove incoming references, and (b) trigger recursive deletion of contained object.

[eclipse-viatra-bot]

By Hans van der Laan on Nov 10, 2020 03:01

(In reply to Gabor Bergmann from comment #3)

I think the interface documentation is correct in the sense that it documents the intended semantics. Or at least what I believe was intended.

The implementation, though, has a bug indeed: it should have called EcoreUtil.delete(object, true) to (a) remove incoming references, and (b) trigger recursive deletion of contained object.

Thanks for the feedback!

There is one use-case where removing an element takes significantly longer then reinitializing the whole network as I have to make many small deletions. I'm going to try to call this method directly and see if this improves performance.

[eclipse-viatra-bot]

By Abel Hegedus on Nov 10, 2020 18:00

Note that as far as I can remember, EcoreUtile.delete(object, true) can be very slow on larger models due to it having to find all cross references to the deleted objects (and all transitively contained objects). I think this is done by traversing the whole resource set at least once. It can also be memory intensive (building CrossReferencer caches).