This is an automated email from the ASF dual-hosted git repository.
ctubbsii pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/accumulo-access.git
The following commit(s) were added to refs/heads/main by this push:
new d272aa9 Exception handling and build improvements (#119)
d272aa9 is described below
commit d272aa931e06d46878a340909017e530e8cae33d
Author: Christopher Tubbs <[email protected]>
AuthorDate: Mon Mar 16 21:37:23 2026 -0400
Exception handling and build improvements (#119)
Exception handling improvements:
* Use InvalidAuthorizationException instead of raw
IllegalArgumentException where it makes sense, especially to follow
the javadoc that says that it will be thrown when parsing an invalid
authorization inside an access expression
* Inline quote/unquote methods from AccessEvaluatorImpl to th eonly
place they are called in AccessImpl, so they can be simplified without
redundant checks for empty strings
* Move (un)escape methods from AccessEvaluatorImpl to CharUtils, since
they are not used in AccessEvaluatorImpl after the quote/unquote
methods were inlined
Build improvements:
* Add junit-jupiter-engine explicitly to the classpath for tests
(required to run tests in IDE; surefire on the command-line adds it
automatically, so it's only needed for running tests in the IDE)
* Relocate tests and generated files into standard directories
* Put common test loader code in separate module, removing unneeded
spotbugs suppressions and redundant test code
* Add build test for benchmark execution (without running benchmarks)
---
.github/workflows/maven.yaml | 1 +
modules/antlr4-example/pom.xml | 15 ++-
.../antlr4-example/src/main/java/module-info.java | 2 +-
.../antlr4/AccessExpressionAntlrEvaluator.java | 10 +-
.../access/antlr4/AccessExpressionAntlrParser.java | 6 +-
.../{grammar/antlr => antlr4}/Antlr4Tests.java | 19 ++--
.../benchmark}/AccessExpressionAntlrBenchmark.java | 12 +-
.../grammars}/SpecificationTest.java | 6 +-
.../src/test/resources/testdata.json | 1 -
modules/core/pom.xml | 16 +--
.../access/InvalidAuthorizationException.java | 36 +++++-
.../accumulo/access/impl/AccessEvaluatorImpl.java | 82 --------------
.../accumulo/access/impl/AccessExpressionImpl.java | 40 -------
.../apache/accumulo/access/impl/AccessImpl.java | 29 ++++-
.../org/apache/accumulo/access/impl/CharUtils.java | 79 +++++++++++++
.../access/impl/ParsedAccessExpressionImpl.java | 11 +-
.../accumulo/access/impl/ParserEvaluator.java | 4 +-
.../AccessExpressionBenchmark.java | 34 +++---
.../{tests => impl}/AccessEvaluatorTest.java | 122 ++++++++-------------
.../{tests => impl}/AccessExpressionTest.java | 4 +-
.../access/{tests => impl}/AuthorizationTest.java | 5 +-
.../ParsedAccessExpressionTest.java | 5 +-
modules/examples/pom.xml | 5 +
.../examples/{test => }/AccessExampleTest.java | 5 +-
.../examples/{test => }/ParseExamplesTest.java | 5 +-
modules/{examples => test-data}/pom.xml | 14 +--
.../src/main/java/module-info.java | 8 +-
.../accumulo/access/testdata}/TestDataLoader.java | 7 +-
.../apache/accumulo/access/testdata}/testdata.json | 0
pom.xml | 33 +++++-
src/build/spotbugs-exclude.xml | 7 +-
31 files changed, 308 insertions(+), 315 deletions(-)
diff --git a/.github/workflows/maven.yaml b/.github/workflows/maven.yaml
index 1935366..23425da 100644
--- a/.github/workflows/maven.yaml
+++ b/.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
diff --git a/modules/antlr4-example/pom.xml b/modules/antlr4-example/pom.xml
index 384c438..f7542ab 100644
--- a/modules/antlr4-example/pom.xml
+++ b/modules/antlr4-example/pom.xml
@@ -38,8 +38,8 @@
<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>
@@ -47,6 +47,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>
@@ -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>
diff --git a/modules/antlr4-example/src/main/java/module-info.java
b/modules/antlr4-example/src/main/java/module-info.java
index aaae71b..0802938 100644
--- a/modules/antlr4-example/src/main/java/module-info.java
+++ b/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;
}
diff --git
a/modules/antlr4-example/src/main/java/org/apache/accumulo/access/antlr4/AccessExpressionAntlrEvaluator.java
b/modules/antlr4-example/src/main/java/org/apache/accumulo/access/antlr4/AccessExpressionAntlrEvaluator.java
index ecf101d..bf0b976 100644
---
a/modules/antlr4-example/src/main/java/org/apache/accumulo/access/antlr4/AccessExpressionAntlrEvaluator.java
+++
b/modules/antlr4-example/src/main/java/org/apache/accumulo/access/antlr4/AccessExpressionAntlrEvaluator.java
@@ -26,11 +26,11 @@ import java.util.Set;
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 {
diff --git
a/modules/antlr4-example/src/main/java/org/apache/accumulo/access/antlr4/AccessExpressionAntlrParser.java
b/modules/antlr4-example/src/main/java/org/apache/accumulo/access/antlr4/AccessExpressionAntlrParser.java
index 95e14c3..71cc1eb 100644
---
a/modules/antlr4-example/src/main/java/org/apache/accumulo/access/antlr4/AccessExpressionAntlrParser.java
+++
b/modules/antlr4-example/src/main/java/org/apache/accumulo/access/antlr4/AccessExpressionAntlrParser.java
@@ -30,9 +30,9 @@ import org.antlr.v4.runtime.LexerNoViableAltException;
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 {
diff --git
a/modules/antlr4-example/src/test/java/org/apache/accumulo/access/grammar/antlr/Antlr4Tests.java
b/modules/antlr4-example/src/test/java/org/apache/accumulo/access/antlr4/Antlr4Tests.java
similarity index 91%
rename from
modules/antlr4-example/src/test/java/org/apache/accumulo/access/grammar/antlr/Antlr4Tests.java
rename to
modules/antlr4-example/src/test/java/org/apache/accumulo/access/antlr4/Antlr4Tests.java
index 0d20ac0..b0f1902 100644
---
a/modules/antlr4-example/src/test/java/org/apache/accumulo/access/grammar/antlr/Antlr4Tests.java
+++
b/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.antlr.v4.runtime.Recognizer;
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();
diff --git
a/modules/antlr4-example/src/test/java/org/apache/accumulo/access/grammar/antlr/AccessExpressionAntlrBenchmark.java
b/modules/antlr4-example/src/test/java/org/apache/accumulo/access/antlr4/benchmark/AccessExpressionAntlrBenchmark.java
similarity index 92%
rename from
modules/antlr4-example/src/test/java/org/apache/accumulo/access/grammar/antlr/AccessExpressionAntlrBenchmark.java
rename to
modules/antlr4-example/src/test/java/org/apache/accumulo/access/antlr4/benchmark/AccessExpressionAntlrBenchmark.java
index ed48963..246c7b5 100644
---
a/modules/antlr4-example/src/test/java/org/apache/accumulo/access/grammar/antlr/AccessExpressionAntlrBenchmark.java
+++
b/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.concurrent.TimeUnit;
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 class AccessExpressionAntlrBenchmark {
@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 class AccessExpressionAntlrBenchmark {
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);
diff --git
a/modules/antlr4-example/src/test/java/org/apache/accumulo/access/grammar/SpecificationTest.java
b/modules/antlr4-example/src/test/java/org/apache/accumulo/access/antlr4/grammars/SpecificationTest.java
similarity index 93%
rename from
modules/antlr4-example/src/test/java/org/apache/accumulo/access/grammar/SpecificationTest.java
rename to
modules/antlr4-example/src/test/java/org/apache/accumulo/access/antlr4/grammars/SpecificationTest.java
index 5535f4b..53dce19 100644
---
a/modules/antlr4-example/src/test/java/org/apache/accumulo/access/grammar/SpecificationTest.java
+++
b/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.ConsoleErrorListener;
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 {
diff --git a/modules/antlr4-example/src/test/resources/testdata.json
b/modules/antlr4-example/src/test/resources/testdata.json
deleted file mode 120000
index 5bc6f30..0000000
--- a/modules/antlr4-example/src/test/resources/testdata.json
+++ /dev/null
@@ -1 +0,0 @@
-../../../../core/src/test/resources/testdata.json
\ No newline at end of file
diff --git a/modules/core/pom.xml b/modules/core/pom.xml
index e351503..2e6eaea 100644
--- a/modules/core/pom.xml
+++ b/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>
diff --git
a/modules/core/src/main/java/org/apache/accumulo/access/InvalidAuthorizationException.java
b/modules/core/src/main/java/org/apache/accumulo/access/InvalidAuthorizationException.java
index 74df35f..1e7b4e5 100644
---
a/modules/core/src/main/java/org/apache/accumulo/access/InvalidAuthorizationException.java
+++
b/modules/core/src/main/java/org/apache/accumulo/access/InvalidAuthorizationException.java
@@ -19,7 +19,7 @@
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
*/
@@ -27,7 +27,37 @@ 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 + ")";
+ }
+
}
diff --git
a/modules/core/src/main/java/org/apache/accumulo/access/impl/AccessEvaluatorImpl.java
b/modules/core/src/main/java/org/apache/accumulo/access/impl/AccessEvaluatorImpl.java
index e71fe29..a568b4b 100644
---
a/modules/core/src/main/java/org/apache/accumulo/access/impl/AccessEvaluatorImpl.java
+++
b/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);
diff --git
a/modules/core/src/main/java/org/apache/accumulo/access/impl/AccessExpressionImpl.java
b/modules/core/src/main/java/org/apache/accumulo/access/impl/AccessExpressionImpl.java
index 75de7ee..b42e1db 100644
---
a/modules/core/src/main/java/org/apache/accumulo/access/impl/AccessExpressionImpl.java
+++
b/modules/core/src/main/java/org/apache/accumulo/access/impl/AccessExpressionImpl.java
@@ -55,44 +55,4 @@ public final class AccessExpressionImpl extends
AccessExpression {
return parseTree;
}
- public static CharSequence quote(CharSequence term) {
- if (term.isEmpty()) {
- throw new IllegalArgumentException("Empty strings are not legal
authorizations.");
- }
-
- boolean needsQuote = false;
- final int len = term.length();
- for (int i = 0; i < len; i++) {
- if (!Tokenizer.isValidAuthChar(term.charAt(i))) {
- needsQuote = true;
- break;
- }
- }
-
- if (!needsQuote) {
- return term;
- }
-
- return AccessEvaluatorImpl.escape(term, true);
- }
-
- public static CharSequence unquote(CharSequence term) {
- final int len = term.length();
- if (len >= 1) {
- final boolean firstIsQuote = term.charAt(0) == '"';
- final boolean lastIsQuote = term.charAt(len - 1) == '"';
- if (firstIsQuote || lastIsQuote) {
- if (len == 1 || (firstIsQuote != lastIsQuote)) {
- throw new IllegalArgumentException("Unbalanced quotes : " + term);
- }
-
- term = len == 2 ? "" :
AccessEvaluatorImpl.unescape(term.subSequence(1, len - 1));
- }
- }
- if (term.isEmpty()) {
- throw new IllegalArgumentException("Empty strings are not legal
authorizations.");
- }
- return term;
- }
-
}
diff --git
a/modules/core/src/main/java/org/apache/accumulo/access/impl/AccessImpl.java
b/modules/core/src/main/java/org/apache/accumulo/access/impl/AccessImpl.java
index 9120ed5..037dad5 100644
--- a/modules/core/src/main/java/org/apache/accumulo/access/impl/AccessImpl.java
+++ b/modules/core/src/main/java/org/apache/accumulo/access/impl/AccessImpl.java
@@ -41,10 +41,10 @@ public class AccessImpl implements Access {
private void validateAuthArgument(CharSequence auth) {
if (auth.isEmpty()) {
- throw new IllegalArgumentException("Empty string is not a valid
authorization");
+ throw InvalidAuthorizationException.emptyString();
}
if (!authValidator.test(auth, ANY)) {
- throw new InvalidAuthorizationException(auth.toString());
+ throw InvalidAuthorizationException.invalidChars(auth);
}
}
@@ -75,14 +75,33 @@ public class AccessImpl implements Access {
@Override
public String quote(String authorization) {
validateAuthArgument(authorization);
- return AccessExpressionImpl.quote(authorization).toString();
+ boolean needsQuote = false;
+ final int len = authorization.length();
+ for (int i = 0; i < len; i++) {
+ if (!Tokenizer.isValidAuthChar(authorization.charAt(i))) {
+ needsQuote = true;
+ break;
+ }
+ }
+ return needsQuote ? CharUtils.escape(authorization, true) : authorization;
}
@Override
public String unquote(String authorization) {
- var unquoted = AccessExpressionImpl.unquote(authorization);
+ String unquoted = authorization;
+ final int len = unquoted.length();
+ if (len >= 1) {
+ final boolean firstIsQuote = unquoted.charAt(0) == '"';
+ final boolean lastIsQuote = unquoted.charAt(len - 1) == '"';
+ if (firstIsQuote || lastIsQuote) {
+ if (len == 1 || (firstIsQuote != lastIsQuote)) {
+ throw InvalidAuthorizationException.unablancedQuotes(authorization);
// unbalanced quotes
+ }
+ unquoted = len == 2 ? "" : CharUtils.unescape(unquoted.substring(1,
len - 1)).toString();
+ }
+ }
validateAuthArgument(unquoted);
- return unquoted.toString();
+ return unquoted;
}
@Override
diff --git
a/modules/core/src/main/java/org/apache/accumulo/access/impl/CharUtils.java
b/modules/core/src/main/java/org/apache/accumulo/access/impl/CharUtils.java
index 696fbb1..1c14db5 100644
--- a/modules/core/src/main/java/org/apache/accumulo/access/impl/CharUtils.java
+++ b/modules/core/src/main/java/org/apache/accumulo/access/impl/CharUtils.java
@@ -18,6 +18,8 @@
*/
package org.apache.accumulo.access.impl;
+import org.apache.accumulo.access.InvalidAuthorizationException;
+
/**
* This class exists to avoid repeat conversions from byte to char as well as
to provide helper
* methods for comparing them.
@@ -55,4 +57,81 @@ final class CharUtils {
static boolean isAndOrOperator(char b) {
return isAndOperator(b) || isOrOperator(b);
}
+
+ /**
+ * 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
+ */
+ static String escape(String 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;
+ }
+
+ 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 InvalidAuthorizationException.illegalEscape(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 InvalidAuthorizationException.illegalEscape(auth);
+ }
+ } else if (isQuoteSymbol(c)) {
+ // should only see quote after a slash
+ throw InvalidAuthorizationException.unescapedQuote(auth);
+ }
+
+ unescapedCopy[pos++] = c;
+ }
+
+ return new String(unescapedCopy);
+ } else {
+ return auth;
+ }
+ }
+
}
diff --git
a/modules/core/src/main/java/org/apache/accumulo/access/impl/ParsedAccessExpressionImpl.java
b/modules/core/src/main/java/org/apache/accumulo/access/impl/ParsedAccessExpressionImpl.java
index c41559b..3d99059 100644
---
a/modules/core/src/main/java/org/apache/accumulo/access/impl/ParsedAccessExpressionImpl.java
+++
b/modules/core/src/main/java/org/apache/accumulo/access/impl/ParsedAccessExpressionImpl.java
@@ -31,6 +31,7 @@ import java.util.List;
import java.util.concurrent.atomic.AtomicReference;
import org.apache.accumulo.access.AuthorizationValidator;
+import org.apache.accumulo.access.InvalidAccessExpressionException;
import org.apache.accumulo.access.InvalidAuthorizationException;
import org.apache.accumulo.access.ParsedAccessExpression;
@@ -51,11 +52,13 @@ public final class ParsedAccessExpressionImpl extends
ParsedAccessExpression {
private ParsedAccessExpressionImpl(char operator, String wholeExpression,
int offset, int length,
List<ParsedAccessExpression> children) {
if (children.isEmpty()) {
- throw new IllegalArgumentException("Must have children with an
operator");
+ throw new InvalidAccessExpressionException("Must have children with an
operator",
+ wholeExpression, offset);
}
if (operator != AND_OPERATOR && operator != OR_OPERATOR) {
- throw new IllegalArgumentException("Unknown operator " + operator);
+ throw new InvalidAccessExpressionException("Unknown operator " +
operator, wholeExpression,
+ offset);
} else if (operator == AND_OPERATOR) {
this.type = AND;
} else {
@@ -182,7 +185,7 @@ public final class ParsedAccessExpressionImpl extends
ParsedAccessExpression {
if (CharUtils.isQuoteSymbol(auth.data[auth.start])) {
wrapper.set(auth.data, auth.start + 1, auth.len - 2);
if (auth.hasEscapes) {
- unquotedAuth = AccessEvaluatorImpl.unescape(wrapper);
+ unquotedAuth = CharUtils.unescape(wrapper);
} else {
unquotedAuth = wrapper;
}
@@ -193,7 +196,7 @@ public final class ParsedAccessExpressionImpl extends
ParsedAccessExpression {
quoting = AuthorizationValidator.AuthorizationCharacters.BASIC;
}
if (!authorizationValidator.test(unquotedAuth, quoting)) {
- throw new InvalidAuthorizationException(unquotedAuth.toString());
+ throw InvalidAuthorizationException.invalidChars(unquotedAuth);
}
return new ParsedAccessExpressionImpl(wholeExpression, auth.start,
auth.len);
}
diff --git
a/modules/core/src/main/java/org/apache/accumulo/access/impl/ParserEvaluator.java
b/modules/core/src/main/java/org/apache/accumulo/access/impl/ParserEvaluator.java
index 557d89a..6b6fd54 100644
---
a/modules/core/src/main/java/org/apache/accumulo/access/impl/ParserEvaluator.java
+++
b/modules/core/src/main/java/org/apache/accumulo/access/impl/ParserEvaluator.java
@@ -63,12 +63,12 @@ public final class ParserEvaluator {
charsWrapper.set(authToken.data, authToken.start, authToken.len);
CharSequence authorizations;
if (authToken.hasEscapes) {
- authorizations = AccessEvaluatorImpl.unescape(charsWrapper);
+ authorizations = CharUtils.unescape(charsWrapper);
} else {
authorizations = charsWrapper;
}
if (!authValidator.test(authorizations, authToken.quoting)) {
- throw new InvalidAuthorizationException(authorizations.toString());
+ throw InvalidAuthorizationException.invalidChars(authorizations);
}
return authorizations;
}
diff --git
a/modules/core/src/test/java/org/apache/accumulo/access/tests/AccessExpressionBenchmark.java
b/modules/core/src/test/java/org/apache/accumulo/access/benchmark/AccessExpressionBenchmark.java
similarity index 90%
rename from
modules/core/src/test/java/org/apache/accumulo/access/tests/AccessExpressionBenchmark.java
rename to
modules/core/src/test/java/org/apache/accumulo/access/benchmark/AccessExpressionBenchmark.java
index b64df5b..f5ff50f 100644
---
a/modules/core/src/test/java/org/apache/accumulo/access/tests/AccessExpressionBenchmark.java
+++
b/modules/core/src/test/java/org/apache/accumulo/access/benchmark/AccessExpressionBenchmark.java
@@ -16,11 +16,10 @@
* specific language governing permissions and limitations
* under the License.
*/
-package org.apache.accumulo.access.tests;
+package org.apache.accumulo.access.benchmark;
import static java.nio.charset.StandardCharsets.UTF_8;
-import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
import java.util.Set;
@@ -30,6 +29,9 @@ import java.util.stream.Stream;
import org.apache.accumulo.access.Access;
import org.apache.accumulo.access.AccessEvaluator;
+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.core.security.ColumnVisibility;
import org.apache.accumulo.core.security.VisibilityEvaluator;
import org.apache.accumulo.core.security.VisibilityParseException;
@@ -42,14 +44,11 @@ import org.openjdk.jmh.infra.Blackhole;
import org.openjdk.jmh.profile.JavaFlightRecorderProfiler;
import org.openjdk.jmh.runner.NoBenchmarksException;
import org.openjdk.jmh.runner.Runner;
-import org.openjdk.jmh.runner.RunnerException;
import org.openjdk.jmh.runner.options.OptionsBuilder;
import org.openjdk.jmh.runner.options.TimeValue;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
-
/**
* Benchmarks Access Expressions using JMH. To run, use the following commands.
*
@@ -63,7 +62,6 @@ import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
*
* </blockquote>
*/
-@SuppressFBWarnings(value = {"NP_UNWRITTEN_FIELD"}, justification = "Field is
written by Gson")
public class AccessExpressionBenchmark {
private static final Logger LOG =
LoggerFactory.getLogger(AccessExpressionBenchmark.class);
@@ -94,9 +92,9 @@ public class AccessExpressionBenchmark {
private ArrayList<VisibilityEvaluatorTests> visibilityEvaluatorTests;
@Setup
- public void loadData() throws IOException {
+ public void loadData() throws Exception {
access = Access.builder().build();
- List<AccessEvaluatorTest.TestDataSet> testData =
AccessEvaluatorTest.readTestData();
+ List<TestDataSet> testData = TestDataLoader.readTestData();
allTestExpressions = new ArrayList<>();
allTestExpressionsStr = new ArrayList<>();
evaluatorTests = new ArrayList<>();
@@ -109,12 +107,12 @@ public class AccessExpressionBenchmark {
vet.expressions = new ArrayList<>();
vet.columnVisibilities = new ArrayList<>();
- if (testDataSet.auths.length == 1) {
+ if (testDataSet.getAuths().length == 1) {
vet.evaluator = List.of(new VisibilityEvaluator(
- new
org.apache.accumulo.core.security.Authorizations(testDataSet.auths[0])));
+ new
org.apache.accumulo.core.security.Authorizations(testDataSet.getAuths()[0])));
} else {
List<VisibilityEvaluator> veList = new ArrayList<>();
- for (String[] auths : testDataSet.auths) {
+ for (String[] auths : testDataSet.getAuths()) {
veList.add(new VisibilityEvaluator(
new org.apache.accumulo.core.security.Authorizations(auths)));
}
@@ -125,17 +123,17 @@ public class AccessExpressionBenchmark {
EvaluatorTests et = new EvaluatorTests();
et.expressions = new ArrayList<>();
- if (testDataSet.auths.length == 1) {
- et.evaluator = access.newEvaluator(Set.of(testDataSet.auths[0]));
+ if (testDataSet.getAuths().length == 1) {
+ et.evaluator =
access.newEvaluator(Set.of(testDataSet.getAuths()[0]));
} else {
var authSets =
- Stream.of(testDataSet.auths).map(a ->
Set.of(a)).collect(Collectors.toList());
+ Stream.of(testDataSet.getAuths()).map(a ->
Set.of(a)).collect(Collectors.toList());
et.evaluator = access.newEvaluator(authSets);
}
- for (var tests : testDataSet.tests) {
- if (tests.expectedResult !=
AccessEvaluatorTest.ExpectedResult.ERROR) {
- for (var exp : tests.expressions) {
+ for (var tests : testDataSet.getTests()) {
+ if (tests.getExpectedResult() != ExpectedResult.ERROR) {
+ for (var exp : tests.getExpressions()) {
allTestExpressionsStr.add(exp);
byte[] byteExp = exp.getBytes(UTF_8);
allTestExpressions.add(byteExp);
@@ -250,7 +248,7 @@ public class AccessExpressionBenchmark {
}
}
- public static void main(String[] args) throws RunnerException, IOException {
+ public static void main(String[] args) throws Exception {
var state = new BenchmarkState();
state.loadData();
diff --git
a/modules/core/src/test/java/org/apache/accumulo/access/tests/AccessEvaluatorTest.java
b/modules/core/src/test/java/org/apache/accumulo/access/impl/AccessEvaluatorTest.java
similarity index 67%
rename from
modules/core/src/test/java/org/apache/accumulo/access/tests/AccessEvaluatorTest.java
rename to
modules/core/src/test/java/org/apache/accumulo/access/impl/AccessEvaluatorTest.java
index c445bf4..c170c95 100644
---
a/modules/core/src/test/java/org/apache/accumulo/access/tests/AccessEvaluatorTest.java
+++
b/modules/core/src/test/java/org/apache/accumulo/access/impl/AccessEvaluatorTest.java
@@ -16,18 +16,13 @@
* specific language governing permissions and limitations
* under the License.
*/
-package org.apache.accumulo.access.tests;
+package org.apache.accumulo.access.impl;
-import static java.nio.charset.StandardCharsets.UTF_8;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
-import java.io.IOException;
-import java.lang.reflect.Type;
-import java.nio.CharBuffer;
-import java.util.ArrayList;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
@@ -38,67 +33,36 @@ import org.apache.accumulo.access.Access;
import org.apache.accumulo.access.AccessEvaluator;
import org.apache.accumulo.access.AuthorizationValidator;
import org.apache.accumulo.access.InvalidAccessExpressionException;
-import org.apache.accumulo.access.impl.AccessEvaluatorImpl;
-import org.apache.accumulo.access.impl.AccessExpressionImpl;
+import org.apache.accumulo.access.InvalidAuthorizationException;
+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.junit.jupiter.api.Test;
-import com.google.gson.Gson;
-import com.google.gson.reflect.TypeToken;
-
-import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
-
-@SuppressFBWarnings(value = {"UWF_UNWRITTEN_FIELD", "NP_UNWRITTEN_FIELD"},
- justification = "Field is written by Gson")
-public class AccessEvaluatorTest {
-
- enum ExpectedResult {
- ACCESSIBLE, INACCESSIBLE, ERROR
- }
-
- public static class TestExpressions {
- ExpectedResult expectedResult;
- String[] expressions;
- }
-
- public static class TestDataSet {
- String description;
- String[][] auths;
- List<TestExpressions> tests;
- }
-
- static List<TestDataSet> readTestData() throws IOException {
- try (var input =
AccessEvaluatorTest.class.getResourceAsStream("/testdata.json")) {
- if (input == null) {
- throw new IllegalStateException("could not find resource :
testdata.json");
- }
- var json = new String(input.readAllBytes(), UTF_8);
-
- Type listType = new TypeToken<ArrayList<TestDataSet>>() {}.getType();
- return new Gson().fromJson(json, listType);
- }
- }
+class AccessEvaluatorTest {
@Test
- public void runTestCases() throws IOException {
- List<TestDataSet> testData = readTestData();
+ public void runTestCases() throws Exception {
+ List<TestDataSet> testData = TestDataLoader.readTestData();
assertFalse(testData.isEmpty());
var access = Access.builder().build();
for (var testSet : testData) {
- System.out.println("runTestCases for " + testSet.description);
+ System.out.println("runTestCases for " + testSet.getDescription());
AccessEvaluator evaluator;
- assertTrue(testSet.auths.length >= 1);
- if (testSet.auths.length == 1) {
- evaluator = access.newEvaluator(Set.of(testSet.auths[0]));
+ assertTrue(testSet.getAuths().length >= 1);
+ if (testSet.getAuths().length == 1) {
+ evaluator = access.newEvaluator(Set.of(testSet.getAuths()[0]));
runTestCases(access, testSet, evaluator);
- Set<String> auths =
Stream.of(testSet.auths[0]).collect(Collectors.toSet());
+ Set<String> auths =
Stream.of(testSet.getAuths()[0]).collect(Collectors.toSet());
evaluator = access.newEvaluator(auths::contains);
runTestCases(access, testSet, evaluator);
} else {
- var authSets = Stream.of(testSet.auths).map(a ->
Set.of(a)).collect(Collectors.toList());
+ var authSets =
+ Stream.of(testSet.getAuths()).map(a ->
Set.of(a)).collect(Collectors.toList());
evaluator = access.newEvaluator(authSets);
runTestCases(access, testSet, evaluator);
}
@@ -108,18 +72,18 @@ public class AccessEvaluatorTest {
private static void runTestCases(Access accumuloAccess, TestDataSet testSet,
AccessEvaluator evaluator) {
- assertFalse(testSet.tests.isEmpty());
+ assertFalse(testSet.getTests().isEmpty());
- for (var tests : testSet.tests) {
+ for (var tests : testSet.getTests()) {
- assertTrue(tests.expressions.length > 0);
+ assertTrue(tests.getExpressions().length > 0);
- for (var expression : tests.expressions) {
+ for (var expression : tests.getExpressions()) {
// Call various APIs with well-formed access expressions to ensure
they do not throw an
// exception
- if (tests.expectedResult == ExpectedResult.ACCESSIBLE
- || tests.expectedResult == ExpectedResult.INACCESSIBLE) {
+ if (tests.getExpectedResult() == ExpectedResult.ACCESSIBLE
+ || tests.getExpectedResult() == ExpectedResult.INACCESSIBLE) {
accumuloAccess.validateExpression(expression);
assertEquals(expression,
accumuloAccess.newExpression(expression).getExpression());
// parsing an expression will strip unneeded outer parens
@@ -128,7 +92,7 @@ public class AccessEvaluatorTest {
accumuloAccess.findAuthorizations(expression, auth -> {});
}
- switch (tests.expectedResult) {
+ switch (tests.getExpectedResult()) {
case ACCESSIBLE -> {
assertTrue(evaluator.canAccess(expression), expression);
assertTrue(evaluator.canAccess(accumuloAccess.newExpression(expression)),
expression);
@@ -165,10 +129,10 @@ public class AccessEvaluatorTest {
@Test
public void testEmptyAuthorizations() {
var access = Access.builder().build();
- assertThrows(IllegalArgumentException.class, () ->
access.newEvaluator(Set.of("")));
- assertThrows(IllegalArgumentException.class, () ->
access.newEvaluator(Set.of("", "A")));
- assertThrows(IllegalArgumentException.class, () ->
access.newEvaluator(Set.of("A", "")));
- assertThrows(IllegalArgumentException.class, () ->
access.newEvaluator(Set.of("")));
+ assertThrows(InvalidAuthorizationException.class, () ->
access.newEvaluator(Set.of("")));
+ assertThrows(InvalidAuthorizationException.class, () ->
access.newEvaluator(Set.of("", "A")));
+ assertThrows(InvalidAuthorizationException.class, () ->
access.newEvaluator(Set.of("A", "")));
+ assertThrows(InvalidAuthorizationException.class, () ->
access.newEvaluator(Set.of("")));
}
@Test
@@ -202,34 +166,29 @@ public class AccessEvaluatorTest {
assertEquals("\"五十\"", access.quote("五十"));
assertEquals("五十", access.unquote(access.quote("五十")));
- var e = assertThrows(IllegalArgumentException.class, () ->
access.quote(""));
- assertTrue(e.getMessage().contains("Empty string"));
+ var e = assertThrows(InvalidAuthorizationException.class, () ->
access.quote(""));
+ assertEquals("authorization : '' (empty string)", e.getMessage());
- testUnquoteError(access, "\"\"\"\"", "Unescaped quote");
+ testUnquoteError(access, "\"\"\"\"", "unescaped quote");
for (var illegalInput : List.of("", "\"\"")) {
- testUnquoteError(access, illegalInput, "Empty string");
+ testUnquoteError(access, illegalInput, "empty string");
}
for (var illegalInput : List.of("\"", "AB\"", "\"AB", "\"A", "B\"")) {
- testUnquoteError(access, illegalInput, "Unbalanced quotes");
+ testUnquoteError(access, illegalInput, "unbalanced quotes");
}
}
private static void testUnquoteError(Access access, String illegalInput,
String expectedErrorMsg) {
- var e = assertThrows(IllegalArgumentException.class, () ->
access.unquote(illegalInput),
- illegalInput);
- assertTrue(e.getMessage().contains(expectedErrorMsg));
- // test with an input that is not a string
- CharSequence charSeq = CharBuffer.wrap(illegalInput);
- e = assertThrows(IllegalArgumentException.class, () ->
AccessExpressionImpl.unquote(charSeq),
+ var e = assertThrows(InvalidAuthorizationException.class, () ->
access.unquote(illegalInput),
illegalInput);
assertTrue(e.getMessage().contains(expectedErrorMsg));
}
private static String unescape(String s) {
- return AccessEvaluatorImpl.unescape(s).toString();
+ return CharUtils.unescape(s).toString();
}
@Test
@@ -240,11 +199,16 @@ public class AccessEvaluatorTest {
assertEquals("\\\"", unescape("\\\\\\\""));
assertEquals("a\\b\\c\\d", unescape("a\\\\b\\\\c\\\\d"));
- final String message = "Expected failure to unescape invalid escape
sequence";
- final var invalidEscapeSeqList = List.of("a\\b", "a\\b\\c", "a\"b\\");
-
- invalidEscapeSeqList
- .forEach(seq -> assertThrows(IllegalArgumentException.class, () ->
unescape(seq), message));
+ List.of("a\\b", "a\\b\\c").forEach(seq -> {
+ var e = assertThrows(InvalidAuthorizationException.class, () ->
unescape(seq),
+ "Expected failure to unescape invalid escape sequence: " + seq);
+ assertTrue(e.getMessage().contains("invalid escape"), seq + " -> " +
e.getMessage());
+ });
+ List.of("a\"b\\").forEach(seq -> {
+ var e = assertThrows(InvalidAuthorizationException.class, () ->
unescape(seq),
+ "Expected failure to unescape invalid escape sequence: " + seq);
+ assertTrue(e.getMessage().contains("unescaped quote"), seq + " -> " +
e.getMessage());
+ });
}
@Test
diff --git
a/modules/core/src/test/java/org/apache/accumulo/access/tests/AccessExpressionTest.java
b/modules/core/src/test/java/org/apache/accumulo/access/impl/AccessExpressionTest.java
similarity index 99%
rename from
modules/core/src/test/java/org/apache/accumulo/access/tests/AccessExpressionTest.java
rename to
modules/core/src/test/java/org/apache/accumulo/access/impl/AccessExpressionTest.java
index 55abca3..bff14a6 100644
---
a/modules/core/src/test/java/org/apache/accumulo/access/tests/AccessExpressionTest.java
+++
b/modules/core/src/test/java/org/apache/accumulo/access/impl/AccessExpressionTest.java
@@ -16,7 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
-package org.apache.accumulo.access.tests;
+package org.apache.accumulo.access.impl;
import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.stream.Collectors.toList;
@@ -44,7 +44,7 @@ import org.apache.accumulo.access.ParsedAccessExpression;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.function.Executable;
-public class AccessExpressionTest {
+class AccessExpressionTest {
@Test
public void testGetAuthorizations() {
diff --git
a/modules/core/src/test/java/org/apache/accumulo/access/tests/AuthorizationTest.java
b/modules/core/src/test/java/org/apache/accumulo/access/impl/AuthorizationTest.java
similarity index 98%
rename from
modules/core/src/test/java/org/apache/accumulo/access/tests/AuthorizationTest.java
rename to
modules/core/src/test/java/org/apache/accumulo/access/impl/AuthorizationTest.java
index 25855e2..c2afc7c 100644
---
a/modules/core/src/test/java/org/apache/accumulo/access/tests/AuthorizationTest.java
+++
b/modules/core/src/test/java/org/apache/accumulo/access/impl/AuthorizationTest.java
@@ -16,7 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
-package org.apache.accumulo.access.tests;
+package org.apache.accumulo.access.impl;
import static java.nio.charset.StandardCharsets.UTF_8;
import static org.junit.jupiter.api.Assertions.assertEquals;
@@ -31,10 +31,9 @@ import java.util.Set;
import org.apache.accumulo.access.Access;
import org.apache.accumulo.access.InvalidAccessExpressionException;
import org.apache.accumulo.access.InvalidAuthorizationException;
-import org.apache.accumulo.access.impl.Tokenizer;
import org.junit.jupiter.api.Test;
-public class AuthorizationTest {
+class AuthorizationTest {
@Test
public void testAuthorizationValidation() {
diff --git
a/modules/core/src/test/java/org/apache/accumulo/access/tests/ParsedAccessExpressionTest.java
b/modules/core/src/test/java/org/apache/accumulo/access/impl/ParsedAccessExpressionTest.java
similarity index 98%
rename from
modules/core/src/test/java/org/apache/accumulo/access/tests/ParsedAccessExpressionTest.java
rename to
modules/core/src/test/java/org/apache/accumulo/access/impl/ParsedAccessExpressionTest.java
index 0acbdce..efaad6f 100644
---
a/modules/core/src/test/java/org/apache/accumulo/access/tests/ParsedAccessExpressionTest.java
+++
b/modules/core/src/test/java/org/apache/accumulo/access/impl/ParsedAccessExpressionTest.java
@@ -16,7 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
-package org.apache.accumulo.access.tests;
+package org.apache.accumulo.access.impl;
import static
org.apache.accumulo.access.ParsedAccessExpression.ExpressionType.AND;
import static
org.apache.accumulo.access.ParsedAccessExpression.ExpressionType.AUTHORIZATION;
@@ -36,7 +36,8 @@ import org.apache.accumulo.access.AuthorizationValidator;
import org.apache.accumulo.access.ParsedAccessExpression;
import org.junit.jupiter.api.Test;
-public class ParsedAccessExpressionTest {
+class ParsedAccessExpressionTest {
+
@Test
public void testParsing() {
HashSet<String> seenAuths = new HashSet<>();
diff --git a/modules/examples/pom.xml b/modules/examples/pom.xml
index d22ce6c..8a17a21 100644
--- a/modules/examples/pom.xml
+++ b/modules/examples/pom.xml
@@ -38,5 +38,10 @@
<artifactId>junit-jupiter-api</artifactId>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.junit.jupiter</groupId>
+ <artifactId>junit-jupiter-engine</artifactId>
+ <scope>test</scope>
+ </dependency>
</dependencies>
</project>
diff --git
a/modules/examples/src/test/java/org/apache/accumulo/access/examples/test/AccessExampleTest.java
b/modules/examples/src/test/java/org/apache/accumulo/access/examples/AccessExampleTest.java
similarity index 92%
rename from
modules/examples/src/test/java/org/apache/accumulo/access/examples/test/AccessExampleTest.java
rename to
modules/examples/src/test/java/org/apache/accumulo/access/examples/AccessExampleTest.java
index 7ec077b..3a023a8 100644
---
a/modules/examples/src/test/java/org/apache/accumulo/access/examples/test/AccessExampleTest.java
+++
b/modules/examples/src/test/java/org/apache/accumulo/access/examples/AccessExampleTest.java
@@ -16,7 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
-package org.apache.accumulo.access.examples.test;
+package org.apache.accumulo.access.examples;
import static java.nio.charset.StandardCharsets.UTF_8;
import static org.junit.jupiter.api.Assertions.assertFalse;
@@ -27,10 +27,9 @@ import java.io.IOException;
import java.io.PrintStream;
import java.util.List;
-import org.apache.accumulo.access.examples.AccessExample;
import org.junit.jupiter.api.Test;
-public class AccessExampleTest {
+class AccessExampleTest {
@Test
public void testExampleCode() throws IOException {
try (final var baos = new ByteArrayOutputStream();
diff --git
a/modules/examples/src/test/java/org/apache/accumulo/access/examples/test/ParseExamplesTest.java
b/modules/examples/src/test/java/org/apache/accumulo/access/examples/ParseExamplesTest.java
similarity index 97%
rename from
modules/examples/src/test/java/org/apache/accumulo/access/examples/test/ParseExamplesTest.java
rename to
modules/examples/src/test/java/org/apache/accumulo/access/examples/ParseExamplesTest.java
index c34d045..c2c0cf1 100644
---
a/modules/examples/src/test/java/org/apache/accumulo/access/examples/test/ParseExamplesTest.java
+++
b/modules/examples/src/test/java/org/apache/accumulo/access/examples/ParseExamplesTest.java
@@ -16,7 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
-package org.apache.accumulo.access.examples.test;
+package org.apache.accumulo.access.examples;
import static org.apache.accumulo.access.examples.ParseExamples.ACCESS;
import static
org.apache.accumulo.access.examples.ParseExamples.replaceAuthorizations;
@@ -26,11 +26,10 @@ import java.util.ArrayList;
import java.util.List;
import java.util.Map;
-import org.apache.accumulo.access.examples.ParseExamples;
import org.junit.jupiter.api.Test;
// In addition to testing the examples, these test also provide extensive
testing of ParsedAccessExpression
-public class ParseExamplesTest {
+class ParseExamplesTest {
@Test
public void testNormalize() {
diff --git a/modules/examples/pom.xml b/modules/test-data/pom.xml
similarity index 82%
copy from modules/examples/pom.xml
copy to modules/test-data/pom.xml
index d22ce6c..68d88a4 100644
--- a/modules/examples/pom.xml
+++ b/modules/test-data/pom.xml
@@ -27,16 +27,14 @@
<version>1.0.0-beta2-SNAPSHOT</version>
<relativePath>../../pom.xml</relativePath>
</parent>
- <artifactId>accumulo-access-examples</artifactId>
+ <artifactId>accumulo-access-test-data</artifactId>
+ <properties>
+ <maven.deploy.skip>releases</maven.deploy.skip>
+ </properties>
<dependencies>
<dependency>
- <groupId>org.apache.accumulo</groupId>
- <artifactId>accumulo-access-core</artifactId>
- </dependency>
- <dependency>
- <groupId>org.junit.jupiter</groupId>
- <artifactId>junit-jupiter-api</artifactId>
- <scope>test</scope>
+ <groupId>com.google.code.gson</groupId>
+ <artifactId>gson</artifactId>
</dependency>
</dependencies>
</project>
diff --git a/modules/antlr4-example/src/main/java/module-info.java
b/modules/test-data/src/main/java/module-info.java
similarity index 76%
copy from modules/antlr4-example/src/main/java/module-info.java
copy to modules/test-data/src/main/java/module-info.java
index aaae71b..276a992 100644
--- a/modules/antlr4-example/src/main/java/module-info.java
+++ b/modules/test-data/src/main/java/module-info.java
@@ -16,9 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
-module org.apache.accumulo.access.examples.antlr {
- exports org.apache.accumulo.access.antlr4;
- exports org.apache.accumulo.access.grammars;
- requires transitive org.apache.accumulo.access.core;
- requires transitive org.antlr.antlr4.runtime;
+module org.apache.accumulo.access.testdata {
+ exports org.apache.accumulo.access.testdata;
+ requires com.google.gson;
}
diff --git
a/modules/antlr4-example/src/test/java/org/apache/accumulo/access/antlr/TestDataLoader.java
b/modules/test-data/src/main/java/org/apache/accumulo/access/testdata/TestDataLoader.java
similarity index 94%
rename from
modules/antlr4-example/src/test/java/org/apache/accumulo/access/antlr/TestDataLoader.java
rename to
modules/test-data/src/main/java/org/apache/accumulo/access/testdata/TestDataLoader.java
index 6201b33..5a0bb37 100644
---
a/modules/antlr4-example/src/test/java/org/apache/accumulo/access/antlr/TestDataLoader.java
+++
b/modules/test-data/src/main/java/org/apache/accumulo/access/testdata/TestDataLoader.java
@@ -16,7 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
-package org.apache.accumulo.access.antlr;
+package org.apache.accumulo.access.testdata;
import static java.nio.charset.StandardCharsets.UTF_8;
@@ -86,9 +86,10 @@ public class TestDataLoader {
}
}
- public static List<TestDataSet> readTestData() throws IOException,
URISyntaxException {
+ private TestDataLoader() {}
- try (var input =
TestDataLoader.class.getResourceAsStream("/testdata.json")) {
+ public static List<TestDataSet> readTestData() throws IOException,
URISyntaxException {
+ try (var input =
TestDataLoader.class.getResourceAsStream("testdata.json")) {
if (input == null) {
throw new IllegalStateException("could not find resource :
testdata.json");
}
diff --git a/modules/core/src/test/resources/testdata.json
b/modules/test-data/src/main/resources/org/apache/accumulo/access/testdata/testdata.json
similarity index 100%
rename from modules/core/src/test/resources/testdata.json
rename to
modules/test-data/src/main/resources/org/apache/accumulo/access/testdata/testdata.json
diff --git a/pom.xml b/pom.xml
index 1203d01..9c546f8 100644
--- a/pom.xml
+++ b/pom.xml
@@ -77,6 +77,7 @@
</mailingList>
</mailingLists>
<modules>
+ <module>modules/test-data</module>
<module>modules/core</module>
<module>modules/examples</module>
<module>modules/antlr4-example</module>
@@ -142,11 +143,6 @@ under the License.
</properties>
<dependencyManagement>
<dependencies>
- <dependency>
- <groupId>com.github.spotbugs</groupId>
- <artifactId>spotbugs-annotations</artifactId>
- <version>4.9.8</version>
- </dependency>
<dependency>
<groupId>com.google.code.gson</groupId>
<artifactId>gson</artifactId>
@@ -162,6 +158,11 @@ under the License.
<artifactId>accumulo-access-core</artifactId>
<version>${project.version}</version>
</dependency>
+ <dependency>
+ <groupId>org.apache.accumulo</groupId>
+ <artifactId>accumulo-access-test-data</artifactId>
+ <version>${project.version}</version>
+ </dependency>
<dependency>
<groupId>org.apache.accumulo</groupId>
<artifactId>accumulo-core</artifactId>
@@ -172,6 +173,11 @@ under the License.
<artifactId>junit-jupiter-api</artifactId>
<version>${version.junit}</version>
</dependency>
+ <dependency>
+ <groupId>org.junit.jupiter</groupId>
+ <artifactId>junit-jupiter-engine</artifactId>
+ <version>${version.junit}</version>
+ </dependency>
<dependency>
<groupId>org.openjdk.jmh</groupId>
<artifactId>jmh-core</artifactId>
@@ -424,6 +430,10 @@ under the License.
</goals>
<configuration>
<failOnWarning>true</failOnWarning>
+ <ignoredUnusedDeclaredDependencies>
+ <!-- runtime requirement for tests -->
+ <unused>org.junit.jupiter:junit-jupiter-engine:jar:*</unused>
+ </ignoredUnusedDeclaredDependencies>
</configuration>
</execution>
<execution>
@@ -915,6 +925,19 @@ under the License.
<ignore />
</action>
</pluginExecution>
+ <pluginExecution>
+ <pluginExecutionFilter>
+ <groupId>com.github.spotbugs</groupId>
+ <artifactId>spotbugs-maven-plugin</artifactId>
+ <versionRange>[0,)</versionRange>
+ <goals>
+ <goal>check</goal>
+ </goals>
+ </pluginExecutionFilter>
+ <action>
+ <ignore />
+ </action>
+ </pluginExecution>
</pluginExecutions>
</lifecycleMappingMetadata>
</configuration>
diff --git a/src/build/spotbugs-exclude.xml b/src/build/spotbugs-exclude.xml
index 756574e..15481e5 100644
--- a/src/build/spotbugs-exclude.xml
+++ b/src/build/spotbugs-exclude.xml
@@ -24,12 +24,7 @@
<Or>
<!-- ignore generated stuff -->
<Class name="~.*[.]jmh_generated[.].*" />
- <Class name="~org[.]apache[.]accumulo[.]access[.]grammars[.].*" />
+ <Class name="~org[.]apache[.]accumulo[.]access[.]antlr4[.]grammars[.].*"
/>
</Or>
</Match>
- <Match>
- <Class name="org.apache.accumulo.access.impl.CharsWrapper" />
- <Method name="set" params="char[],int,int" returns="void" />
- <Bug pattern="EI_EXPOSE_REP2" />
- </Match>
</FindBugsFilter>
