Hi Sven and Andrea,
Sven:
All tests still pass, I've also updated the ImmutableBehaviorIdsTest
with some additional checks. However, the tests I've seen so far are
not very exhaustive on this part. Many of them also pass with a bit of
luck :)
Andrea:
What I mean with 'moved to the front' is that a behavior with a stable
id is assigned a low number. For example, when you add these 4
behaviors in this order: class-x, class-y, an ajax link and a special
stateless behavior which uses its id, the ajax link will currently get
id 0. If you then request the id of class-y, it will get id 2 (just
after the special one). In my current implementation, the ajax link
will get id 2 and class-y will get 1 (using the index). I'm planning
to change this to move behaviors to the front when a stable id is
required. The special behavior can override this method to return
true, in which case the id's will be 0 for link, 1 for special, 2 for
class-x, 3 for class-y. The difference is this is that for the last 2,
this id is not guaranteed to be stable over requests. For example,
when class-x is removed somewhere during a request, class-y will get
id 2 on the next request, because the array storing the behaviors is
compacted, removing null-slots.
The default implementation of the new method will simply look at
getStatelessHint on the behavior, stateless means not a stable id. On
ajax behaviors, this can be overridden to return true, even though the
behavior can be stateless. The point is, that for a stateless
behavior, this information is still stored in the component, which may
also be stateless and not being able to store state at all. This is
already the case in the current implementation, and will remain the
same in my implementation. I'm trying to keep things as close to what
it already was, but without having to store the additional
BehaviorIdList.
Emond
On Thu, May 7, 2020 at 1:13 PM Sven Meier <s...@meiers.net> wrote:
>
> Hi,
>
> we have a test for stateless behaviors:
>
> org.apache.wicket.ajax.markup.html.form.StatelessAjaxSubmitLinkTest
>
> We should just make sure it still works.
>
> Have fun
> Sven
>
>
> On 07.05.20 12:13, Andrea Del Bene wrote:
> >
> > On 07/05/20 10:37, Emond Papegaaij wrote:
> >> Hi Martin,
> >>
> >> I know what these id's are for, and this still works as expected.
> >> However, your claim about stable id's on stateless pages currently
> >> does not work as you describe. The id's of behaviors is stored in the
> >> component they are added to, also when the component is stateless. As
> >> you said, a stateless page is discarded at the end of a request,
> >> including the components and the recording of the stable id's. On the
> >> next request, the page is reconstructed, but Wicket cannot guarantee
> >> that the behavior id's are the same. For example, when you
> >> conditionally add a behavior, it influences the id's of the behaviors
> >> added after it. I want the documentation to reflect this, and use this
> >> to optimize the code. IMHO there's no point in storing id's for
> >> stateless behaviors for the next request. If you absolutely need this
> >> guarantee, you should make your behavior stateful.
> >>
> >> One difference between my implementation and the current one is that
> >> behaviors with stable id's are currently moved to the front.
> >
> > Hi Emond,
> >
> > I don't perfectly understand what you mean with 'moved to the front'.
> > Could please explain it?
> > To be clear about stateless behavior and ids, actually we can have a
> > "predictable" id for them if we avoid things like conditional adding,
> > etc... In short we can have stable ids for stateless behaviors but the
> > responsibility for them is up to the developer and not to the
> > framework. This is critical to make complex stateless components (for
> > example AJAX links) properly work. I guess your PR preserve this
> > condition. And yes, I agree there's no need for storing stateless
> > behaviors ids.
> >
> > Andrea.
> >> This
> >> makes the id's a bit more stable. I'm planning to do something
> >> similar, via a new method on Behavior. This should all be API
> >> compatible. I hope this clarifies my intent.
> >>
> >> Emond
> >>
> >> On Thu, May 7, 2020 at 9:23 AM Martin Grigorov <mgrigo...@apache.org>
> >> wrote:
> >>> Hi Emond,
> >>>
> >>> The behavior id is something internal to Wicket.
> >>> It is used *by* Wicket to find the requested behavior in a Component.
> >>> For example if a Component has more than one Ajax behaviors (yes, Ajax
> >>> behaviors could be stateless too since 7.4.0. Before that there was a
> >>> project in WicketStuff) it uses the id (extracted from the special
> >>> parameter in the url) to find which behavior exactly should be invoked.
> >>> In case of stateful page the ids are stored with page and later
> >>> deserialized.
> >>> In case of stateless page (with stateless Ajax behaviors) the whole
> >>> page is
> >>> discarded at the end of request 1 but at request 2 a new page is
> >>> created
> >>> from scratch with all its components and their behaviors. Here the id
> >>> *must* be the same as in request 1, otherwise Wicket may execute
> >>> another
> >>> behavior's onRequest(). So, the ids must be stable in case of
> >>> stateless as
> >>> well.
> >>>
> >>>
> >>>
> >>> On Wed, May 6, 2020 at 9:28 PM Emond Papegaaij
> >>> <emond.papega...@gmail.com>
> >>> wrote:
> >>>
> >>>> Hi,
> >>>>
> >>>> While running, I came up with a solution for which only a minor change
> >>>> to the contract of the method is needed: ids for stateless behaviors
> >>>> will be stable within a single requests, but can change over requests.
> >>>> I think this is reasonable, given the way stateless components work.
> >>>>
> >>>> I would like to change the documentation to read:
> >>>> Gets a stable id for the specified behavior. The id remains stable
> >>>> from the point this method is first called for the behavior until the
> >>>> behavior has been removed from the component. For {@linkplain
> >>>> Behavior#getStatelessHint(Component) stateful} behaviors, this stable
> >>>> id is retained over requests. For stateless behaviors, the id can
> >>>> change between requests.
> >>>>
> >>>> Emond
> >>>>
> >>>> On Wed, May 6, 2020 at 5:54 PM Emond Papegaaij
> >>>> <emond.papega...@gmail.com> wrote:
> >>>>> Hi Andrea,
> >>>>>
> >>>>> Stateful is fine, stateless is not. A component cannot keep a stable
> >>>>> id when it's stateless. I'm trying out a change that forbids
> >>>>> getBehaviorId() for stateless behaviors, and I'm hitting a few tests
> >>>>> with stateless ajax. I don't see how this is supposed to work anyway.
> >>>>> We request the component to store the id of a certain behavior,
> >>>>> but it
> >>>>> must do so in a stateless way. IMHO that's impossible. The component
> >>>>> will be discarded, along with its state and the stored id at the end
> >>>>> of the request. No way of guaranteeing that the same component with
> >>>>> the same behavior at the same index will exist at the next request.
> >>>>>
> >>>>> This brings me back to my suggested change in the documentation: only
> >>>>> stateful behaviors have guaranteed stable ids. You can request the id
> >>>>> of a stateless behavior, but (in my current implementation) it may
> >>>>> change when you remove behaviors from the component.
> >>>>>
> >>>>> Emond
> >>>>>
> >>>>> On Wed, May 6, 2020 at 5:41 PM Andrea Del Bene <an.delb...@gmail.com>
> >>>> wrote:
> >>>>>>
> >>>>>> On 06/05/20 16:52, Emond Papegaaij wrote:
> >>>>>>> Hi all,
> >>>>>>>
> >>>>>>> During my refactoring of the component state (WICKET-6774) I
> >>>>>>> noticed
> >>>>>>> that behavior ids are currently stored in a very inefficient
> >>>>>>> way: an
> >>>>>>> ArrayList is added to Component data to store references to
> >>>>>>> behaviors
> >>>>>>> with a stable id. On my branch I have eliminated this ArrayList,
> >>>>>>> greatly reducing the size of components with stateful behaviors
> >>>>>>> (such
> >>>>>>> as AjaxLink).
> >>>>>>>
> >>>>>>> A behavior gets a stable id when it is stateful to be able to
> >>>>>>> render
> >>>>>>> this id in an URL. However, at the moment, it also gets a stable id
> >>>>>>> when Component.getBehaviorId is called for that particular
> >>>>>>> behavior.
> >>>>>>> This is also documented in the method's javadoc. Do we really need
> >>>>>>> this last part? It complicates the code a lot. In our code base nor
> >>>> in
> >>>>>>> Wicket can I find a single place where this is actually used.
> >>>>>> Actually I see that Component.getBehaviorId is used in
> >>>>>> AbstractAjaxTimerBehavior.getTimerId() and
> >>>>>> AbstractDefaultAjaxBehavior.onBind() which are however stateful
> >>>> behaviors.
> >>>>>>> I would like to suggest a change in the javadoc to state that
> >>>>>>> stable
> >>>>>>> ids are only guaranteed for stateful behaviors and change this in
> >>>>>>> Wicket 9. The actual change in the implementation is not yet
> >>>>>>> finished
> >>>>>>> and does not need to ship in 9.0.0, but feel I cannot change the
> >>>>>>> contract of a method in a minor release.
> >>>>>>>
> >>>>>>> Best regards,
> >>>>>>> Emond
