mongodb adapter has incorrect updateMany/updateJobs behavior

[lkho]

Describe the Bug

When calling payload.db.updateMany / payload.db.updateJobs without a limit or sort, the returned documents are NOT the updated documents, theoretically.

ref:

payload/packages/db-mongodb/src/updateMany.ts

Lines 75 to 105 in 4278e72

	try {
	if (typeof limit === 'number' && limit > 0) {
	const documentsToUpdate = await Model.find(
	query,
	{},
	{ ...options, limit, projection: { _id: 1 }, sort },
	)
	if (documentsToUpdate.length === 0) {
	return null
	}
	query = { _id: { $in: documentsToUpdate.map((doc) => doc._id) } }
	}
	await Model.updateMany(query, data, options)
	} catch (error) {
	handleError({ collection: collectionSlug, error, req })
	}
	if (returning === false) {
	return null
	}
	const result = await Model.find(
	query,
	{},
	{
	...options,
	sort,
	},
	)

Details:

This issue is triggered when the where condition conflicts with the update data.
e.g. in SQL : UPDATE table SET processing = true WHERE processing = false in the task queue worker

Reason:

when there is no sort or limit, the code resembles the following

  let query = await buildQuery(...)
  await Model.updateMany(query, data, options)
  const result = await Model.find(
    query,
    {},
    options,
  )
  return result

The results will always be empty since after the db is updated, the where clause will never match the updated documents again.

Link to the code that reproduces this issue

https://github.com/lkho/payload/tree/bug/updateMany-updateJobs-return-results

Reproduction Steps

I did try to make a reproduction repo, but don't know how to config with the mongodb adapter. The provided repo is just for illustration only.
Just read test/_community/int.spec.ts for the illustration.

Which area(s) are affected? (Select all that apply)

db-mongodb

Environment Info

Relevant Packages:
  payload: 3.55.0
  next: 15.5.0
  @payloadcms/db-mongodb: 3.55.0
  @payloadcms/graphql: 3.54.0
  @payloadcms/next/utilities: 3.55.0
  @payloadcms/plugin-cloud-storage: 3.54.0
  @payloadcms/storage-s3: 3.55.0
  @payloadcms/translations: 3.54.0
  @payloadcms/ui/shared: 3.55.0

[r1tsuu]

Hi @lkho, I understand your problem. However, I'm not sure if it can be solved, unless we'd build a new where query with { id: { in: idsArray } } (since the DB itself doesn't return the documents to us with Model.updateMany) which might be bad when you have a lot of updated documents. In cases like that, I'd just change your where query manually and call new payload.db.find instead of relying on the result from payload.db.updateMany.
Also, I believe you shouldn't have that problem when using payload.update, although payload.db.updateMany is more performant.

I'm converting this to a discussion since I don't think it's an issue with Payload, let me know if you have any questions/suggestions.

[lkho]

Thanks for the update. I totally understand the behavior.
Actually what I think should change is the updateJobs function, which is mainly used in the runJobs api.

Since that function is already made specific for this use case, it is expected to handle the "update-and-return" job correctly, either by imposing some restrictions (to throw if it can't handle the query), or workaround / mimic the same functionality by it's own way.
So that users don't get surprises or mistakenly go into infinite loops/crashes due to simple configuration issues.

I don't expect the DB drivers can handle that requirement, but think it should let users know it can't do it. At least in a documented way.
Anyway just my opinion.

[lkho]

Just comparing to the other drizzle adapters, they just accept the limit = 0 input and do a findMany + update, although it is inefficient, but correct output.

payload/packages/drizzle/src/updateJobs.ts

Lines 52 to 62 in ae34b6d

	const jobs = await findMany({
	adapter: this,
	collectionSlug: 'payload-jobs',
	fields: collection.flattenedFields,
	limit: id ? 1 : limit,
	pagination: false,
	req,
	sort,
	tableName,
	where: whereToUse,
	})