-
Notifications
You must be signed in to change notification settings - Fork 284
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
feat(core): add configureExpressAppBase() utility function
1. The idea here is to re-use the common basic tasks of configuring an express instance similar to how the API server does it but without having the chicken-egg problem of circular dependencies between the API server and the plugins. 2. More detailed discussion can be seen in this other pull request in the comments: https://github.com/hyperledger/cacti/pull/3169 Signed-off-by: Peter Somogyvari <[email protected]>
- Loading branch information
Showing
6 changed files
with
132 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
91 changes: 91 additions & 0 deletions
91
packages/cactus-core/src/main/typescript/web-services/configure-express-app-base.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,91 @@ | ||
import type { Express } from "express"; | ||
import bodyParser, { OptionsJson } from "body-parser"; | ||
|
||
import { | ||
Checks, | ||
LogLevelDesc, | ||
LoggerProvider, | ||
} from "@hyperledger/cactus-common"; | ||
|
||
import { stringifyBigIntReplacer } from "./stringify-big-int-replacer"; | ||
|
||
export const CACTI_CORE_CONFIGURE_EXPRESS_APP_BASE_MARKER = | ||
"CACTI_CORE_CONFIGURE_EXPRESS_APP_BASE_MARKER"; | ||
|
||
/** | ||
* Implementations of this interface are objects who represent a valid execution | ||
* context for the `configureExpressAppBase()` utility function. | ||
* | ||
* @see {configureExpressAppBase} | ||
* @see {ApiServer} | ||
*/ | ||
export interface IConfigureExpressAppContext { | ||
readonly logLevel?: LogLevelDesc; | ||
readonly app: Express; | ||
readonly bodyParserJsonOpts?: OptionsJson; | ||
} | ||
|
||
/** | ||
* Configures the base functionalities for an Express.js application. | ||
* | ||
* The main purpose of this function is to have a reusable implementation | ||
* of the base setup logic that the API server does. For test cases of | ||
* plugins we can't directly import the API server because it would cause | ||
* circular dependencies in the mono-repo that usually ends up causing mayhem | ||
* with the build in general and also with the architecture longer term. | ||
* | ||
* So, with this function being here in the core package it makes it easy | ||
* to reuse by both plugin test cases and also the API server itself. | ||
* | ||
* The logic here is kept very small because the order of the ExpressJS | ||
* middleware handler's matters a lot and if you mix up the order then | ||
* new bugs can appear. | ||
* | ||
* @param ctx The context object holding information about the log level | ||
* that the caller wants and the ExpressJS instance itself which is to be | ||
* configured. | ||
* | ||
* @throws {Error} If any of the required context properties are missing. | ||
*/ | ||
export async function configureExpressAppBase( | ||
ctx: IConfigureExpressAppContext, | ||
): Promise<void> { | ||
const fn = "configureExpressAppBase()"; | ||
Checks.truthy(ctx, `${fn} arg1 ctx`); | ||
Checks.truthy(ctx.app, `${fn} arg1 ctx.app`); | ||
Checks.truthy(ctx.app.use, `${fn} arg1 ctx.app.use`); | ||
|
||
const logLevel: LogLevelDesc = ctx.logLevel || "WARN"; | ||
|
||
const log = LoggerProvider.getOrCreate({ | ||
level: logLevel, | ||
label: fn, | ||
}); | ||
|
||
log.debug("ENTRY"); | ||
|
||
const didRun = ctx.app.get(CACTI_CORE_CONFIGURE_EXPRESS_APP_BASE_MARKER); | ||
if (didRun) { | ||
const duplicateConfigurationAttemptErrorMsg = | ||
`Already configured this express instance before. Check the ` + | ||
`configuration variable of the ExpressJS instance under the key ` + | ||
`"CACTI_CORE_CONFIGURE_EXPRESS_APP_BASE_MARKER" to determine if an ` + | ||
`instance has already been `; | ||
throw new Error(duplicateConfigurationAttemptErrorMsg); | ||
} | ||
|
||
const bodyParserJsonOpts: OptionsJson = ctx.bodyParserJsonOpts || { | ||
limit: "50mb", | ||
}; | ||
log.debug("body-parser middleware opts: %o", bodyParserJsonOpts); | ||
|
||
const bodyParserMiddleware = bodyParser.json(bodyParserJsonOpts); | ||
ctx.app.use(bodyParserMiddleware); | ||
|
||
// Add custom replacer to handle bigint responses correctly | ||
ctx.app.set("json replacer", stringifyBigIntReplacer); | ||
|
||
ctx.app.set(CACTI_CORE_CONFIGURE_EXPRESS_APP_BASE_MARKER, true); | ||
|
||
log.debug("EXIT"); | ||
} |
13 changes: 13 additions & 0 deletions
13
packages/cactus-core/src/main/typescript/web-services/stringify-big-int-replacer.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
/** | ||
* `JSON.stringify` replacer function to handle BigInt. | ||
* See https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/BigInt#use_within_json | ||
*/ | ||
export function stringifyBigIntReplacer( | ||
_key: string, | ||
value: bigint | unknown, | ||
): string | unknown { | ||
if (typeof value === "bigint") { | ||
return value.toString(); | ||
} | ||
return value; | ||
} |
18 changes: 18 additions & 0 deletions
18
...ages/cactus-core/src/test/typescript/unit/web-services/configure-express-app-base.test.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
import "jest-extended"; | ||
import express from "express"; | ||
|
||
import { configureExpressAppBase } from "../../../../main/typescript/public-api"; | ||
|
||
describe("configureExpressAppBase()", () => { | ||
test("Crashes if missing Express instance from ctx", async () => { | ||
const invocationPromise = configureExpressAppBase({} as never); | ||
await expect(invocationPromise).toReject(); | ||
}); | ||
|
||
test("Does not crash if parameters were valid", async () => { | ||
const app = express(); | ||
await expect( | ||
async () => await configureExpressAppBase({ app, logLevel: "DEBUG" }), | ||
).not.toThrow(); | ||
}); | ||
}); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
383f852
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Possible performance regression was detected for benchmark.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold
0.05
.cmd-api-server_HTTP_GET_getOpenApiSpecV1
583
ops/sec (±1.69%
)579
ops/sec (±1.69%
)0.99
cmd-api-server_gRPC_GetOpenApiSpecV1
364
ops/sec (±1.26%
)359
ops/sec (±1.44%
)0.99
This comment was automatically generated by workflow using github-action-benchmark.
CC: @petermetz