Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Populate Source and Parent if needed #821

Merged
merged 2 commits into from
May 21, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 35 additions & 7 deletions src/main/java/org/kohsuke/github/GHRepository.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -2158,6 +2160,12 @@ GHRepository wrap(GitHub root) {
if (root.isOffline() && owner != null) {
owner.wrapUp(root);
}
if (source != null) {
source.wrap(root);
}
if (parent != null) {
parent.wrap(root);
}
return this;
}

Expand Down Expand Up @@ -2477,10 +2485,13 @@ public List<GHDeployKey> getDeployKeys() throws IOException {
* @see #getParent() #getParent()
*/
public GHRepository getSource() throws IOException {
if (source == null)
if (fork && source == null) {
populate();
}
if (source == null) {
return null;
if (source.root == null)
source = root.getRepository(source.getFullName());
}

return source;
}

Expand All @@ -2495,10 +2506,13 @@ public GHRepository getSource() throws IOException {
* @see #getSource() #getSource()
*/
public GHRepository getParent() throws IOException {
if (parent == null)
if (fork && parent == null) {
populate();
}

if (parent == null) {
return null;
if (parent.root == null)
parent = root.getRepository(parent.getFullName());
}
return parent;
}

Expand Down Expand Up @@ -2828,6 +2842,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;
}
}
}
}
20 changes: 14 additions & 6 deletions src/main/java/org/kohsuke/github/GitHubResponse.java
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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;
Expand All @@ -31,6 +34,8 @@
*/
class GitHubResponse<T> {

private static final Logger LOGGER = Logger.getLogger(GitHubResponse.class.getName());

private final int statusCode;

@Nonnull
Expand Down Expand Up @@ -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;
}
}

Expand All @@ -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;
}
}

Expand Down Expand Up @@ -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 {
Expand Down
74 changes: 70 additions & 4 deletions src/test/java/org/kohsuke/github/GHEventPayloadTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -283,6 +285,70 @@ 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"));

// ensure that root has been bound after populate
event.getRepository().getSource().getRef("heads/master");
event.getRepository().getParent().getRef("heads/master");

// 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 {}
Expand Down
10 changes: 4 additions & 6 deletions src/test/java/org/kohsuke/github/GHRateLimitTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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`"));
}
Expand All @@ -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)"));
}

Expand Down
9 changes: 9 additions & 0 deletions src/test/java/org/kohsuke/github/PayloadRule.java
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
}
Expand Down
36 changes: 21 additions & 15 deletions src/test/java/org/kohsuke/github/junit/GitHubWireMockRule.java
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -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.
Expand All @@ -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);

Expand Down
Loading