mirror of https://github.com/apache/druid.git
javascript filter result convert to java boolean (#10721)
* javascript filter result convert to java boolean * use type convert replace script convert, and add more unit test Co-authored-by: qinzhen <qinzhen@kuaishou.com>
This commit is contained in:
parent
f66fdbfa5d
commit
c62b7c19c3
|
@ -36,6 +36,7 @@ import org.checkerframework.checker.nullness.qual.EnsuresNonNull;
|
|||
import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
|
||||
import org.mozilla.javascript.Context;
|
||||
import org.mozilla.javascript.Function;
|
||||
import org.mozilla.javascript.ScriptRuntime;
|
||||
import org.mozilla.javascript.ScriptableObject;
|
||||
|
||||
import javax.annotation.Nullable;
|
||||
|
@ -155,7 +156,8 @@ public class JavaScriptDimFilter extends AbstractOptimizableDimFilter implements
|
|||
* script compilation.
|
||||
*/
|
||||
@EnsuresNonNull("predicateFactory")
|
||||
private JavaScriptPredicateFactory getPredicateFactory()
|
||||
@VisibleForTesting
|
||||
JavaScriptPredicateFactory getPredicateFactory()
|
||||
{
|
||||
// JavaScript configuration should be checked when it's actually used because someone might still want Druid
|
||||
// nodes to be able to deserialize JavaScript-based objects even though JavaScript is disabled.
|
||||
|
@ -327,7 +329,14 @@ public class JavaScriptDimFilter extends AbstractOptimizableDimFilter implements
|
|||
if (extractionFn != null) {
|
||||
input = extractionFn.apply(input);
|
||||
}
|
||||
return Context.toBoolean(fnApply.call(cx, scope, scope, new Object[]{input}));
|
||||
Object fnResult = fnApply.call(cx, scope, scope, new Object[]{input});
|
||||
if (fnResult instanceof ScriptableObject) {
|
||||
// Direct return js function result (like arr.includes) will return org.mozilla.javascript.NativeBoolean,
|
||||
// Context.toBoolean always treat it as true, even if it is false. Convert it to java.lang.Boolean first to fix this mistake.
|
||||
return Context.toBoolean(((ScriptableObject) fnResult).getDefaultValue(ScriptRuntime.BooleanClass));
|
||||
} else {
|
||||
return Context.toBoolean(fnResult);
|
||||
}
|
||||
}
|
||||
|
||||
@Override
|
||||
|
|
|
@ -113,4 +113,48 @@ public class JavaScriptDimFilterTest
|
|||
JavaScriptDimFilter javaScriptDimFilter = new JavaScriptDimFilter("dim", FN1, null, new JavaScriptConfig(false));
|
||||
Assert.assertEquals(javaScriptDimFilter.getRequiredColumns(), Sets.newHashSet("dim"));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testPredicateFactoryApplyObject()
|
||||
{
|
||||
// test for return org.mozilla.javascript.NativeBoolean
|
||||
JavaScriptDimFilter javaScriptDimFilter = new JavaScriptDimFilter(
|
||||
"dim",
|
||||
"function(id) { return ['123', '456'].includes(id); }",
|
||||
null,
|
||||
JavaScriptConfig.getEnabledInstance()
|
||||
);
|
||||
Assert.assertTrue(javaScriptDimFilter.getPredicateFactory().applyObject("123"));
|
||||
Assert.assertTrue(javaScriptDimFilter.getPredicateFactory().applyObject("456"));
|
||||
Assert.assertFalse(javaScriptDimFilter.getPredicateFactory().applyObject("789"));
|
||||
|
||||
// test for return java.lang.Boolean
|
||||
JavaScriptDimFilter javaScriptDimFilter1 = new JavaScriptDimFilter(
|
||||
"dim",
|
||||
"function(id) { return ['123', '456'].includes(id) == true; }",
|
||||
null,
|
||||
JavaScriptConfig.getEnabledInstance()
|
||||
);
|
||||
Assert.assertTrue(javaScriptDimFilter1.getPredicateFactory().applyObject("123"));
|
||||
Assert.assertTrue(javaScriptDimFilter1.getPredicateFactory().applyObject("456"));
|
||||
Assert.assertFalse(javaScriptDimFilter1.getPredicateFactory().applyObject("789"));
|
||||
|
||||
// test for return other type
|
||||
JavaScriptDimFilter javaScriptDimFilter2 = new JavaScriptDimFilter(
|
||||
"dim",
|
||||
"function(id) { return 'word'; }",
|
||||
null,
|
||||
JavaScriptConfig.getEnabledInstance()
|
||||
);
|
||||
Assert.assertTrue(javaScriptDimFilter2.getPredicateFactory().applyObject("123"));
|
||||
|
||||
// test for return null
|
||||
JavaScriptDimFilter javaScriptDimFilter3 = new JavaScriptDimFilter(
|
||||
"dim",
|
||||
"function(id) { return null; }",
|
||||
null,
|
||||
JavaScriptConfig.getEnabledInstance()
|
||||
);
|
||||
Assert.assertFalse(javaScriptDimFilter3.getPredicateFactory().applyObject("123"));
|
||||
}
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue