Kito, there was no single word inside the PMC that the release will be next week. I just again scanned our internal mailboxes to verify that. Leo is just adding another messy argument which is not funded. Of course there will be a release soon, but there was no agreed date yet.
And again: there is no effective code change. Just making inner classes visible to uncover the mess. LieGrue, strub ----- Original Message ----- > From: Kito Mann <[email protected]> > To: MyFaces Development <[email protected]> > Cc: > Sent: Friday, November 16, 2012 12:54 AM > Subject: Re: StateSaving reviews > > As an observer to this argument and someone whose clients depend on > the stability of the code base, such a refactoring so close to > release seems like a bad idea. Why not do this after the release? > > Sent from my iPhone > > http://www.jsfcentral.com > http://www.Virtua.com > > > On Nov 15, 2012, at 6:36 PM, Mark Struberg <[email protected]> wrote: > >> >>> But you reverted the changes I did months ago. Didn't you? oh yes, > you >>> "refactor" it and get rid what you don't like. >> >> That is OSS Leo. We take what we find and improve it. This is not a revert > - this is evolution! >> >> There is no such thing like 'my code' in the ASF. >> >> >>> That's unfair. I don't think that's true. >> Look at what happened with the RelativeResourceHandler and where it lives > now for example. Just to name one in the not so far past. >> I'm not going further with commenting any of your personal attacks but > will further again only focus on technical arguments. >> >> >> As I said before, we can rollback the viewId to the hashCode if you like. > I'm perfectly fine with that. >> >> I'm also fine with moving the classes to another package root. But > I'm -1 on the original 15++ inner classes implementation because this is > unmaintainable. >> >> >> Again: if we set the key back to hashCode then there is no functional > change to the previous version! It's just own classes instead of inner > classes + removing duplicated code. >> >> >> Actually you really can blame me for something (actually not only me but > all other community members as well): to not have reviewed that part earlier. > I > confess for that part. This is a complicated area and we should have given > feedback way earlier as this is hard to get right. But it doesn't get easier > by having large changes developed locally and then committing it in one big > step. >> >> >>> Since the plan is do a release next week, could >>> we revert the changes, and put them on hold >> >> I still don't see the reason for it. The code is fundamentally the same > (after moving back to the hashCode) and most likely will pass the TCK. >> Please give it a try or provide fundamental feedback. The classes are still > the same, they only got moved from inner classes to own classes. >> >> >>> https://jira.springsource.org/browse/SWF-1365 >> This bug got fixed and released more than 2 years ago in SWF. This was even > done before swf-2.0 went final. I see no reason to keep any old code in > MyFaces > only for that. >> >> LieGrue, >> strub >> >> >> >> ----- Original Message ----- >>> From: Leonardo Uribe <[email protected]> >>> To: MyFaces Development <[email protected]>; Mark Struberg > <[email protected]> >>> Cc: >>> Sent: Friday, November 16, 2012 12:11 AM >>> Subject: Re: StateSaving review >>> >>> Hi >>> >>> 2012/11/15 Mark Struberg <[email protected]>: >>>> Leo, it would be great if you reflect a bit. >>>> >>>>> I have checked the code. I know what SerializedViewKey does. >>>> >>>> good! no other arguments? Well, then all is fine it seems. >>>> >>>> >>>>> it is YOU the one who are reverting code done by other people >>>> >>>> The only code I ever reverted in MyFaces was your revert of my code > change. >>> That's it. >>> >>> But you reverted the changes I did months ago. Didn't you? oh yes, > you >>> "refactor" it and >>> get rid what you don't like. >>> >>>> But actually I'm not the only one who got code reverted by you, > and >>> those people are fed up as well and partly left the community already > because of >>> that. >>>> >>> >>> That's unfair. I don't think that's true. I'm just > exercising my >>> right >>> to participate, that's it. People can disagree from time to time. >>> That's part of the life, if you can't deal with it, it's > your >>> problem. >>> But I'm always willing to look for alternatives and resolve >>> differences, like I'm doing right now, and that's what makes > the >>> difference. >>> >>>> >>>>> I'm sorry to be so direct, but as a member of the > community, I >>>>> have the right to participate and I'm willing to exercise > that >>> right. >>>> >>>> That's perfectly fine as well. So please start your excercise > with >>> listening to other people sometimes. >>>> * I listed 5++ flaws in the design. >>>> * I removed a few classes which contained 99% copy & paste. >>>> * I removed a few classes which are not used anymore since over a > year >>>> * I removed an old resources folder which was just lying around for > no >>> reason. >>>> * I moved the remaining inner classes to toplevel and an own > package to >>> make it easier to maintain and overlook the whole complexity. >>>> * I added the viewId to _one_ of the dozen key implementations. > Actually we >>> don't need it. We also don't need the viewId hashCode which was > there >>> before. So I don't care, let's move this back to your hashCode > solution >>> and fix it properly in the next release. >>>> * I cleaned up your generics because it always only used the String > type >>> anyway. And this cannot change in the future because this fact comes > directly >>> from the JSF spec. >>>> Otherwise there has been no fundamental change to your code. The > mess you >>> see is all yours so to say (sorry for the sarcasm). >>>> >>> >>> That's your opinion, not mine. But I can see some advance. At last, > there is >>> one list of the changes proposed. Since the plan is do a release next >>> week, could >>> we revert the changes, and put them on hold while the necessary steps > are done? >>> Otherwise a release will not be possible for a long time (In fact, you >>> have kidnapped >>> the release). In that way, we can dedicate enough time and resources to > get >>> it out once for all. Do you agree with that? Could you let me complete >>> the planned >>> schedule? >>> >>> Anyway, what's the deal to put the changes in 2.1.x branch? why you > are so >>> desperate for that? You have not given to me any reason why you > can't work >>> with >>> 2.1.x-client-window branch, which is best for what you are trying to >>> do, or why you >>> can wait just 2 weeks to commit the changes. >>> >>> Come on Mark, is not a big deal, right? >>> >>>> >>>>> The concrete case is spring webflow does not support PSS > algorithm, so >>>>> we need to switch back and enable compatibility mode with JSF > 1.2. >>>> >>>> Spring WebFlow uses it's own StateManager which natively > extends >>> javax.faces.StateManager. There is no MyFaces code involved anymore > since ages! >>>> >>> > http://static.springsource.org/spring-webflow/docs/2.0.x/javadoc-api/org/springframework/faces/webflow/FlowViewStateManager.html >>>> >>>> I also googled quite some time and fine nothing pointing to any > problems >>> with the MyFaces StateManager. Not even on stackoverflow. >>>> >>> >>> Look this: >>> >>> https://jira.springsource.org/browse/SWF-1365 >>> >>> Because that problem, I decided to preserve JspStateManagerImpl long > time ago, >>> specifically when MYFACES-3117 was solved, because the change over >>> ResponseStateManager imposed a change over all state management in > MyFaces. >>> Is there a problem? don't care if exists or not, what's > important is >>> provide >>> a way to go back to the old behavior. Be conservative, keep code > stable. >>> >>> regards, >>> >>> Leonardo Uribe >>> >>>> >>>> >>>> LieGrue, >>>> strub >>>> >>>> >>>> >>>> ----- Original Message ----- >>>>> From: Leonardo Uribe <[email protected]> >>>>> To: MyFaces Development <[email protected]>; Mark > Struberg >>> <[email protected]> >>>>> Cc: >>>>> Sent: Thursday, November 15, 2012 11:06 PM >>>>> Subject: Re: StateSaving review >>>>> >>>>> Hi >>>>> >>>>> 2012/11/15 Mark Struberg <[email protected]>: >>>>>> >>>>>>> You did more than that. You removed the following > classes: >>>>>>> >>>>>>> IntIntSerializedViewKey >>>>>>> IntByteArraySerializedViewKey >>>>>>> ReferenceSerializedViewKey >>>>>> >>>>>> Those have been almost 1:1 clones of other classes. Go > check >>>>> SerializedViewKey, it contains all the merged functionality. > Without >>> any >>>>> performance overhead. >>>>>> Look at the code first and then you can complain. You > revert code >>> done by >>>>> other people without even looking at the changes it seems. >>>>>> >>>>>> >>>>> >>>>> I have checked the code. I know what SerializedViewKey does. >>>>> I'm pretty aware of what it does and its effects. I have to > say it, >>>>> it is YOU the one who are reverting code done by other people >>>>> without even looking at the changes, just because you have >>>>> the perception that the changes are wrong, but nothing more. >>>>> Without any proof or any deep consideration. >>>>> >>>>> The changes I have done over the time has follow the same >>>>> protocol, create an issue, provide a patch, wait for feedback > of >>>>> the community, if no objections after a prudent time commit > them >>>>> and close the issue. In this moment the changes done by me has >>>>> the tacit aval of the community. Your changes do not have that, >>>>> because they have objections done 72 hours after they were >>>>> proposed. This is a meritocracy, and the rules of the community >>>>> in this case are on my side. >>>>> >>>>> I'm sorry to be so direct, but as a member of the > community, I >>>>> have the right to participate and I'm willing to exercise > that >>> right. >>>>> I will not look to other side and let some untested code go > into >>>>> myfaces core 2.1.x. I appreciate your efforts to make this part >>>>> better, but I encourage you to respect and play with the rules >>>>> of the community. >>>>> >>>>> Look the dates of the improvements: >>>>> >>>>> MYFACES-3563 >>>>> Created: 10/Jun/12 11:33 >>>>> Resolved: 25/Jun/12 09:05 >>>>> >>>>> MYFACES-3117 >>>>> Created: 26/Apr/11 19:08 >>>>> Resolved: 14/May/11 02:27 >>>>> >>>>> That code has been there for years!!!!!! , the latest changes > were done >>>>> 5 months ago !!!. Nobody complained at that time. Now you came > here, >>>>> complaining about the code and you want to fix it without any > interest >>>>> of what others think about that. That's not cool. >>>>> >>>>>> >>>>>>> and on top of that you removed JspStateManagerImpl, > which is a >>> class >>>>>>> that is there for compatibility with old state > managers that >>> relies on >>>>>>> MyFaces 1.2.x. >>>>>> >>>>>> This is not JSF-1.x since a long time. APIs in the > StateManager >>> got changed >>>>> for AJAX support, etc. >>>>>> >>>>>> But I'm all fine with going back to the > JspStateManagerImpl >>> again as I >>>>> already said. >>>>>> >>>>>> Btw, may I quote your answer to my question if I can > delete it? >>>>>> LU> "Remove that code is a valid option." >>>>>> >>>>> >>>>> The concrete case is spring webflow does not support PSS > algorithm, so >>>>> we need to switch back and enable compatibility mode with JSF > 1.2. >>>>> JspStateManagerImpl is there because the state manager in that > library >>>>> relies on the old behavior. I think there are other cases too. > But I >>> hope to >>>>> remove it in the future (JSF 2.2). >>>>> >>>>> regards, >>>>> >>>>> Leonardo Uribe >>>>> >>>>>> LieGrue, >>>>>> strub >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> ----- Original Message ----- >>>>>>> From: Leonardo Uribe <[email protected]> >>>>>>> To: MyFaces Development > <[email protected]>; Mark >>> Struberg >>>>> <[email protected]> >>>>>>> Cc: >>>>>>> Sent: Thursday, November 15, 2012 10:20 PM >>>>>>> Subject: Re: StateSaving review >>>>>>> >>>>>>> Hi >>>>>>> >>>>>>> 2012/11/15 Mark Struberg <[email protected]>: >>>>>>>> Leo, the stuff which currently got changed is > nothing >>> more than >>>>> just your >>>>>>> code. I only refactored it into an own package and > moved the >>> inner >>>>> classes to >>>>>>> toplevel to show up the complexity of the solution. > There was >>> no code >>>>> change >>>>>>> _yet_ other than the viewId. We can change back the > viewId to >>> the hash >>>>> if you >>>>>>> feel better. It makes no difference anyway. >>>>>>>> >>>>>>> >>>>>>> You did more than that. You removed the following > classes: >>>>>>> >>>>>>> IntIntSerializedViewKey >>>>>>> IntByteArraySerializedViewKey >>>>>>> ReferenceSerializedViewKey >>>>>>> >>>>>>> also you created this one (it was an abstract class > before) >>>>>>> >>>>>>> SerializedViewKey >>>>>>> >>>>>>> and on top of that you removed JspStateManagerImpl, > which is a >>> class >>>>>>> that is there for compatibility with old state > managers that >>> relies on >>>>>>> MyFaces 1.2.x. Obviously you have not considered > those cases, >>> but I >>>>> have >>>>>>> done that long time ago. >>>>>>> >>>>>>> Without ask, without propose a patch, without let the > people >>> who care >>>>> about >>>>>>> to participate. Just do it and let others do the > clean up. >>>>>>> >>>>>>> Mark, I have came here and proposed to you many > possible >>> alternatives >>>>>>> and you're letting me without choices. I beg you > to move >>> your >>>>> changes to >>>>>>> 2.1.x-client-window branch, and when we have certain > about >>> which >>>>> changes >>>>>>> needs to be done, backport them to 2.1.x. In just one > step, >>> without >>>>> this >>>>>>> pain and stress you are causing me. Please. The > reason why it >>> was >>>>> created >>>>>>> 2.1.x-client-window was precisely to try changes for > client >>> window / >>>>> state >>>>>>> manager and flash scope. >>>>>>> >>>>>>> In this moment I can't do any release. You have > tied my >>> hands. >>>>> I'm >>>>>>> trying to be >>>>>>> kind, please. >>>>>>> >>>>>>>> In the long run this stuff needs _heavy_ > cleanup. >>>>>>>> Again: Extracting out an abstraction over Server > vs >>> Client >>>>> stateSaving is >>>>>>> perfectly fine, but the code is now way more > complicated than >>> it ever >>>>> has been. >>>>>>> Premature abstraction is way worse than premature >>> optimisation. >>>>>>>> >>>>>>> >>>>>>> It is a work in progress, I already told you many > times. I >>> know the >>>>> abstraction >>>>>>> is not the best, but it works, and has helped a lot > to me to >>> understand >>>>> how >>>>>>> this algorithm should work. It is like a worm that > became >>> chrysalis and >>>>> in the >>>>>>> future will become a butterfly. "... the > morphing step is >>> not nice >>>>>>> ..." >>>>>>> >>>>>>> regards, >>>>>>> >>>>>>> Leonardo >>>>>>> >>>>>>>> LieGrue, >>>>>>>> strub >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> ----- Original Message ----- >>>>>>>>> From: Leonardo Uribe > <[email protected]> >>>>>>>>> To: MyFaces Development >>> <[email protected]>; Mark >>>>> Struberg >>>>>>> <[email protected]> >>>>>>>>> Cc: >>>>>>>>> Sent: Thursday, November 15, 2012 9:30 PM >>>>>>>>> Subject: Re: StateSaving review >>>>>>>>> >>>>>>>>> Hi >>>>>>>>> >>>>>>>>> 2012/11/15 Mark Struberg > <[email protected]>: >>>>>>>>>> Leo, I'm ok with a revert. But not > to your >>> version >>>>> but to the >>>>>>> original >>>>>>>>> JspStateManagerImpl! >>>>>>>>>> >>>>>>>>>> Really, this currently feels so > overcomplicated >>> without >>>>> giving >>>>>>> much >>>>>>>>> benefit. It would have been perfectly enough > to >>> remove the >>>>> viewId >>>>>>> String and >>>>>>>>> replace it with a XORShift random value if > you >>> don't trust >>>>> the >>>>>>> sequencer. >>>>>>>>>> >>>>>>>>>> I know one goal was to abstract out the > state >>> between >>>>> client and >>>>>>> server. >>>>>>>>> But that doesn't mean that we end up > with Object, >>> Object >>>>> and do >>>>>>> hardcoded >>>>>>>>> upcasts as done right now in the code. This > is no >>> improvement >>>>> over the >>>>>>> original >>>>>>>>> code. The windowId change would have been 30 > LOC btw. >>>>>>>>>> >>>>>>>>> >>>>>>>>> It is fine if you want to change the > implementation >>> to make >>>>> something >>>>>>>>> better. What's not fine, is that you > want to work >>> in 2.1.x >>>>> branch >>>>>>>>> directly, that is right now in maintenance > stage. So, >>> you are >>>>> doing >>>>>>>>> all kind of refactorings, and that makes me > crazy. >>> The last >>>>> release >>>>>>>>> was done on 23/Sep/12 and I'm doing 1 > release >>> each 2 >>>>> months. Your >>>>>>>>> changes will take longer and I need time to > review >>> them, >>>>> because I >>>>>>>>> will not do a release over changes that I > have not >>> seen. >>>>>>>>> >>>>>>>>> My idea is start a release process next > week. Why >>> don't >>>>> you take >>>>>>> your >>>>>>>>> time?, instead you try to do changes without > any >>> clear >>>>> direction. >>>>>>>>> >>>>>>>>> In this moments, we are starting the > necessary steps >>> to work >>>>> on 2.2.x. >>>>>>>>> You should be working in 2.1.x-client-window > branch >>> !!!!! >>>>>>>>> >>>>>>>>> regards, >>>>>>>>> >>>>>>>>> Leonardo Uribe >>>>>>>>> >>>>>>>>>> LieGrue, >>>>>>>>>> strub >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> ----- Original Message ----- >>>>>>>>>>> From: Leonardo Uribe >>> <[email protected]> >>>>>>>>>>> To: MyFaces Development >>>>> <[email protected]>; Mark >>>>>>> Struberg >>>>>>>>> <[email protected]> >>>>>>>>>>> Cc: >>>>>>>>>>> Sent: Thursday, November 15, 2012 > 8:44 PM >>>>>>>>>>> Subject: Re: StateSaving review >>>>>>>>>>> >>>>>>>>>>> Hi >>>>>>>>>>> >>>>>>>>>>> 2012/11/15 Mark Struberg >>> <[email protected]>: >>>>>>>>>>>> Btw, I still do not see where > the trick >>> for >>>>> storing a >>>>>>> separate >>>>>>>>> state list >>>>>>>>>>> for each browser tab is hidden. >>>>>>>>>>>> That was the goal of all the >>> refactoring >>>>> initially. Where >>>>>>> is that >>>>>>>>> done? >>>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> Mark, months ago we created >>> 2.1.x-client-window >>>>> branch: >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>> >>>>>>> >>>>> >>> > http://svn.apache.org/repos/asf/myfaces/core/branches/2.1.x-client-window/ >>>>>>>>>>> >>>>>>>>>>> This branch is the same as 2.1.x, > but only >>> with the >>>>> changes >>>>>>> proposed >>>>>>>>>>> long time ago for JSF 2.2 client > window. >>>>>>>>>>> >>>>>>>>>>> It has what you need to have a > state list >>> for each >>>>> browser >>>>>>> tab. >>>>>>>>>>> The only thing you need to provide > is a >>> ClientWindow >>>>>>> implementation >>>>>>>>>>> and that's it. The state saving > code >>> already has >>>>> included >>>>>>> the >>>>>>>>> client-window >>>>>>>>>>> concept. I also fixed Flash scope > to use >>> client >>>>> window. >>>>>>>>>>> It is also available in a maver > repo: >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>> >>>>>>> >>>>> >>> > https://repository.apache.org/content/repositories/snapshots/org/apache/myfaces/core/myfaces-bundle/ >>>>>>>>>>> >>>>>>>>>>> Let's make a deal. Move your > changes to >>> that >>>>> branch, let >>>>>>> me apply >>>>>>>>> my fix >>>>>>>>>>> in trunk and do a release. When > your changes >>> are >>>>> ready, we can >>>>>>> discuss >>>>>>>>>>> them and backport from > 2.1.x-client-window >>> to trunk. >>>>> Does that >>>>>>> sound >>>>>>>>>>> good for you? >>>>>>>>>>> >>>>>>>>>>> regards, >>>>>>>>>>> >>>>>>>>>>> Leonardo Uribe >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>>> LieGrue, >>>>>>>>>>>> strub >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> ----- Original Message ----- >>>>>>>>>>>>> From: Mark Struberg >>>>> <[email protected]> >>>>>>>>>>>>> To: MyFaces Development >>>>>>> <[email protected]> >>>>>>>>>>>>> Cc: >>>>>>>>>>>>> Sent: Thursday, November > 15, 2012 >>> 6:32 PM >>>>>>>>>>>>> Subject: Re: StateSaving > review >>>>>>>>>>>>> >>>>>>>>>>>>> I did further reviews: >>>>>>>>>>>>> >>>>>>>>>>>>> Currently the > CounterKeyFactory >>> needs some >>>>> random to >>>>>>> be unique >>>>>>>>>>> (according to >>>>>>>>>>>>> Leo) and the > RandomKeyFactory needs >>> a >>>>> counter to be >>>>>>> unique. >>>>>>>>>>>>> >>>>>>>>>>>>> Does that ring a bell? > That stuff >>> is >>>>> completely >>>>>>> useless! >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> Foooooolks wake up, > let's >>> provide ONE >>>>> factory >>>>>>> which is >>>>>>>>> waterproof. >>>>>>>>>>> That >>>>>>>>>>>>> would be much better than > having >>> 15++ >>>>> classes full of >>>>>>> half >>>>>>>>> baken/broken >>>>>>>>>>>>> algorithms. >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> Proposal: Get rid of all > that >>> KeyFactory >>>>> stuff again >>>>>>> and have >>>>>>>>> one GOOD >>>>>>>>>>>>> algorithm: Counter + > XORShift >>> Random. >>>>>>>>>>>>> >>>>>>>>>>>>> The KeyFactory stuff got > only >>> introduced in >>>>> 2.1.9 and >>>>>>> is >>>>>>>>> crashing in >>>>>>>>>>> production. >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> LieGrue, >>>>>>>>>>>>> strub >>>>>>>>>>>>> >>>>>>>>>>>>> PS: don't take it > personal, but >>> from >>>>> looking at >>>>>>> the code I >>>>>>>>> see >>>>>>>>>>> clearly what >>>>>>>>>>>>> happens if someone works > on a stuff >>> alone >>>>> without >>>>>>> reviews. >>>>>>>>> This always >>>>>>>>>>> leads to >>>>>>>>>>>>> deficits, regardless how > good one >>> is. >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> ----- Original Message > ----- >>>>>>>>>>>>>> From: Leonardo Uribe >>>>> <[email protected]> >>>>>>>>>>>>>> To: MyFaces > Development >>>>>>> <[email protected]>; >>>>>>>>> Mark >>>>>>>>>>> Struberg >>>>>>>>>>>>> <[email protected]> >>>>>>>>>>>>>> Cc: >>>>>>>>>>>>>> Sent: Thursday, > November 15, >>> 2012 6:20 >>>>> PM >>>>>>>>>>>>>> Subject: Re: > StateSaving >>> review >>>>>>>>>>>>>> >>>>>>>>>>>>>> Hi >>>>>>>>>>>>>> >>>>>>>>>>>>>> 2012/11/15 Mark > Struberg >>>>>>> <[email protected]>: >>>>>>>>>>>>>>> I'm > currently >>> reviewing all >>>>> this area. >>>>>>> It seems >>>>>>>>> that we >>>>>>>>>>> have quite >>>>>>>>>>>>> some >>>>>>>>>>>>>> stuff to improve. >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> a.) just a gut > feeling >>> yet, but my >>>>> tummy >>>>>>> tells me >>>>>>>>> that we >>>>>>>>>>> have to >>>>>>>>>>>>> review >>>>>>>>>>>>>> our key > generator/storage >>> strategies. >>>>> Too >>>>>>> complicated or >>>>>>>>> too badly >>>>>>>>>>>>> documented. >>>>>>>>>>>>>> At least they are not > self >>> describing. >>>>>>>>>>>>>> >>>>>>>>>>>>>> It's a complex > algorithm. >>> No >>>>> documentation >>>>>>> so far, >>>>>>>>> because it >>>>>>>>>>> is still >>>>>>>>>>>>>> work in progress. >>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> b.) candidate 1: >>>>> CounterKeyFactory. If we >>>>>>> like to >>>>>>>>> prevent >>>>>>>>>>> reboot >>>>>>>>>>>>> clashes >>>>>>>>>>>>>> then we might add > another int >>> which >>>>> contains a >>>>>>> random >>>>>>>>> value. Think >>>>>>>>>>> about >>>>>>>>>>>>> having >>>>>>>>>>>>>> a Server with a > single page >>> right now. >>>>> Click on >>>>>>> it a few >>>>>>>>> times, >>>>>>>>>>> then >>>>>>>>>>>>> restart >>>>>>>>>>>>>> myfaces and issue a > few >>> requests to the >>>>> same >>>>>>> page and go >>>>>>>>> back in >>>>>>>>>>> your >>>>>>>>>>>>> browser >>>>>>>>>>>>>> history. Proposal: > instead of >>> the >>>>> viewId we >>>>>>> should add a >>>>>>>>> random >>>>>>>>>>> number. >>>>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>>>> Since the counter is > also >>> stored into >>>>> session, >>>>>>> the last >>>>>>>>> counter >>>>>>>>>>> values >>>>>>>>>>>>>> is not lost. >>>>>>>>>>>>>> >>>>>>>>>>>>>>> c.) a general > one. We >>> might >>>>> introduce an >>>>>>> own Random >>>>>>>>> which >>>>>>>>>>> either uses >>>>>>>>>>>>>> >>> java.util.concurrent.ThreadLocalRandom >>>>> for java >>>>>>> 7++ or >>>>>>>>> the old >>>>>>>>>>> Random impl. >>>>>>>>>>>>>>> > ThreadLocalRandom has a >>> _much_ >>>>> better >>>>>>> performance >>>>>>>>> on >>>>>>>>>>> servers! Or we >>>>>>>>>>>>> just >>>>>>>>>>>>>> use a simple XORShift > which is >>> surely >>>>> good >>>>>>> enough for >>>>>>>>> most cases >>>>>>>>>>> and >>>>>>>>>>>>> performs >>>>>>>>>>>>>> like hell. The > spreading of >>> XORShift is >>>>> better >>>>>>> than the >>>>>>>>> standard >>>>>>>>>>> Java >>>>>>>>>>>>> algorithm >>>>>>>>>>>>>> even ... >>>>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>>>> It could work. >>>>>>>>>>>>>> >>>>>>>>>>>>>>> d.) KeyFactory > looks a >>> bit >>>>> overengineered. >>>>>>> The >>>>>>>>> return type is >>>>>>>>>>> either >>>>>>>>>>>>>> Integer or byte [] > but the >>> encoded >>>>> value is >>>>>>> always >>>>>>>>> represented as >>>>>>>>>>> String. >>>>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>>>> The key is encrypted > and >>> base64 >>>>> encoded, but >>>>>>> that's >>>>>>>>> done in >>>>>>>>>>>>>> >>>>> HtmlResponseStateManager.writeViewStateField. >>>>>>> The change >>>>>>>>> only has >>>>>>>>>>>>>> sense if we move the >>> responsibility of >>>>> encrypt >>>>>>> to >>>>>>>>>>>>>> >>>>>>> ServerSideStateCacheImpl/ClientSideStateCacheImpl. My >>>>>>>>> opinion is >>>>>>>>>>> it is >>>>>>>>>>>>>> better to keep it as > is. >>>>>>>>>>>>>> >>>>>>>>>>>>>>> e.) Instead of > trashing >>> the >>>>> Session with >>>>>>>>> setAttribute and >>>>>>>>>>> synchronized >>>>>>>>>>>>> >>>>>>>>>>>>>> blocks we should > rather store >>> an >>>>> AtomicInteger. >>>>>>> This is >>>>>>>>> perfectly >>>>>>>>>>> fine now >>>>>>>>>>>>> as we >>>>>>>>>>>>>> do not support java > 1.4 any >>> longer, >>>>> right? >>>>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>>>> The synchronized > blocks are >>> over the >>>>>>>>> SerializedViewCollection, >>>>>>>>>>> which >>>>>>>>>>>>>> is unique per > session, so it >>> does not >>>>> suppose >>>>>>> any >>>>>>>>> overhead (tested >>>>>>>>>>>>>> with YourKit > profiler). But >>> performance >>>>> tests >>>>>>> shows that >>>>>>>>> the >>>>>>>>>>>>>> additional calls to > session >>> map impose >>>>> an small >>>>>>> overhead >>>>>>>>> (the >>>>>>>>>>> ideal is >>>>>>>>>>>>>> minimize those > calls). Since >>> the >>>>> counter is >>>>>>> stored per >>>>>>>>> session, >>>>>>>>>>> there >>>>>>>>>>>>>> is no chance of key > clashing. >>> One >>>>> option could >>>>>>> be >>>>>>>>> something like >>>>>>>>>>> the >>>>>>>>>>>>>> trick used in Flash > Scope. An >>>>> AtomicLong from a >>>>>>> random >>>>>>>>> seed, but >>>>>>>>>>>>>> anyway it is > necessary to >>> change the >>>>> algorithm >>>>>>> for check >>>>>>>>> if there >>>>>>>>>>> is a >>>>>>>>>>>>>> key clashing, > generates >>> another key. >>>>>>>>>>>>>> >>>>>>>>>>>>>>> Just a few > random >>> ideas... >>>>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>>>> MS>> another > one: >>>>>>>>>>>>>> MS>> All that > stuff has >>> nothing >>>>> to do with >>>>>>> the >>>>>>>>> RenderKit and >>>>>>>>>>> should >>>>>>>>>>>>> go >>>>>>>>>>>>>> into >>>>> org.apache.myfaces.application.viewstate. >>>>>>>>>>>>>> MS>> wdyt? >>>>>>>>>>>>>> >>>>>>>>>>>>>> That stuff is used by >>>>>>>>>>>>>> >>>>>>>>> >>> org.apache.myfaces.renderkit.html.HtmlResponseStateManager, so >>>>> in >>>>>>>>>>>>>> theory is related to > the >>> renderkit >>>>> (because >>>>>>>>> ResponseStateManager >>>>>>>>>>> is >>>>>>>>>>>>>> renderkit dependant), > but it >>> is an >>>>> application >>>>>>> scope >>>>>>>>> concept. >>>>>>>>>>> Maybe >>>>>>>>>>>>>> >>>>>>> org.apache.myfaces.renderkit.application.viewstate. > has >>>>>>>>> more sense >>>>>>>>>>> to >>>>>>>>>>>>>> me. >>>>>>>>>>>>>> >>>>>>>>>>>>>> MS>> Also the > following >>> classes >>>>> are >>>>>>> related and >>>>>>>>> should imo >>>>>>>>>>> get moved >>>>>>>>>>>>>> to this new package: >>>>>>>>>>>>>> MS>> * > StateCache >>>>>>>>>>>>>> MS>> * > StateCacheFactory >>>>>>>>>>>>>> >>>>>>>>>>>>>> Yes, a better name >>>>>>>>>>>>>> >>>>>>>>>>>>>> MS>> * > StateManagerImpl >>>>>>>>>>>>>> >>>>>>>>>>>>>> No, StateManagerImpl > deals >>> with the >>>>> logic >>>>>>> related to >>>>>>>>> calculate the >>>>>>>>>>>>>> state to a view, > StateCache >>> deals with >>>>> the >>>>>>> storage of the >>>>>>>>> view >>>>>>>>>>> (maybe >>>>>>>>>>>>>> a better name is > StateStorage, >>> for >>>>> example >>>>>>>>>>>>>> >>>>>>> > ServerSideStateStorageImpl/ClientSideStateStorageImpl? >>>>>>>>>>>>>> >>>>>>>>>>>>>> regards, >>>>>>>>>>>>>> >>>>>>>>>>>>>> Leonardo >>>>>>>>>>>>>> >>>>>>>>>>>>>>> LieGrue, >>>>>>>>>>>>>>> strub >>>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>> >>>>>>>>> >>>>>>> >>>>> >>> >
