Skip to content

Commit fbe3cf5

Browse files
authored
fix(memory/mongodb): $select as only property & force 'id' in '$select' (#3081)
1 parent 2456bf8 commit fbe3cf5

7 files changed

Lines changed: 39 additions & 20 deletions

File tree

packages/adapter-tests/src/methods.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,7 @@ export default (test: AdapterMethodsTest, app: any, _errors: any, serviceName: s
9999
query: { $select: ['name'] }
100100
})
101101

102+
assert.strictEqual(data[idProp].toString(), doug[idProp].toString(), `${idProp} id property matches`)
102103
assert.strictEqual(data.name, 'Doug', 'data.name matches')
103104
assert.ok(!data.age, 'data.age is falsy')
104105
})
@@ -244,6 +245,7 @@ export default (test: AdapterMethodsTest, app: any, _errors: any, serviceName: s
244245
query: { $select: ['name'] }
245246
})
246247

248+
assert.strictEqual(data[idProp].toString(), doug[idProp].toString(), `${idProp} id property matches`)
247249
assert.strictEqual(data.name, 'Dougler', 'data.name matches')
248250
assert.ok(!data.age, 'data.age is falsy')
249251
})
@@ -333,6 +335,7 @@ export default (test: AdapterMethodsTest, app: any, _errors: any, serviceName: s
333335
query: { $select: ['name'] }
334336
})
335337

338+
assert.strictEqual(data[idProp].toString(), doug[idProp].toString(), `${idProp} id property matches`)
336339
assert.strictEqual(data.name, 'PatchDoug', 'data.name matches')
337340
assert.ok(!data.age, 'data.age is falsy')
338341
})
@@ -617,6 +620,7 @@ export default (test: AdapterMethodsTest, app: any, _errors: any, serviceName: s
617620
query: { $select: ['name'] }
618621
})
619622

623+
assert.ok(idProp in data, 'data has id')
620624
assert.strictEqual(data.name, 'William', 'data.name matches')
621625
assert.ok(!data.age, 'data.age is falsy')
622626

packages/adapter-tests/src/syntax.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,7 @@ export default (test: AdapterSyntaxTest, app: any, _errors: any, serviceName: st
127127
})
128128

129129
assert.strictEqual(data.length, 1)
130+
assert.ok(idProp in data[0], 'data has id')
130131
assert.strictEqual(data[0].name, 'Alice')
131132
assert.strictEqual(data[0].age, undefined)
132133
})

packages/memory/src/index.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ export interface MemoryServiceOptions<T = any> extends AdapterServiceOptions {
2222
sorter?: (sort: any) => any
2323
}
2424

25-
const _select = (data: any, params: any, ...args: any[]) => {
25+
const _select = (data: any, params: any, ...args: string[]) => {
2626
const base = select(params, ...args)
2727

2828
return base(JSON.parse(JSON.stringify(data)))
@@ -103,14 +103,16 @@ export class MemoryAdapter<
103103
continue
104104
}
105105

106-
matched.push(_select(value, params))
106+
matched.push(_select(value, params, this.id))
107107

108108
if (hasLimit && filters.$limit === matched.length) {
109109
break
110110
}
111111
}
112112

113113
values = matched
114+
} else {
115+
values = values.map((value) => _select(value, params, this.id))
114116
}
115117

116118
const result: Paginated<Result> = {

packages/memory/test/index.test.ts

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -190,33 +190,34 @@ describe('Feathers Memory Service', () => {
190190
await people.remove(person.id)
191191
})
192192

193-
it('does not $select the id', async () => {
193+
it('update with null throws error', async () => {
194+
try {
195+
await app.service('people').update(null, {})
196+
throw new Error('Should never get here')
197+
} catch (error: any) {
198+
assert.strictEqual(error.message, "You can not replace multiple instances. Did you mean 'patch'?")
199+
}
200+
})
201+
202+
it('use $select as only query property', async () => {
194203
const people = app.service('people')
195204
const person = await people.create({
196-
name: 'Tester'
205+
name: 'Tester',
206+
age: 42
197207
})
208+
198209
const results = await people.find({
199210
paginate: false,
200211
query: {
201-
name: 'Tester',
202212
$select: ['name']
203213
}
204214
})
205215

206-
assert.deepStrictEqual(results[0], { name: 'Tester' }, 'deepEquals the same')
216+
assert.deepStrictEqual(results[0], { id: person.id, name: 'Tester' })
207217

208218
await people.remove(person.id)
209219
})
210220

211-
it('update with null throws error', async () => {
212-
try {
213-
await app.service('people').update(null, {})
214-
throw new Error('Should never get here')
215-
} catch (error: any) {
216-
assert.strictEqual(error.message, "You can not replace multiple instances. Did you mean 'patch'?")
217-
}
218-
})
219-
220221
testSuite(app, errors, 'people')
221222
testSuite(app, errors, 'people-customid', 'customid')
222223
})

packages/mongodb/src/adapter.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,9 @@ export class MongoDbAdapter<
159159

160160
getSelect(select: string[] | { [key: string]: number }) {
161161
if (Array.isArray(select)) {
162+
if (!select.includes(this.id)) {
163+
select = [this.id, ...select]
164+
}
162165
return select.reduce<{ [key: string]: number }>(
163166
(value, name) => ({
164167
...value,
@@ -168,6 +171,13 @@ export class MongoDbAdapter<
168171
)
169172
}
170173

174+
if (!select[this.id]) {
175+
return {
176+
...select,
177+
[this.id]: 1
178+
}
179+
}
180+
171181
return select
172182
}
173183

packages/schema/src/hooks/resolve.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -76,20 +76,21 @@ export const resolveResult = <H extends HookContext>(...resolvers: Resolver<any,
7676
throw new Error('The resolveResult hook must be used as an around hook')
7777
}
7878

79-
const { $resolve, $select: select, ...query } = context.params?.query || {}
80-
const $select = Array.isArray(select) ? select.filter((name) => !virtualProperties.has(name)) : select
79+
const { $resolve, $select, ...query } = context.params?.query || {}
80+
const hasVirtualSelects = Array.isArray($select) && $select.some((name) => virtualProperties.has(name))
81+
8182
const resolve = {
8283
originalContext: context,
8384
...context.params.resolve,
84-
properties: $resolve || select
85+
properties: $resolve || $select
8586
}
8687

8788
context.params = {
8889
...context.params,
8990
resolve,
9091
query: {
9192
...query,
92-
...($select ? { $select } : {})
93+
...(!hasVirtualSelects ? { $select } : {})
9394
}
9495
}
9596

packages/schema/test/hooks.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ describe('@feathersjs/schema/hooks', () => {
129129
$select: ['user', 'text']
130130
}
131131
})
132-
assert.strictEqual(Object.keys(messages[0]).length, 2)
132+
assert.deepStrictEqual(Object.keys(messages[0]), ['text', 'user'])
133133
})
134134

135135
it('resolves find results with paginated result object', async () => {

0 commit comments

Comments
 (0)