eclipse-ocl / org.eclipse.ocl

Eclipse Public License 2.0
0 stars 0 forks source link

[cg] Deleting elements affected by derived references not working #1594

Open eclipse-ocl-bot opened 5 hours ago

eclipse-ocl-bot commented 5 hours ago

| --- | --- | | Bugzilla Link | 474307 | | Status | NEW | | Importance | P3 normal | | Reported | Aug 05, 2015 06:12 EDT | | Modified | Aug 05, 2015 18:32 EDT | | Version | 6.0.0 | | Reporter | Matthias Freund |

Description

When making use of derived references specified via OCL and generating Java code (instead of delegating for interpretation at runtime), deleting elements that are affected by such a derived reference is not possible and leads to NullPointerExceptions in the generated 'DeleteCommand'.

Steps to reproduce:

  1. Use the OCLinEcore Tutorial example
  2. Set 'Realisation of OCL embedded within Ecore models' OCL preference to 'Generate Java Code in *Impl classes'
  3. Generate the model code and spawn a runtime-Eclipse
  4. Open the sample model (Tutorial.xmi)
  5. Delete either of the 3 Loans or Book 'b2'\ -> There is a NullPointerException in the created DeleteCommand so that the element is not deleted properly.

Possible source of the error and fix:

As proposed by Ed Merks (https://www.eclipse.org/forums/index.php?t=msg&th=133405&goto=416076&#msg_416076), it might be nessecary to return an 'UnmodifiableEList' as result of the generated implementation of the derived references. Currently, simple Lists/ELists are returned.\ Consequently, the problem does not arise if you manually change the generated code e.g. of 'getBooks()' from:

...\ return (EList)UNBOXED_collect;

to

...\ return new EcoreEList.UnmodifiableEList.FastCompare \ (this, TutorialPackage.Literals.MEMBER__BOOKS, UNBOXED_collect.size(), UNBOXED_collect.toArray());

eclipse-ocl-bot commented 5 hours ago

By Ed Willink on Aug 05, 2015 11:03

You don't identify what version of OCL you are using and do not show the exception trace.

For Neon M1 (Bug 458359), the return has already changed to an EcoreEList.UnmodifiableEList, but constructed as:

new EcoreEList.UnmodifiableEList(null, null, size, data)

which also gives an NPE for the null EStructuralFeature. The NPE goes away if the two null arguments have useful values. It doesn't look too hard to pass the extra context through to the constructor.

eclipse-ocl-bot commented 5 hours ago

By Matthias Freund on Aug 05, 2015 11:15

Sorry, I forgot about the version. I'm using the current Mars release (6.0.0).

eclipse-ocl-bot commented 5 hours ago

By Ed Willink on Aug 05, 2015 14:11

(In reply to Ed Willink from comment #1)

For Neon M1 (Bug 458359), the return has already changed to an EcoreEList.UnmodifiableEList, but constructed as:

new EcoreEList.UnmodifiableEList(null, null, size, data)

For Mars too it is an EcoreEList.UnmodifiableEList with the same null, null initial arguments.

eclipse-ocl-bot commented 5 hours ago

By Ed Willink on Aug 05, 2015 14:27

Generating

final List UNBOXED_collect = collect.asEcoreObjects(idResolver, \ tutorial.Book.class, this, TutorialPackage.Literals.MEMBER__BOOKS);

rather than

final List UNBOXED_collect = collect.asEcoreObjects(idResolver, \ tutorial.Book.class)

could fix this, but making it backward and forward API compatible will be horrible. For a maintenance release it will have to create another EcoreEList.UnmodifiableEList as suggested in the original report.

eclipse-ocl-bot commented 5 hours ago

By Ed Willink on Aug 05, 2015 15:25

Much simpler solution.

The problem is that DeleteCommand invokes isChangeable() on the list's getEStructuralFeature() which is null. So make sure the list has an EStructuralFeature that returns false for isChangeable().

This avoids plumbing e.g. TutorialPackage.Literals.MEMBER__BOOKS through to asEcoreObjects, particularly since in some call scenarios there may be no genuine EStructuralFeature to use, just a bogus one to search for and re-use. A static really bogus one is much much simpler. No regeneration required by users.

eclipse-ocl-bot commented 5 hours ago

By Ed Willink on Aug 05, 2015 16:59

(In reply to Ed Willink from comment #5)

Much simpler solution.

commit 8797a35d86a730c337c7b74357e42427f4afd1e4 pushed to maintenance/R6_0 for SR1

(In reply to Ed Willink from comment #5)

A static really bogus one is much much simpler.

This is an example of needing to comply with undocumented Ecore APIs. Nowhere is the need for ELists to implement EStructuralFeature.Setting documented, and getEStructrualFeature is not documented as @NonNull.

So going forward we need to ensure the rewriteAsEcore plumbs the extra context through when possible.

eclipse-ocl-bot commented 5 hours ago

By Ed Willink on Aug 05, 2015 18:32

(In reply to Ed Willink from comment #6)

(In reply to Ed Willink from comment #5)

Much simpler solution.

commit ec525e04a8e2692cbe25232215318e40dbe963b0 pushed to master for M1

So going forward we need to ensure the rewriteAsEcore plumbs the extra context through when possible.

Still to do. Bug stays open.