From 6daa11990ceee105c3d8a77cf23f003a15a654dd Mon Sep 17 00:00:00 2001 From: Samuel Padgett Date: Wed, 5 Aug 2020 10:16:39 -0400 Subject: [PATCH] Bug 1866114: Don't store OpenAPI definitions in localStorage The document is too large, so we hit browser limits for localStorage. This can break Safari. Since the API server supports ETags for the OpenAPI document, rely on browser caching instead. --- .../components/editor/yaml-editor-utils.ts | 4 +- .../sidebars/explore-type-sidebar.tsx | 4 +- frontend/public/module/k8s/swagger.ts | 58 ++++++------------- 3 files changed, 21 insertions(+), 45 deletions(-) diff --git a/frontend/packages/console-shared/src/components/editor/yaml-editor-utils.ts b/frontend/packages/console-shared/src/components/editor/yaml-editor-utils.ts index 1f07632019d..3c5db19f3d7 100644 --- a/frontend/packages/console-shared/src/components/editor/yaml-editor-utils.ts +++ b/frontend/packages/console-shared/src/components/editor/yaml-editor-utils.ts @@ -5,7 +5,7 @@ import { } from 'monaco-languageclient/lib/monaco-converter'; import { getLanguageService, TextDocument } from 'yaml-language-server'; import { openAPItoJSONSchema } from '@console/internal/module/k8s/openapi-to-json-schema'; -import { getStoredSwagger } from '@console/internal/module/k8s/swagger'; +import { getSwaggerDefinitions } from '@console/internal/module/k8s/swagger'; import { global_BackgroundColor_dark_100 as editorBackground } from '@patternfly/react-tokens'; window.monaco.editor.defineTheme('console', { @@ -62,7 +62,7 @@ export const createYAMLService = () => { const yamlService = getLanguageService(resolveSchema, workspaceContext, []); // Prepare the schema - const yamlOpenAPI = getStoredSwagger(); + const yamlOpenAPI = getSwaggerDefinitions(); // Convert the openAPI schema to something the language server understands const kubernetesJSONSchema = openAPItoJSONSchema(yamlOpenAPI); diff --git a/frontend/public/components/sidebars/explore-type-sidebar.tsx b/frontend/public/components/sidebars/explore-type-sidebar.tsx index 9f4bbbba777..f8ab068569b 100644 --- a/frontend/public/components/sidebars/explore-type-sidebar.tsx +++ b/frontend/public/components/sidebars/explore-type-sidebar.tsx @@ -4,7 +4,7 @@ import { Breadcrumb, BreadcrumbItem, Button } from '@patternfly/react-core'; import { getDefinitionKey, - getStoredSwagger, + getSwaggerDefinitions, getSwaggerPath, K8sKind, SwaggerDefinition, @@ -29,7 +29,7 @@ export const ExploreType: React.FC = (props) => { return null; } - const allDefinitions: SwaggerDefinitions = getStoredSwagger(); + const allDefinitions: SwaggerDefinitions = getSwaggerDefinitions(); if (!allDefinitions) { return null; } diff --git a/frontend/public/module/k8s/swagger.ts b/frontend/public/module/k8s/swagger.ts index afd50756004..dc4478fe8bf 100644 --- a/frontend/public/module/k8s/swagger.ts +++ b/frontend/public/module/k8s/swagger.ts @@ -21,34 +21,13 @@ export const getDefinitionKey = _.memoize( referenceForModel, ); -// Cache parsed swagger to avoid reparsing the JSON each call. -let swagger: SwaggerDefinitions; -export const getStoredSwagger = (): SwaggerDefinitions => { - if (swagger) { - return swagger; - } - const json = window.localStorage.getItem(SWAGGER_LOCAL_STORAGE_KEY); - if (!json) { - return null; - } - try { - swagger = JSON.parse(json); - return swagger; - } catch (e) { - // eslint-disable-next-line no-console - console.error('Could not parse swagger JSON.', e); - return null; - } -}; - -const storeSwagger = (definitions: SwaggerDefinitions) => { - // Only store definitions to reduce the document size. - const json = JSON.stringify(definitions); - window.localStorage.setItem(SWAGGER_LOCAL_STORAGE_KEY, json); - swagger = definitions; -}; +let swaggerDefinitions: SwaggerDefinitions; +export const getSwaggerDefinitions = (): SwaggerDefinitions => swaggerDefinitions; export const fetchSwagger = async (): Promise => { + // Remove any old definitions from `localSotrage`. We rely on the browser cache now. + // TODO: We should be able to remove this in a future release. + localStorage.removeItem(SWAGGER_LOCAL_STORAGE_KEY); try { const response: SwaggerAPISpec = await coFetchJSON('api/kubernetes/openapi/v2'); if (!response.definitions) { @@ -56,8 +35,8 @@ export const fetchSwagger = async (): Promise => { console.error('Definitions missing in OpenAPI response.'); return null; } - storeSwagger(response.definitions); - return response.definitions; + swaggerDefinitions = response.definitions; + return swaggerDefinitions; } catch (e) { // eslint-disable-next-line no-console console.error('Could not get OpenAPI definitions', e); @@ -66,12 +45,11 @@ export const fetchSwagger = async (): Promise => { }; export const definitionFor = _.memoize((model: K8sKind): SwaggerDefinition => { - const allDefinitions: SwaggerDefinitions = getStoredSwagger(); - if (!allDefinitions) { + if (!swaggerDefinitions) { return null; } - const key = getDefinitionKey(model, allDefinitions); - return _.get(allDefinitions, key); + const key = getDefinitionKey(model, swaggerDefinitions); + return _.get(swaggerDefinitions, key); }, referenceForModel); const getRef = (definition: SwaggerDefinition): string => { @@ -102,12 +80,11 @@ export const getSwaggerPath = ( }; const findDefinition = (kindObj: K8sKind, propertyPath: string[]): SwaggerDefinition => { - const allDefinitions: SwaggerDefinitions = getStoredSwagger(); - if (!allDefinitions) { + if (!swaggerDefinitions) { return null; } - const rootPath = getDefinitionKey(kindObj, allDefinitions); + const rootPath = getDefinitionKey(kindObj, swaggerDefinitions); const path = propertyPath.reduce( (currentPath: string[], nextProperty: string, i: number): string[] => { if (!currentPath) { @@ -115,12 +92,12 @@ const findDefinition = (kindObj: K8sKind, propertyPath: string[]): SwaggerDefini } // Don't follow the last reference since the description is not as good. const followRef = i !== propertyPath.length - 1; - return getSwaggerPath(allDefinitions, currentPath, nextProperty, followRef); + return getSwaggerPath(swaggerDefinitions, currentPath, nextProperty, followRef); }, [rootPath], ); - return path ? (_.get(allDefinitions, path) as SwaggerDefinition) : null; + return path ? (_.get(swaggerDefinitions, path) as SwaggerDefinition) : null; }; export const getPropertyDescription = ( @@ -133,12 +110,11 @@ export const getPropertyDescription = ( }; export const getResourceDescription = _.memoize((kindObj: K8sKind): string => { - const allDefinitions: SwaggerDefinitions = getStoredSwagger(); - if (!allDefinitions) { + if (!swaggerDefinitions) { return null; } - const key = getDefinitionKey(kindObj, allDefinitions); - return _.get(allDefinitions, [key, 'description']); + const key = getDefinitionKey(kindObj, swaggerDefinitions); + return _.get(swaggerDefinitions, [key, 'description']); }, referenceForModel); export type SwaggerDefinition = {