Good catch. Fixed.

Martijn

On Sat, May 30, 2015 at 10:02 PM, Martin Grigorov <[email protected]> wrote:
> On Sat, May 30, 2015 at 10:48 PM, <[email protected]> wrote:
>
>> Repository: wicket
>> Updated Branches:
>>   refs/heads/master 6512a6d45 -> f11391530
>>
>>
>> Cleaned up LoadableDetachableModel state tracking
>>
>> Previous implementation crafted by me was a tad overengineered, and had
>> some
>> lousy naming. Fixed the naming and removed the unnecessary booleans.
>>
>>
>> Project: http://git-wip-us.apache.org/repos/asf/wicket/repo
>> Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/f1139153
>> Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/f1139153
>> Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/f1139153
>>
>> Branch: refs/heads/master
>> Commit: f113915302394460c9048cc1ccd685adec1460b7
>> Parents: 6512a6d
>> Author: Martijn Dashorst <[email protected]>
>> Authored: Sat May 30 21:47:59 2015 +0200
>> Committer: Martijn Dashorst <[email protected]>
>> Committed: Sat May 30 21:48:05 2015 +0200
>>
>> ----------------------------------------------------------------------
>>  .../wicket/model/LoadableDetachableModel.java   | 57 +++++++-------------
>>  1 file changed, 18 insertions(+), 39 deletions(-)
>> ----------------------------------------------------------------------
>>
>>
>>
>> http://git-wip-us.apache.org/repos/asf/wicket/blob/f1139153/wicket-core/src/main/java/org/apache/wicket/model/LoadableDetachableModel.java
>> ----------------------------------------------------------------------
>> diff --git
>> a/wicket-core/src/main/java/org/apache/wicket/model/LoadableDetachableModel.java
>> b/wicket-core/src/main/java/org/apache/wicket/model/LoadableDetachableModel.java
>> index e3a7fc0..16c6271 100644
>> ---
>> a/wicket-core/src/main/java/org/apache/wicket/model/LoadableDetachableModel.java
>> +++
>> b/wicket-core/src/main/java/org/apache/wicket/model/LoadableDetachableModel.java
>> @@ -52,39 +52,16 @@ import org.slf4j.LoggerFactory;
>>   */
>>  public abstract class LoadableDetachableModel<T> implements IModel<T>
>>  {
>> -       /**
>> -        *
>> -        */
>> +       /** */
>>         private static final long serialVersionUID = 1L;
>>
>>         /** Logger. */
>>         private static final Logger log =
>> LoggerFactory.getLogger(LoadableDetachableModel.class);
>>
>> -       private enum AttachingState
>> -       {
>> -               DETACHED(false, false),
>> -               ATTACHING(true, false),
>> -               ATTACHED(true, true);
>> -
>> -               private boolean attaching;
>> -               private boolean attached;
>> -
>> -               private AttachingState(boolean attaching, boolean attached)
>> -               {
>> -                       this.attached = attached;
>> -                       this.attaching = attaching;
>> -               }
>> -
>> -               public boolean isAttached()
>> -               {
>> -                       return attached;
>> -               }
>> +       /** Internal state of the LoadableDetachableModel. */
>> +       private enum InternalState {
>> +               DETACHED, ATTACHING, ATTACHED;
>>
>> -               public boolean isAttaching()
>> -               {
>> -                       return attaching;
>> -               }
>> -
>>                 @Override
>>                 public String toString()
>>                 {
>> @@ -92,14 +69,15 @@ public abstract class LoadableDetachableModel<T>
>> implements IModel<T>
>>                 }
>>         }
>>
>> -       /** keeps track of whether this model is attached or detached */
>> -       private transient AttachingState attached =
>> AttachingState.DETACHED;
>> +       /** Keeps track of whether this model is attached or detached */
>> +       private transient InternalState state = InternalState.DETACHED;
>>
>
> After deserialization state will be null => fourth state.
>
>
>>
>>         /** temporary, transient object. */
>>         private transient T transientModelObject;
>>
>>         /**
>> -        * Construct.
>> +        * Default constructor, constructs the model in detached state
>> with no data associated with the
>> +        * model.
>>          */
>>         public LoadableDetachableModel()
>>         {
>> @@ -107,7 +85,8 @@ public abstract class LoadableDetachableModel<T>
>> implements IModel<T>
>>
>>         /**
>>          * This constructor is used if you already have the object
>> retrieved and want to wrap it with a
>> -        * detachable model.
>> +        * detachable model. Constructs the model in attached state. Calls
>> to {@link #getObject()} will
>> +        * return {@code object} until {@link #detach()} is called.
>>          *
>>          * @param object
>>          *            retrieved instance of the detachable object
>> @@ -115,7 +94,7 @@ public abstract class LoadableDetachableModel<T>
>> implements IModel<T>
>>         public LoadableDetachableModel(T object)
>>         {
>>                 this.transientModelObject = object;
>> -               attached = AttachingState.ATTACHED;
>> +               state = InternalState.ATTACHED;
>>         }
>>
>>         /**
>> @@ -124,7 +103,7 @@ public abstract class LoadableDetachableModel<T>
>> implements IModel<T>
>>         @Override
>>         public void detach()
>>         {
>> -               if (attached == AttachingState.ATTACHED)
>> +               if (state == InternalState.ATTACHED)
>>                 {
>>                         try
>>                         {
>> @@ -132,7 +111,7 @@ public abstract class LoadableDetachableModel<T>
>> implements IModel<T>
>>                         }
>>                         finally
>>                         {
>> -                               attached = AttachingState.DETACHED;
>> +                               state = InternalState.DETACHED;
>>                                 transientModelObject = null;
>>
>>                                 log.debug("removed transient object for
>> {}, requestCycle {}", this,
>> @@ -147,10 +126,10 @@ public abstract class LoadableDetachableModel<T>
>> implements IModel<T>
>>         @Override
>>         public final T getObject()
>>         {
>> -               if (attached == AttachingState.DETACHED)
>> +               if (state == InternalState.DETACHED)
>>
>
> and this will break
>
>
>>                 {
>>                         // prevent infinite attachment loops
>> -                       attached = AttachingState.ATTACHING;
>> +                       state = InternalState.ATTACHING;
>>
>>                         transientModelObject = load();
>>
>> @@ -160,7 +139,7 @@ public abstract class LoadableDetachableModel<T>
>> implements IModel<T>
>>                                         ", requestCycle " +
>> RequestCycle.get());
>>                         }
>>
>> -                       attached = AttachingState.ATTACHED;
>> +                       state = InternalState.ATTACHED;
>>                         onAttach();
>>                 }
>>                 return transientModelObject;
>> @@ -173,7 +152,7 @@ public abstract class LoadableDetachableModel<T>
>> implements IModel<T>
>>          */
>>         public final boolean isAttached()
>>         {
>> -               return attached.isAttached();
>> +               return state == InternalState.ATTACHED;
>>         }
>>
>>         /**
>> @@ -225,7 +204,7 @@ public abstract class LoadableDetachableModel<T>
>> implements IModel<T>
>>         @Override
>>         public void setObject(final T object)
>>         {
>> -               attached = AttachingState.ATTACHED;
>> +               state = InternalState.ATTACHED;
>>                 transientModelObject = object;
>>         }
>>  }
>>
>>



-- 
Become a Wicket expert, learn from the best: http://wicketinaction.com

Reply via email to