Hi Martin,
> isAttached() is a public method and it is valid to call inside load()
that's a valid point to consider: But what purpose could it have to call
isAttached() in load()?
It would return <true> during the complete invocation.
Since Wicket allows a single thread only to access the component tree, I
don't see a race condition either.
Thanks
Sven
On 24.11.2014 23:39, Martijn Dashorst wrote:
It appears innocent enough this change, but we can't be sure this
won't break applications: isAttached() is a public method and it is
valid to call inside load(). If someone depends on isAttached() during
load() we will change this behavior.
Granted this new semantics are much more to my liking, and it fixes a
bug. But does this justify the risk of breaking applications in other
ways?
I checked some of our projects for such mishaps but couldn't find any
misuses of isAttached inside load().
Martijn
On Mon, Nov 24, 2014 at 9:11 PM, Sven Meier (JIRA) <[email protected]> wrote:
[
https://issues.apache.org/jira/browse/WICKET-5772?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Sven Meier resolved WICKET-5772.
--------------------------------
Resolution: Fixed
Fix Version/s: 6.19.0
7.0.0-M5
#attached is now set after calling #load(), as suggested.
Thanks Martin!
LoadableDetachableModel caches null value if load() fails, bug in getObject()
{attached = true;}
------------------------------------------------------------------------------------------------
Key: WICKET-5772
URL: https://issues.apache.org/jira/browse/WICKET-5772
Project: Wicket
Issue Type: Bug
Components: wicket
Affects Versions: 1.4.23, 6.18.0, 7.0.0-M4
Reporter: Martin Makundi
Assignee: Sven Meier
Labels: easyfix, easytest, patch
Fix For: 7.0.0-M5, 6.19.0
Original Estimate: 10m
Remaining Estimate: 10m
If you have a LoadableDetachableModel whose load() operation fails at an
instance, the LoadableDetachableModel will cache null value because
getObject() method sets attached = true; before load() invocation has returned.
This results in methods trusting LoadableDetachableModel using the null value
for their operations which is logically incorrect as null might not be a legal
value at all. Such behavior would result in unexpected difficult-to-debug
behavior in depending components.
Easy fix:
Move:
attached = true;
after line:
transientModelObject = load();
Test case:
/**
* LoadableDetachableModel must not return null as an attached value if
load()
* fails.
*/
public void testWhenLoadFails() {
final LoadableDetachableModel<Void> loadableDetachableModel = new
LoadableDetachableModel<Void>() {
/**
* @see org.apache.wicket.model.LoadableDetachableModel#load()
*/
@Override
protected Void load() {
throw new UnsupportedOperationException("Let's assume this fails for some
reason.");
}
};
try {
loadableDetachableModel.getObject();
fail(UnsupportedOperationException.class + " expected.");
} catch (final UnsupportedOperationException e) {
LoggerFactory.getLogger(LoadableDetachableModel.class).debug("Expected
behavior due to " + UnsupportedOperationException.class);
}
try {
assertNotSame(LoadableDetachableModel.class + " should not return null if
it's load() has failed\n", null,
loadableDetachableModel.getObject());
} catch (final UnsupportedOperationException e) {
LoggerFactory.getLogger(LoadableDetachableModel.class).debug("Expected
behavior due to " + UnsupportedOperationException.class);
}
}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
