diff --git a/server/src/main/java/com/mirth/connect/server/builders/JavaScriptBuilder.java b/server/src/main/java/com/mirth/connect/server/builders/JavaScriptBuilder.java index 65a34f6d83..d69d844e5f 100644 --- a/server/src/main/java/com/mirth/connect/server/builders/JavaScriptBuilder.java +++ b/server/src/main/java/com/mirth/connect/server/builders/JavaScriptBuilder.java @@ -310,15 +310,23 @@ protected static void appendFilterScript(StringBuilder builder, Filter filter) t builder.append("function doFilter() { phase[0] = 'filter'; return ("); // call each of the above functions in a big boolean expression + boolean firstRule = true; for (ListIterator iter = enabledElements.listIterator(); iter.hasNext();) { Rule rule = iter.next(); String operator = ""; - if (Objects.equals(rule.getOperator(), Rule.Operator.AND)) { - operator = " && "; - } else if (Objects.equals(rule.getOperator(), Rule.Operator.OR)) { - operator = " || "; + // The first enabled rule has nothing to combine with, so its operator is never + // emitted. Otherwise a disabled first step leaves the next enabled rule's AND/OR + // as a leading operator, producing invalid JavaScript (e.g. "return ( && ...)") + // that fails to compile on deploy. See OpenIntegrationEngine/engine#76. + if (!firstRule) { + if (Objects.equals(rule.getOperator(), Rule.Operator.AND)) { + operator = " && "; + } else if (Objects.equals(rule.getOperator(), Rule.Operator.OR)) { + operator = " || "; + } } + firstRule = false; builder.append(operator + "(filterRule" + iter.nextIndex() + "() == true)"); } diff --git a/server/src/test/java/com/mirth/connect/server/builders/JavaScriptBuilderTest.java b/server/src/test/java/com/mirth/connect/server/builders/JavaScriptBuilderTest.java index e5bf729348..de949cf3da 100644 --- a/server/src/test/java/com/mirth/connect/server/builders/JavaScriptBuilderTest.java +++ b/server/src/test/java/com/mirth/connect/server/builders/JavaScriptBuilderTest.java @@ -10,6 +10,8 @@ package com.mirth.connect.server.builders; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; import java.util.ArrayList; import java.util.Arrays; @@ -225,4 +227,27 @@ public void testNestedIteratorsOuterDisabled() throws ScriptBuilderException { assertEquals(scriptEmpty, scriptInnerDisabled); } + + @Test + public void testFilterDisabledFirstRuleNoLeadingOperator() throws ScriptBuilderException { + // A disabled first step followed by an enabled rule whose operator is AND must not emit + // the AND as a leading operator in doFilter(); that would be invalid JavaScript and the + // channel would fail to deploy. See OpenIntegrationEngine/engine#76. + Filter filter = new Filter(); + Rule disabledFirst = new RuleBuilderRule(); + disabledFirst.setEnabled(false); + Rule enabledWithAnd = new RuleBuilderRule(); + enabledWithAnd.setOperator(Rule.Operator.AND); + Rule enabledWithAnd2 = new RuleBuilderRule(); + enabledWithAnd2.setOperator(Rule.Operator.AND); + filter.setElements(new ArrayList<>(Arrays.asList(disabledFirst, enabledWithAnd, enabledWithAnd2))); + + StringBuilder builder = new StringBuilder(); + JavaScriptBuilder.appendFilterScript(builder, filter); + String script = builder.toString(); + + assertFalse("doFilter() must not start with a leading operator: " + script, + script.contains("return ( &&") || script.contains("return ( ||")); + assertTrue(script.contains("return ((filterRule1() == true)")); + } }