Hi 2012/11/16 Mark Struberg <[email protected]>: > Btw, you wrote >> that code has been there for years!!!!!! > > I checked the logs now. > > Actually most of the changes got only introduced in 2.1.9! >
Mark, all refactoring started in MYFACES-3117, which has been created in 2011, so I have been working in these improvements for a long time. The latest changes were committed 5 months ago, but I worked on them for a longer time. It were introduced in 2.1.9, because I made it so. 10/Jun/12 Create MYFACES-3563 18/Jun/12 Release of 2.1.8 25/Jun/12 Resolve MYFACES-3563 I had already the changes in my laptop, but I was careful and only committed them after 2.1.8 release. regards, Leonardo Uribe > > LieGrue, > strub > > > > ----- Original Message ----- >> From: Leonardo Uribe <[email protected]> >> To: MyFaces Development <[email protected]>; Mark Struberg >> <[email protected]> >> Cc: >> Sent: Friday, November 16, 2012 2:09 AM >> Subject: Re: StateSaving reviews >> >> Hi >> >> I'll proceed with the necessary changes now. I hope to get it done tomorrow. >> >> regards, >> >> Leonardo >> >> 2012/11/15 Leonardo Uribe <[email protected]>: >>> Hi >>> >>> 2012/11/15 Mark Struberg <[email protected]>: >>>> The usual release cycle is 2 to 3 months. The next expected release is >> early/mid November and not next week. If you are too stressed I can take over >> doing the release. >>>> >>> >>> Today is 15 of November, I plan start release one week, if there is a >>> problem, I have >>> 2 weeks more to fix them, and I take my vacations from December 22. >>> That's the plan. >>> >>>> >>>>> Let's make a deal, let me do it myself. I just take a snapshot >> of your >>>>> changes, >>>>> revert, and apply only the refactoring step by step, then I compare >> it. In that >>>>> way, I'll be 100% sure the change is correct. Sounds good for >> you? >>>> Leo, please learn to read foreign peoples code! It's really not >> that complicated. It's the same others have to do over and over again with >> your code as well. You can perfectly achieve the same by just reading through >> the diffs. That's the reason we send them to the committers list. Those >> diffs contain all the changes. >>>> >>> >>> We have 4 branches to maintain: >>> >>> 2.0.x >>> 2.1.x >>> 2.1.x-client-window >>> 2.2.x >>> >>> I want to keep all of them in sync. Redo the work you already done is >> necessary >>> for me, because I need to do the same steps in all branches. You are doing >> the >>> funny part, but I have to clean the dishes. >>> >>>> >>>> Going back to the old source is also not really an option in my >> personal opinion as it >>>> >>>> a.) contained a bug (NPE). There is a unit test for it now btw. >>>> >>>> b.) worked _completely_ different in ProjectStage.Production and all >> others ProjectStages thus made it tremendously hard to debug/reproduce for >> developers. >>>> >>>> And I'm not yet talking about stylistics like ode duplications, >> unused fields and generic types and endless inner classes. >>>> >>>> >>>> And also we had this kind of discussion quite a few times already and >> you always ended up reverting without giving any technical reasoning. That >> sucks >> to be honest. >>>> >>> >>> That's not true. I give technical reasons, it is just that we have >> different >>> opinions, that's it. >>> >>>> >>>> For gods sake, a very last time I let you revert something. >>>> >>>> Please first rethink (and then probably do if you are still convinced >> about it's necessity) the following: >>>> * revert to the old state >>> >>> Ok, let's go back to rev 1409476. >>> >>>> * remove the ProjectStage.Production and always use the same key >> strategy. This is really crucial for stability! >>> >>> Ok, just one strategy. >>> >>>> * apply the unit test I created and fix the NPE. >>> >>> Already done in 1408093 >>> >>>> * move the viewId.hashCode() to the key strategy constructor and you >> will clearly see the code duplication all over the place >>>> >>> >>> Ok. >>> >>>> * you can also spare the byte[] variant as this can be perfectly done >> inside the key strategy. There goes another code duplication. That will btw >> remove a broken unchecked upcast as well. >>>> >>> >>> Ok >>> >>>> * remove the bloody src/main/old >>> >>> Ok >>> >>>> * I bet I forgot some other cleanup I did >>>> >>> >>> Ok >>> >>>> >>>> I will drop/cleanup the rest do the windowId handling after this >> release if you get it out of the door this week. >>>> >>> >>> Add windowId concept in 2.1.x branch has not been discussed. >>> In theory, the idea was test and improve 2.1.x-client-window branch >>> (target JSF 2.2) and only when the code is stable enough, backport >>> it to 2.1.x. >>> >>>> >>>> Btw you should either copy over Tom and Manfreds @author from the >> JspViewStateManager or remove your own @author from the copied class as it is >> still 80% the old code. >>>> >>> >>> No worries about @author tag. Nobody cares about that, because the >>> license is ASF. It only matters if you hold the intellectual >>> copyright, which is not the case. >>> >>> regards, >>> >>> Leonardo >>> >>>> >>>> LieGrue, >>>> strub >>>> >>>> >>>> ----- Original Message ----- >>>>> From: Leonardo Uribe <[email protected]> >>>>> To: MyFaces Development <[email protected]>; Mark >> Struberg <[email protected]> >>>>> Cc: >>>>> Sent: Friday, November 16, 2012 1:11 AM >>>>> Subject: Re: StateSaving reviews >>>>> >>>>> Hi >>>>> >>>>> 2012/11/15 Mark Struberg <[email protected]>: >>>>>> 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. >>>>>> >>>>> >>>>> There is an agreement of make a release each two months, or earlier >>>>> if something happen. Check the dates, and you will see the >> systematic >>>>> pattern. That was started some years ago, and people already trust >> that. >>>>> >>>>>> And again: there is no effective code change. Just making >> inner classes >>>>> visible to uncover the mess. >>>>>> >>>>> >>>>> Let's make a deal, let me do it myself. I just take a snapshot >> of your >>>>> changes, >>>>> revert, and apply only the refactoring step by step, then I compare >> it. In that >>>>> way, I'll be 100% sure the change is correct. Sounds good for >> you? >>>>> >>>>> regards, >>>>> >>>>> Leonardo Uribe >>>>> >>>>>> 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 >>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>> >>>>>>>>> >>>>>>> >>>>> >>
