fix: operation matching when operation base path moved between path and path item servers object when comparing prefix groups

Author: b41ex

src/apitypes/rest/rest.changes.ts

@@ -278,23 +278,37 @@ export function createCopyWithEmptyPathItems(template: RestOperationData): RestO
 }
 
 export function createCopyWithCurrentGroupOperationsOnly(template: RestOperationData, group: string): RestOperationData {
-  const { paths, ...rest } = template
+  // eslint-disable-next-line @typescript-eslint/no-unused-vars
+  const { paths, servers: rootServers, ...rest } = template
   const groupWithoutEdgeSlashes = trimSlashes(group)
 
-  if (trimSlashes(getOperationBasePath(template.servers)) === groupWithoutEdgeSlashes) {
-    // eslint-disable-next-line @typescript-eslint/no-unused-vars
-    const { servers, ...rest } = template
-    return { ...rest }
-  }
-
+  // Since we are anyway composing synthetic specs for prefix groups comparison, we can incorporate
+  // base paths from root servers and path item servers into the paths.
+  // We also remove servers objects, since changes in servers for prefix groups are not relevant.
+  // Note that servers in operation objects are not taken into account
+  // (it is impossible to support them in api-diff mapping
+  // and they are considered bad practice on OpenAPI specifications anyway)
   return {
     paths: {
       ...Object.fromEntries(
         Object.entries(paths)
-          .filter(([key]) => removeFirstSlash(key).startsWith(`${groupWithoutEdgeSlashes}/`)) // note that 'api/v10' is a substring of 'api/v1000'
+          .map(([pathKey, pathItem]) => {
+            // Path item servers take precedence over root servers
+            const pathItemServers = (pathItem as OpenAPIV3.PathItemObject)?.servers
+            const basePath = getOperationBasePath(pathItemServers || template.servers || [])
+
+            // Prepend base path to the path
+            const fullPath = basePath ? `/${trimSlashes(basePath)}/${trimSlashes(pathKey)}`.replace(/\/+/g, '/') : pathKey
+
+            // Remove servers from path item copy
+            // eslint-disable-next-line @typescript-eslint/no-unused-vars
+            const { servers: pathServers, ...pathItemWithoutServers } = pathItem as OpenAPIV3.PathItemObject
+
+            return [fullPath, pathItemWithoutServers] as const
+          })
+          .filter(([key]) => removeFirstSlash(key as string).startsWith(`${groupWithoutEdgeSlashes}/`)) // note that 'api/v10' is a substring of 'api/v1000'
           // remove prefix group for correct path mapping in apiDiff
-          // todo support the most common case when a group is in servers instead of hardcoded in path, add a test
-          .map(([key, value]) => [removeFirstSlash(key).substring(groupWithoutEdgeSlashes.length), value]),
+          .map(([key, value]) => [removeFirstSlash(key as string).substring(groupWithoutEdgeSlashes.length), value]),
       ),
     },
     ...rest,

test/prefix-groups.test.ts

@@ -162,8 +162,7 @@ describe('Prefix Groups test', () => {
     }))
   })
 
-  // todo: case that we don't support due to shifting to the new changelog calculation approach which involves comparison of the entire docs instead of the operation vs operation comparison
-  test.skip('should compare prefix groups /api/{group} when prefix is overridden in path', async () => {
+  test('should compare prefix groups /api/{group} when prefix is overridden in path', async () => {
     const result = await buildPrefixGroupChangelogPackage({
       packageId: 'prefix-groups/mixed-cases-with-path-prefix-override',
       config: { files: [{ fileId: 'spec1.yaml' }, { fileId: 'spec2.yaml' }] },
@@ -172,12 +171,12 @@ describe('Prefix Groups test', () => {
     expect(result).toEqual(changesSummaryMatcher({
       [BREAKING_CHANGE_TYPE]: 1,
       [NON_BREAKING_CHANGE_TYPE]: 1,
-      [ANNOTATION_CHANGE_TYPE]: 1,// todo
+      [ANNOTATION_CHANGE_TYPE]: 1,
     }))
     expect(result).toEqual(numberOfImpactedOperationsMatcher({
       [BREAKING_CHANGE_TYPE]: 1,
       [NON_BREAKING_CHANGE_TYPE]: 1,
-      [ANNOTATION_CHANGE_TYPE]: 1,// todo
+      [ANNOTATION_CHANGE_TYPE]: 1,
     }))
   })