Community
Participate
Working Groups
Build Identifier: I found that when unset() method is called on delegateEList created in EStoreEcoreEMap (nested in CDOObjectImpl.createList()), BasicEMap.this.doClear() is not called which in certain circumstances leads to invalid behaviour (described below). BasicEMap.this.doClear() should probably be called either directly or through one of the methods that call it (possibly clear()). Observed behaviour: EcoreEMap.set(), when you pass a map to it, is implemented basically in two steps: 1. ((EStructuralFeature.Setting)delegateEList).unset(); 2. putAll(mapValue); When calling EcoreEMap.set() for the second time, unset() will clear delegateEList, but leave BasicEMap.entryData full of whatever entries were created during first call to EcoreEMap.set(). Thus when calling putAll(), BasicEMap.put() is fooled into thinking that entries added during first call to EcoreEMap.set() are already in the map and doesn't add them to delegateEList. This is hard to spot at first because most methods on EcoreEmap use entryData to return information, but when you try to persist the map as part of an object, the map will not be persisted correctly. Note that this issue is specific to CDO and doesn't occur in plain EMF. Also while calling EcoreEMap.set() instead of EcoreEMap.put() may seem rare, that's exactly what happens, when you use EMF Databinding (see org.eclipse.emf.databinding.internal.EMFMapProperty.doSetMap()). Reproducible: Always
Created attachment 187910 [details] Test case Attached test case tests both replacing existing entries and adding new ones. Two methods test what happens to delegateEList after these operations, the other two test what happens when you try to persist the map and then read it back.
Could someone please take a look at it? I'm trying to make maps work well with CDO. I believe that for someone who knows this code it will be easier to find the correct fix than it was for me to identify the problem and prepare the test case. Thanks in advance!
Created attachment 188656 [details] Patch v1 Please see how I overwrite unset() now. this makes the first two tests pass. The last two tests fail with a NPE in the test logic, with or without the fix. Is that a bad test logic or an indication that the fix is not complete?
Michal, please confirm that: 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.
Created attachment 188672 [details] Patch v2 > The last two tests fail with a NPE in the test logic, with or without the fix. > Is that a bad test logic or an indication that the fix is not complete? It was a bad test logic. I assumed I'll be able to retrieve value from the map, using key that was used when I added it to the map, but that doesn't work because the two keys are different objects and equals() returns false for them. I now enumerate the contents of the map manually and compare cdoId() of the keys. See attached corrected version.
> Michal, please confirm that: > 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. I confirm all three points.
Created attachment 188673 [details] Patch v3 Actually I should probably use CDOUtil.getCDOObject instead of casting to Product1Impl.
Committed revision 7046: - trunk/plugins/org.eclipse.emf.cdo - trunk/plugins/org.eclipse.emf.cdo.tests
Thank you, Michal ;-)
Available in R20110608-1407
