search

ThreeTen / threeten

This project was the home of code used to develop a modern date and time library for JDK8. Development has moved to OpenJDK and a separate backport project, threetenbp.
http://threeten.github.io/
191 stars 37 forks source link

Separate temporal interface layer #341

Closed jodastephen closed 10 years ago

jodastephen commented 10 years ago

The only remaining connection between the temporal interfaces and the chrono intrefaces or concrete classes is the definition of resolve on TemporalField. If this is changed from zone and chronology to be TemporalAccessor, the separation can be complete.

The JDK implementations can use package scoped methods to avoid any performance loss.

jodastephen commented 10 years ago 
# HG changeset patch
# User scolebourne
# Date 1380261929 25200
# Node ID c3e5fbaea75d66142da5ab4ce263cb287b5e233b
# Parent  671161b3333ed8650adf9f881d27b0e8fbc734b2
Remove ZoneId and Chronology from TemporalField interface

See #341

diff -r 671161b3333e -r c3e5fbaea75d src/share/classes/java/time/format/Parsed.java
--- a/src/share/classes/java/time/format/Parsed.java    Thu Sep 26 15:19:27 2013 -0700
+++ b/src/share/classes/java/time/format/Parsed.java    Thu Sep 26 23:05:29 2013 -0700
@@ -262,7 +262,7 @@
             while (changedCount < 50) {
                 for (Map.Entry<TemporalField, Long> entry : fieldValues.entrySet()) {
                     TemporalField targetField = entry.getKey();
-                    TemporalAccessor resolvedObject = targetField.resolve(fieldValues, chrono, zone, resolverStyle);
+                    TemporalAccessor resolvedObject = targetField.resolve(fieldValues, this, resolverStyle);
                     if (resolvedObject != null) {
                         if (resolvedObject instanceof ChronoZonedDateTime) {
                             ChronoZonedDateTime czdt = (ChronoZonedDateTime) resolvedObject;
diff -r 671161b3333e -r c3e5fbaea75d src/share/classes/java/time/temporal/IsoFields.java
--- a/src/share/classes/java/time/temporal/IsoFields.java   Thu Sep 26 15:19:27 2013 -0700
+++ b/src/share/classes/java/time/temporal/IsoFields.java   Thu Sep 26 23:05:29 2013 -0700
@@ -69,6 +69,7 @@
 import static java.time.temporal.ChronoUnit.WEEKS;
 import static java.time.temporal.ChronoUnit.YEARS;

+import java.time.DateTimeException;
 import java.time.Duration;
 import java.time.LocalDate;
 import java.time.ZoneId;
@@ -343,7 +344,7 @@
             }
             @Override
             public ChronoLocalDate resolve(
-                    Map<TemporalField, Long> fieldValues, Chronology chronology, ZoneId zone, ResolverStyle resolverStyle) {
+                    Map<TemporalField, Long> fieldValues, TemporalAccessor partialTemporal, ResolverStyle resolverStyle) {
                 Long yearLong = fieldValues.get(YEAR);
                 Long qoyLong = fieldValues.get(QUARTER_OF_YEAR);
                 if (yearLong == null || qoyLong == null) {
@@ -351,6 +352,7 @@
                 }
                 int y = YEAR.checkValidIntValue(yearLong);  // always validate
                 long doq = fieldValues.get(DAY_OF_QUARTER);
+                ensureIso(partialTemporal);
                 LocalDate date;
                 if (resolverStyle == ResolverStyle.LENIENT) {
                     date = LocalDate.of(y, 1, 1).plusMonths(Math.multiplyExact(Math.subtractExact(qoyLong, 1), 3));
@@ -464,7 +466,7 @@
             }
             @Override
             public ChronoLocalDate resolve(
-                    Map<TemporalField, Long> fieldValues, Chronology chronology, ZoneId zone, ResolverStyle resolverStyle) {
+                    Map<TemporalField, Long> fieldValues, TemporalAccessor partialTemporal, ResolverStyle resolverStyle) {
                 Long wbyLong = fieldValues.get(WEEK_BASED_YEAR);
                 Long dowLong = fieldValues.get(DAY_OF_WEEK);
                 if (wbyLong == null || dowLong == null) {
@@ -472,6 +474,7 @@
                 }
                 int wby = WEEK_BASED_YEAR.range().checkValidIntValue(wbyLong, WEEK_BASED_YEAR);  // always validate
                 long wowby = fieldValues.get(WEEK_OF_WEEK_BASED_YEAR);
+                ensureIso(partialTemporal);
                 LocalDate date = LocalDate.of(wby, 1, 4);
                 if (resolverStyle == ResolverStyle.LENIENT) {
                     long dow = dowLong;  // unvalidated
@@ -568,6 +571,12 @@
             return Chronology.from(temporal).equals(IsoChronology.INSTANCE);
         }

+        private static void ensureIso(TemporalAccessor temporal) {
+            if (isIso(temporal) == false) {
+                throw new DateTimeException("Resolve requires IsoChronology");
+            }
+        }
+
         private static ValueRange getWeekRange(LocalDate date) {
             int wby = getWeekBasedYear(date);
             date = date.withDayOfYear(1).withYear(wby);
diff -r 671161b3333e -r c3e5fbaea75d src/share/classes/java/time/temporal/JulianFields.java
--- a/src/share/classes/java/time/temporal/JulianFields.java    Thu Sep 26 15:19:27 2013 -0700
+++ b/src/share/classes/java/time/temporal/JulianFields.java    Thu Sep 26 23:05:29 2013 -0700
@@ -291,13 +291,14 @@
         //-----------------------------------------------------------------------
         @Override
         public ChronoLocalDate resolve(
-                Map<TemporalField, Long> fieldValues, Chronology chronology, ZoneId zone, ResolverStyle resolverStyle) {
+                Map<TemporalField, Long> fieldValues, TemporalAccessor partialTemporal, ResolverStyle resolverStyle) {
             long value = fieldValues.remove(this);
+            Chronology chrono = Chronology.from(partialTemporal);
             if (resolverStyle == ResolverStyle.LENIENT) {
-                return chronology.dateEpochDay(Math.subtractExact(value, offset));
+                return chrono.dateEpochDay(Math.subtractExact(value, offset));
             }
             range().checkValidValue(value, this);
-            return chronology.dateEpochDay(value - offset);
+            return chrono.dateEpochDay(value - offset);
         }

         //-----------------------------------------------------------------------
diff -r 671161b3333e -r c3e5fbaea75d src/share/classes/java/time/temporal/TemporalField.java
--- a/src/share/classes/java/time/temporal/TemporalField.java   Thu Sep 26 15:19:27 2013 -0700
+++ b/src/share/classes/java/time/temporal/TemporalField.java   Thu Sep 26 23:05:29 2013 -0700
@@ -62,7 +62,6 @@
 package java.time.temporal;

 import java.time.DateTimeException;
-import java.time.ZoneId;
 import java.time.chrono.Chronology;
 import java.time.format.ResolverStyle;
 import java.util.Locale;
@@ -338,6 +337,12 @@
      * complete {@code LocalDate}. The resolve method will remove all three
      * fields from the map before returning the {@code LocalDate}.
      * <p>
+     * A partially complete temporal is used to allow the chronology and zone
+     * to be queried. In general, only the chronology will be needed.
+     * Querying items other than the zone or chronology is undefined and
+     * must not be relied on. Other methods such as {@code get}, {@code getLong}
+     * {@code range} and {@code isSupported} must not be called.
+     * <p>
      * If resolution should be possible, but the data is invalid, the resolver
      * style should be used to determine an appropriate level of leniency, which
      * may require throwing a {@code DateTimeException} or {@code ArithmeticException}.
@@ -350,16 +355,14 @@
      * instances that can produce a date, such as {@code EPOCH_DAY}.
      * <p>
      * Not all {@code TemporalAccessor} implementations are accepted as return values.
-     * Implementations must accept {@code ChronoLocalDate}, {@code ChronoLocalDateTime},
-     * {@code ChronoZonedDateTime} and {@code LocalTime}.
-     * <p>
-     * The zone is not normally required for resolution, but is provided for completeness.
+     * Implementations that call this method must accept {@code ChronoLocalDate},
+     * {@code ChronoLocalDateTime}, {@code ChronoZonedDateTime} and {@code LocalTime}.
      * <p>
      * The default implementation must return null.
      *
      * @param fieldValues  the map of fields to values, which can be updated, not null
-     * @param chronology  the effective chronology, not null
-     * @param zone  the effective zone, not null
+     * @param partialTemporal  the partially complete temporal to query for zone and
+     *  chronology; querying for other things is undefined and not recommended, not null
      * @param resolverStyle  the requested type of resolve, not null
      * @return the resolved temporal object; null if resolving only
      *  changed the map, or no resolve occurred
@@ -368,8 +371,9 @@
      *  by querying a field on the temporal without first checking if it is supported
      */
     default TemporalAccessor resolve(
-            Map<TemporalField, Long> fieldValues, Chronology chronology,
-            ZoneId zone, ResolverStyle resolverStyle) {
+            Map<TemporalField, Long> fieldValues,
+            TemporalAccessor partialTemporal,
+            ResolverStyle resolverStyle) {
         return null;
     }

diff -r 671161b3333e -r c3e5fbaea75d src/share/classes/java/time/temporal/WeekFields.java
--- a/src/share/classes/java/time/temporal/WeekFields.java  Thu Sep 26 15:19:27 2013 -0700
+++ b/src/share/classes/java/time/temporal/WeekFields.java  Thu Sep 26 23:05:29 2013 -0700
@@ -892,7 +892,7 @@

         @Override
         public ChronoLocalDate resolve(
-                Map<TemporalField, Long> fieldValues, Chronology chronology, ZoneId zone, ResolverStyle resolverStyle) {
+                Map<TemporalField, Long> fieldValues, TemporalAccessor partialTemporal, ResolverStyle resolverStyle) {
             final long value = fieldValues.get(this);
             final int newValue = Math.toIntExact(value);  // broad limit makes overflow checking lighter
             // first convert localized day-of-week to ISO day-of-week
@@ -915,19 +915,20 @@
             int dow = localizedDayOfWeek(isoDow);

             // build date
+            Chronology chrono = Chronology.from(partialTemporal);
             if (fieldValues.containsKey(YEAR)) {
                 int year = YEAR.checkValidIntValue(fieldValues.get(YEAR));  // validate
                 if (rangeUnit == MONTHS && fieldValues.containsKey(MONTH_OF_YEAR)) {  // week-of-month
                     long month = fieldValues.get(MONTH_OF_YEAR);  // not validated yet
-                    return resolveWoM(fieldValues, chronology, year, month, newValue, dow, resolverStyle);
+                    return resolveWoM(fieldValues, chrono, year, month, newValue, dow, resolverStyle);
                 }
                 if (rangeUnit == YEARS) {  // week-of-year
-                    return resolveWoY(fieldValues, chronology, year, newValue, dow, resolverStyle);
+                    return resolveWoY(fieldValues, chrono, year, newValue, dow, resolverStyle);
                 }
             } else if ((rangeUnit == WEEK_BASED_YEARS || rangeUnit == FOREVER) &&
                     fieldValues.containsKey(weekDef.weekBasedYear) &&
                     fieldValues.containsKey(weekDef.weekOfWeekBasedYear)) { // week-of-week-based-year and year-of-week-based-year
-                return resolveWBY(fieldValues, chronology, dow, resolverStyle);
+                return resolveWBY(fieldValues, chrono, dow, resolverStyle);
             }
             return null;
         }
diff -r 671161b3333e -r c3e5fbaea75d test/java/time/tck/java/time/format/TCKDateTimeParseResolver.java
--- a/test/java/time/tck/java/time/format/TCKDateTimeParseResolver.java Thu Sep 26 15:19:27 2013 -0700
+++ b/test/java/time/tck/java/time/format/TCKDateTimeParseResolver.java Thu Sep 26 23:05:29 2013 -0700
@@ -927,8 +927,7 @@
             }
             @Override
             public TemporalAccessor resolve(
-                    Map<TemporalField, Long> fieldValues, Chronology chronology,
-                    ZoneId zone, ResolverStyle resolverStyle) {
+                    Map<TemporalField, Long> fieldValues, TemporalAccessor partialTemporal, ResolverStyle resolverStyle) {
                 return LocalTime.MIDNIGHT.plusNanos(fieldValues.remove(this));
             }
         };
@@ -979,8 +978,7 @@
             }
             @Override
             public TemporalAccessor resolve(
-                    Map<TemporalField, Long> fieldValues, Chronology chronology,
-                    ZoneId zone, ResolverStyle resolverStyle) {
+                    Map<TemporalField, Long> fieldValues, TemporalAccessor partialTemporal, ResolverStyle resolverStyle) {
                 fieldValues.remove(this);
                 return LocalDateTime.of(2010, 6, 30, 12, 30);
             }
@@ -1032,8 +1030,7 @@
             }
             @Override
             public TemporalAccessor resolve(
-                    Map<TemporalField, Long> fieldValues, Chronology chronology,
-                    ZoneId zone, ResolverStyle resolverStyle) {
+                    Map<TemporalField, Long> fieldValues, TemporalAccessor partialTemporal, ResolverStyle resolverStyle) {
                 return ThaiBuddhistChronology.INSTANCE.dateNow();
             }
         };
@@ -1082,8 +1079,7 @@
             }
             @Override
             public TemporalAccessor resolve(
-                    Map<TemporalField, Long> fieldValues, Chronology chronology,
-                    ZoneId zone, ResolverStyle resolverStyle) {
+                    Map<TemporalField, Long> fieldValues, TemporalAccessor partialTemporal, ResolverStyle resolverStyle) {
                 return ZonedDateTime.of(2010, 6, 30, 12, 30, 0, 0, ZoneId.of("Europe/Paris"));
             }
         };
RogerRiggs commented 10 years ago

JBS issue https://bugs.openjdk.java.net/browse/JDK-8025720

RogerRiggs commented 10 years ago

Resolved by http://hg.openjdk.java.net/jdk8/tl/jdk/rev/ea422834f880