Bugzilla – Full Text Bug Listing
| Summary: | Transparently support legacy models (CDOLegacyAdapter) | ||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Modeling] EMF | Reporter: | Eike Stepper <stepper> | ||||||||||||||||||||||||||||||||||
| Component: | cdo.legacy | Assignee: | Martin Fluegge <martin.fluegge> | ||||||||||||||||||||||||||||||||||
| Status: | CLOSED FIXED | QA Contact: | Eike Stepper <stepper> | ||||||||||||||||||||||||||||||||||
| Severity: | enhancement | ||||||||||||||||||||||||||||||||||||
| Priority: | P2 | CC: | erche, hjoensson, joachim.rietz, mikael.barbero, smcduff, stefan, taifun.oreilly, Thomas.M.Crockett, vroldanbet | ||||||||||||||||||||||||||||||||||
| Version: | 3.0 | Flags: | stepper:
documentation+
stepper: galileo+ stepper: helios+ stepper: review+ |
||||||||||||||||||||||||||||||||||
| Target Milestone: | --- | ||||||||||||||||||||||||||||||||||||
| Hardware: | All | ||||||||||||||||||||||||||||||||||||
| OS: | All | ||||||||||||||||||||||||||||||||||||
| Whiteboard: | Appealing to a Broader Community | ||||||||||||||||||||||||||||||||||||
| Bug Depends on: | 247130 | ||||||||||||||||||||||||||||||||||||
| Bug Blocks: | 237067, 247227 | ||||||||||||||||||||||||||||||||||||
| Attachments: |
|
||||||||||||||||||||||||||||||||||||
|
Description
Eike Stepper
Created attachment 112514 [details]
Patch v1
Removal of all weaver/conversion related stuff.
Addition of the CDOLegacyWrapper based on the new EMF pre-notifications.
Simon, I think I have fixed the containment mess in CDOLegacyWrapper. Can you please verify it? I removed all evidence of CDOID.LEGACY. I think proxying and resolving works now in principle. But there's an issue with inverse updates: java.lang.UnsupportedOperationException: eInverseRemove at org.eclipse.emf.internal.cdo.CDOLegacyWrapper$LegacyProxyInvocationHandler.invoke(CDOLegacyWrapper.java:740) at org.eclipse.emf.internal.cdo.$Proxy4.eInverseRemove(Unknown Source) at org.eclipse.emf.ecore.util.EcoreEList.inverseRemove(EcoreEList.java:329) at org.eclipse.emf.common.notify.impl.NotifyingListImpl.setUnique(NotifyingListImpl.java:1234) at org.eclipse.emf.common.util.BasicEList.set(BasicEList.java:584) at org.eclipse.emf.ecore.util.DelegatingInternalEList.set(DelegatingInternalEList.java:364) at org.eclipse.emf.internal.cdo.CDOLegacyWrapper.resolveProxies(CDOLegacyWrapper.java:600) at org.eclipse.emf.internal.cdo.CDOLegacyWrapper.resolveAllProxies(CDOLegacyWrapper.java:573) at org.eclipse.emf.internal.cdo.CDOLegacyWrapper.handleRead(CDOLegacyWrapper.java:200) at org.eclipse.emf.ecore.impl.BasicEObjectImpl.eFireRead(BasicEObjectImpl.java:2014) at org.eclipse.emf.ecore.util.DelegatingInternalEList.readAccess(DelegatingInternalEList.java:41) at org.eclipse.emf.ecore.util.DelegatingInternalEList.size(DelegatingInternalEList.java:370) at org.eclipse.emf.cdo.tests.LegacyTest.testReferences(LegacyTest.java:117) Just a test to see if the Bugzilla web client is smarter for long lines: java.lang.UnsupportedOperationException: eInverseRemove at org.eclipse.emf.internal.cdo.CDOLegacyWrapper$LegacyProxyInvocationHandler.invoke(CDOLegacyWrapper.java:740) at org.eclipse.emf.internal.cdo.$Proxy4.eInverseRemove(Unknown Source) at org.eclipse.emf.ecore.util.EcoreEList.inverseRemove(EcoreEList.java:329) at org.eclipse.emf.common.notify.impl.NotifyingListImpl.setUnique(NotifyingListImpl.java:1234) at org.eclipse.emf.common.util.BasicEList.set(BasicEList.java:584) at org.eclipse.emf.ecore.util.DelegatingInternalEList.set(DelegatingInternalEList.java:364) at org.eclipse.emf.internal.cdo.CDOLegacyWrapper.resolveProxies(CDOLegacyWrapper.java:600) at org.eclipse.emf.internal.cdo.CDOLegacyWrapper.resolveAllProxies(CDOLegacyWrapper.java:573) at org.eclipse.emf.internal.cdo.CDOLegacyWrapper.handleRead(CDOLegacyWrapper.java:200) at org.eclipse.emf.ecore.impl.BasicEObjectImpl.eFireRead(BasicEObjectImpl.java:2014) at org.eclipse.emf.ecore.util.DelegatingInternalEList.readAccess(DelegatingInternalEList.java:41) at org.eclipse.emf.ecore.util.DelegatingInternalEList.size(DelegatingInternalEList.java:370) at org.eclipse.emf.cdo.tests.LegacyTest.testReferences(LegacyTest.java:117) 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:585) at junit.framework.TestCase.runTest(TestCase.java:164) at org.eclipse.net4j.tests.AbstractOMTest.runTest(AbstractOMTest.java:69) at junit.framework.TestCase.runBare(TestCase.java:130) at junit.framework.TestResult$1.protect(TestResult.java:106) at junit.framework.TestResult.runProtected(TestResult.java:124) at junit.framework.TestResult.run(TestResult.java:109) at junit.framework.TestCase.run(TestCase.java:120) at junit.framework.TestSuite.runTest(TestSuite.java:230) at junit.framework.TestSuite.run(TestSuite.java:225) 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:460) at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:673) at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:386) at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:196) Not really ;-( All tests are passing!!! ;-)
Though with a dirty trick:
// TODO Is InternalEList.basicSet() needed???
if (list instanceof DelegatingInternalEList)
{
list = ((DelegatingInternalEList)list).getDelegateInternalEList();
}
if (list instanceof NotifyingListImpl)
{
((NotifyingListImpl)list).basicSet(i, instance, null);
}
else
{
list.set(i, instance);
}
To make this work at least without reflection I needed to make getDelegateInternalEList() public!
InternalEList.basicSet(int, E) would be nicer ;-)
Committed to HEAD. Whether the feature can work or not depends on the availablity of the fix to bug #247130. Normal native models are not affected by that. Only the workspace would have some compile errors tehn. All tests are now able to run with native orlegacy test models. See AbstractCDOTest.legacyTesting... Created attachment 112894 [details] EMF Patch v1 This one slightly differs from the one in bug #247130 It seems you renamed the setSelfPopulatingPackageRegistry and the other method. Could you please update http://wiki.eclipse.org/Run_a_CDO_container_inside_eclipse_runtime to reflect the change? Thanx ;-) Done. Created attachment 134866 [details]
Contains UMLTest.launch
Created attachment 151009 [details]
LegacyAdapter Patch
Hi,
I attached a patch to make the legacy adapter mode work. Not yet perfect but workes for the simple library example (Lib, Books, Writers) and for parts auf the GMF notaional model (Diagram, Node, Bounds, ShapeStyle, DiagramStyle ).
Cheers,
Martin
Created attachment 151634 [details]
LegacyAdapter Patch v2
Reformatted. Please ping me that we can discuss...
Commited to HEAD. Well done! Created attachment 151651 [details]
LegacyTests v1
Created attachment 152232 [details]
Legacy Tests v2
I attached another patch for the creation of legacy test models. This converts the ?model1?. I only changed the interfaces of the lecagy implementation to the native interface and adjusted some stuff (mostly concerning enums) in the implementations. I think this approach is easier and much cleaner because now we can use different URIs for the packages.
I think this will be important for getting a complete test coverage. I think that in addition to legacy and native mode we will need some sort of a mixed-mode where legacy and native are tested combined.
This patch also contains a change in the CDOResource.
?View.getResourceSet().getEObject(uri, true);? always returned the CDOWrapper and not the instance. I think this should be the cdoInternalInstance.
A similar problem occurs with ?View.getObject(cdoid, true);?. In this case returning of the Wrapper seems o.k. But I am not absolutely sure. If this is o.k. the test-cases should be aware of LegacyWrappers.
We should discuss this. ;)
Cheers,
Martin
Created attachment 153867 [details]
LegacyAdapter Patch V3
I fixed a bug in the CDOLegacyWrapper. The previous version did not set the opposite of a bi-directional reference if it was a transient one. Now transient features will also be adjusted.
Created attachment 153868 [details]
mylyn/context/zip
Created attachment 155030 [details]
LegacyAdapter Patch v4 - ready to be committed
Created attachment 155031 [details]
Legacy Tests v3 - ready to be committed
Please do not send patches for review that create compile errors! See ModelConfig, which I fixed in v3.
The Legacy tests do not pass. E.g. there's an NPE in getAnnotation(), caused by a CDOInvalidationNotification with no feature set...
The normal test suite is not affected.
Martin, please do not attach multiple patches with overlapping changes, like CDOLegacyWrapper, which is part of both patches. Committed to HEAD and sorry for the patch confusion... Created attachment 155181 [details]
LegacyAdapter Patch v5
After being frustrated that only ~120 (out of 530) existing tests were passig in legacy mode I spend some time on converting the legacy test models and fixing some bug.
Now 476 tests are passing when running all tests in legacy mode. :)
Some failures and error rely on feature which are not yet implemented in legacy mode (e.g. locking)
This patch covers the changes in the Legagy wrapper. The patch for the patches will be applied in a minute.
Code is preformatted ;)
Created attachment 155182 [details]
Legacy Tests Patch v4
And here is the patch for the tests. Legacy test are still separated from the "AutomatedTests". All valid tests will be executed by running the AllTestsLegacy.java. This also gives an overview about the tests which are still failing (removed from the test execution)
Created attachment 155400 [details]
LegacyAdapter Patch v6 - ready to be committed
Martin, on occasion please revisit the TRACER.format() calls, they expect a format pattern string plus arguments. If you prefer the string concatenation please use TRACER.trace().
The tests patch looks okay, too. BUT you need to regenerate model4 because I just committed some changes to it. Regeneration must always be followed by "Organize Imports" and "Format Sources" of the whole project! After that you may commit everything ;-) Committed to HEAD. Committed to HEAD (again). sorry, I uploaded Legacy Path v5 and not the v6. Now the extended version is checked in. How is this coming along? This is a much-anticipated feature for us. Hi Tom, we expect to finish the legacy mode in no more than 2-3 month. Please note that legacy will have some drawbacks compared to the native approach: - not designed for scalability - due to it?s nature it is a bit slower than the native approach. - It has a higher memory footprint. The footprint is even higher than the one of pure EMF objects Created attachment 163497 [details]
LegacyAdapter Patch v7 - ready to be committed
Created attachment 163499 [details]
LegacyAdapter Patch v7 - ready to be committed
Previous patch was corrupted!
Committed to HEAD. Legacy Mode is now released. Example:
// Enable legacy support from now on for all views that will be opened by this thread
CDOUtil.setLegacyModeDefault(true);
CDOView view = session.openView();
System.out.println("Legacy model support: " + view.isLegacyModeEnabled());
Available in 3.0 GA: http://download.eclipse.org/modeling/emf/cdo/updates/3.0-releases/ |