hbase-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ndimi...@apache.org
Subject hbase git commit: HBASE-14382 TestInterfaceAudienceAnnotations should hadoop-compt module resources
Date Thu, 10 Sep 2015 23:44:52 GMT
Repository: hbase
Updated Branches:
  refs/heads/0.98 66cb12d86 -> 5f18e7901


HBASE-14382 TestInterfaceAudienceAnnotations should hadoop-compt module resources

Conflicts:
	hbase-client/src/test/java/org/apache/hadoop/hbase/TestInterfaceAudienceAnnotations.java
	hbase-common/src/test/java/org/apache/hadoop/hbase/ClassFinder.java


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

Branch: refs/heads/0.98
Commit: 5f18e7901f4d69b8695771bcff14c71bbdeb15ea
Parents: 66cb12d
Author: Nick Dimiduk <ndimiduk@apache.org>
Authored: Wed Sep 9 11:08:28 2015 -0700
Committer: Nick Dimiduk <ndimiduk@apache.org>
Committed: Thu Sep 10 16:43:24 2015 -0700

----------------------------------------------------------------------
 .../hbase/TestInterfaceAudienceAnnotations.java | 302 +++++++++++++++++++
 .../org/apache/hadoop/hbase/ClassFinder.java    |  52 ++++
 .../apache/hadoop/hbase/ClassTestFinder.java    |   1 +
 3 files changed, 355 insertions(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/5f18e790/hbase-client/src/test/java/org/apache/hadoop/hbase/TestInterfaceAudienceAnnotations.java
----------------------------------------------------------------------
diff --git a/hbase-client/src/test/java/org/apache/hadoop/hbase/TestInterfaceAudienceAnnotations.java
b/hbase-client/src/test/java/org/apache/hadoop/hbase/TestInterfaceAudienceAnnotations.java
new file mode 100644
index 0000000..0e0fbb0
--- /dev/null
+++ b/hbase-client/src/test/java/org/apache/hadoop/hbase/TestInterfaceAudienceAnnotations.java
@@ -0,0 +1,302 @@
+/**
+ * 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.hadoop.hbase;
+
+import java.io.IOException;
+import java.lang.annotation.Annotation;
+import java.lang.reflect.Modifier;
+import java.util.Set;
+
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+import org.apache.hadoop.hbase.classification.InterfaceAudience;
+import org.apache.hadoop.hbase.classification.InterfaceStability;
+import org.apache.hadoop.hbase.testclassification.SmallTests;
+import org.apache.hadoop.hbase.ClassFinder.And;
+import org.apache.hadoop.hbase.ClassFinder.FileNameFilter;
+import org.apache.hadoop.hbase.ClassFinder.Not;
+import org.apache.hadoop.hbase.ClassTestFinder.TestClassFilter;
+import org.apache.hadoop.hbase.ClassTestFinder.TestFileNameFilter;
+import org.junit.Assert;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+/**
+ * Test cases for ensuring our client visible classes have annotations
+ * for {@link InterfaceAudience}.
+ *
+ * All classes in hbase-client and hbase-common module MUST have InterfaceAudience
+ * annotations. All InterfaceAudience.Public annotated classes MUST also have InterfaceStability
+ * annotations. Think twice about marking an interface InterfaceAudience.Public. Make sure
that
+ * it is an interface, not a class (for most cases), and clients will actually depend on
it. Once
+ * something is marked with Public, we cannot change the signatures within the major release.
NOT
+ * everything in the hbase-client module or every java public class has to be marked with
+ * InterfaceAudience.Public. ONLY the ones that an hbase application will directly use (Table,
Get,
+ * etc, versus ProtobufUtil).
+ *
+ * Also note that HBase has it's own annotations in hbase-annotations module with the same
names
+ * as in Hadoop. You should use the HBase's classes.
+ *
+ * See https://hadoop.apache.org/docs/current/hadoop-project-dist/hadoop-common/InterfaceClassification.html
+ * and https://issues.apache.org/jira/browse/HBASE-10462.
+ */
+@Category(SmallTests.class)
+public class TestInterfaceAudienceAnnotations {
+
+  private static final Log LOG = LogFactory.getLog(TestInterfaceAudienceAnnotations.class);
+
+  /** Selects classes with generated in their package name */
+  class GeneratedClassFilter implements ClassFinder.ClassFilter {
+    @Override
+    public boolean isCandidateClass(Class<?> c) {
+      return c.getPackage().getName().contains("generated");
+    }
+  }
+
+  /** Selects classes with one of the {@link InterfaceAudience} annotation in their class
+   * declaration.
+   */
+  class InterfaceAudienceAnnotatedClassFilter implements ClassFinder.ClassFilter {
+    @Override
+    public boolean isCandidateClass(Class<?> c) {
+      if (getAnnotation(c) != null) {
+        // class itself has a declared annotation.
+        return true;
+      }
+
+      // If this is an internal class, look for the encapsulating class to see whether it
has
+      // annotation. All inner classes of private classes are considered annotated.
+      return isAnnotatedPrivate(c.getEnclosingClass());
+    }
+
+    private boolean isAnnotatedPrivate(Class<?> c) {
+      if (c == null) {
+        return false;
+      }
+
+      Class<?> ann = getAnnotation(c);
+      if (ann != null &&
+        !InterfaceAudience.Public.class.equals(ann)) {
+        return true;
+      }
+
+      return isAnnotatedPrivate(c.getEnclosingClass());
+    }
+
+    protected Class<?> getAnnotation(Class<?> c) {
+      // we should get only declared annotations, not inherited ones
+      Annotation[] anns = c.getDeclaredAnnotations();
+
+      for (Annotation ann : anns) {
+        // Hadoop clearly got it wrong for not making the annotation values (private, public,
..)
+        // an enum instead we have three independent annotations!
+        Class<?> type = ann.annotationType();
+        if (isInterfaceAudienceClass(type)) {
+          return type;
+        }
+      }
+      return null;
+    }
+  }
+
+  /** Selects classes with one of the {@link InterfaceStability} annotation in their class
+   * declaration.
+   */
+  class InterfaceStabilityAnnotatedClassFilter implements ClassFinder.ClassFilter {
+    @Override
+    public boolean isCandidateClass(Class<?> c) {
+      if (getAnnotation(c) != null) {
+        // class itself has a declared annotation.
+        return true;
+      }
+      return false;
+    }
+
+    protected Class<?> getAnnotation(Class<?> c) {
+      // we should get only declared annotations, not inherited ones
+      Annotation[] anns = c.getDeclaredAnnotations();
+
+      for (Annotation ann : anns) {
+        // Hadoop clearly got it wrong for not making the annotation values (private, public,
..)
+        // an enum instead we have three independent annotations!
+        Class<?> type = ann.annotationType();
+        if (isInterfaceStabilityClass(type)) {
+          return type;
+        }
+      }
+      return null;
+    }
+  }
+
+  /** Selects classes with one of the {@link InterfaceAudience.Public} annotation in their
+   * class declaration.
+   */
+  class InterfaceAudiencePublicAnnotatedClassFilter extends InterfaceAudienceAnnotatedClassFilter
{
+    @Override
+    public boolean isCandidateClass(Class<?> c) {
+      return (InterfaceAudience.Public.class.equals(getAnnotation(c)));
+    }
+  }
+
+  /**
+   * Selects InterfaceAudience or InterfaceStability classes. Don't go meta!!!
+   */
+  class IsInterfaceStabilityClassFilter implements ClassFinder.ClassFilter {
+    @Override
+    public boolean isCandidateClass(Class<?> c) {
+      return
+          isInterfaceAudienceClass(c) ||
+          isInterfaceStabilityClass(c);
+    }
+  }
+
+  private boolean isInterfaceAudienceClass(Class<?> c) {
+    return
+        c.equals(InterfaceAudience.Public.class) ||
+        c.equals(InterfaceAudience.Private.class) ||
+        c.equals(InterfaceAudience.LimitedPrivate.class);
+  }
+
+  private boolean isInterfaceStabilityClass(Class<?> c) {
+    return
+        c.equals(InterfaceStability.Stable.class) ||
+        c.equals(InterfaceStability.Unstable.class) ||
+        c.equals(InterfaceStability.Evolving.class);
+  }
+
+  /** Selects classes that are declared public */
+  class PublicClassFilter implements ClassFinder.ClassFilter {
+    @Override
+    public boolean isCandidateClass(Class<?> c) {
+      int mod = c.getModifiers();
+      return Modifier.isPublic(mod);
+    }
+  }
+
+  /** Selects paths (jars and class dirs) only from the main code, not test classes */
+  class MainCodeResourcePathFilter implements ClassFinder.ResourcePathFilter {
+    @Override
+    public boolean isCandidatePath(String resourcePath, boolean isJar) {
+      return !resourcePath.contains("test-classes") &&
+          !resourcePath.contains("tests.jar");
+    }
+  }
+
+  /**
+   * Selects classes that appear to be source instrumentation from Clover.
+   * Clover generates instrumented code in order to calculate coverage. Part of the
+   * generated source is a static inner class on each source class.
+   *
+   * - has an enclosing class
+   * - enclosing class is not an interface
+   * - name starts with "__CLR"
+   */
+  class CloverInstrumentationFilter implements ClassFinder.ClassFilter {
+    @Override
+    public boolean isCandidateClass(Class<?> clazz) {
+      boolean clover = false;
+      final Class<?> enclosing = clazz.getEnclosingClass();
+      if (enclosing != null) {
+        if (!(enclosing.isInterface())) {
+          clover = clazz.getSimpleName().startsWith("__CLR");
+        }
+      }
+      return clover;
+    }
+  }
+
+  /**
+   * Checks whether all the classes in client and common modules contain
+   * {@link InterfaceAudience} annotations.
+   */
+  @Test
+  public void testInterfaceAudienceAnnotation()
+      throws ClassNotFoundException, IOException, LinkageError {
+
+    // find classes that are:
+    // In the main jar
+    // AND are not in a hadoop-compat module
+    // AND are public
+    // NOT test classes
+    // AND NOT generated classes
+    // AND are NOT annotated with InterfaceAudience
+    // AND are NOT from Clover rewriting sources
+    ClassFinder classFinder = new ClassFinder(
+      new And(new MainCodeResourcePathFilter(),
+              new TestFileNameFilter()),
+      new Not((FileNameFilter)new TestFileNameFilter()),
+      new And(new PublicClassFilter(),
+              new Not(new TestClassFilter()),
+              new Not(new GeneratedClassFilter()),
+              new Not(new IsInterfaceStabilityClassFilter()),
+              new Not(new InterfaceAudienceAnnotatedClassFilter()),
+              new Not(new CloverInstrumentationFilter()))
+    );
+
+    Set<Class<?>> classes = classFinder.findClasses(false);
+
+    LOG.info("These are the classes that DO NOT have @InterfaceAudience annotation:");
+    for (Class<?> clazz : classes) {
+      LOG.info(clazz);
+    }
+
+    Assert.assertEquals("All classes should have @InterfaceAudience annotation",
+      0, classes.size());
+  }
+
+  /**
+   * Checks whether all the classes in client and common modules that are marked
+   * InterfaceAudience.Public also have {@link InterfaceStability} annotations.
+   */
+  @Test
+  public void testInterfaceStabilityAnnotation()
+      throws ClassNotFoundException, IOException, LinkageError {
+
+    // find classes that are:
+    // In the main jar
+    // AND are not in a hadoop-compat module
+    // AND are public
+    // NOT test classes
+    // AND NOT generated classes
+    // AND are annotated with InterfaceAudience.Public
+    // AND NOT annotated with InterfaceStability
+    ClassFinder classFinder = new ClassFinder(
+      new And(new MainCodeResourcePathFilter(),
+              new TestFileNameFilter()),
+      new Not((FileNameFilter)new TestFileNameFilter()),
+      new And(new PublicClassFilter(),
+              new Not(new TestClassFilter()),
+              new Not(new GeneratedClassFilter()),
+              new InterfaceAudiencePublicAnnotatedClassFilter(),
+              new Not(new IsInterfaceStabilityClassFilter()),
+              new Not(new InterfaceStabilityAnnotatedClassFilter()))
+    );
+
+    Set<Class<?>> classes = classFinder.findClasses(false);
+
+    LOG.info("These are the classes that DO NOT have @InterfaceStability annotation:");
+    for (Class<?> clazz : classes) {
+      LOG.info(clazz);
+    }
+
+    Assert.assertEquals("All classes that are marked with @InterfaceAudience.Public should
"
+        + "have @InterfaceStability annotation as well",
+      0, classes.size());
+  }
+}

http://git-wip-us.apache.org/repos/asf/hbase/blob/5f18e790/hbase-common/src/test/java/org/apache/hadoop/hbase/ClassFinder.java
----------------------------------------------------------------------
diff --git a/hbase-common/src/test/java/org/apache/hadoop/hbase/ClassFinder.java b/hbase-common/src/test/java/org/apache/hadoop/hbase/ClassFinder.java
index 703d15d..2e0436c 100644
--- a/hbase-common/src/test/java/org/apache/hadoop/hbase/ClassFinder.java
+++ b/hbase-common/src/test/java/org/apache/hadoop/hbase/ClassFinder.java
@@ -62,6 +62,58 @@ public class ClassFinder {
     boolean isCandidateClass(Class<?> c);
   };
 
+  public static class Not implements ResourcePathFilter, FileNameFilter, ClassFilter {
+    private ResourcePathFilter resourcePathFilter;
+    private FileNameFilter fileNameFilter;
+    private ClassFilter classFilter;
+
+    public Not(ResourcePathFilter resourcePathFilter){this.resourcePathFilter = resourcePathFilter;}
+    public Not(FileNameFilter fileNameFilter){this.fileNameFilter = fileNameFilter;}
+    public Not(ClassFilter classFilter){this.classFilter = classFilter;}
+
+    @Override
+    public boolean isCandidatePath(String resourcePath, boolean isJar) {
+      return !resourcePathFilter.isCandidatePath(resourcePath, isJar);
+    }
+    @Override
+    public boolean isCandidateFile(String fileName, String absFilePath) {
+      return !fileNameFilter.isCandidateFile(fileName, absFilePath);
+    }
+    @Override
+    public boolean isCandidateClass(Class<?> c) {
+      return !classFilter.isCandidateClass(c);
+    }
+  }
+
+  public static class And implements ClassFilter, ResourcePathFilter {
+    ClassFilter[] classFilters;
+    ResourcePathFilter[] resourcePathFilters;
+
+    public And(ClassFilter...classFilters) { this.classFilters = classFilters; }
+    public And(ResourcePathFilter... resourcePathFilters) {
+      this.resourcePathFilters = resourcePathFilters;
+    }
+
+    @Override
+    public boolean isCandidateClass(Class<?> c) {
+      for (ClassFilter filter : classFilters) {
+        if (!filter.isCandidateClass(c)) {
+          return false;
+        }
+      }
+      return true;
+    }
+
+    @Override public boolean isCandidatePath(String resourcePath, boolean isJar) {
+      for (ResourcePathFilter filter : resourcePathFilters) {
+        if (!filter.isCandidatePath(resourcePath, isJar)) {
+          return false;
+        }
+      }
+      return true;
+    }
+  }
+
   public ClassFinder() {
     this(null, null, null);
   }

http://git-wip-us.apache.org/repos/asf/hbase/blob/5f18e790/hbase-common/src/test/java/org/apache/hadoop/hbase/ClassTestFinder.java
----------------------------------------------------------------------
diff --git a/hbase-common/src/test/java/org/apache/hadoop/hbase/ClassTestFinder.java b/hbase-common/src/test/java/org/apache/hadoop/hbase/ClassTestFinder.java
index 18368a4..fe88623 100644
--- a/hbase-common/src/test/java/org/apache/hadoop/hbase/ClassTestFinder.java
+++ b/hbase-common/src/test/java/org/apache/hadoop/hbase/ClassTestFinder.java
@@ -50,6 +50,7 @@ public class ClassTestFinder extends ClassFinder {
     return new Class<?>[0];
   }
 
+  /** Filters both test classes and anything in the hadoop-compat modules */
   public static class TestFileNameFilter implements FileNameFilter, ResourcePathFilter {
     private static final Pattern hadoopCompactRe =
         Pattern.compile("hbase-hadoop\\d?-compat");


Mime
View raw message