From 34180f2285268fe74ef13f14de3f5442f5ccd624 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Fri, 18 May 2018 21:26:26 -0700 Subject: [PATCH] Scripting: Remove getDate methods from ScriptDocValues (#30690) The getDate() and getDates() existed prior to 5.x on long fields in scripting. In 5.x, a new Date type for ScriptDocValues was added. The getDate() and getDates() methods were left on long fields and added to date fields to ease the transition. This commit removes those methods for 7.0. --- docs/reference/migration/migrate_7_0.asciidoc | 2 + .../migration/migrate_7_0/scripting.asciidoc | 13 +++ .../painless/spi/org.elasticsearch.txt | 4 - .../test/painless/50_script_doc_values.yml | 44 ------- .../index/fielddata/ScriptDocValues.java | 110 ------------------ .../fielddata/ScriptDocValuesDatesTests.java | 38 +----- .../fielddata/ScriptDocValuesLongsTests.java | 68 +---------- 7 files changed, 21 insertions(+), 258 deletions(-) create mode 100644 docs/reference/migration/migrate_7_0/scripting.asciidoc diff --git a/docs/reference/migration/migrate_7_0.asciidoc b/docs/reference/migration/migrate_7_0.asciidoc index aea6d14fac9c8..c68dee287d5a5 100644 --- a/docs/reference/migration/migrate_7_0.asciidoc +++ b/docs/reference/migration/migrate_7_0.asciidoc @@ -34,6 +34,7 @@ Elasticsearch 6.x in order to be readable by Elasticsearch 7.x. * <> * <> * <> +* <> include::migrate_7_0/aggregations.asciidoc[] @@ -47,3 +48,4 @@ include::migrate_7_0/plugins.asciidoc[] include::migrate_7_0/api.asciidoc[] include::migrate_7_0/java.asciidoc[] include::migrate_7_0/settings.asciidoc[] +include::migrate_7_0/scripting.asciidoc[] diff --git a/docs/reference/migration/migrate_7_0/scripting.asciidoc b/docs/reference/migration/migrate_7_0/scripting.asciidoc new file mode 100644 index 0000000000000..df43aaa92eadf --- /dev/null +++ b/docs/reference/migration/migrate_7_0/scripting.asciidoc @@ -0,0 +1,13 @@ +[[breaking_70_scripting_changes]] +=== Scripting changes + +==== getDate() and getDates() removed + +Fields of type `long` and `date` had `getDate()` and `getDates()` methods +(for multi valued fields) to get an object with date specific helper methods +for the current doc value. In 5.3.0, `date` fields were changed to expose +this same date object directly when calling `doc["myfield"].value`, and +the getter methods for date objects were deprecated. These methods have +now been removed. Instead, use `.value` on `date` fields, or explicitly +parse `long` fields into a date object using +`Instance.ofEpochMillis(doc["myfield"].value)`. diff --git a/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/org.elasticsearch.txt b/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/org.elasticsearch.txt index 51a1b7cecb3f8..6495659d9cdc0 100644 --- a/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/org.elasticsearch.txt +++ b/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/org.elasticsearch.txt @@ -74,16 +74,12 @@ class org.elasticsearch.index.fielddata.ScriptDocValues$Longs { Long get(int) long getValue() List getValues() - org.joda.time.ReadableDateTime getDate() - List getDates() } class org.elasticsearch.index.fielddata.ScriptDocValues$Dates { org.joda.time.ReadableDateTime get(int) org.joda.time.ReadableDateTime getValue() List getValues() - org.joda.time.ReadableDateTime getDate() - List getDates() } class org.elasticsearch.index.fielddata.ScriptDocValues$Doubles { diff --git a/modules/lang-painless/src/test/resources/rest-api-spec/test/painless/50_script_doc_values.yml b/modules/lang-painless/src/test/resources/rest-api-spec/test/painless/50_script_doc_values.yml index ede2927b992e0..617b8df61b6bd 100644 --- a/modules/lang-painless/src/test/resources/rest-api-spec/test/painless/50_script_doc_values.yml +++ b/modules/lang-painless/src/test/resources/rest-api-spec/test/painless/50_script_doc_values.yml @@ -106,28 +106,6 @@ setup: source: "doc.date.value" - match: { hits.hits.0.fields.field.0: '2017-01-01T12:11:12.000Z' } - - do: - warnings: - - getDate is no longer necessary on date fields as the value is now a date. - search: - body: - script_fields: - field: - script: - source: "doc['date'].date" - - match: { hits.hits.0.fields.field.0: '2017-01-01T12:11:12.000Z' } - - - do: - warnings: - - getDates is no longer necessary on date fields as the values are now dates. - search: - body: - script_fields: - field: - script: - source: "doc['date'].dates.get(0)" - - match: { hits.hits.0.fields.field.0: '2017-01-01T12:11:12.000Z' } - --- "geo_point": - do: @@ -213,28 +191,6 @@ setup: source: "doc['long'].value" - match: { hits.hits.0.fields.field.0: 12348732141234 } - - do: - warnings: - - getDate on numeric fields is deprecated. Use a date field to get dates. - search: - body: - script_fields: - field: - script: - source: "doc['long'].date" - - match: { hits.hits.0.fields.field.0: '2361-04-26T03:22:21.234Z' } - - - do: - warnings: - - getDates on numeric fields is deprecated. Use a date field to get dates. - search: - body: - script_fields: - field: - script: - source: "doc['long'].dates.get(0)" - - match: { hits.hits.0.fields.field.0: '2361-04-26T03:22:21.234Z' } - --- "integer": - do: diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/ScriptDocValues.java b/server/src/main/java/org/elasticsearch/index/fielddata/ScriptDocValues.java index 3d07a0f87aa5e..dd85e921e4ac5 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/ScriptDocValues.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/ScriptDocValues.java @@ -94,38 +94,19 @@ public final void sort(Comparator c) { } public static final class Longs extends ScriptDocValues { - protected static final DeprecationLogger deprecationLogger = new DeprecationLogger(ESLoggerFactory.getLogger(Longs.class)); - private final SortedNumericDocValues in; - /** - * Callback for deprecated fields. In production this should always point to - * {@link #deprecationLogger} but tests will override it so they can test that - * we use the required permissions when calling it. - */ - private final Consumer deprecationCallback; private long[] values = new long[0]; private int count; - private Dates dates; - private int docId = -1; /** * Standard constructor. */ public Longs(SortedNumericDocValues in) { - this(in, deprecationLogger::deprecated); - } - - /** - * Constructor for testing the deprecation callback. - */ - Longs(SortedNumericDocValues in, Consumer deprecationCallback) { this.in = in; - this.deprecationCallback = deprecationCallback; } @Override public void setNextDocId(int docId) throws IOException { - this.docId = docId; if (in.advanceExact(docId)) { resize(in.docValueCount()); for (int i = 0; i < count; i++) { @@ -134,9 +115,6 @@ public void setNextDocId(int docId) throws IOException { } else { resize(0); } - if (dates != null) { - dates.setNextDocId(docId); - } } /** @@ -148,10 +126,6 @@ protected void resize(int newSize) { values = ArrayUtil.grow(values, count); } - public SortedNumericDocValues getInternalValues() { - return this.in; - } - public long getValue() { if (count == 0) { return 0L; @@ -159,26 +133,6 @@ public long getValue() { return values[0]; } - @Deprecated - public ReadableDateTime getDate() throws IOException { - deprecated("getDate on numeric fields is deprecated. Use a date field to get dates."); - if (dates == null) { - dates = new Dates(in); - dates.setNextDocId(docId); - } - return dates.getValue(); - } - - @Deprecated - public List getDates() throws IOException { - deprecated("getDates on numeric fields is deprecated. Use a date field to get dates."); - if (dates == null) { - dates = new Dates(in); - dates.setNextDocId(docId); - } - return dates; - } - @Override public Long get(int index) { return values[index]; @@ -188,22 +142,6 @@ public Long get(int index) { public int size() { return count; } - - /** - * Log a deprecation log, with the server's permissions, not the permissions of the - * script calling this method. We need to do this to prevent errors when rolling - * the log file. - */ - private void deprecated(String message) { - // Intentionally not calling SpecialPermission.check because this is supposed to be called by scripts - AccessController.doPrivileged(new PrivilegedAction() { - @Override - public Void run() { - deprecationCallback.accept(message); - return null; - } - }); - } } public static final class Dates extends ScriptDocValues { @@ -212,12 +150,6 @@ public static final class Dates extends ScriptDocValues { private static final ReadableDateTime EPOCH = new DateTime(0, DateTimeZone.UTC); private final SortedNumericDocValues in; - /** - * Callback for deprecated fields. In production this should always point to - * {@link #deprecationLogger} but tests will override it so they can test that - * we use the required permissions when calling it. - */ - private final Consumer deprecationCallback; /** * Values wrapped in {@link MutableDateTime}. Null by default an allocated on first usage so we allocate a reasonably size. We keep * this array so we don't have allocate new {@link MutableDateTime}s on every usage. Instead we reuse them for every document. @@ -229,15 +161,7 @@ public static final class Dates extends ScriptDocValues { * Standard constructor. */ public Dates(SortedNumericDocValues in) { - this(in, deprecationLogger::deprecated); - } - - /** - * Constructor for testing deprecation logging. - */ - Dates(SortedNumericDocValues in, Consumer deprecationCallback) { this.in = in; - this.deprecationCallback = deprecationCallback; } /** @@ -251,24 +175,6 @@ public ReadableDateTime getValue() { return get(0); } - /** - * Fetch the first value. Added for backwards compatibility with 5.x when date fields were {@link Longs}. - */ - @Deprecated - public ReadableDateTime getDate() { - deprecated("getDate is no longer necessary on date fields as the value is now a date."); - return getValue(); - } - - /** - * Fetch all the values. Added for backwards compatibility with 5.x when date fields were {@link Longs}. - */ - @Deprecated - public List getDates() { - deprecated("getDates is no longer necessary on date fields as the values are now dates."); - return this; - } - @Override public ReadableDateTime get(int index) { if (index >= count) { @@ -326,22 +232,6 @@ void refreshArray() throws IOException { dates[i] = new MutableDateTime(in.nextValue(), DateTimeZone.UTC); } } - - /** - * Log a deprecation log, with the server's permissions, not the permissions of the - * script calling this method. We need to do this to prevent errors when rolling - * the log file. - */ - private void deprecated(String message) { - // Intentionally not calling SpecialPermission.check because this is supposed to be called by scripts - AccessController.doPrivileged(new PrivilegedAction() { - @Override - public Void run() { - deprecationCallback.accept(message); - return null; - } - }); - } } public static final class Doubles extends ScriptDocValues { diff --git a/server/src/test/java/org/elasticsearch/index/fielddata/ScriptDocValuesDatesTests.java b/server/src/test/java/org/elasticsearch/index/fielddata/ScriptDocValuesDatesTests.java index 7a0a6816a66bf..12eb69bef39d8 100644 --- a/server/src/test/java/org/elasticsearch/index/fielddata/ScriptDocValuesDatesTests.java +++ b/server/src/test/java/org/elasticsearch/index/fielddata/ScriptDocValuesDatesTests.java @@ -50,19 +50,11 @@ public void test() throws IOException { values[d][i] = expectedDates[d][i].getMillis(); } } - Set warnings = new HashSet<>(); - Dates dates = wrap(values, deprecationMessage -> { - warnings.add(deprecationMessage); - /* Create a temporary directory to prove we are running with the - * server's permissions. */ - createTempDir(); - }); - + Dates dates = wrap(values); for (int round = 0; round < 10; round++) { int d = between(0, values.length - 1); dates.setNextDocId(d); assertEquals(expectedDates[d].length > 0 ? expectedDates[d][0] : new DateTime(0, DateTimeZone.UTC), dates.getValue()); - assertEquals(expectedDates[d].length > 0 ? expectedDates[d][0] : new DateTime(0, DateTimeZone.UTC), dates.getDate()); assertEquals(values[d].length, dates.size()); for (int i = 0; i < values[d].length; i++) { @@ -72,33 +64,9 @@ public void test() throws IOException { Exception e = expectThrows(UnsupportedOperationException.class, () -> dates.add(new DateTime())); assertEquals("doc values are unmodifiable", e.getMessage()); } - - /* - * Invoke getDates without any privileges to verify that - * it still works without any. In particularly, this - * verifies that the callback that we've configured - * above works. That callback creates a temporary - * directory which is not possible with "noPermissions". - */ - PermissionCollection noPermissions = new Permissions(); - AccessControlContext noPermissionsAcc = new AccessControlContext( - new ProtectionDomain[] { - new ProtectionDomain(null, noPermissions) - } - ); - AccessController.doPrivileged(new PrivilegedAction() { - public Void run() { - dates.getDates(); - return null; - } - }, noPermissionsAcc); - - assertThat(warnings, containsInAnyOrder( - "getDate is no longer necessary on date fields as the value is now a date.", - "getDates is no longer necessary on date fields as the values are now dates.")); } - private Dates wrap(long[][] values, Consumer deprecationHandler) { + private Dates wrap(long[][] values) { return new Dates(new AbstractSortedNumericDocValues() { long[] current; int i; @@ -117,6 +85,6 @@ public int docValueCount() { public long nextValue() { return current[i++]; } - }, deprecationHandler); + }); } } diff --git a/server/src/test/java/org/elasticsearch/index/fielddata/ScriptDocValuesLongsTests.java b/server/src/test/java/org/elasticsearch/index/fielddata/ScriptDocValuesLongsTests.java index 8b20e9a9f3a71..5fd33da27e399 100644 --- a/server/src/test/java/org/elasticsearch/index/fielddata/ScriptDocValuesLongsTests.java +++ b/server/src/test/java/org/elasticsearch/index/fielddata/ScriptDocValuesLongsTests.java @@ -47,7 +47,7 @@ public void testLongs() throws IOException { values[d][i] = randomLong(); } } - Longs longs = wrap(values, deprecationMessage -> {fail("unexpected deprecation: " + deprecationMessage);}); + Longs longs = wrap(values); for (int round = 0; round < 10; round++) { int d = between(0, values.length - 1); @@ -66,69 +66,7 @@ public void testLongs() throws IOException { } } - public void testDates() throws IOException { - long[][] values = new long[between(3, 10)][]; - ReadableDateTime[][] dates = new ReadableDateTime[values.length][]; - for (int d = 0; d < values.length; d++) { - values[d] = new long[randomBoolean() ? randomBoolean() ? 0 : 1 : between(2, 100)]; - dates[d] = new ReadableDateTime[values[d].length]; - for (int i = 0; i < values[d].length; i++) { - dates[d][i] = new DateTime(randomNonNegativeLong(), DateTimeZone.UTC); - values[d][i] = dates[d][i].getMillis(); - } - } - Set warnings = new HashSet<>(); - Longs longs = wrap(values, deprecationMessage -> { - warnings.add(deprecationMessage); - /* Create a temporary directory to prove we are running with the - * server's permissions. */ - createTempDir(); - }); - - for (int round = 0; round < 10; round++) { - int d = between(0, values.length - 1); - longs.setNextDocId(d); - assertEquals(dates[d].length > 0 ? dates[d][0] : new DateTime(0, DateTimeZone.UTC), longs.getDate()); - - assertEquals(values[d].length, longs.getDates().size()); - for (int i = 0; i < values[d].length; i++) { - assertEquals(dates[d][i], longs.getDates().get(i)); - } - - Exception e = expectThrows(UnsupportedOperationException.class, () -> longs.getDates().add(new DateTime())); - assertEquals("doc values are unmodifiable", e.getMessage()); - } - - /* - * Invoke getDates without any privileges to verify that - * it still works without any. In particularly, this - * verifies that the callback that we've configured - * above works. That callback creates a temporary - * directory which is not possible with "noPermissions". - */ - PermissionCollection noPermissions = new Permissions(); - AccessControlContext noPermissionsAcc = new AccessControlContext( - new ProtectionDomain[] { - new ProtectionDomain(null, noPermissions) - } - ); - AccessController.doPrivileged(new PrivilegedAction() { - public Void run() { - try { - longs.getDates(); - } catch (IOException e) { - throw new RuntimeException("unexpected", e); - } - return null; - } - }, noPermissionsAcc); - - assertThat(warnings, containsInAnyOrder( - "getDate on numeric fields is deprecated. Use a date field to get dates.", - "getDates on numeric fields is deprecated. Use a date field to get dates.")); - } - - private Longs wrap(long[][] values, Consumer deprecationCallback) { + private Longs wrap(long[][] values) { return new Longs(new AbstractSortedNumericDocValues() { long[] current; int i; @@ -147,6 +85,6 @@ public int docValueCount() { public long nextValue() { return current[i++]; } - }, deprecationCallback); + }); } }