Merge pull request #625 from bitwiseman/bug/content-encode

Fixed GHContent to allow spaces in path

Author: bitwiseman

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

@@ -316,7 +316,7 @@ public GHContentUpdateResponse update(byte[] newContentBytes, String commitMessa
             requester.with("branch", branch);
         }
 
-        GHContentUpdateResponse response = requester.to(getApiRoute(), GHContentUpdateResponse.class);
+        GHContentUpdateResponse response = requester.to(getApiRoute(repository, path), GHContentUpdateResponse.class);
 
         response.getContent().wrap(repository);
         response.getCommit().wrapUp(repository);
@@ -359,14 +359,14 @@ public GHContentUpdateResponse delete(String commitMessage, String branch) throw
             requester.with("branch", branch);
         }
 
-        GHContentUpdateResponse response = requester.to(getApiRoute(), GHContentUpdateResponse.class);
+        GHContentUpdateResponse response = requester.to(getApiRoute(repository, path), GHContentUpdateResponse.class);
 
         response.getCommit().wrapUp(repository);
         return response;
     }
 
-    private String getApiRoute() {
-        return "/repos/" + repository.getOwnerName() + "/" + repository.getName() + "/contents/" + path;
+    static String getApiRoute(GHRepository repository, String path) {
+        return repository.getApiTailUrl("contents/" + path);
     }
 
     GHContent wrap(GHRepository owner) {

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

@@ -108,8 +108,7 @@ public GHContentBuilder message(String commitMessage) {
      *             the io exception
      */
     public GHContentUpdateResponse commit() throws IOException {
-        GHContentUpdateResponse response = req.to(repo.getApiTailUrl("contents/" + path),
-                GHContentUpdateResponse.class);
+        GHContentUpdateResponse response = req.to(GHContent.getApiRoute(repo, path), GHContentUpdateResponse.class);
 
         response.getContent().wrap(repo);
         response.getCommit().wrapUp(repo);

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

@@ -34,9 +34,7 @@
 import java.io.InputStreamReader;
 import java.io.InterruptedIOException;
 import java.io.Reader;
-import java.io.UnsupportedEncodingException;
 import java.net.URL;
-import java.net.URLEncoder;
 import java.util.AbstractSet;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -51,8 +49,6 @@
 import java.util.Set;
 import java.util.TreeMap;
 import java.util.WeakHashMap;
-import java.util.logging.Level;
-import java.util.logging.Logger;
 
 import static java.util.Arrays.*;
 import static org.kohsuke.github.Previews.*;
@@ -1460,14 +1456,7 @@ public PagedIterable<GHRef> listRefs(String refType) throws IOException {
      *             on failure communicating with GitHub, potentially due to an invalid ref type being requested
      */
     public GHRef getRef(String refName) throws IOException {
-        // hashes in branch names must be replaced with the url encoded equivalent or this call will fail
-        // FIXME: how about other URL unsafe characters, like space, @, : etc? do we need to be using
-        // URLEncoder.encode()?
-        // OTOH, '/' need no escaping
-        refName = refName.replaceAll("#", "%23");
-        return root.retrieve()
-                .to(String.format("/repos/%s/%s/git/refs/%s", getOwnerName(), name, refName), GHRef.class)
-                .wrap(root);
+        return root.retrieve().to(getApiTailUrl(String.format("git/refs/%s", refName)), GHRef.class).wrap(root);
     }
 
     /**
@@ -2031,25 +2020,6 @@ public Map<String, GHBranch> getBranches() throws IOException {
         return r;
     }
 
-    /**
-     * Replace special characters (e.g. #) with standard values (e.g. %23) so GitHub understands what is being
-     * requested.
-     * 
-     * @param value
-     *            string to be encoded.
-     * @return The encoded string.
-     */
-    private String UrlEncode(String value) {
-        try {
-            return URLEncoder.encode(value, org.apache.commons.codec.CharEncoding.UTF_8);
-        } catch (UnsupportedEncodingException ex) {
-            Logger.getLogger(GHRepository.class.getName()).log(Level.SEVERE, null, ex);
-        }
-
-        // Something went wrong - just return original value as is.
-        return value;
-    }
-
     /**
      * Gets branch.
      *
@@ -2060,7 +2030,7 @@ private String UrlEncode(String value) {
      *             the io exception
      */
     public GHBranch getBranch(String name) throws IOException {
-        return root.retrieve().to(getApiTailUrl("branches/" + UrlEncode(name)), GHBranch.class).wrap(this);
+        return root.retrieve().to(getApiTailUrl("branches/" + name), GHBranch.class).wrap(this);
     }
 
     /**
@@ -2576,7 +2546,7 @@ public boolean equals(Object obj) {
     String getApiTailUrl(String tail) {
         if (tail.length() > 0 && !tail.startsWith("/"))
             tail = '/' + tail;
-        return "/repos/" + getOwnerName() + "/" + name + tail;
+        return Requester.urlPathEncode("/repos/" + getOwnerName() + "/" + name + tail);
     }
 
     /**

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

@@ -40,6 +40,8 @@
 import java.net.MalformedURLException;
 import java.net.ProtocolException;
 import java.net.SocketTimeoutException;
+import java.net.URI;
+import java.net.URISyntaxException;
 import java.net.URL;
 import java.net.URLEncoder;
 import java.util.ArrayList;
@@ -942,6 +944,21 @@ static String transformEnum(Enum en) {
         return en.toString().toLowerCase(Locale.ENGLISH).replace('_', '-');
     }
 
+    /**
+     * Encode the path to url safe string.
+     *
+     * @param value
+     *            string to be path encoded.
+     * @return The encoded string.
+     */
+    public static String urlPathEncode(String value) {
+        try {
+            return new URI(null, null, value, null, null).toString();
+        } catch (URISyntaxException ex) {
+            throw new AssertionError(ex);
+        }
+    }
+
     private static final List<String> METHODS_WITHOUT_BODY = asList("GET", "DELETE");
     private static final Logger LOGGER = Logger.getLogger(Requester.class.getName());
 }

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

@@ -9,13 +9,18 @@
 import java.io.InputStreamReader;
 import java.util.List;
 
+import static org.hamcrest.CoreMatchers.*;
+
 /**
  * Integration test for {@link GHContent}.
  */
 public class GHContentIntegrationTest extends AbstractGitHubWireMockTest {
 
     private GHRepository repo;
-    private String createdFilename = "test-file-to-create.txt";
+
+    // file name with spaces and other chars
+    private final String createdDirectory = "test+directory #50";
+    private final String createdFilename = createdDirectory + "/test file-to+create-#1.txt";
 
     @Before
     @After
@@ -79,8 +84,23 @@ public void testCRUDContent() throws Exception {
         assertNotNull(created.getCommit());
         assertNotNull(created.getContent());
         assertNotNull(createdContent.getContent());
+        assertThat(createdContent.getPath(), equalTo(createdFilename));
         assertEquals("this is an awesome file I created\n", createdContent.getContent());
 
+        GHContent content = repo.getFileContent(createdFilename);
+        assertThat(content, is(notNullValue()));
+        assertThat(content.getSha(), equalTo(createdContent.getSha()));
+        assertThat(content.getContent(), equalTo(createdContent.getContent()));
+        assertThat(content.getPath(), equalTo(createdContent.getPath()));
+
+        List<GHContent> directoryContents = repo.getDirectoryContent(createdDirectory);
+        assertThat(directoryContents, is(notNullValue()));
+        assertThat(directoryContents.size(), equalTo(1));
+        content = directoryContents.get(0);
+        assertThat(content.getSha(), is(created.getContent().getSha()));
+        assertThat(content.getContent(), is(created.getContent().getContent()));
+        assertThat(content.getPath(), equalTo(createdFilename));
+
         GHContentUpdateResponse updatedContentResponse = createdContent.update("this is some new content\n",
                 "Updated file for integration tests.");
         GHContent updatedContent = updatedContentResponse.getContent();
@@ -96,5 +116,13 @@ public void testCRUDContent() throws Exception {
 
         assertNotNull(deleteResponse.getCommit());
         assertNull(deleteResponse.getContent());
+
+        try {
+            repo.getFileContent(createdFilename);
+            fail("Delete didn't work!");
+        } catch (GHFileNotFoundException e) {
+            assertThat(e.getMessage(),
+                    equalTo("{\"message\":\"Not Found\",\"documentation_url\":\"https://developer.github.com/v3/repos/contents/#get-contents\"}"));
+        }
     }
 }

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

@@ -65,7 +65,7 @@ public void getBranchNonExistentBut200Status() throws Exception {
             assertThat(e.getMessage(),
                     equalTo("Server returned HTTP response code: 200, message: 'OK' for URL: "
                             + mockGitHub.apiServer().baseUrl()
-                            + "/repos/github-api-test-org/github-api/branches/test%2FNonExistent"));
+                            + "/repos/github-api-test-org/github-api/branches/test/NonExistent"));
         }
     }
 

src/test/resources/org/kohsuke/github/GHContentIntegrationTest/wiremock/testCRUDContent/__files/repos_github-api-test-org_ghcontentintegrationtest-4f1ed47c-f261-4520-80d7-16b5952dd0e9.json renamed to src/test/resources/org/kohsuke/github/GHContentIntegrationTest/wiremock/testCRUDContent/__files/repos_github-api-test-org_ghcontentintegrationtest-e5080341-7377-4f3f-9427-388a105a91a7.json

@@ -65,14 +65,14 @@
   "releases_url": "https://api.github.com/repos/github-api-test-org/GHContentIntegrationTest/releases{/id}",
   "deployments_url": "https://api.github.com/repos/github-api-test-org/GHContentIntegrationTest/deployments",
   "created_at": "2015-08-15T14:14:57Z",
-  "updated_at": "2019-10-11T22:17:07Z",
-  "pushed_at": "2019-10-11T22:17:06Z",
+  "updated_at": "2019-11-26T01:09:15Z",
+  "pushed_at": "2019-11-26T01:09:14Z",
   "git_url": "git://github.com/github-api-test-org/GHContentIntegrationTest.git",
   "ssh_url": "[email protected]:github-api-test-org/GHContentIntegrationTest.git",
   "clone_url": "https://github.com/github-api-test-org/GHContentIntegrationTest.git",
   "svn_url": "https://github.com/github-api-test-org/GHContentIntegrationTest",
   "homepage": null,
-  "size": 38,
+  "size": 45,
   "stargazers_count": 1,
   "watchers_count": 1,
   "language": null,
@@ -81,13 +81,13 @@
   "has_downloads": true,
   "has_wiki": true,
   "has_pages": false,
-  "forks_count": 40,
+  "forks_count": 41,
   "mirror_url": null,
   "archived": false,
   "disabled": false,
   "open_issues_count": 0,
   "license": null,
-  "forks": 40,
+  "forks": 41,
   "open_issues": 0,
   "watchers": 1,
   "default_branch": "master",
@@ -296,17 +296,17 @@
     "has_downloads": true,
     "has_wiki": true,
     "has_pages": false,
-    "forks_count": 59,
+    "forks_count": 60,
     "mirror_url": null,
     "archived": false,
     "disabled": false,
     "open_issues_count": 0,
     "license": null,
-    "forks": 59,
+    "forks": 60,
     "open_issues": 0,
     "watchers": 0,
     "default_branch": "master"
   },
-  "network_count": 59,
+  "network_count": 60,
   "subscribers_count": 1
 }

src/test/resources/org/kohsuke/github/GHContentIntegrationTest/wiremock/testCRUDContent/__files/repos_github-api-test-org_ghcontentintegrationtest_contents_testdirectory-50-3cdae02b-77ed-4fb7-96fa-fee8aab5e88d.json

@@ -0,0 +1,18 @@
+[
+  {
+    "name": "test file-to+create-#1.txt",
+    "path": "test+directory #50/test file-to+create-#1.txt",
+    "sha": "8db9c31d79dfb9d0411c7af11b7ec7fabc72c5b1",
+    "size": 34,
+    "url": "https://api.github.com/repos/github-api-test-org/GHContentIntegrationTest/contents/test%2Bdirectory%20%2350/test%20file-to%2Bcreate-%231.txt?ref=master",
+    "html_url": "https://github.com/github-api-test-org/GHContentIntegrationTest/blob/master/test%2Bdirectory%20%2350/test%20file-to%2Bcreate-%231.txt",
+    "git_url": "https://api.github.com/repos/github-api-test-org/GHContentIntegrationTest/git/blobs/8db9c31d79dfb9d0411c7af11b7ec7fabc72c5b1",
+    "download_url": "https://raw.githubusercontent.com/github-api-test-org/GHContentIntegrationTest/master/test%2Bdirectory%20%2350/test%20file-to%2Bcreate-%231.txt",
+    "type": "file",
+    "_links": {
+      "self": "https://api.github.com/repos/github-api-test-org/GHContentIntegrationTest/contents/test%2Bdirectory%20%2350/test%20file-to%2Bcreate-%231.txt?ref=master",
+      "git": "https://api.github.com/repos/github-api-test-org/GHContentIntegrationTest/git/blobs/8db9c31d79dfb9d0411c7af11b7ec7fabc72c5b1",
+      "html": "https://github.com/github-api-test-org/GHContentIntegrationTest/blob/master/test%2Bdirectory%20%2350/test%20file-to%2Bcreate-%231.txt"
+    }
+  }
+]

src/test/resources/org/kohsuke/github/GHContentIntegrationTest/wiremock/testCRUDContent/__files/repos_github-api-test-org_ghcontentintegrationtest_contents_test-file-to-createtxt-30d0709c-ebc9-4631-8f44-bd942c44a711.json renamed to src/test/resources/org/kohsuke/github/GHContentIntegrationTest/wiremock/testCRUDContent/__files/repos_github-api-test-org_ghcontentintegrationtest_contents_testdirectory-50_test-file-tocreate-1txt-04f85e50-583c-48ad-8d45-55368c542148.json

@@ -1,45 +1,45 @@
 {
   "content": {
-    "name": "test-file-to-create.txt",
-    "path": "test-file-to-create.txt",
+    "name": "test file-to+create-#1.txt",
+    "path": "test+directory #50/test file-to+create-#1.txt",
     "sha": "da2d3cc78776aec68881668775c46a53f0ee2288",
     "size": 25,
-    "url": "https://api.github.com/repos/github-api-test-org/GHContentIntegrationTest/contents/test-file-to-create.txt?ref=master",
-    "html_url": "https://github.com/github-api-test-org/GHContentIntegrationTest/blob/master/test-file-to-create.txt",
+    "url": "https://api.github.com/repos/github-api-test-org/GHContentIntegrationTest/contents/test%2Bdirectory%20%2350/test%20file-to%2Bcreate-%231.txt?ref=master",
+    "html_url": "https://github.com/github-api-test-org/GHContentIntegrationTest/blob/master/test%2Bdirectory%20%2350/test%20file-to%2Bcreate-%231.txt",
     "git_url": "https://api.github.com/repos/github-api-test-org/GHContentIntegrationTest/git/blobs/da2d3cc78776aec68881668775c46a53f0ee2288",
-    "download_url": "https://raw.githubusercontent.com/github-api-test-org/GHContentIntegrationTest/master/test-file-to-create.txt",
+    "download_url": "https://raw.githubusercontent.com/github-api-test-org/GHContentIntegrationTest/master/test%2Bdirectory%20%2350/test%20file-to%2Bcreate-%231.txt",
     "type": "file",
     "_links": {
-      "self": "https://api.github.com/repos/github-api-test-org/GHContentIntegrationTest/contents/test-file-to-create.txt?ref=master",
+      "self": "https://api.github.com/repos/github-api-test-org/GHContentIntegrationTest/contents/test%2Bdirectory%20%2350/test%20file-to%2Bcreate-%231.txt?ref=master",
       "git": "https://api.github.com/repos/github-api-test-org/GHContentIntegrationTest/git/blobs/da2d3cc78776aec68881668775c46a53f0ee2288",
-      "html": "https://github.com/github-api-test-org/GHContentIntegrationTest/blob/master/test-file-to-create.txt"
+      "html": "https://github.com/github-api-test-org/GHContentIntegrationTest/blob/master/test%2Bdirectory%20%2350/test%20file-to%2Bcreate-%231.txt"
     }
   },
   "commit": {
-    "sha": "bc9cda986e02ae2677358a28f9cb6097865b3536",
-    "node_id": "MDY6Q29tbWl0NDA3NjM1Nzc6YmM5Y2RhOTg2ZTAyYWUyNjc3MzU4YTI4ZjljYjYwOTc4NjViMzUzNg==",
-    "url": "https://api.github.com/repos/github-api-test-org/GHContentIntegrationTest/git/commits/bc9cda986e02ae2677358a28f9cb6097865b3536",
-    "html_url": "https://github.com/github-api-test-org/GHContentIntegrationTest/commit/bc9cda986e02ae2677358a28f9cb6097865b3536",
+    "sha": "00c7107a9d00cfd9116747ce61c91f2fd9a7acbc",
+    "node_id": "MDY6Q29tbWl0NDA3NjM1Nzc6MDBjNzEwN2E5ZDAwY2ZkOTExNjc0N2NlNjFjOTFmMmZkOWE3YWNiYw==",
+    "url": "https://api.github.com/repos/github-api-test-org/GHContentIntegrationTest/git/commits/00c7107a9d00cfd9116747ce61c91f2fd9a7acbc",
+    "html_url": "https://github.com/github-api-test-org/GHContentIntegrationTest/commit/00c7107a9d00cfd9116747ce61c91f2fd9a7acbc",
     "author": {
       "name": "Liam Newman",
       "email": "[email protected]",
-      "date": "2019-10-11T22:17:43Z"
+      "date": "2019-11-26T01:09:46Z"
     },
     "committer": {
       "name": "Liam Newman",
       "email": "[email protected]",
-      "date": "2019-10-11T22:17:43Z"
+      "date": "2019-11-26T01:09:46Z"
     },
     "tree": {
-      "sha": "7aaee6a6cddcf48f94a40093de05be83c580ce10",
-      "url": "https://api.github.com/repos/github-api-test-org/GHContentIntegrationTest/git/trees/7aaee6a6cddcf48f94a40093de05be83c580ce10"
+      "sha": "50e945108f168faeb2eaf15855904e65e70b9336",
+      "url": "https://api.github.com/repos/github-api-test-org/GHContentIntegrationTest/git/trees/50e945108f168faeb2eaf15855904e65e70b9336"
     },
     "message": "Updated file for integration tests.",
     "parents": [
       {
-        "sha": "88bd1f52fc1e8242758364cee7dce2c82790d272",
-        "url": "https://api.github.com/repos/github-api-test-org/GHContentIntegrationTest/git/commits/88bd1f52fc1e8242758364cee7dce2c82790d272",
-        "html_url": "https://github.com/github-api-test-org/GHContentIntegrationTest/commit/88bd1f52fc1e8242758364cee7dce2c82790d272"
+        "sha": "ce407cde9c6e4c970e33a955197512a2c1922ecf",
+        "url": "https://api.github.com/repos/github-api-test-org/GHContentIntegrationTest/git/commits/ce407cde9c6e4c970e33a955197512a2c1922ecf",
+        "html_url": "https://github.com/github-api-test-org/GHContentIntegrationTest/commit/ce407cde9c6e4c970e33a955197512a2c1922ecf"
       }
     ],
     "verification": {

src/test/resources/org/kohsuke/github/GHContentIntegrationTest/wiremock/testCRUDContent/__files/repos_github-api-test-org_ghcontentintegrationtest_contents_test-file-to-createtxt-c2b7d86b-e154-4f46-8ab0-f253f77f5dda.json renamed to src/test/resources/org/kohsuke/github/GHContentIntegrationTest/wiremock/testCRUDContent/__files/repos_github-api-test-org_ghcontentintegrationtest_contents_testdirectory-50_test-file-tocreate-1txt-1d4297f8-515f-48b0-8f13-99894911d416.json

@@ -1,18 +1,18 @@
 {
-  "name": "test-file-to-create.txt",
-  "path": "test-file-to-create.txt",
+  "name": "test file-to+create-#1.txt",
+  "path": "test+directory #50/test file-to+create-#1.txt",
   "sha": "da2d3cc78776aec68881668775c46a53f0ee2288",
   "size": 25,
-  "url": "https://api.github.com/repos/github-api-test-org/GHContentIntegrationTest/contents/test-file-to-create.txt?ref=master",
-  "html_url": "https://github.com/github-api-test-org/GHContentIntegrationTest/blob/master/test-file-to-create.txt",
+  "url": "https://api.github.com/repos/github-api-test-org/GHContentIntegrationTest/contents/test%2Bdirectory%20%2350/test%20file-to%2Bcreate-%231.txt?ref=master",
+  "html_url": "https://github.com/github-api-test-org/GHContentIntegrationTest/blob/master/test%2Bdirectory%20%2350/test%20file-to%2Bcreate-%231.txt",
   "git_url": "https://api.github.com/repos/github-api-test-org/GHContentIntegrationTest/git/blobs/da2d3cc78776aec68881668775c46a53f0ee2288",
-  "download_url": "https://raw.githubusercontent.com/github-api-test-org/GHContentIntegrationTest/master/test-file-to-create.txt",
+  "download_url": "https://raw.githubusercontent.com/github-api-test-org/GHContentIntegrationTest/master/test%2Bdirectory%20%2350/test%20file-to%2Bcreate-%231.txt",
   "type": "file",
   "content": "dGhpcyBpcyBzb21lIG5ldyBjb250ZW50Cg==\n",
   "encoding": "base64",
   "_links": {
-    "self": "https://api.github.com/repos/github-api-test-org/GHContentIntegrationTest/contents/test-file-to-create.txt?ref=master",
+    "self": "https://api.github.com/repos/github-api-test-org/GHContentIntegrationTest/contents/test%2Bdirectory%20%2350/test%20file-to%2Bcreate-%231.txt?ref=master",
     "git": "https://api.github.com/repos/github-api-test-org/GHContentIntegrationTest/git/blobs/da2d3cc78776aec68881668775c46a53f0ee2288",
-    "html": "https://github.com/github-api-test-org/GHContentIntegrationTest/blob/master/test-file-to-create.txt"
+    "html": "https://github.com/github-api-test-org/GHContentIntegrationTest/blob/master/test%2Bdirectory%20%2350/test%20file-to%2Bcreate-%231.txt"
   }
 }