Skip to content

Commit

Permalink
Remove java.util.Date
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 5f9c7de commit d211ef2
Show file tree
Hide file tree
Showing 13 changed files with 104 additions and 143 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
Loading

0 comments on commit d211ef2

Please sign in to comment.