Repository: beam Updated Branches: refs/heads/master a1a022d6b -> 26a2c47f4
[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/29ffaf38 Tree: http://git-wip-us.apache.org/repos/asf/beam/tree/29ffaf38 Diff: http://git-wip-us.apache.org/repos/asf/beam/diff/29ffaf38 Branch: refs/heads/master Commit: 29ffaf3859ba9b4d8ba8529efc96fd5e105e21a3 Parents: a1a022d Author: Stas Levin <stasle...@gmail.com> Authored: Mon Jan 16 16:20:25 2017 +0200 Committer: Kenneth Knowles <k...@google.com> Committed: Mon Jan 23 13:56:45 2017 -0800 ---------------------------------------------------------------------- .../org/apache/beam/sdk/util/ApiSurface.java | 420 ++++++++++++++----- .../org/apache/beam/SdkCoreApiSurfaceTest.java | 61 +++ .../apache/beam/sdk/util/ApiSurfaceTest.java | 152 ++----- .../apache/beam/sdk/io/gcp/ApiSurfaceTest.java | 134 ------ .../beam/sdk/io/gcp/GcpApiSurfaceTest.java | 76 ++++ 5 files changed, 484 insertions(+), 359 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/beam/blob/29ffaf38/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..b6b0b32 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. * - * <p>For the purposes of calculating the public API surface, exposure includes any public - * or protected occurrence of: + * <p>For the purposes of calculating the public API surface, exposure includes any public or + * protected occurrence of: * * <ul> * <li>superclasses @@ -66,42 +80,272 @@ import org.slf4j.LoggerFactory; * <li>wildcard bounds * </ul> * - * <p>Exposure is a transitive property. The resulting map excludes primitives - * and array classes themselves. + * <p>Exposure is a transitive property. The resulting map excludes primitives and array classes + * themselves. * - * <p>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. + * <p>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<Class<?>> classesInPackage(final String packageName) { + return new Matchers.ClassInPackage(packageName); + } + + /** + * 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 Matcher<ApiSurface> containsOnlyClassesMatching( + final Set<Matcher<Class<?>>> classMatchers) { + return new Matchers.ClassesInSurfaceMatcher(classMatchers); + } + + /** See {@link ApiSurface#containsOnlyClassesMatching(Set)}. */ + @SafeVarargs + public static Matcher<ApiSurface> containsOnlyClassesMatching( + final Matcher<Class<?>>... classMatchers) { + return new Matchers.ClassesInSurfaceMatcher(Sets.newHashSet(classMatchers)); + } + + /** See {@link ApiSurface#containsOnlyPackages(Set)}. */ + public static Matcher<ApiSurface> containsOnlyPackages(final String... packageNames) { + return containsOnlyPackages(Sets.newHashSet(packageNames)); + } + + /** + * 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 Matcher<ApiSurface> containsOnlyPackages(final Set<String> packageNames) { + + final Function<String, Matcher<Class<?>>> packageNameToClassMatcher = + new Function<String, Matcher<Class<?>>>() { + + @Override + public Matcher<Class<?>> apply(@Nonnull final String packageName) { + return classesInPackage(packageName); + } + }; + + final ImmutableSet<Matcher<Class<?>>> classesInPackages = + FluentIterable.from(packageNames).transform(packageNameToClassMatcher).toSet(); + + return containsOnlyClassesMatching(classesInPackages); + } + /** - * Returns an empty {@link ApiSurface}. + * {@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<Class<?>> { + + 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<ApiSurface> { + + private final Set<Matcher<Class<?>>> classMatchers; + + private ClassesInSurfaceMatcher(final Set<Matcher<Class<?>>> classMatchers) { + this.classMatchers = classMatchers; + } + + private boolean verifyNoAbandoned( + final ApiSurface checkedApiSurface, + final Set<Matcher<Class<?>>> allowedClasses, + final Description mismatchDescription) { + + // <helper_lambdas> + + final Function<Matcher<Class<?>>, String> toMessage = + new Function<Matcher<Class<?>>, String>() { + + @Override + public String apply(@Nonnull final Matcher<Class<?>> abandonedClassMacther) { + final StringDescription description = new StringDescription(); + description.appendText("No "); + abandonedClassMacther.describeTo(description); + return description.toString(); + } + }; + + final Predicate<Matcher<Class<?>>> matchedByExposedClasses = + new Predicate<Matcher<Class<?>>>() { + + @Override + public boolean apply(@Nonnull final Matcher<Class<?>> classMatcher) { + return FluentIterable.from(checkedApiSurface.getExposedClasses()) + .anyMatch( + new Predicate<Class<?>>() { + + @Override + public boolean apply(@Nonnull final Class<?> aClass) { + return classMatcher.matches(aClass); + } + }); + } + }; + + // </helper_lambdas> + + final ImmutableSet<Matcher<Class<?>>> matchedClassMatchers = + FluentIterable.from(allowedClasses).filter(matchedByExposedClasses).toSet(); + + final Sets.SetView<Matcher<Class<?>>> abandonedClassMatchers = + Sets.difference(allowedClasses, matchedClassMatchers); + + final ImmutableList<String> messages = + FluentIterable.from(abandonedClassMatchers) + .transform(toMessage) + .toSortedList(Ordering.<String>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<Matcher<Class<?>>> allowedClasses, + final Description mismatchDescription) { + + /* <helper_lambdas> */ + + final Function<Class<?>, List<Class<?>>> toExposure = + new Function<Class<?>, List<Class<?>>>() { + + @Override + public List<Class<?>> apply(@Nonnull final Class<?> aClass) { + return checkedApiSurface.getAnyExposurePath(aClass); + } + }; + + final Maps.EntryTransformer<Class<?>, List<Class<?>>, String> toMessage = + new Maps.EntryTransformer<Class<?>, List<Class<?>>, String>() { + + @Override + public String transformEntry( + @Nonnull final Class<?> aClass, @Nonnull final List<Class<?>> exposure) { + return aClass + " exposed via:\n\t\t" + Joiner.on("\n\t\t").join(exposure); + } + }; + + final Predicate<Class<?>> disallowed = + new Predicate<Class<?>>() { + + @Override + public boolean apply(@Nonnull final Class<?> aClass) { + return !classIsAllowed(aClass, allowedClasses); + } + }; + + /* </helper_lambdas> */ + + final FluentIterable<Class<?>> disallowedClasses = + FluentIterable.from(checkedApiSurface.getExposedClasses()).filter(disallowed); + + final ImmutableMap<Class<?>, List<Class<?>>> exposures = + Maps.toMap(disallowedClasses, toExposure); + + final ImmutableList<String> messages = + FluentIterable.from(Maps.transformEntries(exposures, toMessage).values()) + .toSortedList(Ordering.<String>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<Matcher<Class<?>>> 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<Class<?>> 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.<Class<?>>emptySet(), Collections.<Pattern>emptySet()); } - /** - * Returns an {@link ApiSurface} object representing the given package and all subpackages. - */ + /** Returns an {@link ApiSurface} object representing the given package and all subpackages. */ public static ApiSurface ofPackage(String packageName) throws IOException { return ApiSurface.empty().includingPackage(packageName); } - /** - * Returns an {@link ApiSurface} object representing just the surface of the given class. - */ + /** Returns an {@link ApiSurface} object representing the given package and all subpackages. */ + public static ApiSurface ofPackage(Package aPackage) throws IOException { + return ApiSurface.empty().includingPackage(aPackage.getName()); + } + + /** 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()); @@ -119,9 +363,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<Class<?>> newRootClasses = Sets.newHashSet(); LOG.debug("Including class {}", clazz); @@ -131,32 +373,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<Pattern> newPatterns = Sets.newHashSet(); @@ -165,35 +403,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<Class<?>> 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<Class<?>> 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. * - * <p>If there are only cycles, with no path back to a root class, throws - * IllegalStateException. + * <p>If there are only cycles, with no path back to a root class, throws IllegalStateException. */ public List<Class<?>> getAnyExposurePath(Class<?> exposedClass) { Set<Class<?>> excluded = Sets.newHashSet(); @@ -201,16 +430,18 @@ public class ApiSurface { List<Class<?>> 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. * * <p>If there are only cycles or paths through the excluded classes, returns null. * @@ -235,9 +466,8 @@ public class ApiSurface { return exposurePath; } - List<Class<?>> restOfPath = getAnyExposurePath( - exposer, - Sets.union(excluded, Sets.newHashSet(exposer))); + List<Class<?>> restOfPath = + getAnyExposurePath(exposer, Sets.union(excluded, Sets.newHashSet(exposer))); if (restOfPath != null) { exposurePath.addAll(restOfPath); @@ -264,8 +494,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. * * <p>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 +511,25 @@ public class ApiSurface { return exposedToExposers; } - /** - * See {@link #getExposedToExposers}. - */ + /** See {@link #getExposedToExposers}. */ private void constructExposedToExposers() { visited = Sets.newHashSet(); - exposedToExposers = Multimaps.newSetMultimap( - Maps.<Class<?>, Collection<Class<?>>>newHashMap(), - new Supplier<Set<Class<?>>>() { - @Override - public Set<Class<?>> get() { - return Sets.newHashSet(); - } - }); + exposedToExposers = + Multimaps.newSetMultimap( + Maps.<Class<?>, Collection<Class<?>>>newHashMap(), + new Supplier<Set<Class<?>>>() { + @Override + public Set<Class<?>> 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 +537,7 @@ public class ApiSurface { return prunedPattern; } - /** - * See {@link #getPrunedPattern}. - */ + /** See {@link #getPrunedPattern}. */ private void constructPrunedPattern() { Set<String> prunedPatternStrings = Sets.newHashSet(); for (Pattern patternToPrune : patternsToPrune) { @@ -321,25 +546,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 +575,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 +583,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 +609,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 +646,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 +662,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 +696,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 +748,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 +790,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<Invokable> getExposedInvokables(TypeToken<?> type) { Set<Invokable> invokables = Sets.newHashSet(); @@ -598,14 +809,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)); } - //////////////////////////////////////////////////////////////////////////// /** http://git-wip-us.apache.org/repos/asf/beam/blob/29ffaf38/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..547d760 --- /dev/null +++ b/sdks/java/core/src/test/java/org/apache/beam/SdkCoreApiSurfaceTest.java @@ -0,0 +1,61 @@ +/* + * 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<String> 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(), containsOnlyPackages(allowed)); + } +} http://git-wip-us.apache.org/repos/asf/beam/blob/29ffaf38/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<Class<?>, List<Class<?>>> disallowedClasses = Maps.newHashMap(); - for (Class<?> clazz : checkedApiSurface.getExposedClasses()) { - if (!classIsAllowed(clazz)) { - disallowedClasses.put(clazz, checkedApiSurface.getAnyExposurePath(clazz)); - } - } - - List<String> disallowedMessages = Lists.newArrayList(); - for (Map.Entry<Class<?>, List<Class<?>>> 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<Matcher<Class<?>>> 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<Class<?>> inPackage(String packageName) { - return new ClassInPackage(packageName); - } - - private static class ClassInPackage extends TypeSafeDiagnosingMatcher<Class<?>> { - - 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<Matcher<Class<?>>> allowed = + FluentIterable.from( + Iterables.concat(Sets.newHashSet(classToExamine), Sets.newHashSet(exposedClasses))) + .transform( + new Function<Class, Matcher<Class<?>>>() { - @Override - protected boolean matchesSafely(Class<?> clazz, Description mismatchDescription) { - return clazz.getName().startsWith(packageName + "."); - } + @Override + public Matcher<Class<?>> apply(@Nonnull final Class input) { + return Matchers.<Class<?>>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<Class> 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<Exposed> { } + private interface ExposedActualTypeArgument extends List<Exposed> {} @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/29ffaf38/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<Class<?>, List<Class<?>>> disallowedClasses = Maps.newHashMap(); - for (Class<?> clazz : checkedApiSurface.getExposedClasses()) { - if (!classIsAllowed(clazz)) { - disallowedClasses.put(clazz, checkedApiSurface.getAnyExposurePath(clazz)); - } - } - - List<String> disallowedMessages = Lists.newArrayList(); - for (Map.Entry<Class<?>, List<Class<?>>> 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<Matcher<? extends Class<?>>> ALLOWED_PACKAGES = - ImmutableSet.<Matcher<? extends Class<?>>>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<Class<?>> inPackage(String packageName) { - return new ClassInPackage(packageName); - } - - private static class ClassInPackage extends TypeSafeDiagnosingMatcher<Class<?>> { - - 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/29ffaf38/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..542fd53 --- /dev/null +++ b/sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/GcpApiSurfaceTest.java @@ -0,0 +1,76 @@ +/* + * 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 ApiSurface apiSurface = + ApiSurface.ofPackage(getClass().getPackage()) + .pruningPattern("org[.]apache[.]beam[.].*Test.*") + .pruningPattern("org[.]apache[.]beam[.].*IT") + .pruningPattern("java[.]lang.*"); + + @SuppressWarnings("unchecked") + final Set<Matcher<Class<?>>> 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.<Class<?>>equalTo(com.google.cloud.bigtable.grpc.BigtableInstanceName.class), + Matchers.<Class<?>>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)); + } +}