From a2b291294e2fd33200290236b0fa6dd0e6565f76 Mon Sep 17 00:00:00 2001 From: Tilman Vatteroth Date: Sun, 8 Oct 2023 21:40:37 +0200 Subject: [PATCH] fix: usage of internal api Signed-off-by: Tilman Vatteroth --- .../api-request-builder.ts | 15 ++++- .../base-url/base-url-context-provider.tsx | 2 +- .../utils/base-url-from-env-extractor.spec.ts | 4 +- .../src/utils/base-url-from-env-extractor.ts | 57 +++++++------------ 4 files changed, 37 insertions(+), 41 deletions(-) diff --git a/frontend/src/api/common/api-request-builder/api-request-builder.ts b/frontend/src/api/common/api-request-builder/api-request-builder.ts index 290a23de5..93008faa4 100644 --- a/frontend/src/api/common/api-request-builder/api-request-builder.ts +++ b/frontend/src/api/common/api-request-builder/api-request-builder.ts @@ -39,7 +39,20 @@ export abstract class ApiRequestBuilder { * @return the base url */ private determineBaseUrl(baseUrl?: string) { - return typeof window !== 'undefined' ? baseUrl ?? '/' : baseUrlFromEnvExtractor.extractBaseUrls().internalApiUrl + if (this.isSSR()) { + const internalApiUrl = baseUrlFromEnvExtractor.extractBaseUrls().internalApiUrl + const actualBaseUrl = internalApiUrl ?? baseUrl + if (actualBaseUrl === undefined) { + throw new Error("Can't make request without forced base url and without internal api url") + } + return actualBaseUrl + } else { + return baseUrl ?? '/' + } + } + + private isSSR() { + return typeof window === 'undefined' } protected async sendRequestAndVerifyResponse(httpMethod: RequestInit['method']): Promise> { diff --git a/frontend/src/components/common/base-url/base-url-context-provider.tsx b/frontend/src/components/common/base-url/base-url-context-provider.tsx index 17e8d2482..f05a09d0f 100644 --- a/frontend/src/components/common/base-url/base-url-context-provider.tsx +++ b/frontend/src/components/common/base-url/base-url-context-provider.tsx @@ -10,7 +10,7 @@ import React, { createContext } from 'react' export interface BaseUrls { renderer: string editor: string - internalApiUrl: string + internalApiUrl: string | undefined } interface BaseUrlContextProviderProps { diff --git a/frontend/src/utils/base-url-from-env-extractor.spec.ts b/frontend/src/utils/base-url-from-env-extractor.spec.ts index e9b83d112..9563f7971 100644 --- a/frontend/src/utils/base-url-from-env-extractor.spec.ts +++ b/frontend/src/utils/base-url-from-env-extractor.spec.ts @@ -83,7 +83,7 @@ describe('BaseUrlFromEnvExtractor', () => { expect(() => sut.extractBaseUrls()).toThrow() }) - it('should copy editor base url to renderer base/internal api url if url is omitted', () => { + it('should copy editor base url to renderer base url if url is omitted', () => { process.env.HD_BASE_URL = 'https://editor1.example.org/' delete process.env.HD_RENDERER_BASE_URL delete process.env.HD_INTERNAL_API_URL @@ -91,7 +91,7 @@ describe('BaseUrlFromEnvExtractor', () => { expect(sut.extractBaseUrls()).toStrictEqual({ renderer: 'https://editor1.example.org/', - internalApiUrl: 'https://editor1.example.org/', + internalApiUrl: undefined, editor: 'https://editor1.example.org/' }) }) diff --git a/frontend/src/utils/base-url-from-env-extractor.ts b/frontend/src/utils/base-url-from-env-extractor.ts index 6767b0fbf..e17980b40 100644 --- a/frontend/src/utils/base-url-from-env-extractor.ts +++ b/frontend/src/utils/base-url-from-env-extractor.ts @@ -6,7 +6,7 @@ import type { BaseUrls } from '../components/common/base-url/base-url-context-provider' import { Logger } from './logger' import { isTestMode, isBuildTime } from './test-modes' -import { NoSubdirectoryAllowedError, parseUrl } from '@hedgedoc/commons' +import { parseUrl } from '@hedgedoc/commons' import { Optional } from '@mrdrogdrog/optional' /** @@ -17,54 +17,35 @@ export class BaseUrlFromEnvExtractor { private readonly logger = new Logger('Base URL Configuration') private extractUrlFromEnvVar(envVarValue: string | undefined): Optional { - try { - return parseUrl(envVarValue) - } catch (error) { - if (error instanceof NoSubdirectoryAllowedError) { - this.logger.error(error.message) - return Optional.empty() - } else { - throw error - } - } + return parseUrl(envVarValue).orThrow(() => new Error(`${envVarValue} isn't a valid URL`)) } - private extractEditorBaseUrlFromEnv(): Optional { - const envValue = this.extractUrlFromEnvVar(process.env.HD_BASE_URL) - if (envValue.isEmpty()) { - this.logger.error("HD_BASE_URL isn't a valid URL!") - } - return envValue - } - - private extractUrlFromEnv(editorBaseUrl: URL, envVarName: string): Optional { + private extractUrlFromEnv(envVarName: string): Optional { if (isTestMode) { this.logger.info(`Test mode activated. Using editor base url for ${envVarName}.`) - return Optional.of(editorBaseUrl) + return Optional.empty() } if (!process.env[envVarName]) { - this.logger.info(`${envVarName} is unset. Using editor base url.`) - return Optional.of(editorBaseUrl) + this.logger.info(`${envVarName} is unset.`) + return Optional.empty() } return this.extractUrlFromEnvVar(process.env[envVarName]) } private renewBaseUrls(): BaseUrls { - return this.extractEditorBaseUrlFromEnv() - .flatMap((editorBaseUrl) => - this.extractUrlFromEnv(editorBaseUrl, 'HD_RENDERER_BASE_URL').flatMap((rendererBaseUrl) => - this.extractUrlFromEnv(editorBaseUrl, 'HD_INTERNAL_API_URL').map((internalApiUrl) => { - return { - editor: editorBaseUrl.toString(), - renderer: rendererBaseUrl.toString(), - internalApiUrl: internalApiUrl.toString() - } - }) - ) - ) - .orElseThrow(() => new Error('couldnt parse env vars')) + return this.extractUrlFromEnvVar(process.env.HD_BASE_URL) + .map((editorBaseUrl) => { + const rendererBaseUrl = this.extractUrlFromEnv('HD_RENDERER_BASE_URL').orElse(editorBaseUrl) + const internalApiUrl = this.extractUrlFromEnv('HD_INTERNAL_API_URL').orElse(undefined) + return { + editor: editorBaseUrl.toString(), + renderer: rendererBaseUrl.toString(), + internalApiUrl: internalApiUrl?.toString() + } as BaseUrls + }) + .orElseThrow(() => new Error("couldn't parse env")) } /** @@ -94,7 +75,9 @@ export class BaseUrlFromEnvExtractor { } this.logger.info('Editor base URL', this.baseUrls.editor.toString()) this.logger.info('Renderer base URL', this.baseUrls.renderer.toString()) - this.logger.info('Internal API URL', this.baseUrls.internalApiUrl.toString()) + if (this.baseUrls.internalApiUrl !== undefined) { + this.logger.info('Internal API URL', this.baseUrls.internalApiUrl?.toString()) + } } }