Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Scripting: Remove getDate methods from ScriptDocValues #30690

Merged
merged 1 commit into from
May 19, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions docs/reference/migration/migrate_7_0.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ Elasticsearch 6.x in order to be readable by Elasticsearch 7.x.
* <<breaking_70_api_changes>>
* <<breaking_70_java_changes>>
* <<breaking_70_settings_changes>>
* <<breaking_70_scripting_changes>>


include::migrate_7_0/aggregations.asciidoc[]
Expand All @@ -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[]
13 changes: 13 additions & 0 deletions docs/reference/migration/migrate_7_0/scripting.asciidoc
Original file line number Diff line number Diff line change
@@ -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)`.
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,38 +94,19 @@ public final void sort(Comparator<? super T> c) {
}

public static final class Longs extends ScriptDocValues<Long> {
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<String> 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<String> 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++) {
Expand All @@ -134,9 +115,6 @@ public void setNextDocId(int docId) throws IOException {
} else {
resize(0);
}
if (dates != null) {
dates.setNextDocId(docId);
}
}

/**
Expand All @@ -148,37 +126,13 @@ protected void resize(int newSize) {
values = ArrayUtil.grow(values, count);
}

public SortedNumericDocValues getInternalValues() {
return this.in;
}

public long getValue() {
if (count == 0) {
return 0L;
}
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<ReadableDateTime> 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];
Expand All @@ -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<Void>() {
@Override
public Void run() {
deprecationCallback.accept(message);
return null;
}
});
}
}

public static final class Dates extends ScriptDocValues<ReadableDateTime> {
Expand All @@ -212,12 +150,6 @@ public static final class Dates extends ScriptDocValues<ReadableDateTime> {
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<String> 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.
Expand All @@ -229,15 +161,7 @@ public static final class Dates extends ScriptDocValues<ReadableDateTime> {
* Standard constructor.
*/
public Dates(SortedNumericDocValues in) {
this(in, deprecationLogger::deprecated);
}

/**
* Constructor for testing deprecation logging.
*/
Dates(SortedNumericDocValues in, Consumer<String> deprecationCallback) {
this.in = in;
this.deprecationCallback = deprecationCallback;
}

/**
Expand All @@ -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<ReadableDateTime> 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) {
Expand Down Expand Up @@ -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<Void>() {
@Override
public Void run() {
deprecationCallback.accept(message);
return null;
}
});
}
}

public static final class Doubles extends ScriptDocValues<Double> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,19 +50,11 @@ public void test() throws IOException {
values[d][i] = expectedDates[d][i].getMillis();
}
}
Set<String> 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++) {
Expand All @@ -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<Void>() {
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<String> deprecationHandler) {
private Dates wrap(long[][] values) {
return new Dates(new AbstractSortedNumericDocValues() {
long[] current;
int i;
Expand All @@ -117,6 +85,6 @@ public int docValueCount() {
public long nextValue() {
return current[i++];
}
}, deprecationHandler);
});
}
}
Loading