<i18n dev> Request for review: 8000997: Multiple locale sensitive services cannot be loaded

[Masayoshi Okutsu]

Looks good.

Masayoshi

On 10/24/2012 9:20 AM, Naoto Sato wrote:
> Thank you for your review. My comments are embedded below:
>
> On 10/23/12 2:53 PM, Masayoshi Okutsu wrote:
>> Here are my comments.
>>
>> SPILocaleProviderAdapter.java:
>>
>> - Should "." be used for composing a class name of a Delegate instead of
>> "$"?
>
> Since those delegates are inner classes within 
> SPILocaleProviderAdapter, it has to be "$", otherwise the class cannot 
> be found with Class.forName() method.
>
>> - Is it necessary to iterate locale candidate in getImpl? (Iteration is
>> performed outside of adapters when getting an adapter or a provider of
>> pools.)
>
> Yes. Delegates have to check lookup locales within themselves in order 
> to decide which implementation should be returned.
>
>> - addImpl() overrides any previous impl if there are multiple
>> implementations for the same Locale. Should the first one preserved?
>
> There is no rule which implementation is used when there are multiple 
> implementations for a particular locale, so I think overriding is OK. 
> Since in JDK7, the logic used to use Set interface, there is no way to 
> keep compatibility.
>
>> - map.keySet().contains(key) should be map.containsKey(key).
>
> Fixed.
>
>> - When a factory of a provider returns null where null is not allowed,
>> should the provider be removed from the map?
>
> I think this should be fine, since they are just the "delegates." 
> Instead I added assertion to each getImpl() return value, which should 
> always be non null.
>
> The modified webrev is located here. Only 
> SPILocaleProviderAdapter.java is modified this time.
>
> http://cr.openjdk.java.net/~naoto/8000997/webrev.01/
>
> Naoto
>
>>
>> Thanks,
>> Masayoshi
>>
>> On 2012/10/22 15:26, Naoto Sato wrote:
>>> Hello,
>>>
>>> I need a reviewer for the changes for 8000997, where the symptom is:
>>>
>>> "With the locale data deployment feature, locale sensitive services
>>> SPI implementations cannot be loaded more than one implementation.
>>> This is due to the fact that SPILocaleProviderAdapter only uses one
>>> instance of SPI implementations. It should load all the available
>>> installed providers as in JDK7, and correctly selects the instance
>>> depending on the requester's locale."
>>>
>>> The gist of the fix is to prepare LocaleServiceProvider delegates for
>>> each services in SPILocaleProviderAdapter, and they work with the real
>>> SPI implementations. The proposed change is located here:
>>>
>>> http://cr.openjdk.java.net/~naoto/8000997/webrev.00/
>>>
>>> Naoto
>>
>