From ef600b3c8cfb84cb02fdfed6bb767b42257f96da Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Sun, 5 Dec 2021 16:50:09 -0600 Subject: [PATCH] Review feedback. Signed-off-by: Eric Peterson --- packages/techdocs-common/src/stages/publish/local.ts | 10 ++++++---- plugins/techdocs-backend/package.json | 1 + plugins/techdocs-backend/src/cache/TechDocsCache.ts | 4 ++-- .../techdocs-backend/src/cache/cacheMiddleware.test.ts | 10 +++++----- plugins/techdocs-backend/src/cache/cacheMiddleware.ts | 6 +++--- 5 files changed, 17 insertions(+), 14 deletions(-) diff --git a/packages/techdocs-common/src/stages/publish/local.ts b/packages/techdocs-common/src/stages/publish/local.ts index fc64776e4c..9c146d2935 100644 --- a/packages/techdocs-common/src/stages/publish/local.ts +++ b/packages/techdocs-common/src/stages/publish/local.ts @@ -127,15 +127,17 @@ export class LocalPublish implements PublisherBase { // Generate publish response. const techdocsApiUrl = await this.discovery.getBaseUrl('techdocs'); - const objects = (await getFileTreeRecursively(publishDir)).map(abs => { - return abs.split(`${staticDocsDir}/`)[1]; - }); + const publishedFilePaths = (await getFileTreeRecursively(publishDir)).map( + abs => { + return abs.split(`${staticDocsDir}/`)[1]; + }, + ); return { remoteUrl: `${techdocsApiUrl}/static/docs/${encodeURIComponent( entity.metadata.name, )}`, - objects, + objects: publishedFilePaths, }; } diff --git a/plugins/techdocs-backend/package.json b/plugins/techdocs-backend/package.json index ffad4bc4c1..6f3495737a 100644 --- a/plugins/techdocs-backend/package.json +++ b/plugins/techdocs-backend/package.json @@ -40,6 +40,7 @@ "@backstage/search-common": "^0.2.1", "@backstage/techdocs-common": "^0.10.8", "@types/express": "^4.17.6", + "cross-fetch": "^3.0.6", "dockerode": "^3.3.1", "express": "^4.17.1", "express-promise-router": "^4.1.0", diff --git a/plugins/techdocs-backend/src/cache/TechDocsCache.ts b/plugins/techdocs-backend/src/cache/TechDocsCache.ts index 147aed0a18..807d6ca87b 100644 --- a/plugins/techdocs-backend/src/cache/TechDocsCache.ts +++ b/plugins/techdocs-backend/src/cache/TechDocsCache.ts @@ -43,8 +43,8 @@ export class TechDocsCache { config: Config, { cache, logger }: { cache: CacheClient; logger: Logger }, ) { - const readTimeout = - config.getOptionalNumber('techdocs.cache.readTimeout') || 1000; + const timeout = config.getOptionalNumber('techdocs.cache.readTimeout'); + const readTimeout = timeout === undefined ? 1000 : timeout; return new TechDocsCache({ cache, logger, readTimeout }); } diff --git a/plugins/techdocs-backend/src/cache/cacheMiddleware.test.ts b/plugins/techdocs-backend/src/cache/cacheMiddleware.test.ts index 91e4b84222..312674a5bf 100644 --- a/plugins/techdocs-backend/src/cache/cacheMiddleware.test.ts +++ b/plugins/techdocs-backend/src/cache/cacheMiddleware.test.ts @@ -59,7 +59,7 @@ describe('createCacheMiddleware', () => { app = express().use(router); app.use((req, res, next) => { // By default, send cacheable content. - if (req.path !== '/api/static/docs/error.png') { + if (req.path !== '/static/docs/error.png') { res.send('default-response'); } else { next(new Error()); @@ -70,7 +70,7 @@ describe('createCacheMiddleware', () => { describe('middleware', () => { it('does not apply to non-static/docs paths', async () => { await request(app) - .get('/api/static/not-docs') + .get('/static/not-docs') .expect(200, 'default-response'); expect(cache.set).not.toHaveBeenCalled(); @@ -79,7 +79,7 @@ describe('createCacheMiddleware', () => { it('responds with cached response', async () => { cache.get.mockResolvedValueOnce(getMockHttpResponseFor('xyz')); - await request(app).get('/api/static/docs/foo.html').expect(200, 'xyz'); + await request(app).get('/static/docs/foo.html').expect(200, 'xyz'); await waitForSocketClose(); expect(cache.set).not.toHaveBeenCalled(); @@ -88,7 +88,7 @@ describe('createCacheMiddleware', () => { it('sets cache when content is cacheable', async () => { const expectedPath = 'default/api/xyz/index.html'; await request(app) - .get(`/api/static/docs/${expectedPath}`) + .get(`/static/docs/${expectedPath}`) .expect(200, 'default-response'); await waitForSocketClose(); @@ -100,7 +100,7 @@ describe('createCacheMiddleware', () => { }); it('does not set cache on error', async () => { - await request(app).get('/api/static/docs/error.png').expect(500); + await request(app).get('/static/docs/error.png').expect(500); await waitForSocketClose(); expect(cache.set).not.toHaveBeenCalled(); diff --git a/plugins/techdocs-backend/src/cache/cacheMiddleware.ts b/plugins/techdocs-backend/src/cache/cacheMiddleware.ts index 9e545c235c..cb59681304 100644 --- a/plugins/techdocs-backend/src/cache/cacheMiddleware.ts +++ b/plugins/techdocs-backend/src/cache/cacheMiddleware.ts @@ -35,7 +35,7 @@ export const createCacheMiddleware = ({ // loaded from cache. Cache key is the object's path (after `/static/docs/`). cacheMiddleware.use(async (req, res, next) => { const socket = res.socket; - const isCacheable = req.path.includes('/static/docs/'); + const isCacheable = req.path.startsWith('/static/docs/'); // Continue early if this is non-cacheable, or there's no socket. if (!isCacheable || !socket) { @@ -66,11 +66,11 @@ export const createCacheMiddleware = ({ // When a socket is closed, if there were no errors and the data written // over the socket should be cached, cache it! - socket.on('close', hadError => { + socket.on('close', async hadError => { const content = Buffer.concat(chunks); const head = content.toString('utf8', 0, 12); if (writeToCache && !hadError && head.match(/HTTP\/\d\.\d 200/)) { - cache.set(reqPath, content); + await cache.set(reqPath, content); } });