Wicket
  1. Wicket
  2. WICKET-1872

Url for behaviour and isTemporary flag of behaviour.

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Duplicate
    • Affects Version/s: 1.3.4
    • Fix Version/s: None
    • Component/s: wicket
    • Labels:
      None

      Description

      You add two behaviors to the Component. First behavior is temporary and the second is normal.
      Now the wicket creates the url by "urlFor(final Component component, final IBehavior behaviour, final RequestListenerInterface listener)" for second behavior with behaviourId=1. On the end of request the wicket removes the temporary behavior. Now the component contains only 1 but not 2 behaviors. And in the next request in method "public final void processEvents(final RequestCycle requestCycle)" the wicket tries to find the behavior by its id. But the problem is with if (component.getBehaviors().size() > idAsInt) condition, and with the fact that index of behavior is incorrect.

      The problematic classes with their methods:

      org.apache.wicket.RequestCycle:
      public final CharSequence urlFor(final Component component, final IBehavior behaviour,
      final RequestListenerInterface listener)
      {
      int index = component.getBehaviors().indexOf(behaviour);
      if (index == -1)

      { throw new IllegalArgumentException("Behavior " + this + " was not registered with this component: " + component.toString()); }

      RequestParameters params = new RequestParameters();
      params.setBehaviorId(String.valueOf(index));
      ...
      ==============================================================================================
      org.apache.wicket.request.target.component.listener.BehaviorRequestTarget

      public final void processEvents(final RequestCycle requestCycle)
      {
      // Preprocess like standard component request. Do all the initialization
      // necessary
      onProcessEvents(requestCycle);

      // Get the IBehavior for the component based on the request parameters
      final Component component = getTarget();
      final String id = getRequestParameters().getBehaviorId();
      if (id == null)

      { throw new IllegalStateException( "Parameter behaviorId was not provided: unable to locate listener. Component: " + component.toString()); }

      final int idAsInt = Integer.parseInt(id);
      IBehaviorListener behaviorListener = null;
      if (component.getBehaviors().size() > idAsInt)

        Issue Links

          Activity

          Stanislav Dvorscak created issue -
          Igor Vaynberg made changes -
          Field Original Value New Value
          Priority Blocker [ 1 ] Critical [ 2 ]
          Hide
          Johan Compagner added a comment -

          the only quick fix for this is NOT to remove the temp behaviors but to let them be disabled..
          because i dont see that we fix this differently in the 1.3 branch
          maybe 1.4 but in 1.5 matej is busy with this.

          What we maybe also could do quickly is generating a hashkey for all the behaviors and use that one
          instead of an index in a list.

          This key can be as simple as 1,2 or 3 but it is stored in the behavior.

          Show
          Johan Compagner added a comment - the only quick fix for this is NOT to remove the temp behaviors but to let them be disabled.. because i dont see that we fix this differently in the 1.3 branch maybe 1.4 but in 1.5 matej is busy with this. What we maybe also could do quickly is generating a hashkey for all the behaviors and use that one instead of an index in a list. This key can be as simple as 1,2 or 3 but it is stored in the behavior.
          Hide
          Igor Vaynberg added a comment -

          it cannot be stored in the behavior because it is an interface, it has to be stored with the behavior

          instead of ibehavior[] we need to store them as behaviorholder[short uid,ibehavior] and use uid to generate the url.

          Show
          Igor Vaynberg added a comment - it cannot be stored in the behavior because it is an interface, it has to be stored with the behavior instead of ibehavior[] we need to store them as behaviorholder [short uid,ibehavior] and use uid to generate the url.
          Hide
          Johan Compagner added a comment -

          that would mean that there is an extra new object with 2 references for every behavior.

          I know its an interface but if we did make setId(short)/getId() in the interface and have that in our abstract behavior
          then only who are making really custom behaviors have to follow that contract. but the rest of the world doesnt really mind/care

          Show
          Johan Compagner added a comment - that would mean that there is an extra new object with 2 references for every behavior. I know its an interface but if we did make setId(short)/getId() in the interface and have that in our abstract behavior then only who are making really custom behaviors have to follow that contract. but the rest of the world doesnt really mind/care
          Hide
          Johan Compagner added a comment -

          or if we could use hashcode or something

          Show
          Johan Compagner added a comment - or if we could use hashcode or something
          Hide
          Igor Vaynberg added a comment -

          what about letting component store it as metadata, so in urlfor() it can keep a reference to behavior and index and in ondetach we check if the behavior is gone and remove the metadata, kinda like our own weakhashmap

          then we can implement this transparently for 1.3 also

          Show
          Igor Vaynberg added a comment - what about letting component store it as metadata, so in urlfor() it can keep a reference to behavior and index and in ondetach we check if the behavior is gone and remove the metadata, kinda like our own weakhashmap then we can implement this transparently for 1.3 also
          Hide
          Johan Compagner added a comment -

          you mean we keep the meta data if the behavior is removed?
          and if there are no removed behaviors we just remove it?

          what about keeping the index but just let there be a hole in it he behaviors list?

          so if we have

          behavior1
          tempbehavior2
          behavior3

          and then we just make sure this happens:

          behavior1
          null
          behavior3

          dont know if our current storage can handle that easily and of course things like getBehaviors() must filter that

          Show
          Johan Compagner added a comment - you mean we keep the meta data if the behavior is removed? and if there are no removed behaviors we just remove it? what about keeping the index but just let there be a hole in it he behaviors list? so if we have behavior1 tempbehavior2 behavior3 and then we just make sure this happens: behavior1 null behavior3 dont know if our current storage can handle that easily and of course things like getBehaviors() must filter that
          Hide
          Igor Vaynberg added a comment -

          i mean

          when someone calls urlfor we add a metadata

          {behavior,uid}

          to the component. in ondetach() we have a check that iterates this metadata and checks if the behavior is still there, removing the metadata if it isnt.

          so there is only overhead (of metadata) for behaviors that use urls, and it is no problem removing the behaviors because metadata keeps a uid independent of the index. and its all cleaned up if the behavior that called urlfor is removed.

          i also thought about being able to allow the holes in the behavior list, but taht seems like a pita to maintain...

          Show
          Igor Vaynberg added a comment - i mean when someone calls urlfor we add a metadata {behavior,uid} to the component. in ondetach() we have a check that iterates this metadata and checks if the behavior is still there, removing the metadata if it isnt. so there is only overhead (of metadata) for behaviors that use urls, and it is no problem removing the behaviors because metadata keeps a uid independent of the index. and its all cleaned up if the behavior that called urlfor is removed. i also thought about being able to allow the holes in the behavior list, but taht seems like a pita to maintain...
          Hide
          Johan Compagner added a comment -

          meta data is expensive.
          so i want that to be cleanup if the index is still valid (so there is no behavior removed before it)

          If for every ajax behavior we (talking about servoy now) have we would have a meta data in our component structure our memory usage would explode.
          Because we have many ajax behaviors on many components and we never use temp behaviors almost never is a behavior removed.
          So that would for use mean a memory over head for completely nothing.

          So we could use meta data but then purely for behaviors that are out of sync with the index..

          Show
          Johan Compagner added a comment - meta data is expensive. so i want that to be cleanup if the index is still valid (so there is no behavior removed before it) If for every ajax behavior we (talking about servoy now) have we would have a meta data in our component structure our memory usage would explode. Because we have many ajax behaviors on many components and we never use temp behaviors almost never is a behavior removed. So that would for use mean a memory over head for completely nothing. So we could use meta data but then purely for behaviors that are out of sync with the index..
          Hide
          Igor Vaynberg added a comment -

          doing it just for out of sync stuff is too complicated, i would rather have the ibehavior[] arrray support nulls.

          Show
          Igor Vaynberg added a comment - doing it just for out of sync stuff is too complicated, i would rather have the ibehavior[] arrray support nulls.
          Igor Vaynberg made changes -
          Link This issue relates to WICKET-1483 [ WICKET-1483 ]
          Igor Vaynberg made changes -
          Assignee Igor Vaynberg [ ivaynberg ]
          Igor Vaynberg made changes -
          Link This issue duplicates WICKET-1483 [ WICKET-1483 ]
          Igor Vaynberg made changes -
          Link This issue relates to WICKET-1483 [ WICKET-1483 ]
          Igor Vaynberg made changes -
          Resolution Duplicate [ 3 ]
          Status Open [ 1 ] Closed [ 6 ]

            People

            • Assignee:
              Igor Vaynberg
              Reporter:
              Stanislav Dvorscak
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development