[15] RFR: 8239836: ZoneRules.of() doesn't check transitionList/standardOffsetTL arguments validity

[Roger Riggs]

Looks good.

Give it a day to see if anyone else has comments.

Thanks, Roger

On 3/2/20 1:35 PM, naoto.sato at oracle.com wrote:
> Hi Roger, thanks for the review.
>
> On 3/2/20 8:44 AM, Roger Riggs wrote:
>> Hi Naoto,
>>
>> look ok.
>>
>> ZoneRules.java: 488, 644, 761, 791
>> I'd suggest calling isFixedOffset() instead of repeating the code:
>> standardTransitions.length == 0 && savingsInstantTransitions.length == 0
>
> Modified as suggested:
>
> http://cr.openjdk.java.net/~naoto/8239836/webrev.01/
>
>>
>> It should be inlined in cases where the performance matters.
>
> None of those locations is invoked during ZoneRules object 
> instantiation. So I believe it is OK to replace them with 
> isFixedOffset().
>
> Naoto
>
>>
>> Thanks, Roger
>>
>>
>> On 2/27/20 3:41 PM, naoto.sato at oracle.com wrote:
>>> Hello,
>>>
>>> Please review the fix to the following issue:
>>>
>>> https://bugs.openjdk.java.net/browse/JDK-8239836
>>>
>>> The proposed changeset is located at:
>>>
>>> https://cr.openjdk.java.net/~naoto/8239836/webrev.00/
>>>
>>> It turned out that 'transitionList' is not necessarily a superset of
>>> 'standardOffsetTransitionList', as in some occasions where a 
>>> standard offset change and a savings change happen at the same time 
>>> (canceling each other), resulting in no wall time transition. This 
>>> means that the "rules" in the sample code is a valid ZoneRules 
>>> instance.
>>>
>>> However, there is an assumption in ZoneRules where no (wall time) 
>>> transition means the fixed offset zone. With that assumption, the 
>>> example rule is considered a fixed offset zone which is not correct. 
>>> So the fix proposed here is not to throw an IAE but to recognize the 
>>> rule as a valid, non-fixed offset ZoneRules instance.
>>>
>>> Naoto
>>>
>>>
>>