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