[
https://issues.apache.org/jira/browse/TAP5-577?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12681499#action_12681499
]
Andy Blower commented on TAP5-577:
----------------------------------
Howard,
I agree that using cookies to track user's persistent locale is not industry
standard, but it happens to be the method used by the final public release of
T5.0 which is the important point here. The fact that the default method for
persisting locales is different is really only a backwards compatibility issue
for anyone who's relying on the generated URLs for some reason or has issues
with locale being in the URLs. (like our site which needs durable and shareable
URLs regardless of a users' locale preference)
To my mind this is a judgement call for you to take as it's quite a subjective
aspect of backwards compatibility, assuming of course that it's easy to
override the default behaviour like it was in T5.0 where you provided an easy
way with a public interface. This is where I have the main (less subjective,
more objective) issue...
You are right that the functionality of persisting locale once set is still
retained and is just done using a different method in T5.1, but only for people
who haven't implemented their own PersistentLocale service. If they have then
it will break on upgrading to T5.1 because the service interface (which is
public and not internal) is now used in a different way by the internals - in
other words the contract you made for this public interface is broken. If it
were as simple as changing the symbol to false to re-enable the old contract
with the public PersistentLocale service then it wouldn't be so bad, but
currently it's non-trivial for someone to get their custom PersistentLocale
services working in T5.1 which is why I consider backwards compatibility to be
broken in this instance.
Regarding your statement "vast majority of users (who were not mucking about in
the internals of Tapestry to customize persistent locales)" - I really don't
think that creating and contributing a custom implementation of a public
service interface can be characterized in this way. Especially by you. I
thought that this was what T5 was all about; functional out of the box, but
allowing powerful customization where it just "gets out of the way" to let you
do what you need. Have I got this wrong?
I definitely have mucked about with Tapestry internals and I will have some
upgrade pains due to these modification, but I knew that when I did it and you
will not hear a thing from me about backwards compatibility when I've messed
with internals (even if there was no other way in T5.0 to achieve what I needed
to do) but that is not the case here.
I think I can safely assume that you do not consider reverting the default
behaviour of T5.1 to cookie locale persistence an option which is fair enough.
I don't think much of cookie persistence myself so I'm certainly not bothered
by it, I just hope it doesn't trip up too many others. That's my only concern,
although you seem to have confidence that it wont so my fears are most likely
unfounded. As I've said before it will only really be a minor inconvenience to
me personally if you don't resolve this issue - I think I know what I'm doing
and have the ability to cope with this for my own work with Tapestry.
However, I do think the other issue of overriding the default method of locale
persistence becoming so much harder in T5.1 is a major issue and does break
backwards compatibility. The aim should be that if the ENCODE_LOCALE_INTO_PATH
symbol is set to false, then a custom PersistentLocale service that worked with
T5.0 should work the same with T5.1 so I guess the only sticking point is the
lack of a LocalizationFilter in T5.1 which is enabled when the symbol is false
and allows a custom PersistentLocale service written for T5.0 to work with T5.1
without a lot of hassle and migration work. This should fix the main
(objective) part of this issue.
> TAP5-422 changes break persistent locale backwards compatibility.
> -----------------------------------------------------------------
>
> Key: TAP5-577
> URL: https://issues.apache.org/jira/browse/TAP5-577
> Project: Tapestry 5
> Issue Type: Bug
> Components: tapestry-core
> Affects Versions: 5.1.0.0, 5.1.0.1
> Reporter: Andy Blower
> Priority: Critical
>
> I think that the changes made in T5.1 for TAP5-422 break backwards
> compatibility with T5.0's locale persistence. In T5.0 it was a simple matter
> to override the default cookie persistence by creating a custom
> implementation of the PersistentLocale service and contributing it to be used
> instead of the standard internal T5 implementation.
> The TAP5-422 changes broke backwards compatibility because anyone who's
> created their own implementation of PersistentLocale, or just wants the 5.0
> cookie persistence behaviour, would have found that it's a lot more work and
> involves some heavy changes to Tapestry internals. Now with the recent
> changes for TAP5-418 (committed yesterday), the situation had been alleviated
> somewhat by allowing the the hard-wired URl locale persistence to be switched
> off using a new symbol.
> However, I still think that this breaks backwards compatibility in two ways:
> 1) By changing the default behaviour of locale persistence so that anyone
> relying on the locale persistence behaviour of 5.0 will have to make
> non-trivial changes when they upgrade to 5.1 to keep the same operation.
> 2) By requiring so much work for anyone wanting to keep the 5.0 cookie
> persistence behaviour or define their own custom locale persistence. (In 5.0
> it was easy to figure out and implement a custom locale persistence method)
> From my analysis of the changes made by TAP5-422 & TAP5-418, I think anyone
> wanting non-URL based locale persistence will need to do the following when
> upgrading from 5.0 to 5.1:
> 1) Set the ENCODE_LOCALE_INTO_PATH symbol to false.
> 2) Create an implementation of PersistentLocale and contribute it to the IOC.
> (copied from the standard 5.0 code if the old default cookie persistence is
> desired)
> 3) Create a custom filter written and created to do the same job as the 5.0
> LocalizationFilter and contribute it to the IOC RequestHandler. This filter
> will need to call the LocalizationSetter setLocaleFromLocaleName() method
> instead of the old setThreadLocale() method.
> My suggested resolution would be to re-instate the 5.0 cookie persistence
> (LocalizationFilter & PersistentLocaleImpl) and have the new
> ENCODE_LOCALE_INTO_PATH symbol default to false allowing 5.1 to work the same
> way as 5.0 out of the box. If the symbol is set to true, then the
> LocalizationFilter is disabled (not contributed to RequestHandler) and the
> PersistentLocale service will need to just store the locale (not set it in a
> cookie) for later use by LinkSourceImpl.
> LocalizationSetterImpl.setLocaleFromLocaleName(String localeName) would also
> need changing back to overriding the passed localeName if a persistent one
> had been set into the PersistentLocale service. There may by a much better
> solution than this as I've not spent much time on it, but I though I should
> try to be helpful as possible.
> (It should be noted that this is purely a product of my analysis of the 5.1
> code, I have not found the time to actually run T5.1 and test this out - I
> should be able to do this in about a week and a half, but I'm currently
> approaching a major milestone deadline. Hopefully someone else will find the
> time to prove or disprove my hypothesis.)
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
