Upgrade error-prone to 3.3.4 (#848)

* Upgrade error-prone to 3.3.4

This would fix the failure with openjdk 11.0.9 in
3.3.3.

Fixed new antipatterns raised by the new version:
- Replaced unnecessary lambdas with methods.
- Switched wait/sleep calls to equivalent methods using java.time types
- Types inheriting Object.toString() should not be assigned to string
parameter in logging statements.
This commit is contained in:
Weimin Yu 2020-10-23 11:17:57 -04:00 committed by GitHub
parent 96cb9abcf3
commit 9ddde4799c
53 changed files with 495 additions and 358 deletions

View file

@ -24,7 +24,6 @@ import static google.registry.persistence.transaction.TransactionManagerFactory.
import static google.registry.util.CollectionUtils.nullToEmpty;
import static google.registry.util.CollectionUtils.nullToEmptyImmutableCopy;
import static google.registry.util.DateTimeUtils.END_OF_TIME;
import static java.util.concurrent.TimeUnit.MILLISECONDS;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.cache.CacheBuilder;
@ -387,7 +386,7 @@ public abstract class EppResource extends BackupGroupRoot implements Buildable {
private static LoadingCache<VKey<? extends EppResource>, EppResource> createEppResourcesCache(
Duration expiry) {
return CacheBuilder.newBuilder()
.expireAfterWrite(expiry.getMillis(), MILLISECONDS)
.expireAfterWrite(java.time.Duration.ofMillis(expiry.getMillis()))
.maximumSize(getEppResourceMaxCachedEntries())
.build(CACHE_LOADER);
}

View file

@ -59,25 +59,11 @@ public class OteStats {
private OteStats() {}
private static final Predicate<EppInput> HAS_CLAIMS_NOTICE =
eppInput -> {
Optional<LaunchCreateExtension> launchCreate =
eppInput.getSingleExtension(LaunchCreateExtension.class);
return launchCreate.isPresent() && launchCreate.get().getNotice() != null;
};
private static final Predicate<EppInput> HAS_SEC_DNS =
eppInput ->
eppInput.getSingleExtension(SecDnsCreateExtension.class).isPresent()
|| eppInput.getSingleExtension(SecDnsUpdateExtension.class).isPresent();
private static final Predicate<EppInput> IS_SUNRISE =
eppInput -> {
Optional<LaunchCreateExtension> launchCreate =
eppInput.getSingleExtension(LaunchCreateExtension.class);
return launchCreate.isPresent() && !isNullOrEmpty(launchCreate.get().getSignedMarks());
};
private static final Predicate<EppInput> IS_IDN =
eppInput ->
((DomainCommand.Create)
@ -94,6 +80,18 @@ public class OteStats {
.getResourceCommand())
.getInetAddresses());
private static boolean hasClaimsNotice(EppInput eppInput) {
Optional<LaunchCreateExtension> launchCreate =
eppInput.getSingleExtension(LaunchCreateExtension.class);
return launchCreate.isPresent() && launchCreate.get().getNotice() != null;
}
private static boolean isSunrise(EppInput eppInput) {
Optional<LaunchCreateExtension> launchCreate =
eppInput.getSingleExtension(LaunchCreateExtension.class);
return launchCreate.isPresent() && !isNullOrEmpty(launchCreate.get().getSignedMarks());
}
/** Enum defining the distinct statistics (types of registrar actions) to record. */
public enum StatType {
CONTACT_CREATES(0, equalTo(Type.CONTACT_CREATE)),
@ -107,8 +105,8 @@ public class OteStats {
DOMAIN_CREATES(0, equalTo(Type.DOMAIN_CREATE)),
DOMAIN_CREATES_ASCII(1, equalTo(Type.DOMAIN_CREATE), IS_IDN.negate()),
DOMAIN_CREATES_IDN(1, equalTo(Type.DOMAIN_CREATE), IS_IDN),
DOMAIN_CREATES_START_DATE_SUNRISE(1, equalTo(Type.DOMAIN_CREATE), IS_SUNRISE),
DOMAIN_CREATES_WITH_CLAIMS_NOTICE(1, equalTo(Type.DOMAIN_CREATE), HAS_CLAIMS_NOTICE),
DOMAIN_CREATES_START_DATE_SUNRISE(1, equalTo(Type.DOMAIN_CREATE), OteStats::isSunrise),
DOMAIN_CREATES_WITH_CLAIMS_NOTICE(1, equalTo(Type.DOMAIN_CREATE), OteStats::hasClaimsNotice),
DOMAIN_CREATES_WITH_FEE(
1,
equalTo(Type.DOMAIN_CREATE),

View file

@ -20,7 +20,6 @@ import static google.registry.config.RegistryConfig.getEppResourceMaxCachedEntri
import static google.registry.model.ofy.ObjectifyService.ofy;
import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
import static google.registry.util.TypeUtils.instantiate;
import static java.util.concurrent.TimeUnit.MILLISECONDS;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.cache.CacheBuilder;
@ -244,7 +243,7 @@ public abstract class ForeignKeyIndex<E extends EppResource> extends BackupGroup
private static LoadingCache<Key<ForeignKeyIndex<?>>, Optional<ForeignKeyIndex<?>>>
createForeignKeyIndexesCache(Duration expiry) {
return CacheBuilder.newBuilder()
.expireAfterWrite(expiry.getMillis(), MILLISECONDS)
.expireAfterWrite(java.time.Duration.ofMillis(expiry.getMillis()))
.maximumSize(getEppResourceMaxCachedEntries())
.build(CACHE_LOADER);
}

View file

@ -28,7 +28,6 @@ import static google.registry.util.CollectionUtils.nullToEmptyImmutableCopy;
import static google.registry.util.DateTimeUtils.END_OF_TIME;
import static google.registry.util.DateTimeUtils.START_OF_TIME;
import static google.registry.util.PreconditionsUtils.checkArgumentNotNull;
import static java.util.concurrent.TimeUnit.MILLISECONDS;
import static org.joda.money.CurrencyUnit.USD;
import com.google.common.annotations.VisibleForTesting;
@ -260,7 +259,8 @@ public class Registry extends ImmutableObject implements Buildable, DatastoreAnd
/** A cache that loads the {@link Registry} for a given tld. */
private static final LoadingCache<String, Optional<Registry>> CACHE =
CacheBuilder.newBuilder()
.expireAfterWrite(getSingletonCacheRefreshDuration().getMillis(), MILLISECONDS)
.expireAfterWrite(
java.time.Duration.ofMillis(getSingletonCacheRefreshDuration().getMillis()))
.build(
new CacheLoader<String, Optional<Registry>>() {
@Override

View file

@ -25,7 +25,6 @@ import static google.registry.model.common.EntityGroupRoot.getCrossTldKey;
import static google.registry.model.ofy.ObjectifyService.allocateId;
import static google.registry.model.ofy.ObjectifyService.ofy;
import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
import static java.util.concurrent.TimeUnit.MILLISECONDS;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Splitter;
@ -197,7 +196,7 @@ public final class PremiumList extends BaseDomainLabelList<Money, PremiumList.Pr
@VisibleForTesting
static LoadingCache<String, PremiumList> createCachePremiumLists(Duration cachePersistDuration) {
return CacheBuilder.newBuilder()
.expireAfterWrite(cachePersistDuration.getMillis(), MILLISECONDS)
.expireAfterWrite(java.time.Duration.ofMillis(cachePersistDuration.getMillis()))
.build(
new CacheLoader<String, PremiumList>() {
@Override
@ -221,7 +220,8 @@ public final class PremiumList extends BaseDomainLabelList<Money, PremiumList.Pr
static final LoadingCache<Key<PremiumListRevision>, PremiumListRevision>
cachePremiumListRevisions =
CacheBuilder.newBuilder()
.expireAfterWrite(getSingletonCachePersistDuration().getMillis(), MILLISECONDS)
.expireAfterWrite(
java.time.Duration.ofMillis(getSingletonCachePersistDuration().getMillis()))
.build(
new CacheLoader<Key<PremiumListRevision>, PremiumListRevision>() {
@Override
@ -260,14 +260,14 @@ public final class PremiumList extends BaseDomainLabelList<Money, PremiumList.Pr
static LoadingCache<Key<PremiumListEntry>, Optional<PremiumListEntry>>
createCachePremiumListEntries(Duration cachePersistDuration) {
return CacheBuilder.newBuilder()
.expireAfterWrite(cachePersistDuration.getMillis(), MILLISECONDS)
.expireAfterWrite(java.time.Duration.ofMillis(cachePersistDuration.getMillis()))
.maximumSize(getStaticPremiumListMaxCachedEntries())
.build(
new CacheLoader<Key<PremiumListEntry>, Optional<PremiumListEntry>>() {
@Override
public Optional<PremiumListEntry> load(final Key<PremiumListEntry> entryKey) {
return tm()
.doTransactionless(() -> Optional.ofNullable(ofy().load().key(entryKey).now()));
return tm().doTransactionless(
() -> Optional.ofNullable(ofy().load().key(entryKey).now()));
}
});
}

View file

@ -20,7 +20,6 @@ import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static google.registry.config.RegistryConfig.getDomainLabelListCacheDuration;
import static google.registry.model.registry.label.ReservationType.FULLY_BLOCKED;
import static google.registry.util.CollectionUtils.nullToEmpty;
import static java.util.concurrent.TimeUnit.MILLISECONDS;
import static org.joda.time.DateTimeZone.UTC;
import com.google.common.base.Splitter;
@ -241,7 +240,8 @@ public final class ReservedList
private static LoadingCache<String, ReservedList> cache =
CacheBuilder.newBuilder()
.expireAfterWrite(getDomainLabelListCacheDuration().getMillis(), MILLISECONDS)
.expireAfterWrite(
java.time.Duration.ofMillis(getDomainLabelListCacheDuration().getMillis()))
.build(
new CacheLoader<String, ReservedList>() {
@Override

View file

@ -57,7 +57,6 @@ import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.concurrent.Callable;
import javax.annotation.Nullable;
import javax.persistence.CollectionTable;
import javax.persistence.Column;
@ -146,8 +145,7 @@ public class ClaimsListShard extends ImmutableObject implements DatastoreAndSqlE
private static final Retrier LOADER_RETRIER = new Retrier(new SystemSleeper(), 2);
private static final Callable<ClaimsListShard> LOADER_CALLABLE =
() -> {
private static ClaimsListShard loadClaimsListShard() {
// Find the most recent revision.
Key<ClaimsListRevision> revisionKey = getCurrentRevision();
@ -246,7 +244,9 @@ public class ClaimsListShard extends ImmutableObject implements DatastoreAndSqlE
*/
private static final Supplier<ClaimsListShard> CACHE =
memoizeWithShortExpiration(
() -> LOADER_RETRIER.callWithRetry(LOADER_CALLABLE, IllegalStateException.class));
() ->
LOADER_RETRIER.callWithRetry(
ClaimsListShard::loadClaimsListShard, IllegalStateException.class));
/** Returns the revision id of this claims list, or throws exception if it is null. */
public Long getRevisionId() {

View file

@ -144,16 +144,19 @@ public class DomainTransferData extends TransferData<DomainTransferData.Builder>
rootKey, serverApproveAutorenewPollMessage, serverApproveAutorenewPollMessageHistoryId);
}
@SuppressWarnings("unused") // For Hibernate.
private void loadServerApproveBillingEventHistoryId(
@AlsoLoad("serverApproveBillingEvent") VKey<BillingEvent.OneTime> val) {
serverApproveBillingEventHistoryId = DomainBase.getHistoryId(val);
}
@SuppressWarnings("unused") // For Hibernate.
private void loadServerApproveAutorenewEventHistoryId(
@AlsoLoad("serverApproveAutorenewEvent") VKey<BillingEvent.Recurring> val) {
serverApproveAutorenewEventHistoryId = DomainBase.getHistoryId(val);
}
@SuppressWarnings("unused") // For Hibernate.
private void loadServerApproveAutorenewPollMessageHistoryId(
@AlsoLoad("serverApproveAutorenewPollMessage") VKey<PollMessage.Autorenew> val) {
serverApproveAutorenewPollMessageHistoryId = DomainBase.getHistoryId(val);

View file

@ -22,7 +22,6 @@ import google.registry.request.RequestHandler;
import google.registry.util.SystemClock;
import java.io.IOException;
import java.security.Security;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
@ -51,13 +50,16 @@ public class ServletBase extends HttpServlet {
// etc), we log the error but keep the main thread running. Also the shutdown hook will only be
// registered if metric reporter starts up correctly.
try {
metricReporter.get().startAsync().awaitRunning(10, TimeUnit.SECONDS);
metricReporter.get().startAsync().awaitRunning(java.time.Duration.ofSeconds(10));
logger.atInfo().log("Started up MetricReporter");
LifecycleManager.getInstance()
.setShutdownHook(
() -> {
try {
metricReporter.get().stopAsync().awaitTerminated(10, TimeUnit.SECONDS);
metricReporter
.get()
.stopAsync()
.awaitTerminated(java.time.Duration.ofSeconds(10));
logger.atInfo().log("Shut down MetricReporter");
} catch (TimeoutException e) {
logger.atSevere().withCause(e).log("Failed to stop MetricReporter.");

View file

@ -18,7 +18,6 @@ import static google.registry.config.RegistryConfig.getDomainLabelListCacheDurat
import static google.registry.config.RegistryConfig.getSingletonCachePersistDuration;
import static google.registry.config.RegistryConfig.getStaticPremiumListMaxCachedEntries;
import static google.registry.schema.tld.PremiumListDao.getPriceForLabel;
import static java.util.concurrent.TimeUnit.MILLISECONDS;
import com.google.auto.value.AutoValue;
import com.google.common.annotations.VisibleForTesting;
@ -48,7 +47,7 @@ class PremiumListCache {
static LoadingCache<String, Optional<PremiumList>> createCachePremiumLists(
Duration cachePersistDuration) {
return CacheBuilder.newBuilder()
.expireAfterWrite(cachePersistDuration.getMillis(), MILLISECONDS)
.expireAfterWrite(java.time.Duration.ofMillis(cachePersistDuration.getMillis()))
.build(
new CacheLoader<String, Optional<PremiumList>>() {
@Override
@ -81,7 +80,7 @@ class PremiumListCache {
static LoadingCache<RevisionIdAndLabel, Optional<BigDecimal>> createCachePremiumEntries(
Duration cachePersistDuration) {
return CacheBuilder.newBuilder()
.expireAfterWrite(cachePersistDuration.getMillis(), MILLISECONDS)
.expireAfterWrite(java.time.Duration.ofMillis(cachePersistDuration.getMillis()))
.maximumSize(getStaticPremiumListMaxCachedEntries())
.build(
new CacheLoader<RevisionIdAndLabel, Optional<BigDecimal>>() {

View file

@ -18,7 +18,6 @@ import static google.registry.config.RegistryConfig.ConfigModule.TmchCaMode.PILO
import static google.registry.config.RegistryConfig.ConfigModule.TmchCaMode.PRODUCTION;
import static google.registry.config.RegistryConfig.getSingletonCacheRefreshDuration;
import static google.registry.util.ResourceUtils.readResourceUtf8;
import static java.util.concurrent.TimeUnit.MILLISECONDS;
import com.google.common.cache.CacheBuilder;
import com.google.common.cache.CacheLoader;
@ -77,7 +76,8 @@ public final class TmchCertificateAuthority {
*/
private static final LoadingCache<TmchCaMode, X509CRL> CRL_CACHE =
CacheBuilder.newBuilder()
.expireAfterWrite(getSingletonCacheRefreshDuration().getMillis(), MILLISECONDS)
.expireAfterWrite(
java.time.Duration.ofMillis(getSingletonCacheRefreshDuration().getMillis()))
.build(
new CacheLoader<TmchCaMode, X509CRL>() {
@Override

View file

@ -18,7 +18,6 @@ import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Sets;
import com.google.common.collect.Sets.SetView;
import java.io.File;
import java.util.function.Predicate;
/**
* Compares two Datastore backups in V3 format on local file system. This is for use in tests and
@ -30,8 +29,10 @@ import java.util.function.Predicate;
*/
class CompareDbBackups {
private static final String DS_V3_BACKUP_FILE_PREFIX = "output-";
private static final Predicate<File> DATA_FILE_MATCHER =
file -> file.isFile() && file.getName().startsWith(DS_V3_BACKUP_FILE_PREFIX);
private static boolean isDatastoreV3File(File file) {
return file.isFile() && file.getName().startsWith(DS_V3_BACKUP_FILE_PREFIX);
}
public static void main(String[] args) {
if (args.length != 2) {
@ -40,9 +41,11 @@ class CompareDbBackups {
}
ImmutableSet<EntityWrapper> entities1 =
RecordAccumulator.readDirectory(new File(args[0]), DATA_FILE_MATCHER).getEntityWrapperSet();
RecordAccumulator.readDirectory(new File(args[0]), CompareDbBackups::isDatastoreV3File)
.getEntityWrapperSet();
ImmutableSet<EntityWrapper> entities2 =
RecordAccumulator.readDirectory(new File(args[1]), DATA_FILE_MATCHER).getEntityWrapperSet();
RecordAccumulator.readDirectory(new File(args[1]), CompareDbBackups::isDatastoreV3File)
.getEntityWrapperSet();
// Calculate the entities added and removed.
SetView<EntityWrapper> added = Sets.difference(entities2, entities1);

View file

@ -64,7 +64,6 @@ import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.function.Predicate;
import javax.inject.Inject;
import org.joda.time.DateTime;
@ -96,8 +95,9 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA
@Inject RegistrarSettingsAction() {}
private static final Predicate<RegistrarContact> HAS_PHONE =
contact -> contact.getPhoneNumber() != null;
private static boolean hasPhone(RegistrarContact contact) {
return contact.getPhoneNumber() != null;
}
@Override
public void run() {
@ -512,8 +512,8 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA
Multimap<Type, RegistrarContact> newContactsByType,
Type... types) {
for (Type type : types) {
if (oldContactsByType.get(type).stream().anyMatch(HAS_PHONE)
&& newContactsByType.get(type).stream().noneMatch(HAS_PHONE)) {
if (oldContactsByType.get(type).stream().anyMatch(RegistrarSettingsAction::hasPhone)
&& newContactsByType.get(type).stream().noneMatch(RegistrarSettingsAction::hasPhone)) {
throw new ContactRequirementException(
String.format(
"Please provide a phone number for at least one %s contact",

View file

@ -143,6 +143,7 @@ public class TestPipelineExtension extends Pipeline
// Null until the pipeline has been run
@Nullable private List<TransformHierarchy.Node> runVisitedNodes;
@SuppressWarnings("UnnecessaryLambda") // Stay true to the original class.
private final Predicate<Node> isPAssertNode =
node ->
node.getTransform() instanceof PAssert.GroupThenAssert

View file

@ -27,7 +27,6 @@ import static google.registry.testing.DatastoreHelper.persistActiveContact;
import static google.registry.util.DateTimeUtils.END_OF_TIME;
import static google.registry.util.DateTimeUtils.START_OF_TIME;
import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.concurrent.TimeUnit.MILLISECONDS;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.fail;
@ -225,7 +224,7 @@ public class OfyTest {
if (firstAttemptTime == null) {
// Sleep a bit to ensure that the next attempt is at a new millisecond.
firstAttemptTime = tm().getTransactionTime();
sleepUninterruptibly(10, MILLISECONDS);
sleepUninterruptibly(java.time.Duration.ofMillis(10));
throw new ConcurrentModificationException();
}
assertThat(tm().getTransactionTime()).isGreaterThan(firstAttemptTime);

View file

@ -93,7 +93,7 @@ public class TestSftpServer implements FtpServer {
try (PEMParser pemParser = new PEMParser(new StringReader(key))) {
PEMKeyPair pemPair = (PEMKeyPair) pemParser.readObject();
KeyPair result = new JcaPEMKeyConverter().setProvider("BC").getKeyPair(pemPair);
logger.atInfo().log("Read key pair %s", result);
logger.atInfo().log("Read key pair successfully.");
return result;
} catch (IOException e) {
logger.atSevere().withCause(e).log("Couldn't read key pair from string.");