RFR: JDK-8079063 ZoneOffsetTransition constructor should ignore nanoseconds

[Peter Levart]

Hi,

Now that JDK-8074003 is in, I'd like to discuss how to tackle JDK-8079063.

Package-private ZoneOffsetTransition constructor that takes 
LocalDateTime transition is invoked from the following 4 places:

1 - the public static factory method:

     /**
      * Obtains an instance defining a transition between two offsets.
      * <p>
      * Applications should normally obtain an instance from {@link 
ZoneRules}.
      * This factory is only intended for use when creating {@link 
ZoneRules}.
      *
      * @param transition  the transition date-time at the transition, 
which never
      *  actually occurs, expressed local to the before offset, not null
      * @param offsetBefore  the offset before the transition, not null
      * @param offsetAfter  the offset at and after the transition, not null
      * @return the transition, not null
      * @throws IllegalArgumentException if {@code offsetBefore} and 
{@code offsetAfter}
      *         are equal, or {@code transition.getNano()} returns 
non-zero value
      */
     public static ZoneOffsetTransition of(LocalDateTime transition, 
ZoneOffset offsetBefore, ZoneOffset offsetAfter) {

...this one already disallows transition parameters that have 
transition.getNano() != 0.


2 - Lines 498..500 of ZoneOffsetTransitionRule:

         LocalDateTime localDT = LocalDateTime.of(date, time);
         LocalDateTime transition = 
timeDefinition.createDateTime(localDT, standardOffset, offsetBefore);
         return new ZoneOffsetTransition(transition, offsetBefore, 
offsetAfter);

...where 'time' is an instance field of ZoneOffsetTransitionRule. The 
ZoneOffsetTransitionRule public static factory method has the following 
definition:

     /**
      * Obtains an instance defining the yearly rule to create 
transitions between two offsets.
      * <p>
      * Applications should normally obtain an instance from {@link 
ZoneRules}.
      * This factory is only intended for use when creating {@link 
ZoneRules}.
      *
      * @param month  the month of the month-day of the first day of the 
cutover week, not null
      * @param dayOfMonthIndicator  the day of the month-day of the 
cutover week, positive if the week is that
      *  day or later, negative if the week is that day or earlier, 
counting from the last day of the month,
      *  from -28 to 31 excluding 0
      * @param dayOfWeek  the required day-of-week, null if the 
month-day should not be changed
      * @param time  the cutover time in the 'before' offset, not null
      * @param timeEndOfDay  whether the time is midnight at the end of day
      * @param timeDefnition  how to interpret the cutover
      * @param standardOffset  the standard offset in force at the 
cutover, not null
      * @param offsetBefore  the offset before the cutover, not null
      * @param offsetAfter  the offset after the cutover, not null
      * @return the rule, not null
      * @throws IllegalArgumentException if the day of month indicator 
is invalid
      * @throws IllegalArgumentException if the end of day flag is true 
when the time is not midnight
      */
     public static ZoneOffsetTransitionRule of(
             Month month,
             int dayOfMonthIndicator,
             DayOfWeek dayOfWeek,
             LocalTime time,
             boolean timeEndOfDay,
             TimeDefinition timeDefnition,
             ZoneOffset standardOffset,
             ZoneOffset offsetBefore,
             ZoneOffset offsetAfter) {
         Objects.requireNonNull(month, "month");
         Objects.requireNonNull(time, "time");
         Objects.requireNonNull(timeDefnition, "timeDefnition");
         Objects.requireNonNull(standardOffset, "standardOffset");
         Objects.requireNonNull(offsetBefore, "offsetBefore");
         Objects.requireNonNull(offsetAfter, "offsetAfter");
         if (dayOfMonthIndicator < -28 || dayOfMonthIndicator > 31 || 
dayOfMonthIndicator == 0) {
             throw new IllegalArgumentException("Day of month indicator 
must be between -28 and 31 inclusive excluding zero");
         }
         if (timeEndOfDay && time.equals(LocalTime.MIDNIGHT) == false) {
             throw new IllegalArgumentException("Time must be midnight 
when end of day flag is true");
         }
         return new ZoneOffsetTransitionRule(month, dayOfMonthIndicator, 
dayOfWeek, time, timeEndOfDay, timeDefnition, standardOffset, 
offsetBefore, offsetAfter);
     }

...which allows 'time' parameter (that becomes 
ZoneOffsetTransitionRule's 'time' field) with no restriction as to 
whether it can contain nanos != 0 or not.


3, 4 - Lines 668..678 of ZoneRules private getOffsetInfo method:

             LocalDateTime dtBefore = savingsLocalTransitions[index];
             LocalDateTime dtAfter = savingsLocalTransitions[index + 1];
             ZoneOffset offsetBefore = wallOffsets[index / 2];
             ZoneOffset offsetAfter = wallOffsets[index / 2 + 1];
             if (offsetAfter.getTotalSeconds() > 
offsetBefore.getTotalSeconds()) {
                 // gap
                 return new ZoneOffsetTransition(dtBefore, offsetBefore, 
offsetAfter);
             } else {
                 // overlap
                 return new ZoneOffsetTransition(dtAfter, offsetBefore, 
offsetAfter);
             }

...where dtBefore/dtAfter "transition" parameters are taken from 
savingsLocalTransitions[] array that is filled-in in ZoneRules 
constructors from passed-in ZoneOffsetTransition objects. So here no 
nanos != 0 can sneak in if ZoneOffsetTransition invariant holds.

The only place where nanos can sneak-in therefore seems to be the public 
ZoneOffsetTransitionRule.of() factory method. The question is whether 
the spec. could be changed so that ZoneOffsetTransitionRule.of() factory 
method would add another @throws definition:

      * @throws IllegalArgumentException if {@code time.getNano()} 
returns non-zero value


This, I think, would be consistent with ZoneOffsetTransition.of() 
factory method and I believe early enough in the live of the API so that 
no custom software would be affected:

http://cr.openjdk.java.net/~plevart/jdk9-dev/ZoneOffsetTransitionRule.time/webrev.01/

What do you think?

Regards, Peter