Skip to content

Commit

Permalink
Fix: middleware crashes when manifest.json is missing or not where it…
Browse files Browse the repository at this point in the history
… was expected (#1054)

* quick and dirty fix

* Linting auto fix commit

* proper code quality and checks

* testing the new scenarios

* changeset

* avoid mutating the passed in manifest

* Linting auto fix commit

* fixed broken links

* also check for undefined as return value

* added missing test

* enhanced test app for local testing

* review feedback

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
  • Loading branch information
tobiasqueck and github-actions[bot] authored Jun 12, 2023
1 parent e91e129 commit 7f1971c
Show file tree
Hide file tree
Showing 19 changed files with 21,999 additions and 238 deletions.
5 changes: 5 additions & 0 deletions .changeset/happy-hornets-fix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sap-ux/ui5-proxy-middleware': patch
---

Fix: handle missing manifest.json
11 changes: 0 additions & 11 deletions packages/ui5-proxy-middleware/src/base/types.ts
Original file line number Diff line number Diff line change
@@ -1,24 +1,13 @@
import type { NextFunction } from 'express';
import type { IncomingMessage } from 'http';

import type { UI5ProxyConfig } from '@sap-ux/ui5-config';

export type Ui5MiddlewareConfig = UI5ProxyConfig;

export interface ProxyConfig {
path: string;
url: string;
version?: string;
proxy?: string;
}

export interface MiddlewareParameters<T> {
resources: object;
options: {
configuration: T;
};
}

export interface UI5ProxyRequest {
next?: NextFunction;
}
Expand Down
33 changes: 3 additions & 30 deletions packages/ui5-proxy-middleware/src/base/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -189,42 +189,15 @@ export const setHtmlResponse = (res: any, html: string): void => {
}
};

/**
* Gets the manifest.json for a given application.
*
* @param args list of runtime arguments
* @returns The content of the manifest.json
*/
export const getManifest = async (args: string[]): Promise<Manifest> => {
const projectRoot = process.cwd();
const yamlFileName = getYamlFile(args);
const ui5YamlPath = join(projectRoot, yamlFileName);
const webAppFolder = await getWebAppFolderFromYaml(ui5YamlPath);
const manifestPath = join(projectRoot, webAppFolder, 'manifest.json');
const manifest: Manifest = JSON.parse(readFileSync(manifestPath, { encoding: 'utf8' }));

return manifest;
};

/**
* Gets the minUI5Version from the manifest.json.
*
* @param args list of runtime args
* @returns The minUI5Version from manifest.json or undefined otherwise
*/
export async function getUI5VersionFromManifest(args: string[]): Promise<string | undefined> {
const manifest = await getManifest(args);
return manifest['sap.ui5']?.dependencies?.minUI5Version;
}

/**
* Determines which UI5 version to use when previewing the application.
*
* @param version ui5 version as defined in the yaml or via cli argument
* @param log logger for outputing information from where ui5 version config is coming
* @param manifest optional already loaded manifest.json
* @returns The UI5 version with which the application will be started
*/
export async function resolveUI5Version(version?: string, log?: ToolsLogger): Promise<string> {
export async function resolveUI5Version(version?: string, log?: ToolsLogger, manifest?: Manifest): Promise<string> {
let ui5Version: string;
let ui5VersionInfo: string;
let ui5VersionLocation: string;
Expand All @@ -236,7 +209,7 @@ export async function resolveUI5Version(version?: string, log?: ToolsLogger): Pr
ui5Version = version ? version : '';
ui5VersionLocation = getYamlFile(process.argv);
} else {
const minUI5Version = await getUI5VersionFromManifest(process.argv);
const minUI5Version = manifest?.['sap.ui5']?.dependencies?.minUI5Version;
ui5Version = minUI5Version && !isNaN(parseFloat(minUI5Version)) ? minUI5Version : '';
ui5VersionLocation = 'manifest.json';
}
Expand Down
38 changes: 35 additions & 3 deletions packages/ui5-proxy-middleware/src/ui5/middleware.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type { RequestHandler, NextFunction, Request, Response } from 'express';
import type { Options } from 'http-proxy-middleware';
import { ToolsLogger, UI5ToolingTransport } from '@sap-ux/logger';
import type { MiddlewareParameters, Ui5MiddlewareConfig, ProxyConfig } from '../base';
import type { ProxyConfig } from '../base';
import {
getCorporateProxyServer,
HTML_MOUNT_PATHS,
Expand All @@ -12,6 +12,9 @@ import {
} from '../base';
import dotenv from 'dotenv';
import type { UI5ProxyConfig } from '@sap-ux/ui5-config';
import type { Manifest } from '@sap-ux/project-access';
import type { MiddlewareParameters } from '@ui5/server';
import type { ReaderCollection } from '@ui5/fs';

/**
* Create proxy options based on the middleware config.
Expand Down Expand Up @@ -46,13 +49,42 @@ function createRequestHandler(routes: { route: string; handler: RequestHandler }
};
}

module.exports = async ({ options }: MiddlewareParameters<Ui5MiddlewareConfig>): Promise<RequestHandler> => {
/**
* Search the project for the manifest.json.
*
* @param rootProject @ui5/fs reader collection with access to the project files
* @returns manifest.json as object or undefined if not found
*/
async function loadManifest(rootProject: ReaderCollection): Promise<Manifest | undefined> {
const files = await rootProject.byGlob('**/manifest.json');
if (files?.length > 0) {
return JSON.parse(await files[0].getString());
} else {
return undefined;
}
}

module.exports = async ({ resources, options }: MiddlewareParameters<UI5ProxyConfig>): Promise<RequestHandler> => {
const logger = new ToolsLogger({
transports: [new UI5ToolingTransport({ moduleName: 'ui5-proxy-middleware' })]
});

if (!options.configuration?.ui5) {
logger.error('Configuration missing, no proxy created.');
return (_req, _res, next) => next();
}

dotenv.config();
const config = options.configuration;
const ui5Version = await resolveUI5Version(config.version, logger);
let ui5Version: string = '';
try {
const manifest = await loadManifest(resources.rootProject);
ui5Version = await resolveUI5Version(config.version, logger, manifest);
} catch (error) {
logger.warn('Retrieving UI5 version failed, using latest version instead.');
logger.debug(error);
}

const envUI5Url = process.env.FIORI_TOOLS_UI5_URI;
const directLoad = !!config.directLoad;
const corporateProxyServer = getCorporateProxyServer(config.proxy);
Expand Down
118 changes: 118 additions & 0 deletions packages/ui5-proxy-middleware/src/ui5/ui5.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
declare module '@ui5/fs' {
/**
* https://sap.github.io/ui5-tooling/v2/api/module-@ui5_fs.Resource.html
*/
export class Resource {
/**
* Gets the resources path
*/
getPath(): string;

/**
* Gets a buffer with the resource content.
*/
getBuffer(): Promise<Buffer>;

/**
* Gets a string with the resource content.
*/
getString(): Promise<string>;
}

/**
* https://sap.github.io/ui5-tooling/v2/api/module-@ui5_fs.ReaderCollection.html
*/
export class ReaderCollection {
/**
* Locates resources by matching glob patterns.
*/
byGlob(virPattern: string | string[], options?: object): Promise<Resource[]>;

/**
* Locates resources by matching a given path.
*/
byPath(virPattern: string | string[], options?: object): Promise<Resource[]>;
}

/**
* https://sap.github.io/ui5-tooling/v2/api/module-@ui5_fs.DuplexCollection.html
*/
export class DuplexCollection extends ReaderCollection {}

/**
* https://sap.github.io/ui5-tooling/v2/api/module-@ui5_fs.AbstractReader.html
*/
export class AbstractReader {}
}

declare module '@ui5/server' {
export interface MiddlewareParameters<C> {
/**
* DuplexCollection to read and write files
*/
resources: {
all: ReaderCollection;
dependencies: ReaderCollection;
rootProject: ReaderCollection;
};

/**
* Project specific options
*/
options: {
/**
* Optional middleware configuration if provided in ui5*.yaml
*/
configuration?: C;
};

/**
* Middleware utilities (not yet used in our middlewares)
*/
middlewareUtils: unknown;
}
}

declare module '@ui5/builder' {
/**
* https://sap.github.io/ui5-tooling/v2/api/module-@ui5_builder.tasks.TaskUtil.html
*/
export class TaskUtil {}

export interface TaskParameters<C> {
/**
* DuplexCollection to read and write files
*/
workspace: DuplexCollection;

/**
* Reader or Collection to read dependency files
*/
dependencies: AbstractReader;

/**
* Specification Version dependent interface to a @ui5/builder.tasks.TaskUtil instance
*/
taskUtil: TaskUtil;

/**
* Project specific options
*/
options: {
/**
* Project name
*/
projectName: string;

/**
* Project namespace if available
*/
projectNamespace?: string;

/**
* Optional task configuration if provided in ui5*.yaml
*/
configuration?: C;
};
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`Utils injectUI5Url injects UI5 URL in html 1`] = `
exports[`utils injectUI5Url injects UI5 URL in html 1`] = `
"
<html>
<head>
Expand All @@ -20,7 +20,7 @@ exports[`Utils injectUI5Url injects UI5 URL in html 1`] = `
</html>"
`;
exports[`Utils injectUI5Url injects UI5 URL in html, latest ui5 version 1`] = `
exports[`utils injectUI5Url injects UI5 URL in html, latest ui5 version 1`] = `
"
<html>
<head>
Expand Down
2 changes: 1 addition & 1 deletion packages/ui5-proxy-middleware/test/base/proxy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import * as hpm from 'http-proxy-middleware';
import * as utils from '../../src/base/utils';
import { ToolsLogger } from '@sap-ux/logger';

describe('ui5Proxy', () => {
describe('proxy', () => {
const createProxyMiddlewareSpy = jest.spyOn(hpm, 'createProxyMiddleware').mockImplementation(jest.fn());

beforeEach(() => {
Expand Down
69 changes: 4 additions & 65 deletions packages/ui5-proxy-middleware/test/base/utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import {
filterCompressedHtmlFiles,
getCorporateProxyServer,
getHtmlFile,
getManifest,
getWebAppFolderFromYaml,
getYamlFile,
hideProxyCredentials,
Expand All @@ -20,8 +19,9 @@ import * as baseUtils from '../../src/base/utils';
import type { ProxyConfig } from '../../src/base/types';
import type { IncomingMessage } from 'http';
import { NullTransport, ToolsLogger } from '@sap-ux/logger';
import { Manifest } from '@sap-ux/project-access';

describe('Utils', () => {
describe('utils', () => {
const existsMock = jest.spyOn(fs, 'existsSync').mockReturnValue(true);
const readFileMock = jest.spyOn(fs, 'readFileSync').mockReturnValue('');
const logger = new ToolsLogger({
Expand Down Expand Up @@ -321,64 +321,6 @@ describe('Utils', () => {
expect(mockSend).toHaveBeenCalledWith(html);
});

test('getManifest: returns manifest.json', async () => {
const manifest = {
_version: '1.32.0'
};
readFileMock.mockImplementation((path) =>
(path as string).endsWith('manifest.json') ? JSON.stringify(manifest) : ''
);
const result = await getManifest([]);
expect(result).toEqual(manifest);
});

test('getUI5VersionFromManifest: return undefined if sap.ui5 section is missing in manifest.json', async () => {
const manifest = {
_version: '1.32.0'
};
readFileMock.mockImplementation((path) =>
(path as string).endsWith('manifest.json') ? JSON.stringify(manifest) : ''
);
const result = await baseUtils.getUI5VersionFromManifest([]);
expect(result).toBeUndefined();
});

test('getUI5VersionFromManifest: return undefined if sap.ui5.dependencies section is missing in manifest.json', async () => {
const manifest = {
_version: '1.32.0',
'sap.ui5': {}
};
readFileMock.mockImplementation((path) =>
(path as string).endsWith('manifest.json') ? JSON.stringify(manifest) : ''
);
const result = await baseUtils.getUI5VersionFromManifest([]);
expect(result).toBeUndefined();
});

test('getUI5VersionFromManifest: return undefined if sap.ui5.dependencies.minUI5Version is missing in manifest.json', async () => {
const manifest = {
_version: '1.32.0',
'sap.ui5': { dependencies: {} }
};
readFileMock.mockImplementation((path) =>
(path as string).endsWith('manifest.json') ? JSON.stringify(manifest) : ''
);
const result = await baseUtils.getUI5VersionFromManifest([]);
expect(result).toBeUndefined();
});

test('getUI5VersionFromManifest: return minUI5Version from manifest.json', async () => {
const manifest = {
_version: '1.32.0',
'sap.ui5': { dependencies: { minUI5Version: '1.86.4' } }
};
readFileMock.mockImplementation((path) =>
(path as string).endsWith('manifest.json') ? JSON.stringify(manifest) : ''
);
const result = await baseUtils.getUI5VersionFromManifest([]);
expect(result).toBe('1.86.4');
});

test('setUI5Version: take version from YAML', async () => {
const version = '1.90.0';
const log: any = {
Expand Down Expand Up @@ -410,11 +352,8 @@ describe('Utils', () => {
const manifest = {
_version: '1.32.0',
'sap.ui5': { dependencies: { minUI5Version: '1.96.0' } }
};
readFileMock.mockImplementation((path) =>
(path as string).endsWith('manifest.json') ? JSON.stringify(manifest) : ''
);
const result = await baseUtils.resolveUI5Version(undefined, log);
} as Manifest;
const result = await baseUtils.resolveUI5Version(undefined, log, manifest);
expect(result).toEqual('1.96.0');
expect(log.info).toBeCalledTimes(1);
expect(log.info).toHaveBeenCalledWith('Using UI5 version 1.96.0 based on manifest.json');
Expand Down
Loading

0 comments on commit 7f1971c

Please sign in to comment.