From 73d3b76a89c952293c4e7fc3b26d7b2ff809fda0 Mon Sep 17 00:00:00 2001 From: Lai Jiang Date: Tue, 7 May 2024 20:50:01 -0400 Subject: [PATCH] Remove more usage of AutoValue (#2432) This PR also removes `SerializedForm` used to serialize `PendingDeposit`, as it is now a simple record. --- config/presubmits.py | 12 ++ .../transaction/DatabaseException.java | 2 +- .../secretmanager/SqlCredential.java | 2 +- .../google/registry/rdap/RdapMetrics.java | 135 ++++++++---------- .../google/registry/rdap/RdapResultSet.java | 26 ++-- .../google/registry/rde/PendingDeposit.java | 95 ++++-------- .../registry/bsa/BsaDiffCreatorTest.java | 2 - .../bsa/persistence/LabelDiffUpdatesTest.java | 1 - .../registry/proxy/handler/QuotaHandler.java | 2 +- .../registry/proxy/quota/QuotaManager.java | 37 +---- .../proxy/handler/EppQuotaHandlerTest.java | 28 ++-- .../proxy/handler/WhoisQuotaHandlerTest.java | 26 ++-- .../proxy/quota/QuotaManagerTest.java | 6 +- 13 files changed, 144 insertions(+), 230 deletions(-) diff --git a/config/presubmits.py b/config/presubmits.py index b88b35dff..9a412dab9 100644 --- a/config/presubmits.py +++ b/config/presubmits.py @@ -178,6 +178,18 @@ PRESUBMITS = { {"/node_modules/"}, ): "Do not use shaded dependencies from testcontainers.", + PresubmitCheck( + r".*autovalue\.shaded.*", + "java", + {"/node_modules/"}, + ): + "Do not use shaded dependencies from autovalue.", + PresubmitCheck( + r".*avro\.shaded.*", + "java", + {"/node_modules/"}, + ): + "Do not use shaded dependencies from avro.", PresubmitCheck( r".*com\.google\.common\.truth\.Truth8.*", "java", diff --git a/core/src/main/java/google/registry/persistence/transaction/DatabaseException.java b/core/src/main/java/google/registry/persistence/transaction/DatabaseException.java index 89cf8c5d9..ce3a26bb7 100644 --- a/core/src/main/java/google/registry/persistence/transaction/DatabaseException.java +++ b/core/src/main/java/google/registry/persistence/transaction/DatabaseException.java @@ -14,9 +14,9 @@ package google.registry.persistence.transaction; -import autovalue.shaded.com.google.common.collect.ImmutableList; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Joiner; +import com.google.common.collect.ImmutableList; import java.sql.SQLException; import java.util.Optional; import javax.persistence.PersistenceException; diff --git a/core/src/main/java/google/registry/privileges/secretmanager/SqlCredential.java b/core/src/main/java/google/registry/privileges/secretmanager/SqlCredential.java index 1d8a8c44d..ad3dc6627 100644 --- a/core/src/main/java/google/registry/privileges/secretmanager/SqlCredential.java +++ b/core/src/main/java/google/registry/privileges/secretmanager/SqlCredential.java @@ -14,7 +14,7 @@ package google.registry.privileges.secretmanager; -import static avro.shaded.com.google.common.base.Preconditions.checkState; +import static com.google.common.base.Preconditions.checkState; import java.util.List; diff --git a/core/src/main/java/google/registry/rdap/RdapMetrics.java b/core/src/main/java/google/registry/rdap/RdapMetrics.java index b698e8d3d..8228f76d0 100644 --- a/core/src/main/java/google/registry/rdap/RdapMetrics.java +++ b/core/src/main/java/google/registry/rdap/RdapMetrics.java @@ -14,7 +14,7 @@ package google.registry.rdap; -import com.google.auto.value.AutoValue; +import com.google.auto.value.AutoBuilder; import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableSet; import com.google.monitoring.metrics.DistributionFitter; @@ -102,10 +102,7 @@ public class RdapMetrics { static final IncrementableMetric requests = MetricRegistryImpl.getDefault() .newIncrementableMetric( - "/rdap/requests", - "Count of RDAP Requests", - "count", - LABEL_DESCRIPTORS_FOR_REQUESTS); + "/rdap/requests", "Count of RDAP Requests", "count", LABEL_DESCRIPTORS_FOR_REQUESTS); @VisibleForTesting static final IncrementableMetric responses = @@ -163,8 +160,7 @@ public class RdapMetrics { * ways of looking at the data, since cardinality constraints prevent us from saving all the * information in a single metric. */ - public void updateMetrics( - RdapMetricInformation rdapMetricInformation) { + public void updateMetrics(RdapMetricInformation rdapMetricInformation) { requests.increment( rdapMetricInformation.endpointType().toString(), rdapMetricInformation.includeDeleted() ? "YES" : "NO", @@ -206,95 +202,76 @@ public class RdapMetrics { } } - @AutoValue - abstract static class RdapMetricInformation { + /** + * Information on RDAP metrics. + * + * @param endpointType The type of RDAP endpoint (domain, domains, nameserver, etc.). + * @param searchType The search type (by domain name, by nameserver name, etc.). + * @param wildcardType The type of wildcarding requested (prefix, suffix, etc.). + * @param prefixLength The length of the prefix string before the wildcard, if any; any length + * longer than MAX_RECORDED_PREFIX_LENGTH is limited to MAX_RECORDED_PREFIX_LENGTH when + * recording the metric, to avoid cardinality problems. + * @param includeDeleted Whether the search included deleted records. + * @param registrarSpecified Whether the search requested a specific registrar. + * @param role Type of authentication/authorization: public, admin or registrar. + * @param requestMethod Http request method (GET, POST, HEAD, etc.). + * @param statusCode Http status code. + * @param incompletenessWarningType Incompleteness warning type (e.g. truncated). + * @param numDomainsRetrieved Number of domains retrieved from the database; this might be more + * than were actually returned in the response; absent if a search was not performed. + * @param numHostsRetrieved Number of hosts retrieved from the database; this might be more than + * were actually returned in the response; absent if a search was not performed. + * @param numContactsRetrieved Number of contacts retrieved from the database; this might be more + * than were actually returned in the response; absent if a search was not performed. + */ + record RdapMetricInformation( + EndpointType endpointType, + SearchType searchType, + WildcardType wildcardType, + int prefixLength, + boolean includeDeleted, + boolean registrarSpecified, + RdapAuthorization.Role role, + Action.Method requestMethod, + int statusCode, + IncompletenessWarningType incompletenessWarningType, + Optional numDomainsRetrieved, + Optional numHostsRetrieved, + Optional numContactsRetrieved) { - /** The type of RDAP endpoint (domain, domains, nameserver, etc.). */ - abstract EndpointType endpointType(); + @AutoBuilder + interface Builder { + Builder setEndpointType(EndpointType endpointType); - /** The search type (by domain name, by nameserver name, etc.). */ - abstract SearchType searchType(); + Builder setSearchType(SearchType searchType); - /** The type of wildcarding requested (prefix, suffix, etc.). */ - abstract WildcardType wildcardType(); + Builder setWildcardType(WildcardType wildcardType); - /** - * The length of the prefix string before the wildcard, if any; any length longer than - * MAX_RECORDED_PREFIX_LENGTH is limited to MAX_RECORDED_PREFIX_LENGTH when recording the - * metric, to avoid cardinality problems. - */ - abstract int prefixLength(); + Builder setPrefixLength(int prefixLength); - /** Whether the search included deleted records. */ - abstract boolean includeDeleted(); + Builder setIncludeDeleted(boolean includeDeleted); - /** Whether the search requested a specific registrar. */ - abstract boolean registrarSpecified(); + Builder setRegistrarSpecified(boolean registrarSpecified); - /** Type of authentication/authorization: public, admin or registrar. */ - abstract RdapAuthorization.Role role(); + Builder setRole(RdapAuthorization.Role role); - /** Http request method (GET, POST, HEAD, etc.). */ - abstract Action.Method requestMethod(); + Builder setRequestMethod(Action.Method requestMethod); - /** Http status code. */ - abstract int statusCode(); + Builder setStatusCode(int statusCode); - /** Incompleteness warning type (e.g. truncated). */ - abstract IncompletenessWarningType incompletenessWarningType(); + Builder setIncompletenessWarningType(IncompletenessWarningType incompletenessWarningType); - /** - * Number of domains retrieved from the database; this might be more than were actually returned - * in the response; absent if a search was not performed. - */ - abstract Optional numDomainsRetrieved(); + Builder setNumDomainsRetrieved(long numDomainsRetrieved); - /** - * Number of hosts retrieved from the database; this might be more than were actually returned - * in the response; absent if a search was not performed. - */ - abstract Optional numHostsRetrieved(); + Builder setNumHostsRetrieved(long numHostsRetrieved); - /** - * Number of contacts retrieved from the database; this might be more than were actually - * returned in the response; absent if a search was not performed. - */ - abstract Optional numContactsRetrieved(); + Builder setNumContactsRetrieved(long numContactRetrieved); - @AutoValue.Builder - abstract static class Builder { - abstract Builder setEndpointType(EndpointType endpointType); - - abstract Builder setSearchType(SearchType searchType); - - abstract Builder setWildcardType(WildcardType wildcardType); - - abstract Builder setPrefixLength(int prefixLength); - - abstract Builder setIncludeDeleted(boolean includeDeleted); - - abstract Builder setRegistrarSpecified(boolean registrarSpecified); - - abstract Builder setRole(RdapAuthorization.Role role); - - abstract Builder setRequestMethod(Action.Method requestMethod); - - abstract Builder setStatusCode(int statusCode); - - abstract Builder setIncompletenessWarningType( - IncompletenessWarningType incompletenessWarningType); - - abstract Builder setNumDomainsRetrieved(long numDomainsRetrieved); - - abstract Builder setNumHostsRetrieved(long numHostsRetrieved); - - abstract Builder setNumContactsRetrieved(long numContactRetrieved); - - abstract RdapMetricInformation build(); + RdapMetricInformation build(); } static Builder builder() { - return new AutoValue_RdapMetrics_RdapMetricInformation.Builder() + return new AutoBuilder_RdapMetrics_RdapMetricInformation_Builder() .setSearchType(SearchType.NONE) .setWildcardType(WildcardType.INVALID) .setPrefixLength(0) diff --git a/core/src/main/java/google/registry/rdap/RdapResultSet.java b/core/src/main/java/google/registry/rdap/RdapResultSet.java index 005081046..bbaa41760 100644 --- a/core/src/main/java/google/registry/rdap/RdapResultSet.java +++ b/core/src/main/java/google/registry/rdap/RdapResultSet.java @@ -14,14 +14,23 @@ package google.registry.rdap; -import com.google.auto.value.AutoValue; import com.google.common.collect.ImmutableList; import google.registry.model.EppResource; import google.registry.rdap.RdapSearchResults.IncompletenessWarningType; import java.util.List; -@AutoValue -abstract class RdapResultSet { +/** + * RDAP result set. + * + * @param resources List of EPP resources. + * @param incompletenessWarningType Type of warning to display regarding possible incomplete data. + * @param numResourcesRetrieved Number of resources retrieved from the database in the process of + * assembling the data set. + */ +record RdapResultSet( + ImmutableList resources, + IncompletenessWarningType incompletenessWarningType, + int numResourcesRetrieved) { static RdapResultSet create(List resources) { return create(resources, IncompletenessWarningType.COMPLETE, resources.size()); @@ -31,16 +40,7 @@ abstract class RdapResultSet { List resources, IncompletenessWarningType incompletenessWarningType, int numResourcesRetrieved) { - return new AutoValue_RdapResultSet<>( + return new RdapResultSet<>( ImmutableList.copyOf(resources), incompletenessWarningType, numResourcesRetrieved); } - - /** List of EPP resources. */ - abstract ImmutableList resources(); - - /** Type of warning to display regarding possible incomplete data. */ - abstract IncompletenessWarningType incompletenessWarningType(); - - /** Number of resources retrieved from the database in the process of assembling the data set. */ - abstract int numResourcesRetrieved(); } diff --git a/core/src/main/java/google/registry/rde/PendingDeposit.java b/core/src/main/java/google/registry/rde/PendingDeposit.java index 86048b8a4..c9e28a1ad 100644 --- a/core/src/main/java/google/registry/rde/PendingDeposit.java +++ b/core/src/main/java/google/registry/rde/PendingDeposit.java @@ -14,16 +14,13 @@ package google.registry.rde; -import static com.google.common.base.Preconditions.checkState; import google.registry.model.common.Cursor.CursorType; import google.registry.model.rde.RdeMode; import java.io.IOException; import java.io.InputStream; -import java.io.ObjectInputStream; -import java.io.ObjectOutputStream; -import java.io.ObjectStreamException; import java.io.OutputStream; +import java.io.Serial; import java.io.Serializable; import java.util.Optional; import javax.annotation.Nullable; @@ -49,10 +46,12 @@ import org.joda.time.Duration; * @param manual True if deposits should be generated via manual operation, which does not update * the cursor, and saves the generated deposits in a special manual subdirectory tree. * @param tld TLD for which a deposit should be generated. - * @param watermark Watermark date for which a deposit should be generated. + * @param watermarkStr String representation of the watermark date for which a deposit should be + * generated. * @param mode Which type of deposit to generate: full (RDE) or thin (BRDA). * @param cursor The cursor type to update (not used in manual operation). - * @param interval Amount of time to increment the cursor (not used in manual operation). + * @param intervalStr String representation of the amount of time to increment the cursor (not used + * in manual operation). * @param directoryWithTrailingSlash Subdirectory of bucket/manual in which files should be placed, * including a trailing slash (used only in manual operation). * @param revision Revision number for generated files; if absent, use the next available in the @@ -61,19 +60,28 @@ import org.joda.time.Duration; public record PendingDeposit( boolean manual, String tld, - DateTime watermark, + String watermarkStr, RdeMode mode, - CursorType cursor, - Duration interval, - String directoryWithTrailingSlash, + @Nullable CursorType cursor, + @Nullable String intervalStr, + @Nullable String directoryWithTrailingSlash, @Nullable Integer revision) implements Serializable { - private static final long serialVersionUID = 3141095605225904433L; + public DateTime watermark() { + return DateTime.parse(watermarkStr); + } + + public Duration interval() { + return intervalStr == null ? null : Duration.parse(intervalStr); + } + + @Serial private static final long serialVersionUID = 3141095605225904433L; public static PendingDeposit create( String tld, DateTime watermark, RdeMode mode, CursorType cursor, Duration interval) { - return new PendingDeposit(false, tld, watermark, mode, cursor, interval, null, null); + return new PendingDeposit( + false, tld, watermark.toString(), mode, cursor, interval.toString(), null, null); } public static PendingDeposit createInManualOperation( @@ -83,55 +91,7 @@ public record PendingDeposit( String directoryWithTrailingSlash, @Nullable Integer revision) { return new PendingDeposit( - true, tld, watermark, mode, null, null, directoryWithTrailingSlash, revision); - } - - /** - * Specifies that {@link SerializedForm} be used for {@code SafeObjectInputStream}-compatible - * custom-serialization of {@link PendingDeposit the AutoValue implementation class}. - * - *

This method is package-protected so that the AutoValue implementation class inherits this - * behavior. - * - *

This method leverages {@link PendingDepositCoder} to serializes an instance. However, it is - * not invoked in Beam pipelines. - */ - Object writeReplace() throws ObjectStreamException { - return new SerializedForm(this); - } - - /** - * Proxy for custom-serialization of {@link PendingDeposit}. This is necessary because the actual - * class to be (de)serialized is the generated AutoValue implementation. See also {@link - * #writeReplace}. - * - *

This class leverages {@link PendingDepositCoder} to safely deserializes an instance. - * However, it is not used in Beam pipelines. - */ - private static class SerializedForm implements Serializable { - - private static final long serialVersionUID = 3141095605225904433L; - - private PendingDeposit value; - - private SerializedForm(PendingDeposit value) { - this.value = value; - } - - private void writeObject(ObjectOutputStream os) throws IOException { - checkState(value != null, "Non-null value expected for serialization."); - PendingDepositCoder.INSTANCE.encode(value, os); - } - - private void readObject(ObjectInputStream is) throws IOException, ClassNotFoundException { - checkState(value == null, "Non-null value unexpected for deserialization."); - this.value = PendingDepositCoder.INSTANCE.decode(is); - } - - @SuppressWarnings("unused") - private Object readResolve() throws ObjectStreamException { - return this.value; - } + true, tld, watermark.toString(), mode, null, null, directoryWithTrailingSlash, revision); } /** @@ -158,15 +118,12 @@ public record PendingDeposit( public void encode(PendingDeposit value, OutputStream outStream) throws IOException { BooleanCoder.of().encode(value.manual(), outStream); StringUtf8Coder.of().encode(value.tld(), outStream); - StringUtf8Coder.of().encode(value.watermark().toString(), outStream); + StringUtf8Coder.of().encode(value.watermarkStr(), outStream); StringUtf8Coder.of().encode(value.mode().name(), outStream); NullableCoder.of(StringUtf8Coder.of()) .encode( Optional.ofNullable(value.cursor()).map(CursorType::name).orElse(null), outStream); - NullableCoder.of(StringUtf8Coder.of()) - .encode( - Optional.ofNullable(value.interval()).map(Duration::toString).orElse(null), - outStream); + NullableCoder.of(StringUtf8Coder.of()).encode(value.intervalStr(), outStream); NullableCoder.of(StringUtf8Coder.of()).encode(value.directoryWithTrailingSlash(), outStream); NullableCoder.of(VarIntCoder.of()).encode(value.revision(), outStream); } @@ -176,14 +133,12 @@ public record PendingDeposit( return new PendingDeposit( BooleanCoder.of().decode(inStream), StringUtf8Coder.of().decode(inStream), - DateTime.parse(StringUtf8Coder.of().decode(inStream)), + StringUtf8Coder.of().decode(inStream), RdeMode.valueOf(StringUtf8Coder.of().decode(inStream)), Optional.ofNullable(NullableCoder.of(StringUtf8Coder.of()).decode(inStream)) .map(CursorType::valueOf) .orElse(null), - Optional.ofNullable(NullableCoder.of(StringUtf8Coder.of()).decode(inStream)) - .map(Duration::parse) - .orElse(null), + NullableCoder.of(StringUtf8Coder.of()).decode(inStream), NullableCoder.of(StringUtf8Coder.of()).decode(inStream), NullableCoder.of(VarIntCoder.of()).decode(inStream)); } diff --git a/core/src/test/java/google/registry/bsa/BsaDiffCreatorTest.java b/core/src/test/java/google/registry/bsa/BsaDiffCreatorTest.java index 7ad9583c7..4d0e737be 100644 --- a/core/src/test/java/google/registry/bsa/BsaDiffCreatorTest.java +++ b/core/src/test/java/google/registry/bsa/BsaDiffCreatorTest.java @@ -47,11 +47,9 @@ class BsaDiffCreatorTest { @Mock GcsClient gcsClient; - @SuppressWarnings("DoNotMockAutoValue") @Mock DownloadSchedule schedule; - @SuppressWarnings("DoNotMockAutoValue") @Mock CompletedJob completedJob; diff --git a/core/src/test/java/google/registry/bsa/persistence/LabelDiffUpdatesTest.java b/core/src/test/java/google/registry/bsa/persistence/LabelDiffUpdatesTest.java index 5c8e6d6e3..81477b0e1 100644 --- a/core/src/test/java/google/registry/bsa/persistence/LabelDiffUpdatesTest.java +++ b/core/src/test/java/google/registry/bsa/persistence/LabelDiffUpdatesTest.java @@ -60,7 +60,6 @@ class LabelDiffUpdatesTest { @Mock IdnChecker idnChecker; - @SuppressWarnings("DoNotMockAutoValue") @Mock DownloadSchedule schedule; diff --git a/proxy/src/main/java/google/registry/proxy/handler/QuotaHandler.java b/proxy/src/main/java/google/registry/proxy/handler/QuotaHandler.java index 183e89a7b..79002a1d3 100644 --- a/proxy/src/main/java/google/registry/proxy/handler/QuotaHandler.java +++ b/proxy/src/main/java/google/registry/proxy/handler/QuotaHandler.java @@ -59,7 +59,7 @@ public abstract class QuotaHandler extends ChannelInboundHandlerAdapter { if (quotaResponse == null) { String userId = getUserId(ctx); checkNotNull(userId, "Cannot obtain User ID"); - quotaResponse = quotaManager.acquireQuota(QuotaRequest.create(userId)); + quotaResponse = quotaManager.acquireQuota(new QuotaRequest(userId)); if (!quotaResponse.success()) { String protocolName = ctx.channel().attr(PROTOCOL_KEY).get().name(); metrics.registerQuotaRejection(protocolName, isUserIdPii() ? "none" : userId); diff --git a/proxy/src/main/java/google/registry/proxy/quota/QuotaManager.java b/proxy/src/main/java/google/registry/proxy/quota/QuotaManager.java index 5515e478a..f6928223e 100644 --- a/proxy/src/main/java/google/registry/proxy/quota/QuotaManager.java +++ b/proxy/src/main/java/google/registry/proxy/quota/QuotaManager.java @@ -14,7 +14,6 @@ package google.registry.proxy.quota; -import com.google.auto.value.AutoValue; import google.registry.proxy.quota.TokenStore.TimestampedInteger; import java.util.concurrent.ExecutorService; import java.util.concurrent.Future; @@ -40,42 +39,16 @@ import org.joda.time.DateTime; public class QuotaManager { /** Value class representing a quota request. */ - @AutoValue - public abstract static class QuotaRequest { - - public static QuotaRequest create(String userId) { - return new AutoValue_QuotaManager_QuotaRequest(userId); - } - - abstract String userId(); - } + public record QuotaRequest(String userId) {} /** Value class representing a quota response. */ - @AutoValue - public abstract static class QuotaResponse { - public static QuotaResponse create( - boolean success, String userId, DateTime grantedTokenRefillTime) { - return new AutoValue_QuotaManager_QuotaResponse(success, userId, grantedTokenRefillTime); - } - - public abstract boolean success(); - - abstract String userId(); - - abstract DateTime grantedTokenRefillTime(); - } + public record QuotaResponse(boolean success, String userId, DateTime grantedTokenRefillTime) {} /** Value class representing a quota rebate. */ - @AutoValue - public abstract static class QuotaRebate { + public record QuotaRebate(String userId, DateTime grantedTokenRefillTime) { public static QuotaRebate create(QuotaResponse response) { - return new AutoValue_QuotaManager_QuotaRebate( - response.userId(), response.grantedTokenRefillTime()); + return new QuotaRebate(response.userId(), response.grantedTokenRefillTime()); } - - abstract String userId(); - - abstract DateTime grantedTokenRefillTime(); } private final TokenStore tokenStore; @@ -91,7 +64,7 @@ public class QuotaManager { /** Attempts to acquire requested quota, synchronously. */ public QuotaResponse acquireQuota(QuotaRequest request) { TimestampedInteger tokens = tokenStore.take(request.userId()); - return QuotaResponse.create(tokens.value() != 0, request.userId(), tokens.timestamp()); + return new QuotaResponse(tokens.value() != 0, request.userId(), tokens.timestamp()); } /** Returns granted quota to the token store, asynchronously. */ diff --git a/proxy/src/test/java/google/registry/proxy/handler/EppQuotaHandlerTest.java b/proxy/src/test/java/google/registry/proxy/handler/EppQuotaHandlerTest.java index 7f579a576..c2ee30c0c 100644 --- a/proxy/src/test/java/google/registry/proxy/handler/EppQuotaHandlerTest.java +++ b/proxy/src/test/java/google/registry/proxy/handler/EppQuotaHandlerTest.java @@ -78,14 +78,14 @@ class EppQuotaHandlerTest { @Test void testSuccess_quotaGrantedAndReturned() { - when(quotaManager.acquireQuota(QuotaRequest.create(clientCertHash))) - .thenReturn(QuotaResponse.create(true, clientCertHash, now)); + when(quotaManager.acquireQuota(new QuotaRequest(clientCertHash))) + .thenReturn(new QuotaResponse(true, clientCertHash, now)); // First read, acquire quota. assertThat(channel.writeInbound(message)).isTrue(); assertThat((Object) channel.readInbound()).isEqualTo(message); assertThat(channel.isActive()).isTrue(); - verify(quotaManager).acquireQuota(QuotaRequest.create(clientCertHash)); + verify(quotaManager).acquireQuota(new QuotaRequest(clientCertHash)); // Second read, should not acquire quota again. Object newMessage = new Object(); @@ -96,19 +96,19 @@ class EppQuotaHandlerTest { // Channel closed, release quota. ChannelFuture unusedFuture = channel.close(); verify(quotaManager) - .releaseQuota(QuotaRebate.create(QuotaResponse.create(true, clientCertHash, now))); + .releaseQuota(QuotaRebate.create(new QuotaResponse(true, clientCertHash, now))); verifyNoMoreInteractions(quotaManager); } @Test void testFailure_quotaNotGranted() { - when(quotaManager.acquireQuota(QuotaRequest.create(clientCertHash))) - .thenReturn(QuotaResponse.create(false, clientCertHash, now)); + when(quotaManager.acquireQuota(new QuotaRequest(clientCertHash))) + .thenReturn(new QuotaResponse(false, clientCertHash, now)); OverQuotaException e = assertThrows(OverQuotaException.class, () -> channel.writeInbound(message)); ChannelFuture unusedFuture = channel.close(); assertThat(e).hasMessageThat().contains(clientCertHash); - verify(quotaManager).acquireQuota(QuotaRequest.create(clientCertHash)); + verify(quotaManager).acquireQuota(new QuotaRequest(clientCertHash)); // Make sure that quotaManager.releaseQuota() is not called when the channel closes. verifyNoMoreInteractions(quotaManager); verify(metrics).registerQuotaRejection("epp", clientCertHash); @@ -125,10 +125,10 @@ class EppQuotaHandlerTest { setProtocol(otherChannel); final DateTime later = now.plus(Duration.standardSeconds(1)); - when(quotaManager.acquireQuota(QuotaRequest.create(clientCertHash))) - .thenReturn(QuotaResponse.create(true, clientCertHash, now)); - when(quotaManager.acquireQuota(QuotaRequest.create(otherClientCertHash))) - .thenReturn(QuotaResponse.create(false, otherClientCertHash, later)); + when(quotaManager.acquireQuota(new QuotaRequest(clientCertHash))) + .thenReturn(new QuotaResponse(true, clientCertHash, now)); + when(quotaManager.acquireQuota(new QuotaRequest(otherClientCertHash))) + .thenReturn(new QuotaResponse(false, otherClientCertHash, later)); // Allows the first user. assertThat(channel.writeInbound(message)).isTrue(); @@ -152,9 +152,9 @@ class EppQuotaHandlerTest { setProtocol(otherChannel); final DateTime later = now.plus(Duration.standardSeconds(1)); - when(quotaManager.acquireQuota(QuotaRequest.create(clientCertHash))) - .thenReturn(QuotaResponse.create(true, clientCertHash, now)) - .thenReturn(QuotaResponse.create(false, clientCertHash, later)); + when(quotaManager.acquireQuota(new QuotaRequest(clientCertHash))) + .thenReturn(new QuotaResponse(true, clientCertHash, now)) + .thenReturn(new QuotaResponse(false, clientCertHash, later)); // Allows the first channel. assertThat(channel.writeInbound(message)).isTrue(); diff --git a/proxy/src/test/java/google/registry/proxy/handler/WhoisQuotaHandlerTest.java b/proxy/src/test/java/google/registry/proxy/handler/WhoisQuotaHandlerTest.java index 1020b9f83..c5940b1f2 100644 --- a/proxy/src/test/java/google/registry/proxy/handler/WhoisQuotaHandlerTest.java +++ b/proxy/src/test/java/google/registry/proxy/handler/WhoisQuotaHandlerTest.java @@ -77,14 +77,14 @@ class WhoisQuotaHandlerTest { @Test void testSuccess_quotaGranted() { - when(quotaManager.acquireQuota(QuotaRequest.create(remoteAddress))) - .thenReturn(QuotaResponse.create(true, remoteAddress, now)); + when(quotaManager.acquireQuota(new QuotaRequest(remoteAddress))) + .thenReturn(new QuotaResponse(true, remoteAddress, now)); // First read, acquire quota. assertThat(channel.writeInbound(message)).isTrue(); assertThat((Object) channel.readInbound()).isEqualTo(message); assertThat(channel.isActive()).isTrue(); - verify(quotaManager).acquireQuota(QuotaRequest.create(remoteAddress)); + verify(quotaManager).acquireQuota(new QuotaRequest(remoteAddress)); // Second read, should not acquire quota again. assertThat(channel.writeInbound(message)).isTrue(); @@ -97,8 +97,8 @@ class WhoisQuotaHandlerTest { @Test void testFailure_quotaNotGranted() { - when(quotaManager.acquireQuota(QuotaRequest.create(remoteAddress))) - .thenReturn(QuotaResponse.create(false, remoteAddress, now)); + when(quotaManager.acquireQuota(new QuotaRequest(remoteAddress))) + .thenReturn(new QuotaResponse(false, remoteAddress, now)); OverQuotaException e = assertThrows(OverQuotaException.class, () -> channel.writeInbound(message)); assertThat(e).hasMessageThat().contains("none"); @@ -116,10 +116,10 @@ class WhoisQuotaHandlerTest { setProtocol(otherChannel); final DateTime later = now.plus(Duration.standardSeconds(1)); - when(quotaManager.acquireQuota(QuotaRequest.create(remoteAddress))) - .thenReturn(QuotaResponse.create(true, remoteAddress, now)); - when(quotaManager.acquireQuota(QuotaRequest.create(otherRemoteAddress))) - .thenReturn(QuotaResponse.create(false, otherRemoteAddress, later)); + when(quotaManager.acquireQuota(new QuotaRequest(remoteAddress))) + .thenReturn(new QuotaResponse(true, remoteAddress, now)); + when(quotaManager.acquireQuota(new QuotaRequest(otherRemoteAddress))) + .thenReturn(new QuotaResponse(false, otherRemoteAddress, later)); // Allows the first user. assertThat(channel.writeInbound(message)).isTrue(); @@ -149,12 +149,12 @@ class WhoisQuotaHandlerTest { thirdChannel.attr(REMOTE_ADDRESS_KEY).set(remoteAddress); final DateTime evenLater = now.plus(Duration.standardSeconds(60)); - when(quotaManager.acquireQuota(QuotaRequest.create(remoteAddress))) - .thenReturn(QuotaResponse.create(true, remoteAddress, now)) + when(quotaManager.acquireQuota(new QuotaRequest(remoteAddress))) + .thenReturn(new QuotaResponse(true, remoteAddress, now)) // Throttles the second connection. - .thenReturn(QuotaResponse.create(false, remoteAddress, later)) + .thenReturn(new QuotaResponse(false, remoteAddress, later)) // Allows the third connection because token refilled. - .thenReturn(QuotaResponse.create(true, remoteAddress, evenLater)); + .thenReturn(new QuotaResponse(true, remoteAddress, evenLater)); // Allows the first channel. assertThat(channel.writeInbound(message)).isTrue(); diff --git a/proxy/src/test/java/google/registry/proxy/quota/QuotaManagerTest.java b/proxy/src/test/java/google/registry/proxy/quota/QuotaManagerTest.java index 450cd73e3..027ad9a3e 100644 --- a/proxy/src/test/java/google/registry/proxy/quota/QuotaManagerTest.java +++ b/proxy/src/test/java/google/registry/proxy/quota/QuotaManagerTest.java @@ -48,7 +48,7 @@ class QuotaManagerTest { void testSuccess_requestApproved() { when(tokenStore.take(anyString())).thenReturn(TimestampedInteger.create(1, clock.nowUtc())); - request = QuotaRequest.create(USER_ID); + request = new QuotaRequest(USER_ID); response = quotaManager.acquireQuota(request); assertThat(response.success()).isTrue(); assertThat(response.userId()).isEqualTo(USER_ID); @@ -59,7 +59,7 @@ class QuotaManagerTest { void testSuccess_requestDenied() { when(tokenStore.take(anyString())).thenReturn(TimestampedInteger.create(0, clock.nowUtc())); - request = QuotaRequest.create(USER_ID); + request = new QuotaRequest(USER_ID); response = quotaManager.acquireQuota(request); assertThat(response.success()).isFalse(); assertThat(response.userId()).isEqualTo(USER_ID); @@ -69,7 +69,7 @@ class QuotaManagerTest { @Test void testSuccess_rebate() { DateTime grantedTokenRefillTime = clock.nowUtc(); - response = QuotaResponse.create(true, USER_ID, grantedTokenRefillTime); + response = new QuotaResponse(true, USER_ID, grantedTokenRefillTime); QuotaRebate rebate = QuotaRebate.create(response); Future unusedFuture = quotaManager.releaseQuota(rebate); verify(tokenStore).scheduleRefresh();