From e9316021b415e59120a186a649604bf8ecffc513 Mon Sep 17 00:00:00 2001 From: Henri Sara Date: Fri, 11 Aug 2017 10:29:07 +0300 Subject: Ensure wrong Action is not executed after detaching a component (#9806) Due to action key generation on ActionManager a wrong action may be executed if the component that fired the wanted action is already detached. This patch makes action keys globally unique, simplifying the approach of #8495 but reusing its tests. Fixes #5864 --- .../main/java/com/vaadin/event/ActionManager.java | 22 ++++++++++-- .../src/main/java/com/vaadin/server/KeyMapper.java | 42 ++++++++++++++++------ 2 files changed, 51 insertions(+), 13 deletions(-) (limited to 'server/src') diff --git a/server/src/main/java/com/vaadin/event/ActionManager.java b/server/src/main/java/com/vaadin/event/ActionManager.java index 1056106b16..fbc5c8411f 100644 --- a/server/src/main/java/com/vaadin/event/ActionManager.java +++ b/server/src/main/java/com/vaadin/event/ActionManager.java @@ -19,6 +19,7 @@ import java.util.Arrays; import java.util.HashSet; import java.util.LinkedHashSet; import java.util.Map; +import java.util.concurrent.atomic.AtomicInteger; import com.vaadin.event.Action.Container; import com.vaadin.event.Action.Handler; @@ -166,14 +167,16 @@ public class ActionManager * removed but still exist on client side */ if (!actions.isEmpty() || clientHasActions) { - actionMapper = new KeyMapper<>(); + actionMapper = new ActionKeyMapper(); paintTarget.addVariable((VariableOwner) viewer, "action", ""); paintTarget.startTag("actions"); for (final Action a : actions) { + paintTarget.startTag("action"); final String akey = actionMapper.key(a); + paintTarget.addAttribute("key", akey); if (a.getCaption() != null) { paintTarget.addAttribute("caption", a.getCaption()); @@ -244,7 +247,6 @@ public class ActionManager LinkedHashSet actions = new LinkedHashSet<>(); if (ownActions != null) { actions.addAll(ownActions); - } if (actionHandlers != null) { for (Action.Handler h : actionHandlers) { @@ -256,4 +258,20 @@ public class ActionManager } return actions; } + + /** + * Extension of KeyMapper that avoids reusing keys even across different + * instances. + * + * @since 8.1.2 + */ + private static class ActionKeyMapper extends KeyMapper { + private static AtomicInteger lastKey = new AtomicInteger(0); + + @Override + protected String createKey() { + return String.valueOf(lastKey.incrementAndGet()); + } + } + } diff --git a/server/src/main/java/com/vaadin/server/KeyMapper.java b/server/src/main/java/com/vaadin/server/KeyMapper.java index f7b878dd27..d20eb7c605 100644 --- a/server/src/main/java/com/vaadin/server/KeyMapper.java +++ b/server/src/main/java/com/vaadin/server/KeyMapper.java @@ -43,9 +43,11 @@ public class KeyMapper implements DataKeyMapper, Serializable { /** * Constructs a new mapper * - * @param identifierGetter has to return a unique key for every bean, and the returned key has to - * follow general {@code hashCode()} and {@code equals()} contract, - * see {@link Object#hashCode()} for details. + * @param identifierGetter + * has to return a unique key for every bean, and the returned + * key has to follow general {@code hashCode()} and + * {@code equals()} contract, see {@link Object#hashCode()} for + * details. * @since 8.1 */ public KeyMapper(ValueProvider identifierGetter) { @@ -62,7 +64,8 @@ public class KeyMapper implements DataKeyMapper, Serializable { /** * Gets key for an object. * - * @param o the object. + * @param o + * the object. */ @Override public String key(V o) { @@ -79,13 +82,25 @@ public class KeyMapper implements DataKeyMapper, Serializable { } // If the object is not yet mapped, map it - key = String.valueOf(++lastKey); + key = createKey(); objectIdKeyMap.put(id, key); keyObjectMap.put(key, o); return key; } + /** + * Creates a key for a new item. + * + * This method can be overridden to customize the keys used. + * + * @return new key + * @since 8.1.2 + */ + protected String createKey() { + return String.valueOf(++lastKey); + } + @Override public boolean has(V o) { return objectIdKeyMap.containsKey(identifierGetter.apply(o)); @@ -94,7 +109,8 @@ public class KeyMapper implements DataKeyMapper, Serializable { /** * Retrieves object with the key. * - * @param key the name with the desired value. + * @param key + * the name with the desired value. * @return the object with the key. */ @Override @@ -105,11 +121,13 @@ public class KeyMapper implements DataKeyMapper, Serializable { /** * Removes object from the mapper. * - * @param removeobj the object to be removed. + * @param removeobj + * the object to be removed. */ @Override public void remove(V removeobj) { - final String key = objectIdKeyMap.remove(identifierGetter.apply(removeobj)); + final String key = objectIdKeyMap + .remove(identifierGetter.apply(removeobj)); if (key != null) { keyObjectMap.remove(key); } @@ -127,9 +145,10 @@ public class KeyMapper implements DataKeyMapper, Serializable { /** * Checks if the given key is mapped to an object. * - * @param key the key to check + * @param key + * the key to check * @return true if the key is currently mapped, - * false otherwise + * false otherwise * @since 7.7 */ public boolean containsKey(String key) { @@ -151,7 +170,8 @@ public class KeyMapper implements DataKeyMapper, Serializable { this.identifierGetter = identifierGetter; objectIdKeyMap.clear(); for (Map.Entry entry : keyObjectMap.entrySet()) { - objectIdKeyMap.put(identifierGetter.apply(entry.getValue()),entry.getKey()); + objectIdKeyMap.put(identifierGetter.apply(entry.getValue()), + entry.getKey()); } } } -- cgit v1.2.3