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.

Sign up for GitHub

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

[typescript] add call-time middleware support #20430

Merged
merged 24 commits into from
Feb 26, 2025
Merged

[typescript] add call-time middleware support #20430

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
6328625
add call-time middleware
davidgamero Jan 9, 2025
9821963
update dependencies to run on current tsc
davidgamero Jan 9, 2025
852829d
exclude middleware from inversify
davidgamero Jan 9, 2025
126d7e6
update samples
davidgamero Jan 9, 2025
53c55a6
update service
davidgamero Jan 9, 2025
47df511
space
davidgamero Jan 9, 2025
4ecba42
inversify exclude
davidgamero Jan 9, 2025
58f001e
braces
davidgamero Jan 9, 2025
fc4fd1c
import in objectparamapi
davidgamero Jan 9, 2025
80b3629
add integration test for middleware
davidgamero Jan 10, 2025
87be984
switch to configuration options merge
davidgamero Jan 30, 2025
1407e0d
enable options on non inversify builds
davidgamero Jan 30, 2025
38b0008
remove unused middlware export
davidgamero Jan 30, 2025
cc898ee
remove merge strategy from inversify
davidgamero Jan 30, 2025
ab57038
gen samples
davidgamero Jan 30, 2025
9ff3ec6
remove old middlware imports
davidgamero Jan 30, 2025
c380cbd
tab to space
davidgamero Jan 30, 2025
84e79b8
separate promise and observable options
davidgamero Jan 30, 2025
df9cb12
gen samples
davidgamero Feb 1, 2025
788b22c
use allmiddleware var across inversify
davidgamero Feb 1, 2025
43874f5
generate encode samples again
davidgamero Feb 1, 2025
d717a85
add middleware call-order tests for default typescript build
davidgamero Feb 6, 2025
9fbf9f3
type refactor
davidgamero Feb 21, 2025
f1468a4
add semicolons, default replace, let to const
Feb 26, 2025
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
2 changes: 1 addition & 1 deletion modules/openapi-generator/src/main/resources/typescript/api/api.mustache
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ export class {{classname}}RequestFactory extends BaseAPIRequestFactory {
{{/authMethods}}

{{^useInversify}}
const defaultAuth: SecurityAuthentication | undefined = _options?.authMethods?.default || this.configuration?.authMethods?.default
const defaultAuth: SecurityAuthentication | undefined = _config?.authMethods?.default
if (defaultAuth?.applySecurityAuthentication) {
await defaultAuth?.applySecurityAuthentication(requestContext);
}
Expand Down
24 changes: 18 additions & 6 deletions modules/openapi-generator/src/main/resources/typescript/configuration.mustache
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,25 @@ import { JQueryHttpLibrary as DefaultHttpLibrary } from "./http/jquery";
import { BaseServerConfiguration, server1 } from "./servers{{importFileExtension}}";
import { configureAuthMethods, AuthMethods, AuthMethodsConfiguration } from "./auth/auth{{importFileExtension}}";

export interface Configuration {
readonly baseServer: BaseServerConfiguration;
readonly httpApi: HttpLibrary;
readonly middleware: Middleware[];
readonly authMethods: AuthMethods;
export interface Configuration<M = Middleware> {
readonly baseServer: BaseServerConfiguration;
readonly httpApi: HttpLibrary;
readonly middleware: M[];
readonly authMethods: AuthMethods;
}

// Additional option specific to middleware merge strategy
export interface MiddlewareMergeOptions {
// default is `'replace'` for backwards compatibility
middlewareMergeStrategy?: 'replace' | 'append' | 'prepend';
}

// Unify configuration options using Partial plus extra merge strategy
export type ConfigurationOptions<M = Middleware> = Partial<Configuration<M>> & MiddlewareMergeOptions;

// aliases for convenience
export type StandardConfigurationOptions = ConfigurationOptions<Middleware>;
export type PromiseConfigurationOptions = ConfigurationOptions<PromiseMiddleware>;

/**
* Interface with which a configuration object can be configured.
Expand Down Expand Up @@ -86,4 +98,4 @@ export function createConfiguration(conf: ConfigurationParameters = {}): Configu
);
}
return configuration;
}
}
5 changes: 3 additions & 2 deletions modules/openapi-generator/src/main/resources/typescript/index.mustache
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ export * from "./auth/auth{{importFileExtension}}";
export * from "./models/all{{importFileExtension}}";
{{/models.0}}
export { createConfiguration } from "./configuration{{importFileExtension}}"
export type { Configuration } from "./configuration{{importFileExtension}}"
export type { Configuration{{^useInversify}}, ConfigurationOptions, PromiseConfigurationOptions{{/useInversify}} } from "./configuration{{importFileExtension}}"
export * from "./apis/exception{{importFileExtension}}";
export * from "./servers{{importFileExtension}}";
export { RequiredError } from "./apis/baseapi{{importFileExtension}}";
Expand All @@ -13,7 +13,8 @@ export { RequiredError } from "./apis/baseapi{{importFileExtension}}";
export { Middleware } from './middleware{{importFileExtension}}';
{{/useRxJS}}
{{^useRxJS}}
export type { PromiseMiddleware as Middleware } from './middleware{{importFileExtension}}';
export type { PromiseMiddleware as Middleware, Middleware as ObservableMiddleware } from './middleware{{importFileExtension}}';
export { Observable } from './rxjsStub{{importFileExtension}}';
{{/useRxJS}}
{{#useObjectParameters}}
export { {{#apiInfo}}{{#apis}}{{#operations}}{{#operation}}type {{classname}}{{operationIdCamelCase}}Request, {{/operation}}Object{{classname}} as {{classname}}{{^-last}}, {{/-last}} {{/operations}}{{/apis}}{{/apiInfo}}} from './types/ObjectParamAPI{{importFileExtension}}';
Expand Down
6 changes: 3 additions & 3 deletions modules/openapi-generator/src/main/resources/typescript/services/ObservableAPI.mustache
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import type { HttpFile } from "../http/http";
import type { Observable } from {{#useRxJS}}"rxjs"{{/useRxJS}}{{^useRxJS}}"../rxjsStub"{{/useRxJS}};
import type { Configuration } from "../configuration";
import type { Configuration{{^useInversify}}Options{{/useInversify}} } from "../configuration";

{{#models}}
{{#model}}
Expand All @@ -14,10 +14,10 @@ import { {{{ classname }}} } from "{{{ importPath }}}";

export abstract class AbstractObservable{{classname}} {
{{#operation}}
public abstract {{nickname}}({{#allParams}}{{paramName}}{{^required}}?{{/required}}: {{{dataType}}}, {{/allParams}}options?: Configuration): Observable<{{{returnType}}}{{^returnType}}void{{/returnType}}>;
public abstract {{nickname}}({{#allParams}}{{paramName}}{{^required}}?{{/required}}: {{{dataType}}}, {{/allParams}}options?: Configuration{{^useInversify}}Options{{/useInversify}}): Observable<{{{returnType}}}{{^returnType}}void{{/returnType}}>;

{{/operation}}
}
{{/operations}}
{{/apis}}
{{/apiInfo}}
{{/apiInfo}}
9 changes: 6 additions & 3 deletions modules/openapi-generator/src/main/resources/typescript/types/ObjectParamAPI.mustache
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
import { ResponseContext, RequestContext, HttpFile, HttpInfo } from '../http/http{{importFileExtension}}';
import { Configuration} from '../configuration{{importFileExtension}}'
import { Configuration{{^useInversify}}, ConfigurationOptions{{/useInversify}} } from '../configuration{{importFileExtension}}'
{{^useInversify}}
import type { Middleware } from "../middleware";
{{/useInversify}}
{{#useRxJS}}
import { Observable } from 'rxjs';
{{/useRxJS}}
Expand Down Expand Up @@ -55,7 +58,7 @@ export class Object{{classname}} {
{{/summary}}
* @param param the request object
*/
public {{nickname}}WithHttpInfo(param: {{classname}}{{operationIdCamelCase}}Request{{^hasRequiredParams}} = {}{{/hasRequiredParams}}, options?: Configuration): {{#useRxJS}}Observable{{/useRxJS}}{{^useRxJS}}Promise{{/useRxJS}}<HttpInfo<{{{returnType}}}{{^returnType}}void{{/returnType}}>> {
public {{nickname}}WithHttpInfo(param: {{classname}}{{operationIdCamelCase}}Request{{^hasRequiredParams}} = {}{{/hasRequiredParams}}, options?: Configuration{{^useInversify}}Options{{/useInversify}}): {{#useRxJS}}Observable{{/useRxJS}}{{^useRxJS}}Promise{{/useRxJS}}<HttpInfo<{{{returnType}}}{{^returnType}}void{{/returnType}}>> {
return this.api.{{nickname}}WithHttpInfo({{#allParams}}param.{{paramName}}, {{/allParams}} options){{^useRxJS}}.toPromise(){{/useRxJS}};
}

Expand All @@ -68,7 +71,7 @@ export class Object{{classname}} {
{{/summary}}
* @param param the request object
*/
public {{nickname}}(param: {{classname}}{{operationIdCamelCase}}Request{{^hasRequiredParams}} = {}{{/hasRequiredParams}}, options?: Configuration): {{#useRxJS}}Observable{{/useRxJS}}{{^useRxJS}}Promise{{/useRxJS}}<{{{returnType}}}{{^returnType}}void{{/returnType}}> {
public {{nickname}}(param: {{classname}}{{operationIdCamelCase}}Request{{^hasRequiredParams}} = {}{{/hasRequiredParams}}, options?: Configuration{{^useInversify}}Options{{/useInversify}}): {{#useRxJS}}Observable{{/useRxJS}}{{^useRxJS}}Promise{{/useRxJS}}<{{{returnType}}}{{^returnType}}void{{/returnType}}> {
return this.api.{{nickname}}({{#allParams}}param.{{paramName}}, {{/allParams}} options){{^useRxJS}}.toPromise(){{/useRxJS}};
}

Expand Down
50 changes: 44 additions & 6 deletions modules/openapi-generator/src/main/resources/typescript/types/ObservableAPI.mustache
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { ResponseContext, RequestContext, HttpFile, HttpInfo } from '../http/http{{importFileExtension}}';
import { Configuration} from '../configuration{{importFileExtension}}'
import { Configuration{{^useInversify}}, ConfigurationOptions{{/useInversify}} } from '../configuration{{importFileExtension}}'
import type { Middleware } from "../middleware";
import { Observable, of, from } from {{#useRxJS}}'rxjs'{{/useRxJS}}{{^useRxJS}}'../rxjsStub{{importFileExtension}}'{{/useRxJS}};
import {mergeMap, map} from {{#useRxJS}}'rxjs/operators'{{/useRxJS}}{{^useRxJS}}'../rxjsStub{{importFileExtension}}'{{/useRxJS}};
{{#useInversify}}
Expand Down Expand Up @@ -61,19 +62,56 @@ export class Observable{{classname}} {
* @param {{#required}}{{paramName}}{{/required}}{{^required}}[{{paramName}}]{{/required}}{{#description}} {{description}}{{/description}}
{{/allParams}}
*/
public {{nickname}}WithHttpInfo({{#allParams}}{{paramName}}{{^required}}?{{/required}}: {{{dataType}}}, {{/allParams}}_options?: Configuration): Observable<HttpInfo<{{{returnType}}}{{^returnType}}void{{/returnType}}>> {
const requestContextPromise = this.requestFactory.{{nickname}}({{#allParams}}{{paramName}}, {{/allParams}}_options);
public {{nickname}}WithHttpInfo({{#allParams}}{{paramName}}{{^required}}?{{/required}}: {{{dataType}}}, {{/allParams}}_options?: Configuration{{^useInversify}}Options{{/useInversify}}): Observable<HttpInfo<{{{returnType}}}{{^returnType}}void{{/returnType}}>> {
let _config = this.configuration;
let allMiddleware: Middleware[] = [];
{{#useInversify}}
if (_options){
_config = _options;
}
allMiddleware = _config?.middleware;
{{/useInversify}}
{{^useInversify}}
if (_options && _options.middleware){
const middlewareMergeStrategy = _options.middlewareMergeStrategy || 'replace' // default to replace behavior
// call-time middleware provided
const calltimeMiddleware: Middleware[] = _options.middleware;

switch(middlewareMergeStrategy){
case 'append':
allMiddleware = this.configuration.middleware.concat(calltimeMiddleware);
break;
case 'prepend':
allMiddleware = calltimeMiddleware.concat(this.configuration.middleware)
break;
case 'replace':
allMiddleware = calltimeMiddleware
break;
default:
throw new Error(`unrecognized middleware merge strategy '${middlewareMergeStrategy}'`)
}
}
if (_options){
_config = {
baseServer: _options.baseServer || this.configuration.baseServer,
httpApi: _options.httpApi || this.configuration.httpApi,
authMethods: _options.authMethods || this.configuration.authMethods,
middleware: allMiddleware || this.configuration.middleware
};
}
{{/useInversify}}

const requestContextPromise = this.requestFactory.{{nickname}}({{#allParams}}{{paramName}}, {{/allParams}}{{#useInversify}}_options{{/useInversify}}{{^useInversify}}_config{{/useInversify}});
// build promise chain
let middlewarePreObservable = from<RequestContext>(requestContextPromise);
for (const middleware of this.configuration.middleware) {
for (const middleware of allMiddleware) {
middlewarePreObservable = middlewarePreObservable.pipe(mergeMap((ctx: RequestContext) => middleware.pre(ctx)));
}

return middlewarePreObservable.pipe(mergeMap((ctx: RequestContext) => this.configuration.httpApi.send(ctx))).
pipe(mergeMap((response: ResponseContext) => {
let middlewarePostObservable = of(response);
for (const middleware of this.configuration.middleware) {
for (const middleware of allMiddleware.reverse()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Am I missing code here where allMiddleware is a reversed list of the original before?

Thought: If I had two middlewares before this change that did something based on each other - for the sake of an example: one that parses JSON and one that transforms it - i.e. they couldn't be run in any order but one is dependent on the mutation of the other, then this change would mean that after an upgrade of my generated code it would now try to transform end then parse?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you are correct that the middleware was not a reversed list before, so your example is correct in that by upgrading this code the order of two post() middleware methods would be changed.

my interpretation of the current implementation is that it is not actually executing a middleware pattern

in your example, you'd have two middlewares: a ParseJSON and a TransformJSON each composed of a pre and post method. the presumed goal would be to allow the TransformJSON to wrap the ParseJSON such that it does not access non-json content

the current call order would be
ParseJSON.pre() => TransformJSON.post() => external response => ParseJSON.post() => TransformJSON.post()

since the middleware are being called in the same order while traversing down and up the call stack, there is no implementation that allows encapsulating something like a JSON Parse on both ends. at one side, the Transform is being called on non-parsed JSON. Reversing this list correctly implements the middleware pattern to allow each middleware to send requests to and receive responses from the next middleware. Unparsed JSON would have to reach the transform middleware on either the request or response calls since they're in the same order, assuming the pre and post methods are doing inverse steps of parsing/stringifying

the current _options parameter replaces the whole middleware array, so stacking middleware requires reconstructing the full configuration object, which is why i suspect this wasn't identified sooner.

this would change the order, so we can push that particular change to a later release if needed, but I understood this to be a bug

middlewarePostObservable = middlewarePostObservable.pipe(mergeMap((rsp: ResponseContext) => middleware.post(rsp)));
}
return middlewarePostObservable.pipe(map((rsp: ResponseContext) => this.responseProcessor.{{nickname}}WithHttpInfo(rsp)));
Expand All @@ -91,7 +129,7 @@ export class Observable{{classname}} {
* @param {{#required}}{{paramName}}{{/required}}{{^required}}[{{paramName}}]{{/required}}{{#description}} {{description}}{{/description}}
{{/allParams}}
*/
public {{nickname}}({{#allParams}}{{paramName}}{{^required}}?{{/required}}: {{{dataType}}}, {{/allParams}}_options?: Configuration): Observable<{{{returnType}}}{{^returnType}}void{{/returnType}}> {
public {{nickname}}({{#allParams}}{{paramName}}{{^required}}?{{/required}}: {{{dataType}}}, {{/allParams}}_options?: Configuration{{^useInversify}}Options{{/useInversify}}): Observable<{{{returnType}}}{{^returnType}}void{{/returnType}}> {
return this.{{nickname}}WithHttpInfo({{#allParams}}{{paramName}}, {{/allParams}}_options).pipe(map((apiResponse: HttpInfo<{{{returnType}}}{{^returnType}}void{{/returnType}}>) => apiResponse.data));
}

Expand Down
41 changes: 36 additions & 5 deletions modules/openapi-generator/src/main/resources/typescript/types/PromiseAPI.mustache
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
import { ResponseContext, RequestContext, HttpFile, HttpInfo } from '../http/http{{importFileExtension}}';
import { Configuration} from '../configuration{{importFileExtension}}'
import { Configuration{{^useInversify}}, ConfigurationOptions, PromiseConfigurationOptions{{/useInversify}} } from '../configuration{{importFileExtension}}'
{{^useInversify}}
import { PromiseMiddleware, Middleware, PromiseMiddlewareWrapper } from "../middleware";
{{/useInversify}}
{{#useInversify}}
import { injectable, inject, optional } from "inversify";
import { AbstractConfiguration } from "../services/configuration";
Expand Down Expand Up @@ -51,8 +54,22 @@ export class Promise{{classname}} {
* @param {{#required}}{{paramName}}{{/required}}{{^required}}[{{paramName}}]{{/required}}{{#description}} {{description}}{{/description}}
{{/allParams}}
*/
public {{nickname}}WithHttpInfo({{#allParams}}{{paramName}}{{^required}}?{{/required}}: {{{dataType}}}, {{/allParams}}_options?: Configuration): Promise<HttpInfo<{{{returnType}}}{{^returnType}}void{{/returnType}}>> {
const result = this.api.{{nickname}}WithHttpInfo({{#allParams}}{{paramName}}, {{/allParams}}_options);
public {{nickname}}WithHttpInfo({{#allParams}}{{paramName}}{{^required}}?{{/required}}: {{{dataType}}}, {{/allParams}}_options?: {{#useInversify}}Configuration{{/useInversify}}{{^useInversify}}PromiseConfigurationOptions{{/useInversify}}): Promise<HttpInfo<{{{returnType}}}{{^returnType}}void{{/returnType}}>> {
let observableOptions: undefined | Configuration{{^useInversify}}Options{{/useInversify}}{{#useInversify}} = _options{{/useInversify}}
{{^useInversify}}
if (_options){
observableOptions = {
baseServer: _options.baseServer,
httpApi: _options.httpApi,
middleware: _options.middleware?.map(
m => new PromiseMiddlewareWrapper(m)
),
middlewareMergeStrategy: _options.middlewareMergeStrategy,
authMethods: _options.authMethods
}
}
{{/useInversify}}
const result = this.api.{{nickname}}WithHttpInfo({{#allParams}}{{paramName}}, {{/allParams}}observableOptions);
return result.toPromise();
}

Expand All @@ -67,8 +84,22 @@ export class Promise{{classname}} {
* @param {{#required}}{{paramName}}{{/required}}{{^required}}[{{paramName}}]{{/required}}{{#description}} {{description}}{{/description}}
{{/allParams}}
*/
public {{nickname}}({{#allParams}}{{paramName}}{{^required}}?{{/required}}: {{{dataType}}}, {{/allParams}}_options?: Configuration): Promise<{{{returnType}}}{{^returnType}}void{{/returnType}}> {
const result = this.api.{{nickname}}({{#allParams}}{{paramName}}, {{/allParams}}_options);
public {{nickname}}({{#allParams}}{{paramName}}{{^required}}?{{/required}}: {{{dataType}}}, {{/allParams}}_options?: {{#useInversify}}Configuration{{/useInversify}}{{^useInversify}}PromiseConfigurationOptions{{/useInversify}}): Promise<{{{returnType}}}{{^returnType}}void{{/returnType}}> {
let observableOptions: undefined | Configuration{{^useInversify}}Options{{/useInversify}}{{#useInversify}} = _options{{/useInversify}}
{{^useInversify}}
if (_options){
observableOptions = {
baseServer: _options.baseServer,
httpApi: _options.httpApi,
middleware: _options.middleware?.map(
m => new PromiseMiddlewareWrapper(m)
),
middlewareMergeStrategy: _options.middlewareMergeStrategy,
authMethods: _options.authMethods
}
}
{{/useInversify}}
const result = this.api.{{nickname}}({{#allParams}}{{paramName}}, {{/allParams}}observableOptions);
return result.toPromise();
}

Expand Down
2 changes: 1 addition & 1 deletion samples/client/others/typescript/builds/array-of-lists/apis/DefaultApi.ts
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

24 changes: 18 additions & 6 deletions samples/client/others/typescript/builds/array-of-lists/configuration.ts
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 3 additions & 2 deletions samples/client/others/typescript/builds/array-of-lists/index.ts
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

43 changes: 43 additions & 0 deletions samples/client/others/typescript/builds/array-of-lists/package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading