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

Bug 359992

Summary: [Legacy] CDODeltaNotification.getNewValue() returns a CDOLegacyWrapper instead of the wrapped EObject
Product: [Modeling] EMF Reporter: Esteban DUGUEPEROUX <esteban.dugueperoux>
Component: cdo.legacyAssignee: Christian Damus <give.a.damus>
Status: REOPENED --- QA Contact: Eike Stepper <stepper>
Severity: normal    
Priority: P3 CC: steve.monnier
Version: 4.13Flags: stepper: review+
Target Milestone: ---   
Hardware: PC   
OS: Linux   
See Also: https://bugs.eclipse.org/bugs/show_bug.cgi?id=355915
Whiteboard:
Attachments:
Description Flags
A fix which call getEObject() to get the wrapped object
none
second release of the fix to fix getOldValue()
none
A patch of the Bugzilla_355915_Test to test these cases
none
The complete test case
none
Test v1
none
Test v1
none
Patch v1
none
Patch v2 (incl. test) none

Description Esteban DUGUEPEROUX CLA 2011-10-05 10:19:31 EDT 
Build Identifier: CDO Model Repository SDK	4.0.0.v20110831-1303	org.eclipse.emf.cdo.sdk.feature.group

In legacy mode, with CDOTransaction.options().addChangeSubscriptionPolicy(CDOAdapterPolicy.ALL) set, on commit of a new legacy object added, the remote clients receives a CDODeltaNotification with newValue instanceof CDOLegacyWrapper

Reproducible: Always
Comment 1 Esteban DUGUEPEROUX CLA 2011-10-05 10:22:14 EDT 
Created attachment 204596 [details]
A fix which call getEObject() to get the wrapped object
Comment 2 Esteban DUGUEPEROUX CLA 2011-10-05 11:13:04 EDT 
Created attachment 204603 [details]
second release of the fix to fix getOldValue()
Comment 3 Esteban DUGUEPEROUX CLA 2011-10-05 11:13:35 EDT 
Created attachment 204604 [details]
A patch of the Bugzilla_355915_Test to test these cases
Comment 4 Martin Fluegge CLA 2011-10-05 14:21:44 EDT 
Hi Esteban,

thanks for the patch. But somehow it seems to be outdated. I cannot apply it neither non head nor on the maintenance branch. Could you please update it?

Thanks.
Comment 5 Martin Fluegge CLA 2011-10-05 14:29:22 EDT 
Just to avoid confusions: I meant the test, not the fix ;)
Comment 6 Esteban DUGUEPEROUX CLA 2011-10-06 02:45:06 EDT 
Created attachment 204651 [details]
The complete test case

Hi,

I hava attached the complete test case to avoid conflict on patch application.
Comment 7 Martin Fluegge CLA 2011-10-09 05:20:19 EDT 
Created attachment 204824 [details]
Test v1

Hi Esteban,

thanks for the test. I attached it as patch and renamed it to make align it with the bug number.
Comment 8 Martin Fluegge CLA 2011-10-09 05:21:33 EDT 
Created attachment 204825 [details]
Test v1

Mixed test and fix. This is the test.
Comment 9 Martin Fluegge CLA 2011-10-09 05:22:35 EDT 
Created attachment 204826 [details]
Patch v1

And here comes the patch. I changed it a bit and moved the conversion to the adapt method.
Comment 10 Eike Stepper CLA 2011-10-09 06:24:49 EDT 
Assuming 4.1
Comment 11 Martin Fluegge CLA 2011-10-09 06:40:59 EDT 
Correct assumption. I'll provide a bug for 4.0 maintenance as soon as HEAD is fixed.
Comment 12 Eike Stepper CLA 2011-10-09 06:46:12 EDT 
Created attachment 204831 [details]
Patch v2 (incl. test)

I removed the reference to the test for bug 359669 that does not compile. Executing tests...
Comment 13 Eike Stepper CLA 2011-10-09 06:52:46 EDT 
All tests pass. Good work!
Comment 14 Martin Fluegge CLA 2011-10-09 07:20:24 EDT 
Committed revision 9467
Comment 15 Eike Stepper CLA 2011-10-09 08:53:57 EDT 
And revision 9468 ;-)
Comment 16 Eike Stepper CLA 2011-10-12 23:41:00 EDT 
Hudson tests keep failing (in legacy scenarios) with:

junit.framework.AssertionFailedError: expected:<true> but was:<false>
	at junit.framework.Assert.fail(Assert.java:47)
	at junit.framework.Assert.failNotEquals(Assert.java:283)
	at junit.framework.Assert.assertEquals(Assert.java:64)
	at junit.framework.Assert.assertEquals(Assert.java:143)
	at junit.framework.Assert.assertEquals(Assert.java:149)
	at org.eclipse.emf.cdo.tests.bugzilla.Bugzilla_359992_Test.testDeltaNotification(Bugzilla_359992_Test.java:140)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
	at java.lang.reflect.Method.invoke(Method.java:597)
	at junit.framework.TestCase.runTest(TestCase.java:168)
	at org.eclipse.net4j.util.tests.AbstractOMTest.runBare(AbstractOMTest.java:218)
	at org.eclipse.emf.cdo.tests.config.impl.ConfigTest.runBare(ConfigTest.java:526)
	at junit.framework.TestResult$1.protect(TestResult.java:110)
	at junit.framework.TestResult.runProtected(TestResult.java:128)
	at junit.framework.TestResult.run(TestResult.java:113)
	at junit.framework.TestCase.run(TestCase.java:124)
	at org.eclipse.net4j.util.tests.AbstractOMTest.run(AbstractOMTest.java:264)
	at junit.framework.TestSuite.runTest(TestSuite.java:243)
	at org.eclipse.emf.cdo.tests.config.impl.ConfigTestSuite$TestWrapper.runTest(ConfigTestSuite.java:119)
	at junit.framework.TestSuite.run(TestSuite.java:238)
	at junit.framework.TestSuite.runTest(TestSuite.java:243)
	at junit.framework.TestSuite.run(TestSuite.java:238)
	at junit.framework.TestSuite.runTest(TestSuite.java:243)
	at junit.framework.TestSuite.run(TestSuite.java:238)
	at org.eclipse.jdt.internal.junit.runner.junit3.JUnit3TestReference.run(JUnit3TestReference.java:130)
	at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:38)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:467)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:683)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:390)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:197)

Please investigate
Comment 17 Martin Fluegge CLA 2011-10-13 03:14:09 EDT 
Eike,

the patch was committed on 9th october and build #1884 passed on 10th. As far as I remember this test failed several times before, so I do not think that there is a direct relationship between the test failure in this bug. 

I think we should open a new bug for this issue as it seems that legacy might have a general problem with delta notification or worse: There is a timing issue related to delta notifications.
Comment 18 Eike Stepper CLA 2011-10-13 03:32:25 EDT 
I'm not sure about that. The test outcome depends on timing (timeouts) and that's always a little indeterministic.
Comment 19 Martin Fluegge CLA 2011-10-14 05:22:44 EDT 
Ah. I did not recognize that the failing build was due to the test of this bug. I thought it was a test method from another test.

It might be a small mistake in the test logic (Should block until the notifier was notified).
Comment 20 Eike Stepper CLA 2012-08-14 22:57:04 EDT 
Moving all open issues to 4.2. Open bugs can be ported to 4.1 maintenance after they've been fixed in master.
Comment 21 Eike Stepper CLA 2013-06-03 05:54:11 EDT 
Hi Christian, I assign this legacy mode zilla to you, just in case you're ineterested. Don't feel obliged ;-)
Comment 22 Eike Stepper CLA 2013-06-29 12:18:27 EDT 
We'll try to address open problems in 4.3 (master) first and then port fixes back to 4.2.
Comment 23 Eike Stepper CLA 2015-07-14 02:20:29 EDT 
Moving all open bugzillas to 4.5.
Comment 24 Eike Stepper CLA 2016-07-31 01:03:22 EDT 
Moving all unaddressed bugzillas to 4.6.
Comment 25 Eike Stepper CLA 2017-12-28 01:11:54 EST 
Moving all open bugs to 4.7
Comment 26 Eike Stepper CLA 2019-11-08 02:13:54 EST 
Moving all unresolved issues to version 4.8-
Comment 27 Eike Stepper CLA 2019-12-13 12:43:38 EST 
Moving all unresolved issues to version 4.9
Comment 28 Eike Stepper CLA 2020-12-11 10:38:42 EST 
Moving to 4.13.