[WICKET-6351] Form.anyFormComponentError performance degradation

• Type: Improvement
• Status: Resolved
• Priority: Minor
• Resolution: Fixed
• Affects Version/s: 7.6.0, 8.0.0-M4
• Fix Version/s: 6.27.0, 7.7.0, 8.0.0-M6
• Assignee: Martin Tzvetanov Grigorov
• Reporter: Lajos Gáthy
• Created: 03/Apr/17 08:31
• Updated: 10/Apr/17 07:53
• Resolved: 03/Apr/17 09:21

Description

This is related to the changes made in https://issues.apache.org/jira/browse/WICKET-5883

Inside the visitor instead of checking:

if (component.isVisibleInHierarchy() && component.isEnabledInHierarchy() && component.hasErrorMessage())

it would be much better to do:

if (component.hasErrorMessage() && component.isVisibleInHierarchy() && component.isEnabledInHierarchy())

This not only would save a lot of unnecessary computation when there is no error on a component (most cases) but computing isEnabled() or isVisible() is not trivial, but also would avoid reloading some detachable models too early (before models change after form submit). An example for the later: when we have a PagingNavigator where the PagingNavigationLink-s have autoEnable set to true (which is the default), then computing isEnabled() for these link would trigger model reload because PagingNavigationLink.linksTo(Page) needs getPageNumber(). Not only this, but it will make AbstractPageableView.getItemCount() cache the wrong itemCount (before model update happens) and return that on render phase incorrectly.

On the other hand I do not see any downside of changing the order of the conditions so that hasErrorMessage() is computed before others.