Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: middleware crashes when manifest.json is missing or not where it was expected #1054

Merged
merged 17 commits into from
Jun 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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> {
Klaus-Keller marked this conversation as resolved.
Show resolved Hide resolved
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 : '';
ullasholla marked this conversation as resolved.
Show resolved Hide resolved
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