diff --git a/src/Orm/Relations/ManyToMany/QueryBuilder.ts b/src/Orm/Relations/ManyToMany/QueryBuilder.ts index 77570472..a7921ce7 100644 --- a/src/Orm/Relations/ManyToMany/QueryBuilder.ts +++ b/src/Orm/Relations/ManyToMany/QueryBuilder.ts @@ -14,7 +14,7 @@ import { ModelContract, ManyToManyQueryBuilderContract } from '@ioc:Adonis/Lucid import { QueryClientContract, TransactionClientContract } from '@ioc:Adonis/Lucid/Database' import { ManyToMany } from './index' -import { unique, difference } from '../../../utils' +import { unique, syncDiff } from '../../../utils' import { BaseRelationQueryBuilder } from '../Base/QueryBuilder' /** @@ -374,8 +374,16 @@ export class ManyToManyQueryBuilder ids: (string | number)[] | { [key: string]: any }, checkExisting: boolean, ) { - let idsList = unique(Array.isArray(ids) ? ids : Object.keys(ids)) + const pivotFKValue = this.$getRelatedValue(parent, this._relation.localKey, 'attach') + const hasAttributes = !Array.isArray(ids) + const idsList = hasAttributes ? Object.keys(ids) : ids as string[] + + /** + * Initial diff has all the ids under the insert array. If `checkExisting = true` + * then we will re-compute the diff from the existing database rows. + */ + let diff: { update: any[], insert: any[] } = { update: [], insert: idsList } /** * Pull existing pivot rows when `checkExisting = true` and persist only @@ -385,38 +393,51 @@ export class ManyToManyQueryBuilder const existingRows = await client .query() .from(this._relation.pivotTable) - .select(this._relation.pivotRelatedForeignKey) .whereIn(this._relation.pivotRelatedForeignKey, idsList) - .where( - this._relation.pivotForeignKey, - this.$getRelatedValue(parent, this._relation.localKey, 'attach'), - ) + .where(this._relation.pivotForeignKey, pivotFKValue) - const existingIds = existingRows.map((row) => row[this._relation.pivotRelatedForeignKey]) - idsList = difference(idsList, existingIds) + /** + * Computing the diff using the existing database rows + */ + diff = syncDiff(existingRows, ids, (rows, forId) => { + /* eslint eqeqeq: "off" */ + return rows.find((row) => row[this._relation.pivotRelatedForeignKey] == forId) + }) } /** - * Ignore when there is nothing to insert + * Update rows where attributes have changed. The query is exactly the + * same as the above fetch query with two changes. + * + * 1. Performing an updating, instead of select + * 2. Instead of whereIn on the `pivotRelatedForeignKey`, we are using `where` + * cause of the nature of the update action. */ - if (!idsList.length) { - return + if (diff.update.length) { + await Promise.all(unique(diff.update).map((id) => { + return client + .query() + .from(this._relation.pivotTable) + .where(this._relation.pivotForeignKey, pivotFKValue) + .where(this._relation.pivotRelatedForeignKey, id) + .update(ids[id]) + })) } /** * Perform multiple inserts in one go */ - await client - .insertQuery() - .table(this._relation.pivotTable) - .multiInsert(idsList.map((id) => { - const payload = { - [this._relation.pivotForeignKey]: this.$getRelatedValue(parent, this._relation.localKey), - [this._relation.pivotRelatedForeignKey]: id, - } - - return hasAttributes ? Object.assign(payload, ids[id]) : payload - })) + if (diff.insert.length) { + await client + .insertQuery() + .table(this._relation.pivotTable) + .multiInsert(unique(diff.insert).map((id) => { + return Object.assign({}, hasAttributes ? ids[id] : {}, { + [this._relation.pivotForeignKey]: pivotFKValue, + [this._relation.pivotRelatedForeignKey]: id, + }) + })) + } } /** diff --git a/src/utils/index.ts b/src/utils/index.ts index cfb69a76..7598801b 100644 --- a/src/utils/index.ts +++ b/src/utils/index.ts @@ -49,10 +49,17 @@ export function getValue ( return value } +/** + * Helper to find if value is a valid Object or + * not + */ export function isObject (value: any): boolean { return value !== null && typeof (value) === 'object' && !Array.isArray(value) } +/** + * Drops duplicate values from an array + */ export function unique (value: any[]) { if (!Array.isArray(value)) { return [] @@ -60,6 +67,9 @@ export function unique (value: any[]) { return [...new Set(value)] } +/** + * Finds the diff between 2 arrays + */ export function difference (main: any[], other: []) { return [main, other].reduce((a, b) => { return a.filter(c => { @@ -69,6 +79,67 @@ export function difference (main: any[], other: []) { }) } +/** + * A helper to know file ends with a script file + * extension or not + */ export function isJavaScriptFile (file: string) { return ['.js', '.ts'].includes(extname(file)) } + +/** + * Returns a diff of rows to be updated or inserted when performing + * a many to many `attach` + */ +export function syncDiff ( + dbRows: any[], + attributesToSync: any[] | { [key: string]: any }, + rowIdResolver: (rows: any, forId: string) => any, +) { + /** + * When attributes to sync are not defined as an array. Then we expect it + * to be an object + */ + const hasExtraAttributes = !Array.isArray(attributesToSync) + + /** + * An array of ids we want to sync + */ + const idsToSync = (hasExtraAttributes ? Object.keys(attributesToSync) : attributesToSync) as string[] + + return idsToSync.reduce((result: { insert: any[], update: any[] }, id) => { + /** + * Find the matching row for the given id + */ + const matchingRow = rowIdResolver(dbRows, id) + + /** + * When there isn't any matching row, we need to insert + * the id + */ + if (!matchingRow) { + result.insert.push(id) + return result + } + + /** + * When there aren't any extra attributes to check, we skip the + * given id, since it already exists. + */ + if (!hasExtraAttributes) { + return result + } + + /** + * When one or more attributes inside the update payload are different + * from the actual row, then we perform an update + */ + const attributes = attributesToSync[id] + /* eslint eqeqeq: "off" */ + if (Object.keys(attributes).find((key) => matchingRow[key] != attributes[key])) { + result.update.push(id) + } + + return result + }, { insert: [], update: [] }) +} diff --git a/test/orm/model-many-to-many.spec.ts b/test/orm/model-many-to-many.spec.ts index f3b09883..975fe15f 100644 --- a/test/orm/model-many-to-many.spec.ts +++ b/test/orm/model-many-to-many.spec.ts @@ -3184,7 +3184,7 @@ test.group('Model | ManyToMany | sync', (group) => { assert.equal(skillUsersAfterSync[0].skill_id, 2) }) - test('insert new ids metioned in sync', async (assert) => { + test('insert new ids mentioned in sync', async (assert) => { class Skill extends BaseModel { @column({ primary: true }) public id: number @@ -3264,10 +3264,61 @@ test.group('Model | ManyToMany | sync', (group) => { assert.equal(skillUsers[0].id, skillUsersAfterSync[0].id) assert.equal(skillUsersAfterSync[0].user_id, user.id) assert.equal(skillUsersAfterSync[0].skill_id, 1) - assert.isNull(skillUsersAfterSync[0].proficiency) + assert.equal(skillUsersAfterSync[0].proficiency, 'master') assert.equal(skillUsersAfterSync[1].user_id, user.id) assert.equal(skillUsersAfterSync[1].skill_id, 2) assert.equal(skillUsersAfterSync[1].proficiency, 'beginner') }) + + test('sync update extra properties when rows are same', async (assert) => { + class Skill extends BaseModel { + @column({ primary: true }) + public id: number + + @column() + public name: string + } + + class User extends BaseModel { + @column({ primary: true }) + public id: number + + @column() + public username: string + + @manyToMany(() => Skill) + public skills: Skill[] + } + + const user = new User() + user.username = 'virk' + await user.save() + + await user.related<'manyToMany', 'skills'>('skills').attach([1]) + const skillUsers = await db.query().from('skill_user') + + await user.related<'manyToMany', 'skills'>('skills').sync({ + 1: { proficiency: 'master' }, + 2: { proficiency: 'beginner' }, + }) + + await user.related<'manyToMany', 'skills'>('skills').sync({ + 1: { proficiency: 'master' }, + 2: { proficiency: 'intermediate' }, + }) + const skillUsersAfterSync = await db.query().from('skill_user') + + assert.lengthOf(skillUsers, 1) + assert.lengthOf(skillUsersAfterSync, 2) + + assert.equal(skillUsers[0].id, skillUsersAfterSync[0].id) + assert.equal(skillUsersAfterSync[0].user_id, user.id) + assert.equal(skillUsersAfterSync[0].skill_id, 1) + assert.equal(skillUsersAfterSync[0].proficiency, 'master') + + assert.equal(skillUsersAfterSync[1].user_id, user.id) + assert.equal(skillUsersAfterSync[1].skill_id, 2) + assert.equal(skillUsersAfterSync[1].proficiency, 'intermediate') + }) }) diff --git a/test/utils.spec.ts b/test/utils.spec.ts new file mode 100644 index 00000000..4a11a253 --- /dev/null +++ b/test/utils.spec.ts @@ -0,0 +1,95 @@ +/* +* @adonisjs/lucid +* +* (c) Harminder Virk +* +* For the full copyright and license information, please view the LICENSE +* file that was distributed with this source code. +*/ + +/// + +import test from 'japa' +import { syncDiff } from '../src/utils' + +test.group('Utils | attachDiff', () => { + test('return ids to be inserted', (assert) => { + const dbRows = [{ + id: '1', + user_id: '1', + skill_id: '1', + score: 1, + }] + + const idsToSync = ['1', '2', '3'] + + const diff = syncDiff(dbRows, idsToSync, (rows, id) => rows.find(({ skill_id }) => skill_id === id)) + assert.deepEqual(diff, { insert: ['2', '3'], update: [] }) + }) + + test('return ids when ids to sync are represented as an object', (assert) => { + const dbRows = [{ + id: '1', + user_id: '1', + skill_id: '1', + score: 1, + }] + + const idsToSync = { + '1': {}, + '2': {}, + '3': {}, + } + + const diff = syncDiff(dbRows, idsToSync, (rows, id) => rows.find(({ skill_id }) => skill_id === id)) + assert.deepEqual(diff, { insert: ['2', '3'], update: [] }) + }) + + test('return ids to be updated when attributes are different', (assert) => { + const dbRows = [{ + id: '1', + user_id: '1', + skill_id: '1', + score: 1, + }] + + const idsToSync = { + '1': { + score: 4, + }, + '2': { + score: 2, + }, + '3': { + score: 1, + }, + } + + const diff = syncDiff(dbRows, idsToSync, (rows, id) => rows.find(({ skill_id }) => skill_id === id)) + assert.deepEqual(diff, { insert: ['2', '3'], update: ['1'] }) + }) + + test('ignore rows whose attributes value is same', (assert) => { + const dbRows = [{ + id: '1', + user_id: '1', + skill_id: '1', + score: 1, + }] + + const idsToSync = { + '1': { + score: 1, + }, + '2': { + score: 2, + }, + '3': { + score: 1, + }, + } + + const diff = syncDiff(dbRows, idsToSync, (rows, id) => rows.find(({ skill_id }) => skill_id === id)) + assert.deepEqual(diff, { insert: ['2', '3'], update: [] }) + }) +})