From a50ef39c0452278c4ad474019b394f5034f3ca76 Mon Sep 17 00:00:00 2001 From: mcilwain Date: Fri, 29 Sep 2017 14:20:02 -0700 Subject: [PATCH] Fix mismatch in types of Predicates being used We're going to need to switch away from Guava's Functions and Predicates for everything and replace them with the java.util versions. Unfortunately there does not appear to be an automated tool to do this all at once. Refaster got close but doesn't seem to care about these particular types of mismatch (I suspect we're using a different version of the JDK than the outside world; ours is OK with Guava classes). This also bumps up Guava to 0.23, which is needed for some new functionality used in combination with Java 8 features. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=170531539 --- .../registry/export/ExportConstants.java | 23 ++++----- .../registry/model/ofy/ObjectifyService.java | 16 ++++--- java/google/registry/repositories.bzl | 14 +++--- .../request/lock/LockHandlerImpl.java | 10 +++- .../registry/util/AppEngineTimeLimiter.java | 2 +- java/google/registry/util/TypeUtils.java | 9 +--- .../registry/model/EntityClassesTest.java | 48 ++++++++++--------- .../google/registry/server/TestServer.java | 21 ++++---- .../google/registry/server/UrlChecker.java | 5 +- 9 files changed, 80 insertions(+), 68 deletions(-) diff --git a/java/google/registry/export/ExportConstants.java b/java/google/registry/export/ExportConstants.java index d10d61ab9..82bf77fa6 100644 --- a/java/google/registry/export/ExportConstants.java +++ b/java/google/registry/export/ExportConstants.java @@ -14,11 +14,10 @@ package google.registry.export; -import static com.google.common.base.Predicates.not; +import static com.google.common.collect.ImmutableSortedSet.toImmutableSortedSet; import static google.registry.model.EntityClasses.CLASS_TO_KIND_FUNCTION; import static google.registry.util.TypeUtils.hasAnnotation; -import com.google.common.collect.FluentIterable; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Ordering; import google.registry.model.EntityClasses; @@ -33,19 +32,21 @@ public final class ExportConstants { public static ImmutableSet getBackupKinds() { // Back up all entity classes that aren't annotated with @VirtualEntity (never even persisted // to Datastore, so they can't be backed up) or @NotBackedUp (intentionally omitted). - return FluentIterable.from(EntityClasses.ALL_CLASSES) - .filter(not(hasAnnotation(VirtualEntity.class))) - .filter(not(hasAnnotation(NotBackedUp.class))) - .transform(CLASS_TO_KIND_FUNCTION) - .toSortedSet(Ordering.natural()); + return EntityClasses.ALL_CLASSES + .stream() + .filter(hasAnnotation(VirtualEntity.class).negate()) + .filter(hasAnnotation(NotBackedUp.class).negate()) + .map(CLASS_TO_KIND_FUNCTION) + .collect(toImmutableSortedSet(Ordering.natural())); } /** Returns the names of kinds to import into reporting tools (e.g. BigQuery). */ public static ImmutableSet getReportingKinds() { - return FluentIterable.from(EntityClasses.ALL_CLASSES) + return EntityClasses.ALL_CLASSES + .stream() .filter(hasAnnotation(ReportedOn.class)) - .filter(not(hasAnnotation(VirtualEntity.class))) - .transform(CLASS_TO_KIND_FUNCTION) - .toSortedSet(Ordering.natural()); + .filter(hasAnnotation(VirtualEntity.class).negate()) + .map(CLASS_TO_KIND_FUNCTION) + .collect(toImmutableSortedSet(Ordering.natural())); } } diff --git a/java/google/registry/model/ofy/ObjectifyService.java b/java/google/registry/model/ofy/ObjectifyService.java index 99022a9a9..f2ee8800a 100644 --- a/java/google/registry/model/ofy/ObjectifyService.java +++ b/java/google/registry/model/ofy/ObjectifyService.java @@ -15,7 +15,7 @@ package google.registry.model.ofy; import static com.google.common.base.Preconditions.checkState; -import static com.google.common.base.Predicates.not; +import static com.google.common.collect.ImmutableSet.toImmutableSet; import static com.googlecode.objectify.ObjectifyService.factory; import static google.registry.util.TypeUtils.hasAnnotation; @@ -24,7 +24,7 @@ import com.google.appengine.api.datastore.DatastoreServiceConfig; import com.google.appengine.api.datastore.DatastoreServiceFactory; import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableList; -import com.google.common.collect.Iterables; +import com.google.common.collect.ImmutableSet; import com.googlecode.objectify.Key; import com.googlecode.objectify.Objectify; import com.googlecode.objectify.ObjectifyFactory; @@ -45,6 +45,7 @@ import google.registry.model.translators.InetAddressTranslatorFactory; import google.registry.model.translators.ReadableInstantUtcTranslatorFactory; import google.registry.model.translators.UpdateAutoTimestampTranslatorFactory; import java.util.concurrent.atomic.AtomicLong; +import java.util.stream.Stream; /** * An instance of Ofy, obtained via {@code #ofy()}, should be used to access all persistable @@ -133,13 +134,16 @@ public class ObjectifyService { /** Register classes that can be persisted via Objectify as Datastore entities. */ private static void registerEntityClasses( - Iterable> entityClasses) { + ImmutableSet> entityClasses) { // Register all the @Entity classes before any @EntitySubclass classes so that we can check // that every @Entity registration is a new kind and every @EntitySubclass registration is not. // This is future-proofing for Objectify 5.x where the registration logic gets less lenient. - for (Class clazz : Iterables.concat( - Iterables.filter(entityClasses, hasAnnotation(Entity.class)), - Iterables.filter(entityClasses, not(hasAnnotation(Entity.class))))) { + + for (Class clazz : + Stream.concat( + entityClasses.stream().filter(hasAnnotation(Entity.class)), + entityClasses.stream().filter(hasAnnotation(Entity.class).negate())) + .collect(toImmutableSet())) { String kind = Key.getKind(clazz); boolean registered = factory().getMetadata(kind) != null; if (clazz.isAnnotationPresent(Entity.class)) { diff --git a/java/google/registry/repositories.bzl b/java/google/registry/repositories.bzl index 7461956b8..37e7a1c81 100644 --- a/java/google/registry/repositories.bzl +++ b/java/google/registry/repositories.bzl @@ -957,11 +957,10 @@ def com_google_gdata_core(): def com_google_guava(): java_import_external( name = "com_google_guava", - jar_sha256 = "36a666e3b71ae7f0f0dca23654b67e086e6c93d192f60ba5dfd5519db6c288c8", + jar_sha256 = "7baa80df284117e5b945b19b98d367a85ea7b7801bd358ff657946c3bd1b6596", jar_urls = [ - "http://domain-registry-maven.storage.googleapis.com/repo1.maven.org/maven2/com/google/guava/guava/20.0/guava-20.0.jar", - "http://repo1.maven.org/maven2/com/google/guava/guava/20.0/guava-20.0.jar", - "http://maven.ibiblio.org/maven2/com/google/guava/guava/20.0/guava-20.0.jar", + "http://repo1.maven.org/maven2/com/google/guava/guava/23.0/guava-23.0.jar", + "http://domain-registry-maven.storage.googleapis.com/repo1.maven.org/maven2/com/google/guava/guava/23.0/guava-23.0.jar", ], licenses = ["notice"], # The Apache Software License, Version 2.0 exports = [ @@ -973,11 +972,10 @@ def com_google_guava(): def com_google_guava_testlib(): java_import_external( name = "com_google_guava_testlib", - jar_sha256 = "a9f52f328ac024e420c8995a107ea0dbef3fc169ddf97b3426e634f28d6b3663", + jar_sha256 = "7e328d0f89a5ea103de4f9b689130eb555ff277e83bf86294bc14c2c40a59a80", jar_urls = [ - "http://domain-registry-maven.storage.googleapis.com/repo1.maven.org/maven2/com/google/guava/guava-testlib/20.0/guava-testlib-20.0.jar", - "http://maven.ibiblio.org/maven2/com/google/guava/guava-testlib/20.0/guava-testlib-20.0.jar", - "http://repo1.maven.org/maven2/com/google/guava/guava-testlib/20.0/guava-testlib-20.0.jar", + "http://repo1.maven.org/maven2/com/google/guava/guava-testlib/23.0/guava-testlib-23.0.jar", + "http://domain-registry-maven.storage.googleapis.com/repo1.maven.org/maven2/com/google/guava/guava-testlib/23.0/guava-testlib-23.0.jar", ], licenses = ["notice"], # The Apache Software License, Version 2.0 testonly_ = True, diff --git a/java/google/registry/request/lock/LockHandlerImpl.java b/java/google/registry/request/lock/LockHandlerImpl.java index b6df55f89..6b5712512 100644 --- a/java/google/registry/request/lock/LockHandlerImpl.java +++ b/java/google/registry/request/lock/LockHandlerImpl.java @@ -21,6 +21,7 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Optional; import com.google.common.base.Strings; import com.google.common.collect.ImmutableSortedSet; +import com.google.common.util.concurrent.UncheckedExecutionException; import google.registry.model.server.Lock; import google.registry.util.AppEngineTimeLimiter; import google.registry.util.FormattingLogger; @@ -28,6 +29,7 @@ import google.registry.util.RequestStatusChecker; import java.util.HashSet; import java.util.Set; import java.util.concurrent.Callable; +import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; import javax.annotation.Nullable; import javax.inject.Inject; @@ -67,8 +69,12 @@ public class LockHandlerImpl implements LockHandler { return AppEngineTimeLimiter.create().callWithTimeout( new LockingCallable(callable, Strings.emptyToNull(tld), leaseLength, lockNames), leaseLength.minus(LOCK_TIMEOUT_FUDGE).getMillis(), - TimeUnit.MILLISECONDS, - true); + TimeUnit.MILLISECONDS); + } catch (ExecutionException | UncheckedExecutionException e) { + // Unwrap the execution exception and throw its root cause. + Throwable cause = e.getCause(); + throwIfUnchecked(cause); + throw new RuntimeException(cause); } catch (Exception e) { throwIfUnchecked(e); throw new RuntimeException(e); diff --git a/java/google/registry/util/AppEngineTimeLimiter.java b/java/google/registry/util/AppEngineTimeLimiter.java index eab7df838..462df1b91 100644 --- a/java/google/registry/util/AppEngineTimeLimiter.java +++ b/java/google/registry/util/AppEngineTimeLimiter.java @@ -76,6 +76,6 @@ public class AppEngineTimeLimiter { } public static TimeLimiter create() { - return new SimpleTimeLimiter(new NewRequestThreadExecutorService()); + return SimpleTimeLimiter.create(new NewRequestThreadExecutorService()); } } diff --git a/java/google/registry/util/TypeUtils.java b/java/google/registry/util/TypeUtils.java index c04174107..b06b4dd25 100644 --- a/java/google/registry/util/TypeUtils.java +++ b/java/google/registry/util/TypeUtils.java @@ -19,13 +19,13 @@ import static google.registry.util.CollectionUtils.difference; import static java.lang.reflect.Modifier.isFinal; import static java.lang.reflect.Modifier.isStatic; -import com.google.common.base.Predicate; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.reflect.TypeToken; import java.lang.annotation.Annotation; import java.lang.reflect.Field; import java.lang.reflect.Modifier; +import java.util.function.Predicate; /** Utilities methods related to reflection. */ public class TypeUtils { @@ -103,12 +103,7 @@ public class TypeUtils { /** Returns a predicate that tests whether classes are annotated with the given annotation. */ public static final Predicate> hasAnnotation( final Class annotation) { - return new Predicate>() { - @Override - public boolean apply(Class clazz) { - return clazz.isAnnotationPresent(annotation); - } - }; + return (Class clazz) -> clazz.isAnnotationPresent(annotation); } public static void checkNoInheritanceRelationships(ImmutableSet> resourceClasses) { diff --git a/javatests/google/registry/model/EntityClassesTest.java b/javatests/google/registry/model/EntityClassesTest.java index c5c3483e0..2754edeed 100644 --- a/javatests/google/registry/model/EntityClassesTest.java +++ b/javatests/google/registry/model/EntityClassesTest.java @@ -14,13 +14,13 @@ package google.registry.model; +import static com.google.common.collect.ImmutableSet.toImmutableSet; import static com.google.common.truth.Truth.assertThat; import static google.registry.model.EntityClasses.ALL_CLASSES; import static google.registry.model.EntityClasses.CLASS_TO_KIND_FUNCTION; import static google.registry.util.TypeUtils.hasAnnotation; +import static java.util.stream.Collectors.toSet; -import com.google.common.base.Function; -import com.google.common.collect.FluentIterable; import com.google.common.collect.Ordering; import com.googlecode.objectify.annotation.Entity; import com.googlecode.objectify.annotation.EntitySubclass; @@ -35,14 +35,9 @@ public class EntityClassesTest { // This implements the manual ordering we've been using for the EntityClasses class lists. private static final Ordering> QUALIFIED_CLASS_NAME_ORDERING = - Ordering.natural().onResultOf(new Function, String>() { - @Override - public String apply(Class clazz) { - // Return only the part of the class name after the package name, which is the class - // name plus any outer class names. - return clazz.getCanonicalName().substring(clazz.getPackage().getName().length()); - } - }); + Ordering.natural() + .onResultOf( + clazz -> clazz.getCanonicalName().substring(clazz.getPackage().getName().length())); @Test public void testEntityClasses_inAlphabeticalOrder() throws Exception { @@ -51,23 +46,30 @@ public class EntityClassesTest { @Test public void testEntityClasses_baseEntitiesHaveUniqueKinds() throws Exception { - assertThat(FluentIterable.from(ALL_CLASSES) - .filter(hasAnnotation(Entity.class)) - .transform(CLASS_TO_KIND_FUNCTION)) - .named("base entity kinds") - .containsNoDuplicates(); + assertThat( + ALL_CLASSES + .stream() + .filter(hasAnnotation(Entity.class)) + .map(CLASS_TO_KIND_FUNCTION) + .collect(toImmutableSet())) + .named("base entity kinds") + .containsNoDuplicates(); } @Test public void testEntityClasses_entitySubclassesHaveKindsMatchingBaseEntities() throws Exception { - Set baseEntityKinds = FluentIterable.from(ALL_CLASSES) - .filter(hasAnnotation(Entity.class)) - .transform(CLASS_TO_KIND_FUNCTION) - .toSet(); - Set entitySubclassKinds = FluentIterable.from(ALL_CLASSES) - .filter(hasAnnotation(EntitySubclass.class)) - .transform(CLASS_TO_KIND_FUNCTION) - .toSet(); + Set baseEntityKinds = + ALL_CLASSES + .stream() + .filter(hasAnnotation(Entity.class)) + .map(CLASS_TO_KIND_FUNCTION) + .collect(toSet()); + Set entitySubclassKinds = + ALL_CLASSES + .stream() + .filter(hasAnnotation(EntitySubclass.class)) + .map(CLASS_TO_KIND_FUNCTION) + .collect(toSet()); assertThat(baseEntityKinds).named("base entity kinds").containsAllIn(entitySubclassKinds); } diff --git a/javatests/google/registry/server/TestServer.java b/javatests/google/registry/server/TestServer.java index 054f2e980..daa9ce5bc 100644 --- a/javatests/google/registry/server/TestServer.java +++ b/javatests/google/registry/server/TestServer.java @@ -18,6 +18,7 @@ import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Throwables.throwIfUnchecked; import static com.google.common.util.concurrent.Runnables.doNothing; import static google.registry.util.NetworkUtils.getCanonicalHostName; +import static java.util.concurrent.Executors.newCachedThreadPool; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; @@ -128,14 +129,18 @@ public final class TestServer { /** Stops the HTTP server. */ public void stop() { try { - new SimpleTimeLimiter().callWithTimeout(new Callable() { - @Nullable - @Override - public Void call() throws Exception { - server.stop(); - return null; - } - }, SHUTDOWN_TIMEOUT_MS, TimeUnit.MILLISECONDS, true); + SimpleTimeLimiter.create(newCachedThreadPool()) + .callWithTimeout( + new Callable() { + @Nullable + @Override + public Void call() throws Exception { + server.stop(); + return null; + } + }, + SHUTDOWN_TIMEOUT_MS, + TimeUnit.MILLISECONDS); } catch (Exception e) { throwIfUnchecked(e); throw new RuntimeException(e); diff --git a/javatests/google/registry/server/UrlChecker.java b/javatests/google/registry/server/UrlChecker.java index 9aac6c5e5..bda4d5a4d 100644 --- a/javatests/google/registry/server/UrlChecker.java +++ b/javatests/google/registry/server/UrlChecker.java @@ -15,6 +15,7 @@ package google.registry.server; import static com.google.common.base.Throwables.throwIfUnchecked; +import static java.util.concurrent.Executors.newCachedThreadPool; import com.google.common.util.concurrent.SimpleTimeLimiter; import java.io.IOException; @@ -32,7 +33,7 @@ final class UrlChecker { /** Probes {@code url} until it becomes available. */ static void waitUntilAvailable(final URL url, int timeoutMs) { try { - new SimpleTimeLimiter().callWithTimeout(new Callable() { + SimpleTimeLimiter.create(newCachedThreadPool()).callWithTimeout(new Callable() { @Nullable @Override public Void call() throws InterruptedException, IOException { @@ -44,7 +45,7 @@ final class UrlChecker { Thread.sleep(exponentialBackoffMs *= 2); } } - }, timeoutMs, TimeUnit.MILLISECONDS, true); + }, timeoutMs, TimeUnit.MILLISECONDS); } catch (Exception e) { throwIfUnchecked(e); throw new RuntimeException(e);