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

chore: implement JSON Schema to NodeType adapter #1401

Merged
merged 12 commits into from
Feb 16, 2024
Prev Previous commit
Next Next commit
feat: resolve oneOfs in arrays items; update portal config and theme …
…config schemas according to the latest monorepo changes
  • Loading branch information
tatomyr committed Feb 16, 2024
commit a8e0c1e43d4bbec03a531ad183465f06ba58d22c
2 changes: 1 addition & 1 deletion __tests__/lint-config/config-structure/snapshot.js
Original file line number Diff line number Diff line change
Expand Up @@ -423,7 +423,7 @@ Error was generated by the configuration spec rule.

[30] .redocly.yaml:108:7 at #/developerOnboarding/adapters/0

Expected type \`rootRedoclyConfigSchema.developerOnboarding.adapters_0_items\` (object) but got \`string\`
Expected type \`APIGEE_X\` (object) but got \`string\`

106 | wrong: A not allowed field
107 | adapters:
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/__tests__/lint.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -979,7 +979,7 @@ describe('lint', () => {
"source": "",
},
],
"message": "Expected type \`rootRedoclyConfigSchema.developerOnboarding.adapters_0_items\` (object) but got \`string\`",
"message": "Expected type \`APIGEE_X\` (object) but got \`string\`",
"ruleId": "configuration spec",
"severity": "error",
"suggest": [],
Expand Down
14 changes: 13 additions & 1 deletion packages/core/src/resolve.ts
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,19 @@ export async function resolveDocument(opts: {
return;
}
for (let i = 0; i < node.length; i++) {
walk(node[i], itemsType || unknownType, joinPointer(nodeAbsoluteRef, i));
const itemType =
tatomyr marked this conversation as resolved.
Show resolved Hide resolved
typeof itemsType === 'function'
? itemsType(node[i], joinPointer(nodeAbsoluteRef, i))
: itemsType;
// we continue resolving unknown types, but stop early on known scalars
if (itemType === undefined && type !== unknownType && type !== SpecExtension) {
Copy link
Member

Choose a reason for hiding this comment

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

I think this if statement maybe be causing slower benchmark but looks like we need this code

continue;
}
walk(
node[i],
isNamedType(itemType) ? itemType : unknownType,
joinPointer(nodeAbsoluteRef, i)
);
}
return;
}
Expand Down
15 changes: 5 additions & 10 deletions packages/core/src/types/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export type NormalizedScalarSchema = {
export type NodeType = {
properties: Record<string, PropType | ResolveTypeFn>;
additionalProperties?: PropType | ResolveTypeFn;
items?: string;
items?: PropType | ResolveTypeFn;
required?: string[] | ((value: any, key: string | number | undefined) => string[]);
requiredOneOf?: string[];
allowed?: (value: any) => string[] | undefined;
Expand All @@ -34,18 +34,15 @@ export type NormalizedNodeType = {
name: string;
properties: Record<string, NormalizedPropType | NormalizedResolveTypeFn>;
additionalProperties?: NormalizedPropType | NormalizedResolveTypeFn;
items?: NormalizedNodeType;
items?: NormalizedPropType | NormalizedResolveTypeFn;
required?: string[] | ((value: any, key: string | number | undefined) => string[]);
requiredOneOf?: string[];
allowed?: (value: any) => string[] | undefined;
extensionsPrefix?: string;
};

type NormalizedPropType = NormalizedNodeType | NormalizedScalarSchema | undefined | null;
type NormalizedResolveTypeFn = (
value: any,
key: string
) => NormalizedNodeType | NormalizedScalarSchema | undefined | null;
type NormalizedPropType = NormalizedNodeType | NormalizedScalarSchema | null | undefined;
type NormalizedResolveTypeFn = (value: any, key: string) => NormalizedPropType;

export function listOf(typeName: string) {
return {
Expand Down Expand Up @@ -142,8 +139,6 @@ export function normalizeTypes(
}
}

export function isNamedType(
t: NormalizedNodeType | NormalizedScalarSchema | null | undefined
): t is NormalizedNodeType {
export function isNamedType(t: NormalizedPropType): t is NormalizedNodeType {
return typeof t?.name === 'string';
}
33 changes: 19 additions & 14 deletions packages/core/src/types/json-schema-adapter.ts
Copy link
Member

Choose a reason for hiding this comment

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

I did not review this file.

Original file line number Diff line number Diff line change
Expand Up @@ -84,22 +84,20 @@ const transformJSONSchemaToNodeType = (
throw new Error('Unexpected anyOf.');
}

if (isPlainObject(schema.items) && schema.items.oneOf) {
throw new Error('Unexpected oneOf in items.');
}

if (
isPlainObject(schema.properties) ||
isPlainObject(schema.additionalProperties) ||
(isPlainObject(schema.items) &&
(isPlainObject(schema.items.properties) || isPlainObject(schema.items.additionalProperties)))
(isPlainObject(schema.items.properties) ||
isPlainObject(schema.items.additionalProperties) ||
schema.items.oneOf)) // exclude scalar array types
) {
return extractNodeToContext(propertyName, schema, ctx);
}

if (schema.oneOf) {
if ((schema as Oas3Schema).discriminator) {
const discriminatedPropertyName = (schema as Oas3Schema).discriminator?.propertyName ;
const discriminatedPropertyName = (schema as Oas3Schema).discriminator?.propertyName;
if (!discriminatedPropertyName) {
throw new Error('Unexpected discriminator without a propertyName.');
}
Expand All @@ -116,10 +114,13 @@ const transformJSONSchemaToNodeType = (
});

return (value: any, key: string) => {
if (!isPlainObject(value)) {
return findOneOf(schema.oneOf as JSONSchema[], oneOfs)(value, key);
if (isPlainObject(value)) {
const discriminatedTypeName = value[discriminatedPropertyName];
if (typeof discriminatedTypeName === 'string' && ctx[discriminatedTypeName]) {
return discriminatedTypeName;
}
}
return value[discriminatedPropertyName] as PropType;
return findOneOf(schema.oneOf as JSONSchema[], oneOfs)(value, key);
};
} else {
const oneOfs = schema.oneOf.map((option, i) =>
Expand Down Expand Up @@ -173,10 +174,11 @@ const extractNodeToContext = (
let items;
if (
isPlainObject(schema.items) &&
(isPlainObject(schema.items.properties) || isPlainObject(schema.items.additionalProperties))
(isPlainObject(schema.items.properties) ||
isPlainObject(schema.items.additionalProperties) ||
schema.items.oneOf) // exclude scalar array types
) {
items = propertyName + '_items';
transformJSONSchemaToNodeType(propertyName + '_items', schema.items, ctx);
items = transformJSONSchemaToNodeType(propertyName + '_items', schema.items, ctx);
}

let required = schema.required as NodeType['required'];
Expand All @@ -203,8 +205,11 @@ const extractNodeToContext = (
return propertyName;
};

export const getNodeTypesFromJSONSchema = (schemaName: string, entrySchema: JSONSchema): Record<string, NodeType> => {
export const getNodeTypesFromJSONSchema = (
schemaName: string,
entrySchema: JSONSchema
): Record<string, NodeType> => {
const ctx: Record<string, NodeType> = {};
transformJSONSchemaToNodeType(schemaName, entrySchema, ctx);
return ctx;
}
};
28 changes: 7 additions & 21 deletions packages/core/src/types/portal-config-schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,11 @@ const oidcProviderConfigSchema = {
properties: {
type: { type: 'string', const: AuthProviderType.OIDC },
title: { type: 'string' },
pkce: { type: 'boolean', default: false },
configurationUrl: { type: 'string', minLength: 1 },
configuration: oidcIssuerMetadataSchema,
clientId: { type: 'string', minLength: 1 },
clientSecret: { type: 'string', minLength: 1 },
clientSecret: { type: 'string', minLength: 0 },
teamsClaimName: { type: 'string' },
teamsClaimMap: { type: 'object', additionalProperties: { type: 'string' } },
defaultTeams: { type: 'array', items: { type: 'string' } },
Expand All @@ -38,7 +39,7 @@ const oidcProviderConfigSchema = {
tokenRequestCustomParams: { type: 'object', additionalProperties: { type: 'string' } },
audience: { type: 'array', items: { type: 'string' } },
},
required: ['type', 'clientId', 'clientSecret'],
required: ['type', 'clientId'],
oneOf: [{ required: ['configurationUrl'] }, { required: ['configuration'] }],
additionalProperties: false,
} as const;
Expand Down Expand Up @@ -274,25 +275,9 @@ const devOnboardingConfigSchema = {
required: ['adapters'],
additionalProperties: false,
properties: {
// adapters: {
// type: 'array',
// items: devOnboardingAdapterConfigSchema,
// }, // TODO: figure out how to make oneOf work with arrays
adapters: {
oneOf: [
{
type: 'array',
items: apigeeXAdapterConfigSchema,
},
{
type: 'array',
items: apigeeEdgeAdapterConfigSchema,
},
{
type: 'array',
items: graviteeAdapterConfigSchema,
},
],
type: 'array',
items: devOnboardingAdapterConfigSchema,
},
},
} as const;
Expand Down Expand Up @@ -376,7 +361,7 @@ const redoclyConfigSchema = {
theme: themeConfigSchema,
},
default: { redirects: {} },
additionalProperties: false,
additionalProperties: true,
} as const;

const environmentSchema = {
Expand All @@ -399,6 +384,7 @@ export const rootRedoclyConfigSchema = {
},
default: {},
// required: ['redirects'], // FIXME: why redirects is required?
additionalProperties: false,
} as const;

export type RedoclyConfig<T = ThemeConfig> = FromSchema<typeof rootRedoclyConfigSchema> & {
Expand Down
31 changes: 30 additions & 1 deletion packages/core/src/types/theme-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,21 @@ const gtmAnalyticsConfigSchema = {
required: ['trackingId'],
} as const;

const productGoogleAnalyticsConfigSchema = {
type: 'object',
properties: {
includeInDevelopment: { type: 'boolean' },
trackingId: { type: 'string' },

conversionId: { type: 'string' },
floodlightId: { type: 'string' },
optimizeId: { type: 'string' },
exclude: { type: 'array', items: { type: 'string' } },
},
additionalProperties: false,
required: ['trackingId'],
} as const;

const googleAnalyticsConfigSchema = {
type: 'object',
properties: {
Expand All @@ -210,6 +225,12 @@ const googleAnalyticsConfigSchema = {
optimizeId: { type: 'string' },
anonymizeIp: { type: 'boolean' },
cookieExpires: { type: 'number' },

// All enabled tracking configs
trackers: {
type: 'object',
additionalProperties: productGoogleAnalyticsConfigSchema,
},
},
additionalProperties: false,
required: ['trackingId'],
Expand Down Expand Up @@ -698,6 +719,12 @@ export const productThemeOverrideSchema = {
search: themeConfigSchema.properties.search,
codeSnippet: themeConfigSchema.properties.codeSnippet,
breadcrumbs: themeConfigSchema.properties.breadcrumbs,
analytics: {
type: 'object',
properties: {
ga: productGoogleAnalyticsConfigSchema,
},
},
},
additionalProperties: true,
default: {},
Expand Down Expand Up @@ -730,11 +757,13 @@ export type ThemeConfig = FromSchema<typeof themeConfigSchema>;
// };

export type ProductConfig = FromSchema<typeof productConfigSchema>;

export type ProductGoogleAnalyticsConfig = FromSchema<typeof productGoogleAnalyticsConfigSchema>;
// TODO: cannot export as it relies on external types
// export type ProductThemeOverrideConfig = Pick<
// ThemeUIConfig,
// 'logo' | 'navbar' | 'footer' | 'sidebar' | 'search' | 'codeSnippet' | 'breadcrumbs'
// >;
// > & { analytics?: { ga?: ProductGoogleAnalyticsConfig } };
// export type ProductUiConfig = ProductConfig & {
// slug: string;
// link: string;
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/visitors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,7 @@ export function normalizeVisitors<T extends BaseVisitor>(
possibleChildren.add(from.additionalProperties);
}
}
if (from.items) {
if (from.items && typeof from.items !== 'function') {
if (from.items === to) {
addWeakFromStack(ruleConf, stack);
} else if (from.items.name !== undefined) {
Expand Down
8 changes: 7 additions & 1 deletion packages/core/src/walk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,13 @@ export function walkDocument<T extends BaseVisitor>(opts: {
const itemsType = type.items;
if (itemsType !== undefined) {
for (let i = 0; i < resolvedNode.length; i++) {
walkNode(resolvedNode[i], itemsType, resolvedLocation.child([i]), resolvedNode, i);
const itemType =
typeof itemsType === 'function'
? itemsType(resolvedNode[i], resolvedLocation.child([i]).absolutePointer)
: itemsType;
if (isNamedType(itemType)) {
walkNode(resolvedNode[i], itemType, resolvedLocation.child([i]), resolvedNode, i);
}
}
}
} else if (typeof resolvedNode === 'object' && resolvedNode !== null) {
Expand Down