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 08:15:51 GMT
Btw, you wrote
> that code has been there for years!!!!!! 

I checked the logs now.

Actually most of the changes got only introduced in 2.1.9!


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