Skip to content

Commit 18f2567

Browse files
pshao25Pan Shao
pshao25
and
Pan Shao
authored
Add definition check to tsmv report (#36483)
* Add definition check to tsmv * update * update * update --------- Co-authored-by: Pan Shao <[email protected]>
1 parent 5bbb162 commit 18f2567

File tree

3 files changed

+298
-14
lines changed

3 files changed

+298
-14
lines changed

‎eng/tools/typespec-migration-validation/src/commonType.ts

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
import path from 'path';
2+
import { readFileContent } from './helper.js';
3+
import { OpenAPI2Document, OpenAPI2Schema } from '@azure-tools/typespec-autorest';
4+
import { isRef } from './compare.js';
5+
6+
export function getCommonTypeDefinition(refPath: string): [OpenAPI2Schema, OpenAPI2Document] | undefined {
7+
const commonTypeInfo = getCommonTypeInfo(refPath);
8+
if (!commonTypeInfo) {
9+
return undefined;
10+
}
11+
const [filePath, definitionName] = commonTypeInfo;
12+
const fileContent = readCommonTypeFile(filePath);
13+
const commonTypeDefinition = fileContent.definitions?.[definitionName];
14+
if (!commonTypeDefinition) {
15+
return undefined;
16+
}
17+
return [resolveCommonTypeDefinition(commonTypeDefinition, fileContent), fileContent];
18+
}
19+
20+
// For all the properties, we need to inline its type if is a reference to another common type, because we cannot resolve it outside the common type file
21+
function resolveCommonTypeDefinition(commonTypeDefinition: OpenAPI2Schema, fileContent: OpenAPI2Document): OpenAPI2Schema {
22+
for (const property in commonTypeDefinition.properties) {
23+
const propertySchema = commonTypeDefinition.properties[property];
24+
if (isRef(propertySchema)) {
25+
const refPath = propertySchema.$ref;
26+
if (!refPath.startsWith('#/definitions/')) {
27+
console.warn(`Reference ${propertySchema.$ref} is not a local definition, skipping.`);
28+
continue;
29+
}
30+
const refDefinitionName = refPath.replace('#/definitions/', '');
31+
const refDefinitionSchema = fileContent.definitions?.[refDefinitionName];
32+
commonTypeDefinition.properties[property] = refDefinitionSchema ? resolveCommonTypeDefinition(refDefinitionSchema, fileContent) : propertySchema;
33+
}
34+
}
35+
return commonTypeDefinition;
36+
}
37+
38+
function getCommonTypeInfo(refPath: string): [string, string] | undefined {
39+
const matchResult = refPath.match(/\/common-types\/resource-management\/(v\d)\/(.*)#\/definitions\/(.*)/);
40+
if (!matchResult) {
41+
return undefined;
42+
}
43+
const [, version, fileName, definitionName] = matchResult;
44+
return [path.join(process.cwd(), '..', '..', '..', 'specification', 'common-types', 'resource-management', version, fileName), definitionName];
45+
}
46+
47+
const commonTypeFileCache: Record<string, OpenAPI2Document> = {};
48+
function readCommonTypeFile(filePath: string): OpenAPI2Document {
49+
if (commonTypeFileCache[filePath]) {
50+
return commonTypeFileCache[filePath];
51+
}
52+
const fileContent = readFileContent(filePath);
53+
const jsonContent: OpenAPI2Document = JSON.parse(fileContent);
54+
commonTypeFileCache[filePath] = jsonContent;
55+
return jsonContent;
56+
}

‎eng/tools/typespec-migration-validation/src/compare.ts

Lines changed: 240 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
import { HttpMethod, OpenAPI2Document, OpenAPI2Operation, OpenAPI2Response, Refable } from "@azure-tools/typespec-autorest";
1+
import { HttpMethod, OpenAPI2Document, OpenAPI2Operation, OpenAPI2Response, OpenAPI2Schema, OpenAPI2SchemaProperty, Ref, Refable } from "@azure-tools/typespec-autorest";
2+
import { getCommonTypeDefinition } from "./commonType.js";
23

34
interface Diff {
45
before: any;
@@ -21,16 +22,188 @@ interface PathDiff extends Diff {
2122
type: "path" | "parameters" | "pageable" | "longrunning" | "finalstate" | "finalresult" | "responses" | "response";
2223
}
2324

24-
export function printPathDiff(diff: PathDiff): string {
25-
return `| ${diff.type} | ${diff.level} | The ${diff.type} of operation "${diff.operationId}" changed: ${JSON.stringify(diff.before)} -> ${JSON.stringify(diff.after)} |\n`;
25+
/**
26+
* properties: the property names of the definition should be the same
27+
* property: the property should have same schema
28+
* required: the required properties of the definition should be the same
29+
*/
30+
interface DefinitionDiff extends Diff {
31+
name: string;
32+
propertyName?: string;
33+
changeType?: string;
34+
type: "properties" | "property" | "required";
35+
}
36+
37+
export function printDiff(diff: PathDiff | DefinitionDiff): string {
38+
if ("operationId" in diff) {
39+
return `| ${diff.type} | ${diff.level} | ${getPathDiffMessage(diff)} ${JSON.stringify(diff.before)} -> ${JSON.stringify(diff.after)} |\n`;
40+
} else {
41+
return `| ${diff.type} | ${diff.level} | ${getDefinitionDiffMessage(diff)} ${JSON.stringify(diff.before)} -> ${JSON.stringify(diff.after)} |\n`;
42+
}
2643
}
2744

28-
export function compareDocuments(oldDocument: OpenAPI2Document, newDocument: OpenAPI2Document) {
29-
const diffs: PathDiff[] = [];
45+
function getPathDiffMessage(diff: PathDiff): string {
46+
switch (diff.type) {
47+
case "path":
48+
return `The path for operation "${diff.operationId}" changed:`;
49+
case "parameters":
50+
return `The number of parameters for operation "${diff.operationId}" changed:`;
51+
case "pageable":
52+
return `The pageable for operation "${diff.operationId}" changed:`;
53+
case "longrunning":
54+
return `The long-running status for operation "${diff.operationId}" changed:`;
55+
case "finalstate":
56+
return `The final state for operation "${diff.operationId}" changed:`;
57+
case "finalresult":
58+
return `The final result schema for operation "${diff.operationId}" changed:`;
59+
case "responses":
60+
return `The response codes for operation "${diff.operationId}" changed:`;
61+
case "response":
62+
return `The response schema for operation "${diff.operationId}" changed:`;
63+
default:
64+
return `The ${diff.type} for operation "${diff.operationId}" changed:`;
65+
}
66+
}
67+
68+
function getDefinitionDiffMessage(diff: DefinitionDiff): string {
69+
switch (diff.type) {
70+
case "properties":
71+
return `The property names of definition "${diff.name}" changed:`;
72+
case "property":
73+
return `The ${diff.changeType} of property "${diff.propertyName}" in definition "${diff.name}" changed:`;
74+
case "required":
75+
return `The required properties of definition "${diff.name}" changed:`;
76+
}
77+
}
78+
79+
export function compareDocuments(oldDocument: OpenAPI2Document, newDocument: OpenAPI2Document): (PathDiff | DefinitionDiff)[] {
80+
const diffs: (PathDiff | DefinitionDiff)[] = [];
3081
diffs.push(...comparePaths(oldDocument, newDocument));
82+
83+
// If not exists in the new document, it might be an orphaned definition previously
84+
for (const definition in newDocument.definitions ?? {}) {
85+
// If not exists in old document, it might be an anonymous definition previously
86+
if (oldDocument.definitions?.[definition]) {
87+
diffs.push(...compareNamedDefinition(oldDocument.definitions[definition], oldDocument, newDocument.definitions![definition], newDocument, definition));
88+
}
89+
}
90+
return diffs;
91+
}
92+
93+
const definitionNameCache: Set<string> = new Set();
94+
function compareNamedDefinition(oldDefinition: OpenAPI2Schema, oldDocument: OpenAPI2Document, newDefinition: OpenAPI2Schema, newDocument: OpenAPI2Document, name: string): DefinitionDiff[] {
95+
if (definitionNameCache.has(name)) {
96+
console.warn(`Definition "${name}" has been compared before, skipping.`);
97+
return [];
98+
}
99+
definitionNameCache.add(name);
100+
101+
const diffs: DefinitionDiff[] = [];
102+
103+
const oldRequired = oldDefinition.required || [];
104+
const newRequired = newDefinition.required || [];
105+
106+
if (oldRequired.length !== newRequired.length ||
107+
!oldRequired.every(item => newRequired.includes(item)) ||
108+
!newRequired.every(item => oldRequired.includes(item))) {
109+
diffs.push({
110+
before: oldRequired,
111+
after: newRequired,
112+
name,
113+
type: "required",
114+
level: "warning",
115+
});
116+
}
117+
118+
const oldProperties = getAllProperties(oldDefinition, oldDocument);
119+
const sortedOldProperties = Object.keys(oldProperties).sort().reduce((obj: Record<string, OpenAPI2SchemaProperty>, key) => {
120+
obj[key] = oldProperties[key];
121+
return obj;
122+
}, {});
123+
124+
const newProperties = getAllProperties(newDefinition, newDocument);
125+
const sortedNewProperties = Object.keys(newProperties).sort().reduce((obj: Record<string, OpenAPI2SchemaProperty>, key) => {
126+
obj[key] = newProperties[key];
127+
return obj;
128+
}, {});
129+
130+
// First check if the properties are the same
131+
const oldKeys = Object.keys(sortedOldProperties);
132+
const newKeys = Object.keys(sortedNewProperties);
133+
if (oldKeys.length !== newKeys.length) {
134+
// Check if newKeys has exactly one more key and it's systemData
135+
const isSystemDataOnlyDifference = newKeys.length === oldKeys.length + 1 &&
136+
newKeys.filter(key => !oldKeys.includes(key)).length === 1 &&
137+
newKeys.filter(key => !oldKeys.includes(key))[0] === 'systemData';
138+
139+
diffs.push({
140+
before: oldKeys,
141+
after: newKeys,
142+
name,
143+
type: "properties",
144+
level: isSystemDataOnlyDifference ? "warning" : "error"
145+
});
146+
}
147+
for (const key of oldKeys) {
148+
if (!newKeys.includes(key)) {
149+
diffs.push({
150+
before: oldKeys,
151+
after: newKeys,
152+
name,
153+
type: "properties",
154+
level: "error"
155+
});
156+
}
157+
}
158+
for (const key of newKeys) {
159+
if (!oldKeys.includes(key)) {
160+
// Check if the only additional key is systemData
161+
const additionalKeys = newKeys.filter(k => !oldKeys.includes(k));
162+
const isOnlySystemData = additionalKeys.length === 1 && additionalKeys[0] === 'systemData';
163+
164+
diffs.push({
165+
before: oldKeys,
166+
after: newKeys,
167+
name,
168+
type: "properties",
169+
level: isOnlySystemData ? "warning" : "error"
170+
});
171+
}
172+
}
173+
174+
// Then check if the property types are the same
175+
for (const key of oldKeys) {
176+
const oldProperty = sortedOldProperties[key];
177+
const newProperty = sortedNewProperties[key];
178+
diffs.push(...compareDefinitionProperty(oldProperty, newProperty, oldDocument, newDocument, key, name));
179+
}
180+
31181
return diffs;
32182
}
33183

184+
function compareDefinitionProperty(oldProperty: OpenAPI2SchemaProperty, newProperty: OpenAPI2SchemaProperty, oldDocument: OpenAPI2Document, newDocument: OpenAPI2Document, propertyName: string, modelName: string): DefinitionDiff[] {
185+
const oldPropertySchema = resolveCommonType(oldProperty);
186+
const newPropertySchema = resolveCommonType(newProperty);
187+
if (isRef(oldPropertySchema) && isRef(newPropertySchema)) {
188+
if (oldPropertySchema.$ref !== newPropertySchema.$ref) {
189+
return [{
190+
before: oldPropertySchema.$ref,
191+
after: newPropertySchema.$ref,
192+
name: modelName,
193+
propertyName,
194+
type: "property",
195+
changeType: "reference",
196+
level: "warning"
197+
}];
198+
}
199+
}
200+
else if (!isRef(oldPropertySchema) && !isRef(newPropertySchema)) {
201+
return compareNamedDefinition(oldPropertySchema, oldDocument, newPropertySchema, newDocument, `${modelName}.${propertyName}`);
202+
}
203+
204+
return [];
205+
}
206+
34207
function comparePaths(oldDocument: OpenAPI2Document, newDocument: OpenAPI2Document): PathDiff[] {
35208
const oldOperations = organizeOperationById(oldDocument);
36209
const newOperations = organizeOperationById(newDocument);
@@ -48,7 +221,7 @@ function comparePaths(oldDocument: OpenAPI2Document, newDocument: OpenAPI2Docume
48221
}
49222
else {
50223
pathDiffs.push(...compareOperation(oldOperations[operationId][1], newOperations[operationId][1], operationId));
51-
}
224+
}
52225
}
53226
for (const operationId in newOperations) {
54227
if (!oldOperations[operationId]) {
@@ -191,7 +364,7 @@ function compareLongRunning(oldOperation: OpenAPI2Operation, newOperation: OpenA
191364
level: "error"
192365
});
193366
}
194-
}
367+
}
195368

196369
return pathDiffs;
197370
}
@@ -204,7 +377,7 @@ function getResponseSchema(response: Refable<OpenAPI2Response> | undefined): str
204377

205378
return undefined;
206379
}
207-
380+
208381

209382
function organizeOperationById(document: OpenAPI2Document): Record<string, [string, OpenAPI2Operation]> {
210383
function isHttpMethod(key: string): key is HttpMethod {
@@ -220,14 +393,14 @@ function organizeOperationById(document: OpenAPI2Document): Record<string, [stri
220393
];
221394
return httpMethods.includes(key as HttpMethod);
222395
}
223-
224-
const operationMap: Record<string, [string, OpenAPI2Operation]> = {};
396+
397+
const operationMap: Record<string, [string, OpenAPI2Operation]> = {};
225398
if (!document.paths) {
226399
return operationMap;
227400
}
228401

229402
for (const route in document.paths) {
230-
const pathItem = document.paths[route];
403+
const pathItem = document.paths[route];
231404
for (const verb in pathItem) {
232405
if (isHttpMethod(verb)) {
233406
const operation = pathItem[verb];
@@ -237,7 +410,62 @@ function organizeOperationById(document: OpenAPI2Document): Record<string, [stri
237410
}
238411
}
239412
}
240-
413+
241414
return operationMap;
242415
}
243416

417+
function getAllProperties(schema: OpenAPI2Schema, document: OpenAPI2Document, properties: Record<string, OpenAPI2SchemaProperty> = {}): Record<string, OpenAPI2SchemaProperty> {
418+
for (const baseSchema of schema.allOf ?? []) {
419+
if (isRef(baseSchema)) {
420+
const refPath = baseSchema.$ref;
421+
const commonTypeDefinition = getCommonTypeDefinition(refPath);
422+
if (commonTypeDefinition) {
423+
getAllProperties(commonTypeDefinition[0], commonTypeDefinition[1], properties);
424+
}
425+
else {
426+
const baseDefinition = getLocalDefinition(refPath, document);
427+
if (baseDefinition) {
428+
getAllProperties(baseDefinition, document, properties);
429+
}
430+
}
431+
}
432+
else {
433+
if (baseSchema.properties) {
434+
for (const key in baseSchema.properties) {
435+
properties[key] = baseSchema.properties[key];
436+
}
437+
}
438+
}
439+
}
440+
441+
if (schema.properties) {
442+
for (const key in schema.properties) {
443+
properties[key] = schema.properties[key];
444+
}
445+
}
446+
return properties;
447+
}
448+
449+
function resolveCommonType(property: OpenAPI2SchemaProperty): OpenAPI2SchemaProperty {
450+
if (isRef(property)) {
451+
const { $ref, ...propertyWithoutRef } = property;
452+
const commonTypeDefinition = getCommonTypeDefinition($ref);
453+
if (commonTypeDefinition) {
454+
return { ...propertyWithoutRef, ...commonTypeDefinition[0] };
455+
}
456+
}
457+
458+
return property;
459+
}
460+
461+
function getLocalDefinition(refPath: string, document: OpenAPI2Document): OpenAPI2Schema | undefined {
462+
const definitionName = refPath.split("/").pop();
463+
if (!document.definitions || !document.definitions[definitionName!]) {
464+
console.warn(`Reference to ${definitionName} cannot be found, skipping.`);
465+
}
466+
return document.definitions?.[definitionName!];
467+
}
468+
469+
export function isRef<T>(value: Refable<T>): value is Ref<T> {
470+
return (value as Ref<T>).$ref !== undefined;
471+
}

‎eng/tools/typespec-migration-validation/src/index.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import {
1818
formatDifferenceReport,
1919
formatModifiedValuesReport,
2020
} from "./summary.js";
21-
import { compareDocuments, printPathDiff } from "./compare.js";
21+
import { compareDocuments, printDiff } from "./compare.js";
2222
import { writeFile } from "fs/promises";
2323

2424
function parseArguments() {
@@ -218,7 +218,7 @@ export async function main() {
218218
outputMarkdown += "| Type | Level | Message |\n";
219219
outputMarkdown += "| ---- | ----- | ------- |\n";
220220
for (const diff of compareResult) {
221-
outputMarkdown += printPathDiff(diff);
221+
outputMarkdown += printDiff(diff);
222222
}
223223
console.log(outputMarkdown);
224224
}

0 commit comments

Comments
 (0)