Community
Participate
Working Groups
Suppose I want to create a model programmatically (MWindow > ... > MPart) and one of the parts expects to get a domain specific type (DST) injected. The DST should have a scope of e.g. MWindow and is not in the MApplication. This currently fails because the (SWT) renderer wants to create the IEclipseContext by itself (see org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.safeCreateGui(MUIElement, Object, IEclipseContext) line 605 as of today). The following code exemplifies this scenario: --- package com.example.e4.rcp.todo.handler; import javax.annotation.PostConstruct; import org.eclipse.e4.core.contexts.IEclipseContext; import org.eclipse.e4.core.di.annotations.Execute; import org.eclipse.e4.ui.model.application.MApplication; import org.eclipse.e4.ui.model.application.ui.basic.MPart; import org.eclipse.e4.ui.model.application.ui.basic.MTrimmedWindow; import org.eclipse.e4.ui.model.application.ui.basic.MWindow; import org.eclipse.e4.ui.workbench.modeling.EModelService; import org.eclipse.swt.SWT; import org.eclipse.swt.layout.GridData; import org.eclipse.swt.widgets.Composite; import org.eclipse.swt.widgets.Label; public class NewHierarchy { // User domain type static class Person { private String n; public Person(String n) { this.n = n; } public String getName() { return n; } } // MPart Impl static class MyPart { @PostConstruct public void createPart(Composite parent, Person p) { Label label = new Label(parent, SWT.NONE); label.setLayoutData(new GridData(SWT.BEGINNING, SWT.CENTER, false, false)); label.setText(p.getName()); } } // Handler Impl @Execute public void execute(EModelService ems, MApplication mApplication) { // Create new MWindow MWindow mWindow = ems.createModelElement(MTrimmedWindow.class); mWindow.setHeight(200); mWindow.setWidth(400); // Create MPart as child of window MPart mPart = ems.createModelElement(MPart.class); mPart.setElementId("myElementId"); mPart.setContributionURI("bundleclass://com.example.e4.rcp.todo/" + "com.example.e4.rcp.todo.handler.NewHierarchy$MyPart"); mWindow.getChildren().add(mPart); // Person will be used by MyPart in its @PostConstruct method IEclipseContext parentCtx = mApplication.getContext(); IEclipseContext lclCtx = parentCtx.createChild(); lclCtx.set(Person.class, new Person("a Label to be shown")); mWindow.setContext(lclCtx); mApplication.getChildren().add(mWindow); } }
https://git.eclipse.org/r/17934
This is not the pattern we recommend. The current pattern is to add an handler for org.eclipse.e4.ui.workbench.UIEvents.Context.TOPIC_CONTEXT and when that property goes from null to IEclipseContext you can fill in whatever variables you deem necessary. With our work in Luna that might even change from adding property handler to adding lifecycle event handler. PW
(In reply to Paul Webster from comment #2) > This is not the pattern we recommend. > > The current pattern is to add an handler for > org.eclipse.e4.ui.workbench.UIEvents.Context.TOPIC_CONTEXT and when that > property goes from null to IEclipseContext you can fill in whatever > variables you deem necessary. > > With our work in Luna that might even change from adding property handler to > adding lifecycle event handler. Hi Paul, this sound like a lot of overhead to me. Just adding a handler to pre-populate the IEC with objects. Any chance my pattern could be accepted in addition to the handler?
This is a bad idea because only the System knows who is the correct parent context, there are many parts that dependt on a correct context hierachy to eg activate the active branch, a big -100 from me, please also note that we sometimes reparent contexts!
(In reply to Thomas Schindl from comment #4) > This is a bad idea because only the System knows who is the correct parent > context, there are many parts that dependt on a correct context hierachy to > eg activate the active branch, a big -100 from me, please also note that we > sometimes reparent contexts! I do not need the parenting. The example code could easily be adapted to only create a "standalone" IEC and let the renderer handle the parenting.
Creating and assigningn the context is and stays the frameworks responsibility, no way around this, makeing it easier to contribue like Paul said through lifecycle stuff is the way to go
(In reply to Thomas Schindl from comment #6) > Creating and assigningn the context is and stays the frameworks > responsibility, no way around this, makeing it easier to contribue like Paul > said through lifecycle stuff is the way to go Hi Tom, for my understanding, can you explain in more detail why you see this responsibility with the framework? Especially the fact that the renderer builds-up the IEC. Doesn't this cause extra work for new renderer implementations which isn't really renderer specific? Btw. would it make sense to marker all methods related to context chaining/creation with Javadoc tags (@noreference) to indicate that it is the renderers responsiblity only? Right now the example code causes a rather cryptic exception that does not really reveal its cause: !ENTRY org.eclipse.e4.ui.workbench 4 0 2013-11-01 08:39:21.203 !MESSAGE Internal Error !STACK 0 java.lang.IllegalStateException: Application does not have an active window at org.eclipse.e4.ui.internal.workbench.ApplicationPartServiceImpl.getActiveWindowService(ApplicationPartServiceImpl.java:43) at org.eclipse.e4.ui.internal.workbench.ApplicationPartServiceImpl.activate(ApplicationPartServiceImpl.java:67) at org.eclipse.e4.ui.internal.workbench.ApplicationPartServiceImpl.activate(ApplicationPartServiceImpl.java:67) at org.eclipse.e4.ui.internal.workbench.swt.AbstractPartRenderer.activate(AbstractPartRenderer.java:104) at org.eclipse.e4.ui.workbench.renderers.swt.ContributedPartRenderer$1.handleEvent(ContributedPartRenderer.java:59) at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:84) at org.eclipse.swt.widgets.Display.sendEvent(Display.java:4390) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1387) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1411) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1396) at org.eclipse.swt.widgets.Shell.setActiveControl(Shell.java:1672) at org.eclipse.swt.widgets.Shell.setActiveControl(Shell.java:1635) at org.eclipse.swt.widgets.Control.sendFocusEvent(Control.java:3861) at org.eclipse.swt.widgets.Control.gtk_event_after(Control.java:3129) at org.eclipse.swt.widgets.Widget.windowProc(Widget.java:2072) at org.eclipse.swt.widgets.Control.windowProc(Control.java:5458) at org.eclipse.swt.widgets.Display.windowProc(Display.java:4592) at org.eclipse.swt.internal.gtk.OS._gtk_main_do_event(Native Method) at org.eclipse.swt.internal.gtk.OS.gtk_main_do_event(OS.java:8701) at org.eclipse.swt.widgets.Display.eventProc(Display.java:1243) at org.eclipse.swt.internal.gtk.OS._g_main_context_iteration(Native Method) at org.eclipse.swt.internal.gtk.OS.g_main_context_iteration(OS.java:2260) at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3359) at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine$9.run(PartRenderingEngine.java:1128) at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:332) at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.run(PartRenderingEngine.java:1012) at org.eclipse.e4.ui.internal.workbench.E4Workbench.createAndRunUI(E4Workbench.java:146) at org.eclipse.e4.ui.internal.workbench.swt.E4Application.start(E4Application.java:164) at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:196) at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:109) at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:80) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:372) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:226) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:616) at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:636) at org.eclipse.equinox.launcher.Main.basicRun(Main.java:591) at org.eclipse.equinox.launcher.Main.run(Main.java:1450) at org.eclipse.equinox.launcher.Main.main(Main.java:1426)
http://www.eclipse.org/forums/index.php/t/573869/ is similar in what Felix tries to accomplish.
(In reply to Markus Kuppe from comment #5) > (In reply to Thomas Schindl from comment #4) > > This is a bad idea because only the System knows who is the correct parent > > context, there are many parts that dependt on a correct context hierachy to > > eg activate the active branch, a big -100 from me, please also note that we > > sometimes reparent contexts! > > I do not need the parenting. The example code could easily be adapted to > only create a "standalone" IEC and let the renderer handle the parenting. Markus: it's not that *you* don't need the parenting, but the E4 renderers do. They may assume that certain values have been stored in the parent contexts. I started some discussion on populating a context in bug 401931. For now, I use an approach like the ESelectionService, where it exposes a little generic IContextFunction at a well-known name as an OSGi service, and which installs an instance for appropriate contexts. It was an easy hack, but it doesn't scale as well. We could generalize a solution based on TOPIC_CONTEXT that would be configured from an extension point.
(In reply to Brian de Alwis from comment #9) > Markus: it's not that *you* don't need the parenting, but the E4 renderers > do. They may assume that certain values have been stored in the parent > contexts. I didn't mean to imply that the renderer shouldn't do parenting at all. I fail to see though, why the renderer can't do proper parenting when an MContext object has a pre-populated IEC attached to it already. > I started some discussion on populating a context in bug 401931. For now, I > use an approach like the ESelectionService, where it exposes a little > generic IContextFunction at a well-known name as an OSGi service, and which > installs an instance for appropriate contexts. It was an easy hack, but it > doesn't scale as well. > > We could generalize a solution based on TOPIC_CONTEXT that would be > configured from an extension point. An extension point just to get objects in to the context? Wow.
Right I would mark the method @NoReference noone should ever call it! Generally speaking one should interface with IEclipseContext as less as possible. IMHO the solution in Luna would be: MPart extends MLifecycle { List<String> lifecycleUris; } class Lifecycle { @PostContextCreate void fillContext() { } @PreOpen void doStuffOnPreopen() { } @PreClose boolean avoidClosing() { } } Beside that what would you do when you come from an restore state? At least at this point you'd need to get the info into the context using the event Paul described and remember them somewhere like persistedState!
FTR here's an update implementation that uses the handler approach: package com.example.e4.rcp.todo.handler; import javax.annotation.PostConstruct; import org.eclipse.e4.core.contexts.IEclipseContext; import org.eclipse.e4.core.di.annotations.Execute; import org.eclipse.e4.core.services.events.IEventBroker; import org.eclipse.e4.ui.model.application.MApplication; import org.eclipse.e4.ui.model.application.ui.basic.MPart; import org.eclipse.e4.ui.model.application.ui.basic.MTrimmedWindow; import org.eclipse.e4.ui.model.application.ui.basic.MWindow; import org.eclipse.e4.ui.workbench.UIEvents; import org.eclipse.e4.ui.workbench.modeling.EModelService; import org.eclipse.swt.SWT; import org.eclipse.swt.layout.GridData; import org.eclipse.swt.widgets.Composite; import org.eclipse.swt.widgets.Label; import org.osgi.service.event.Event; import org.osgi.service.event.EventHandler; public class NewHierarchy { // User domain type static class Person { private String n; public Person(String n) { this.n = n; } public String getName() { return n; } } // MPart Impl static class MyPart { @PostConstruct public void createPart(Composite parent, Person p) { Label label = new Label(parent, SWT.NONE); label.setLayoutData(new GridData(SWT.BEGINNING, SWT.CENTER, false, false)); label.setText(p.getName()); } } private static int CNT = 0; // Handler Impl @Execute public void execute(EModelService ems, MApplication mApplication, final IEventBroker eventBroker) { // Create new MWindow MWindow mWindow = ems.createModelElement(MTrimmedWindow.class); mWindow.setElementId("myMWindowElement#" + CNT); mWindow.setHeight(200); mWindow.setWidth(400); // Create MPart as child of window MPart mPart = ems.createModelElement(MPart.class); final String elementId = "myElementId#" + CNT ++; mPart.setElementId(elementId); mPart.setContributionURI("bundleclass://com.example.e4.rcp.todo/" + "com.example.e4.rcp.todo.handler.NewHierarchy$MyPart"); mWindow.getChildren().add(mPart); // Register broker at the last moment to receive as few events as possible // The purpose of the handler is to populate the Part's IEC with a Person eventBroker.subscribe(UIEvents.Context.TOPIC_CONTEXT, new EventHandler() { @Override public void handleEvent(Event event) { Object origin = event.getProperty(UIEvents.EventTags.ELEMENT); Object context = event.getProperty(UIEvents.EventTags.NEW_VALUE); if ((origin instanceof MPart) && (UIEvents.EventTypes.SET.equals(event .getProperty(UIEvents.EventTags.TYPE)) && context instanceof IEclipseContext) && ((MPart) origin).getElementId().equals(elementId)) { IEclipseContext castedContext = (IEclipseContext) context; castedContext.set(Person.class, new Person("a Label to be shown")); // handler has served its purpose eventBroker.unsubscribe(this); } } }); mApplication.getChildren().add(mWindow); } }
(In reply to Thomas Schindl from comment #11) > Right I would mark the method @NoReference noone should ever call it! Btw. we now have "@NoExtend" (http://download.eclipse.org/eclipse/downloads/drops4/S-4.4M3-201310302000/news/#PDE) > Beside that what would you do when you come from an restore state? At least > at this point you'd need to get the info into the context using the event > Paul described and remember them somewhere like persistedState! Since I handle this sub-graph of the model programmatically anyway, I'd claim that I also be responsible to handle persistence myself (if even needed). Also the handler approach adds quite some boilerplate, not to speak of the runtime overhead imposed when the handler is called multiple times by unrelated SET events.
That's what I'm trying to tell you, when you are coming from a restored state the model is loaded and populated when first rendered. You'd have to write the IEclipseContext event code anyways so why having 2 source paths to achieve the same? Like I said in 4.4 we'll hopefully make the event boilerplate go away and because the Lifecycles scope is defined for only this very model element the call overhead is not there anymore!
(In reply to Thomas Schindl from comment #14) > That's what I'm trying to tell you, when you are coming from a restored > state the model is loaded and populated when first rendered. Remember, I said I do not need persistence for my use case (some of our customers do not use model persistence). > You'd have to > write the IEclipseContext event code anyways so why having 2 source paths to > achieve the same? No, I do not need the event code as I do not use persistence. And thus, there is just an (under-documented) complex solution and API (for the what seems to be the simple solution) that leads to an IllegalStateException with no indication where it is coming from. > Like I said in 4.4 we'll hopefully make the event boilerplate go away and > because the Lifecycles scope is defined for only this very model element the > call overhead is not there anymore! What's the bug number unless it's bug #392903? Btw. you have never responded why it's the renderer's responsibility to create IECs in the first place.
(In reply to Markus Kuppe from comment #15) > > Btw. you have never responded why it's the renderer's responsibility to > create IECs in the first place. It was designed that way. The PartRenderingEngine is responsible for create the IEclipseContext for an MContext before rendering and for disposing it after unrendering. It was designed that way because an unrendered part has no need for a context. Also there's no generic way to "merge" information from one context into another, and each call to set(*) comes with a price ... RATs and other computational values are given the option to run if they need to. It's not that it can never be changed, but it would require a lot of changes and investigation for even a small change, the IEclipseContext is so fundamental to our story. That being said, it looks like the least boilerplate approach in your case would look like: ... final MPart mPart = ems.createModelElement(MPart.class); ... contextHandler = new EventHandler() { public void handleEvent(Event event) { Object elementObj = event .getProperty(UIEvents.EventTags.ELEMENT); Object newObj = event.getProperty(UIEvents.EventTags.NEW_VALUE); if (elementObj == mPart && newObj instanceof IEclipseContext) { IEclipseContext castedContext = (IEclipseContext) newObj; castedContext.set(Person.class, new Person("a Label to be shown")); eventBroker.unsubscribe(this); } } };
(In reply to Paul Webster from comment #16) > It was designed that way. The PartRenderingEngine is responsible for create > the IEclipseContext for an MContext before rendering and for disposing it > after unrendering. It was designed that way because an unrendered part has > no need for a context. Also there's no generic way to "merge" information > from one context into another, and each call to set(*) comes with a price > ... RATs and other computational values are given the option to run if they > need to. > > It's not that it can never be changed, but it would require a lot of changes > and investigation for even a small change, the IEclipseContext is so > fundamental to our story. Thx for your explanation. From the above I take it that manipulating the ICE hierarchy is a dead end for know. That said, creating and adding to IEC has never been my primary target, but just the means to get my original use case done which is to pass domain objects to model element impls before they get rendered/instantiated. Thus, with IEC architecturally being kind of the implementation, why not expose API at the MContext level (e.g. MContext#set(Object k, Object v))? Adding elements into IEC would then be the renderer's responsibility and the user only has to use the model.
This something we can discuss problems I see: a) we need to register a listener and handle those changes b) how do we make sure content of IEC and the Map in the model our getting out of sync? I still believe the Luna Lifecycle-API makes it that easy that the overhead is not justifiable. Generally speaking I want people to interface with IEC as less as possible treating it an implementation detail or SPI which would speak for adding the MContext#set API
(In reply to Thomas Schindl from comment #18) > This something we can discuss problems I see: > a) we need to register a listener and handle those changes > b) how do we make sure content of IEC and the Map in the model our getting > out of sync? Can't we get away with creating the MContext map only for as long as there is no IEC yet? It's the renderer's job then to add the content of the MContext map to the (newly created) IEC and to null the map. MContext#set/#get from there on just delegate to the corresponding IEC methods. The reverse happens when the renderer disposes the IEC.
I wonder if Markus' needs might be met by injecting from the model: @Model(TRANSIENT) // key taken from field private String keyName; @Model(TRANSIENT, "alternativeKeyName") private String field; @Model(PERSISTED) // key taken from field private String value; @Model(PERSISTED, Json.class) // Uses Jackson or Gson or something else private JSONObject map; // Custom deserializer, key taken from field name @Model(PERSISTED, SomeSerializationFactoryClass.class) private SomeClass object; @Model(PERSISTED, "keyName", SomeSerializationFactoryClass.class) private SomeClass object; @Model(TRANSIENT) Provider<String> fieldName; // sets the field value @Model(PERSISTENT, SomeSerializationFactoryClass.class) Provider<SomeClass> object; // deserializes and sets the persistent value Then you can set values in the model and have the code be injected.
