diff --git a/server/auth-realtime/src/hocuspocus.test.ts b/server/auth-realtime/src/hocuspocus.test.ts index 45daa1d2..02acf432 100644 --- a/server/auth-realtime/src/hocuspocus.test.ts +++ b/server/auth-realtime/src/hocuspocus.test.ts @@ -426,6 +426,56 @@ describe("createDocumentHooks", () => { ).toEqual(["user-1", "user-2"]) }) + it("moves the latest editor to the end", async ({ expect }) => { + const core = stubCore() + const hooks = hooksWith(core) + + await hooks.onChange({ + context: { session: SESSION }, + documentName: "doc1-branch1", + }) + await hooks.onChange({ + context: { + session: { user: { id: "user-2" } }, + }, + documentName: "doc1-branch1", + }) + await hooks.onChange({ + context: { session: SESSION }, + documentName: "doc1-branch1", + }) + await hooks.onStoreDocument({ + documentName: "doc1-branch1", + document: seededDocument(), + }) + + expect( + core.storeBranchContent.mock.calls[0]?.[2] + .maintainers, + ).toEqual(["user-2", "user-1"]) + }) + + it("records the user a direct connection acts for", async ({ + expect, + }) => { + const core = stubCore() + const hooks = hooksWith(core) + + await hooks.onChange({ + context: { userId: "user-3" }, + documentName: "doc1-branch1", + }) + await hooks.onStoreDocument({ + documentName: "doc1-branch1", + document: seededDocument(), + }) + + expect( + core.storeBranchContent.mock.calls[0]?.[2] + .maintainers, + ).toEqual(["user-3"]) + }) + it("ignores a change carrying no session", async ({ expect, }) => { @@ -829,6 +879,65 @@ describe("createDocumentHooks", () => { ).toBe(false) }) + // a direct connection stores on disconnect even when its batch + // changed nothing, so the window has no provenance of its own + it.for([ + { + name: "a system connection", + input: { system: true }, + expected: true, + }, + { + name: "an ordinary connection", + input: { userId: "user-1" }, + expected: false, + }, + { + name: "no connection context", + input: undefined, + expected: false, + }, + ])( + "persists an unchanged window with the permission of $name", + async ({ input, expected }, { expect }) => { + const core = stubCore() + const hooks = hooksWith(core) + + await hooks.onStoreDocument({ + documentName: "doc1-branch1", + document: seededDocument(), + lastContext: input, + }) + + expect( + core.storeBranchContent.mock + .calls[0]?.[2].system, + ).toBe(expected) + }, + ) + + it("persists an edited window as an ordinary write whoever stores it", async ({ + expect, + }) => { + const core = stubCore() + const hooks = hooksWith(core) + + await hooks.onChange({ + context: { session: SESSION }, + documentName: "doc1-branch1", + }) + await hooks.onStoreDocument({ + documentName: "doc1-branch1", + document: seededDocument(), + lastContext: { system: true }, + }) + + expect( + core.storeBranchContent.mock.calls[0]?.[2] + .system, + ).toBe(false) + }) + // the editors keep their unsaved work either way; telling them // the persist failed is the only thing left to do it("warns the connected editors instead of throwing when the persist fails", async ({ @@ -1064,8 +1173,21 @@ describe("createDocumentHooks", () => { }) describe("resetConnections", () => { + // socket builds a writable connection whose close records whether the + // connection was already read-only when its socket was closed. function socket() { - return { webSocket: { close: vi.fn() } } + const connection = { + readOnly: false, + readOnlyAtClose: undefined as boolean | undefined, + webSocket: { + close: vi.fn(() => { + connection.readOnlyAtClose = + connection.readOnly + }), + }, + } + + return connection } // the provider reopens a document after losing its socket, which is @@ -1090,14 +1212,15 @@ describe("resetConnections", () => { resetConnections(server, "doc1-branch1") - expect(first.webSocket.close).toHaveBeenCalledWith( - 4205, - "Reset Connection", - ) - expect(second.webSocket.close).toHaveBeenCalledWith( - 4205, - "Reset Connection", - ) + // an update that arrives before the socket reports the close is + // refused, so the connection turns read-only before the close. + for (const connection of [first, second]) { + expect(connection.webSocket.close.mock.calls).toEqual([ + [4205, "Reset Connection"], + ]) + expect(connection.readOnlyAtClose).toBe(true) + expect(connection.readOnly).toBe(true) + } }) it("leaves other documents' sockets alone", ({ expect }) => { @@ -1114,6 +1237,7 @@ describe("resetConnections", () => { resetConnections(server, "doc1-branch1") expect(other.webSocket.close).toHaveBeenCalledTimes(0) + expect(other.readOnly).toBe(false) }) }) diff --git a/server/auth-realtime/src/hocuspocus.ts b/server/auth-realtime/src/hocuspocus.ts index 142b3834..b3213f98 100644 --- a/server/auth-realtime/src/hocuspocus.ts +++ b/server/auth-realtime/src/hocuspocus.ts @@ -47,14 +47,15 @@ interface FlushableServer { } } -// the part a reset reaches into: the socket behind each of a document's -// client connections. +// the part a reset reaches into: each of a document's client connections +// and the socket behind it. interface ResettableServer { documents: { get(documentName: string): | { connections: Map< { + readOnly: boolean webSocket: { close( code?: number, @@ -77,6 +78,10 @@ const resetConnection = { code: 4205, reason: "Reset Connection" } // only tells the provider the document is closed, and the provider leaves // it closed: it reopens a document after losing its socket, not after that // message. Direct connections have no socket and are unaffected. +// +// each connection turns read-only first. It stays on the document until +// its socket reports the close, and until then hocuspocus still applies +// its updates. export function resetConnections( server: ResettableServer, documentName: string, @@ -87,6 +92,7 @@ export function resetConnections( } for (const [connection] of document.connections) { + connection.readOnly = true connection.webSocket.close( resetConnection.code, resetConnection.reason, @@ -284,6 +290,7 @@ export function createDocumentHooks({ auth, core }: DocumentHookDeps) { context?: { session?: AuthSession | null system?: boolean + userId?: string } | null documentName: string }) { @@ -294,7 +301,8 @@ export function createDocumentHooks({ auth, core }: DocumentHookDeps) { isSystemContext(context), ) - const userId = context?.session?.user.id + const userId = + context?.session?.user.id ?? context?.userId if (!userId) { return Promise.resolve() } @@ -305,6 +313,9 @@ export function createDocumentHooks({ auth, core }: DocumentHookDeps) { documentMaintainers.set(documentName, set) } + // core takes the last maintainer as the author, so the + // latest editor moves to the end. + set.delete(userId) set.add(userId) return Promise.resolve() @@ -389,6 +400,7 @@ export function createDocumentHooks({ auth, core }: DocumentHookDeps) { async onStoreDocument(data: { documentName: string document: ConnectedDocument + lastContext?: { system?: boolean } | null }) { // both maps describe the window this persist closes, so // they are drained before anything that can fail: a @@ -403,10 +415,12 @@ export function createDocumentHooks({ auth, core }: DocumentHookDeps) { // nothing but core changed the document, so the persist // carries core's own permission and a protected branch // accepts it. One edit in the window is enough to make - // it an ordinary write. + // it an ordinary write. A window with no change at all, + // such as a core batch whose operations all failed, takes + // the permission of the connection that stores it. const system = documentSystemOnly.get(data.documentName) ?? - false + isSystemContext(data.lastContext) documentSystemOnly.delete(data.documentName) diff --git a/server/auth-realtime/src/routes.test.ts b/server/auth-realtime/src/routes.test.ts index a78c9bf5..f449f975 100644 --- a/server/auth-realtime/src/routes.test.ts +++ b/server/auth-realtime/src/routes.test.ts @@ -1578,6 +1578,14 @@ describe("createRoutes", () => { name: "a system field that is a number", input: { system: 1 }, }, + { + name: "a userId that is not a string", + input: { userId: 1 }, + }, + { + name: "an empty userId", + input: { userId: "" }, + }, ])( "opens the connection as an ordinary one given $name", async ({ input }, { expect }) => { @@ -1618,6 +1626,79 @@ describe("createRoutes", () => { }, ) + it("opens the connection for the user the batch names", async ({ + expect, + }) => { + const doc = new Y.Doc() + const { registry, openDirectConnection } = stubRegistry( + { + documents: new Map([["doc1-b1", doc]]), + transact: (fn) => { + fn(doc) + + return Promise.resolve() + }, + }, + ) + const { app } = build({ hocuspocus: registry }) + + await app.request(path, { + method: "POST", + body: JSON.stringify({ + userId: "user-1", + operations: [ + { + kind: "set_icon", + icon: "lucide:siren", + }, + ], + }), + headers: { "Content-Type": "application/json" }, + }) + + expect(openDirectConnection).toHaveBeenCalledWith( + "doc1-b1", + { userId: "user-1" }, + ) + }) + + it("opens a system batch as core's own even when it names a user", async ({ + expect, + }) => { + const doc = new Y.Doc() + const { registry, openDirectConnection } = stubRegistry( + { + documents: new Map([["doc1-b1", doc]]), + transact: (fn) => { + fn(doc) + + return Promise.resolve() + }, + }, + ) + const { app } = build({ hocuspocus: registry }) + + await app.request(path, { + method: "POST", + body: JSON.stringify({ + system: true, + userId: "user-1", + operations: [ + { + kind: "set_icon", + icon: "lucide:siren", + }, + ], + }), + headers: { "Content-Type": "application/json" }, + }) + + expect(openDirectConnection).toHaveBeenCalledWith( + "doc1-b1", + { system: true }, + ) + }) + it("reports a failing operation without failing the request", async ({ expect, }) => { diff --git a/server/auth-realtime/src/routes.ts b/server/auth-realtime/src/routes.ts index 74240445..97c742be 100644 --- a/server/auth-realtime/src/routes.ts +++ b/server/auth-realtime/src/routes.ts @@ -696,6 +696,18 @@ export function createRoutes({ .system === true : false + // the user the batch is made for, such as the one the + // assistant acts for. The persist names them as its author. + const userId = + body && typeof body === "object" + ? (body as { userId?: unknown }).userId + : undefined + const context = system + ? systemOrigin.context + : typeof userId === "string" && userId !== "" + ? { userId } + : {} + const operations = body && typeof body === "object" ? (body as { operations?: unknown }) @@ -727,9 +739,7 @@ export function createRoutes({ connection = await hocuspocus.openDirectConnection( documentName, - system - ? systemOrigin.context - : {}, + context, ) } catch (err) { Sentry.captureException(err) diff --git a/server/core/internal/assistant/edit/client.go b/server/core/internal/assistant/edit/client.go index 5d5ba4f2..4912f14e 100644 --- a/server/core/internal/assistant/edit/client.go +++ b/server/core/internal/assistant/edit/client.go @@ -63,7 +63,9 @@ func NewClient(httpClient *http.Client, baseURL string) *Client { // A system batch is one core originated rather than a person: its persist // is allowed onto a protected branch, where the writes an editor or the // assistant makes are refused. Only a caller acting for core itself may -// pass true — nothing reachable from a prompt may. +// pass true — nothing reachable from a prompt may. The userID is the +// person an ordinary batch is made for, and its persist credits them. It +// is empty for a system batch. // Any transport-level failure (bad status, malformed response, // canonical expansion error) is returned as an error and no // operations are considered applied; failures of individual @@ -73,6 +75,7 @@ func (c *Client) Apply( ctx context.Context, documentID, branchID xid.ID, ops []Operation, + userID string, system bool, ) (Result, error) { if len(ops) == 0 { @@ -92,8 +95,9 @@ func (c *Client) Apply( body, err := json.Marshal(struct { Operations []wireOp `json:"operations"` + UserID string `json:"userId,omitempty"` System bool `json:"system"` - }{Operations: wires, System: system}) + }{Operations: wires, UserID: userID, System: system}) if err != nil { return Result{}, fmt.Errorf("marshaling operations: %w", err) } diff --git a/server/core/internal/assistant/edit/client_test.go b/server/core/internal/assistant/edit/client_test.go index d5ed9fab..256da848 100644 --- a/server/core/internal/assistant/edit/client_test.go +++ b/server/core/internal/assistant/edit/client_test.go @@ -48,6 +48,7 @@ func Test_Client_Apply(t *testing.T) { cc := map[string]struct { Ops []Operation + UserID string System bool BaseURL string CloseEarly bool @@ -93,6 +94,17 @@ func Test_Client_Apply(t *testing.T) { ExpectedErrors: []OpError{}, ExpectedPath: path, }, + "An ordinary batch names its user": { + Ops: []Operation{ + Append(paragraph()), + }, + UserID: "user-1", + StatusCode: 200, + ResponseBody: `{"applied": 1, "errors": []}`, + ExpectedApplied: 1, + ExpectedErrors: []OpError{}, + ExpectedPath: path, + }, "Partial failure surfaces per-op errors": { Ops: []Operation{ Delete("missing"), @@ -190,7 +202,7 @@ func Test_Client_Apply(t *testing.T) { client := NewClient(srv.Client(), baseURL) - res, err := client.Apply(context.Background(), docID, branchID, c.Ops, c.System) + res, err := client.Apply(context.Background(), docID, branchID, c.Ops, c.UserID, c.System) testutil.AssertEqualError(t, c.Err, err) if c.NoRequest { @@ -212,6 +224,12 @@ func Test_Client_Apply(t *testing.T) { require.True(t, ok, "request body should carry operations array") assert.Len(t, ops, len(c.Ops)) assert.Equal(t, c.System, captured.body["system"]) + + if c.UserID == "" { + assert.NotContains(t, captured.body, "userId") + } else { + assert.Equal(t, c.UserID, captured.body["userId"]) + } } }) } diff --git a/server/core/internal/assistant/session_test.go b/server/core/internal/assistant/session_test.go index b64b76c2..1cf0a468 100644 --- a/server/core/internal/assistant/session_test.go +++ b/server/core/internal/assistant/session_test.go @@ -187,7 +187,7 @@ var ( // stubEditApplier accepts every edit it is handed. func stubEditApplier() *toolsMock.EditApplier { return &toolsMock.EditApplier{ - ApplyFunc: func(_ context.Context, _, _ xid.ID, _ []edit.Operation, _ bool) (edit.Result, error) { + ApplyFunc: func(_ context.Context, _, _ xid.ID, _ []edit.Operation, _ string, _ bool) (edit.Result, error) { return edit.Result{Applied: 1, Errors: []edit.OpError{}}, nil }, } diff --git a/server/core/internal/assistant/tools/0mock_db_test.go b/server/core/internal/assistant/tools/0mock_db_test.go index c82fbc5b..974a8c21 100644 --- a/server/core/internal/assistant/tools/0mock_db_test.go +++ b/server/core/internal/assistant/tools/0mock_db_test.go @@ -82,6 +82,9 @@ var _ DB = &DBMock{} // InsertTagFunc: func(ctx context.Context, t tag.Tag) error { // panic("mock out the InsertTag method") // }, +// RecordDocumentBranchHistoryEntryFunc: func(ctx context.Context, branchID xid.ID, organizationID string, by null.String, boundary bool) (xid.ID, error) { +// panic("mock out the RecordDocumentBranchHistoryEntry method") +// }, // UnassignBranchTagFunc: func(ctx context.Context, organizationID string, documentID xid.ID, branchID xid.ID, tagID xid.ID) error { // panic("mock out the UnassignBranchTag method") // }, @@ -161,6 +164,9 @@ type DBMock struct { // InsertTagFunc mocks the InsertTag method. InsertTagFunc func(ctx context.Context, t tag.Tag) error + // RecordDocumentBranchHistoryEntryFunc mocks the RecordDocumentBranchHistoryEntry method. + RecordDocumentBranchHistoryEntryFunc func(ctx context.Context, branchID xid.ID, organizationID string, by null.String, boundary bool) (xid.ID, error) + // UnassignBranchTagFunc mocks the UnassignBranchTag method. UnassignBranchTagFunc func(ctx context.Context, organizationID string, documentID xid.ID, branchID xid.ID, tagID xid.ID) error @@ -347,6 +353,19 @@ type DBMock struct { // T is the t argument value. T tag.Tag } + // RecordDocumentBranchHistoryEntry holds details about calls to the RecordDocumentBranchHistoryEntry method. + RecordDocumentBranchHistoryEntry []struct { + // Ctx is the ctx argument value. + Ctx context.Context + // BranchID is the branchID argument value. + BranchID xid.ID + // OrganizationID is the organizationID argument value. + OrganizationID string + // By is the by argument value. + By null.String + // Boundary is the boundary argument value. + Boundary bool + } // UnassignBranchTag holds details about calls to the UnassignBranchTag method. UnassignBranchTag []struct { // Ctx is the ctx argument value. @@ -418,6 +437,7 @@ type DBMock struct { lockFetchTagTree sync.RWMutex lockInsertDocumentHook sync.RWMutex lockInsertTag sync.RWMutex + lockRecordDocumentBranchHistoryEntry sync.RWMutex lockUnassignBranchTag sync.RWMutex lockUpdateDocumentHook sync.RWMutex lockUpdateDocumentParentID sync.RWMutex @@ -1250,6 +1270,58 @@ func (mock *DBMock) InsertTagCalls() []struct { return calls } +// RecordDocumentBranchHistoryEntry calls RecordDocumentBranchHistoryEntryFunc. +func (mock *DBMock) RecordDocumentBranchHistoryEntry(ctx context.Context, branchID xid.ID, organizationID string, by null.String, boundary bool) (xid.ID, error) { + callInfo := struct { + Ctx context.Context + BranchID xid.ID + OrganizationID string + By null.String + Boundary bool + }{ + Ctx: ctx, + BranchID: branchID, + OrganizationID: organizationID, + By: by, + Boundary: boundary, + } + mock.lockRecordDocumentBranchHistoryEntry.Lock() + mock.calls.RecordDocumentBranchHistoryEntry = append(mock.calls.RecordDocumentBranchHistoryEntry, callInfo) + mock.lockRecordDocumentBranchHistoryEntry.Unlock() + if mock.RecordDocumentBranchHistoryEntryFunc == nil { + var ( + iDOut xid.ID + errOut error + ) + return iDOut, errOut + } + return mock.RecordDocumentBranchHistoryEntryFunc(ctx, branchID, organizationID, by, boundary) +} + +// RecordDocumentBranchHistoryEntryCalls gets all the calls that were made to RecordDocumentBranchHistoryEntry. +// Check the length with: +// +// len(mockedDB.RecordDocumentBranchHistoryEntryCalls()) +func (mock *DBMock) RecordDocumentBranchHistoryEntryCalls() []struct { + Ctx context.Context + BranchID xid.ID + OrganizationID string + By null.String + Boundary bool +} { + var calls []struct { + Ctx context.Context + BranchID xid.ID + OrganizationID string + By null.String + Boundary bool + } + mock.lockRecordDocumentBranchHistoryEntry.RLock() + calls = mock.calls.RecordDocumentBranchHistoryEntry + mock.lockRecordDocumentBranchHistoryEntry.RUnlock() + return calls +} + // UnassignBranchTag calls UnassignBranchTagFunc. func (mock *DBMock) UnassignBranchTag(ctx context.Context, organizationID string, documentID xid.ID, branchID xid.ID, tagID xid.ID) error { callInfo := struct { diff --git a/server/core/internal/assistant/tools/0mock_edit_applier_test.go b/server/core/internal/assistant/tools/0mock_edit_applier_test.go index fc5c1f0b..eba4577d 100644 --- a/server/core/internal/assistant/tools/0mock_edit_applier_test.go +++ b/server/core/internal/assistant/tools/0mock_edit_applier_test.go @@ -21,7 +21,7 @@ var _ EditApplier = &EditApplierMock{} // // // make and configure a mocked EditApplier // mockedEditApplier := &EditApplierMock{ -// ApplyFunc: func(ctx context.Context, documentID xid.ID, branchID xid.ID, ops []edit.Operation, system bool) (edit.Result, error) { +// ApplyFunc: func(ctx context.Context, documentID xid.ID, branchID xid.ID, ops []edit.Operation, userID string, system bool) (edit.Result, error) { // panic("mock out the Apply method") // }, // } @@ -32,7 +32,7 @@ var _ EditApplier = &EditApplierMock{} // } type EditApplierMock struct { // ApplyFunc mocks the Apply method. - ApplyFunc func(ctx context.Context, documentID xid.ID, branchID xid.ID, ops []edit.Operation, system bool) (edit.Result, error) + ApplyFunc func(ctx context.Context, documentID xid.ID, branchID xid.ID, ops []edit.Operation, userID string, system bool) (edit.Result, error) // calls tracks calls to the methods. calls struct { @@ -46,6 +46,8 @@ type EditApplierMock struct { BranchID xid.ID // Ops is the ops argument value. Ops []edit.Operation + // UserID is the userID argument value. + UserID string // System is the system argument value. System bool } @@ -54,18 +56,20 @@ type EditApplierMock struct { } // Apply calls ApplyFunc. -func (mock *EditApplierMock) Apply(ctx context.Context, documentID xid.ID, branchID xid.ID, ops []edit.Operation, system bool) (edit.Result, error) { +func (mock *EditApplierMock) Apply(ctx context.Context, documentID xid.ID, branchID xid.ID, ops []edit.Operation, userID string, system bool) (edit.Result, error) { callInfo := struct { Ctx context.Context DocumentID xid.ID BranchID xid.ID Ops []edit.Operation + UserID string System bool }{ Ctx: ctx, DocumentID: documentID, BranchID: branchID, Ops: ops, + UserID: userID, System: system, } mock.lockApply.Lock() @@ -78,7 +82,7 @@ func (mock *EditApplierMock) Apply(ctx context.Context, documentID xid.ID, branc ) return resultOut, errOut } - return mock.ApplyFunc(ctx, documentID, branchID, ops, system) + return mock.ApplyFunc(ctx, documentID, branchID, ops, userID, system) } // ApplyCalls gets all the calls that were made to Apply. @@ -90,6 +94,7 @@ func (mock *EditApplierMock) ApplyCalls() []struct { DocumentID xid.ID BranchID xid.ID Ops []edit.Operation + UserID string System bool } { var calls []struct { @@ -97,6 +102,7 @@ func (mock *EditApplierMock) ApplyCalls() []struct { DocumentID xid.ID BranchID xid.ID Ops []edit.Operation + UserID string System bool } mock.lockApply.RLock() diff --git a/server/core/internal/assistant/tools/_mock/db.go b/server/core/internal/assistant/tools/_mock/db.go index 1e4bf8c3..0b774a95 100644 --- a/server/core/internal/assistant/tools/_mock/db.go +++ b/server/core/internal/assistant/tools/_mock/db.go @@ -83,6 +83,9 @@ var _ tools.DB = &DB{} // InsertTagFunc: func(ctx context.Context, t tag.Tag) error { // panic("mock out the InsertTag method") // }, +// RecordDocumentBranchHistoryEntryFunc: func(ctx context.Context, branchID xid.ID, organizationID string, by null.String, boundary bool) (xid.ID, error) { +// panic("mock out the RecordDocumentBranchHistoryEntry method") +// }, // UnassignBranchTagFunc: func(ctx context.Context, organizationID string, documentID xid.ID, branchID xid.ID, tagID xid.ID) error { // panic("mock out the UnassignBranchTag method") // }, @@ -162,6 +165,9 @@ type DB struct { // InsertTagFunc mocks the InsertTag method. InsertTagFunc func(ctx context.Context, t tag.Tag) error + // RecordDocumentBranchHistoryEntryFunc mocks the RecordDocumentBranchHistoryEntry method. + RecordDocumentBranchHistoryEntryFunc func(ctx context.Context, branchID xid.ID, organizationID string, by null.String, boundary bool) (xid.ID, error) + // UnassignBranchTagFunc mocks the UnassignBranchTag method. UnassignBranchTagFunc func(ctx context.Context, organizationID string, documentID xid.ID, branchID xid.ID, tagID xid.ID) error @@ -348,6 +354,19 @@ type DB struct { // T is the t argument value. T tag.Tag } + // RecordDocumentBranchHistoryEntry holds details about calls to the RecordDocumentBranchHistoryEntry method. + RecordDocumentBranchHistoryEntry []struct { + // Ctx is the ctx argument value. + Ctx context.Context + // BranchID is the branchID argument value. + BranchID xid.ID + // OrganizationID is the organizationID argument value. + OrganizationID string + // By is the by argument value. + By null.String + // Boundary is the boundary argument value. + Boundary bool + } // UnassignBranchTag holds details about calls to the UnassignBranchTag method. UnassignBranchTag []struct { // Ctx is the ctx argument value. @@ -419,6 +438,7 @@ type DB struct { lockFetchTagTree sync.RWMutex lockInsertDocumentHook sync.RWMutex lockInsertTag sync.RWMutex + lockRecordDocumentBranchHistoryEntry sync.RWMutex lockUnassignBranchTag sync.RWMutex lockUpdateDocumentHook sync.RWMutex lockUpdateDocumentParentID sync.RWMutex @@ -1251,6 +1271,58 @@ func (mock *DB) InsertTagCalls() []struct { return calls } +// RecordDocumentBranchHistoryEntry calls RecordDocumentBranchHistoryEntryFunc. +func (mock *DB) RecordDocumentBranchHistoryEntry(ctx context.Context, branchID xid.ID, organizationID string, by null.String, boundary bool) (xid.ID, error) { + callInfo := struct { + Ctx context.Context + BranchID xid.ID + OrganizationID string + By null.String + Boundary bool + }{ + Ctx: ctx, + BranchID: branchID, + OrganizationID: organizationID, + By: by, + Boundary: boundary, + } + mock.lockRecordDocumentBranchHistoryEntry.Lock() + mock.calls.RecordDocumentBranchHistoryEntry = append(mock.calls.RecordDocumentBranchHistoryEntry, callInfo) + mock.lockRecordDocumentBranchHistoryEntry.Unlock() + if mock.RecordDocumentBranchHistoryEntryFunc == nil { + var ( + iDOut xid.ID + errOut error + ) + return iDOut, errOut + } + return mock.RecordDocumentBranchHistoryEntryFunc(ctx, branchID, organizationID, by, boundary) +} + +// RecordDocumentBranchHistoryEntryCalls gets all the calls that were made to RecordDocumentBranchHistoryEntry. +// Check the length with: +// +// len(mockedDB.RecordDocumentBranchHistoryEntryCalls()) +func (mock *DB) RecordDocumentBranchHistoryEntryCalls() []struct { + Ctx context.Context + BranchID xid.ID + OrganizationID string + By null.String + Boundary bool +} { + var calls []struct { + Ctx context.Context + BranchID xid.ID + OrganizationID string + By null.String + Boundary bool + } + mock.lockRecordDocumentBranchHistoryEntry.RLock() + calls = mock.calls.RecordDocumentBranchHistoryEntry + mock.lockRecordDocumentBranchHistoryEntry.RUnlock() + return calls +} + // UnassignBranchTag calls UnassignBranchTagFunc. func (mock *DB) UnassignBranchTag(ctx context.Context, organizationID string, documentID xid.ID, branchID xid.ID, tagID xid.ID) error { callInfo := struct { diff --git a/server/core/internal/assistant/tools/_mock/edit_applier.go b/server/core/internal/assistant/tools/_mock/edit_applier.go index a850462a..339c397a 100644 --- a/server/core/internal/assistant/tools/_mock/edit_applier.go +++ b/server/core/internal/assistant/tools/_mock/edit_applier.go @@ -22,7 +22,7 @@ var _ tools.EditApplier = &EditApplier{} // // // make and configure a mocked tools.EditApplier // mockedEditApplier := &EditApplier{ -// ApplyFunc: func(ctx context.Context, documentID xid.ID, branchID xid.ID, ops []edit.Operation, system bool) (edit.Result, error) { +// ApplyFunc: func(ctx context.Context, documentID xid.ID, branchID xid.ID, ops []edit.Operation, userID string, system bool) (edit.Result, error) { // panic("mock out the Apply method") // }, // } @@ -33,7 +33,7 @@ var _ tools.EditApplier = &EditApplier{} // } type EditApplier struct { // ApplyFunc mocks the Apply method. - ApplyFunc func(ctx context.Context, documentID xid.ID, branchID xid.ID, ops []edit.Operation, system bool) (edit.Result, error) + ApplyFunc func(ctx context.Context, documentID xid.ID, branchID xid.ID, ops []edit.Operation, userID string, system bool) (edit.Result, error) // calls tracks calls to the methods. calls struct { @@ -47,6 +47,8 @@ type EditApplier struct { BranchID xid.ID // Ops is the ops argument value. Ops []edit.Operation + // UserID is the userID argument value. + UserID string // System is the system argument value. System bool } @@ -55,18 +57,20 @@ type EditApplier struct { } // Apply calls ApplyFunc. -func (mock *EditApplier) Apply(ctx context.Context, documentID xid.ID, branchID xid.ID, ops []edit.Operation, system bool) (edit.Result, error) { +func (mock *EditApplier) Apply(ctx context.Context, documentID xid.ID, branchID xid.ID, ops []edit.Operation, userID string, system bool) (edit.Result, error) { callInfo := struct { Ctx context.Context DocumentID xid.ID BranchID xid.ID Ops []edit.Operation + UserID string System bool }{ Ctx: ctx, DocumentID: documentID, BranchID: branchID, Ops: ops, + UserID: userID, System: system, } mock.lockApply.Lock() @@ -79,7 +83,7 @@ func (mock *EditApplier) Apply(ctx context.Context, documentID xid.ID, branchID ) return resultOut, errOut } - return mock.ApplyFunc(ctx, documentID, branchID, ops, system) + return mock.ApplyFunc(ctx, documentID, branchID, ops, userID, system) } // ApplyCalls gets all the calls that were made to Apply. @@ -91,6 +95,7 @@ func (mock *EditApplier) ApplyCalls() []struct { DocumentID xid.ID BranchID xid.ID Ops []edit.Operation + UserID string System bool } { var calls []struct { @@ -98,6 +103,7 @@ func (mock *EditApplier) ApplyCalls() []struct { DocumentID xid.ID BranchID xid.ID Ops []edit.Operation + UserID string System bool } mock.lockApply.RLock() diff --git a/server/core/internal/assistant/tools/document_test.go b/server/core/internal/assistant/tools/document_test.go index 443e8f71..fa47edc7 100644 --- a/server/core/internal/assistant/tools/document_test.go +++ b/server/core/internal/assistant/tools/document_test.go @@ -871,7 +871,7 @@ func Test_updateDocument_Execute(t *testing.T) { "Error returned by the edit": { DB: stubDocumentDB(), Applier: &EditApplierMock{ - ApplyFunc: func(context.Context, xid.ID, xid.ID, []edit.Operation, bool) (edit.Result, error) { + ApplyFunc: func(context.Context, xid.ID, xid.ID, []edit.Operation, string, bool) (edit.Result, error) { return edit.Result{}, assert.AnError }, }, diff --git a/server/core/internal/assistant/tools/input.go b/server/core/internal/assistant/tools/input.go index 9b5ac619..67555b53 100644 --- a/server/core/internal/assistant/tools/input.go +++ b/server/core/internal/assistant/tools/input.go @@ -661,7 +661,14 @@ func (i *input) ApplyEdit(documentID, branchID xid.ID, ops []edit.Operation) err return err } - res, err := i.applier.Apply(i.ctx, ref.DocumentID, ref.BranchID, ops, false) + res, err := i.applier.Apply( + i.ctx, + ref.DocumentID, + ref.BranchID, + ops, + i.userID, + false, + ) if err != nil { i.log.Error( "edit apply failed", @@ -1093,6 +1100,7 @@ func (i *input) CreateHook(documentID, branchID xid.ID, blockUID string, tp hook return nil, fmt.Errorf("insert: %w", err) } + i.recordHookHistory(hk) i.hookChanged(hk) return hk, nil @@ -1109,6 +1117,7 @@ func (i *input) UpdateHook(hk *hook.Hook, settings processor.Settings) error { return fmt.Errorf("update: %w", err) } + i.recordHookHistory(hk) i.hookChanged(hk) return nil @@ -1141,11 +1150,36 @@ func (i *input) DeleteHook(hk *hook.Hook) error { return fmt.Errorf("delete: %w", err) } + i.recordHookHistory(hk) i.hookChanged(hk) return nil } +// recordHookHistory records the hook's branch in its history, credited to +// the user the assistant acts for. The hook write is already done, so a +// failure is logged rather than returned. +func (i *input) recordHookHistory(hk *hook.Hook) { + if !hk.BranchID.Valid { + return + } + + _, err := i.db.RecordDocumentBranchHistoryEntry( + i.ctx, + hk.BranchID.V, + i.orgID, + null.StringFrom(i.userID), + false, + ) + if err != nil { + i.log.Error( + "cannot record the hook change in the branch history", + slog.String("hook_id", hk.ID.String()), + slog.String("error", err.Error()), + ) + } +} + // hookChanged records the branch a hook write changed and announces it // to the document's subscribers. A hook whose branch was deleted, and // which the sweep has not yet removed, has no branch to link to. @@ -1307,6 +1341,17 @@ type HookDB interface { // DeleteDocumentHook should remove a hook the caller has already // fetched org-scoped. Used by delete_hook. DeleteDocumentHook(ctx context.Context, id xid.ID) error + + // RecordDocumentBranchHistoryEntry should record the branch as it + // stands, with its live hooks, and return the id of the entry. Used + // by every hook write that changes what the branch carries. + RecordDocumentBranchHistoryEntry( + ctx context.Context, + branchID xid.ID, + organizationID string, + by null.String, + boundary bool, + ) (xid.ID, error) } // Tx is the transactional half of DB, so a tool whose write spans @@ -1407,5 +1452,5 @@ type EditApplier interface { // for the (documentID, branchID) document and return the per-op // outcome. A tool's writes are a person's, never core's own, so // this package always asks for an ordinary one. - Apply(ctx context.Context, documentID, branchID xid.ID, ops []edit.Operation, system bool) (edit.Result, error) + Apply(ctx context.Context, documentID, branchID xid.ID, ops []edit.Operation, userID string, system bool) (edit.Result, error) } diff --git a/server/core/internal/assistant/tools/input_test.go b/server/core/internal/assistant/tools/input_test.go index 550c9ca9..e5a6e916 100644 --- a/server/core/internal/assistant/tools/input_test.go +++ b/server/core/internal/assistant/tools/input_test.go @@ -377,7 +377,7 @@ func failingDocumentDB() *DBMock { // stubApplier accepts every edit it is handed. func stubApplier() *EditApplierMock { return &EditApplierMock{ - ApplyFunc: func(_ context.Context, _, _ xid.ID, _ []edit.Operation, _ bool) (edit.Result, error) { + ApplyFunc: func(_ context.Context, _, _ xid.ID, _ []edit.Operation, _ string, _ bool) (edit.Result, error) { return edit.Result{Applied: 1, Errors: []edit.OpError{}}, nil }, } @@ -851,7 +851,7 @@ func Test_input_ApplyEdit(t *testing.T) { "Error returned by applier.Apply": { DB: stubDocumentDB(), Applier: &EditApplierMock{ - ApplyFunc: func(context.Context, xid.ID, xid.ID, []edit.Operation, bool) (edit.Result, error) { + ApplyFunc: func(context.Context, xid.ID, xid.ID, []edit.Operation, string, bool) (edit.Result, error) { return edit.Result{}, assert.AnError }, }, @@ -919,7 +919,7 @@ func Test_input_ApplyEdit(t *testing.T) { "Nothing applied is a failure, not a result": { DB: stubDocumentDB(), Applier: &EditApplierMock{ - ApplyFunc: func(context.Context, xid.ID, xid.ID, []edit.Operation, bool) (edit.Result, error) { + ApplyFunc: func(context.Context, xid.ID, xid.ID, []edit.Operation, string, bool) (edit.Result, error) { return edit.Result{ Errors: []edit.OpError{{Index: 0, Message: "block_uid not found: a"}}, }, nil @@ -931,7 +931,7 @@ func Test_input_ApplyEdit(t *testing.T) { "Partial failure still reports the outcome": { DB: stubDocumentDB(), Applier: &EditApplierMock{ - ApplyFunc: func(context.Context, xid.ID, xid.ID, []edit.Operation, bool) (edit.Result, error) { + ApplyFunc: func(context.Context, xid.ID, xid.ID, []edit.Operation, string, bool) (edit.Result, error) { return edit.Result{ Applied: 1, Errors: []edit.OpError{{Index: 1, Message: "uid not found"}}, @@ -978,9 +978,11 @@ func Test_input_ApplyEdit(t *testing.T) { // a tool's write is a person's, never core's own: an // edit asking otherwise would land on a protected // branch, which is the one place a tool may not reach. + // The persist credits the user the assistant acts for. if c.Applier != nil { for _, call := range c.Applier.ApplyCalls() { assert.False(t, call.System) + assert.Equal(t, inp.UserID(), call.UserID) } } }) @@ -2427,6 +2429,7 @@ func Test_input_CreateHook(t *testing.T) { Settings processor.Settings Inserts int Notify int + Records int Touched []Touched Err error }{ @@ -2481,6 +2484,7 @@ func Test_input_CreateHook(t *testing.T) { Settings: scheduled, Inserts: 1, Notify: 1, + Records: 1, Touched: []Touched{{DocumentID: _testDocID, BranchID: _stubBranchID}}, }, "Created on a block": { @@ -2491,6 +2495,7 @@ func Test_input_CreateHook(t *testing.T) { Settings: scheduled, Inserts: 1, Notify: 1, + Records: 1, Touched: []Touched{{DocumentID: _testDocID, BranchID: _stubBranchID}}, }, } @@ -2510,6 +2515,17 @@ func Test_input_CreateHook(t *testing.T) { assert.Len(t, hooks.NotifyHooksChangeCalls(), c.Notify) assert.Equal(t, c.Touched, inp.touched) + // a hook write that landed is recorded in the branch history, + // credited to the user the assistant acts for. + assert.Len(t, c.DB.RecordDocumentBranchHistoryEntryCalls(), c.Records) + + for _, call := range c.DB.RecordDocumentBranchHistoryEntryCalls() { + assert.Equal(t, _stubBranchID, call.BranchID) + assert.Equal(t, "org", call.OrganizationID) + assert.Equal(t, null.StringFrom(inp.UserID()), call.By) + assert.False(t, call.Boundary) + } + if err != nil { assert.Nil(t, hk) return @@ -2541,6 +2557,7 @@ func Test_input_UpdateHook(t *testing.T) { Settings processor.Settings Updates int Notify int + Records int Touched []Touched Err error }{ @@ -2560,6 +2577,7 @@ func Test_input_UpdateHook(t *testing.T) { Settings: processor.Settings(`{"scale":"linear","duration":"custom","schedule":"2031-01-01T00:00:00Z"}`), Updates: 1, Notify: 1, + Records: 1, Touched: []Touched{{DocumentID: _testDocID, BranchID: _stubBranchID}}, }, } @@ -2580,6 +2598,17 @@ func Test_input_UpdateHook(t *testing.T) { assert.Len(t, hooks.NotifyHooksChangeCalls(), c.Notify) assert.Equal(t, c.Touched, inp.touched) + // a hook write that landed is recorded in the branch history, + // credited to the user the assistant acts for. + assert.Len(t, c.DB.RecordDocumentBranchHistoryEntryCalls(), c.Records) + + for _, call := range c.DB.RecordDocumentBranchHistoryEntryCalls() { + assert.Equal(t, _stubBranchID, call.BranchID) + assert.Equal(t, "org", call.OrganizationID) + assert.Equal(t, null.StringFrom(inp.UserID()), call.By) + assert.False(t, call.Boundary) + } + if err != nil { return } @@ -2648,6 +2677,10 @@ func Test_input_ResetHook(t *testing.T) { assert.Len(t, hooks.NotifyHooksChangeCalls(), c.Notify) assert.Equal(t, c.Touched, inp.touched) + // a reset changes only watcher state, which history does not + // hold. + assert.Empty(t, c.DB.RecordDocumentBranchHistoryEntryCalls()) + if err != nil { return } @@ -2680,6 +2713,7 @@ func Test_input_DeleteHook(t *testing.T) { Hook *hook.Hook Deletes int Notify int + Records int Touched []Touched Err error }{ @@ -2699,6 +2733,7 @@ func Test_input_DeleteHook(t *testing.T) { Hook: stubHook(), Deletes: 1, Notify: 1, + Records: 1, Touched: []Touched{{DocumentID: _testDocID, BranchID: _stubBranchID}}, }, } @@ -2718,6 +2753,17 @@ func Test_input_DeleteHook(t *testing.T) { assert.Len(t, hooks.NotifyHooksChangeCalls(), c.Notify) assert.Equal(t, c.Touched, inp.touched) + // a hook write that landed is recorded in the branch history, + // credited to the user the assistant acts for. + assert.Len(t, c.DB.RecordDocumentBranchHistoryEntryCalls(), c.Records) + + for _, call := range c.DB.RecordDocumentBranchHistoryEntryCalls() { + assert.Equal(t, _stubBranchID, call.BranchID) + assert.Equal(t, "org", call.OrganizationID) + assert.Equal(t, null.StringFrom(inp.UserID()), call.By) + assert.False(t, call.Boundary) + } + if err != nil { return } @@ -2727,6 +2773,60 @@ func Test_input_DeleteHook(t *testing.T) { } } +func Test_input_recordHookHistory(t *testing.T) { + t.Parallel() + + branchless := stubHook() + branchless.BranchID = null.Value[xid.ID]{} + + cc := map[string]struct { + Hook *hook.Hook + Err error + Records int + Log string + }{ + "Hook whose branch is gone": {Hook: branchless}, + // the hook write is already done, so the failure is logged + // rather than returned. + "Error returned by db.RecordDocumentBranchHistoryEntry": { + Hook: stubHook(), + Err: assert.AnError, + Records: 1, + Log: "cannot record the hook change in the branch history", + }, + "Successful record": {Hook: stubHook(), Records: 1}, + } + + for cn, c := range cc { + t.Run(cn, func(t *testing.T) { + t.Parallel() + + db := &DBMock{ + RecordDocumentBranchHistoryEntryFunc: func(context.Context, xid.ID, string, null.String, bool) (xid.ID, error) { + return xid.New(), c.Err + }, + } + + var buf bytes.Buffer + + d := testDeps(db, nil, nil) + d.log = slog.New(slog.NewTextHandler(&buf, nil)) + + inp := testInput(d, NameDeleteHook, `{}`) + inp.recordHookHistory(c.Hook) + + assert.Len(t, db.RecordDocumentBranchHistoryEntryCalls(), c.Records) + + if c.Log == "" { + assert.NotContains(t, buf.String(), "level=ERROR") + return + } + + assert.Contains(t, buf.String(), c.Log) + }) + } +} + func Test_input_hookChanged(t *testing.T) { t.Parallel() diff --git a/server/core/internal/datasource/simulation/0mock_applier_test.go b/server/core/internal/datasource/simulation/0mock_applier_test.go index bd89b9f4..973113de 100644 --- a/server/core/internal/datasource/simulation/0mock_applier_test.go +++ b/server/core/internal/datasource/simulation/0mock_applier_test.go @@ -21,7 +21,7 @@ var _ Applier = &ApplierMock{} // // // make and configure a mocked Applier // mockedApplier := &ApplierMock{ -// ApplyFunc: func(ctx context.Context, documentID xid.ID, branchID xid.ID, ops []edit.Operation, system bool) (edit.Result, error) { +// ApplyFunc: func(ctx context.Context, documentID xid.ID, branchID xid.ID, ops []edit.Operation, userID string, system bool) (edit.Result, error) { // panic("mock out the Apply method") // }, // } @@ -32,7 +32,7 @@ var _ Applier = &ApplierMock{} // } type ApplierMock struct { // ApplyFunc mocks the Apply method. - ApplyFunc func(ctx context.Context, documentID xid.ID, branchID xid.ID, ops []edit.Operation, system bool) (edit.Result, error) + ApplyFunc func(ctx context.Context, documentID xid.ID, branchID xid.ID, ops []edit.Operation, userID string, system bool) (edit.Result, error) // calls tracks calls to the methods. calls struct { @@ -46,6 +46,8 @@ type ApplierMock struct { BranchID xid.ID // Ops is the ops argument value. Ops []edit.Operation + // UserID is the userID argument value. + UserID string // System is the system argument value. System bool } @@ -54,18 +56,20 @@ type ApplierMock struct { } // Apply calls ApplyFunc. -func (mock *ApplierMock) Apply(ctx context.Context, documentID xid.ID, branchID xid.ID, ops []edit.Operation, system bool) (edit.Result, error) { +func (mock *ApplierMock) Apply(ctx context.Context, documentID xid.ID, branchID xid.ID, ops []edit.Operation, userID string, system bool) (edit.Result, error) { callInfo := struct { Ctx context.Context DocumentID xid.ID BranchID xid.ID Ops []edit.Operation + UserID string System bool }{ Ctx: ctx, DocumentID: documentID, BranchID: branchID, Ops: ops, + UserID: userID, System: system, } mock.lockApply.Lock() @@ -78,7 +82,7 @@ func (mock *ApplierMock) Apply(ctx context.Context, documentID xid.ID, branchID ) return resultOut, errOut } - return mock.ApplyFunc(ctx, documentID, branchID, ops, system) + return mock.ApplyFunc(ctx, documentID, branchID, ops, userID, system) } // ApplyCalls gets all the calls that were made to Apply. @@ -90,6 +94,7 @@ func (mock *ApplierMock) ApplyCalls() []struct { DocumentID xid.ID BranchID xid.ID Ops []edit.Operation + UserID string System bool } { var calls []struct { @@ -97,6 +102,7 @@ func (mock *ApplierMock) ApplyCalls() []struct { DocumentID xid.ID BranchID xid.ID Ops []edit.Operation + UserID string System bool } mock.lockApply.RLock() diff --git a/server/core/internal/datasource/simulation/simulation.go b/server/core/internal/datasource/simulation/simulation.go index 3f22cfe0..50fadf6a 100644 --- a/server/core/internal/datasource/simulation/simulation.go +++ b/server/core/internal/datasource/simulation/simulation.go @@ -259,11 +259,18 @@ func (c *Checker) clear( documentID, branchID xid.ID, blockUID string, ) error { - res, err := c.applier.Apply(ctx, documentID, branchID, []edit.Operation{ - edit.UpdateAttrs(blockUID, map[string]any{ - document.AttrSimulationActive: false, - }), - }, true) + res, err := c.applier.Apply( + ctx, + documentID, + branchID, + []edit.Operation{ + edit.UpdateAttrs(blockUID, map[string]any{ + document.AttrSimulationActive: false, + }), + }, + "", + true, + ) if err != nil { return err } @@ -382,5 +389,5 @@ type Applier interface { // the (documentID, branchID) document and return the per-op // outcome. The clear is core's own write, so this package always // asks for a system one. - Apply(ctx context.Context, documentID, branchID xid.ID, ops []edit.Operation, system bool) (edit.Result, error) + Apply(ctx context.Context, documentID, branchID xid.ID, ops []edit.Operation, userID string, system bool) (edit.Result, error) } diff --git a/server/core/internal/datasource/simulation/simulation_test.go b/server/core/internal/datasource/simulation/simulation_test.go index f051c2a0..4eb280a3 100644 --- a/server/core/internal/datasource/simulation/simulation_test.go +++ b/server/core/internal/datasource/simulation/simulation_test.go @@ -461,7 +461,7 @@ func Test_Checker_Check(t *testing.T) { } } - d.applier.ApplyFunc = func(context.Context, xid.ID, xid.ID, []edit.Operation, bool) (edit.Result, error) { + d.applier.ApplyFunc = func(context.Context, xid.ID, xid.ID, []edit.Operation, string, bool) (edit.Result, error) { return c.ApplyResult, c.ApplyErr } diff --git a/server/core/internal/db/document_branch.go b/server/core/internal/db/document_branch.go index 370a0e4c..9e1ece1d 100644 --- a/server/core/internal/db/document_branch.go +++ b/server/core/internal/db/document_branch.go @@ -7,10 +7,12 @@ import ( "time" sq "github.com/Masterminds/squirrel" + "github.com/guregu/null/v5" "github.com/jmoiron/sqlx" "github.com/oxynote/oxynote/server/core/internal/document" "github.com/oxynote/oxynote/server/core/internal/document/history" "github.com/oxynote/oxynote/server/core/pkg/sqlutil" + "github.com/oxynote/oxynote/server/core/pkg/timeutil" "github.com/rs/xid" ) @@ -259,65 +261,138 @@ func (a *agent) selectBranchSummary(b sq.SelectBuilder) sq.SelectBuilder { ).From("document_branches") } -// InsertDocumentBranchHistoryEntry records a snapshot of a branch. Ordinary -// entries aggregate: when the branch's newest entry is an ordinary one from -// the same time bucket, that entry is updated in place instead of a row -// being added. A boundary entry (merge, fork) is always its own row and -// closes the bucket, so the state right before it survives and the next -// edit starts a new entry. The count trim runs afterwards; age trimming -// lives in the file manager, since it has to reach branches that stopped -// inserting altogether. -func (a *agent) InsertDocumentBranchHistoryEntry(ctx context.Context, entry history.Entry) error { - return sqlutil.WrapTx(ctx, a.sql, func(tx *sqlx.Tx) error { - newest, err := a.fetchNewestDocumentBranchHistoryEntry(ctx, tx, entry.BranchID) - if err != nil { +// RecordDocumentBranchHistoryEntry records the branch as it stands, with +// its live hooks, and returns the id of the entry it wrote. It locks the +// branch row before reading it, so two writers cannot race on the newest +// entry. Called inside the transaction that wrote the branch, it records +// what that transaction wrote. +// +// Ordinary entries aggregate: an edit in the same 30-minute bucket as the +// branch's newest ordinary entry updates that entry in place. A boundary +// entry (create, duplicate, fork, merge) is always its own row and closes +// the bucket. The count trim runs afterwards; age trimming lives in the +// file manager, since it has to reach branches that stopped writing. +func (a *agent) RecordDocumentBranchHistoryEntry( + ctx context.Context, + branchID xid.ID, + organizationID string, + by null.String, + boundary bool, +) (xid.ID, error) { + hooks, err := a.FetchDocumentHooksByBranchID(ctx, branchID, organizationID) + if err != nil { + return xid.ID{}, err + } + + var id xid.ID + + if err := sqlutil.WrapTx(ctx, a.sql, func(tx *sqlx.Tx) error { + q, args := a.selectDocumentBranch(a.builder.Select()). + Where(sq.Eq{ + "db.id": branchID, + "documents.fk_organization_id": organizationID, + }). + Suffix("FOR UPDATE OF db"). + MustSql() + + var doc document.Document + + if err := sqlx.GetContext(ctx, tx, &doc, q, args...); err != nil { return err } - aggregates := newest != nil && - !newest.Boundary && - !entry.Boundary && - newest.CreatedAt.Truncate(_historyAggregationDuration).Equal(entry.CreatedAt.Truncate(_historyAggregationDuration)) + entry := history.NewEntry( + doc, + timeutil.Now(), + by, + history.NewHooks(hooks), + boundary, + ) - if aggregates { - q, args := a.builder.Update("document_branch_history_entries"). - SetMap(map[string]any{ - "document_name": entry.DocumentName, - "icon": entry.Icon, - "content": entry.Content, - "hooks": entry.Hooks, - "fk_last_updated_by": entry.LastUpdatedBy, - "created_at": entry.CreatedAt, - }). - Where(sq.Eq{"id": newest.ID}). - MustSql() + var err error - _, err := tx.ExecContext(ctx, q, args...) + id, err = a.insertDocumentBranchHistoryEntry(ctx, tx, entry) - return err - } + return err + }); err != nil { + return xid.ID{}, err + } - q, args := a.builder.Insert("document_branch_history_entries"). + return id, nil +} + +// insertDocumentBranchHistoryEntry writes the entry, or folds it into the +// branch's newest one, and returns the id of the row it wrote. The caller +// holds the branch row lock. +func (a *agent) insertDocumentBranchHistoryEntry(ctx context.Context, tx *sqlx.Tx, entry history.Entry) (xid.ID, error) { + newest, err := a.fetchNewestDocumentBranchHistoryEntry(ctx, tx, entry.BranchID) + if err != nil { + return xid.ID{}, err + } + + aggregates := newest != nil && + !newest.Boundary && + !entry.Boundary && + newest.CreatedAt.Truncate(_historyAggregationDuration).Equal(entry.CreatedAt.Truncate(_historyAggregationDuration)) + + if aggregates { + q, args := a.builder.Update("document_branch_history_entries"). SetMap(map[string]any{ - "id": entry.ID, - "fk_document_id": entry.DocumentID, - "fk_branch_id": entry.BranchID, "document_name": entry.DocumentName, "icon": entry.Icon, "content": entry.Content, "hooks": entry.Hooks, "fk_last_updated_by": entry.LastUpdatedBy, - "boundary": entry.Boundary, "created_at": entry.CreatedAt, }). + Where(sq.Eq{"id": newest.ID}). MustSql() if _, err := tx.ExecContext(ctx, q, args...); err != nil { - return err + return xid.ID{}, err } - return a.trimDocumentBranchHistoryEntries(ctx, tx, entry.BranchID) - }) + return newest.ID, nil + } + + q, args := a.builder.Insert("document_branch_history_entries"). + SetMap(map[string]any{ + "id": entry.ID, + "fk_document_id": entry.DocumentID, + "fk_branch_id": entry.BranchID, + "document_name": entry.DocumentName, + "icon": entry.Icon, + "content": entry.Content, + "hooks": entry.Hooks, + "fk_last_updated_by": entry.LastUpdatedBy, + "boundary": entry.Boundary, + "created_at": entry.CreatedAt, + }). + MustSql() + + if _, err := tx.ExecContext(ctx, q, args...); err != nil { + return xid.ID{}, err + } + + if err := a.trimDocumentBranchHistoryEntries(ctx, tx, entry.BranchID); err != nil { + return xid.ID{}, err + } + + return entry.ID, nil +} + +// UpdateDocumentBranchHistoryEntryHooks replaces the hooks an entry lists. +// A boundary entry is written before its branch's hooks are copied, so it +// gets the copied ones afterwards. +func (a *agent) UpdateDocumentBranchHistoryEntryHooks(ctx context.Context, id xid.ID, hooks history.Hooks) error { + q, args := a.builder.Update("document_branch_history_entries"). + Set("hooks", hooks). + Where(sq.Eq{"id": id}). + MustSql() + + _, err := a.sql.ExecContext(ctx, q, args...) + + return err } // fetchNewestDocumentBranchHistoryEntry retrieves the branch's newest diff --git a/server/core/internal/db/document_branch_test.go b/server/core/internal/db/document_branch_test.go index 3607be0c..a782e651 100644 --- a/server/core/internal/db/document_branch_test.go +++ b/server/core/internal/db/document_branch_test.go @@ -672,7 +672,7 @@ func prepHistoryEntries(t *testing.T, db *DB, doc *document.Document, count int) false, ) - require.NoError(t, db.InsertDocumentBranchHistoryEntry(context.Background(), entry)) + insertHistoryEntry(t, db, entry) res[i] = entry } @@ -680,6 +680,18 @@ func prepHistoryEntries(t *testing.T, db *DB, doc *document.Document, count int) return res } +// insertHistoryEntry writes the entry through +// insertDocumentBranchHistoryEntry in a transaction of its own. +func insertHistoryEntry(t *testing.T, db *DB, entry history.Entry) { + t.Helper() + + require.NoError(t, sqlutil.WrapTx(context.Background(), db.sql, func(tx *sqlx.Tx) error { + _, err := db.insertDocumentBranchHistoryEntry(context.Background(), tx, entry) + + return err + })) +} + // assertHistoryEntryEqual compares two entries, matching hook settings as // JSON since jsonb normalises their spacing. func assertHistoryEntryEqual(t *testing.T, exp, got history.Entry) { @@ -793,7 +805,7 @@ func Test_agent_DeleteExpiredDocumentBranchHistoryEntries(t *testing.T) { assert.Equal(t, 1, countHistoryEntries(t, db, doc.BranchID)) } -func Test_agent_InsertDocumentBranchHistoryEntry(t *testing.T) { +func Test_agent_insertDocumentBranchHistoryEntry(t *testing.T) { type tcase struct { Entry history.Entry // Rows is how many entries the branch holds afterwards. @@ -840,7 +852,7 @@ func Test_agent_InsertDocumentBranchHistoryEntry(t *testing.T) { user := prepUsers(t, db, 1)[0] first := history.NewEntry(*doc, base.Add(5*time.Minute), null.String{}, history.Hooks{}, false) - require.NoError(t, db.InsertDocumentBranchHistoryEntry(context.Background(), first)) + insertHistoryEntry(t, db, first) doc.DocumentName = "Renamed" doc.Icon = "📕" @@ -861,7 +873,7 @@ func Test_agent_InsertDocumentBranchHistoryEntry(t *testing.T) { doc := prepDocuments(t, db, 1, nil)[0] first := history.NewEntry(*doc, base.Add(5*time.Minute), null.String{}, history.Hooks{}, false) - require.NoError(t, db.InsertDocumentBranchHistoryEntry(context.Background(), first)) + insertHistoryEntry(t, db, first) entry := history.NewEntry(*doc, base.Add(31*time.Minute), null.String{}, hooks, false) @@ -875,7 +887,7 @@ func Test_agent_InsertDocumentBranchHistoryEntry(t *testing.T) { doc := prepDocuments(t, db, 1, nil)[0] first := history.NewEntry(*doc, base.Add(5*time.Minute), null.String{}, history.Hooks{}, false) - require.NoError(t, db.InsertDocumentBranchHistoryEntry(context.Background(), first)) + insertHistoryEntry(t, db, first) entry := history.NewEntry(*doc, base.Add(6*time.Minute), null.String{}, hooks, true) @@ -889,7 +901,7 @@ func Test_agent_InsertDocumentBranchHistoryEntry(t *testing.T) { doc := prepDocuments(t, db, 1, nil)[0] boundary := history.NewEntry(*doc, base.Add(5*time.Minute), null.String{}, history.Hooks{}, true) - require.NoError(t, db.InsertDocumentBranchHistoryEntry(context.Background(), boundary)) + insertHistoryEntry(t, db, boundary) entry := history.NewEntry(*doc, base.Add(6*time.Minute), null.String{}, hooks, false) @@ -923,7 +935,15 @@ func Test_agent_InsertDocumentBranchHistoryEntry(t *testing.T) { db := prepTempDB(t) c := cfn(t, db) - err := db.InsertDocumentBranchHistoryEntry(context.Background(), c.Entry) + var id xid.ID + + err := sqlutil.WrapTx(context.Background(), db.sql, func(tx *sqlx.Tx) error { + var err error + + id, err = db.insertDocumentBranchHistoryEntry(context.Background(), tx, c.Entry) + + return err + }) testutil.RequireEqualError(t, c.Err, err) if err != nil { @@ -936,10 +956,168 @@ func Test_agent_InsertDocumentBranchHistoryEntry(t *testing.T) { require.NoError(t, err) require.NotNil(t, newest) assertHistoryEntryEqual(t, c.Newest, *newest) + + // the id returned is the row written, which is the newest. + assert.Equal(t, newest.ID, id) }) } } +func Test_agent_RecordDocumentBranchHistoryEntry(t *testing.T) { + type tcase struct { + // Tx, when set, is the transaction the record runs in. It is + // committed afterwards. + Tx *Tx + BranchID xid.ID + OrganizationID string + By null.String + Boundary bool + // Rows is how many entries the branch holds afterwards. + Rows int + // Newest is the entry the branch's newest row is expected to + // equal afterwards, apart from its id and time. + Newest history.Entry + Err error + } + + // prepBranch returns a branch with one hook on it, fetched back so it + // matches what the record reads. + prepBranch := func(t *testing.T, db *DB) (*document.Document, hook.Hook) { + t.Helper() + + hk := prepDocumentHooks(t, db, 1, nil)[0] + + doc, err := db.FetchDocumentUnsafeByBranchID(context.Background(), hk.BranchID.V) + require.NoError(t, err) + + return doc, hk + } + + cc := map[string]func(*testing.T, *DB) tcase{ + "Branch of another organization": func(t *testing.T, db *DB) tcase { + doc, _ := prepBranch(t, db) + + return tcase{ + BranchID: doc.BranchID, + OrganizationID: "other", + Err: sql.ErrNoRows, + } + }, + "Ordinary entry lists the live hooks": func(t *testing.T, db *DB) tcase { + doc, hk := prepBranch(t, db) + user := prepUsers(t, db, 1)[0] + + return tcase{ + BranchID: doc.BranchID, + OrganizationID: doc.OrganizationID, + By: null.StringFrom(user), + Rows: 1, + Newest: history.NewEntry(*doc, time.Time{}, null.StringFrom(user), history.NewHooks([]hook.Hook{hk}), false), + } + }, + "Boundary entry adds its own row": func(t *testing.T, db *DB) tcase { + doc, hk := prepBranch(t, db) + + insertHistoryEntry(t, db, history.NewEntry(*doc, timeutil.Now(), null.String{}, history.Hooks{}, false)) + + return tcase{ + BranchID: doc.BranchID, + OrganizationID: doc.OrganizationID, + Boundary: true, + Rows: 2, + Newest: history.NewEntry(*doc, time.Time{}, null.String{}, history.NewHooks([]hook.Hook{hk}), true), + } + }, + // a persist records inside the transaction that wrote the branch, + // so the entry has to hold that write. + "Inside a transaction the entry holds its write": func(t *testing.T, db *DB) tcase { + doc, hk := prepBranch(t, db) + + var tx *Tx + + require.NoError(t, db.BeginTx(context.Background(), &tx)) + + doc.Content.Content[0].Text = "Written in the transaction." + + require.NoError(t, tx.UpdateDocument(context.Background(), *doc)) + + return tcase{ + Tx: tx, + BranchID: doc.BranchID, + OrganizationID: doc.OrganizationID, + Rows: 1, + Newest: history.NewEntry(*doc, time.Time{}, null.String{}, history.NewHooks([]hook.Hook{hk}), false), + } + }, + } + + for cn, cfn := range cc { + t.Run(cn, func(t *testing.T) { + t.Parallel() + + db := prepTempDB(t) + c := cfn(t, db) + + record := db.RecordDocumentBranchHistoryEntry + + if c.Tx != nil { + defer c.Tx.Rollback() //nolint:errcheck // error provides no meaningful info + + record = c.Tx.RecordDocumentBranchHistoryEntry + } + + before := timeutil.Now().Truncate(time.Microsecond) + + id, err := record(context.Background(), c.BranchID, c.OrganizationID, c.By, c.Boundary) + testutil.RequireEqualError(t, c.Err, err) + + if err != nil { + return + } + + if c.Tx != nil { + require.NoError(t, c.Tx.Commit()) + } + + assert.Equal(t, c.Rows, countHistoryEntries(t, db, c.BranchID)) + + newest, err := db.fetchNewestDocumentBranchHistoryEntry(context.Background(), db.sql, c.BranchID) + require.NoError(t, err) + require.NotNil(t, newest) + assert.Equal(t, newest.ID, id) + assert.False(t, newest.CreatedAt.Before(before)) + + exp := c.Newest + exp.ID = newest.ID + exp.CreatedAt = newest.CreatedAt + assertHistoryEntryEqual(t, exp, *newest) + }) + } +} + +func Test_agent_UpdateDocumentBranchHistoryEntryHooks(t *testing.T) { + t.Parallel() + + db := prepTempDB(t) + doc := prepDocuments(t, db, 1, nil)[0] + entry := prepHistoryEntries(t, db, doc, 1)[0] + + entry.Hooks = history.Hooks{ + { + Type: hook.TypeURLWatcher, + BlockID: null.StringFrom("b1"), + Settings: processor.Settings(`{"url":"https://example.com"}`), + }, + } + + require.NoError(t, db.UpdateDocumentBranchHistoryEntryHooks(context.Background(), entry.ID, entry.Hooks)) + + newest, err := db.fetchNewestDocumentBranchHistoryEntry(context.Background(), db.sql, doc.BranchID) + require.NoError(t, err) + require.NotNil(t, newest) + assertHistoryEntryEqual(t, entry, *newest) +} + func Test_agent_fetchNewestDocumentBranchHistoryEntry(t *testing.T) { t.Parallel() diff --git a/server/core/internal/db/document_file_test.go b/server/core/internal/db/document_file_test.go index b203af0e..2d92d7b7 100644 --- a/server/core/internal/db/document_file_test.go +++ b/server/core/internal/db/document_file_test.go @@ -243,10 +243,7 @@ func Test_agent_CheckDocumentFileReferenced(t *testing.T) { doc := prepDocuments(t, db, 1, nil)[0] doc.Content.Content[0].Attrs = document.Attributes{"uid": "file-ref"} - require.NoError(t, db.InsertDocumentBranchHistoryEntry( - context.Background(), - history.NewEntry(*doc, timeutil.Now(), null.String{}, nil, false), - )) + insertHistoryEntry(t, db, history.NewEntry(*doc, timeutil.Now(), null.String{}, nil, false)) return "file-ref", doc.ID, true }, diff --git a/server/core/internal/document/AGENTS.md b/server/core/internal/document/AGENTS.md index 78ee6617..997a795a 100644 --- a/server/core/internal/document/AGENTS.md +++ b/server/core/internal/document/AGENTS.md @@ -22,16 +22,18 @@ Cross-service storage rules (Hocuspocus, Yjs) live in `document_branch_history_entries` are restorable snapshots: name, icon, content, the branch's live hook definitions (type, block, settings; never -watcher state) and the author, written in the same transaction as the -branch update through `InsertDocumentBranchHistoryEntry`. Rules: +watcher state) and the author. Every write goes through +`RecordDocumentBranchHistoryEntry`, which locks the branch row and reads +it, inside the transaction that wrote the branch when there is one. - Ordinary edits within one 30-minute bucket update the branch's newest entry in place. A persist that changes nothing writes no entry; a system - write is unattributed (null author) and still takes the time of the - write. -- Merge and fork write a **boundary** entry that never aggregates and closes - its bucket: the next edit starts a new entry. The merge entry lists the - source's hooks. + write has a null author. +- A hook create, update or delete records an ordinary entry; a reset does + not. +- Create, duplicate, fork and merge write a **boundary** entry that never + aggregates and closes its bucket. It starts without hooks and gets the + ones `copyHooksToBranch` created after the commit. - Entries pin the files they reference, so `DB_MAX_DOCUMENT_HISTORY_ENTRIES` and `DB_DOCUMENT_HISTORY_RETENTION` also decide how long a removed image survives. diff --git a/server/core/internal/server/0mock_db_test.go b/server/core/internal/server/0mock_db_test.go index 651cda21..5e55f53e 100644 --- a/server/core/internal/server/0mock_db_test.go +++ b/server/core/internal/server/0mock_db_test.go @@ -209,9 +209,6 @@ var _ DB = &DBMock{} // InsertDocumentBranchFunc: func(ctx context.Context, doc document.Document) error { // panic("mock out the InsertDocumentBranch method") // }, -// InsertDocumentBranchHistoryEntryFunc: func(ctx context.Context, entry history.Entry) error { -// panic("mock out the InsertDocumentBranchHistoryEntry method") -// }, // InsertDocumentCommentFunc: func(ctx context.Context, c comment.Comment) error { // panic("mock out the InsertDocumentComment method") // }, @@ -248,6 +245,9 @@ var _ DB = &DBMock{} // PromoteBranchApprovalsFunc: func(ctx context.Context, fromBranchID xid.ID, toBranchID xid.ID, organizationID string) error { // panic("mock out the PromoteBranchApprovals method") // }, +// RecordDocumentBranchHistoryEntryFunc: func(ctx context.Context, branchID xid.ID, organizationID string, by null.String, boundary bool) (xid.ID, error) { +// panic("mock out the RecordDocumentBranchHistoryEntry method") +// }, // ReplaceBranchTagsFunc: func(ctx context.Context, organizationID string, fromBranchID xid.ID, toBranchID xid.ID) error { // panic("mock out the ReplaceBranchTags method") // }, @@ -275,6 +275,9 @@ var _ DB = &DBMock{} // UpdateDocumentFunc: func(ctx context.Context, doc document.Document) error { // panic("mock out the UpdateDocument method") // }, +// UpdateDocumentBranchHistoryEntryHooksFunc: func(ctx context.Context, id xid.ID, hooks history.Hooks) error { +// panic("mock out the UpdateDocumentBranchHistoryEntryHooks method") +// }, // UpdateDocumentBranchMetadataFunc: func(ctx context.Context, doc document.Document) error { // panic("mock out the UpdateDocumentBranchMetadata method") // }, @@ -498,9 +501,6 @@ type DBMock struct { // InsertDocumentBranchFunc mocks the InsertDocumentBranch method. InsertDocumentBranchFunc func(ctx context.Context, doc document.Document) error - // InsertDocumentBranchHistoryEntryFunc mocks the InsertDocumentBranchHistoryEntry method. - InsertDocumentBranchHistoryEntryFunc func(ctx context.Context, entry history.Entry) error - // InsertDocumentCommentFunc mocks the InsertDocumentComment method. InsertDocumentCommentFunc func(ctx context.Context, c comment.Comment) error @@ -537,6 +537,9 @@ type DBMock struct { // PromoteBranchApprovalsFunc mocks the PromoteBranchApprovals method. PromoteBranchApprovalsFunc func(ctx context.Context, fromBranchID xid.ID, toBranchID xid.ID, organizationID string) error + // RecordDocumentBranchHistoryEntryFunc mocks the RecordDocumentBranchHistoryEntry method. + RecordDocumentBranchHistoryEntryFunc func(ctx context.Context, branchID xid.ID, organizationID string, by null.String, boundary bool) (xid.ID, error) + // ReplaceBranchTagsFunc mocks the ReplaceBranchTags method. ReplaceBranchTagsFunc func(ctx context.Context, organizationID string, fromBranchID xid.ID, toBranchID xid.ID) error @@ -564,6 +567,9 @@ type DBMock struct { // UpdateDocumentFunc mocks the UpdateDocument method. UpdateDocumentFunc func(ctx context.Context, doc document.Document) error + // UpdateDocumentBranchHistoryEntryHooksFunc mocks the UpdateDocumentBranchHistoryEntryHooks method. + UpdateDocumentBranchHistoryEntryHooksFunc func(ctx context.Context, id xid.ID, hooks history.Hooks) error + // UpdateDocumentBranchMetadataFunc mocks the UpdateDocumentBranchMetadata method. UpdateDocumentBranchMetadataFunc func(ctx context.Context, doc document.Document) error @@ -1122,13 +1128,6 @@ type DBMock struct { // Doc is the doc argument value. Doc document.Document } - // InsertDocumentBranchHistoryEntry holds details about calls to the InsertDocumentBranchHistoryEntry method. - InsertDocumentBranchHistoryEntry []struct { - // Ctx is the ctx argument value. - Ctx context.Context - // Entry is the entry argument value. - Entry history.Entry - } // InsertDocumentComment holds details about calls to the InsertDocumentComment method. InsertDocumentComment []struct { // Ctx is the ctx argument value. @@ -1221,6 +1220,19 @@ type DBMock struct { // OrganizationID is the organizationID argument value. OrganizationID string } + // RecordDocumentBranchHistoryEntry holds details about calls to the RecordDocumentBranchHistoryEntry method. + RecordDocumentBranchHistoryEntry []struct { + // Ctx is the ctx argument value. + Ctx context.Context + // BranchID is the branchID argument value. + BranchID xid.ID + // OrganizationID is the organizationID argument value. + OrganizationID string + // By is the by argument value. + By null.String + // Boundary is the boundary argument value. + Boundary bool + } // ReplaceBranchTags holds details about calls to the ReplaceBranchTags method. ReplaceBranchTags []struct { // Ctx is the ctx argument value. @@ -1300,6 +1312,15 @@ type DBMock struct { // Doc is the doc argument value. Doc document.Document } + // UpdateDocumentBranchHistoryEntryHooks holds details about calls to the UpdateDocumentBranchHistoryEntryHooks method. + UpdateDocumentBranchHistoryEntryHooks []struct { + // Ctx is the ctx argument value. + Ctx context.Context + // ID is the id argument value. + ID xid.ID + // Hooks is the hooks argument value. + Hooks history.Hooks + } // UpdateDocumentBranchMetadata holds details about calls to the UpdateDocumentBranchMetadata method. UpdateDocumentBranchMetadata []struct { // Ctx is the ctx argument value. @@ -1471,7 +1492,6 @@ type DBMock struct { lockInsertDataSource sync.RWMutex lockInsertDocument sync.RWMutex lockInsertDocumentBranch sync.RWMutex - lockInsertDocumentBranchHistoryEntry sync.RWMutex lockInsertDocumentComment sync.RWMutex lockInsertDocumentCommentReply sync.RWMutex lockInsertDocumentFile sync.RWMutex @@ -1484,6 +1504,7 @@ type DBMock struct { lockInsertTag sync.RWMutex lockMarkReadByNotificationsIDs sync.RWMutex lockPromoteBranchApprovals sync.RWMutex + lockRecordDocumentBranchHistoryEntry sync.RWMutex lockReplaceBranchTags sync.RWMutex lockReplaceDocumentComment sync.RWMutex lockSetTagVisibility sync.RWMutex @@ -1493,6 +1514,7 @@ type DBMock struct { lockUpdateBranchReviewer sync.RWMutex lockUpdateDataSource sync.RWMutex lockUpdateDocument sync.RWMutex + lockUpdateDocumentBranchHistoryEntryHooks sync.RWMutex lockUpdateDocumentBranchMetadata sync.RWMutex lockUpdateDocumentComment sync.RWMutex lockUpdateDocumentCommentReply sync.RWMutex @@ -4054,45 +4076,6 @@ func (mock *DBMock) InsertDocumentBranchCalls() []struct { return calls } -// InsertDocumentBranchHistoryEntry calls InsertDocumentBranchHistoryEntryFunc. -func (mock *DBMock) InsertDocumentBranchHistoryEntry(ctx context.Context, entry history.Entry) error { - callInfo := struct { - Ctx context.Context - Entry history.Entry - }{ - Ctx: ctx, - Entry: entry, - } - mock.lockInsertDocumentBranchHistoryEntry.Lock() - mock.calls.InsertDocumentBranchHistoryEntry = append(mock.calls.InsertDocumentBranchHistoryEntry, callInfo) - mock.lockInsertDocumentBranchHistoryEntry.Unlock() - if mock.InsertDocumentBranchHistoryEntryFunc == nil { - var ( - errOut error - ) - return errOut - } - return mock.InsertDocumentBranchHistoryEntryFunc(ctx, entry) -} - -// InsertDocumentBranchHistoryEntryCalls gets all the calls that were made to InsertDocumentBranchHistoryEntry. -// Check the length with: -// -// len(mockedDB.InsertDocumentBranchHistoryEntryCalls()) -func (mock *DBMock) InsertDocumentBranchHistoryEntryCalls() []struct { - Ctx context.Context - Entry history.Entry -} { - var calls []struct { - Ctx context.Context - Entry history.Entry - } - mock.lockInsertDocumentBranchHistoryEntry.RLock() - calls = mock.calls.InsertDocumentBranchHistoryEntry - mock.lockInsertDocumentBranchHistoryEntry.RUnlock() - return calls -} - // InsertDocumentComment calls InsertDocumentCommentFunc. func (mock *DBMock) InsertDocumentComment(ctx context.Context, c comment.Comment) error { callInfo := struct { @@ -4577,6 +4560,58 @@ func (mock *DBMock) PromoteBranchApprovalsCalls() []struct { return calls } +// RecordDocumentBranchHistoryEntry calls RecordDocumentBranchHistoryEntryFunc. +func (mock *DBMock) RecordDocumentBranchHistoryEntry(ctx context.Context, branchID xid.ID, organizationID string, by null.String, boundary bool) (xid.ID, error) { + callInfo := struct { + Ctx context.Context + BranchID xid.ID + OrganizationID string + By null.String + Boundary bool + }{ + Ctx: ctx, + BranchID: branchID, + OrganizationID: organizationID, + By: by, + Boundary: boundary, + } + mock.lockRecordDocumentBranchHistoryEntry.Lock() + mock.calls.RecordDocumentBranchHistoryEntry = append(mock.calls.RecordDocumentBranchHistoryEntry, callInfo) + mock.lockRecordDocumentBranchHistoryEntry.Unlock() + if mock.RecordDocumentBranchHistoryEntryFunc == nil { + var ( + iDOut xid.ID + errOut error + ) + return iDOut, errOut + } + return mock.RecordDocumentBranchHistoryEntryFunc(ctx, branchID, organizationID, by, boundary) +} + +// RecordDocumentBranchHistoryEntryCalls gets all the calls that were made to RecordDocumentBranchHistoryEntry. +// Check the length with: +// +// len(mockedDB.RecordDocumentBranchHistoryEntryCalls()) +func (mock *DBMock) RecordDocumentBranchHistoryEntryCalls() []struct { + Ctx context.Context + BranchID xid.ID + OrganizationID string + By null.String + Boundary bool +} { + var calls []struct { + Ctx context.Context + BranchID xid.ID + OrganizationID string + By null.String + Boundary bool + } + mock.lockRecordDocumentBranchHistoryEntry.RLock() + calls = mock.calls.RecordDocumentBranchHistoryEntry + mock.lockRecordDocumentBranchHistoryEntry.RUnlock() + return calls +} + // ReplaceBranchTags calls ReplaceBranchTagsFunc. func (mock *DBMock) ReplaceBranchTags(ctx context.Context, organizationID string, fromBranchID xid.ID, toBranchID xid.ID) error { callInfo := struct { @@ -4960,6 +4995,49 @@ func (mock *DBMock) UpdateDocumentCalls() []struct { return calls } +// UpdateDocumentBranchHistoryEntryHooks calls UpdateDocumentBranchHistoryEntryHooksFunc. +func (mock *DBMock) UpdateDocumentBranchHistoryEntryHooks(ctx context.Context, id xid.ID, hooks history.Hooks) error { + callInfo := struct { + Ctx context.Context + ID xid.ID + Hooks history.Hooks + }{ + Ctx: ctx, + ID: id, + Hooks: hooks, + } + mock.lockUpdateDocumentBranchHistoryEntryHooks.Lock() + mock.calls.UpdateDocumentBranchHistoryEntryHooks = append(mock.calls.UpdateDocumentBranchHistoryEntryHooks, callInfo) + mock.lockUpdateDocumentBranchHistoryEntryHooks.Unlock() + if mock.UpdateDocumentBranchHistoryEntryHooksFunc == nil { + var ( + errOut error + ) + return errOut + } + return mock.UpdateDocumentBranchHistoryEntryHooksFunc(ctx, id, hooks) +} + +// UpdateDocumentBranchHistoryEntryHooksCalls gets all the calls that were made to UpdateDocumentBranchHistoryEntryHooks. +// Check the length with: +// +// len(mockedDB.UpdateDocumentBranchHistoryEntryHooksCalls()) +func (mock *DBMock) UpdateDocumentBranchHistoryEntryHooksCalls() []struct { + Ctx context.Context + ID xid.ID + Hooks history.Hooks +} { + var calls []struct { + Ctx context.Context + ID xid.ID + Hooks history.Hooks + } + mock.lockUpdateDocumentBranchHistoryEntryHooks.RLock() + calls = mock.calls.UpdateDocumentBranchHistoryEntryHooks + mock.lockUpdateDocumentBranchHistoryEntryHooks.RUnlock() + return calls +} + // UpdateDocumentBranchMetadata calls UpdateDocumentBranchMetadataFunc. func (mock *DBMock) UpdateDocumentBranchMetadata(ctx context.Context, doc document.Document) error { callInfo := struct { diff --git a/server/core/internal/server/internal/document/0mock_db_test.go b/server/core/internal/server/internal/document/0mock_db_test.go index db21760e..f06a12e0 100644 --- a/server/core/internal/server/internal/document/0mock_db_test.go +++ b/server/core/internal/server/internal/document/0mock_db_test.go @@ -107,9 +107,6 @@ var _ DB = &DBMock{} // InsertDocumentBranchFunc: func(ctx context.Context, doc documentCore.Document) error { // panic("mock out the InsertDocumentBranch method") // }, -// InsertDocumentBranchHistoryEntryFunc: func(ctx context.Context, entry history.Entry) error { -// panic("mock out the InsertDocumentBranchHistoryEntry method") -// }, // InsertDocumentFileFunc: func(ctx context.Context, f file.File) error { // panic("mock out the InsertDocumentFile method") // }, @@ -122,6 +119,9 @@ var _ DB = &DBMock{} // PromoteBranchApprovalsFunc: func(ctx context.Context, fromBranchID xid.ID, toBranchID xid.ID, organizationID string) error { // panic("mock out the PromoteBranchApprovals method") // }, +// RecordDocumentBranchHistoryEntryFunc: func(ctx context.Context, branchID xid.ID, organizationID string, by null.String, boundary bool) (xid.ID, error) { +// panic("mock out the RecordDocumentBranchHistoryEntry method") +// }, // ReplaceBranchTagsFunc: func(ctx context.Context, organizationID string, fromBranchID xid.ID, toBranchID xid.ID) error { // panic("mock out the ReplaceBranchTags method") // }, @@ -131,6 +131,9 @@ var _ DB = &DBMock{} // UpdateDocumentFunc: func(ctx context.Context, doc documentCore.Document) error { // panic("mock out the UpdateDocument method") // }, +// UpdateDocumentBranchHistoryEntryHooksFunc: func(ctx context.Context, id xid.ID, hooks history.Hooks) error { +// panic("mock out the UpdateDocumentBranchHistoryEntryHooks method") +// }, // UpdateDocumentBranchMetadataFunc: func(ctx context.Context, doc documentCore.Document) error { // panic("mock out the UpdateDocumentBranchMetadata method") // }, @@ -231,9 +234,6 @@ type DBMock struct { // InsertDocumentBranchFunc mocks the InsertDocumentBranch method. InsertDocumentBranchFunc func(ctx context.Context, doc documentCore.Document) error - // InsertDocumentBranchHistoryEntryFunc mocks the InsertDocumentBranchHistoryEntry method. - InsertDocumentBranchHistoryEntryFunc func(ctx context.Context, entry history.Entry) error - // InsertDocumentFileFunc mocks the InsertDocumentFile method. InsertDocumentFileFunc func(ctx context.Context, f file.File) error @@ -246,6 +246,9 @@ type DBMock struct { // PromoteBranchApprovalsFunc mocks the PromoteBranchApprovals method. PromoteBranchApprovalsFunc func(ctx context.Context, fromBranchID xid.ID, toBranchID xid.ID, organizationID string) error + // RecordDocumentBranchHistoryEntryFunc mocks the RecordDocumentBranchHistoryEntry method. + RecordDocumentBranchHistoryEntryFunc func(ctx context.Context, branchID xid.ID, organizationID string, by null.String, boundary bool) (xid.ID, error) + // ReplaceBranchTagsFunc mocks the ReplaceBranchTags method. ReplaceBranchTagsFunc func(ctx context.Context, organizationID string, fromBranchID xid.ID, toBranchID xid.ID) error @@ -255,6 +258,9 @@ type DBMock struct { // UpdateDocumentFunc mocks the UpdateDocument method. UpdateDocumentFunc func(ctx context.Context, doc documentCore.Document) error + // UpdateDocumentBranchHistoryEntryHooksFunc mocks the UpdateDocumentBranchHistoryEntryHooks method. + UpdateDocumentBranchHistoryEntryHooksFunc func(ctx context.Context, id xid.ID, hooks history.Hooks) error + // UpdateDocumentBranchMetadataFunc mocks the UpdateDocumentBranchMetadata method. UpdateDocumentBranchMetadataFunc func(ctx context.Context, doc documentCore.Document) error @@ -508,13 +514,6 @@ type DBMock struct { // Doc is the doc argument value. Doc documentCore.Document } - // InsertDocumentBranchHistoryEntry holds details about calls to the InsertDocumentBranchHistoryEntry method. - InsertDocumentBranchHistoryEntry []struct { - // Ctx is the ctx argument value. - Ctx context.Context - // Entry is the entry argument value. - Entry history.Entry - } // InsertDocumentFile holds details about calls to the InsertDocumentFile method. InsertDocumentFile []struct { // Ctx is the ctx argument value. @@ -547,6 +546,19 @@ type DBMock struct { // OrganizationID is the organizationID argument value. OrganizationID string } + // RecordDocumentBranchHistoryEntry holds details about calls to the RecordDocumentBranchHistoryEntry method. + RecordDocumentBranchHistoryEntry []struct { + // Ctx is the ctx argument value. + Ctx context.Context + // BranchID is the branchID argument value. + BranchID xid.ID + // OrganizationID is the organizationID argument value. + OrganizationID string + // By is the by argument value. + By null.String + // Boundary is the boundary argument value. + Boundary bool + } // ReplaceBranchTags holds details about calls to the ReplaceBranchTags method. ReplaceBranchTags []struct { // Ctx is the ctx argument value. @@ -572,6 +584,15 @@ type DBMock struct { // Doc is the doc argument value. Doc documentCore.Document } + // UpdateDocumentBranchHistoryEntryHooks holds details about calls to the UpdateDocumentBranchHistoryEntryHooks method. + UpdateDocumentBranchHistoryEntryHooks []struct { + // Ctx is the ctx argument value. + Ctx context.Context + // ID is the id argument value. + ID xid.ID + // Hooks is the hooks argument value. + Hooks history.Hooks + } // UpdateDocumentBranchMetadata holds details about calls to the UpdateDocumentBranchMetadata method. UpdateDocumentBranchMetadata []struct { // Ctx is the ctx argument value. @@ -611,45 +632,46 @@ type DBMock struct { MaintainerIDs []string } } - lockBeginTx sync.RWMutex - lockCheckDocumentCycle sync.RWMutex - lockCheckDocumentExists sync.RWMutex - lockCheckOrganizationMember sync.RWMutex - lockCopyBranchTags sync.RWMutex - lockCountDocumentBranches sync.RWMutex - lockDeleteBranchReviewer sync.RWMutex - lockDeleteDocument sync.RWMutex - lockDeleteDocumentBranchByID sync.RWMutex - lockDeleteDocumentCommentsByBranchID sync.RWMutex - lockDetachDocumentHooksByBranchID sync.RWMutex - lockFetchBranchReviewer sync.RWMutex - lockFetchBranchReviewers sync.RWMutex - lockFetchDocument sync.RWMutex - lockFetchDocumentBranches sync.RWMutex - lockFetchDocumentBranchesUnsafe sync.RWMutex - lockFetchDocumentByBranchID sync.RWMutex - lockFetchDocumentFile sync.RWMutex - lockFetchDocumentHooksByBranchID sync.RWMutex - lockFetchDocumentHooksByDocumentID sync.RWMutex - lockFetchDocumentMaintainers sync.RWMutex - lockFetchDocumentTree sync.RWMutex - lockFetchDocumentTreeByDocumentParentID sync.RWMutex - lockFetchDocumentUnsafeByBranchID sync.RWMutex - lockInsertBranchReviewer sync.RWMutex - lockInsertDocument sync.RWMutex - lockInsertDocumentBranch sync.RWMutex - lockInsertDocumentBranchHistoryEntry sync.RWMutex - lockInsertDocumentFile sync.RWMutex - lockInsertDocumentHook sync.RWMutex - lockInsertSearchJob sync.RWMutex - lockPromoteBranchApprovals sync.RWMutex - lockReplaceBranchTags sync.RWMutex - lockUpdateBranchReviewer sync.RWMutex - lockUpdateDocument sync.RWMutex - lockUpdateDocumentBranchMetadata sync.RWMutex - lockUpdateDocumentParentID sync.RWMutex - lockUpdateDocumentTree sync.RWMutex - lockUpsertDocumentMaintainers sync.RWMutex + lockBeginTx sync.RWMutex + lockCheckDocumentCycle sync.RWMutex + lockCheckDocumentExists sync.RWMutex + lockCheckOrganizationMember sync.RWMutex + lockCopyBranchTags sync.RWMutex + lockCountDocumentBranches sync.RWMutex + lockDeleteBranchReviewer sync.RWMutex + lockDeleteDocument sync.RWMutex + lockDeleteDocumentBranchByID sync.RWMutex + lockDeleteDocumentCommentsByBranchID sync.RWMutex + lockDetachDocumentHooksByBranchID sync.RWMutex + lockFetchBranchReviewer sync.RWMutex + lockFetchBranchReviewers sync.RWMutex + lockFetchDocument sync.RWMutex + lockFetchDocumentBranches sync.RWMutex + lockFetchDocumentBranchesUnsafe sync.RWMutex + lockFetchDocumentByBranchID sync.RWMutex + lockFetchDocumentFile sync.RWMutex + lockFetchDocumentHooksByBranchID sync.RWMutex + lockFetchDocumentHooksByDocumentID sync.RWMutex + lockFetchDocumentMaintainers sync.RWMutex + lockFetchDocumentTree sync.RWMutex + lockFetchDocumentTreeByDocumentParentID sync.RWMutex + lockFetchDocumentUnsafeByBranchID sync.RWMutex + lockInsertBranchReviewer sync.RWMutex + lockInsertDocument sync.RWMutex + lockInsertDocumentBranch sync.RWMutex + lockInsertDocumentFile sync.RWMutex + lockInsertDocumentHook sync.RWMutex + lockInsertSearchJob sync.RWMutex + lockPromoteBranchApprovals sync.RWMutex + lockRecordDocumentBranchHistoryEntry sync.RWMutex + lockReplaceBranchTags sync.RWMutex + lockUpdateBranchReviewer sync.RWMutex + lockUpdateDocument sync.RWMutex + lockUpdateDocumentBranchHistoryEntryHooks sync.RWMutex + lockUpdateDocumentBranchMetadata sync.RWMutex + lockUpdateDocumentParentID sync.RWMutex + lockUpdateDocumentTree sync.RWMutex + lockUpsertDocumentMaintainers sync.RWMutex } // BeginTx calls BeginTxFunc. @@ -1822,45 +1844,6 @@ func (mock *DBMock) InsertDocumentBranchCalls() []struct { return calls } -// InsertDocumentBranchHistoryEntry calls InsertDocumentBranchHistoryEntryFunc. -func (mock *DBMock) InsertDocumentBranchHistoryEntry(ctx context.Context, entry history.Entry) error { - callInfo := struct { - Ctx context.Context - Entry history.Entry - }{ - Ctx: ctx, - Entry: entry, - } - mock.lockInsertDocumentBranchHistoryEntry.Lock() - mock.calls.InsertDocumentBranchHistoryEntry = append(mock.calls.InsertDocumentBranchHistoryEntry, callInfo) - mock.lockInsertDocumentBranchHistoryEntry.Unlock() - if mock.InsertDocumentBranchHistoryEntryFunc == nil { - var ( - errOut error - ) - return errOut - } - return mock.InsertDocumentBranchHistoryEntryFunc(ctx, entry) -} - -// InsertDocumentBranchHistoryEntryCalls gets all the calls that were made to InsertDocumentBranchHistoryEntry. -// Check the length with: -// -// len(mockedDB.InsertDocumentBranchHistoryEntryCalls()) -func (mock *DBMock) InsertDocumentBranchHistoryEntryCalls() []struct { - Ctx context.Context - Entry history.Entry -} { - var calls []struct { - Ctx context.Context - Entry history.Entry - } - mock.lockInsertDocumentBranchHistoryEntry.RLock() - calls = mock.calls.InsertDocumentBranchHistoryEntry - mock.lockInsertDocumentBranchHistoryEntry.RUnlock() - return calls -} - // InsertDocumentFile calls InsertDocumentFileFunc. func (mock *DBMock) InsertDocumentFile(ctx context.Context, f file.File) error { callInfo := struct { @@ -2025,6 +2008,58 @@ func (mock *DBMock) PromoteBranchApprovalsCalls() []struct { return calls } +// RecordDocumentBranchHistoryEntry calls RecordDocumentBranchHistoryEntryFunc. +func (mock *DBMock) RecordDocumentBranchHistoryEntry(ctx context.Context, branchID xid.ID, organizationID string, by null.String, boundary bool) (xid.ID, error) { + callInfo := struct { + Ctx context.Context + BranchID xid.ID + OrganizationID string + By null.String + Boundary bool + }{ + Ctx: ctx, + BranchID: branchID, + OrganizationID: organizationID, + By: by, + Boundary: boundary, + } + mock.lockRecordDocumentBranchHistoryEntry.Lock() + mock.calls.RecordDocumentBranchHistoryEntry = append(mock.calls.RecordDocumentBranchHistoryEntry, callInfo) + mock.lockRecordDocumentBranchHistoryEntry.Unlock() + if mock.RecordDocumentBranchHistoryEntryFunc == nil { + var ( + iDOut xid.ID + errOut error + ) + return iDOut, errOut + } + return mock.RecordDocumentBranchHistoryEntryFunc(ctx, branchID, organizationID, by, boundary) +} + +// RecordDocumentBranchHistoryEntryCalls gets all the calls that were made to RecordDocumentBranchHistoryEntry. +// Check the length with: +// +// len(mockedDB.RecordDocumentBranchHistoryEntryCalls()) +func (mock *DBMock) RecordDocumentBranchHistoryEntryCalls() []struct { + Ctx context.Context + BranchID xid.ID + OrganizationID string + By null.String + Boundary bool +} { + var calls []struct { + Ctx context.Context + BranchID xid.ID + OrganizationID string + By null.String + Boundary bool + } + mock.lockRecordDocumentBranchHistoryEntry.RLock() + calls = mock.calls.RecordDocumentBranchHistoryEntry + mock.lockRecordDocumentBranchHistoryEntry.RUnlock() + return calls +} + // ReplaceBranchTags calls ReplaceBranchTagsFunc. func (mock *DBMock) ReplaceBranchTags(ctx context.Context, organizationID string, fromBranchID xid.ID, toBranchID xid.ID) error { callInfo := struct { @@ -2150,6 +2185,49 @@ func (mock *DBMock) UpdateDocumentCalls() []struct { return calls } +// UpdateDocumentBranchHistoryEntryHooks calls UpdateDocumentBranchHistoryEntryHooksFunc. +func (mock *DBMock) UpdateDocumentBranchHistoryEntryHooks(ctx context.Context, id xid.ID, hooks history.Hooks) error { + callInfo := struct { + Ctx context.Context + ID xid.ID + Hooks history.Hooks + }{ + Ctx: ctx, + ID: id, + Hooks: hooks, + } + mock.lockUpdateDocumentBranchHistoryEntryHooks.Lock() + mock.calls.UpdateDocumentBranchHistoryEntryHooks = append(mock.calls.UpdateDocumentBranchHistoryEntryHooks, callInfo) + mock.lockUpdateDocumentBranchHistoryEntryHooks.Unlock() + if mock.UpdateDocumentBranchHistoryEntryHooksFunc == nil { + var ( + errOut error + ) + return errOut + } + return mock.UpdateDocumentBranchHistoryEntryHooksFunc(ctx, id, hooks) +} + +// UpdateDocumentBranchHistoryEntryHooksCalls gets all the calls that were made to UpdateDocumentBranchHistoryEntryHooks. +// Check the length with: +// +// len(mockedDB.UpdateDocumentBranchHistoryEntryHooksCalls()) +func (mock *DBMock) UpdateDocumentBranchHistoryEntryHooksCalls() []struct { + Ctx context.Context + ID xid.ID + Hooks history.Hooks +} { + var calls []struct { + Ctx context.Context + ID xid.ID + Hooks history.Hooks + } + mock.lockUpdateDocumentBranchHistoryEntryHooks.RLock() + calls = mock.calls.UpdateDocumentBranchHistoryEntryHooks + mock.lockUpdateDocumentBranchHistoryEntryHooks.RUnlock() + return calls +} + // UpdateDocumentBranchMetadata calls UpdateDocumentBranchMetadataFunc. func (mock *DBMock) UpdateDocumentBranchMetadata(ctx context.Context, doc documentCore.Document) error { callInfo := struct { diff --git a/server/core/internal/server/internal/document/0mock_tx_test.go b/server/core/internal/server/internal/document/0mock_tx_test.go index 54d3c573..e26c7c0e 100644 --- a/server/core/internal/server/internal/document/0mock_tx_test.go +++ b/server/core/internal/server/internal/document/0mock_tx_test.go @@ -107,9 +107,6 @@ var _ Tx = &TxMock{} // InsertDocumentBranchFunc: func(ctx context.Context, doc documentCore.Document) error { // panic("mock out the InsertDocumentBranch method") // }, -// InsertDocumentBranchHistoryEntryFunc: func(ctx context.Context, entry history.Entry) error { -// panic("mock out the InsertDocumentBranchHistoryEntry method") -// }, // InsertDocumentFileFunc: func(ctx context.Context, f file.File) error { // panic("mock out the InsertDocumentFile method") // }, @@ -122,6 +119,9 @@ var _ Tx = &TxMock{} // PromoteBranchApprovalsFunc: func(ctx context.Context, fromBranchID xid.ID, toBranchID xid.ID, organizationID string) error { // panic("mock out the PromoteBranchApprovals method") // }, +// RecordDocumentBranchHistoryEntryFunc: func(ctx context.Context, branchID xid.ID, organizationID string, by null.String, boundary bool) (xid.ID, error) { +// panic("mock out the RecordDocumentBranchHistoryEntry method") +// }, // ReplaceBranchTagsFunc: func(ctx context.Context, organizationID string, fromBranchID xid.ID, toBranchID xid.ID) error { // panic("mock out the ReplaceBranchTags method") // }, @@ -134,6 +134,9 @@ var _ Tx = &TxMock{} // UpdateDocumentFunc: func(ctx context.Context, doc documentCore.Document) error { // panic("mock out the UpdateDocument method") // }, +// UpdateDocumentBranchHistoryEntryHooksFunc: func(ctx context.Context, id xid.ID, hooks history.Hooks) error { +// panic("mock out the UpdateDocumentBranchHistoryEntryHooks method") +// }, // UpdateDocumentBranchMetadataFunc: func(ctx context.Context, doc documentCore.Document) error { // panic("mock out the UpdateDocumentBranchMetadata method") // }, @@ -234,9 +237,6 @@ type TxMock struct { // InsertDocumentBranchFunc mocks the InsertDocumentBranch method. InsertDocumentBranchFunc func(ctx context.Context, doc documentCore.Document) error - // InsertDocumentBranchHistoryEntryFunc mocks the InsertDocumentBranchHistoryEntry method. - InsertDocumentBranchHistoryEntryFunc func(ctx context.Context, entry history.Entry) error - // InsertDocumentFileFunc mocks the InsertDocumentFile method. InsertDocumentFileFunc func(ctx context.Context, f file.File) error @@ -249,6 +249,9 @@ type TxMock struct { // PromoteBranchApprovalsFunc mocks the PromoteBranchApprovals method. PromoteBranchApprovalsFunc func(ctx context.Context, fromBranchID xid.ID, toBranchID xid.ID, organizationID string) error + // RecordDocumentBranchHistoryEntryFunc mocks the RecordDocumentBranchHistoryEntry method. + RecordDocumentBranchHistoryEntryFunc func(ctx context.Context, branchID xid.ID, organizationID string, by null.String, boundary bool) (xid.ID, error) + // ReplaceBranchTagsFunc mocks the ReplaceBranchTags method. ReplaceBranchTagsFunc func(ctx context.Context, organizationID string, fromBranchID xid.ID, toBranchID xid.ID) error @@ -261,6 +264,9 @@ type TxMock struct { // UpdateDocumentFunc mocks the UpdateDocument method. UpdateDocumentFunc func(ctx context.Context, doc documentCore.Document) error + // UpdateDocumentBranchHistoryEntryHooksFunc mocks the UpdateDocumentBranchHistoryEntryHooks method. + UpdateDocumentBranchHistoryEntryHooksFunc func(ctx context.Context, id xid.ID, hooks history.Hooks) error + // UpdateDocumentBranchMetadataFunc mocks the UpdateDocumentBranchMetadata method. UpdateDocumentBranchMetadataFunc func(ctx context.Context, doc documentCore.Document) error @@ -510,13 +516,6 @@ type TxMock struct { // Doc is the doc argument value. Doc documentCore.Document } - // InsertDocumentBranchHistoryEntry holds details about calls to the InsertDocumentBranchHistoryEntry method. - InsertDocumentBranchHistoryEntry []struct { - // Ctx is the ctx argument value. - Ctx context.Context - // Entry is the entry argument value. - Entry history.Entry - } // InsertDocumentFile holds details about calls to the InsertDocumentFile method. InsertDocumentFile []struct { // Ctx is the ctx argument value. @@ -549,6 +548,19 @@ type TxMock struct { // OrganizationID is the organizationID argument value. OrganizationID string } + // RecordDocumentBranchHistoryEntry holds details about calls to the RecordDocumentBranchHistoryEntry method. + RecordDocumentBranchHistoryEntry []struct { + // Ctx is the ctx argument value. + Ctx context.Context + // BranchID is the branchID argument value. + BranchID xid.ID + // OrganizationID is the organizationID argument value. + OrganizationID string + // By is the by argument value. + By null.String + // Boundary is the boundary argument value. + Boundary bool + } // ReplaceBranchTags holds details about calls to the ReplaceBranchTags method. ReplaceBranchTags []struct { // Ctx is the ctx argument value. @@ -577,6 +589,15 @@ type TxMock struct { // Doc is the doc argument value. Doc documentCore.Document } + // UpdateDocumentBranchHistoryEntryHooks holds details about calls to the UpdateDocumentBranchHistoryEntryHooks method. + UpdateDocumentBranchHistoryEntryHooks []struct { + // Ctx is the ctx argument value. + Ctx context.Context + // ID is the id argument value. + ID xid.ID + // Hooks is the hooks argument value. + Hooks history.Hooks + } // UpdateDocumentBranchMetadata holds details about calls to the UpdateDocumentBranchMetadata method. UpdateDocumentBranchMetadata []struct { // Ctx is the ctx argument value. @@ -616,46 +637,47 @@ type TxMock struct { MaintainerIDs []string } } - lockCheckDocumentCycle sync.RWMutex - lockCheckDocumentExists sync.RWMutex - lockCheckOrganizationMember sync.RWMutex - lockCommit sync.RWMutex - lockCopyBranchTags sync.RWMutex - lockCountDocumentBranches sync.RWMutex - lockDeleteBranchReviewer sync.RWMutex - lockDeleteDocument sync.RWMutex - lockDeleteDocumentBranchByID sync.RWMutex - lockDeleteDocumentCommentsByBranchID sync.RWMutex - lockDetachDocumentHooksByBranchID sync.RWMutex - lockFetchBranchReviewer sync.RWMutex - lockFetchBranchReviewers sync.RWMutex - lockFetchDocument sync.RWMutex - lockFetchDocumentBranches sync.RWMutex - lockFetchDocumentBranchesUnsafe sync.RWMutex - lockFetchDocumentByBranchID sync.RWMutex - lockFetchDocumentFile sync.RWMutex - lockFetchDocumentHooksByBranchID sync.RWMutex - lockFetchDocumentHooksByDocumentID sync.RWMutex - lockFetchDocumentMaintainers sync.RWMutex - lockFetchDocumentTree sync.RWMutex - lockFetchDocumentTreeByDocumentParentID sync.RWMutex - lockFetchDocumentUnsafeByBranchID sync.RWMutex - lockInsertBranchReviewer sync.RWMutex - lockInsertDocument sync.RWMutex - lockInsertDocumentBranch sync.RWMutex - lockInsertDocumentBranchHistoryEntry sync.RWMutex - lockInsertDocumentFile sync.RWMutex - lockInsertDocumentHook sync.RWMutex - lockInsertSearchJob sync.RWMutex - lockPromoteBranchApprovals sync.RWMutex - lockReplaceBranchTags sync.RWMutex - lockRollback sync.RWMutex - lockUpdateBranchReviewer sync.RWMutex - lockUpdateDocument sync.RWMutex - lockUpdateDocumentBranchMetadata sync.RWMutex - lockUpdateDocumentParentID sync.RWMutex - lockUpdateDocumentTree sync.RWMutex - lockUpsertDocumentMaintainers sync.RWMutex + lockCheckDocumentCycle sync.RWMutex + lockCheckDocumentExists sync.RWMutex + lockCheckOrganizationMember sync.RWMutex + lockCommit sync.RWMutex + lockCopyBranchTags sync.RWMutex + lockCountDocumentBranches sync.RWMutex + lockDeleteBranchReviewer sync.RWMutex + lockDeleteDocument sync.RWMutex + lockDeleteDocumentBranchByID sync.RWMutex + lockDeleteDocumentCommentsByBranchID sync.RWMutex + lockDetachDocumentHooksByBranchID sync.RWMutex + lockFetchBranchReviewer sync.RWMutex + lockFetchBranchReviewers sync.RWMutex + lockFetchDocument sync.RWMutex + lockFetchDocumentBranches sync.RWMutex + lockFetchDocumentBranchesUnsafe sync.RWMutex + lockFetchDocumentByBranchID sync.RWMutex + lockFetchDocumentFile sync.RWMutex + lockFetchDocumentHooksByBranchID sync.RWMutex + lockFetchDocumentHooksByDocumentID sync.RWMutex + lockFetchDocumentMaintainers sync.RWMutex + lockFetchDocumentTree sync.RWMutex + lockFetchDocumentTreeByDocumentParentID sync.RWMutex + lockFetchDocumentUnsafeByBranchID sync.RWMutex + lockInsertBranchReviewer sync.RWMutex + lockInsertDocument sync.RWMutex + lockInsertDocumentBranch sync.RWMutex + lockInsertDocumentFile sync.RWMutex + lockInsertDocumentHook sync.RWMutex + lockInsertSearchJob sync.RWMutex + lockPromoteBranchApprovals sync.RWMutex + lockRecordDocumentBranchHistoryEntry sync.RWMutex + lockReplaceBranchTags sync.RWMutex + lockRollback sync.RWMutex + lockUpdateBranchReviewer sync.RWMutex + lockUpdateDocument sync.RWMutex + lockUpdateDocumentBranchHistoryEntryHooks sync.RWMutex + lockUpdateDocumentBranchMetadata sync.RWMutex + lockUpdateDocumentParentID sync.RWMutex + lockUpdateDocumentTree sync.RWMutex + lockUpsertDocumentMaintainers sync.RWMutex } // CheckDocumentCycle calls CheckDocumentCycleFunc. @@ -1819,45 +1841,6 @@ func (mock *TxMock) InsertDocumentBranchCalls() []struct { return calls } -// InsertDocumentBranchHistoryEntry calls InsertDocumentBranchHistoryEntryFunc. -func (mock *TxMock) InsertDocumentBranchHistoryEntry(ctx context.Context, entry history.Entry) error { - callInfo := struct { - Ctx context.Context - Entry history.Entry - }{ - Ctx: ctx, - Entry: entry, - } - mock.lockInsertDocumentBranchHistoryEntry.Lock() - mock.calls.InsertDocumentBranchHistoryEntry = append(mock.calls.InsertDocumentBranchHistoryEntry, callInfo) - mock.lockInsertDocumentBranchHistoryEntry.Unlock() - if mock.InsertDocumentBranchHistoryEntryFunc == nil { - var ( - errOut error - ) - return errOut - } - return mock.InsertDocumentBranchHistoryEntryFunc(ctx, entry) -} - -// InsertDocumentBranchHistoryEntryCalls gets all the calls that were made to InsertDocumentBranchHistoryEntry. -// Check the length with: -// -// len(mockedTx.InsertDocumentBranchHistoryEntryCalls()) -func (mock *TxMock) InsertDocumentBranchHistoryEntryCalls() []struct { - Ctx context.Context - Entry history.Entry -} { - var calls []struct { - Ctx context.Context - Entry history.Entry - } - mock.lockInsertDocumentBranchHistoryEntry.RLock() - calls = mock.calls.InsertDocumentBranchHistoryEntry - mock.lockInsertDocumentBranchHistoryEntry.RUnlock() - return calls -} - // InsertDocumentFile calls InsertDocumentFileFunc. func (mock *TxMock) InsertDocumentFile(ctx context.Context, f file.File) error { callInfo := struct { @@ -2022,6 +2005,58 @@ func (mock *TxMock) PromoteBranchApprovalsCalls() []struct { return calls } +// RecordDocumentBranchHistoryEntry calls RecordDocumentBranchHistoryEntryFunc. +func (mock *TxMock) RecordDocumentBranchHistoryEntry(ctx context.Context, branchID xid.ID, organizationID string, by null.String, boundary bool) (xid.ID, error) { + callInfo := struct { + Ctx context.Context + BranchID xid.ID + OrganizationID string + By null.String + Boundary bool + }{ + Ctx: ctx, + BranchID: branchID, + OrganizationID: organizationID, + By: by, + Boundary: boundary, + } + mock.lockRecordDocumentBranchHistoryEntry.Lock() + mock.calls.RecordDocumentBranchHistoryEntry = append(mock.calls.RecordDocumentBranchHistoryEntry, callInfo) + mock.lockRecordDocumentBranchHistoryEntry.Unlock() + if mock.RecordDocumentBranchHistoryEntryFunc == nil { + var ( + iDOut xid.ID + errOut error + ) + return iDOut, errOut + } + return mock.RecordDocumentBranchHistoryEntryFunc(ctx, branchID, organizationID, by, boundary) +} + +// RecordDocumentBranchHistoryEntryCalls gets all the calls that were made to RecordDocumentBranchHistoryEntry. +// Check the length with: +// +// len(mockedTx.RecordDocumentBranchHistoryEntryCalls()) +func (mock *TxMock) RecordDocumentBranchHistoryEntryCalls() []struct { + Ctx context.Context + BranchID xid.ID + OrganizationID string + By null.String + Boundary bool +} { + var calls []struct { + Ctx context.Context + BranchID xid.ID + OrganizationID string + By null.String + Boundary bool + } + mock.lockRecordDocumentBranchHistoryEntry.RLock() + calls = mock.calls.RecordDocumentBranchHistoryEntry + mock.lockRecordDocumentBranchHistoryEntry.RUnlock() + return calls +} + // ReplaceBranchTags calls ReplaceBranchTagsFunc. func (mock *TxMock) ReplaceBranchTags(ctx context.Context, organizationID string, fromBranchID xid.ID, toBranchID xid.ID) error { callInfo := struct { @@ -2177,6 +2212,49 @@ func (mock *TxMock) UpdateDocumentCalls() []struct { return calls } +// UpdateDocumentBranchHistoryEntryHooks calls UpdateDocumentBranchHistoryEntryHooksFunc. +func (mock *TxMock) UpdateDocumentBranchHistoryEntryHooks(ctx context.Context, id xid.ID, hooks history.Hooks) error { + callInfo := struct { + Ctx context.Context + ID xid.ID + Hooks history.Hooks + }{ + Ctx: ctx, + ID: id, + Hooks: hooks, + } + mock.lockUpdateDocumentBranchHistoryEntryHooks.Lock() + mock.calls.UpdateDocumentBranchHistoryEntryHooks = append(mock.calls.UpdateDocumentBranchHistoryEntryHooks, callInfo) + mock.lockUpdateDocumentBranchHistoryEntryHooks.Unlock() + if mock.UpdateDocumentBranchHistoryEntryHooksFunc == nil { + var ( + errOut error + ) + return errOut + } + return mock.UpdateDocumentBranchHistoryEntryHooksFunc(ctx, id, hooks) +} + +// UpdateDocumentBranchHistoryEntryHooksCalls gets all the calls that were made to UpdateDocumentBranchHistoryEntryHooks. +// Check the length with: +// +// len(mockedTx.UpdateDocumentBranchHistoryEntryHooksCalls()) +func (mock *TxMock) UpdateDocumentBranchHistoryEntryHooksCalls() []struct { + Ctx context.Context + ID xid.ID + Hooks history.Hooks +} { + var calls []struct { + Ctx context.Context + ID xid.ID + Hooks history.Hooks + } + mock.lockUpdateDocumentBranchHistoryEntryHooks.RLock() + calls = mock.calls.UpdateDocumentBranchHistoryEntryHooks + mock.lockUpdateDocumentBranchHistoryEntryHooks.RUnlock() + return calls +} + // UpdateDocumentBranchMetadata calls UpdateDocumentBranchMetadataFunc. func (mock *TxMock) UpdateDocumentBranchMetadata(ctx context.Context, doc documentCore.Document) error { callInfo := struct { diff --git a/server/core/internal/server/internal/document/branch_http.go b/server/core/internal/server/internal/document/branch_http.go index ae425122..3a54faed 100644 --- a/server/core/internal/server/internal/document/branch_http.go +++ b/server/core/internal/server/internal/document/branch_http.go @@ -15,7 +15,6 @@ import ( "github.com/oxynote/oxynote/server/core/internal/server/internal/auth" "github.com/oxynote/oxynote/server/core/pkg/httpserver" "github.com/oxynote/oxynote/server/core/pkg/logutil" - "github.com/oxynote/oxynote/server/core/pkg/timeutil" "github.com/rs/xid" ) @@ -202,22 +201,19 @@ func (h *Handler) UpdateDocumentBranchByIDUnsafe(w http.ResponseWriter, r *http. // sends on first load, leaves history alone. A system write is not // attributed: no user made that change. if !doc.SnapshotEqual(ndoc) { - var hooks []hook.Hook - - hooks, err = tx.FetchDocumentHooksByBranchID(r.Context(), doc.BranchID, doc.OrganizationID) - if err != nil { - httpserver.RespondError(h.log, w, err) - return - } - var lastUpdatedBy null.String if !ui.System { lastUpdatedBy = ndoc.LastUpdatedBy } - entry := history.NewEntry(ndoc, timeutil.Now(), lastUpdatedBy, history.NewHooks(hooks), false) - - if err = tx.InsertDocumentBranchHistoryEntry(r.Context(), entry); err != nil { + _, err = tx.RecordDocumentBranchHistoryEntry( + r.Context(), + doc.BranchID, + doc.OrganizationID, + lastUpdatedBy, + false, + ) + if err != nil { httpserver.RespondError(h.log, w, err) return } @@ -324,29 +320,38 @@ func (h *Handler) MergeBranches(w http.ResponseWriter, r *http.Request) { var tx Tx - if err := h.db.BeginTx(r.Context(), &tx); err != nil { + if err = h.db.BeginTx(r.Context(), &tx); err != nil { httpserver.RespondError(h.log, w, err) return } defer tx.Rollback() //nolint:errcheck // error provides no meaningful info - if err := tx.DetachDocumentHooksByBranchID(r.Context(), toDoc.BranchID, session.ActiveOrganizationID); err != nil { + if err = tx.DetachDocumentHooksByBranchID(r.Context(), toDoc.BranchID, session.ActiveOrganizationID); err != nil { httpserver.RespondError(h.log, w, err) return } - if err := tx.DeleteDocumentCommentsByBranchID(r.Context(), toDoc.BranchID, session.ActiveOrganizationID); err != nil { + if err = tx.DeleteDocumentCommentsByBranchID(r.Context(), toDoc.BranchID, session.ActiveOrganizationID); err != nil { httpserver.RespondError(h.log, w, err) return } - if err := tx.UpdateDocument(r.Context(), ndoc); err != nil { + if err = tx.UpdateDocument(r.Context(), ndoc); err != nil { httpserver.RespondError(h.log, w, err) return } - if err := h.insertBoundaryEntry(r.Context(), tx, ndoc, fromDoc.BranchID, session); err != nil { + // the target's hooks were just detached, so the entry starts without + // any. updateEntryHooks adds the copies after the commit. + entryID, err := tx.RecordDocumentBranchHistoryEntry( + r.Context(), + toDoc.BranchID, + session.ActiveOrganizationID, + null.StringFrom(session.UserID), + true, + ) + if err != nil { httpserver.RespondError(h.log, w, err) return } @@ -373,7 +378,7 @@ func (h *Handler) MergeBranches(w http.ResponseWriter, r *http.Request) { h.searchTrigger.Trigger() - h.copyHooksToBranch( + hooks := h.copyHooksToBranch( r.Context(), fromDoc.BranchID, toDoc.BranchID, @@ -382,6 +387,8 @@ func (h *Handler) MergeBranches(w http.ResponseWriter, r *http.Request) { nil, ) + h.updateEntryHooks(r.Context(), entryID, hooks) + if h.metadata.changeCallback != nil { h.metadata.changeCallback(session.ActiveOrganizationID, ndoc) } @@ -481,7 +488,16 @@ func (h *Handler) CreateDocumentBranch(w http.ResponseWriter, r *http.Request) { return } - if err := h.insertBoundaryEntry(r.Context(), tx, newDoc, sourceDoc.BranchID, session); err != nil { + // the fork has no hooks yet. updateEntryHooks adds the copies after + // the commit. + entryID, err := tx.RecordDocumentBranchHistoryEntry( + r.Context(), + newDoc.BranchID, + session.ActiveOrganizationID, + null.StringFrom(session.UserID), + true, + ) + if err != nil { httpserver.RespondError(h.log, w, err) return } @@ -503,7 +519,7 @@ func (h *Handler) CreateDocumentBranch(w http.ResponseWriter, r *http.Request) { h.searchTrigger.Trigger() - h.copyHooksToBranch( + hooks := h.copyHooksToBranch( r.Context(), sourceDoc.BranchID, newDoc.BranchID, @@ -512,6 +528,8 @@ func (h *Handler) CreateDocumentBranch(w http.ResponseWriter, r *http.Request) { nil, ) + h.updateEntryHooks(r.Context(), entryID, hooks) + httpserver.Respond( h.log, w, @@ -608,7 +626,8 @@ func (h *Handler) DeleteDocumentBranch(w http.ResponseWriter, r *http.Request) { // watcher back, and the row that would have pointed at it is gone. A failed // insert tears the just-created resource down again for the same reason. A // failure stops the copy and leaves the target standing with fewer hooks than -// its source, which the critical log reports. +// its source, which the critical log reports. The hooks created up to that +// point are returned either way. // // A branch whose content was duplicated carries fresh block uids, so the // caller passes the old-to-new uid map and a hook anchored to a block is @@ -620,7 +639,7 @@ func (h *Handler) copyHooksToBranch( fromBranchID, toBranchID, documentID xid.ID, organizationID string, uids map[string]string, -) { +) []hook.Hook { hooks, err := h.db.FetchDocumentHooksByBranchID(ctx, fromBranchID, organizationID) if err != nil { logutil.Critical(h.log, err).Error( @@ -628,11 +647,13 @@ func (h *Handler) copyHooksToBranch( slog.String("branch_id", fromBranchID.String()), ) - return + return nil } inp := hook.NewInput(organizationID, h.githubMan, h.webchangeClient) + created := make([]hook.Hook, 0, len(hooks)) + for _, hk := range hooks { blockID := hk.BlockID @@ -668,7 +689,7 @@ func (h *Handler) copyHooksToBranch( slog.String("branch_id", toBranchID.String()), ) - return + return created } if err := h.db.InsertDocumentHook(ctx, *newHk); err != nil { @@ -685,22 +706,25 @@ func (h *Handler) copyHooksToBranch( slog.String("branch_id", toBranchID.String()), ) - return + return created } + + created = append(created, *newHk) } + + return created } -// insertBoundaryEntry records a boundary history entry of the document's -// branch, attributed to the session user. The entry lists the hooks of the -// branch the content came from, which are what the branch carries once -// copyHooksToBranch has run after the commit. -func (h *Handler) insertBoundaryEntry(ctx context.Context, tx Tx, doc documentCore.Document, sourceBranchID xid.ID, session auth.Session) error { - hooks, err := tx.FetchDocumentHooksByBranchID(ctx, sourceBranchID, session.ActiveOrganizationID) +// updateEntryHooks sets the hooks of a boundary entry to the ones +// copyHooksToBranch created. The entry's operation has already committed, +// so a failure is logged rather than returned. +func (h *Handler) updateEntryHooks(ctx context.Context, entryID xid.ID, hooks []hook.Hook) { + err := h.db.UpdateDocumentBranchHistoryEntryHooks(ctx, entryID, history.NewHooks(hooks)) if err != nil { - return err + h.log.Error( + "cannot record the copied hooks on the history entry", + slog.String("entry_id", entryID.String()), + slog.String("error", err.Error()), + ) } - - entry := history.NewEntry(doc, timeutil.Now(), null.StringFrom(session.UserID), history.NewHooks(hooks), true) - - return tx.InsertDocumentBranchHistoryEntry(ctx, entry) } diff --git a/server/core/internal/server/internal/document/branch_http_test.go b/server/core/internal/server/internal/document/branch_http_test.go index b57633f7..73522622 100644 --- a/server/core/internal/server/internal/document/branch_http_test.go +++ b/server/core/internal/server/internal/document/branch_http_test.go @@ -1,8 +1,10 @@ package document import ( + "bytes" "context" "errors" + "log/slog" "net/http" "net/http/httptest" "testing" @@ -13,8 +15,6 @@ import ( hookCore "github.com/oxynote/oxynote/server/core/internal/document/hook" "github.com/oxynote/oxynote/server/core/internal/document/hook/processor" "github.com/oxynote/oxynote/server/core/internal/search" - "github.com/oxynote/oxynote/server/core/internal/server/internal/auth" - "github.com/oxynote/oxynote/server/core/pkg/testutil" "github.com/rs/xid" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -151,8 +151,8 @@ func Test_Handler_UpdateDocumentBranchByIDUnsafe(t *testing.T) { BranchID xid.ID // History is how many history entries the persist records. History int - // HistoryCheck inspects the recorded entry. - HistoryCheck func(t *testing.T, entry history.Entry) + // HistoryBy is who the recorded entry is credited to. + HistoryBy null.String }{ "Unchanged persist writes no history entry": { DB: &DBMock{FetchDocumentUnsafeByBranchIDFunc: fetchStored}, @@ -164,26 +164,17 @@ func Test_Handler_UpdateDocumentBranchByIDUnsafe(t *testing.T) { Maintainers: 1, SearchJobs: 1, }, - "History hook fetch error": { + "Error returned by Tx.RecordDocumentBranchHistoryEntry": { DB: &DBMock{FetchDocumentUnsafeByBranchIDFunc: fetchStored}, Tx: &TxMock{ - FetchDocumentHooksByBranchIDFunc: func(context.Context, xid.ID, string) ([]hookCore.Hook, error) { - return nil, errors.New("boom") + RecordDocumentBranchHistoryEntryFunc: func(context.Context, xid.ID, string, null.String, bool) (xid.ID, error) { + return xid.ID{}, errors.New("boom") }, }, - Body: validBody, - RespCode: http.StatusInternalServerError, - }, - "History entry insert error": { - DB: &DBMock{FetchDocumentUnsafeByBranchIDFunc: fetchStored}, - Tx: &TxMock{ - InsertDocumentBranchHistoryEntryFunc: func(context.Context, history.Entry) error { - return errors.New("boom") - }, - }, - Body: validBody, - RespCode: http.StatusInternalServerError, - History: 1, + Body: validBody, + RespCode: http.StatusInternalServerError, + History: 1, + HistoryBy: null.StringFrom("u9"), }, "System write records an unattributed entry": { DB: &DBMock{ @@ -195,9 +186,6 @@ func Test_Handler_UpdateDocumentBranchByIDUnsafe(t *testing.T) { }, }, Tx: &TxMock{ - FetchDocumentHooksByBranchIDFunc: func(context.Context, xid.ID, string) ([]hookCore.Hook, error) { - return []hookCore.Hook{storedHook(hookCore.TypeScheduledReminder)}, nil - }, FetchDocumentMaintainersFunc: func(context.Context, xid.ID, string) ([]string, error) { return []string{"u1", "u9"}, nil }, @@ -208,18 +196,6 @@ func Test_Handler_UpdateDocumentBranchByIDUnsafe(t *testing.T) { Metadata: 1, SearchJobs: 1, History: 1, - HistoryCheck: func(t *testing.T, entry history.Entry) { - assert.Equal(t, _branchID, entry.BranchID) - assert.Equal(t, "Renamed", entry.DocumentName) - assert.False(t, entry.LastUpdatedBy.Valid) - assert.False(t, entry.Boundary) - assert.Equal(t, history.Hooks{ - { - Type: hookCore.TypeScheduledReminder, - Settings: processor.Settings(`{"scale":"linear"}`), - }, - }, entry.Hooks) - }, }, "Missing branch ID parameter": { DB: &DBMock{}, @@ -281,9 +257,10 @@ func Test_Handler_UpdateDocumentBranchByIDUnsafe(t *testing.T) { return nil, errors.New("boom") }, }, - Body: validBody, - RespCode: http.StatusInternalServerError, - History: 1, + Body: validBody, + RespCode: http.StatusInternalServerError, + History: 1, + HistoryBy: null.StringFrom("u9"), }, "Maintainer upsert error": { DB: &DBMock{FetchDocumentUnsafeByBranchIDFunc: fetchStored}, @@ -292,9 +269,10 @@ func Test_Handler_UpdateDocumentBranchByIDUnsafe(t *testing.T) { return errors.New("boom") }, }, - Body: validBody, - RespCode: http.StatusInternalServerError, - History: 1, + Body: validBody, + RespCode: http.StatusInternalServerError, + History: 1, + HistoryBy: null.StringFrom("u9"), }, "Search job insertion error on default branch": { DB: &DBMock{FetchDocumentUnsafeByBranchIDFunc: fetchStored}, @@ -310,6 +288,7 @@ func Test_Handler_UpdateDocumentBranchByIDUnsafe(t *testing.T) { RespCode: http.StatusInternalServerError, SearchJobs: 1, History: 1, + HistoryBy: null.StringFrom("u9"), }, "Commit error": { DB: &DBMock{FetchDocumentUnsafeByBranchIDFunc: fetchStored}, @@ -326,6 +305,7 @@ func Test_Handler_UpdateDocumentBranchByIDUnsafe(t *testing.T) { Committed: 1, SearchJobs: 1, History: 1, + HistoryBy: null.StringFrom("u9"), }, "Successful update with new maintainers": { DB: &DBMock{FetchDocumentUnsafeByBranchIDFunc: fetchStored}, @@ -337,6 +317,7 @@ func Test_Handler_UpdateDocumentBranchByIDUnsafe(t *testing.T) { Maintainers: 1, SearchJobs: 1, History: 1, + HistoryBy: null.StringFrom("u9"), }, "Successful update without new maintainers": { DB: &DBMock{FetchDocumentUnsafeByBranchIDFunc: fetchStored}, @@ -351,6 +332,7 @@ func Test_Handler_UpdateDocumentBranchByIDUnsafe(t *testing.T) { Metadata: 1, SearchJobs: 1, History: 1, + HistoryBy: null.StringFrom("u9"), }, "Non-default branch queues its own search job": { DB: &DBMock{ @@ -370,6 +352,7 @@ func Test_Handler_UpdateDocumentBranchByIDUnsafe(t *testing.T) { SearchJobs: 1, BranchID: _branchID2, History: 1, + HistoryBy: null.StringFrom("u9"), }, } @@ -388,16 +371,23 @@ func Test_Handler_UpdateDocumentBranchByIDUnsafe(t *testing.T) { assert.Equal(t, c.Metadata, cnt.metadata) assert.Equal(t, c.Maintainers, cnt.maintainers) assert.Len(t, c.Tx.InsertSearchJobCalls(), c.SearchJobs) - assert.Len(t, c.Tx.InsertDocumentBranchHistoryEntryCalls(), c.History) - if c.HistoryCheck != nil { - c.HistoryCheck(t, c.Tx.InsertDocumentBranchHistoryEntryCalls()[0].Entry) - } + // the entry is ordinary and records the persisted branch, + // credited to the last maintainer. A system write is not + // credited to anyone. + hh := c.Tx.RecordDocumentBranchHistoryEntryCalls() + require.Len(t, hh, c.History) - if cn == "Successful update with new maintainers" { - entry := c.Tx.InsertDocumentBranchHistoryEntryCalls()[0].Entry - assert.Equal(t, null.StringFrom("u9"), entry.LastUpdatedBy) - assert.Equal(t, history.Hooks{}, entry.Hooks) + if c.History > 0 { + branchID := c.BranchID + if branchID.IsZero() { + branchID = _branchID + } + + assert.Equal(t, branchID, hh[0].BranchID) + assert.Equal(t, "org1", hh[0].OrganizationID) + assert.Equal(t, c.HistoryBy, hh[0].By) + assert.False(t, hh[0].Boundary) } // the job is scoped to the persisted branch, whichever branch @@ -450,22 +440,14 @@ func Test_Handler_MergeBranches(t *testing.T) { Reviewers int // History is how many history entries the merge records. History int + // EntryHooks is how many hooks the merge entry lists afterwards. + EntryHooks int }{ - "History hook fetch error": { - DB: &DBMock{FetchDocumentByBranchIDFunc: fetchByBranch}, - Tx: &TxMock{ - FetchDocumentHooksByBranchIDFunc: func(context.Context, xid.ID, string) ([]hookCore.Hook, error) { - return nil, errors.New("boom") - }, - }, - Body: validBody, - RespCode: http.StatusInternalServerError, - }, "History entry insert error": { DB: &DBMock{FetchDocumentByBranchIDFunc: fetchByBranch}, Tx: &TxMock{ - InsertDocumentBranchHistoryEntryFunc: func(context.Context, history.Entry) error { - return errors.New("boom") + RecordDocumentBranchHistoryEntryFunc: func(context.Context, xid.ID, string, null.String, bool) (xid.ID, error) { + return xid.ID{}, errors.New("boom") }, }, Body: validBody, @@ -662,13 +644,14 @@ func Test_Handler_MergeBranches(t *testing.T) { return []hookCore.Hook{storedHook(hookCore.TypeScheduledReminder)}, nil }, }, - Tx: &TxMock{}, - Body: validBody, - RespCode: http.StatusOK, - Committed: 1, - Metadata: 1, - Reviewers: 1, - History: 1, + Tx: &TxMock{}, + Body: validBody, + RespCode: http.StatusOK, + Committed: 1, + Metadata: 1, + Reviewers: 1, + History: 1, + EntryHooks: 1, }, } @@ -691,19 +674,22 @@ func Test_Handler_MergeBranches(t *testing.T) { assert.Len(t, c.Tx.CommitCalls(), c.Committed) assert.Equal(t, c.Metadata, cnt.metadata) assert.Equal(t, c.Reviewers, cnt.reviewers) - assert.Len(t, c.Tx.InsertDocumentBranchHistoryEntryCalls(), c.History) + assert.Len(t, c.Tx.RecordDocumentBranchHistoryEntryCalls(), c.History) if c.RespCode == http.StatusOK { - // the merge is a boundary entry of the target, listing the - // source's hooks and attributed to the merging user. - entry := c.Tx.InsertDocumentBranchHistoryEntryCalls()[0].Entry + // the merge is a boundary entry of the target, attributed to + // the merging user. It takes the hooks the copy created once + // the merge commits. + entry := c.Tx.RecordDocumentBranchHistoryEntryCalls()[0] assert.Equal(t, _branchID, entry.BranchID) + assert.Equal(t, "org1", entry.OrganizationID) assert.True(t, entry.Boundary) - assert.Equal(t, null.StringFrom("u1"), entry.LastUpdatedBy) + assert.Equal(t, null.StringFrom("u1"), entry.By) - hh := c.Tx.FetchDocumentHooksByBranchIDCalls() + hh := c.DB.UpdateDocumentBranchHistoryEntryHooksCalls() require.Len(t, hh, 1) - assert.Equal(t, _branchID2, hh[0].BranchID) + assert.Equal(t, _entryID, hh[0].ID) + assert.Len(t, hh[0].Hooks, c.EntryHooks) // hooks are soft-deleted on the target inside the // transaction and re-created from the source branch once it @@ -803,20 +789,6 @@ func Test_Handler_CreateDocumentBranch(t *testing.T) { // History is how many history entries the fork records. History int }{ - "History hook fetch error": { - DB: &DBMock{ - FetchDocumentByBranchIDFunc: func(context.Context, xid.ID, string) (*documentCore.Document, error) { - return storedDoc(), nil - }, - }, - Tx: &TxMock{ - FetchDocumentHooksByBranchIDFunc: func(context.Context, xid.ID, string) ([]hookCore.Hook, error) { - return nil, errors.New("boom") - }, - }, - Body: validBody, - RespCode: http.StatusInternalServerError, - }, "History entry insert error": { DB: &DBMock{ FetchDocumentByBranchIDFunc: func(context.Context, xid.ID, string) (*documentCore.Document, error) { @@ -824,8 +796,8 @@ func Test_Handler_CreateDocumentBranch(t *testing.T) { }, }, Tx: &TxMock{ - InsertDocumentBranchHistoryEntryFunc: func(context.Context, history.Entry) error { - return errors.New("boom") + RecordDocumentBranchHistoryEntryFunc: func(context.Context, xid.ID, string, null.String, bool) (xid.ID, error) { + return xid.ID{}, errors.New("boom") }, }, Body: validBody, @@ -997,7 +969,7 @@ func Test_Handler_CreateDocumentBranch(t *testing.T) { assert.Equal(t, c.RespCode, rec.Code) assert.Len(t, c.Tx.CommitCalls(), c.Committed) assert.Empty(t, c.Tx.InsertDocumentHookCalls()) - assert.Len(t, c.Tx.InsertDocumentBranchHistoryEntryCalls(), c.History) + assert.Len(t, c.Tx.RecordDocumentBranchHistoryEntryCalls(), c.History) if c.RespCode == http.StatusCreated { require.Len(t, c.Tx.InsertDocumentBranchCalls(), 1) @@ -1007,14 +979,15 @@ func Test_Handler_CreateDocumentBranch(t *testing.T) { assert.False(t, forked.Protected) assert.Nil(t, forked.RawContent) - // the fork starts with a boundary entry of its own content, - // listing the source's hooks. - entry := c.Tx.InsertDocumentBranchHistoryEntryCalls()[0].Entry + // the fork starts with a boundary entry of its own content. + // It takes the hooks the copy created once the fork commits. + entry := c.Tx.RecordDocumentBranchHistoryEntryCalls()[0] assert.Equal(t, forked.BranchID, entry.BranchID) + assert.Equal(t, "org1", entry.OrganizationID) assert.True(t, entry.Boundary) - assert.Equal(t, null.StringFrom("u1"), entry.LastUpdatedBy) - require.Len(t, c.Tx.FetchDocumentHooksByBranchIDCalls(), 1) - assert.Equal(t, _branchID, c.Tx.FetchDocumentHooksByBranchIDCalls()[0].BranchID) + assert.Equal(t, null.StringFrom("u1"), entry.By) + require.Len(t, c.DB.UpdateDocumentBranchHistoryEntryHooksCalls(), 1) + assert.Equal(t, _entryID, c.DB.UpdateDocumentBranchHistoryEntryHooksCalls()[0].ID) // the fork takes the source's tags before the commit. require.Len(t, c.Tx.CopyBranchTagsCalls(), 1) @@ -1037,6 +1010,10 @@ func Test_Handler_CreateDocumentBranch(t *testing.T) { require.Len(t, c.DB.InsertDocumentHookCalls(), 1) assert.Equal(t, null.ValueFrom(forkedID), c.DB.InsertDocumentHookCalls()[0].Hk.BranchID) + assert.Equal(t, + history.NewHooks([]hookCore.Hook{c.DB.InsertDocumentHookCalls()[0].Hk}), + c.DB.UpdateDocumentBranchHistoryEntryHooksCalls()[0].Hooks, + ) }) } } @@ -1225,11 +1202,12 @@ func Test_Handler_copyHooksToBranch(t *testing.T) { hdl, _ := newTestHandler(db, &fakePublisher{}) - hdl.copyHooksToBranch(context.Background(), _branchID2, _branchID, _documentID, "org1", nil) + created := hdl.copyHooksToBranch(context.Background(), _branchID2, _branchID, _documentID, "org1", nil) ff := db.InsertDocumentHookCalls() require.Len(t, ff, 1) assert.Equal(t, hookCore.TypeScheduledReminder, ff[0].Hk.Type) + assert.Equal(t, []hookCore.Hook{ff[0].Hk}, created) }) // a duplicated branch carries fresh block uids, so a hook anchored to a @@ -1284,11 +1262,12 @@ func Test_Handler_copyHooksToBranch(t *testing.T) { hdl, _ := newTestHandler(db, &fakePublisher{}) - hdl.copyHooksToBranch(context.Background(), _branchID2, _branchID, _documentID, "org1", nil) + created := hdl.copyHooksToBranch(context.Background(), _branchID2, _branchID, _documentID, "org1", nil) // the failure stops the copy rather than working through the rest of // the branch's hooks and creating a watcher for each. require.Len(t, db.InsertDocumentHookCalls(), 1) + assert.Empty(t, created) } func Test_Handler_UpdateDocumentBranch(t *testing.T) { @@ -1455,78 +1434,51 @@ func Test_Handler_UpdateDocumentBranch(t *testing.T) { } } -func Test_Handler_insertBoundaryEntry(t *testing.T) { +func Test_Handler_updateEntryHooks(t *testing.T) { cc := map[string]struct { - Tx *TxMock - Err error - Hooks history.Hooks + Err error + Log string }{ - "Error returned by Tx.FetchDocumentHooksByBranchID": { - Tx: &TxMock{ - FetchDocumentHooksByBranchIDFunc: func(context.Context, xid.ID, string) ([]hookCore.Hook, error) { - return nil, assert.AnError - }, - }, + // the entry's operation has already committed, so the failure + // is logged rather than returned. + "Error returned by db.UpdateDocumentBranchHistoryEntryHooks": { Err: assert.AnError, + Log: "cannot record the copied hooks on the history entry", }, - "Error returned by Tx.InsertDocumentBranchHistoryEntry": { - Tx: &TxMock{ - InsertDocumentBranchHistoryEntryFunc: func(context.Context, history.Entry) error { - return assert.AnError - }, - }, - Err: assert.AnError, - }, - "Successful insert": { - Tx: &TxMock{ - FetchDocumentHooksByBranchIDFunc: func(context.Context, xid.ID, string) ([]hookCore.Hook, error) { - return []hookCore.Hook{storedHook(hookCore.TypeScheduledReminder)}, nil - }, - }, - Hooks: history.Hooks{ - { - Type: hookCore.TypeScheduledReminder, - Settings: processor.Settings(`{"scale":"linear"}`), - }, - }, - }, + "Successful update": {}, } for cn, c := range cc { t.Run(cn, func(t *testing.T) { t.Parallel() - hdl, _ := newTestHandler(&DBMock{}, &fakePublisher{}) - doc := branchDoc(_branchID2) + db := &DBMock{ + UpdateDocumentBranchHistoryEntryHooksFunc: func(context.Context, xid.ID, history.Hooks) error { + return c.Err + }, + } - err := hdl.insertBoundaryEntry( - context.Background(), - c.Tx, - *doc, - _branchID, - auth.Session{UserID: "u1", ActiveOrganizationID: "org1"}, - ) - testutil.AssertEqualError(t, c.Err, err) + var buf bytes.Buffer - ff := c.Tx.FetchDocumentHooksByBranchIDCalls() + hdl, _ := newTestHandler(db, &fakePublisher{}) + hdl.log = slog.New(slog.NewTextHandler(&buf, nil)) + + entryID := xid.New() + hk := storedHook(hookCore.TypeScheduledReminder) + + hdl.updateEntryHooks(context.Background(), entryID, []hookCore.Hook{hk}) + + ff := db.UpdateDocumentBranchHistoryEntryHooksCalls() require.Len(t, ff, 1) - assert.Equal(t, _branchID, ff[0].BranchID) - assert.Equal(t, "org1", ff[0].OrganizationID) + assert.Equal(t, entryID, ff[0].ID) + assert.Equal(t, history.NewHooks([]hookCore.Hook{hk}), ff[0].Hooks) - if err != nil && len(c.Tx.InsertDocumentBranchHistoryEntryCalls()) == 0 { + if c.Log == "" { + assert.Empty(t, buf.String()) return } - require.Len(t, c.Tx.InsertDocumentBranchHistoryEntryCalls(), 1) - entry := c.Tx.InsertDocumentBranchHistoryEntryCalls()[0].Entry - assert.Equal(t, _branchID2, entry.BranchID) - assert.Equal(t, doc.DocumentName, entry.DocumentName) - assert.True(t, entry.Boundary) - assert.Equal(t, null.StringFrom("u1"), entry.LastUpdatedBy) - - if c.Hooks != nil { - assert.Equal(t, c.Hooks, entry.Hooks) - } + assert.Contains(t, buf.String(), c.Log) }) } } diff --git a/server/core/internal/server/internal/document/document_http.go b/server/core/internal/server/internal/document/document_http.go index 1114459b..90ba3e31 100644 --- a/server/core/internal/server/internal/document/document_http.go +++ b/server/core/internal/server/internal/document/document_http.go @@ -535,7 +535,7 @@ func (h *Handler) CreateDocument(w http.ResponseWriter, r *http.Request) { doc := documentCore.NewDocument(di, session.ActiveOrganizationID, session.UserID) - if err := h.insertDocumentTx(r.Context(), doc, null.Value[xid.ID]{}, session); err != nil { + if _, err := h.insertDocumentTx(r.Context(), doc, null.Value[xid.ID]{}, session); err != nil { httpserver.RespondError(h.log, w, err) return } @@ -723,12 +723,13 @@ func (h *Handler) DuplicateDocument(w http.ResponseWriter, r *http.Request) { duplDoc, files, uids := doc.Duplicate(session.UserID) - if err = h.insertDocumentTx(r.Context(), duplDoc, null.ValueFrom(doc.BranchID), session); err != nil { + entryID, err := h.insertDocumentTx(r.Context(), duplDoc, null.ValueFrom(doc.BranchID), session) + if err != nil { httpserver.RespondError(h.log, w, err) return } - h.copyHooksToBranch( + hooks := h.copyHooksToBranch( r.Context(), doc.BranchID, duplDoc.BranchID, @@ -737,6 +738,8 @@ func (h *Handler) DuplicateDocument(w http.ResponseWriter, r *http.Request) { uids, ) + h.updateEntryHooks(r.Context(), entryID, hooks) + h.copyDocumentFiles(r.Context(), files, duplDoc.ID, session.ActiveOrganizationID) if h.tree.changeCallback != nil { @@ -843,75 +846,87 @@ func (h *Handler) upsertBranchReviewer( } } -// insertDocumentTx inserts a new document together with its maintainer and -// its search job, and slots it at the top of its parent's tree, all in one -// transaction. A document duplicated from a branch also takes that branch's -// tags. The tree-change notification is left to the caller, since it must -// not fire before the commit. +// insertDocumentTx inserts a new document together with its maintainer, +// its search job and the boundary history entry of its first content, and +// slots it at the top of its parent's tree, all in one transaction. It +// returns the entry's id. A document duplicated from a branch also takes +// that branch's tags. The tree-change notification is left to the caller, +// since it must not fire before the commit. func (h *Handler) insertDocumentTx( ctx context.Context, doc documentCore.Document, sourceBranchID null.Value[xid.ID], session auth.Session, -) error { +) (xid.ID, error) { var tx Tx if err := h.db.BeginTx(ctx, &tx); err != nil { - return err + return xid.ID{}, err } defer tx.Rollback() //nolint:errcheck // error provides no meaningful info if doc.ParentID.Valid { if err := tx.CheckDocumentExists(ctx, doc.ParentID.V, session.ActiveOrganizationID); err != nil { - return err + return xid.ID{}, err } } if err := tx.InsertDocument(ctx, doc); err != nil { - return err + return xid.ID{}, err + } + + entryID, err := tx.RecordDocumentBranchHistoryEntry( + ctx, + doc.BranchID, + session.ActiveOrganizationID, + null.StringFrom(session.UserID), + true, + ) + if err != nil { + return xid.ID{}, err } if sourceBranchID.Valid { - if err := tx.CopyBranchTags(ctx, session.ActiveOrganizationID, sourceBranchID.V, doc.BranchID); err != nil { - return err + if err = tx.CopyBranchTags(ctx, session.ActiveOrganizationID, sourceBranchID.V, doc.BranchID); err != nil { + return xid.ID{}, err } } - if err := tx.UpsertDocumentMaintainers( + if err = tx.UpsertDocumentMaintainers( ctx, doc.ID, session.ActiveOrganizationID, []string{session.UserID}, ); err != nil { - return err + return xid.ID{}, err } - if err := tx.InsertSearchJob(ctx, search.BranchScope(session.ActiveOrganizationID, doc.ID, doc.BranchID)); err != nil { - return err + if err = tx.InsertSearchJob(ctx, search.BranchScope(session.ActiveOrganizationID, doc.ID, doc.BranchID)); err != nil { + return xid.ID{}, err } tree, err := tx.FetchDocumentTreeByDocumentParentID(ctx, doc.ParentID, session.ActiveOrganizationID) if err != nil { - return err + return xid.ID{}, err } swappedTree, err := tree.Swap(doc.ID, 0) if err != nil { - return err + return xid.ID{}, err } if err = tx.UpdateDocumentTree(ctx, swappedTree, session.ActiveOrganizationID); err != nil { - return err + return xid.ID{}, err } if err = tx.Commit(); err != nil { - return err + return xid.ID{}, err } h.searchTrigger.Trigger() - return nil + return entryID, nil } // DB is an interface that combines sqlutil.DB and DBAgent. @@ -994,8 +1009,15 @@ type DocumentsDBAgent interface { // UpdateDocument should update the document. UpdateDocument(ctx context.Context, doc documentCore.Document) error - // InsertDocumentBranchHistoryEntry should record a snapshot of a branch. - InsertDocumentBranchHistoryEntry(ctx context.Context, entry history.Entry) error + // RecordDocumentBranchHistoryEntry should record the branch as it + // stands, with its live hooks, and return the id of the entry. + RecordDocumentBranchHistoryEntry( + ctx context.Context, + branchID xid.ID, + organizationID string, + by null.String, + boundary bool, + ) (xid.ID, error) // DeleteDocument should delete the document and report the ids of // the document and of every cascade-deleted descendant. @@ -1033,6 +1055,10 @@ type BranchesDBAgent interface { // ReplaceBranchTags should make the target branch carry exactly the // tags the source branch carries. ReplaceBranchTags(ctx context.Context, organizationID string, fromBranchID, toBranchID xid.ID) error + + // UpdateDocumentBranchHistoryEntryHooks should replace the hooks the + // history entry lists. + UpdateDocumentBranchHistoryEntryHooks(ctx context.Context, id xid.ID, hooks history.Hooks) error } // TreeDBAgent is an interface that handles communication with the document diff --git a/server/core/internal/server/internal/document/document_http_test.go b/server/core/internal/server/internal/document/document_http_test.go index ea39c571..3bc8ea23 100644 --- a/server/core/internal/server/internal/document/document_http_test.go +++ b/server/core/internal/server/internal/document/document_http_test.go @@ -35,6 +35,7 @@ var ( _documentID = xid.New() _branchID = xid.New() _branchID2 = xid.New() + _entryID = xid.New() ) // fakePublisher captures published notifications. @@ -81,8 +82,15 @@ func newTestHandler(db DB, pub *fakePublisher) (*Handler, *callbackCounts) { return hdl, cnt } -// withTx wires the DB mock's BeginTx to hand out the provided Tx mock. +// withTx wires the DB mock's BeginTx to hand out the provided Tx mock. A +// history record the Tx mock does not stub returns _entryID. func withTx(db *DBMock, tx *TxMock, err error) *DBMock { + if tx != nil && tx.RecordDocumentBranchHistoryEntryFunc == nil { + tx.RecordDocumentBranchHistoryEntryFunc = func(context.Context, xid.ID, string, null.String, bool) (xid.ID, error) { + return _entryID, nil + } + } + db.BeginTxFunc = func(_ context.Context, dest any) error { if err != nil { return err @@ -1298,6 +1306,15 @@ func Test_Handler_CreateDocument(t *testing.T) { Body: validBody, RespCode: http.StatusInternalServerError, }, + "History entry insert error": { + Tx: &TxMock{ + RecordDocumentBranchHistoryEntryFunc: func(context.Context, xid.ID, string, null.String, bool) (xid.ID, error) { + return xid.ID{}, errors.New("boom") + }, + }, + Body: validBody, + RespCode: http.StatusInternalServerError, + }, "Maintainer upsert error": { Tx: &TxMock{ UpsertDocumentMaintainersFunc: func(context.Context, xid.ID, string, []string) error { @@ -1381,6 +1398,15 @@ func Test_Handler_CreateDocument(t *testing.T) { if c.RespCode == http.StatusCreated { assert.Contains(t, rec.Body.String(), `"New doc"`) assert.Equal(t, []string{"u1"}, c.Tx.UpsertDocumentMaintainersCalls()[0].MaintainerIDs) + + // the first content is a boundary entry, so a later edit + // can be rolled back to it. + require.Len(t, c.Tx.RecordDocumentBranchHistoryEntryCalls(), 1) + entry := c.Tx.RecordDocumentBranchHistoryEntryCalls()[0] + assert.Equal(t, c.Tx.InsertDocumentCalls()[0].Doc.BranchID, entry.BranchID) + assert.Equal(t, "org1", entry.OrganizationID) + assert.True(t, entry.Boundary) + assert.Equal(t, null.StringFrom("u1"), entry.By) } }) } @@ -1756,6 +1782,18 @@ func Test_Handler_DuplicateDocument(t *testing.T) { assert.Empty(t, c.Tx.InsertDocumentHookCalls()) require.Len(t, c.DB.InsertDocumentHookCalls(), c.CopiedHooks) + // the duplicate starts with a boundary entry, which takes + // the copied hooks afterwards. + require.Len(t, c.Tx.RecordDocumentBranchHistoryEntryCalls(), 1) + entry := c.Tx.RecordDocumentBranchHistoryEntryCalls()[0] + assert.Equal(t, dupl.BranchID, entry.BranchID) + assert.True(t, entry.Boundary) + + hh := c.DB.UpdateDocumentBranchHistoryEntryHooksCalls() + require.Len(t, hh, 1) + assert.Equal(t, _entryID, hh[0].ID) + assert.Len(t, hh[0].Hooks, c.CopiedHooks) + // a hook anchored to a block follows the block's regenerated // uid rather than pointing at the source's. for _, call := range c.DB.InsertDocumentHookCalls() { diff --git a/server/core/internal/server/internal/document/hook/0mock_db_test.go b/server/core/internal/server/internal/document/hook/0mock_db_test.go index 063f940a..b838ea91 100644 --- a/server/core/internal/server/internal/document/hook/0mock_db_test.go +++ b/server/core/internal/server/internal/document/hook/0mock_db_test.go @@ -7,6 +7,7 @@ import ( "context" "sync" + "github.com/guregu/null/v5" "github.com/oxynote/oxynote/server/core/internal/document" hookCore "github.com/oxynote/oxynote/server/core/internal/document/hook" "github.com/rs/xid" @@ -37,6 +38,9 @@ var _ DB = &DBMock{} // InsertDocumentHookFunc: func(ctx context.Context, hk hookCore.Hook) error { // panic("mock out the InsertDocumentHook method") // }, +// RecordDocumentBranchHistoryEntryFunc: func(ctx context.Context, branchID xid.ID, organizationID string, by null.String, boundary bool) (xid.ID, error) { +// panic("mock out the RecordDocumentBranchHistoryEntry method") +// }, // UpdateDocumentHookFunc: func(ctx context.Context, hk hookCore.Hook) error { // panic("mock out the UpdateDocumentHook method") // }, @@ -62,6 +66,9 @@ type DBMock struct { // InsertDocumentHookFunc mocks the InsertDocumentHook method. InsertDocumentHookFunc func(ctx context.Context, hk hookCore.Hook) error + // RecordDocumentBranchHistoryEntryFunc mocks the RecordDocumentBranchHistoryEntry method. + RecordDocumentBranchHistoryEntryFunc func(ctx context.Context, branchID xid.ID, organizationID string, by null.String, boundary bool) (xid.ID, error) + // UpdateDocumentHookFunc mocks the UpdateDocumentHook method. UpdateDocumentHookFunc func(ctx context.Context, hk hookCore.Hook) error @@ -108,6 +115,19 @@ type DBMock struct { // Hk is the hk argument value. Hk hookCore.Hook } + // RecordDocumentBranchHistoryEntry holds details about calls to the RecordDocumentBranchHistoryEntry method. + RecordDocumentBranchHistoryEntry []struct { + // Ctx is the ctx argument value. + Ctx context.Context + // BranchID is the branchID argument value. + BranchID xid.ID + // OrganizationID is the organizationID argument value. + OrganizationID string + // By is the by argument value. + By null.String + // Boundary is the boundary argument value. + Boundary bool + } // UpdateDocumentHook holds details about calls to the UpdateDocumentHook method. UpdateDocumentHook []struct { // Ctx is the ctx argument value. @@ -116,12 +136,13 @@ type DBMock struct { Hk hookCore.Hook } } - lockDeleteDocumentHook sync.RWMutex - lockFetchDocumentByBranchID sync.RWMutex - lockFetchDocumentHook sync.RWMutex - lockFetchDocumentHooksByBranchID sync.RWMutex - lockInsertDocumentHook sync.RWMutex - lockUpdateDocumentHook sync.RWMutex + lockDeleteDocumentHook sync.RWMutex + lockFetchDocumentByBranchID sync.RWMutex + lockFetchDocumentHook sync.RWMutex + lockFetchDocumentHooksByBranchID sync.RWMutex + lockInsertDocumentHook sync.RWMutex + lockRecordDocumentBranchHistoryEntry sync.RWMutex + lockUpdateDocumentHook sync.RWMutex } // DeleteDocumentHook calls DeleteDocumentHookFunc. @@ -334,6 +355,58 @@ func (mock *DBMock) InsertDocumentHookCalls() []struct { return calls } +// RecordDocumentBranchHistoryEntry calls RecordDocumentBranchHistoryEntryFunc. +func (mock *DBMock) RecordDocumentBranchHistoryEntry(ctx context.Context, branchID xid.ID, organizationID string, by null.String, boundary bool) (xid.ID, error) { + callInfo := struct { + Ctx context.Context + BranchID xid.ID + OrganizationID string + By null.String + Boundary bool + }{ + Ctx: ctx, + BranchID: branchID, + OrganizationID: organizationID, + By: by, + Boundary: boundary, + } + mock.lockRecordDocumentBranchHistoryEntry.Lock() + mock.calls.RecordDocumentBranchHistoryEntry = append(mock.calls.RecordDocumentBranchHistoryEntry, callInfo) + mock.lockRecordDocumentBranchHistoryEntry.Unlock() + if mock.RecordDocumentBranchHistoryEntryFunc == nil { + var ( + iDOut xid.ID + errOut error + ) + return iDOut, errOut + } + return mock.RecordDocumentBranchHistoryEntryFunc(ctx, branchID, organizationID, by, boundary) +} + +// RecordDocumentBranchHistoryEntryCalls gets all the calls that were made to RecordDocumentBranchHistoryEntry. +// Check the length with: +// +// len(mockedDB.RecordDocumentBranchHistoryEntryCalls()) +func (mock *DBMock) RecordDocumentBranchHistoryEntryCalls() []struct { + Ctx context.Context + BranchID xid.ID + OrganizationID string + By null.String + Boundary bool +} { + var calls []struct { + Ctx context.Context + BranchID xid.ID + OrganizationID string + By null.String + Boundary bool + } + mock.lockRecordDocumentBranchHistoryEntry.RLock() + calls = mock.calls.RecordDocumentBranchHistoryEntry + mock.lockRecordDocumentBranchHistoryEntry.RUnlock() + return calls +} + // UpdateDocumentHook calls UpdateDocumentHookFunc. func (mock *DBMock) UpdateDocumentHook(ctx context.Context, hk hookCore.Hook) error { callInfo := struct { diff --git a/server/core/internal/server/internal/document/hook/http.go b/server/core/internal/server/internal/document/hook/http.go index d4687449..fe7d901d 100644 --- a/server/core/internal/server/internal/document/hook/http.go +++ b/server/core/internal/server/internal/document/hook/http.go @@ -6,6 +6,7 @@ import ( "log/slog" "net/http" + "github.com/guregu/null/v5" "github.com/oxynote/oxynote/server/core/internal/apps/github" "github.com/oxynote/oxynote/server/core/internal/apps/webchange" "github.com/oxynote/oxynote/server/core/internal/document" @@ -171,6 +172,8 @@ func (h *Handler) CreateDocumentHook(w http.ResponseWriter, r *http.Request) { return } + h.recordHistory(r.Context(), *hk, session) + h.NotifyHooksChange(session.ActiveOrganizationID, hk.DocumentID, hk.BranchID) httpserver.Respond( @@ -239,6 +242,8 @@ func (h *Handler) UpdateDocumentHook(w http.ResponseWriter, r *http.Request) { return } + h.recordHistory(r.Context(), *hk, session) + h.NotifyHooksChange(session.ActiveOrganizationID, hk.DocumentID, hk.BranchID) httpserver.Respond( @@ -353,6 +358,8 @@ func (h *Handler) DeleteDocumentHook(w http.ResponseWriter, r *http.Request) { return } + h.recordHistory(r.Context(), *hk, session) + h.NotifyHooksChange(session.ActiveOrganizationID, hk.DocumentID, hk.BranchID) httpserver.Respond( @@ -363,6 +370,30 @@ func (h *Handler) DeleteDocumentHook(w http.ResponseWriter, r *http.Request) { ) } +// recordHistory records the hook's branch in its history, credited to the +// session user. The hook change is already written, so a failure is logged +// rather than returned. +func (h *Handler) recordHistory(ctx context.Context, hk hookCore.Hook, session auth.Session) { + if !hk.BranchID.Valid { + return + } + + _, err := h.db.RecordDocumentBranchHistoryEntry( + ctx, + hk.BranchID.V, + session.ActiveOrganizationID, + null.StringFrom(session.UserID), + false, + ) + if err != nil { + h.log.Error( + "cannot record the hook change in the branch history", + slog.String("hook_id", hk.ID.String()), + slog.String("error", err.Error()), + ) + } +} + // DB is an interface that handles communication with the document hooks database. // //go:generate ../../../../../scripts/codegen/mock -t internal DB db @@ -385,4 +416,14 @@ type DB interface { // DeleteDocumentHook should delete the document hook for the given id. // The caller is expected to have fetched the hook org-scoped first. DeleteDocumentHook(ctx context.Context, id xid.ID) error + + // RecordDocumentBranchHistoryEntry should record the branch as it + // stands, with its live hooks, and return the id of the entry. + RecordDocumentBranchHistoryEntry( + ctx context.Context, + branchID xid.ID, + organizationID string, + by null.String, + boundary bool, + ) (xid.ID, error) } diff --git a/server/core/internal/server/internal/document/hook/http_test.go b/server/core/internal/server/internal/document/hook/http_test.go index 932dc7bc..10b95550 100644 --- a/server/core/internal/server/internal/document/hook/http_test.go +++ b/server/core/internal/server/internal/document/hook/http_test.go @@ -1,6 +1,7 @@ package hook import ( + "bytes" "context" "errors" "log/slog" @@ -324,6 +325,7 @@ func Test_Handler_CreateDocumentHook(t *testing.T) { Checks: checks( hasResp(http.StatusUnauthorized, `{"code":"account.not_authenticated","message":"not authenticated"}`), wasInsertCalled(0), + wasRecordCalled(0), ), }, "Missing document ID parameter": { @@ -333,6 +335,7 @@ func Test_Handler_CreateDocumentHook(t *testing.T) { Checks: checks( hasResp(http.StatusNotFound, `{"code":"general","message":"not found"}`), wasInsertCalled(0), + wasRecordCalled(0), ), }, "Invalid JSON body": { @@ -341,6 +344,7 @@ func Test_Handler_CreateDocumentHook(t *testing.T) { Checks: checks( hasResp(http.StatusBadRequest, `{"code":"request.invalid_json","message":"invalid JSON body"}`), wasInsertCalled(0), + wasRecordCalled(0), ), }, "Branch document fetch error": { @@ -353,6 +357,7 @@ func Test_Handler_CreateDocumentHook(t *testing.T) { Checks: checks( hasResp(http.StatusInternalServerError, `{"code":"general","message":"internal server error"}`), wasInsertCalled(0), + wasRecordCalled(0), ), }, "Invalid hook type": { @@ -361,6 +366,7 @@ func Test_Handler_CreateDocumentHook(t *testing.T) { Checks: checks( hasResp(http.StatusBadRequest, `{"code":"document_hook.invalid_type","message":"invalid hook type"}`), wasInsertCalled(0), + wasRecordCalled(0), ), }, "Hook insertion error": { @@ -373,6 +379,7 @@ func Test_Handler_CreateDocumentHook(t *testing.T) { Checks: checks( hasResp(http.StatusInternalServerError, `{"code":"general","message":"internal server error"}`), wasInsertCalled(1), + wasRecordCalled(0), ), }, "Branch belongs to another document": { @@ -385,6 +392,7 @@ func Test_Handler_CreateDocumentHook(t *testing.T) { Checks: checks( hasResp(http.StatusNotFound, `{"code":"document.branch_mismatch","message":"branch does not belong to the document"}`), wasInsertCalled(0), + wasRecordCalled(0), ), }, "Block not in the branch": { @@ -393,6 +401,7 @@ func Test_Handler_CreateDocumentHook(t *testing.T) { Checks: checks( hasResp(http.StatusNotFound, `{"code":"document.hook_block_not_found","message":"block not found in the branch"}`), wasInsertCalled(0), + wasRecordCalled(0), ), }, "Successful creation on a block": { @@ -407,6 +416,7 @@ func Test_Handler_CreateDocumentHook(t *testing.T) { assert.Equal(t, null.StringFrom("b1"), ff[0].Hk.BlockID) }, wasInsertCalled(1), + wasRecordCalled(1), ), }, "URL watcher without changedetection": { @@ -415,6 +425,7 @@ func Test_Handler_CreateDocumentHook(t *testing.T) { Checks: checks( hasResp(http.StatusConflict, `{"code":"changedetection.not_configured","message":"changedetection is not configured"}`), wasInsertCalled(0), + wasRecordCalled(0), ), }, "Successful creation": { @@ -426,6 +437,7 @@ func Test_Handler_CreateDocumentHook(t *testing.T) { assert.Contains(t, rec.Body.String(), `"scheduled-reminder"`) }, wasInsertCalled(1), + wasRecordCalled(1), ), }, } @@ -532,6 +544,7 @@ func Test_Handler_UpdateDocumentHook(t *testing.T) { Checks: checks( hasResp(http.StatusUnauthorized, `{"code":"account.not_authenticated","message":"not authenticated"}`), wasUpdateCalled(0), + wasRecordCalled(0), ), }, "Missing document ID parameter": { @@ -541,6 +554,7 @@ func Test_Handler_UpdateDocumentHook(t *testing.T) { Checks: checks( hasResp(http.StatusNotFound, `{"code":"general","message":"not found"}`), wasUpdateCalled(0), + wasRecordCalled(0), ), }, "Missing hook ID parameter": { @@ -550,6 +564,7 @@ func Test_Handler_UpdateDocumentHook(t *testing.T) { Checks: checks( hasResp(http.StatusNotFound, `{"code":"general","message":"not found"}`), wasUpdateCalled(0), + wasRecordCalled(0), ), }, "Hook of another document": { @@ -565,6 +580,7 @@ func Test_Handler_UpdateDocumentHook(t *testing.T) { Checks: checks( hasResp(http.StatusNotFound, `{"code":"document.hook_mismatch","message":"hook does not belong to the document"}`), wasUpdateCalled(0), + wasRecordCalled(0), ), }, "Hook fetch error": { @@ -577,6 +593,7 @@ func Test_Handler_UpdateDocumentHook(t *testing.T) { Checks: checks( hasResp(http.StatusInternalServerError, `{"code":"general","message":"internal server error"}`), wasUpdateCalled(0), + wasRecordCalled(0), ), }, "Invalid JSON body": { @@ -589,6 +606,7 @@ func Test_Handler_UpdateDocumentHook(t *testing.T) { Checks: checks( hasResp(http.StatusBadRequest, `{"code":"request.invalid_json","message":"invalid JSON body"}`), wasUpdateCalled(0), + wasRecordCalled(0), ), }, "Update application error": { @@ -601,6 +619,7 @@ func Test_Handler_UpdateDocumentHook(t *testing.T) { Checks: checks( hasResp(http.StatusInternalServerError, `{"code":"general","message":"internal server error"}`), wasUpdateCalled(0), + wasRecordCalled(0), ), }, "Hook update error": { @@ -616,6 +635,7 @@ func Test_Handler_UpdateDocumentHook(t *testing.T) { Checks: checks( hasResp(http.StatusInternalServerError, `{"code":"general","message":"internal server error"}`), wasUpdateCalled(1), + wasRecordCalled(0), ), }, "URL watcher without changedetection": { @@ -628,6 +648,7 @@ func Test_Handler_UpdateDocumentHook(t *testing.T) { Checks: checks( hasResp(http.StatusConflict, `{"code":"changedetection.not_configured","message":"changedetection is not configured"}`), wasUpdateCalled(0), + wasRecordCalled(0), ), }, "Successful update": { @@ -643,6 +664,7 @@ func Test_Handler_UpdateDocumentHook(t *testing.T) { assert.Contains(t, rec.Body.String(), _hookID.String()) }, wasUpdateCalled(1), + wasRecordCalled(1), ), }, } @@ -718,19 +740,19 @@ func Test_Handler_ResetDocumentHook(t *testing.T) { DB: &DBMock{}, NoSession: true, RespCode: http.StatusUnauthorized, - Checks: checks(wasUpdateCalled(0)), + Checks: checks(wasUpdateCalled(0), wasRecordCalled(0)), }, "Missing document ID parameter": { DB: &DBMock{}, OmitDoc: true, RespCode: http.StatusNotFound, - Checks: checks(wasUpdateCalled(0)), + Checks: checks(wasUpdateCalled(0), wasRecordCalled(0)), }, "Missing hook ID parameter": { DB: &DBMock{}, OmitID: true, RespCode: http.StatusNotFound, - Checks: checks(wasUpdateCalled(0)), + Checks: checks(wasUpdateCalled(0), wasRecordCalled(0)), }, "Hook of another document": { DB: &DBMock{ @@ -742,7 +764,7 @@ func Test_Handler_ResetDocumentHook(t *testing.T) { }, }, RespCode: http.StatusNotFound, - Checks: checks(wasUpdateCalled(0)), + Checks: checks(wasUpdateCalled(0), wasRecordCalled(0)), }, "Hook fetch error": { DB: &DBMock{ @@ -751,7 +773,7 @@ func Test_Handler_ResetDocumentHook(t *testing.T) { }, }, RespCode: http.StatusInternalServerError, - Checks: checks(wasUpdateCalled(0)), + Checks: checks(wasUpdateCalled(0), wasRecordCalled(0)), }, "Reset error": { DB: &DBMock{ @@ -760,7 +782,7 @@ func Test_Handler_ResetDocumentHook(t *testing.T) { }, }, RespCode: http.StatusInternalServerError, - Checks: checks(wasUpdateCalled(0)), + Checks: checks(wasUpdateCalled(0), wasRecordCalled(0)), }, "Hook update error": { DB: &DBMock{ @@ -772,7 +794,7 @@ func Test_Handler_ResetDocumentHook(t *testing.T) { }, }, RespCode: http.StatusInternalServerError, - Checks: checks(wasUpdateCalled(1)), + Checks: checks(wasUpdateCalled(1), wasRecordCalled(0)), }, "URL watcher without changedetection": { DB: &DBMock{ @@ -781,7 +803,7 @@ func Test_Handler_ResetDocumentHook(t *testing.T) { }, }, RespCode: http.StatusConflict, - Checks: checks(wasUpdateCalled(0)), + Checks: checks(wasUpdateCalled(0), wasRecordCalled(0)), }, "Successful reset": { DB: &DBMock{ @@ -790,7 +812,7 @@ func Test_Handler_ResetDocumentHook(t *testing.T) { }, }, RespCode: http.StatusOK, - Checks: checks(wasUpdateCalled(1)), + Checks: checks(wasUpdateCalled(1), wasRecordCalled(0)), }, } @@ -866,19 +888,19 @@ func Test_Handler_DeleteDocumentHook(t *testing.T) { DB: &DBMock{}, NoSession: true, RespCode: http.StatusUnauthorized, - Checks: checks(wasDeleteCalled(0)), + Checks: checks(wasDeleteCalled(0), wasRecordCalled(0)), }, "Missing document ID parameter": { DB: &DBMock{}, OmitDoc: true, RespCode: http.StatusNotFound, - Checks: checks(wasDeleteCalled(0)), + Checks: checks(wasDeleteCalled(0), wasRecordCalled(0)), }, "Missing hook ID parameter": { DB: &DBMock{}, OmitID: true, RespCode: http.StatusNotFound, - Checks: checks(wasDeleteCalled(0)), + Checks: checks(wasDeleteCalled(0), wasRecordCalled(0)), }, "Hook of another document": { DB: &DBMock{ @@ -890,7 +912,7 @@ func Test_Handler_DeleteDocumentHook(t *testing.T) { }, }, RespCode: http.StatusNotFound, - Checks: checks(wasDeleteCalled(0)), + Checks: checks(wasDeleteCalled(0), wasRecordCalled(0)), }, "Hook fetch error": { DB: &DBMock{ @@ -899,7 +921,7 @@ func Test_Handler_DeleteDocumentHook(t *testing.T) { }, }, RespCode: http.StatusInternalServerError, - Checks: checks(wasDeleteCalled(0)), + Checks: checks(wasDeleteCalled(0), wasRecordCalled(0)), }, "External cleanup error": { DB: &DBMock{ @@ -908,7 +930,7 @@ func Test_Handler_DeleteDocumentHook(t *testing.T) { }, }, RespCode: http.StatusInternalServerError, - Checks: checks(wasDeleteCalled(0)), + Checks: checks(wasDeleteCalled(0), wasRecordCalled(0)), }, "Hook deletion error": { DB: &DBMock{ @@ -920,7 +942,7 @@ func Test_Handler_DeleteDocumentHook(t *testing.T) { }, }, RespCode: http.StatusInternalServerError, - Checks: checks(wasDeleteCalled(1)), + Checks: checks(wasDeleteCalled(1), wasRecordCalled(0)), }, "Successful deletion": { DB: &DBMock{ @@ -929,7 +951,7 @@ func Test_Handler_DeleteDocumentHook(t *testing.T) { }, }, RespCode: http.StatusNoContent, - Checks: checks(wasDeleteCalled(1)), + Checks: checks(wasDeleteCalled(1), wasRecordCalled(1)), }, // an unconfigured changedetection must not trap the row: the @@ -941,7 +963,7 @@ func Test_Handler_DeleteDocumentHook(t *testing.T) { }, }, RespCode: http.StatusNoContent, - Checks: checks(wasDeleteCalled(1)), + Checks: checks(wasDeleteCalled(1), wasRecordCalled(1)), }, } @@ -986,3 +1008,91 @@ func Test_Handler_DeleteDocumentHook(t *testing.T) { }) } } + +func Test_Handler_recordHistory(t *testing.T) { + cc := map[string]struct { + Hook hookCore.Hook + Err error + Records int + Log string + }{ + "Hook without a branch": { + Hook: func() hookCore.Hook { + hk := *urlWatcherHook() + hk.BranchID = null.Value[xid.ID]{} + + return hk + }(), + }, + // the hook change is already written, so the failure is logged + // rather than returned. + "Error returned by db.RecordDocumentBranchHistoryEntry": { + Hook: *urlWatcherHook(), + Err: assert.AnError, + Records: 1, + Log: "cannot record the hook change in the branch history", + }, + "Successful record": { + Hook: *urlWatcherHook(), + Records: 1, + }, + } + + for cn, c := range cc { + t.Run(cn, func(t *testing.T) { + t.Parallel() + + db := &DBMock{ + RecordDocumentBranchHistoryEntryFunc: func(context.Context, xid.ID, string, null.String, bool) (xid.ID, error) { + return xid.New(), c.Err + }, + } + + var buf bytes.Buffer + + hdl := Handler{ + log: slog.New(slog.NewTextHandler(&buf, nil)), + db: db, + } + + hdl.recordHistory(context.Background(), c.Hook, auth.Session{UserID: "u1", ActiveOrganizationID: "org1"}) + + ff := db.RecordDocumentBranchHistoryEntryCalls() + require.Len(t, ff, c.Records) + + if c.Records > 0 { + assert.Equal(t, _branchID, ff[0].BranchID) + assert.Equal(t, "org1", ff[0].OrganizationID) + assert.Equal(t, null.StringFrom("u1"), ff[0].By) + assert.False(t, ff[0].Boundary) + } + + if c.Log == "" { + assert.Empty(t, buf.String()) + return + } + + assert.Contains(t, buf.String(), c.Log) + }) + } +} + +// wasRecordCalled checks that the handler recorded the hook's branch in its +// history count times, credited to the session user. +func wasRecordCalled(count int) func(*testing.T, *DBMock, *httptest.ResponseRecorder) { + return func(t *testing.T, db *DBMock, _ *httptest.ResponseRecorder) { + t.Helper() + + ff := db.RecordDocumentBranchHistoryEntryCalls() + require.Len(t, ff, count) + + if count == 0 { + return + } + + assert.Equal(t, _branchID, ff[0].BranchID) + assert.Equal(t, "org1", ff[0].OrganizationID) + assert.Equal(t, null.StringFrom("u1"), ff[0].By) + assert.False(t, ff[0].Boundary) + } +}