From 266e93b47831b2c058612d4ee643b1a0df4e8030 Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Tue, 24 Nov 2020 23:29:18 +0100 Subject: [PATCH] TechDocs: Refactor metadata retrieval 1. Don't use mkdocs in name of APIs or variables. It is an implementation detail and liable to change in future. 2. techDocsApi.getMetadata('mkdocs') and techDocsAPI.getMetadata('entity') should be two separate functions just because their responses differ in structure. They should also be type checked. 3. Use either 'techdocs' or 'TechDocs' consistently. 'techDocs' seems like an unnecessary third way to write TechDocs, which can be avoided. 4. Remove unused /plugins/techdocs-backend/src/service/metadata.ts file --- .../techdocs-backend/src/service/metadata.ts | 41 ------------------- .../techdocs-backend/src/service/router.ts | 6 +-- plugins/techdocs/src/api.ts | 36 ++++++++++++++-- .../reader/components/TechDocsPage.test.tsx | 11 ++--- .../src/reader/components/TechDocsPage.tsx | 14 +++---- .../components/TechDocsPageHeader.test.tsx | 4 +- .../reader/components/TechDocsPageHeader.tsx | 11 +++-- 7 files changed, 57 insertions(+), 66 deletions(-) delete mode 100644 plugins/techdocs-backend/src/service/metadata.ts diff --git a/plugins/techdocs-backend/src/service/metadata.ts b/plugins/techdocs-backend/src/service/metadata.ts deleted file mode 100644 index 760180f8d2..0000000000 --- a/plugins/techdocs-backend/src/service/metadata.ts +++ /dev/null @@ -1,41 +0,0 @@ -/* - * Copyright 2020 Spotify AB - * - * 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. - */ - -import fetch from 'cross-fetch'; - -export class TechDocsMetadata { - private async getMetadataFile(docsUrl: String) { - const metadataURL = `${docsUrl}/techdocs_metadata.json`; - - try { - const req = await fetch(metadataURL); - - return await req.json(); - } catch (error) { - throw new Error(error); - } - } - - public async getMkDocsMetaData(docsUrl: any) { - const mkDocsMetadata = await this.getMetadataFile(docsUrl); - - if (!mkDocsMetadata) return null; - - return { - ...mkDocsMetadata, - }; - } -} diff --git a/plugins/techdocs-backend/src/service/router.ts b/plugins/techdocs-backend/src/service/router.ts index 1d932a1fb9..625252dd7e 100644 --- a/plugins/techdocs-backend/src/service/router.ts +++ b/plugins/techdocs-backend/src/service/router.ts @@ -61,7 +61,7 @@ export async function createRouter({ }: RouterOptions): Promise { const router = Router(); - router.get('/metadata/mkdocs/*', async (req, res) => { + router.get('/metadata/techdocs/*', async (req, res) => { let storageUrl = config.getString('techdocs.storageUrl'); if (publisher instanceof LocalPublish) { storageUrl = new URL( @@ -74,8 +74,8 @@ export async function createRouter({ const metadataURL = `${storageUrl}/${path}/techdocs_metadata.json`; try { - const mkDocsMetadata = await (await fetch(metadataURL)).json(); - res.send(mkDocsMetadata); + const techdocsMetadata = await (await fetch(metadataURL)).json(); + res.send(techdocsMetadata); } catch (err) { logger.info(`Unable to get metadata for ${path} with error ${err}`); throw new Error(`Unable to get metadata for ${path} with error ${err}`); diff --git a/plugins/techdocs/src/api.ts b/plugins/techdocs/src/api.ts index 7281ebdf13..98e1e6c310 100644 --- a/plugins/techdocs/src/api.ts +++ b/plugins/techdocs/src/api.ts @@ -15,7 +15,6 @@ */ import { createApiRef } from '@backstage/core'; - import { ParsedEntityId } from './types'; export const techdocsStorageApiRef = createApiRef({ @@ -38,7 +37,8 @@ export interface TechDocsStorage { } export interface TechDocs { - getMetadata(metadataType: string, entityId: ParsedEntityId): Promise; + getTechDocsMetadata(entityId: ParsedEntityId): Promise; + getEntityMetadata(entityId: ParsedEntityId): Promise; } /** @@ -53,10 +53,38 @@ export class TechDocsApi implements TechDocs { this.apiOrigin = apiOrigin; } - async getMetadata(metadataType: string, entityId: ParsedEntityId) { + /** + * Retrieve TechDocs metadata. + * + * When docs are built, we generate a techdocs_metadata.json and store it along with the generated + * static files. It includes necessary data about the docs site. This method requests techdocs-backend + * which retries the TechDocs metadata. + * + * @param {ParsedEntityId} entityId Object containing entity data like name, namespace, etc. + */ + async getTechDocsMetadata(entityId: ParsedEntityId) { const { kind, namespace, name } = entityId; - const requestUrl = `${this.apiOrigin}/metadata/${metadataType}/${namespace}/${kind}/${name}`; + const requestUrl = `${this.apiOrigin}/metadata/techdocs/${namespace}/${kind}/${name}`; + + const request = await fetch(`${requestUrl}`); + const res = await request.json(); + + return res; + } + + /** + * Retrieve metadata about an entity. + * + * This method requests techdocs-backend which uses the catalog APIs to respond with filtered + * information required here. + * + * @param {ParsedEntityId} entityId Object containing entity data like name, namespace, etc. + */ + async getEntityMetadata(entityId: ParsedEntityId) { + const { kind, namespace, name } = entityId; + + const requestUrl = `${this.apiOrigin}/metadata/entity/${namespace}/${kind}/${name}`; const request = await fetch(`${requestUrl}`); const res = await request.json(); diff --git a/plugins/techdocs/src/reader/components/TechDocsPage.test.tsx b/plugins/techdocs/src/reader/components/TechDocsPage.test.tsx index 464115bab7..c05c0488ff 100644 --- a/plugins/techdocs/src/reader/components/TechDocsPage.test.tsx +++ b/plugins/techdocs/src/reader/components/TechDocsPage.test.tsx @@ -50,17 +50,18 @@ describe('', () => { entityId: 'Component::backstage', }); - const techDocsApi: Partial = { - getMetadata: () => Promise.resolve([]), + const techdocsApi: Partial = { + getEntityMetadata: () => Promise.resolve([]), + getTechDocsMetadata: () => Promise.resolve([]), }; - const techDocsStorageApi: Partial = { + const techdocsStorageApi: Partial = { getEntityDocs: (): Promise => Promise.resolve('String'), getBaseUrl: (): string => '', }; const apiRegistry = ApiRegistry.from([ - [techdocsApiRef, techDocsApi], - [techdocsStorageApiRef, techDocsStorageApi], + [techdocsApiRef, techdocsApi], + [techdocsStorageApiRef, techdocsStorageApi], ]); await act(async () => { diff --git a/plugins/techdocs/src/reader/components/TechDocsPage.tsx b/plugins/techdocs/src/reader/components/TechDocsPage.tsx index 00f0b3c9d3..f2635c0dfb 100644 --- a/plugins/techdocs/src/reader/components/TechDocsPage.tsx +++ b/plugins/techdocs/src/reader/components/TechDocsPage.tsx @@ -26,19 +26,19 @@ export const TechDocsPage = () => { const [documentReady, setDocumentReady] = useState(false); const { namespace, kind, name } = useParams(); - const techDocsApi = useApi(techdocsApiRef); + const techdocsApi = useApi(techdocsApiRef); - const mkdocsMetadataRequest = useAsync(() => { + const techdocsMetadataRequest = useAsync(() => { if (documentReady) { - return techDocsApi.getMetadata('mkdocs', { kind, namespace, name }); + return techdocsApi.getTechDocsMetadata({ kind, namespace, name }); } return Promise.resolve({ loading: true }); - }, [kind, namespace, name, techDocsApi, documentReady]); + }, [kind, namespace, name, techdocsApi, documentReady]); const entityMetadataRequest = useAsync(() => { - return techDocsApi.getMetadata('entity', { kind, namespace, name }); - }, [kind, namespace, name, techDocsApi]); + return techdocsApi.getEntityMetadata({ kind, namespace, name }); + }, [kind, namespace, name, techdocsApi]); const onReady = () => { setDocumentReady(true); @@ -48,7 +48,7 @@ export const TechDocsPage = () => { ', () => { }, }, }, - mkdocs: { + techdocs: { loading: false, value: { site_name: 'test-site-name', @@ -73,7 +73,7 @@ describe('', () => { entity: { loading: false, }, - mkdocs: { + techdocs: { loading: false, }, }} diff --git a/plugins/techdocs/src/reader/components/TechDocsPageHeader.tsx b/plugins/techdocs/src/reader/components/TechDocsPageHeader.tsx index f4c447bae2..475b87da6f 100644 --- a/plugins/techdocs/src/reader/components/TechDocsPageHeader.tsx +++ b/plugins/techdocs/src/reader/components/TechDocsPageHeader.tsx @@ -25,7 +25,7 @@ type TechDocsPageHeaderProps = { entityId: ParsedEntityId; metadataRequest: { entity: AsyncState; - mkdocs: AsyncState; + techdocs: AsyncState; }; }; @@ -33,15 +33,18 @@ export const TechDocsPageHeader = ({ entityId, metadataRequest, }: TechDocsPageHeaderProps) => { - const { mkdocs: mkdocsMetadata, entity: entityMetadata } = metadataRequest; + const { + techdocs: techdocsMetadata, + entity: entityMetadata, + } = metadataRequest; - const { value: mkDocsMetadataValues } = mkdocsMetadata; + const { value: techdocsMetadataValues } = techdocsMetadata; const { value: entityMetadataValues } = entityMetadata; const { kind, name } = entityId; const { site_name: siteName, site_description: siteDescription } = - mkDocsMetadataValues || {}; + techdocsMetadataValues || {}; const { locationMetadata,