[ https://issues.apache.org/jira/browse/TAP5-658?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12701141#action_12701141 ]
Andy Blower edited comment on TAP5-658 at 4/21/09 5:06 AM: ----------------------------------------------------------- You're correct about the calls to PersistentLocale.set() - I have no idea why I thought it was being called for every request, it's not. I do think that the symbol could be used to stop the checks for supported locale being the first part of a URL. It's never going to be if ENCODE_LOCALE_INTO_PATH is false. Regarding LocalizationSetter, I have not rewritten CELE to handle locales, and I actually decided against doing it for our URL manipulation too - I advised the service instead which worked very nicely. (shame I couldn't just have decorated a service for 5.0 but that did it in the dispatcher...) It seems you're still conflating these two *separate* aspects of my Tapestry work: * URL manipulation, where I did mess with internals in 5.0 with full knowledge of the implications. (took less than a day to move this code to advice in T5.1, no problem at all once I figured out the best approach) * Overriding locale persistence, where I did not touch the internals and there is still a backwards compatibility issue that seems to bother no one so it's pretty irrelevant. So, what I've actually done to handle locales my own way in T5.1 is to follow the three steps from TAP5-577. 1) Set the ENCODE_LOCALE_INTO_PATH symbol to false. 2) Created an implementation of PersistentLocale and contribute it to the IOC. (same as 5.0 version we had) 3) Created 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 calls the LocalizationSetter setLocaleFromLocaleName() method instead of the old setThreadLocale() method. I attached the relevant changes to our module for step 3 to TAP5-577, I don't know if you've seen that. If you take a look you can see that Locale is converted to a string just so it can be passed. I could add the functionality from LocalizationSetter to the LocalizationFilter I had to create, but that seems wrong if I've not rewritten CELE for locale functionality or anything else. I certainly could add the functionality into LocalizationFilter but that seems to be almost like if you don't want URL locales then you need to implement it all yourself which is anti-tapestry ethos I would have thought. was (Author: andyb): You're correct about the calls to PersistentLocale.set() - I have no idea why I thought it was being called for every request, it's not. I do think that the symbol could be used to stop the checks for supported locale being the first part of a URL. It's never going to be if ENCODE_LOCALE_INTO_PATH is false. Regarding LocalizationSetter, I have not rewritten CELE to handle locales, and I actually decided against doing it for our URL manipulation too - I advised the service instead which worked very nicely. (shame I couldn't just have decorated a service for 5.0 but that did it in the dispatcher...) It seems you're still conflating these two *separate* aspects of my Tapestry work: * URL manipulation, where I did mess with internals in 5.0 with full knowledge of the implications. (took less than a day to move this code to advice in T5.1, no problem at all once I figured out the best approach) * Overriding locale persistence, where I did not touch the internals and there is still a backwards compatibility issue that seems to bother no one so it's pretty irrelevant. So, what I've actually done to handle locales my own way in T5.1 is to follow the three steps from TAP5-577. 1) Set the ENCODE_LOCALE_INTO_PATH symbol to false. 2) Created an implementation of PersistentLocale and contribute it to the IOC. (same as 5.0 version we had) 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. I attached the relevant changes to our module for step 3 to TAP5-577, I don't know if you've seen that. If you take a look you can see that Locale is converted to a string just so it can be passed. I could add the functionality from LocalizationSetter to the LocalizationFilter I had to create, but that seems wrong if I've not rewritten CELE for locale functionality or anything else. I certainly could add the functionality into LocalizationFilter but that seems to be almost like if you don't want URL locales then you need to implement it all yourself which is anti-tapestry ethos I would have thought. > Optimizations for custom persistent locale implementations > ---------------------------------------------------------- > > Key: TAP5-658 > URL: https://issues.apache.org/jira/browse/TAP5-658 > Project: Tapestry 5 > Issue Type: Improvement > Components: tapestry-core > Affects Versions: 5.1.0.4 > Reporter: Andy Blower > Priority: Minor > > I was wondering if having a method in LocalizationSetter that allows a Locale > to be passed would be a good idea. This would remove the conversion to String > and back again which occurs for every request if you are actually storing a > locale with a custom persistent locale implementation. > The other slight annoyance I have with this area of T5 is that > 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/optimization. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.