Skip to content

Commit

Permalink
Remove java.util.Date (#2373)
Browse files Browse the repository at this point in the history
There is one remaining instance in JpaTransactionManagerImpl that cannot
be removed because DetachingTypedQuery is implementing TypedQuery, which has
a method that expectred java.util.Date.
  • Loading branch information
jianglai committed Mar 15, 2024
1 parent 6d2eb2e commit c68583f
Show file tree
Hide file tree
Showing 14 changed files with 108 additions and 147 deletions.
11 changes: 9 additions & 2 deletions config/presubmits.py
Original file line number Diff line number Diff line change
Expand Up @@ -173,17 +173,24 @@ def fails(self, file):
):
"JavaScript files should not include console logging.",
PresubmitCheck(
r"org\.testcontainers\.shaded\.",
r".*org\.testcontainers\.shaded.*",
"java",
{"/node_modules/"},
):
"Do not use shaded dependencies from testcontainers.",
PresubmitCheck(
r"com\.google\.common\.truth\.Truth8",
r".*com\.google\.common\.truth\.Truth8.*",
"java",
{"/node_modules/"},
):
"Truth8 is deprecated. Use Truth instead.",
PresubmitCheck(
r".*java\.util\.Date.*",
"java",
{"/node_modules/", "JpaTransactionManagerImpl.java"},
):
"Do not use java.util.Date. Use classes in java.time package instead.",

}

# Note that this regex only works for one kind of Flyway file. If we want to
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@
import java.math.BigDecimal;
import java.time.Duration;
import java.util.Collection;
import java.util.Date;
import java.util.Map;
import java.util.ServiceLoader;
import org.apache.commons.codec.binary.Base64;
Expand Down Expand Up @@ -195,10 +194,7 @@ public AccessToken refreshAccessToken() throws IOException {
GenericData responseData = response.parseAs(GenericData.class);
String accessToken = validateString(responseData, "access_token", PARSE_ERROR_PREFIX);
int expiresInSeconds = validateInt32(responseData, "expires_in", PARSE_ERROR_PREFIX);
long expiresAtMilliseconds = clock.nowUtc().getMillis() + expiresInSeconds * 1000L;
@SuppressWarnings("JavaUtilDate")
AccessToken token = new AccessToken(accessToken, new Date(expiresAtMilliseconds));
return token;
return new AccessToken(accessToken, clock.nowUtc().plusSeconds(expiresInSeconds).toDate());
}

String createAssertion(JsonFactory jsonFactory, long currentTime) throws IOException {
Expand Down Expand Up @@ -257,8 +253,7 @@ static int validateInt32(Map<String, Object> map, String key, String errorPrefix
if (value == null) {
throw new IOException(String.format(VALUE_NOT_FOUND_MESSAGE, errorPrefix, key));
}
if (value instanceof BigDecimal) {
BigDecimal bigDecimalValue = (BigDecimal) value;
if (value instanceof BigDecimal bigDecimalValue) {
return bigDecimalValue.intValueExact();
}
if (!(value instanceof Integer)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
import java.security.cert.X509Certificate;
import java.security.interfaces.ECPublicKey;
import java.security.interfaces.RSAPublicKey;
import java.util.Date;
import java.util.stream.Collectors;
import javax.inject.Inject;
import org.bouncycastle.jcajce.provider.asymmetric.util.EC5Util;
Expand Down Expand Up @@ -106,12 +105,9 @@ private static boolean checkCurveName(PublicKey key, ImmutableSet<String> allowe
// These 2 different instances of PublicKey need to be handled separately since their OIDs are
// encoded differently. More details on this can be found at
// https://stackoverflow.com/questions/49895713/how-to-find-the-matching-curve-name-from-an-ecpublickey.
if (key instanceof ECPublicKey) {
ECPublicKey ecKey = (ECPublicKey) key;
if (key instanceof ECPublicKey ecKey) {
params = EC5Util.convertSpec(ecKey.getParams());
} else if (key instanceof org.bouncycastle.jce.interfaces.ECPublicKey) {
org.bouncycastle.jce.interfaces.ECPublicKey ecKey =
(org.bouncycastle.jce.interfaces.ECPublicKey) key;
} else if (key instanceof org.bouncycastle.jce.interfaces.ECPublicKey ecKey) {
params = ecKey.getParameters();
} else {
throw new IllegalArgumentException("Unrecognized instance of PublicKey.");
Expand Down Expand Up @@ -148,7 +144,7 @@ private void handleCertViolations(ImmutableSet<CertificateViolation> violations)
if (!violations.isEmpty()) {
String displayMessages =
violations.stream()
.map(violation -> getViolationDisplayMessage(violation))
.map(this::getViolationDisplayMessage)
.collect(Collectors.joining("\n"));
throw new InsecureCertificateException(violations, displayMessages);
}
Expand All @@ -162,7 +158,7 @@ public ImmutableSet<CertificateViolation> checkCertificate(X509Certificate certi
ImmutableSet.Builder<CertificateViolation> violations = new ImmutableSet.Builder<>();

// Check if currently in validity period
Date now = clock.nowUtc().toDate();
DateTime now = clock.nowUtc();
if (DateTimeComparator.getInstance().compare(certificate.getNotAfter(), now) < 0) {
violations.add(CertificateViolation.EXPIRED);
} else if (DateTimeComparator.getInstance().compare(certificate.getNotBefore(), now) > 0) {
Expand Down Expand Up @@ -231,13 +227,13 @@ public boolean shouldReceiveExpiringNotification(
DateTime lastExpiringNotificationSentDate, String certificateStr) {
X509Certificate certificate = getCertificate(certificateStr);
DateTime now = clock.nowUtc();
// expiration date is one day after lastValidDate
// the expiration date is one day after lastValidDate
DateTime lastValidDate = new DateTime(certificate.getNotAfter());
if (lastValidDate.isBefore(now)) {
return false;
}
/*
* Client should receive a notification if :
* Client should receive a notification if:
* 1) client has never received notification (lastExpiringNotificationSentDate is initially
* set to START_OF_TIME) and the certificate has entered the expiring period, OR
* 2) client has received notification but the interval between now and
Expand All @@ -254,29 +250,21 @@ private String getViolationDisplayMessage(CertificateViolation certificateViolat
// Yes, we'd rather do this as an instance method on the CertificateViolation enum itself, but
// we can't because we need access to configuration (injected as instance variables) which you
// can't get in a static enum context.
switch (certificateViolation) {
case EXPIRED:
return "Certificate is expired.";
case NOT_YET_VALID:
return "Certificate start date is in the future.";
case ALGORITHM_CONSTRAINED:
return "Certificate key algorithm must be RSA or ECDSA.";
case RSA_KEY_LENGTH_TOO_SHORT:
return String.format(
"RSA key length is too short; the minimum allowed length is %d bits.",
this.minimumRsaKeyLength);
case VALIDITY_LENGTH_TOO_LONG:
return String.format(
"Certificate validity period is too long; it must be less than or equal to %d days.",
this.maxValidityLengthSchedule.lastEntry().getValue());
case INVALID_ECDSA_CURVE:
return String.format(
"The ECDSA key must use one of these algorithms: %s", allowedEcdsaCurves);
default:
throw new IllegalArgumentException(
String.format(
"Unknown CertificateViolation enum value: %s", certificateViolation.name()));
}
return switch (certificateViolation) {
case EXPIRED -> "Certificate is expired.";
case NOT_YET_VALID -> "Certificate start date is in the future.";
case ALGORITHM_CONSTRAINED -> "Certificate key algorithm must be RSA or ECDSA.";
case RSA_KEY_LENGTH_TOO_SHORT ->
String.format(
"RSA key length is too short; the minimum allowed length is %d bits.",
this.minimumRsaKeyLength);
case VALIDITY_LENGTH_TOO_LONG ->
String.format(
"Certificate validity period is too long; it must be less than or equal to %d days.",
this.maxValidityLengthSchedule.lastEntry().getValue());
case INVALID_ECDSA_CURVE ->
String.format("The ECDSA key must use one of these algorithms: %s", allowedEcdsaCurves);
};
}

/**
Expand All @@ -295,7 +283,7 @@ public enum CertificateViolation {
* Gets a suitable end-user-facing display message for this particular certificate violation.
*
* <p>Note that the {@link CertificateChecker} instance must be passed in because it contains
* configuration values (e.g. minimum RSA key length) that go into the error message text.
* configuration values (e.g., minimum RSA key length) that go into the error message text.
*/
public String getDisplayMessage(CertificateChecker certificateChecker) {
return certificateChecker.getViolationDisplayMessage(this);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ private static ImmutableMap<TmchCaMode, X509Certificate> loadRootCertificates()
*/
public void verify(X509Certificate cert) throws GeneralSecurityException {
synchronized (TmchCertificateAuthority.class) {
X509Utils.verifyCertificate(getAndValidateRoot(), getCrl(), cert, clock.nowUtc().toDate());
X509Utils.verifyCertificate(getAndValidateRoot(), getCrl(), cert, clock.nowUtc());
}
}

Expand All @@ -151,7 +151,7 @@ public void updateCrl(String asciiCrl, String url) throws GeneralSecurityExcepti
} catch (Exception e) {
logger.atWarning().withCause(e).log("Old CRL is invalid, ignored during CRL update.");
}
X509Utils.verifyCrl(getAndValidateRoot(), oldCrl, newCrl, clock.nowUtc().toDate());
X509Utils.verifyCrl(getAndValidateRoot(), oldCrl, newCrl, clock.nowUtc());
TmchCrl.set(asciiCrl, url);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
import google.registry.model.domain.token.AllocationToken.TokenType;
import google.registry.model.domain.token.BulkPricingPackage;
import google.registry.persistence.VKey;
import java.util.Date;
import google.registry.tools.params.DateTimeParameter;
import java.util.List;
import java.util.Optional;
import javax.annotation.Nullable;
Expand Down Expand Up @@ -56,9 +56,11 @@ abstract class CreateOrUpdateBulkPricingPackageCommand extends MutatingCommand {
@Nullable
@Parameter(
names = "--next_billing_date",
converter = DateTimeParameter.class,
validateWith = DateTimeParameter.class,
description =
"The next date that the bulk pricing package should be billed for its annual fee")
Date nextBillingDate;
DateTime nextBillingDate;

/** Returns the existing BulkPricingPackage or null if it does not exist. */
@Nullable
Expand Down Expand Up @@ -105,9 +107,7 @@ protected final void init() throws Exception {
Optional.ofNullable(maxCreates).ifPresent(builder::setMaxCreates);
Optional.ofNullable(price).ifPresent(builder::setBulkPrice);
Optional.ofNullable(nextBillingDate)
.ifPresent(
nextBillingDate ->
builder.setNextBillingDate(new DateTime(nextBillingDate)));
.ifPresent(nextBillingDate -> builder.setNextBillingDate(nextBillingDate));
if (clearLastNotificationSent()) {
builder.setLastNotificationSent(null);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import com.google.common.base.Splitter;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import google.registry.module.backend.BackendRequestComponent;
import google.registry.module.bsa.BsaRequestComponent;
import google.registry.module.frontend.FrontendRequestComponent;
Expand All @@ -28,7 +29,6 @@
import java.util.ArrayList;
import java.util.List;
import org.junit.jupiter.api.Test;
import org.testcontainers.shaded.com.google.common.collect.ImmutableMap;

/** Unit tests for {@link RequestComponent}. */
public class RequestComponentTest {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
import com.google.common.collect.Multimaps;
import com.google.common.net.HttpHeaders;
import com.google.common.net.MediaType;
import com.google.common.truth.Truth8;
import com.google.protobuf.Timestamp;
import com.google.protobuf.util.Timestamps;
import dagger.Module;
Expand Down Expand Up @@ -133,7 +132,7 @@ public List<Task> getTestTasksFor(String queue) {
public void assertTasksEnqueuedWithProperty(
String queueName, Function<Task, String> propertyGetter, String... expectedTaskProperties) {
// Ordering is irrelevant but duplicates should be considered independently.
Truth8.assertThat(getTestTasksFor(queueName).stream().map(propertyGetter))
assertThat(getTestTasksFor(queueName).stream().map(propertyGetter))
.containsExactly((Object[]) expectedTaskProperties);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ void testSuccess() throws Exception {
"--max_domains=100",
"--max_creates=500",
"--price=USD 1000.00",
"--next_billing_date=2012-03-17",
"--next_billing_date=2012-03-17T00:00:00Z",
"abc123");

Optional<BulkPricingPackage> bulkPricingPackageOptional =
Expand Down Expand Up @@ -161,7 +161,7 @@ void testSuccess_missingMaxDomainsAndCreatesInitializesToZero() throws Exception
.setAllowedEppActions(ImmutableSet.of(CommandName.CREATE))
.setDiscountFraction(1)
.build());
runCommandForced("--price=USD 1000.00", "--next_billing_date=2012-03-17", "abc123");
runCommandForced("--price=USD 1000.00", "--next_billing_date=2012-03-17T00:00:00Z", "abc123");
Optional<BulkPricingPackage> bulkPricingPackageOptional =
tm().transact(() -> BulkPricingPackage.loadByTokenString("abc123"));
assertThat(bulkPricingPackageOptional).isPresent();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ void testSuccess() throws Exception {
"--max_domains=200",
"--max_creates=1000",
"--price=USD 2000.00",
"--next_billing_date=2013-03-17",
"--next_billing_date=2013-03-17T00:00:00Z",
"--clear_last_notification_sent",
"abc123");

Expand Down Expand Up @@ -106,7 +106,7 @@ void testFailure_bulkPackageDoesNotExist() throws Exception {
"--max_domains=100",
"--max_creates=500",
"--price=USD 1000.00",
"--next_billing_date=2012-03-17",
"--next_billing_date=2012-03-17T00:00:00Z",
"nullPackage"));
Truth.assertThat(thrown.getMessage())
.isEqualTo("BulkPricingPackage with token nullPackage does not exist");
Expand All @@ -117,7 +117,7 @@ void testSuccess_missingMaxDomains() throws Exception {
runCommandForced(
"--max_creates=1000",
"--price=USD 2000.00",
"--next_billing_date=2013-03-17",
"--next_billing_date=2013-03-17T00:00:00Z",
"--clear_last_notification_sent",
"abc123");

Expand Down Expand Up @@ -159,7 +159,7 @@ void testSuccess_missingPrice() throws Exception {
runCommandForced(
"--max_domains=200",
"--max_creates=1000",
"--next_billing_date=2013-03-17",
"--next_billing_date=2013-03-17T00:00:00Z",
"--clear_last_notification_sent",
"abc123");

Expand Down
Loading

0 comments on commit c68583f

Please sign in to comment.