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 >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>> >>>>>>>> >>>>>> >>>> >>
