From 531f595043b1961f513c49d7166ed554282bfc8b Mon Sep 17 00:00:00 2001 From: Mihai Nita Date: Thu, 19 Sep 2024 08:16:23 -0700 Subject: [PATCH] ICU-22853 Do not try to format java.time.Instant and Clock --- .../com/ibm/icu/impl/JavaTimeConverters.java | 55 ++----------------- .../message2/DateTimeFormatterFactory.java | 5 +- .../icu/message2/MFDataModelFormatter.java | 2 - .../java/com/ibm/icu/text/DateFormat.java | 4 -- .../java/com/ibm/icu/text/MessageFormat.java | 4 -- .../com/ibm/icu/text/SimpleDateFormat.java | 3 - .../test/format/JavaTimeConvertersTest.java | 22 +------- .../dev/test/format/JavaTimeFormatTest.java | 44 +++++++++------ 8 files changed, 38 insertions(+), 101 deletions(-) diff --git a/icu4j/main/core/src/main/java/com/ibm/icu/impl/JavaTimeConverters.java b/icu4j/main/core/src/main/java/com/ibm/icu/impl/JavaTimeConverters.java index eaa6b01af0a..95ef822bfbb 100644 --- a/icu4j/main/core/src/main/java/com/ibm/icu/impl/JavaTimeConverters.java +++ b/icu4j/main/core/src/main/java/com/ibm/icu/impl/JavaTimeConverters.java @@ -5,7 +5,6 @@ package com.ibm.icu.impl; import static java.time.temporal.ChronoField.MILLI_OF_SECOND; -import java.time.Clock; import java.time.Instant; import java.time.LocalDate; import java.time.LocalDateTime; @@ -34,7 +33,7 @@ import com.ibm.icu.util.ULocale; * *

* The class includes methods for converting various temporal types, such as - * {@link Instant}, {@link ZonedDateTime}, {@link OffsetTime}, {@link OffsetDateTime}, {@link LocalTime}, + * {@link ZonedDateTime}, {@link OffsetTime}, {@link OffsetDateTime}, {@link LocalTime}, * {@link ChronoLocalDate}, and {@link ChronoLocalDateTime}, to {@link Calendar} instances. * *

@@ -52,47 +51,6 @@ public class JavaTimeConverters { // Prevent instantiation, making this an utility class } - /** - * Converts the current instant from a {@link Clock} to a {@link Calendar}. - * - *

- * This method creates a {@link Calendar} instance that represents the current - * instant as provided by the specified {@link Clock}. - * - * @param clock The {@link Clock} providing the current instant. - * @return A {@link Calendar} instance representing the current instant as - * provided by the specified {@link Clock}. - * - * @deprecated This API is ICU internal only. - */ - @Deprecated - public static Calendar temporalToCalendar(Clock clock) { - long epochMillis = clock.millis(); - String timeZone = clock.getZone().getId(); - TimeZone icuTimeZone = TimeZone.getTimeZone(timeZone); - return millisToCalendar(epochMillis, icuTimeZone); - } - - /** - * Converts an {@link Instant} to a {@link Calendar}. - * - *

- * This method creates a {@link Calendar} instance that represents the same - * point in time as the specified {@link Instant}. The resulting - * {@link Calendar} will be in the default time zone of the JVM. - * - * @param instant The {@link Instant} to convert. - * @return A {@link Calendar} instance representing the same point in time as - * the specified {@link Instant}. - * - * @deprecated This API is ICU internal only. - */ - @Deprecated - public static Calendar temporalToCalendar(Instant instant) { - long epochMillis = instant.toEpochMilli(); - return millisToCalendar(epochMillis, TimeZone.GMT_ZONE); - } - /** * Converts a {@link ZonedDateTime} to a {@link Calendar}. * @@ -232,10 +190,9 @@ public class JavaTimeConverters { */ @Deprecated public static Calendar temporalToCalendar(Temporal temp) { - if (temp instanceof Clock) { - return temporalToCalendar((Clock) temp); - } else if (temp instanceof Instant) { - return temporalToCalendar((Instant) temp); + if (temp instanceof Instant) { + throw new IllegalArgumentException("java.time.Instant cannot be formatted," + + " it does not have enough information"); } else if (temp instanceof ZonedDateTime) { return temporalToCalendar((ZonedDateTime) temp); } else if (temp instanceof OffsetDateTime) { @@ -253,8 +210,8 @@ public class JavaTimeConverters { } else if (temp instanceof ChronoLocalDateTime) { return temporalToCalendar((ChronoLocalDateTime) temp); } else { - System.out.println("WTF is " + temp.getClass()); - return null; + throw new IllegalArgumentException("This type cannot be formatted: " + + temp.getClass().getName()); } } diff --git a/icu4j/main/core/src/main/java/com/ibm/icu/message2/DateTimeFormatterFactory.java b/icu4j/main/core/src/main/java/com/ibm/icu/message2/DateTimeFormatterFactory.java index ce06605761d..8b42d9ad13e 100644 --- a/icu4j/main/core/src/main/java/com/ibm/icu/message2/DateTimeFormatterFactory.java +++ b/icu4j/main/core/src/main/java/com/ibm/icu/message2/DateTimeFormatterFactory.java @@ -3,7 +3,6 @@ package com.ibm.icu.message2; -import java.time.Clock; import java.time.temporal.Temporal; import java.util.Calendar; import java.util.Locale; @@ -352,12 +351,10 @@ class DateTimeFormatterFactory implements FormatterFactory { return new FormattedPlaceholder( toFormat, new PlainStringFormattedValue("{|" + toFormat + "|}")); } - } else if (toFormat instanceof Clock) { - toFormat = JavaTimeConverters.temporalToCalendar((Clock) toFormat); } else if (toFormat instanceof Temporal) { toFormat = JavaTimeConverters.temporalToCalendar((Temporal) toFormat); } - // Not an else-if here, because the `Clock` & `Temporal` conditions before make `toFormat` a `Calendar` + // Not an else-if here, because the `Temporal` conditions before make `toFormat` a `Calendar` if (toFormat instanceof Calendar) { TimeZone tz = ((Calendar) toFormat).getTimeZone(); long milis = ((Calendar) toFormat).getTimeInMillis(); diff --git a/icu4j/main/core/src/main/java/com/ibm/icu/message2/MFDataModelFormatter.java b/icu4j/main/core/src/main/java/com/ibm/icu/message2/MFDataModelFormatter.java index 89c424c0970..63b6f5bfe8d 100644 --- a/icu4j/main/core/src/main/java/com/ibm/icu/message2/MFDataModelFormatter.java +++ b/icu4j/main/core/src/main/java/com/ibm/icu/message2/MFDataModelFormatter.java @@ -3,7 +3,6 @@ package com.ibm.icu.message2; -import java.time.Clock; import java.time.temporal.Temporal; import java.util.ArrayList; import java.util.Date; @@ -62,7 +61,6 @@ class MFDataModelFormatter { .setDefaultFormatterNameForType(Date.class, "datetime") .setDefaultFormatterNameForType(Calendar.class, "datetime") .setDefaultFormatterNameForType(java.util.Calendar.class, "datetime") - .setDefaultFormatterNameForType(Clock.class, "datetime") .setDefaultFormatterNameForType(Temporal.class, "datetime") // Number formatting diff --git a/icu4j/main/core/src/main/java/com/ibm/icu/text/DateFormat.java b/icu4j/main/core/src/main/java/com/ibm/icu/text/DateFormat.java index 0bb72bf2ad6..07a90e65648 100644 --- a/icu4j/main/core/src/main/java/com/ibm/icu/text/DateFormat.java +++ b/icu4j/main/core/src/main/java/com/ibm/icu/text/DateFormat.java @@ -14,7 +14,6 @@ import java.text.FieldPosition; import java.text.Format; import java.text.ParseException; import java.text.ParsePosition; -import java.time.Clock; import java.time.temporal.Temporal; import java.util.Arrays; import java.util.Date; @@ -633,9 +632,6 @@ public abstract class DateFormat extends UFormat { } else if (obj instanceof Number) { return format( new Date(((Number)obj).longValue()), toAppendTo, fieldPosition ); - } else if (obj instanceof Clock) { - return format(JavaTimeConverters.temporalToCalendar((Clock) obj), - toAppendTo, fieldPosition); } else if (obj instanceof Temporal) { return format( (Temporal)obj, toAppendTo, fieldPosition ); } else { diff --git a/icu4j/main/core/src/main/java/com/ibm/icu/text/MessageFormat.java b/icu4j/main/core/src/main/java/com/ibm/icu/text/MessageFormat.java index 0319294bcf4..6a9a6cc37b8 100644 --- a/icu4j/main/core/src/main/java/com/ibm/icu/text/MessageFormat.java +++ b/icu4j/main/core/src/main/java/com/ibm/icu/text/MessageFormat.java @@ -24,7 +24,6 @@ import java.text.FieldPosition; import java.text.Format; import java.text.ParseException; import java.text.ParsePosition; -import java.time.Clock; import java.time.temporal.Temporal; import java.util.ArrayList; import java.util.Date; @@ -1754,9 +1753,6 @@ public class MessageFormat extends UFormat { } else if (arg instanceof Calendar) { // format a Calendar if can dest.formatAndAppend(getStockDateFormatter(), arg); - } else if (arg instanceof Clock) { - // format a Clock if can - dest.formatAndAppend(getStockDateFormatter(), arg); } else if (arg instanceof Temporal) { // format a Temporal if can dest.formatAndAppend(getStockDateFormatter(), arg); diff --git a/icu4j/main/core/src/main/java/com/ibm/icu/text/SimpleDateFormat.java b/icu4j/main/core/src/main/java/com/ibm/icu/text/SimpleDateFormat.java index 06bcbf44c9e..1a6acb8d102 100644 --- a/icu4j/main/core/src/main/java/com/ibm/icu/text/SimpleDateFormat.java +++ b/icu4j/main/core/src/main/java/com/ibm/icu/text/SimpleDateFormat.java @@ -17,7 +17,6 @@ import java.text.AttributedString; import java.text.FieldPosition; import java.text.Format; import java.text.ParsePosition; -import java.time.Clock; import java.time.temporal.Temporal; import java.util.ArrayList; import java.util.Date; @@ -3920,8 +3919,6 @@ public class SimpleDateFormat extends DateFormat { calendar.setTime((Date)obj); } else if (obj instanceof Number) { calendar.setTimeInMillis(((Number)obj).longValue()); - } else if (obj instanceof Clock) { - cal = JavaTimeConverters.temporalToCalendar((Clock) obj); } else if (obj instanceof Temporal) { cal = JavaTimeConverters.temporalToCalendar((Temporal) obj); } else { diff --git a/icu4j/main/core/src/test/java/com/ibm/icu/dev/test/format/JavaTimeConvertersTest.java b/icu4j/main/core/src/test/java/com/ibm/icu/dev/test/format/JavaTimeConvertersTest.java index acc278fd7f3..a56699e40e1 100644 --- a/icu4j/main/core/src/test/java/com/ibm/icu/dev/test/format/JavaTimeConvertersTest.java +++ b/icu4j/main/core/src/test/java/com/ibm/icu/dev/test/format/JavaTimeConvertersTest.java @@ -3,7 +3,6 @@ package com.ibm.icu.dev.test.format; -import java.time.Clock; import java.time.Instant; import java.time.LocalDate; import java.time.LocalDateTime; @@ -125,25 +124,10 @@ public class JavaTimeConvertersTest extends CoreTestFmwk { } - @Test - public void testInstantAndClock() { - // Instant has no time zone, assumes GMT. - EXPECTED_CALENDAR.setTimeZone(TimeZone.GMT_ZONE); + @Test(expected = IllegalArgumentException.class) + public void testInstantFails() { Instant instant = Instant.ofEpochMilli(EXPECTED_CALENDAR.getTimeInMillis()); - Calendar calendar = JavaTimeConverters.temporalToCalendar(instant); - assertCalendarsEquals(EXPECTED_CALENDAR, calendar, DATE_ONLY_FIELDS); - assertCalendarsEquals(EXPECTED_CALENDAR, calendar, TIME_ONLY_FIELDS); - assertEquals("", EXPECTED_CALENDAR.getTimeZone().getID(), calendar.getTimeZone().getID()); - assertEquals("", EXPECTED_CALENDAR.getTimeZone().getRawOffset(), calendar.getTimeZone().getRawOffset()); - // Restore the time zone on the expected calendar - EXPECTED_CALENDAR.setTimeZone(TimeZone.getTimeZone(TIME_ZONE_ID)); - - Clock clock = Clock.fixed(instant, ZoneId.of(TIME_ZONE_ID)); - calendar = JavaTimeConverters.temporalToCalendar(clock); - assertCalendarsEquals(EXPECTED_CALENDAR, calendar, DATE_ONLY_FIELDS); - assertCalendarsEquals(EXPECTED_CALENDAR, calendar, TIME_ONLY_FIELDS); - assertEquals("", EXPECTED_CALENDAR.getTimeZone().getID(), calendar.getTimeZone().getID()); - assertEquals("", EXPECTED_CALENDAR.getTimeZone().getRawOffset(), calendar.getTimeZone().getRawOffset()); + JavaTimeConverters.temporalToCalendar(instant); } // Compare the expected / actual calendar, but using an allowlist diff --git a/icu4j/main/core/src/test/java/com/ibm/icu/dev/test/format/JavaTimeFormatTest.java b/icu4j/main/core/src/test/java/com/ibm/icu/dev/test/format/JavaTimeFormatTest.java index 7f5d8b9a377..80e4116cb0e 100644 --- a/icu4j/main/core/src/test/java/com/ibm/icu/dev/test/format/JavaTimeFormatTest.java +++ b/icu4j/main/core/src/test/java/com/ibm/icu/dev/test/format/JavaTimeFormatTest.java @@ -142,15 +142,11 @@ public class JavaTimeFormatTest extends CoreTestFmwk { } } - @Test - public void testInstantAndClockFormatting() { + @Test(expected = IllegalArgumentException.class) + public void testInstantFormattingFails() { DateFormat formatFromSkeleton = DateFormat.getInstanceForSkeleton("yMMMMd jmsSSSvvvv", Locale.US); - Instant instant = LDT.toInstant(ZoneOffset.UTC); - assertEquals("", "September 27, 2013 at 7:43:56.123 PM Greenwich Mean Time", formatFromSkeleton.format(instant)); - - Clock clock = Clock.fixed(instant, ZoneId.of("America/Los_Angeles")); - assertEquals("", "September 27, 2013 at 12:43:56.123 PM Pacific Time", formatFromSkeleton.format(clock)); + formatFromSkeleton.format(instant); } @Test @@ -208,15 +204,6 @@ public class JavaTimeFormatTest extends CoreTestFmwk { arguments.put("expDate", LDT.atOffset(ZoneOffset.ofHours(2))); assertEquals("", expectedMf1Result, mf.format(arguments)); assertEquals("", expectedMf2Result, mf2.formatToString(arguments)); - // Instant - Instant instant = LDT.toInstant(ZoneOffset.UTC); - arguments.put("expDate", instant); - assertEquals("", expectedMf1Result, mf.format(arguments)); - assertEquals("", expectedMf2Result, mf2.formatToString(arguments)); - // Clock - arguments.put("expDate", Clock.fixed(instant, ZoneId.of("Europe/Paris"))); - assertEquals("", expectedMf1Result, mf.format(arguments)); - assertEquals("", expectedMf2Result, mf2.formatToString(arguments)); // Test that both JDK and ICU Calendar are recognized as types. arguments.put("expDate", new java.util.GregorianCalendar(2013, 8, 27)); @@ -225,6 +212,31 @@ public class JavaTimeFormatTest extends CoreTestFmwk { // I filed https://unicode-org.atlassian.net/browse/ICU-22852 // MF2 converts the JDK Calendar to an ICU Calendar, so it works. assertEquals("", expectedMf2Result, mf2.formatToString(arguments)); + + // Make sure that Instant and Clock are not formatted + + // Instant + Instant instant = LDT.toInstant(ZoneOffset.UTC); + arguments.put("expDate", instant); + try { + mf.format(arguments); + fail("Should not be able to format java.time.Instant"); + } catch (IllegalArgumentException ex) { /* expected to throw */ } + try { + mf2.formatToString(arguments); + fail("Should not be able to format java.time.Instant"); + } catch (IllegalArgumentException ex) { /* expected to throw */ } + + // Clock + arguments.put("expDate", Clock.fixed(instant, ZoneId.of("Europe/Paris"))); + try { + mf.format(arguments); + fail("Should not be able to format java.time.Clock"); + } catch (IllegalArgumentException ex) { /* expected to throw */ } + try { + mf2.formatToString(arguments); + fail("Should not be able to format java.time.Clock"); + } catch (IllegalArgumentException ex) { /* expected to throw */ } } @Test