This is an automated email from the ASF dual-hosted git repository. sdedic pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/netbeans.git
The following commit(s) were added to refs/heads/master by this push: new 97b6f5e [NETBEANS-1985] Honour initial values of stateful delegate actions. (#1579) 97b6f5e is described below commit 97b6f5e3ac44870a0b85a4ab95a2750fdd9d8bb1 Author: Svatopluk Dedic <svatopluk.de...@oracle.com> AuthorDate: Wed Nov 13 14:15:57 2019 +0100 [NETBEANS-1985] Honour initial values of stateful delegate actions. (#1579) [NETBEANS-1985] Honour initial values of stateful delegate actions. --- .../src/org/openide/awt/{awt => }/ActionState.java | 0 .../src/org/openide/awt/ContextAction.java | 8 +- .../src/org/openide/awt/GeneralAction.java | 8 +- .../org/openide/awt/{awt => }/PropertyMonitor.java | 0 .../org/openide/awt/{awt => }/StatefulAction.java | 28 +++ .../org/openide/awt/ContextActionNonAWTTest.java | 16 ++ .../src/org/openide/awt/ContextActionTest.java | 231 ++++++++++++++++++++- 7 files changed, 283 insertions(+), 8 deletions(-) diff --git a/platform/openide.awt/src/org/openide/awt/awt/ActionState.java b/platform/openide.awt/src/org/openide/awt/ActionState.java similarity index 100% rename from platform/openide.awt/src/org/openide/awt/awt/ActionState.java rename to platform/openide.awt/src/org/openide/awt/ActionState.java diff --git a/platform/openide.awt/src/org/openide/awt/ContextAction.java b/platform/openide.awt/src/org/openide/awt/ContextAction.java index bea899b..3510e38 100644 --- a/platform/openide.awt/src/org/openide/awt/ContextAction.java +++ b/platform/openide.awt/src/org/openide/awt/ContextAction.java @@ -276,10 +276,7 @@ implements Action, ContextAwareAction, ChangeListener, Runnable { @Override public int hashCode() { - int t = type.hashCode(); - int m = selectMode.hashCode(); - int p = performer.hashCode(); - return (t << 2) + (m << 1) + p; + return Objects.hash(type, selectMode, performer, enableMonitor); } @Override @@ -293,7 +290,8 @@ implements Action, ContextAwareAction, ChangeListener, Runnable { return type.equals(c.type) && selectMode.equals(c.selectMode) && - performer.equals(c.performer); + performer.equals(c.performer) && + Objects.equals(enableMonitor, c.enableMonitor); } return false; } diff --git a/platform/openide.awt/src/org/openide/awt/GeneralAction.java b/platform/openide.awt/src/org/openide/awt/GeneralAction.java index 960caac..eda09c6 100644 --- a/platform/openide.awt/src/org/openide/awt/GeneralAction.java +++ b/platform/openide.awt/src/org/openide/awt/GeneralAction.java @@ -470,7 +470,13 @@ final class GeneralAction { } BaseDelAction other = copyDelegate(f, actionContext); if (attrs != null) { - other.attrs = new HashMap<String,Object>(attrs); + if (other.attrs == null) { + other.attrs = new HashMap<>(attrs); + } else { + for (String k : attrs.keySet()) { + other.attrs.putIfAbsent(k, attrs.get(k)); + } + } } return other; } diff --git a/platform/openide.awt/src/org/openide/awt/awt/PropertyMonitor.java b/platform/openide.awt/src/org/openide/awt/PropertyMonitor.java similarity index 100% rename from platform/openide.awt/src/org/openide/awt/awt/PropertyMonitor.java rename to platform/openide.awt/src/org/openide/awt/PropertyMonitor.java diff --git a/platform/openide.awt/src/org/openide/awt/awt/StatefulAction.java b/platform/openide.awt/src/org/openide/awt/StatefulAction.java similarity index 84% rename from platform/openide.awt/src/org/openide/awt/awt/StatefulAction.java rename to platform/openide.awt/src/org/openide/awt/StatefulAction.java index 8c2cd1f..a1bfa69 100644 --- a/platform/openide.awt/src/org/openide/awt/awt/StatefulAction.java +++ b/platform/openide.awt/src/org/openide/awt/StatefulAction.java @@ -19,6 +19,7 @@ package org.openide.awt; import java.util.Collections; +import java.util.Objects; import java.util.logging.Level; import javax.swing.Action; import org.openide.util.Lookup; @@ -82,6 +83,33 @@ final class StatefulAction<T> extends ContextAction<T> { } @Override + public int hashCode() { + int hash = super.hashCode(); + hash = 97 * hash + Objects.hashCode(this.checkValueMonitor); + return hash; + } + + /** + * Overrides ContextAction. Must found in the {@link #checkValueMonitor} otherwise + * two actions driven by different property could clash in {@link ContextManager} + * and won't get 'enable' notification when data appears. + * + * @param obj other object + * @return true, if equal + */ + @Override + public boolean equals(Object obj) { + if (!super.equals(obj)) { + return false; + } + final StatefulAction<?> other = (StatefulAction<?>) obj; + if (!Objects.equals(this.checkValueMonitor, other.checkValueMonitor)) { + return false; + } + return true; + } + + @Override public Object getValue(String key) { if (SELECTED_KEY.equals(key)) { LOG.log(Level.FINER, "Action {0} state: {1}", new Object[] { diff --git a/platform/openide.awt/test/unit/src/org/openide/awt/ContextActionNonAWTTest.java b/platform/openide.awt/test/unit/src/org/openide/awt/ContextActionNonAWTTest.java index 1697fd5..b053c15 100644 --- a/platform/openide.awt/test/unit/src/org/openide/awt/ContextActionNonAWTTest.java +++ b/platform/openide.awt/test/unit/src/org/openide/awt/ContextActionNonAWTTest.java @@ -52,6 +52,22 @@ public class ContextActionNonAWTTest extends ContextActionTest { SwingUtilities.invokeAndWait(run); return run.is; } + + @Override + protected boolean getIsChecked(final Action a1) throws InterruptedException, InvocationTargetException { + assertFalse("Not in AWT", EventQueue.isDispatchThread()); + + class R implements Runnable { + boolean is; + public void run() { + is = Boolean.TRUE.equals(a1.getValue(Action.SELECTED_KEY)); + } + } + R run = new R(); + SwingUtilities.invokeAndWait(run); + return run.is; + } + protected void doActionPerformed(final Action a1, final ActionEvent ev) throws InterruptedException, InvocationTargetException { assertFalse("Not in AWT", EventQueue.isDispatchThread()); diff --git a/platform/openide.awt/test/unit/src/org/openide/awt/ContextActionTest.java b/platform/openide.awt/test/unit/src/org/openide/awt/ContextActionTest.java index 7208cc9..96938bf 100644 --- a/platform/openide.awt/test/unit/src/org/openide/awt/ContextActionTest.java +++ b/platform/openide.awt/test/unit/src/org/openide/awt/ContextActionTest.java @@ -23,26 +23,31 @@ import java.awt.EventQueue; import java.awt.event.ActionEvent; import java.beans.PropertyChangeEvent; import java.beans.PropertyChangeListener; +import java.beans.PropertyChangeSupport; import java.lang.ref.WeakReference; import java.lang.reflect.InvocationTargetException; import java.net.URL; import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; +import java.util.HashSet; import java.util.List; +import java.util.Set; import javax.swing.AbstractAction; import javax.swing.Action; import javax.swing.ActionMap; -import static junit.framework.Assert.assertFalse; -import static junit.framework.Assert.assertTrue; import org.netbeans.junit.NbTestCase; import org.openide.filesystems.FileObject; import org.openide.filesystems.FileUtil; import org.openide.util.ContextAwareAction; +import org.openide.util.ContextGlobalProvider; import org.openide.util.Lookup; +import org.openide.util.Utilities; import org.openide.util.lookup.AbstractLookup; import org.openide.util.lookup.InstanceContent; import org.openide.util.lookup.Lookups; import org.openide.util.lookup.ProxyLookup; +import org.openide.util.test.MockLookup; /** Test that cookie actions are in fact sensitive to the correct cookies in the * correct numbers, and that changes to either node selection or cookies on the @@ -66,6 +71,25 @@ implements Lookup.Provider, ContextActionEnabler<ContextActionTest.Openable> { private int expectedEnabledmentCount = 2; + private static class CGP implements ContextGlobalProvider, Lookup.Provider { + private final Lookup prx = Lookups.proxy(this); + + volatile Lookup current; + + @Override + public Lookup createGlobalContext() { + return prx; + } + + @Override + public Lookup getLookup() { + Lookup c = current; + return c != null ? c : Lookup.EMPTY; + } + } + + static CGP actionLookup = new CGP(); + @Override protected void setUp() throws Exception { lookup = Lookup.EMPTY; @@ -82,6 +106,18 @@ implements Lookup.Provider, ContextActionEnabler<ContextActionTest.Openable> { n4 = new LookupWithOpenable(n1.lookup(Openable.class)); // share the same cookie instance with n1 SimpleCookieAction.runOn.clear(); + + actionLookup.current = lookupProxy; + + MockLookup.setLookup(Lookups.metaInfServices(getClass().getClassLoader()), + lookupProxy, + Lookups.fixed(actionLookup)); + } + + @Override + protected void tearDown() throws Exception { + actionLookup.current = null; + super.tearDown(); } @Override @@ -619,6 +655,13 @@ implements Lookup.Provider, ContextActionEnabler<ContextActionTest.Openable> { return a1.isEnabled(); } + + protected boolean getIsChecked(final Action a1) throws InterruptedException, InvocationTargetException { + assertTrue("In AWT", EventQueue.isDispatchThread()); + + return Boolean.TRUE.equals(a1.getValue(Action.SELECTED_KEY)); + } + protected void doActionPerformed(final Action a1, final ActionEvent ev) throws InterruptedException, InvocationTargetException { assertTrue("In AWT", EventQueue.isDispatchThread()); a1.actionPerformed(ev); @@ -696,5 +739,189 @@ implements Lookup.Provider, ContextActionEnabler<ContextActionTest.Openable> { } } + private static class TestData { + + private boolean stateValue; + + public static final String PROP_STATEVALUE = "stateValue"; + + public boolean isStateValue() { + return stateValue; + } + + public void setStateValue(boolean stateValue) { + boolean oldStateValue = this.stateValue; + this.stateValue = stateValue; + propertyChangeSupport.firePropertyChange(PROP_STATEVALUE, oldStateValue, stateValue); + } + + private transient final PropertyChangeSupport propertyChangeSupport = new PropertyChangeSupport(this); + + public void addPropertyChangeListener(PropertyChangeListener listener) { + propertyChangeSupport.addPropertyChangeListener(listener); + } + + public void removePropertyChangeListener(PropertyChangeListener listener) { + propertyChangeSupport.removePropertyChangeListener(listener); + } + } + + /** + * Test action, whose state is driven by a model + */ + @ActionID(category = "Test", id = "CheckedTest") + @ActionRegistration(displayName = "Test Action", + checkedOn = @ActionState( + type = TestData.class, + property = "stateValue" + ) + ) + public static class CheckAction extends AbstractAction { + private final TestData data; + + public CheckAction(TestData data) { + this.data = data; + } + + @Override + public void actionPerformed(ActionEvent e) { + } + } + + /** + * Checks that a stateful action properly changes its state, based + * on the supplied data. + * @throws Exception + */ + public void testActionGlobalState() throws Exception { + Action baseA = Actions.forID("Test", "CheckedTest"); + + // baseAction is not enabled - no data: + assertFalse("No data, action must be disabled", getIsEnabled(baseA)); + // baseAction is also not checked: + assertFalse("No data, action cannot be checked", getIsChecked(baseA)); + + // now provide the action: + TestData data = new TestData(); + activate(Lookups.fixed(data)); + + assertTrue("Data was published, action must enable", getIsEnabled(baseA)); + assertFalse("Data is false, action must be unchedked", getIsChecked(baseA)); + + data.setStateValue(true); + + assertTrue(getIsEnabled(baseA)); + assertTrue("Data changed to true, action must be checked", getIsChecked(baseA)); + } + public void testActionGlobalStateStartUnchecked() throws Exception { + TestData data = new TestData(); + + activate(Lookups.fixed(data)); + Action baseA = Actions.forID("Test", "CheckedTest"); + + // baseAction is not enabled - no data: + assertTrue("Action must start up enabled", getIsEnabled(baseA)); + assertFalse("Data is false, action must be unchecked", getIsChecked(baseA)); + + data.setStateValue(true); + assertTrue("Data changed to true, action checked", getIsChecked(baseA)); + } + + public void testActionGlobalStateStartChecked() throws Exception { + TestData data = new TestData(); + data.setStateValue(true); + + activate(Lookups.fixed(data)); + Action baseA = Actions.forID("Test", "CheckedTest"); + + // baseAction is not enabled - no data: + assertTrue(getIsEnabled(baseA)); + // baseAction is also not checked: + assertTrue(getIsChecked(baseA)); + + data.setStateValue(false); + assertFalse("Data is false, action must be unchecked", getIsChecked(baseA)); + } + + public void testContextDelegate() throws Exception { + TestData data = new TestData(); + data.setStateValue(true); + + TestData otherData = new TestData(); + + + Action baseA = Actions.forID("Test", "CheckedTest"); + + // must change main action Lookup after the main delegate exists, + // so it gets property change and changes its Action.SELECTED_KEY + // otherwise the .map is null and action just delegates to Fallback + activate(Lookups.fixed(data)); + + assertSame(data, Utilities.actionsGlobalContext().lookup(TestData.class)); + + InstanceContent ic = new InstanceContent(); + Lookup context = new AbstractLookup(ic); + ic.add(data); + assertTrue(getIsEnabled(baseA)); + assertTrue(getIsChecked(baseA)); + + Action actionA = ((ContextAwareAction)baseA).createContextAwareInstance(context); + + assertTrue(getIsEnabled(actionA)); + assertTrue(getIsChecked(actionA)); + + // let's have completely different local context: + Lookup context2 = Lookups.fixed(otherData); + Action actionB = ((ContextAwareAction)baseA).createContextAwareInstance(context2); + assertTrue(getIsEnabled(actionB)); + // in this context, action should be enabled, but UNchecked. + assertFalse(getIsChecked(actionB)); + + class PCL implements PropertyChangeListener { + Set<String> propChanges = Collections.synchronizedSet(new HashSet<>()); + + @Override + public void propertyChange(PropertyChangeEvent evt) { + String n = evt.getPropertyName(); + if (n != null) { + propChanges.add(n); + } + } + } + + PCL listenerBase = new PCL(); + PCL listenerA = new PCL(); + PCL listenerB = new PCL(); + baseA.addPropertyChangeListener(listenerBase); + actionA.addPropertyChangeListener(listenerA); + actionB.addPropertyChangeListener(listenerB); + + TestData data3 = new TestData(); + // the data has property false, so actionA should fire & change, not the other ones + ic.set(Collections.singleton(data3), null); + + // also potentially replans to AWT, so the pending change event from lookup + // will be probably processed. + assertFalse(getIsChecked(actionA)); + assertTrue(getIsChecked(baseA)); + + assertTrue(listenerBase.propChanges.isEmpty()); + assertTrue(listenerB.propChanges.isEmpty()); + + assertTrue(listenerA.propChanges.contains(Action.SELECTED_KEY)); + + listenerA.propChanges.clear(); + + otherData.setStateValue(true); + + // again sync with AWT + assertTrue(getIsChecked(baseA)); + assertTrue(getIsChecked(actionB)); + + assertTrue(listenerBase.propChanges.isEmpty()); + assertTrue(listenerA.propChanges.isEmpty()); + + assertTrue(listenerB.propChanges.contains(Action.SELECTED_KEY)); + } } --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@netbeans.apache.org For additional commands, e-mail: commits-h...@netbeans.apache.org For further information about the NetBeans mailing lists, visit: https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists