[Seaside-dev] session tracking refactoring: all tests green

Philippe Marschall philippe.marschall at gmail.com
Mon Jul 18 12:40:24 UTC 2011


2011/7/18 Julian Fitzell <jfitzell at gmail.com>:
> On Sun, Jul 17, 2011 at 1:47 PM, Philippe Marschall
> <philippe.marschall at gmail.com> wrote:
>> Hi
>>
>> I'm happy to report that the session tracking refactoring in the WIP
>> repository [1] is mostly done. Have a look at
>
> I'll take a look.
>
>> WAHandlerTrackingStrategy to get started. The missing things are:
>>  - more tests
>>  - class comments
>>  - WARegistryKeyHandlingTest and WAApplicationKeyHandlingTest need to
>> be refactored, there are some nonsensical tests there like two
>> sessions, one identified by a query field and one by a cookie
>
> Why is that a nonsensical test? It happens any time someone clicks on
> a link/bookmark and also has a cookie from a previous session hanging
> around...

If the user agent accepts cookies the session key will not show up in
the URL. Only the very first link he clicks linking to an action page
will have it but the following render page won't. So it won't show up
the address bar. Also we probably do the wrong thing. We should
probably take the session from the cookie. However we currently give
preference to the query field because document handlers are tracked
using query fields.

>>  - configuration with the admin UI is broken, see previous mail
>>
>> A small side effect is that WARegistry uses '_s' as well to track
>> things instead of '_key'. Since it isn't used anyway I didn't bother
>> to preserve this behavior. This obviously breaks all users of
>
> I don't think that should be hardcoded. First, the _s clearly implies
> "session"; second, registries could easily be nested so having a
> hardcoded key sucks.

For the last then years only _s has ever been used. Even for document
handlers. If you wanted something else you had to subclass
WAApplication. With the new solution you still have to subclass, just
the strategy instead of the application. So nothing really changed.

I figured that in the hypothetical case when you wanted some other key
subclassing a strategy class and overriding a method is preferable to
introducing a configuration option that nobody ever uses.

>> #useCookies and I also had to delete some deprecated methods on
>> WARegistry. This bears the question do we merge it anyway or do we
>> start with 3.1?
>
> I've broken #useCookies as well in my experiments. It would end up
> being an attribute of the session request filter instead of the
> application.

That doesn't answer the question ;-)

Cheers
Philippe


More information about the seaside-dev mailing list