[492935] Ensure IPropertySheetPage can be injected.

- Changed contract of UndoablePropertySheetPage to obtain reference to related IWorkbenchPart rather then UndoRedoActionGroup (which can be maintained locally, and is not necessarily shared with the workbench part). - Created IUndoablePropertySheetPageFactory that supports assisted injection of an UndoablePropertySheetPage. - Changed AbstractFXEditor and AbstractFXView to have IPropertySheetPageFactory injected. - Enabled assisted inject as extra classpath dependency within releng/pom.xml - Provided dependencies to assisted inject for MVC.UI manifest and feature.xml.
eclipse · May 4, 2016 · c273cc4 · c273cc4
1 parent 613642a
commit c273cc4
Show file tree

Hide file tree

Showing 8 changed files with 187 additions and 78 deletions.
diff --git a/org.eclipse.gef4.mvc.fx.ui/src/org/eclipse/gef4/mvc/fx/ui/parts/AbstractFXEditor.java b/org.eclipse.gef4.mvc.fx.ui/src/org/eclipse/gef4/mvc/fx/ui/parts/AbstractFXEditor.java
@@ -22,7 +22,7 @@
 import org.eclipse.gef4.mvc.fx.domain.FXDomain;
 import org.eclipse.gef4.mvc.fx.viewer.FXViewer;
 import org.eclipse.gef4.mvc.operations.ITransactionalOperation;
-import org.eclipse.gef4.mvc.ui.properties.UndoablePropertySheetPage;
+import org.eclipse.gef4.mvc.ui.properties.IPropertySheetPageFactory;
 import org.eclipse.jface.viewers.ISelectionProvider;
 import org.eclipse.swt.SWT;
 import org.eclipse.swt.widgets.Composite;
@@ -55,20 +55,21 @@ public abstract class AbstractFXEditor extends EditorPart {
 
 	@Inject
 	private IFXCanvasFactory canvasFactory;
+	private FXCanvas canvas = null;
 
 	@Inject(optional = true)
 	private ISelectionProvider selectionProvider;
-
 	private SelectionForwarder<Node> selectionForwarder;
 
-	private FXCanvas canvas = null;
-
-	private UndoRedoActionGroup undoRedoActionGroup;
+	@Inject(optional = true)
+	private IPropertySheetPageFactory propertySheetPageFactory;
 	private IPropertySheetPage propertySheetPage;
 
 	private IOperationHistoryListener operationHistoryListener;
 	private boolean isDirty;
 
+	private UndoRedoActionGroup undoRedoActionGroup;
+
 	/**
 	 * Constructs a new {@link AbstractFXEditor} and uses the given
 	 * {@link Injector} to inject its members.
@@ -114,6 +115,20 @@ public void createPartControl(final Composite parent) {
 		activate();
 	}
 
+	/**
+	 * Creates an {@link IPropertySheetPage} using the injected
+	 * {@link IPropertySheetPageFactory}, if present.
+	 *
+	 * @return An {@link IPropertySheetPage}, or <code>null</code> in case no
+	 *         factory was injected.
+	 */
+	protected IPropertySheetPage createPropertySheetPage() {
+		if (propertySheetPageFactory != null) {
+			return propertySheetPageFactory.create(this);
+		}
+		return null;
+	}
+
 	/**
 	 * Deactivates the editor by deactivating its {@link FXDomain}.
 	 */
@@ -133,6 +148,10 @@ public void dispose() {
 		domain.getOperationHistory()
 				.removeOperationHistoryListener(operationHistoryListener);
 
+		if (undoRedoActionGroup != null) {
+			undoRedoActionGroup.dispose();
+		}
+
 		// unregister selection provider
 		if (selectionProvider != null) {
 			getSite().setSelectionProvider(null);
@@ -154,19 +173,9 @@ public Object getAdapter(final Class key) {
 		// contribute to Properties view
 		else if (IPropertySheetPage.class.equals(key)) {
 			if (propertySheetPage == null) {
-				propertySheetPage = new UndoablePropertySheetPage(
-						(IOperationHistory) getAdapter(IOperationHistory.class),
-						(IUndoContext) getAdapter(IUndoContext.class),
-						(UndoRedoActionGroup) getAdapter(
-								UndoRedoActionGroup.class));
+				propertySheetPage = createPropertySheetPage();
 			}
 			return propertySheetPage;
-		} else if (UndoRedoActionGroup.class.equals(key)) {
-			if (undoRedoActionGroup == null) {
-				undoRedoActionGroup = new UndoRedoActionGroup(getSite(),
-						(IUndoContext) getAdapter(IUndoContext.class), true);
-			}
-			return undoRedoActionGroup;
 		} else if (IUndoContext.class.equals(key)) {
 			return domain.getUndoContext();
 		} else if (IOperationHistory.class.equals(key)) {
@@ -206,8 +215,8 @@ protected FXDomain getDomain() {
 	 */
 	// TODO: rename to content viewer (or main viewer)
 	protected FXViewer getViewer() {
-		return domain.getAdapter(AdapterKey.get(FXViewer.class,
-				FXDomain.CONTENT_VIEWER_ROLE));
+		return domain.getAdapter(
+				AdapterKey.get(FXViewer.class, FXDomain.CONTENT_VIEWER_ROLE));
 	}
 
 	/**
@@ -255,6 +264,11 @@ public void historyNotification(final OperationHistoryEvent event) {
 				}
 			}
 		};
+
+		undoRedoActionGroup = new UndoRedoActionGroup(getSite(),
+				(IUndoContext) getAdapter(IUndoContext.class), true);
+		undoRedoActionGroup.fillActionBars(site.getActionBars());
+
 		getDomain().getOperationHistory()
 				.addOperationHistoryListener(operationHistoryListener);
 	}

diff --git a/org.eclipse.gef4.mvc.fx.ui/src/org/eclipse/gef4/mvc/fx/ui/parts/AbstractFXView.java b/org.eclipse.gef4.mvc.fx.ui/src/org/eclipse/gef4/mvc/fx/ui/parts/AbstractFXView.java
@@ -17,7 +17,7 @@
 import org.eclipse.gef4.fx.swt.canvas.IFXCanvasFactory;
 import org.eclipse.gef4.mvc.fx.domain.FXDomain;
 import org.eclipse.gef4.mvc.fx.viewer.FXViewer;
-import org.eclipse.gef4.mvc.ui.properties.UndoablePropertySheetPage;
+import org.eclipse.gef4.mvc.ui.properties.IPropertySheetPageFactory;
 import org.eclipse.jface.viewers.ISelectionProvider;
 import org.eclipse.swt.SWT;
 import org.eclipse.swt.widgets.Composite;
@@ -48,17 +48,18 @@ public abstract class AbstractFXView extends ViewPart {
 
 	@Inject
 	private IFXCanvasFactory canvasFactory;
+	private FXCanvas canvas = null;
 
 	@Inject(optional = true)
 	private ISelectionProvider selectionProvider;
-
 	private SelectionForwarder<Node> selectionForwarder;
 
-	private FXCanvas canvas = null;
+	@Inject(optional = true)
+	private IPropertySheetPageFactory propertySheetPageFactory;
+	private IPropertySheetPage propertySheetPage;
 
 	private UndoRedoActionGroup undoRedoActionGroup;
 	private DeleteActionHandler deleteActionHandler;
-	private IPropertySheetPage propertySheetPage;
 
 	/**
 	 * Constructs a new {@link AbstractFXView} that uses the given
@@ -110,6 +111,20 @@ public void createPartControl(final Composite parent) {
 		activate();
 	}
 
+	/**
+	 * Creates an {@link IPropertySheetPage} using the injected
+	 * {@link IPropertySheetPageFactory}, if present.
+	 *
+	 * @return An {@link IPropertySheetPage}, or <code>null</code> in case no
+	 *         factory was injected.
+	 */
+	protected IPropertySheetPage createPropertySheetPage() {
+		if (propertySheetPageFactory != null) {
+			return propertySheetPageFactory.create(this);
+		}
+		return null;
+	}
+
 	/**
 	 * Deactivates this {@link AbstractFXView} by deactivating its
 	 * {@link FXDomain} that was previously injected.
@@ -131,6 +146,10 @@ public void dispose() {
 			getSite().setSelectionProvider(null);
 		}
 
+		if (undoRedoActionGroup != null) {
+			undoRedoActionGroup.dispose();
+		}
+
 		deleteActionHandler.init(null);
 
 		domain.dispose();
@@ -149,21 +168,9 @@ public Object getAdapter(final Class key) {
 		// contribute to Properties view
 		else if (IPropertySheetPage.class.equals(key)) {
 			if (propertySheetPage == null) {
-				// TODO: use assisted inject here, so UndoablePropertySheetPage
-				// can be bound in module
-				propertySheetPage = new UndoablePropertySheetPage(
-						(IOperationHistory) getAdapter(IOperationHistory.class),
-						(IUndoContext) getAdapter(IUndoContext.class),
-						(UndoRedoActionGroup) getAdapter(
-								UndoRedoActionGroup.class));
+				propertySheetPage = createPropertySheetPage();
 			}
 			return propertySheetPage;
-		} else if (UndoRedoActionGroup.class.equals(key)) {
-			if (undoRedoActionGroup == null) {
-				undoRedoActionGroup = new UndoRedoActionGroup(getSite(),
-						(IUndoContext) getAdapter(IUndoContext.class), true);
-			}
-			return undoRedoActionGroup;
 		} else if (IUndoContext.class.equals(key)) {
 			return domain.getUndoContext();
 		} else if (IOperationHistory.class.equals(key)) {
@@ -183,15 +190,6 @@ protected FXCanvas getCanvas() {
 		return canvas;
 	}
 
-	/**
-	 * Returns the {@link FXDomain} that was previously injected.
-	 *
-	 * @return The {@link FXDomain} that was previously injected.
-	 */
-	protected FXDomain getDomain() {
-		return domain;
-	}
-
 	/**
 	 * Returns the {@link FXViewer} of the {@link FXDomain} that was previously
 	 * injected.
@@ -200,8 +198,17 @@ protected FXDomain getDomain() {
 	 *         injected.
 	 */
 	protected FXViewer getContentViewer() {
-		return domain.getAdapter(AdapterKey.get(FXViewer.class,
-				FXDomain.CONTENT_VIEWER_ROLE));
+		return domain.getAdapter(
+				AdapterKey.get(FXViewer.class, FXDomain.CONTENT_VIEWER_ROLE));
+	}
+
+	/**
+	 * Returns the {@link FXDomain} that was previously injected.
+	 *
+	 * @return The {@link FXDomain} that was previously injected.
+	 */
+	protected FXDomain getDomain() {
+		return domain;
 	}
 
 	/**
@@ -222,16 +229,12 @@ protected void hookViewers() {
 	public void init(final IViewSite site) throws PartInitException {
 		super.init(site);
 
-		final UndoRedoActionGroup undoRedoActionGroup = (UndoRedoActionGroup) getAdapter(
-				UndoRedoActionGroup.class);
-
-		if (undoRedoActionGroup != null) {
-			undoRedoActionGroup.fillActionBars(site.getActionBars());
-		}
+		undoRedoActionGroup = new UndoRedoActionGroup(getSite(),
+				(IUndoContext) getAdapter(IUndoContext.class), true);
+		undoRedoActionGroup.fillActionBars(site.getActionBars());
 
 		deleteActionHandler = new DeleteActionHandler();
 		deleteActionHandler.init(getContentViewer());
-
 		site.getActionBars().setGlobalActionHandler(
 				ActionFactory.DELETE.getId(), deleteActionHandler);
 

diff --git a/org.eclipse.gef4.mvc.ui-feature/feature.xml b/org.eclipse.gef4.mvc.ui-feature/feature.xml
@@ -22,6 +22,7 @@
 
    <requires>
       <import plugin="com.google.inject" version="3.0.0" match="compatible"/>
+      <import plugin="com.google.inject.assistedinject" version="3.0.0" match="compatible"/>
       <import feature="org.eclipse.gef4.mvc" version="1.0.0" match="equivalent"/>
       <import feature="org.eclipse.fx.runtime.min.feature" version="2.0.0" match="greaterOrEqual"/>
    </requires>

diff --git a/org.eclipse.gef4.mvc.ui/META-INF/MANIFEST.MF b/org.eclipse.gef4.mvc.ui/META-INF/MANIFEST.MF
@@ -9,10 +9,11 @@ Require-Bundle: org.eclipse.core.runtime;bundle-version="[3.9.0,4.0.0)",
  org.eclipse.ui;bundle-version="[3.105.0,4.0.0)",
  org.eclipse.ui.views;bundle-version="[3.6.0,4.0.0)",
  org.eclipse.gef4.mvc;bundle-version="[1.0.0,2.0.0)"
-Export-Package: org.eclipse.gef4.mvc.ui,
- org.eclipse.gef4.mvc.ui.parts,
+Export-Package: org.eclipse.gef4.mvc.ui;uses:="com.google.inject",
+ org.eclipse.gef4.mvc.ui.parts;uses:="org.eclipse.jface.viewers",
  org.eclipse.gef4.mvc.ui.properties
 Bundle-ActivationPolicy: lazy
 Bundle-Activator: org.eclipse.gef4.mvc.ui.MvcUiBundle
 Import-Package: com.google.inject;version="[1.3.0,2.0.0)",
+ com.google.inject.assistedinject;version="[1.3.0,2.0.0)",
  com.google.inject.binder;version="[1.3.0,2.0.0)"
diff --git a/org.eclipse.gef4.mvc.ui/src/org/eclipse/gef4/mvc/ui/MvcUiModule.java b/org.eclipse.gef4.mvc.ui/src/org/eclipse/gef4/mvc/ui/MvcUiModule.java
@@ -13,10 +13,14 @@
 
 import org.eclipse.core.commands.operations.IOperationHistory;
 import org.eclipse.gef4.mvc.ui.parts.DefaultSelectionProvider;
+import org.eclipse.gef4.mvc.ui.properties.IPropertySheetPageFactory;
+import org.eclipse.gef4.mvc.ui.properties.UndoablePropertySheetPage;
 import org.eclipse.jface.viewers.ISelectionProvider;
 import org.eclipse.ui.PlatformUI;
+import org.eclipse.ui.views.properties.IPropertySheetPage;
 
 import com.google.inject.AbstractModule;
+import com.google.inject.assistedinject.FactoryModuleBuilder;
 
 /**
  * The {@link MvcUiModule} contains Eclipse UI specific bindings in the context
@@ -36,6 +40,17 @@ protected void bindIOperationHistory() {
 				.getWorkbench().getOperationSupport().getOperationHistory());
 	}
 
+	/**
+	 * Binds a factory for assisted injection of
+	 * {@link UndoablePropertySheetPage} as {@link IPropertySheetPage}.
+	 */
+	protected void bindIPropertySheetPageFactory() {
+		install(new FactoryModuleBuilder()
+				.implement(IPropertySheetPage.class,
+						UndoablePropertySheetPage.class)
+				.build(IPropertySheetPageFactory.class));
+	}
+
 	/**
 	 * Binds {@link ISelectionProvider} to {@link DefaultSelectionProvider}.
 	 */
@@ -49,6 +64,6 @@ protected void configure() {
 		// bindings related to workbench integration
 		bindISelectionProvider();
 		bindIOperationHistory();
+		bindIPropertySheetPageFactory();
 	}
-
 }
diff --git a/...eclipse.gef4.mvc.ui/src/org/eclipse/gef4/mvc/ui/properties/IPropertySheetPageFactory.java b/...eclipse.gef4.mvc.ui/src/org/eclipse/gef4/mvc/ui/properties/IPropertySheetPageFactory.java
@@ -0,0 +1,34 @@
+/*******************************************************************************
+ * Copyright (c) 2016 itemis AG and others.
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ *     Alexander Nyßen (itemis AG) - initial API and implementation
+ *
+ *******************************************************************************/
+package org.eclipse.gef4.mvc.ui.properties;
+
+import org.eclipse.ui.IWorkbenchPart;
+import org.eclipse.ui.views.properties.IPropertySheetPage;
+
+/**
+ * A factory to create a new {@link IPropertySheetPage}.
+ *
+ * @author anyssen
+ */
+public interface IPropertySheetPageFactory {
+
+	/**
+	 * Creates a new {@link IPropertySheetPage} for the given
+	 * {@link IWorkbenchPart}.
+	 *
+	 * @param workbenchPart
+	 *            The {@link IWorkbenchPart} this {@link IPropertySheetPage} is
+	 *            related to.
+	 * @return A new {@link IPropertySheetPage} instance.
+	 */
+	public IPropertySheetPage create(IWorkbenchPart workbenchPart);
+}