Skip to main content

Notice: this Wiki will be going read only early in 2024 and edits will no longer be possible. Please see: https://gitlab.eclipse.org/eclipsefdn/helpdesk/-/wikis/Wiki-shutdown-plan for the plan.

Jump to: navigation, search

Difference between revisions of "MoDisco/MDT Migration"

(Difficulties)
 
(2 intermediate revisions by the same user not shown)
Line 172: Line 172:
 
* For methods that return a collection (list, map, queue, set, etc.), the user can then proceed to call methods on this collection to modify it. And we would expect the underlying collection to be modified accordingly. So, we can't just get away with copying the collection when making a proxy for it. We have to provide a collection proxy that delegates every operation to its delegate collection. And this, for each and every collection that is used in MoDisco (lists, sets, maps, etc.)
 
* For methods that return a collection (list, map, queue, set, etc.), the user can then proceed to call methods on this collection to modify it. And we would expect the underlying collection to be modified accordingly. So, we can't just get away with copying the collection when making a proxy for it. We have to provide a collection proxy that delegates every operation to its delegate collection. And this, for each and every collection that is used in MoDisco (lists, sets, maps, etc.)
 
* Similarly, for collections that are passed as parameters to methods that modify it, we must also create a proxy collection, in the other direction this time.
 
* Similarly, for collections that are passed as parameters to methods that modify it, we must also create a proxy collection, in the other direction this time.
* A sub-class can call protected methods on its super-class. But since these methods are protected, they can't normally be called from the proxy (this can be solved by the use of reflection) [[Image:MoDisco_ProxyCallProtectedMethod.png]]
+
* A sub-class can call protected methods on its super-class. But since these methods are protected, they can't normally be called from the proxy (this can be solved by the use of reflection) [[Image:MoDisco_ProxyCallProtectedMethod.png.png]]
* If the super-class of a MoDisco class is not itself a MoDisco class, then we won't have a proxy for it. This means that method calls won't execute on the right object. So, we have to create a proxy method for every method, including those inherited from super-classes. This is bad for memory use, and we risk PermGen space exceptions.
+
* If the super-class of a MoDisco class is not itself a MoDisco class, then we won't have a proxy for it. This means that method calls won't execute on the right object. So, we have to create a proxy method for every method, including those inherited from super-classes. This is bad for memory use, and we risk PermGen space exceptions. [[Image:MoDisco_SuperclassIsNonMoDiscoClass.png]]
  
 
=== Can not be handled ===
 
=== Can not be handled ===
Line 179: Line 179:
 
We can't have proxies for public fields, so they wouldn't be handled at all.
 
We can't have proxies for public fields, so they wouldn't be handled at all.
  
Number of occurrences of the case in MoDisco source code: '''<to be counted>'''
+
Number of occurrences of the case in MoDisco source code: '''1464'''
  
 
==== Protected fields====
 
==== Protected fields====
 
We can't have proxies for them either, and they can be accessed by sub-classes made by users. So, this wouldn't work properly.
 
We can't have proxies for them either, and they can be accessed by sub-classes made by users. So, this wouldn't work properly.
  
Number of occurrences of the case in MoDisco source code: '''<to be counted>'''
+
Number of occurrences of the case in MoDisco source code: '''5354'''
  
 
==== Implicit knowledge - Generics====
 
==== Implicit knowledge - Generics====
Line 201: Line 201:
 
We can't create a proxy for an array, so we have to return a copy of it with the other type. So, if a method returns an array and the user modifies this array, these modifications won't be propagated to the right array.
 
We can't create a proxy for an array, so we have to return a copy of it with the other type. So, if a method returns an array and the user modifies this array, these modifications won't be propagated to the right array.
  
Number of occurrences of the case in MoDisco source code: '''<to be counted>'''
+
Number of occurrences of the case in MoDisco source code: '''85'''
  
 
==== Equality test ====
 
==== Equality test ====
Line 207: Line 207:
 
As seen above, MoDisco objects with the <code>org.eclipse.modisco</code> namespace can leak through (under the cover of an <code>Object</code> for example). If the user uses the <code>==</code> comparison operator, s/he will get the wrong result when comparing a proxy with an original MoDisco element.
 
As seen above, MoDisco objects with the <code>org.eclipse.modisco</code> namespace can leak through (under the cover of an <code>Object</code> for example). If the user uses the <code>==</code> comparison operator, s/he will get the wrong result when comparing a proxy with an original MoDisco element.
  
Number of occurrences of the case in MoDisco source code: '''<to be counted>'''
+
Number of occurrences of the case in MoDisco source code: '''12215'''
  
 
==== non-MoDisco code that returns MoDisco objects ====
 
==== non-MoDisco code that returns MoDisco objects ====
Line 219: Line 219:
 
Number of occurrences of the problem cannot be determined because this problem is located in non-MoDisco code.
 
Number of occurrences of the problem cannot be determined because this problem is located in non-MoDisco code.
  
= Coding rules to respect to avoid such problems the next times =
+
{{Message|The number of occurrences were counted with this plug-in : [[Image:Org.eclipse.modisco.dev.mdtMigrationCounts.zip]]}}
 +
 
 +
= Coding rules to respect to avoid such problems in the future =

Latest revision as of 11:28, 7 July 2010

Overview

MoDisco moved from Modeling/GMT to Modeling/MDT (see this bug). As part of this migration, we should have renamed all Java packages from org.eclipse.gmt.modisco.* to org.eclipse.modisco.*.

Unfortunately, this means breaking every existing API, which is in contradiction with the API policy that was established for MoDisco. Among other things, this policy states that API cannot be broken without notifying adopters at least one year before that.

This means for MoDisco that we cannot just rename the Java packages, because we still have to support the API that existed in the org.eclipse.gmt.modisco.* namespace.

To reconcile these two contradicting requirements, we explored several solutions, which are presented below.

Possible solutions

duplicate

two parallel sets of plug-ins

Description

To create version 0.9.0 of org.eclipse.modisco, we create a copy of org.eclipse.gmt.modisco 0.8.0. Two sets of plug-ins would be delivered in Indigo :

  • org.eclipse.modisco.*-0.9.0
  • org.eclipse.gmt.modisco.*-0.9.0 with all the API tagged as deprecated.

Problems

This would imply that the installation would be exclusive (org.eclipse.modisco.*-0.9.0 or org.eclipse.gmt.modisco.*-0.9.0 but not both) because if we installed both we would get all the MoDisco features (including UI, extension point, extension, etc) duplicated. The problem with this solution is for adopters that rely on plug-ins with the old namespace, but that want to start developing plug-ins for the latest version of MoDisco. This would be an all-or-nothing solution: either migrate everything to the latest version of MoDisco, or develop new projects with the old version.

Furthermore, it would not be easy to inform users of the existence of an additional set of plug-ins whose purpose is to satisfy the backward compatibility requirement. Amalgamation would only propose a new set of plug-ins.

do it the other way

Description

Since MoDisco keeps its API from one version to the next, we can also create org.eclipse.gmt.modisco plug-ins by automatically renaming all packages from the org.eclipse.modisco namespace. This could be done during each build for example, effectively providing two branches with different namespaces but the exact same feature-set.

Problems

Same problems as for the previous solution.

GMT and MDT bundles in the same set of plug-ins

Description

This strategy implies removing extensions contributing to the UI to avoid UI feature duplications. To maintain the extension points existence we would keep them in the GMT plug-ins.

Problems

This would imply that the elements registered through GMT's extension points would not appear in the MDT UI and API. This is not an acceptable solution.

delegate

Description

Another solution would be to create a "backward compatibility SDK" (with GMT packages) that delegates to the MDT version of MoDisco. The idea looks straightforward: every call to a GMT API are forwarded to the corresponding MDT API.

This could be automated through a refactoring that would create proxies (in the GMT namespace) for each MoDisco API, that delegate to the corresponding methods with the same name, but in the org.eclipse.modisco namespace.

This would look like this:

New API (org.eclipse.modisco namespace) Proxy implementation (org.eclipse.gmt.modisco namespace)
public class A {
  public A() {
    // do some initialization
  }
 
  public void doStuff() {
    // do some stuff
  }
}
public class A {
  private org.eclipse.modisco.A proxy;
 
  public org.eclipse.modisco.A getProxy() {
    return proxy;
  }
 
  public A() {
    // guard to avoid creating 2 proxies 
    // (Java adds super call automatically)
    if (!(this.getClass() == A.class))
      return;
    proxy = new org.eclipse.modisco.A();
  }
 
  public void doStuff() {
    getProxy().doStuff();
  }
}


A proxy knows its delegate, but a delegate doesn't know its proxy. But we need this information to be able to find the proxy that was associated with an element that is returned from a method. So, we have to keep this information somewhere. For example in a HashMap:

New API (org.eclipse.modisco namespace) Proxy implementation (org.eclipse.gmt.modisco namespace)
public class C {
  private int value;
 
  public C() {
    this(123);
  }
 
  public C(int value) {
    this.value = value;
  }
 
  public int getValue() {
    return value;
  }
 
  public void setValue(int value) {
    this.value = value;
  }
}
public class C {
  private org.eclipse.modisco.C proxy;
 
  public org.eclipse.modisco.C getProxy() {
    return proxy;
  }
 
  public C() {
    proxy = new org.eclipse.modisco.C();
    proxies.put(proxy, this);
  }
 
  public C(int value) {
    proxy = new org.eclipse.modisco.C(value);
    proxies.put(proxy, this);
  }
 
  public C(org.eclipse.modisco.C element) {
    this.proxy = element;
  }
 
  public int getValue() {
    return getProxy().getValue();
  }
 
  public void setValue(int value) {
    getProxy().setValue(value);
  }
 
  private static Map<Object, Object> proxies = 
      new HashMap<Object, Object>();
 
  public static C resolveProxy(
        org.eclipse.modisco.C element) {
    C proxy = (C) proxies.get(element);
    if (proxy == null) {
      proxy = new C(element);
      proxies.put(proxy, element);
    }
    return proxy;
  }
}

So far, so good. But in practice, we discovered that this is not always so simple, and there are many roadblocks that prevent this solution from working 100%. Moreover, even getting it to work only for common cases that can work is far from trivial, and requires a fair amount of work.

Difficulties

  • For methods that return a collection (list, map, queue, set, etc.), the user can then proceed to call methods on this collection to modify it. And we would expect the underlying collection to be modified accordingly. So, we can't just get away with copying the collection when making a proxy for it. We have to provide a collection proxy that delegates every operation to its delegate collection. And this, for each and every collection that is used in MoDisco (lists, sets, maps, etc.)
  • Similarly, for collections that are passed as parameters to methods that modify it, we must also create a proxy collection, in the other direction this time.
  • A sub-class can call protected methods on its super-class. But since these methods are protected, they can't normally be called from the proxy (this can be solved by the use of reflection) MoDisco ProxyCallProtectedMethod.png.png
  • If the super-class of a MoDisco class is not itself a MoDisco class, then we won't have a proxy for it. This means that method calls won't execute on the right object. So, we have to create a proxy method for every method, including those inherited from super-classes. This is bad for memory use, and we risk PermGen space exceptions. MoDisco SuperclassIsNonMoDiscoClass.png

Can not be handled

Public fields

We can't have proxies for public fields, so they wouldn't be handled at all.

Number of occurrences of the case in MoDisco source code: 1464

Protected fields

We can't have proxies for them either, and they can be accessed by sub-classes made by users. So, this wouldn't work properly.

Number of occurrences of the case in MoDisco source code: 5354

Implicit knowledge - Generics

Of the return type is List<EObject> for example, they we will create a proxy for List. But the user can cast this to BasicEList<EObject>, knowing that the collection will always be this type. This is bad practice, but it happens anyway, and we wouldn't be able to handle this kind of uses.

Number of occurrences of the case in MoDisco source code: <to be counted>

Implicit knowledge - not well typed elements

If a method returns something like Object, we don't know (statically) whether we should create a proxy for it. For example, when using the IAdaptable interface, the method Object getAdapter(Class adapter) always returns an Object, but since by contract this Object is supposed to be of the type that is passed as a parameter, the user will cast it right away. And that would fail because we haven't made a proxy for this. (In fact, this is even worse, since getAdapter would be passed a Class in the GMT namespace, which it wouldn't know about).

Number of occurrences of the case in MoDisco source code: <to be counted>

Arrays

We can't create a proxy for an array, so we have to return a copy of it with the other type. So, if a method returns an array and the user modifies this array, these modifications won't be propagated to the right array.

Number of occurrences of the case in MoDisco source code: 85

Equality test

As seen above, MoDisco objects with the org.eclipse.modisco namespace can leak through (under the cover of an Object for example). If the user uses the == comparison operator, s/he will get the wrong result when comparing a proxy with an original MoDisco element.

Number of occurrences of the case in MoDisco source code: 12215

non-MoDisco code that returns MoDisco objects

For example, a user who wants to work with the ModelBrowser can use an Eclipse API that will return an instance of EcoreBrowser from the org.eclipse.modisco namespace, and there is nothing we can do to substitute it with a proxy:

IEditorPart editor = IDE.openEditor(page, file, EcoreBrowser.EDITOR_ID);
EcoreBrowser browser = (EcoreBrowser) editor; // this will fail!!!

Number of occurrences of the problem cannot be determined because this problem is located in non-MoDisco code.

Idea.png
The number of occurrences were counted with this plug-in : File:Org.eclipse.modisco.dev.mdtMigrationCounts.zip


Coding rules to respect to avoid such problems in the future

Copyright © Eclipse Foundation, Inc. All Rights Reserved.