From 1e9c11d1344ff0746388412ba8ca8139926be8ca Mon Sep 17 00:00:00 2001 From: Rachel Tannenbaum Date: Fri, 17 Aug 2018 16:43:31 -0400 Subject: [PATCH] Code review --- .../auth/google/GoogleAuthDataGenerator.java | 2 +- .../auth/instagram/InstagramAuthDataGenerator.java | 2 +- .../auth/microsoft/MicrosoftAuthDataGenerator.java | 2 +- .../auth/offline/OfflineDemoAuthDataGenerator.java | 2 +- .../api/action/transfer/GetTransferJobAction.java | 1 + .../api/action/transfer/StartTransferJobAction.java | 1 + .../datatransferproject/spi/api/auth/AuthDataGenerator.java | 6 +++++- .../spi/api/types/AuthFlowConfiguration.java | 2 +- 8 files changed, 12 insertions(+), 6 deletions(-) diff --git a/extensions/auth/portability-auth-google/src/main/java/org/datatransferproject/auth/google/GoogleAuthDataGenerator.java b/extensions/auth/portability-auth-google/src/main/java/org/datatransferproject/auth/google/GoogleAuthDataGenerator.java index 0099672bc..09c06f023 100644 --- a/extensions/auth/portability-auth-google/src/main/java/org/datatransferproject/auth/google/GoogleAuthDataGenerator.java +++ b/extensions/auth/portability-auth-google/src/main/java/org/datatransferproject/auth/google/GoogleAuthDataGenerator.java @@ -120,7 +120,7 @@ public AuthFlowConfiguration generateConfiguration(String callbackBaseUrl, Strin .setRedirectUri(callbackBaseUrl + redirectPath) .setState(encodedJobId) .build(); - return new AuthFlowConfiguration(url, getTokenUrl(), AUTH_PROTOCOL); + return new AuthFlowConfiguration(url, AUTH_PROTOCOL, getTokenUrl()); } @Override diff --git a/extensions/auth/portability-auth-instagram/src/main/java/org/datatransferproject/auth/instagram/InstagramAuthDataGenerator.java b/extensions/auth/portability-auth-instagram/src/main/java/org/datatransferproject/auth/instagram/InstagramAuthDataGenerator.java index f046e6df7..847f4754a 100644 --- a/extensions/auth/portability-auth-instagram/src/main/java/org/datatransferproject/auth/instagram/InstagramAuthDataGenerator.java +++ b/extensions/auth/portability-auth-instagram/src/main/java/org/datatransferproject/auth/instagram/InstagramAuthDataGenerator.java @@ -66,7 +66,7 @@ public AuthFlowConfiguration generateConfiguration(String callbackBaseUrl, Strin .setRedirectUri(callbackBaseUrl + CALLBACK_PATH) .setState(encodedJobId) .build(); - return new AuthFlowConfiguration(url, getTokenUrl(), AUTHORIZATION_PROTOCOL); + return new AuthFlowConfiguration(url, AUTHORIZATION_PROTOCOL, getTokenUrl()); } @Override diff --git a/extensions/auth/portability-auth-microsoft/src/main/java/org/datatransferproject/auth/microsoft/MicrosoftAuthDataGenerator.java b/extensions/auth/portability-auth-microsoft/src/main/java/org/datatransferproject/auth/microsoft/MicrosoftAuthDataGenerator.java index 61eeabbfb..c686e2c45 100644 --- a/extensions/auth/portability-auth-microsoft/src/main/java/org/datatransferproject/auth/microsoft/MicrosoftAuthDataGenerator.java +++ b/extensions/auth/portability-auth-microsoft/src/main/java/org/datatransferproject/auth/microsoft/MicrosoftAuthDataGenerator.java @@ -108,7 +108,7 @@ public AuthFlowConfiguration generateConfiguration(String callbackBaseUrl, Strin // constructs a request for the Microsoft Graph authorization code. String redirectUrl = callbackBaseUrl + redirectPath; String queryPart = constructAuthQueryPart(redirectUrl, id, scopes); - return new AuthFlowConfiguration(AUTHORIZATION_URL + "?" + queryPart, getTokenUrl(), AUTHORIZATION_PROTOCOL); + return new AuthFlowConfiguration(AUTHORIZATION_URL + "?" + queryPart, AUTHORIZATION_PROTOCOL, getTokenUrl()); } public TokenAuthData generateAuthData( diff --git a/extensions/auth/portability-auth-offline-demo/src/main/java/org/datatransferproject/auth/offline/OfflineDemoAuthDataGenerator.java b/extensions/auth/portability-auth-offline-demo/src/main/java/org/datatransferproject/auth/offline/OfflineDemoAuthDataGenerator.java index 7ea3f348b..5c5ab4850 100644 --- a/extensions/auth/portability-auth-offline-demo/src/main/java/org/datatransferproject/auth/offline/OfflineDemoAuthDataGenerator.java +++ b/extensions/auth/portability-auth-offline-demo/src/main/java/org/datatransferproject/auth/offline/OfflineDemoAuthDataGenerator.java @@ -34,7 +34,7 @@ public class OfflineDemoAuthDataGenerator implements AuthDataGenerator { @Override public AuthFlowConfiguration generateConfiguration(String callbackBaseUrl, String id) { - return new AuthFlowConfiguration(callbackBaseUrl + "/callback/offline-demo?code=123", getTokenUrl(), AUTH_PROTOCOL); + return new AuthFlowConfiguration(callbackBaseUrl + "/callback/offline-demo?code=123", AUTH_PROTOCOL, getTokenUrl()); } @Override diff --git a/portability-api/src/main/java/org/datatransferproject/api/action/transfer/GetTransferJobAction.java b/portability-api/src/main/java/org/datatransferproject/api/action/transfer/GetTransferJobAction.java index c90e383a5..b974d2533 100644 --- a/portability-api/src/main/java/org/datatransferproject/api/action/transfer/GetTransferJobAction.java +++ b/portability-api/src/main/java/org/datatransferproject/api/action/transfer/GetTransferJobAction.java @@ -34,6 +34,7 @@ public TransferJob handle(GetTransferJob transferRequest) { PortabilityJob job = jobStore.findJob(jobId); + // TODO(#553): This list of nulls should be cleaned up when we refactor TransferJob. return new TransferJob(id, job.exportService(), job.importService(), job.transferDataType(), null, null, null, null, null, null); } diff --git a/portability-api/src/main/java/org/datatransferproject/api/action/transfer/StartTransferJobAction.java b/portability-api/src/main/java/org/datatransferproject/api/action/transfer/StartTransferJobAction.java index eced357ca..6f3d925e9 100644 --- a/portability-api/src/main/java/org/datatransferproject/api/action/transfer/StartTransferJobAction.java +++ b/portability-api/src/main/java/org/datatransferproject/api/action/transfer/StartTransferJobAction.java @@ -66,6 +66,7 @@ public TransferJob handle(StartTransferJob startTransferJob) { startTransferJob.getImportAuthData()); } + // TODO(#553): This list of nulls should be cleaned up when we refactor TransferJob. return new TransferJob(id, job.exportService(), job.importService(), job.transferDataType(), null, null, null, null, null, null); } diff --git a/portability-spi-api/src/main/java/org/datatransferproject/spi/api/auth/AuthDataGenerator.java b/portability-spi-api/src/main/java/org/datatransferproject/spi/api/auth/AuthDataGenerator.java index 4a3c55e58..bfe50b94b 100644 --- a/portability-spi-api/src/main/java/org/datatransferproject/spi/api/auth/AuthDataGenerator.java +++ b/portability-spi-api/src/main/java/org/datatransferproject/spi/api/auth/AuthDataGenerator.java @@ -32,7 +32,11 @@ public interface AuthDataGenerator { AuthData generateAuthData( String callbackBaseUrl, String authCode, String id, AuthData initialAuthData, String extra); - // TODO(#553): Implement this for auth extensions. Currently unused by the demo-server frontend. + /** + * Return the URL used to exchange an access code for token. + * + *

TODO(#553): Implement this for auth extensions. Currently unused by the demo-server frontend. + */ default String getTokenUrl() { return ""; } diff --git a/portability-spi-api/src/main/java/org/datatransferproject/spi/api/types/AuthFlowConfiguration.java b/portability-spi-api/src/main/java/org/datatransferproject/spi/api/types/AuthFlowConfiguration.java index 4c87cedd2..49aa6a134 100644 --- a/portability-spi-api/src/main/java/org/datatransferproject/spi/api/types/AuthFlowConfiguration.java +++ b/portability-spi-api/src/main/java/org/datatransferproject/spi/api/types/AuthFlowConfiguration.java @@ -24,7 +24,7 @@ public class AuthFlowConfiguration extends PortableType { * * @param authUrl the initial URL. */ - public AuthFlowConfiguration(String authUrl, String tokenUrl, AuthProtocol authProtocol) { + public AuthFlowConfiguration(String authUrl, AuthProtocol authProtocol, String tokenUrl) { this.authUrl = authUrl; this.tokenUrl = tokenUrl; this.authProtocol = authProtocol;