search

viatra / EMF-IncQuery

This repository is only kept for historic reasons. All development happens on eclipse.org
http://eclipse.org/viatra
13 stars 4 forks source link

Fix selection synchronization between pattern registry panel and query explorer tree #221

Closed istvanrath closed 12 years ago

istvanrath commented 12 years ago

title says it all.

szabta89 commented 12 years ago

@ujhelyiz @istvanrath I proposed a fix for the issue, however when clicking on the label (inside the middle viewer) associated to a generated pattern I get the following Exception. 

!ENTRY org.eclipse.core.databinding 4 0 2012-07-09 14:59:39.482
!MESSAGE Unhandled exception: null
!STACK 0
java.lang.NullPointerException
    at org.eclipse.viatra2.emf.incquery.tooling.generator.genmodel.GenModelMetamodelProviderService.getGeneratorModel(GenModelMetamodelProviderService.java:117)
    at org.eclipse.viatra2.emf.incquery.tooling.generator.genmodel.GenModelMetamodelProviderService.findGenPackage(GenModelMetamodelProviderService.java:171)
    at org.eclipse.viatra2.emf.incquery.tooling.generator.genmodel.GenModelMetamodelProviderService.findGenPackage(GenModelMetamodelProviderService.java:166)
    at org.eclipse.viatra2.emf.incquery.tooling.generator.types.GenModelBasedTypeProvider.resolve(GenModelBasedTypeProvider.java:42)
    at org.eclipse.viatra2.patternlanguage.types.EMFPatternTypeProvider.resolve(EMFPatternTypeProvider.java:244)
    at org.eclipse.viatra2.patternlanguage.types.EMFPatternTypeProvider.resolve(EMFPatternTypeProvider.java:227)
    at org.eclipse.viatra2.patternlanguage.types.EMFPatternTypeProvider.resolve(EMFPatternTypeProvider.java:164)
    at org.eclipse.viatra2.patternlanguage.types.EMFPatternTypeProvider.resolve(EMFPatternTypeProvider.java:125)
    at org.eclipse.viatra2.patternlanguage.types.EMFPatternTypeProvider.resolve(EMFPatternTypeProvider.java:103)
    at org.eclipse.viatra2.patternlanguage.types.EMFPatternTypeProvider._typeForIdentifiable(EMFPatternTypeProvider.java:85)
    at org.eclipse.viatra2.patternlanguage.types.EMFPatternTypeProvider.typeForIdentifiable(EMFPatternTypeProvider.java:67)
    at org.eclipse.xtext.xbase.typing.AbstractTypeProvider$4.doComputation(AbstractTypeProvider.java:454)
    at org.eclipse.xtext.xbase.typing.AbstractTypeProvider$4.doComputation(AbstractTypeProvider.java:1)
    at org.eclipse.xtext.xbase.typing.AbstractTypeProvider$CyclicHandlingSupport$3.get(AbstractTypeProvider.java:658)
    at org.eclipse.xtext.xbase.typing.AbstractTypeProvider$CyclicHandlingSupport$3.get(AbstractTypeProvider.java:1)
    at org.eclipse.xtext.xbase.typing.AbstractTypeProvider$1.get(AbstractTypeProvider.java:159)
    at org.eclipse.xtext.xbase.typing.AbstractTypeProvider$CyclicHandlingSupport.getType(AbstractTypeProvider.java:655)
    at org.eclipse.xtext.xbase.typing.AbstractTypeProvider.doGetType(AbstractTypeProvider.java:348)
    at org.eclipse.xtext.xbase.typing.AbstractTypeProvider.getTypeForIdentifiable(AbstractTypeProvider.java:477)
    at org.eclipse.xtext.xbase.typing.AbstractTypeProvider.getTypeForIdentifiable(AbstractTypeProvider.java:473)
    at org.eclipse.viatra2.emf.incquery.queryexplorer.content.detail.TableViewerUtil.prepareTableViewerForMatcherConfiguration(TableViewerUtil.java:120)
    at org.eclipse.viatra2.emf.incquery.queryexplorer.QueryExplorer$MatcherTreeViewerSelectionChangeListener.handleValueChange(QueryExplorer.java:243)
    at org.eclipse.core.databinding.observable.value.ValueChangeEvent.dispatch(ValueChangeEvent.java:62)
    at org.eclipse.core.databinding.observable.ChangeManager.fireEvent(ChangeManager.java:119)
    at org.eclipse.core.databinding.observable.value.DecoratingObservableValue.fireValueChange(DecoratingObservableValue.java:55)
    at org.eclipse.core.databinding.observable.value.DecoratingObservableValue.handleValueChange(DecoratingObservableValue.java:93)
    at org.eclipse.core.databinding.observable.value.DecoratingObservableValue$1.handleValueChange(DecoratingObservableValue.java:67)
    at org.eclipse.core.databinding.observable.value.ValueChangeEvent.dispatch(ValueChangeEvent.java:62)
    at org.eclipse.core.databinding.observable.ChangeManager.fireEvent(ChangeManager.java:119)
    at org.eclipse.core.databinding.observable.value.AbstractObservableValue.fireValueChange(AbstractObservableValue.java:71)
    at org.eclipse.core.internal.databinding.property.value.SimplePropertyObservableValue.notifyIfChanged(SimplePropertyObservableValue.java:120)
    at org.eclipse.core.internal.databinding.property.value.SimplePropertyObservableValue.access$1(SimplePropertyObservableValue.java:112)
    at org.eclipse.core.internal.databinding.property.value.SimplePropertyObservableValue$2.run(SimplePropertyObservableValue.java:66)
    at org.eclipse.core.databinding.observable.Realm$1.run(Realm.java:148)
    at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:42)
    at org.eclipse.core.databinding.observable.Realm.safeRun(Realm.java:152)
    at org.eclipse.core.databinding.observable.Realm.exec(Realm.java:170)
    at org.eclipse.core.internal.databinding.property.value.SimplePropertyObservableValue$1.handleEvent(SimplePropertyObservableValue.java:63)
    at org.eclipse.core.databinding.property.NativePropertyListener.fireChange(NativePropertyListener.java:63)
    at org.eclipse.jface.internal.databinding.viewers.SelectionChangedListener.selectionChanged(SelectionChangedListener.java:35)
    at org.eclipse.jface.viewers.Viewer$2.run(Viewer.java:164)
    at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:42)
    at org.eclipse.ui.internal.JFaceUtil$1.run(JFaceUtil.java:49)
    at org.eclipse.jface.util.SafeRunnable.run(SafeRunnable.java:175)
    at org.eclipse.jface.viewers.Viewer.fireSelectionChanged(Viewer.java:162)
    at org.eclipse.jface.viewers.StructuredViewer.updateSelection(StructuredViewer.java:2188)
    at org.eclipse.jface.viewers.StructuredViewer.handleSelect(StructuredViewer.java:1211)
    at org.eclipse.jface.viewers.StructuredViewer$4.widgetSelected(StructuredViewer.java:1241)
    at org.eclipse.jface.util.OpenStrategy.fireSelectionEvent(OpenStrategy.java:239)
    at org.eclipse.jface.util.OpenStrategy.access$4(OpenStrategy.java:233)
    at org.eclipse.jface.util.OpenStrategy$1.handleEvent(OpenStrategy.java:403)
    at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:84)
    at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1053)
    at org.eclipse.swt.widgets.Display.runDeferredEvents(Display.java:4165)
    at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3754)
    at org.eclipse.ui.internal.Workbench.runEventLoop(Workbench.java:2701)
    at org.eclipse.ui.internal.Workbench.runUI(Workbench.java:2665)
    at org.eclipse.ui.internal.Workbench.access$4(Workbench.java:2499)
    at org.eclipse.ui.internal.Workbench$7.run(Workbench.java:679)
    at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:332)
    at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:668)
    at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:149)
    at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:123)
    at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:196)
    at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:110)
    at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:79)
    at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:344)
    at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:179)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source)
    at java.lang.reflect.Method.invoke(Unknown Source)
    at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:622)
    at org.eclipse.equinox.launcher.Main.basicRun(Main.java:577)
    at org.eclipse.equinox.launcher.Main.run(Main.java:1410)
    at org.eclipse.equinox.launcher.Main.main(Main.java:1386)```
ujhelyiz commented 12 years ago

I couldn't reproduce the issue in my computer - probably some details were missing required for activation.

However, by reading the stack trace, I found a single way it could go wrong, and have written sanity check code to avoid the issue. Please test it.

szabta89 commented 12 years ago

@ujhelyiz It did not solve the problem. The same (trace) Exception is thrown still. For example you can try the school example. Import the school.incquery project into your host workspace and annotate at least one pattern with the DisplayInExplorer(mode="on"). In the runtime Eclipse load an instancemodel from the school.instancemodel project and you will get the Exception when clicking on the label of a generated pattern.

ujhelyiz commented 12 years ago

I could not reproduce it after the commit 5d839d2

szabta89 commented 12 years ago

@ujhelyiz I'm still able to reproduce it with the above mentioned scenario. I have an up-to-date copy of the master branch.

ujhelyiz commented 12 years ago

And have you updated the school model after @istvanrath updated it in #174?

szabta89 commented 12 years ago

Yep! No changes.

istvanrath commented 12 years ago

By the way, the Query Explorer should not allow such an NPE to be thrown even if the GenMetamodelProvider returns a null. So the UI needs to be fixed with respect to error management regardless of the underlying issue.

szabta89 commented 12 years ago

@istvanrath It is not the case that the Query Explorer throws this Exception. It comes from the GenMetamodelProvider. I can wrap the root method call (TableViewerUtil) in a try-catch but then what is the desired handling?

istvanrath commented 12 years ago

Log the exception properly and ensure that normal operation can continue.

szabta89 commented 12 years ago

Normal operation can continue properly at the moment, but I think the logging should happen inside GenMetamodelProvider as I think it is a bad design to catch NPE.

ujhelyiz commented 12 years ago

According to the trace @szabta89 is right that this is not the Query Explorers responsibility to handle. On the other hand, as I did not manage to reproduce the issue yet, I frankly don't know how to resolve it.

ujhelyiz commented 12 years ago

Ok, now I have found it. What I was missing before is that the annotation changed in #185, and missed the clicking step in Tamás's original description (bad idea to try to read such things around midnight :) ).

The issue was that there is no eiqgen available when loading generated patterns (by design of course), but the API did not handle this case gracefully.

istvanrath commented 12 years ago

Let me clarify: the goal is NOT to catch NPEs, but to avoid them in the first place:

Additionally, I would really like the overall system to have a well-designed state model of the "normal operation can continue" problem, i.e. to make it explicit which problems (exceptions) can be safely tolerated and which ones require a reinitialization of e.g. the viewer. We will talk about this on the next meeting.

ujhelyiz commented 12 years ago

It was a design issue at genmodelprovider - it did not handle well a corner case - that was the cause of the exception. In that case, a simply fallthrough is appropriate to the extension based genmodel resolution - it was what I was doing.

In other words, the query explorer should (or might) log errors/exceptions coming from this resolution phase - however, in this isolated case the error was clearly inside the genmodel provider.

istvanrath commented 12 years ago

It is clear what the source of the error was. My remarks are intended as a general guidance towards making the Query Explorer (and the entire system) better at error management, and they are not strongly focused on this particular issue.

szabta89 commented 12 years ago

The issue is solved now. However, if the GenModel cannot be found for the generated pattern the type provider cannot infer the type of a matcher parameter properly, so one can set any type of object as a filter. This will not cause errors, as a wrong filter will result the match set to be empty.