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

Make reindex and lang-javascript compatible #19933

Merged
merged 1 commit into from
Aug 11, 2016
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
Original file line number Diff line number Diff line change
Expand Up @@ -450,6 +450,8 @@ public RequestWrapper<?> apply(RequestWrapper<?> request, ScrollableHitSource.Hi
}
if (context == null) {
context = new HashMap<>();
} else {
context.clear();
}

context.put(IndexFieldMapper.NAME, doc.getIndex());
Expand Down Expand Up @@ -485,50 +487,50 @@ public RequestWrapper<?> apply(RequestWrapper<?> request, ScrollableHitSource.Hi
*/
request.setSource((Map<String, Object>) resultCtx.remove(SourceFieldMapper.NAME));

Object newValue = context.remove(IndexFieldMapper.NAME);
Object newValue = resultCtx.remove(IndexFieldMapper.NAME);
if (false == doc.getIndex().equals(newValue)) {
scriptChangedIndex(request, newValue);
}
newValue = context.remove(TypeFieldMapper.NAME);
newValue = resultCtx.remove(TypeFieldMapper.NAME);
if (false == doc.getType().equals(newValue)) {
scriptChangedType(request, newValue);
}
newValue = context.remove(IdFieldMapper.NAME);
newValue = resultCtx.remove(IdFieldMapper.NAME);
if (false == doc.getId().equals(newValue)) {
scriptChangedId(request, newValue);
}
newValue = context.remove(VersionFieldMapper.NAME);
newValue = resultCtx.remove(VersionFieldMapper.NAME);
if (false == Objects.equals(oldVersion, newValue)) {
scriptChangedVersion(request, newValue);
}
newValue = context.remove(ParentFieldMapper.NAME);
newValue = resultCtx.remove(ParentFieldMapper.NAME);
if (false == Objects.equals(oldParent, newValue)) {
scriptChangedParent(request, newValue);
}
/*
* Its important that routing comes after parent in case you want to
* change them both.
*/
newValue = context.remove(RoutingFieldMapper.NAME);
newValue = resultCtx.remove(RoutingFieldMapper.NAME);
if (false == Objects.equals(oldRouting, newValue)) {
scriptChangedRouting(request, newValue);
}
newValue = context.remove(TimestampFieldMapper.NAME);
newValue = resultCtx.remove(TimestampFieldMapper.NAME);
if (false == Objects.equals(oldTimestamp, newValue)) {
scriptChangedTimestamp(request, newValue);
}
newValue = context.remove(TTLFieldMapper.NAME);
newValue = resultCtx.remove(TTLFieldMapper.NAME);
if (false == Objects.equals(oldTTL, newValue)) {
scriptChangedTTL(request, newValue);
}

OpType newOpType = OpType.fromString(newOp);
if (newOpType != oldOpType) {
if (newOpType != oldOpType) {
return scriptChangedOpType(request, oldOpType, newOpType);
}

if (false == context.isEmpty()) {
throw new IllegalArgumentException("Invalid fields added to context [" + String.join(",", context.keySet()) + ']');
if (false == resultCtx.isEmpty()) {
throw new IllegalArgumentException("Invalid fields added to context [" + String.join(",", resultCtx.keySet()) + ']');
}
return request;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,12 @@

import org.elasticsearch.script.ExecutableScript;

import java.util.HashMap;
import java.util.Map;
import java.util.function.Consumer;

import static org.elasticsearch.test.ESTestCase.randomBoolean;

public class SimpleExecutableScript implements ExecutableScript {
private final Consumer<Map<String, Object>> script;
private Map<String, Object> ctx;
Expand All @@ -50,6 +53,13 @@ public void setNextVar(String name, Object value) {

@Override
public Object unwrap(Object value) {
// Some script engines (javascript) copy any maps they unwrap
if (randomBoolean()) {
if (value instanceof Map) {
return new HashMap<>((Map<?, ?>) value);
}
}
// Others just return the objects plain (groovy, painless)
return value;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this makes the executable script not that simple anymore ;)

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -172,14 +172,16 @@ public Object compile(String scriptName, String scriptSource, Map<String, String
}

@Override
public ExecutableScript executable(CompiledScript compiledScript, Map<String, Object> vars) {
public ExecutableScript executable(CompiledScript compiledScript, @Nullable Map<String, Object> vars) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is outside the scope of the PR, but we should look into requiring vars to be non null in the future

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me.

Context ctx = Context.enter();
try {
Scriptable scope = ctx.newObject(globalScope);
scope.setPrototype(globalScope);
scope.setParentScope(null);
for (Map.Entry<String, Object> entry : vars.entrySet()) {
ScriptableObject.putProperty(scope, entry.getKey(), entry.getValue());
if (vars != null) {
for (Map.Entry<String, Object> entry : vars.entrySet()) {
ScriptableObject.putProperty(scope, entry.getKey(), entry.getValue());
}
}

return new JavaScriptExecutableScript((Script) compiledScript.compiled(), scope);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@
import java.util.List;
import java.util.Map;

import static java.util.Collections.emptyMap;

import org.elasticsearch.common.collect.MapBuilder;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.script.CompiledScript;
Expand Down Expand Up @@ -59,6 +61,13 @@ public void testSimpleEquation() {
assertThat(((Number) o).intValue(), equalTo(3));
}

public void testNullVars() {
CompiledScript script = new CompiledScript(ScriptService.ScriptType.INLINE, "testSimpleEquation", "js",
se.compile(null, "1 + 2", emptyMap()));
Object o = se.executable(script, null).run();
assertThat(((Number) o).intValue(), equalTo(3));
}

@SuppressWarnings("unchecked")
public void testMapAccess() {
Map<String, Object> vars = new HashMap<String, Object>();
Expand Down