RFR: JDK-8079063 ZoneOffsetTransition constructor should ignore nanoseconds (original) (raw)

Roger Riggs Roger.Riggs at Oracle.com
Wed May 6 14:43:22 UTC 2015

Hi Peter,

Thanks for the analysis and followup.

Yes, I think thesimple check as you propose is the desired clarification. I agree that the change should not affect any existing code outside the JDK and does not raise a compatibility issue.

Roger

On 5/4/2015 6:22 AM, Peter Levart wrote:

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

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

* 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

More information about the core-libs-dev mailing list