This is an automated email from the ASF dual-hosted git repository. papegaaij pushed a commit to branch performance-improvements-10.x in repository https://gitbox.apache.org/repos/asf/wicket.git
commit f6415fce9eeb330f159caf1acd8161159de634db Author: Emond Papegaaij <[email protected]> AuthorDate: Fri Sep 11 22:05:34 2026 +0200 WICKET-6774: rework of component state A component's flexible state - model, behaviors and meta data - is no longer an Object[] with a packing convention but a ComponentState instance. One final class rather than one subclass per combination: the combinations all cost the same 24 bytes on a 64 bit VM with compressed oops, so specialised classes save nothing, while four implementations of the same six accessors make every unpacking call site megamorphic and stop it from being inlined. A behavior's id is now its index in that state rather than a position in a BehaviorIdList kept in the component's meta data, which removes the list entirely. Reading meta data on a link or ajax component gets cheaper as a side effect, because it no longer has to walk past that entry. Behaviour change: behavior ids are only maintained for stateful behaviors. Ids can change for other behaviors, also when combined on the same component. AbstractDefaultAjaxBehavior no longer forces an id at bind time; an id is add order by construction now, and forcing one would fix the behavior positions of every ajax-enabled component for its whole life. Backported from master. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> --- .../org/apache/wicket/BehaviorsDetachTest.java | 3 +- .../wicket/ComponentStateBehaviorMetaDataTest.java | 83 ++ .../org/apache/wicket/behavior/BehaviorTest.java | 21 + .../wicket/behavior/ImmutableBehaviorIdsTest.java | 16 +- .../html/basic/SimplePageExpectedResult_13.html | 2 +- .../src/main/java/org/apache/wicket/Behaviors.java | 367 --------- .../src/main/java/org/apache/wicket/Component.java | 322 ++------ .../java/org/apache/wicket/ComponentState.java | 871 +++++++++++++++++++++ .../wicket/ajax/AbstractDefaultAjaxBehavior.java | 6 - .../request/component/IRequestableComponent.java | 13 +- 10 files changed, 1063 insertions(+), 641 deletions(-) diff --git a/wicket-core-tests/src/test/java/org/apache/wicket/BehaviorsDetachTest.java b/wicket-core-tests/src/test/java/org/apache/wicket/BehaviorsDetachTest.java index ab9a6096f4..08599bd51d 100644 --- a/wicket-core-tests/src/test/java/org/apache/wicket/BehaviorsDetachTest.java +++ b/wicket-core-tests/src/test/java/org/apache/wicket/BehaviorsDetachTest.java @@ -22,7 +22,8 @@ import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.assertFalse; /** - * Tests for {@link Behaviors#detach(Component)} method + * Tests for behavior detaching, see + * {@link ComponentState#detachBehaviors(Component, Object, boolean, boolean)} */ class BehaviorsDetachTest extends WicketTestCase { diff --git a/wicket-core-tests/src/test/java/org/apache/wicket/ComponentStateBehaviorMetaDataTest.java b/wicket-core-tests/src/test/java/org/apache/wicket/ComponentStateBehaviorMetaDataTest.java new file mode 100644 index 0000000000..accc49f708 --- /dev/null +++ b/wicket-core-tests/src/test/java/org/apache/wicket/ComponentStateBehaviorMetaDataTest.java @@ -0,0 +1,83 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.wicket; + +import static org.junit.jupiter.api.Assertions.assertNull; + +import org.apache.wicket.behavior.Behavior; +import org.apache.wicket.markup.html.basic.Label; +import org.apache.wicket.util.tester.WicketTestCase; +import org.junit.jupiter.api.Test; + +/** + * A behavior can change the component's state from within its callbacks. Such a change replaces + * the component's state object, so it must not be reverted by the behavior bookkeeping that is + * running at the same time. See WICKET-6877. + */ +class ComponentStateBehaviorMetaDataTest extends WicketTestCase +{ + private static final MetaDataKey<Boolean> KEY = new MetaDataKey<>() + { + private static final long serialVersionUID = 1L; + }; + + /** clears the component's only meta data entry while being detached */ + private static class ClearMetaDataOnDetach extends Behavior + { + private static final long serialVersionUID = 1L; + + @Override + public void detach(Component component) + { + component.setMetaData(KEY, null); + super.detach(component); + } + } + + private static Label newLabelWithBehaviors(Behavior clearing) + { + Label label = new Label("id", "body"); + label.add(clearing, new Behavior() + { + private static final long serialVersionUID = 1L; + }); + label.setMetaData(KEY, Boolean.TRUE); + return label; + } + + @Test + void metaDataClearedWhileDetachingStaysCleared() + { + Label label = newLabelWithBehaviors(new ClearMetaDataOnDetach()); + + label.detach(); + + assertNull(label.getMetaData(KEY), "meta data cleared while detaching must stay cleared"); + } + + @Test + void metaDataClearedWhileRemovingStaysCleared() + { + ClearMetaDataOnDetach clearing = new ClearMetaDataOnDetach(); + Label label = newLabelWithBehaviors(clearing); + + // removing a behavior detaches it, which clears the meta data + label.remove(clearing); + + assertNull(label.getMetaData(KEY), "meta data cleared while removing must stay cleared"); + } +} diff --git a/wicket-core-tests/src/test/java/org/apache/wicket/behavior/BehaviorTest.java b/wicket-core-tests/src/test/java/org/apache/wicket/behavior/BehaviorTest.java index f02d502c9d..66045b75ca 100644 --- a/wicket-core-tests/src/test/java/org/apache/wicket/behavior/BehaviorTest.java +++ b/wicket-core-tests/src/test/java/org/apache/wicket/behavior/BehaviorTest.java @@ -89,6 +89,27 @@ class BehaviorTest extends WicketTestCase assertFalse(container.getBehaviors().contains(temp2)); } + /** + * {@link Component#add(Behavior...)} validates the whole argument list before it stores any of + * it. A behavior's id is its position in the component's list of behaviors, so a behavior that + * was already stored when a later {@code null} was rejected would keep the position it took and + * shift the id of everything added after it. + */ + @Test + void nullBehaviorIsRejectedBeforeAnythingIsStored() { + WebMarkupContainer container = new WebMarkupContainer("test"); + Behavior first = Behavior.onTag((c, tag) -> {}); + + assertThrows(IllegalArgumentException.class, () -> container.add((Behavior[])null)); + assertThrows(IllegalArgumentException.class, () -> container.add(first, null)); + + // the rejected call must leave no trace: a behavior stored before the null was found + // would keep the slot it took, shifting the id of everything added afterwards + assertTrue(container.getBehaviors().isEmpty()); + container.add(first); + assertEquals(0, container.getBehaviorId(first)); + } + public static class TestTemporaryBehavior extends Behavior { private static final long serialVersionUID = 1L; diff --git a/wicket-core-tests/src/test/java/org/apache/wicket/behavior/ImmutableBehaviorIdsTest.java b/wicket-core-tests/src/test/java/org/apache/wicket/behavior/ImmutableBehaviorIdsTest.java index 054093822d..1bb775eec2 100644 --- a/wicket-core-tests/src/test/java/org/apache/wicket/behavior/ImmutableBehaviorIdsTest.java +++ b/wicket-core-tests/src/test/java/org/apache/wicket/behavior/ImmutableBehaviorIdsTest.java @@ -75,10 +75,10 @@ class ImmutableBehaviorIdsTest extends WicketTestCase assertTrue(output.contains("autocomplete=\"off\"")); assertTrue(output.contains("class2=\"border\"")); assertTrue(output.contains("autocomplete2=\"off\"")); - assertTrue(output.contains(".0")); - assertTrue(output.contains(".1")); - assertEquals(link, page.getContainer().getBehaviorById(0)); - assertEquals(link2, page.getContainer().getBehaviorById(1)); + assertTrue(output.contains(".2")); + assertTrue(output.contains(".4")); + assertEquals(link, page.getContainer().getBehaviorById(2)); + assertEquals(link2, page.getContainer().getBehaviorById(4)); // if we remove a behavior that is before the ibehaviorlistener its url index should not // change @@ -90,10 +90,10 @@ class ImmutableBehaviorIdsTest extends WicketTestCase tester.startPage(page); output = tester.getLastResponseAsString(); // System.out.println(output); - assertTrue(output.contains(".0")); - assertTrue(output.contains(".1")); - assertEquals(link, page.getContainer().getBehaviorById(0)); - assertEquals(link2, page.getContainer().getBehaviorById(1)); + assertTrue(output.contains(".2")); + assertTrue(output.contains(".4")); + assertEquals(link, page.getContainer().getBehaviorById(2)); + assertEquals(link2, page.getContainer().getBehaviorById(4)); } /** diff --git a/wicket-core-tests/src/test/java/org/apache/wicket/markup/html/basic/SimplePageExpectedResult_13.html b/wicket-core-tests/src/test/java/org/apache/wicket/markup/html/basic/SimplePageExpectedResult_13.html index 65944097e1..b39d40f1e6 100644 --- a/wicket-core-tests/src/test/java/org/apache/wicket/markup/html/basic/SimplePageExpectedResult_13.html +++ b/wicket-core-tests/src/test/java/org/apache/wicket/markup/html/basic/SimplePageExpectedResult_13.html @@ -18,7 +18,7 @@ Wicket.Ajax.baseUrl="wicket/bookmarkable/org.apache.wicket.markup.html.basic.Sim <script type="text/javascript"> /*<![CDATA[*/ Wicket.Event.add(window, "domready", function(event) { -Wicket.Ajax.ajax({"u":"./org.apache.wicket.markup.html.basic.SimplePage_13?0-1.0-html","c":"html1","e":"click","pd":true});; +Wicket.Ajax.ajax({"u":"./org.apache.wicket.markup.html.basic.SimplePage_13?0-1.1-html","c":"html1","e":"click","pd":true});; Wicket.Event.publish(Wicket.Event.Topic.AJAX_HANDLERS_BOUND); ;}); /*]]>*/ diff --git a/wicket-core/src/main/java/org/apache/wicket/Behaviors.java b/wicket-core/src/main/java/org/apache/wicket/Behaviors.java deleted file mode 100644 index ed0298be07..0000000000 --- a/wicket-core/src/main/java/org/apache/wicket/Behaviors.java +++ /dev/null @@ -1,367 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You under the Apache License, Version 2.0 - * (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.apache.wicket; - -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; - -import org.apache.wicket.behavior.Behavior; -import org.apache.wicket.behavior.InvalidBehaviorIdException; -import org.apache.wicket.util.lang.Args; - -/** - * Manages behaviors for {@link Component} instances - * - * @author igor - */ -final class Behaviors -{ - - private Behaviors() - { - // utility class - } - - public static void add(Component component, Behavior... behaviors) - { - Args.notNull(behaviors, "behaviors"); - - for (Behavior behavior : behaviors) - { - Args.notNull(behavior, "behavior"); - - internalAdd(component, behavior); - - if (!behavior.isTemporary(component)) - { - component.addStateChange(); - } - - // Give handler the opportunity to bind this component - behavior.bind(component); - } - } - - private static void internalAdd(Component component, Behavior behavior) - { - component.data_add(behavior); - if (behavior.getStatelessHint(component) == false) - { - getBehaviorId(component, behavior); - } - } - - @SuppressWarnings("unchecked") - public static <M extends Behavior> List<M> getBehaviors(Component component, Class<M> type) - { - int len = component.data_length(); - if (len == 0) - { - return Collections.emptyList(); - } - int start = component.data_start(); - if (len < start) - { - return Collections.emptyList(); - } - - List<M> subset = null; - for (int i = start; i < len; i++) - { - Object obj = component.data_get(i); - if (obj instanceof Behavior) - { - if (type == null || type.isAssignableFrom(obj.getClass())) - { - if (subset == null) - { - subset = new ArrayList<>(len); - } - subset.add((M)obj); - } - } - } - if (subset == null || subset.isEmpty()) - { - return Collections.emptyList(); - } - else - { - return Collections.unmodifiableList(subset); - } - } - - - public static void remove(Component component, Behavior behavior) - { - Args.notNull(behavior, "behavior"); - - if (internalRemove(component, behavior)) - { - if (!behavior.isTemporary(component)) - { - component.addStateChange(); - } - behavior.detach(component); - } - else - { - throw new IllegalStateException( - "Tried to remove a behavior that was not added to the component. Behavior: " + - behavior.toString()); - } - } - - /** - * THIS IS WICKET INTERNAL ONLY. DO NOT USE IT. - * - * Traverses all behaviors and calls detachModel() on them. This is needed to cleanup behavior - * after render. This method is necessary for {@link org.apache.wicket.ajax.AjaxRequestTarget} to be able to cleanup - * component's behaviors after header contribution has been done (which is separated from - * component render). - */ - public static void detach(Component component) - { - int len = component.data_length(); - if (len == 0) - { - return; - } - int start = component.data_start(); - if (len < start) - { - return; - } - for (int i = start; i < len; i++) - { - Object obj = component.data_get(i); - if (obj instanceof Behavior) - { - final Behavior behavior = (Behavior)obj; - - behavior.detach(component); - - final int currentLength = component.data_length(); - if (len != currentLength) - { - // if the length has changed then reset 'i' and 'len' - for (int j = start; j < currentLength; j++) - { - // find the new index of the current behavior by identity - if (behavior == component.data_get(j)) - { - i = j; - len = currentLength; - break; - } - } - } - - if (behavior.isTemporary(component)) - { - internalRemove(component, behavior); - i--; - len--; - } - } - } - } - - private static boolean internalRemove(Component component, Behavior behavior) - { - final int len = component.data_length(); - for (int i = component.data_start(); i < len; i++) - { - Object o = component.data_get(i); - if (o != null && o.equals(behavior)) - { - component.data_remove(i); - behavior.unbind(component); - - // remove behavior from behavior-ids - ArrayList<Behavior> ids = getBehaviorsIdList(component, false); - if (ids != null) - { - int idx = ids.indexOf(behavior); - if (idx == ids.size() - 1) - { - ids.remove(idx); - } - else if (idx >= 0) - { - ids.set(idx, null); - } - ids.trimToSize(); - - if (ids.isEmpty()) - { - removeBehaviorsIdList(component); - } - - } - return true; - } - } - return false; - } - - private static void removeBehaviorsIdList(Component component) - { - for (int i = component.data_start(); i < component.data_length(); i++) - { - Object obj = component.data_get(i); - if (obj instanceof BehaviorIdList) - { - component.data_remove(i); - return; - } - } - } - - private static BehaviorIdList getBehaviorsIdList(Component component, boolean createIfNotFound) - { - int len = component.data_length(); - for (int i = component.data_start(); i < len; i++) - { - Object obj = component.data_get(i); - if (obj instanceof BehaviorIdList) - { - return (BehaviorIdList)obj; - } - } - if (createIfNotFound) - { - BehaviorIdList list = new BehaviorIdList(); - component.data_add(list); - return list; - } - return null; - } - - /** - * Called when the component is going to be removed. Notifies all - * behaviors assigned to this component. - * - * @param component - * the component that will be removed from its parent - */ - public static void onRemove(Component component) - { - int len = component.data_length(); - if (len == 0) - { - return; - } - int start = component.data_start(); - if (len < start) - { - return; - } - for (int i = start; i < len; i++) - { - Object obj = component.data_get(i); - if (obj instanceof Behavior) - { - final Behavior behavior = (Behavior)obj; - - behavior.onRemove(component); - } - } - } - - private static class BehaviorIdList extends ArrayList<Behavior> - { - private static final long serialVersionUID = 1L; - - public BehaviorIdList() - { - super(1); - } - } - - public static int getBehaviorId(Component component, Behavior behavior) - { - Args.notNull(behavior, "behavior"); - - boolean found = false; - for (int i = component.data_start(); i < component.data_length(); i++) - { - if (behavior == component.data_get(i)) - { - found = true; - break; - } - } - if (!found) - { - throw new IllegalStateException( - "Behavior must be added to component before its id can be generated. Behavior: " + - behavior + ", Component: " + component); - } - - ArrayList<Behavior> ids = getBehaviorsIdList(component, true); - - int id = ids.indexOf(behavior); - - if (id < 0) - { - // try to find an unused slot - for (int i = 0; i < ids.size(); i++) - { - if (ids.get(i) == null) - { - ids.set(i, behavior); - id = i; - break; - } - } - } - - if (id < 0) - { - // no unused slots, add to the end - id = ids.size(); - ids.add(behavior); - ids.trimToSize(); - } - - return id; - } - - public static Behavior getBehaviorById(Component component, int id) - { - Behavior behavior = null; - - ArrayList<Behavior> ids = getBehaviorsIdList(component, false); - if (ids != null) - { - if (id >= 0 && id < ids.size()) - { - behavior = ids.get(id); - } - } - - if (behavior != null) - { - return behavior; - } - throw new InvalidBehaviorIdException(component, id); - } - - -} diff --git a/wicket-core/src/main/java/org/apache/wicket/Component.java b/wicket-core/src/main/java/org/apache/wicket/Component.java index 371fbc9a70..9bef4432b0 100644 --- a/wicket-core/src/main/java/org/apache/wicket/Component.java +++ b/wicket-core/src/main/java/org/apache/wicket/Component.java @@ -382,11 +382,17 @@ public abstract class Component /** * Flag that determines whether the model is set. This is necessary because of the way we * represent component state ({@link #data}). We can't distinguish between model and behavior - * using instanceof, because one object can implement both interfaces. Thus we need this flag - - * when the flag is set, first object in {@link #data} is always model. + * using instanceof, because one object can implement both interfaces. */ private static final int FLAG_MODEL_SET = 0x100000; + /** + * Flag that is set when {@link #getBehaviorId(Behavior)} is called on this component. Once this + * flag is set, the indexes of all behaviors must remain fixed to keep the contract of + * {@link #getBehaviorId(Behavior)}. + */ + private static final int FLAG_BEHAVIOR_IDS_FIXED = 0x200000; + /** * Flag that restricts visibility of a component when set to true. This is usually used when a * component wants to restrict visibility of another component. Calling @@ -465,8 +471,7 @@ public abstract class Component /** * Instead of remembering the whole markupId, we just remember the number for this component so - * we can "reconstruct" the markupId on demand. While this could be part of {@link #data}, - * profiling showed that having it as separate property consumes less memory. + * we can "reconstruct" the markupId on demand. */ int generatedMarkupId = -1; @@ -489,170 +494,11 @@ public abstract class Component * <li>MetaDataEntry (optionally {@link MetaDataEntry}[] if more metadata entries are present) * * <li>{@link Behavior}(s) added to component. The behaviors are not stored in separate array, * they are part of the {@link #data} array (this is in order to save the space of the pointer - * to an empty array as most components have no behaviours). - FIXME - explain why - is this - * correct? + * to an empty array as most components have no behaviours). + * <li>A {@link ComponentState} if a combination of the attributes is set. * </ul> - * If there is only one attribute set (i.e. model or MetaDataEntry([]) or one behavior), the - * #data object points directly to value of that attribute. Otherwise the data is of type - * Object[] where the attributes are ordered as specified above. - * <p> */ - Object data = null; - - final int data_start() - { - return getFlag(FLAG_MODEL_SET) ? 1 : 0; - } - - final int data_length() - { - if (data == null) - { - return 0; - } - else if (data instanceof Object[] && !(data instanceof MetaDataEntry<?>[])) - { - return ((Object[])data).length; - } - else - { - return 1; - } - } - - final Object data_get(int index) - { - if (data == null) - { - return null; - } - else if (data instanceof Object[] && !(data instanceof MetaDataEntry<?>[])) - { - Object[] array = (Object[])data; - return index < array.length ? array[index] : null; - } - else if (index == 0) - { - return data; - } - else - { - return null; - } - } - - final void data_set(int index, Object object) - { - if (index > data_length() - 1) - { - throw new IndexOutOfBoundsException("can not set data at " + index + - " when data_length() is " + data_length()); - } - else if (index == 0 && !(data instanceof Object[] && !(data instanceof MetaDataEntry<?>[]))) - { - data = object; - } - else - { - Object[] array = (Object[])data; - array[index] = object; - } - } - - final void data_add(Object object) - { - data_insert(-1, object); - } - - final void data_insert(int position, Object object) - { - int currentLength = data_length(); - if (position == -1) - { - position = currentLength; - } - if (position > currentLength) - { - throw new IndexOutOfBoundsException("can not insert data at " + position + - " when data_length() is " + currentLength); - } - if (currentLength == 0) - { - data = object; - } - else if (currentLength == 1) - { - Object[] array = new Object[2]; - if (position == 0) - { - array[0] = object; - array[1] = data; - } - else - { - array[0] = data; - array[1] = object; - } - data = array; - } - else - { - Object[] array = new Object[currentLength + 1]; - Object[] current = (Object[])data; - int after = currentLength - position; - if (position > 0) - { - System.arraycopy(current, 0, array, 0, position); - } - array[position] = object; - if (after > 0) - { - System.arraycopy(current, position, array, position + 1, after); - } - data = array; - } - } - - final void data_remove(int position) - { - int currentLength = data_length(); - - if (position > currentLength - 1) - { - throw new IndexOutOfBoundsException(); - } - else if (currentLength == 1) - { - data = null; - } - else if (currentLength == 2) - { - Object[] current = (Object[])data; - if (position == 0) - { - data = current[1]; - } - else - { - data = current[0]; - } - } - else - { - Object[] current = (Object[])data; - data = new Object[currentLength - 1]; - - if (position > 0) - { - System.arraycopy(current, 0, data, 0, position); - } - if (position != currentLength - 1) - { - final int left = currentLength - position - 1; - System.arraycopy(current, position + 1, data, position, left); - } - } - } + private Object data = null; /** * Constructor. All components have names. A component's id cannot be null. This is the minimal @@ -1099,7 +945,7 @@ public abstract class Component getClass().getName() + " has not called super.onRemove() in the override of onRemove() method"); } - Behaviors.onRemove(this); + ComponentState.onRemoveBehaviors(this, data, getFlag(FLAG_MODEL_SET)); removeChildren(); } @@ -1127,8 +973,12 @@ public abstract class Component // children component's getmodelobject is called detachModels(); - // detach any behaviors - Behaviors.detach(this); + // detach any behaviors. A behavior can replace this component's state while it is + // being detached (WICKET-6877), so the remaining behaviors are stored in the state as + // it is after detaching, not in the state as it was captured before. + Object detachedBehaviors = ComponentState.detachBehaviors(this, data, + getFlag(FLAG_MODEL_SET), getFlag(FLAG_BEHAVIOR_IDS_FIXED)); + data = ComponentState.setBehaviors(data, getFlag(FLAG_MODEL_SET), detachedBehaviors); } catch (Exception x) { @@ -1521,39 +1371,20 @@ public abstract class Component * @see MetaDataKey */ @Override + @SuppressWarnings("unchecked") public final <M extends Serializable> M getMetaData(final MetaDataKey<M> key) { - return key.get(getMetaData()); - } - - /** - * Gets the meta data entries for this component as an array of {@link MetaDataEntry} objects. - * - * @return the meta data entries for this component - */ - private MetaDataEntry<?>[] getMetaData() - { - MetaDataEntry<?>[] metaData = null; - - // index where we should expect the entry - int index = getFlag(FLAG_MODEL_SET) ? 1 : 0; - - int length = data_length(); - - if (index < length) + Object metaData = ComponentState.getMetaData(data, getFlag(FLAG_MODEL_SET)); + if (metaData == null) { - Object object = data_get(index); - if (object instanceof MetaDataEntry<?>[]) - { - metaData = (MetaDataEntry<?>[])object; - } - else if (object instanceof MetaDataEntry) - { - metaData = new MetaDataEntry[] { (MetaDataEntry<?>)object }; - } + return null; } - - return metaData; + else if (metaData instanceof MetaDataEntry) + { + MetaDataEntry< ? > entry = (MetaDataEntry< ? >) metaData; + return entry.key.equals(key) ? (M) entry.object : null; + } + return key.get((MetaDataEntry< ? >[]) metaData); } /** @@ -1569,7 +1400,10 @@ public abstract class Component { // give subclass a chance to lazy-init model model = initModel(); - setModelImpl(model); + if (model != null) + { + setModelImpl(model); + } } return model; @@ -2893,29 +2727,7 @@ public abstract class Component @Override public final <M extends Serializable> Component setMetaData(final MetaDataKey<M> key, final M object) { - MetaDataEntry<?>[] old = getMetaData(); - - Object metaData = null; - MetaDataEntry<?>[] metaDataArray = key.set(old, object); - if (metaDataArray != null && metaDataArray.length > 0) - { - metaData = (metaDataArray.length > 1) ? metaDataArray : metaDataArray[0]; - } - - int index = getFlag(FLAG_MODEL_SET) ? 1 : 0; - - if (old == null && metaData != null) - { - data_insert(index, metaData); - } - else if (old != null && metaData != null) - { - data_set(index, metaData); - } - else if (old != null && metaData == null) - { - data_remove(index); - } + data = ComponentState.setMetaData(data, getFlag(FLAG_MODEL_SET), key, object); return this; } @@ -2965,11 +2777,7 @@ public abstract class Component */ IModel<?> getModelImpl() { - if (getFlag(FLAG_MODEL_SET)) - { - return (IModel<?>)data_get(0); - } - return null; + return ComponentState.getModel(data, getFlag(FLAG_MODEL_SET)); } /** @@ -2978,26 +2786,8 @@ public abstract class Component */ void setModelImpl(IModel<?> model) { - if (getFlag(FLAG_MODEL_SET)) - { - if (model != null) - { - data_set(0, model); - } - else - { - data_remove(0); - setFlag(FLAG_MODEL_SET, false); - } - } - else - { - if (model != null) - { - data_insert(0, model); - setFlag(FLAG_MODEL_SET, true); - } - } + data = ComponentState.setModel(model, data, getFlag(FLAG_MODEL_SET)); + setFlag(FLAG_MODEL_SET, model != null); } /** @@ -3648,7 +3438,7 @@ public abstract class Component */ public <M extends Behavior> List<M> getBehaviors(Class<M> type) { - return Behaviors.getBehaviors(this, type); + return ComponentState.getBehaviors(type, data, getFlag(FLAG_MODEL_SET)); } /** @@ -4466,10 +4256,12 @@ public abstract class Component */ public Component remove(final Behavior... behaviors) { - for (Behavior behavior : behaviors) - { - Behaviors.remove(this, behavior); - } + // removing a behavior unbinds and detaches it, which can replace this component's state + // (WICKET-6877), so the remaining behaviors are stored in the state as it is after + // removal, not in the state as it was captured before. + Object remainingBehaviors = ComponentState.removeBehaviors(this, data, + getFlag(FLAG_MODEL_SET), behaviors); + data = ComponentState.setBehaviors(data, getFlag(FLAG_MODEL_SET), remainingBehaviors); return this; } @@ -4477,7 +4269,10 @@ public abstract class Component @Override public final Behavior getBehaviorById(int id) { - return Behaviors.getBehaviorById(this, id); + data = ComponentState.compactBehaviors(this, data, getFlag(FLAG_MODEL_SET), + getFlag(FLAG_BEHAVIOR_IDS_FIXED)); + setFlag(FLAG_BEHAVIOR_IDS_FIXED, true); + return ComponentState.getBehaviorById(this, id, data, getFlag(FLAG_MODEL_SET)); } /** {@inheritDoc} */ @@ -4489,7 +4284,10 @@ public abstract class Component throw new IllegalArgumentException( "Cannot get a stable id for temporary behavior " + behavior); } - return Behaviors.getBehaviorId(this, behavior); + data = ComponentState.compactBehaviors(this, data, getFlag(FLAG_MODEL_SET), + getFlag(FLAG_BEHAVIOR_IDS_FIXED)); + setFlag(FLAG_BEHAVIOR_IDS_FIXED, true); + return ComponentState.getBehaviorId(this, behavior, data, getFlag(FLAG_MODEL_SET)); } /** @@ -4498,10 +4296,24 @@ public abstract class Component * @param behaviors * The behavior modifier(s) to be added * @return this (to allow method call chaining) + * @throws IllegalArgumentException + * if the array or any behavior in it is {@code null} */ public Component add(final Behavior... behaviors) { - Behaviors.add(this, behaviors); + // checked before anything is stored: a null found halfway through would leave the + // behaviors before it added, and the slot it took would shift every id handed out after + Args.notNull(behaviors, "behaviors"); + for (Behavior curBehavior : behaviors) + { + Args.notNull(curBehavior, "behavior"); + } + + data = ComponentState.addBehaviors(this, data, getFlag(FLAG_MODEL_SET), behaviors); + for (Behavior curBehavior : behaviors) + { + ComponentState.bindBehavior(this, curBehavior); + } return this; } diff --git a/wicket-core/src/main/java/org/apache/wicket/ComponentState.java b/wicket-core/src/main/java/org/apache/wicket/ComponentState.java new file mode 100644 index 0000000000..737726926e --- /dev/null +++ b/wicket-core/src/main/java/org/apache/wicket/ComponentState.java @@ -0,0 +1,871 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.wicket; + +import java.io.Serializable; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; + +import org.apache.wicket.behavior.Behavior; +import org.apache.wicket.behavior.InvalidBehaviorIdException; +import org.apache.wicket.model.IModel; + +/** + * This class keeps track of the flexible state of a component: model, behaviors and meta data. + * These types of state vary per component. Not every component contains these elements or their + * numbers differ. The state is stored in the {@code data} field in {@link Component}. To keep the + * size of this state as small as possible, the following cases are identified: + * <ul> + * <li>No state at all: {@code data} is {@code null} + * <li>Only a model: {@code data} contains the model + * <li>Only one or more behaviors: {@code data} contains the behavior, or an array of behaviors + * <li>Only one or more meta data entries: {@code data} contains the entry, or an array of entries + * <li>Any two or three of them: {@code data} contains a {@link ComponentState}, holding each + * kind in its own field + * </ul> + * <p> + * A single state class rather than one class per combination: the combinations all cost the same + * 24 bytes on a 64 bit VM with compressed oops, so specialised classes save nothing there, while + * four implementations of the same accessors make every unpacking call site megamorphic and stop + * it from being inlined. + * + * @author papegaaij + */ +final class ComponentState implements Serializable +{ + private static final long serialVersionUID = 1L; + + private IModel< ? > model; + + private Object behaviors; + + private Object metaData; + + private ComponentState(IModel< ? > model, Object behaviors, Object metaData) + { + this.model = model; + this.behaviors = behaviors; + this.metaData = metaData; + } + + /** + * @return The model stored by this state, or null. + */ + IModel< ? > getModel() + { + return model; + } + + /** + * @param model + * the new model or null + * @return the new state for the component using the rules defined above + */ + Object setModel(IModel< ? > model) + { + this.model = model; + return pack(); + } + + /** + * @return The behaviors stored by this state: null, a single behavior or an array of behaviors. + */ + Object getBehaviors() + { + return behaviors; + } + + /** + * @param behaviors + * the new behaviors (null, one behavior or an array of behaviors) + * @return the new state for the component using the rules defined above + */ + Object setBehaviors(Object behaviors) + { + this.behaviors = behaviors; + return pack(); + } + + /** + * @return The meta data entries stored by this state: null, a single entry or an array of + * entries. + */ + Object getMetaData() + { + return metaData; + } + + /** + * @param metaData + * the new meta data entries (null, one entry or an array of entries) + * @return the new state for the component using the rules defined above + */ + Object setMetaData(Object metaData) + { + this.metaData = metaData; + return pack(); + } + + /** + * This state is only worth its own allocation while it holds more than one kind of state. Once + * a single kind is left, that value is stored in {@code Component.data} directly, and once + * none is left the field is cleared. + * + * @return this state, the single remaining value, or null + */ + private Object pack() + { + if (model == null) + { + if (behaviors == null) + { + return metaData; + } + return metaData == null ? behaviors : this; + } + if (behaviors == null && metaData == null) + { + return model; + } + return this; + } + + /** + * @param state + * the component state + * @param modelSet + * a boolean indicating if the model is set + * @return the model from the given state or null + */ + static IModel< ? > getModel(Object state, boolean modelSet) + { + // the flag is authoritative: with no model set there is nothing to find, so the type + // check can be skipped entirely. Reading a model is far more common than reading the + // other two kinds of state, and most components have no model at all. + if (!modelSet) + { + return null; + } + if (state instanceof ComponentState) + { + return ((ComponentState) state).getModel(); + } + return (IModel< ? >) state; + } + + /** + * @param state + * the component state + * @param modelSet + * a boolean indicating if the model is set + * @return the behaviors from the given state: null, one behavior or an array of behaviors + */ + static Object getBehaviors(Object state, boolean modelSet) + { + if (state instanceof ComponentState) + { + return ((ComponentState) state).getBehaviors(); + } + return modelSet || !(state instanceof Behavior || state instanceof Behavior[]) ? null + : state; + } + + /** + * @param state + * the component state + * @param modelSet + * a boolean indicating if the model is set + * @return the meta data entries from the given state: null, one entry or an array of entries + */ + static Object getMetaData(Object state, boolean modelSet) + { + if (state instanceof ComponentState) + { + return ((ComponentState) state).getMetaData(); + } + return modelSet || !(state instanceof MetaDataEntry || state instanceof MetaDataEntry[]) + ? null : state; + } + + /** + * Construct a new component state with the given model value + * + * @param model + * the new model to set or null to clear + * @param state + * the current component state + * @param modelSet + * a boolean indicating if the model is set + * @return the new component state + */ + static Object setModel(IModel< ? > model, Object state, boolean modelSet) + { + if (state instanceof ComponentState) + { + ComponentState compState = (ComponentState) state; + return compState.setModel(model); + } + else if (modelSet || state == null) + { + return model; + } + // state does not have a model, clear is a no-op + else if (model == null) + { + return state; + } + else if (state instanceof MetaDataEntry || state instanceof MetaDataEntry[]) + { + return new ComponentState(model, null, state); + } + else + { + return new ComponentState(model, state, null); + } + } + + /** + * Construct a new component state with the given behaviors added + * + * @param component + * the component to add the behaviors to + * @param state + * the current component state + * @param modelSet + * a boolean indicating if the model is set + * @param behaviorsToAdd + * the behaviors to add + * @return the new component state + */ + static Object addBehaviors(Component component, Object state, boolean modelSet, + Behavior... behaviorsToAdd) + { + if (behaviorsToAdd.length == 0) + { + return state; + } + else if (state instanceof ComponentState) + { + ComponentState compState = (ComponentState) state; + return compState + .setBehaviors(addBehaviors(component, compState.getBehaviors(), behaviorsToAdd)); + } + else if (modelSet) + { + return new ComponentState((IModel< ? >) state, + addBehaviors(component, null, behaviorsToAdd), null); + } + else if (state instanceof MetaDataEntry || state instanceof MetaDataEntry[]) + { + return new ComponentState(null, + addBehaviors(component, null, behaviorsToAdd), state); + } + else + { + return addBehaviors(component, state, behaviorsToAdd); + } + } + + /** + * Remove the given behaviors from the component's behaviors + * + * @param component + * the component to remove the behaviors from + * @param state + * the current component state + * @param modelSet + * a boolean indicating if the model is set + * @param behaviorsToRemove + * the behaviors to removed + * @return the remaining behaviors: null, one behavior or an array of behaviors + */ + static Object removeBehaviors(Component component, Object state, boolean modelSet, + Behavior... behaviorsToRemove) + { + Object behaviors = getBehaviors(state, modelSet); + if (behaviorsToRemove.length == 0) + { + return behaviors; + } + return removeBehaviors(component, behaviors, behaviorsToRemove); + } + + /** + * Construct a new component state with the behaviors replaced + * + * @param state + * the current component state + * @param modelSet + * a boolean indicating if the model is set + * @param behaviors + * the new value for the behaviors: null, one behavior or an array of behaviors + * @return the new component state + */ + static Object setBehaviors(Object state, boolean modelSet, Object behaviors) + { + if (state instanceof ComponentState) + { + ComponentState compState = (ComponentState) state; + return compState.setBehaviors(behaviors); + } + else if (state instanceof Behavior || state instanceof Behavior[] || state == null) + { + return behaviors; + } + else if (behaviors == null) + { + return state; + } + else if (modelSet) + { + return new ComponentState((IModel< ? >) state, behaviors, null); + } + else + { + return new ComponentState(null, behaviors, state); + } + } + + /** + * Construct a new component state with the given meta data entry set or reset + * + * @param state + * the current component state + * @param modelSet + * a boolean indicating if the model is set + * @param key + * the key to replace the value for + * @param data + * the new value for the meta data entry, null to clear + * @return the new component state + */ + static <T> Object setMetaData(Object state, boolean modelSet, MetaDataKey<T> key, T data) + { + if (state instanceof ComponentState) + { + ComponentState compState = (ComponentState) state; + return compState.setMetaData(setMetaData(compState.getMetaData(), key, data)); + } + else if (state instanceof MetaDataEntry || state instanceof MetaDataEntry[] + || state == null) + { + return setMetaData(state, key, data); + } + else if (data == null) + { + return state; + } + else if (modelSet) + { + return new ComponentState((IModel< ? >) state, null, + new MetaDataEntry<>(key, data)); + } + else + { + return new ComponentState(null, state, new MetaDataEntry<>(key, data)); + } + } + + /** + * Bind a behavior to a component, adding a state change if needed. + * + * @param component + * @param behavior + */ + static void bindBehavior(Component component, Behavior behavior) + { + if (!behavior.isTemporary(component)) + { + component.addStateChange(); + } + behavior.bind(component); + } + + private static Object addBehaviors(Component component, Object behaviors, + Behavior... behaviorsToAdd) + { + // nothing to add + if (behaviorsToAdd.length == 0) + { + return behaviors; + } + + // the existing array is compact, adding cannot shrink it + int curLength = getBehaviorsLength(behaviors); + int newSize = Math.max(curLength, behaviorsToAdd.length + getBehaviorsLength(behaviors) + - getEmptyBehaviorsSlots(behaviors)); + + // new size is 1, it must be we are adding 1 to 0 + if (newSize == 1) + { + return behaviorsToAdd[0]; + } + + // construct the return array and copy existing behaviors + Behavior[] ret = new Behavior[newSize]; + if (behaviors instanceof Behavior[]) + { + System.arraycopy(behaviors, 0, ret, 0, curLength); + } + else + { + ret[0] = (Behavior) behaviors; + } + + // fill empty slots with behaviors to add + int checkSlot = 0; + for (Behavior behaviorToAdd : behaviorsToAdd) + { + while (ret[checkSlot] != null) + { + checkSlot++; + } + ret[checkSlot] = behaviorToAdd; + } + return ret; + } + + private static Object removeBehaviors(Component component, Object behaviors, + Behavior... behaviorsToRemove) + { + // nothing to remove + if (behaviorsToRemove.length == 0) + { + return behaviors; + } + if (behaviors == null) + { + throw cannotRemove(behaviorsToRemove[0]); + } + + if (behaviors instanceof Behavior) + { + if (!behaviorsToRemove[0].equals(behaviors)) + { + throw cannotRemove(behaviorsToRemove[0]); + } + if (behaviorsToRemove.length > 1) + { + throw cannotRemove(behaviorsToRemove[1]); + } + unbindBehavior(component, (Behavior) behaviors); + return null; + } + + Behavior[] behaviorArr = (Behavior[]) behaviors; + for (Behavior behaviorToRemove : behaviorsToRemove) + { + boolean found = false; + for (int i = 0; i < behaviorArr.length; i++) + { + Behavior curBehavior = behaviorArr[i]; + if (curBehavior != null && behaviorToRemove.equals(curBehavior)) + { + found = true; + unbindBehavior(component, curBehavior); + behaviorArr[i] = null; + break; + } + } + if (!found) + { + throw cannotRemove(behaviorToRemove); + } + } + return behaviorArr; + } + + private static IllegalStateException cannotRemove(Behavior behavior) + { + return new IllegalStateException( + "Tried to remove a behavior that was not added to the component. Behavior: " + + behavior.toString()); + } + + private static void unbindBehavior(Component component, Behavior behavior) + { + behavior.unbind(component); + if (!behavior.isTemporary(component)) + { + component.addStateChange(); + } + behavior.detach(component); + } + + private static int getBehaviorsLength(Object behaviors) + { + if (behaviors == null) + { + return 0; + } + return behaviors instanceof Behavior[] ? ((Behavior[]) behaviors).length : 1; + } + + private static int getEmptyBehaviorsSlots(Object behaviors) + { + if (!(behaviors instanceof Behavior[])) + { + return 0; + } + Behavior[] arr = (Behavior[]) behaviors; + int emptyCount = 0; + for (Behavior curBehavior : arr) + { + if (curBehavior == null) + { + emptyCount++; + } + } + return emptyCount; + } + + private static <T> Object setMetaData(Object metadata, MetaDataKey<T> key, T data) + { + if (metadata == null) + { + if (data == null) + { + return null; + } + else + { + return new MetaDataEntry<>(key, data); + } + } + else if (metadata instanceof MetaDataEntry) + { + MetaDataEntry< ? > curEntry = (MetaDataEntry< ? >) metadata; + if (curEntry.key.equals(key)) + { + if (data == null) + { + return null; + } + else + { + curEntry.object = data; + return curEntry; + } + } + else + { + if (data == null) + { + return metadata; + } + else + { + MetaDataEntry< ? >[] ret = new MetaDataEntry< ? >[2]; + ret[0] = (MetaDataEntry< ? >) metadata; + ret[1] = new MetaDataEntry<>(key, data); + return ret; + } + } + } + else + { + MetaDataEntry< ? >[] metadataArr = (MetaDataEntry< ? >[]) metadata; + for (int i = 0; i < metadataArr.length; i++) + { + MetaDataEntry< ? > curEntry = metadataArr[i]; + if (curEntry.key.equals(key)) + { + if (data == null) + { + if (metadataArr.length == 2) + { + return metadataArr[i == 0 ? 1 : 0]; + } + else + { + MetaDataEntry< ? >[] ret = + new MetaDataEntry< ? >[metadataArr.length - 1]; + System.arraycopy(metadataArr, 0, ret, 0, i); + System.arraycopy(metadataArr, i + 1, ret, i, ret.length - i); + return ret; + } + } + else + { + curEntry.object = data; + return metadataArr; + } + } + } + if (data == null) + { + return metadataArr; + } + MetaDataEntry< ? >[] ret = new MetaDataEntry< ? >[metadataArr.length + 1]; + System.arraycopy(metadataArr, 0, ret, 0, metadataArr.length); + ret[metadataArr.length] = new MetaDataEntry<>(key, data); + return ret; + } + } + + static Behavior getBehaviorById(Component component, int id, Object state, boolean modelSet) + { + Object behaviors = getBehaviors(state, modelSet); + if (behaviors instanceof Behavior) + { + if (id == 0) + { + return (Behavior) behaviors; + } + } + else if (behaviors instanceof Behavior[]) + { + Behavior[] behaviorsArr = (Behavior[]) behaviors; + if (behaviorsArr.length > id && behaviorsArr[id] != null) + { + return behaviorsArr[id]; + } + } + throw new InvalidBehaviorIdException(component, id); + } + + static int getBehaviorId(Component component, Behavior behavior, Object state, boolean modelSet) + { + Object behaviors = getBehaviors(state, modelSet); + if (behavior.equals(behaviors)) + { + return 0; + } + else if (behaviors instanceof Behavior[]) + { + Behavior[] behaviorsArr = (Behavior[]) behaviors; + for (int i = 0; i < behaviorsArr.length; i++) + { + if (behavior.equals(behaviorsArr[i])) + { + return i; + } + } + } + throw new IllegalStateException( + "Behavior must be added to component before its id can be generated. Behavior: " + + behavior + ", Component: " + component); + } + + @SuppressWarnings("unchecked") + static <M extends Behavior> List<M> getBehaviors(Class<M> type, Object state, boolean modelSet) + { + Object behaviors = getBehaviors(state, modelSet); + if (behaviors == null) + { + return List.of(); + } + + if (behaviors instanceof Behavior) + { + if (type == null || type.isInstance(behaviors)) + { + return List.of((M) behaviors); + } + return List.of(); + } + + Behavior[] behaviorsArr = (Behavior[]) behaviors; + List<M> subset = new ArrayList<>(behaviorsArr.length); + for (Behavior curBehavior : behaviorsArr) + { + if (curBehavior != null && (type == null || type.isInstance(curBehavior))) + { + subset.add((M) curBehavior); + } + } + if (subset.isEmpty()) + { + return List.of(); + } + return Collections.unmodifiableList(subset); + } + + static void onRemoveBehaviors(Component component, Object state, boolean modelSet) + { + Object behaviors = getBehaviors(state, modelSet); + if (behaviors instanceof Behavior) + { + ((Behavior) behaviors).onRemove(component); + } + else if (behaviors instanceof Behavior[]) + { + Behavior[] behaviorsArr = (Behavior[]) behaviors; + for (Behavior curBehavior : behaviorsArr) + { + if (curBehavior != null) + { + curBehavior.onRemove(component); + } + } + } + } + + static Object compactBehaviors(Component component, Object state, boolean modelSet, + boolean fixedIds) + { + if (fixedIds) + { + return state; + } + Object behaviors = getBehaviors(state, modelSet); + if (!(behaviors instanceof Behavior[])) + { + return state; + } + + Behavior[] behaviorsArr = (Behavior[]) behaviors; + int setIndex = 0; + int endIndex = behaviorsArr.length - 1; + int checkIndex = 0; + while (checkIndex <= endIndex) + { + Behavior curBehavior = behaviorsArr[checkIndex]; + if (curBehavior == null) + { + checkIndex++; + continue; + } + + // move tmp behaviors to the end of the array, swap with what's there + if (curBehavior.isTemporary(component)) + { + Behavior tmp = behaviorsArr[endIndex]; + behaviorsArr[endIndex] = curBehavior; + behaviorsArr[checkIndex] = tmp; + endIndex--; + continue; + } + behaviorsArr[setIndex] = curBehavior; + checkIndex++; + setIndex++; + } + + // wipe the remainder of the array + for (; setIndex <= endIndex; setIndex++) + { + behaviorsArr[setIndex] = null; + } + return state; + } + + /** + * Detach the component's behaviors, unbinding and removing the temporary ones. + * <p> + * Detaching a behavior runs arbitrary code, which may replace the component's state (for + * example by clearing a meta data entry, see WICKET-6877). This method therefore only returns + * the remaining behaviors; it is up to the caller to store them in the component's state as it + * is <em>after</em> detaching. + * + * @param component + * the component whose behaviors are detached + * @param state + * the current component state + * @param modelSet + * a boolean indicating if the model is set + * @param fixedIds + * a boolean indicating if the behavior ids must be kept stable + * @return the remaining behaviors: null, one behavior or an array of behaviors + */ + static Object detachBehaviors(Component component, Object state, boolean modelSet, + boolean fixedIds) + { + Object behaviors = getBehaviors(state, modelSet); + if (behaviors instanceof Behavior) + { + Behavior behavior = (Behavior) behaviors; + behavior.detach(component); + if (behavior.isTemporary(component)) + { + behavior.unbind(component); + return null; + } + return behavior; + } + else if (behaviors instanceof Behavior[]) + { + // remove temporary behaviors and compact the array + int highestId = -1; + int filledSlots = 0; + Behavior[] behaviorsArr = (Behavior[]) behaviors; + + // iterate over all behaviors, detaching them and removing temporary behaviors + // remaining behaviors are counted and for stateful behaviors slots assigned + for (int i = 0; i < behaviorsArr.length; i++) + { + Behavior curBehavior = behaviorsArr[i]; + if (curBehavior != null) + { + curBehavior.detach(component); + if (curBehavior.isTemporary(component)) + { + curBehavior.unbind(component); + behaviorsArr[i] = null; + } + else + { + filledSlots++; + highestId = i; + } + } + } + + // if at most 1 behavior remains, no array is needed + int newSize = fixedIds ? Math.max(highestId + 1, filledSlots) : filledSlots; + if (newSize == 0) + { + return null; + } + if (newSize == 1) + { + return behaviorsArr[highestId]; + } + + // the calculated size is equal to the current size, cannot compact + if (newSize == behaviorsArr.length) + { + return behaviorsArr; + } + + // multiple behaviors (or one with an id > 0) + // construct a new array and compact the behaviors + Behavior[] ret = new Behavior[newSize]; + + if (fixedIds) + { + System.arraycopy(behaviorsArr, 0, ret, 0, ret.length); + } + else + { + int targetIndex = 0; + for (int i = 0; i < behaviorsArr.length; i++) + { + Behavior curBehavior = behaviorsArr[i]; + if (curBehavior == null) + { + continue; + } + ret[targetIndex] = curBehavior; + targetIndex++; + } + } + return ret; + } + return behaviors; + } +} diff --git a/wicket-core/src/main/java/org/apache/wicket/ajax/AbstractDefaultAjaxBehavior.java b/wicket-core/src/main/java/org/apache/wicket/ajax/AbstractDefaultAjaxBehavior.java index 9bdda56772..6e7ae69d70 100644 --- a/wicket-core/src/main/java/org/apache/wicket/ajax/AbstractDefaultAjaxBehavior.java +++ b/wicket-core/src/main/java/org/apache/wicket/ajax/AbstractDefaultAjaxBehavior.java @@ -89,12 +89,6 @@ public abstract class AbstractDefaultAjaxBehavior extends AbstractAjaxBehavior final Component component = getComponent(); component.setOutputMarkupId(true); - - if (getStatelessHint(component)) - { - //generate behavior id - component.getBehaviorId(this); - } } /** diff --git a/wicket-core/src/main/java/org/apache/wicket/request/component/IRequestableComponent.java b/wicket-core/src/main/java/org/apache/wicket/request/component/IRequestableComponent.java index 57bd59757e..c2fb8714be 100644 --- a/wicket-core/src/main/java/org/apache/wicket/request/component/IRequestableComponent.java +++ b/wicket-core/src/main/java/org/apache/wicket/request/component/IRequestableComponent.java @@ -66,8 +66,14 @@ public interface IRequestableComponent * Gets a stable id for the specified non-temporary behavior. The id remains stable from the * point this method is first called for the behavior until the behavior has been removed from * the component. This includes from one request to the next, when the component itself is - * retained for the next request (i.e. is stateful). Note that the bookkeeping required for - * these stable ids increases the memory footprint of the component. + * retained for the next request (i.e. is stateful). + * <p> + * An id is a position in the component's list of behaviors, so ids are not dense and do not + * necessarily start at zero: a behavior that never asks for an id still occupies a position. + * Which position a behavior ends up at therefore depends on what else was added to the + * component before this method was first called on it, and nothing is fixed until then. From + * that point on the list is no longer compacted, so the gaps left by removed behaviors are + * kept for as long as the component lives. * * @param behavior * @return a stable id for the specified behavior @@ -79,7 +85,8 @@ public interface IRequestableComponent * Gets the behavior for the specified id * * @param id - * @return behavior or {@code null} if none + * an id handed out by {@link #getBehaviorId(Behavior)} + * @return the behavior with the given id, never {@code null} * @throws InvalidBehaviorIdException * when behavior with this id cannot be found */
