From b3fc57c7f7d03e02c68dc41577c649d686c88d23 Mon Sep 17 00:00:00 2001 From: gbrodman Date: Fri, 22 May 2026 11:20:23 -0400 Subject: [PATCH] Inject a singleton Gson instead of creating it many times (#3058) Creation of Gson objects is nontrivial and it's thread-safe so we might as well just use some singleton objects as much as possible rather than recreating them. --- .../google/registry/bsa/api/BsaCredential.java | 7 +++++-- .../java/google/registry/request/Modules.java | 9 +++++++++ .../google/registry/request/RequestModule.java | 8 -------- .../java/google/registry/tools/AuthModule.java | 16 ++++++++-------- .../registry/bsa/api/BsaCredentialTest.java | 6 +++++- .../registry/testing/ConsoleApiParamsUtils.java | 4 ++-- .../google/registry/tools/AuthModuleTest.java | 9 +++++++-- 7 files changed, 36 insertions(+), 23 deletions(-) diff --git a/core/src/main/java/google/registry/bsa/api/BsaCredential.java b/core/src/main/java/google/registry/bsa/api/BsaCredential.java index f85e1fb29..39fc8fe18 100644 --- a/core/src/main/java/google/registry/bsa/api/BsaCredential.java +++ b/core/src/main/java/google/registry/bsa/api/BsaCredential.java @@ -68,6 +68,8 @@ public class BsaCredential { private final Keyring keyring; + private final Gson gson; + private final Clock clock; @Nullable private String authToken; @@ -79,11 +81,13 @@ public class BsaCredential { @Config("bsaAuthUrl") String authUrl, @Config("bsaAuthTokenExpiry") Duration authTokenExpiry, Keyring keyring, + Gson gson, Clock clock) { this.urlConnectionService = urlConnectionService; this.authUrl = authUrl; this.authTokenExpiry = authTokenExpiry; this.keyring = keyring; + this.gson = gson; this.clock = clock; } @@ -143,8 +147,7 @@ public class BsaCredential { // TODO: catch json syntax exception @SuppressWarnings("unchecked") String idToken = - new Gson() - .fromJson(new String(getResponseBytes(connection), UTF_8), Map.class) + gson.fromJson(new String(getResponseBytes(connection), UTF_8), Map.class) .getOrDefault(ID_TOKEN, "") .toString(); if (idToken.isEmpty()) { diff --git a/core/src/main/java/google/registry/request/Modules.java b/core/src/main/java/google/registry/request/Modules.java index 0dbd86e52..01c8899ab 100644 --- a/core/src/main/java/google/registry/request/Modules.java +++ b/core/src/main/java/google/registry/request/Modules.java @@ -18,8 +18,10 @@ import com.google.api.client.googleapis.javanet.GoogleNetHttpTransport; import com.google.api.client.http.javanet.NetHttpTransport; import com.google.api.client.json.JsonFactory; import com.google.api.client.json.gson.GsonFactory; +import com.google.gson.Gson; import dagger.Module; import dagger.Provides; +import google.registry.tools.GsonUtils; import jakarta.inject.Singleton; import java.net.HttpURLConnection; import javax.net.ssl.HttpsURLConnection; @@ -49,9 +51,16 @@ public final class Modules { @Module public static final class GsonModule { @Provides + @Singleton static JsonFactory provideJsonFactory() { return GsonFactory.getDefaultInstance(); } + + @Provides + @Singleton + public static Gson provideGson() { + return GsonUtils.provideGson(); + } } /** Dagger module that provides standard {@link NetHttpTransport}. */ diff --git a/core/src/main/java/google/registry/request/RequestModule.java b/core/src/main/java/google/registry/request/RequestModule.java index d31e6917a..3355a2268 100644 --- a/core/src/main/java/google/registry/request/RequestModule.java +++ b/core/src/main/java/google/registry/request/RequestModule.java @@ -44,7 +44,6 @@ import google.registry.request.HttpException.UnsupportedMediaTypeException; import google.registry.request.auth.AuthResult; import google.registry.request.lock.LockHandler; import google.registry.request.lock.LockHandlerImpl; -import google.registry.tools.GsonUtils; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; import jakarta.servlet.http.HttpSession; @@ -74,13 +73,6 @@ public final class RequestModule { this.authResult = authResult; } - @RequestScope - @VisibleForTesting - @Provides - public static Gson provideGson() { - return GsonUtils.provideGson(); - } - @Provides @Parameter(RequestParameters.PARAM_TLD) static String provideTld(HttpServletRequest req) { diff --git a/core/src/main/java/google/registry/tools/AuthModule.java b/core/src/main/java/google/registry/tools/AuthModule.java index c385d0c48..4de2c6670 100644 --- a/core/src/main/java/google/registry/tools/AuthModule.java +++ b/core/src/main/java/google/registry/tools/AuthModule.java @@ -146,20 +146,20 @@ public class AuthModule { @LocalCredentialJson public static String provideLocalCredentialJson( Lazy clientSecrets, + Gson gson, @StoredCredential Lazy credential, @Nullable @Config("credentialFilePath") String credentialFilePath) { try { if (credentialFilePath != null) { return Files.readString(Paths.get(credentialFilePath)); } else { - return new Gson() - .toJson( - ImmutableMap.builder() - .put("type", "authorized_user") - .put("client_id", clientSecrets.get().getDetails().getClientId()) - .put("client_secret", clientSecrets.get().getDetails().getClientSecret()) - .put("refresh_token", credential.get().getRefreshToken()) - .build()); + return gson.toJson( + ImmutableMap.builder() + .put("type", "authorized_user") + .put("client_id", clientSecrets.get().getDetails().getClientId()) + .put("client_secret", clientSecrets.get().getDetails().getClientSecret()) + .put("refresh_token", credential.get().getRefreshToken()) + .build()); } } catch (IOException e) { throw new RuntimeException(e); diff --git a/core/src/test/java/google/registry/bsa/api/BsaCredentialTest.java b/core/src/test/java/google/registry/bsa/api/BsaCredentialTest.java index 63510e7d0..2d0cbbdae 100644 --- a/core/src/test/java/google/registry/bsa/api/BsaCredentialTest.java +++ b/core/src/test/java/google/registry/bsa/api/BsaCredentialTest.java @@ -28,9 +28,11 @@ import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import com.google.gson.Gson; import google.registry.keyring.api.Keyring; import google.registry.request.UrlConnectionService; import google.registry.testing.FakeClock; +import google.registry.tools.GsonUtils; import java.io.ByteArrayInputStream; import java.io.IOException; import java.io.OutputStream; @@ -49,6 +51,7 @@ import org.mockito.junit.jupiter.MockitoExtension; class BsaCredentialTest { private static final Duration AUTH_TOKEN_EXPIRY = Duration.ofMinutes(30); + private static final Gson GSON = GsonUtils.provideGson(); @Mock OutputStream connectionOutputStream; @Mock HttpsURLConnection connection; @@ -60,7 +63,8 @@ class BsaCredentialTest { @BeforeEach void setup() throws Exception { credential = - new BsaCredential(connectionService, "https://authUrl", AUTH_TOKEN_EXPIRY, keyring, clock); + new BsaCredential( + connectionService, "https://authUrl", AUTH_TOKEN_EXPIRY, keyring, GSON, clock); } void setupHttp() throws Exception { diff --git a/core/src/test/java/google/registry/testing/ConsoleApiParamsUtils.java b/core/src/test/java/google/registry/testing/ConsoleApiParamsUtils.java index f116af329..37e973f31 100644 --- a/core/src/test/java/google/registry/testing/ConsoleApiParamsUtils.java +++ b/core/src/test/java/google/registry/testing/ConsoleApiParamsUtils.java @@ -20,7 +20,7 @@ import static org.mockito.Mockito.when; import com.google.common.collect.ImmutableList; import google.registry.groups.GmailClient; import google.registry.model.console.User; -import google.registry.request.RequestModule; +import google.registry.request.Modules; import google.registry.request.auth.AuthResult; import google.registry.security.XsrfTokenManager; import google.registry.ui.server.SendEmailUtils; @@ -50,6 +50,6 @@ public final class ConsoleApiParamsUtils { authResult, sendEmailUtils, xsrfTokenManager, - RequestModule.provideGson()); + Modules.GsonModule.provideGson()); } } diff --git a/core/src/test/java/google/registry/tools/AuthModuleTest.java b/core/src/test/java/google/registry/tools/AuthModuleTest.java index b1b550c6d..27a3be20a 100644 --- a/core/src/test/java/google/registry/tools/AuthModuleTest.java +++ b/core/src/test/java/google/registry/tools/AuthModuleTest.java @@ -55,6 +55,8 @@ class AuthModuleTest { private static final String ACCESS_TOKEN = "FakeAccessToken"; private static final String REFRESH_TOKEN = "FakeReFreshToken"; + private static final Gson GSON = GsonUtils.provideGson(); + @SuppressWarnings("WeakerAccess") @TempDir Path folder; @@ -166,7 +168,7 @@ class AuthModuleTest { void test_provideLocalCredentialJson() { String credentialJson = AuthModule.provideLocalCredentialJson( - AuthModuleTest::getSecrets, this::getCredential, null); + AuthModuleTest::getSecrets, GSON, this::getCredential, null); Map jsonMap = new Gson().fromJson(credentialJson, new TypeToken>() {}.getType()); assertThat(jsonMap.get("type")).isEqualTo("authorized_user"); @@ -182,7 +184,10 @@ class AuthModuleTest { Files.writeString(credentialFile.toPath(), "{some_field: some_value}"); String credentialJson = AuthModule.provideLocalCredentialJson( - AuthModuleTest::getSecrets, this::getCredential, credentialFile.getCanonicalPath()); + AuthModuleTest::getSecrets, + GSON, + this::getCredential, + credentialFile.getCanonicalPath()); assertThat(credentialJson).isEqualTo("{some_field: some_value}"); }