-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
javax scripting added #237
Conversation
Hi Guys, this should fix everything including broken travis. Can we please merge it? |
Thank you for opening this PR, this is really awesome! However, I'm not sure I will be able to maintain it in the long term 😞 Can you create a repo with this new module and I will add a link to it in the home page here ? This way, all the credit goes to you for creating and maintaining the module. |
Hi Ben, |
Hi Nabarun, I reviewed your PR and had a couple of thoughts:
@Test
public void testJSConditionWithExpressionAndParserContext() {
// given
Bindings context = new SimpleBindings();
context.put("x", 42);
context.put("y", 42);
Condition condition = new JScriptingCondition(DEFAULT_JVM_LINGO,"x == y ", context);
Facts facts = new Facts();
// when
boolean evaluationResult = condition.evaluate(facts);
// then
assertThat(evaluationResult).isTrue();
} In this example, the context is bound to the condition itself, and the condition is evaluated against its own bindings, not against the set of facts. The example should be rather: @Test
public void testJSConditionWithExpressionAndParserContext() {
// given
Facts facts = new Facts();
facts.put("x", 42);
facts.put("y", 42);
Condition condition = new JScriptingCondition(DEFAULT_JVM_LINGO,"x == y ");
// when
boolean evaluationResult = condition.evaluate(facts);
// then
assertThat(evaluationResult).isTrue();
} This is the same for actions. This approach makes rules stateful, which was a drawback of Easy Rules v2 (and fixed in v3, see #37). So there is a design issue with the current approach.
As you know, maintenance is not only about the code. Please don't get me wrong, if you want to join the org, you are welcome! My point is that the burden of support (I get dozens of emails, DMs on twitter, gitter, etc) will still land in my inbox. The effort on releases will still be on my side, etc. To be honest, I believe the best way to use Easy Rules in javascript/typescript/whateverscript is to port it to javascript/typescript/whateverscript, and not trough a bridge. Personally, I would never use Java to call a python script, I would rather use python directly (using Java to call python via jython is just wrong to me, but I could be wrong). That's why I believe the best option for now is that you maintain the module on your side as an extension (or a plugin) which will be very welcome to get a reference on Easy Rules in other languages section of the home page. You are not forking off, you are contributing an extension that you own. I really appreciate your effort and I hope you understand my position. Best regards, |
Hi Ben, this code runs : @Test
public void testJSConditionWithExpressionAndParserContext() {
// given
Facts facts = new Facts();
facts.put("x", 42);
facts.put("y", 42);
Condition condition = new JScriptingCondition(DEFAULT_JVM_LINGO,"x == y ");
// when
boolean evaluationResult = condition.evaluate(facts);
// then
assertThat(evaluationResult).isTrue();
} As is, even now. I probably did not understand what it meant when I was writing the test case but it does not matter, no code change is required to run it - because things are taken care of already. I can fix that test code immediately. Now coming back the NashHorn problem, it is not a problem at all, because in a real app scenario people would drop appropriate expression engine, and will use them instead of JS. Why? Implementation is NOT tied with JS at all, only the test cases are, really. private JScriptingRuleFactory factory = new JScriptingRuleFactory(new YamlRuleDefinitionReader(),DEFAULT_JVM_LINGO); We are supposed to pass it through. Imagine the case of MS-Excel with VBScript macro. That is what makes excel truly powerful. In case of Windows itself -- it is PowerShell that makes it programmable. However it is your call, entirely. |
Hi,
I have added javax.scripting support, which now ensures all jsr-223 languages can be supported by easy-rules, something drools does not have!
Default is set to be Nashorn, JavaScript engine.