Populate Source and Parent if needed

NOTE: this also addresses a bug in push events where the repository url is incorrect
hub4j · May 20, 2020 · 66f22de · 66f22de
1 parent ba12efe
commit 66f22de
Show file tree

Hide file tree

Showing 19 changed files with 1,932 additions and 32 deletions.
diff --git a/src/main/java/org/kohsuke/github/GHRepository.java b/src/main/java/org/kohsuke/github/GHRepository.java
@@ -24,6 +24,7 @@
 package org.kohsuke.github;
 
 import com.fasterxml.jackson.annotation.JsonProperty;
+import com.fasterxml.jackson.core.JsonParseException;
 import com.infradna.tool.bridge_method_injector.WithBridgeMethods;
 import edu.umd.cs.findbugs.annotations.CheckForNull;
 import edu.umd.cs.findbugs.annotations.NonNull;
@@ -47,6 +48,7 @@
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
 import java.util.TreeMap;
 import java.util.WeakHashMap;
@@ -2477,6 +2479,9 @@ public List<GHDeployKey> getDeployKeys() throws IOException {
      * @see #getParent() #getParent()
      */
     public GHRepository getSource() throws IOException {
+        if (fork && source == null) {
+            populate();
+        }
         if (source == null)
             return null;
         if (source.root == null)
@@ -2495,6 +2500,10 @@ public GHRepository getSource() throws IOException {
      * @see #getSource() #getSource()
      */
     public GHRepository getParent() throws IOException {
+        if (fork && parent == null) {
+            populate();
+        }
+
         if (parent == null)
             return null;
         if (parent.root == null)
@@ -2828,6 +2837,20 @@ void populate() throws IOException {
         if (root.isOffline())
             return; // can't populate if the root is offline
 
-        root.createRequest().withApiUrl(root.getApiUrl() + full_name).fetchInto(this).wrap(root);
+        final URL url = Objects.requireNonNull(getUrl(), "Missing instance URL!");
+
+        try {
+            // IMPORTANT: the url for repository records is does not reliably point to the API url.
+            // There is bug in Push event payloads that returns the wrong url.
+            // All other occurrences of "url" take the form "https://api.github.com/...".
+            // For Push event repository records, they take the form "https://github.com/{fullName}".
+            root.createRequest().setRawUrlPath(url.toString()).fetchInto(this).wrap(root);
+        } catch (HttpException e) {
+            if (e.getCause() instanceof JsonParseException) {
+                root.createRequest().withUrlPath("/repos/" + full_name).fetchInto(this).wrap(root);
+            } else {
+                throw e;
+            }
+        }
     }
 }
diff --git a/src/main/java/org/kohsuke/github/GitHubResponse.java b/src/main/java/org/kohsuke/github/GitHubResponse.java
@@ -1,5 +1,6 @@
 package org.kohsuke.github;
 
+import com.fasterxml.jackson.core.JsonParseException;
 import com.fasterxml.jackson.databind.InjectableValues;
 import com.fasterxml.jackson.databind.JsonMappingException;
 import org.apache.commons.io.IOUtils;
@@ -16,6 +17,8 @@
 import java.util.List;
 import java.util.Map;
 import java.util.TreeMap;
+import java.util.logging.Level;
+import java.util.logging.Logger;
 
 import javax.annotation.CheckForNull;
 import javax.annotation.Nonnull;
@@ -31,6 +34,8 @@
  */
 class GitHubResponse<T> {
 
+    private static final Logger LOGGER = Logger.getLogger(GitHubResponse.class.getName());
+
     private final int statusCode;
 
     @Nonnull
@@ -83,9 +88,10 @@ static <T> T parseBody(ResponseInfo responseInfo, Class<T> type) throws IOExcept
             inject.addValue(ResponseInfo.class, responseInfo);
 
             return GitHubClient.getMappingObjectReader(responseInfo).forType(type).readValue(data);
-        } catch (JsonMappingException e) {
-            String message = "Failed to deserialize " + data;
-            throw new IOException(message, e);
+        } catch (JsonMappingException | JsonParseException e) {
+            String message = "Failed to deserialize: " + data;
+            LOGGER.log(Level.FINE, message);
+            throw e;
         }
     }
 
@@ -108,9 +114,10 @@ static <T> T parseBody(ResponseInfo responseInfo, T instance) throws IOException
         String data = responseInfo.getBodyAsString();
         try {
             return GitHubClient.getMappingObjectReader(responseInfo).withValueToUpdate(instance).readValue(data);
-        } catch (JsonMappingException e) {
-            String message = "Failed to deserialize " + data;
-            throw new IOException(message, e);
+        } catch (JsonMappingException | JsonParseException e) {
+            String message = "Failed to deserialize: " + data;
+            LOGGER.log(Level.FINE, message);
+            throw e;
         }
     }
 
@@ -307,6 +314,7 @@ public Map<String, List<String>> headers() {
          * @throws IOException
          *             if an I/O Exception occurs.
          */
+        @Nonnull
         String getBodyAsString() throws IOException {
             InputStreamReader r = null;
             try {

diff --git a/src/test/java/org/kohsuke/github/GHEventPayloadTest.java b/src/test/java/org/kohsuke/github/GHEventPayloadTest.java
@@ -7,15 +7,17 @@
 import java.util.Collections;
 import java.util.TimeZone;
 
-import static org.hamcrest.CoreMatchers.is;
-import static org.hamcrest.CoreMatchers.nullValue;
-import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.*;
 
-public class GHEventPayloadTest {
+public class GHEventPayloadTest extends AbstractGitHubWireMockTest {
 
     @Rule
     public final PayloadRule payload = new PayloadRule(".json");
 
+    public GHEventPayloadTest() {
+        useDefaultGitHub = false;
+    }
+
     @Test
     public void commit_comment() throws Exception {
         GHEventPayload.CommitComment event = GitHub.offline()
@@ -283,6 +285,65 @@ public void push() throws Exception {
         assertThat(event.getSender().getLogin(), is("baxterthehacker"));
     }
 
+    @Test
+    @Payload("push.fork")
+    public void pushToFork() throws Exception {
+        gitHub = getGitHubBuilder().withEndpoint(mockGitHub.apiServer().baseUrl()).build();
+
+        GHEventPayload.Push event = GitHub.offline().parseEventPayload(payload.asReader(), GHEventPayload.Push.class);
+        assertThat(event.getRef(), is("refs/heads/changes"));
+        assertThat(event.getBefore(), is("85c44b352958bf6d81b74ab8b21920f1d313a287"));
+        assertThat(event.getHead(), is("1393706f1364742defbc28ba459082630ca979af"));
+        assertThat(event.isCreated(), is(false));
+        assertThat(event.isDeleted(), is(false));
+        assertThat(event.isForced(), is(false));
+        assertThat(event.getCommits().size(), is(1));
+        assertThat(event.getCommits().get(0).getSha(), is("1393706f1364742defbc28ba459082630ca979af"));
+        assertThat(event.getCommits().get(0).getAuthor().getEmail(), is("bitwiseman@gmail.com"));
+        assertThat(event.getCommits().get(0).getCommitter().getEmail(), is("bitwiseman@gmail.com"));
+        assertThat(event.getCommits().get(0).getAdded().size(), is(6));
+        assertThat(event.getCommits().get(0).getRemoved().size(), is(0));
+        assertThat(event.getCommits().get(0).getModified().size(), is(2));
+        assertThat(event.getCommits().get(0).getModified().get(0),
+                is("src/main/java/org/kohsuke/github/GHLicense.java"));
+        assertThat(event.getRepository().getName(), is("github-api"));
+        assertThat(event.getRepository().getOwnerName(), is("hub4j-test-org"));
+        assertThat(event.getRepository().getUrl().toExternalForm(), is("https://github.com/hub4j-test-org/github-api"));
+        assertThat(event.getPusher().getName(), is("bitwiseman"));
+        assertThat(event.getPusher().getEmail(), is("bitwiseman@gmail.com"));
+        assertThat(event.getSender().getLogin(), is("bitwiseman"));
+
+        assertThat(event.getRepository().isFork(), is(true));
+
+        // in offliine mode, we should not populate missing fields
+        assertThat(event.getRepository().getSource(), is(nullValue()));
+        assertThat(event.getRepository().getParent(), is(nullValue()));
+
+        assertThat(event.getRepository().getUrl().toString(), is("https://github.com/hub4j-test-org/github-api"));
+        assertThat(event.getRepository().getHttpTransportUrl().toString(),
+                is("https://github.com/hub4j-test-org/github-api.git"));
+
+        // Test repository populate
+        event = gitHub.parseEventPayload(payload.asReader(mockGitHub::mapToMockGitHub), GHEventPayload.Push.class);
+        assertThat(event.getRepository().getUrl().toString(), is("https://github.com/hub4j-test-org/github-api"));
+        assertThat(event.getRepository().getHttpTransportUrl(), is("https://github.com/hub4j-test-org/github-api.git"));
+
+        event.getRepository().populate();
+
+        // After populate the url is fixed to point to the correct API endpoint
+        assertThat(event.getRepository().getUrl().toString(),
+                is(mockGitHub.apiServer().baseUrl() + "/repos/hub4j-test-org/github-api"));
+        assertThat(event.getRepository().getHttpTransportUrl(), is("https://github.com/hub4j-test-org/github-api.git"));
+
+        // Source
+        event = gitHub.parseEventPayload(payload.asReader(mockGitHub::mapToMockGitHub), GHEventPayload.Push.class);
+        assertThat(event.getRepository().getSource().getFullName(), is("hub4j/github-api"));
+
+        // Parent
+        event = gitHub.parseEventPayload(payload.asReader(mockGitHub::mapToMockGitHub), GHEventPayload.Push.class);
+        assertThat(event.getRepository().getParent().getFullName(), is("hub4j/github-api"));
+    }
+
     // TODO implement support classes and write test
     // @Test
     // public void release() throws Exception {}

diff --git a/src/test/java/org/kohsuke/github/GHRateLimitTest.java b/src/test/java/org/kohsuke/github/GHRateLimitTest.java
@@ -340,9 +340,8 @@ public void testGitHubRateLimitWithBadData() throws Exception {
             fail("Invalid rate limit missing some records should throw");
         } catch (Exception e) {
             assertThat(e, instanceOf(HttpException.class));
-            assertThat(e.getCause(), instanceOf(IOException.class));
-            assertThat(e.getCause().getCause(), instanceOf(ValueInstantiationException.class));
-            assertThat(e.getCause().getCause().getMessage(),
+            assertThat(e.getCause(), instanceOf(ValueInstantiationException.class));
+            assertThat(e.getCause().getMessage(),
                     containsString(
                             "Cannot construct instance of `org.kohsuke.github.GHRateLimit`, problem: `java.lang.NullPointerException`"));
         }
@@ -352,9 +351,8 @@ public void testGitHubRateLimitWithBadData() throws Exception {
             fail("Invalid rate limit record missing a value should throw");
         } catch (Exception e) {
             assertThat(e, instanceOf(HttpException.class));
-            assertThat(e.getCause(), instanceOf(IOException.class));
-            assertThat(e.getCause().getCause(), instanceOf(MismatchedInputException.class));
-            assertThat(e.getCause().getCause().getMessage(),
+            assertThat(e.getCause(), instanceOf(MismatchedInputException.class));
+            assertThat(e.getCause().getMessage(),
                     containsString("Missing required creator property 'reset' (index 2)"));
         }
 

diff --git a/src/test/java/org/kohsuke/github/PayloadRule.java b/src/test/java/org/kohsuke/github/PayloadRule.java
@@ -10,7 +10,11 @@
 import java.io.InputStream;
 import java.io.InputStreamReader;
 import java.io.Reader;
+import java.io.StringReader;
 import java.nio.charset.Charset;
+import java.util.function.Function;
+
+import javax.annotation.Nonnull;
 
 /**
  * @author Stephen Connolly
@@ -79,6 +83,11 @@ public Reader asReader() throws FileNotFoundException {
         return new InputStreamReader(asInputStream(), Charset.defaultCharset());
     }
 
+    public Reader asReader(@Nonnull Function<String, String> transformer) throws IOException {
+        String payloadString = asString();
+        return new StringReader(transformer.apply(payloadString));
+    }
+
     public Reader asReader(String encoding) throws IOException {
         return new InputStreamReader(asInputStream(), encoding);
     }

diff --git a/src/test/java/org/kohsuke/github/junit/GitHubWireMockRule.java b/src/test/java/org/kohsuke/github/junit/GitHubWireMockRule.java
@@ -1,14 +1,12 @@
 package org.kohsuke.github.junit;
 
 import com.github.tomakehurst.wiremock.WireMockServer;
-import com.github.tomakehurst.wiremock.client.WireMock;
 import com.github.tomakehurst.wiremock.common.FileSource;
 import com.github.tomakehurst.wiremock.core.WireMockConfiguration;
 import com.github.tomakehurst.wiremock.extension.Parameters;
 import com.github.tomakehurst.wiremock.extension.ResponseTransformer;
 import com.github.tomakehurst.wiremock.http.*;
 import com.github.tomakehurst.wiremock.matching.RequestPatternBuilder;
-import com.github.tomakehurst.wiremock.verification.*;
 import com.google.gson.*;
 
 import java.io.File;
@@ -20,6 +18,8 @@
 import java.util.HashMap;
 import java.util.Map;
 
+import javax.annotation.Nonnull;
+
 import static com.github.tomakehurst.wiremock.client.WireMock.*;
 import static com.github.tomakehurst.wiremock.common.Gzip.unGzipToString;
 
@@ -264,6 +264,24 @@ private Path renameFileToIndex(Path filePath, Map.Entry<String, String> idToInde
         return targetPath;
     }
 
+    @Nonnull
+    public String mapToMockGitHub(String body) {
+        body = body.replace("https://api.github.com", this.apiServer().baseUrl());
+
+        if (this.rawServer() != null) {
+            body = body.replace("https://raw.githubusercontent.com", this.rawServer().baseUrl());
+        } else {
+            body = body.replace("https://raw.githubusercontent.com", this.apiServer().baseUrl() + "/raw");
+        }
+
+        if (this.uploadsServer() != null) {
+            body = body.replace("https://uploads.github.com", this.uploadsServer().baseUrl());
+        } else {
+            body = body.replace("https://uploads.github.com", this.apiServer().baseUrl() + "/uploads");
+        }
+        return body;
+    }
+
     /**
      * A number of modifications are needed as runtime to make responses target the WireMock server and not accidentally
      * switch to using the live github servers.
@@ -286,19 +304,7 @@ public Response transform(Request request, Response response, FileSource files,
 
                 String body;
                 body = getBodyAsString(response, headers);
-                body = body.replace("https://api.github.com", rule.apiServer().baseUrl());
-
-                if (rule.rawServer() != null) {
-                    body = body.replace("https://raw.githubusercontent.com", rule.rawServer().baseUrl());
-                } else {
-                    body = body.replace("https://raw.githubusercontent.com", rule.apiServer().baseUrl() + "/raw");
-                }
-
-                if (rule.uploadsServer() != null) {
-                    body = body.replace("https://uploads.github.com", rule.uploadsServer().baseUrl());
-                } else {
-                    body = body.replace("https://uploads.github.com", rule.apiServer().baseUrl() + "/uploads");
-                }
+                body = rule.mapToMockGitHub(body);
 
                 builder.body(body);