Skip to content

Commit

Permalink
Merge pull request #22181 from mkouba/issue-21857
Browse files Browse the repository at this point in the history
Qute If section - fix evaluation of composite params with a logical complement operator
  • Loading branch information
gsmet committed Dec 14, 2021
2 parents 5059047 + 9710961 commit 22ddc60
Show file tree
Hide file tree
Showing 2 changed files with 112 additions and 55 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
import java.util.Objects;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.CompletionStage;
import java.util.stream.Collectors;

/**
* Basic {@code if} statement.
Expand Down Expand Up @@ -422,18 +421,77 @@ static BigDecimal getDecimal(Object value) {

static List<Object> parseParams(List<Object> params, ParserDelegate parserDelegate) {

int highestPrecedence = 0;
// Replace operators and composite params if needed
replaceOperatorsAndCompositeParams(params, parserDelegate);
int highestPrecedence = getHighestPrecedence(params);

if (!isGroupingNeeded(params)) {
// No operators or all of the same precedence
return params;
}

// Take the operators with highest precedence and form groups
// For example "user.active && target.status == NEW && !target.voted" becomes "user.active && [target.status == NEW] && [!target.voted]"
// The algorithm used is not very robust and should be improved later
List<Object> highestGroup = null;
List<Object> ret = new ArrayList<>();
int lastGroupdIdx = 0;

for (ListIterator<Object> iterator = params.listIterator(); iterator.hasNext();) {
int prevIdx = iterator.previousIndex();
Object param = iterator.next();
if (param instanceof Operator) {
Operator op = (Operator) param;
if (op.precedence == highestPrecedence) {
if (highestGroup == null) {
highestGroup = new ArrayList<>();
if (op.isBinary()) {
highestGroup.add(params.get(prevIdx));
}
}
highestGroup.add(param);
// Add non-grouped elements
if (prevIdx > lastGroupdIdx) {
int from = lastGroupdIdx > 0 ? lastGroupdIdx + 1 : 0;
int to = op.isBinary() ? prevIdx : prevIdx + 1;
params.subList(from, to).forEach(ret::add);
}
} else if (op.precedence < highestPrecedence) {
if (highestGroup != null) {
ret.add(highestGroup);
lastGroupdIdx = prevIdx;
highestGroup = null;
}
} else {
throw new IllegalStateException();
}
} else if (highestGroup != null) {
highestGroup.add(param);
}
}
if (highestGroup != null) {
ret.add(highestGroup);
} else {
// Add all remaining non-grouped elements
if (lastGroupdIdx + 1 != params.size()) {
params.subList(lastGroupdIdx + 1, params.size()).forEach(ret::add);
}
}
return parseParams(ret, parserDelegate);
}

private static boolean isGroupingNeeded(List<Object> params) {
// No operators or all of the same precedence
return params.stream().filter(p -> (p instanceof Operator)).map(p -> ((Operator) p).getPrecedence()).distinct()
.count() > 1;
}

private static void replaceOperatorsAndCompositeParams(List<Object> params, ParserDelegate parserDelegate) {
for (ListIterator<Object> iterator = params.listIterator(); iterator.hasNext();) {
Object param = iterator.next();
if (param instanceof String) {
String stringParam = param.toString();
Operator operator = Operator.from(stringParam);
if (operator != null) {
// Binary operator
if (operator.getPrecedence() > highestPrecedence) {
highestPrecedence = operator.getPrecedence();
}
if (operator.isBinary() && !iterator.hasNext()) {
throw parserDelegate.createParserError(
"binary operator [" + operator + "] set but the second operand not present for {#if} section");
Expand All @@ -457,50 +515,19 @@ static List<Object> parseParams(List<Object> params, ParserDelegate parserDelega
}
}
}
}

if (params.stream().filter(p -> p instanceof Operator).map(p -> ((Operator) p).getPrecedence())
.collect(Collectors.toSet()).size() <= 1) {
// No binary operators or all of the same precedence
return params;
}

// Take the operators with highest precedence and form groups
List<Object> highestGroup = null;
List<Object> ret = new ArrayList<>();
int lastGroupdIdx = 0;

for (ListIterator<Object> iterator = params.listIterator(); iterator.hasNext();) {
int prevIdx = iterator.previousIndex();
Object param = iterator.next();
if (isBinaryOperatorEq(param, highestPrecedence)) {
if (highestGroup == null) {
highestGroup = new ArrayList<>();
highestGroup.add(params.get(prevIdx));
}
highestGroup.add(param);
// Add non-grouped elements
if (prevIdx > lastGroupdIdx) {
params.subList(lastGroupdIdx > 0 ? lastGroupdIdx + 1 : 0, prevIdx).forEach(ret::add);
}
} else if (isBinaryOperatorLt(param, highestPrecedence)) {
if (highestGroup != null) {
ret.add(highestGroup);
lastGroupdIdx = prevIdx;
highestGroup = null;
private static int getHighestPrecedence(List<Object> params) {
int highestPrecedence = 0;
for (Object param : params) {
if (param instanceof Operator) {
Operator op = (Operator) param;
if (op.precedence > highestPrecedence) {
highestPrecedence = op.precedence;
}
} else if (highestGroup != null) {
highestGroup.add(param);
}
}
if (highestGroup != null) {
ret.add(highestGroup);
} else {
// Add all remaining non-grouped elements
if (lastGroupdIdx + 1 != params.size()) {
params.subList(lastGroupdIdx + 1, params.size()).forEach(ret::add);
}
}
return parseParams(ret, parserDelegate);
return highestPrecedence;
}

static List<Object> processCompositeParam(String stringParam, ParserDelegate parserDelegate) {
Expand All @@ -514,14 +541,6 @@ static List<Object> processCompositeParam(String stringParam, ParserDelegate par
return parseParams(split, parserDelegate);
}

private static boolean isBinaryOperatorEq(Object val, int precedence) {
return val instanceof Operator && ((Operator) val).getPrecedence() == precedence;
}

private static boolean isBinaryOperatorLt(Object val, int precedence) {
return val instanceof Operator && ((Operator) val).getPrecedence() < precedence;
}

@SuppressWarnings("unchecked")
static Condition createCondition(Object param, SectionBlock block, Operator operator, SectionInitContext context) {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,12 @@ public void testNestedIf() {
public void testCompositeParameters() {
Engine engine = Engine.builder().addDefaults().build();
assertEquals("OK", engine.parse("{#if (true || false) && true}OK{/if}").render());
assertEquals("OK", engine.parse("{#if (true || false) && true && !false}OK{/if}").render());
assertEquals("OK", engine.parse("{#if true && true && !(true || false)}NOK{#else}OK{/if}").render());
assertEquals("OK", engine.parse("{#if true && true && !(true && false)}OK{#else}NOK{/if}").render());
assertEquals("OK", engine.parse("{#if true && !true && (true || false)}NOK{#else}OK{/if}").render());
assertEquals("OK", engine.parse("{#if true && (true && ( true && false))}NOK{#else}OK{/if}").render());
assertEquals("OK", engine.parse("{#if true && (!false || false || (true || false))}OK{#else}NOK{/if}").render());
assertEquals("OK", engine.parse("{#if (foo.or(false) || false || true) && (true)}OK{/if}").render());
assertEquals("NOK", engine.parse("{#if foo.or(false) || false}OK{#else}NOK{/if}").render());
assertEquals("OK", engine.parse("{#if false || (foo.or(false) || (false || true))}OK{#else}NOK{/if}").render());
Expand Down Expand Up @@ -222,6 +228,38 @@ public void testSafeExpression() {
assertEquals("OK", engine.parse("{#if hero??}OK{#else}NOK{/if}").data("hero", true).render());
}

@Test
public void testFromageCondition() {
Engine engine = Engine.builder().addDefaults().addValueResolver(new ReflectionValueResolver())
.addNamespaceResolver(NamespaceResolver.builder("ContentStatus").resolve(ec -> ContentStatus.NEW).build())
.build();
assertEquals("OK",
engine.parse("{#if user && target.status == ContentStatus:NEW && !target.voted(user)}NOK{#else}OK{/if}")
.data("user", "Stef", "target", new Target(ContentStatus.ACCEPTED)).render());
assertEquals("OK",
engine.parse("{#if user && target.status == ContentStatus:NEW && !target.voted(user)}OK{#else}NOK{/if}")
.data("user", "Stef", "target", new Target(ContentStatus.NEW)).render());
}

public static class Target {

public ContentStatus status;

public Target(ContentStatus status) {
this.status = status;
}

public boolean voted(String user) {
return false;
}

}

public enum ContentStatus {
NEW,
ACCEPTED
}

private void assertParserError(String template, String message, int line) {
Engine engine = Engine.builder().addDefaultSectionHelpers().build();
try {
Expand Down

0 comments on commit 22ddc60

Please sign in to comment.