[ https://issues.apache.org/jira/browse/TAP5-577?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Andy Blower updated TAP5-577:
-----------------------------

    Attachment: T5.1persistentLocale.txt

It looks rather like no one else is bothered by this, so I've gone ahead and converted my app's locale persistence to work in T5.1 and I'm attaching the changes I made to our module to support custom implementations of the PersistentLocale service.

Howard, can you check that I've not missed a trick here please?

Also, I was wondering if having a method in LocalizationSetter that allows a Locale to be passed would be a good idea. (to remove the conversion to String and back again which occurs for every request)

The only  other slight annoyance I have with this is PersistentLocale.set() being constantly called by the LocalizationSetter for each request, this doesn't fit in with the old way that the PersistentLocale service was used. I think that only executing persistentLocale.set(locale); (line 113) if ENCODE_LOCALE_INTO_PATH is true would be a nice little improvement.

> 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
>         Attachments: T5.1persistentLocale.txt
>
>
> 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.