spotify · dziemba · Apr 15, 2022 · Apr 13, 2022
diff --git a/src/main/java/com/spotify/github/v3/repos/Branch.java b/src/main/java/com/spotify/github/v3/repos/Branch.java
@@ -34,7 +34,7 @@
 @Value.Immutable
 @GithubStyle
 @JsonSerialize(as = ImmutableBranch.class)
-@JsonDeserialize(as = ImmutableBranch.class, using = BranchDeserializer.class)
+@JsonDeserialize(as = ImmutableBranch.class)
 public interface Branch {

  /** Branch name */
@@ -50,6 +50,7 @@ public interface Branch {
  Optional<Boolean> isProtected();

  /** Branch protection API URL */
+  @JsonDeserialize(using = BranchProtectionUrlDeserializer.class)
  Optional<URI> protectionUrl();
 }

diff --git a/...y/github/v3/repos/BranchDeserializer.java → ...epos/BranchProtectionUrlDeserializer.java b/...y/github/v3/repos/BranchDeserializer.java → ...epos/BranchProtectionUrlDeserializer.java
@@ -2,7 +2,7 @@
 * -\-\-
 * github-api
 * --
- * Copyright (C) 2016 - 2020 Spotify AB
+ * Copyright (C) 2016 - 2021 Spotify AB
 * --
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
@@ -21,19 +21,19 @@
 package com.spotify.github.v3.repos;

 import com.fasterxml.jackson.core.JsonParser;
+import com.fasterxml.jackson.core.type.TypeReference;
 import com.fasterxml.jackson.databind.DeserializationContext;
 import com.fasterxml.jackson.databind.JsonDeserializer;
-import com.fasterxml.jackson.databind.JsonNode;
-import com.spotify.github.v3.git.ImmutableShaLink;
 import java.io.IOException;
 import java.net.URI;
 import java.net.URISyntaxException;
 import java.net.URLEncoder;
 import java.nio.charset.StandardCharsets;
+import java.util.Optional;

-public class BranchDeserializer extends JsonDeserializer<ImmutableBranch> {
+public class BranchProtectionUrlDeserializer extends JsonDeserializer<Optional<URI>> {

-  private URI fixInvalidGithubUrl(final String invalidUrl) throws IOException {
+  private URI fixInvalidGithubUrl(final String invalidUrl) {
    // There's a bug in github where it gives you back non-url-encoded characters
    // in the protection_url field. For example if your branch has a single "%" in its name.
    // As of this writing, the protection URL looks something like this
@@ -48,29 +48,20 @@ private URI fixInvalidGithubUrl(final String invalidUrl) throws IOException {
  }

  @Override
-  public ImmutableBranch deserialize(final JsonParser jsonParser,
-      final DeserializationContext deserializationContext)
+  public Optional<URI> deserialize(
+      final JsonParser jsonParser, final DeserializationContext deserializationContext)
      throws IOException {
-    JsonNode node = jsonParser.getCodec().readTree(jsonParser);
-    URI protectionUrl;
-    var immutableBranchBuilder = ImmutableBranch.builder();
-    if (node.has("protected")) {
-      immutableBranchBuilder.isProtected(node.get("protected").asBoolean());
-      String protectionUrlString = node.get("protection_url").asText();
-      try {
-        protectionUrl = new URI(protectionUrlString);
-      } catch (URISyntaxException e) {
-        protectionUrl = fixInvalidGithubUrl(protectionUrlString);
-      }
-      immutableBranchBuilder.protectionUrl(protectionUrl);
-    }
-    ImmutableShaLink shaLink = ImmutableShaLink.builder()
-        .sha(node.get("commit").get("sha").asText())
-        .url(URI.create(node.at("/commit/url").asText()))
-        .build();
-    return immutableBranchBuilder
-        .name(node.get("name").textValue())
-        .commit(shaLink)
-        .build();
+
+    TypeReference<Optional<String>> ref = new TypeReference<>() {};
+    Optional<String> protectionUrlStringOpt = jsonParser.readValueAs(ref);
+
+    return protectionUrlStringOpt.map(
+        protectionUrlString -> {
+          try {
+            return new URI(protectionUrlString);
+          } catch (URISyntaxException e) {
+            return fixInvalidGithubUrl(protectionUrlString);
+          }
+        });
  }
 }
diff --git a/src/test/java/com/spotify/github/v3/clients/RepositoryClientTest.java b/src/test/java/com/spotify/github/v3/clients/RepositoryClientTest.java
@@ -251,21 +251,34 @@ public void getBranch() throws Exception {
        .thenReturn(fixture);
    final Branch branch = repoClient.getBranch("somebranch").get();
    assertThat(branch.isProtected().orElse(false), is(true));
+    assertThat(branch.protectionUrl().get().toString(), is("https://api.github.com/repos/octocat/Hello-World/branches/master/protection"));
    assertThat(branch.commit().sha(), is("6dcb09b5b57875f334f61aebed695e2e4193db5e"));
    assertThat(
        branch.commit().url().toString(),
        is("https://api.github.com/repos/octocat/Hello-World/commits/c5b97d5ae6c19d5c5df71a34c7fbeeda2479ccbc"));
  }

  @Test
-  public void getBranchWithoutProtection() throws Exception {
-    // Make sure the custom deserialiser correctly handles the optional protected fields
+  public void getBranchWithNoProtection() throws Exception {
    final CompletableFuture<Branch> fixture =
        completedFuture(json.fromJson(getFixture("branch-not-protected.json"), Branch.class));
    when(github.request("/repos/someowner/somerepo/branches/somebranch", Branch.class))
        .thenReturn(fixture);
    final Branch branch = repoClient.getBranch("somebranch").get();
    assertThat(branch.isProtected().orElse(false), is(false));
+    assertTrue(branch.protectionUrl().isEmpty());
+    assertThat(branch.commit().sha(), is("6dcb09b5b57875f334f61aebed695e2e4193db5e"));
+  }
+
+  @Test
+  public void getBranchWithoutProtectionFields() throws Exception {
+    final CompletableFuture<Branch> fixture =
+        completedFuture(json.fromJson(getFixture("branch-no-protection-fields.json"), Branch.class));
+    when(github.request("/repos/someowner/somerepo/branches/somebranch", Branch.class))
+        .thenReturn(fixture);
+    final Branch branch = repoClient.getBranch("somebranch").get();
+    assertThat(branch.isProtected().orElse(false), is(false));
+    assertTrue(branch.protectionUrl().isEmpty());
    assertThat(branch.commit().sha(), is("6dcb09b5b57875f334f61aebed695e2e4193db5e"));
    assertThat(
        branch.commit().url().toString(),

diff --git a/src/test/resources/com/spotify/github/v3/repos/branch-no-protection-fields.json b/src/test/resources/com/spotify/github/v3/repos/branch-no-protection-fields.json
@@ -0,0 +1,7 @@
+{
+  "name": "master",
+  "commit": {
+    "sha": "6dcb09b5b57875f334f61aebed695e2e4193db5e",
+    "url": "https://api.github.com/repos/octocat/Hello-World/commits/c5b97d5ae6c19d5c5df71a34c7fbeeda2479ccbc"
+  }
+}
diff --git a/src/test/resources/com/spotify/github/v3/repos/branch-not-protected.json b/src/test/resources/com/spotify/github/v3/repos/branch-not-protected.json
@@ -3,5 +3,6 @@
  "commit": {
    "sha": "6dcb09b5b57875f334f61aebed695e2e4193db5e",
    "url": "https://api.github.com/repos/octocat/Hello-World/commits/c5b97d5ae6c19d5c5df71a34c7fbeeda2479ccbc"
-  }
+  },
+  "protected": false
 }