[jira] Created: (TAP5-577) TAP5-422 changes break persistent locale backwards compatibility.

classic Classic list List threaded Threaded
17 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[jira] Created: (TAP5-577) TAP5-422 changes break persistent locale backwards compatibility.

JIRA jira@apache.org
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.1, 5.1.0.0
            Reporter: Andy Blower
            Priority: Critical


I think that the changes made in T5.1 for TAP5-422 break backwards compatability 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 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 changes when they upgrade to 5.1 to keep the same functionality.
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 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 localization setter calls in PageRenderDispatcher & ComponentEventLinkEncoderImpl are enabled to allow the URL locale persisting to happen. 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.

(It should be noted that this is a product of my analysis of the 5.1 code, I have not found the time to actually run it and test this out - I should be able to do this in about a week and a half, but I'm currently approaching a 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.

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[jira] Updated: (TAP5-577) TAP5-422 changes break persistent locale backwards compatibility.

JIRA jira@apache.org

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

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

    Description:
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 calls to LocalizationSetter in PageRenderDispatcher & ComponentEventLinkEncoderImpl are enabled to allow the URL locale persisting to happen. 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.

(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.)

  was:
I think that the changes made in T5.1 for TAP5-422 break backwards compatability 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 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 changes when they upgrade to 5.1 to keep the same functionality.
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 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 localization setter calls in PageRenderDispatcher & ComponentEventLinkEncoderImpl are enabled to allow the URL locale persisting to happen. 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.

(It should be noted that this is a product of my analysis of the 5.1 code, I have not found the time to actually run it and test this out - I should be able to do this in about a week and a half, but I'm currently approaching a milestone deadline. Hopefully someone else will find the time to prove or disprove my hypothesis.)


> 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 calls to LocalizationSetter in PageRenderDispatcher & ComponentEventLinkEncoderImpl are enabled to allow the URL locale persisting to happen. 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.
> (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.

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[jira] Updated: (TAP5-577) TAP5-422 changes break persistent locale backwards compatibility.

JIRA jira@apache.org
In reply to this post by JIRA jira@apache.org

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

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

    Description:
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.)

  was:
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 calls to LocalizationSetter in PageRenderDispatcher & ComponentEventLinkEncoderImpl are enabled to allow the URL locale persisting to happen. 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.

(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.)


> 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.

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[jira] Commented: (TAP5-577) TAP5-422 changes break persistent locale backwards compatibility.

JIRA jira@apache.org
In reply to this post by JIRA jira@apache.org

    [ https://issues.apache.org/jira/browse/TAP5-577?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12681430#action_12681430 ]

Howard M. Lewis Ship commented on TAP5-577:
-------------------------------------------

I'm not sure if this really qualifies as breaking backwards compatibility, since so much of this behavior was internal in Tapestry 5.0.

The key features haven't changed:

- A users locale is initially determined from browser headers
- A specific locale may be selected via PersistentLocale.set()
- After invoking set(), later requests ignore the header and use the preferred locale

The big difference is that 5.0 used a client-side cookie, and 5.1 (by default) encodes the locale into the URL path.

Assuming users don't care about URLs, they won't see any difference ... except that when first accessing the 5.1 application they may revert to the default locale (determined from browser headers). However, this might occur anyway, if their cookie was deleted or expired.

The remaining difference is in how the locale is persisted between requests. Tapestry 5.1 already includes the ability to turn off encoding of the locale into the URL path.

You three steps should work in theory.

What we have here is a change in behavior but not functionality; i.e., a change in the implementation of persistent locales. I don't see this as a backwards compatibility problem as the vast majority of users (who were not mucking about in the internals of Tapestry to customize persistent locales) will not see a difference in the upgrade (just slightly different URLs).

> 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.

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[jira] Commented: (TAP5-577) TAP5-422 changes break persistent locale backwards compatibility.

JIRA jira@apache.org
In reply to this post by JIRA jira@apache.org

    [ https://issues.apache.org/jira/browse/TAP5-577?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12681447#action_12681447 ]

Fernando commented on TAP5-577:
-------------------------------

This just sounds like a HUGE change in behavior, without easily reverting to old way of doing things.

1) we can't cache any generated content across users!!!!!
2) we now totally mess up how we generate urls ( if we do/have to do it in javascript, or external services )
3) though the idea of putting locales in the url sounds cool, you can't just force that on us!!
  that is a huge change in site architecture, url planning, etc etc.
4) users might not care about urls, but we do, browsers do, caches do.....

So I like the suggestion that we implement the old simpler more industry standard method be default, but let us easily change it to experiment and try out new things..


> 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.

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[jira] Commented: (TAP5-577) TAP5-422 changes break persistent locale backwards compatibility.

JIRA jira@apache.org
In reply to this post by JIRA jira@apache.org

    [ https://issues.apache.org/jira/browse/TAP5-577?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12681470#action_12681470 ]

Howard M. Lewis Ship commented on TAP5-577:
-------------------------------------------

Using cookies to track the user's locale is not industry default.  I beg to differ ... the majority of localized applications I've used encode the locale into the URL.

I don't see how this intersects caching of content ... unless you expect to cache English and German text as if it were interchangeable.  

> 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.

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[jira] Commented: (TAP5-577) TAP5-422 changes break persistent locale backwards compatibility.

JIRA jira@apache.org
In reply to this post by JIRA jira@apache.org

    [ https://issues.apache.org/jira/browse/TAP5-577?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12681482#action_12681482 ]

Fernando commented on TAP5-577:
-------------------------------

fair. ok. so I'm only left with : "fear of change". :)

and allowing us to slowly switch to it, bit by bit, rather than forcing us to do it next time I upgrade :)
just to make sure that there really are no issues that we have to deal with ( i do worry about previously generated urls, manually generated urls, javascript, etc ) :)

> 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.

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[jira] Commented: (TAP5-577) TAP5-422 changes break persistent locale backwards compatibility.

JIRA jira@apache.org
In reply to this post by JIRA jira@apache.org

    [ https://issues.apache.org/jira/browse/TAP5-577?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12681490#action_12681490 ]

Robert Zeigler commented on TAP5-577:
-------------------------------------

1) previously generated urls => should be gracefully handled as simply a url with no locale info? So the user loses his/her locale for that request...
2) manually generated urls/javascript: Hm... shouldn't those be generated via componentResources.create{Action,Event,Form,Page}Link, in which case: no change required? :)


> 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.

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[jira] Commented: (TAP5-577) TAP5-422 changes break persistent locale backwards compatibility.

JIRA jira@apache.org
In reply to this post by JIRA jira@apache.org

    [ https://issues.apache.org/jira/browse/TAP5-577?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12681491#action_12681491 ]

Fernando commented on TAP5-577:
-------------------------------

1) you mean every time a user comes to the site?
1a) we are a facebook/opensocial app, so they proxy back to us, and the base url can't really change.. else we will be redirecting on every single request.
 (actually, because of this I just realized that we can never use the locale in url :( :( :( )
2) yes all urls should be generated by tapestry, but we have a handful of apps/sites, handful of developers, tonnes of features we need to implements and just
   a slight lack of time to do it all in.. I can't guarantee that all of our developers and efforts are on the up and up... so I can't roll this out without copious amounts
   of testing. (hence the slow/controlled rollout ability, in our own terms)


> 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.

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[jira] Commented: (TAP5-577) TAP5-422 changes break persistent locale backwards compatibility.

JIRA jira@apache.org
In reply to this post by JIRA jira@apache.org

    [ 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.

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[jira] Issue Comment Edited: (TAP5-577) TAP5-422 changes break persistent locale backwards compatibility.

JIRA jira@apache.org
In reply to this post by JIRA jira@apache.org

    [ https://issues.apache.org/jira/browse/TAP5-577?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12681499#action_12681499 ]

Andy Blower edited comment on TAP5-577 at 3/12/09 2:31 PM:
-----------------------------------------------------------

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.

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. 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.

      was (Author: andyb):
    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.

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[jira] Updated: (TAP5-577) TAP5-422 changes break persistent locale backwards compatibility.

JIRA jira@apache.org
In reply to this post by JIRA jira@apache.org

     [ 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.

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[jira] Commented: (TAP5-577) TAP5-422 changes break persistent locale backwards compatibility.

JIRA jira@apache.org
In reply to this post by JIRA jira@apache.org

    [ https://issues.apache.org/jira/browse/TAP5-577?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12703460#action_12703460 ]

Ferdy Nagy commented on TAP5-577:
---------------------------------

You're not the only one bothered by this, and I think there are reasonable expectations that locale could passed around by cookie.  We use a central SSO that allows people to change their preferred language and they can move from application to application within the domain with each application displaying in their language choice.

The process outlined by Andy doesn't really work because the ComponentEventLinkEncoder always calls the LocalizationSetter which in turn always sets the ThreadLocale; thus overriding the set made by the filter.  To make it work I had to override the LocalizationSetter service to always return false, and change the filter to set the ThreadLocale directly.

This just doesn't "feel" right.  It seems like there should be a simple way for a single concern to deal with the localization setting that can handle a choice of either the URL containing the locale, or a cookie, or from the request.  I shouldn't have to deal with a filter and a localization setter and a persistence layer just to say "here is the locale" - and I certainly shouldn't have to look at the internals of LocalizationSetter and CELE to understand how this works (but I'm very happy I can, and also grok the process easily).

> 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.

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[jira] Commented: (TAP5-577) TAP5-422 changes break persistent locale backwards compatibility.

JIRA jira@apache.org
In reply to this post by JIRA jira@apache.org

    [ https://issues.apache.org/jira/browse/TAP5-577?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12703567#action_12703567 ]

Andy Blower commented on TAP5-577:
----------------------------------

"The process outlined by Andy doesn't really work because the ComponentEventLinkEncoder always calls the LocalizationSetter which in turn always sets the ThreadLocale; thus overriding the set made by the filter. To make it work I had to override the LocalizationSetter service to always return false, and change the filter to set the ThreadLocale directly. "

Nice to hear I'm not the only one bothered by this Ferdy, and you're quite correct on this - I just traced though to double check! The weird thing is what I've done at least half works... when the filter sets the locale, the messages displayed on my page are correctly localised even though the thread locale is subsequently set back to the browser default rather than our persistent locale. I have no idea why this is and I honestly haven't the time or inclination any more to look into this problem more so I'm not going to be finding out. I must have spent 20+ hours on this one issue, looking at code & writing emails/issues,  with absolutely no result. So I have better things to do with my time, but I definitely think that localisation has become a mess in T5.1 which is a shame since it was so nice to simply contribute a service implementation of a public interface in 5.0 and the job was done.

I will be following the method you outlined, which will also mean I'll be following Howards advice from TAP5-658. (you might find that issue interesting to read also) My filter will also need the fall back to browser request locale copied from the setter, but that's no biggie. Thanks for the catch as this probably would have bitten me in the ass in a few months.



> 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.

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[jira] Commented: (TAP5-577) TAP5-422 changes break persistent locale backwards compatibility.

JIRA jira@apache.org
In reply to this post by JIRA jira@apache.org

    [ https://issues.apache.org/jira/browse/TAP5-577?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12747829#action_12747829 ]

Ilya Obshadko commented on TAP5-577:
------------------------------------

<i>The weird thing is what I've done at least half works... when the filter sets the locale, the messages displayed on my page are correctly localised even though the thread locale is subsequently set back to the browser default rather than our persistent locale. I have no idea why this is and I honestly haven't the time or inclination any more to look into this problem more so I'm not going to be finding out.</i>

This is because of the following code in LocalizationSetterImpl

    public boolean setLocaleFromLocaleName(String localeName)
    {
        boolean supported = acceptedLocaleNames.contains(localeName);

        if (supported)
        {
            Locale locale = findClosestSupportedLocale(toLocale(localeName));
            persistentLocale.set(locale);
            threadLocale.setLocale(locale);
        }
        else
        {
            Locale requestLocale = request.getLocale();
//          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
            Locale supportedLocale = findClosestSupportedLocale(requestLocale);
            threadLocale.setLocale(supportedLocale);
        }

        return supported;
    }

I found out that during normal request processing this method is being called even if there is no persistent locale in the URL. So, when URL is, for example, http://localhost:8080/app/index, the methods takes string "index" as an argument; then of course it sets supported to false, and eventually falls back to the request locale.

I believe the whole implementation is flawed; LocalizationSetter implementation should not guess if it's supplied with locale name or not, the argument must be valid locale code (already defined in the list of possible locales in configuration) from the very start. So locale code should be checked for validity before it's passed to LocalizationSetter.

My solution was just copy-pasting LocalizationSetterImpl; then I've modified the code so LocalizationSetter would fall back to 'default' locale (first in supported locales list) rather than request locale. That solved my particular problem (I need Russian language version of the website by default).


> 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.

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[jira] Issue Comment Edited: (TAP5-577) TAP5-422 changes break persistent locale backwards compatibility.

JIRA jira@apache.org
In reply to this post by JIRA jira@apache.org

    [ https://issues.apache.org/jira/browse/TAP5-577?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12747829#action_12747829 ]

Ilya Obshadko edited comment on TAP5-577 at 8/26/09 1:20 AM:
-------------------------------------------------------------

>> The weird thing is what I've done at least half works... when the filter sets the locale, the messages displayed on
>> my page are correctly localised even though the thread locale is subsequently set back to the browser default
>> rather than our persistent locale. I have no idea why this is and I honestly haven't the time or inclination any
>> more to look into this problem more so I'm not going to be finding out.

This is because of the following code in LocalizationSetterImpl

    public boolean setLocaleFromLocaleName(String localeName)
    {
        boolean supported = acceptedLocaleNames.contains(localeName);

        if (supported)
        {
            Locale locale = findClosestSupportedLocale(toLocale(localeName));
            persistentLocale.set(locale);
            threadLocale.setLocale(locale);
        }
        else
        {
            Locale requestLocale = request.getLocale();
//          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
            Locale supportedLocale = findClosestSupportedLocale(requestLocale);
            threadLocale.setLocale(supportedLocale);
        }

        return supported;
    }

I found out that during normal request processing this method is being called even if there is no persistent locale in the URL. So, when URL is, for example, http://localhost:8080/app/index, the methods takes string "index" as an argument; then of course it sets supported to false, and eventually falls back to the request locale.

I believe the whole implementation is flawed; LocalizationSetter implementation should not guess if it's supplied with locale name or not, the argument must be valid locale code (already defined in the list of possible locales in configuration) from the very start. So locale code should be checked for validity before it's passed to LocalizationSetter.

My solution was just copy-pasting LocalizationSetterImpl; then I've modified the code so LocalizationSetter would fall back to 'default' locale (first in supported locales list) rather than request locale; then I've overridden default LocalizationSetter service. That solved my particular problem (I need Russian language version of the website by default).

      was (Author: xfyre):
    <i>The weird thing is what I've done at least half works... when the filter sets the locale, the messages displayed on my page are correctly localised even though the thread locale is subsequently set back to the browser default rather than our persistent locale. I have no idea why this is and I honestly haven't the time or inclination any more to look into this problem more so I'm not going to be finding out.</i>

This is because of the following code in LocalizationSetterImpl

    public boolean setLocaleFromLocaleName(String localeName)
    {
        boolean supported = acceptedLocaleNames.contains(localeName);

        if (supported)
        {
            Locale locale = findClosestSupportedLocale(toLocale(localeName));
            persistentLocale.set(locale);
            threadLocale.setLocale(locale);
        }
        else
        {
            Locale requestLocale = request.getLocale();
//          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
            Locale supportedLocale = findClosestSupportedLocale(requestLocale);
            threadLocale.setLocale(supportedLocale);
        }

        return supported;
    }

I found out that during normal request processing this method is being called even if there is no persistent locale in the URL. So, when URL is, for example, http://localhost:8080/app/index, the methods takes string "index" as an argument; then of course it sets supported to false, and eventually falls back to the request locale.

I believe the whole implementation is flawed; LocalizationSetter implementation should not guess if it's supplied with locale name or not, the argument must be valid locale code (already defined in the list of possible locales in configuration) from the very start. So locale code should be checked for validity before it's passed to LocalizationSetter.

My solution was just copy-pasting LocalizationSetterImpl; then I've modified the code so LocalizationSetter would fall back to 'default' locale (first in supported locales list) rather than request locale. That solved my particular problem (I need Russian language version of the website by default).

 

> 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.

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[jira] Closed: (TAP5-577) TAP5-422 changes break persistent locale backwards compatibility.

JIRA jira@apache.org
In reply to this post by JIRA jira@apache.org

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

Howard M. Lewis Ship closed TAP5-577.
-------------------------------------

      Assignee: Howard M. Lewis Ship
    Resolution: Won't Fix

Your observations are right, but the intent in the change was that if you don't like how T5 does this by default, you could monkey patch it to do what you want, and that option is still open.

> 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
>            Assignee: Howard M. Lewis Ship
>            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.

Loading...