Remove GitHubClient.login field

src/main/java/org/kohsuke/github/GitHub.java

@@ -28,6 +28,8 @@
 import com.infradna.tool.bridge_method_injector.WithBridgeMethods;
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import org.kohsuke.github.authorization.AuthorizationProvider;
+import org.kohsuke.github.authorization.ImmutableAuthorizationProvider;
+import org.kohsuke.github.authorization.UserAuthorizationProvider;
 import org.kohsuke.github.internal.Previews;
 
 import java.io.*;
@@ -115,23 +117,69 @@ public class GitHub {
             AuthorizationProvider authorizationProvider) throws IOException {
         if (authorizationProvider instanceof DependentAuthorizationProvider) {
             ((DependentAuthorizationProvider) authorizationProvider).bind(this);
+        } else if (authorizationProvider instanceof ImmutableAuthorizationProvider
+                && authorizationProvider instanceof UserAuthorizationProvider) {
+            UserAuthorizationProvider provider = (UserAuthorizationProvider) authorizationProvider;
+            if (provider.getLogin() == null && provider.getEncodedAuthorization() != null
+                    && provider.getEncodedAuthorization().startsWith("token")) {
+                authorizationProvider = new LoginLoadingUserAuthorizationProvider(provider, this);
+            }
         }
 
+        users = new ConcurrentHashMap<>();
+        orgs = new ConcurrentHashMap<>();
+
         this.client = new GitHubHttpUrlConnectionClient(apiUrl,
                 connector,
                 rateLimitHandler,
                 abuseLimitHandler,
                 rateLimitChecker,
-                (myself) -> setMyself(myself),
                 authorizationProvider);
-        users = new ConcurrentHashMap<>();
-        orgs = new ConcurrentHashMap<>();
+
+        // reproduce previously existing behavior the gets the login early
+        if (authorizationProvider instanceof LoginLoadingUserAuthorizationProvider) {
+            ((LoginLoadingUserAuthorizationProvider) authorizationProvider).getLogin();
+        }
     }
 
     private GitHub(GitHubClient client) {
-        this.client = client;
         users = new ConcurrentHashMap<>();
         orgs = new ConcurrentHashMap<>();
+        this.client = client;
+    }
+
+    private static class LoginLoadingUserAuthorizationProvider implements UserAuthorizationProvider {
+        private final GitHub gitHub;
+        private final AuthorizationProvider authorizationProvider;
+        private boolean loginLoaded = false;
+        private String login;
+
+        LoginLoadingUserAuthorizationProvider(AuthorizationProvider authorizationProvider, GitHub gitHub) {
+            this.gitHub = gitHub;
+            this.authorizationProvider = authorizationProvider;
+        }
+
+        @Override
+        public String getEncodedAuthorization() throws IOException {
+            return authorizationProvider.getEncodedAuthorization();
+        }
+
+        @Override
+        public String getLogin() {
+            synchronized (this) {
+                if (!loginLoaded) {
+                    loginLoaded = true;
+                    try {
+                        GHMyself u = gitHub.setMyself();
+                        if (u != null) {
+                            login = u.getLogin();
+                        }
+                    } catch (IOException e) {
+                    }
+                }
+                return login;
+            }
+        }
     }
 
     public static abstract class DependentAuthorizationProvider implements AuthorizationProvider {
@@ -506,21 +554,18 @@ public GHRateLimit rateLimit() throws IOException {
     @WithBridgeMethods(value = GHUser.class)
     public GHMyself getMyself() throws IOException {
         client.requireCredential();
+        return setMyself();
+    }
+
+    private GHMyself setMyself() throws IOException {
         synchronized (this) {
             if (this.myself == null) {
-                GHMyself u = createRequest().withUrlPath("/user").fetch(GHMyself.class);
-                setMyself(u);
+                this.myself = createRequest().withUrlPath("/user").fetch(GHMyself.class);
             }
             return myself;
         }
     }
 
-    private void setMyself(GHMyself myself) {
-        synchronized (this) {
-            this.myself = myself;
-        }
-    }
-
     /**
      * Obtains the object that represents the named user.
      *

src/main/java/org/kohsuke/github/GitHubClient.java

@@ -15,7 +15,6 @@
 import java.time.temporal.ChronoUnit;
 import java.util.*;
 import java.util.concurrent.atomic.AtomicReference;
-import java.util.function.Consumer;
 import java.util.logging.Logger;
 
 import javax.annotation.CheckForNull;
@@ -41,7 +40,6 @@ abstract class GitHubClient {
      * If timeout issues let's retry after milliseconds.
      */
     static final int retryTimeoutMillis = 100;
-    /* private */ final String login;
 
     // Cache of myself object.
     private final String apiUrl;
@@ -76,7 +74,6 @@ abstract class GitHubClient {
             RateLimitHandler rateLimitHandler,
             AbuseLimitHandler abuseLimitHandler,
             GitHubRateLimitChecker rateLimitChecker,
-            Consumer<GHMyself> myselfConsumer,
             AuthorizationProvider authorizationProvider) throws IOException {
 
         if (apiUrl.endsWith("/")) {
@@ -95,37 +92,25 @@ abstract class GitHubClient {
         this.rateLimitHandler = rateLimitHandler;
         this.abuseLimitHandler = abuseLimitHandler;
         this.rateLimitChecker = rateLimitChecker;
-
-        this.login = getCurrentUser(myselfConsumer);
     }
 
-    private String getCurrentUser(Consumer<GHMyself> myselfConsumer) throws IOException {
-        String login = null;
-        if (this.authorizationProvider instanceof UserAuthorizationProvider
-                && this.authorizationProvider.getEncodedAuthorization() != null) {
-
-            UserAuthorizationProvider userAuthorizationProvider = (UserAuthorizationProvider) this.authorizationProvider;
+    String getLogin() {
+        try {
+            if (this.authorizationProvider instanceof UserAuthorizationProvider
+                    && this.authorizationProvider.getEncodedAuthorization() != null) {
 
-            login = userAuthorizationProvider.getLogin();
+                UserAuthorizationProvider userAuthorizationProvider = (UserAuthorizationProvider) this.authorizationProvider;
 
-            if (login == null) {
-                try {
-                    GHMyself myself = fetch(GHMyself.class, "/user");
-                    if (myselfConsumer != null) {
-                        myselfConsumer.accept(myself);
-                    }
-                    login = myself.getLogin();
-                } catch (IOException e) {
-                    return null;
-                }
+                return userAuthorizationProvider.getLogin();
             }
+        } catch (IOException e) {
         }
-        return login;
+        return null;
     }
 
     private <T> T fetch(Class<T> type, String urlPath) throws IOException {
         GitHubRequest request = GitHubRequest.newBuilder().withApiUrl(getApiUrl()).withUrlPath(urlPath).build();
-        return this.sendRequest(request, (responseInfo) -> GitHubResponse.parseBody(responseInfo, type)).body();
+        return sendRequest(request, (responseInfo) -> GitHubResponse.parseBody(responseInfo, type)).body();
     }
 
     /**
@@ -141,7 +126,7 @@ public boolean isCredentialValid() {
             return true;
         } catch (IOException e) {
             LOGGER.log(FINE,
-                    "Exception validating credentials on " + getApiUrl() + " with login '" + login + "' " + e,
+                    "Exception validating credentials on " + getApiUrl() + " with login '" + getLogin() + "' " + e,
                     e);
             return false;
         }
@@ -185,7 +170,7 @@ public void setConnector(HttpConnector connector) {
      */
     public boolean isAnonymous() {
         try {
-            return login == null && this.authorizationProvider.getEncodedAuthorization() == null;
+            return getLogin() == null && this.authorizationProvider.getEncodedAuthorization() == null;
         } catch (IOException e) {
             // An exception here means that the provider failed to provide authorization parameters,
             // basically meaning the same as "no auth"
@@ -319,7 +304,7 @@ private GHRateLimit updateRateLimit(@Nonnull GHRateLimit observed) {
      */
     public void checkApiUrlValidity() throws IOException {
         try {
-            fetch(GHApiInfo.class, "/").check(getApiUrl());
+            this.fetch(GHApiInfo.class, "/").check(getApiUrl());
         } catch (IOException e) {
             if (isPrivateModeEnabled()) {
                 throw (IOException) new IOException(
@@ -419,8 +404,8 @@ public <T> GitHubResponse<T> sendRequest(GitHubRequest request, @CheckForNull Gi
 
     private void logRequest(@Nonnull final GitHubRequest request) {
         LOGGER.log(FINE,
-                () -> "GitHub API request [" + (login == null ? "anonymous" : login) + "]: " + request.method() + " "
-                        + request.url().toString());
+                () -> "GitHub API request [" + (getLogin() == null ? "anonymous" : getLogin()) + "]: "
+                        + request.method() + " " + request.url().toString());
     }
 
     @Nonnull

src/main/java/org/kohsuke/github/GitHubHttpUrlConnectionClient.java

@@ -12,7 +12,6 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.function.Consumer;
 import java.util.logging.Logger;
 import java.util.zip.GZIPInputStream;
 
@@ -38,15 +37,8 @@ class GitHubHttpUrlConnectionClient extends GitHubClient {
             RateLimitHandler rateLimitHandler,
             AbuseLimitHandler abuseLimitHandler,
             GitHubRateLimitChecker rateLimitChecker,
-            Consumer<GHMyself> myselfConsumer,
             AuthorizationProvider authorizationProvider) throws IOException {
-        super(apiUrl,
-                connector,
-                rateLimitHandler,
-                abuseLimitHandler,
-                rateLimitChecker,
-                myselfConsumer,
-                authorizationProvider);
+        super(apiUrl, connector, rateLimitHandler, abuseLimitHandler, rateLimitChecker, authorizationProvider);
     }
 
     @Nonnull

src/test/java/org/kohsuke/github/AppTest.java

@@ -453,6 +453,7 @@ public void testRepoPermissions() throws Exception {
     public void testGetMyself() throws Exception {
         GHMyself me = gitHub.getMyself();
         assertThat(me, notNullValue());
+        assertThat(me.root(), sameInstance(gitHub));
         assertThat(gitHub.getUser("bitwiseman"), notNullValue());
         PagedIterable<GHRepository> ghRepositories = me.listRepositories();
         assertThat(ghRepositories, is(not(emptyIterable())));

src/test/java/org/kohsuke/github/GitHubConnectionTest.java

@@ -295,7 +295,7 @@ public void testGithubBuilderWithAppInstallationToken() throws Exception {
         GitHub github = builder.build();
         // change this to get a request
         assertThat(github.getClient().getEncodedAuthorization(), equalTo("token bogus app token"));
-        assertThat(github.getClient().login, is(emptyString()));
+        assertThat(github.getClient().getLogin(), is(emptyString()));
     }
 
     @Test

src/test/java/org/kohsuke/github/WireMockStatusReporterTest.java

@@ -25,7 +25,7 @@ public void user_whenProxying_AuthCorrectlyConfigured() throws Exception {
 
         verifyAuthenticated(gitHub);
 
-        assertThat(gitHub.getClient().login, not(equalTo(STUBBED_USER_LOGIN)));
+        assertThat(gitHub.getClient().getLogin(), not(equalTo(STUBBED_USER_LOGIN)));
 
         // If this user query fails, either the proxying config has broken (unlikely)
         // or your auth settings are not being retrieved from the environment.
@@ -47,7 +47,7 @@ public void user_whenNotProxying_Stubbed() throws Exception {
         assumeFalse("Test only valid when not proxying", mockGitHub.isUseProxy());
 
         verifyAuthenticated(gitHub);
-        assertThat(gitHub.getClient().login, equalTo(STUBBED_USER_LOGIN));
+        assertThat(gitHub.getClient().getLogin(), equalTo(STUBBED_USER_LOGIN));
 
         GHUser user = gitHub.getMyself();
         // NOTE: the stubbed user does not have to match the login provided from the github object