You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@accumulo.apache.org by el...@apache.org on 2015/04/07 16:45:03 UTC

accumulo git commit: ACCUMULO-2325 Attempt to make example constraints more efficient.

Repository: accumulo
Updated Branches:
  refs/heads/master 810b1e6ea -> edc080c83


ACCUMULO-2325 Attempt to make example constraints more efficient.

Partially applied patch from Vikram Srivastava. Due to the nature
of constraints, it is far more likely that it will never fire. As
such, it makes sense that we optimize for that case. Avoiding the
allocation of a new collection (as the code already did) is far
better than always allocating a new list/set for every invocation
of the constraint. Tests were directly applied from the original patch.


Project: http://git-wip-us.apache.org/repos/asf/accumulo/repo
Commit: http://git-wip-us.apache.org/repos/asf/accumulo/commit/edc080c8
Tree: http://git-wip-us.apache.org/repos/asf/accumulo/tree/edc080c8
Diff: http://git-wip-us.apache.org/repos/asf/accumulo/diff/edc080c8

Branch: refs/heads/master
Commit: edc080c830d2e9689e973a8b8b3447d3bbf661b5
Parents: 810b1e6
Author: Josh Elser <el...@apache.org>
Authored: Tue Apr 7 10:42:38 2015 -0400
Committer: Josh Elser <el...@apache.org>
Committed: Tue Apr 7 10:42:38 2015 -0400

----------------------------------------------------------------------
 .../constraints/AlphaNumKeyConstraint.java      | 26 ++++++----
 .../constraints/NumericValueConstraint.java     | 24 +++------
 .../constraints/AlphaNumKeyConstraintTest.java  | 53 ++++++++++++++++++++
 .../constraints/NumericValueConstraintTest.java | 51 +++++++++++++++++++
 4 files changed, 128 insertions(+), 26 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/accumulo/blob/edc080c8/examples/simple/src/main/java/org/apache/accumulo/examples/simple/constraints/AlphaNumKeyConstraint.java
----------------------------------------------------------------------
diff --git a/examples/simple/src/main/java/org/apache/accumulo/examples/simple/constraints/AlphaNumKeyConstraint.java b/examples/simple/src/main/java/org/apache/accumulo/examples/simple/constraints/AlphaNumKeyConstraint.java
index 8099b7e..f265e18 100644
--- a/examples/simple/src/main/java/org/apache/accumulo/examples/simple/constraints/AlphaNumKeyConstraint.java
+++ b/examples/simple/src/main/java/org/apache/accumulo/examples/simple/constraints/AlphaNumKeyConstraint.java
@@ -18,7 +18,9 @@ package org.apache.accumulo.examples.simple.constraints;
 
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.LinkedHashSet;
 import java.util.List;
+import java.util.Set;
 
 import org.apache.accumulo.core.constraints.Constraint;
 import org.apache.accumulo.core.data.ColumnUpdate;
@@ -33,9 +35,13 @@ import org.apache.accumulo.core.data.Mutation;
 
 public class AlphaNumKeyConstraint implements Constraint {
 
-  private static final short NON_ALPHA_NUM_ROW = 1;
-  private static final short NON_ALPHA_NUM_COLF = 2;
-  private static final short NON_ALPHA_NUM_COLQ = 3;
+  static final short NON_ALPHA_NUM_ROW = 1;
+  static final short NON_ALPHA_NUM_COLF = 2;
+  static final short NON_ALPHA_NUM_COLQ = 3;
+
+  static final String ROW_VIOLATION_MESSAGE = "Row was not alpha numeric";
+  static final String COLF_VIOLATION_MESSAGE = "Column family was not alpha numeric";
+  static final String COLQ_VIOLATION_MESSAGE = "Column qualifier was not alpha numeric";
 
   private boolean isAlphaNum(byte bytes[]) {
     for (byte b : bytes) {
@@ -47,9 +53,9 @@ public class AlphaNumKeyConstraint implements Constraint {
     return true;
   }
 
-  private List<Short> addViolation(List<Short> violations, short violation) {
+  private Set<Short> addViolation(Set<Short> violations, short violation) {
     if (violations == null) {
-      violations = new ArrayList<Short>();
+      violations = new LinkedHashSet<Short>();
       violations.add(violation);
     } else if (!violations.contains(violation)) {
       violations.add(violation);
@@ -59,7 +65,7 @@ public class AlphaNumKeyConstraint implements Constraint {
 
   @Override
   public List<Short> check(Environment env, Mutation mutation) {
-    List<Short> violations = null;
+    Set<Short> violations = null;
 
     if (!isAlphaNum(mutation.getRow()))
       violations = addViolation(violations, NON_ALPHA_NUM_ROW);
@@ -73,7 +79,7 @@ public class AlphaNumKeyConstraint implements Constraint {
         violations = addViolation(violations, NON_ALPHA_NUM_COLQ);
     }
 
-    return violations;
+    return null == violations ? null : new ArrayList<>(violations);
   }
 
   @Override
@@ -81,11 +87,11 @@ public class AlphaNumKeyConstraint implements Constraint {
 
     switch (violationCode) {
       case NON_ALPHA_NUM_ROW:
-        return "Row was not alpha numeric";
+        return ROW_VIOLATION_MESSAGE;
       case NON_ALPHA_NUM_COLF:
-        return "Column family was not alpha numeric";
+        return COLF_VIOLATION_MESSAGE;
       case NON_ALPHA_NUM_COLQ:
-        return "Column qualifier was not alpha numeric";
+        return COLQ_VIOLATION_MESSAGE;
     }
 
     return null;

http://git-wip-us.apache.org/repos/asf/accumulo/blob/edc080c8/examples/simple/src/main/java/org/apache/accumulo/examples/simple/constraints/NumericValueConstraint.java
----------------------------------------------------------------------
diff --git a/examples/simple/src/main/java/org/apache/accumulo/examples/simple/constraints/NumericValueConstraint.java b/examples/simple/src/main/java/org/apache/accumulo/examples/simple/constraints/NumericValueConstraint.java
index f1e6d5a..2b22e6b 100644
--- a/examples/simple/src/main/java/org/apache/accumulo/examples/simple/constraints/NumericValueConstraint.java
+++ b/examples/simple/src/main/java/org/apache/accumulo/examples/simple/constraints/NumericValueConstraint.java
@@ -16,8 +16,9 @@
  */
 package org.apache.accumulo.examples.simple.constraints;
 
-import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.List;
 
 import org.apache.accumulo.core.constraints.Constraint;
@@ -29,7 +30,10 @@ import org.apache.accumulo.core.data.Mutation;
  */
 public class NumericValueConstraint implements Constraint {
 
-  private static final short NON_NUMERIC_VALUE = 1;
+  static final short NON_NUMERIC_VALUE = 1;
+  static final String VIOLATION_MESSAGE = "Value is not numeric";
+
+  private static final List<Short> VIOLATION_LIST = Collections.unmodifiableList(Arrays.asList(NON_NUMERIC_VALUE));
 
   private boolean isNumeric(byte bytes[]) {
     for (byte b : bytes) {
@@ -41,28 +45,16 @@ public class NumericValueConstraint implements Constraint {
     return true;
   }
 
-  private List<Short> addViolation(List<Short> violations, short violation) {
-    if (violations == null) {
-      violations = new ArrayList<Short>();
-      violations.add(violation);
-    } else if (!violations.contains(violation)) {
-      violations.add(violation);
-    }
-    return violations;
-  }
-
   @Override
   public List<Short> check(Environment env, Mutation mutation) {
-    List<Short> violations = null;
-
     Collection<ColumnUpdate> updates = mutation.getUpdates();
 
     for (ColumnUpdate columnUpdate : updates) {
       if (!isNumeric(columnUpdate.getValue()))
-        violations = addViolation(violations, NON_NUMERIC_VALUE);
+        return VIOLATION_LIST;
     }
 
-    return violations;
+    return null;
   }
 
   @Override

http://git-wip-us.apache.org/repos/asf/accumulo/blob/edc080c8/examples/simple/src/test/java/org/apache/accumulo/examples/simple/constraints/AlphaNumKeyConstraintTest.java
----------------------------------------------------------------------
diff --git a/examples/simple/src/test/java/org/apache/accumulo/examples/simple/constraints/AlphaNumKeyConstraintTest.java b/examples/simple/src/test/java/org/apache/accumulo/examples/simple/constraints/AlphaNumKeyConstraintTest.java
new file mode 100644
index 0000000..8ef3f0f
--- /dev/null
+++ b/examples/simple/src/test/java/org/apache/accumulo/examples/simple/constraints/AlphaNumKeyConstraintTest.java
@@ -0,0 +1,53 @@
+/*
+ * 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.accumulo.examples.simple.constraints;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
+
+import org.apache.accumulo.core.data.Mutation;
+import org.apache.accumulo.core.data.Value;
+import org.apache.hadoop.io.Text;
+import org.junit.Test;
+
+import com.google.common.collect.ImmutableList;
+
+public class AlphaNumKeyConstraintTest {
+
+  private AlphaNumKeyConstraint ankc = new AlphaNumKeyConstraint();
+
+  @Test
+  public void test() {
+    Mutation goodMutation = new Mutation(new Text("Row1"));
+    goodMutation.put(new Text("Colf2"), new Text("ColQ3"), new Value("value".getBytes()));
+    assertNull(ankc.check(null, goodMutation));
+
+    // Check that violations are in row, cf, cq order
+    Mutation badMutation = new Mutation(new Text("Row#1"));
+    badMutation.put(new Text("Colf$2"), new Text("Colq%3"), new Value("value".getBytes()));
+    assertEquals(ImmutableList.of(AlphaNumKeyConstraint.NON_ALPHA_NUM_ROW, AlphaNumKeyConstraint.NON_ALPHA_NUM_COLF, AlphaNumKeyConstraint.NON_ALPHA_NUM_COLQ),
+        ankc.check(null, badMutation));
+  }
+
+  @Test
+  public void testGetViolationDescription() {
+    assertEquals(AlphaNumKeyConstraint.ROW_VIOLATION_MESSAGE, ankc.getViolationDescription(AlphaNumKeyConstraint.NON_ALPHA_NUM_ROW));
+    assertEquals(AlphaNumKeyConstraint.COLF_VIOLATION_MESSAGE, ankc.getViolationDescription(AlphaNumKeyConstraint.NON_ALPHA_NUM_COLF));
+    assertEquals(AlphaNumKeyConstraint.COLQ_VIOLATION_MESSAGE, ankc.getViolationDescription(AlphaNumKeyConstraint.NON_ALPHA_NUM_COLQ));
+    assertNull(ankc.getViolationDescription((short) 4));
+  }
+}

http://git-wip-us.apache.org/repos/asf/accumulo/blob/edc080c8/examples/simple/src/test/java/org/apache/accumulo/examples/simple/constraints/NumericValueConstraintTest.java
----------------------------------------------------------------------
diff --git a/examples/simple/src/test/java/org/apache/accumulo/examples/simple/constraints/NumericValueConstraintTest.java b/examples/simple/src/test/java/org/apache/accumulo/examples/simple/constraints/NumericValueConstraintTest.java
new file mode 100644
index 0000000..7d1fc49
--- /dev/null
+++ b/examples/simple/src/test/java/org/apache/accumulo/examples/simple/constraints/NumericValueConstraintTest.java
@@ -0,0 +1,51 @@
+/*
+ * 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.accumulo.examples.simple.constraints;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
+
+import org.apache.accumulo.core.data.Mutation;
+import org.apache.accumulo.core.data.Value;
+import org.apache.hadoop.io.Text;
+import org.junit.Test;
+
+import com.google.common.collect.Iterables;
+
+public class NumericValueConstraintTest {
+
+  private NumericValueConstraint nvc = new NumericValueConstraint();
+
+  @Test
+  public void testCheck() {
+    Mutation goodMutation = new Mutation(new Text("r"));
+    goodMutation.put(new Text("cf"), new Text("cq"), new Value("1234".getBytes()));
+    assertNull(nvc.check(null, goodMutation));
+
+    // Check that multiple bad mutations result in one violation only
+    Mutation badMutation = new Mutation(new Text("r"));
+    badMutation.put(new Text("cf"), new Text("cq"), new Value("foo1234".getBytes()));
+    badMutation.put(new Text("cf2"), new Text("cq2"), new Value("foo1234".getBytes()));
+    assertEquals(NumericValueConstraint.NON_NUMERIC_VALUE, Iterables.getOnlyElement(nvc.check(null, badMutation)).shortValue());
+  }
+
+  @Test
+  public void testGetViolationDescription() {
+    assertEquals(NumericValueConstraint.VIOLATION_MESSAGE, nvc.getViolationDescription(NumericValueConstraint.NON_NUMERIC_VALUE));
+    assertNull(nvc.getViolationDescription((short) 2));
+  }
+}