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

["ASF subversion and git services (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/WICKET-5772?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14223089#comment-14223089 ] 

ASF subversion and git services commented on WICKET-5772:
---------------------------------------------------------

Commit 61d6264b2bfa1a68b18a1c223945ad1cd9a3b8de in wicket's branch refs/heads/master from [~svenmeier]
[ https://git-wip-us.apache.org/repos/asf?p=wicket.git;h=61d6264 ]

WICKET-5772 set attached after load, so failuer of load() will not keep null value


> 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
>              Labels: easyfix, easytest, patch
>   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)