Author: rdonkin
Date: Wed Apr 16 13:19:59 2008
New Revision: 648831
URL: http://svn.apache.org/viewvc?rev=648831&view=rev
Log:
Addressed JSIEVE-8 (https://issues.apache.org/jira/browse/JSIEVE-8). I think
that the raiser of the issue is right that throwing an exception during parsing
is more specification compliant. If anyone disagrees with this reading please
jump in.
Added:
james/jsieve/trunk/src/main/java/org/apache/jsieve/SieveValidationVisitor.java
james/jsieve/trunk/src/test/java/org/apache/jsieve/junit/RequireMissingTest.java
Modified:
james/jsieve/trunk/src/main/java/org/apache/jsieve/Command.java
james/jsieve/trunk/src/main/java/org/apache/jsieve/SieveFactory.java
james/jsieve/trunk/src/main/java/org/apache/jsieve/Test.java
james/jsieve/trunk/src/test/java/org/apache/jsieve/junit/RequireTest.java
james/jsieve/trunk/src/test/java/org/apache/jsieve/junit/utils/JUnitUtils.java
Modified: james/jsieve/trunk/src/main/java/org/apache/jsieve/Command.java
URL:
http://svn.apache.org/viewvc/james/jsieve/trunk/src/main/java/org/apache/jsieve/Command.java?rev=648831&r1=648830&r2=648831&view=diff
==============================================================================
--- james/jsieve/trunk/src/main/java/org/apache/jsieve/Command.java (original)
+++ james/jsieve/trunk/src/main/java/org/apache/jsieve/Command.java Wed Apr 16
13:19:59 2008
@@ -20,6 +20,7 @@
package org.apache.jsieve;
import org.apache.commons.logging.Log;
+import org.apache.jsieve.exception.LookupException;
import org.apache.jsieve.exception.SieveException;
import org.apache.jsieve.mail.MailAdapter;
@@ -31,30 +32,24 @@
* <code>command = identifier arguments ( ";" / block )</code>
*/
public class Command implements Executable {
+
/**
- * @see org.apache.jsieve.Executable#execute(MailAdapter)
+ * Looks up an executable command with the given name.
+ * @param name not null
+ * @return <code>ExecutableCommand</code>, not null
+ * @throws LookupException if the command is not available
*/
- public Object execute(MailAdapter mail) throws SieveException {
- Log log = Logger.getLog();
- if (log.isDebugEnabled()) {
- log.debug(toString());
- coordinate.debugDiagnostics(log);
- }
- // commands are executed after the parsing phase
- // recursively from the top level block
- // so need to use the coordinate recorded from the parse
- context.setCoordinate(coordinate);
- return CommandManager.getInstance().newInstance(getName()).execute(
- mail, getArguments(), getBlock(), context);
+ public static ExecutableCommand lookup(final String name) throws
LookupException {
+ return CommandManager.getInstance().newInstance(name);
}
-
- // The name of this Command
+
+ /** The name of this Command */
private String fieldName;
- // The Arguments for this Command
+ /** The Arguments for this Command */
private Arguments fieldArguments;
- // The Block for this Command
+ /** The Block for this Command */
private Block fieldBlock;
private SieveContext context;
@@ -158,5 +153,28 @@
protected void setBlock(Block block) {
fieldBlock = block;
}
+
+ /**
+ * @see org.apache.jsieve.Executable#execute(MailAdapter)
+ */
+ public Object execute(MailAdapter mail) throws SieveException {
+ Log log = Logger.getLog();
+ if (log.isDebugEnabled()) {
+ log.debug(toString());
+ coordinate.debugDiagnostics(log);
+ }
+ // commands are executed after the parsing phase
+ // recursively from the top level block
+ // so need to use the coordinate recorded from the parse
+ context.setCoordinate(coordinate);
+ final ExecutableCommand executable = getExecutable();
+ final Object result = executable.execute(
+ mail, getArguments(), getBlock(), context);
+ return result;
+ }
+ private ExecutableCommand getExecutable() throws LookupException {
+ final String name = getName();
+ return lookup(name);
+ }
}
Modified: james/jsieve/trunk/src/main/java/org/apache/jsieve/SieveFactory.java
URL:
http://svn.apache.org/viewvc/james/jsieve/trunk/src/main/java/org/apache/jsieve/SieveFactory.java?rev=648831&r1=648830&r2=648831&view=diff
==============================================================================
--- james/jsieve/trunk/src/main/java/org/apache/jsieve/SieveFactory.java
(original)
+++ james/jsieve/trunk/src/main/java/org/apache/jsieve/SieveFactory.java Wed
Apr 16 13:19:59 2008
@@ -30,6 +30,7 @@
import org.apache.jsieve.parser.generated.ParseException;
import org.apache.jsieve.parser.generated.SieveParser;
import org.apache.jsieve.parser.generated.SieveParserVisitor;
+import org.apache.jsieve.parser.generated.SimpleNode;
/**
* <p>
@@ -83,7 +84,10 @@
*/
public Node parse(InputStream inputStream) throws ParseException {
try {
- return new SieveParser(inputStream, "UTF-8").start();
+ final SimpleNode node = new SieveParser(inputStream,
"UTF-8").start();
+ SieveValidationVisitor visitor = new SieveValidationVisitor();
+ node.jjtAccept(visitor, null);
+ return node;
} catch (ParseException ex) {
Log log = Logger.getLog();
if (log.isErrorEnabled())
@@ -91,6 +95,13 @@
if (log.isDebugEnabled())
log.debug("Parse failed.", ex);
throw ex;
+ } catch (SieveException ex) {
+ Log log = Logger.getLog();
+ if (log.isErrorEnabled())
+ log.error("Parse failed. Reason: " + ex.getMessage());
+ if (log.isDebugEnabled())
+ log.debug("Parse failed.", ex);
+ throw new ParseException(ex);
}
}
Added:
james/jsieve/trunk/src/main/java/org/apache/jsieve/SieveValidationVisitor.java
URL:
http://svn.apache.org/viewvc/james/jsieve/trunk/src/main/java/org/apache/jsieve/SieveValidationVisitor.java?rev=648831&view=auto
==============================================================================
---
james/jsieve/trunk/src/main/java/org/apache/jsieve/SieveValidationVisitor.java
(added)
+++
james/jsieve/trunk/src/main/java/org/apache/jsieve/SieveValidationVisitor.java
Wed Apr 16 13:19:59 2008
@@ -0,0 +1,107 @@
+package org.apache.jsieve;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import org.apache.jsieve.exception.LookupException;
+import org.apache.jsieve.exception.SieveException;
+import org.apache.jsieve.parser.generated.ASTargument;
+import org.apache.jsieve.parser.generated.ASTarguments;
+import org.apache.jsieve.parser.generated.ASTblock;
+import org.apache.jsieve.parser.generated.ASTcommand;
+import org.apache.jsieve.parser.generated.ASTcommands;
+import org.apache.jsieve.parser.generated.ASTstart;
+import org.apache.jsieve.parser.generated.ASTstring;
+import org.apache.jsieve.parser.generated.ASTstring_list;
+import org.apache.jsieve.parser.generated.ASTtest;
+import org.apache.jsieve.parser.generated.ASTtest_list;
+import org.apache.jsieve.parser.generated.SieveParserVisitor;
+import org.apache.jsieve.parser.generated.SimpleNode;
+
+/**
+ * Validates nodes visited.
+ * Some checks are more conveniently carried out what then
+ * tree has already been constructed.
+ */
+public class SieveValidationVisitor implements SieveParserVisitor {
+
+ private boolean requireAllowed = true;
+ private boolean isInRequire = false;
+
+ public Object visit(SimpleNode node, Object data) throws SieveException {
+ return visitNode(node, data);
+ }
+
+ private Object visitNode(SimpleNode node, Object data) throws
SieveException {
+ List children = new ArrayList(node.jjtGetNumChildren());
+ node.childrenAccept(this, children);
+ return data;
+ }
+
+ public Object visit(ASTstart node, Object data) throws SieveException {
+ return visitNode(node, data);
+ }
+
+ public Object visit(ASTcommands node, Object data) throws SieveException {
+ return visitNode(node, data);
+ }
+
+ public Object visit(ASTcommand node, Object data) throws SieveException {
+ final String name = node.getName();
+ Command.lookup(name);
+ if ("require".equalsIgnoreCase(name)) {
+ if (requireAllowed) {
+ isInRequire = true;
+ } else {
+ throw new SieveException("'require' is only allowed before
other commands");
+ }
+ } else {
+ requireAllowed = false;
+ isInRequire = false;
+ }
+ return visitNode(node, data);
+ }
+
+ public Object visit(ASTblock node, Object data) throws SieveException {
+ return visitNode(node, data);
+ }
+
+ public Object visit(ASTarguments node, Object data) throws SieveException {
+
+ return visitNode(node, data);
+ }
+
+ public Object visit(ASTargument node, Object data) throws SieveException {
+ return visitNode(node, data);
+ }
+
+ public Object visit(ASTtest node, Object data) throws SieveException {
+ return visitNode(node, data);
+ }
+
+ public Object visit(ASTtest_list node, Object data) throws SieveException {
+ return visitNode(node, data);
+ }
+
+ public Object visit(ASTstring node, Object data) throws SieveException {
+ if (isInRequire) {
+ final Object value = node.getValue();
+ if (value != null && value instanceof String) {
+ final String quotedName = (String) value;
+ final String name = quotedName.substring(1,
quotedName.length()-1);
+ try {
+ Command.lookup(name);
+ } catch (LookupException e) {
+ //TODO: catching is inefficient, should just check
+ Test.lookup(name);
+ }
+ }
+ }
+ return visitNode(node, data);
+ }
+
+ public Object visit(ASTstring_list node, Object data) throws
SieveException {
+ return visitNode(node, data);
+ }
+
+}
Modified: james/jsieve/trunk/src/main/java/org/apache/jsieve/Test.java
URL:
http://svn.apache.org/viewvc/james/jsieve/trunk/src/main/java/org/apache/jsieve/Test.java?rev=648831&r1=648830&r2=648831&view=diff
==============================================================================
--- james/jsieve/trunk/src/main/java/org/apache/jsieve/Test.java (original)
+++ james/jsieve/trunk/src/main/java/org/apache/jsieve/Test.java Wed Apr 16
13:19:59 2008
@@ -21,8 +21,10 @@
import org.apache.commons.logging.Log;
+import org.apache.jsieve.exception.LookupException;
import org.apache.jsieve.exception.SieveException;
import org.apache.jsieve.mail.*;
+import org.apache.jsieve.tests.ExecutableTest;
/**
* <p>
@@ -32,12 +34,17 @@
* <code>test = identifier arguments</code>
*/
public class Test implements Executable {
+
+ public static ExecutableTest lookup(final String name) throws
LookupException {
+ return TestManager.getInstance().newInstance(name);
+ }
+
private SieveContext context;
- // The name of this Test
+ /** The name of this Test */
private String fieldName;
- // The arguments for this Test
+ /** The arguments for this Test */
private Arguments fieldArguments;
/**
@@ -48,8 +55,10 @@
if (log.isDebugEnabled()) {
log.debug(toString());
}
- return new Boolean(TestManager.getInstance().newInstance(getName())
- .execute(mail, getArguments(), context));
+ final String name = getName();
+ final ExecutableTest test = lookup(name);
+ final boolean result = test.execute(mail, getArguments(), context);
+ return new Boolean(result);
}
/**
Added:
james/jsieve/trunk/src/test/java/org/apache/jsieve/junit/RequireMissingTest.java
URL:
http://svn.apache.org/viewvc/james/jsieve/trunk/src/test/java/org/apache/jsieve/junit/RequireMissingTest.java?rev=648831&view=auto
==============================================================================
---
james/jsieve/trunk/src/test/java/org/apache/jsieve/junit/RequireMissingTest.java
(added)
+++
james/jsieve/trunk/src/test/java/org/apache/jsieve/junit/RequireMissingTest.java
Wed Apr 16 13:19:59 2008
@@ -0,0 +1,116 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.jsieve.junit;
+
+import junit.framework.TestCase;
+
+import org.apache.jsieve.CommandManager;
+import org.apache.jsieve.TestManager;
+import org.apache.jsieve.junit.utils.JUnitUtils;
+import org.apache.jsieve.parser.generated.ParseException;
+
+/**
+ * Class AddressTest
+ */
+public class RequireMissingTest extends TestCase {
+
+
+ /**
+ * @see TestCase#setUp()
+ */
+ protected void setUp() throws Exception {
+ super.setUp();
+ CommandManager.resetInstance();
+ TestManager.resetInstance();
+ }
+
+ /**
+ * @see TestCase#tearDown()
+ */
+ protected void tearDown() throws Exception {
+ super.tearDown();
+ }
+
+ /**
+ * Tests that unsupported requires are caught before script execution.
+ */
+ public void testUnsupportedRequireNoBrackets() throws Exception {
+ String script = "require \"whatever\"; if address :contains [\"To\",
\"From\"] \"Fish!\"{ fileinto \"aFolder\"; }";
+ try {
+ JUnitUtils.parse(script);
+ fail("Expect exception to be throw during parse since command is
unsupported");
+ } catch (ParseException e) {
+// expected
+ }
+ }
+
+ /**
+ * Tests that unsupported requires are caught before script execution.
+ */
+ public void testUnsupportedRequireMultiple() throws Exception {
+ String script = "require [\"fileinto\",\"whatever\"]; if address
:contains [\"To\", \"From\"] \"Fish!\"{ fileinto \"aFolder\"; }";
+ try {
+ JUnitUtils.parse(script);
+ fail("Expect exception to be throw during parse since command is
unsupported");
+ } catch (ParseException e) {
+// expected
+ }
+ }
+
+ /**
+ * Tests that unsupported requires are caught before script execution.
+ */
+ public void testUnsupportedRequire() throws Exception {
+ String script = "require [\"whatever\"]; if address :contains [\"To\",
\"From\"] \"Fish!\"{ fileinto \"aFolder\"; }";
+ try {
+ JUnitUtils.parse(script);
+ fail("Expect exception to be throw during parse since command is
unsupported");
+ } catch (ParseException e) {
+// expected
+ }
+ }
+
+ /**
+ * Tests 2.10.5 Extensions and Optional Features: If an extension is not
enabled with "required"
+ * they must treat it as if they do not support it at all.
+ */
+ public void testMissingRequire() throws Exception {
+ String script = "if address :contains [\"To\", \"From\"] \"Fish!\"{
bogus \"aFolder\"; }";
+ try {
+ JUnitUtils.parse(script);
+ fail("Expect exception to be throw during parse since command is
missing");
+ } catch (ParseException e) {
+// expected
+ }
+ }
+
+ /**
+ * Tests 3.2 Control Structure Require: Require MUST NOT be used after any
other command.
+ */
+ public void testRequireAfterOtherCommand() throws Exception {
+ String script = "if address :contains [\"To\", \"From\"] \"Fish!\"{
fileinto \"aFolder\"; } require [\"whatever\"]; ";
+ try {
+ JUnitUtils.parse(script);
+ fail("Expect exception to be throw during parse");
+ } catch (ParseException e) {
+ //expected
+ }
+ }
+}
Modified:
james/jsieve/trunk/src/test/java/org/apache/jsieve/junit/RequireTest.java
URL:
http://svn.apache.org/viewvc/james/jsieve/trunk/src/test/java/org/apache/jsieve/junit/RequireTest.java?rev=648831&r1=648830&r2=648831&view=diff
==============================================================================
--- james/jsieve/trunk/src/test/java/org/apache/jsieve/junit/RequireTest.java
(original)
+++ james/jsieve/trunk/src/test/java/org/apache/jsieve/junit/RequireTest.java
Wed Apr 16 13:19:59 2008
@@ -83,64 +83,38 @@
/**
* Test for Command 'require' with a single test that is present
*/
- public void testSingleTestSatisfied() {
- boolean isTestPassed = false;
+ public void testSingleTestSatisfied() throws Exception {
String script = "require \"true\";";
-
- try {
- JUnitUtils.interpret(JUnitUtils.createMail(), script);
- isTestPassed = true;
- } catch (ParseException e) {
- } catch (SieveException e) {
- }
- assertTrue(isTestPassed);
+ JUnitUtils.interpret(JUnitUtils.createMail(), script);
}
/**
* Test for Command 'require' with multiple commands that are present
*/
- public void testMultipleCommandSatisfied() {
- boolean isTestPassed = false;
+ public void testMultipleCommandSatisfied() throws Exception {
String script = "require [\"if\", \"elsif\", \"else\"];";
-
- try {
- JUnitUtils.interpret(JUnitUtils.createMail(), script);
- isTestPassed = true;
- } catch (ParseException e) {
- } catch (SieveException e) {
- }
- assertTrue(isTestPassed);
+ JUnitUtils.interpret(JUnitUtils.createMail(), script);
}
/**
* Test for Command 'require' with multiple tests that are present
*/
- public void testMultipleTestSatisfied() {
- boolean isTestPassed = false;
+ public void testMultipleTestSatisfied() throws Exception {
String script = "require [\"true\", \"false\", \"not\"];";
-
- try {
- JUnitUtils.interpret(JUnitUtils.createMail(), script);
- isTestPassed = true;
- } catch (ParseException e) {
- } catch (SieveException e) {
- }
- assertTrue(isTestPassed);
+ JUnitUtils.interpret(JUnitUtils.createMail(), script);
}
/**
* Test for Command 'require' with a single command that is absent
*/
- public void testSingleCommandUnsatisfied() {
+ public void testSingleCommandUnsatisfied() throws Exception {
boolean isTestPassed = false;
String script = "require \"absent\";";
try {
JUnitUtils.interpret(JUnitUtils.createMail(), script);
- } catch (FeatureException e) {
- isTestPassed = true;
} catch (ParseException e) {
- } catch (SieveException e) {
+ isTestPassed = true;
}
assertTrue(isTestPassed);
}
@@ -148,16 +122,14 @@
/**
* Test for Command 'require' with a single test that is absent
*/
- public void testSingleTestUnsatisfied() {
+ public void testSingleTestUnsatisfied() throws Exception {
boolean isTestPassed = false;
String script = "require \"absent\";";
try {
JUnitUtils.interpret(JUnitUtils.createMail(), script);
- } catch (FeatureException e) {
- isTestPassed = true;
} catch (ParseException e) {
- } catch (SieveException e) {
+ isTestPassed = true;
}
assertTrue(isTestPassed);
}
@@ -165,7 +137,7 @@
/**
* Test for Command 'require' for missing argument
*/
- public void testMissingArgument() {
+ public void testMissingArgument() throws Exception {
boolean isTestPassed = false;
String script = "require;";
@@ -173,16 +145,14 @@
JUnitUtils.interpret(JUnitUtils.createMail(), script);
} catch (SyntaxException e) {
isTestPassed = true;
- } catch (ParseException e) {
- } catch (SieveException e) {
- }
+ }
assertTrue(isTestPassed);
}
/**
* Test for Command 'require' for extra argument
*/
- public void testExtraArgument() {
+ public void testExtraArgument() throws Exception {
boolean isTestPassed = false;
String script = "require \"if\" 1;";
@@ -190,8 +160,6 @@
JUnitUtils.interpret(JUnitUtils.createMail(), script);
} catch (SyntaxException e) {
isTestPassed = true;
- } catch (ParseException e) {
- } catch (SieveException e) {
}
assertTrue(isTestPassed);
}
@@ -199,7 +167,7 @@
/**
* Test for Command 'require' rejecting Blocks
*/
- public void testRejectBlock() {
+ public void testRejectBlock() throws Exception {
boolean isTestPassed = false;
String script = "require \"if\" {stop;}";
@@ -207,25 +175,21 @@
JUnitUtils.interpret(JUnitUtils.createMail(), script);
} catch (SyntaxException e) {
isTestPassed = true;
- } catch (ParseException e) {
- } catch (SieveException e) {
- }
+ }
assertTrue(isTestPassed);
}
/**
* Test for Command 'require' after a Command
*/
- public void testInterveningCommand() {
+ public void testInterveningCommand() throws Exception {
boolean isTestPassed = false;
String script = "fileinto \"someplace\"; require \"fileinto\";";
try {
JUnitUtils.interpret(JUnitUtils.createMail(), script);
- } catch (CommandException e) {
- isTestPassed = true;
} catch (ParseException e) {
- } catch (SieveException e) {
+ isTestPassed = true;
}
assertTrue(isTestPassed);
}
@@ -233,7 +197,7 @@
/**
* Test for Command 'require' rejecting invalid arguments
*/
- public void testRejectInvalidArgument() {
+ public void testRejectInvalidArgument() throws Exception {
boolean isTestPassed = false;
String script = "require 1 ;";
@@ -241,8 +205,6 @@
JUnitUtils.interpret(JUnitUtils.createMail(), script);
} catch (SyntaxException e) {
isTestPassed = true;
- } catch (ParseException e) {
- } catch (SieveException e) {
}
assertTrue(isTestPassed);
}
@@ -251,16 +213,14 @@
* Test for Command 'require' with a multiple commands of which one is
* absent
*/
- public void testMultipleCommandsUnsatisfied() {
+ public void testMultipleCommandsUnsatisfied() throws Exception {
boolean isTestPassed = false;
String script = "require [\"if\", \"elsif\", \"absent\"];";
try {
JUnitUtils.interpret(JUnitUtils.createMail(), script);
- } catch (FeatureException e) {
- isTestPassed = true;
} catch (ParseException e) {
- } catch (SieveException e) {
+ isTestPassed = true;
}
assertTrue(isTestPassed);
}
@@ -268,17 +228,15 @@
/**
* Test for Command 'require' with a multiple tests of which one is absent
*/
- public void testMultipleTestsUnsatisfied() {
+ public void testMultipleTestsUnsatisfied() throws Exception {
boolean isTestPassed = false;
String script = "require [\"true\", \"false\", \"absent\"];";
try {
JUnitUtils.interpret(JUnitUtils.createMail(), script);
- } catch (FeatureException e) {
- isTestPassed = true;
} catch (ParseException e) {
- } catch (SieveException e) {
- }
+ isTestPassed = true;
+ }
assertTrue(isTestPassed);
}
Modified:
james/jsieve/trunk/src/test/java/org/apache/jsieve/junit/utils/JUnitUtils.java
URL:
http://svn.apache.org/viewvc/james/jsieve/trunk/src/test/java/org/apache/jsieve/junit/utils/JUnitUtils.java?rev=648831&r1=648830&r2=648831&view=diff
==============================================================================
---
james/jsieve/trunk/src/test/java/org/apache/jsieve/junit/utils/JUnitUtils.java
(original)
+++
james/jsieve/trunk/src/test/java/org/apache/jsieve/junit/utils/JUnitUtils.java
Wed Apr 16 13:19:59 2008
@@ -50,6 +50,20 @@
mail,
new ByteArrayInputStream(script.getBytes()));
}
+
+ /**
+ * Method interpret parses a script and evaluates it against a MailAdapter.
+ * @param mail
+ * @param script
+ * @throws SieveException
+ * @throws ParseException
+ */
+ static public void parse(String script)
+ throws SieveException, ParseException
+ {
+ SieveFactory.getInstance().parse(
+ new ByteArrayInputStream(script.getBytes()));
+ }
/**
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
