myfaces-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mark Struberg <strub...@yahoo.de>
Subject Re: StateSaving reviews
Date Fri, 16 Nov 2012 21:10:45 GMT
That was it? 

To sum up:

We are now exactly where I have been 2 days ago. Except that you now rolled back all my changes and trashed all the commit logs with dozen of 'copy to move history' messages. Just to apply them again - that's pure waste of time! Not only yours, but also of all other MyFaces committers. 

Oh and we still have unused classes, unused generics, unused parameters, unused fields lying around. To get rid of those and streamline the code was the goal of this exercise btw... I will get rid of them now. 
At least it will be a bit more maintainable and I hope. Will check...

I hope you feel better now at least.

LieGrue,
strub




----- Original Message -----
> From: Leonardo Uribe <lu4242@gmail.com>
> To: MyFaces Development <dev@myfaces.apache.org>; Mark Struberg <struberg@yahoo.de>
> 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 <lu4242@gmail.com>:
>>  Hi
>> 
>>  2012/11/15 Mark Struberg <struberg@yahoo.de>:
>>>  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 <lu4242@gmail.com>
>>>>  To: MyFaces Development <dev@myfaces.apache.org>; Mark 
> Struberg <struberg@yahoo.de>
>>>>  Cc:
>>>>  Sent: Friday, November 16, 2012 1:11 AM
>>>>  Subject: Re: StateSaving reviews
>>>> 
>>>>  Hi
>>>> 
>>>>  2012/11/15 Mark Struberg <struberg@yahoo.de>:
>>>>>   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 <kito.mann@virtua.com>
>>>>>>   To: MyFaces Development <dev@myfaces.apache.org>
>>>>>>   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 
> <struberg@yahoo.de>
>>>>  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 <lu4242@gmail.com>
>>>>>>>>    To: MyFaces Development 
> <dev@myfaces.apache.org>; Mark
>>>>  Struberg
>>>>>>   <struberg@yahoo.de>
>>>>>>>>    Cc:
>>>>>>>>    Sent: Friday, November 16, 2012 12:11 AM
>>>>>>>>    Subject: Re: StateSaving review
>>>>>>>> 
>>>>>>>>    Hi
>>>>>>>> 
>>>>>>>>    2012/11/15 Mark Struberg 
> <struberg@yahoo.de>:
>>>>>>>>>    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 
> <lu4242@gmail.com>
>>>>>>>>>>    To: MyFaces Development
>>>>  <dev@myfaces.apache.org>; Mark
>>>>>>   Struberg
>>>>>>>>    <struberg@yahoo.de>
>>>>>>>>>>    Cc:
>>>>>>>>>>    Sent: Thursday, November 15, 2012 11:06 
> PM
>>>>>>>>>>    Subject: Re: StateSaving review
>>>>>>>>>> 
>>>>>>>>>>    Hi
>>>>>>>>>> 
>>>>>>>>>>    2012/11/15 Mark Struberg 
> <struberg@yahoo.de>:
>>>>>>>>>>> 
>>>>>>>>>>>>      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
>>>>  <lu4242@gmail.com>
>>>>>>>>>>>>      To: MyFaces Development
>>>>>>   <dev@myfaces.apache.org>; Mark
>>>>>>>>    Struberg
>>>>>>>>>>    <struberg@yahoo.de>
>>>>>>>>>>>>      Cc:
>>>>>>>>>>>>      Sent: Thursday, November 15, 
> 2012 10:20 PM
>>>>>>>>>>>>      Subject: Re: StateSaving review
>>>>>>>>>>>> 
>>>>>>>>>>>>      Hi
>>>>>>>>>>>> 
>>>>>>>>>>>>      2012/11/15 Mark Struberg
>>>>  <struberg@yahoo.de>:
>>>>>>>>>>>>>       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
>>>>>>   <lu4242@gmail.com>
>>>>>>>>>>>>>>       To: MyFaces 
> Development
>>>>>>>>    <dev@myfaces.apache.org>; Mark
>>>>>>>>>>    Struberg
>>>>>>>>>>>>      <struberg@yahoo.de>
>>>>>>>>>>>>>>       Cc:
>>>>>>>>>>>>>>       Sent: Thursday, 
> November 15, 2012
>>>>  9:30 PM
>>>>>>>>>>>>>>       Subject: Re: 
> StateSaving review
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>       Hi
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>       2012/11/15 Mark 
> Struberg
>>>>>>   <struberg@yahoo.de>:
>>>>>>>>>>>>>>>        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
>>>>>>>>    <lu4242@gmail.com>
>>>>>>>>>>>>>>>>        To: MyFaces 
> Development
>>>>>>>>>>    <dev@myfaces.apache.org>; Mark
>>>>>>>>>>>>      Struberg
>>>>>>>>>>>>>>       
> <struberg@yahoo.de>
>>>>>>>>>>>>>>>>        Cc:
>>>>>>>>>>>>>>>>        Sent: 
> Thursday, November
>>>>  15, 2012
>>>>>>   8:44 PM
>>>>>>>>>>>>>>>>        Subject: Re: 
> StateSaving
>>>>  review
>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>        Hi
>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>        2012/11/15 
> Mark Struberg
>>>>>>>>    <struberg@yahoo.de>:
>>>>>>>>>>>>>>>>>         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
>>>>>>>>>>    <struberg@yahoo.de>
>>>>>>>>>>>>>>>>>>         To: 
> MyFaces
>>>>  Development
>>>>>>>>>>>>      <dev@myfaces.apache.org>
>>>>>>>>>>>>>>>>>>         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
>>>>>>>>>>    <lu4242@gmail.com>
>>>>>>>>>>>>>>>>>>>         
> To: MyFaces
>>>>>>   Development
>>>>>>>>>>>>      <dev@myfaces.apache.org>;
>>>>>>>>>>>>>>       Mark
>>>>>>>>>>>>>>>>        Struberg
>>>>>>>>>>>>>>>>>> 
>>>>  <struberg@yahoo.de>
>>>>>>>>>>>>>>>>>>>         
> Cc:
>>>>>>>>>>>>>>>>>>>         
> Sent:
>>>>  Thursday,
>>>>>>   November 15,
>>>>>>>>    2012 6:20
>>>>>>>>>>    PM
>>>>>>>>>>>>>>>>>>>         
> Subject: Re:
>>>>>>   StateSaving
>>>>>>>>    review
>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>         
> Hi
>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>         
> 2012/11/15
>>>>  Mark
>>>>>>   Struberg
>>>>>>>>>>>>      <struberg@yahoo.de>:
>>>>>>>>>>>>>>>>>>>>     
>       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
>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>> 
>>>>>>>>>>>> 
>>>>>>>>>> 
>>>>>>>> 
>>>>>> 
>>>> 
> 

Mime
View raw message