Bug 495567 - allow "saveable" parts define dirty state behavior

Implementation for solution drafted in bug 495567 comment 18.

This patch makes the "dirty" state propagation behavior for Properties
view configurable. Per default, Properties view will not be "dirty".
After applying this patch bug 495567 can be considered as fixed.

To be backwards compatible, "dirty" state propagation behavior is
implemented as "opt-in": per default, framework do not shows the "dirty"
state for Properties view (or any of ISecondarySaveableSource
instances), even if the connected ISaveablePart is dirty. To do so, a
new interface "org.eclipse.ui.internal.ISecondarySaveableSource" is
added, which
implements isDirtyStateSupported() method and by default returns
"false".

PropertySheet implements this ISecondarySaveableSource interface now and
re-implements this in the way that it delegates the
isDirtyStateSupported() decision to the PropertySheetPage / connected
ISaveablePart. Therefore, if desired by a some tool/workflow, the
"dirty" state tracking of ISaveablePart can be enabled for Properties
view.

The default "false" can be changed to "true" by:
1) Contributing custom IPropertySheetPage implementation to the tracked
part and overriding getAdapter() to return custom
ISecondarySaveableSource.
2) Contributing custom ISecondarySaveableSource adapter to the tracked
part.
3) Contributing global IAdapterFactory which provides custom adapter to
ISecondarySaveableSource (not recommended, because if not properly
implemented this can affect unrelated code).

Change-Id: Iab67d0404a86a154340ba95b061adf40879704dc
Signed-off-by: Andrey Loskutov <loskutov@gmx.de>

10 files changed, 343 insertions, 8 deletions

diff --git a/bundles/org.eclipse.ui.views/.settings/.api_filters b/bundles/org.eclipse.ui.views/.settings/.api_filters
new file mode 100644
index 00000000000..beab339bd86
--- /dev/null
+++ b/bundles/org.eclipse.ui.views/.settings/.api_filters
@@ -0,0 +1,11 @@
+<?xml version="1.0" encoding="UTF-8" standalone="no"?>
+<component id="org.eclipse.ui.views" version="2">
+    <resource path="src/org/eclipse/ui/views/properties/PropertySheet.java" type="org.eclipse.ui.views.properties.PropertySheet">
+        <filter comment="See bug 495567" id="576725006">
+            <message_arguments>
+                <message_argument value="ISecondarySaveableSource"/>
+                <message_argument value="PropertySheet"/>
+            </message_arguments>
+        </filter>
+    </resource>
+</component>
diff --git a/bundles/org.eclipse.ui.views/src/org/eclipse/ui/views/properties/PropertySheet.java b/bundles/org.eclipse.ui.views/src/org/eclipse/ui/views/properties/PropertySheet.java
index dc979368f24..19cb34944e9 100644
--- a/bundles/org.eclipse.ui.views/src/org/eclipse/ui/views/properties/PropertySheet.java
+++ b/bundles/org.eclipse.ui.views/src/org/eclipse/ui/views/properties/PropertySheet.java
@@ -18,6 +18,7 @@ package org.eclipse.ui.views.properties;
 import java.util.HashSet;
 
 import org.eclipse.core.runtime.Adapters;
+import org.eclipse.core.runtime.IAdaptable;
 import org.eclipse.core.runtime.IConfigurationElement;
 import org.eclipse.core.runtime.IExtension;
 import org.eclipse.core.runtime.IExtensionPoint;
@@ -50,6 +51,7 @@ import org.eclipse.ui.PartInitException;
 import org.eclipse.ui.Saveable;
 import org.eclipse.ui.SaveablesLifecycleEvent;
 import org.eclipse.ui.internal.DefaultSaveable;
+import org.eclipse.ui.internal.ISecondarySaveableSource;
 import org.eclipse.ui.internal.SaveablesList;
 import org.eclipse.ui.internal.views.properties.PropertiesMessages;
 import org.eclipse.ui.part.IContributedContentsView;
@@ -97,7 +99,8 @@ import org.eclipse.ui.part.ShowInContext;
  * @noinstantiate This class is not intended to be instantiated by clients.
  * @noextend This class is not intended to be subclassed by clients.
  */
-public class PropertySheet extends PageBookView implements ISelectionListener, IShowInTarget, IShowInSource, IRegistryEventListener {
+public class PropertySheet extends PageBookView
+		implements ISelectionListener, IShowInTarget, IShowInSource, IRegistryEventListener, ISecondarySaveableSource {
     /**
      * No longer used but preserved to avoid api change
      */
@@ -145,7 +148,8 @@ public class PropertySheet extends PageBookView implements ISelectionListener, I
 
 		@Override
 		public void handleLifecycleEvent(SaveablesLifecycleEvent event) {
-			if (currentPart == null || event.getEventType() != SaveablesLifecycleEvent.DIRTY_CHANGED) {
+			if (currentPart == null || event.getEventType() != SaveablesLifecycleEvent.DIRTY_CHANGED
+					|| !isDirtyStateSupported()) {
 				return;
 			}
 			// to avoid endless loop we must ignore our own instance which
@@ -473,7 +477,59 @@ public class PropertySheet extends PageBookView implements ISelectionListener, I
 		firePropertyChange(IWorkbenchPartConstants.PROP_DIRTY);
 	}
 
-    /**
+	/**
+	 * Defines the dirty state indication behavior of the {@link PropertySheet}
+	 * instance for the current tracked part if it is a {@link ISaveablePart}
+	 * instance or provides an adapter to {@link ISaveablePart}.
+	 * <p>
+	 * Default return value is {@code false} - the Properties view will not show
+	 * the '*' sign if the tracked part is dirty.
+	 * <p>
+	 * This behavior can be changed by either contributing custom
+	 * {@link IPropertySheetPage} to the tracked part, or providing
+	 * {@link ISecondarySaveableSource} adapter by the tracked part or by
+	 * contributing {@link ISecondarySaveableSource} adapter to the
+	 * {@link PropertySheet} class.
+	 * <p>
+	 * Strategy for the search is going from the smallest scope to the global
+	 * scope, searching for the first {@link ISecondarySaveableSource} adapter.
+	 * <p>
+	 * The current page is asked for the {@link ISecondarySaveableSource}
+	 * adapter first, if the adapter is not defined, the current tracked part is
+	 * asked for it, and finally the platform adapter manager is consulted. The
+	 * first adapter found in the steps above defines the return value of this
+	 * method.
+	 * <p>
+	 * If the contributed page wants change the behavior The page must implement
+	 * {@link IAdaptable} and return adapter to
+	 * {@link ISecondarySaveableSource}.
+	 *
+	 * @return returns {@code false} by default.
+	 * @since 3.9
+	 */
+	@Override
+	public boolean isDirtyStateSupported() {
+		if (currentPart == null) {
+			return false;
+		}
+		// first: ask page if we should show dirty state
+		ISecondarySaveableSource source = getAdapter(ISecondarySaveableSource.class);
+		if (source != null && source != this) {
+			return source.isDirtyStateSupported();
+		}
+		// second: ask the tracked part if the part provides the adapter;
+		// platform adapter manager is asked in the last step
+		source = Adapters.adapt(currentPart, ISecondarySaveableSource.class);
+		if (source != null && source != this) {
+			return source.isDirtyStateSupported();
+		}
+
+		// TODO delegate to default implementation if bug 490988 is fixed
+		// return ISecondarySaveableSource.super.isDirtyStateIndicationSupported();
+		return false;
+	}
+
+	/**
 	 * The <code>PropertySheet</code> implementation of this
 	 * <code>PageBookView</code> method handles the <code>ISaveablePart</code>
 	 * adapter case by calling <code>getSaveablePart()</code>.
diff --git a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/ISecondarySaveableSource.java b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/ISecondarySaveableSource.java
new file mode 100644
index 00000000000..499d8c63a50
--- /dev/null
+++ b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/ISecondarySaveableSource.java
@@ -0,0 +1,54 @@
+/*******************************************************************************
+ * Copyright (c) 2016 Andrey Loskutov <loskutov@gmx.de>.
+ * 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:
+ *     Andrey Loskutov <loskutov@gmx.de> - initial API and implementation
+ *******************************************************************************/
+
+package org.eclipse.ui.internal;
+
+import org.eclipse.core.runtime.IAdaptable;
+import org.eclipse.ui.ISaveablePart;
+
+/**
+ * Interface for parts providing an adapter to {@link ISaveablePart} objects
+ * created or managed originally by other parts.
+ * <p>
+ * In case the same {@link ISaveablePart} object is created originally by a
+ * "primary" part and shown or edited by multiple parts, the "primary" part
+ * might want be the only UI element showing the "dirty" state in the UI.
+ * <p>
+ * This interface allows "primary" parts define the default behavior for all
+ * "secondary" parts; and allows "secondary" parts to override this and decide
+ * how they should behave and how they should be represented in the UI.
+ * <p>
+ * <li>Parts implementing this interface directly are considered to be
+ * "secondary" parts and define only their own behavior.
+ * <li>Parts can also provide an adapter to this interface via
+ * {@link IAdaptable#getAdapter(Class)}. If such part is not implementing this
+ * interface directly, it can considered as primary "source" part, and can
+ * define a default behavior for all secondary parts.
+ * <p>
+ * Per default, dirty state of "secondary" parts is ignored by the framework.
+ *
+ */
+public interface ISecondarySaveableSource {
+
+	/**
+	 * Whether the dirty state changes should be supported by the framework if
+	 * the part directly implements {@link ISecondarySaveableSource}.
+	 * <p>
+	 * If the part providing the adapter is not implementing
+	 * {@link ISecondarySaveableSource}, return value defines the default
+	 * behavior of "secondary" parts connected to this part.
+	 *
+	 * @return default implementation returns {@code false}
+	 */
+	default boolean isDirtyStateSupported() {
+		return false;
+	}
+}
diff --git a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/SaveableHelper.java b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/SaveableHelper.java
index 0759ae8433d..058bcfac3f2 100644
--- a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/SaveableHelper.java
+++ b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/SaveableHelper.java
@@ -516,4 +516,11 @@ public class SaveableHelper {
 		return getSaveable2(o) != null;
 	}
 
+	public static boolean isDirtyStateSupported(IWorkbenchPart part) {
+		if (part instanceof ISecondarySaveableSource) {
+			return ((ISecondarySaveableSource) part).isDirtyStateSupported();
+		}
+		return isSaveable(part);
+	}
+
 }
diff --git a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/e4/compatibility/CompatibilityPart.java b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/e4/compatibility/CompatibilityPart.java
index a43bbf9159d..bb499fc44fb 100644
--- a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/e4/compatibility/CompatibilityPart.java
+++ b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/e4/compatibility/CompatibilityPart.java
@@ -351,7 +351,7 @@ public abstract class CompatibilityPart implements ISelectionChangedListener {
 		}
 
 		ISaveablePart saveable = SaveableHelper.getSaveable(wrapped);
-		if (saveable != null) {
+		if (saveable != null && SaveableHelper.isDirtyStateSupported(wrapped)) {
 			part.setDirty(saveable.isDirty());
 		}
 
@@ -373,6 +373,11 @@ public abstract class CompatibilityPart implements ISelectionChangedListener {
 					}
 					break;
 				case IWorkbenchPartConstants.PROP_DIRTY:
+					boolean supportsDirtyState = SaveableHelper.isDirtyStateSupported(wrapped);
+					if (!supportsDirtyState) {
+						part.setDirty(false);
+						return;
+					}
 					ISaveablePart saveable = SaveableHelper.getSaveable(wrapped);
 					if (saveable != null) {
 						part.setDirty(saveable.isDirty());
@@ -388,6 +393,7 @@ public abstract class CompatibilityPart implements ISelectionChangedListener {
 					break;
 				}
 			}
+
 		});
 	}
 
diff --git a/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/api/MockPart.java b/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/api/MockPart.java
index c3c788308cf..6c8caaf99ef 100644
--- a/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/api/MockPart.java
+++ b/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/api/MockPart.java
@@ -153,10 +153,11 @@ public class MockPart extends EventManager implements IExecutableExtension {
         callTrace.add("setFocus");
     }
 
-    /**
-     * @see IAdaptable#getAdapter(Class)
-     */
-    public Object getAdapter(Class arg0) {
+	/**
+	 * @param adapter
+	 * @see IAdaptable#getAdapter(Class)
+	 */
+	public <T> T getAdapter(Class<T> adapter) {
         return null;
     }
 
diff --git a/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/propertysheet/AdaptingSaveableView.java b/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/propertysheet/AdaptingSaveableView.java
new file mode 100644
index 00000000000..52ac97f8c5c
--- /dev/null
+++ b/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/propertysheet/AdaptingSaveableView.java
@@ -0,0 +1,75 @@
+/*******************************************************************************
+ * Copyright (c) 2016 Andrey Loskutov <loskutov@gmx.de>.
+ * 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:
+ *     Andrey Loskutov <loskutov@gmx.de> - initial API and implementation
+ *******************************************************************************/
+
+package org.eclipse.ui.tests.propertysheet;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import org.eclipse.core.runtime.IProgressMonitor;
+import org.eclipse.jface.viewers.StructuredSelection;
+import org.eclipse.ui.ISaveablePart;
+import org.eclipse.ui.tests.api.MockViewPart;
+
+/**
+ * A view which implements {@link ISaveablePart}
+ *
+ * @since 3.5
+ */
+public class AdaptingSaveableView extends MockViewPart implements ISaveablePart {
+
+	@SuppressWarnings("hiding")
+	public static final String ID = AdaptingSaveableView.class.getName();
+
+	private Map<Object, Object> adaptersMap;
+
+	public boolean dirty;
+
+	public AdaptingSaveableView() {
+		adaptersMap = new HashMap<>();
+	}
+
+	public <T> void setAdapter(Class<T> clazz, T adapter) {
+		adaptersMap.put(clazz, adapter);
+	}
+
+	@Override
+	public <T> T getAdapter(Class<T> adapter) {
+		return adapter.cast(adaptersMap.get(adapter));
+	}
+
+	public void setSelection(Object selection) {
+		getSelectionProvider().setSelection(new StructuredSelection(selection));
+	}
+
+	@Override
+	public void doSave(IProgressMonitor monitor) {
+	}
+
+	@Override
+	public void doSaveAs() {
+	}
+
+	@Override
+	public boolean isDirty() {
+		return dirty;
+	}
+
+	@Override
+	public boolean isSaveAsAllowed() {
+		return false;
+	}
+
+	@Override
+	public boolean isSaveOnCloseNeeded() {
+		return false;
+	}
+}
diff --git a/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/propertysheet/DirtyStatePropertySheetTest.java b/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/propertysheet/DirtyStatePropertySheetTest.java
new file mode 100644
index 00000000000..608aab331de
--- /dev/null
+++ b/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/propertysheet/DirtyStatePropertySheetTest.java
@@ -0,0 +1,119 @@
+/*******************************************************************************
+ * Copyright (c) 2016 Andrey Loskutov <loskutov@gmx.de>.
+ * 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:
+ *     Andrey Loskutov <loskutov@gmx.de> - initial API and implementation
+ *******************************************************************************/
+
+package org.eclipse.ui.tests.propertysheet;
+
+import static org.eclipse.ui.internal.SaveableHelper.isDirtyStateSupported;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import org.eclipse.core.runtime.IAdapterFactory;
+import org.eclipse.core.runtime.Platform;
+import org.eclipse.ui.IPageLayout;
+import org.eclipse.ui.ISaveablePart;
+import org.eclipse.ui.internal.ISecondarySaveableSource;
+import org.eclipse.ui.views.properties.PropertySheet;
+import org.eclipse.ui.views.properties.PropertySheetPage;
+import org.junit.Test;
+
+/**
+ * @since 3.5
+ */
+public class DirtyStatePropertySheetTest extends AbstractPropertySheetTest {
+
+    private AdaptingSaveableView saveableView;
+    private ISecondarySaveableSource dirtyDisallowed;
+    private ISecondarySaveableSource dirtyAllowed;
+    private MockAdapterFactory adapterFactory;
+
+    static class MockAdapterFactory implements IAdapterFactory {
+
+        private Map<Class<?>, Object> adaptersMap;
+
+        public MockAdapterFactory() {
+            adaptersMap = new HashMap<>();
+        }
+
+        @Override
+        public Class<?>[] getAdapterList() {
+            return adaptersMap.keySet().toArray(new Class[0]);
+        }
+
+        public <T> void setAdapter(Class<T> clazz, T adapter) {
+            adaptersMap.put(clazz, adapter);
+        }
+
+        @Override
+        public <T> T getAdapter(Object adaptableObject, Class<T> adapterType) {
+            return adapterType.cast(adaptersMap.get(adapterType));
+        }
+    }
+
+    public DirtyStatePropertySheetTest(String testName) {
+        super(testName);
+    }
+
+    @Override
+    protected void doSetUp() throws Exception {
+        super.doSetUp();
+        PropertySheetPerspectiveFactory.applyPerspective(activePage);
+        dirtyDisallowed = new ISecondarySaveableSource() {
+        };
+        dirtyAllowed = new ISecondarySaveableSource() {
+            @Override
+            public boolean isDirtyStateSupported() {
+                return true;
+            }
+        };
+        adapterFactory = new MockAdapterFactory();
+        propertySheet = (PropertySheet) activePage.showView(IPageLayout.ID_PROP_SHEET);
+        saveableView = (AdaptingSaveableView) activePage.showView(AdaptingSaveableView.ID);
+
+        // some basic checks
+        assertEquals(activePage.getActivePart(), saveableView);
+        PropertySheetPage page = (PropertySheetPage) propertySheet.getCurrentPage();
+        assertEquals(saveableView, page.getAdapter(ISaveablePart.class));
+        assertEquals(saveableView, propertySheet.getAdapter(ISaveablePart.class));
+        assertFalse(propertySheet.isDirtyStateSupported());
+    }
+
+    @Override
+    protected void doTearDown() throws Exception {
+        Platform.getAdapterManager().unregisterAdapters(adapterFactory);
+        activePage.resetPerspective();
+        super.doTearDown();
+    }
+
+    @Test
+    public void testIsDirtyStateIndicationSupported() throws Exception {
+        assertTrue(isDirtyStateSupported(saveableView));
+
+        // override default for this view
+        saveableView.setAdapter(ISecondarySaveableSource.class, dirtyAllowed);
+        assertTrue(isDirtyStateSupported(propertySheet));
+
+        // check if we can also reset it back
+        saveableView.setAdapter(ISecondarySaveableSource.class, dirtyDisallowed);
+        assertFalse(isDirtyStateSupported(propertySheet));
+
+        // check if we can set global adapter, set to default first (no
+        // adapters)
+        saveableView.setAdapter(ISecondarySaveableSource.class, null);
+        assertFalse(isDirtyStateSupported(propertySheet));
+
+        // register a global adapter and test
+        adapterFactory.setAdapter(ISecondarySaveableSource.class, dirtyAllowed);
+        Platform.getAdapterManager().registerAdapters(adapterFactory, ISecondarySaveableSource.class);
+        assertTrue(isDirtyStateSupported(propertySheet));
+    }
+
+}
diff --git a/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/propertysheet/PropertySheetTestSuite.java b/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/propertysheet/PropertySheetTestSuite.java
index 8be88580659..53640bf79de 100644
--- a/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/propertysheet/PropertySheetTestSuite.java
+++ b/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/propertysheet/PropertySheetTestSuite.java
@@ -39,5 +39,6 @@ public class PropertySheetTestSuite extends TestSuite {
         addTest(new TestSuite(NewPropertySheetHandlerTest.class));
         addTest(new TestSuite(PropertySheetAuto.class));
         addTest(new TestSuite(ComboBoxPropertyDescriptorTest.class));
+        addTest(new TestSuite(DirtyStatePropertySheetTest.class));
     }
 }
diff --git a/tests/org.eclipse.ui.tests/plugin.xml b/tests/org.eclipse.ui.tests/plugin.xml
index e13135f8670..53caccd222c 100644
--- a/tests/org.eclipse.ui.tests/plugin.xml
+++ b/tests/org.eclipse.ui.tests/plugin.xml
@@ -493,6 +493,11 @@
             name="Text Control view"
             restorable="true">
       </view>
+      <view
+            class="org.eclipse.ui.tests.propertysheet.AdaptingSaveableView"
+            id="org.eclipse.ui.tests.propertysheet.AdaptingSaveableView"
+            name="AdaptingSaveableView">
+      </view>
 
    </extension>
    <extension