Skip to content

Commit 8e9b258

Browse files
Copilotjehernandezr
Copilot
and
jehernandezr
authored
Fix route matching to support overlapping path parameters and static routes (#1)
* Initial plan * Add test case for overlapping path parameters routing Co-authored-by: jehernandezr <47890623+jehernandezr@users.noreply.github.com> * Implement route prioritization for overlapping path parameters Co-authored-by: jehernandezr <47890623+jehernandezr@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: jehernandezr <47890623+jehernandezr@users.noreply.github.com>
1 parent b37d62d commit 8e9b258

3 files changed

Lines changed: 195 additions & 7 deletions

File tree

‎lib/proxyIntegration.ts‎

Lines changed: 42 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -224,15 +224,35 @@ const convertError = (error: ProxyIntegrationError | Error, errorMapping?: Proxy
224224
const findMatchingActionConfig = (httpMethod: HttpMethod, httpPath: string, routeConfig: ProxyIntegrationConfig):
225225
ProxyIntegrationRoute & ProxyIntegrationParams | null => {
226226

227-
const paths: ProxyIntegrationParams['paths'] = {}
228227
const matchingMethodRoutes = routeConfig.routes.filter(route => route.method === httpMethod)
229-
for (const route of matchingMethodRoutes) {
228+
229+
// Sort routes by specificity (more specific routes first)
230+
const sortedRoutes = matchingMethodRoutes.sort((a, b) => {
231+
const aSpecificity = calculateRouteSpecificity(a.path)
232+
const bSpecificity = calculateRouteSpecificity(b.path)
233+
234+
// Higher specificity comes first
235+
if (aSpecificity.staticSegments !== bSpecificity.staticSegments) {
236+
return bSpecificity.staticSegments - aSpecificity.staticSegments
237+
}
238+
239+
// If same static segments, fewer parameters comes first
240+
if (aSpecificity.paramSegments !== bSpecificity.paramSegments) {
241+
return aSpecificity.paramSegments - bSpecificity.paramSegments
242+
}
243+
244+
// If same everything, longer path comes first (more segments)
245+
return bSpecificity.totalSegments - aSpecificity.totalSegments
246+
})
247+
248+
for (const route of sortedRoutes) {
230249
if (routeConfig.debug) {
231250
console.log(`Examining route ${route.path} to match ${httpPath}`)
232251
}
233252
const pathPartNames = extractPathNames(route.path)
234253
const pathValues = extractPathValues(route.path, httpPath)
235254
if (pathValues && pathPartNames) {
255+
const paths: ProxyIntegrationParams['paths'] = {}
236256
for (let ii = 0; ii < pathValues.length; ii++) {
237257
paths[pathPartNames[ii]] = decodeURIComponent(pathValues[ii])
238258
}
@@ -253,6 +273,26 @@ const findMatchingActionConfig = (httpMethod: HttpMethod, httpPath: string, rout
253273
return null
254274
}
255275

276+
const calculateRouteSpecificity = (path: string) => {
277+
const segments = path.split('/').filter(segment => segment.length > 0)
278+
let staticSegments = 0
279+
let paramSegments = 0
280+
281+
for (const segment of segments) {
282+
if (segment.startsWith(':') || (segment.startsWith('{') && segment.endsWith('}'))) {
283+
paramSegments++
284+
} else {
285+
staticSegments++
286+
}
287+
}
288+
289+
return {
290+
staticSegments,
291+
paramSegments,
292+
totalSegments: segments.length
293+
}
294+
}
295+
256296
const extractPathValues = (pathExpression: string, httpPath: string) => {
257297
const pathExpressionPattern = pathExpression.replace(/{[\w]+}|:[\w]+/g, '([^/]+)')
258298
const pathValueRegex = new RegExp(`^${pathExpressionPattern}$`)

‎test/proxyIntegration.test.ts‎

Lines changed: 153 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -740,3 +740,156 @@ describe('proxyIntegration.onError', () => {
740740
})
741741
})
742742
})
743+
744+
describe('proxyIntegration.routeHandler.pathPriority', () => {
745+
it('should handle overlapping path parameters and static routes correctly', async () => {
746+
const foodIdAction = jest.fn().mockReturnValue({ result: 'foodId' })
747+
const mostCommonsAction = jest.fn().mockReturnValue({ result: 'most-commons' })
748+
const mostCommonsFoodIdAction = jest.fn().mockReturnValue({ result: 'most-commons-foodId' })
749+
750+
const routeConfig: ProxyIntegrationConfig = {
751+
routes: [
752+
{ path: '/api/foods/:foodId', method: 'GET', action: foodIdAction },
753+
{ path: '/api/foods/most-commons', method: 'GET', action: mostCommonsAction },
754+
{ path: '/api/foods/most-commons/:foodId', method: 'GET', action: mostCommonsFoodIdAction }
755+
]
756+
}
757+
758+
// Test case 1: /api/foods/123 should match /api/foods/:foodId
759+
const result1 = await proxyIntegration(routeConfig, {
760+
path: '/api/foods/123',
761+
httpMethod: 'GET'
762+
} as APIGatewayProxyEvent, context)
763+
764+
expect(foodIdAction).toHaveBeenCalledWith({
765+
path: '/api/foods/123',
766+
httpMethod: 'GET',
767+
paths: { foodId: '123' },
768+
routePath: '/api/foods/:foodId'
769+
}, context)
770+
if (result1) {
771+
expect(result1.body).toBe(JSON.stringify({ result: 'foodId' }))
772+
}
773+
774+
// Reset mocks
775+
foodIdAction.mockClear()
776+
mostCommonsAction.mockClear()
777+
mostCommonsFoodIdAction.mockClear()
778+
779+
// Test case 2: /api/foods/most-commons should match /api/foods/most-commons exactly
780+
const result2 = await proxyIntegration(routeConfig, {
781+
path: '/api/foods/most-commons',
782+
httpMethod: 'GET'
783+
} as APIGatewayProxyEvent, context)
784+
785+
expect(mostCommonsAction).toHaveBeenCalledWith({
786+
path: '/api/foods/most-commons',
787+
httpMethod: 'GET',
788+
paths: {},
789+
routePath: '/api/foods/most-commons'
790+
}, context)
791+
if (result2) {
792+
expect(result2.body).toBe(JSON.stringify({ result: 'most-commons' }))
793+
}
794+
795+
// Reset mocks
796+
foodIdAction.mockClear()
797+
mostCommonsAction.mockClear()
798+
mostCommonsFoodIdAction.mockClear()
799+
800+
// Test case 3: /api/foods/most-commons/456 should match /api/foods/most-commons/:foodId
801+
const result3 = await proxyIntegration(routeConfig, {
802+
path: '/api/foods/most-commons/456',
803+
httpMethod: 'GET'
804+
} as APIGatewayProxyEvent, context)
805+
806+
expect(mostCommonsFoodIdAction).toHaveBeenCalledWith({
807+
path: '/api/foods/most-commons/456',
808+
httpMethod: 'GET',
809+
paths: { foodId: '456' },
810+
routePath: '/api/foods/most-commons/:foodId'
811+
}, context)
812+
if (result3) {
813+
expect(result3.body).toBe(JSON.stringify({ result: 'most-commons-foodId' }))
814+
}
815+
})
816+
817+
it('should prioritize more specific routes with mixed parameter formats', async () => {
818+
const paramActionColon = jest.fn().mockReturnValue({ result: 'colon-param' })
819+
const paramActionBrace = jest.fn().mockReturnValue({ result: 'brace-param' })
820+
const staticAction = jest.fn().mockReturnValue({ result: 'static' })
821+
822+
const routeConfig: ProxyIntegrationConfig = {
823+
routes: [
824+
{ path: '/users/:userId', method: 'GET', action: paramActionColon },
825+
{ path: '/users/{userId}', method: 'GET', action: paramActionBrace },
826+
{ path: '/users/profile', method: 'GET', action: staticAction }
827+
]
828+
}
829+
830+
// Static route should win over parameter routes
831+
const result = await proxyIntegration(routeConfig, {
832+
path: '/users/profile',
833+
httpMethod: 'GET'
834+
} as APIGatewayProxyEvent, context)
835+
836+
expect(staticAction).toHaveBeenCalledWith({
837+
path: '/users/profile',
838+
httpMethod: 'GET',
839+
paths: {},
840+
routePath: '/users/profile'
841+
}, context)
842+
expect(paramActionColon).not.toHaveBeenCalled()
843+
expect(paramActionBrace).not.toHaveBeenCalled()
844+
if (result) {
845+
expect(result.body).toBe(JSON.stringify({ result: 'static' }))
846+
}
847+
})
848+
849+
it('should handle complex nested route scenarios', async () => {
850+
const userIdAction = jest.fn().mockReturnValue({ result: 'userId' })
851+
const userPostsAction = jest.fn().mockReturnValue({ result: 'user-posts' })
852+
const userPostAction = jest.fn().mockReturnValue({ result: 'user-post' })
853+
const adminAction = jest.fn().mockReturnValue({ result: 'admin' })
854+
855+
const routeConfig: ProxyIntegrationConfig = {
856+
routes: [
857+
{ path: '/users/:userId', method: 'GET', action: userIdAction },
858+
{ path: '/users/:userId/posts', method: 'GET', action: userPostsAction },
859+
{ path: '/users/:userId/posts/:postId', method: 'GET', action: userPostAction },
860+
{ path: '/users/admin', method: 'GET', action: adminAction }
861+
]
862+
}
863+
864+
// Test /users/admin should match the static route, not the parameter route
865+
const result1 = await proxyIntegration(routeConfig, {
866+
path: '/users/admin',
867+
httpMethod: 'GET'
868+
} as APIGatewayProxyEvent, context)
869+
870+
expect(adminAction).toHaveBeenCalledWith({
871+
path: '/users/admin',
872+
httpMethod: 'GET',
873+
paths: {},
874+
routePath: '/users/admin'
875+
}, context)
876+
expect(userIdAction).not.toHaveBeenCalled()
877+
878+
// Reset and test /users/123/posts
879+
adminAction.mockClear()
880+
userIdAction.mockClear()
881+
userPostsAction.mockClear()
882+
883+
const result2 = await proxyIntegration(routeConfig, {
884+
path: '/users/123/posts',
885+
httpMethod: 'GET'
886+
} as APIGatewayProxyEvent, context)
887+
888+
expect(userPostsAction).toHaveBeenCalledWith({
889+
path: '/users/123/posts',
890+
httpMethod: 'GET',
891+
paths: { userId: '123' },
892+
routePath: '/users/:userId/posts'
893+
}, context)
894+
})
895+
})

‎yarn.lock‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1327,11 +1327,6 @@ fs.realpath@^1.0.0:
13271327
resolved "https://registry.npmjs.org/fs.realpath/-/fs.realpath-1.0.0.tgz"
13281328
integrity sha512-OO0pH2lK6a0hZnAdau5ItzHPI6pUlvI7jMVnxUQRtw4owF2wk8lOSabtGDCTP4Ggrg2MbGnWO9X8K1t4+fGMDw==
13291329

1330-
fsevents@^2.1.2:
1331-
version "2.3.2"
1332-
resolved "https://registry.npmjs.org/fsevents/-/fsevents-2.3.2.tgz"
1333-
integrity sha512-xiqMQR4xAeHTuB9uWm+fFRcIOgKBMiOBP+eXiyT7jsgVCq1bkVygt00oASowB7EdtpOHaaPgKt812P9ab+DDKA==
1334-
13351330
function-bind@^1.1.2:
13361331
version "1.1.2"
13371332
resolved "https://registry.npmjs.org/function-bind/-/function-bind-1.1.2.tgz"

0 commit comments

Comments
 (0)