Exception handling and build improvements

.github/workflows/maven.yaml

@@ -62,6 +62,7 @@ jobs:
           - {name: 'unit-tests',    javaver: 21, args: 'verify -PskipQA -DskipTests=false'}
           - {name: 'qa-checks',     javaver: 21, args: 'verify javadoc:jar -DskipTests -Dspotbugs.timeout=3600000'}
           - {name: 'errorprone',    javaver: 21, args: 'verify -Perrorprone,skipQA'}
+          - {name: 'benchmarks',    javaver: 21, args: 'verify -DskipQA -Dbenchmark=none'}
           - {name: 'jdk25',         javaver: 25, args: 'verify javadoc:jar -DskipTests'}
       fail-fast: false
     runs-on: ubuntu-latest

modules/antlr4-example/pom.xml

@@ -38,15 +38,20 @@
       <artifactId>accumulo-access-core</artifactId>
     </dependency>
     <dependency>
-      <groupId>com.google.code.gson</groupId>
-      <artifactId>gson</artifactId>
+      <groupId>org.apache.accumulo</groupId>
+      <artifactId>accumulo-access-test-data</artifactId>
       <scope>test</scope>
     </dependency>
     <dependency>
       <groupId>org.junit.jupiter</groupId>
       <artifactId>junit-jupiter-api</artifactId>
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <groupId>org.junit.jupiter</groupId>
+      <artifactId>junit-jupiter-engine</artifactId>
+      <scope>test</scope>
+    </dependency>
     <dependency>
       <groupId>org.openjdk.jmh</groupId>
       <artifactId>jmh-core</artifactId>
@@ -88,9 +93,9 @@
             <configuration>
               <arguments>
                 <arg>-package</arg>
-                <arg>org.apache.accumulo.access.grammars</arg>
+                <arg>org.apache.accumulo.access.antlr4.grammars</arg>
                 <arg>-o</arg>
-                <arg>${project.build.directory}/generated-sources/antlr4/org/apache/accumulo/access/grammars</arg>
+                <arg>${project.build.directory}/generated-sources/antlr4/org/apache/accumulo/access/antlr4/grammars</arg>
               </arguments>
               <listener>false</listener>
               <visitor>false</visitor>
@@ -132,7 +137,7 @@
                   <arguments>
                     <argument>-classpath</argument>
                     <classpath />
-                    <argument>org.apache.accumulo.access.grammar.antlr.AccessExpressionAntlrBenchmark</argument>
+                    <argument>org.apache.accumulo.access.antlr4.benchmark.AccessExpressionAntlrBenchmark</argument>
                   </arguments>
                   <environmentVariables>
                     <ACCESS_BENCHMARK>${benchmark}</ACCESS_BENCHMARK>

modules/antlr4-example/src/main/java/module-info.java

@@ -18,7 +18,7 @@
  */
 module org.apache.accumulo.access.examples.antlr {
   exports org.apache.accumulo.access.antlr4;
-  exports org.apache.accumulo.access.grammars;
+  exports org.apache.accumulo.access.antlr4.grammars;
   requires transitive org.apache.accumulo.access.core;
   requires transitive org.antlr.antlr4.runtime;
 }

modules/antlr4-example/src/main/java/org/apache/accumulo/access/antlr4/AccessExpressionAntlrEvaluator.java

@@ -26,11 +26,11 @@
 import org.antlr.v4.runtime.tree.ParseTree;
 import org.antlr.v4.runtime.tree.TerminalNode;
 import org.apache.accumulo.access.Access;
-import org.apache.accumulo.access.grammars.AccessExpressionParser.Access_expressionContext;
-import org.apache.accumulo.access.grammars.AccessExpressionParser.Access_tokenContext;
-import org.apache.accumulo.access.grammars.AccessExpressionParser.And_operatorContext;
-import org.apache.accumulo.access.grammars.AccessExpressionParser.Or_expressionContext;
-import org.apache.accumulo.access.grammars.AccessExpressionParser.Or_operatorContext;
+import org.apache.accumulo.access.antlr4.grammars.AccessExpressionParser.Access_expressionContext;
+import org.apache.accumulo.access.antlr4.grammars.AccessExpressionParser.Access_tokenContext;
+import org.apache.accumulo.access.antlr4.grammars.AccessExpressionParser.And_operatorContext;
+import org.apache.accumulo.access.antlr4.grammars.AccessExpressionParser.Or_expressionContext;
+import org.apache.accumulo.access.antlr4.grammars.AccessExpressionParser.Or_operatorContext;
 
 public class AccessExpressionAntlrEvaluator {
 

modules/antlr4-example/src/main/java/org/apache/accumulo/access/antlr4/AccessExpressionAntlrParser.java

@@ -30,9 +30,9 @@
 import org.antlr.v4.runtime.RecognitionException;
 import org.antlr.v4.runtime.Recognizer;
 import org.apache.accumulo.access.InvalidAccessExpressionException;
-import org.apache.accumulo.access.grammars.AccessExpressionLexer;
-import org.apache.accumulo.access.grammars.AccessExpressionParser;
-import org.apache.accumulo.access.grammars.AccessExpressionParser.Access_expressionContext;
+import org.apache.accumulo.access.antlr4.grammars.AccessExpressionLexer;
+import org.apache.accumulo.access.antlr4.grammars.AccessExpressionParser;
+import org.apache.accumulo.access.antlr4.grammars.AccessExpressionParser.Access_expressionContext;
 
 public class AccessExpressionAntlrParser {
 

modules/antlr4-example/src/test/java/org/apache/accumulo/access/grammar/antlr/Antlr4Tests.java → modules/antlr4-example/src/test/java/org/apache/accumulo/access/antlr4/Antlr4Tests.java

@@ -16,7 +16,7 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-package org.apache.accumulo.access.grammar.antlr;
+package org.apache.accumulo.access.antlr4;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
@@ -41,17 +41,16 @@
 import org.apache.accumulo.access.Access;
 import org.apache.accumulo.access.AccessEvaluator;
 import org.apache.accumulo.access.InvalidAccessExpressionException;
-import org.apache.accumulo.access.antlr.TestDataLoader;
-import org.apache.accumulo.access.antlr.TestDataLoader.ExpectedResult;
-import org.apache.accumulo.access.antlr.TestDataLoader.TestDataSet;
-import org.apache.accumulo.access.antlr.TestDataLoader.TestExpressions;
-import org.apache.accumulo.access.antlr4.AccessExpressionAntlrEvaluator;
-import org.apache.accumulo.access.grammars.AccessExpressionLexer;
-import org.apache.accumulo.access.grammars.AccessExpressionParser;
-import org.apache.accumulo.access.grammars.AccessExpressionParser.Access_expressionContext;
+import org.apache.accumulo.access.antlr4.grammars.AccessExpressionLexer;
+import org.apache.accumulo.access.antlr4.grammars.AccessExpressionParser;
+import org.apache.accumulo.access.antlr4.grammars.AccessExpressionParser.Access_expressionContext;
+import org.apache.accumulo.access.testdata.TestDataLoader;
+import org.apache.accumulo.access.testdata.TestDataLoader.ExpectedResult;
+import org.apache.accumulo.access.testdata.TestDataLoader.TestDataSet;
+import org.apache.accumulo.access.testdata.TestDataLoader.TestExpressions;
 import org.junit.jupiter.api.Test;
 
-public class Antlr4Tests {
+class Antlr4Tests {
 
   public static final Access ACCESS = Access.builder().build();
 

modules/antlr4-example/src/test/java/org/apache/accumulo/access/grammar/antlr/AccessExpressionAntlrBenchmark.java → modules/antlr4-example/src/test/java/org/apache/accumulo/access/antlr4/benchmark/AccessExpressionAntlrBenchmark.java

@@ -16,7 +16,7 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-package org.apache.accumulo.access.grammar.antlr;
+package org.apache.accumulo.access.antlr4.benchmark;
 
 import static java.nio.charset.StandardCharsets.UTF_8;
 
@@ -29,10 +29,12 @@
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
-import org.apache.accumulo.access.antlr.TestDataLoader;
 import org.apache.accumulo.access.antlr4.AccessExpressionAntlrEvaluator;
 import org.apache.accumulo.access.antlr4.AccessExpressionAntlrParser;
-import org.apache.accumulo.access.grammars.AccessExpressionParser.Access_expressionContext;
+import org.apache.accumulo.access.antlr4.grammars.AccessExpressionParser.Access_expressionContext;
+import org.apache.accumulo.access.testdata.TestDataLoader;
+import org.apache.accumulo.access.testdata.TestDataLoader.ExpectedResult;
+import org.apache.accumulo.access.testdata.TestDataLoader.TestDataSet;
 import org.openjdk.jmh.annotations.Benchmark;
 import org.openjdk.jmh.annotations.Mode;
 import org.openjdk.jmh.annotations.Scope;
@@ -79,7 +81,7 @@ public static class BenchmarkState {
 
     @Setup
     public void loadData() throws IOException, URISyntaxException {
-      List<TestDataLoader.TestDataSet> testData = TestDataLoader.readTestData();
+      List<TestDataSet> testData = TestDataLoader.readTestData();
       allTestExpressions = new ArrayList<>();
       allTestExpressionsStr = new ArrayList<>();
       evaluatorTests = new ArrayList<>();
@@ -93,7 +95,7 @@ public void loadData() throws IOException, URISyntaxException {
             Stream.of(testDataSet.getAuths()).map(a -> Set.of(a)).collect(Collectors.toList()));
 
         for (var tests : testDataSet.getTests()) {
-          if (tests.getExpectedResult() != TestDataLoader.ExpectedResult.ERROR) {
+          if (tests.getExpectedResult() != ExpectedResult.ERROR) {
             for (var exp : tests.getExpressions()) {
               allTestExpressionsStr.add(exp);
               byte[] byteExp = exp.getBytes(UTF_8);

modules/antlr4-example/src/test/java/org/apache/accumulo/access/grammar/SpecificationTest.java → modules/antlr4-example/src/test/java/org/apache/accumulo/access/antlr4/grammars/SpecificationTest.java

@@ -16,7 +16,7 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-package org.apache.accumulo.access.grammar;
+package org.apache.accumulo.access.antlr4.grammars;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
@@ -29,14 +29,12 @@
 import org.antlr.v4.runtime.LexerNoViableAltException;
 import org.antlr.v4.runtime.RecognitionException;
 import org.antlr.v4.runtime.Recognizer;
-import org.apache.accumulo.access.grammars.AbnfLexer;
-import org.apache.accumulo.access.grammars.AbnfParser;
 import org.junit.jupiter.api.Test;
 
 // This test uses the ANTLR ABNF grammar to parse the
 // AccessExpression ANBF specification to validate that
 // it is proper ANBF.
-public class SpecificationTest {
+class SpecificationTest {
 
   @Test
   public void testAbnfSpecificationParses() throws Exception {

modules/core/pom.xml

@@ -30,13 +30,8 @@
   <artifactId>accumulo-access-core</artifactId>
   <dependencies>
     <dependency>
-      <groupId>com.github.spotbugs</groupId>
-      <artifactId>spotbugs-annotations</artifactId>
-      <scope>test</scope>
-    </dependency>
-    <dependency>
-      <groupId>com.google.code.gson</groupId>
-      <artifactId>gson</artifactId>
+      <groupId>org.apache.accumulo</groupId>
+      <artifactId>accumulo-access-test-data</artifactId>
       <scope>test</scope>
     </dependency>
     <dependency>
@@ -49,6 +44,11 @@
       <artifactId>junit-jupiter-api</artifactId>
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <groupId>org.junit.jupiter</groupId>
+      <artifactId>junit-jupiter-engine</artifactId>
+      <scope>test</scope>
+    </dependency>
     <dependency>
       <groupId>org.openjdk.jmh</groupId>
       <artifactId>jmh-core</artifactId>
@@ -120,7 +120,7 @@
                   <arguments>
                     <argument>-classpath</argument>
                     <classpath />
-                    <argument>org.apache.accumulo.access.tests.AccessExpressionBenchmark</argument>
+                    <argument>org.apache.accumulo.access.benchmark.AccessExpressionBenchmark</argument>
                   </arguments>
                   <environmentVariables>
                     <ACCESS_BENCHMARK>${benchmark}</ACCESS_BENCHMARK>

modules/core/src/main/java/org/apache/accumulo/access/InvalidAuthorizationException.java

@@ -19,15 +19,45 @@
 package org.apache.accumulo.access;
 
 /**
- * A RuntimeException for invalid authorizations.
+ * An exception that is thrown when an authorization is not valid.
  *
  * @since 1.0.0
  */
 public class InvalidAuthorizationException extends IllegalArgumentException {
 
   private static final long serialVersionUID = 1L;
 
-  public InvalidAuthorizationException(String auth) {
-    super("authorization : '" + auth + "'");
+  private final String auth;
+  private final String reason;
+
+  public static InvalidAuthorizationException emptyString() {
+    return new InvalidAuthorizationException("", "empty string");
+  }
+
+  public static InvalidAuthorizationException unablancedQuotes(CharSequence auth) {
+    return new InvalidAuthorizationException(auth, "unbalanced quotes");
+  }
+
+  public static InvalidAuthorizationException invalidChars(CharSequence auth) {
+    return new InvalidAuthorizationException(auth, "invalid characters");
+  }
+
+  public static InvalidAuthorizationException illegalEscape(CharSequence auth) {
+    return new InvalidAuthorizationException(auth, "invalid escape sequence");
   }
+
+  public static InvalidAuthorizationException unescapedQuote(CharSequence auth) {
+    return new InvalidAuthorizationException(auth, "unescaped quote");
+  }
+
+  private InvalidAuthorizationException(CharSequence auth, String reason) {
+    this.auth = auth.toString();
+    this.reason = reason;
+  }
+
+  @Override
+  public String getMessage() {
+    return "authorization : '" + auth + "' (" + reason + ")";
+  }
+
 }

modules/core/src/main/java/org/apache/accumulo/access/impl/AccessEvaluatorImpl.java

@@ -18,12 +18,6 @@
  */
 package org.apache.accumulo.access.impl;
 
-import static org.apache.accumulo.access.impl.CharUtils.BACKSLASH;
-import static org.apache.accumulo.access.impl.CharUtils.QUOTE;
-import static org.apache.accumulo.access.impl.CharUtils.isBackslashSymbol;
-import static org.apache.accumulo.access.impl.CharUtils.isQuoteOrSlash;
-import static org.apache.accumulo.access.impl.CharUtils.isQuoteSymbol;
-
 import java.util.HashSet;
 import java.util.Set;
 import java.util.function.Consumer;
@@ -63,82 +57,6 @@ public final class AccessEvaluatorImpl implements AccessEvaluator {
     this.authorizationValidator = authorizationValidator;
   }
 
-  public static CharSequence unescape(CharSequence auth) {
-    int escapeCharCount = 0;
-    final int authLength = auth.length();
-    for (int i = 0; i < authLength; i++) {
-      char c = auth.charAt(i);
-      if (isQuoteOrSlash(c)) {
-        escapeCharCount++;
-      }
-    }
-
-    if (escapeCharCount > 0) {
-      if (escapeCharCount % 2 == 1) {
-        throw new IllegalArgumentException("Illegal escape sequence in auth : " + auth);
-      }
-
-      char[] unescapedCopy = new char[authLength - escapeCharCount / 2];
-      int pos = 0;
-      for (int i = 0; i < authLength; i++) {
-        char c = auth.charAt(i);
-        if (isBackslashSymbol(c)) {
-          i++;
-          c = auth.charAt(i);
-          if (!isQuoteOrSlash(c)) {
-            throw new IllegalArgumentException("Illegal escape sequence in auth : " + auth);
-          }
-        } else if (isQuoteSymbol(c)) {
-          // should only see quote after a slash
-          throw new IllegalArgumentException("Unescaped quote in auth : " + auth);
-        }
-
-        unescapedCopy[pos++] = c;
-      }
-
-      return new String(unescapedCopy);
-    } else {
-      return auth;
-    }
-  }
-
-  /**
-   * Properly escapes an authorization string. The string can be quoted if desired.
-   *
-   * @param auth authorization string, as UTF-8 encoded bytes
-   * @param shouldQuote true to wrap escaped authorization in quotes
-   * @return escaped authorization string
-   */
-  public static CharSequence escape(CharSequence auth, boolean shouldQuote) {
-    int escapeCount = 0;
-    final int authLength = auth.length();
-    for (int i = 0; i < authLength; i++) {
-      if (isQuoteOrSlash(auth.charAt(i))) {
-        escapeCount++;
-      }
-    }
-
-    if (escapeCount > 0 || shouldQuote) {
-      char[] escapedAuth = new char[authLength + escapeCount + (shouldQuote ? 2 : 0)];
-      int index = shouldQuote ? 1 : 0;
-      for (int i = 0; i < authLength; i++) {
-        char c = auth.charAt(i);
-        if (isQuoteOrSlash(c)) {
-          escapedAuth[index++] = BACKSLASH;
-        }
-        escapedAuth[index++] = c;
-      }
-
-      if (shouldQuote) {
-        escapedAuth[0] = QUOTE;
-        escapedAuth[escapedAuth.length - 1] = QUOTE;
-      }
-
-      auth = new String(escapedAuth);
-    }
-    return auth;
-  }
-
   @Override
   public boolean canAccess(String expression) throws InvalidAccessExpressionException {
     return evaluate(expression);