From 17d21e86bf86ad4d7183978ccf8a51b5bffb2ada Mon Sep 17 00:00:00 2001 From: Samuel Stroschein <35429197+samuelstroschein@users.noreply.github.com> Date: Tue, 15 Oct 2024 23:22:30 +0200 Subject: [PATCH 1/2] refactor: add entity id --- lix/packages/sdk/src/change-queue.test.ts | 115 +++++++-------- lix/packages/sdk/src/commit.test.ts | 133 ++++++------------ lix/packages/sdk/src/database/applySchema.ts | 1 + lix/packages/sdk/src/database/initDb.test.ts | 1 + lix/packages/sdk/src/database/schema.ts | 12 +- .../sdk/src/discussion/discussion.test.ts | 53 +------ lix/packages/sdk/src/file-handlers.ts | 98 ++++++------- lix/packages/sdk/src/merge/merge.test.ts | 28 ++-- .../sdk/src/mock/mock-csv-plugin.test.ts | 1 + lix/packages/sdk/src/newLix.test.ts | 1 + lix/packages/sdk/src/plugin.ts | 5 +- .../query-utilities/get-leaf-change.test.ts | 3 + .../get-leaf-changes-only-in-source.test.ts | 17 ++- .../get-lowest-common-ancestor.test.ts | 10 ++ .../is-in-simulated-branch.test.ts | 9 ++ .../resolve-conflict-by-selecting.test.ts | 2 + .../resolve-conflict-with-new-change.test.ts | 11 ++ 17 files changed, 234 insertions(+), 266 deletions(-) diff --git a/lix/packages/sdk/src/change-queue.test.ts b/lix/packages/sdk/src/change-queue.test.ts index f1f669de75..c4c94200ff 100644 --- a/lix/packages/sdk/src/change-queue.test.ts +++ b/lix/packages/sdk/src/change-queue.test.ts @@ -4,64 +4,28 @@ import { newLixFile } from "./newLix.js"; import type { DiffReport, LixPlugin } from "./plugin.js"; test("should use queue and settled correctly", async () => { - const mockPlugin: LixPlugin<{ - text: { id: string; text: string }; - }> = { + const mockPlugin: LixPlugin = { key: "mock-plugin", glob: "*", diff: { file: async ({ before, after }) => { - const dec = new TextDecoder(); - // console.log("diff", after, before?.data, after?.data); - const textBefore = dec.decode(after?.data); - const textAfter = dec.decode(before?.data); + const textBefore = before + ? new TextDecoder().decode(before?.data) + : undefined; + const textAfter = after + ? new TextDecoder().decode(after.data) + : undefined; if (textBefore === textAfter) { return []; } - - return await mockPlugin.diff.text({ - before: before - ? { - id: "test", - text: textAfter, - } - : undefined, - after: after - ? { - id: "test", - text: textBefore, - } - : undefined, - }); - }, - text: async ({ before, after }) => { - // console.log("text", before, after); - if (before?.text === after?.text) { - return []; - } - return [ - !before - ? { - type: "text", - before: undefined, - after: { - id: "test", - text: after?.text, - }, - } - : { - type: "text", - before: { - id: "test", - text: before?.text, - }, - after: { - id: "test", - text: after?.text, - }, - }, + { + type: "text", + entity_id: "test", + before: textBefore ? { text: textBefore } : undefined, + after: textAfter ? { text: textAfter } : undefined, + }, ]; }, }, @@ -118,7 +82,20 @@ test("should use queue and settled correctly", async () => { const changes = await lix.db .selectFrom("change") .innerJoin("snapshot", "snapshot.id", "change.snapshot_id") - .selectAll() + .select([ + "change.id", + "change.author", + "change.created_at", + "change.snapshot_id", + "change.parent_id", + "change.entity_id", + "change.type", + "change.file_id", + "change.plugin_key", + "snapshot.value as value", + "change.meta", + "change.commit_id", + ]) .execute(); expect(changes).toEqual([ @@ -128,11 +105,11 @@ test("should use queue and settled correctly", async () => { created_at: changes[0]?.created_at, snapshot_id: changes[0]?.snapshot_id, parent_id: null, + entity_id: "test", type: "text", file_id: "test", plugin_key: "mock-plugin", value: { - id: "test", text: "test", }, meta: null, @@ -187,7 +164,20 @@ test("should use queue and settled correctly", async () => { const updatedChanges = await lix.db .selectFrom("change") .innerJoin("snapshot", "snapshot.id", "change.snapshot_id") - .selectAll() + .select([ + "change.id", + "change.author", + "change.created_at", + "change.snapshot_id", + "change.parent_id", + "change.entity_id", + "change.type", + "change.file_id", + "change.plugin_key", + "snapshot.value as value", + "change.meta", + "change.commit_id", + ]) .execute(); expect(updatedChanges).toEqual([ @@ -197,11 +187,11 @@ test("should use queue and settled correctly", async () => { created_at: updatedChanges[0]?.created_at, snapshot_id: updatedChanges[0]?.snapshot_id, parent_id: null, + entity_id: "test", type: "text", file_id: "test", plugin_key: "mock-plugin", value: { - id: "test", text: "test", }, meta: null, @@ -210,6 +200,7 @@ test("should use queue and settled correctly", async () => { { author: null, commit_id: null, + entity_id: "test", created_at: updatedChanges[1]?.created_at, snapshot_id: updatedChanges[1]?.snapshot_id, file_id: "test", @@ -219,7 +210,6 @@ test("should use queue and settled correctly", async () => { plugin_key: "mock-plugin", type: "text", value: { - id: "test", text: "test updated text", }, }, @@ -232,10 +222,10 @@ test("should use queue and settled correctly", async () => { id: updatedChanges[2]?.id, meta: null, parent_id: updatedChanges[1]?.id, + entity_id: "test", plugin_key: "mock-plugin", type: "text", value: { - id: "test", text: "test updated text second update", }, }, @@ -252,8 +242,21 @@ test("changes should contain the author", async () => { file: vi.fn().mockResolvedValue([ { type: "mock", + entity_id: "mock", before: undefined, - after: {} as any, + after: { + text: "value1", + }, + }, + { + type: "mock", + entity_id: "mock", + before: { + text: "value1", + }, + after: { + text: "value2", + }, }, ] satisfies DiffReport[]), }, diff --git a/lix/packages/sdk/src/commit.test.ts b/lix/packages/sdk/src/commit.test.ts index 5661b9ac4a..8e8fd45ab8 100644 --- a/lix/packages/sdk/src/commit.test.ts +++ b/lix/packages/sdk/src/commit.test.ts @@ -10,28 +10,24 @@ test("should be able to add and commit changes", async () => { key: "mock-plugin", glob: "*", diff: { - file: async ({ before: old }) => { + file: async ({ before, after }) => { + const textBefore = before + ? new TextDecoder().decode(before?.data) + : undefined; + const textAfter = after + ? new TextDecoder().decode(after.data) + : undefined; + + if (textBefore === textAfter) { + return []; + } return [ - !old - ? { - type: "text", - before: undefined, - after: { - id: "test", - text: "inserted text", - }, - } - : { - type: "text", - before: { - id: "test", - text: "inserted text", - }, - after: { - id: "test", - text: "updated text", - }, - }, + { + type: "text", + entity_id: "test", + before: textBefore ? { text: textBefore } : undefined, + after: textAfter ? { text: textAfter } : undefined, + }, ]; }, }, @@ -53,39 +49,27 @@ test("should be able to add and commit changes", async () => { await lix.db .insertInto("file") - .values({ id: "test", path: "test.txt", data: enc.encode("test") }) + .values({ id: "test", path: "test.txt", data: enc.encode("value1") }) .execute(); await lix.settled(); const changes = await lix.db .selectFrom("change") - .innerJoin("snapshot", "snapshot.id", "change.snapshot_id") + .innerJoin("snapshot as snapshot", "snapshot.id", "change.snapshot_id") .selectAll() .execute(); - // console.log(await lix.db.selectFrom("queue").selectAll().execute()); - expect(changes).toEqual([ - { - id: changes[0]?.id, - author: null, - created_at: changes[0]?.created_at, - snapshot_id: changes[0]?.snapshot_id, + expect.objectContaining({ parent_id: null, - type: "text", - file_id: "test", - plugin_key: "mock-plugin", value: { - id: "test", - text: "inserted text", + text: "value1", }, - commit_id: null, - meta: null, - }, + }), ]); - await lix.commit({ description: "test" }); + await lix.commit({ description: "first commit" }); const secondRef = await lix.db .selectFrom("ref") @@ -103,22 +87,10 @@ test("should be able to add and commit changes", async () => { .execute(); expect(commitedChanges).toEqual([ - { - id: commitedChanges[0]?.id, - author: null, - created_at: changes[0]?.created_at, - snapshot_id: changes[0]?.snapshot_id, - parent_id: null, - type: "text", - file_id: "test", - plugin_key: "mock-plugin", - value: { - id: "test", - text: "inserted text", - }, - meta: null, + expect.objectContaining({ + ...changes[0], commit_id: commits[0]?.id!, - }, + }), ]); expect(commits).toEqual([ @@ -127,61 +99,48 @@ test("should be able to add and commit changes", async () => { author: null, created: commits[0]?.created!, created_at: commits[0]?.created_at!, - description: "test", + description: "first commit", parent_id: "00000000-0000-0000-0000-000000000000", }, ]); await lix.db .updateTable("file") - .set({ data: enc.encode("test updated text") }) + .set({ data: enc.encode("value2") }) .where("id", "=", "test") .execute(); await lix.settled(); - const updatedChanges = await lix.db + const changesAfter = await lix.db .selectFrom("change") .innerJoin("snapshot", "snapshot.id", "change.snapshot_id") - .selectAll() + .select([ + "change.id", + "snapshot.value", + "change.commit_id", + "change.parent_id", + ]) .execute(); - expect(updatedChanges).toEqual([ - { - id: updatedChanges[0]?.id!, - author: null, - created_at: updatedChanges[0]?.created_at, - snapshot_id: updatedChanges[0]?.snapshot_id, + expect(changesAfter).toEqual([ + expect.objectContaining({ parent_id: null, - type: "text", - file_id: "test", - plugin_key: "mock-plugin", value: { - id: "test", - text: "inserted text", + text: "value1", }, - meta: null, commit_id: commits[0]?.id, - }, - { - id: updatedChanges[1]?.id!, - author: null, - parent_id: updatedChanges[0]?.id!, - created_at: updatedChanges[1]?.created_at, - snapshot_id: updatedChanges[1]?.snapshot_id, - type: "text", - file_id: "test", - plugin_key: "mock-plugin", + }), + expect.objectContaining({ + parent_id: changesAfter[0]?.id!, value: { - id: "test", - text: "updated text", + text: "value2", }, - meta: null, commit_id: null, - }, + }), ]); - await lix.commit({ description: "test 2" }); + await lix.commit({ description: "second commit" }); const newCommits = await lix.db.selectFrom("commit").selectAll().execute(); expect(newCommits).toEqual([ { @@ -189,7 +148,7 @@ test("should be able to add and commit changes", async () => { author: null, created: commits[0]?.created!, created_at: newCommits[0]?.created_at!, - description: "test", + description: "first commit", parent_id: "00000000-0000-0000-0000-000000000000", }, { @@ -197,7 +156,7 @@ test("should be able to add and commit changes", async () => { author: null, created: commits[0]?.created!, created_at: newCommits[1]?.created_at!, - description: "test 2", + description: "second commit", parent_id: newCommits[0]?.id!, }, ]); diff --git a/lix/packages/sdk/src/database/applySchema.ts b/lix/packages/sdk/src/database/applySchema.ts index feecdb7d51..28c78a4503 100644 --- a/lix/packages/sdk/src/database/applySchema.ts +++ b/lix/packages/sdk/src/database/applySchema.ts @@ -34,6 +34,7 @@ export async function applySchema(args: { sqlite: SqliteDatabase }) { id TEXT PRIMARY KEY DEFAULT (uuid_v4()), author TEXT, parent_id TEXT, + entity_id TEXT NOT NULL, type TEXT NOT NULL, file_id TEXT NOT NULL, plugin_key TEXT NOT NULL, diff --git a/lix/packages/sdk/src/database/initDb.test.ts b/lix/packages/sdk/src/database/initDb.test.ts index 85f3757ace..96e98e10da 100644 --- a/lix/packages/sdk/src/database/initDb.test.ts +++ b/lix/packages/sdk/src/database/initDb.test.ts @@ -44,6 +44,7 @@ test("change ids should default to uuid", async () => { .values({ commit_id: "mock", type: "file", + entity_id: "value1", file_id: "mock", plugin_key: "mock-plugin", snapshot_id: "sn1", diff --git a/lix/packages/sdk/src/database/schema.ts b/lix/packages/sdk/src/database/schema.ts index cbe1d0a21e..d46aeba20c 100644 --- a/lix/packages/sdk/src/database/schema.ts +++ b/lix/packages/sdk/src/database/schema.ts @@ -72,6 +72,10 @@ type ChangeTable = { id: Generated; parent_id: Generated | null; author?: string; + /** + * The entity the change refers to. + */ + entity_id: string; file_id: string; /** * If no commit id exists on a change, @@ -110,18 +114,18 @@ type ChangeTable = { export type Snapshot = Selectable; export type NewSnapshot = Insertable; type SnapshotTable = { - id: Generated + id: Generated; /** * The value of the change. * - * The value is `undefined` for a delete operation. + * Lix interprets an undefined value as delete operation. * * @example * - For a csv cell change, the value would be the new cell value. * - For an inlang message change, the value would be the new message. */ - value?: Record & { id: string }; -} + value?: Record; +}; // TODO #185 rename value to snapshot_value export type ChangeWithSnapshot = Change & { value: SnapshotTable['value'] }; diff --git a/lix/packages/sdk/src/discussion/discussion.test.ts b/lix/packages/sdk/src/discussion/discussion.test.ts index e982437c51..d1c62f63b3 100644 --- a/lix/packages/sdk/src/discussion/discussion.test.ts +++ b/lix/packages/sdk/src/discussion/discussion.test.ts @@ -15,19 +15,18 @@ const mockPlugin: LixPlugin = { ? { type: "text", before: undefined, + entity_id: "test", after: { - id: "test", text: "inserted text", }, } : { type: "text", + entity_id: "test", before: { - id: "test", text: "inserted text", }, after: { - id: "test", text: "updated text", }, }, @@ -59,27 +58,6 @@ test("should be able to start a discussion on changes", async () => { .innerJoin("snapshot", "snapshot.id", "change.snapshot_id") .execute(); - // console.log(await lix.db.selectFrom("queue").selectAll().execute()); - - expect(changes).toEqual([ - { - id: changes[0]?.id, - author: "Test User", - created_at: changes[0]?.created_at, - snapshot_id: changes[0]?.snapshot_id, - parent_id: null, - type: "text", - file_id: "test", - plugin_key: "mock-plugin", - value: { - id: "test", - text: "inserted text", - }, - meta: null, - commit_id: null, - }, - ]); - const discussion = await lix.createDiscussion({ changeIds: [changes[0]!.id], body: "comment on a change", @@ -136,33 +114,6 @@ test("should fail to create a disussion on non existing changes", async () => { await lix.settled(); - const changes = await lix.db - .selectFrom("change") - .innerJoin("snapshot", "snapshot.id", "change.snapshot_id") - .selectAll() - .execute(); - - // console.log(await lix.db.selectFrom("queue").selectAll().execute()); - - expect(changes).toEqual([ - { - id: changes[0]?.id, - author: "Test User", - created_at: changes[0]?.created_at, - snapshot_id: changes[0]?.snapshot_id, - parent_id: null, - type: "text", - file_id: "test", - plugin_key: "mock-plugin", - value: { - id: "test", - text: "inserted text", - }, - meta: null, - commit_id: null, - }, - ]); - await lix.createDiscussion({ changeIds: ["I DON'T EXIST"], body: "comment on a change", diff --git a/lix/packages/sdk/src/file-handlers.ts b/lix/packages/sdk/src/file-handlers.ts index 4a6391a2c3..a1ff696cd1 100644 --- a/lix/packages/sdk/src/file-handlers.ts +++ b/lix/packages/sdk/src/file-handlers.ts @@ -1,6 +1,5 @@ -import { v4 } from "uuid"; import type { LixDatabaseSchema, LixFile } from "./database/schema.js"; -import type { LixPlugin } from "./plugin.js"; +import type { DiffReport, LixPlugin } from "./plugin.js"; import { minimatch } from "minimatch"; import { Kysely } from "kysely"; @@ -20,8 +19,10 @@ export async function handleFileInsert(args: { currentAuthor?: string; queueEntry: any; }) { - const pluginDiffs: any[] = []; - + const pluginDiffs: { + diffs: DiffReport[]; + pluginKey: string; + }[] = []; // console.log({ args }); for (const plugin of args.plugins) { // glob expressions are expressed relative without leading / but path has leading / @@ -29,13 +30,12 @@ export async function handleFileInsert(args: { break; } - const diffs = await plugin.diff!.file!({ + const diffs = await plugin.diff?.file?.({ before: undefined, after: args.after, }); - // console.log({ diffs }); - pluginDiffs.push({ diffs, pluginKey: plugin.key }); + pluginDiffs.push({ diffs: diffs ?? [], pluginKey: plugin.key }); } await args.db.transaction().execute(async (trx) => { @@ -43,27 +43,24 @@ export async function handleFileInsert(args: { for (const diff of diffs ?? []) { const value = diff.before ?? diff.after; - const snapshotId = ( - await trx - .insertInto("snapshot") - .values({ - id: v4(), - // @ts-expect-error - database expects stringified json - value: JSON.stringify(value), - }) - .returning("id") - .executeTakeFirstOrThrow() - ).id; + const snapshot = await trx + .insertInto("snapshot") + .values({ + // @ts-expect-error - database expects stringified json + value: JSON.stringify(value), + }) + .returning("id") + .executeTakeFirstOrThrow(); await trx .insertInto("change") .values({ - id: v4(), type: diff.type, file_id: args.after.id, author: args.currentAuthor, + entity_id: diff.entity_id, plugin_key: pluginKey, - snapshot_id: snapshotId, + snapshot_id: snapshot.id, // @ts-expect-error - database expects stringified json meta: JSON.stringify(diff.meta), // add queueId interesting for debugging or knowning what changes were generated in same worker run @@ -90,7 +87,10 @@ export async function handleFileChange(args: { }) { const fileId = args.after?.id ?? args.before?.id; - const pluginDiffs: any[] = []; + const pluginDiffs: { + diffs: DiffReport[]; + pluginKey: string; + }[] = []; for (const plugin of args.plugins) { // glob expressions are expressed relative without leading / but path has leading / @@ -98,20 +98,19 @@ export async function handleFileChange(args: { break; } - const diffs = await plugin.diff!.file!({ + const diffs = await plugin.diff?.file?.({ before: args.before, after: args.after, }); pluginDiffs.push({ - diffs, + diffs: diffs ?? [], pluginKey: plugin.key, - pluginDiffFunction: plugin.diff, }); } await args.db.transaction().execute(async (trx) => { - for (const { diffs, pluginKey, pluginDiffFunction } of pluginDiffs) { + for (const { diffs, pluginKey } of pluginDiffs) { for (const diff of diffs ?? []) { // assume an insert or update operation as the default // if diff.neu is not present, it's a delete operationd @@ -121,13 +120,11 @@ export async function handleFileChange(args: { const previousChanges = await trx .selectFrom("change") - .innerJoin("snapshot", "snapshot.id", "change.snapshot_id") .selectAll() .where("file_id", "=", fileId) .where("plugin_key", "=", pluginKey) .where("type", "=", diff.type) - .where((eb) => eb.ref("value", "->>").key("id"), "=", value.id) - // TODO don't rely on created at. a plugin should report the parent id. + .where("entity_id", "=", diff.entity_id) .execute(); // we need to finde the real leaf change as multiple changes can be set in the same created timestamp second or clockskew @@ -142,47 +139,42 @@ export async function handleFileChange(args: { break; } - // working change exists but is different from previously committed change - // -> update the working change or delete if it is the same as previous uncommitted change - // overwrite the (uncomitted) change - // to avoid (potentially) saving every keystroke change - let previousCommittedDiff = []; - // working change exists but is identical to previously committed change if (previousChange) { - previousCommittedDiff = await pluginDiffFunction?.[diff.type]?.({ - before: previousChange?.value, - after: diff.after, - }); - - if (previousCommittedDiff?.length === 0) { + const previousSnapshot = await trx + .selectFrom("snapshot") + .selectAll() + .where("id", "=", previousChange.snapshot_id) + .executeTakeFirstOrThrow(); + + if ( + // json stringify should be good enough if the plugin diff function is deterministic + JSON.stringify(previousSnapshot.value) === JSON.stringify(value) + ) { // drop the change because it's identical continue; } } - const snapshotId = ( - await trx - .insertInto("snapshot") - .values({ - id: v4(), - // @ts-expect-error - database expects stringified json - value: JSON.stringify(value), - }) - .returning("id") - .executeTakeFirstOrThrow() - ).id; + const snapshot = await trx + .insertInto("snapshot") + .values({ + // @ts-expect-error - database expects stringified json + value: JSON.stringify(value), + }) + .returning("id") + .executeTakeFirstOrThrow(); await trx .insertInto("change") .values({ - id: v4(), type: diff.type, file_id: fileId, plugin_key: pluginKey, + entity_id: diff.entity_id, author: args.currentAuthor, parent_id: previousChange?.id, - snapshot_id: snapshotId, + snapshot_id: snapshot.id, }) .execute(); } diff --git a/lix/packages/sdk/src/merge/merge.test.ts b/lix/packages/sdk/src/merge/merge.test.ts index c5eb053228..b0e1e7066a 100644 --- a/lix/packages/sdk/src/merge/merge.test.ts +++ b/lix/packages/sdk/src/merge/merge.test.ts @@ -25,6 +25,7 @@ test("it should copy changes from the sourceLix into the targetLix that do not e const mockChanges: NewChange[] = [ { id: "1", + entity_id: "value1", type: "mock", snapshot_id: "sn1", file_id: "mock-file", @@ -32,6 +33,7 @@ test("it should copy changes from the sourceLix into the targetLix that do not e }, { id: "2", + entity_id: "value1", type: "mock", snapshot_id: "sn2", file_id: "mock-file", @@ -39,6 +41,7 @@ test("it should copy changes from the sourceLix into the targetLix that do not e }, { id: "3", + entity_id: "value1", type: "mock", snapshot_id: "sn3", file_id: "mock-file", @@ -139,6 +142,7 @@ test("it should save change conflicts", async () => { { id: "1", type: "mock", + entity_id: "value1", snapshot_id: "sn1", file_id: "mock-file", plugin_key: "mock-plugin", @@ -146,6 +150,7 @@ test("it should save change conflicts", async () => { { id: "2", type: "mock", + entity_id: "value1", snapshot_id: "sn2", file_id: "mock-file", plugin_key: "mock-plugin", @@ -153,6 +158,7 @@ test("it should save change conflicts", async () => { { id: "3", type: "mock", + entity_id: "value1", snapshot_id: "sn3", file_id: "mock-file", plugin_key: "mock-plugin", @@ -239,6 +245,7 @@ test("diffing should not be invoked to prevent the generation of duplicate chang id: "1", type: "mock", snapshot_id: "sn1", + entity_id: "value1", file_id: "mock-file", plugin_key: "mock-plugin", }, @@ -257,6 +264,7 @@ test("diffing should not be invoked to prevent the generation of duplicate chang { id: "2", type: "mock", + entity_id: "value1", snapshot_id: "sn2", file_id: "mock-file", plugin_key: "mock-plugin", @@ -333,11 +341,11 @@ test("it should apply changes that are not conflicting", async () => { const mockSnapshots: NewSnapshot[] = [ { id: "sn1", - value: { id: "mock-id", color: "red" }, + value: { color: "red" }, }, { id: "sn2", - value: { id: "mock-id", color: "blue" }, + value: { color: "blue" }, }, ]; @@ -345,6 +353,7 @@ test("it should apply changes that are not conflicting", async () => { { id: "1", type: "mock", + entity_id: "value1", snapshot_id: "sn1", file_id: "mock-file", plugin_key: "mock-plugin", @@ -352,6 +361,7 @@ test("it should apply changes that are not conflicting", async () => { { id: "2", parent_id: "1", + entity_id: "value1", type: "mock", snapshot_id: "sn2", file_id: "mock-file", @@ -447,6 +457,7 @@ test("subsequent merges should not lead to duplicate changes and/or conflicts", { id: "1", type: "mock", + entity_id: "value1", snapshot_id: "sn1", file_id: "mock-file", plugin_key: "mock-plugin", @@ -465,6 +476,7 @@ test("subsequent merges should not lead to duplicate changes and/or conflicts", { id: "2", type: "mock", + entity_id: "value1", snapshot_id: "sn2", file_id: "mock-file", plugin_key: "mock-plugin", @@ -564,6 +576,7 @@ test("it should naively copy changes from the sourceLix into the targetLix that { id: "2", commit_id: "commit-1", + entity_id: "value1", type: "mock", snapshot_id: "sn2", file_id: "mock-file", @@ -640,19 +653,18 @@ test("it should copy discussion and related comments and mappings", async () => ? { type: "text", before: undefined, + entity_id: "test", after: { - id: "test", text: "inserted text", }, } : { type: "text", + entity_id: "test", before: { - id: "test", text: "inserted text", }, after: { - id: "test", text: "updated text", }, }, @@ -689,8 +701,8 @@ test("it should copy discussion and related comments and mappings", async () => const changes = await lix1.db .selectFrom("change") .innerJoin("snapshot", "snapshot.id", "change.snapshot_id") - .selectAll().execute(); - + .selectAll() + .execute(); expect(changes).toEqual([ { @@ -701,9 +713,9 @@ test("it should copy discussion and related comments and mappings", async () => parent_id: null, type: "text", file_id: "test", + entity_id: "test", plugin_key: "mock-plugin", value: { - id: "test", text: "inserted text", }, meta: null, diff --git a/lix/packages/sdk/src/mock/mock-csv-plugin.test.ts b/lix/packages/sdk/src/mock/mock-csv-plugin.test.ts index faa4674632..56b7821c6e 100644 --- a/lix/packages/sdk/src/mock/mock-csv-plugin.test.ts +++ b/lix/packages/sdk/src/mock/mock-csv-plugin.test.ts @@ -58,6 +58,7 @@ describe("applyChanges()", () => { .insertInto("change") .values({ id: "parent_change_id", + entity_id: "value1", file_id: "random", plugin_key: "csv", type: "cell", diff --git a/lix/packages/sdk/src/newLix.test.ts b/lix/packages/sdk/src/newLix.test.ts index 693a747399..053b75aa0e 100644 --- a/lix/packages/sdk/src/newLix.test.ts +++ b/lix/packages/sdk/src/newLix.test.ts @@ -9,6 +9,7 @@ test("inserting a change should auto fill the created_at column", async () => { .insertInto("change") .values({ id: "test", + entity_id: "test", commit_id: "test", type: "file", file_id: "mock", diff --git a/lix/packages/sdk/src/plugin.ts b/lix/packages/sdk/src/plugin.ts index 532b78d6ae..26d20a5d83 100644 --- a/lix/packages/sdk/src/plugin.ts +++ b/lix/packages/sdk/src/plugin.ts @@ -75,6 +75,7 @@ type MaybePromise = T | Promise; */ export type DiffReport = { type: string; - before?: Record & { id: string }; - after?: Record & { id: string }; + entity_id: string; + before?: Record; + after?: Record; }; diff --git a/lix/packages/sdk/src/query-utilities/get-leaf-change.test.ts b/lix/packages/sdk/src/query-utilities/get-leaf-change.test.ts index b0cc100c66..9943e64f74 100644 --- a/lix/packages/sdk/src/query-utilities/get-leaf-change.test.ts +++ b/lix/packages/sdk/src/query-utilities/get-leaf-change.test.ts @@ -29,6 +29,7 @@ test("it should find the latest child of a given change", async () => { { id: "1", parent_id: undefined, + entity_id: "value1", file_id: "mock", plugin_key: "mock", type: "mock", @@ -37,6 +38,7 @@ test("it should find the latest child of a given change", async () => { { id: "2", parent_id: "1", + entity_id: "value1", file_id: "mock", plugin_key: "mock", type: "mock", @@ -45,6 +47,7 @@ test("it should find the latest child of a given change", async () => { { id: "3", parent_id: "2", + entity_id: "value1", file_id: "mock", plugin_key: "mock", type: "mock", diff --git a/lix/packages/sdk/src/query-utilities/get-leaf-changes-only-in-source.test.ts b/lix/packages/sdk/src/query-utilities/get-leaf-changes-only-in-source.test.ts index 92905b7bf6..50fbda4a91 100644 --- a/lix/packages/sdk/src/query-utilities/get-leaf-changes-only-in-source.test.ts +++ b/lix/packages/sdk/src/query-utilities/get-leaf-changes-only-in-source.test.ts @@ -24,6 +24,7 @@ test("it should get the leaf changes that only exist in source", async () => { { id: "c1", file_id: "mock", + entity_id: "value1", plugin_key: "mock", type: "mock", snapshot_id: "snc1", @@ -31,6 +32,7 @@ test("it should get the leaf changes that only exist in source", async () => { { id: "c2", file_id: "mock", + entity_id: "value1", parent_id: "c1", plugin_key: "mock", type: "mock", @@ -57,6 +59,7 @@ test("it should get the leaf changes that only exist in source", async () => { { id: "s1", file_id: "mock", + entity_id: "value1", plugin_key: "mock", type: "mock", snapshot_id: "sns1", @@ -64,6 +67,7 @@ test("it should get the leaf changes that only exist in source", async () => { { id: "s2", parent_id: "s1", + entity_id: "value1", file_id: "mock", plugin_key: "mock", type: "mock", @@ -72,6 +76,7 @@ test("it should get the leaf changes that only exist in source", async () => { { id: "s3", parent_id: "s2", + entity_id: "value1", file_id: "mock", plugin_key: "mock", type: "mock", @@ -79,16 +84,18 @@ test("it should get the leaf changes that only exist in source", async () => { }, ]; - const snapshotsOnlyInTarget = [{ - id: 'snt1', - value: { id: "mock-id", color: "black" }, - }] - + const snapshotsOnlyInTarget = [ + { + id: "snt1", + value: { id: "mock-id", color: "black" }, + }, + ]; const changesOnlyInTarget: NewChange[] = [ { id: "t1", parent_id: "c2", + entity_id: "value1", file_id: "mock", plugin_key: "mock", type: "mock", diff --git a/lix/packages/sdk/src/query-utilities/get-lowest-common-ancestor.test.ts b/lix/packages/sdk/src/query-utilities/get-lowest-common-ancestor.test.ts index c4bfc92930..8737a68ad7 100644 --- a/lix/packages/sdk/src/query-utilities/get-lowest-common-ancestor.test.ts +++ b/lix/packages/sdk/src/query-utilities/get-lowest-common-ancestor.test.ts @@ -31,6 +31,7 @@ test("it should find the common parent of two changes recursively", async () => { id: "0", parent_id: undefined, + entity_id: "value1", file_id: "mock", plugin_key: "mock", type: "mock", @@ -39,6 +40,7 @@ test("it should find the common parent of two changes recursively", async () => { id: "1", parent_id: "0", + entity_id: "value1", file_id: "mock", plugin_key: "mock", type: "mock", @@ -48,6 +50,7 @@ test("it should find the common parent of two changes recursively", async () => id: "2", parent_id: "1", file_id: "mock", + entity_id: "value1", plugin_key: "mock", type: "mock", snapshot_id: "sn3", @@ -117,7 +120,9 @@ test("it should return undefined if no common parent exists", async () => { const mockChanges: NewChange[] = [ { id: "0", + // no parent == create change parent_id: undefined, + entity_id: "value1", file_id: "mock", plugin_key: "mock", type: "mock", @@ -126,6 +131,7 @@ test("it should return undefined if no common parent exists", async () => { { id: "1", parent_id: "0", + entity_id: "value1", file_id: "mock", plugin_key: "mock", type: "mock", @@ -133,6 +139,8 @@ test("it should return undefined if no common parent exists", async () => { }, { id: "2", + entity_id: "value2", + // no parent == create change parent_id: undefined, file_id: "mock", plugin_key: "mock", @@ -202,6 +210,7 @@ test("it should return the source change if its the common parent", async () => const mockChanges: NewChange[] = [ { id: "0", + entity_id: "value1", parent_id: undefined, file_id: "mock", plugin_key: "mock", @@ -210,6 +219,7 @@ test("it should return the source change if its the common parent", async () => }, { id: "1", + entity_id: "value1", parent_id: "0", file_id: "mock", plugin_key: "mock", diff --git a/lix/packages/sdk/src/query-utilities/is-in-simulated-branch.test.ts b/lix/packages/sdk/src/query-utilities/is-in-simulated-branch.test.ts index 61ac34c053..9beaf7505e 100644 --- a/lix/packages/sdk/src/query-utilities/is-in-simulated-branch.test.ts +++ b/lix/packages/sdk/src/query-utilities/is-in-simulated-branch.test.ts @@ -11,6 +11,7 @@ test("as long as a conflict is unresolved, the conflicting change should not app .values([ { id: "change0", + entity_id: "value1", file_id: "mock", plugin_key: "mock", type: "mock", @@ -18,6 +19,7 @@ test("as long as a conflict is unresolved, the conflicting change should not app }, { id: "change1", + entity_id: "value1", file_id: "mock", plugin_key: "mock", type: "mock", @@ -26,6 +28,7 @@ test("as long as a conflict is unresolved, the conflicting change should not app { id: "change2", file_id: "mock", + entity_id: "value1", plugin_key: "mock", type: "mock", snapshot_id: "sn2", @@ -68,6 +71,7 @@ test(`if the conflict has been resolved by selecting the 'original' change, { id: "change0", file_id: "mock", + entity_id: "value1", plugin_key: "mock", type: "mock", snapshot_id: "sn0", @@ -75,6 +79,7 @@ test(`if the conflict has been resolved by selecting the 'original' change, { id: "change1", file_id: "mock", + entity_id: "value1", plugin_key: "mock", type: "mock", snapshot_id: "sn1", @@ -82,6 +87,7 @@ test(`if the conflict has been resolved by selecting the 'original' change, { id: "change2", file_id: "mock", + entity_id: "value1", plugin_key: "mock", type: "mock", snapshot_id: "sn2", @@ -126,6 +132,7 @@ test(` { id: "change0", file_id: "mock", + entity_id: "value1", plugin_key: "mock", type: "mock", snapshot_id: "sn0", @@ -133,6 +140,7 @@ test(` { id: "change1", file_id: "mock", + entity_id: "value1", plugin_key: "mock", type: "mock", snapshot_id: "sn1", @@ -140,6 +148,7 @@ test(` { id: "change2", file_id: "mock", + entity_id: "value1", plugin_key: "mock", type: "mock", snapshot_id: "sn2", diff --git a/lix/packages/sdk/src/resolve-conflict/resolve-conflict-by-selecting.test.ts b/lix/packages/sdk/src/resolve-conflict/resolve-conflict-by-selecting.test.ts index 8169e8840a..85999da55c 100644 --- a/lix/packages/sdk/src/resolve-conflict/resolve-conflict-by-selecting.test.ts +++ b/lix/packages/sdk/src/resolve-conflict/resolve-conflict-by-selecting.test.ts @@ -28,11 +28,13 @@ test("it should resolve a conflict by applying the change and marking the confli plugin_key: "plugin1", type: "mock", file_id: "mock", + entity_id: "value1", snapshot_id: "sn1", }, { plugin_key: "plugin1", file_id: "mock", + entity_id: "value2", type: "mock", snapshot_id: "sn2", }, diff --git a/lix/packages/sdk/src/resolve-conflict/resolve-conflict-with-new-change.test.ts b/lix/packages/sdk/src/resolve-conflict/resolve-conflict-with-new-change.test.ts index c3f19f117b..f28a839873 100644 --- a/lix/packages/sdk/src/resolve-conflict/resolve-conflict-with-new-change.test.ts +++ b/lix/packages/sdk/src/resolve-conflict/resolve-conflict-with-new-change.test.ts @@ -33,9 +33,11 @@ test("it should throw if the to be resolved with change already exists", async ( type: "mock", file_id: "mock", snapshot_id: "sn1", + entity_id: "value1", }, { file_id: "mock", + entity_id: "value2", plugin_key: "plugin1", type: "mock", snapshot_id: "sn2", @@ -120,12 +122,14 @@ test("resolving a conflict should throw if the to be resolved with change is not const mockChanges: NewChange[] = [ { plugin_key: "plugin1", + entity_id: "value1", type: "mock", file_id: "mock", snapshot_id: "sn1", }, { file_id: "mock", + entity_id: "value2", plugin_key: "plugin1", type: "mock", snapshot_id: "sn2", @@ -187,6 +191,7 @@ test("resolving a conflict should throw if the to be resolved with change is not plugin_key: "plugin1", type: "mock", snapshot_id: "sn3", + entity_id: "value3", value: { id: "value3", }, @@ -216,11 +221,13 @@ test("resolving a conflict should throw if the change to resolve with does not b { plugin_key: "plugin1", type: "mock", + entity_id: "value1", file_id: "mock", snapshot_id: "sn1", }, { file_id: "mock", + entity_id: "value2", plugin_key: "plugin1", type: "mock", snapshot_id: "sn2", @@ -274,6 +281,7 @@ test("resolving a conflict should throw if the change to resolve with does not b plugin_key: "plugin1", type: "mock", snapshot_id: "sn3", + entity_id: "value3", value: { id: "value3", }, @@ -301,12 +309,14 @@ test("resolving a conflict with a new change should insert the change and mark t const mockChanges: NewChange[] = [ { plugin_key: "plugin1", + entity_id: "value1", type: "mock", file_id: "mock", snapshot_id: "sn1", }, { file_id: "mock", + entity_id: "value2", plugin_key: "plugin1", type: "mock", snapshot_id: "sn2", @@ -362,6 +372,7 @@ test("resolving a conflict with a new change should insert the change and mark t file_id: "mock", parent_id: changes[0]!.id, plugin_key: "plugin1", + entity_id: "value3", type: "mock", snapshot_id: "sn3", value: { From 754eff8f8996d5487712008f46c7f6354d7f414a Mon Sep 17 00:00:00 2001 From: Martin Lysk Date: Wed, 16 Oct 2024 09:41:19 +0200 Subject: [PATCH 2/2] fixes innerJoin queries on change table --- lix/packages/sdk/src/change-queue.test.ts | 32 +++---------------- lix/packages/sdk/src/commit.test.ts | 14 ++++---- .../sdk/src/discussion/discussion.test.ts | 3 +- lix/packages/sdk/src/merge/merge.test.ts | 6 ++-- lix/packages/sdk/src/merge/merge.ts | 4 +-- lix/packages/sdk/src/mock/mock-csv-plugin.ts | 3 +- .../get-leaf-changes-only-in-source.ts | 4 +-- .../resolve-conflict-by-selecting.ts | 4 +-- .../resolve-conflict-with-new-change.test.ts | 4 +-- 9 files changed, 26 insertions(+), 48 deletions(-) diff --git a/lix/packages/sdk/src/change-queue.test.ts b/lix/packages/sdk/src/change-queue.test.ts index c4c94200ff..202aadc1d9 100644 --- a/lix/packages/sdk/src/change-queue.test.ts +++ b/lix/packages/sdk/src/change-queue.test.ts @@ -82,20 +82,8 @@ test("should use queue and settled correctly", async () => { const changes = await lix.db .selectFrom("change") .innerJoin("snapshot", "snapshot.id", "change.snapshot_id") - .select([ - "change.id", - "change.author", - "change.created_at", - "change.snapshot_id", - "change.parent_id", - "change.entity_id", - "change.type", - "change.file_id", - "change.plugin_key", - "snapshot.value as value", - "change.meta", - "change.commit_id", - ]) + .selectAll('change') + .select('snapshot.value') .execute(); expect(changes).toEqual([ @@ -164,20 +152,8 @@ test("should use queue and settled correctly", async () => { const updatedChanges = await lix.db .selectFrom("change") .innerJoin("snapshot", "snapshot.id", "change.snapshot_id") - .select([ - "change.id", - "change.author", - "change.created_at", - "change.snapshot_id", - "change.parent_id", - "change.entity_id", - "change.type", - "change.file_id", - "change.plugin_key", - "snapshot.value as value", - "change.meta", - "change.commit_id", - ]) + .selectAll('change') + .select('snapshot.value') .execute(); expect(updatedChanges).toEqual([ diff --git a/lix/packages/sdk/src/commit.test.ts b/lix/packages/sdk/src/commit.test.ts index 8e8fd45ab8..671459f894 100644 --- a/lix/packages/sdk/src/commit.test.ts +++ b/lix/packages/sdk/src/commit.test.ts @@ -57,7 +57,8 @@ test("should be able to add and commit changes", async () => { const changes = await lix.db .selectFrom("change") .innerJoin("snapshot as snapshot", "snapshot.id", "change.snapshot_id") - .selectAll() + .selectAll('change') + .select('snapshot.value') .execute(); expect(changes).toEqual([ @@ -83,7 +84,8 @@ test("should be able to add and commit changes", async () => { const commitedChanges = await lix.db .selectFrom("change") .innerJoin("snapshot", "snapshot.id", "change.snapshot_id") - .selectAll() + .selectAll('change') + .select('snapshot.value') .execute(); expect(commitedChanges).toEqual([ @@ -115,12 +117,8 @@ test("should be able to add and commit changes", async () => { const changesAfter = await lix.db .selectFrom("change") .innerJoin("snapshot", "snapshot.id", "change.snapshot_id") - .select([ - "change.id", - "snapshot.value", - "change.commit_id", - "change.parent_id", - ]) + .selectAll('change') + .select('snapshot.value') .execute(); expect(changesAfter).toEqual([ diff --git a/lix/packages/sdk/src/discussion/discussion.test.ts b/lix/packages/sdk/src/discussion/discussion.test.ts index d1c62f63b3..5fd59faf02 100644 --- a/lix/packages/sdk/src/discussion/discussion.test.ts +++ b/lix/packages/sdk/src/discussion/discussion.test.ts @@ -54,8 +54,9 @@ test("should be able to start a discussion on changes", async () => { const changes = await lix.db .selectFrom("change") - .selectAll() .innerJoin("snapshot", "snapshot.id", "change.snapshot_id") + .selectAll('change') + .select('snapshot.value') .execute(); const discussion = await lix.createDiscussion({ diff --git a/lix/packages/sdk/src/merge/merge.test.ts b/lix/packages/sdk/src/merge/merge.test.ts index b0e1e7066a..e8dc3461c2 100644 --- a/lix/packages/sdk/src/merge/merge.test.ts +++ b/lix/packages/sdk/src/merge/merge.test.ts @@ -426,7 +426,8 @@ test("it should apply changes that are not conflicting", async () => { const changes = await targetLix.db .selectFrom("change") .innerJoin("snapshot", "snapshot.id", "change.snapshot_id") - .selectAll() + .selectAll('change') + .select('snapshot.value') .execute(); const conflicts = await targetLix.db @@ -701,7 +702,8 @@ test("it should copy discussion and related comments and mappings", async () => const changes = await lix1.db .selectFrom("change") .innerJoin("snapshot", "snapshot.id", "change.snapshot_id") - .selectAll() + .selectAll('change') + .select('snapshot.value') .execute(); expect(changes).toEqual([ diff --git a/lix/packages/sdk/src/merge/merge.ts b/lix/packages/sdk/src/merge/merge.ts index 92f5ccfc67..e42359c3d4 100644 --- a/lix/packages/sdk/src/merge/merge.ts +++ b/lix/packages/sdk/src/merge/merge.ts @@ -18,8 +18,8 @@ export async function merge(args: { const sourceChangesWithSnapshot = await args.sourceLix.db .selectFrom("change") .innerJoin("snapshot", "snapshot.id", "change.snapshot_id") - .selectAll("change") - .select("snapshot.value as value") + .selectAll('change') + .select('snapshot.value') .execute(); // TODO increase performance by only getting commits diff --git a/lix/packages/sdk/src/mock/mock-csv-plugin.ts b/lix/packages/sdk/src/mock/mock-csv-plugin.ts index d8e19d8a2a..d57be9b229 100644 --- a/lix/packages/sdk/src/mock/mock-csv-plugin.ts +++ b/lix/packages/sdk/src/mock/mock-csv-plugin.ts @@ -41,7 +41,8 @@ export const mockCsvPlugin: LixPlugin<{ const parent = await lix.db .selectFrom("change") .innerJoin("snapshot", "snapshot.id", "change.snapshot_id") - .selectAll() + .selectAll('change') + .select('snapshot.value') .where("change.id", "=", change.parent_id) .executeTakeFirstOrThrow(); diff --git a/lix/packages/sdk/src/query-utilities/get-leaf-changes-only-in-source.ts b/lix/packages/sdk/src/query-utilities/get-leaf-changes-only-in-source.ts index b8c7956619..21d6128d19 100644 --- a/lix/packages/sdk/src/query-utilities/get-leaf-changes-only-in-source.ts +++ b/lix/packages/sdk/src/query-utilities/get-leaf-changes-only-in-source.ts @@ -18,8 +18,8 @@ export async function getLeafChangesOnlyInSource(args: { const leafChangesInSource = await args.sourceLix.db .selectFrom("change") .innerJoin("snapshot", "snapshot.id", "change.snapshot_id") - .selectAll("change") - .select("snapshot.value as value") + .selectAll('change') + .select('snapshot.value') .where( "change.id", "not in", diff --git a/lix/packages/sdk/src/resolve-conflict/resolve-conflict-by-selecting.ts b/lix/packages/sdk/src/resolve-conflict/resolve-conflict-by-selecting.ts index 276451ca30..3f2333515b 100644 --- a/lix/packages/sdk/src/resolve-conflict/resolve-conflict-by-selecting.ts +++ b/lix/packages/sdk/src/resolve-conflict/resolve-conflict-by-selecting.ts @@ -34,8 +34,8 @@ export async function resolveConflictBySelecting(args: { const selectedChange = await args.lix.db .selectFrom("change") .innerJoin("snapshot", "snapshot.id", "change.snapshot_id") - .selectAll("change") - .select("snapshot.value as value") + .selectAll('change') + .select('snapshot.value') .where("change.id", "=", args.selectChangeId) .executeTakeFirst(); diff --git a/lix/packages/sdk/src/resolve-conflict/resolve-conflict-with-new-change.test.ts b/lix/packages/sdk/src/resolve-conflict/resolve-conflict-with-new-change.test.ts index f28a839873..5dd89c279a 100644 --- a/lix/packages/sdk/src/resolve-conflict/resolve-conflict-with-new-change.test.ts +++ b/lix/packages/sdk/src/resolve-conflict/resolve-conflict-with-new-change.test.ts @@ -397,8 +397,8 @@ test("resolving a conflict with a new change should insert the change and mark t const changesAfterResolve = await lix.db .selectFrom("change") .innerJoin("snapshot", "snapshot.id", "change.snapshot_id") - .selectAll("change") - .select("snapshot.value as value") + .selectAll('change') + .select('snapshot.value') .execute(); expect(changesAfterResolve.length).toBe(3);