myfaces-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Leonardo Uribe <lu4...@gmail.com>
Subject Re: StateSaving reviews
Date Fri, 16 Nov 2012 15:16:16 GMT
Hi

2012/11/16 Mark Struberg <struberg@yahoo.de>:
> Btw, you wrote
>> that code has been there for years!!!!!!
>
> I checked the logs now.
>
> Actually most of the changes got only introduced in 2.1.9!
>

Mark, all refactoring started in MYFACES-3117, which has been
created in 2011, so I have been working in these improvements
for a long time.

The latest changes were committed 5 months ago, but I worked
on them for a longer time. It were introduced in 2.1.9, because
I made it so.

10/Jun/12 Create MYFACES-3563
18/Jun/12 Release of 2.1.8
25/Jun/12 Resolve MYFACES-3563

I had already the changes in my laptop, but I was careful and only
committed them after 2.1.8 release.

regards,

Leonardo Uribe

>
> LieGrue,
> strub
>
>
>
> ----- Original Message -----
>> From: Leonardo Uribe <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