Skip to content

Commit

Permalink
Fix: freezing arrays returned by public methods (#1736)
Browse files Browse the repository at this point in the history
This is a fix that prevents changing arrays after they have been
assigned upon building an Endpoint, however, it could be potentially
breaking, therefore I'm addressing it to v19.
  • Loading branch information
RobinTail authored May 7, 2024
1 parent ce3b2c1 commit 2e0c2e1
Show file tree
Hide file tree
Showing 7 changed files with 63 additions and 41 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@
- `zod`: 3.23.0.
- The deprecated ~~`withMeta()`~~ is removed:
- See the changes to [v18.5.0](#v1850) on details.
- Several public methods and properties exposing arrays from class instances made readonly and frozen:
- On `Endpoint`: `.getMethods()`, `.getMimeTypes()`, `.getResponses()`, `.getScopes()`, `.getTags()`,
- On `DependsOnMethod`: `.pairs`, `.siblingMethods`.

## Version 18

Expand Down
16 changes: 10 additions & 6 deletions src/depends-on-method.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,20 @@ import { AbstractEndpoint } from "./endpoint";
import { Method } from "./method";

export class DependsOnMethod {
public readonly pairs: [Method, AbstractEndpoint][];
public readonly pairs: ReadonlyArray<[Method, AbstractEndpoint]>;
public readonly firstEndpoint: AbstractEndpoint | undefined;
public readonly siblingMethods: Method[];
public readonly siblingMethods: ReadonlyArray<Method>;

constructor(endpoints: Partial<Record<Method, AbstractEndpoint>>) {
this.pairs = toPairs(endpoints).filter(
(pair): pair is [Method, AbstractEndpoint] =>
pair !== undefined && pair[1] !== undefined,
this.pairs = Object.freeze(
toPairs(endpoints).filter(
(pair): pair is [Method, AbstractEndpoint] =>
pair !== undefined && pair[1] !== undefined,
),
);
this.firstEndpoint = head(this.pairs)?.[1];
this.siblingMethods = tail(this.pairs).map(([method]) => method);
this.siblingMethods = Object.freeze(
tail(this.pairs).map(([method]) => method),
);
}
}
2 changes: 1 addition & 1 deletion src/documentation-helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ interface ReqResDepictHelperCommonProps
"serializer" | "getRef" | "makeRef" | "path" | "method"
> {
schema: z.ZodTypeAny;
mimeTypes: string[];
mimeTypes: ReadonlyArray<string>;
composition: "inline" | "components";
description?: string;
}
Expand Down
2 changes: 1 addition & 1 deletion src/documentation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ export class Documentation extends OpenApiBuilder {
const scopes = ["oauth2", "openIdConnect"].includes(
securitySchema.type,
)
? endpoint.getScopes()
? endpoint.getScopes().slice()
: [];
this.addSecurityScheme(name, securitySchema);
return { name, scopes };
Expand Down
77 changes: 46 additions & 31 deletions src/endpoint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,19 +47,21 @@ export abstract class AbstractEndpoint {
response: Response;
logger: AbstractLogger;
config: CommonConfig;
siblingMethods?: Method[];
siblingMethods?: ReadonlyArray<Method>;
}): Promise<void>;
public abstract getDescription(
variant: DescriptionVariant,
): string | undefined;
public abstract getMethods(): Method[];
public abstract getMethods(): ReadonlyArray<Method>;
public abstract getSchema(variant: IOVariant): IOSchema;
public abstract getSchema(variant: ResponseVariant): z.ZodTypeAny;
public abstract getMimeTypes(variant: MimeVariant): string[];
public abstract getResponses(variant: ResponseVariant): NormalizedResponse[];
public abstract getMimeTypes(variant: MimeVariant): ReadonlyArray<string>;
public abstract getResponses(
variant: ResponseVariant,
): ReadonlyArray<NormalizedResponse>;
public abstract getSecurity(): LogicalContainer<Security>;
public abstract getScopes(): string[];
public abstract getTags(): string[];
public abstract getScopes(): ReadonlyArray<string>;
public abstract getTags(): ReadonlyArray<string>;
public abstract getOperationId(method: Method): string | undefined;
}

Expand All @@ -71,15 +73,18 @@ export class Endpoint<
TAG extends string,
> extends AbstractEndpoint {
readonly #descriptions: Record<DescriptionVariant, string | undefined>;
readonly #methods: Method[];
readonly #methods: ReadonlyArray<Method>;
readonly #middlewares: AnyMiddlewareDef[];
readonly #mimeTypes: Record<MimeVariant, string[]>;
readonly #responses: Record<ResponseVariant, NormalizedResponse[]>;
readonly #mimeTypes: Record<MimeVariant, ReadonlyArray<string>>;
readonly #responses: Record<
ResponseVariant,
ReadonlyArray<NormalizedResponse>
>;
readonly #handler: Handler<z.output<IN>, z.input<OUT>, OPT>;
readonly #resultHandler: AnyResultHandlerDefinition;
readonly #schemas: { input: IN; output: OUT };
readonly #scopes: SCO[];
readonly #tags: TAG[];
readonly #scopes: ReadonlyArray<SCO>;
readonly #tags: ReadonlyArray<TAG>;
readonly #getOperationId: (method: Method) => string | undefined;

constructor({
Expand Down Expand Up @@ -112,9 +117,9 @@ export class Endpoint<
this.#resultHandler = resultHandler;
this.#middlewares = middlewares;
this.#getOperationId = getOperationId;
this.#methods = methods;
this.#scopes = scopes;
this.#tags = tags;
this.#methods = Object.freeze(methods);
this.#scopes = Object.freeze(scopes);
this.#tags = Object.freeze(tags);
this.#descriptions = { long, short };
this.#schemas = { input: inputSchema, output: outputSchema };
for (const [variant, schema] of Object.entries(this.#schemas)) {
Expand All @@ -126,14 +131,18 @@ export class Endpoint<
);
}
this.#responses = {
positive: normalizeApiResponse(
resultHandler.getPositiveResponse(outputSchema),
{ mimeTypes: [mimeJson], statusCodes: [defaultStatusCodes.positive] },
positive: Object.freeze(
normalizeApiResponse(resultHandler.getPositiveResponse(outputSchema), {
mimeTypes: [mimeJson],
statusCodes: [defaultStatusCodes.positive],
}),
),
negative: Object.freeze(
normalizeApiResponse(resultHandler.getNegativeResponse(), {
mimeTypes: [mimeJson],
statusCodes: [defaultStatusCodes.negative],
}),
),
negative: normalizeApiResponse(resultHandler.getNegativeResponse(), {
mimeTypes: [mimeJson],
statusCodes: [defaultStatusCodes.negative],
}),
};
for (const [variant, responses] of Object.entries(this.#responses)) {
assert(
Expand All @@ -144,21 +153,27 @@ export class Endpoint<
);
}
this.#mimeTypes = {
input: hasUpload(inputSchema)
? [mimeMultipart]
: hasRaw(inputSchema)
? [mimeRaw]
: [mimeJson],
positive: this.#responses.positive.flatMap(({ mimeTypes }) => mimeTypes),
negative: this.#responses.negative.flatMap(({ mimeTypes }) => mimeTypes),
input: Object.freeze(
hasUpload(inputSchema)
? [mimeMultipart]
: hasRaw(inputSchema)
? [mimeRaw]
: [mimeJson],
),
positive: Object.freeze(
this.#responses.positive.flatMap(({ mimeTypes }) => mimeTypes),
),
negative: Object.freeze(
this.#responses.negative.flatMap(({ mimeTypes }) => mimeTypes),
),
};
}

public override getDescription(variant: DescriptionVariant) {
return this.#descriptions[variant];
}

public override getMethods(): Method[] {
public override getMethods() {
return this.#methods;
}

Expand Down Expand Up @@ -190,11 +205,11 @@ export class Endpoint<
);
}

public override getScopes(): SCO[] {
public override getScopes() {
return this.#scopes;
}

public override getTags(): TAG[] {
public override getTags() {
return this.#tags;
}

Expand Down
2 changes: 1 addition & 1 deletion src/integration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ export class Integration {
{ method: Method; path: string },
Partial<Record<IOKind, string>> & {
isJson: boolean;
tags: string[];
tags: ReadonlyArray<string>;
}
>();
protected paths: string[] = [];
Expand Down
2 changes: 1 addition & 1 deletion src/routing-walker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ export interface RoutingWalkerParams {
endpoint: AbstractEndpoint,
path: string,
method: Method | AuxMethod,
siblingMethods?: Method[],
siblingMethods?: ReadonlyArray<Method>,
) => void;
onStatic?: (path: string, handler: StaticHandler) => void;
parentPath?: string;
Expand Down

0 comments on commit 2e0c2e1

Please sign in to comment.