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

Reply via email to