Skip to content

Commit fb06f63

Browse files
committed
fix: access route data property correctly for conflict detection
1 parent d4b41ac commit fb06f63

File tree

3 files changed

+109
-6
lines changed

3 files changed

+109
-6
lines changed

packages/feathers/src/application.test.ts

Lines changed: 83 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
/* eslint-disable @typescript-eslint/ban-ts-comment */
22
import { describe, it } from 'vitest'
33
import assert from 'assert'
4-
import { feathers, Feathers, getServiceOptions, Id, version } from '../src/index.js'
4+
import { feathers, Feathers, getServiceOptions, Id, version, method } from '../src/index.js'
55

66
describe('Feathers application', () => {
77
it('initializes', () => {
@@ -563,4 +563,86 @@ describe('Feathers application', () => {
563563
})
564564
})
565565
})
566+
567+
describe('route conflicts', () => {
568+
it('throws when custom method path conflicts with another service custom method', () => {
569+
const app = feathers()
570+
571+
class MessageService {
572+
@method({ args: ['id', 'params'], http: 'GET', path: ':id/status' })
573+
async status(id: Id) {
574+
return { id, status: 'message' }
575+
}
576+
}
577+
578+
class NotificationService {
579+
// Same path pattern - this would conflict with messages/:id/status
580+
@method({ args: ['id', 'params'], http: 'GET', path: ':id/status' })
581+
async status(id: Id) {
582+
return { id, status: 'notification' }
583+
}
584+
}
585+
586+
app.use('messages', new MessageService())
587+
app.use('notifications', new NotificationService())
588+
589+
// These have different base paths, so they should NOT conflict
590+
assert.ok(app.service('messages'))
591+
assert.ok(app.service('notifications'))
592+
})
593+
594+
it('throws when custom method path conflicts with another service base path', () => {
595+
const app = feathers()
596+
597+
// Register a nested service first
598+
app.use('messages/archived', {
599+
async find() {
600+
return []
601+
}
602+
})
603+
604+
// Now try to register a service with a custom method that would conflict
605+
class MessageService {
606+
async find() {
607+
return []
608+
}
609+
610+
@method({ args: ['params'], http: 'GET', path: 'archived' })
611+
async archived() {
612+
return []
613+
}
614+
}
615+
616+
assert.throws(
617+
() => app.use('messages', new MessageService()),
618+
/Path 'messages\/archived' for method 'archived' conflicts with another service/
619+
)
620+
})
621+
622+
it('allows different custom method paths on the same service', () => {
623+
const app = feathers()
624+
625+
class MessageService {
626+
@method({ args: ['id', 'params'], http: 'GET', path: ':id/status' })
627+
async status(id: Id) {
628+
return { id, status: 'active' }
629+
}
630+
631+
@method({ args: ['id', 'params'], http: 'POST', path: ':id/archive' })
632+
async archive(id: Id) {
633+
return { id, archived: true }
634+
}
635+
636+
@method({ args: ['params'], http: 'GET', path: 'stats' })
637+
async stats() {
638+
return { total: 100 }
639+
}
640+
}
641+
642+
// Should not throw
643+
app.use('messages', new MessageService())
644+
645+
assert.ok(app.service('messages'))
646+
})
647+
})
566648
})

packages/feathers/src/application.ts

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -286,11 +286,28 @@ export class Feathers<Services, Settings>
286286
const customPath = methodConfig.path.replace(/:id\b/g, ':__id')
287287
const fullPath = `${path}/${customPath}`
288288

289-
this.routes.insert(fullPath, {
290-
...routerParams,
291-
method: methodName,
292-
httpMethod: methodConfig.http || 'POST'
293-
})
289+
// The router.insert() will throw if an exact path already exists.
290+
// This catches duplicate custom method paths and conflicts with other services.
291+
try {
292+
this.routes.insert(fullPath, {
293+
...routerParams,
294+
method: methodName,
295+
httpMethod: methodConfig.http || 'POST'
296+
})
297+
} catch (error: any) {
298+
if (error.message?.includes('already exists')) {
299+
// Check what's already registered at this path to give a better error message
300+
const existingRoute = this.routes.lookup(fullPath)
301+
const existingMethod = existingRoute?.data?.method
302+
? `method '${existingRoute.data.method}'`
303+
: 'another service'
304+
throw new Error(
305+
`Path '${fullPath}' for method '${methodName}' conflicts with ${existingMethod}. ` +
306+
`Custom method paths must be unique.`
307+
)
308+
}
309+
throw error
310+
}
294311

295312
debug(`Registered custom path \`${fullPath}\` for method \`${methodName}\``)
296313
}

website/content/api/services.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -269,6 +269,10 @@ For more control over custom methods, you can use the `@method` decorator to con
269269
- **Clean URL paths** (e.g., `/messages/123/status` instead of using `X-Service-Method` header)
270270
- **Internal-only methods** (hooks run but not exposed via HTTP)
271271

272+
::info[Note]
273+
There are two alternatives to the `@method` decorator. See the next sections.
274+
::
275+
272276
```ts
273277
import { feathers, method, hooks } from '@feathersjs/feathers'
274278
import type { Id, Params } from '@feathersjs/feathers'

0 commit comments

Comments
 (0)