From 2698b30788a9e954865ab774489487529e751390 Mon Sep 17 00:00:00 2001 From: DaddyWarbucks Date: Fri, 1 Dec 2023 13:38:19 -0600 Subject: [PATCH 01/26] Update docs --- docs/api/databases/mongodb.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/api/databases/mongodb.md b/docs/api/databases/mongodb.md index 6ad74f35c7..a54876fb7c 100644 --- a/docs/api/databases/mongodb.md +++ b/docs/api/databases/mongodb.md @@ -208,7 +208,7 @@ Here's how it works with the operators that match the Feathers Query syntax. Let const query = { text: { $regex: 'feathersjs', $options: 'igm' }, $sort: { createdAt: -1 }, - $skip: 0, + $skip: 20, $limit: 10 } ``` From 8a4840fd72c34b0d8e262a89207f0a44214d939b Mon Sep 17 00:00:00 2001 From: DaddyWarbucks Date: Fri, 1 Dec 2023 19:05:58 -0600 Subject: [PATCH 02/26] Allow _get method to use pipeline --- packages/mongodb/src/adapter.ts | 22 ++++++++++++++++++++++ packages/mongodb/test/index.test.ts | 15 +++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/packages/mongodb/src/adapter.ts b/packages/mongodb/src/adapter.ts index 4461b5d52b..610d22a61c 100644 --- a/packages/mongodb/src/adapter.ts +++ b/packages/mongodb/src/adapter.ts @@ -219,6 +219,28 @@ export class MongoDbAdapter< ...projection } + if (params.pipeline) { + const aggregateParams = { + ...params, + query: { + ...params.query, + $and: (params.query.$and || []).concat({ + [this.id]: this.getObjectId(id) + }) + } + } + return this.aggregateRaw(aggregateParams) + .then((result) => result.toArray()) + .then(([data]) => { + if (data === undefined) { + throw new NotFound(`No record found for id '${id}'`) + } + + return data + }) + .catch(errorHandler) + } + return this.getModel(params) .then((model) => model.findOne(query, findOptions)) .then((data) => { diff --git a/packages/mongodb/test/index.test.ts b/packages/mongodb/test/index.test.ts index 6b30394536..4a06f05aba 100644 --- a/packages/mongodb/test/index.test.ts +++ b/packages/mongodb/test/index.test.ts @@ -474,6 +474,21 @@ describe('Feathers MongoDB Service', () => { assert.deepEqual(result[0].person, bob) assert.equal(result.length, 1) }) + + it('can use aggregation in _get', async () => { + const dave = await app.service('people').create({ name: 'Dave', age: 25 }) + const result1 = await app.service('people').get(dave._id, { + query: { $select: ['name'] } + }) + const result2 = await app.service('people').get(dave._id, { + query: { $select: ['name'] }, + pipeline: [] + }) + + assert.deepStrictEqual(result1, result2) + + app.service('people').remove(dave._id) + }) }) describe('query validation', () => { From 39ac4a1656f80fb9d85d36af28fbdb04bf27b255 Mon Sep 17 00:00:00 2001 From: DaddyWarbucks Date: Fri, 1 Dec 2023 19:10:58 -0600 Subject: [PATCH 03/26] Add comment --- packages/mongodb/src/adapter.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/mongodb/src/adapter.ts b/packages/mongodb/src/adapter.ts index 610d22a61c..ddd940f80d 100644 --- a/packages/mongodb/src/adapter.ts +++ b/packages/mongodb/src/adapter.ts @@ -224,6 +224,8 @@ export class MongoDbAdapter< ...params, query: { ...params.query, + // TODO: Do we need this $and like its needed in find? + // Or should it just be `[this.id]: this.getObjectId(id)` $and: (params.query.$and || []).concat({ [this.id]: this.getObjectId(id) }) From a2a1d55efa338586251b20516e1f65173bde5cfc Mon Sep 17 00:00:00 2001 From: DaddyWarbucks Date: Fri, 1 Dec 2023 19:23:10 -0600 Subject: [PATCH 04/26] Disallow pipeline with mongodb param --- packages/mongodb/src/adapter.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/mongodb/src/adapter.ts b/packages/mongodb/src/adapter.ts index ddd940f80d..030ff221cf 100644 --- a/packages/mongodb/src/adapter.ts +++ b/packages/mongodb/src/adapter.ts @@ -219,7 +219,7 @@ export class MongoDbAdapter< ...projection } - if (params.pipeline) { + if (!params.mongodb && params.pipeline) { const aggregateParams = { ...params, query: { From 47962fb3c98bdc54200f0a2341bda1849e28930e Mon Sep 17 00:00:00 2001 From: DaddyWarbucks Date: Mon, 4 Dec 2023 23:04:02 -0600 Subject: [PATCH 05/26] Use findOneAndReplace in _update method --- packages/mongodb/src/adapter.ts | 34 +++++++++++++++++++++++++---- packages/mongodb/test/index.test.ts | 17 +++++++++++++++ 2 files changed, 47 insertions(+), 4 deletions(-) diff --git a/packages/mongodb/src/adapter.ts b/packages/mongodb/src/adapter.ts index 030ff221cf..f995a3f3c9 100644 --- a/packages/mongodb/src/adapter.ts +++ b/packages/mongodb/src/adapter.ts @@ -7,6 +7,7 @@ import { DeleteOptions, CountDocumentsOptions, ReplaceOptions, + FindOneAndReplaceOptions, Document } from 'mongodb' import { BadRequest, MethodNotAllowed, NotFound } from '@feathersjs/errors' @@ -39,6 +40,7 @@ export interface MongoDBAdapterParams | DeleteOptions | CountDocumentsOptions | ReplaceOptions + | FindOneAndReplaceOptions } export type AdapterId = Id | ObjectId @@ -387,13 +389,30 @@ export class MongoDbAdapter< throw new BadRequest("You can not replace multiple instances. Did you mean 'patch'?") } + const { + query, + filters: { $select } + } = this.filterQuery(id, params) const model = await this.getModel(params) - const { query } = this.filterQuery(id, params) - const replaceOptions = { ...params.mongodb } - await model.replaceOne(query, this.normalizeId(id, data), replaceOptions) + const projection = $select + ? { + ...this.getSelect($select), + [this.id]: 1 + } + : {} + + const result = await model.findOneAndReplace(query, this.normalizeId(id, data), { + ...(params.mongodb as FindOneAndReplaceOptions), + returnDocument: 'after', + projection + }) - return this._findOrGet(id, params).catch(errorHandler) + if (result.value === null) { + throw new NotFound(`No record found for id '${id}'`) + } + + return result.value as Result } async _remove(id: null, params?: ServiceParams): Promise @@ -430,3 +449,10 @@ export class MongoDbAdapter< .catch(errorHandler) } } + +// function hasAggregation(params: AdapterParams) { +// if (!params.query || Object.keys(params.query).length === 0) { +// return false +// } +// return true +// } diff --git a/packages/mongodb/test/index.test.ts b/packages/mongodb/test/index.test.ts index 4a06f05aba..289b52689d 100644 --- a/packages/mongodb/test/index.test.ts +++ b/packages/mongodb/test/index.test.ts @@ -507,6 +507,23 @@ describe('Feathers MongoDB Service', () => { }) }) + // TODO: Should this test be part of the adapterTests? + describe('Updates mutated query', () => { + it('Can re-query mutated data', async () => { + const dave = await app.service('people').create({ name: 'Dave' }) + const result = await app + .service('people') + .update(dave._id, { name: 'Marshal' }, { query: { name: 'Dave' } }) + + assert.deepStrictEqual(result, { + ...dave, + name: 'Marshal' + }) + + app.service('people').remove(dave._id) + }) + }) + testSuite(app, errors, 'people', '_id') testSuite(app, errors, 'people-customid', 'customid') }) From 39b0387459b6be43d55853708ef6f44d71c6608d Mon Sep 17 00:00:00 2001 From: DaddyWarbucks Date: Mon, 4 Dec 2023 23:24:36 -0600 Subject: [PATCH 06/26] Use findOneAndUpdate in patch method --- packages/mongodb/src/adapter.ts | 64 ++++++++++++++++++++++----------- 1 file changed, 44 insertions(+), 20 deletions(-) diff --git a/packages/mongodb/src/adapter.ts b/packages/mongodb/src/adapter.ts index f995a3f3c9..f5514e0abd 100644 --- a/packages/mongodb/src/adapter.ts +++ b/packages/mongodb/src/adapter.ts @@ -8,6 +8,7 @@ import { CountDocumentsOptions, ReplaceOptions, FindOneAndReplaceOptions, + FindOneAndUpdateOptions, Document } from 'mongodb' import { BadRequest, MethodNotAllowed, NotFound } from '@feathersjs/errors' @@ -345,8 +346,8 @@ export class MongoDbAdapter< query, filters: { $select } } = this.filterQuery(id, params) - const updateOptions = { ...params.mongodb } - const modifier = Object.keys(data).reduce((current, key) => { + + const replacement = Object.keys(data).reduce((current, key) => { const value = (data as any)[key] if (key.charAt(0) !== '$') { @@ -360,28 +361,51 @@ export class MongoDbAdapter< return current }, {} as any) - const originalIds = await this._findOrGet(id, { - ...params, - query: { - ...query, - $select: [this.id] - }, - paginate: false - }) - const items = Array.isArray(originalIds) ? originalIds : [originalIds] - const idList = items.map((item: any) => item[this.id]) - const findParams = { - ...params, - paginate: false, - query: { - [this.id]: { $in: idList }, - $select + + if (id === null) { + const updateOptions = { ...params.mongodb } + const originalIds = await this._findOrGet(id, { + ...params, + query: { + ...query, + $select: [this.id] + }, + paginate: false + }) + const items = Array.isArray(originalIds) ? originalIds : [originalIds] + const idList = items.map((item: any) => item[this.id]) + const findParams = { + ...params, + paginate: false, + query: { + [this.id]: { $in: idList }, + $select + } } + + await model.updateMany(query, replacement, updateOptions) + + return this._findOrGet(id, findParams).catch(errorHandler) } - await model.updateMany(query, modifier, updateOptions) + const projection = $select + ? { + ...this.getSelect($select), + [this.id]: 1 + } + : {} - return this._findOrGet(id, findParams).catch(errorHandler) + const result = await model.findOneAndUpdate(query, replacement, { + ...(params.mongodb as FindOneAndUpdateOptions), + returnDocument: 'after', + projection + }) + + if (result.value === null) { + throw new NotFound(`No record found for id '${id}'`) + } + + return result.value as Result } async _update(id: AdapterId, data: Data, params: ServiceParams = {} as ServiceParams): Promise { From 4795929ae0bfee7ee3926754b6fea29827a723ce Mon Sep 17 00:00:00 2001 From: DaddyWarbucks Date: Mon, 4 Dec 2023 23:30:59 -0600 Subject: [PATCH 07/26] Allow $sort with _delete method --- packages/mongodb/src/adapter.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/mongodb/src/adapter.ts b/packages/mongodb/src/adapter.ts index f5514e0abd..7aea72bc0c 100644 --- a/packages/mongodb/src/adapter.ts +++ b/packages/mongodb/src/adapter.ts @@ -453,21 +453,21 @@ export class MongoDbAdapter< const model = await this.getModel(params) const { query, - filters: { $select } + filters: { $select, $sort } } = this.filterQuery(id, params) - const deleteOptions = { ...params.mongodb } const findParams = { ...params, paginate: false, query: { ...query, - $select + $select, + $sort } } return this._findOrGet(id, findParams) .then(async (items) => { - await model.deleteMany(query, deleteOptions) + await model.deleteMany(query, params.mongodb) return items }) .catch(errorHandler) From bf252e57712a9dcdb1b87f94165d856420436d3a Mon Sep 17 00:00:00 2001 From: DaddyWarbucks Date: Mon, 4 Dec 2023 23:49:17 -0600 Subject: [PATCH 08/26] Allow pipeline and better mongo methods in _delete --- packages/mongodb/src/adapter.ts | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/packages/mongodb/src/adapter.ts b/packages/mongodb/src/adapter.ts index 7aea72bc0c..63bc8a3156 100644 --- a/packages/mongodb/src/adapter.ts +++ b/packages/mongodb/src/adapter.ts @@ -465,10 +465,20 @@ export class MongoDbAdapter< } } - return this._findOrGet(id, findParams) - .then(async (items) => { - await model.deleteMany(query, params.mongodb) - return items + if (id === null) { + return this._find(findParams) + .then(async (result) => { + const idList = (result as Result[]).map((item: any) => item[this.id]) + await model.deleteMany({ [this.id]: { $in: idList } }, params.mongodb) + return result + }) + .catch(errorHandler) + } + + return this._get(id, findParams) + .then(async (result) => { + await model.deleteOne({ [this.id]: id }, params.mongodb) + return result }) .catch(errorHandler) } From 8a49c54ba88d774775766fa46b67e1c753a9221b Mon Sep 17 00:00:00 2001 From: DaddyWarbucks Date: Tue, 5 Dec 2023 00:07:42 -0600 Subject: [PATCH 09/26] Sort before select in pipeline --- packages/mongodb/src/adapter.ts | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/packages/mongodb/src/adapter.ts b/packages/mongodb/src/adapter.ts index 63bc8a3156..0d8b03fbb8 100644 --- a/packages/mongodb/src/adapter.ts +++ b/packages/mongodb/src/adapter.ts @@ -106,7 +106,7 @@ export class MongoDbAdapter< async findRaw(params: ServiceParams) { const { filters, query } = this.filterQuery(null, params) const model = await this.getModel(params) - const q = model.find(query, { ...params.mongodb }) + const q = model.find(query, params.mongodb) if (filters.$select !== undefined) { q.project(this.getSelect(filters.$select)) @@ -142,13 +142,14 @@ export class MongoDbAdapter< const { filters, query } = this.filterQuery(null, params) const pipeline: Document[] = [{ $match: query }] + if (filters.$sort !== undefined) { + pipeline.push({ $sort: filters.$sort }) + } + if (filters.$select !== undefined) { pipeline.push({ $project: this.getSelect(filters.$select) }) } - if (filters.$sort !== undefined) { - pipeline.push({ $sort: filters.$sort }) - } if (filters.$skip !== undefined) { pipeline.push({ $skip: filters.$skip }) @@ -222,7 +223,8 @@ export class MongoDbAdapter< ...projection } - if (!params.mongodb && params.pipeline) { + if (params.pipeline) { + /* We wouldn't need this aggregateParams if aggregateRaw took signature aggregateRaw(id, params) instead of just aggregateRaw(params). Because aggregateRaw ultimately calls makeFeathersPipeline(params) without id. That also makes aggregateRaw more flexible and consistent with other methods like _findOrGet. But, its a breaking change. */ const aggregateParams = { ...params, query: { From 60c1819a6f434db5f15912b08cb18080f2cf5eee Mon Sep 17 00:00:00 2001 From: DaddyWarbucks Date: Tue, 5 Dec 2023 00:56:09 -0600 Subject: [PATCH 10/26] USe mongo projection in create instead of common select --- packages/mongodb/src/adapter.ts | 69 +++++++++++++-------------------- 1 file changed, 27 insertions(+), 42 deletions(-) diff --git a/packages/mongodb/src/adapter.ts b/packages/mongodb/src/adapter.ts index 0d8b03fbb8..13c79b2d63 100644 --- a/packages/mongodb/src/adapter.ts +++ b/packages/mongodb/src/adapter.ts @@ -15,7 +15,6 @@ import { BadRequest, MethodNotAllowed, NotFound } from '@feathersjs/errors' import { _ } from '@feathersjs/commons' import { AdapterBase, - select, AdapterParams, AdapterServiceOptions, PaginationOptions, @@ -109,7 +108,7 @@ export class MongoDbAdapter< const q = model.find(query, params.mongodb) if (filters.$select !== undefined) { - q.project(this.getSelect(filters.$select)) + q.project(this.getProjection(filters.$select)) } if (filters.$sort !== undefined) { @@ -135,7 +134,7 @@ export class MongoDbAdapter< const feathersPipeline = this.makeFeathersPipeline(params) const after = index >= 0 ? pipeline.slice(index + 1) : pipeline - return model.aggregate([...before, ...feathersPipeline, ...after]) + return model.aggregate([...before, ...feathersPipeline, ...after], params.mongodb) } makeFeathersPipeline(params: ServiceParams) { @@ -147,10 +146,9 @@ export class MongoDbAdapter< } if (filters.$select !== undefined) { - pipeline.push({ $project: this.getSelect(filters.$select) }) + pipeline.push({ $project: this.getProjection(filters.$select) }) } - if (filters.$skip !== undefined) { pipeline.push({ $skip: filters.$skip }) } @@ -161,7 +159,7 @@ export class MongoDbAdapter< return pipeline } - getSelect(select: string[] | { [key: string]: number }) { + getProjection(select: string[] | { [key: string]: number }) { if (Array.isArray(select)) { if (!select.includes(this.id)) { select = [this.id, ...select] @@ -210,17 +208,10 @@ export class MongoDbAdapter< query, filters: { $select } } = this.filterQuery(id, params) - const projection = $select - ? { - projection: { - ...this.getSelect($select), - [this.id]: 1 - } - } - : {} + const projection = $select ? this.getProjection($select) : {} const findOptions: FindOptions = { ...params.mongodb, - ...projection + projection } if (params.pipeline) { @@ -229,6 +220,7 @@ export class MongoDbAdapter< ...params, query: { ...params.query, + $limit: 1, // TODO: Do we need this $and like its needed in find? // Or should it just be `[this.id]: this.getObjectId(id)` $and: (params.query.$and || []).concat({ @@ -238,24 +230,24 @@ export class MongoDbAdapter< } return this.aggregateRaw(aggregateParams) .then((result) => result.toArray()) - .then(([data]) => { - if (data === undefined) { + .then(([result]) => { + if (result === undefined) { throw new NotFound(`No record found for id '${id}'`) } - return data + return result }) .catch(errorHandler) } return this.getModel(params) .then((model) => model.findOne(query, findOptions)) - .then((data) => { - if (data == null) { + .then((result) => { + if (result == null) { throw new NotFound(`No record found for id '${id}'`) } - return data + return result }) .catch(errorHandler) } @@ -301,7 +293,6 @@ export class MongoDbAdapter< data: Data | Data[], params: ServiceParams = {} as ServiceParams ): Promise { - const writeOptions = params.mongodb const model = await this.getModel(params) const setId = (item: any) => { const entry = Object.assign({}, item) @@ -316,18 +307,23 @@ export class MongoDbAdapter< return entry } + const projection = params.query?.$select ? this.getProjection(params.query.$select) : {} + const findOptions: FindOptions = { + ...params.mongodb, + projection + } const promise = Array.isArray(data) ? model - .insertMany(data.map(setId), writeOptions) - .then(async (result) => - model.find({ _id: { $in: Object.values(result.insertedIds) } }, params.mongodb).toArray() + .insertMany(data.map(setId), params.mongodb) + .then((result) => + model.find({ _id: { $in: Object.values(result.insertedIds) } }, findOptions).toArray() ) : model - .insertOne(setId(data), writeOptions) - .then(async (result) => model.findOne({ _id: result.insertedId }, params.mongodb)) + .insertOne(setId(data), params.mongodb) + .then((result) => model.findOne({ _id: result.insertedId }, findOptions)) - return promise.then(select(params, this.id)).catch(errorHandler) + return promise.catch(errorHandler) } async _patch(id: null, data: PatchData, params?: ServiceParams): Promise @@ -365,7 +361,6 @@ export class MongoDbAdapter< }, {} as any) if (id === null) { - const updateOptions = { ...params.mongodb } const originalIds = await this._findOrGet(id, { ...params, query: { @@ -385,17 +380,12 @@ export class MongoDbAdapter< } } - await model.updateMany(query, replacement, updateOptions) + await model.updateMany(query, replacement, params.mongodb) return this._findOrGet(id, findParams).catch(errorHandler) } - const projection = $select - ? { - ...this.getSelect($select), - [this.id]: 1 - } - : {} + const projection = $select ? this.getProjection($select) : {} const result = await model.findOneAndUpdate(query, replacement, { ...(params.mongodb as FindOneAndUpdateOptions), @@ -421,12 +411,7 @@ export class MongoDbAdapter< } = this.filterQuery(id, params) const model = await this.getModel(params) - const projection = $select - ? { - ...this.getSelect($select), - [this.id]: 1 - } - : {} + const projection = $select ? this.getProjection($select) : {} const result = await model.findOneAndReplace(query, this.normalizeId(id, data), { ...(params.mongodb as FindOneAndReplaceOptions), From ec45ecf2953f6f70b35ad5926a36213180102953 Mon Sep 17 00:00:00 2001 From: DaddyWarbucks Date: Tue, 5 Dec 2023 01:03:37 -0600 Subject: [PATCH 11/26] Simplify getProjection method --- packages/mongodb/src/adapter.ts | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/packages/mongodb/src/adapter.ts b/packages/mongodb/src/adapter.ts index 13c79b2d63..6ba70f5785 100644 --- a/packages/mongodb/src/adapter.ts +++ b/packages/mongodb/src/adapter.ts @@ -159,7 +159,10 @@ export class MongoDbAdapter< return pipeline } - getProjection(select: string[] | { [key: string]: number }) { + getProjection(select?: string[] | { [key: string]: number }) { + if (!select) { + return undefined + } if (Array.isArray(select)) { if (!select.includes(this.id)) { select = [this.id, ...select] @@ -208,10 +211,9 @@ export class MongoDbAdapter< query, filters: { $select } } = this.filterQuery(id, params) - const projection = $select ? this.getProjection($select) : {} const findOptions: FindOptions = { ...params.mongodb, - projection + projection: this.getProjection($select) } if (params.pipeline) { @@ -307,10 +309,9 @@ export class MongoDbAdapter< return entry } - const projection = params.query?.$select ? this.getProjection(params.query.$select) : {} const findOptions: FindOptions = { ...params.mongodb, - projection + projection: this.getProjection(params.query?.$select) } const promise = Array.isArray(data) @@ -385,12 +386,10 @@ export class MongoDbAdapter< return this._findOrGet(id, findParams).catch(errorHandler) } - const projection = $select ? this.getProjection($select) : {} - const result = await model.findOneAndUpdate(query, replacement, { ...(params.mongodb as FindOneAndUpdateOptions), returnDocument: 'after', - projection + projection: this.getProjection($select) }) if (result.value === null) { @@ -411,12 +410,10 @@ export class MongoDbAdapter< } = this.filterQuery(id, params) const model = await this.getModel(params) - const projection = $select ? this.getProjection($select) : {} - const result = await model.findOneAndReplace(query, this.normalizeId(id, data), { ...(params.mongodb as FindOneAndReplaceOptions), returnDocument: 'after', - projection + projection: this.getProjection($select) }) if (result.value === null) { From c0e3c05ff914e35d9592cea481d6ec8aee85cf66 Mon Sep 17 00:00:00 2001 From: DaddyWarbucks Date: Tue, 5 Dec 2023 02:12:55 -0600 Subject: [PATCH 12/26] Start working on pipeline count, add pipeline to _update --- packages/mongodb/src/adapter.ts | 89 +++++++++++++++++++++++++-------- 1 file changed, 68 insertions(+), 21 deletions(-) diff --git a/packages/mongodb/src/adapter.ts b/packages/mongodb/src/adapter.ts index 6ba70f5785..9c9c7a6861 100644 --- a/packages/mongodb/src/adapter.ts +++ b/packages/mongodb/src/adapter.ts @@ -126,6 +126,7 @@ export class MongoDbAdapter< return q } + /* TODO: Remove $out and $merge stages, else it returns an empty cursor. I think its same to assume this is primarily for querying. */ async aggregateRaw(params: ServiceParams) { const model = await this.getModel(params) const pipeline = params.pipeline || [] @@ -156,6 +157,7 @@ export class MongoDbAdapter< if (filters.$limit !== undefined) { pipeline.push({ $limit: filters.$limit }) } + return pipeline } @@ -206,6 +208,20 @@ export class MongoDbAdapter< return data } + async countDocuments(params: ServiceParams) { + const { paginate, useEstimatedDocumentCount } = this.getOptions(params) + const { query } = this.filterQuery(null, params) + if (paginate && paginate.default) { + const model = await this.getModel(params) + if (useEstimatedDocumentCount && typeof model.estimatedDocumentCount === 'function') { + return model.estimatedDocumentCount() + } else { + return model.countDocuments(query, params.mongodb) + } + } + return Promise.resolve(0) + } + async _get(id: AdapterId, params: ServiceParams = {} as ServiceParams): Promise { const { query, @@ -223,8 +239,6 @@ export class MongoDbAdapter< query: { ...params.query, $limit: 1, - // TODO: Do we need this $and like its needed in find? - // Or should it just be `[this.id]: this.getObjectId(id)` $and: (params.query.$and || []).concat({ [this.id]: this.getObjectId(id) }) @@ -258,31 +272,52 @@ export class MongoDbAdapter< async _find(params?: ServiceParams & { paginate: false }): Promise async _find(params?: ServiceParams): Promise | Result[]> async _find(params: ServiceParams = {} as ServiceParams): Promise | Result[]> { - const { paginate, useEstimatedDocumentCount } = this.getOptions(params) - const { filters, query } = this.filterQuery(null, params) - const useAggregation = !params.mongodb && filters.$limit !== 0 - const countDocuments = async () => { - if (paginate && paginate.default) { - const model = await this.getModel(params) - if (useEstimatedDocumentCount && typeof model.estimatedDocumentCount === 'function') { - return model.estimatedDocumentCount() - } else { - return model.countDocuments(query, { ...params.mongodb }) + const { paginate } = this.getOptions(params) + const { filters } = this.filterQuery(null, params) + + // TODO: Handle accurate aggregation count + if (params.pipeline) { + if (filters.$limit === 0) { + const page = { + total: await this.countDocuments(params), + limit: filters.$limit, + skip: filters.$skip || 0, + data: [] as Result[] } + + return paginate && paginate.default ? page : page.data + } + + const [data, total] = await Promise.all([this.aggregateRaw(params), this.countDocuments(params)]) + + const page = { + total, + limit: filters.$limit, + skip: filters.$skip || 0, + data: (await data.toArray()) as unknown as Result[] + } + + return paginate && paginate.default ? page : page.data + } + + if (filters.$limit === 0) { + const page = { + total: await this.countDocuments(params), + limit: filters.$limit, + skip: filters.$skip || 0, + data: [] as Result[] } - return Promise.resolve(0) + + return paginate && paginate.default ? page : page.data } - const [request, total] = await Promise.all([ - useAggregation ? this.aggregateRaw(params) : this.findRaw(params), - countDocuments() - ]) + const [data, total] = await Promise.all([this.findRaw(params), this.countDocuments(params)]) const page = { total, limit: filters.$limit, skip: filters.$skip || 0, - data: filters.$limit === 0 ? [] : ((await request.toArray()) as any as Result[]) + data: (await data.toArray()) as unknown as Result[] } return paginate && paginate.default ? page : page.data @@ -409,12 +444,24 @@ export class MongoDbAdapter< filters: { $select } } = this.filterQuery(id, params) const model = await this.getModel(params) - - const result = await model.findOneAndReplace(query, this.normalizeId(id, data), { + const findOptions: FindOneAndReplaceOptions = { ...(params.mongodb as FindOneAndReplaceOptions), returnDocument: 'after', projection: this.getProjection($select) - }) + } + + if (params.pipeline) { + return this._get(id, params) + .then(async (result) => { + await model.replaceOne({ [this.id]: id }, findOptions) + return result + }) + .catch(errorHandler) + } + + const result = await model + .findOneAndReplace(query, this.normalizeId(id, data), findOptions) + .catch(errorHandler) if (result.value === null) { throw new NotFound(`No record found for id '${id}'`) From 55100a451c605c167b9ceb9e728de2ecc7da1f7d Mon Sep 17 00:00:00 2001 From: DaddyWarbucks Date: Tue, 5 Dec 2023 03:44:19 -0600 Subject: [PATCH 13/26] Improve _update and _get, add tests --- packages/mongodb/src/adapter.ts | 83 ++++++++++++++++++----------- packages/mongodb/test/index.test.ts | 61 ++++++++++++++++++--- 2 files changed, 107 insertions(+), 37 deletions(-) diff --git a/packages/mongodb/src/adapter.ts b/packages/mongodb/src/adapter.ts index 9c9c7a6861..39f355acb8 100644 --- a/packages/mongodb/src/adapter.ts +++ b/packages/mongodb/src/adapter.ts @@ -9,7 +9,8 @@ import { ReplaceOptions, FindOneAndReplaceOptions, FindOneAndUpdateOptions, - Document + Document, + FindOneAndDeleteOptions } from 'mongodb' import { BadRequest, MethodNotAllowed, NotFound } from '@feathersjs/errors' import { _ } from '@feathersjs/commons' @@ -41,6 +42,7 @@ export interface MongoDBAdapterParams | CountDocumentsOptions | ReplaceOptions | FindOneAndReplaceOptions + | FindOneAndDeleteOptions } export type AdapterId = Id | ObjectId @@ -227,10 +229,6 @@ export class MongoDbAdapter< query, filters: { $select } } = this.filterQuery(id, params) - const findOptions: FindOptions = { - ...params.mongodb, - projection: this.getProjection($select) - } if (params.pipeline) { /* We wouldn't need this aggregateParams if aggregateRaw took signature aggregateRaw(id, params) instead of just aggregateRaw(params). Because aggregateRaw ultimately calls makeFeathersPipeline(params) without id. That also makes aggregateRaw more flexible and consistent with other methods like _findOrGet. But, its a breaking change. */ @@ -256,6 +254,11 @@ export class MongoDbAdapter< .catch(errorHandler) } + const findOptions: FindOptions = { + ...params.mongodb, + projection: this.getProjection($select) + } + return this.getModel(params) .then((model) => model.findOne(query, findOptions)) .then((result) => { @@ -444,24 +447,34 @@ export class MongoDbAdapter< filters: { $select } } = this.filterQuery(id, params) const model = await this.getModel(params) - const findOptions: FindOneAndReplaceOptions = { - ...(params.mongodb as FindOneAndReplaceOptions), - returnDocument: 'after', - projection: this.getProjection($select) - } + const replacement = this.normalizeId(id, data) if (params.pipeline) { + const { query: findQuery } = this.filterQuery(null, params) + + if (Object.keys(findQuery).length === 0) { + await model.replaceOne({ [this.id]: id }, replacement, params.mongodb) + return this._get(id, params) + } + return this._get(id, params) - .then(async (result) => { - await model.replaceOne({ [this.id]: id }, findOptions) - return result + .then(async () => { + await model.replaceOne({ [this.id]: id }, replacement, params.mongodb) + return this._get(id, { + ...params, + query: { $select } + }) }) .catch(errorHandler) } - const result = await model - .findOneAndReplace(query, this.normalizeId(id, data), findOptions) - .catch(errorHandler) + const replaceOptions: FindOneAndReplaceOptions = { + ...(params.mongodb as FindOneAndReplaceOptions), + returnDocument: 'after', + projection: this.getProjection($select) + } + + const result = await model.findOneAndReplace(query, replacement, replaceOptions).catch(errorHandler) if (result.value === null) { throw new NotFound(`No record found for id '${id}'`) @@ -482,18 +495,10 @@ export class MongoDbAdapter< } const model = await this.getModel(params) - const { - query, - filters: { $select, $sort } - } = this.filterQuery(id, params) + const { query } = this.filterQuery(id, params) const findParams = { ...params, - paginate: false, - query: { - ...query, - $select, - $sort - } + paginate: false } if (id === null) { @@ -506,10 +511,28 @@ export class MongoDbAdapter< .catch(errorHandler) } - return this._get(id, findParams) - .then(async (result) => { - await model.deleteOne({ [this.id]: id }, params.mongodb) - return result + if (params.pipeline) { + return this._get(id, params) + .then(async (result) => { + await model.deleteOne({ [this.id]: id }, params.mongodb) + return result + }) + .catch(errorHandler) + } + + const deleteOptions = { + ...(params.mongodb as FindOneAndDeleteOptions), + projection: this.getProjection(params.query?.$select) + } + + return model + .findOneAndDelete(query, deleteOptions) + .then((result) => { + if (result.value === null) { + throw new NotFound(`No record found for id '${id}'`) + } + + return result.value as Result }) .catch(errorHandler) } diff --git a/packages/mongodb/test/index.test.ts b/packages/mongodb/test/index.test.ts index 289b52689d..939c026985 100644 --- a/packages/mongodb/test/index.test.ts +++ b/packages/mongodb/test/index.test.ts @@ -477,18 +477,65 @@ describe('Feathers MongoDB Service', () => { it('can use aggregation in _get', async () => { const dave = await app.service('people').create({ name: 'Dave', age: 25 }) - const result1 = await app.service('people').get(dave._id, { - query: { $select: ['name'] } - }) - const result2 = await app.service('people').get(dave._id, { - query: { $select: ['name'] }, - pipeline: [] + const result = await app.service('people').get(dave._id, { + pipeline: [{ $addFields: { aggregation: true } }] }) - assert.deepStrictEqual(result1, result2) + assert.deepStrictEqual(result, { ...dave, aggregation: true }) + + app.service('people').remove(dave._id) + }) + + it('can use aggregation in _update', async () => { + const dave = await app.service('people').create({ name: 'Dave' }) + const result = await app.service('people').update( + dave._id, + { + name: 'Marshal' + }, + { + pipeline: [{ $addFields: { aggregation: true } }] + } + ) + + assert.deepStrictEqual(result, { ...dave, name: 'Marshal', aggregation: true }) app.service('people').remove(dave._id) }) + + it('can use aggregation and query in _update', async () => { + const dave = await app.service('people').create({ name: 'Dave' }) + const result = await app.service('people').update( + dave._id, + { + name: 'Marshal' + }, + { + query: { name: 'Dave' }, + pipeline: [{ $addFields: { aggregation: true } }] + } + ) + + assert.deepStrictEqual(result, { ...dave, name: 'Marshal', aggregation: true }) + + app.service('people').remove(dave._id) + }) + + it('can use aggregation in _remove', async () => { + const dave = await app.service('people').create({ name: 'Dave' }) + const result = await app.service('people').remove(dave._id, { + pipeline: [{ $addFields: { aggregation: true } }] + }) + + assert.deepStrictEqual(result, { ...dave, aggregation: true }) + + try { + await await app.service('people').get(dave._id) + throw new Error('Should never get here') + } catch (error: any) { + assert.strictEqual(error.name, 'NotFound', 'Got a NotFound Feathers error') + } + }) }) describe('query validation', () => { From d0b5b11fe3de27742cf702fc24bc4d970e612e6b Mon Sep 17 00:00:00 2001 From: DaddyWarbucks Date: Tue, 5 Dec 2023 03:51:52 -0600 Subject: [PATCH 14/26] Cleanup formatting --- packages/mongodb/src/adapter.ts | 35 +++++++++++++++++++-------------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/packages/mongodb/src/adapter.ts b/packages/mongodb/src/adapter.ts index 39f355acb8..df80821a04 100644 --- a/packages/mongodb/src/adapter.ts +++ b/packages/mongodb/src/adapter.ts @@ -424,17 +424,21 @@ export class MongoDbAdapter< return this._findOrGet(id, findParams).catch(errorHandler) } - const result = await model.findOneAndUpdate(query, replacement, { + const updateOptions: FindOneAndUpdateOptions = { ...(params.mongodb as FindOneAndUpdateOptions), returnDocument: 'after', projection: this.getProjection($select) - }) - - if (result.value === null) { - throw new NotFound(`No record found for id '${id}'`) } - return result.value as Result + return model + .findOneAndUpdate(query, replacement, updateOptions) + .then((result) => { + if (result.value === null) { + throw new NotFound(`No record found for id '${id}'`) + } + return result.value as Result + }) + .catch(errorHandler) } async _update(id: AdapterId, data: Data, params: ServiceParams = {} as ServiceParams): Promise { @@ -474,13 +478,15 @@ export class MongoDbAdapter< projection: this.getProjection($select) } - const result = await model.findOneAndReplace(query, replacement, replaceOptions).catch(errorHandler) - - if (result.value === null) { - throw new NotFound(`No record found for id '${id}'`) - } - - return result.value as Result + return model + .findOneAndReplace(query, replacement, replaceOptions) + .then((result) => { + if (result.value === null) { + throw new NotFound(`No record found for id '${id}'`) + } + return result.value as Result + }) + .catch(errorHandler) } async _remove(id: null, params?: ServiceParams): Promise @@ -520,7 +526,7 @@ export class MongoDbAdapter< .catch(errorHandler) } - const deleteOptions = { + const deleteOptions: FindOneAndDeleteOptions = { ...(params.mongodb as FindOneAndDeleteOptions), projection: this.getProjection(params.query?.$select) } @@ -531,7 +537,6 @@ export class MongoDbAdapter< if (result.value === null) { throw new NotFound(`No record found for id '${id}'`) } - return result.value as Result }) .catch(errorHandler) From ca5917836413377ae658e734e56423afff2b863e Mon Sep 17 00:00:00 2001 From: DaddyWarbucks Date: Tue, 5 Dec 2023 04:25:34 -0600 Subject: [PATCH 15/26] Better pipeline in _patch --- packages/mongodb/src/adapter.ts | 50 +++++++++++++++++++++++---------- 1 file changed, 35 insertions(+), 15 deletions(-) diff --git a/packages/mongodb/src/adapter.ts b/packages/mongodb/src/adapter.ts index df80821a04..05d64b21e4 100644 --- a/packages/mongodb/src/adapter.ts +++ b/packages/mongodb/src/adapter.ts @@ -381,7 +381,7 @@ export class MongoDbAdapter< const model = await this.getModel(params) const { query, - filters: { $select } + filters: { $sort, $select } } = this.filterQuery(id, params) const replacement = Object.keys(data).reduce((current, key) => { @@ -400,28 +400,48 @@ export class MongoDbAdapter< }, {} as any) if (id === null) { - const originalIds = await this._findOrGet(id, { - ...params, - query: { - ...query, - $select: [this.id] - }, - paginate: false - }) - const items = Array.isArray(originalIds) ? originalIds : [originalIds] - const idList = items.map((item: any) => item[this.id]) const findParams = { ...params, paginate: false, query: { - [this.id]: { $in: idList }, - $select + ...params.query, + $select: [this.id] } } - await model.updateMany(query, replacement, params.mongodb) + return this._find(findParams) + .then(async (result) => { + const idList = (result as Result[]).map((item: any) => item[this.id]) + await model.updateMany({ [this.id]: { $in: idList } }, replacement, params.mongodb) + return this._find({ + ...findParams, + query: { + [this.id]: { $in: idList }, + $sort, + $select + } + }) + }) + .catch(errorHandler) + } - return this._findOrGet(id, findParams).catch(errorHandler) + if (params.pipeline) { + const { query: findQuery } = this.filterQuery(null, params) + + if (Object.keys(findQuery).length === 0) { + await model.updateOne({ [this.id]: id }, replacement, params.mongodb) + return this._get(id, params) + } + + return this._get(id, params) + .then(async () => { + await model.updateOne({ [this.id]: id }, replacement, params.mongodb) + return this._get(id, { + ...params, + query: { $select } + }) + }) + .catch(errorHandler) } const updateOptions: FindOneAndUpdateOptions = { From f77ff988c053690276d9767ca6ff747f9224a849 Mon Sep 17 00:00:00 2001 From: DaddyWarbucks Date: Sat, 9 Dec 2023 00:33:17 -0600 Subject: [PATCH 16/26] Count documents --- packages/mongodb/src/adapter.ts | 99 ++++++++++++++++------------- packages/mongodb/test/index.test.ts | 18 ++++++ 2 files changed, 73 insertions(+), 44 deletions(-) diff --git a/packages/mongodb/src/adapter.ts b/packages/mongodb/src/adapter.ts index 05d64b21e4..bdbf0896db 100644 --- a/packages/mongodb/src/adapter.ts +++ b/packages/mongodb/src/adapter.ts @@ -49,6 +49,11 @@ export type AdapterId = Id | ObjectId export type NullableAdapterId = AdapterId | null +type Page = { + total: number + data: any +} + // Create the service. export class MongoDbAdapter< Result, @@ -109,14 +114,14 @@ export class MongoDbAdapter< const model = await this.getModel(params) const q = model.find(query, params.mongodb) - if (filters.$select !== undefined) { - q.project(this.getProjection(filters.$select)) - } - if (filters.$sort !== undefined) { q.sort(filters.$sort) } + if (filters.$select !== undefined) { + q.project(this.getProjection(filters.$select)) + } + if (filters.$skip !== undefined) { q.skip(filters.$skip) } @@ -211,17 +216,31 @@ export class MongoDbAdapter< } async countDocuments(params: ServiceParams) { - const { paginate, useEstimatedDocumentCount } = this.getOptions(params) + const { useEstimatedDocumentCount } = this.getOptions(params) const { query } = this.filterQuery(null, params) - if (paginate && paginate.default) { - const model = await this.getModel(params) - if (useEstimatedDocumentCount && typeof model.estimatedDocumentCount === 'function') { - return model.estimatedDocumentCount() - } else { - return model.countDocuments(query, params.mongodb) + + if (params.pipeline) { + const aggregateParams = { + ...params, + query: { + ...params.query, + $select: [this.id], + $sort: undefined, + $skip: undefined, + $limit: undefined + } } + const result = await this.aggregateRaw(aggregateParams).then((result) => result.toArray()) + return result.length + } + + const model = await this.getModel(params) + + if (useEstimatedDocumentCount && typeof model.estimatedDocumentCount === 'function') { + return model.estimatedDocumentCount() } - return Promise.resolve(0) + + return model.countDocuments(query, params.mongodb) } async _get(id: AdapterId, params: ServiceParams = {} as ServiceParams): Promise { @@ -278,52 +297,44 @@ export class MongoDbAdapter< const { paginate } = this.getOptions(params) const { filters } = this.filterQuery(null, params) - // TODO: Handle accurate aggregation count - if (params.pipeline) { - if (filters.$limit === 0) { - const page = { - total: await this.countDocuments(params), + const page = ({ total, data }: Page) => { + if (paginate && paginate.default) { + return { limit: filters.$limit, skip: filters.$skip || 0, - data: [] as Result[] + total, + data } - - return paginate && paginate.default ? page : page.data - } - - const [data, total] = await Promise.all([this.aggregateRaw(params), this.countDocuments(params)]) - - const page = { - total, - limit: filters.$limit, - skip: filters.$skip || 0, - data: (await data.toArray()) as unknown as Result[] } - - return paginate && paginate.default ? page : page.data + return data } if (filters.$limit === 0) { - const page = { + return page({ total: await this.countDocuments(params), - limit: filters.$limit, - skip: filters.$skip || 0, data: [] as Result[] - } - - return paginate && paginate.default ? page : page.data + }) } - const [data, total] = await Promise.all([this.findRaw(params), this.countDocuments(params)]) + const result = params.pipeline ? this.aggregateRaw(params) : this.findRaw(params) - const page = { - total, - limit: filters.$limit, - skip: filters.$skip || 0, - data: (await data.toArray()) as unknown as Result[] + if (params.paginate === false) { + const data = await result.then((result) => result.toArray()) + return page({ + total: data.length, + data: data as Result[] + }) } - return paginate && paginate.default ? page : page.data + const [data, total] = await Promise.all([ + result.then((result) => result.toArray()), + this.countDocuments(params) + ]) + + return page({ + total, + data: data as Result[] + }) } async _create(data: Data, params?: ServiceParams): Promise diff --git a/packages/mongodb/test/index.test.ts b/packages/mongodb/test/index.test.ts index 939c026985..2cff0e04c4 100644 --- a/packages/mongodb/test/index.test.ts +++ b/packages/mongodb/test/index.test.ts @@ -83,6 +83,11 @@ const testSuite = adapterTests([ 'params.adapter + multi' ]) +const defaultPaginate = { + default: 10, + max: 50 +} + describe('Feathers MongoDB Service', () => { const personSchema = { $id: 'Person', @@ -475,6 +480,19 @@ describe('Feathers MongoDB Service', () => { assert.equal(result.length, 1) }) + it('can count documents with aggregation', async () => { + const service = app.service('people') + const paginateBefore = service.options.paginate + service.options.paginate = defaultPaginate + const query = { age: { $gte: 25 } } + const findResult = await app.service('people').find({ query }) + const aggregationResult = await app.service('people').find({ query, pipeline: [] }) + + assert.deepStrictEqual(findResult, aggregationResult) + + service.options.paginate = paginateBefore + }) + it('can use aggregation in _get', async () => { const dave = await app.service('people').create({ name: 'Dave', age: 25 }) const result = await app.service('people').get(dave._id, { From 64ee3997dc9ba3f6ca3e511ca9fb4ebf6e9baa57 Mon Sep 17 00:00:00 2001 From: DaddyWarbucks Date: Sat, 9 Dec 2023 01:26:58 -0600 Subject: [PATCH 17/26] Cleanup comments --- packages/mongodb/src/adapter.ts | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/packages/mongodb/src/adapter.ts b/packages/mongodb/src/adapter.ts index bdbf0896db..65d472b616 100644 --- a/packages/mongodb/src/adapter.ts +++ b/packages/mongodb/src/adapter.ts @@ -309,6 +309,7 @@ export class MongoDbAdapter< return data } + /* TODO: This seems a little funky. The user shouldn't need to have service.options.paginate to count documents necessarily. Shouldn't this just always return { total: 123 } if no pagination option? Else it always return an empty array. Or we can keep it this way, and document that you can use `this.countDocuments` if you don't have paginate option. Just seems weird. */ if (filters.$limit === 0) { return page({ total: await this.countDocuments(params), @@ -320,10 +321,12 @@ export class MongoDbAdapter< if (params.paginate === false) { const data = await result.then((result) => result.toArray()) - return page({ - total: data.length, - data: data as Result[] - }) + return data as Result[] + /* TODO: Wait...how does the below code work? Is there somewhere else that is handling params.paginate only returning an array? Comment the return statement above and uncomment this block...shouldn't that throw an error? */ + // return page({ + // total: data.length, + // data: data as Result[] + // }) } const [data, total] = await Promise.all([ @@ -348,7 +351,6 @@ export class MongoDbAdapter< const setId = (item: any) => { const entry = Object.assign({}, item) - // Generate a MongoId if we use a custom id if (this.id !== '_id' && typeof entry[this.id] === 'undefined') { return { [this.id]: new ObjectId().toHexString(), @@ -573,10 +575,3 @@ export class MongoDbAdapter< .catch(errorHandler) } } - -// function hasAggregation(params: AdapterParams) { -// if (!params.query || Object.keys(params.query).length === 0) { -// return false -// } -// return true -// } From ced2cdc94f598abd6fff3cc24192cec6cebf22a8 Mon Sep 17 00:00:00 2001 From: DaddyWarbucks Date: Wed, 24 Jan 2024 15:54:49 -0600 Subject: [PATCH 18/26] Fix count and pagination --- packages/mongodb/src/adapter.ts | 91 ++++++++++++++------------------- 1 file changed, 39 insertions(+), 52 deletions(-) diff --git a/packages/mongodb/src/adapter.ts b/packages/mongodb/src/adapter.ts index 65d472b616..f57e9403b3 100644 --- a/packages/mongodb/src/adapter.ts +++ b/packages/mongodb/src/adapter.ts @@ -49,11 +49,6 @@ export type AdapterId = Id | ObjectId export type NullableAdapterId = AdapterId | null -type Page = { - total: number - data: any -} - // Create the service. export class MongoDbAdapter< Result, @@ -133,7 +128,7 @@ export class MongoDbAdapter< return q } - /* TODO: Remove $out and $merge stages, else it returns an empty cursor. I think its same to assume this is primarily for querying. */ + /* TODO: Remove $out and $merge stages, else it returns an empty cursor. I think its safe to assume this is primarily for querying. */ async aggregateRaw(params: ServiceParams) { const model = await this.getModel(params) const pipeline = params.pipeline || [] @@ -250,7 +245,6 @@ export class MongoDbAdapter< } = this.filterQuery(id, params) if (params.pipeline) { - /* We wouldn't need this aggregateParams if aggregateRaw took signature aggregateRaw(id, params) instead of just aggregateRaw(params). Because aggregateRaw ultimately calls makeFeathersPipeline(params) without id. That also makes aggregateRaw more flexible and consistent with other methods like _findOrGet. But, its a breaking change. */ const aggregateParams = { ...params, query: { @@ -296,48 +290,38 @@ export class MongoDbAdapter< async _find(params: ServiceParams = {} as ServiceParams): Promise | Result[]> { const { paginate } = this.getOptions(params) const { filters } = this.filterQuery(null, params) + const paginationDisabled = params.paginate === false || !paginate || !paginate.default - const page = ({ total, data }: Page) => { - if (paginate && paginate.default) { - return { - limit: filters.$limit, - skip: filters.$skip || 0, - total, - data - } + const getData = () => { + const result = params.pipeline ? this.aggregateRaw(params) : this.findRaw(params) + return result.then((result) => result.toArray()) + } + + if (paginationDisabled) { + if (filters.$limit === 0) { + return [] as Result[] } - return data + const data = await getData() + return data as Result[] } - /* TODO: This seems a little funky. The user shouldn't need to have service.options.paginate to count documents necessarily. Shouldn't this just always return { total: 123 } if no pagination option? Else it always return an empty array. Or we can keep it this way, and document that you can use `this.countDocuments` if you don't have paginate option. Just seems weird. */ if (filters.$limit === 0) { - return page({ + return { total: await this.countDocuments(params), - data: [] as Result[] - }) - } - - const result = params.pipeline ? this.aggregateRaw(params) : this.findRaw(params) - - if (params.paginate === false) { - const data = await result.then((result) => result.toArray()) - return data as Result[] - /* TODO: Wait...how does the below code work? Is there somewhere else that is handling params.paginate only returning an array? Comment the return statement above and uncomment this block...shouldn't that throw an error? */ - // return page({ - // total: data.length, - // data: data as Result[] - // }) + data: [] as Result[], + limit: filters.$limit, + skip: filters.$skip || 0 + } } - const [data, total] = await Promise.all([ - result.then((result) => result.toArray()), - this.countDocuments(params) - ]) + const [data, total] = await Promise.all([getData(), this.countDocuments(params)]) - return page({ + return { total, - data: data as Result[] - }) + data: data as Result[], + limit: filters.$limit, + skip: filters.$skip || 0 + } } async _create(data: Data, params?: ServiceParams): Promise @@ -427,7 +411,8 @@ export class MongoDbAdapter< const idList = (result as Result[]).map((item: any) => item[this.id]) await model.updateMany({ [this.id]: { $in: idList } }, replacement, params.mongodb) return this._find({ - ...findParams, + ...params, + paginate: false, query: { [this.id]: { $in: idList }, $sort, @@ -439,14 +424,15 @@ export class MongoDbAdapter< } if (params.pipeline) { - const { query: findQuery } = this.filterQuery(null, params) - - if (Object.keys(findQuery).length === 0) { - await model.updateOne({ [this.id]: id }, replacement, params.mongodb) - return this._get(id, params) + const getParams = { + ...params, + query: { + ...params.query, + $select: [this.id] + } } - return this._get(id, params) + return this._get(id, getParams) .then(async () => { await model.updateOne({ [this.id]: id }, replacement, params.mongodb) return this._get(id, { @@ -487,14 +473,15 @@ export class MongoDbAdapter< const replacement = this.normalizeId(id, data) if (params.pipeline) { - const { query: findQuery } = this.filterQuery(null, params) - - if (Object.keys(findQuery).length === 0) { - await model.replaceOne({ [this.id]: id }, replacement, params.mongodb) - return this._get(id, params) + const getParams = { + ...params, + query: { + ...params.query, + $select: [this.id] + } } - return this._get(id, params) + return this._get(id, getParams) .then(async () => { await model.replaceOne({ [this.id]: id }, replacement, params.mongodb) return this._get(id, { From 06790a0dc9b5d9dae3f61acf7f5ee3eb4abca991 Mon Sep 17 00:00:00 2001 From: DaddyWarbucks Date: Wed, 24 Jan 2024 16:03:53 -0600 Subject: [PATCH 19/26] Add tests --- packages/mongodb/test/index.test.ts | 37 ++++++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/packages/mongodb/test/index.test.ts b/packages/mongodb/test/index.test.ts index 2cff0e04c4..25e680e054 100644 --- a/packages/mongodb/test/index.test.ts +++ b/packages/mongodb/test/index.test.ts @@ -488,7 +488,7 @@ describe('Feathers MongoDB Service', () => { const findResult = await app.service('people').find({ query }) const aggregationResult = await app.service('people').find({ query, pipeline: [] }) - assert.deepStrictEqual(findResult, aggregationResult) + assert.deepStrictEqual(findResult.total, aggregationResult.total) service.options.paginate = paginateBefore }) @@ -521,6 +521,23 @@ describe('Feathers MongoDB Service', () => { app.service('people').remove(dave._id) }) + it('can use aggregation in _patch', async () => { + const dave = await app.service('people').create({ name: 'Dave' }) + const result = await app.service('people').patch( + dave._id, + { + name: 'Marshal' + }, + { + pipeline: [{ $addFields: { aggregation: true } }] + } + ) + + assert.deepStrictEqual(result, { ...dave, name: 'Marshal', aggregation: true }) + + app.service('people').remove(dave._id) + }) + it('can use aggregation and query in _update', async () => { const dave = await app.service('people').create({ name: 'Dave' }) const result = await app.service('people').update( @@ -539,6 +556,24 @@ describe('Feathers MongoDB Service', () => { app.service('people').remove(dave._id) }) + it('can use aggregation and query in _patch', async () => { + const dave = await app.service('people').create({ name: 'Dave' }) + const result = await app.service('people').patch( + dave._id, + { + name: 'Marshal' + }, + { + query: { name: 'Dave' }, + pipeline: [{ $addFields: { aggregation: true } }] + } + ) + + assert.deepStrictEqual(result, { ...dave, name: 'Marshal', aggregation: true }) + + app.service('people').remove(dave._id) + }) + it('can use aggregation in _remove', async () => { const dave = await app.service('people').create({ name: 'Dave' }) const result = await app.service('people').remove(dave._id, { From cd067fdf0ead51a7f9f83fb7827f748d1b6dfee0 Mon Sep 17 00:00:00 2001 From: DaddyWarbucks Date: Sat, 30 Mar 2024 21:08:46 -0500 Subject: [PATCH 20/26] Use aggregation in create --- packages/mongodb/src/adapter.ts | 79 ++++++++++++++++++----------- packages/mongodb/test/index.test.ts | 59 +++++++++++++++++++++ 2 files changed, 107 insertions(+), 31 deletions(-) diff --git a/packages/mongodb/src/adapter.ts b/packages/mongodb/src/adapter.ts index f57e9403b3..ac89400b17 100644 --- a/packages/mongodb/src/adapter.ts +++ b/packages/mongodb/src/adapter.ts @@ -268,8 +268,8 @@ export class MongoDbAdapter< } const findOptions: FindOptions = { - ...params.mongodb, - projection: this.getProjection($select) + projection: this.getProjection($select), + ...params.mongodb } return this.getModel(params) @@ -331,6 +331,10 @@ export class MongoDbAdapter< data: Data | Data[], params: ServiceParams = {} as ServiceParams ): Promise { + if (Array.isArray(data) && !this.allowsMulti('create', params)) { + throw new MethodNotAllowed('Can not create multiple entries') + } + const model = await this.getModel(params) const setId = (item: any) => { const entry = Object.assign({}, item) @@ -344,22 +348,30 @@ export class MongoDbAdapter< return entry } - const findOptions: FindOptions = { - ...params.mongodb, - projection: this.getProjection(params.query?.$select) - } - const promise = Array.isArray(data) - ? model - .insertMany(data.map(setId), params.mongodb) - .then((result) => - model.find({ _id: { $in: Object.values(result.insertedIds) } }, findOptions).toArray() - ) - : model - .insertOne(setId(data), params.mongodb) - .then((result) => model.findOne({ _id: result.insertedId }, findOptions)) + if (Array.isArray(data)) { + const created = await model.insertMany(data.map(setId), params.mongodb).catch(errorHandler) + return this._find({ + ...params, + paginate: false, + query: { + _id: { $in: Object.values(created.insertedIds) }, + $select: params.query?.$select + } + }) + } - return promise.catch(errorHandler) + const created = await model.insertOne(setId(data), params.mongodb).catch(errorHandler) + const result = await this._find({ + ...params, + paginate: false, + query: { + _id: created.insertedId, + $select: params.query?.$select, + $limit: 1 + } + }) + return result[0] } async _patch(id: null, data: PatchData, params?: ServiceParams): Promise @@ -381,20 +393,25 @@ export class MongoDbAdapter< filters: { $sort, $select } } = this.filterQuery(id, params) - const replacement = Object.keys(data).reduce((current, key) => { - const value = (data as any)[key] + const replacement = Object.keys(data).reduce( + (current, key) => { + const value = (data as any)[key] - if (key.charAt(0) !== '$') { - current.$set = { - ...current.$set, - [key]: value + if (key.charAt(0) !== '$') { + current.$set[key] = value + } else if (key === '$set') { + current.$set = { + ...current.$set, + ...value + } + } else { + current[key] = value } - } else { - current[key] = value - } - return current - }, {} as any) + return current + }, + { $set: {} } as any + ) if (id === null) { const findParams = { @@ -444,9 +461,9 @@ export class MongoDbAdapter< } const updateOptions: FindOneAndUpdateOptions = { + projection: this.getProjection($select), ...(params.mongodb as FindOneAndUpdateOptions), - returnDocument: 'after', - projection: this.getProjection($select) + returnDocument: 'after' } return model @@ -493,9 +510,9 @@ export class MongoDbAdapter< } const replaceOptions: FindOneAndReplaceOptions = { + projection: this.getProjection($select), ...(params.mongodb as FindOneAndReplaceOptions), - returnDocument: 'after', - projection: this.getProjection($select) + returnDocument: 'after' } return model diff --git a/packages/mongodb/test/index.test.ts b/packages/mongodb/test/index.test.ts index 25e680e054..a377f9ba6c 100644 --- a/packages/mongodb/test/index.test.ts +++ b/packages/mongodb/test/index.test.ts @@ -504,6 +504,31 @@ describe('Feathers MongoDB Service', () => { app.service('people').remove(dave._id) }) + it('can use aggregation in _create', async () => { + const dave = (await app.service('people').create( + { name: 'Dave' }, + { + pipeline: [{ $addFields: { aggregation: true } }] + } + )) as any + + assert.deepStrictEqual(dave.aggregation, true) + + app.service('people').remove(dave._id) + }) + + it('can use aggregation in multi _create', async () => { + app.service('people').options.multi = true + const dave = (await app.service('people').create([{ name: 'Dave' }], { + pipeline: [{ $addFields: { aggregation: true } }] + })) as any + + assert.deepStrictEqual(dave[0].aggregation, true) + + app.service('people').options.multi = false + app.service('people').remove(dave[0]._id) + }) + it('can use aggregation in _update', async () => { const dave = await app.service('people').create({ name: 'Dave' }) const result = await app.service('people').update( @@ -538,6 +563,26 @@ describe('Feathers MongoDB Service', () => { app.service('people').remove(dave._id) }) + it('can use aggregation in multi _patch', async () => { + app.service('people').options.multi = true + const dave = await app.service('people').create({ name: 'Dave' }) + const result = await app.service('people').patch( + null, + { + name: 'Marshal' + }, + { + query: { _id: dave._id }, + pipeline: [{ $addFields: { aggregation: true } }] + } + ) + + assert.deepStrictEqual(result[0], { ...dave, name: 'Marshal', aggregation: true }) + + app.service('people').options.multi = false + app.service('people').remove(dave._id) + }) + it('can use aggregation and query in _update', async () => { const dave = await app.service('people').create({ name: 'Dave' }) const result = await app.service('people').update( @@ -589,6 +634,20 @@ describe('Feathers MongoDB Service', () => { assert.strictEqual(error.name, 'NotFound', 'Got a NotFound Feathers error') } }) + + it('can use aggregation in multi _remove', async () => { + app.service('people').options.multi = true + const dave = await app.service('people').create({ name: 'Dave' }) + const result = await app.service('people').remove(null, { + query: { _id: dave._id }, + pipeline: [{ $addFields: { aggregation: true } }] + }) + + assert.deepStrictEqual(result[0], { ...dave, aggregation: true }) + + app.service('people').options.multi = false + // app.service('people').remove(dave._id) + }) }) describe('query validation', () => { From a7f683d6a969ac6af20e80378b53c64f8dfc0ce1 Mon Sep 17 00:00:00 2001 From: DaddyWarbucks Date: Sat, 30 Mar 2024 21:31:40 -0500 Subject: [PATCH 21/26] Update docs --- docs/api/databases/mongodb.md | 17 ++++++++--------- packages/mongodb/src/adapter.ts | 1 + 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/docs/api/databases/mongodb.md b/docs/api/databases/mongodb.md index a54876fb7c..dc1d0cf02e 100644 --- a/docs/api/databases/mongodb.md +++ b/docs/api/databases/mongodb.md @@ -66,8 +66,7 @@ MongoDB adapter specific options are: The [common API options](./common.md#options) are: -- `id {string}` (_optional_, default: `'_id'`) - The name of the id field property. By design, MongoDB will always add an `_id` property. -- `id {string}` (_optional_) - The name of the id field property (usually set by default to `id` or `_id`). +- `id {string}` (_optional_, default: `'_id'`) - The name of the id field property. By design, MongoDB will always add an `_id` property. But you can choose to use a different property as your primary key. - `paginate {Object}` (_optional_) - A [pagination object](#pagination) containing a `default` and `max` page size - `multi {string[]|boolean}` (_optional_, default: `false`) - Allow `create` with arrays and `patch` and `remove` with id `null` to change multiple items. Can be `true` for all methods or an array of allowed methods (e.g. `[ 'remove', 'create' ]`) @@ -79,19 +78,19 @@ There are additionally several legacy options in the [common API options](./comm ### aggregateRaw(params) -The `find` method has been split into separate utilities for converting params into different types of MongoDB requests. By default, requests are processed by this method and are run through the MongoDB Aggregation Pipeline. This method returns a raw MongoDB Cursor object, which can be used to perform custom pagination or in custom server scripts, if desired. +The `find` method has been split into separate utilities for converting params into different types of MongoDB requests. When using `params.pipeline`, the `aggregateRaw` method is used to convert the Feathers params into a MongoDB aggregation pipeline with the `model.aggregate` method. This method returns a raw MongoDB Cursor object, which can be used to perform custom pagination or in custom server scripts, if desired. ### findRaw(params) -`findRaw(params)` is used when `params.mongodb` is set to retrieve data using `params.mongodb` as the `FindOptions` object. This method returns a raw MongoDB Cursor object, which can be used to perform custom pagination or in custom server scripts, if desired. +`findRaw(params)` This method is used when there is no `params.pipeline` and uses the common `model.find` method. It returns a raw MongoDB Cursor object, which can be used to perform custom pagination or in custom server scripts, if desired. ### makeFeathersPipeline(params) -`makeFeathersPipeline(params)` takes a set of Feathers params and converts them to a pipeline array, ready to pass to `collection.aggregate`. This utility comprises the bulk of the `aggregateRaw` functionality, but does not use `params.pipeline`. +`makeFeathersPipeline(params)` takes a set of Feathers params and converts them to a pipeline array, ready to pass to `modal.aggregate`. This utility comprises the bulk of the `aggregateRaw` functionality, but does not use `params.pipeline`. ### Custom Params -The `@feathersjs/mongodb` adapter utilizes two custom params which control adapter-specific features: `params.pipeline` and `params.mongodb`. +The `@feathersjs/mongodb` adapter utilizes three custom params which control adapter-specific features: `params.pipeline`, `params.mongodb`, and `params.adapter`. #### params.adapter @@ -99,11 +98,11 @@ Allows to dynamically set the [adapter options](#options) (like the `Model` coll #### params.pipeline -Used for [aggregation pipelines](#aggregation-pipeline). +Used for [aggregation pipelines](#aggregation-pipeline). Whenever this property is set, the adapter will use the `collection.aggregate` method instead of the `collection.find` method. The `pipeline` property should be an array of [aggregation stages](https://www.mongodb.com/docs/manual/reference/operator/aggregation-pipeline/). #### params.mongodb -When making a [service method](/api/services.md) call, `params` can contain an`mongodb` property (for example, `{upsert: true}`) which allows modifying the options used to run the MongoDB query. The adapter will use the `collection.find` method and not the [aggregation pipeline](#aggregation-pipeline) when you use `params.mongodb`. +When making a [service method](/api/services.md) call, `params` can contain an`mongodb` property (for example, `{ upsert: true }`) which allows modifying the options used to run the MongoDB query. This param can be used for both find and aggregation queries. ## Transactions @@ -198,7 +197,7 @@ See the MongoDB documentation for instructions on performing full-text search us ## Aggregation Pipeline -In Feathers v5 Dove, we added support for the full power of MongoDB's Aggregation Framework and blends it seamlessly with the familiar Feathers Query syntax. All `find` queries now use the Aggregation Framework, by default. +In Feathers v5 Dove, we added support for the full power of MongoDB's Aggregation Framework and blends it seamlessly with the familiar Feathers Query syntax. The `find` method automatically uses the aggregation pipeline when `params.pipeline` is set. The Aggregation Framework is accessed through the mongoClient's `collection.aggregate` method, which accepts an array of "stages". Each stage contains an operator which describes an operation to apply to the previous step's data. Each stage applies the operation to the results of the previous step. It’s now possible to perform any of the [Aggregation Stages](https://www.mongodb.com/docs/upcoming/reference/operator/aggregation-pipeline/) like `$lookup` and `$unwind`, integration with the normal Feathers queries. diff --git a/packages/mongodb/src/adapter.ts b/packages/mongodb/src/adapter.ts index ac89400b17..6cc2e3a7b2 100644 --- a/packages/mongodb/src/adapter.ts +++ b/packages/mongodb/src/adapter.ts @@ -255,6 +255,7 @@ export class MongoDbAdapter< }) } } + return this.aggregateRaw(aggregateParams) .then((result) => result.toArray()) .then(([result]) => { From 4f52ad82b15faf9322becf51c1e462ced32ed44d Mon Sep 17 00:00:00 2001 From: DaddyWarbucks Date: Sat, 30 Mar 2024 21:54:16 -0500 Subject: [PATCH 22/26] Add test --- packages/mongodb/src/adapter.ts | 2 ++ packages/mongodb/test/index.test.ts | 22 ++++++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/packages/mongodb/src/adapter.ts b/packages/mongodb/src/adapter.ts index 6cc2e3a7b2..931a170aca 100644 --- a/packages/mongodb/src/adapter.ts +++ b/packages/mongodb/src/adapter.ts @@ -424,6 +424,8 @@ export class MongoDbAdapter< } } + console.log(findParams, params) + return this._find(findParams) .then(async (result) => { const idList = (result as Result[]).map((item: any) => item[this.id]) diff --git a/packages/mongodb/test/index.test.ts b/packages/mongodb/test/index.test.ts index a377f9ba6c..b98e1b4a71 100644 --- a/packages/mongodb/test/index.test.ts +++ b/packages/mongodb/test/index.test.ts @@ -389,6 +389,28 @@ describe('Feathers MongoDB Service', () => { assert.strictEqual(patched[0].friends?.length, 2) }) + it('can use $limit in patch', async () => { + const data = { name: 'ddd' } + const query = { $limit: 1 } + + // TODO: Why is $limit not working? + + const result = await peopleService.patch(null, data, { + query + }) + + assert.strictEqual(result.length, 1) + assert.strictEqual(result[0].name, 'ddd') + + const pipelineResult = await peopleService.patch(null, data, { + pipeline: [], + query + }) + + assert.strictEqual(pipelineResult.length, 1) + assert.strictEqual(pipelineResult[0].name, 'ddd') + }) + it('overrides default index selection using hint param if present', async () => { const indexed = await peopleService.create({ name: 'Indexed', From cf0c9d5f65a19d160129631ccecd0eb3ade8e0e1 Mon Sep 17 00:00:00 2001 From: DaddyWarbucks Date: Thu, 18 Apr 2024 09:09:59 -0500 Subject: [PATCH 23/26] Fix typos --- docs/api/databases/mongodb.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/api/databases/mongodb.md b/docs/api/databases/mongodb.md index dc1d0cf02e..37feb704dd 100644 --- a/docs/api/databases/mongodb.md +++ b/docs/api/databases/mongodb.md @@ -86,7 +86,7 @@ The `find` method has been split into separate utilities for converting params i ### makeFeathersPipeline(params) -`makeFeathersPipeline(params)` takes a set of Feathers params and converts them to a pipeline array, ready to pass to `modal.aggregate`. This utility comprises the bulk of the `aggregateRaw` functionality, but does not use `params.pipeline`. +`makeFeathersPipeline(params)` takes a set of Feathers params and converts them to a pipeline array, ready to pass to `model.aggregate`. This utility comprises the bulk of the `aggregateRaw` functionality, but does not use `params.pipeline`. ### Custom Params @@ -98,7 +98,7 @@ Allows to dynamically set the [adapter options](#options) (like the `Model` coll #### params.pipeline -Used for [aggregation pipelines](#aggregation-pipeline). Whenever this property is set, the adapter will use the `collection.aggregate` method instead of the `collection.find` method. The `pipeline` property should be an array of [aggregation stages](https://www.mongodb.com/docs/manual/reference/operator/aggregation-pipeline/). +Used for [aggregation pipelines](#aggregation-pipeline). Whenever this property is set, the adapter will use the `model.aggregate` method instead of the `model.find` method. The `pipeline` property should be an array of [aggregation stages](https://www.mongodb.com/docs/manual/reference/operator/aggregation-pipeline/). #### params.mongodb @@ -199,7 +199,7 @@ See the MongoDB documentation for instructions on performing full-text search us In Feathers v5 Dove, we added support for the full power of MongoDB's Aggregation Framework and blends it seamlessly with the familiar Feathers Query syntax. The `find` method automatically uses the aggregation pipeline when `params.pipeline` is set. -The Aggregation Framework is accessed through the mongoClient's `collection.aggregate` method, which accepts an array of "stages". Each stage contains an operator which describes an operation to apply to the previous step's data. Each stage applies the operation to the results of the previous step. It’s now possible to perform any of the [Aggregation Stages](https://www.mongodb.com/docs/upcoming/reference/operator/aggregation-pipeline/) like `$lookup` and `$unwind`, integration with the normal Feathers queries. +The Aggregation Framework is accessed through the mongoClient's `model.aggregate` method, which accepts an array of "stages". Each stage contains an operator which describes an operation to apply to the previous step's data. Each stage applies the operation to the results of the previous step. It’s now possible to perform any of the [Aggregation Stages](https://www.mongodb.com/docs/upcoming/reference/operator/aggregation-pipeline/) like `$lookup` and `$unwind`, integration with the normal Feathers queries. Here's how it works with the operators that match the Feathers Query syntax. Let's convert the following Feathers query: From dd6fcc44792e530ba06ed99a49f20d46389d24f9 Mon Sep 17 00:00:00 2001 From: DaddyWarbucks Date: Thu, 25 Apr 2024 18:01:49 -0500 Subject: [PATCH 24/26] Add $sort/$limit tests --- packages/mongodb/src/adapter.ts | 2 - packages/mongodb/test/index.test.ts | 76 +++++++++++++++++++++++++++-- 2 files changed, 72 insertions(+), 6 deletions(-) diff --git a/packages/mongodb/src/adapter.ts b/packages/mongodb/src/adapter.ts index 931a170aca..6cc2e3a7b2 100644 --- a/packages/mongodb/src/adapter.ts +++ b/packages/mongodb/src/adapter.ts @@ -424,8 +424,6 @@ export class MongoDbAdapter< } } - console.log(findParams, params) - return this._find(findParams) .then(async (result) => { const idList = (result as Result[]).map((item: any) => item[this.id]) diff --git a/packages/mongodb/test/index.test.ts b/packages/mongodb/test/index.test.ts index b98e1b4a71..44b09d0b73 100644 --- a/packages/mongodb/test/index.test.ts +++ b/packages/mongodb/test/index.test.ts @@ -393,16 +393,14 @@ describe('Feathers MongoDB Service', () => { const data = { name: 'ddd' } const query = { $limit: 1 } - // TODO: Why is $limit not working? - - const result = await peopleService.patch(null, data, { + const result = await peopleService._patch(null, data, { query }) assert.strictEqual(result.length, 1) assert.strictEqual(result[0].name, 'ddd') - const pipelineResult = await peopleService.patch(null, data, { + const pipelineResult = await peopleService._patch(null, data, { pipeline: [], query }) @@ -411,6 +409,76 @@ describe('Feathers MongoDB Service', () => { assert.strictEqual(pipelineResult[0].name, 'ddd') }) + it('can use $limit in remove', async () => { + const query = { $limit: 1 } + + const result = await peopleService._remove(null, { + query + }) + + assert.strictEqual(result.length, 1) + + const pipelineResult = await peopleService._remove(null, { + pipeline: [], + query + }) + + assert.strictEqual(pipelineResult.length, 1) + }) + + it('can use $sort in patch', async () => { + const updated = await peopleService._patch( + null, + { name: 'ddd' }, + { + query: { $limit: 1, $sort: { name: -1 } } + } + ) + + const result = await peopleService.find({ + paginate: false, + query: { $limit: 1, $sort: { name: -1 } } + }) + + assert.strictEqual(updated.length, 1) + assert.strictEqual(result[0].name, 'ddd') + + const pipelineUpdated = await peopleService._patch( + null, + { name: 'eee' }, + { + pipeline: [], + query: { $limit: 1, $sort: { name: -1 } } + } + ) + + const pipelineResult = await peopleService.find({ + paginate: false, + pipeline: [], + query: { $limit: 1, $sort: { name: -1 } } + }) + + assert.strictEqual(pipelineUpdated.length, 1) + assert.strictEqual(pipelineResult[0].name, 'eee') + }) + + it('can use $sort in remove', async () => { + const removed = await peopleService._remove(null, { + query: { $limit: 1, $sort: { name: -1 } } + }) + + assert.strictEqual(removed.length, 1) + assert.strictEqual(removed[0].name, 'ccc') + + const pipelineRemoved = await peopleService._remove(null, { + pipeline: [], + query: { $limit: 1, $sort: { name: -1 } } + }) + + assert.strictEqual(pipelineRemoved.length, 1) + assert.strictEqual(pipelineRemoved[0].name, 'aaa') + }) + it('overrides default index selection using hint param if present', async () => { const indexed = await peopleService.create({ name: 'Indexed', From 3908eeef511563a61567f27ae6002bdcd9a0d2de Mon Sep 17 00:00:00 2001 From: DaddyWarbucks Date: Fri, 26 Apr 2024 10:35:22 -0500 Subject: [PATCH 25/26] Remove unused method. --- packages/mongodb/src/adapter.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/mongodb/src/adapter.ts b/packages/mongodb/src/adapter.ts index 6cc2e3a7b2..1e9546fd35 100644 --- a/packages/mongodb/src/adapter.ts +++ b/packages/mongodb/src/adapter.ts @@ -190,10 +190,6 @@ export class MongoDbAdapter< return select } - async _findOrGet(id: NullableAdapterId, params: ServiceParams) { - return id === null ? await this._find(params) : await this._get(id, params) - } - normalizeId(id: NullableAdapterId, data: D): D { if (this.id === '_id') { // Default Mongo IDs cannot be updated. The Mongo library handles From 4be451bca022bf2d9b8202eabd5b1646b8f3095f Mon Sep 17 00:00:00 2001 From: DaddyWarbucks Date: Fri, 10 May 2024 11:54:56 -0500 Subject: [PATCH 26/26] Fix tests --- packages/mongodb/src/adapter.ts | 16 +++---- packages/mongodb/test/index.test.ts | 67 ++++++++++++++++++++++++++++- 2 files changed, 73 insertions(+), 10 deletions(-) diff --git a/packages/mongodb/src/adapter.ts b/packages/mongodb/src/adapter.ts index 1e9546fd35..e5d08511a4 100644 --- a/packages/mongodb/src/adapter.ts +++ b/packages/mongodb/src/adapter.ts @@ -255,7 +255,7 @@ export class MongoDbAdapter< return this.aggregateRaw(aggregateParams) .then((result) => result.toArray()) .then(([result]) => { - if (result === undefined) { + if (!result) { throw new NotFound(`No record found for id '${id}'`) } @@ -272,7 +272,7 @@ export class MongoDbAdapter< return this.getModel(params) .then((model) => model.findOne(query, findOptions)) .then((result) => { - if (result == null) { + if (!result) { throw new NotFound(`No record found for id '${id}'`) } @@ -466,10 +466,10 @@ export class MongoDbAdapter< return model .findOneAndUpdate(query, replacement, updateOptions) .then((result) => { - if (result.value === null) { + if (!result) { throw new NotFound(`No record found for id '${id}'`) } - return result.value as Result + return result as Result }) .catch(errorHandler) } @@ -515,10 +515,10 @@ export class MongoDbAdapter< return model .findOneAndReplace(query, replacement, replaceOptions) .then((result) => { - if (result.value === null) { + if (!result) { throw new NotFound(`No record found for id '${id}'`) } - return result.value as Result + return result as Result }) .catch(errorHandler) } @@ -568,10 +568,10 @@ export class MongoDbAdapter< return model .findOneAndDelete(query, deleteOptions) .then((result) => { - if (result.value === null) { + if (!result) { throw new NotFound(`No record found for id '${id}'`) } - return result.value as Result + return result as Result }) .catch(errorHandler) } diff --git a/packages/mongodb/test/index.test.ts b/packages/mongodb/test/index.test.ts index 44b09d0b73..7aec6e887d 100644 --- a/packages/mongodb/test/index.test.ts +++ b/packages/mongodb/test/index.test.ts @@ -760,16 +760,79 @@ describe('Feathers MongoDB Service', () => { describe('Updates mutated query', () => { it('Can re-query mutated data', async () => { const dave = await app.service('people').create({ name: 'Dave' }) - const result = await app + const dave2 = await app.service('people').create({ name: 'Dave' }) + app.service('people').options.multi = true + + const updated = await app .service('people') .update(dave._id, { name: 'Marshal' }, { query: { name: 'Dave' } }) - assert.deepStrictEqual(result, { + assert.deepStrictEqual(updated, { + ...dave, + name: 'Marshal' + }) + + const patched = await app + .service('people') + .patch(dave._id, { name: 'Dave' }, { query: { name: 'Marshal' } }) + + assert.deepStrictEqual(patched, { + ...dave, + name: 'Dave' + }) + + const updatedPipeline = await app + .service('people') + .update(dave._id, { name: 'Marshal' }, { query: { name: 'Dave' }, pipeline: [] }) + + assert.deepStrictEqual(updatedPipeline, { ...dave, name: 'Marshal' }) + const patchedPipeline = await app + .service('people') + .patch(dave._id, { name: 'Dave' }, { query: { name: 'Marshal' }, pipeline: [] }) + + assert.deepStrictEqual(patchedPipeline, { + ...dave, + name: 'Dave' + }) + + const multiPatch = await app + .service('people') + .patch(null, { name: 'Marshal' }, { query: { name: 'Dave' } }) + + assert.deepStrictEqual(multiPatch, [ + { + ...dave, + name: 'Marshal' + }, + { + ...dave2, + name: 'Marshal' + } + ]) + + const multiPatchPipeline = await app + .service('people') + .patch(null, { name: 'Dave' }, { query: { name: 'Marshal' }, pipeline: [] }) + + assert.deepStrictEqual(multiPatchPipeline, [ + { + ...dave, + name: 'Dave' + }, + { + ...dave2, + name: 'Dave' + } + ]) + + app.service('people').options.multi = false + app.service('people').remove(dave._id) + app.service('people').remove(dave2._id) }) })