Skip to content

Commit

Permalink
refactor(core): Post-release refactorings of Public API (#3495)
Browse files Browse the repository at this point in the history
* ⚡ Post-release refactorings

* 🧪 Add `--forceExit`

* 🛠 typing refactor (#3486)

* 🐛 Fix middleware arguments

* 👕 Fix lint

* ⚡ Restore commented out block

Co-authored-by: Ben Hesseldieck <1849459+BHesseldieck@users.noreply.github.com>
  • Loading branch information
ivov and BHesseldieck authored Jun 14, 2022
1 parent 2ebcf4b commit b8e3bcc
Show file tree
Hide file tree
Showing 19 changed files with 306 additions and 342 deletions.
2 changes: 1 addition & 1 deletion packages/cli/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
"start:default": "cd bin && ./n8n",
"start:windows": "cd bin && n8n",
"test": "npm run test:sqlite",
"test:sqlite": "export N8N_LOG_LEVEL=silent; export DB_TYPE=sqlite; jest",
"test:sqlite": "export N8N_LOG_LEVEL=silent; export DB_TYPE=sqlite; jest --forceExit",
"test:postgres": "export N8N_LOG_LEVEL=silent; export DB_TYPE=postgresdb; jest",
"test:postgres:alt-schema": "export DB_POSTGRESDB_SCHEMA=alt_schema; npm run test:postgres",
"test:mysql": "export N8N_LOG_LEVEL=silent; export DB_TYPE=mysqldb; jest",
Expand Down
4 changes: 1 addition & 3 deletions packages/cli/src/Interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -178,8 +178,6 @@ export interface ICredentialsDecryptedResponse extends ICredentialsDecryptedDb {
export type DatabaseType = 'mariadb' | 'postgresdb' | 'mysqldb' | 'sqlite';
export type SaveExecutionDataType = 'all' | 'none';

export type ExecutionDataFieldFormat = 'empty' | 'flattened' | 'json';

export interface IExecutionBase {
id?: number | string;
mode: WorkflowExecuteMode;
Expand Down Expand Up @@ -240,7 +238,7 @@ export interface IExecutionResponseApi {
finished: boolean;
retryOf?: number | string;
retrySuccessId?: number | string;
data?: string; // Just that we can remove it
data?: object;
waitTill?: Date | null;
workflowData: IWorkflowBase;
}
Expand Down
52 changes: 29 additions & 23 deletions packages/cli/src/PublicApi/index.ts
Original file line number Diff line number Diff line change
@@ -1,38 +1,38 @@
/* eslint-disable @typescript-eslint/no-unused-vars */
/* eslint-disable no-restricted-syntax */
/* eslint-disable @typescript-eslint/naming-convention */
/* eslint-disable @typescript-eslint/no-unsafe-call */
/* eslint-disable import/no-cycle */
import express, { Router } from 'express';
import fs from 'fs/promises';
import path from 'path';

import * as OpenApiValidator from 'express-openapi-validator';
import { HttpError } from 'express-openapi-validator/dist/framework/types';
import fs from 'fs/promises';
import { OpenAPIV3 } from 'openapi-types';
import path from 'path';
import * as swaggerUi from 'swagger-ui-express';
import swaggerUi from 'swagger-ui-express';
import validator from 'validator';
import * as YAML from 'yamljs';
import { Db, InternalHooksManager } from '..';
import YAML from 'yamljs';

import config from '../../config';
import { Db, InternalHooksManager } from '..';
import { getInstanceBaseUrl } from '../UserManagement/UserManagementHelper';

function createApiRouter(
version: string,
openApiSpecPath: string,
hanldersDirectory: string,
handlersDirectory: string,
swaggerThemeCss: string,
publicApiEndpoint: string,
): Router {
const n8nPath = config.getEnv('path');
const swaggerDocument = YAML.load(openApiSpecPath) as swaggerUi.JsonObject;
// add the server depeding on the config so the user can interact with the API
// from the swagger UI
// from the Swagger UI
swaggerDocument.server = [
{
url: `${getInstanceBaseUrl()}/${publicApiEndpoint}/${version}}`,
},
];
const apiController = express.Router();

apiController.use(
`/${publicApiEndpoint}/${version}/docs`,
swaggerUi.serveFiles(swaggerDocument),
Expand All @@ -42,12 +42,14 @@ function createApiRouter(
customfavIcon: `${n8nPath}favicon.ico`,
}),
);

apiController.use(`/${publicApiEndpoint}/${version}`, express.json());

apiController.use(
`/${publicApiEndpoint}/${version}`,
OpenApiValidator.middleware({
apiSpec: openApiSpecPath,
operationHandlers: hanldersDirectory,
operationHandlers: handlersDirectory,
validateRequests: true,
validateApiSpec: true,
formats: [
Expand All @@ -71,16 +73,12 @@ function createApiRouter(
schema: OpenAPIV3.ApiKeySecurityScheme,
): Promise<boolean> => {
const apiKey = req.headers[schema.name.toLowerCase()];
const user = await Db.collections.User?.findOne({
where: {
apiKey,
},
const user = await Db.collections.User.findOne({
where: { apiKey },
relations: ['globalRole'],
});

if (!user) {
return false;
}
if (!user) return false;

void InternalHooksManager.getInstance().onUserInvokedApi({
user_id: user.id,
Expand All @@ -97,13 +95,20 @@ function createApiRouter(
},
}),
);

apiController.use(
(error: HttpError, req: express.Request, res: express.Response, next: express.NextFunction) => {
(
error: HttpError,
_req: express.Request,
res: express.Response,
_next: express.NextFunction,
) => {
return res.status(error.status || 400).json({
message: error.message,
});
},
);

return apiController;
}

Expand All @@ -114,11 +119,12 @@ export const loadPublicApiVersions = async (
const folders = await fs.readdir(__dirname);
const css = (await fs.readFile(swaggerThemePath)).toString();
const versions = folders.filter((folderName) => folderName.startsWith('v'));
const apiRouters: express.Router[] = [];
for (const version of versions) {

const apiRouters = versions.map((version) => {
const openApiPath = path.join(__dirname, version, 'openapi.yml');
apiRouters.push(createApiRouter(version, openApiPath, __dirname, css, publicApiEndpoint));
}
return createApiRouter(version, openApiPath, __dirname, css, publicApiEndpoint);
});

return {
apiRouters,
apiLatestVersion: Number(versions.pop()?.charAt(1)) ?? 1,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import express = require('express');
import express from 'express';

import { CredentialsHelper } from '../../../../CredentialsHelper';
import { CredentialTypes } from '../../../../CredentialTypes';

import { CredentialsEntity } from '../../../../databases/entities/CredentialsEntity';
import { CredentialRequest } from '../../../../requests';
import { CredentialTypeRequest } from '../../../types';
Expand Down Expand Up @@ -29,7 +29,7 @@ export = {
res: express.Response,
): Promise<express.Response<Partial<CredentialsEntity>>> => {
try {
const newCredential = await createCredential(req.body as Partial<CredentialsEntity>);
const newCredential = await createCredential(req.body);

const encryptedData = await encryptCredential(newCredential);

Expand All @@ -56,7 +56,7 @@ export = {
res: express.Response,
): Promise<express.Response<Partial<CredentialsEntity>>> => {
const { id: credentialId } = req.params;
let credentials: CredentialsEntity | undefined;
let credential: CredentialsEntity | undefined;

if (req.user.globalRole.name !== 'owner') {
const shared = await getSharedCredentials(req.user.id, credentialId, [
Expand All @@ -65,27 +65,20 @@ export = {
]);

if (shared?.role.name === 'owner') {
credentials = shared.credentials;
} else {
// LoggerProxy.info('Attempt to delete credential blocked due to lack of permissions', {
// credentialId,
// userId: req.user.id,
// });
credential = shared.credentials;
}
} else {
credentials = (await getCredentials(credentialId)) as CredentialsEntity;
credential = (await getCredentials(credentialId)) as CredentialsEntity;
}

if (!credentials) {
return res.status(404).json({
message: 'Not Found',
});
if (!credential) {
return res.status(404).json({ message: 'Not Found' });
}

await removeCredential(credentials);
credentials.id = Number(credentialId);
await removeCredential(credential);
credential.id = Number(credentialId);

return res.json(sanitizeCredentials(credentials));
return res.json(sanitizeCredentials(credential));
},
],

Expand All @@ -97,14 +90,12 @@ export = {
try {
CredentialTypes().getByName(credentialTypeName);
} catch (error) {
return res.status(404).json({
message: 'Not Found',
});
return res.status(404).json({ message: 'Not Found' });
}

let schema = new CredentialsHelper('').getCredentialsProperties(credentialTypeName);

schema = schema.filter((nodeProperty) => nodeProperty.type !== 'hidden');
const schema = new CredentialsHelper('')
.getCredentialsProperties(credentialTypeName)
.filter((property) => property.type !== 'hidden');

return res.json(toJsonSchema(schema));
},
Expand Down
Original file line number Diff line number Diff line change
@@ -1,37 +1,36 @@
/* eslint-disable @typescript-eslint/no-explicit-any */
/* eslint-disable consistent-return */
import { RequestHandler } from 'express';
/* eslint-disable @typescript-eslint/no-invalid-void-type */

import express from 'express';
import { validate } from 'jsonschema';

import { CredentialsHelper, CredentialTypes } from '../../../..';
import { CredentialRequest } from '../../../types';
import { toJsonSchema } from './credentials.service';

export const validCredentialType: RequestHandler = async (
export const validCredentialType = (
req: CredentialRequest.Create,
res,
next,
): Promise<any> => {
const { type } = req.body;
res: express.Response,
next: express.NextFunction,
): express.Response | void => {
try {
CredentialTypes().getByName(type);
} catch (error) {
return res.status(400).json({
message: 'req.body.type is not a known type',
});
CredentialTypes().getByName(req.body.type);
} catch (_) {
return res.status(400).json({ message: 'req.body.type is not a known type' });
}
next();

return next();
};

export const validCredentialsProperties: RequestHandler = async (
export const validCredentialsProperties = (
req: CredentialRequest.Create,
res,
next,
): Promise<any> => {
res: express.Response,
next: express.NextFunction,
): express.Response | void => {
const { type, data } = req.body;

let properties = new CredentialsHelper('').getCredentialsProperties(type);

properties = properties.filter((nodeProperty) => nodeProperty.type !== 'hidden');
const properties = new CredentialsHelper('')
.getCredentialsProperties(type)
.filter((property) => property.type !== 'hidden');

const schema = toJsonSchema(properties);

Expand All @@ -43,5 +42,5 @@ export const validCredentialsProperties: RequestHandler = async (
});
}

next();
return next();
};
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { SharedCredentials } from '../../../../databases/entities/SharedCredenti
import { User } from '../../../../databases/entities/User';
import { externalHooks } from '../../../../Server';
import { IDependency, IJsonSchema } from '../../../types';
import { CredentialRequest } from '../../../../requests';

export async function getCredentials(
credentialId: number | string,
Expand Down Expand Up @@ -38,7 +39,7 @@ export async function getSharedCredentials(
}

export async function createCredential(
properties: Partial<CredentialsEntity>,
properties: CredentialRequest.CredentialProperties,
): Promise<CredentialsEntity> {
const newCredential = new CredentialsEntity();

Expand Down
Loading

0 comments on commit b8e3bcc

Please sign in to comment.