[WICKET-5772] LoadableDetachableModel caches null value if load() fails, bug in getObject() {attached = true;}

• Type: Bug
• Status: Resolved
• Priority: Major
• Resolution: Fixed
• Affects Version/s: 1.4.23, 6.18.0, 7.0.0-M4
• Fix Version/s: 7.0.0-M5
• Component/s: wicket
• Labels: easyfix easytest patch
• Assignee: Sven Meier
• Reporter: Martin Terra
• Created: 22/Nov/14 01:23
• Updated: 29/May/15 17:06
• Resolved: 24/Nov/14 20:10

Description

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); }

  }

Issue Links

• breaks: WICKET-5916 StackOverflowError when calling getObject() from load() in LDM
• links to: Mail discussion about the change at dev@