From bda2ebcf5ccd3e15526e0d372eb465a6b855e93a Mon Sep 17 00:00:00 2001 From: Marc-Antoine Parent Date: Tue, 29 Sep 2026 20:02:54 -0400 Subject: [PATCH 1/3] ENG-2335 Add a shared check for whether an imported node is published to a group --- .../database/src/lib/__tests__/groups.test.ts | 312 ++++++++++++++++++ packages/database/src/lib/groups.ts | 143 ++++++++ 2 files changed, 455 insertions(+) create mode 100644 packages/database/src/lib/__tests__/groups.test.ts diff --git a/packages/database/src/lib/__tests__/groups.test.ts b/packages/database/src/lib/__tests__/groups.test.ts new file mode 100644 index 0000000000..3e4dbd6f06 --- /dev/null +++ b/packages/database/src/lib/__tests__/groups.test.ts @@ -0,0 +1,312 @@ +import { describe, expect, it, vi } from "vitest"; +import type { DGSupabaseClient } from "../client"; +import { getPublishedGroupIdsByRid } from "../groups"; + +type Row = Record; + +const readPath = (row: Row, path: string): unknown => + path + .split(".") + .reduce( + (value, key) => + value && typeof value === "object" ? (value as Row)[key] : undefined, + row, + ); + +// Applies `.in()` and `.not(…, "is", null)` like PostgREST; records `.in()` calls. +const makeClient = (tables: Record) => { + const inCalls: { table: string; column: string; values: unknown[] }[] = []; + const from = vi.fn((table: string) => { + const filters: ((row: Row) => boolean)[] = []; + const builder = { + select: vi.fn(() => builder), + in: vi.fn((column: string, values: unknown[]) => { + inCalls.push({ table, column, values }); + filters.push((row) => values.includes(readPath(row, column))); + return builder; + }), + not: vi.fn((column: string) => { + filters.push((row) => readPath(row, column) != null); + return builder; + }), + then: ( + resolve: (value: { data: Row[]; error: null }) => unknown, + reject?: (reason: unknown) => unknown, + ) => + Promise.resolve({ + data: (tables[table] ?? []).filter((row) => + filters.every((filter) => filter(row)), + ), + error: null, + }).then(resolve, reject), + }; + return builder; + }); + return { client: { from } as unknown as DGSupabaseClient, from, inCalls }; +}; + +const G1 = "group-1"; +const G2 = "group-2"; +const obsidianUrl = "obsidian:vault-a"; +const roamUrl = "https://roamresearch.com/#/app/graph-b"; +const obsidianRid = "orn:obsidian.note:vault-a/node-1"; +const roamRid = `${roamUrl}/uid-1`; + +const obsidianSpace = { spaceId: 20, url: obsidianUrl }; +const roamSpace = { spaceId: 30, url: roamUrl }; + +const sharing = ({ + space, + groupId, + permissions, +}: { + space: { spaceId: number; url: string }; + groupId: string; + permissions: string | null; +}): Row => ({ + space_id: space.spaceId, + group_id: groupId, + sharing_permissions: permissions, + Space: { url: space.url }, +}); + +const grant = ({ + groupId, + space, + sourceLocalId, +}: { + groupId: string; + space: { spaceId: number }; + sourceLocalId: string; +}): Row => ({ + account_uid: groupId, + space_id: space.spaceId, + source_local_id: sourceLocalId, +}); + +describe("getPublishedGroupIdsByRid", () => { + it("returns only the group the node is published to under partial space access", async () => { + const { client } = makeClient({ + my_pseudo_accounts: [ + sharing({ space: obsidianSpace, groupId: G1, permissions: "partial" }), + sharing({ space: obsidianSpace, groupId: G2, permissions: "partial" }), + ], + ResourceAccess: [ + grant({ groupId: G1, space: obsidianSpace, sourceLocalId: "node-1" }), + grant({ groupId: G2, space: obsidianSpace, sourceLocalId: "node-2" }), + ], + }); + + await expect( + getPublishedGroupIdsByRid({ client, rids: [obsidianRid] }), + ).resolves.toEqual({ [obsidianRid]: [G1] }); + }); + + it("returns every group the node is published to", async () => { + const { client } = makeClient({ + my_pseudo_accounts: [ + sharing({ space: roamSpace, groupId: G1, permissions: "partial" }), + sharing({ space: roamSpace, groupId: G2, permissions: "partial" }), + ], + ResourceAccess: [ + grant({ groupId: G1, space: roamSpace, sourceLocalId: "uid-1" }), + grant({ groupId: G2, space: roamSpace, sourceLocalId: "uid-1" }), + ], + }); + + const result = await getPublishedGroupIdsByRid({ client, rids: [roamRid] }); + + expect(result[roamRid]?.sort()).toEqual([G1, G2]); + }); + + it("returns no groups for a node published to none of them", async () => { + const { client } = makeClient({ + my_pseudo_accounts: [ + sharing({ space: obsidianSpace, groupId: G1, permissions: "partial" }), + ], + ResourceAccess: [ + grant({ groupId: G1, space: obsidianSpace, sourceLocalId: "node-2" }), + ], + }); + + await expect( + getPublishedGroupIdsByRid({ client, rids: [obsidianRid] }), + ).resolves.toEqual({ [obsidianRid]: [] }); + }); + + it("counts a group with reader access to the space without a ResourceAccess grant", async () => { + const { client, from } = makeClient({ + my_pseudo_accounts: [ + sharing({ space: obsidianSpace, groupId: G1, permissions: "reader" }), + ], + }); + + await expect( + getPublishedGroupIdsByRid({ client, rids: [obsidianRid] }), + ).resolves.toEqual({ [obsidianRid]: [G1] }); + expect(from).not.toHaveBeenCalledWith("ResourceAccess"); + }); + + it("ignores a group the space belongs to but is not shared with", async () => { + const { client } = makeClient({ + my_pseudo_accounts: [ + sharing({ space: obsidianSpace, groupId: G1, permissions: "partial" }), + sharing({ space: obsidianSpace, groupId: G2, permissions: null }), + ], + ResourceAccess: [ + grant({ groupId: G1, space: obsidianSpace, sourceLocalId: "node-1" }), + grant({ groupId: G2, space: obsidianSpace, sourceLocalId: "node-1" }), + ], + }); + + await expect( + getPublishedGroupIdsByRid({ client, rids: [obsidianRid] }), + ).resolves.toEqual({ [obsidianRid]: [G1] }); + }); + + it("matches grants on both the space and the local id", async () => { + const { client } = makeClient({ + my_pseudo_accounts: [ + sharing({ space: obsidianSpace, groupId: G1, permissions: "partial" }), + sharing({ space: roamSpace, groupId: G2, permissions: "partial" }), + ], + ResourceAccess: [ + grant({ groupId: G1, space: obsidianSpace, sourceLocalId: "node-1" }), + // Same local id as the Obsidian node, in the other space. + grant({ groupId: G2, space: roamSpace, sourceLocalId: "node-1" }), + grant({ groupId: G2, space: roamSpace, sourceLocalId: "uid-1" }), + ], + }); + + await expect( + getPublishedGroupIdsByRid({ client, rids: [obsidianRid, roamRid] }), + ).resolves.toEqual({ [obsidianRid]: [G1], [roamRid]: [G2] }); + }); + + it("returns no groups for a node whose space is not shared with the user's groups", async () => { + const { client, from } = makeClient({ my_pseudo_accounts: [] }); + + await expect( + getPublishedGroupIdsByRid({ client, rids: [obsidianRid] }), + ).resolves.toEqual({ [obsidianRid]: [] }); + expect(from).not.toHaveBeenCalledWith("ResourceAccess"); + }); + + it("queries local ids from all spaces in shared batches", async () => { + const localIds = Array.from({ length: 30 }, (_, i) => `node-${i}`); + const rids = [ + ...localIds.map((id) => `orn:obsidian.note:vault-a/${id}`), + ...localIds.map((id) => `${roamUrl}/uid-${id}`), + ]; + const { client, inCalls } = makeClient({ + my_pseudo_accounts: [ + sharing({ space: obsidianSpace, groupId: G1, permissions: "partial" }), + sharing({ space: roamSpace, groupId: G1, permissions: "partial" }), + ], + }); + + await getPublishedGroupIdsByRid({ client, rids }); + + expect( + inCalls + .filter(({ column }) => column === "source_local_id") + .map(({ values }) => values.length), + ).toEqual([50, 10]); + }); + + it("shrinks batches so local ids × partial groups stay under max_rows", async () => { + const groupIds = Array.from({ length: 30 }, (_, i) => `group-${i}`); + const localIds = Array.from({ length: 100 }, (_, i) => `node-${i}`); + const { client, inCalls } = makeClient({ + my_pseudo_accounts: groupIds.map((groupId) => + sharing({ space: obsidianSpace, groupId, permissions: "partial" }), + ), + ResourceAccess: localIds.flatMap((sourceLocalId) => + groupIds.map((groupId) => + grant({ groupId, space: obsidianSpace, sourceLocalId }), + ), + ), + }); + + const result = await getPublishedGroupIdsByRid({ + client, + rids: localIds.map((id) => `orn:obsidian.note:vault-a/${id}`), + }); + + expect( + inCalls + .filter(({ column }) => column === "source_local_id") + .map(({ values }) => values.length), + ).toEqual([33, 33, 33, 1]); + expect(result["orn:obsidian.note:vault-a/node-99"]).toEqual(groupIds); + }); + + it("lists a group once for a rid repeated across batches", async () => { + const rids = Array.from( + { length: 60 }, + (_, i) => `orn:obsidian.note:vault-a/node-${i}`, + ); + rids[55] = obsidianRid; + const { client } = makeClient({ + my_pseudo_accounts: [ + sharing({ space: obsidianSpace, groupId: G1, permissions: "partial" }), + ], + ResourceAccess: [ + grant({ groupId: G1, space: obsidianSpace, sourceLocalId: "node-1" }), + ], + }); + + const result = await getPublishedGroupIdsByRid({ client, rids }); + + expect(result[obsidianRid]).toEqual([G1]); + }); + + it("counts a grant only for a group with partial access to that space", async () => { + const G3 = "group-3"; + const { client } = makeClient({ + my_pseudo_accounts: [ + sharing({ space: obsidianSpace, groupId: G1, permissions: "reader" }), + sharing({ space: roamSpace, groupId: G1, permissions: "partial" }), + sharing({ space: roamSpace, groupId: G3, permissions: "partial" }), + ], + ResourceAccess: [ + // G1 already reads the whole Obsidian space. + grant({ groupId: G1, space: obsidianSpace, sourceLocalId: "node-1" }), + // G3 is not shared with the Obsidian space. + grant({ groupId: G3, space: obsidianSpace, sourceLocalId: "node-1" }), + grant({ groupId: G3, space: roamSpace, sourceLocalId: "uid-1" }), + ], + }); + + await expect( + getPublishedGroupIdsByRid({ client, rids: [obsidianRid, roamRid] }), + ).resolves.toEqual({ [obsidianRid]: [G1], [roamRid]: [G3] }); + }); + + it("skips the database when there are no rids", async () => { + const { client, from } = makeClient({}); + + await expect( + getPublishedGroupIdsByRid({ client, rids: [] }), + ).resolves.toEqual({}); + expect(from).not.toHaveBeenCalled(); + }); + + it("throws the query error", async () => { + const error = { message: "boom" }; + const builder = { + select: vi.fn(() => builder), + in: vi.fn(() => builder), + not: vi.fn(() => builder), + then: (resolve: (value: unknown) => unknown) => + Promise.resolve({ data: null, error }).then(resolve), + }; + const client = { + from: vi.fn(() => builder), + } as unknown as DGSupabaseClient; + + await expect( + getPublishedGroupIdsByRid({ client, rids: [obsidianRid] }), + ).rejects.toEqual(error); + }); +}); diff --git a/packages/database/src/lib/groups.ts b/packages/database/src/lib/groups.ts index 66848d6171..14597bc2f3 100644 --- a/packages/database/src/lib/groups.ts +++ b/packages/database/src/lib/groups.ts @@ -1,6 +1,7 @@ import type { DGSupabaseClient } from "./client"; import type { Tables, Enums } from "../dbTypes"; import { isIgnorableUpsertError } from "./contextFunctions"; +import { ridToSpaceUriAndLocalId } from "./rid"; export type MyGroup = { id: string; @@ -103,3 +104,145 @@ export const ensurePartialSpaceAccess = async ({ }); return { existing: existingAccessByGroupId }; }; + +// `.in()` filters go in the GET URL, which caps the local ids per batch. +const LOCAL_ID_BATCH_SIZE = 50; +// `max_rows` in supabase/config.toml. Past it, rows are dropped silently, so a +// batch's rows (local ids × groups) must stay under it. +const MAX_ROWS = 1000; + +type SpaceSharing = { + spaceId: number; + fullAccessGroupIds: string[]; + partialAccessGroupIds: string[]; +}; + +// Spaces share with groups by membership. Unlike SpaceAccess, whose select +// policy needs `reader`, this view shows `partial` pairs. +const getSpaceSharingByUrl = async ({ + client, + spaceUris, +}: { + client: DGSupabaseClient; + spaceUris: string[]; +}): Promise> => { + const { data, error } = await client + .from("my_pseudo_accounts") + .select("space_id, group_id, sharing_permissions, Space!inner(url)") + .in("Space.url", spaceUris) + .not("sharing_permissions", "is", null); + if (error) throw error; + const sharingByUrl = new Map(); + for (const { + space_id: spaceId, + group_id: groupId, + sharing_permissions: permissions, + Space: { url }, + } of data) { + if (spaceId === null || groupId === null || permissions === null) continue; + let sharing = sharingByUrl.get(url); + if (!sharing) { + sharing = { spaceId, fullAccessGroupIds: [], partialAccessGroupIds: [] }; + sharingByUrl.set(url, sharing); + } + if (permissions === "partial") sharing.partialAccessGroupIds.push(groupId); + else sharing.fullAccessGroupIds.push(groupId); + } + return sharingByUrl; +}; + +const getGrantedGroupIdsByRid = async ({ + client, + rids, + sharingByUrl, +}: { + client: DGSupabaseClient; + rids: string[]; + sharingByUrl: Map; +}): Promise> => { + const partialRids = [...new Set(rids)].flatMap((rid) => { + const { spaceUri, sourceLocalId } = ridToSpaceUriAndLocalId(rid); + const sharing = sharingByUrl.get(spaceUri); + return sharing?.partialAccessGroupIds.length + ? [{ rid, sourceLocalId, sharing }] + : []; + }); + const groupCount = new Set( + partialRids.flatMap(({ sharing }) => sharing.partialAccessGroupIds), + ).size; + const batchSize = Math.max( + 1, + Math.min(LOCAL_ID_BATCH_SIZE, Math.floor(MAX_ROWS / groupCount)), + ); + const groupIdsByRid = new Map(); + for (let start = 0; start < partialRids.length; start += batchSize) { + const batch = partialRids.slice(start, start + batchSize); + const { data, error } = await client + .from("ResourceAccess") + .select("account_uid, space_id, source_local_id") + .in("space_id", [...new Set(batch.map(({ sharing }) => sharing.spaceId))]) + .in("account_uid", [ + ...new Set( + batch.flatMap(({ sharing }) => sharing.partialAccessGroupIds), + ), + ]) + .in("source_local_id", [ + ...new Set(batch.map(({ sourceLocalId }) => sourceLocalId)), + ]); + if (error) throw error; + // Independent filters: a row can pair a local id with another space of + // the batch, or with a group that is not `partial` there. + const nodeByResource = new Map( + batch.map((node) => [ + `${node.sharing.spaceId}/${node.sourceLocalId}`, + node, + ]), + ); + for (const { account_uid, space_id, source_local_id } of data) { + const node = nodeByResource.get(`${space_id}/${source_local_id}`); + if (!node?.sharing.partialAccessGroupIds.includes(account_uid)) continue; + groupIdsByRid.set(node.rid, [ + ...(groupIdsByRid.get(node.rid) ?? []), + account_uid, + ]); + } + } + return groupIdsByRid; +}; + +// A group counts for a node with `reader` access or higher to its space, or +// with `partial` access and a ResourceAccess grant. +export const getPublishedGroupIdsByRid = async ({ + client, + rids, +}: { + client: DGSupabaseClient; + rids: string[]; +}): Promise> => { + const result: Record = Object.fromEntries( + rids.map((rid) => [rid, []]), + ); + if (rids.length === 0) return result; + const spaceUriByRid = new Map( + rids.map((rid) => [rid, ridToSpaceUriAndLocalId(rid).spaceUri]), + ); + const sharingByUrl = await getSpaceSharingByUrl({ + client, + spaceUris: [...new Set(spaceUriByRid.values())], + }); + const grantedGroupIdsByRid = await getGrantedGroupIdsByRid({ + client, + rids, + sharingByUrl, + }); + + for (const [rid, spaceUri] of spaceUriByRid) { + const sharing = sharingByUrl.get(spaceUri); + if (!sharing) continue; + result[rid] = [ + ...sharing.fullAccessGroupIds, + ...(grantedGroupIdsByRid.get(rid) ?? []), + ]; + } + return result; +}; From 0199fda456b5b399372bf1e50bae9e51d9cc7d3a Mon Sep 17 00:00:00 2001 From: Marc-Antoine Parent Date: Sun, 4 Oct 2026 11:08:22 -0400 Subject: [PATCH 2/3] Correction found against a live check --- packages/database/src/lib/groups.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/database/src/lib/groups.ts b/packages/database/src/lib/groups.ts index 14597bc2f3..0df069399a 100644 --- a/packages/database/src/lib/groups.ts +++ b/packages/database/src/lib/groups.ts @@ -126,9 +126,12 @@ const getSpaceSharingByUrl = async ({ client: DGSupabaseClient; spaceUris: string[]; }): Promise> => { + // The view also reaches Space through LocalAccess; PostgREST needs the join named. const { data, error } = await client .from("my_pseudo_accounts") - .select("space_id, group_id, sharing_permissions, Space!inner(url)") + .select( + "space_id, group_id, sharing_permissions, Space!SpaceAccess_space_id_fkey!inner(url)", + ) .in("Space.url", spaceUris) .not("sharing_permissions", "is", null); if (error) throw error; From 19ffd2709b94717277b6ba7591a5b7530c748144 Mon Sep 17 00:00:00 2001 From: Marc-Antoine Parent Date: Sat, 3 Oct 2026 09:43:50 -0400 Subject: [PATCH 3/3] ENG-2336 Publish cross-space relations from Obsidian --- .../src/utils/__tests__/publishNode.test.ts | 622 ++++++++++++++++++ apps/obsidian/src/utils/publishNode.ts | 199 ++++-- 2 files changed, 774 insertions(+), 47 deletions(-) create mode 100644 apps/obsidian/src/utils/__tests__/publishNode.test.ts diff --git a/apps/obsidian/src/utils/__tests__/publishNode.test.ts b/apps/obsidian/src/utils/__tests__/publishNode.test.ts new file mode 100644 index 0000000000..40b27e6d36 --- /dev/null +++ b/apps/obsidian/src/utils/__tests__/publishNode.test.ts @@ -0,0 +1,622 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import type { DGSupabaseClient } from "@repo/database/lib/client"; +import type DiscourseGraphPlugin from "~/index"; +import type { RelationInstance } from "~/types"; +import type { DiscourseNodeInVault } from "~/utils/getDiscourseNodes"; +import type { SupabaseContext } from "~/utils/supabaseContext"; +import type { TFile } from "obsidian"; +import { + ensurePartialSpaceAccess, + getAvailableGroupIds, + getPublishedGroupIdsByRid, +} from "@repo/database/lib/groups"; +import { + ensurePublishedRelationsAccuracy, + getPublishedGroupsByNode, + publishNewRelation, + publishNodeRelations, + publishNodeToGroup, +} from "~/utils/publishNode"; +import { getLoggedInClient, getSupabaseContext } from "~/utils/supabaseContext"; +import { + findEmbeddedAttachments, + syncAllNodesAndRelations, +} from "~/utils/syncDgNodesToSupabase"; +import { + getFileForNodeInstanceId, + getFileForNodeInstanceIds, + getRelationsForNodeInstanceId, + loadRelations, + saveRelations, +} from "~/utils/relationsStore"; + +vi.mock("@repo/database/lib/groups", () => ({ + ensurePartialSpaceAccess: vi.fn(), + getAvailableGroupIds: vi.fn(), + getPublishedGroupIdsByRid: vi.fn(), +})); + +vi.mock("~/utils/relationsStore", async (importOriginal) => ({ + ...(await importOriginal()), + saveRelations: vi.fn(), + getRelationsForNodeInstanceId: vi.fn(), + getFileForNodeInstanceIds: vi.fn(), + loadRelations: vi.fn(), + getFileForNodeInstanceId: vi.fn(), +})); + +vi.mock("~/utils/supabaseContext", () => ({ + getLoggedInClient: vi.fn(), + getSupabaseContext: vi.fn(), +})); + +vi.mock("~/utils/syncDgNodesToSupabase", () => ({ + syncAllNodesAndRelations: vi.fn(), + findEmbeddedAttachments: vi.fn(), + syncPublishedNodeAssets: vi.fn(), +})); + +const SPACE_ID = 7; +const IMPORTED_RID = "orn:obsidian.note:other-vault/imported-node"; +const OTHER_IMPORTED_RID = "orn:obsidian.note:other-vault/other-imported-node"; + +const client = {} as DGSupabaseClient; + +beforeEach(() => { + vi.mocked(getPublishedGroupIdsByRid).mockReset(); + vi.mocked(getAvailableGroupIds).mockReset(); + vi.mocked(ensurePartialSpaceAccess).mockReset(); + vi.mocked(ensurePartialSpaceAccess).mockImplementation(({ groupIds }) => + Promise.resolve({ + existing: Object.fromEntries(groupIds.map((g) => [g, "partial"])), + }), + ); +}); + +afterEach(() => { + vi.restoreAllMocks(); +}); + +const failSpaceAccess = (): void => { + vi.spyOn(console, "error").mockImplementation(() => {}); + vi.mocked(ensurePartialSpaceAccess).mockImplementation(({ groupIds }) => + Promise.resolve({ + existing: {}, + missing: Object.fromEntries(groupIds.map((g) => [g, "partial"])), + }), + ); +}; + +describe("getPublishedGroupsByNode", () => { + it("reads a local node's groups from its frontmatter", async () => { + vi.mocked(getPublishedGroupIdsByRid).mockResolvedValue({}); + + const groups = await getPublishedGroupsByNode({ + client, + frontmatterByNode: { local: { publishedToGroups: ["g1", "g2"] } }, + }); + + expect(groups).toEqual({ local: new Set(["g1", "g2"]) }); + expect(getPublishedGroupIdsByRid).toHaveBeenCalledWith({ + client, + rids: [], + }); + }); + + it("asks the shared check for an imported node's groups, ignoring its frontmatter", async () => { + vi.mocked(getPublishedGroupIdsByRid).mockResolvedValue({ + [IMPORTED_RID]: ["g1"], + }); + + const groups = await getPublishedGroupsByNode({ + client, + frontmatterByNode: { + imported: { importedFromRid: IMPORTED_RID, publishedToGroups: ["g2"] }, + }, + }); + + expect(groups).toEqual({ imported: new Set(["g1"]) }); + }); + + it("looks up a rid shared by two nodes once", async () => { + vi.mocked(getPublishedGroupIdsByRid).mockResolvedValue({ + [IMPORTED_RID]: ["g1"], + }); + + const groups = await getPublishedGroupsByNode({ + client, + frontmatterByNode: { + first: { importedFromRid: IMPORTED_RID }, + second: { importedFromRid: IMPORTED_RID }, + }, + }); + + expect(groups).toEqual({ + first: new Set(["g1"]), + second: new Set(["g1"]), + }); + expect(getPublishedGroupIdsByRid).toHaveBeenCalledWith({ + client, + rids: [IMPORTED_RID], + }); + }); + + it("gives no groups to an imported node the shared check does not return", async () => { + vi.mocked(getPublishedGroupIdsByRid).mockResolvedValue({}); + + const groups = await getPublishedGroupsByNode({ + client, + frontmatterByNode: { imported: { importedFromRid: IMPORTED_RID } }, + }); + + expect(groups).toEqual({ imported: new Set() }); + }); +}); + +type Call = { table: string; operation: string; args: unknown[] }; + +// Builder methods chain; awaiting records the call and resolves per table and operation. +const createFakeClient = ({ + syncedRelationIds, + grantedLocalIdsByGroup, + failingWriteTables = [], +}: { + syncedRelationIds: string[]; + grantedLocalIdsByGroup: Record; + failingWriteTables?: string[]; +}): { client: DGSupabaseClient; calls: Call[] } => { + const calls: Call[] = []; + const from = (table: string) => { + const call: Call = { table, operation: "", args: [] }; + const filters: Record = {}; + const builder: Record = {}; + for (const operation of ["select", "delete", "upsert"]) { + builder[operation] = (...args: unknown[]) => { + call.operation = operation; + call.args = args; + return builder; + }; + } + builder.eq = (column: string, value: unknown) => { + filters[column] = value; + return builder; + }; + builder.in = () => builder; + builder.maybeSingle = () => builder; + builder.then = (resolve: (result: unknown) => void) => { + calls.push(call); + if (call.operation !== "select") + return resolve({ + error: failingWriteTables.includes(table) + ? { code: "42501", message: "denied" } + : null, + }); + if (table === "my_contents") + return resolve({ + data: { last_modified: "2026-01-01T00:00:00" }, + error: null, + }); + const ids = + table === "Concept" + ? syncedRelationIds + : (grantedLocalIdsByGroup[filters.account_uid as string] ?? []); + resolve({ + data: ids.map((source_local_id) => ({ source_local_id })), + error: null, + }); + }; + return builder; + }; + return { client: { from } as unknown as DGSupabaseClient, calls }; +}; + +const node = ( + nodeInstanceId: string, + frontmatter: Record, +): DiscourseNodeInVault => + ({ + nodeInstanceId, + nodeTypeId: "claim", + frontmatter, + file: { basename: nodeInstanceId }, + }) as unknown as DiscourseNodeInVault; + +const relation = ( + id: string, + source: string, + destination: string, +): RelationInstance => ({ + id, + type: "supports", + source, + destination, + created: 0, +}); + +const allNodesById = Object.fromEntries( + [ + node("localG1", { publishedToGroups: ["g1"] }), + node("localG2", { publishedToGroups: ["g2"] }), + node("imported", { importedFromRid: IMPORTED_RID }), + node("otherImported", { importedFromRid: OTHER_IMPORTED_RID }), + ].map((n) => [n.nodeInstanceId, n]), +); + +const runAccuracyCheck = async ({ + relations, + grantedLocalIdsByGroup = {}, +}: { + relations: RelationInstance[]; + grantedLocalIdsByGroup?: Record; +}): Promise => { + vi.mocked(getAvailableGroupIds).mockResolvedValue(["g1", "g2"]); + const fake = createFakeClient({ + syncedRelationIds: relations.map((r) => r.id), + grantedLocalIdsByGroup, + }); + await ensurePublishedRelationsAccuracy({ + client: fake.client, + context: { spaceId: SPACE_ID } as SupabaseContext, + plugin: {} as DiscourseGraphPlugin, + allNodesById, + relationInstancesData: { + version: 1, + lastModified: 0, + relations: Object.fromEntries(relations.map((r) => [r.id, r])), + }, + }); + return fake.calls; +}; + +const upsertedRows = (calls: Call[]): unknown => + calls + .filter((c) => c.table === "ResourceAccess" && c.operation === "upsert") + .flatMap((c) => c.args[0] as unknown[]); + +describe("ensurePublishedRelationsAccuracy", () => { + beforeEach(() => { + vi.mocked(getPublishedGroupIdsByRid).mockResolvedValue({ + [IMPORTED_RID]: ["g1"], + [OTHER_IMPORTED_RID]: ["g1"], + }); + }); + + it("publishes a relation between a local and an imported node to their shared group", async () => { + const calls = await runAccuracyCheck({ + relations: [relation("rel", "localG1", "imported")], + }); + + expect(upsertedRows(calls)).toEqual([ + { source_local_id: "rel", space_id: SPACE_ID, account_uid: "g1" }, + ]); + }); + + it("publishes a relation between two imported nodes to their shared group", async () => { + const calls = await runAccuracyCheck({ + relations: [relation("rel", "imported", "otherImported")], + }); + + expect(upsertedRows(calls)).toEqual([ + { source_local_id: "rel", space_id: SPACE_ID, account_uid: "g1" }, + ]); + expect(ensurePartialSpaceAccess).toHaveBeenCalledWith( + expect.objectContaining({ groupIds: ["g1"], spaceId: SPACE_ID }), + ); + }); + + it("does not publish a relation to a group whose space access cannot be written", async () => { + failSpaceAccess(); + + const calls = await runAccuracyCheck({ + relations: [relation("rel", "imported", "otherImported")], + }); + + expect(upsertedRows(calls)).toEqual([]); + expect(console.error).toHaveBeenCalledWith( + expect.stringContaining("access to the space"), + { g1: "partial" }, + ); + }); + + it("still saves deleted grants when the space access lookup throws", async () => { + vi.mocked(ensurePartialSpaceAccess).mockRejectedValue(new Error("down")); + vi.spyOn(console, "error").mockImplementation(() => {}); + vi.mocked(saveRelations).mockClear(); + const stale = { + ...relation("stale", "localG2", "imported"), + publishedToGroupId: ["g1"], + }; + + const calls = await runAccuracyCheck({ + relations: [relation("rel", "localG1", "imported"), stale], + grantedLocalIdsByGroup: { g1: ["stale"] }, + }); + + expect(upsertedRows(calls)).toEqual([]); + expect(stale.publishedToGroupId).toEqual([]); + expect(saveRelations).toHaveBeenCalled(); + }); + + it("does not publish a relation whose ends share no group", async () => { + const calls = await runAccuracyCheck({ + relations: [relation("rel", "localG2", "imported")], + }); + + expect(upsertedRows(calls)).toEqual([]); + }); + + it("leaves existing grants alone when the shared check fails", async () => { + vi.mocked(getPublishedGroupIdsByRid).mockRejectedValue(new Error("down")); + vi.spyOn(console, "error").mockImplementation(() => {}); + + const calls = await runAccuracyCheck({ + relations: [relation("rel", "localG1", "imported")], + grantedLocalIdsByGroup: { g1: ["rel"] }, + }); + + expect( + calls.filter( + (c) => c.table === "ResourceAccess" && c.operation !== "select", + ), + ).toEqual([]); + }); +}); + +const TRIPLE = { + id: "claim-supports-claim", + relationshipTypeId: "supports", + sourceId: "claim", + destinationId: "claim", +}; + +const pluginWithNodes = ( + nodes: Record, +): DiscourseGraphPlugin => { + const frontmatterById = Object.fromEntries( + Object.values(nodes).map((n) => [ + n.nodeInstanceId, + { + nodeInstanceId: n.nodeInstanceId, + nodeTypeId: "claim", + ...n.frontmatter, + }, + ]), + ); + return { + settings: { discourseRelations: [TRIPLE], relationTypes: [] }, + app: { + metadataCache: { + getFileCache: (file: TFile) => ({ + frontmatter: frontmatterById[file.basename], + }), + }, + }, + } as unknown as DiscourseGraphPlugin; +}; + +const grantRows = (group: string, relationId: string): unknown[] => + [relationId, "supports", TRIPLE.id].map((source_local_id) => ({ + account_uid: group, + source_local_id, + space_id: SPACE_ID, + })); + +describe("publishNewRelation", () => { + const runPublish = async ( + newRelation: RelationInstance, + ): Promise<{ published: boolean; calls: Call[] }> => { + vi.mocked(getAvailableGroupIds).mockResolvedValue(["g1", "g2"]); + vi.mocked(getFileForNodeInstanceId).mockImplementation( + (_plugin, id) => allNodesById[id]?.file ?? null, + ); + const fake = createFakeClient({ + syncedRelationIds: [], + grantedLocalIdsByGroup: {}, + }); + vi.mocked(getLoggedInClient).mockResolvedValue(fake.client); + vi.mocked(getSupabaseContext).mockResolvedValue({ + spaceId: SPACE_ID, + } as SupabaseContext); + const published = await publishNewRelation( + pluginWithNodes(allNodesById), + newRelation, + ); + return { published, calls: fake.calls }; + }; + + beforeEach(() => { + vi.mocked(syncAllNodesAndRelations).mockReset(); + vi.mocked(getPublishedGroupIdsByRid).mockResolvedValue({ + [IMPORTED_RID]: ["g1"], + [OTHER_IMPORTED_RID]: ["g1"], + }); + }); + + it("publishes a new relation between a local and an imported node to their shared group", async () => { + const newRelation = relation("rel", "localG1", "imported"); + + const { published, calls } = await runPublish(newRelation); + + expect(published).toBe(true); + expect(upsertedRows(calls)).toEqual(grantRows("g1", "rel")); + expect(newRelation.publishedToGroupId).toEqual(["g1"]); + }); + + it("publishes a new relation between two imported nodes to their shared group", async () => { + const { published, calls } = await runPublish( + relation("rel", "imported", "otherImported"), + ); + + expect(published).toBe(true); + expect(upsertedRows(calls)).toEqual(grantRows("g1", "rel")); + expect(ensurePartialSpaceAccess).toHaveBeenCalledWith( + expect.objectContaining({ groupIds: ["g1"], spaceId: SPACE_ID }), + ); + }); + + it("does not publish a new relation to a group whose space access cannot be written", async () => { + failSpaceAccess(); + + const { published, calls } = await runPublish( + relation("rel", "imported", "otherImported"), + ); + + expect(published).toBe(false); + expect(upsertedRows(calls)).toEqual([]); + }); + + it("does not publish a new relation whose ends share no group", async () => { + const { published, calls } = await runPublish( + relation("rel", "localG2", "imported"), + ); + + expect(published).toBe(false); + expect(upsertedRows(calls)).toEqual([]); + expect(syncAllNodesAndRelations).not.toHaveBeenCalled(); + }); +}); + +describe("publishNodeRelations", () => { + // `publishing` is being published to g1, so its frontmatter lacks g1. + const runPublish = async (relations: RelationInstance[]): Promise => { + vi.mocked(getRelationsForNodeInstanceId).mockResolvedValue(relations); + vi.mocked(loadRelations).mockResolvedValue({ + version: 1, + lastModified: 0, + relations: Object.fromEntries(relations.map((r) => [r.id, r])), + }); + const nodes: Record = { + ...allNodesById, + publishing: node("publishing", { nodeTypeId: "claim" }), + }; + vi.mocked(getFileForNodeInstanceIds).mockImplementation((_plugin, ids) => + Object.fromEntries( + [...ids].flatMap((id) => { + const n = nodes[id]; + return n ? [[id, n.file]] : []; + }), + ), + ); + const plugin = pluginWithNodes(nodes); + const fake = createFakeClient({ + syncedRelationIds: [], + grantedLocalIdsByGroup: {}, + }); + await publishNodeRelations({ + plugin, + client: fake.client, + nodeId: "publishing", + myGroup: "g1", + spaceId: SPACE_ID, + }); + return fake.calls; + }; + + beforeEach(() => { + vi.mocked(getPublishedGroupIdsByRid).mockResolvedValue({ + [IMPORTED_RID]: ["g1"], + [OTHER_IMPORTED_RID]: ["g2"], + }); + }); + + it("publishes a relation to an imported node published to the group", async () => { + const calls = await runPublish([relation("rel", "publishing", "imported")]); + + expect(upsertedRows(calls)).toEqual(grantRows("g1", "rel")); + }); + + it("does not publish a relation to an imported node published to another group", async () => { + const calls = await runPublish([ + relation("rel", "publishing", "otherImported"), + ]); + + expect(upsertedRows(calls)).toEqual([]); + }); + + it("does not publish a relation imported from another space", async () => { + const calls = await runPublish([ + { + ...relation("rel", "publishing", "imported"), + importedFromRid: IMPORTED_RID, + }, + ]); + + expect(upsertedRows(calls)).toEqual([]); + }); +}); + +describe("publishNodeToGroup", () => { + const runPublishToGroup = async ( + failingWriteTables: string[] = [], + ): Promise<{ result: Promise; calls: Call[] }> => { + const relations = [relation("rel", "publishing", "imported")]; + vi.mocked(getRelationsForNodeInstanceId).mockResolvedValue(relations); + vi.mocked(loadRelations).mockResolvedValue({ + version: 1, + lastModified: 0, + relations: Object.fromEntries(relations.map((r) => [r.id, r])), + }); + const nodes: Record = { + ...allNodesById, + publishing: node("publishing", {}), + }; + vi.mocked(getFileForNodeInstanceIds).mockImplementation((_plugin, ids) => + Object.fromEntries( + [...ids].flatMap((id) => { + const n = nodes[id]; + return n ? [[id, n.file]] : []; + }), + ), + ); + vi.mocked(findEmbeddedAttachments).mockReturnValue([]); + const fake = createFakeClient({ + syncedRelationIds: [], + grantedLocalIdsByGroup: {}, + failingWriteTables, + }); + vi.mocked(getLoggedInClient).mockResolvedValue(fake.client); + vi.mocked(getSupabaseContext).mockResolvedValue({ + spaceId: SPACE_ID, + } as SupabaseContext); + const result = publishNodeToGroup({ + plugin: pluginWithNodes(nodes), + file: { stat: { mtime: 0 } } as unknown as TFile, + frontmatter: { nodeInstanceId: "publishing" }, + myGroup: "g1", + skipFrontmatterUpdate: true, + }); + await result.catch(() => {}); + return { result, calls: fake.calls }; + }; + + // The node's own grant is a single row; relation grants are an array. + const isRelationGrant = (c: Call): boolean => + c.table === "ResourceAccess" && + c.operation === "upsert" && + Array.isArray(c.args[0]); + + beforeEach(() => { + vi.mocked(getPublishedGroupIdsByRid).mockResolvedValue({ + [IMPORTED_RID]: ["g1"], + }); + }); + + it("publishes the node's relations after its space access", async () => { + const { result, calls } = await runPublishToGroup(); + + await expect(result).resolves.toBeUndefined(); + const spaceAccessIndex = calls.findIndex( + (c) => c.table === "SpaceAccess" && c.operation === "upsert", + ); + const relationGrantIndex = calls.findIndex(isRelationGrant); + expect(spaceAccessIndex).toBeGreaterThanOrEqual(0); + expect(relationGrantIndex).toBeGreaterThan(spaceAccessIndex); + }); + + it("does not publish the node's relations when its space access cannot be written", async () => { + const { result, calls } = await runPublishToGroup(["SpaceAccess"]); + + await expect(result).rejects.toMatchObject({ code: "42501" }); + expect(calls.filter(isRelationGrant)).toEqual([]); + }); +}); diff --git a/apps/obsidian/src/utils/publishNode.ts b/apps/obsidian/src/utils/publishNode.ts index ca63b9546b..6a527ddfb7 100644 --- a/apps/obsidian/src/utils/publishNode.ts +++ b/apps/obsidian/src/utils/publishNode.ts @@ -11,7 +11,11 @@ import { type RelationsFile, } from "./relationsStore"; import type { RelationInstance } from "~/types"; -import { getAvailableGroupIds } from "@repo/database/lib/groups"; +import { + ensurePartialSpaceAccess, + getAvailableGroupIds, + getPublishedGroupIdsByRid, +} from "@repo/database/lib/groups"; import { findEmbeddedAttachments, syncAllNodesAndRelations, @@ -32,6 +36,66 @@ export const getPublishedToGroups = ( return publishedToGroups.filter((g): g is string => typeof g === "string"); }; +// An imported node's `publishedToGroups` is a stale copy from its source; ask its source space. +export const getPublishedGroupsByNode = async ({ + client, + frontmatterByNode, +}: { + client: DGSupabaseClient; + frontmatterByNode: Record>; +}): Promise>> => { + const importedRidByNode = Object.fromEntries( + Object.entries>( + frontmatterByNode, + ).flatMap(([node, fm]) => + typeof fm.importedFromRid === "string" + ? [[node, fm.importedFromRid]] + : [], + ), + ); + const groupIdsByRid = await getPublishedGroupIdsByRid({ + client, + rids: [...new Set(Object.values(importedRidByNode))], + }); + return Object.fromEntries( + Object.entries>( + frontmatterByNode, + ).map(([node, fm]) => { + const importedRid = importedRidByNode[node]; + return [ + node, + new Set( + importedRid === undefined + ? getPublishedToGroups(fm) + : (groupIdsByRid[importedRid] ?? []), + ), + ]; + }), + ) as Record>; +}; + +// Group members cannot see a space without a SpaceAccess row, so a grant +// without one cannot be imported. +const getGroupIdsWithSpaceAccess = async ({ + client, + spaceId, + groupIds, +}: { + client: DGSupabaseClient; + spaceId: number; + groupIds: string[]; +}): Promise> => { + if (groupIds.length === 0) return new Set(); + const { existing, missing } = await ensurePartialSpaceAccess({ + client, + groupIds, + spaceId, + }); + if (missing) + console.error("Could not give these groups access to the space", missing); + return new Set(groupIds.filter((groupId) => groupId in existing)); +}; + const publishSchema = async ({ client, spaceId, @@ -98,12 +162,6 @@ export const publishNewRelation = async ( plugin.app.metadataCache.getFileCache(destinationFile)?.frontmatter; if (!sourceFm || !destinationFm) return false; - const sourceGroups = sourceFm.publishedToGroups as string[] | undefined; - const destinationGroups = destinationFm.publishedToGroups as - | string[] - | undefined; - if (!Array.isArray(sourceGroups) || !Array.isArray(destinationGroups)) - return false; const relationTriples = plugin.settings.discourseRelations ?? []; const triple = relationTriples.find( (triple) => @@ -119,14 +177,23 @@ export const publishNewRelation = async ( if (relationType && isProvisionalSchema(relationType)) return false; if (relation.tentative === false) return false; const resourceIds = [relation.id, relation.type, triple.id]; + const { source: sourceGroups, destination: destinationGroups } = + await getPublishedGroupsByNode({ + client, + frontmatterByNode: { source: sourceFm, destination: destinationFm }, + }); + if (!sourceGroups.size || !destinationGroups.size) return false; const myGroups = await getAvailableGroupIds(client); - const targetGroups = intersection( - new Set(myGroups), - intersection( - new Set(sourceGroups), - new Set(destinationGroups), - ), - ); + const targetGroups = await getGroupIdsWithSpaceAccess({ + client, + spaceId: context.spaceId, + groupIds: [ + ...intersection( + new Set(myGroups), + intersection(sourceGroups, destinationGroups), + ), + ], + }); if (!targetGroups.size) return false; // in that case, sync all relations (only) before publishing await syncAllNodesAndRelations(plugin, context, true); @@ -167,7 +234,9 @@ export const publishNodeRelations = async ({ myGroup: string; spaceId: number; }): Promise => { - const relations = await getRelationsForNodeInstanceId(plugin, nodeId); + const relations = ( + await getRelationsForNodeInstanceId(plugin, nodeId) + ).filter((relation) => !relation.importedFromRid); const resourceIds: Set = new Set(); const relationTriples = plugin.settings.discourseRelations ?? []; const relevantNodeIds: Set = new Set(); @@ -176,18 +245,25 @@ export const publishNodeRelations = async ({ relevantNodeIds.add(relation.destination); }); const relevantNodeFiles = getFileForNodeInstanceIds(plugin, relevantNodeIds); - const relevantNodeTypeById: Record = {}; + const frontmatterById: Record = {}; Object.entries(relevantNodeFiles).map(([id, file]: [string, TFile]) => { const fm = plugin.app.metadataCache.getFileCache(file)?.frontmatter; - if (fm === undefined) return; - if (fm.nodeInstanceId !== nodeId) { - // check if published to same group. - // Note: current node's pub status not in cache yet! - if (!Array.isArray(fm.publishedToGroups)) return; - const publishedToGroups: string[] = - (fm.publishedToGroups as string[]) || []; - if (!publishedToGroups.includes(myGroup)) return; - } + if (fm !== undefined) frontmatterById[id] = fm; + }); + // The current node's frontmatter does not list myGroup yet. + const otherNodeFrontmatterById = Object.fromEntries( + Object.entries(frontmatterById).filter( + ([, fm]) => fm.nodeInstanceId !== nodeId, + ), + ); + const groupsByOtherNodeId = await getPublishedGroupsByNode({ + client, + frontmatterByNode: otherNodeFrontmatterById, + }); + const relevantNodeTypeById: Record = {}; + Object.entries(frontmatterById).map(([id, fm]) => { + if (fm.nodeInstanceId !== nodeId && !groupsByOtherNodeId[id]?.has(myGroup)) + return; relevantNodeTypeById[id] = fm.nodeTypeId as string; }); relations.map((relation) => { @@ -308,23 +384,35 @@ export const ensurePublishedRelationsAccuracy = async ({ } } } + const ownRelations = relationInstances.filter( + (r) => !r.importedFromRid && syncedRelationIds.has(r.id), + ); + let groupsByNodeId: Record>; + try { + groupsByNodeId = await getPublishedGroupsByNode({ + client, + frontmatterByNode: Object.fromEntries( + ownRelations + .flatMap((r) => [r.source, r.destination]) + .flatMap((id) => { + const frontmatter = allNodesById[id]?.frontmatter; + return frontmatter ? [[id, frontmatter]] : []; + }), + ), + }); + } catch (error) { + // Going on without these groups would delete valid grants. Local-only + // relations are not reconciled either; the next full sync recovers. + console.error("Could not get the groups of imported nodes", error); + return; + } let changed = false; const missingPublishRecords: TablesInsert<"ResourceAccess">[] = []; for (const group of myGroups) { - const publishableRelations = relationInstances.filter( + const publishableRelations = ownRelations.filter( (r) => - !r.importedFromRid && - syncedRelationIds.has(r.id) && - ( - (allNodesById[r.source]?.frontmatter?.publishedToGroups as - | string[] - | undefined) || [] - ).indexOf(group) >= 0 && - ( - (allNodesById[r.destination]?.frontmatter?.publishedToGroups as - | string[] - | undefined) || [] - ).indexOf(group) >= 0, + groupsByNodeId[r.source]?.has(group) && + groupsByNodeId[r.destination]?.has(group), ); const publishableRelationIds = new Set( publishableRelations.map((x) => x.id), @@ -379,11 +467,27 @@ export const ensurePublishedRelationsAccuracy = async ({ } } } - if (missingPublishRecords.length > 0) { - const r = await client.from("ResourceAccess").upsert(missingPublishRecords); + let groupIdsWithSpaceAccess = new Set(); + try { + groupIdsWithSpaceAccess = await getGroupIdsWithSpaceAccess({ + client, + spaceId: context.spaceId, + groupIds: [ + ...new Set(missingPublishRecords.map((record) => record.account_uid)), + ], + }); + } catch (error) { + // No early return: the deletes above still need saving. + console.error("Could not give groups access to the space", error); + } + const publishRecords = missingPublishRecords.filter((record) => + groupIdsWithSpaceAccess.has(record.account_uid), + ); + if (publishRecords.length > 0) { + const r = await client.from("ResourceAccess").upsert(publishRecords); if (r.error) console.error(r.error); else { - for (const record of missingPublishRecords) { + for (const record of publishRecords) { const rel = relationInstancesData.relations[record.source_local_id]; const group = record.account_uid; const pos = (rel?.publishedToGroupId || []).indexOf(group); @@ -436,12 +540,6 @@ export const publishNodeToGroup = async ({ const lastModifiedDb = new Date( idResponse.data.last_modified + "Z", ).getTime(); - try { - await publishNodeRelations({ plugin, client, nodeId, myGroup, spaceId }); - } catch (error) { - // do not fail to publish node for that reason - console.error("Could not publish relations", error); - } const attachments = findEmbeddedAttachments(plugin, file); const lastModified = Math.max( file.stat.mtime, @@ -487,6 +585,13 @@ export const publishNodeToGroup = async ({ }); } } + // After SpaceAccess: group members cannot import a grant without it. + try { + await publishNodeRelations({ plugin, client, nodeId, myGroup, spaceId }); + } catch (error) { + // do not fail to publish node for that reason + console.error("Could not publish relations", error); + } await syncPublishedNodeAssets({ plugin, client,