Difference between revisions of "Papyrus/Neon Work Description/Improvements/Bundle Dependencies"

Latest revision as of 11:48, 25 January 2016

Architectural Issues in Papyrus Bundle Dependencies — Infra Layer

This document captures the problems of bundle dependencies in the "infra layer" bundles of Papyrus.^[1] In particular, the focus is on invalid dependencies:

• dependencies on bundles in higher layers, causing layer dependency cycles in the aggregate
• dependencies on non-Papyrus technologies associated with other layers (e.g., org.eclipse.uml2.uml which should be used only within the UML layer or org.eclipse.gef which should be used only within the GEF3 Diagrams layer
• dependencies on UI in bundles that should be deployable in a headless environment

A detailed Migration Guide is being developed in this wiki as development of this task progresses.

Illegal UI dependencies

In core bundles

• [done] org.eclipse.papyrus.infra.constraints
  • org.eclipse.ui — usage of IStructuredSelection is baked into the constraint API
  • org.eclipse.papyrus.infra.widgets — for ConstraintTypeContentProvider which doesn't belong in this bundle
• [done] org.eclipse.papyrus.infra.core
  • org.eclipse.ui.ide
  • org.eclipse.ui.views.properties.tabbed
  • org.eclipse.emf.edit.ui
  • org.eclipse.papyrus.infra.core.sasheditor.di
• [done] org.eclipse.papyrus.infra.emf
  • org.eclipse.ui
  • org.eclipse.emf.edit.ui
  • org.eclipse.papyrus.emf.facet.custom.ui
  • org.eclipse.papyrus.infra.services.labelprovider
  • org.eclipse.papyrus.infra.widgets — to provide an IAtomicOperationExecutor adapter and an observable that implements ICommitListener
• [done] org.eclipse.papyrus.infra.emf.readonly
  • org.eclipse.ui
  • org.eclipse.papyrus.infra.gmfdiag.commands
  • org.eclipse.papyrus.infra.tools — for access to a UI-thread executor
• [done] org.eclipse.papyrus.infra.extendedtypes.emf — [omitted] this bundle should probably just be renamed as org.eclipse.papyrus.infra.extendedtypes.ui
  • org.eclipse.ui
  • org.eclipse.papyrus.views.properties
• [done] org.eclipse.papyrus.infra.elementtypesconfigurations
  • org.eclipse.ui
• [done] org.eclipse.papyrus.infra.elementtypesconfigurations.emf
  • org.eclipse.ui
  • org.eclipse.papyrus.views.properties
• [done] org.eclipse.papyrus.infra.elementtypesconfigurations.invarianttypes
  • org.eclipse.ui — used only for the AbstractUIPlugin class, which isn't necessary
• [done] org.eclipse.papyrus.infra.extendedtypes
  • org.eclipse.ui
  • org.eclipse.gmf.runtime.common.ui.services
• [done] org.eclipse.papyrus.infra.onefile
  • org.eclipse.ui
  • org.eclipse.ui.ide
  • org.eclipse.ui.navigator
  • org.eclipse.ui.navigator.resources
  • org.eclipse.ltk.ui.refactoring
• [done] org.eclipse.papyrus.infra.sync
  • org.eclipse.ui — only for the AbstractUIPlugin, which isn't necessary
• [done] org.eclipse.papyrus.infra.tools
  • org.eclipse.ui

There is also the case of the org.eclipse.papyrus.infra.widgets.toolbox plug-in, which presents a few different problems:

• the name is very odd. This bundle is not a toolbox for building widgets. In fact, it is mostly a notification system like that provided by the Mylyn project, which is not in the least suggested by the bundle name
• clients that wish to post notifications to the user may well need to do so from non-UI code, so the dependency on this UI bundle is problematic. There should be a generic notification API in the core (non-UI) layer for which this bundle provides the UI presentation (in the absence of which, perhaps, notifications go to stdout or log or some other destination)

In services bundles

• [in progress] org.eclipse.papyrus.infra.services.controlmode
  • org.eclipse.emf.edit.ui
  • org.eclipse.papyrus.infra.widgets.toolbox
  • especially because it defines a core API for control-mode invocation and participants, this bundle should be partitioned into separate core and UI components
• [done] org.eclipse.papyrus.infra.services.edit
  • org.eclipse.ui — only to provide a class implementing ISelectionStatusValidator
• [in progress] org.eclipse.papyrus.infra.services.markerlistener
  • org.eclipse.ui — marker changes often don't occur on the UI thread and reacting to them should not require the UI
  • org.eclipse.gmf.runtime.common.ui
  • org.eclipse.emf.edit.ui
• [in progress] org.eclipse.papyrus.infra.services.resourceloading
  • org.eclipse.ui.ide
  • org.eclipse.papyrus.infra.core.sasheditor.di
  • org.eclipse.papyrus.infra.widgets.toolbox - because there is no core notification API, only the API in this UI bundle
  • org.eclipse.gmf.runtime.diagram.common.ui
  • headless clients should be able to use the on-demand-loading ModelSet and take advantage of the smart resource unloading facilities
• [in progress] org.eclipse.papyrus.infra.services.validation
  • org.eclipse.ui — headless clients should be able to invoke validation
  • org.eclipse.emf.edit.ui
  • org.eclipse.papyrus.infra.widgets
  • org.eclipse.papyrus.infra.services.decoration
• [done] org.eclipse.papyrus.services.viewersearch
  • org.eclipse.ui - only for the AbstractUIPlugin class, which is not necessary

Illegal UML Dependencies

In core bundles

• [done] org.eclipse.papyrus.infra.core
  • actions contributed to the org.eclipse.papyrus.uml.diagram.ui.toolbar, since moved to org.eclipse.papyrus.infra.ui bundle
• [done] org.eclipse.papyrus.emf.readonly
  • references to org.eclipse.papyrus.uml.diagram.common.IsPapyrusActiveWithUMLModel enablement expression definitions in menu actions
• [done] org.eclipse.papyrus.infra.extendedtypes
  • edit advice registrations against the org.eclipse.uml2.uml.Element type
• [done] org.eclipse.papyrus.infra.extendedtypes.editor
  • org.eclipse.uml2.uml.edit — not needed because of delegation to adapter-factory registry and the core extended-types model does not use UML
• [done] org.eclipse.papyrus.infra.newchild
  • org.eclipse.papyrus.uml.tools — for the SemanticUMLContentProvider
• [done] org.eclipse.papyrus.infra.hyperlink
  • org.eclipse.papyrus.uml.diagram.common — not declared in the bundle manifest, but the Hyperlinks dialog grabs (and leaks!) images from the UML diagram bundle for display on its buttons

There are a couple of interesting cases in this component:

• org.eclipse.papyrus.infra.elementtypesconfigurations.emf
  • org.eclipse.uml2.uml — for feature value configuration strategies
• org.eclipse.papyrus.infra.extendedtypes.emf
  • org.eclipse.papyrus.uml2.uml — for feature value configuration strategies

In both of these cases, it seems like a legitimate use of the `ValueSpecification` capability of the UML for configuring element-type advices to set values in features of elements. This is not a dependency on the UML Layer of Papyrus, which would be obviously invalid, but simply a use of the UML language in the element-types model in the same way as Ecore might be used.

In services bundles

• [done] org.eclipse.papyrus.infra.services.resourceloading
  • org.eclipse.papyrus.uml.tools - for access to the semantic model via UmlModel

In diagram bundles

• org.eclipse.papyrus.infra.gmfdiag.common
  • org.eclipse.papyrus.uml.tools — for UmlModel to get the semantic model resource and for CommandBasedObservable which is more general than just UML anyways
• org.eclipse.papyrus.infra.gmfdiag.css
  • org.eclipse.papyrus.uml.diagram.common — for StereotypeDisplayUtil
• org.eclipse.papyrus.infra.gmfdiag.css.configuration
  • action contribution to org.eclipse.papyrus.uml.diagram.ui.popupmenu.format menu
• org.eclipse.papyrus.infra.gmfdiag.css.model
  • org.eclipse.uml2.uml — unused
• org.eclipse.papyrus.infra.gmfdiag.css.palette
  • org.eclipse.uml2.uml
  • org.eclipse.papyrus.uml.diagram.common — extension on the org.eclipse.papyrus.uml.diagram.common.aspectToolProvider point
• org.eclipse.papyrus.infra.gmfdiag.css.properties
  • org.eclipse.papyrus.uml.tools — unused
• org.eclipse.papyrus.infra.gmfdiag.css3.xtext.ui
  • org.eclipse.uml2.uml — for content-assist proposals of UML metaclasses and attributes
• org.eclipse.papyrus.infra.gmfdiag.export
  • org.eclipse.papyrus.uml.utils — for access to the semantic model in UmlModel
  • menu action contributions to UML menus
• org.eclipse.papyrus.infra.gmfdiag.menu
  • menu actions defined by commands in org.eclipse.papyrus.uml.diagram.common
  • enablement expression references to properties in org.eclipse.papyrus.uml.diagram.common
• org.eclipse.papyrus.infra.gmfdiag.outline
  • org.eclipse.uml2.uml.editor — to get the UMLItemProviderAdapterFactory, which should be necessary if the ComposedAdapterFactory were used, which it should be
• org.eclipse.papyrus.infra.gmfdiag.properties
  • org.eclipse.papyrus.uml.tools — for data-binding APIs that have nothing to do with UML
• org.eclipse.papyrus.infra.gmfdiag.welcome
  • org.eclipse.papyrus.uml.tools — for the SemanticUMLContentProvider

In table bundles

• [in progress] org.eclipse.papyrus.infra.nattable.common
  • menu contributions referencing menus with UML identifiers
  • edit-helper advice binding to the org.eclipse.uml2.uml.Element type
• [done] org.eclipse.papyrus.infra.nattable.controlmode
  • org.eclipse.uml2.uml — unused
• [done] org.eclipse.papyrus.infra.nattable.modelexplorer
  • org.eclipse.papyrus.uml.diagram.menu — unused
• [done] org.eclipse.papyrus.infra.nattable.views.config
  • org.eclipse.papyrus.uml.tools — for UmlModel API to determine whether a resource is a semantic model resource

Illegal GMF/GEF Dependencies

In core bundles

Note that the GMF Run-time component's architecture has two distinct personalities: an extension of the EMF run-time's editing capabilities historically named the MSL ("Model Services Layer", references to which still may linger in the Javadocs and other comments in the code) and the GEF-based diagram editor implementation and related frameworks and services. This latter has the dependency on the GEF3 APIs specifically and is what should not be referenced by the Infra layer bundles (for the transitive GEF requirement). The EMF editing services (MSL) layer of the GMF Run-time is permitted in the Infra layer, especially for such components as the Element Types Registry.

• [done] org.eclipse.papyrus.infra.core
  • org.eclipse.gef
• [done] org.eclipse.papyrus.infra.core.sasheditor
  • org.eclipse.gef — the GEF editor delegate needs to be factored out into the diagrams layer
• [done] org.eclipse.papyrus.infra.emf
  • org.eclipse.gmf.runtime.notation
• [in progress] org.eclipse.papyrus.infra.emf.readonly
  • org.eclipse.papyrus.infra.gmfdiag.commands
• [in progress] org.eclipse.papyrus.infra.hyperlink
  • org.eclipse.gmf.runtime.notation
  • org.eclipse.papyrus.infra.gmfdiag.commands
• [in progress] org.eclipse.papyrus.infra.newchild
  • org.eclipse.papyrus.infra.gmfdiag.commands

In services bundles

• org.eclipse.papyrus.infra.services.resourceloading
  • org.eclipse.gef
  • org.eclipse.gmf.runtime.notation
  • org.eclipse.gmf.runtime.diagram.ui
  • org.eclipse.papyrus.infra.gmfdiag.common
• org.eclipse.papyrus.infra.services.validation
  • org.eclipse.gmf.runtime.emf.commands.core — only so that the validation commands can be AbstractTransactionalCommands (a kind of GMF ICommand). There should be no need for them to be that; they can just be AbstractEMFOperations or not even commands at all (why do they need to be undoable?)

In table bundles

• org.eclipse.papyrus.infra.emf.nattable
  • org.eclipse.papyrus.infra.gmfdiag.commands — for GMFtoEMFCommandWrapper, which should be in a lower infra-layer bundle
• org.eclipse.papyrus.infra.nattable
  • org.eclipse.papyrus.infra.gmfdiag.commands — for APIs such as GMFtoEMFCommandWrapper, CheckedOperationHistory, and GMFUnsafe that should be in a lower infra-layer bundle
  • org.eclipse.papyrus.infra.gmfdiag.common — only to get JFace Text's IContentAssistProcessor type
• org.eclipse.papyrus.infra.nattable.common
  • org.eclipse.papyrus.infra.gmfdiag.commands — for GMFtoEMFCommandWrapper, which should be in a lower infra-layer bundle
  • org.eclipse.papyrus.infra.gmfdiag.common — for registration of a paste strategy
• org.eclipse.papyrus.infra.nattable.controlmode
  • org.eclipse.papyrus.infra.gmfdiag.commands — unused
  • org.eclipse.papyrus.infra.gmfdiag.common — unused
  • the two dependencies above re-export org.eclipse.gm.runtime.emf.type.core, which is all that this bundle actually uses
• org.eclipse.papyrus.infra.nattable.modelexplorer
  • org.eclipse.gmf.runtime.notation — unused
  • org.eclipse.papyrus.infra.gmfdiag.commands — for GMFtoEMFCommandWrapper, which should be in a lower infra-layer bundle
• org.eclipse.papyrus.infra.nattable.properties
  • org.eclipse.papyrus.infra.gmfdiag.commands — for GMFtoEMFCommandWrapper, which should be in a lower infra-layer bundle
• org.eclipse.papyrus.infra.nattable.views.config
  • org.eclipse.papyrus.infra.gmfdiag.commands — for GMFtoEMFCommandWrapper, which should be in a lower infra-layer bundle
• org.eclipse.papyrus.infra.nattable.views.editor
  • org.eclipse.papyrus.infra.gmfdiag.commands — unused

Other suspicious dependencies

In core bundles

• org.eclipse.papyrus.infra.emf
  • org.eclipse.papyrus.emf.facet.custom.core
  • org.eclipse.gmf.runtime.common.core — seems odd in a bundled named "emf"
  • org.eclipse.gmf.runtime.emf.commands.core — likewise
  • org.eclipse.gmf.runtime.emf.type.core — likewise
• [done] org.eclipse.papyrus.infra.extendedtypes.emf
  • org.eclipse.papyrus.views.properties.model — has been pulled into the Infra Layer as org.eclipse.papyrus.infra.properties

In services bundles

• org.eclipse.papyrus.infra.services.resourceloading.preferences
  • should be renamed as org.eclipse.papyrus.infra.services.resourceloading.ui because the preference page is only one small part of the UI. Other UI-dependent APIs would be moved into this bundle

Evolution Strategies

Dependencies on UI

This is the simplest case. Simply divide the bundle into two, a "core" bundle and a "ui" bundle. In many cases, this would simply amount to creating a new bundle for the UI bits, leaving the core bits in the current bundle, on the assumption that this has the least impact on clients (UI content often being not intended as API, but merely contributions of menu actions etc. to the platform UI).

Note that, where UI classes and non-UI classes reside in the same package, moving the UI classes into a new bundle implies changing their package name. We should not employ the discouraged OSGi capability of splitting packages.

A common problem in Papyrus APIs that should be "core" (headless) is the need to post asynchronous execution of tasks to defer reactions to changes in the UI (in the case that there is a UI and that updates do need to be deferred). For this purpose, a root Papyrus plug-in (org.eclipse.papyrus.core or probably org.eclipse.papyrus.tools) should define an API for obtaining a shared executor that posts tasks on the UI thread, and the org.eclipse.papyrus.ui bundle should provide that executor (via an OSGi service is probably the best mechanism).

Dependencies on UML

A common dependency on the Papyrus UML Layer is for access to the semantic model, either as the simple root model element (via UmlModel) or for presentation of a tree of model content (SemanticUMLContentProvider). Both of these ought to be available in a generic fashion via some service that can provide:

• what is/are the root(s) of the EMF-based semantic model content
• a SemanticEMFContentProvider best suited to presenting the semantic model content

The ILanguageService API should be expanded to provide these facilities. In particular,

• an ILanguage can provide the IModel that matches its semantic content. If that IModel is an IEMFModel that, with a new API, can provide its model roots as EObjects, then clients can use that to get the model roots
• an IModel can be adapted to a new factory interface for ITreeContentProviders that can accept the ModelSet as input (hence making this a responsibility of the IModel). When such a content provider is a SemanticEMFContentProvider, it can be used to populate the various Papyrus tree viewers accordingly. Adaptation would be required to plug in the factory from a UI bundle
• the SemanticService in the org.eclipse.papyrus.infra.services.semantic bundle could be expanded to synthesize these new capabilities (it already provides all contents of all resources as the "semantic" model, which often is not what clients really need)

Other Considerations

Deprecated APIs

This is an opportunity, with the version 2.0, to remove some deprecated APIs from certain core components. In particular,

• IFile and other workspace resource assumptions in deprecated APIs in the IModel and ModelSet. Very few bundles should have any dependency on org.eclipse.core.resources
• the IReadOnlyHandler interface and related deprecated APIs in the read-only framework

Internal APIs

Most packages in the Papyrus bundles are exported as public API (not internal), although the intent probably was not to publish them as frozen stable API.

to be completed

Bundle Activators

This is a special case of the internal API problem discussed above, but with particular insidious twists:

• almost all Papyrus plug-ins export their activator classes as public API
• almost all of these have the same name, Activator
• a lot of code references the bundle activator to use the LogHelper for logging exceptions and other problems
• consequently, whenever a compilation unit is refactored from one bundle to another, it often happens that it can still see the activator that it used formerly and so it continues to do so without the user noticing. A relatively benign result is that the user's log messages are a bit confusing because they are tagged with the wrong bundle ID. More serious problems arise when extension points move and the utilities that read them, being initialized with a bundle namespace via Activator.PLUGIN_ID, now use the wrong namespace and no longer find any extensions

Recommendations:

• all bundle activators should be moved to an internal package. Possibly even one that isn't even exported at all, so that it cannot be used as API by other bundles, although the restriction warning really ought to be sufficient to alert developers when refactorings go awry
• APIs that load contributions from an extension point should never be initialized by clients with the namespace in which the extension point is defined. The extension point API should know that for itself and internalize it. For example:
  • the ExtensionServicesRegistry should be hard-coded to use the org.eclipse.papyrus.infra.core namespace
  • the CreationCommandRegistry should be hard-coded to use the org.eclipse.papyrus.infra.ui namespace
  • etc.

Traceability

Bugzilla

This effort is tracked in bugzilla under bug 485220.

Gerrit

Some Gerrit reviews propose changes to implement the dependency refactorings:

• Change 63777 illustrates the impact of removing UI-dependent APIs from the org.eclipse.papyrus.infra.tools and org.eclipse.papyrus.infra.core bundles to org.eclipse.papyrus.infra.ui
• Change 64168 implements UI refactorings in additional Infra Layer bundles for EMF APIs (including read-only handling) and Services
• Change 64516 completes the UI dependency refactorings of the Infra Layer bundles

Footnotes

1. ↑ Excluding the infra.gmfdiag, infra.nattable, and infra.viewpoints layers, which are not actually core-infrastructural. They should simply be named diagram, nattable, and viewpoints, respectively.