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

Bug 335675

Summary:

CDOClassInfoImpl broken for EClasses with transient features

Product:

[Modeling] EMF

Reporter:

Caspar D. <caspar_d>

Component:

cdo.core

Assignee:

Caspar D. <caspar_d>

Status:

CLOSED FIXED

QA Contact:

Eike Stepper <stepper>

Severity:

normal

Priority:

CC:

saulius.tvarijonas

Version:

4.0

Flags:

stepper: review+

Target Milestone:

---

Hardware:

All

OS:

All

Whiteboard:

Attachments:

Description	Flags
Patch (including testcase)	none
Enables EMF notifications on transaction.rollback()	stepper: iplog+
Patch v2 - ready to be committed	none

Description Caspar D.

2011-01-28 06:18:26 EST

The way the featureIDs is constructed and populated is
incorrect. The length of the array must be equal to
the number of ALL features, not just the persistent
features, and it should be

Currently, if one calls CDOClassInfoImpl.getFeatureIndex
with either a transient feature or its featureID as the
argument, one of 2 things will happen:

1) If the transient feat is not the last feat of the EClass,
no error will be detected, but 0 will be returned -- 
simply the default init value for every element in an int[]

2) if the transient feat is the last feature of the
EClass, an ArrayIndexOutOfBoundsException will occur,
which gets converted to an IllegalStateException with
the message that the feature is not mapped. This is
true, but the mechanism only works this way if indeed
the featureID is beyond the size of the array.

Patch forthcoming.

Comment 1 Caspar D.

2011-01-28 06:30:13 EST

Created attachment 187829 [details]
Patch (including testcase)

Comment 2 Caspar D.

2011-01-28 06:34:14 EST

(In reply to comment #0)
> The length of the array must be equal to
> the number of ALL features, not just the persistent
> features, and it should be

Right... I see I left off in mid-sentence. Well, what I
meant was:

... it should be initialized with some marker values so
that unmapped features can be recognized.

After weighing many, many options, such as -29, -11,
-43, and other prime numbers, I decided that the marker
value shall be.......... -1 !!!

See patch, & have a good weekend.

Comment 3 Erwin Betschart

2011-01-28 08:49:46 EST

Created attachment 187838 [details]
Enables EMF notifications on transaction.rollback()

Updated patch (187818).
Does now only a single load revision call to the server.

I confirm to
1) The number of lines that you changed is smaller than 250.
2) You are the only author of these changed lines.
3) You apply the EPL to these changed lines.

Comment 4 Eike Stepper

2011-01-29 11:33:48 EST

(In reply to comment #2)
> After weighing many, many options, such as -29, -11,
> -43, 

Primes are over-appreciated, why not -42 ?! :P

Comment 5 Eike Stepper

2011-01-29 11:54:11 EST

Created attachment 187909 [details]
Patch v2 - ready to be committed

Just some minor changes:

I introduced a symbolic constant for the marker: int NOT_MAPPED = -1
I changed the exception message: "Feature not mapped: " + getEClass().getEStructuralFeature(featureID)
I changed the array initialization: Arrays.fill(featureIDMappings, NOT_MAPPED)

Comment 6 Caspar D.

2011-01-30 21:33:01 EST

(In reply to comment #5)
> I changed the array initialization: Arrays.fill(featureIDMappings, NOT_MAPPED)

Amazing, I was wondering whether Java had something like that :-)

Committed to trunk, r. 6982.

Comment 7 Eike Stepper

2011-06-23 03:39:38 EDT

Available in R20110608-1407