Return-Path: X-Original-To: archive-asf-public-internal@cust-asf2.ponee.io Delivered-To: archive-asf-public-internal@cust-asf2.ponee.io Received: from cust-asf.ponee.io (cust-asf.ponee.io [163.172.22.183]) by cust-asf2.ponee.io (Postfix) with ESMTP id 876BD200C16 for ; Thu, 9 Feb 2017 16:56:24 +0100 (CET) Received: by cust-asf.ponee.io (Postfix) id 85E5A160B50; Thu, 9 Feb 2017 15:56:24 +0000 (UTC) Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by cust-asf.ponee.io (Postfix) with SMTP id B5A9F160B4C for ; Thu, 9 Feb 2017 16:56:22 +0100 (CET) Received: (qmail 24745 invoked by uid 500); 9 Feb 2017 15:56:22 -0000 Mailing-List: contact commits-help@beam.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@beam.apache.org Delivered-To: mailing list commits@beam.apache.org Received: (qmail 24736 invoked by uid 99); 9 Feb 2017 15:56:21 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 09 Feb 2017 15:56:21 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id CE503DFD9E; Thu, 9 Feb 2017 15:56:21 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: staslevin@apache.org To: commits@beam.apache.org Date: Thu, 09 Feb 2017 15:56:21 -0000 Message-Id: <24e86f8c3b354dd89b851e0f503159fd@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: [1/2] beam git commit: [BEAM-882, BEAM-883, BEAM-878] Simplified API surface verifications. archived-at: Thu, 09 Feb 2017 15:56:24 -0000 Repository: beam Updated Branches: refs/heads/master e21f9ae86 -> 6b31c14fa [BEAM-882,BEAM-883,BEAM-878] Simplified API surface verifications. Project: http://git-wip-us.apache.org/repos/asf/beam/repo Commit: http://git-wip-us.apache.org/repos/asf/beam/commit/cde550fe Tree: http://git-wip-us.apache.org/repos/asf/beam/tree/cde550fe Diff: http://git-wip-us.apache.org/repos/asf/beam/diff/cde550fe Branch: refs/heads/master Commit: cde550fe9b90be5fc9548735d5854359fca6a9cf Parents: e21f9ae Author: Stas Levin Authored: Mon Jan 16 16:20:25 2017 +0200 Committer: Stas Levin Committed: Thu Feb 9 17:40:36 2017 +0200 ---------------------------------------------------------------------- .../org/apache/beam/sdk/util/ApiSurface.java | 446 +++++++++++++------ .../org/apache/beam/SdkCoreApiSurfaceTest.java | 62 +++ .../apache/beam/sdk/util/ApiSurfaceTest.java | 152 ++----- .../apache/beam/sdk/io/gcp/ApiSurfaceTest.java | 134 ------ .../beam/sdk/io/gcp/GcpApiSurfaceTest.java | 79 ++++ 5 files changed, 495 insertions(+), 378 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/beam/blob/cde550fe/sdks/java/core/src/main/java/org/apache/beam/sdk/util/ApiSurface.java ---------------------------------------------------------------------- diff --git a/sdks/java/core/src/main/java/org/apache/beam/sdk/util/ApiSurface.java b/sdks/java/core/src/main/java/org/apache/beam/sdk/util/ApiSurface.java index 2040161..9530e88 100644 --- a/sdks/java/core/src/main/java/org/apache/beam/sdk/util/ApiSurface.java +++ b/sdks/java/core/src/main/java/org/apache/beam/sdk/util/ApiSurface.java @@ -17,12 +17,21 @@ */ package org.apache.beam.sdk.util; +import static org.hamcrest.Matchers.anyOf; + +import com.google.common.base.Function; import com.google.common.base.Joiner; +import com.google.common.base.Predicate; import com.google.common.base.Supplier; +import com.google.common.collect.FluentIterable; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; import com.google.common.collect.Lists; import com.google.common.collect.Maps; import com.google.common.collect.Multimap; import com.google.common.collect.Multimaps; +import com.google.common.collect.Ordering; import com.google.common.collect.Sets; import com.google.common.reflect.ClassPath; import com.google.common.reflect.ClassPath.ClassInfo; @@ -45,15 +54,20 @@ import java.util.Collections; import java.util.List; import java.util.Set; import java.util.regex.Pattern; +import javax.annotation.Nonnull; +import org.hamcrest.Description; +import org.hamcrest.Matcher; +import org.hamcrest.StringDescription; +import org.hamcrest.TypeSafeDiagnosingMatcher; import org.slf4j.Logger; import org.slf4j.LoggerFactory; /** - * Represents the API surface of a package prefix. Used for accessing public classes, - * methods, and the types they reference, to control what dependencies are re-exported. + * Represents the API surface of a package prefix. Used for accessing public classes, methods, and + * the types they reference, to control what dependencies are re-exported. * - *

For the purposes of calculating the public API surface, exposure includes any public - * or protected occurrence of: + *

For the purposes of calculating the public API surface, exposure includes any public or + * protected occurrence of: * *

    *
  • superclasses @@ -66,45 +80,277 @@ import org.slf4j.LoggerFactory; *
  • wildcard bounds *
* - *

Exposure is a transitive property. The resulting map excludes primitives - * and array classes themselves. + *

Exposure is a transitive property. The resulting map excludes primitives and array classes + * themselves. * - *

It is prudent (though not required) to prune prefixes like "java" via the builder - * method {@link #pruningPrefix} to halt the traversal so it does not uselessly catalog references - * that are not interesting. + *

It is prudent (though not required) to prune prefixes like "java" via the builder method + * {@link #pruningPrefix} to halt the traversal so it does not uselessly catalog references that are + * not interesting. */ @SuppressWarnings("rawtypes") public class ApiSurface { private static final Logger LOG = LoggerFactory.getLogger(ApiSurface.class); + /** A factory method to create a {@link Class} matcher for classes residing in a given package. */ + public static Matcher> classesInPackage(final String packageName) { + return new Matchers.ClassInPackage(packageName); + } + /** - * Returns an empty {@link ApiSurface}. + * A factory method to create an {@link ApiSurface} matcher, producing a positive match if the + * queried api surface contains ONLY classes described by the provided matchers. */ - public static ApiSurface empty() { - LOG.debug("Returning an empty ApiSurface"); - return new ApiSurface(Collections.>emptySet(), Collections.emptySet()); + public static Matcher containsOnlyClassesMatching( + final Set>> classMatchers) { + return new Matchers.ClassesInSurfaceMatcher(classMatchers); + } + + /** See {@link ApiSurface#containsOnlyClassesMatching(Set)}. */ + @SafeVarargs + public static Matcher containsOnlyClassesMatching( + final Matcher>... classMatchers) { + return new Matchers.ClassesInSurfaceMatcher(Sets.newHashSet(classMatchers)); + } + + /** See {@link ApiSurface#containsOnlyPackages(Set)}. */ + public static Matcher containsOnlyPackages(final String... packageNames) { + return containsOnlyPackages(Sets.newHashSet(packageNames)); } /** - * Returns an {@link ApiSurface} object representing the given package and all subpackages. + * A factory method to create an {@link ApiSurface} matcher, producing a positive match if the + * queried api surface contains classes ONLY from specified package names. */ - public static ApiSurface ofPackage(String packageName) throws IOException { - return ApiSurface.empty().includingPackage(packageName); + public static Matcher containsOnlyPackages(final Set packageNames) { + + final Function>> packageNameToClassMatcher = + new Function>>() { + + @Override + public Matcher> apply(@Nonnull final String packageName) { + return classesInPackage(packageName); + } + }; + + final ImmutableSet>> classesInPackages = + FluentIterable.from(packageNames).transform(packageNameToClassMatcher).toSet(); + + return containsOnlyClassesMatching(classesInPackages); } /** - * Returns an {@link ApiSurface} object representing just the surface of the given class. + * {@link Matcher}s for use in {@link ApiSurface} related tests that aim to keep the public API + * conformant to a hard-coded policy by controlling what classes are allowed to be exposed by an + * API surface. */ + // based on previous code by @kennknowles and others. + private static class Matchers { + + private static class ClassInPackage extends TypeSafeDiagnosingMatcher> { + + private final String packageName; + + private ClassInPackage(final String packageName) { + this.packageName = packageName; + } + + @Override + public void describeTo(final Description description) { + description.appendText("Classes in package \""); + description.appendText(packageName); + description.appendText("\""); + } + + @Override + protected boolean matchesSafely(final Class clazz, final Description mismatchDescription) { + return clazz.getName().startsWith(packageName + "."); + } + } + + private static class ClassesInSurfaceMatcher extends TypeSafeDiagnosingMatcher { + + private final Set>> classMatchers; + + private ClassesInSurfaceMatcher(final Set>> classMatchers) { + this.classMatchers = classMatchers; + } + + private boolean verifyNoAbandoned( + final ApiSurface checkedApiSurface, + final Set>> allowedClasses, + final Description mismatchDescription) { + + // + + final Function>, String> toMessage = + new Function>, String>() { + + @Override + public String apply(@Nonnull final Matcher> abandonedClassMacther) { + final StringDescription description = new StringDescription(); + description.appendText("No "); + abandonedClassMacther.describeTo(description); + return description.toString(); + } + }; + + final Predicate>> matchedByExposedClasses = + new Predicate>>() { + + @Override + public boolean apply(@Nonnull final Matcher> classMatcher) { + return FluentIterable.from(checkedApiSurface.getExposedClasses()) + .anyMatch( + new Predicate>() { + + @Override + public boolean apply(@Nonnull final Class aClass) { + return classMatcher.matches(aClass); + } + }); + } + }; + + // + + final ImmutableSet>> matchedClassMatchers = + FluentIterable.from(allowedClasses).filter(matchedByExposedClasses).toSet(); + + final Sets.SetView>> abandonedClassMatchers = + Sets.difference(allowedClasses, matchedClassMatchers); + + final ImmutableList messages = + FluentIterable.from(abandonedClassMatchers) + .transform(toMessage) + .toSortedList(Ordering.natural()); + + if (!messages.isEmpty()) { + mismatchDescription.appendText( + "The following white-listed scopes did not have matching classes on the API surface:" + + "\n\t" + + Joiner.on("\n\t").join(messages)); + } + + return messages.isEmpty(); + } + + private boolean verifyNoDisallowed( + final ApiSurface checkedApiSurface, + final Set>> allowedClasses, + final Description mismatchDescription) { + + /* */ + + final Function, List>> toExposure = + new Function, List>>() { + + @Override + public List> apply(@Nonnull final Class aClass) { + return checkedApiSurface.getAnyExposurePath(aClass); + } + }; + + final Maps.EntryTransformer, List>, String> toMessage = + new Maps.EntryTransformer, List>, String>() { + + @Override + public String transformEntry( + @Nonnull final Class aClass, @Nonnull final List> exposure) { + return aClass + " exposed via:\n\t\t" + Joiner.on("\n\t\t").join(exposure); + } + }; + + final Predicate> disallowed = + new Predicate>() { + + @Override + public boolean apply(@Nonnull final Class aClass) { + return !classIsAllowed(aClass, allowedClasses); + } + }; + + /* */ + + final FluentIterable> disallowedClasses = + FluentIterable.from(checkedApiSurface.getExposedClasses()).filter(disallowed); + + final ImmutableMap, List>> exposures = + Maps.toMap(disallowedClasses, toExposure); + + final ImmutableList messages = + FluentIterable.from(Maps.transformEntries(exposures, toMessage).values()) + .toSortedList(Ordering.natural()); + + if (!messages.isEmpty()) { + mismatchDescription.appendText( + "The following disallowed classes appeared on the API surface:\n\t" + + Joiner.on("\n\t").join(messages)); + } + + return messages.isEmpty(); + } + + @SuppressWarnings({"rawtypes", "unchecked"}) + private boolean classIsAllowed( + final Class clazz, final Set>> allowedClasses) { + // Safe cast inexpressible in Java without rawtypes + return anyOf((Iterable) allowedClasses).matches(clazz); + } + + @Override + protected boolean matchesSafely( + final ApiSurface apiSurface, final Description mismatchDescription) { + final boolean noDisallowed = + verifyNoDisallowed(apiSurface, classMatchers, mismatchDescription); + + final boolean noAbandoned = + verifyNoAbandoned(apiSurface, classMatchers, mismatchDescription); + + return noDisallowed & noAbandoned; + } + + @Override + public void describeTo(final Description description) { + description.appendText("API surface to include only:" + "\n\t"); + for (final Matcher> classMatcher : classMatchers) { + classMatcher.describeTo(description); + description.appendText("\n\t"); + } + } + } + } + + /////////////// + + /** Returns an empty {@link ApiSurface}. */ + public static ApiSurface empty() { + LOG.debug("Returning an empty ApiSurface"); + return new ApiSurface(Collections.>emptySet(), Collections.emptySet()); + } + + /** Returns an {@link ApiSurface} object representing the given package and all subpackages. */ + public static ApiSurface ofPackage(String packageName, ClassLoader classLoader) + throws IOException { + return ApiSurface.empty().includingPackage(packageName, classLoader); + } + + /** Returns an {@link ApiSurface} object representing the given package and all subpackages. */ + public static ApiSurface ofPackage(Package aPackage, ClassLoader classLoader) throws IOException { + return ofPackage(aPackage.getName(), classLoader); + } + + /** Returns an {@link ApiSurface} object representing just the surface of the given class. */ public static ApiSurface ofClass(Class clazz) { return ApiSurface.empty().includingClass(clazz); } /** - * Returns an {@link ApiSurface} like this one, but also including the named - * package and all of its subpackages. + * Returns an {@link ApiSurface} like this one, but also including the named package and all of + * its subpackages. */ - public ApiSurface includingPackage(String packageName) throws IOException { - ClassPath classPath = ClassPath.from(ClassLoader.getSystemClassLoader()); + public ApiSurface includingPackage(String packageName, ClassLoader classLoader) + throws IOException { + ClassPath classPath = ClassPath.from(classLoader); Set> newRootClasses = Sets.newHashSet(); for (ClassInfo classInfo : classPath.getTopLevelClassesRecursive(packageName)) { @@ -119,9 +365,7 @@ public class ApiSurface { return new ApiSurface(newRootClasses, patternsToPrune); } - /** - * Returns an {@link ApiSurface} like this one, but also including the given class. - */ + /** Returns an {@link ApiSurface} like this one, but also including the given class. */ public ApiSurface includingClass(Class clazz) { Set> newRootClasses = Sets.newHashSet(); LOG.debug("Including class {}", clazz); @@ -131,32 +375,28 @@ public class ApiSurface { } /** - * Returns an {@link ApiSurface} like this one, but pruning transitive - * references from classes whose full name (including package) begins with the provided prefix. + * Returns an {@link ApiSurface} like this one, but pruning transitive references from classes + * whose full name (including package) begins with the provided prefix. */ public ApiSurface pruningPrefix(String prefix) { return pruningPattern(Pattern.compile(Pattern.quote(prefix) + ".*")); } - /** - * Returns an {@link ApiSurface} like this one, but pruning references from the named - * class. - */ + /** Returns an {@link ApiSurface} like this one, but pruning references from the named class. */ public ApiSurface pruningClassName(String className) { return pruningPattern(Pattern.compile(Pattern.quote(className))); } /** - * Returns an {@link ApiSurface} like this one, but pruning references from the - * provided class. + * Returns an {@link ApiSurface} like this one, but pruning references from the provided class. */ public ApiSurface pruningClass(Class clazz) { return pruningClassName(clazz.getName()); } /** - * Returns an {@link ApiSurface} like this one, but pruning transitive - * references from classes whose full name (including package) begins with the provided prefix. + * Returns an {@link ApiSurface} like this one, but pruning transitive references from classes + * whose full name (including package) begins with the provided prefix. */ public ApiSurface pruningPattern(Pattern pattern) { Set newPatterns = Sets.newHashSet(); @@ -165,35 +405,26 @@ public class ApiSurface { return new ApiSurface(rootClasses, newPatterns); } - /** - * See {@link #pruningPattern(Pattern)}. - */ + /** See {@link #pruningPattern(Pattern)}. */ public ApiSurface pruningPattern(String patternString) { return pruningPattern(Pattern.compile(patternString)); } - /** - * Returns all public classes originally belonging to the package - * in the {@link ApiSurface}. - */ + /** Returns all public classes originally belonging to the package in the {@link ApiSurface}. */ public Set> getRootClasses() { return rootClasses; } - /** - * Returns exposed types in this set, including arrays and primitives as - * specified. - */ + /** Returns exposed types in this set, including arrays and primitives as specified. */ public Set> getExposedClasses() { return getExposedToExposers().keySet(); } /** - * Returns a path from an exposed class to a root class. There may be many, but this - * gives only one. + * Returns a path from an exposed class to a root class. There may be many, but this gives only + * one. * - *

If there are only cycles, with no path back to a root class, throws - * IllegalStateException. + *

If there are only cycles, with no path back to a root class, throws IllegalStateException. */ public List> getAnyExposurePath(Class exposedClass) { Set> excluded = Sets.newHashSet(); @@ -201,16 +432,18 @@ public class ApiSurface { List> path = getAnyExposurePath(exposedClass, excluded); if (path == null) { throw new IllegalArgumentException( - "Class " + exposedClass + " has no path back to any root class." - + " It should never have been considered exposed."); + "Class " + + exposedClass + + " has no path back to any root class." + + " It should never have been considered exposed."); } else { return path; } } /** - * Returns a path from an exposed class to a root class. There may be many, but this - * gives only one. It will not return a path that crosses the excluded classes. + * Returns a path from an exposed class to a root class. There may be many, but this gives only + * one. It will not return a path that crosses the excluded classes. * *

If there are only cycles or paths through the excluded classes, returns null. * @@ -235,9 +468,8 @@ public class ApiSurface { return exposurePath; } - List> restOfPath = getAnyExposurePath( - exposer, - Sets.union(excluded, Sets.newHashSet(exposer))); + List> restOfPath = + getAnyExposurePath(exposer, Sets.union(excluded, Sets.newHashSet(exposer))); if (restOfPath != null) { exposurePath.addAll(restOfPath); @@ -264,8 +496,8 @@ public class ApiSurface { } /** - * A map from exposed types to place where they are exposed, in the sense of being a part - * of a public-facing API surface. + * A map from exposed types to place where they are exposed, in the sense of being a part of a + * public-facing API surface. * *

This map is the adjencency list representation of a directed graph, where an edge from type * {@code T1} to type {@code T2} indicates that {@code T2} directly exposes {@code T1} in its API @@ -281,28 +513,25 @@ public class ApiSurface { return exposedToExposers; } - /** - * See {@link #getExposedToExposers}. - */ + /** See {@link #getExposedToExposers}. */ private void constructExposedToExposers() { visited = Sets.newHashSet(); - exposedToExposers = Multimaps.newSetMultimap( - Maps., Collection>>newHashMap(), - new Supplier>>() { - @Override - public Set> get() { - return Sets.newHashSet(); - } - }); + exposedToExposers = + Multimaps.newSetMultimap( + Maps., Collection>>newHashMap(), + new Supplier>>() { + @Override + public Set> get() { + return Sets.newHashSet(); + } + }); for (Class clazz : rootClasses) { addExposedTypes(clazz, null); } } - /** - * A combined {@code Pattern} that implements all the pruning specified. - */ + /** A combined {@code Pattern} that implements all the pruning specified. */ private Pattern getPrunedPattern() { if (prunedPattern == null) { constructPrunedPattern(); @@ -310,9 +539,7 @@ public class ApiSurface { return prunedPattern; } - /** - * See {@link #getPrunedPattern}. - */ + /** See {@link #getPrunedPattern}. */ private void constructPrunedPattern() { Set prunedPatternStrings = Sets.newHashSet(); for (Pattern patternToPrune : patternsToPrune) { @@ -321,25 +548,19 @@ public class ApiSurface { prunedPattern = Pattern.compile("(" + Joiner.on(")|(").join(prunedPatternStrings) + ")"); } - /** - * Whether a type and all that it references should be pruned from the graph. - */ + /** Whether a type and all that it references should be pruned from the graph. */ private boolean pruned(Type type) { return pruned(TypeToken.of(type).getRawType()); } - /** - * Whether a class and all that it references should be pruned from the graph. - */ + /** Whether a class and all that it references should be pruned from the graph. */ private boolean pruned(Class clazz) { return clazz.isPrimitive() || clazz.isArray() || getPrunedPattern().matcher(clazz.getName()).matches(); } - /** - * Whether a type has already beens sufficiently processed. - */ + /** Whether a type has already beens sufficiently processed. */ private boolean done(Type type) { return visited.contains(type); } @@ -356,9 +577,7 @@ public class ApiSurface { visited.add(type); } - /** - * See {@link #addExposedTypes(Type, Class)}. - */ + /** See {@link #addExposedTypes(Type, Class)}. */ private void addExposedTypes(TypeToken type, Class cause) { LOG.debug( "Adding exposed types from {}, which is the type in type token {}", type.getType(), type); @@ -366,9 +585,9 @@ public class ApiSurface { } /** - * Adds any references learned by following a link from {@code cause} to {@code type}. - * This will dispatch according to the concrete {@code Type} implementation. See the - * other overloads of {@code addExposedTypes} for their details. + * Adds any references learned by following a link from {@code cause} to {@code type}. This will + * dispatch according to the concrete {@code Type} implementation. See the other overloads of + * {@code addExposedTypes} for their details. */ private void addExposedTypes(Type type, Class cause) { if (type instanceof TypeVariable) { @@ -392,8 +611,7 @@ public class ApiSurface { } /** - * Adds any types exposed to this set. These will - * come from the (possibly absent) bounds on the + * Adds any types exposed to this set. These will come from the (possibly absent) bounds on the * type variable. */ private void addExposedTypes(TypeVariable type, Class cause) { @@ -430,9 +648,8 @@ public class ApiSurface { } /** - * Adds any types exposed from the given array type. The array type itself is not added. The - * cause of the exposure of the underlying type is considered whatever type exposed the array - * type. + * Adds any types exposed from the given array type. The array type itself is not added. The cause + * of the exposure of the underlying type is considered whatever type exposed the array type. */ private void addExposedTypes(GenericArrayType type, Class cause) { if (done(type)) { @@ -447,9 +664,8 @@ public class ApiSurface { } /** - * Adds any types exposed to this set. Even if the - * root type is to be pruned, the actual type arguments - * are processed. + * Adds any types exposed to this set. Even if the root type is to be pruned, the actual type + * arguments are processed. */ private void addExposedTypes(ParameterizedType type, Class cause) { // Even if the type is already done, this link to it may be new @@ -482,9 +698,8 @@ public class ApiSurface { } /** - * Adds a class and all of the types it exposes. The cause - * of the class being exposed is given, and the cause - * of everything within the class is that class itself. + * Adds a class and all of the types it exposes. The cause of the class being exposed is given, + * and the cause of everything within the class is that class itself. */ private void addExposedTypes(Class clazz, Class cause) { if (pruned(clazz)) { @@ -535,7 +750,7 @@ public class ApiSurface { "Adding exposed types from {}, which is an annotation on invokable {}", annotation, invokable); - addExposedTypes(annotation.annotationType(), cause); + addExposedTypes(annotation.annotationType(), cause); } for (Parameter parameter : invokable.getParameters()) { LOG.debug( @@ -577,9 +792,7 @@ public class ApiSurface { } } - /** - * Returns an {@link Invokable} for each public methods or constructors of a type. - */ + /** Returns an {@link Invokable} for each public methods or constructors of a type. */ private Set getExposedInvokables(TypeToken type) { Set invokables = Sets.newHashSet(); @@ -598,14 +811,11 @@ public class ApiSurface { return invokables; } - /** - * Returns true of the given modifier bitmap indicates exposure (public or protected access). - */ + /** Returns true of the given modifier bitmap indicates exposure (public or protected access). */ private boolean exposed(int modifiers) { return 0 != (modifiers & (Modifier.PUBLIC | Modifier.PROTECTED)); } - //////////////////////////////////////////////////////////////////////////// /** @@ -613,25 +823,11 @@ public class ApiSurface { * *

Note that our idea of "public" does not include various internal-only APIs. */ - public static ApiSurface getSdkApiSurface() throws IOException { - return ApiSurface.ofPackage("org.apache.beam") + public static ApiSurface getSdkApiSurface(final ClassLoader classLoader) throws IOException { + return ApiSurface.ofPackage("org.apache.beam", classLoader) .pruningPattern("org[.]apache[.]beam[.].*Test") - // Exposes Guava, but not intended for users .pruningClassName("org.apache.beam.sdk.util.common.ReflectHelpers") .pruningPrefix("java"); } - - public static void main(String[] args) throws Exception { - List names = Lists.newArrayList(); - for (Class clazz : getSdkApiSurface().getExposedClasses()) { - names.add(clazz.getName()); - } - List sortedNames = Lists.newArrayList(names); - Collections.sort(sortedNames); - - for (String name : sortedNames) { - System.out.println(name); - } - } } http://git-wip-us.apache.org/repos/asf/beam/blob/cde550fe/sdks/java/core/src/test/java/org/apache/beam/SdkCoreApiSurfaceTest.java ---------------------------------------------------------------------- diff --git a/sdks/java/core/src/test/java/org/apache/beam/SdkCoreApiSurfaceTest.java b/sdks/java/core/src/test/java/org/apache/beam/SdkCoreApiSurfaceTest.java new file mode 100644 index 0000000..c4b3a9f --- /dev/null +++ b/sdks/java/core/src/test/java/org/apache/beam/SdkCoreApiSurfaceTest.java @@ -0,0 +1,62 @@ +/* + * 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.beam; + +import static org.apache.beam.sdk.util.ApiSurface.containsOnlyPackages; +import static org.hamcrest.MatcherAssert.assertThat; + +import com.google.common.collect.ImmutableSet; +import java.util.Set; +import org.apache.beam.sdk.util.ApiSurface; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** API surface verification for {@link org.apache.beam}. */ +@RunWith(JUnit4.class) +public class SdkCoreApiSurfaceTest { + + @Test + public void testSdkApiSurface() throws Exception { + + @SuppressWarnings("unchecked") + final Set allowed = + ImmutableSet.of( + "org.apache.beam", + "com.google.api.client", + "com.google.api.services.bigquery", + "com.google.api.services.cloudresourcemanager", + "com.google.api.services.pubsub", + "com.google.api.services.storage", + "com.google.auth", + "com.google.protobuf", + "com.fasterxml.jackson.annotation", + "com.fasterxml.jackson.core", + "com.fasterxml.jackson.databind", + "org.apache.avro", + "org.hamcrest", + // via DataflowMatchers + "org.codehaus.jackson", + // via Avro + "org.joda.time", + "org.junit"); + + assertThat( + ApiSurface.getSdkApiSurface(getClass().getClassLoader()), containsOnlyPackages(allowed)); + } +} http://git-wip-us.apache.org/repos/asf/beam/blob/cde550fe/sdks/java/core/src/test/java/org/apache/beam/sdk/util/ApiSurfaceTest.java ---------------------------------------------------------------------- diff --git a/sdks/java/core/src/test/java/org/apache/beam/sdk/util/ApiSurfaceTest.java b/sdks/java/core/src/test/java/org/apache/beam/sdk/util/ApiSurfaceTest.java index 0f3e2ff..9ed6e6c 100644 --- a/sdks/java/core/src/test/java/org/apache/beam/sdk/util/ApiSurfaceTest.java +++ b/sdks/java/core/src/test/java/org/apache/beam/sdk/util/ApiSurfaceTest.java @@ -17,138 +17,50 @@ */ package org.apache.beam.sdk.util; -import static org.hamcrest.Matchers.anyOf; +import static org.apache.beam.sdk.util.ApiSurface.containsOnlyClassesMatching; import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.emptyIterable; import static org.junit.Assert.assertThat; -import static org.junit.Assert.fail; -import com.google.common.base.Joiner; +import com.google.common.base.Function; +import com.google.common.collect.FluentIterable; import com.google.common.collect.ImmutableSet; -import com.google.common.collect.Lists; -import com.google.common.collect.Maps; +import com.google.common.collect.Iterables; import com.google.common.collect.Sets; -import java.util.Collections; import java.util.List; -import java.util.Map; -import java.util.Set; -import org.hamcrest.Description; +import javax.annotation.Nonnull; import org.hamcrest.Matcher; -import org.hamcrest.TypeSafeDiagnosingMatcher; +import org.hamcrest.Matchers; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; -/** - * Tests for ApiSurface. These both test the functionality and also that our - * public API is conformant to a hard-coded policy. - */ +/** Functionality tests for ApiSurface. */ @RunWith(JUnit4.class) public class ApiSurfaceTest { - @Test - public void testOurApiSurface() throws Exception { - ApiSurface checkedApiSurface = ApiSurface.getSdkApiSurface(); - - Map, List>> disallowedClasses = Maps.newHashMap(); - for (Class clazz : checkedApiSurface.getExposedClasses()) { - if (!classIsAllowed(clazz)) { - disallowedClasses.put(clazz, checkedApiSurface.getAnyExposurePath(clazz)); - } - } - - List disallowedMessages = Lists.newArrayList(); - for (Map.Entry, List>> entry : disallowedClasses.entrySet()) { - disallowedMessages.add(entry.getKey() + " exposed via:\n\t\t" - + Joiner.on("\n\t\t").join(entry.getValue())); - } - Collections.sort(disallowedMessages); - - if (!disallowedMessages.isEmpty()) { - fail("The following disallowed classes appear in the public API surface of the SDK:\n\t" - + Joiner.on("\n\t").join(disallowedMessages)); - } - } - - @SuppressWarnings("unchecked") - private static final Set>> ALLOWED_PACKAGES = - ImmutableSet.of( - inPackage("org.apache.beam"), - inPackage("com.google.api.client"), - inPackage("com.google.api.services.bigquery"), - inPackage("com.google.api.services.cloudresourcemanager"), - inPackage("com.google.api.services.dataflow"), - inPackage("com.google.api.services.pubsub"), - inPackage("com.google.api.services.storage"), - inPackage("com.google.auth"), - inPackage("com.google.bigtable.v1"), - inPackage("com.google.cloud.bigtable.config"), - inPackage("com.google.cloud.bigtable.grpc"), - inPackage("com.google.datastore"), - inPackage("com.google.protobuf"), - inPackage("com.google.rpc"), - inPackage("com.google.type"), - inPackage("com.fasterxml.jackson.annotation"), - inPackage("com.fasterxml.jackson.core"), - inPackage("com.fasterxml.jackson.databind"), - inPackage("com.fasterxml.jackson.deser"), - inPackage("io.grpc"), - inPackage("org.apache.avro"), - inPackage("org.apache.commons.logging"), // via BigTable - inPackage("org.hamcrest"), // via DataflowMatchers - inPackage("org.codehaus.jackson"), // via Avro - inPackage("org.joda.time"), - inPackage("org.junit"), - inPackage("java")); - @SuppressWarnings({"rawtypes", "unchecked"}) - private boolean classIsAllowed(Class clazz) { - // Safe cast inexpressible in Java without rawtypes - return anyOf((Iterable) ALLOWED_PACKAGES).matches(clazz); - } - - private static Matcher> inPackage(String packageName) { - return new ClassInPackage(packageName); - } - - private static class ClassInPackage extends TypeSafeDiagnosingMatcher> { - - private final String packageName; + private void assertExposed(final Class classToExamine, final Class... exposedClasses) { - public ClassInPackage(String packageName) { - this.packageName = packageName; - } + final ApiSurface apiSurface = ApiSurface.ofClass(classToExamine).pruningPrefix("java"); - @Override - public void describeTo(Description description) { - description.appendText("Class in package \""); - description.appendText(packageName); - description.appendText("\""); - } + final ImmutableSet>> allowed = + FluentIterable.from( + Iterables.concat(Sets.newHashSet(classToExamine), Sets.newHashSet(exposedClasses))) + .transform( + new Function>>() { - @Override - protected boolean matchesSafely(Class clazz, Description mismatchDescription) { - return clazz.getName().startsWith(packageName + "."); - } + @Override + public Matcher> apply(@Nonnull final Class input) { + return Matchers.>equalTo(input); + } + }) + .toSet(); + assertThat(apiSurface, containsOnlyClassesMatching(allowed)); } - ////////////////////////////////////////////////////////////////////////////////// - - @SuppressWarnings({"rawtypes", "unchecked"}) - private void assertExposed(Class classToExamine, Class... exposedClasses) { - ApiSurface apiSurface = ApiSurface - .ofClass(classToExamine) - .pruningPrefix("java"); - - Set expectedExposed = Sets.newHashSet(classToExamine); - for (Class clazz : exposedClasses) { - expectedExposed.add(clazz); - } - assertThat(apiSurface.getExposedClasses(), containsInAnyOrder(expectedExposed.toArray())); - } - - private interface Exposed { } + private interface Exposed {} private interface ExposedReturnType { Exposed zero(); @@ -177,7 +89,7 @@ public class ApiSurfaceTest { assertExposed(ExposedWildcardBound.class, Exposed.class); } - private interface ExposedActualTypeArgument extends List { } + private interface ExposedActualTypeArgument extends List {} @Test public void testExposedActualTypeArgument() throws Exception { @@ -186,25 +98,27 @@ public class ApiSurfaceTest { @Test public void testIgnoreAll() throws Exception { - ApiSurface apiSurface = ApiSurface.ofClass(ExposedWildcardBound.class) - .includingClass(Object.class) - .includingClass(ApiSurface.class) - .pruningPattern(".*"); + ApiSurface apiSurface = + ApiSurface.ofClass(ExposedWildcardBound.class) + .includingClass(Object.class) + .includingClass(ApiSurface.class) + .pruningPattern(".*"); assertThat(apiSurface.getExposedClasses(), emptyIterable()); } - private interface PrunedPattern { } - private interface NotPruned extends PrunedPattern { } + private interface PrunedPattern {} + + private interface NotPruned extends PrunedPattern {} @Test public void testprunedPattern() throws Exception { - ApiSurface apiSurface = ApiSurface.ofClass(NotPruned.class) - .pruningClass(PrunedPattern.class); + ApiSurface apiSurface = ApiSurface.ofClass(NotPruned.class).pruningClass(PrunedPattern.class); assertThat(apiSurface.getExposedClasses(), containsInAnyOrder((Class) NotPruned.class)); } private interface ExposedTwice { Exposed zero(); + Exposed one(); } http://git-wip-us.apache.org/repos/asf/beam/blob/cde550fe/sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/ApiSurfaceTest.java ---------------------------------------------------------------------- diff --git a/sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/ApiSurfaceTest.java b/sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/ApiSurfaceTest.java deleted file mode 100644 index 0abf01d..0000000 --- a/sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/ApiSurfaceTest.java +++ /dev/null @@ -1,134 +0,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. - */ -package org.apache.beam.sdk.io.gcp; - -import static org.hamcrest.Matchers.anyOf; -import static org.hamcrest.Matchers.equalTo; -import static org.junit.Assert.fail; - -import com.google.cloud.bigtable.grpc.BigtableInstanceName; -import com.google.cloud.bigtable.grpc.BigtableTableName; -import com.google.common.base.Joiner; -import com.google.common.collect.ImmutableSet; -import com.google.common.collect.Lists; -import com.google.common.collect.Maps; -import java.util.Collections; -import java.util.List; -import java.util.Map; -import java.util.Set; -import org.apache.beam.sdk.util.ApiSurface; -import org.hamcrest.Description; -import org.hamcrest.Matcher; -import org.hamcrest.TypeSafeDiagnosingMatcher; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.JUnit4; - -/** - * Tests for the API surface of the gcp-io module. Tests that our public API is conformant to a - * hard-coded policy. - */ -@RunWith(JUnit4.class) -public class ApiSurfaceTest { - - @Test - public void testOurApiSurface() throws Exception { - ApiSurface checkedApiSurface = - ApiSurface.ofPackage("org.apache.beam.sdk.io.gcp") - .pruningPattern("org[.]apache[.]beam[.].*Test.*") - .pruningPattern("org[.]apache[.]beam[.].*IT") - .pruningPattern("java[.]lang.*"); - - Map, List>> disallowedClasses = Maps.newHashMap(); - for (Class clazz : checkedApiSurface.getExposedClasses()) { - if (!classIsAllowed(clazz)) { - disallowedClasses.put(clazz, checkedApiSurface.getAnyExposurePath(clazz)); - } - } - - List disallowedMessages = Lists.newArrayList(); - for (Map.Entry, List>> entry : disallowedClasses.entrySet()) { - disallowedMessages.add(entry.getKey() + " exposed via:\n\t\t" - + Joiner.on("\n\t\t").join(entry.getValue())); - } - Collections.sort(disallowedMessages); - - if (!disallowedMessages.isEmpty()) { - fail("The following disallowed classes appear in the public API surface of the SDK:\n\t" - + Joiner.on("\n\t").join(disallowedMessages)); - } - } - - @SuppressWarnings("unchecked") - private static final Set>> ALLOWED_PACKAGES = - ImmutableSet.>>of( - inPackage("com.google.api.client.json"), - inPackage("com.google.api.client.util"), - inPackage("com.google.api.services.bigquery.model"), - inPackage("com.google.auth"), - inPackage("com.google.bigtable.v2"), - inPackage("com.google.cloud.bigtable.config"), - equalTo(BigtableInstanceName.class), - equalTo(BigtableTableName.class), - // https://github.com/GoogleCloudPlatform/cloud-bigtable-client/pull/1056 - inPackage("com.google.common.collect"), // via Bigtable, PR above out to fix. - inPackage("com.google.datastore.v1"), - inPackage("com.google.protobuf"), - inPackage("com.google.type"), - inPackage("com.fasterxml.jackson.annotation"), - inPackage("com.fasterxml.jackson.core"), - inPackage("com.fasterxml.jackson.databind"), - inPackage("io.grpc"), - inPackage("java"), - inPackage("javax"), - inPackage("org.apache.beam"), - inPackage("org.apache.commons.logging"), // via Bigtable - inPackage("org.joda.time")); - - @SuppressWarnings({"rawtypes", "unchecked"}) - private boolean classIsAllowed(Class clazz) { - // Safe cast inexpressible in Java without rawtypes - return anyOf((Iterable) ALLOWED_PACKAGES).matches(clazz); - } - - private static Matcher> inPackage(String packageName) { - return new ClassInPackage(packageName); - } - - private static class ClassInPackage extends TypeSafeDiagnosingMatcher> { - - private final String packageName; - - public ClassInPackage(String packageName) { - this.packageName = packageName; - } - - @Override - public void describeTo(Description description) { - description.appendText("Class in package \""); - description.appendText(packageName); - description.appendText("\""); - } - - @Override - protected boolean matchesSafely(Class clazz, Description mismatchDescription) { - return clazz.getName().startsWith(packageName + "."); - } - - } -} http://git-wip-us.apache.org/repos/asf/beam/blob/cde550fe/sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/GcpApiSurfaceTest.java ---------------------------------------------------------------------- diff --git a/sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/GcpApiSurfaceTest.java b/sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/GcpApiSurfaceTest.java new file mode 100644 index 0000000..717c6d3 --- /dev/null +++ b/sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/GcpApiSurfaceTest.java @@ -0,0 +1,79 @@ +/* + * 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.beam.sdk.io.gcp; + +import static org.apache.beam.sdk.util.ApiSurface.classesInPackage; +import static org.apache.beam.sdk.util.ApiSurface.containsOnlyClassesMatching; +import static org.hamcrest.MatcherAssert.assertThat; + +import com.google.common.collect.ImmutableSet; +import java.util.Set; +import org.apache.beam.sdk.util.ApiSurface; +import org.hamcrest.Matcher; +import org.hamcrest.Matchers; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** API surface verification for {@link org.apache.beam.sdk.io.gcp}. */ +@RunWith(JUnit4.class) +public class GcpApiSurfaceTest { + + @Test + public void testGcpApiSurface() throws Exception { + + final Package thisPackage = getClass().getPackage(); + final ClassLoader thisClassLoader = getClass().getClassLoader(); + + final ApiSurface apiSurface = + ApiSurface.ofPackage(thisPackage, thisClassLoader) + .pruningPattern("org[.]apache[.]beam[.].*Test.*") + .pruningPattern("org[.]apache[.]beam[.].*IT") + .pruningPattern("java[.]lang.*"); + + @SuppressWarnings("unchecked") + final Set>> allowedClasses = + ImmutableSet.of( + classesInPackage("com.google.api.client.json"), + classesInPackage("com.google.api.client.util"), + classesInPackage("com.google.api.services.bigquery.model"), + classesInPackage("com.google.auth"), + classesInPackage("com.google.bigtable.v2"), + classesInPackage("com.google.cloud.bigtable.config"), + Matchers.>equalTo(com.google.cloud.bigtable.grpc.BigtableInstanceName.class), + Matchers.>equalTo(com.google.cloud.bigtable.grpc.BigtableTableName.class), + // https://github.com/GoogleCloudPlatform/cloud-bigtable-client/pull/1056 + classesInPackage("com.google.common.collect"), + // via Bigtable, PR above out to fix. + classesInPackage("com.google.datastore.v1"), + classesInPackage("com.google.protobuf"), + classesInPackage("com.google.type"), + classesInPackage("com.fasterxml.jackson.annotation"), + classesInPackage("com.fasterxml.jackson.core"), + classesInPackage("com.fasterxml.jackson.databind"), + classesInPackage("io.grpc"), + classesInPackage("java"), + classesInPackage("javax"), + classesInPackage("org.apache.beam"), + classesInPackage("org.apache.commons.logging"), + // via Bigtable + classesInPackage("org.joda.time")); + + assertThat(apiSurface, containsOnlyClassesMatching(allowedClasses)); + } +}