Skip to content

Commit 1ae590c

Browse files
authored
Cache GitHub App Installations (#334)
This caches GitHub App Installations after their initial creation, so future invocations return the exact same installation struct without making upstream API calls. To minimize lock contention, values written into the map are closures which are resolved on invocation. This allows parallel goroutines to resolve installations while still maintaining lock integrity.
1 parent eab2704 commit 1ae590c

File tree

2 files changed

+56
-26
lines changed

2 files changed

+56
-26
lines changed

githubauth/app.go

+53-25
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import (
2828
"io"
2929
"net/http"
3030
"strings"
31+
"sync"
3132
"time"
3233

3334
"golang.org/x/oauth2"
@@ -42,6 +43,9 @@ type App struct {
4243
appID string
4344
privateKey *rsa.PrivateKey
4445

46+
installationCache map[string](func() (*AppInstallation, error))
47+
installationCacheLock sync.Mutex
48+
4549
baseURL string
4650
httpClient *http.Client
4751
}
@@ -99,8 +103,9 @@ func NewApp[T *rsa.PrivateKey | string | []byte](appID string, privateKeyT T, op
99103
}
100104

101105
app := &App{
102-
appID: appID,
103-
privateKey: privateKey,
106+
appID: appID,
107+
privateKey: privateKey,
108+
installationCache: make(map[string](func() (*AppInstallation, error)), 8),
104109

105110
baseURL: defaultBaseURL,
106111
httpClient: &http.Client{
@@ -200,58 +205,81 @@ func (i *AppInstallation) App() *App {
200205

201206
// InstallationForID returns an AccessTokensURLFunc that gets the access token
202207
// url for the given installation.
208+
//
209+
// The initial invocation will make an API call to GitHub to get the access
210+
// token URL for the installation; future calls will return the cached
211+
// installation.
203212
func (a *App) InstallationForID(ctx context.Context, installationID string) (*AppInstallation, error) {
204-
u, err := a.accessTokenURL(ctx, fmt.Sprintf("%s/app/installations/%s", a.baseURL, installationID))
213+
i, err := a.withInstallationCaching(ctx, "i:"+installationID, "/app/installations/"+installationID)()
205214
if err != nil {
206215
return nil, fmt.Errorf("failed to get access token url for installation %s: %w", installationID, err)
207216
}
208-
209-
return &AppInstallation{
210-
app: a,
211-
accessTokenURL: u,
212-
}, nil
217+
return i, nil
213218
}
214219

215220
// InstallationForOrg returns an AccessTokensURLFunc that gets the access token url for the
216221
// given org context.
222+
//
223+
// The initial invocation will make an API call to GitHub to get the access
224+
// token URL for the installation; future calls will return the cached
225+
// installation.
217226
func (a *App) InstallationForOrg(ctx context.Context, org string) (*AppInstallation, error) {
218-
u, err := a.accessTokenURL(ctx, fmt.Sprintf("%s/orgs/%s/installation", a.baseURL, org))
227+
i, err := a.withInstallationCaching(ctx, "org:"+org, "/orgs/"+org+"/installation")()
219228
if err != nil {
220229
return nil, fmt.Errorf("failed to get access token url for org %s: %w", org, err)
221230
}
222-
223-
return &AppInstallation{
224-
app: a,
225-
accessTokenURL: u,
226-
}, nil
231+
return i, nil
227232
}
228233

229234
// InstallationForRepo returns an AccessTokensURLFunc that gets the access token url for the
230235
// given repo context.
236+
//
237+
// The initial invocation will make an API call to GitHub to get the access
238+
// token URL for the installation; future calls will return the cached
239+
// installation.
231240
func (a *App) InstallationForRepo(ctx context.Context, org, repo string) (*AppInstallation, error) {
232-
u, err := a.accessTokenURL(ctx, fmt.Sprintf("%s/repos/%s/%s/installation", a.baseURL, org, repo))
241+
i, err := a.withInstallationCaching(ctx, "repo:"+org+"/"+repo, "/repos/"+org+"/"+repo+"/installation")()
233242
if err != nil {
234243
return nil, fmt.Errorf("failed to get access token url for repo %s/%s: %w", org, repo, err)
235244
}
236-
237-
return &AppInstallation{
238-
app: a,
239-
accessTokenURL: u,
240-
}, nil
245+
return i, nil
241246
}
242247

243248
// InstallationForUser returns an AccessTokensURLFunc that gets the access token url for the
244249
// given user context.
250+
//
251+
// The initial invocation will make an API call to GitHub to get the access
252+
// token URL for the installation; future calls will return the cached
253+
// installation.
245254
func (a *App) InstallationForUser(ctx context.Context, user string) (*AppInstallation, error) {
246-
u, err := a.accessTokenURL(ctx, fmt.Sprintf("%s/users/%s/installation", a.baseURL, user))
255+
i, err := a.withInstallationCaching(ctx, "user:"+user, "/users/"+user+"/installation")()
247256
if err != nil {
248257
return nil, fmt.Errorf("failed to get access token url for user %s: %w", user, err)
249258
}
259+
return i, nil
260+
}
250261

251-
return &AppInstallation{
252-
app: a,
253-
accessTokenURL: u,
254-
}, nil
262+
// withInstallationCaching returns a closure that caches the app installation by
263+
// key.
264+
func (a *App) withInstallationCaching(ctx context.Context, cacheKey, tokenPath string) func() (*AppInstallation, error) {
265+
a.installationCacheLock.Lock()
266+
defer a.installationCacheLock.Unlock()
267+
268+
entry, ok := a.installationCache[cacheKey]
269+
if !ok {
270+
entry = sync.OnceValues(func() (*AppInstallation, error) {
271+
u, err := a.accessTokenURL(ctx, a.baseURL+tokenPath)
272+
if err != nil {
273+
return nil, err
274+
}
275+
276+
return &AppInstallation{
277+
app: a,
278+
accessTokenURL: u,
279+
}, nil
280+
})
281+
}
282+
return entry
255283
}
256284

257285
// AccessToken calls the GitHub API to generate a new access token for this

githubauth/app_test.go

+3-1
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,9 @@ func TestNew(t *testing.T) {
155155

156156
opts := []cmp.Option{
157157
cmp.AllowUnexported(App{}),
158-
cmpopts.IgnoreFields(App{}),
158+
cmpopts.IgnoreFields(App{},
159+
"installationCache",
160+
"installationCacheLock"),
159161
}
160162
if diff := cmp.Diff(tc.want, got, opts...); diff != "" {
161163
t.Errorf("mismatch (-want, +got):\n%s", diff)

0 commit comments

Comments
 (0)