From 4f2d6aa591ab1cbb29f8e392adfd340269ac70ac Mon Sep 17 00:00:00 2001 From: Jim McBeath Date: Mon, 23 Oct 2017 14:30:16 -0700 Subject: [PATCH] Add caching to github-file-manager. (#1762) --- .../modules/api-manager/api-manager.ts | 24 +++++- .../github-file-manager.ts | 73 ++++++++++++++++++- .../test/github-file-manager-test.html | 28 +++++++ .../polymer/test/github-file-manager-test.ts | 39 ++++++++++ sources/web/datalab/polymer/test/index.html | 1 + 5 files changed, 161 insertions(+), 4 deletions(-) create mode 100644 sources/web/datalab/polymer/test/github-file-manager-test.html create mode 100644 sources/web/datalab/polymer/test/github-file-manager-test.ts diff --git a/sources/web/datalab/polymer/modules/api-manager/api-manager.ts b/sources/web/datalab/polymer/modules/api-manager/api-manager.ts index 27b086aa2..1627355e2 100644 --- a/sources/web/datalab/polymer/modules/api-manager/api-manager.ts +++ b/sources/web/datalab/polymer/modules/api-manager/api-manager.ts @@ -116,6 +116,15 @@ class ApiManager { } } + public static async sendRawRequestAsync(url: string, options?: XhrOptions, prependBasepath = true) + : Promise { + if (prependBasepath) { + const basepath = await this.getBasePath(); + url = basepath + url; + } + return this._xhrRequestAsync(url, options); + } + public static async sendRequestAsync(url: string, options?: XhrOptions, prependBasepath = true) : Promise { if (prependBasepath) { @@ -179,7 +188,8 @@ class ApiManager { * the response text. This method returns immediately with a promise * that resolves with the response text when the request completes. */ - protected static _xhrTextAsync(url: string, options?: XhrOptions): Promise { + protected static _xhrRequestAsync(url: string, options?: XhrOptions): + Promise { options = options || {}; const method = options.method || 'GET'; @@ -202,7 +212,7 @@ class ApiManager { this.isConnected = true; try { - resolve(request.responseText); + resolve(request); } catch (e) { reject(e); } @@ -242,6 +252,16 @@ class ApiManager { }); } + /** + * Sends an XMLHttpRequest to the specified URL, and returns the + * the response text. This method returns immediately with a promise + * that resolves with the response text when the request completes. + */ + protected static _xhrTextAsync(url: string, options?: XhrOptions): Promise { + return this._xhrRequestAsync(url, options) + .then((request) => request.responseText); + } + /** * Sends an XMLHttpRequest to the specified URL, and parses the * the response text as json. This method returns immediately with a promise diff --git a/sources/web/datalab/polymer/modules/github-file-manager/github-file-manager.ts b/sources/web/datalab/polymer/modules/github-file-manager/github-file-manager.ts index f3c9d8cd2..39864b072 100644 --- a/sources/web/datalab/polymer/modules/github-file-manager/github-file-manager.ts +++ b/sources/web/datalab/polymer/modules/github-file-manager/github-file-manager.ts @@ -54,12 +54,43 @@ interface GhFileResponse { url: string; // the url to access this file via the api } +interface GithubCacheEntry { + data?: object; // The payload + etag?: string; // The value of the etag header in the github response + promise?: Promise; // The fetch promise if we don't yet have the data +} + +/** + * A cache that holds github responses. + */ +class GithubCache { + + cache_: {[key: string]: GithubCacheEntry} = {}; + + // TODO(jimmc) - allow specifying some limits for the cache, + // such as time limit, count limit, or entry size limit + + // Returns the entry for the given path, or null if not in the cache. + public get(path: string): GithubCacheEntry | null { + const entry = this.cache_[path]; + return entry; + } + + // Puts the given data into the cache. If there is an existing entry + // at that path, updates that entry. + public put(path: string, entry: GithubCacheEntry) { + this.cache_[path] = entry; + } +} + /** * A file manager that wraps the Github API so that we can browse github * repositories. */ class GithubFileManager extends BaseFileManager { + private static cache_ = new GithubCache(); + public get(fileId: DatalabFileId): Promise { if (fileId.path === '' || fileId.path === '/') { return Promise.resolve(this._ghRootDatalabFile()); @@ -169,7 +200,36 @@ class GithubFileManager extends BaseFileManager { } as DatalabFile); } - private _githubApiPathRequest(githubPath: string): Promise { + // Gets the requested data, from our cache if we have it and it is + // up to date, else from the github API. + private _githubApiPathRequest(githubPath: string): Promise { + const entry = GithubFileManager.cache_.get(githubPath) || {} as GithubCacheEntry; + if (entry.promise) { + // There is already a fetch in progress for this data + return entry.promise; + } + const fetchPromise = this._sendApiPathRequest(githubPath, entry.etag) + .then((request) => { + entry.promise = undefined; + if (request.status === 304) { + // Item has not changed since our last request. + // This request did not count against the rate limit. + return entry.data; + } + const newEtag = request.getResponseHeader('etag'); + const newData = JSON.parse(request.responseText || 'null'); + if (newEtag) { + entry.etag = newEtag; + } + entry.data = newData; + return newData; + }); + entry.promise = fetchPromise; + GithubFileManager.cache_.put(githubPath, entry); + return fetchPromise; + } + + private _sendApiPathRequest(githubPath: string, etag?: string): Promise { const githubBaseUrl = 'https://api.github.com'; const restUrl = githubBaseUrl + githubPath; const options: XhrOptions = { @@ -183,7 +243,16 @@ class GithubFileManager extends BaseFileManager { 'X-Requested-With': 'XMLHttpRequest; googledatalab-datalab-app', }, }; - return ApiManager.sendRequestAsync(restUrl, options, false); + if (etag) { + // This item is in our cache, don't retrieve it if it hasn't changed. + // Hack: TS compiler thinks options.header 'is possibly undefined'; + // we know it is defined, this shuts up the compiler. + if (options.headers) { + options.headers['If-None-Match'] = etag; + options.successCodes = [200, 304]; + } + } + return ApiManager.sendRawRequestAsync(restUrl, options, false); } private _ghRootDatalabFile(): DatalabFile { diff --git a/sources/web/datalab/polymer/test/github-file-manager-test.html b/sources/web/datalab/polymer/test/github-file-manager-test.html new file mode 100644 index 000000000..3e7e71a26 --- /dev/null +++ b/sources/web/datalab/polymer/test/github-file-manager-test.html @@ -0,0 +1,28 @@ + + + + + + + + + + + + + + + + diff --git a/sources/web/datalab/polymer/test/github-file-manager-test.ts b/sources/web/datalab/polymer/test/github-file-manager-test.ts new file mode 100644 index 000000000..1e2aa0cf4 --- /dev/null +++ b/sources/web/datalab/polymer/test/github-file-manager-test.ts @@ -0,0 +1,39 @@ +/* + * Copyright 2017 Google Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ + +describe('GithubCache', () => { + + it('should return undefined for path not in the cache', () => { + const cache = new GithubCache(); + assert(cache.get('/no/such/path') === undefined, + 'unexpected entry for unknown path'); + }); + + it('should return the entry for a stored values', () => { + const cache = new GithubCache(); + const path = '/path/to/our/data'; + const entry = { + data: { foo: 'bar' }, + etag: '1234', + } as any as GithubCacheEntry; + + cache.put(path, entry); + + assert(cache.get(path) === entry, + 'unexpected entry for known path'); + assert(cache.get('/no/such/path') === undefined, + 'unexpected entry for unknown path'); + }); + +}); diff --git a/sources/web/datalab/polymer/test/index.html b/sources/web/datalab/polymer/test/index.html index 3471e8f1d..e46aaef13 100644 --- a/sources/web/datalab/polymer/test/index.html +++ b/sources/web/datalab/polymer/test/index.html @@ -28,6 +28,7 @@ WCT.loadSuites([ 'datalab-toolbar-test.html', 'file-browser-test.html', + 'github-file-manager-test.html', 'item-list-test.html', 'resizable-divider-test.html', 'settings-manager-test.html',