Add import-control checkstyle for checking API (#34)

* Also moved maven files to contrib directory, dropping build directory
* Updated checkstyle to latest version
* Add exceptions to hold the line for API
* Use guava PeekingIterator

Author: milleruntime

build/Eclipse-Accumulo-Codestyle.xml renamed to contrib/Eclipse-Accumulo-Codestyle.xml

@@ -17,11 +17,9 @@
 <module name="Checker">
   <property name="charset" value="UTF-8"/>
   <module name="TreeWalker">
-    <module name="RegexpSinglelineJava">
-      <!--check that only Accumulo public APIs are imported-->
-      <property name="format" value="import\s+org\.apache\.accumulo\.(.*\.(impl|thrift|crypto)\..*|(?!core|minicluster|examples).*|core\.(?!client|data|security|cli|iterators).*)"/>
-      <property name="ignoreComments" value="true" />
-      <property name="message" value="Accumulo non-public classes imported" />
+    <!--check that only Accumulo public APIs are imported-->
+    <module name="ImportControl">
+      <property name="file" value="contrib/import-control.xml"/>
     </module>
   </module>
 </module>

build/checkstyle.xml renamed to contrib/checkstyle.xml

@@ -0,0 +1,45 @@
+<?xml version="1.0"?>
+<!--
+  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.
+-->
+<!DOCTYPE import-control PUBLIC
+    "-//Checkstyle//DTD ImportControl Configuration 1.4//EN"
+    "https://checkstyle.org/dtds/import_control_1_4.dtd">
+
+<!-- This checkstyle rule is configured to ensure only use of Accumulo API -->
+<import-control pkg="org.apache.accumulo.examples" strategyOnMismatch="allowed">
+    <!-- allow this package -->
+    <allow pkg="org.apache.accumulo.examples"/>
+    <!-- API packages -->
+    <allow pkg="org.apache.accumulo.core.client"/>
+    <allow pkg="org.apache.accumulo.core.data"/>
+    <allow pkg="org.apache.accumulo.core.security"/>
+    <allow pkg="org.apache.accumulo.core.iterators"/>
+    <allow pkg="org.apache.accumulo.minicluster"/>
+    <allow pkg="org.apache.accumulo.hadoop.mapreduce"/>
+
+    <!-- exceptions for this project -->
+    <allow pkg="org.apache.accumulo.core.conf"/>
+    <allow class="org.apache.accumulo.core.constraints.Constraint"/>
+
+    <!-- TODO refactor code to remove the following exceptions -->
+    <allow class="org.apache.accumulo.tracer.TraceDump"/>
+    <allow class="org.apache.accumulo.core.trace.DistributedTrace"/>
+    <allow class="org.apache.accumulo.core.util.format.DefaultFormatter"/>
+    <allow class="org.apache.accumulo.core.util.TextUtil"/>
+    <!-- End TODO section -->
+
+    <!-- disallow everything else coming from accumulo -->
+    <disallow pkg="org.apache.accumulo"/>
+</import-control>

contrib/import-control.xml

@@ -34,7 +34,7 @@
     <slf4j.version>1.7.21</slf4j.version>
     <maven.compiler.source>1.8</maven.compiler.source>
     <maven.compiler.target>1.8</maven.compiler.target>
-    <eclipseFormatterStyle>build/Eclipse-Accumulo-Codestyle.xml</eclipseFormatterStyle>
+    <eclipseFormatterStyle>contrib/Eclipse-Accumulo-Codestyle.xml</eclipseFormatterStyle>
   </properties>
   <dependencyManagement>
     <dependencies>
@@ -186,14 +186,27 @@
         </executions>
       </plugin>
       <plugin>
-        <!-- This was added to ensure project only uses public API. Run with the following:
-              mvn checkstyle:checkstyle
-         -->
+        <!-- This was added to ensure project only uses public API -->
         <groupId>org.apache.maven.plugins</groupId>
         <artifactId>maven-checkstyle-plugin</artifactId>
-        <version>2.17</version>
+        <version>3.0.0</version>
+        <executions>
+          <execution>
+            <id>check-style</id>
+            <goals>
+              <goal>check</goal>
+            </goals>
+          </execution>
+        </executions>
+        <dependencies>
+          <dependency>
+            <groupId>com.puppycrawl.tools</groupId>
+            <artifactId>checkstyle</artifactId>
+            <version>8.17</version>
+          </dependency>
+        </dependencies>
         <configuration>
-          <configLocation>build/checkstyle.xml</configLocation>
+          <configLocation>contrib/checkstyle.xml</configLocation>
         </configuration>
       </plugin>
       <plugin>

pom.xml

@@ -25,12 +25,14 @@
 import org.apache.accumulo.core.client.mapreduce.InputFormatBase;
 import org.apache.accumulo.core.data.Key;
 import org.apache.accumulo.core.data.Value;
-import org.apache.accumulo.core.util.PeekingIterator;
 import org.apache.accumulo.core.util.format.DefaultFormatter;
 import org.apache.hadoop.mapreduce.InputSplit;
 import org.apache.hadoop.mapreduce.RecordReader;
 import org.apache.hadoop.mapreduce.TaskAttemptContext;
 
+import com.google.common.collect.Iterators;
+import com.google.common.collect.PeekingIterator;
+
 /**
  * An InputFormat that turns the file data ingested with {@link FileDataIngest} into an InputStream
  * using {@link ChunkInputStream}. Mappers used with this InputFormat must close the InputStream.
@@ -45,7 +47,7 @@ public RecordReader<List<Entry<Key,Value>>,InputStream> createRecordReader(Input
       @Override
       public void initialize(InputSplit inSplit, TaskAttemptContext attempt) throws IOException {
         super.initialize(inSplit, attempt);
-        peekingScannerIterator = new PeekingIterator<>(scannerIterator);
+        peekingScannerIterator = Iterators.peekingIterator(scannerIterator);
         currentK = new ArrayList<>();
         currentV = new ChunkInputStream();
       }

src/main/java/org/apache/accumulo/examples/filedata/ChunkInputFormat.java

@@ -25,11 +25,12 @@
 import org.apache.accumulo.core.data.Key;
 import org.apache.accumulo.core.data.PartialKey;
 import org.apache.accumulo.core.data.Value;
-import org.apache.accumulo.core.util.PeekingIterator;
 import org.apache.hadoop.io.Text;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import com.google.common.collect.PeekingIterator;
+
 /**
  * An input stream that reads file data stored in one or more Accumulo values. Used by
  * {@link ChunkInputFormat} to present input streams to a mapper.

src/main/java/org/apache/accumulo/examples/filedata/ChunkInputStream.java

@@ -30,7 +30,9 @@
 import org.apache.accumulo.core.data.Range;
 import org.apache.accumulo.core.data.Value;
 import org.apache.accumulo.core.security.Authorizations;
-import org.apache.accumulo.core.util.PeekingIterator;
+
+import com.google.common.collect.Iterators;
+import com.google.common.collect.PeekingIterator;
 
 /**
  * Retrieves file data based on the hash of the file. Used by the
@@ -56,7 +58,7 @@ public ChunkInputStream getData(String hash) throws IOException {
     scanner.setRange(new Range(hash));
     scanner.setBatchSize(1);
     lastRefs.clear();
-    PeekingIterator<Entry<Key,Value>> pi = new PeekingIterator<>(scanner.iterator());
+    PeekingIterator<Entry<Key,Value>> pi = Iterators.peekingIterator(scanner.iterator());
     if (pi.hasNext()) {
       while (!pi.peek().getKey().getColumnFamily().equals(FileDataIngest.CHUNK_CF)) {
         lastRefs.add(pi.peek());

src/main/java/org/apache/accumulo/examples/filedata/FileDataQuery.java

@@ -40,7 +40,6 @@
 import org.apache.accumulo.core.data.Value;
 import org.apache.accumulo.core.security.Authorizations;
 import org.apache.accumulo.core.security.ColumnVisibility;
-import org.apache.accumulo.core.util.PeekingIterator;
 import org.apache.accumulo.harness.AccumuloClusterHarness;
 import org.apache.accumulo.miniclusterImpl.MiniAccumuloConfigImpl;
 import org.apache.hadoop.conf.Configuration;
@@ -49,6 +48,9 @@
 import org.junit.Before;
 import org.junit.Test;
 
+import com.google.common.collect.Iterators;
+import com.google.common.collect.PeekingIterator;
+
 public class ChunkInputStreamIT extends AccumuloClusterHarness {
 
   @Override
@@ -155,7 +157,7 @@ public void testWithAccumulo() throws AccumuloException, AccumuloSecurityExcepti
     ChunkInputStream cis = new ChunkInputStream();
     byte[] b = new byte[20];
     int read;
-    PeekingIterator<Entry<Key,Value>> pi = new PeekingIterator<>(scan.iterator());
+    PeekingIterator<Entry<Key,Value>> pi = Iterators.peekingIterator(scan.iterator());
 
     cis.setSource(pi);
     assertEquals(read = cis.read(b), 8);

src/test/java/org/apache/accumulo/examples/filedata/ChunkInputStreamIT.java

@@ -28,13 +28,15 @@
 import org.apache.accumulo.core.data.Key;
 import org.apache.accumulo.core.data.KeyValue;
 import org.apache.accumulo.core.data.Value;
-import org.apache.accumulo.core.util.PeekingIterator;
 import org.apache.hadoop.io.Text;
 import org.junit.Before;
 import org.junit.Test;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import com.google.common.collect.Iterators;
+import com.google.common.collect.PeekingIterator;
+
 public class ChunkInputStreamTest {
   private static final Logger log = LoggerFactory.getLogger(ChunkInputStream.class);
   private List<Entry<Key,Value>> data;
@@ -103,8 +105,7 @@ private static void addData(List<Entry<Key,Value>> data, String row, String cf,
   @Test
   public void testExceptionOnMultipleSetSourceWithoutClose() throws IOException {
     ChunkInputStream cis = new ChunkInputStream();
-    PeekingIterator<Entry<Key,Value>> pi = new PeekingIterator<>(data.iterator());
-    pi = new PeekingIterator<>(data.iterator());
+    PeekingIterator<Entry<Key,Value>> pi = Iterators.peekingIterator(data.iterator());
     cis.setSource(pi);
     try {
       cis.setSource(pi);
@@ -118,7 +119,7 @@ public void testExceptionOnMultipleSetSourceWithoutClose() throws IOException {
   @Test
   public void testExceptionOnGetVisBeforeClose() throws IOException {
     ChunkInputStream cis = new ChunkInputStream();
-    PeekingIterator<Entry<Key,Value>> pi = new PeekingIterator<>(data.iterator());
+    PeekingIterator<Entry<Key,Value>> pi = Iterators.peekingIterator(data.iterator());
 
     cis.setSource(pi);
     try {
@@ -136,7 +137,7 @@ public void testReadIntoBufferSmallerThanChunks() throws IOException {
     ChunkInputStream cis = new ChunkInputStream();
     byte[] b = new byte[5];
 
-    PeekingIterator<Entry<Key,Value>> pi = new PeekingIterator<>(data.iterator());
+    PeekingIterator<Entry<Key,Value>> pi = Iterators.peekingIterator(data.iterator());
 
     cis.setSource(pi);
     int read;
@@ -188,7 +189,7 @@ public void testReadIntoBufferLargerThanChunks() throws IOException {
     ChunkInputStream cis = new ChunkInputStream();
     byte[] b = new byte[20];
     int read;
-    PeekingIterator<Entry<Key,Value>> pi = new PeekingIterator<>(data.iterator());
+    PeekingIterator<Entry<Key,Value>> pi = Iterators.peekingIterator(data.iterator());
 
     cis.setSource(pi);
     assertEquals(read = cis.read(b), 8);
@@ -247,7 +248,7 @@ public void testBadData() throws IOException {
     ChunkInputStream cis = new ChunkInputStream();
     byte[] b = new byte[20];
     int read;
-    PeekingIterator<Entry<Key,Value>> pi = new PeekingIterator<>(baddata.iterator());
+    PeekingIterator<Entry<Key,Value>> pi = Iterators.peekingIterator(baddata.iterator());
 
     cis.setSource(pi);
     assumeExceptionOnRead(cis, b);
@@ -293,7 +294,7 @@ public void testBadData() throws IOException {
 
     assertFalse(pi.hasNext());
 
-    pi = new PeekingIterator<>(baddata.iterator());
+    pi = Iterators.peekingIterator(baddata.iterator());
     cis.setSource(pi);
     assumeExceptionOnClose(cis);
   }
@@ -303,7 +304,7 @@ public void testBadDataWithoutClosing() throws IOException {
     ChunkInputStream cis = new ChunkInputStream();
     byte[] b = new byte[20];
     int read;
-    PeekingIterator<Entry<Key,Value>> pi = new PeekingIterator<>(baddata.iterator());
+    PeekingIterator<Entry<Key,Value>> pi = Iterators.peekingIterator(baddata.iterator());
 
     cis.setSource(pi);
     assumeExceptionOnRead(cis, b);
@@ -344,7 +345,7 @@ public void testBadDataWithoutClosing() throws IOException {
 
     assertFalse(pi.hasNext());
 
-    pi = new PeekingIterator<>(baddata.iterator());
+    pi = Iterators.peekingIterator(baddata.iterator());
     cis.setSource(pi);
     assumeExceptionOnClose(cis);
   }
@@ -354,7 +355,7 @@ public void testMultipleChunkSizes() throws IOException {
     ChunkInputStream cis = new ChunkInputStream();
     byte[] b = new byte[20];
     int read;
-    PeekingIterator<Entry<Key,Value>> pi = new PeekingIterator<>(multidata.iterator());
+    PeekingIterator<Entry<Key,Value>> pi = Iterators.peekingIterator(multidata.iterator());
 
     b = new byte[20];
 
@@ -381,7 +382,7 @@ public void testMultipleChunkSizes() throws IOException {
   @Test
   public void testSingleByteRead() throws IOException {
     ChunkInputStream cis = new ChunkInputStream();
-    PeekingIterator<Entry<Key,Value>> pi = new PeekingIterator<>(data.iterator());
+    PeekingIterator<Entry<Key,Value>> pi = Iterators.peekingIterator(data.iterator());
 
     cis.setSource(pi);
     assertEquals((byte) 'a', (byte) cis.read());