diff options
author | Tomas Ljunggren <tomas.ljunggren@tieto.com> | 2012-01-24 08:24:00 +0100 |
---|---|---|
committer | Tomas Ljunggren <tomas.ljunggren@tieto.com> | 2012-01-24 08:44:27 +0100 |
commit | 1482e279a2dbf59320c1482a537fcc87b7eabad7 (patch) | |
tree | 43a0fb8e78a8bd8e21fcd5ed4e7aba02dbc052de | |
parent | 1ea397ea80dbaf52c7a6853cde1027bb02de19c1 (diff) | |
parent | f1d4b8b86b8914689fee6b145c71290bd01677ca (diff) |
Integration release of NQt GerritRC-V2.2.1-INT-008BL-QTQAINFRA-462BL-QTQAINFRA-382
Fixed JIRA issues:
167 Fixed gerrit email verification link requires insecure login
220 Added a check to prevent NumberFormatException
270 Fixed review database update from StagingApprove
335 Fixed automatic update of reviewer list
347 Fixed late removal of review approvals
348 Fixed cherry pick footer settings
350 Consider sanity review column
352 Fix typo in project config's topic review checkbox label
355 Hide review panel also on diff and topic pages
372 Validate topic current change set
375 Topic permalink copy to clipboard corrected
381 Do not permit trailing slash when pushing
385 Keep highlight on review request
389 Set patch approval changeOpen to false
Change-Id: I33f254b32bd01804e0830fcf5039b8f08e36f702
3 files changed, 435 insertions, 2 deletions
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PublishComments.java b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PublishComments.java index 99c11dea3c..ce2e796d4d 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PublishComments.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PublishComments.java @@ -27,6 +27,7 @@ import com.google.gerrit.reviewdb.PatchSetApproval; import com.google.gerrit.reviewdb.ReviewDb; import com.google.gerrit.server.ChangeUtil; import com.google.gerrit.server.IdentifiedUser; +import com.google.gerrit.server.config.GerritServerConfig; import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.MergeOp; import com.google.gerrit.server.git.MergeQueue; @@ -36,17 +37,20 @@ import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.project.InvalidChangeOperationException; import com.google.gerrit.server.project.NoSuchChangeException; import com.google.gerrit.server.project.NoSuchRefException; +import com.google.gerrit.server.util.BooleanExpression; import com.google.gerrit.server.workflow.FunctionState; import com.google.gwtjsonrpc.client.VoidResult; import com.google.gwtorm.client.OrmException; import com.google.inject.Inject; import com.google.inject.assistedinject.Assisted; +import org.eclipse.jgit.lib.Config; import org.eclipse.jgit.lib.Repository; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import java.io.IOException; +import java.text.ParseException; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; @@ -78,6 +82,8 @@ public class PublishComments implements Callable<VoidResult> { private final MergeOp.Factory mergeFactory; private final MergeQueue merger; + private final Config config; + private final PatchSet.Id patchSetId; private final String messageText; private final Set<ApprovalCategoryValue.Id> approvals; @@ -98,7 +104,7 @@ public class PublishComments implements Callable<VoidResult> { final GitRepositoryManager gitManager, final MergeOp.Factory mergeFactory, final MergeQueue merger, - + @GerritServerConfig final Config config, @Assisted final PatchSet.Id patchSetId, @Assisted final String messageText, @Assisted final Set<ApprovalCategoryValue.Id> approvals) { @@ -113,7 +119,7 @@ public class PublishComments implements Callable<VoidResult> { this.gitManager = gitManager; this.mergeFactory = mergeFactory; this.merger = merger; - + this.config = config; this.patchSetId = patchSetId; this.messageText = messageText; this.approvals = approvals; @@ -307,6 +313,21 @@ public class PublishComments implements Callable<VoidResult> { private void email() { try { + String f = config.getString("review", null, "filter"); + if (f != null) { + // Only care about filter if available in configuration + BooleanExpression ef = new BooleanExpression(f); + HashMap<String, String> am = new HashMap<String, String>(); + for (ApprovalCategoryValue.Id a : approvals) { + am.put(a.getParentKey().get(), Short.toString(a.get())); + } + am.put("reviewer", user.getUserName()); + if (!ef.evaluate(am)) { + // If filter expression evaluates to false + // no email will be sent + return; + } + } final CommentSender cm = commentSenderFactory.create(change); cm.setFrom(user.getAccountId()); cm.setPatchSet(patchSet, patchSetInfoFactory.get(patchSetId)); @@ -317,6 +338,8 @@ public class PublishComments implements Callable<VoidResult> { log.error("Cannot send comments by email for patch set " + patchSetId, e); } catch (PatchSetInfoNotAvailableException e) { log.error("Failed to obtain PatchSetInfo for patch set " + patchSetId, e); + } catch (ParseException e) { + log.error("Failed to parse filter expression from configuration", e); } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/util/BooleanExpression.java b/gerrit-server/src/main/java/com/google/gerrit/server/util/BooleanExpression.java new file mode 100644 index 0000000000..8b986cdb3c --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/util/BooleanExpression.java @@ -0,0 +1,368 @@ +package com.google.gerrit.server.util; + +import java.text.ParseException; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; + +public class BooleanExpression { + + private enum Token { + IDENTIFIER, NUMBER, TRUE, FALSE, LEFT_PARENTHESIS, RIGHT_PARENTHESIS, LOGICAL_AND, LOGICAL_OR, GREATER_THAN, GREATER_THAN_OR_EQUAL_TO, EQUAL_TO, NOT_EQUAL_TO, LESS_THAN, LESS_THAN_OR_EQUAL_TO, UNKNOWN, END_OF_INPUT, MINUS + } + + private static final int O_IDENTIFIER = -1; + private static final int O_NUMBER = -2; + private static final int O_TRUE = -3; + private static final int O_FALSE = -4; + private static final int O_AND = -5; + private static final int O_OR = -6; + private static final int O_EQUAL = -7; + private static final int O_GT = -8; + private static final int O_LT = -9; + private static final int O_LTOREQUAL = -10; + private static final int O_GTOREQUAL = -11; + private static final int O_NOTEQUAL = -12; + private static final int O_NONE = -1000; + + private List<Integer> program = new ArrayList<Integer>(); + private List<String> identifiers = new ArrayList<String>(); + private List<Integer> numbers = new ArrayList<Integer>(); + + private String input; + private int position; + private String value; + + private Token currentToken; + + /** + * Constructor. + * + * @param input Filter expression + */ + public BooleanExpression(String input) throws ParseException { + this.input = input; + position = 0; + nextToken(); + parseExpression(); + } + + public boolean evaluate(Map<String, String> arguments) { + String[] stack = new String[16]; + int ppos = 0; + int spos = 0; + while (ppos < program.size()) { + int o = program.get(ppos); + if (o == O_IDENTIFIER) { + stack[spos++] = identifiers.get(program.get(++ppos)); + } else if (o == O_NUMBER) { + stack[spos++] = Integer.toString(numbers.get(program.get(++ppos))); + } else if (isLogicalOperator(o)) { + String b = stack[--spos]; + String a = stack[--spos]; + if (!isBooleanLiteral(a)) { + throw new IllegalArgumentException(a + " is not a boolean expression"); + } + if (!isBooleanLiteral(b)) { + throw new IllegalArgumentException(b + " is not a boolean expression"); + } + if (o == O_AND) { + stack[spos++] = logicalAnd(a, b); + } else if (o == O_OR) { + stack[spos++] = logicalOr(a, b); + } + } else if (isComparatorOperator(o)) { + String b = stack[--spos]; + String a = stack[--spos]; + if (arguments != null && arguments.get(a) != null) { + a = arguments.get(a); + } + if (arguments != null && arguments.get(b) != null) { + b = arguments.get(b); + } + if (o == O_EQUAL) { + stack[spos++] = Boolean.toString(a.equals(b)); + } else if (o == O_NOTEQUAL) { + stack[spos++] = Boolean.toString(!a.equals(b)); + } else { + // Only integer numbers can be compared + if (!isNumber(a) || !isNumber(b)) { + stack[spos++] = "false"; + } else { + int ai = Integer.parseInt(a); + int bi = Integer.parseInt(b); + if (o == O_GT) { + stack[spos++] = Boolean.toString(ai > bi); + } else if (o == O_GTOREQUAL) { + stack[spos++] = Boolean.toString(ai >= bi); + } else if (o == O_LT) { + stack[spos++] = Boolean.toString(ai < bi); + } else if (o == O_LTOREQUAL) { + stack[spos++] = Boolean.toString(ai <= bi); + } + } + } + } else if (o == O_TRUE) { + stack[spos++] = "true"; + } else if (o == O_FALSE) { + stack[spos++] = "false"; + } + ppos++; + } + + return "true".equals(stack[spos - 1]); + } + + private boolean isBooleanLiteral(String value) { + if ("true".equals(value) || "false".equals(value)) { + return true; + } + return false; + } + + private boolean isNumber(String value) { + try { + Integer.parseInt(value); + return true; + } catch (NumberFormatException e) { + return false; + } + } + + private String logicalAnd(String a, String b) { + if ("true".equals(a) && "true".equals(b)) { + return "true"; + } + return "false"; + } + + private String logicalOr(String a, String b) { + if ("true".equals(a) || "true".equals(b)) { + return "true"; + } + return "false"; + } + + private boolean isLogicalOperator(int o) { + return (o == O_AND || o == O_OR); + } + + private boolean isComparatorOperator(int o) { + return (o == O_EQUAL || o == O_NOTEQUAL || o == O_GT || o == O_GTOREQUAL || o == O_LT || o == O_LTOREQUAL); + } + + private void nextToken() { + currentToken = Token.END_OF_INPUT; + if (position == input.length()) { + return; + } + char c = input.charAt(position); + while (Character.isWhitespace(c)) { + position++; + if (position == input.length()) { + return; + } + c = input.charAt(position); + } + if (Character.isLetter(c)) { + parseIdentifier(); + } else if (Character.isDigit(c)) { + parseNumber(); + } else { + currentToken = Token.UNKNOWN; + if (c == '(') { + currentToken = Token.LEFT_PARENTHESIS; + } else if (c == ')') { + currentToken = Token.RIGHT_PARENTHESIS; + } else if (c == '>') { + currentToken = Token.GREATER_THAN; + position++; + if (position == input.length()) { + return; + } + c = input.charAt(position); + if (c == '=') { + currentToken = Token.GREATER_THAN_OR_EQUAL_TO; + } + } else if (c == '<') { + currentToken = Token.LESS_THAN; + position++; + if (position == input.length()) { + return; + } + c = input.charAt(position); + if (c == '=') { + currentToken = Token.LESS_THAN_OR_EQUAL_TO; + } + } else if (c == '&') { + position++; + if (position == input.length()) { + return; + } + c = input.charAt(position); + if (c == '&') { + currentToken = Token.LOGICAL_AND; + } + } else if (c == '|') { + position++; + if (position == input.length()) { + return; + } + c = input.charAt(position); + if (c == '|') { + currentToken = Token.LOGICAL_OR; + } + } else if (c == '=') { + currentToken = Token.EQUAL_TO; + } else if (c == '!') { + position++; + if (position == input.length()) { + return; + } + c = input.charAt(position); + if (c == '=') { + currentToken = Token.NOT_EQUAL_TO; + } + } else if (c == '-') { + currentToken = Token.MINUS; + } + if (currentToken != Token.UNKNOWN) { + position++; + } + } + } + + private void parseIdentifier() { + int s = position; + char c = input.charAt(position); + currentToken = Token.IDENTIFIER; + while (Character.isLetter(c) || c == '-' || c == '.' || c == '_') { + position++; + if (position == input.length()) { + break; + } + c = input.charAt(position); + } + value = input.substring(s, position); + if ("true".equalsIgnoreCase(value)) { + currentToken = Token.TRUE; + } else if ("false".equalsIgnoreCase(value)) { + currentToken = Token.FALSE; + } + } + + private void parseNumber() { + int s = position; + char c = input.charAt(position); + currentToken = Token.NUMBER; + while (Character.isDigit(c)) { + position++; + if (position == input.length()) { + break; + } + c = input.charAt(position); + } + value = input.substring(s, position); + } + + private void parseExpression() throws ParseException { + parseTerm(); + Token token = currentToken; + while (token == Token.LOGICAL_AND || token == Token.LOGICAL_OR) { + nextToken(); + parseTerm(); + program.add(toOperation(token)); + token = currentToken; + } + } + + private Integer toOperation(Token token) { + + int result = O_NONE; + + switch (token) { + case LOGICAL_AND: + result = O_AND; + break; + case LOGICAL_OR: + result = O_OR; + break; + case EQUAL_TO: + result = O_EQUAL; + break; + case NOT_EQUAL_TO: + result = O_NOTEQUAL; + break; + case GREATER_THAN: + result = O_GT; + break; + case GREATER_THAN_OR_EQUAL_TO: + result = O_GTOREQUAL; + break; + case LESS_THAN: + result = O_LT; + break; + case LESS_THAN_OR_EQUAL_TO: + result = O_LTOREQUAL; + break; + case TRUE: + result = O_TRUE; + break; + case FALSE: + result = O_FALSE; + break; + } + + return result; + + } + + private void parseTerm() throws ParseException { + parseElement(); + Token token = currentToken; + while (token == Token.EQUAL_TO || token == Token.NOT_EQUAL_TO || token == Token.GREATER_THAN + || token == Token.GREATER_THAN_OR_EQUAL_TO || token == Token.LESS_THAN + || token == Token.LESS_THAN_OR_EQUAL_TO) { + nextToken(); + parseElement(); + program.add(toOperation(token)); + token = currentToken; + } + } + + private void parseElement() throws ParseException { + if (currentToken == Token.LEFT_PARENTHESIS) { + nextToken(); + parseExpression(); + if (currentToken != Token.RIGHT_PARENTHESIS) { + throw new ParseException("Expected ')'", position); + } + nextToken(); + } else if (currentToken == Token.TRUE || currentToken == Token.FALSE) { + program.add(toOperation(currentToken)); + nextToken(); + } else if (currentToken == Token.IDENTIFIER) { + program.add(O_IDENTIFIER); + program.add(identifiers.size()); + identifiers.add(value); + nextToken(); + } else if (currentToken == Token.NUMBER) { + program.add(O_NUMBER); + program.add(numbers.size()); + numbers.add(Integer.parseInt(value)); + nextToken(); + } else if (currentToken == Token.MINUS) { + nextToken(); + if (currentToken == Token.NUMBER) { + program.add(O_NUMBER); + program.add(numbers.size()); + numbers.add(-Integer.parseInt(value)); + } + nextToken(); + } + else { + throw new ParseException("Unknown token", position); + } + + } +} diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/util/BooleanExpressionTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/util/BooleanExpressionTest.java new file mode 100644 index 0000000000..357fd7de23 --- /dev/null +++ b/gerrit-server/src/test/java/com/google/gerrit/server/util/BooleanExpressionTest.java @@ -0,0 +1,42 @@ +package com.google.gerrit.server.util; + +import static org.junit.Assert.*; + +import java.text.ParseException; +import java.util.HashMap; + +import org.junit.Test; + +public class BooleanExpressionTest { + + @Test + public void testEvaluate() { + + try { + assertTrue(new BooleanExpression("true && true && true").evaluate(null)); + assertTrue(new BooleanExpression("true || true || false").evaluate(null)); + assertTrue(new BooleanExpression("true || false || false").evaluate(null)); + assertTrue(new BooleanExpression("true || false || true").evaluate(null)); + assertTrue(new BooleanExpression("false || true || true").evaluate(null)); + assertTrue(new BooleanExpression("false || true || false").evaluate(null)); + assertFalse(new BooleanExpression("false || false || false").evaluate(null)); + assertFalse(new BooleanExpression("false && false || false").evaluate(null)); + assertFalse(new BooleanExpression("false && false && false").evaluate(null)); + assertTrue(new BooleanExpression("false && false || true").evaluate(null)); + assertTrue(new BooleanExpression("false = false").evaluate(null)); + assertTrue(new BooleanExpression("true = true").evaluate(null)); + assertFalse(new BooleanExpression("true = false").evaluate(null)); + assertFalse(new BooleanExpression("false = true").evaluate(null)); + + HashMap<String,String> hashMap = new HashMap<String, String>(); + hashMap.put("SRVW", "-1"); + hashMap.put("reviewer", "qt_sanity_bot"); + + assertTrue(new BooleanExpression("SRVW < 0 || reviewer != qt_sanity_bot").evaluate(hashMap)); + + } catch (ParseException e) { + fail(e.getMessage()); + } + } + +} |