From bae3b4e4178c20ac24fece8e82099abe3b2630d0 Mon Sep 17 00:00:00 2001 From: Sarat Vemulapalli Date: Wed, 18 May 2022 17:19:35 -0700 Subject: [PATCH] Revert "Remove deprecated methods from JodaCompatibleZonedDateTime which are called by scripts (#3346) (#3348)" (#3393) This reverts commit a764fde5af6032c47bc158821d88d9de988b88c3. Signed-off-by: Sarat Vemulapalli --- .../painless/spi/org.opensearch.txt | 22 +++ .../script/JodaCompatibleZonedDateTime.java | 139 +++++++++++++++ .../JodaCompatibleZonedDateTimeTests.java | 163 +++++++++++++++--- 3 files changed, 304 insertions(+), 20 deletions(-) diff --git a/modules/lang-painless/src/main/resources/org/opensearch/painless/spi/org.opensearch.txt b/modules/lang-painless/src/main/resources/org/opensearch/painless/spi/org.opensearch.txt index 3741a13122bc8..debccf8ed4b9b 100644 --- a/modules/lang-painless/src/main/resources/org/opensearch/painless/spi/org.opensearch.txt +++ b/modules/lang-painless/src/main/resources/org/opensearch/painless/spi/org.opensearch.txt @@ -124,7 +124,29 @@ class org.opensearch.script.JodaCompatibleZonedDateTime { ZonedDateTime withYear(int) ZonedDateTime withZoneSameLocal(ZoneId) ZonedDateTime withZoneSameInstant(ZoneId) + + #### Joda time methods + long getMillis() + int getCenturyOfEra() + int getEra() + int getHourOfDay() + int getMillisOfDay() + int getMillisOfSecond() + int getMinuteOfDay() + int getMinuteOfHour() + int getMonthOfYear() + int getSecondOfDay() + int getSecondOfMinute() + int getWeekOfWeekyear() + int getWeekyear() + int getYearOfCentury() + int getYearOfEra() + String toString(String) + String toString(String,Locale) + + # conflicting methods DayOfWeek getDayOfWeekEnum() + int getDayOfWeek() } class org.opensearch.index.fielddata.ScriptDocValues$Dates { diff --git a/server/src/main/java/org/opensearch/script/JodaCompatibleZonedDateTime.java b/server/src/main/java/org/opensearch/script/JodaCompatibleZonedDateTime.java index 08306b3f275a8..8f48da739359a 100644 --- a/server/src/main/java/org/opensearch/script/JodaCompatibleZonedDateTime.java +++ b/server/src/main/java/org/opensearch/script/JodaCompatibleZonedDateTime.java @@ -32,9 +32,16 @@ package org.opensearch.script; +import org.joda.time.DateTime; import org.opensearch.common.SuppressForbidden; +import org.opensearch.common.SuppressLoggerChecks; +import org.opensearch.common.logging.DeprecationLogger; import org.opensearch.common.time.DateFormatter; +import org.opensearch.common.time.DateFormatters; +import org.opensearch.common.time.DateUtils; +import java.security.AccessController; +import java.security.PrivilegedAction; import java.time.DayOfWeek; import java.time.Instant; import java.time.LocalDate; @@ -48,6 +55,7 @@ import java.time.chrono.ChronoZonedDateTime; import java.time.chrono.Chronology; import java.time.format.DateTimeFormatter; +import java.time.temporal.ChronoField; import java.time.temporal.Temporal; import java.time.temporal.TemporalAccessor; import java.time.temporal.TemporalAdjuster; @@ -56,6 +64,7 @@ import java.time.temporal.TemporalQuery; import java.time.temporal.TemporalUnit; import java.time.temporal.ValueRange; +import java.util.Locale; import java.util.Objects; /** @@ -71,6 +80,23 @@ public class JodaCompatibleZonedDateTime TemporalAccessor { private static final DateFormatter DATE_FORMATTER = DateFormatter.forPattern("strict_date_time"); + private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(JodaCompatibleZonedDateTime.class); + + private static void logDeprecated(String key, String message, Object... params) { + AccessController.doPrivileged(new PrivilegedAction() { + @SuppressLoggerChecks(reason = "safely delegates to logger") + @Override + public Void run() { + deprecationLogger.deprecate(key, message, params); + return null; + } + }); + } + + private static void logDeprecatedMethod(String oldMethod, String newMethod) { + logDeprecated(oldMethod, "Use of the joda time method [{}] is deprecated. Use [{}] instead.", oldMethod, newMethod); + } + private ZonedDateTime dt; public JodaCompatibleZonedDateTime(Instant instant, ZoneId zone) { @@ -401,7 +427,120 @@ public ZonedDateTime withZoneSameInstant(ZoneId zone) { return dt.withZoneSameInstant(zone); } + @Deprecated + public long getMillis() { + logDeprecatedMethod("getMillis()", "toInstant().toEpochMilli()"); + return dt.toInstant().toEpochMilli(); + } + + @Deprecated + public int getCenturyOfEra() { + logDeprecatedMethod("getCenturyOfEra()", "get(ChronoField.YEAR_OF_ERA) / 100"); + return dt.get(ChronoField.YEAR_OF_ERA) / 100; + } + + @Deprecated + public int getEra() { + logDeprecatedMethod("getEra()", "get(ChronoField.ERA)"); + return dt.get(ChronoField.ERA); + } + + @Deprecated + public int getHourOfDay() { + logDeprecatedMethod("getHourOfDay()", "getHour()"); + return dt.getHour(); + } + + @Deprecated + public int getMillisOfDay() { + logDeprecatedMethod("getMillisOfDay()", "get(ChronoField.MILLI_OF_DAY)"); + return dt.get(ChronoField.MILLI_OF_DAY); + } + + @Deprecated + public int getMillisOfSecond() { + logDeprecatedMethod("getMillisOfSecond()", "get(ChronoField.MILLI_OF_SECOND)"); + return dt.get(ChronoField.MILLI_OF_SECOND); + } + + @Deprecated + public int getMinuteOfDay() { + logDeprecatedMethod("getMinuteOfDay()", "get(ChronoField.MINUTE_OF_DAY)"); + return dt.get(ChronoField.MINUTE_OF_DAY); + } + + @Deprecated + public int getMinuteOfHour() { + logDeprecatedMethod("getMinuteOfHour()", "getMinute()"); + return dt.getMinute(); + } + + @Deprecated + public int getMonthOfYear() { + logDeprecatedMethod("getMonthOfYear()", "getMonthValue()"); + return dt.getMonthValue(); + } + + @Deprecated + public int getSecondOfDay() { + logDeprecatedMethod("getSecondOfDay()", "get(ChronoField.SECOND_OF_DAY)"); + return dt.get(ChronoField.SECOND_OF_DAY); + } + + @Deprecated + public int getSecondOfMinute() { + logDeprecatedMethod("getSecondOfMinute()", "getSecond()"); + return dt.getSecond(); + } + + @Deprecated + public int getWeekOfWeekyear() { + logDeprecatedMethod("getWeekOfWeekyear()", "get(DateFormatters.WEEK_FIELDS_ROOT.weekOfWeekBasedYear())"); + return dt.get(DateFormatters.WEEK_FIELDS_ROOT.weekOfWeekBasedYear()); + } + + @Deprecated + public int getWeekyear() { + logDeprecatedMethod("getWeekyear()", "get(DateFormatters.WEEK_FIELDS_ROOT.weekBasedYear())"); + return dt.get(DateFormatters.WEEK_FIELDS_ROOT.weekBasedYear()); + } + + @Deprecated + public int getYearOfCentury() { + logDeprecatedMethod("getYearOfCentury()", "get(ChronoField.YEAR_OF_ERA) % 100"); + return dt.get(ChronoField.YEAR_OF_ERA) % 100; + } + + @Deprecated + public int getYearOfEra() { + logDeprecatedMethod("getYearOfEra()", "get(ChronoField.YEAR_OF_ERA)"); + return dt.get(ChronoField.YEAR_OF_ERA); + } + + @Deprecated + public String toString(String format) { + logDeprecatedMethod("toString(String)", "a DateTimeFormatter"); + // TODO: replace with bwc formatter + return new DateTime(dt.toInstant().toEpochMilli(), DateUtils.zoneIdToDateTimeZone(dt.getZone())).toString(format); + } + + @Deprecated + public String toString(String format, Locale locale) { + logDeprecatedMethod("toString(String,Locale)", "a DateTimeFormatter"); + // TODO: replace with bwc formatter + return new DateTime(dt.toInstant().toEpochMilli(), DateUtils.zoneIdToDateTimeZone(dt.getZone())).toString(format, locale); + } + public DayOfWeek getDayOfWeekEnum() { return dt.getDayOfWeek(); } + + @Deprecated + public int getDayOfWeek() { + logDeprecated( + "getDayOfWeek()", + "The return type of [getDayOfWeek()] will change to an enum in 7.0. Use getDayOfWeekEnum().getValue()." + ); + return dt.getDayOfWeek().getValue(); + } } diff --git a/server/src/test/java/org/opensearch/script/JodaCompatibleZonedDateTimeTests.java b/server/src/test/java/org/opensearch/script/JodaCompatibleZonedDateTimeTests.java index a3156897540b2..81d0078d3d960 100644 --- a/server/src/test/java/org/opensearch/script/JodaCompatibleZonedDateTimeTests.java +++ b/server/src/test/java/org/opensearch/script/JodaCompatibleZonedDateTimeTests.java @@ -32,25 +32,42 @@ package org.opensearch.script; -import org.opensearch.common.time.DateFormatters; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.apache.logging.log4j.core.Appender; +import org.apache.logging.log4j.core.LogEvent; +import org.apache.logging.log4j.core.appender.AbstractAppender; +import org.opensearch.common.logging.Loggers; import org.opensearch.test.OpenSearchTestCase; import org.joda.time.DateTime; import org.joda.time.DateTimeZone; import org.junit.Before; +import java.security.AccessControlContext; +import java.security.AccessController; +import java.security.PermissionCollection; +import java.security.Permissions; +import java.security.PrivilegedAction; +import java.security.ProtectionDomain; import java.time.DayOfWeek; import java.time.Instant; import java.time.LocalDate; import java.time.LocalDateTime; import java.time.Month; import java.time.ZoneOffset; -import java.time.format.DateTimeFormatter; -import java.time.temporal.ChronoField; import java.util.Locale; import static org.hamcrest.Matchers.equalTo; public class JodaCompatibleZonedDateTimeTests extends OpenSearchTestCase { + private static final Logger DEPRECATION_LOGGER = LogManager.getLogger("org.opensearch.deprecation.script.JodaCompatibleZonedDateTime"); + + // each call to get or getValue will be run with limited permissions, just as they are in scripts + private static PermissionCollection NO_PERMISSIONS = new Permissions(); + private static AccessControlContext NO_PERMISSIONS_ACC = new AccessControlContext( + new ProtectionDomain[] { new ProtectionDomain(null, NO_PERMISSIONS) } + ); + private JodaCompatibleZonedDateTime javaTime; private DateTime jodaTime; @@ -61,6 +78,35 @@ public void setupTime() { jodaTime = new DateTime(millis, DateTimeZone.forOffsetHours(-7)); } + void assertDeprecation(Runnable assertions, String message) { + Appender appender = new AbstractAppender("test", null, null) { + @Override + public void append(LogEvent event) { + /* Create a temporary directory to prove we are running with the + * server's permissions. */ + createTempDir(); + } + }; + appender.start(); + Loggers.addAppender(DEPRECATION_LOGGER, appender); + try { + // the assertions are run with the same reduced privileges scripts run with + AccessController.doPrivileged((PrivilegedAction) () -> { + assertions.run(); + return null; + }, NO_PERMISSIONS_ACC); + } finally { + appender.stop(); + Loggers.removeAppender(DEPRECATION_LOGGER, appender); + } + + assertWarnings(message); + } + + void assertMethodDeprecation(Runnable assertions, String oldMethod, String newMethod) { + assertDeprecation(assertions, "Use of the joda time method [" + oldMethod + "] is deprecated. Use [" + newMethod + "] instead."); + } + public void testEquals() { assertThat(javaTime, equalTo(javaTime)); } @@ -127,77 +173,154 @@ public void testZone() { } public void testMillis() { - assertThat(javaTime.toInstant().toEpochMilli(), equalTo(jodaTime.getMillis())); + assertMethodDeprecation( + () -> assertThat(javaTime.getMillis(), equalTo(jodaTime.getMillis())), + "getMillis()", + "toInstant().toEpochMilli()" + ); } public void testCenturyOfEra() { - assertThat(javaTime.get(ChronoField.YEAR_OF_ERA) / 100, equalTo(jodaTime.getCenturyOfEra())); + assertMethodDeprecation( + () -> assertThat(javaTime.getCenturyOfEra(), equalTo(jodaTime.getCenturyOfEra())), + "getCenturyOfEra()", + "get(ChronoField.YEAR_OF_ERA) / 100" + ); } public void testEra() { - assertThat(javaTime.get(ChronoField.ERA), equalTo(jodaTime.getEra())); + assertMethodDeprecation(() -> assertThat(javaTime.getEra(), equalTo(jodaTime.getEra())), "getEra()", "get(ChronoField.ERA)"); } public void testHourOfDay() { - assertThat(javaTime.getHour(), equalTo(jodaTime.getHourOfDay())); + assertMethodDeprecation(() -> assertThat(javaTime.getHourOfDay(), equalTo(jodaTime.getHourOfDay())), "getHourOfDay()", "getHour()"); } public void testMillisOfDay() { - assertThat(javaTime.get(ChronoField.MILLI_OF_DAY), equalTo(jodaTime.getMillisOfDay())); + assertMethodDeprecation( + () -> assertThat(javaTime.getMillisOfDay(), equalTo(jodaTime.getMillisOfDay())), + "getMillisOfDay()", + "get(ChronoField.MILLI_OF_DAY)" + ); } public void testMillisOfSecond() { - assertThat(javaTime.get(ChronoField.MILLI_OF_SECOND), equalTo(jodaTime.getMillisOfSecond())); + assertMethodDeprecation( + () -> assertThat(javaTime.getMillisOfSecond(), equalTo(jodaTime.getMillisOfSecond())), + "getMillisOfSecond()", + "get(ChronoField.MILLI_OF_SECOND)" + ); } public void testMinuteOfDay() { - assertThat(javaTime.get(ChronoField.MINUTE_OF_DAY), equalTo(jodaTime.getMinuteOfDay())); + assertMethodDeprecation( + () -> assertThat(javaTime.getMinuteOfDay(), equalTo(jodaTime.getMinuteOfDay())), + "getMinuteOfDay()", + "get(ChronoField.MINUTE_OF_DAY)" + ); } public void testMinuteOfHour() { - assertThat(javaTime.getMinute(), equalTo(jodaTime.getMinuteOfHour())); + assertMethodDeprecation( + () -> assertThat(javaTime.getMinuteOfHour(), equalTo(jodaTime.getMinuteOfHour())), + "getMinuteOfHour()", + "getMinute()" + ); } public void testMonthOfYear() { - assertThat(javaTime.getMonthValue(), equalTo(jodaTime.getMonthOfYear())); + assertMethodDeprecation( + () -> assertThat(javaTime.getMonthOfYear(), equalTo(jodaTime.getMonthOfYear())), + "getMonthOfYear()", + "getMonthValue()" + ); } public void testSecondOfDay() { - assertThat(javaTime.get(ChronoField.SECOND_OF_DAY), equalTo(jodaTime.getSecondOfDay())); + assertMethodDeprecation( + () -> assertThat(javaTime.getSecondOfDay(), equalTo(jodaTime.getSecondOfDay())), + "getSecondOfDay()", + "get(ChronoField.SECOND_OF_DAY)" + ); } public void testSecondOfMinute() { - assertThat(javaTime.getSecond(), equalTo(jodaTime.getSecondOfMinute())); + assertMethodDeprecation( + () -> assertThat(javaTime.getSecondOfMinute(), equalTo(jodaTime.getSecondOfMinute())), + "getSecondOfMinute()", + "getSecond()" + ); } public void testWeekOfWeekyear() { - assertThat(javaTime.get(DateFormatters.WEEK_FIELDS_ROOT.weekOfWeekBasedYear()), equalTo(jodaTime.getWeekOfWeekyear())); + assertMethodDeprecation( + () -> assertThat(javaTime.getWeekOfWeekyear(), equalTo(jodaTime.getWeekOfWeekyear())), + "getWeekOfWeekyear()", + "get(DateFormatters.WEEK_FIELDS_ROOT.weekOfWeekBasedYear())" + ); } public void testWeekyear() { - assertThat(javaTime.get(DateFormatters.WEEK_FIELDS_ROOT.weekBasedYear()), equalTo(jodaTime.getWeekyear())); + assertMethodDeprecation( + () -> assertThat(javaTime.getWeekyear(), equalTo(jodaTime.getWeekyear())), + "getWeekyear()", + "get(DateFormatters.WEEK_FIELDS_ROOT.weekBasedYear())" + ); } public void testYearOfCentury() { - assertThat(javaTime.get(ChronoField.YEAR_OF_ERA) % 100, equalTo(jodaTime.getYearOfCentury())); + assertMethodDeprecation( + () -> assertThat(javaTime.getYearOfCentury(), equalTo(jodaTime.getYearOfCentury())), + "getYearOfCentury()", + "get(ChronoField.YEAR_OF_ERA) % 100" + ); } public void testYearOfEra() { - assertThat(javaTime.get(ChronoField.YEAR_OF_ERA), equalTo(jodaTime.getYearOfEra())); + assertMethodDeprecation( + () -> assertThat(javaTime.getYearOfEra(), equalTo(jodaTime.getYearOfEra())), + "getYearOfEra()", + "get(ChronoField.YEAR_OF_ERA)" + ); + } + + public void testToString1() { + assertMethodDeprecation( + () -> assertThat(javaTime.toString("YYYY/MM/dd HH:mm:ss.SSS"), equalTo(jodaTime.toString("YYYY/MM/dd HH:mm:ss.SSS"))), + "toString(String)", + "a DateTimeFormatter" + ); } public void testToString2() { - assertThat(DateTimeFormatter.ofPattern("EEE", Locale.GERMANY).format(javaTime), equalTo(jodaTime.toString("EEE", Locale.GERMANY))); + assertMethodDeprecation( + () -> assertThat(javaTime.toString("EEE", Locale.GERMANY), equalTo(jodaTime.toString("EEE", Locale.GERMANY))), + "toString(String,Locale)", + "a DateTimeFormatter" + ); } public void testDayOfWeek() { - assertThat(javaTime.getDayOfWeekEnum().getValue(), equalTo(jodaTime.getDayOfWeek())); + assertDeprecation( + () -> assertThat(javaTime.getDayOfWeek(), equalTo(jodaTime.getDayOfWeek())), + "The return type of [getDayOfWeek()] will change to an enum in 7.0. Use getDayOfWeekEnum().getValue()." + ); } public void testDayOfWeekEnum() { assertThat(javaTime.getDayOfWeekEnum(), equalTo(DayOfWeek.of(jodaTime.getDayOfWeek()))); } + public void testToStringWithLocaleAndZeroOffset() { + JodaCompatibleZonedDateTime dt = new JodaCompatibleZonedDateTime(Instant.EPOCH, ZoneOffset.ofTotalSeconds(0)); + assertMethodDeprecation(() -> dt.toString("yyyy-MM-dd hh:mm", Locale.ROOT), "toString(String,Locale)", "a DateTimeFormatter"); + } + + public void testToStringAndZeroOffset() { + JodaCompatibleZonedDateTime dt = new JodaCompatibleZonedDateTime(Instant.EPOCH, ZoneOffset.ofTotalSeconds(0)); + assertMethodDeprecation(() -> dt.toString("yyyy-MM-dd hh:mm"), "toString(String)", "a DateTimeFormatter"); + } + public void testIsEqual() { assertTrue(javaTime.isEqual(javaTime)); }