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

[Naoto Sato]

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
>