mirror of
https://github.com/google/nomulus
synced 2026-05-23 16:21:55 +00:00
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.
This commit is contained in:
@@ -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()) {
|
||||
|
||||
@@ -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}. */
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -146,20 +146,20 @@ public class AuthModule {
|
||||
@LocalCredentialJson
|
||||
public static String provideLocalCredentialJson(
|
||||
Lazy<GoogleClientSecrets> clientSecrets,
|
||||
Gson gson,
|
||||
@StoredCredential Lazy<Credential> credential,
|
||||
@Nullable @Config("credentialFilePath") String credentialFilePath) {
|
||||
try {
|
||||
if (credentialFilePath != null) {
|
||||
return Files.readString(Paths.get(credentialFilePath));
|
||||
} else {
|
||||
return new Gson()
|
||||
.toJson(
|
||||
ImmutableMap.<String, String>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.<String, String>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);
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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());
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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<String, String> jsonMap =
|
||||
new Gson().fromJson(credentialJson, new TypeToken<Map<String, String>>() {}.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}");
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user