Fix decoding bug, improve docs, update deps (#32)

* Upgrade GitHub actions * Upgrade dependencies - Upgrade nubank/clj-github-app from 0.2.1 to 0.3.0 - Mark optional dependencies with scope provided - clj-commons/clj-yaml - http-kit.fake - dev.nubank/clj-github-mock * Add support for tokens managed via gh CLI Also, improve documentation about credentials. * Add failing test case * Fix decoding bug base64 decoding each line and then joining decoded lines is wrong, because the encoded base64 string may have a line break in the middle of a multibyte codepoint. The correct behavior is to strip all line breaks first, and only them decode the entire string. * Fix reflective access * Add support for binary files via get-content-raw! * Version 0.7.0
nubank · Dec 7, 2024 · 8f1dba8 · 8f1dba8
1 parent 4edb67a
commit 8f1dba8
Show file tree

Hide file tree

Showing 9 changed files with 171 additions and 65 deletions.
diff --git a/.github/workflows/clojure.yml b/.github/workflows/clojure.yml
@@ -12,15 +12,16 @@ jobs:
 
     strategy:
       matrix:
-        java-version: [8, 11]
+        java-version: [11, 17, 21]
 
     runs-on: ubuntu-latest
 
     steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v4
 
-      - uses: actions/setup-java@v1
+      - uses: actions/setup-java@v4
         with:
+          distribution: temurin
           java-version: ${{ matrix.java-version }}
 
       - name: Print java version

diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml
@@ -9,15 +9,16 @@ jobs:
   test-clojure:
     strategy:
       matrix:
-        java-version: [8, 11]
+        java-version: [11, 17, 21]
 
     runs-on: ubuntu-latest
 
     steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v4
 
-      - uses: actions/setup-java@v1
+      - uses: actions/setup-java@v4
         with:
+          distribution: temurin
           java-version: ${{ matrix.java-version }}
 
       - name: Print java version
@@ -34,7 +35,7 @@ jobs:
     runs-on: ubuntu-latest
     needs: [test-clojure]
     steps:
-    - uses: actions/checkout@v2
+    - uses: actions/checkout@v4.2.2
 
     - name: Install dependencies
       run: lein deps

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,17 @@
 # Changelog
 
+## 0.7.0
+
+- Fix decoding bug in `get-content!`
+- Add `get-content-raw` function that decodes file's content to a byte array
+- Support providing a token via `gh auth token`
+- Upgrade dependencies
+  - Upgrade nubank/clj-github-app from 0.2.1 to 0.3.0
+- Mark optional dependencies with scope provided
+  - clj-commons/clj-yaml
+  - http-kit.fake
+  - dev.nubank/clj-github-mock
+
 ## 0.6.5
 
 - Non-success status codes should not always result in a thrown exception

diff --git a/README.md b/README.md
@@ -23,28 +23,42 @@ that the client will automatically convert to a url with the github address.
 
 ### Credentials options
 
-When create a client you can use a number options to determine how it will obtain the app
+When creating a client you can use a number of options to determine how it will obtain the app
 credentials.
 
-When looking for credentials the client will by default first:
+#### `:app-id` + `:private-key`
 
- 1. Look for your personal token at `~/.config/hub` (this is the configuration file of `hub` tool).
- 2. Look for an environment variable named `GITHUB_TOKEN`.
+The client uses the provided app ID and private key to generate an [installation access token](https://docs.github.com/en/apps/creating-github-apps/authenticating-with-a-github-app/generating-an-installation-access-token-for-a-github-app)
+for a GitHub App.
 
-#### `:app-id` + `:private-key`
+The generated token is cached and will be automatically refreshed when needed.
 
-If you have the credentials stored in a different place, you can pass them directly to the client.
-The client will always use them and will not fallback to local configurations.
+`:private-key` must be a PEM encoded string.
 
 #### `:token`
 
-For quick test you can also pass a token directly to the client. The client will use it and not
-try to fetch one from the app.
+The client uses the provided hardcoded token string. Useful for experimentation, but not recommended for production 
+workloads.
 
 #### `:token-fn`
 
-If you have special needs, you can pass a function without parameters, the client will always
-call that function when it makes a request.
+You can provide an arbitrary zero-argument function that when invoked returns a valid token string.
+
+Some common token functions are available in `clj-github.token`, and `clj-github.token/chain` can
+be used to try multiple token functions in order.
+
+In the example below, the chain will look for:
+1. an environment variable named `GITHUB_TOKEN`.
+2. a token managed by `gh` CLI (by running `gh auth token`)
+3. a personal token at `~/.config/hub` (this is the configuration file of `hub` tool)
+
+```clojure
+(require '[clj-github.token :as token])
+
+(github-client/new-client {:token-fn (token/chain [token/env-var
+                                                   token/gh-cli
+                                                   token/hub-config])})
+```
 
 ### Managing repositories
 

diff --git a/project.clj b/project.clj
@@ -1,4 +1,4 @@
-(defproject dev.nubank/clj-github "0.6.5"
+(defproject dev.nubank/clj-github "0.7.0"
   :description "A Clojure library for interacting with the github developer API"
   :url "https://github.com/nubank/clj-github"
   :license {:name "EPL-2.0 OR GPL-2.0-or-later WITH Classpath-exception-2.0"
@@ -9,30 +9,30 @@
                              :password :env/clojars_passwd
                              :sign-releases false}]]
 
-  :plugins [[lein-cljfmt "0.9.2" :exclusions [org.clojure/clojure]]
-            [lein-nsorg "0.3.0" :exclusions [org.clojure/clojure]]
-            [lein-ancient "0.7.0" :exclusions [commons-logging com.fasterxml.jackson.core/jackson-databind com.fasterxml.jackson.core/jackson-core]]]
+  :plugins [[lein-cljfmt "0.9.2"]
+            [lein-nsorg "0.3.0"]
+            [lein-ancient "0.7.0"]]
 
-  :dependencies [[org.clojure/clojure "1.11.3"]
+  :dependencies [[org.clojure/clojure "1.12.0"]
                  [cheshire "5.13.0"]
-                 [clj-commons/clj-yaml "1.0.27"]
                  [http-kit "2.8.0"]
-                 [nubank/clj-github-app "0.2.1"]
-                 [nubank/state-flow "5.17.0"]
+                 [nubank/clj-github-app "0.3.0"]
                  [clj-commons/fs "1.6.311"]
-                 [ring/ring-codec "1.2.0"]]
+                 [ring/ring-codec "1.2.0"]
+                 ; Optional dependency used by clj-github.token/hub-config
+                 [clj-commons/clj-yaml "1.0.29" :scope "provided"]
+                 ; Dependencies required by clj-github.test-helpers and clj-github.state-flow-helper.
+                 ; Must be provided by the user (typically only used in tests)
+                 [http-kit.fake "0.2.2" :scope "provided"]
+                 [nubank/state-flow "5.18.0" :scope "provided"]
+                 [dev.nubank/clj-github-mock "0.4.0" :scope "provided"]]
 
   :cljfmt {:indents {flow       [[:block 1]]
                      assoc-some [[:block 0]]}}
 
   :profiles {:dev {:plugins [[lein-project-version "0.1.0"]]
-                   :dependencies [[ch.qos.logback/logback-classic "1.3.0" :exclusions [com.sun.mail/javax.mail]]
-                                  [org.clojure/test.check "1.1.1"]
-                                  [nubank/matcher-combinators "3.9.1" :exclusions [mvxcvi/puget commons-codec]]
-                                  [tortue/spy "2.14.0"]
-                                  [http-kit.fake "0.2.2"]
-                                  [metosin/reitit-core "0.7.0"]
-                                  [dev.nubank/clj-github-mock "0.2.0"]]}}
+                   :dependencies [[ch.qos.logback/logback-classic "1.5.12"]
+                                  [nubank/matcher-combinators "3.9.1"]]}}
 
   :aliases {"coverage" ["cloverage" "-s" "coverage"]
             "lint"     ["do" ["cljfmt" "check"] ["nsorg"]]

diff --git a/src/clj_github/changeset.clj b/src/clj_github/changeset.clj
@@ -54,6 +54,15 @@
       (or content
           (repository/get-content! client org repo path {:ref base-revision})))))
 
+(defn get-content-raw
+  "Returns the content of a file (as a byte array) for a given changeset."
+  ^bytes [{:keys [client org repo base-revision changes]} path]
+  (let [content (get changes path)]
+    (case content
+      ::deleted nil
+      (or content
+          (repository/get-content-raw! client org repo path {:ref base-revision})))))
+
 (defn put-content
   "Returns a new changeset with the file under path with new content.
   `content` can be a string or a byte-array.
@@ -85,8 +94,7 @@
   (#{::deleted} content))
 
 (defn- byte-array->base64
-  ([byte-array] (byte-array->base64 byte-array (Base64/getEncoder)))
-  ([byte-array encoder] (.encodeToString encoder byte-array)))
+  [byte-array] (.encodeToString (Base64/getEncoder) byte-array))
 
 (defn- content->sha-blob! [{:keys [client org repo]} content]
   (-> (repository/create-blob! client org repo {:content (byte-array->base64 content)

diff --git a/src/clj_github/repository.clj b/src/clj_github/repository.clj
@@ -10,17 +10,11 @@
   (:import [clojure.lang ExceptionInfo]
            [java.util Base64]))
 
-(defn- base64->string
-  ([base64] (base64->string base64 (Base64/getDecoder)))
-  ([base64 decoder] (String. (.decode decoder ^String base64) "UTF-8")))
-
-(defn- split-lines [content]
-  (string/split content #"\r?\n" -1)) ; make sure we don't lose \n at the end of the string
+(defn- base64-lines->bytes ^bytes [^String content]
+  (.decode (Base64/getDecoder) (.replace content "\n" "")))
 
 (defn- base64-lines->string [content]
-  (->> (split-lines content)
-       (map base64->string)
-       (string/join)))
+  (String. (base64-lines->bytes content) "UTF-8"))
 
 (defn get-contents!
   "Returns the list of contents of a repository default branch (usually `master`).
@@ -40,6 +34,22 @@
          nil
          (throw e))))))
 
+(defn- get-content*
+  "Returns the base64 encoded contents of a file"
+  [client org repo path ref branch]
+  (try
+    (-> (fetch-body! client (merge {:method :get
+                                    :path (format "/repos/%s/%s/contents/%s" org repo path)}
+                                   (cond
+                                     ref    {:query-params {"ref" ref}}
+                                     branch {:query-params {"branch" branch}}
+                                     :else  {})))
+        :content)
+    (catch ExceptionInfo e
+      (if (= 404 (-> (ex-data e) :response :status))
+        nil
+        (throw e)))))
+
 (defn get-content!
   "Returns the content of a text file from the repository default branch (usually `master`).
   An optional `:ref` parameter can be used to fetch content from a different commit/branch/tag.
@@ -50,19 +60,18 @@
   ([client org repo path]
    (get-content! client org repo path {}))
   ([client org repo path {:keys [ref branch]}]
-   (try
-     (-> (fetch-body! client (merge {:method :get
-                                     :path (format "/repos/%s/%s/contents/%s" org repo path)}
-                                    (cond
-                                      ref    {:query-params {"ref" ref}}
-                                      branch {:query-params {"branch" branch}}
-                                      :else  {})))
-         :content
-         base64-lines->string)
-     (catch ExceptionInfo e
-       (if (= 404 (-> (ex-data e) :response :status))
-         nil
-         (throw e))))))
+   (base64-lines->string (get-content* client org repo path ref branch))))
+
+(defn get-content-raw!
+  "Returns the bytes contents of a file from the repository default branch (usually `master`).
+  An optional `:ref` parameter can be used to fetch content from a different commit/branch/tag.
+  If the file does not exist, nil is returned.
+
+  Note: only works for blobs."
+  (^bytes [client org repo path]
+   (get-content-raw! client org repo path {}))
+  (^bytes [client org repo path {:keys [ref branch]}]
+   (base64-lines->bytes (get-content* client org repo path ref branch))))
 
 (defn get-repo!
   [client org repo]

diff --git a/src/clj_github/token.clj b/src/clj_github/token.clj
@@ -1,31 +1,59 @@
 (ns clj-github.token
   (:require [cheshire.core :as cheshire]
             [clj-github-app.token-manager :as token-manager]
-            [clj-yaml.core :as yaml]
             [clojure.java.io :as io]
-            [org.httpkit.client :as httpkit]))
+            [org.httpkit.client :as httpkit])
+  (:import (java.io File IOException)))
 
-(defn- file-exists-or-nil [file]
+(set! *warn-on-reflection* true)
+
+(defn- file-exists-or-nil [^File file]
   (when (.exists file)
     file))
 
+(defn- parse-yaml [s]
+  (let [parse-string (requiring-resolve 'clj-yaml.core/parse-string)]
+    (parse-string s)))
+
 (def hub-config
+  "Read token from `~/.config/hub` if the file exists.
+
+  This credentials file is managed by https://github.com/mislav/hub.
+
+  Users must provide their own dependency on `clj-commons/clj-yaml`."
   (memoize
    (fn []
      (some-> (io/file (System/getenv "HOME") ".config/hub")
              file-exists-or-nil
              (slurp)
-             yaml/parse-string
+             parse-yaml
              (get :github.com)
              first
              (get :oauth_token)))))
 
 (def env-var
+  "Get token from the GITHUB_TOKEN environment variable."
   (memoize
    (fn [] (System/getenv "GITHUB_TOKEN"))))
 
 (def github-url "https://api.github.com")
 
+(def gh-cli
+  "Get token by invoking `gh auth token` if command is available.
+
+  Requires https://github.com/cli/cli."
+  (memoize
+    (fn []
+      (try
+        (let [process (.start (ProcessBuilder. ["gh" "auth" "token"]))
+              output (with-open [stdout (.getInputStream process)]
+                       (String. (.readAllBytes stdout)))]
+          (when (zero? (.waitFor process))
+            (.trim ^String output)))
+        (catch IOException _
+          ; gh cli not available
+          nil)))))
+
 (def ^:private get-token-manager
   (memoize
    (fn [{:keys [github-app-id github-private-key]}]

diff --git a/test/clj_github/changeset_test.clj b/test/clj_github/changeset_test.clj
@@ -1,9 +1,10 @@
 (ns clj-github.changeset-test
-  (:require [clojure.test :refer :all]
-            [clj-github-mock.core :as mock.core]
+  (:require [clj-github-mock.core :as mock.core]
             [clj-github.changeset :as sut]
             [clj-github.httpkit-client :as client]
-            [org.httpkit.fake :as fake]))
+            [clojure.test :refer :all]
+            [org.httpkit.fake :as fake])
+  (:import (java.util Arrays)))
 
 (defmacro with-client [[client initial-state] & body]
   `(fake/with-fake-http
@@ -14,6 +15,15 @@
 (def initial-state {:orgs [{:name "nubank" :repos [{:name           "repo"
                                                     :default_branch "master"}]}]})
 
+(def string-content-with-special-chars
+  "This is a string with special characters: \uD83C\uDF89\uD83C\uDF89\uD83C\uDF89\uD83D\uDD25\uD83D\uDD25\uD83D\uDD25")
+
+(def binary-content
+  (byte-array 55 (unchecked-byte 255)))
+
+(def binary-content-2
+  (byte-array 7 (unchecked-byte 0)))
+
 (deftest get-content-test
   (testing "get content from client if there is no change"
     (with-client [client initial-state]
@@ -24,6 +34,29 @@
       (let [revision (sut/from-branch! client "nubank" "repo" "master")]
         (is (= "content"
                (sut/get-content revision "file"))))))
+  (testing "get string content with special characters"
+    (with-client [client initial-state]
+      (-> (sut/orphan client "nubank" "repo")
+          (sut/put-content "file" string-content-with-special-chars)
+          (sut/commit! "initial commit")
+          (sut/create-branch! "master"))
+      (let [revision (sut/from-branch! client "nubank" "repo" "master")]
+        (is (= string-content-with-special-chars
+               (sut/get-content revision "file"))))))
+  (testing "binary contents"
+    (with-client [client initial-state]
+      (-> (sut/orphan client "nubank" "repo")
+          (sut/put-content "file" binary-content)
+          (sut/commit! "initial commit")
+          (sut/create-branch! "master"))
+      (let [revision (sut/from-branch! client "nubank" "repo" "master")]
+        (testing "read remote contents"
+          (is (Arrays/equals ^bytes binary-content
+                             (sut/get-content-raw revision "file"))))
+        (testing "read local changes"
+          (is (Arrays/equals ^bytes binary-content-2
+                             (-> (sut/put-content revision "file" binary-content-2)
+                                 (sut/get-content-raw "file"))))))))
   (testing "get changed content"
     (with-client [client initial-state]
       (-> (sut/orphan client "nubank" "repo")