Re: Stable behavior ids
+1 On 15/05/20 09:01, Emond Papegaaij wrote: Hi All, With Sven's comment in mind and the fact that Andrea did not directly understand the implications of the new method, I decided that the new method was indeed not a good idea. Don't worry about me, it appends to me quite often to not understand someone else code, sometimes even mine :-) It complicates the API and brings too little benefit. I've updated the PR once more to clarify the impact and usefulness of getBehaviorId without introducting a new method. I did make one minor change to the API: you can no longer request stable ids for temporary behaviors. IMHO that does not make sense. The behavior will be gone the next request. Can this now be merged? Best regards, Emond On Mon, May 11, 2020 at 11:12 PM Emond Papegaaij wrote:
Re: Stable behavior ids
Hi Edmond, +1 from me. Thanks Sven On 15.05.20 09:01, Emond Papegaaij wrote: Hi All, With Sven's comment in mind and the fact that Andrea did not directly understand the implications of the new method, I decided that the new method was indeed not a good idea. It complicates the API and brings too little benefit. I've updated the PR once more to clarify the impact and usefulness of getBehaviorId without introducting a new method. I did make one minor change to the API: you can no longer request stable ids for temporary behaviors. IMHO that does not make sense. The behavior will be gone the next request. Can this now be merged? Best regards, Emond On Mon, May 11, 2020 at 11:12 PM Emond Papegaaij wrote: Hi Sven, I agree with you that this method is not very nice. It indeed complicates the API even more and it is very hard to explain why it is needed. It however is the most efficient implementation I could think of. There is a compromise we can use: we can freeze all behavior ids when one is requested. This can be stored via a flag in the component. The downside is that after this flag has been set, it is no longer possible to compact the array of behaviors. This can lead to an increased serialization size when behaviors are removed from the component. It is a compromise I can live with. The API is cleaner and the increase in size is small and only in corner cases. What do you think? Even with this change, we should clarify the docs. Best regards, Emond Op ma 11 mei 2020 17:56 schreef Sven Meier : Hi Edmond, I agree that we should get rid of BehaviorIdList, it's probably the most inefficient implementation. But I'm against complicating matters even more with an additional method - at least until we know how the implementation will be improved post-9.0. Why would anyone need a non-stable id? How often does this happen? What if the method returns true, but getId() is never actually called? IMHO this is the wrong timing to adjust the API. Have fun Sven On 08.05.20 07:59, Emond Papegaaij wrote: Hi Sven, Yes, this is all about optimization. It already was. It is not difficult to keep stable ids for behaviors, you can simply store an id per behavior. However, this is very inefficient. A long time ago, the decision was made to only store these ids when needed. Unfortunately, there is no way of knowing when this is actually needed. Currently, wicket will start recording these ids when a single id is requested. The getBehaviorId method has this as a side effect. The method is also called when a behavior that is not stateless is added to a component. There are 3 things I don't like about this: first the side effect, second the way the ids are stored is very inefficient and last the fact that whether a behavior is stateless or not has nothing to do with its id. That's why I propose to introduce this new method. It allows me to solve all 3 cases. To complicate things even more, there is also stateless ajax, which I would rather call ajax by luck at the moment. Stateless ajax requests a stable id and then discards the component storing this id. There is no guarantee at all that the behavior will get the same id in the next request. This currently is not explained in the javadoc. Does this make it more clear? I really feel we should have this ironed out before the 9 release, even though it has been the way it currently is since somewhere in 7.x. To me it feels like a loose end in the API and this is preventing me from improving the code that implements this API. Best regards, Emond Op do 7 mei 2020 22:19 schreef Sven Meier : Hi Emond, I'm not happy with that method. It's one additional method like many others in Wicket which are not clear what they are good for. I didn't understand the description. What's the reason for this stable id hassle? Why can't the id be stable when a behavior wants it? Is this really needed for optimization? Thanks Sven On 07.05.20 21:04, Emond Papegaaij wrote: Hi all, I've updated my pull request at https://github.com/apache/wicket/pull/432 . I've added the method I described and changed the current code to use this new method. Also, the documentation is updated and clarified to better reflect the rules a developer has to follow to keep stable ids. This PR should not change any behavior, but it does allow me to keep on working on my rewrite without having to break the contract of getBehaviorId. The PR splits the stable id part from the behavior being stateful or stateless: a stateless behavior can still request a stable id and a stateful behavior may not need it. IMHO this change makes it more clear to a developer what to expect. I would like to merge this into 9 before the release, even though strictly speaking it is not an API break. What do you think? Best regards, Emond On Thu, May 7, 2020 at 1:58 PM Emond Papegaaij wrote: Hi Sven and Andrea, Sven: All tests still pass, I've also updated the ImmutableBehaviorIdsTest with some additional
Re: Stable behavior ids
Hi All, With Sven's comment in mind and the fact that Andrea did not directly understand the implications of the new method, I decided that the new method was indeed not a good idea. It complicates the API and brings too little benefit. I've updated the PR once more to clarify the impact and usefulness of getBehaviorId without introducting a new method. I did make one minor change to the API: you can no longer request stable ids for temporary behaviors. IMHO that does not make sense. The behavior will be gone the next request. Can this now be merged? Best regards, Emond On Mon, May 11, 2020 at 11:12 PM Emond Papegaaij wrote: > > Hi Sven, > > I agree with you that this method is not very nice. It indeed complicates the > API even more and it is very hard to explain why it is needed. It however is > the most efficient implementation I could think of. > > There is a compromise we can use: we can freeze all behavior ids when one is > requested. This can be stored via a flag in the component. The downside is > that after this flag has been set, it is no longer possible to compact the > array of behaviors. This can lead to an increased serialization size when > behaviors are removed from the component. It is a compromise I can live with. > The API is cleaner and the increase in size is small and only in corner > cases. What do you think? > > Even with this change, we should clarify the docs. > > Best regards, > Emond > > Op ma 11 mei 2020 17:56 schreef Sven Meier : >> >> Hi Edmond, >> >> I agree that we should get rid of BehaviorIdList, it's probably the most >> inefficient implementation. >> >> But I'm against complicating matters even more with an additional method >> - at least until we know how the implementation will be improved post-9.0. >> >> Why would anyone need a non-stable id? How often does this happen? >> What if the method returns true, but getId() is never actually called? >> >> IMHO this is the wrong timing to adjust the API. >> >> Have fun >> Sven >> >> >> On 08.05.20 07:59, Emond Papegaaij wrote: >> > Hi Sven, >> > >> > Yes, this is all about optimization. It already was. It is not difficult to >> > keep stable ids for behaviors, you can simply store an id per behavior. >> > However, this is very inefficient. A long time ago, the decision was made >> > to only store these ids when needed. Unfortunately, there is no way of >> > knowing when this is actually needed. Currently, wicket will start >> > recording these ids when a single id is requested. The getBehaviorId method >> > has this as a side effect. The method is also called when a behavior that >> > is not stateless is added to a component. >> > >> > There are 3 things I don't like about this: first the side effect, second >> > the way the ids are stored is very inefficient and last the fact that >> > whether a behavior is stateless or not has nothing to do with its id. >> > That's why I propose to introduce this new method. It allows me to solve >> > all 3 cases. >> > >> > To complicate things even more, there is also stateless ajax, which I would >> > rather call ajax by luck at the moment. Stateless ajax requests a stable id >> > and then discards the component storing this id. There is no guarantee at >> > all that the behavior will get the same id in the next request. This >> > currently is not explained in the javadoc. >> > >> > Does this make it more clear? I really feel we should have this ironed out >> > before the 9 release, even though it has been the way it currently is since >> > somewhere in 7.x. To me it feels like a loose end in the API and this is >> > preventing me from improving the code that implements this API. >> > >> > Best regards, >> > Emond >> > >> > Op do 7 mei 2020 22:19 schreef Sven Meier : >> > >> >> Hi Emond, >> >> >> >> I'm not happy with that method. >> >> >> >> It's one additional method like many others in Wicket which are not >> >> clear what they are good for. I didn't understand the description. >> >> >> >> What's the reason for this stable id hassle? Why can't the id be stable >> >> when a behavior wants it? >> >> Is this really needed for optimization? >> >> >> >> Thanks >> >> Sven >> >> >> >> >> >> On 07.05.20 21:04, Emond Papegaaij wrote: >> >>> Hi all, >> >>> >> >>> I've updated my pull request at >> >>> https://github.com/apache/wicket/pull/432 . I've added the method I >> >>> described and changed the current code to use this new method. Also, >> >>> the documentation is updated and clarified to better reflect the rules >> >>> a developer has to follow to keep stable ids. This PR should not >> >>> change any behavior, but it does allow me to keep on working on my >> >>> rewrite without having to break the contract of getBehaviorId. The PR >> >>> splits the stable id part from the behavior being stateful or >> >>> stateless: a stateless behavior can still request a stable id and a >> >>> stateful behavior may not need it. IMHO this change makes it more >> >>> clear to a developer
Re: Stable behavior ids
Hi Sven, I agree with you that this method is not very nice. It indeed complicates the API even more and it is very hard to explain why it is needed. It however is the most efficient implementation I could think of. There is a compromise we can use: we can freeze all behavior ids when one is requested. This can be stored via a flag in the component. The downside is that after this flag has been set, it is no longer possible to compact the array of behaviors. This can lead to an increased serialization size when behaviors are removed from the component. It is a compromise I can live with. The API is cleaner and the increase in size is small and only in corner cases. What do you think? Even with this change, we should clarify the docs. Best regards, Emond Op ma 11 mei 2020 17:56 schreef Sven Meier : > Hi Edmond, > > I agree that we should get rid of BehaviorIdList, it's probably the most > inefficient implementation. > > But I'm against complicating matters even more with an additional method > - at least until we know how the implementation will be improved post-9.0. > > Why would anyone need a non-stable id? How often does this happen? > What if the method returns true, but getId() is never actually called? > > IMHO this is the wrong timing to adjust the API. > > Have fun > Sven > > > On 08.05.20 07:59, Emond Papegaaij wrote: > > Hi Sven, > > > > Yes, this is all about optimization. It already was. It is not difficult > to > > keep stable ids for behaviors, you can simply store an id per behavior. > > However, this is very inefficient. A long time ago, the decision was made > > to only store these ids when needed. Unfortunately, there is no way of > > knowing when this is actually needed. Currently, wicket will start > > recording these ids when a single id is requested. The getBehaviorId > method > > has this as a side effect. The method is also called when a behavior that > > is not stateless is added to a component. > > > > There are 3 things I don't like about this: first the side effect, second > > the way the ids are stored is very inefficient and last the fact that > > whether a behavior is stateless or not has nothing to do with its id. > > That's why I propose to introduce this new method. It allows me to solve > > all 3 cases. > > > > To complicate things even more, there is also stateless ajax, which I > would > > rather call ajax by luck at the moment. Stateless ajax requests a stable > id > > and then discards the component storing this id. There is no guarantee at > > all that the behavior will get the same id in the next request. This > > currently is not explained in the javadoc. > > > > Does this make it more clear? I really feel we should have this ironed > out > > before the 9 release, even though it has been the way it currently is > since > > somewhere in 7.x. To me it feels like a loose end in the API and this is > > preventing me from improving the code that implements this API. > > > > Best regards, > > Emond > > > > Op do 7 mei 2020 22:19 schreef Sven Meier : > > > >> Hi Emond, > >> > >> I'm not happy with that method. > >> > >> It's one additional method like many others in Wicket which are not > >> clear what they are good for. I didn't understand the description. > >> > >> What's the reason for this stable id hassle? Why can't the id be stable > >> when a behavior wants it? > >> Is this really needed for optimization? > >> > >> Thanks > >> Sven > >> > >> > >> On 07.05.20 21:04, Emond Papegaaij wrote: > >>> Hi all, > >>> > >>> I've updated my pull request at > >>> https://github.com/apache/wicket/pull/432 . I've added the method I > >>> described and changed the current code to use this new method. Also, > >>> the documentation is updated and clarified to better reflect the rules > >>> a developer has to follow to keep stable ids. This PR should not > >>> change any behavior, but it does allow me to keep on working on my > >>> rewrite without having to break the contract of getBehaviorId. The PR > >>> splits the stable id part from the behavior being stateful or > >>> stateless: a stateless behavior can still request a stable id and a > >>> stateful behavior may not need it. IMHO this change makes it more > >>> clear to a developer what to expect. I would like to merge this into 9 > >>> before the release, even though strictly speaking it is not an API > >>> break. What do you think? > >>> > >>> Best regards, > >>> Emond > >>> > >>> On Thu, May 7, 2020 at 1:58 PM Emond Papegaaij > >>> wrote: > 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
Re: Stable behavior ids
Hi Edmond, I agree that we should get rid of BehaviorIdList, it's probably the most inefficient implementation. But I'm against complicating matters even more with an additional method - at least until we know how the implementation will be improved post-9.0. Why would anyone need a non-stable id? How often does this happen? What if the method returns true, but getId() is never actually called? IMHO this is the wrong timing to adjust the API. Have fun Sven On 08.05.20 07:59, Emond Papegaaij wrote: Hi Sven, Yes, this is all about optimization. It already was. It is not difficult to keep stable ids for behaviors, you can simply store an id per behavior. However, this is very inefficient. A long time ago, the decision was made to only store these ids when needed. Unfortunately, there is no way of knowing when this is actually needed. Currently, wicket will start recording these ids when a single id is requested. The getBehaviorId method has this as a side effect. The method is also called when a behavior that is not stateless is added to a component. There are 3 things I don't like about this: first the side effect, second the way the ids are stored is very inefficient and last the fact that whether a behavior is stateless or not has nothing to do with its id. That's why I propose to introduce this new method. It allows me to solve all 3 cases. To complicate things even more, there is also stateless ajax, which I would rather call ajax by luck at the moment. Stateless ajax requests a stable id and then discards the component storing this id. There is no guarantee at all that the behavior will get the same id in the next request. This currently is not explained in the javadoc. Does this make it more clear? I really feel we should have this ironed out before the 9 release, even though it has been the way it currently is since somewhere in 7.x. To me it feels like a loose end in the API and this is preventing me from improving the code that implements this API. Best regards, Emond Op do 7 mei 2020 22:19 schreef Sven Meier : Hi Emond, I'm not happy with that method. It's one additional method like many others in Wicket which are not clear what they are good for. I didn't understand the description. What's the reason for this stable id hassle? Why can't the id be stable when a behavior wants it? Is this really needed for optimization? Thanks Sven On 07.05.20 21:04, Emond Papegaaij wrote: Hi all, I've updated my pull request at https://github.com/apache/wicket/pull/432 . I've added the method I described and changed the current code to use this new method. Also, the documentation is updated and clarified to better reflect the rules a developer has to follow to keep stable ids. This PR should not change any behavior, but it does allow me to keep on working on my rewrite without having to break the contract of getBehaviorId. The PR splits the stable id part from the behavior being stateful or stateless: a stateless behavior can still request a stable id and a stateful behavior may not need it. IMHO this change makes it more clear to a developer what to expect. I would like to merge this into 9 before the release, even though strictly speaking it is not an API break. What do you think? Best regards, Emond On Thu, May 7, 2020 at 1:58 PM Emond Papegaaij wrote: 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
Re: Stable behavior ids
Hi Sven, Yes, this is all about optimization. It already was. It is not difficult to keep stable ids for behaviors, you can simply store an id per behavior. However, this is very inefficient. A long time ago, the decision was made to only store these ids when needed. Unfortunately, there is no way of knowing when this is actually needed. Currently, wicket will start recording these ids when a single id is requested. The getBehaviorId method has this as a side effect. The method is also called when a behavior that is not stateless is added to a component. There are 3 things I don't like about this: first the side effect, second the way the ids are stored is very inefficient and last the fact that whether a behavior is stateless or not has nothing to do with its id. That's why I propose to introduce this new method. It allows me to solve all 3 cases. To complicate things even more, there is also stateless ajax, which I would rather call ajax by luck at the moment. Stateless ajax requests a stable id and then discards the component storing this id. There is no guarantee at all that the behavior will get the same id in the next request. This currently is not explained in the javadoc. Does this make it more clear? I really feel we should have this ironed out before the 9 release, even though it has been the way it currently is since somewhere in 7.x. To me it feels like a loose end in the API and this is preventing me from improving the code that implements this API. Best regards, Emond Op do 7 mei 2020 22:19 schreef Sven Meier : > Hi Emond, > > I'm not happy with that method. > > It's one additional method like many others in Wicket which are not > clear what they are good for. I didn't understand the description. > > What's the reason for this stable id hassle? Why can't the id be stable > when a behavior wants it? > Is this really needed for optimization? > > Thanks > Sven > > > On 07.05.20 21:04, Emond Papegaaij wrote: > > Hi all, > > > > I've updated my pull request at > > https://github.com/apache/wicket/pull/432 . I've added the method I > > described and changed the current code to use this new method. Also, > > the documentation is updated and clarified to better reflect the rules > > a developer has to follow to keep stable ids. This PR should not > > change any behavior, but it does allow me to keep on working on my > > rewrite without having to break the contract of getBehaviorId. The PR > > splits the stable id part from the behavior being stateful or > > stateless: a stateless behavior can still request a stable id and a > > stateful behavior may not need it. IMHO this change makes it more > > clear to a developer what to expect. I would like to merge this into 9 > > before the release, even though strictly speaking it is not an API > > break. What do you think? > > > > Best regards, > > Emond > > > > On Thu, May 7, 2020 at 1:58 PM Emond Papegaaij > > wrote: > >> 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,
Re: Stable behavior ids
Hi Emond, I'm not happy with that method. It's one additional method like many others in Wicket which are not clear what they are good for. I didn't understand the description. What's the reason for this stable id hassle? Why can't the id be stable when a behavior wants it? Is this really needed for optimization? Thanks Sven On 07.05.20 21:04, Emond Papegaaij wrote: Hi all, I've updated my pull request at https://github.com/apache/wicket/pull/432 . I've added the method I described and changed the current code to use this new method. Also, the documentation is updated and clarified to better reflect the rules a developer has to follow to keep stable ids. This PR should not change any behavior, but it does allow me to keep on working on my rewrite without having to break the contract of getBehaviorId. The PR splits the stable id part from the behavior being stateful or stateless: a stateless behavior can still request a stable id and a stateful behavior may not need it. IMHO this change makes it more clear to a developer what to expect. I would like to merge this into 9 before the release, even though strictly speaking it is not an API break. What do you think? Best regards, Emond On Thu, May 7, 2020 at 1:58 PM Emond Papegaaij wrote: 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 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
Re: Stable behavior ids
Hi all, I've updated my pull request at https://github.com/apache/wicket/pull/432 . I've added the method I described and changed the current code to use this new method. Also, the documentation is updated and clarified to better reflect the rules a developer has to follow to keep stable ids. This PR should not change any behavior, but it does allow me to keep on working on my rewrite without having to break the contract of getBehaviorId. The PR splits the stable id part from the behavior being stateful or stateless: a stateless behavior can still request a stable id and a stateful behavior may not need it. IMHO this change makes it more clear to a developer what to expect. I would like to merge this into 9 before the release, even though strictly speaking it is not an API break. What do you think? Best regards, Emond On Thu, May 7, 2020 at 1:58 PM Emond Papegaaij wrote: > > 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 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
Re: Stable behavior ids
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 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 > >> 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
Re: Stable behavior ids
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
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
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
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
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
Re: Stable behavior ids
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 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
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
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
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
Re: Stable behavior ids
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. 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 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
> 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
> > 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
> > 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
Re: Stable behavior ids
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
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
> 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
> 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
>
Re: Stable behavior ids
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
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 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
Re: Stable behavior ids
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 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
Re: Stable behavior ids
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
