You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@accumulo.apache.org by dl...@apache.org on 2023/09/11 18:22:23 UTC
[accumulo-access] 05/10: improves unit test coverage (#1)
This is an automated email from the ASF dual-hosted git repository.
dlmarion pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/accumulo-access.git
commit 6e1c8d25f2835ee5597b32be079649aa01c75030
Author: Keith Turner <kt...@apache.org>
AuthorDate: Fri Sep 8 09:44:45 2023 -0400
improves unit test coverage (#1)
Did the following for this commit.
* Looked at Accumulo existing test and brough test cases over
* Looked at the specification and thought of new cases to cover
* Looked at the result code coverage and added new test to increase
coverage
---
.../apache/accumulo/access/AccessEvaluator.java | 4 +-
.../apache/accumulo/access/AccessExpression.java | 3 +-
.../accumulo/access/AccessExpressionImpl.java | 2 +-
.../org/apache/accumulo/access/Authorizations.java | 2 +-
.../org/apache/accumulo/access/BytesWrapper.java | 10 --
.../accumulo/access/AccessEvaluatorTest.java | 17 ++++
.../accumulo/access/AccessExpressionTest.java | 109 +++++++++++++++++++++
src/test/resources/testdata.json | 96 +++++++++++++++++-
8 files changed, 226 insertions(+), 17 deletions(-)
diff --git a/src/main/java/org/apache/accumulo/access/AccessEvaluator.java b/src/main/java/org/apache/accumulo/access/AccessEvaluator.java
index 0ec696b..def37f4 100644
--- a/src/main/java/org/apache/accumulo/access/AccessEvaluator.java
+++ b/src/main/java/org/apache/accumulo/access/AccessEvaluator.java
@@ -39,7 +39,7 @@ import java.util.List;
* </pre>
*
*
- * @since ???
+ * @since 1.0.0
*/
public interface AccessEvaluator {
/**
@@ -57,7 +57,7 @@ public interface AccessEvaluator {
boolean canAccess(AccessExpression accessExpression) throws IllegalAccessExpressionException;
/**
- * @since ???
+ * @since 1.0.0
*/
interface Authorizer {
boolean isAuthorized(String auth);
diff --git a/src/main/java/org/apache/accumulo/access/AccessExpression.java b/src/main/java/org/apache/accumulo/access/AccessExpression.java
index 2a5625f..8bf8ada 100644
--- a/src/main/java/org/apache/accumulo/access/AccessExpression.java
+++ b/src/main/java/org/apache/accumulo/access/AccessExpression.java
@@ -53,9 +53,8 @@ package org.apache.accumulo.access;
* [🦖, CAT, 🦕]
* </pre>
*
- * @since ???
+ * @since 1.0.0
*/
-// TODO could name VisibilityLabel
public interface AccessExpression {
/**
diff --git a/src/main/java/org/apache/accumulo/access/AccessExpressionImpl.java b/src/main/java/org/apache/accumulo/access/AccessExpressionImpl.java
index d21dd82..5941316 100644
--- a/src/main/java/org/apache/accumulo/access/AccessExpressionImpl.java
+++ b/src/main/java/org/apache/accumulo/access/AccessExpressionImpl.java
@@ -518,7 +518,7 @@ class AccessExpressionImpl implements AccessExpression {
@Override
public String toString() {
- return "[" + new String(expression, UTF_8) + "]";
+ return getExpression();
}
/**
diff --git a/src/main/java/org/apache/accumulo/access/Authorizations.java b/src/main/java/org/apache/accumulo/access/Authorizations.java
index a438c69..cbeccb9 100644
--- a/src/main/java/org/apache/accumulo/access/Authorizations.java
+++ b/src/main/java/org/apache/accumulo/access/Authorizations.java
@@ -23,7 +23,7 @@ import java.util.Set;
/**
*
- * @since ????
+ * @since 1.0.0
*/
public class Authorizations {
private final Set<String> authorizations;
diff --git a/src/main/java/org/apache/accumulo/access/BytesWrapper.java b/src/main/java/org/apache/accumulo/access/BytesWrapper.java
index 598e4e2..ff5bb07 100644
--- a/src/main/java/org/apache/accumulo/access/BytesWrapper.java
+++ b/src/main/java/org/apache/accumulo/access/BytesWrapper.java
@@ -81,16 +81,6 @@ class BytesWrapper implements Comparable<BytesWrapper> {
return length;
}
- public byte[] toArray() {
- if (offset == 0 && length == data.length) {
- return data;
- }
-
- byte[] copy = new byte[length];
- System.arraycopy(data, offset, copy, 0, length);
- return copy;
- }
-
@Override
public int compareTo(BytesWrapper obs) {
return Arrays.compare(data, offset, offset + length(), obs.data, obs.offset,
diff --git a/src/test/java/org/apache/accumulo/access/AccessEvaluatorTest.java b/src/test/java/org/apache/accumulo/access/AccessEvaluatorTest.java
index f49af8d..1afa67a 100644
--- a/src/test/java/org/apache/accumulo/access/AccessEvaluatorTest.java
+++ b/src/test/java/org/apache/accumulo/access/AccessEvaluatorTest.java
@@ -118,20 +118,27 @@ public class AccessEvaluatorTest {
assertTrue(tests.expressions.length > 0);
for (var expression : tests.expressions) {
+
switch (tests.expectedResult) {
case ACCESSIBLE:
assertTrue(evaluator.canAccess(expression), expression);
assertTrue(evaluator.canAccess(expression.getBytes(UTF_8)), expression);
assertTrue(evaluator.canAccess(AccessExpression.of(expression)), expression);
+ assertTrue(evaluator.canAccess(AccessExpression.of(expression.getBytes(UTF_8))), expression);
assertTrue(evaluator.canAccess(AccessExpression.of(expression).normalize()),
expression);
+ assertEquals(expression, AccessExpression.of(expression.getBytes(UTF_8)).getExpression());
+ assertEquals(expression, AccessExpression.of(expression).getExpression());
break;
case INACCESSIBLE:
assertFalse(evaluator.canAccess(expression), expression);
assertFalse(evaluator.canAccess(expression.getBytes(UTF_8)), expression);
assertFalse(evaluator.canAccess(AccessExpression.of(expression)), expression);
+ assertFalse(evaluator.canAccess(AccessExpression.of(expression.getBytes(UTF_8))), expression);
assertFalse(evaluator.canAccess(AccessExpression.of(expression).normalize()),
expression);
+ assertEquals(expression, AccessExpression.of(expression.getBytes(UTF_8)).getExpression());
+ assertEquals(expression, AccessExpression.of(expression).getExpression());
break;
case ERROR:
assertThrows(IllegalAccessExpressionException.class,
@@ -140,6 +147,8 @@ public class AccessEvaluatorTest {
() -> evaluator.canAccess(expression.getBytes(UTF_8)), expression);
assertThrows(IllegalAccessExpressionException.class,
() -> evaluator.canAccess(AccessExpression.of(expression)), expression);
+ assertThrows(IllegalAccessExpressionException.class,
+ () -> evaluator.canAccess(AccessExpression.of(expression.getBytes(UTF_8))), expression);
break;
default:
throw new IllegalArgumentException();
@@ -148,6 +157,14 @@ public class AccessEvaluatorTest {
}
}
+ @Test
+ public void testEmptyAuthorizations(){
+ assertThrows(IllegalArgumentException.class, ()->AccessEvaluator.builder().authorizations("").build());
+ assertThrows(IllegalArgumentException.class, ()->AccessEvaluator.builder().authorizations("","A").build());
+ assertThrows(IllegalArgumentException.class, ()->AccessEvaluator.builder().authorizations("A","").build());
+ assertThrows(IllegalArgumentException.class, ()->AccessEvaluator.builder().authorizations(Authorizations.of("")).build());
+ }
+
@Test
public void testSpecialChars() {
// special chars do not need quoting
diff --git a/src/test/java/org/apache/accumulo/access/AccessExpressionTest.java b/src/test/java/org/apache/accumulo/access/AccessExpressionTest.java
new file mode 100644
index 0000000..e48790a
--- /dev/null
+++ b/src/test/java/org/apache/accumulo/access/AccessExpressionTest.java
@@ -0,0 +1,109 @@
+/*
+ * 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
+ *
+ * https://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.accumulo.access;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.stream.Collectors;
+
+import org.junit.jupiter.api.Test;
+
+public class AccessExpressionTest {
+
+ @Test
+ public void testEmptyExpression(){
+ assertEquals("", AccessExpression.of().getExpression());
+ }
+
+ @Test
+ public void testGetAuthorizations(){
+ // Test data pairs where the first entry of each pair is an expression to normalize and second
+ // is the expected authorization in the expression
+ var testData = new ArrayList<List<String>>();
+
+ testData.add(List.of("", ""));
+ testData.add(List.of("a", "a"));
+ testData.add(List.of("(a)", "a"));
+ testData.add(List.of("Z|M|A", "A,M,Z"));
+ testData.add(List.of("Z&M&A", "A,M,Z"));
+ testData.add(List.of("(Y|B|Y)&(Z|A|Z)", "A,B,Y,Z"));
+ testData.add(List.of("(Y&B&Y)|(Z&A&Z)", "A,B,Y,Z"));
+ testData.add(List.of("(A1|B1)&((A1|B2)&(B2|C1))", "A1,B1,B2,C1"));
+
+ for (var testCase : testData) {
+ assertEquals(2, testCase.size());
+ var expression = testCase.get(0);
+ var expected = testCase.get(1);
+ var actual = AccessExpression.of(expression).getAuthorizations().asSet().stream().sorted().collect(Collectors.joining(","));
+ assertEquals(expected,actual);
+ actual = AccessExpression.of(expression.getBytes(UTF_8)).getAuthorizations().asSet().stream().sorted().collect(Collectors.joining(","));
+ assertEquals(expected,actual);
+ }
+
+ }
+
+
+ @Test
+ public void testNormalize() {
+ // Test data pairs where the first entry of each pair is an expression to normalize and second
+ // is the expected normalized value.
+ var testData = new ArrayList<List<String>>();
+
+ testData.add(List.of("", ""));
+ testData.add(List.of("a", "a"));
+ testData.add(List.of("(a)", "a"));
+ testData.add(List.of("b|a", "a|b"));
+ testData.add(List.of("(b)|a", "a|b"));
+ testData.add(List.of("(b)|((a))", "a|b"));
+ testData.add(List.of("(b|(a|c))&x", "x&(a|b|c)"));
+ testData.add(List.of("(((a)))", "a"));
+ testData.add(List.of("Z|M|A", "A|M|Z"));
+ testData.add(List.of("Z&M&A", "A&M&Z"));
+ testData.add(List.of("(Y&B)|(Z&A)", "(A&Z)|(B&Y)"));
+ testData.add(List.of("(Y&B&Y)|(Z&A&Z)", "(A&Z)|(B&Y)"));
+ testData.add(List.of("(Y|B)&(Z|A)", "(A|Z)&(B|Y)"));
+ testData.add(List.of("(Y|B|Y)&(Z|A|Z)", "(A|Z)&(B|Y)"));
+ testData.add(List.of("((Z&B)|(Y&C))&((V&D)|(X&A))", "((A&X)|(D&V))&((B&Z)|(C&Y))"));
+ testData.add(List.of("((Z&B&B)|(Y&C&Y))&((V&D)|(X&A))", "((A&X)|(D&V))&((B&Z)|(C&Y))"));
+ testData.add(List.of("((Z&B)|(Y&C))&((V&D&D)|(X&A))", "((A&X)|(D&V))&((B&Z)|(C&Y))"));
+ testData.add(List.of("((Z|B)&(Y|C))|((V|D)&(X|A))", "((A|X)&(D|V))|((B|Z)&(C|Y))"));
+ testData.add(List.of("bz1|bm3|c9|ba4|am", "am|ba4|bm3|bz1|c9"));
+ testData.add(List.of("bz1&bm3&c9&ba4&am", "am&ba4&bm3&bz1&c9"));
+ testData.add(List.of("((V&D)|(X&A))&A", "A&((A&X)|(D&V))"));
+ testData.add(List.of("((V|D)&(X|A))|A", "A|((A|X)&(D|V))"));
+ testData.add(List.of("(Z|(X|M))|C|(A|B)", "A|B|C|M|X|Z"));
+ testData.add(List.of("(Z&(X&M))&C&(A&B)", "A&B&C&M&X&Z"));
+ testData.add(List.of("(Z&(X&(M|L)))&C&(A&B)", "A&B&C&X&Z&(L|M)"));
+ testData.add(List.of("(Z|(X|(M&L)))|C|(A|B)", "A|B|C|X|Z|(L&M)"));
+
+ for (var testCase : testData) {
+ assertEquals(2, testCase.size());
+ var expression = testCase.get(0);
+ var expected = testCase.get(1);
+ var normalized = AccessExpression.of(expression).normalize();
+ assertEquals(expected, normalized);
+ assertEquals(expected, AccessExpression.of(expression.getBytes(UTF_8)).normalize());
+ assertEquals(expected, AccessExpression.of(normalized).normalize());
+ }
+ }
+}
diff --git a/src/test/resources/testdata.json b/src/test/resources/testdata.json
index 2367b31..9539675 100644
--- a/src/test/resources/testdata.json
+++ b/src/test/resources/testdata.json
@@ -101,6 +101,24 @@
"|A",
"A&",
"&A",
+ "&(five)",
+ "|(five)",
+ "(five)&",
+ "five|",
+ "a|(b)&",
+ "(&five)",
+ "(five|)",
+ "one(five)",
+ "(five)one",
+ "(one)(two)",
+ "a|(b(c))",
+ "(", ")",
+ "(a&b",
+ "b|a)",
+ "A|B)",
+ "(A&B)|(C&D)&(E)",
+ "a|b&c",
+ "A&B&C|D",
"A|(B|)",
"A|(|B)",
"A|(B&)",
@@ -131,7 +149,45 @@
"\"\\c\"",
"\"\\\"",
"\"\"\"",
- "\"\"\"&A"
+ "\"\"\"&A",
+ "!",
+ "!|a",
+ "a|!",
+ "@",
+ "@|a",
+ "(@|a)",
+ "a|@",
+ "(a|@)",
+ "#",
+ "$",
+ "%",
+ "^",
+ "*",
+ "=",
+ "+",
+ "~",
+ "`",
+ "[",
+ "]",
+ "[A|Z]",
+ "{",
+ "{a|c}",
+ "}",
+ ",",
+ "<",
+ ">",
+ "?",
+ "&&",
+ "a&&b",
+ "a&b&&c",
+ "||",
+ "a||b",
+ "a|b||c",
+ "&|",
+ "|&",
+ "a|b&",
+ "(|a)",
+ "&a&b(a&b)"
]
}
]
@@ -249,6 +305,44 @@
}
]
},
+ {
+ "description": "expressions with all possible chars not needing quotes",
+ "auths": [
+ [
+ "abcdefghijklmnopqrstuzwxyz",
+ "ABCDEFGHIJKLMNOPQRSTUVWXYZ",
+ "0123456789",
+ "/.-_:",
+ "Ab3/zy8"
+ ]
+ ],
+ "tests": [
+ {
+ "expectedResult": "ACCESSIBLE",
+ "expressions": [
+ "abcdefghijklmnopqrstuzwxyz",
+ "ABCDEFGHIJKLMNOPQRSTUVWXYZ",
+ "0123456789",
+ "/.-_:",
+ "Ab3/zy8",
+ "abcdefghijklmnopqrstuzwxyz&ABCDEFGHIJKLMNOPQRSTUVWXYZ&0123456789&/.-_:&Ab3/zy8",
+ "abcdefghijklmnopqrstuzwxyz|ABCDEFGHIJKLMNOPQRSTUVWXYZ|0123456789|/.-_:|Ab3/zy8",
+ "(abcdefghijklmnopqrstuzwxyz&ABCDEFGHIJKLMNOPQRSTUVWXYZ)|(0123456789&/.-_:&Ab3/zy8)",
+ "(abcdefghijklmnopqrstuzwxyz|ABCDEFGHIJKLMNOPQRSTUVWXYZ)&(0123456789|/.-_:|Ab3/zy8)"
+ ]
+ },
+ {
+ "expectedResult": "INACCESSIBLE",
+ "expressions": [
+ "abcdefghijklmnopqrstuzwxyz&abcdefghijklmnopqrstuzwxyz0123456789",
+ "ABCDEFGHIJKLMNOPQRSTUVWXYZAb3/zy8&0123456789",
+ "abcdefghijklmnopqrstuzwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789",
+ "Ab4/zy7|(z&Ab3/zy8)",
+ "/.-_:&999"
+ ]
+ }
+ ]
+ },
{
"description": "non ascii expressions",
"auths": [