github-mirror/oxynote
1
0
Fork 0
mirror of https://github.com/oxynote/oxynote.git synced 2026-10-17 02:49:18 +02:00
Code Issues Actions Packages Projects Releases Wiki Activity Graph

improve hooks rewrite tests

Browse Source
This commit is contained in:
david 2026-10-04 18:37:34 +03:00
parent 500257107b
commit 7aa44a082a
50 changed files with 1522 additions and 1153 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
server/auth-realtime/src/routes.test.ts
View File

@ -1461,7 +1461,7 @@ describe("createRoutes", () => {
it("flushes the branch and answers once core has it", async ({
expect,
}) => {
const { app, flushDocument } = build()
const { app, core, flushDocument } = build()
const res = await app.request(
"/internal/documents/doc1/branches/b1/flush",
@ -1469,6 +1469,8 @@ describe("createRoutes", () => {
)
expect(res.status).toBe(204)
expect(core.fetchBranches).toHaveBeenCalledTimes(1)
expect(core.fetchBranches).toHaveBeenCalledWith("doc1")
expect(
flushDocument.mock.calls.map((call) => call[0]),
).toEqual(["doc1-b1"])
@ -1480,7 +1482,7 @@ describe("createRoutes", () => {
.mockRejectedValue(
new Error("core unreachable"),
)
const { app } = build({ flushDocument })
const { app, core } = build({ flushDocument })
const res = await app.request(
"/internal/documents/doc1/branches/b1/flush",
@ -1488,6 +1490,13 @@ describe("createRoutes", () => {
)
expect(res.status).toBe(500)
expect(await res.json()).toEqual({
error: "pending changes could not be stored",
})
expect(core.fetchBranches).toHaveBeenCalledTimes(1)
expect(core.fetchBranches).toHaveBeenCalledWith("doc1")
expect(flushDocument).toHaveBeenCalledTimes(1)
expect(flushDocument).toHaveBeenCalledWith("doc1-b1")
})
})

6
server/core/AGENTS.md
View File

@ -127,9 +127,9 @@ both**; a swallowed error is logged with a comment saying why.
- Mutexes named `mu` (or `<thing>Mu`); lock-then-defer-unlock. For
operations a caller may abandon use a context-aware mutex
(`syncutil.Mutex`, to be ported into `pkg/` before first use).
- One consumer: a `Bind*` callback. Registries return an unsubscriber
func; dispatch reads under `RLock` and fans out via the supervisor,
never under the write lock.
- A `Bind*` method sets the sole consumer; registries return an unsubscribe
func. Dispatch holds `RLock` and fans out via the supervisor, never the
write lock.
- Periodic work: `timeutil.NewPeriodicExec` or `timeutil.NewCron`, never
hand-rolled tickers. Shutdown via a once-closed `chan struct{}` exposed as
`ShutDownCh()`. Sleeps `select` on `ctx.Done()`, never bare `time.Sleep`.

5
server/core/internal/assistant/edit/client.go
View File

@ -30,10 +30,9 @@ type Result struct {
Errors []OpError `json:"errors"`
}
// Client posts to the Node hocuspocus service's internal per-branch
// Client calls the Node hocuspocus service's internal per-branch
// endpoints: batched operations and flushes. The base URL points at the
// Node service (e.g. http://auth-realtime:8081); endpoint replaces its
// path with the per-branch action path.
// Node service (e.g. http://auth-realtime:8081).
type Client struct {
// httpClient is the underlying HTTP client. Callers should
// pass one with a reasonable timeout configured.

22
server/core/internal/assistant/edit/client_test.go
View File

@ -242,11 +242,12 @@ func Test_Client_Flush(t *testing.T) {
path := "/api/internal/documents/" + docID.String() + "/branches/" + branchID.String() + "/flush"
cc := map[string]struct {
BaseURL string
CloseEarly bool
StatusCode int
Body string
Err error
BaseURL string
CloseEarly bool
StatusCode int
Body string
TruncateBody bool
Err error
}{
"Invalid base URL": {
BaseURL: "://bad",
@ -261,6 +262,11 @@ func Test_Client_Flush(t *testing.T) {
Body: `{"error":"pending changes could not be stored"}`,
Err: assert.AnError,
},
"Unreadable error body": {
StatusCode: http.StatusInternalServerError,
TruncateBody: true,
Err: assert.AnError,
},
"Successful flush": {
StatusCode: http.StatusNoContent,
},
@ -275,6 +281,12 @@ func Test_Client_Flush(t *testing.T) {
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
gotPath, gotMethod = r.URL.Path, r.Method
// declare a longer body than is written, so the client's
// read of the error body fails.
if c.TruncateBody {
w.Header().Set("Content-Length", "100")
}
w.WriteHeader(c.StatusCode)
_, err := w.Write([]byte(c.Body))
assert.NoError(t, err)

15
server/core/internal/assistant/manager.go
View File

@ -88,13 +88,14 @@ type Manager struct {
// backs context summarisation and may be the same model. The editClient
// is the edit pipe to the Node hocuspocus service; the search client
// backs the search_documents tool and searchTrigger runs the search-job
// worker once a document write has committed; providerName labels token metrics so
// usage stays readable across a provider change; githubMan and
// webchangeClient are the integrations the hook tools check for; hookMan
// runs the hook writes; the tree notifier broadcasts sidebar refresh events after
// document tree mutations and is wired post-construction via
// SetTreeNotifier because the document handler that satisfies it is
// built later, inside server.NewServer.
// worker once a document write has committed; providerName labels token
// metrics so usage stays readable across a provider change.
//
// githubMan and webchangeClient tell the hook tools which integrations
// exist; hookMan runs hook writes. The tree notifier broadcasts sidebar
// refresh events after document tree mutations and is wired
// post-construction via SetTreeNotifier because the document handler
// that satisfies it is built later, inside server.NewServer.
func NewManager(
log *slog.Logger,
db tools.DB,

11
server/core/internal/assistant/manager_test.go
View File

@ -17,6 +17,7 @@ import (
"github.com/oxynote/oxynote/server/core/internal/assistant/persist"
persistMock "github.com/oxynote/oxynote/server/core/internal/assistant/persist/_mock"
protocolMock "github.com/oxynote/oxynote/server/core/internal/assistant/protocol/_mock"
"github.com/oxynote/oxynote/server/core/internal/assistant/tools"
toolsMock "github.com/oxynote/oxynote/server/core/internal/assistant/tools/_mock"
"github.com/oxynote/oxynote/server/core/pkg/errutil"
"github.com/oxynote/oxynote/server/core/pkg/memkit"
@ -43,6 +44,12 @@ type stubSearchTrigger struct{}
func (*stubSearchTrigger) Trigger() {}
// stubHookManager satisfies the tools' HookManager. Its methods are never
// called.
type stubHookManager struct {
tools.HookManager
}
// stubChatModel builds a chat model that answers with the given
// messages in order. The last one repeats once the script runs out, so
// a test that under-specifies gets a terminating answer rather than a
@ -240,7 +247,8 @@ func Test_NewManager(t *testing.T) {
fc := metricutil.NewFactory("test", prometheus.NewRegistry())
gh := &github.Manager{}
wc := &webchange.Client{}
m := NewManager(discardLog(), nil, c.Pool, nil, nil, fc, nil, nil, &stubSearchTrigger{}, nil, gh, wc, nil, "claude")
hm := &stubHookManager{}
m := NewManager(discardLog(), nil, c.Pool, nil, nil, fc, nil, nil, &stubSearchTrigger{}, nil, gh, wc, hm, "claude")
require.NotNil(t, m)
assert.NotNil(t, m.log)
@ -252,6 +260,7 @@ func Test_NewManager(t *testing.T) {
assert.NotNil(t, m.turns.m)
assert.Same(t, gh, m.githubMan)
assert.Same(t, wc, m.webchangeClient)
assert.Same(t, hm, m.hookMan)
assert.Nil(t, m.tree)
assert.Nil(t, m.tags)

76
server/core/internal/assistant/tools/hook_test.go
View File

@ -31,49 +31,34 @@ func hookDeps(db *DBMock, man *HookManagerMock) *Deps {
return d
}
// stubHookManager runs every hook write the way the hook manager does,
// leaving out the database: a write reads a copy of the stored hook, which
// is stored when given and stubHook otherwise.
func stubHookManager(stored ...*hook.Hook) *HookManagerMock {
inp := func(organizationID string) *hook.Input {
return hook.NewInput(organizationID, unconfiguredGithub(), webchange.NewClient("", ""))
}
lookup := func() *hook.Hook {
if len(stored) > 0 {
hk := *stored[0]
return &hk
}
return stubHook()
}
// stubHookManager accepts every hook write. A write answers with
// stubHook changed as the call asks, and an update or reset stamps
// UpdatedAt so a test can tell the answer from the hook it handed in.
func stubHookManager() *HookManagerMock {
return &HookManagerMock{
CreateHookFunc: func(ctx context.Context, ci hook.CreateInput, documentID xid.ID, organizationID, _ string) (*hook.Hook, error) {
// the manager refuses a block the branch does not hold.
if ci.BlockID.Valid && ci.BlockID.String != _stubContentUID {
return nil, hook.ErrBlockNotFound
}
return hook.NewHook(ctx, ci, documentID, ci.BranchID, organizationID, inp(organizationID))
},
UpdateHookFunc: func(ctx context.Context, _, _ xid.ID, organizationID string, ui hook.UpdateInput, _ string) (*hook.Hook, error) {
hk := lookup()
if err := hk.ApplyUpdate(ctx, ui, inp(organizationID)); err != nil {
return nil, err
}
CreateHookFunc: func(_ context.Context, ci hook.CreateInput, documentID xid.ID, organizationID, _ string) (*hook.Hook, error) {
hk := stubHook()
hk.Type = ci.Type
hk.DocumentID = null.ValueFrom(documentID)
hk.BranchID = null.ValueFrom(ci.BranchID)
hk.OrganizationID = null.StringFrom(organizationID)
hk.BlockID = ci.BlockID
hk.Settings = ci.Settings
return hk, nil
},
DeleteHookFunc: func(ctx context.Context, _, _ xid.ID, organizationID, _ string) error {
return lookup().Delete(ctx, inp(organizationID))
UpdateHookFunc: func(_ context.Context, id, _ xid.ID, _ string, ui hook.UpdateInput, _ string) (*hook.Hook, error) {
hk := stubHook()
hk.ID = id
hk.Settings = ui.Settings
hk.UpdatedAt = null.TimeFrom(_stubScheduleTime)
return hk, nil
},
ResetHookFunc: func(ctx context.Context, _, _ xid.ID, organizationID string) (*hook.Hook, error) {
hk := lookup()
if err := hk.Reset(ctx, inp(organizationID)); err != nil {
return nil, err
}
ResetHookFunc: func(_ context.Context, id, _ xid.ID, _ string) (*hook.Hook, error) {
hk := stubHook()
hk.ID = id
hk.UpdatedAt = null.TimeFrom(_stubScheduleTime)
return hk, nil
},
@ -524,6 +509,11 @@ func Test_createHook_Execute(t *testing.T) {
return nil, assert.AnError
}
blockless := stubHookManager()
blockless.CreateHookFunc = func(context.Context, hook.CreateInput, xid.ID, string, string) (*hook.Hook, error) {
return nil, hook.ErrBlockNotFound
}
cc := map[string]struct {
DB *DBMock
Man *HookManagerMock
@ -550,6 +540,7 @@ func Test_createHook_Execute(t *testing.T) {
},
"Unknown block": {
DB: stubHookDB(),
Man: blockless,
Args: `{` + targetArgs(_stubBranchID) + `,` + scheduled + `,"block_uid":"nope"}`,
Err: fmt.Errorf("create_hook: %w", fmt.Errorf("block %s: %w", "nope", errUnknownBlock)),
},
@ -770,8 +761,6 @@ func Test_updateHook_Execute(t *testing.T) {
return
}
// the settings are replaced and the state reset: a fresh
// start rather than the old one carried over.
ff := d.hookMan.(*HookManagerMock).UpdateHookCalls()
require.Len(t, ff, 1)
assert.Equal(t, _testHookID, ff[0].ID)
@ -780,8 +769,7 @@ func Test_updateHook_Execute(t *testing.T) {
row := decodeHookRow(t, res)
assert.Equal(t, _testHookID, row.ID)
assert.JSONEq(t, string(ff[0].UI.Settings), string(row.Settings))
assert.NotContains(t, string(row.State), "2026-01-01")
assert.Equal(t, "100", row.Score.String())
assert.Equal(t, null.TimeFrom(_stubScheduleTime), row.UpdatedAt)
assert.Equal(t, []Touched{{DocumentID: _testDocID, BranchID: _stubBranchID}}, inp.touched)
})
@ -906,7 +894,6 @@ func Test_resetHook_Execute(t *testing.T) {
return
}
// the settings stay as they were; only the state starts over.
ff := d.hookMan.(*HookManagerMock).ResetHookCalls()
require.Len(t, ff, 1)
assert.Equal(t, _testHookID, ff[0].ID)
@ -914,8 +901,7 @@ func Test_resetHook_Execute(t *testing.T) {
row := decodeHookRow(t, res)
assert.Equal(t, _testHookID, row.ID)
assert.JSONEq(t, string(stubHook().Settings), string(row.Settings))
assert.NotContains(t, string(row.State), "2026-01-01")
assert.Equal(t, "100", row.Score.String())
assert.Equal(t, null.TimeFrom(_stubScheduleTime), row.UpdatedAt)
assert.Equal(t, []Touched{{DocumentID: _testDocID, BranchID: _stubBranchID}}, inp.touched)
})

18
server/core/internal/assistant/tools/input.go
View File

@ -66,13 +66,11 @@ type Deps struct {
// runners hands out the runner a data-source tool reads through.
runners DataSourceRunners
// githubMan is the GitHub App integration a github-tracking hook
// reads through; unconfigured on a deployment without the app.
// githubMan tells create_hook whether the GitHub App is configured.
githubMan *github.Manager
// webchangeClient is the changedetection.io integration a
// url-watcher hook holds its watcher in; unconfigured on a
// deployment without it.
// webchangeClient tells create_hook whether changedetection.io is
// configured.
webchangeClient *webchange.Client
// hookMan runs the hook writes.
@ -1042,8 +1040,8 @@ func (i *input) CreateHook(documentID, branchID xid.ID, blockUID string, tp hook
return nil, err
}
// a hook whose integration is missing is refused anyway. Naming the
// integration's own error tells the model what is missing.
// the manager would refuse this too, but the integration's own error
// tells the model what is missing.
switch tp {
case hook.TypeGithubTracking:
if !i.githubMan.Configured() {
@ -1119,14 +1117,12 @@ func (i *input) DeleteHook(hk *hook.Hook) error {
}
// docRef wraps the (documentID, branchID) pair the edit client needs to
// address a live Y.Doc. The branch is resolved to the document's
// default branch — multi-branch editing is out of scope for the
// assistant.
// address a live Y.Doc.
type docRef struct {
// DocumentID is the document's id.
DocumentID xid.ID
// BranchID is the id of the document's default branch.
// BranchID is the id of the addressed branch.
BranchID xid.ID
}

59
server/core/internal/assistant/tools/input_test.go
View File

@ -2409,6 +2409,11 @@ func Test_input_CreateHook(t *testing.T) {
return nil, assert.AnError
}
blockless := stubHookManager()
blockless.CreateHookFunc = func(context.Context, hook.CreateInput, xid.ID, string, string) (*hook.Hook, error) {
return nil, hook.ErrBlockNotFound
}
cc := map[string]struct {
DB *DBMock
Man *HookManagerMock
@ -2429,6 +2434,7 @@ func Test_input_CreateHook(t *testing.T) {
},
"Unknown block": {
DB: stubHookDB(),
Man: blockless,
Branch: _stubBranchID,
BlockUID: "nope",
Type: hook.TypeScheduledReminder,
@ -2450,14 +2456,6 @@ func Test_input_CreateHook(t *testing.T) {
Settings: processor.Settings(`{"url":"https://example.com"}`),
Err: fmt.Errorf("%s: %w", hook.TypeURLWatcher, webchange.ErrNotConfigured),
},
"Settings the processor refuses": {
DB: stubHookDB(),
Branch: _stubBranchID,
Type: hook.TypeScheduledReminder,
Settings: processor.Settings(`{"scale":"bogus"}`),
Inserts: 1,
Err: processor.ErrInvalidScaleType,
},
"Error returned by hookMan.CreateHook": {
DB: stubHookDB(),
Man: failing,
@ -2520,8 +2518,6 @@ func Test_input_CreateHook(t *testing.T) {
assert.Equal(t, null.StringFrom("org"), hk.OrganizationID)
assert.Equal(t, null.NewString(c.BlockUID, c.BlockUID != ""), hk.BlockID)
assert.Equal(t, c.Settings, hk.Settings)
assert.Equal(t, "100", hk.Score.String())
assert.True(t, hk.State.Valid)
})
}
}
@ -2542,12 +2538,6 @@ func Test_input_UpdateHook(t *testing.T) {
Touched []Touched
Err error
}{
"Settings the processor refuses": {
DB: stubHookDB(),
Settings: processor.Settings(`{"scale":"bogus"}`),
Updates: 1,
Err: processor.ErrInvalidScaleType,
},
"Error returned by hookMan.UpdateHook": {
DB: stubHookDB(),
Man: failing,
@ -2587,12 +2577,11 @@ func Test_input_UpdateHook(t *testing.T) {
return
}
// the hook handed in takes the stored one, with the settings
// replaced and a fresh state.
// the hook handed in takes the stored one.
assert.Equal(t, hk.ID, ff[0].ID)
assert.Equal(t, "org", ff[0].OrganizationID)
assert.Equal(t, c.Settings, ff[0].UI.Settings)
assert.Equal(t, c.Settings, hk.Settings)
assert.NotContains(t, string(hk.State.V), "2026-01-01")
assert.True(t, hk.UpdatedAt.Valid)
})
}
@ -2606,9 +2595,6 @@ func Test_input_ResetHook(t *testing.T) {
return nil, assert.AnError
}
invalid := stubHook()
invalid.Settings = processor.Settings(`{"scale":1}`)
cc := map[string]struct {
DB *DBMock
Man *HookManagerMock
@ -2617,13 +2603,6 @@ func Test_input_ResetHook(t *testing.T) {
Touched []Touched
Err error
}{
"Settings that do not decode": {
DB: stubHookDB(),
Man: stubHookManager(invalid),
Hook: invalid,
Updates: 1,
Err: assert.AnError,
},
"Error returned by hookMan.ResetHook": {
DB: stubHookDB(),
Man: failing,
@ -2657,10 +2636,10 @@ func Test_input_ResetHook(t *testing.T) {
return
}
// the settings stay; only the state starts over.
// the hook handed in takes the stored one.
assert.Equal(t, c.Hook.ID, ff[0].ID)
assert.Equal(t, stubHook().Settings, c.Hook.Settings)
assert.NotContains(t, string(c.Hook.State.V), "2026-01-01")
assert.Equal(t, "org", ff[0].OrganizationID)
assert.True(t, c.Hook.UpdatedAt.Valid)
})
}
}
@ -2673,13 +2652,6 @@ func Test_input_DeleteHook(t *testing.T) {
return assert.AnError
}
// a url watcher tears its changedetection.io watcher down from its
// state, and a state it cannot read stops the delete before the row.
watcher := stubHook()
watcher.Type = hook.TypeURLWatcher
watcher.Settings = processor.Settings(`{"url":"https://example.com"}`)
watcher.State = null.ValueFrom(processor.State(`{`))
cc := map[string]struct {
DB *DBMock
Man *HookManagerMock
@ -2688,13 +2660,6 @@ func Test_input_DeleteHook(t *testing.T) {
Touched []Touched
Err error
}{
"External teardown failed": {
DB: stubHookDB(),
Man: stubHookManager(watcher),
Hook: watcher,
Deletes: 1,
Err: assert.AnError,
},
"Error returned by hookMan.DeleteHook": {
DB: stubHookDB(),
Man: failing,
@ -2734,6 +2699,8 @@ func Test_input_DeleteHook(t *testing.T) {
}
assert.Equal(t, _testHookID, ff[0].ID)
assert.Equal(t, _testDocID, ff[0].DocumentID)
assert.Equal(t, "org", ff[0].OrganizationID)
})
}
}

11
server/core/internal/db/document_branch.go
View File

@ -345,8 +345,8 @@ func (a *agent) insertDocumentBranchHistoryEntry(ctx context.Context, tx *sqlx.T
"updated_at": entry.UpdatedAt,
}
// a system write has no author, and the entry still holds the
// edits of the one it has.
// a system write has no author. Keep the user whose edits the
// entry still holds.
if entry.LastUpdatedBy.Valid {
set["fk_last_updated_by"] = entry.LastUpdatedBy
}
@ -385,10 +385,9 @@ func (a *agent) insertDocumentBranchHistoryEntry(ctx context.Context, tx *sqlx.T
return a.trimDocumentBranchHistoryEntries(ctx, tx, entry.BranchID)
}
// fetchDocumentBranchHistoryHooks fetches every hook of the branch a
// history entry may list, soft-deleted ones included, in a stable order so
// equal hook sets compare equal. history.NewHooks picks the ones the
// entry's content holds.
// fetchDocumentBranchHistoryHooks fetches every hook of the branch,
// soft-deleted ones included. The fixed order keeps the entry checksum
// stable.
func (a *agent) fetchDocumentBranchHistoryHooks(ctx context.Context, q sqlx.QueryerContext, branchID xid.ID) ([]hook.Hook, error) {
sqlq, args := a.selectDocumentHook(a.builder.Select()).
Where(sq.Eq{"document_hooks.fk_branch_id": branchID}).

273
server/core/internal/db/document_branch_test.go
View File

@ -694,8 +694,8 @@ func insertHistoryEntry(t *testing.T, db *DB, entry history.Entry) {
}
// assertHistoryEntryEqual compares two entries, matching hook settings as
// JSON since jsonb normalises their spacing. The stored checksum must be
// the sum of what the expected entry records.
// JSON since jsonb normalises their spacing. The checksum is expected to
// be exp.Sum().
func assertHistoryEntryEqual(t *testing.T, exp, got history.Entry) {
t.Helper()
@ -789,48 +789,75 @@ func Test_agent_trimDocumentBranchHistoryEntries(t *testing.T) {
}
func Test_agent_DeleteExpiredDocumentBranchHistoryEntries(t *testing.T) {
t.Parallel()
type tcase struct {
Before time.Time
// Rows maps each branch to how many entries it holds afterwards.
Rows map[xid.ID]int
// Newest maps each branch to its newest entry afterwards.
Newest map[xid.ID]xid.ID
}
db := prepTempDB(t)
docs := prepDocuments(t, db, 2, nil)
doc, other := docs[0], docs[1]
cc := map[string]func(*testing.T, *DB) tcase{
"Nothing expired": func(t *testing.T, db *DB) tcase {
doc := prepDocuments(t, db, 1, nil)[0]
entries := prepHistoryEntries(t, db, doc, 4)
entries := prepHistoryEntries(t, db, doc, 4)
require.Equal(t, 4, countHistoryEntries(t, db, doc.BranchID))
return tcase{
Before: entries[0].CreatedAt.Add(-time.Second),
Rows: map[xid.ID]int{doc.BranchID: 4},
Newest: map[xid.ID]xid.ID{doc.BranchID: entries[3].ID},
}
},
"Expired entries go": func(t *testing.T, db *DB) tcase {
doc := prepDocuments(t, db, 1, nil)[0]
entries := prepHistoryEntries(t, db, doc, 4)
otherEntries := prepHistoryEntries(t, db, other, 2)
require.Equal(t, 2, countHistoryEntries(t, db, other.BranchID))
return tcase{
Before: entries[2].CreatedAt,
Rows: map[xid.ID]int{doc.BranchID: 2},
Newest: map[xid.ID]xid.ID{doc.BranchID: entries[3].ID},
}
},
"Newest entry of each branch stays": func(t *testing.T, db *DB) tcase {
docs := prepDocuments(t, db, 2, nil)
entries := prepHistoryEntries(t, db, docs[0], 4)
other := prepHistoryEntries(t, db, docs[1], 2)
// nothing is old enough yet.
require.NoError(t, db.DeleteExpiredDocumentBranchHistoryEntries(
context.Background(),
entries[0].CreatedAt.Add(-time.Second),
))
assert.Equal(t, 4, countHistoryEntries(t, db, doc.BranchID))
assert.Equal(t, 2, countHistoryEntries(t, db, other.BranchID))
return tcase{
Before: entries[3].CreatedAt.Add(time.Hour),
Rows: map[xid.ID]int{
docs[0].BranchID: 1,
docs[1].BranchID: 1,
},
Newest: map[xid.ID]xid.ID{
docs[0].BranchID: entries[3].ID,
docs[1].BranchID: other[1].ID,
},
}
},
}
// everything created before the newest entry goes.
require.NoError(t, db.DeleteExpiredDocumentBranchHistoryEntries(
context.Background(),
entries[3].CreatedAt,
))
assert.Equal(t, 1, countHistoryEntries(t, db, doc.BranchID))
for cn, cfn := range cc {
t.Run(cn, func(t *testing.T) {
t.Parallel()
// each branch keeps its newest entry even once it has expired.
require.NoError(t, db.DeleteExpiredDocumentBranchHistoryEntries(
context.Background(),
entries[3].CreatedAt.Add(time.Hour),
))
db := prepTempDB(t)
c := cfn(t, db)
var ids []xid.ID
require.NoError(t, db.DeleteExpiredDocumentBranchHistoryEntries(context.Background(), c.Before))
q, args := db.builder.Select("id").
From("document_branch_history_entries").
Where(sq.Eq{"fk_branch_id": []xid.ID{doc.BranchID, other.BranchID}}).
MustSql()
for branchID, rows := range c.Rows {
assert.Equal(t, rows, countHistoryEntries(t, db, branchID))
}
require.NoError(t, db.sql.Select(&ids, q, args...))
assert.ElementsMatch(t, []xid.ID{entries[3].ID, otherEntries[1].ID}, ids)
for branchID, id := range c.Newest {
newest, err := db.fetchNewestDocumentBranchHistoryEntry(context.Background(), db.sql, branchID)
require.NoError(t, err)
require.NotNil(t, newest)
assert.Equal(t, id, newest.ID)
}
})
}
}
func Test_agent_insertDocumentBranchHistoryEntry(t *testing.T) {
@ -949,6 +976,21 @@ func Test_agent_insertDocumentBranchHistoryEntry(t *testing.T) {
Newest: first,
}
},
"Equal entry in the same bucket keeps the newest untouched": func(t *testing.T, db *DB) tcase {
doc := prepDocuments(t, db, 1, nil)[0]
user := prepUsers(t, db, 1)[0]
first := history.NewEntry(*doc, base.Add(5*time.Minute), null.StringFrom(user), hooks, false)
insertHistoryEntry(t, db, first)
entry := history.NewEntry(*doc, base.Add(10*time.Minute), null.String{}, hooks, false)
return tcase{
Entry: entry,
Rows: 1,
Newest: first,
}
},
"Boundary entry equal to the newest one inserts": func(t *testing.T, db *DB) tcase {
doc := prepDocuments(t, db, 1, nil)[0]
@ -1034,34 +1076,6 @@ func Test_agent_insertDocumentBranchHistoryEntry(t *testing.T) {
}
}
// prepHistoryBranch returns a branch whose content holds block "b1" and
// the hooks fn describes on it, fetched back so they match what a record
// reads.
func prepHistoryBranch(t *testing.T, db *DB, count int, fn func(int, *hook.Hook)) (*document.Document, []hook.Hook) {
t.Helper()
branch := prepDocumentBranches(t, db, 1, func(_ int, doc *document.Document) {
doc.Content.Content[0].Attrs = document.Attributes{document.AttrUID: "b1"}
})[0]
hooks := prepDocumentHooks(t, db, count, func(i int, hk *hook.Hook) {
hk.DocumentID = null.ValueFrom(branch.ID)
hk.OrganizationID = null.StringFrom(branch.OrganizationID)
hk.BranchID = null.ValueFrom(branch.BranchID)
hk.BlockID = null.StringFrom("b1")
hk.CreatedAt = hk.CreatedAt.Add(time.Duration(i) * time.Second)
if fn != nil {
fn(i, hk)
}
})
doc, err := db.FetchDocumentUnsafeByBranchID(context.Background(), branch.BranchID)
require.NoError(t, err)
return doc, hooks
}
func Test_agent_RecordDocumentBranchHistoryEntry(t *testing.T) {
type tcase struct {
// Tx, when set, is the transaction the record runs in. It is
@ -1079,9 +1093,37 @@ func Test_agent_RecordDocumentBranchHistoryEntry(t *testing.T) {
Err error
}
// prepBranch returns a branch whose content holds block "b1" and the
// hooks fn describes on it, fetched back so they match what a record
// reads.
prepBranch := func(t *testing.T, db *DB, count int, fn func(int, *hook.Hook)) (*document.Document, []hook.Hook) {
t.Helper()
branch := prepDocumentBranches(t, db, 1, func(_ int, doc *document.Document) {
doc.Content.Content[0].Attrs = document.Attributes{document.AttrUID: "b1"}
})[0]
hooks := prepDocumentHooks(t, db, count, func(i int, hk *hook.Hook) {
hk.DocumentID = null.ValueFrom(branch.ID)
hk.OrganizationID = null.StringFrom(branch.OrganizationID)
hk.BranchID = null.ValueFrom(branch.BranchID)
hk.BlockID = null.StringFrom("b1")
hk.CreatedAt = hk.CreatedAt.Add(time.Duration(i) * time.Second)
if fn != nil {
fn(i, hk)
}
})
doc, err := db.FetchDocumentUnsafeByBranchID(context.Background(), branch.BranchID)
require.NoError(t, err)
return doc, hooks
}
cc := map[string]func(*testing.T, *DB) tcase{
"Branch of another organization": func(t *testing.T, db *DB) tcase {
doc, _ := prepHistoryBranch(t, db, 1, nil)
doc, _ := prepBranch(t, db, 1, nil)
return tcase{
BranchID: doc.BranchID,
@ -1090,13 +1132,14 @@ func Test_agent_RecordDocumentBranchHistoryEntry(t *testing.T) {
}
},
"Ordinary entry lists the hooks its content holds": func(t *testing.T, db *DB) tcase {
doc, hooks := prepHistoryBranch(t, db, 4, func(i int, hk *hook.Hook) {
doc, hooks := prepBranch(t, db, 4, func(i int, hk *hook.Hook) {
switch i {
case 1:
// the block is back; the sweep lifts the mark later.
// soft-deleted, but its block is in the content, so it
// is kept.
hk.SoftDeletedAt = null.TimeFrom(timeutil.Now())
case 2:
// the block is gone; the sweep marks it later.
// its block is not in the content, so it is left out.
hk.BlockID = null.StringFrom("removed")
case 3:
hk.BlockID = null.String{}
@ -1119,7 +1162,7 @@ func Test_agent_RecordDocumentBranchHistoryEntry(t *testing.T) {
}
},
"Boundary entry adds its own row": func(t *testing.T, db *DB) tcase {
doc, hooks := prepHistoryBranch(t, db, 1, nil)
doc, hooks := prepBranch(t, db, 1, nil)
insertHistoryEntry(t, db, history.NewEntry(*doc, timeutil.Now(), null.String{}, history.Hooks{}, false))
@ -1134,7 +1177,7 @@ func Test_agent_RecordDocumentBranchHistoryEntry(t *testing.T) {
// 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, hooks := prepHistoryBranch(t, db, 1, nil)
doc, hooks := prepBranch(t, db, 1, nil)
var tx *Tx
@ -1198,6 +1241,98 @@ func Test_agent_RecordDocumentBranchHistoryEntry(t *testing.T) {
}
}
func Test_agent_fetchDocumentBranchHistoryHooks(t *testing.T) {
type tcase struct {
Context context.Context
BranchID xid.ID
Result []hook.Hook
Err error
}
cc := map[string]func(*testing.T, *DB) tcase{
"Cancelled context": func(_ *testing.T, _ *DB) tcase {
ctx, cancel := context.WithCancel(context.Background())
cancel()
return tcase{
Context: ctx,
BranchID: xid.New(),
Err: assert.AnError,
}
},
"Branch without hooks": func(_ *testing.T, _ *DB) tcase {
return tcase{
Context: context.Background(),
BranchID: xid.New(),
Result: []hook.Hook{},
}
},
"Soft deleted hooks are included": func(t *testing.T, db *DB) tcase {
hooks := prepDocumentHooks(t, db, 2, func(i int, hk *hook.Hook) {
hk.CreatedAt = hk.CreatedAt.Add(time.Duration(i) * time.Second)
if i == 1 {
hk.SoftDeletedAt = null.TimeFrom(timeutil.Now().Truncate(time.Second))
}
})
return tcase{
Context: context.Background(),
BranchID: hooks[0].BranchID.V,
Result: hooks,
}
},
"Hooks of another branch are excluded": func(t *testing.T, db *DB) tcase {
hooks := prepDocumentHooks(t, db, 1, nil)
prepDocumentHooks(t, db, 1, nil)
return tcase{
Context: context.Background(),
BranchID: hooks[0].BranchID.V,
Result: hooks,
}
},
"Ordered by creation time then id": func(t *testing.T, db *DB) tcase {
ids := []xid.ID{xid.New(), xid.New()}
hooks := prepDocumentHooks(t, db, 3, func(i int, hk *hook.Hook) {
switch i {
case 0:
hk.CreatedAt = hk.CreatedAt.Add(time.Second)
case 1:
hk.ID = ids[1]
case 2:
hk.ID = ids[0]
}
})
return tcase{
Context: context.Background(),
BranchID: hooks[0].BranchID.V,
Result: []hook.Hook{hooks[2], hooks[1], hooks[0]},
}
},
}
for cn, cfn := range cc {
t.Run(cn, func(t *testing.T) {
t.Parallel()
db := prepTempDB(t)
c := cfn(t, db)
res, err := db.fetchDocumentBranchHistoryHooks(c.Context, db.sql, c.BranchID)
testutil.AssertEqualError(t, c.Err, err)
if err != nil {
return
}
testutil.AssertFilterEqual(t, c.Result, res)
})
}
}
func Test_agent_fetchNewestDocumentBranchHistoryEntry(t *testing.T) {
t.Parallel()

7
server/core/internal/db/document_file.go
View File

@ -139,10 +139,9 @@ func (a *agent) CheckDocumentFileReferenced(ctx context.Context, id string, docu
// DeleteExpiredDocumentBranchHistoryEntries removes history entries created
// before the given time, except the newest entry of each branch. Age
// trimming happens here rather than on insert because an idle branch never
// inserts again, and its entries would otherwise pin the files they
// reference forever. The newest entry pins nothing the live content does
// not.
// trimming runs here, not on insert, because an idle branch never inserts
// again. The newest entry is kept because it pins only files the live
// content holds.
func (a *agent) DeleteExpiredDocumentBranchHistoryEntries(ctx context.Context, before time.Time) error {
q, args := a.builder.Delete("document_branch_history_entries e").
Where(sq.Lt{"e.created_at": before}).

8
server/core/internal/document/AGENTS.md
View File

@ -28,8 +28,7 @@ reads it and its hooks, in the transaction that wrote the change.
- Edits in one 30-minute bucket (by `created_at`) update the newest
entry and its `updated_at`. An entry equal to the newest is skipped. A
null author (system write, no maintainers) never replaces a set one.
- An entry lists the hooks whose block its content holds.
null author (system write) never replaces a set one.
- Hook create, update and delete flush the branch, then record an entry
in their transaction. Reset does not.
- Create, duplicate, fork and merge write a **boundary** entry that never
@ -60,9 +59,8 @@ request handler:
later; NULL-FK rows skip the wait; files younger than a day are never
touched.
- `hook/manager` does the same for hooks, calling `Hook.Delete` before
dropping a row with a NULL FK. A merge detaches
the target's hooks (`DetachDocumentHooksByBranchID`) rather than
soft-deleting them.
dropping a row with a NULL FK. A merge detaches the target's hooks
(`DetachDocumentHooksByBranchID`) rather than soft-deleting them.
- **Organization deletion is announced**: auth-realtime's
`beforeDeleteOrganization` calls `POST /api/x/organizations/{id}/teardown`
while rows still exist and throws if core fails.

14
server/core/internal/document/history/history.go
View File

@ -54,8 +54,7 @@ type Entry struct {
// UpdatedAt is the timestamp of the last edit folded into the entry.
UpdatedAt time.Time `json:"updatedAt" db:"updated_at"`
// Checksum is the stored Sum of the entry. Empty until the entry is
// stored.
// Checksum is the Sum stored with the entry. Unset on a new entry.
Checksum string `json:"-" db:"checksum"`
}
@ -83,17 +82,12 @@ func NewEntry(doc document.Document, at time.Time, by null.String, hooks Hooks,
// Sum sums what the entry records: name, icon, content and hooks. Two
// entries with the same sum restore the same branch.
func (e Entry) Sum() (string, error) {
hooks := e.Hooks
if hooks == nil {
hooks = Hooks{}
}
raw, err := json.Marshal(struct {
DocumentName string `json:"documentName"`
Icon string `json:"icon"`
Content document.RootBlock `json:"content"`
Hooks Hooks `json:"hooks"`
}{e.DocumentName, e.Icon, e.Content, hooks})
}{e.DocumentName, e.Icon, e.Content, e.Hooks})
if err != nil {
return "", fmt.Errorf("marshaling history entry: %w", err)
}
@ -120,8 +114,8 @@ type Hook struct {
// Hooks is the list of hooks stored on a history entry.
type Hooks []Hook
// NewHooks reduces hooks to what a history entry of the document records:
// those whose block its content holds, soft-deleted or not.
// NewHooks keeps what a history entry of doc records: document-level hooks
// and those whose block doc's content holds, soft-deleted or not.
func NewHooks(doc document.Document, hooks []hook.Hook) Hooks {
res := make(Hooks, 0, len(hooks))

129
server/core/internal/document/history/history_test.go
View File

@ -1,7 +1,6 @@
package history
import (
"slices"
"testing"
"time"
@ -65,6 +64,8 @@ func Test_NewEntry(t *testing.T) {
}
func Test_Entry_Sum(t *testing.T) {
t.Parallel()
base := NewEntry(
document.Document{
ID: xid.New(),
@ -90,6 +91,9 @@ func Test_Entry_Sum(t *testing.T) {
false,
)
baseSum, err := base.Sum()
require.NoError(t, err)
cc := map[string]struct {
Edit func(e *Entry)
Same bool
@ -107,7 +111,7 @@ func Test_Entry_Sum(t *testing.T) {
"Other hooks": {
Edit: func(e *Entry) { e.Hooks = Hooks{} },
},
"Other id, author, times, boundary and stored checksum": {
"Fields outside the snapshot": {
Edit: func(e *Entry) {
e.ID = xid.New()
e.Checksum = "stale"
@ -141,36 +145,19 @@ func Test_Entry_Sum(t *testing.T) {
t.Parallel()
entry := base
entry.Content.Content = slices.Clone(base.Content.Content)
c.Edit(&entry)
sum, err := entry.Sum()
testutil.RequireEqualError(t, c.Err, err)
testutil.AssertEqualError(t, c.Err, err)
if err != nil {
return
}
baseSum, err := base.Sum()
require.NoError(t, err)
assert.Len(t, sum, 64)
assert.Equal(t, c.Same, sum == baseSum)
})
}
// nil hooks sum like no hooks, as both are stored as [].
empty := base
empty.Hooks = Hooks{}
emptySum, err := empty.Sum()
require.NoError(t, err)
empty.Hooks = nil
nilSum, err := empty.Sum()
require.NoError(t, err)
assert.Equal(t, emptySum, nilSum)
}
func Test_NewHooks(t *testing.T) {
@ -185,40 +172,84 @@ func Test_NewHooks(t *testing.T) {
},
}
assert.Equal(t, Hooks{}, NewHooks(doc, nil))
assert.Equal(t, Hooks{
{
Type: hook.TypeURLWatcher,
BlockID: null.StringFrom("b1"),
Settings: processor.Settings(`{"url":"https://example.com"}`),
cc := map[string]struct {
Hooks []hook.Hook
Result Hooks
}{
"Nil hooks": {
Result: Hooks{},
},
{
Type: hook.TypeScheduledReminder,
Settings: processor.Settings(`{"cron":"0 9 * * 1"}`),
"Document level hook is kept": {
Hooks: []hook.Hook{
{
ID: xid.New(),
Type: hook.TypeScheduledReminder,
Settings: processor.Settings(`{"cron":"0 9 * * 1"}`),
},
},
Result: Hooks{
{
Type: hook.TypeScheduledReminder,
Settings: processor.Settings(`{"cron":"0 9 * * 1"}`),
},
},
},
}, NewHooks(doc, []hook.Hook{
{
ID: xid.New(),
Type: hook.TypeURLWatcher,
BlockID: null.StringFrom("b1"),
Settings: processor.Settings(`{"url":"https://example.com"}`),
State: null.ValueFrom(processor.State(`{"watcher":"w1"}`)),
// the block is back, so the sweep lifts the mark on its next
// run.
SoftDeletedAt: null.TimeFrom(time.Now()),
"Hook whose block is present is kept": {
Hooks: []hook.Hook{
{
ID: xid.New(),
Type: hook.TypeURLWatcher,
BlockID: null.StringFrom("b1"),
Settings: processor.Settings(`{"url":"https://example.com"}`),
State: null.ValueFrom(processor.State(`{"watcher":"w1"}`)),
},
},
Result: Hooks{
{
Type: hook.TypeURLWatcher,
BlockID: null.StringFrom("b1"),
Settings: processor.Settings(`{"url":"https://example.com"}`),
},
},
},
{
ID: xid.New(),
Type: hook.TypeURLWatcher,
BlockID: null.StringFrom("removed"),
Settings: processor.Settings(`{"url":"https://example.org"}`),
"Soft deleted hook whose block is present is kept": {
Hooks: []hook.Hook{
{
ID: xid.New(),
Type: hook.TypeURLWatcher,
BlockID: null.StringFrom("b1"),
Settings: processor.Settings(`{"url":"https://example.com"}`),
SoftDeletedAt: null.TimeFrom(time.Now()),
},
},
Result: Hooks{
{
Type: hook.TypeURLWatcher,
BlockID: null.StringFrom("b1"),
Settings: processor.Settings(`{"url":"https://example.com"}`),
},
},
},
{
ID: xid.New(),
Type: hook.TypeScheduledReminder,
Settings: processor.Settings(`{"cron":"0 9 * * 1"}`),
"Hook whose block is absent is dropped": {
Hooks: []hook.Hook{
{
ID: xid.New(),
Type: hook.TypeURLWatcher,
BlockID: null.StringFrom("removed"),
Settings: processor.Settings(`{"url":"https://example.org"}`),
},
},
Result: Hooks{},
},
}))
}
for cn, c := range cc {
t.Run(cn, func(t *testing.T) {
t.Parallel()
assert.Equal(t, c.Result, NewHooks(doc, c.Hooks))
})
}
}
func Test_Hooks_Value(t *testing.T) {

10
server/core/internal/document/hook/hook.go
View File

@ -177,12 +177,10 @@ func NewHook(
}
// NewCopy returns a copy of the hook for another branch, anchored to the
// given block. It is initializing, with a null state, until its first run
// sets it up, since the source's state can name the source's own watcher.
// The score is copied, so that run does not announce a change the source
// already had.
// A copy within the hook's document keeps its cross-branch ID; a copy into
// another document is a hook of its own.
// given block. The copy is initializing, with no state, since the source's
// state may name the source's own watcher. It keeps the score, so its
// first run does not announce a change the source already had. A copy
// within the document keeps the cross-branch ID.
func (h *Hook) NewCopy(documentID, branchID xid.ID, blockID null.String) Hook {
id := xid.New()

384
server/core/internal/document/hook/hook_test.go
View File

@ -59,72 +59,75 @@ func Test_NewHook(t *testing.T) {
documentID, branchID := xid.New(), xid.New()
t.Run("Valid settings create a reset hook", func(t *testing.T) {
t.Parallel()
cc := map[string]struct {
Inp CreateInput
Input *Input
Err error
}{
"Malformed settings": {
Inp: CreateInput{
Type: TypeScheduledReminder,
Settings: processor.Settings(`{not json`),
},
Err: ErrInvalidSettings,
},
"Invalid settings": {
Inp: CreateInput{
Type: TypeScheduledReminder,
Settings: processor.Settings(`{"scale":"bogus","schedule":"2030-01-01T00:00:00Z"}`),
},
Err: processor.ErrInvalidScaleType,
},
"Unknown type": {
Inp: CreateInput{
Type: Type("bogus"),
Settings: processor.Settings(`{}`),
},
Err: ErrInvalidType,
},
"Hook that cannot check its target is refused": {
Inp: CreateInput{
Type: TypeURLWatcher,
Settings: processor.Settings(`{"url":"https://example.com"}`),
},
Input: NewInput("org-1", nil, webchange.NewClient("", "")),
Err: errutil.New(http.StatusUnprocessableEntity, "document_hook.unconfigured", "the integration the hook needs is not configured"),
},
"Successful creation": {
Inp: CreateInput{
Type: TypeScheduledReminder,
BlockID: null.StringFrom("block-1"),
Settings: reminderSettings(t, time.Now().Add(time.Hour)),
},
},
}
h, err := NewHook(context.Background(), CreateInput{
Type: TypeScheduledReminder,
BlockID: null.StringFrom("block-1"),
Settings: reminderSettings(t, time.Now().Add(time.Hour)),
}, documentID, branchID, "org-1", nil)
require.NoError(t, err)
for cn, c := range cc {
t.Run(cn, func(t *testing.T) {
t.Parallel()
assert.False(t, h.ID.IsZero())
assert.Equal(t, h.ID, h.CrossBranchID)
assert.Equal(t, TypeScheduledReminder, h.Type)
assert.Equal(t, null.ValueFrom(documentID), h.DocumentID)
assert.Equal(t, null.ValueFrom(branchID), h.BranchID)
assert.Equal(t, null.StringFrom("org-1"), h.OrganizationID)
assert.Equal(t, null.StringFrom("block-1"), h.BlockID)
assert.False(t, h.CreatedAt.IsZero())
h, err := NewHook(context.Background(), c.Inp, documentID, branchID, "org-1", c.Input)
testutil.AssertEqualError(t, c.Err, err)
// the reset scored the fresh future schedule at full.
assert.True(t, h.Score.Equal(decimal.NewFromInt(100)))
assert.True(t, h.State.Valid)
assert.Equal(t, processor.StatusActive, h.Status)
})
if err != nil {
return
}
t.Run("Malformed settings fail", func(t *testing.T) {
t.Parallel()
assert.False(t, h.ID.IsZero())
assert.Equal(t, h.ID, h.CrossBranchID)
assert.Equal(t, c.Inp.Type, h.Type)
assert.Equal(t, null.ValueFrom(documentID), h.DocumentID)
assert.Equal(t, null.ValueFrom(branchID), h.BranchID)
assert.Equal(t, null.StringFrom("org-1"), h.OrganizationID)
assert.Equal(t, c.Inp.BlockID, h.BlockID)
assert.False(t, h.CreatedAt.IsZero())
_, err := NewHook(context.Background(), CreateInput{
Type: TypeScheduledReminder,
Settings: processor.Settings(`{not json`),
}, documentID, branchID, "org-1", nil)
assert.Equal(t, ErrInvalidSettings, err)
})
t.Run("Invalid settings fail", func(t *testing.T) {
t.Parallel()
_, err := NewHook(context.Background(), CreateInput{
Type: TypeScheduledReminder,
Settings: processor.Settings(`{"scale":"bogus","schedule":"2030-01-01T00:00:00Z"}`),
}, documentID, branchID, "org-1", nil)
assert.Equal(t, processor.ErrInvalidScaleType, err)
})
t.Run("Unknown type fails", func(t *testing.T) {
t.Parallel()
_, err := NewHook(context.Background(), CreateInput{
Type: Type("bogus"),
Settings: processor.Settings(`{}`),
}, documentID, branchID, "org-1", nil)
assert.Equal(t, ErrInvalidType, err)
})
t.Run("A hook that cannot check its target is refused", func(t *testing.T) {
t.Parallel()
_, err := NewHook(context.Background(), CreateInput{
Type: TypeURLWatcher,
Settings: processor.Settings(`{"url":"https://example.com"}`),
}, documentID, branchID, "org-1", NewInput("org-1", nil, webchange.NewClient("", "")))
require.Error(t, err)
assert.Equal(t, http.StatusUnprocessableEntity, errutil.StatusCode(err, false))
assert.EqualError(t, err, "the integration the hook needs is not configured")
})
// the reset scored the fresh future schedule at full.
assert.True(t, h.Score.Equal(decimal.NewFromInt(100)))
assert.True(t, h.State.Valid)
assert.Equal(t, processor.StatusActive, h.Status)
})
}
}
func Test_Hook_NewCopy(t *testing.T) {
@ -194,113 +197,166 @@ func Test_Hook_NewCopy(t *testing.T) {
func Test_Hook_ApplyUpdate(t *testing.T) {
t.Parallel()
h, err := NewHook(context.Background(), CreateInput{
Type: TypeScheduledReminder,
Settings: reminderSettings(t, time.Now().Add(time.Hour)),
}, xid.New(), xid.New(), "org-1", nil)
require.NoError(t, err)
cc := map[string]struct {
Settings processor.Settings
Err error
}{
"Malformed settings": {
Settings: processor.Settings(`{not json`),
Err: ErrInvalidSettings,
},
// an already elapsed schedule resets the score straight to zero.
"Successful update": {
Settings: reminderSettings(t, time.Now().Add(time.Second)),
},
}
// an already-elapsed schedule resets the score straight to zero.
newSettings := reminderSettings(t, time.Now().Add(time.Second))
for cn, c := range cc {
t.Run(cn, func(t *testing.T) {
t.Parallel()
require.NoError(t, h.ApplyUpdate(context.Background(), UpdateInput{Settings: newSettings}, nil))
h, err := NewHook(context.Background(), CreateInput{
Type: TypeScheduledReminder,
Settings: reminderSettings(t, time.Now().Add(time.Hour)),
}, xid.New(), xid.New(), "org-1", nil)
require.NoError(t, err)
assert.Equal(t, newSettings, h.Settings)
assert.True(t, h.UpdatedAt.Valid)
assert.True(t, h.Score.Equal(decimal.Zero))
err = h.ApplyUpdate(context.Background(), UpdateInput{Settings: c.Settings}, nil)
testutil.AssertEqualError(t, c.Err, err)
assert.Equal(t, ErrInvalidSettings, h.ApplyUpdate(context.Background(), UpdateInput{
Settings: processor.Settings(`{not json`),
}, nil))
if err != nil {
return
}
assert.Equal(t, c.Settings, h.Settings)
assert.True(t, h.UpdatedAt.Valid)
assert.True(t, h.Score.Equal(decimal.Zero))
})
}
}
func Test_Hook_Process(t *testing.T) {
t.Parallel()
t.Run("Elapsed schedule scores zero", func(t *testing.T) {
t.Parallel()
// the state started two hours ago, so the schedule has elapsed.
elapsed, err := json.Marshal(processor.ScheduledReminderState{
StartedAt: time.Now().Add(-2 * time.Hour),
})
require.NoError(t, err)
h, err := NewHook(context.Background(), CreateInput{
Type: TypeScheduledReminder,
Settings: reminderSettings(t, time.Now().Add(-time.Hour)),
}, xid.New(), xid.New(), "org-1", nil)
require.NoError(t, err)
cc := map[string]struct {
Hook Hook
Input *Input
Err error
Check func(*testing.T, Hook)
}{
"Malformed settings": {
Hook: Hook{
Type: TypeScheduledReminder,
Settings: processor.Settings(`{not json`),
State: null.ValueFrom(processor.State(`{}`)),
},
Err: assert.AnError,
},
"Elapsed schedule scores zero": {
Hook: Hook{
Type: TypeScheduledReminder,
Settings: reminderSettings(t, time.Now().Add(-time.Hour)),
State: null.ValueFrom(processor.State(elapsed)),
Score: decimal.NewFromInt(100),
Status: processor.StatusActive,
},
Check: func(t *testing.T, h Hook) {
assert.Equal(t, processor.StatusActive, h.Status)
assert.True(t, h.Score.Equal(decimal.Zero))
},
},
"Hook never set up is reset": {
Hook: Hook{
Type: TypeScheduledReminder,
Settings: reminderSettings(t, time.Now().Add(time.Hour)),
Status: processor.StatusActive,
},
Check: func(t *testing.T, h Hook) {
assert.True(t, h.State.Valid)
assert.True(t, h.Score.Equal(decimal.NewFromInt(100)))
},
},
"Run that cannot check keeps score and state": {
Hook: Hook{
Type: TypeURLWatcher,
Settings: processor.Settings(`{"url":"https://example.com"}`),
State: null.ValueFrom(processor.State(`{"watcherId":"w1"}`)),
Score: decimal.NewFromInt(40),
Status: processor.StatusActive,
},
Input: NewInput("org-1", nil, webchange.NewClient("", "")),
Check: func(t *testing.T, h Hook) {
assert.Equal(t, processor.StatusUnconfigured, h.Status)
assert.True(t, h.Score.Equal(decimal.NewFromInt(40)))
assert.Equal(t, processor.State(`{"watcherId":"w1"}`), h.State.V)
},
},
}
// backdate the started-at state so the schedule has elapsed.
state, merr := json.Marshal(processor.ScheduledReminderState{
StartedAt: time.Now().Add(-2 * time.Hour),
for cn, c := range cc {
t.Run(cn, func(t *testing.T) {
t.Parallel()
h := c.Hook
err := h.Process(context.Background(), c.Input)
testutil.AssertEqualError(t, c.Err, err)
if err != nil {
return
}
c.Check(t, h)
})
require.NoError(t, merr)
h.State = null.ValueFrom(processor.State(state))
require.NoError(t, h.Process(context.Background(), nil))
assert.True(t, h.Score.Equal(decimal.Zero))
})
t.Run("A hook never set up is reset", func(t *testing.T) {
t.Parallel()
h := &Hook{
Type: TypeScheduledReminder,
Settings: reminderSettings(t, time.Now().Add(time.Hour)),
Status: processor.StatusActive,
}
require.NoError(t, h.Process(context.Background(), nil))
assert.True(t, h.State.Valid)
assert.True(t, h.Score.Equal(decimal.NewFromInt(100)))
})
t.Run("A run that cannot check keeps score and state", func(t *testing.T) {
t.Parallel()
h := &Hook{
Type: TypeURLWatcher,
Settings: processor.Settings(`{"url":"https://example.com"}`),
State: null.ValueFrom(processor.State(`{"watcherId":"w1"}`)),
Score: decimal.NewFromInt(40),
Status: processor.StatusActive,
}
require.NoError(t, h.Process(context.Background(), NewInput("org-1", nil, webchange.NewClient("", ""))))
assert.Equal(t, processor.StatusUnconfigured, h.Status)
assert.True(t, h.Score.Equal(decimal.NewFromInt(40)))
assert.Equal(t, processor.State(`{"watcherId":"w1"}`), h.State.V)
})
t.Run("Malformed settings fail", func(t *testing.T) {
t.Parallel()
h := &Hook{
Type: TypeScheduledReminder,
Settings: processor.Settings(`{not json`),
State: null.ValueFrom(processor.State(`{}`)),
}
require.Error(t, h.Process(context.Background(), nil))
})
}
}
func Test_Hook_Delete(t *testing.T) {
t.Parallel()
h, err := NewHook(context.Background(), CreateInput{
Type: TypeScheduledReminder,
Settings: reminderSettings(t, time.Now().Add(time.Hour)),
}, xid.New(), xid.New(), "org-1", nil)
require.NoError(t, err)
// scheduled reminders have no external resources; delete is a no-op.
assert.NoError(t, h.Delete(context.Background(), nil))
// a url watcher never set up has no watcher yet, so its teardown
// reaches nothing outside.
unset := Hook{
Type: TypeURLWatcher,
Settings: processor.Settings(`{"url":"https://example.com"}`),
cc := map[string]struct {
Hook Hook
Err error
}{
"Malformed settings": {
Hook: Hook{
Type: TypeScheduledReminder,
Settings: processor.Settings(`{not json`),
State: null.ValueFrom(processor.State(`{}`)),
},
Err: assert.AnError,
},
// a url watcher never set up has no watcher yet, so its teardown
// reaches nothing outside.
"Hook never set up holds nothing": {
Hook: Hook{
Type: TypeURLWatcher,
Settings: processor.Settings(`{"url":"https://example.com"}`),
},
},
// scheduled reminders have no external resources.
"Hook without external resources": {
Hook: Hook{
Type: TypeScheduledReminder,
Settings: reminderSettings(t, time.Now().Add(time.Hour)),
State: null.ValueFrom(processor.State(`{}`)),
},
},
}
for cn, c := range cc {
t.Run(cn, func(t *testing.T) {
t.Parallel()
testutil.AssertEqualError(t, c.Err, c.Hook.Delete(context.Background(), nil))
})
}
assert.NoError(t, unset.Delete(context.Background(), nil))
}
func Test_Hook_ChangedFrom(t *testing.T) {
@ -358,9 +414,9 @@ func Test_Hook_ensurePrepared(t *testing.T) {
t.Parallel()
cc := map[string]struct {
Type Type
Settings processor.Settings
ExpectErr bool
Type Type
Settings processor.Settings
Err error
}{
"Scheduled reminder settings prepare": {
Type: TypeScheduledReminder,
@ -379,14 +435,14 @@ func Test_Hook_ensurePrepared(t *testing.T) {
Settings: processor.Settings(`{"image": "nginx:latest"}`),
},
"Malformed settings fail": {
Type: TypeScheduledReminder,
Settings: processor.Settings(`{not json`),
ExpectErr: true,
Type: TypeScheduledReminder,
Settings: processor.Settings(`{not json`),
Err: assert.AnError,
},
"Unknown type fails": {
Type: Type("bogus"),
Settings: processor.Settings(`{}`),
ExpectErr: true,
Type: Type("bogus"),
Settings: processor.Settings(`{}`),
Err: assert.AnError,
},
}
@ -397,16 +453,14 @@ func Test_Hook_ensurePrepared(t *testing.T) {
h := &Hook{Type: c.Type, Settings: c.Settings}
err := h.ensurePrepared()
testutil.AssertEqualError(t, c.Err, err)
assert.Equal(t, err == nil, h.prepared)
if c.ExpectErr {
require.Error(t, err)
assert.False(t, h.prepared)
if err != nil {
return
}
require.NoError(t, err)
assert.True(t, h.prepared)
assert.NotNil(t, h.runner)
assert.NotNil(t, h.runner)
// preparation is memoized.

13
server/core/internal/document/hook/manager/copy.go
View File

@ -2,15 +2,16 @@ package manager
import (
"context"
"fmt"
"github.com/guregu/null/v5"
"github.com/rs/xid"
)
// CopyHooks copies the branch's hooks to another in the caller's
// transaction, not set up yet. newBlockIDs maps the source's block ids to
// the copy's, for a copy whose blocks were given new ids. When set, it
// re-anchors block hooks and drops those whose block it does not name.
// CopyHooks inserts copies of the branch's hooks into another branch, in
// the caller's transaction. The copies are not set up. When newBlockIDs is
// set, block hooks follow it to their new block and those it does not name
// are dropped.
func CopyHooks(
ctx context.Context,
tx CopyTx,
@ -20,7 +21,7 @@ func CopyHooks(
) error {
hooks, err := tx.FetchDocumentHooksByBranchID(ctx, fromBranchID, organizationID)
if err != nil {
return err
return fmt.Errorf("fetching branch hooks: %w", err)
}
for _, hk := range hooks {
@ -36,7 +37,7 @@ func CopyHooks(
}
if err := tx.InsertDocumentHook(ctx, hk.NewCopy(documentID, toBranchID, blockID)); err != nil {
return err
return fmt.Errorf("inserting hook copy: %w", err)
}
}

4
server/core/internal/document/hook/manager/copy_test.go
View File

@ -26,11 +26,11 @@ func Test_CopyHooks(t *testing.T) {
Blocks []null.String
Err error
}{
"Error returned by tx.FetchDocumentHooksByBranchID": {
"Error returned by CopyTx.FetchDocumentHooksByBranchID": {
FetchErr: assert.AnError,
Err: assert.AnError,
},
"Error returned by tx.InsertDocumentHook": {
"Error returned by CopyTx.InsertDocumentHook": {
InsertErr: assert.AnError,
Blocks: []null.String{null.StringFrom("old-uid")},
Err: assert.AnError,

18
server/core/internal/document/hook/manager/manager.go
View File

@ -110,7 +110,7 @@ func (m *Manager) SetUpBranch(ctx context.Context, branchID xid.ID, organization
if err != nil {
m.log.With("branch_id", branchID).
With("error", err).
Error("fetching branch hooks")
Error("cannot fetch branch hooks")
return
}
@ -170,23 +170,23 @@ func (m *Manager) processHook(ctx context.Context, ps *ProcessingState, h hook.H
if err != nil {
m.log.With("hook_id", h.ID).
With("error", err).
Error("waiting for the hook's lock")
Error("cannot lock hook")
return
}
defer unlock()
// the row may have changed between the page read and the lock. A hook
// cut loose from its organization is out of any write's reach, so its
// row is as it was read.
// re-read the row, since a write may have changed it before the lock.
// A hook without an organization cannot be written, so the row read
// stands.
if h.OrganizationID.Valid {
stored, err := m.db.FetchDocumentHook(ctx, h.ID, h.OrganizationID.String)
if err != nil {
if !errutil.IsNotFound(err) {
m.log.With("hook_id", h.ID).
With("error", err).
Error("fetching document hook")
Error("cannot fetch document hook")
}
return
@ -358,9 +358,9 @@ func (m *Manager) notifyTransition(ctx context.Context, prev, h hook.Hook) {
m.notifPub.PublishNotifications(h.OrganizationID.String, core, maintainers...)
}
// undoSetup tears down the resource h got when it was set up, since its
// row was not stored. Nothing else points at the resource, so a failure
// is reported.
// undoSetup tears down the resource h got in this run, since the row
// naming it was not stored. Nothing else names it, so a failure is
// reported.
func (m *Manager) undoSetup(ctx context.Context, prev, h hook.Hook) {
if prev.State.Valid || !h.State.Valid {
return

22
server/core/internal/document/hook/manager/manager_test.go
View File

@ -317,9 +317,9 @@ func Test_Manager_processHooks(t *testing.T) {
}
}
wasNotified := func(count int) check {
return func(t *testing.T, _ *DBMock, notifier *changeRecorder, _ *fakePublisher, _ error) {
require.Len(t, notifier.hooks, count)
wasChangeCalled := func(count int) check {
return func(t *testing.T, _ *DBMock, changes *changeRecorder, _ *fakePublisher, _ error) {
require.Len(t, changes.hooks, count)
}
}
@ -384,7 +384,7 @@ func Test_Manager_processHooks(t *testing.T) {
Write func(hook.Hook) hook.Hook
Checks []check
}{
"Hook fetch failure is propagated": {
"Error returned by db.FetchPaginatedDocumentHooks": {
Hooks: func(*testing.T) []hook.Hook { return nil },
FetchErr: assert.AnError,
Checks: checks(
@ -442,7 +442,7 @@ func Test_Manager_processHooks(t *testing.T) {
wasDeleteCalled(1),
wasFetchDocumentCalled(0),
wasUpdateCalled(0),
wasNotified(1),
wasChangeCalled(1),
),
},
"Orphaned hook without a document is deleted": {
@ -640,7 +640,7 @@ func Test_Manager_processHooks(t *testing.T) {
wasUpdateCalled(1),
hasUpdatedScore(decimal.NewFromInt(100)),
wasPublished(),
wasNotified(0),
wasChangeCalled(0),
),
},
"Full-to-zero score drop notifies the maintainers": {
@ -653,7 +653,7 @@ func Test_Manager_processHooks(t *testing.T) {
wasUpdateCalled(1),
hasUpdatedScore(decimal.Zero),
wasPublished(notification.NotificationDocumentHookTriggered),
wasNotified(1),
wasChangeCalled(1),
),
},
// the score decays gradually, so by the time it reaches zero the
@ -717,7 +717,7 @@ func Test_Manager_processHooks(t *testing.T) {
hasUpdatedStatus(processor.StatusActive),
hasUpdatedScore(decimal.Zero),
wasPublished(notification.NotificationDocumentHookTriggered),
wasNotified(1),
wasChangeCalled(1),
),
},
"Maintainer fetch failure suppresses the notification": {
@ -852,10 +852,10 @@ func Test_Manager_processHooks(t *testing.T) {
}
pub := &fakePublisher{}
notifier := &changeRecorder{}
changes := &changeRecorder{}
man := newTestManager(t, db, pub, wc)
man.BindHookChange(notifier.record)
man.BindHookChange(changes.record)
var err error
@ -866,7 +866,7 @@ func Test_Manager_processHooks(t *testing.T) {
}
for _, ch := range c.Checks {
ch(t, db, notifier, pub, err)
ch(t, db, changes, pub, err)
}
if cd != nil {

129
server/core/internal/document/hook/manager/write_test.go
View File

@ -70,6 +70,7 @@ func Test_Manager_CreateHook(t *testing.T) {
Tx *TxMock
Inserts int
Commits int
Rollbacks int
Watchers []string
Err error
}{
@ -138,9 +139,10 @@ func Test_Manager_CreateHook(t *testing.T) {
return assert.AnError
},
},
Inserts: 1,
Watchers: []string{"w-new"},
Err: assert.AnError,
Inserts: 1,
Watchers: []string{"w-new"},
Rollbacks: 1,
Err: assert.AnError,
},
"Error returned by tx.Commit": {
Type: hook.TypeURLWatcher,
@ -151,10 +153,11 @@ func Test_Manager_CreateHook(t *testing.T) {
return assert.AnError
},
},
Inserts: 1,
Commits: 1,
Watchers: []string{"w-new"},
Err: assert.AnError,
Inserts: 1,
Commits: 1,
Watchers: []string{"w-new"},
Rollbacks: 1,
Err: assert.AnError,
},
// the hook was not stored, so the watcher it created is torn down.
"Error returned by tx.InsertDocumentHook": {
@ -166,18 +169,20 @@ func Test_Manager_CreateHook(t *testing.T) {
return assert.AnError
},
},
Inserts: 1,
Watchers: []string{"w-new"},
Err: assert.AnError,
Inserts: 1,
Watchers: []string{"w-new"},
Rollbacks: 1,
Err: assert.AnError,
},
"Successful creation": {
Type: hook.TypeURLWatcher,
BlockID: null.StringFrom("b1"),
Settings: processor.Settings(`{"url":"https://example.com"}`),
CD: true,
Tx: &TxMock{},
Inserts: 1,
Commits: 1,
Type: hook.TypeURLWatcher,
BlockID: null.StringFrom("b1"),
Settings: processor.Settings(`{"url":"https://example.com"}`),
CD: true,
Tx: &TxMock{},
Inserts: 1,
Commits: 1,
Rollbacks: 1,
},
}
@ -233,6 +238,7 @@ func Test_Manager_CreateHook(t *testing.T) {
require.Len(t, c.Tx.InsertDocumentHookCalls(), c.Inserts)
assert.Len(t, c.Tx.CommitCalls(), c.Commits)
assert.Len(t, c.Tx.RollbackCalls(), c.Rollbacks)
if cd != nil {
assert.Equal(t, c.Watchers, cd.deletedWatchers())
@ -302,6 +308,7 @@ func Test_Manager_UpdateHook(t *testing.T) {
CommitErr error
Updates int
Records int
Rollbacks int
Watchers []string
Err error
}{
@ -345,6 +352,7 @@ func Test_Manager_UpdateHook(t *testing.T) {
UpdateErr: assert.AnError,
Updates: 1,
Watchers: []string{"w-new"},
Rollbacks: 1,
Err: assert.AnError,
},
"Error returned by tx.RecordDocumentBranchHistoryEntry tears down the setup": {
@ -353,6 +361,7 @@ func Test_Manager_UpdateHook(t *testing.T) {
Updates: 1,
Records: 1,
Watchers: []string{"w-new"},
Rollbacks: 1,
Err: assert.AnError,
},
"Error returned by tx.Commit tears down the setup": {
@ -361,17 +370,20 @@ func Test_Manager_UpdateHook(t *testing.T) {
Updates: 1,
Records: 1,
Watchers: []string{"w-new"},
Rollbacks: 1,
Err: assert.AnError,
},
"Hook without a branch has no history": {
Settings: settings,
Branchless: true,
Updates: 1,
Rollbacks: 1,
},
"Successful update": {
Settings: settings,
Updates: 1,
Records: 1,
Settings: settings,
Updates: 1,
Records: 1,
Rollbacks: 1,
},
}
@ -438,6 +450,7 @@ func Test_Manager_UpdateHook(t *testing.T) {
testutil.AssertEqualError(t, c.Err, err)
assert.Len(t, tx.UpdateDocumentHookCalls(), c.Updates)
assert.Len(t, tx.RollbackCalls(), c.Rollbacks)
assert.Equal(t, c.Watchers, cd.deletedWatchers())
rr := tx.RecordDocumentBranchHistoryEntryCalls()
@ -478,6 +491,9 @@ func Test_Manager_DeleteHook(t *testing.T) {
branchID := xid.New()
cc := map[string]struct {
// Held has another write hold the hook's lock until the delete
// gives up.
Held bool
Hook func(*testing.T) hook.Hook
OtherDocument bool
// CDURL points changedetection at this address.
@ -490,8 +506,16 @@ func Test_Manager_DeleteHook(t *testing.T) {
CommitErr error
Deletes int
Records int
Rollbacks int
Err error
}{
"Hook another write holds": {
Held: true,
Hook: func(t *testing.T) hook.Hook {
return stubHook(t, branchID, time.Now().Add(time.Hour), time.Now())
},
Err: context.DeadlineExceeded,
},
"Error returned by db.FetchDocumentHook": {
Hook: func(t *testing.T) hook.Hook {
return stubHook(t, branchID, time.Now().Add(time.Hour), time.Now())
@ -543,9 +567,10 @@ func Test_Manager_DeleteHook(t *testing.T) {
Hook: func(t *testing.T) hook.Hook {
return stubHook(t, branchID, time.Now().Add(time.Hour), time.Now())
},
DelErr: assert.AnError,
Deletes: 1,
Err: assert.AnError,
DelErr: assert.AnError,
Deletes: 1,
Rollbacks: 1,
Err: assert.AnError,
},
"Error returned by tx.RecordDocumentBranchHistoryEntry": {
Hook: func(t *testing.T) hook.Hook {
@ -554,6 +579,7 @@ func Test_Manager_DeleteHook(t *testing.T) {
RecordErr: assert.AnError,
Deletes: 1,
Records: 1,
Rollbacks: 1,
Err: assert.AnError,
},
"Error returned by tx.Commit": {
@ -563,6 +589,7 @@ func Test_Manager_DeleteHook(t *testing.T) {
CommitErr: assert.AnError,
Deletes: 1,
Records: 1,
Rollbacks: 1,
Err: assert.AnError,
},
"Hook never set up is deleted": {
@ -572,8 +599,9 @@ func Test_Manager_DeleteHook(t *testing.T) {
return hk
},
Deletes: 1,
Records: 1,
Deletes: 1,
Records: 1,
Rollbacks: 1,
},
"Hook without a branch has no history": {
Hook: func(t *testing.T) hook.Hook {
@ -582,14 +610,16 @@ func Test_Manager_DeleteHook(t *testing.T) {
return hk
},
Deletes: 1,
Deletes: 1,
Rollbacks: 1,
},
"Successful deletion": {
Hook: func(t *testing.T) hook.Hook {
return stubHook(t, branchID, time.Now().Add(time.Hour), time.Now())
},
Deletes: 1,
Records: 1,
Deletes: 1,
Records: 1,
Rollbacks: 1,
},
}
@ -630,12 +660,26 @@ func Test_Manager_DeleteHook(t *testing.T) {
changes := &changeRecorder{}
man.BindHookChange(changes.record)
ctx := context.Background()
if c.Held {
unlock, err := man.hookMu.Lock(ctx, hk.ID)
require.NoError(t, err)
defer unlock()
var cancel context.CancelFunc
ctx, cancel = context.WithTimeout(ctx, 50*time.Millisecond)
defer cancel()
}
documentID := hk.DocumentID.V
if c.OtherDocument {
documentID = xid.New()
}
err := man.DeleteHook(context.Background(), hk.ID, documentID, "org-1", "u1")
err := man.DeleteHook(ctx, hk.ID, documentID, "org-1", "u1")
testutil.AssertEqualError(t, c.Err, err)
// only a service the hook could not reach is reported as one.
@ -646,6 +690,7 @@ func Test_Manager_DeleteHook(t *testing.T) {
ff := tx.DeleteDocumentHookCalls()
require.Len(t, ff, c.Deletes)
assert.Len(t, tx.RecordDocumentBranchHistoryEntryCalls(), c.Records)
assert.Len(t, tx.RollbackCalls(), c.Rollbacks)
if err != nil {
assert.Empty(t, changes.hooks)
@ -671,6 +716,9 @@ func Test_Manager_ResetHook(t *testing.T) {
branchID := xid.New()
cc := map[string]struct {
// Held has another write hold the hook's lock until the reset
// gives up.
Held bool
Hook func(*testing.T) hook.Hook
OtherDocument bool
CD bool
@ -683,6 +731,13 @@ func Test_Manager_ResetHook(t *testing.T) {
Watchers []string
Err error
}{
"Hook another write holds": {
Held: true,
Hook: func(t *testing.T) hook.Hook {
return stubHook(t, branchID, time.Now().Add(time.Hour), time.Now())
},
Err: context.DeadlineExceeded,
},
"Error returned by db.FetchDocumentHook": {
Hook: func(t *testing.T) hook.Hook {
return stubHook(t, branchID, time.Now().Add(time.Hour), time.Now())
@ -774,12 +829,26 @@ func Test_Manager_ResetHook(t *testing.T) {
changes := &changeRecorder{}
man.BindHookChange(changes.record)
ctx := context.Background()
if c.Held {
unlock, err := man.hookMu.Lock(ctx, stored.ID)
require.NoError(t, err)
defer unlock()
var cancel context.CancelFunc
ctx, cancel = context.WithTimeout(ctx, 50*time.Millisecond)
defer cancel()
}
documentID := stored.DocumentID.V
if c.OtherDocument {
documentID = xid.New()
}
hk, err := man.ResetHook(context.Background(), stored.ID, documentID, "org-1")
hk, err := man.ResetHook(ctx, stored.ID, documentID, "org-1")
testutil.AssertEqualError(t, c.Err, err)
ff := db.UpdateDocumentHookCalls()

232
server/core/internal/document/hook/processor/container_image_watcher_test.go
View File

@ -74,134 +74,148 @@ func imageWatcherState(t *testing.T, digest string) State {
func Test_ContainerImageWatcher_Validate(t *testing.T) {
t.Parallel()
assert.NoError(t, (&ContainerImageWatcher{Image: "nginx:1.27"}).Validate())
testutil.AssertEqualError(t, ErrInvalidImage, (&ContainerImageWatcher{Image: "INVALID image ref"}).Validate())
cc := map[string]struct {
Image string
Err error
}{
"Valid image reference": {Image: "nginx:1.27"},
"Invalid image is rejected": {Image: "INVALID image ref", Err: ErrInvalidImage},
}
for cn, c := range cc {
t.Run(cn, func(t *testing.T) {
t.Parallel()
testutil.AssertEqualError(t, c.Err, (&ContainerImageWatcher{Image: c.Image}).Validate())
})
}
}
func Test_ContainerImageWatcher_Process(t *testing.T) {
t.Parallel()
t.Run("Unchanged digest keeps the full score", func(t *testing.T) {
t.Parallel()
cc := map[string]struct {
// Image is watched instead of the fake registry's image.
Image string
Unauthorized bool
// Missing watches a tag the fake registry does not hold.
Missing bool
// Stale has the state hold an old digest instead of the current one.
Stale bool
// State is sent instead of a state built from the digest.
State State
Status Status
Score decimal.Decimal
Err error
}{
"Malformed state": {
Image: "INVALID image ref",
State: State(`{not json`),
Err: assert.AnError,
},
"Invalid image reference": {
Image: "INVALID image ref",
Err: assert.AnError,
},
"Unauthorized registry is a status": {
Unauthorized: true,
Status: StatusUnauthorized,
},
"Missing image is a status": {
Missing: true,
Status: StatusImageNotFound,
},
"Changed digest drops the score to zero": {
Stale: true,
Status: StatusActive,
Score: decimal.Zero,
},
"Unchanged digest keeps the full score": {
Status: StatusActive,
Score: decimal.NewFromInt(100),
},
}
image, digest := newFakeRegistryImage(t, false)
for cn, c := range cc {
t.Run(cn, func(t *testing.T) {
t.Parallel()
ciw := ContainerImageWatcher{Image: image}
image, digest := c.Image, "sha256:old"
if image == "" {
image, digest = newFakeRegistryImage(t, c.Unauthorized)
}
res, err := ciw.Process(context.Background(), stubInput{
state: imageWatcherState(t, digest),
if c.Missing {
image = strings.Replace(image, ":v1", ":v2", 1)
}
if c.Stale {
digest = "sha256:old"
}
state := c.State
if state == nil {
state = imageWatcherState(t, digest)
}
res, err := (&ContainerImageWatcher{Image: image}).Process(context.Background(), stubInput{state: state})
testutil.AssertEqualError(t, c.Err, err)
if err != nil {
return
}
assert.Equal(t, c.Status, res.Status)
assert.True(t, res.Score.Equal(c.Score), "score %s", res.Score)
if c.Status != StatusActive {
assert.Nil(t, res.State)
return
}
assert.Equal(t, state, res.State)
})
require.NoError(t, err)
assert.Equal(t, StatusActive, res.Status)
assert.True(t, res.Score.Equal(decimal.NewFromInt(100)))
var ciws ContainerImageWatcherState
require.NoError(t, json.Unmarshal(res.State, &ciws))
assert.Equal(t, digest, ciws.Digest)
})
t.Run("Changed digest drops the score to zero", func(t *testing.T) {
t.Parallel()
image, _ := newFakeRegistryImage(t, false)
ciw := ContainerImageWatcher{Image: image}
res, err := ciw.Process(context.Background(), stubInput{
state: imageWatcherState(t, "sha256:old"),
})
require.NoError(t, err)
assert.True(t, res.Score.Equal(decimal.Zero))
})
t.Run("Unauthorized registry is a status", func(t *testing.T) {
t.Parallel()
image, _ := newFakeRegistryImage(t, true)
ciw := ContainerImageWatcher{Image: image}
res, err := ciw.Process(context.Background(), stubInput{
state: imageWatcherState(t, "sha256:old"),
})
require.NoError(t, err)
assert.Equal(t, StatusUnauthorized, res.Status)
assert.Nil(t, res.State)
})
t.Run("Missing image is a status", func(t *testing.T) {
t.Parallel()
image, _ := newFakeRegistryImage(t, false)
ciw := ContainerImageWatcher{Image: strings.Replace(image, ":v1", ":v2", 1)}
res, err := ciw.Process(context.Background(), stubInput{
state: imageWatcherState(t, "sha256:old"),
})
require.NoError(t, err)
assert.Equal(t, StatusImageNotFound, res.Status)
})
t.Run("Invalid image reference fails", func(t *testing.T) {
t.Parallel()
ciw := ContainerImageWatcher{Image: "INVALID image ref"}
_, err := ciw.Process(context.Background(), stubInput{
state: imageWatcherState(t, ""),
})
require.Error(t, err)
})
t.Run("Malformed state fails", func(t *testing.T) {
t.Parallel()
ciw := ContainerImageWatcher{}
_, err := ciw.Process(context.Background(), stubInput{state: State(`{not json`)})
require.Error(t, err)
})
}
}
func Test_ContainerImageWatcher_Reset(t *testing.T) {
t.Parallel()
t.Run("Reset adopts the current digest at full score", func(t *testing.T) {
t.Parallel()
cc := map[string]struct {
Unauthorized bool
Status Status
Score decimal.Decimal
}{
"Unauthorized registry is a status": {
Unauthorized: true,
Status: StatusUnauthorized,
},
"Reset adopts the current digest at full score": {
Status: StatusActive,
Score: decimal.NewFromInt(100),
},
}
image, digest := newFakeRegistryImage(t, false)
for cn, c := range cc {
t.Run(cn, func(t *testing.T) {
t.Parallel()
ciw := ContainerImageWatcher{Image: image}
image, digest := newFakeRegistryImage(t, c.Unauthorized)
res, err := ciw.Reset(context.Background(), stubInput{})
require.NoError(t, err)
res, err := (&ContainerImageWatcher{Image: image}).Reset(context.Background(), stubInput{})
require.NoError(t, err)
assert.Equal(t, StatusActive, res.Status)
assert.True(t, res.Score.Equal(decimal.NewFromInt(100)))
assert.Equal(t, c.Status, res.Status)
assert.True(t, res.Score.Equal(c.Score), "score %s", res.Score)
var ciws ContainerImageWatcherState
if c.Status != StatusActive {
assert.Nil(t, res.State)
require.NoError(t, json.Unmarshal(res.State, &ciws))
assert.Equal(t, digest, ciws.Digest)
})
return
}
t.Run("Unauthorized registry is a status", func(t *testing.T) {
t.Parallel()
image, _ := newFakeRegistryImage(t, true)
ciw := ContainerImageWatcher{Image: image}
res, err := ciw.Reset(context.Background(), stubInput{})
require.NoError(t, err)
assert.Equal(t, StatusUnauthorized, res.Status)
assert.Nil(t, res.State)
})
assert.Equal(t, imageWatcherState(t, digest), res.State)
})
}
}

2
server/core/internal/document/hook/processor/github_tracking_test.go
View File

@ -58,7 +58,7 @@ func Test_GithubTracking_Validate(t *testing.T) {
t.Run(cn, func(t *testing.T) {
t.Parallel()
assert.Equal(t, c.Err, c.Tracking.Validate())
testutil.AssertEqualError(t, c.Err, c.Tracking.Validate())
})
}
}

121
server/core/internal/document/hook/processor/scheduled_reminder_test.go
View File

@ -6,6 +6,7 @@ import (
"testing"
"time"
"github.com/oxynote/oxynote/server/core/pkg/testutil"
"github.com/shopspring/decimal"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
@ -26,65 +27,63 @@ func Test_ScheduledReminder_Process(t *testing.T) {
now := time.Now()
tests := map[string]struct {
Reminder ScheduledReminder
State State
ExpectErr bool
ExpectedScore decimal.Decimal
cc := map[string]struct {
Reminder ScheduledReminder
State State
Score decimal.Decimal
Err error
}{
"Halfway through scores about half": {
Reminder: ScheduledReminder{
Scale: ScaleTypeLinear,
Schedule: now.Add(time.Hour),
},
State: reminderState(t, now.Add(-time.Hour)),
ExpectedScore: decimal.NewFromInt(50),
State: reminderState(t, now.Add(-time.Hour)),
Score: decimal.NewFromInt(50),
},
"Elapsed schedule scores zero": {
Reminder: ScheduledReminder{
Scale: ScaleTypeLinear,
Schedule: now.Add(-time.Minute),
},
State: reminderState(t, now.Add(-time.Hour)),
ExpectedScore: decimal.Zero,
State: reminderState(t, now.Add(-time.Hour)),
Score: decimal.Zero,
},
"Fresh schedule scores full": {
Reminder: ScheduledReminder{
Scale: ScaleTypeLinear,
Schedule: now.Add(1000 * time.Hour),
},
State: reminderState(t, now),
ExpectedScore: decimal.NewFromInt(100),
State: reminderState(t, now),
Score: decimal.NewFromInt(100),
},
"Malformed state fails": {
Reminder: ScheduledReminder{
Scale: ScaleTypeLinear,
},
State: State(`{not json`),
ExpectErr: true,
State: State(`{not json`),
Err: assert.AnError,
},
}
for name, tc := range tests {
t.Run(name, func(t *testing.T) {
for cn, c := range cc {
t.Run(cn, func(t *testing.T) {
t.Parallel()
res, err := tc.Reminder.Process(context.Background(), stubInput{state: tc.State})
if tc.ExpectErr {
require.Error(t, err)
res, err := c.Reminder.Process(context.Background(), stubInput{state: c.State})
testutil.AssertEqualError(t, c.Err, err)
if err != nil {
return
}
require.NoError(t, err)
assert.Equal(t, StatusActive, res.Status)
assert.True(
t,
res.Score.Sub(tc.ExpectedScore).Abs().LessThanOrEqual(decimal.NewFromInt(1)),
"score %s should be within 1 of %s", res.Score, tc.ExpectedScore,
res.Score.Sub(c.Score).Abs().LessThanOrEqual(decimal.NewFromInt(1)),
"score %s should be within 1 of %s", res.Score, c.Score,
)
assert.JSONEq(t, string(tc.State), string(res.State))
assert.JSONEq(t, string(c.State), string(res.State))
})
}
}
@ -92,47 +91,49 @@ func Test_ScheduledReminder_Process(t *testing.T) {
func Test_ScheduledReminder_Reset(t *testing.T) {
t.Parallel()
t.Run("Future schedule starts at full score", func(t *testing.T) {
t.Parallel()
sr := ScheduledReminder{
Scale: ScaleTypeLinear,
cc := map[string]struct {
Schedule time.Time
Score decimal.Decimal
}{
"Future schedule starts at full score": {
Schedule: time.Now().Add(time.Hour),
}
res, err := sr.Reset(context.Background(), stubInput{})
require.NoError(t, err)
assert.Equal(t, StatusActive, res.Status)
assert.True(t, res.Score.Equal(decimal.NewFromInt(100)))
var srs ScheduledReminderState
require.NoError(t, json.Unmarshal(res.State, &srs))
assert.False(t, srs.StartedAt.IsZero())
})
t.Run("Schedule inside the grace period starts at zero", func(t *testing.T) {
t.Parallel()
sr := ScheduledReminder{
Scale: ScaleTypeLinear,
Score: decimal.NewFromInt(100),
},
"Schedule inside the grace period starts at zero": {
Schedule: time.Now().Add(time.Second),
}
Score: decimal.Zero,
},
}
res, err := sr.Reset(context.Background(), stubInput{})
require.NoError(t, err)
for cn, c := range cc {
t.Run(cn, func(t *testing.T) {
t.Parallel()
assert.True(t, res.Score.Equal(decimal.Zero))
})
sr := ScheduledReminder{
Scale: ScaleTypeLinear,
Schedule: c.Schedule,
}
res, err := sr.Reset(context.Background(), stubInput{})
require.NoError(t, err)
assert.Equal(t, StatusActive, res.Status)
assert.True(t, res.Score.Equal(c.Score), "score %s", res.Score)
var srs ScheduledReminderState
require.NoError(t, json.Unmarshal(res.State, &srs))
assert.False(t, srs.StartedAt.IsZero())
})
}
}
func Test_ScheduledReminder_Validate(t *testing.T) {
t.Parallel()
tests := map[string]struct {
Reminder ScheduledReminder
ExpectedErr error
cc := map[string]struct {
Reminder ScheduledReminder
Err error
}{
"Linear scale with a schedule is valid": {
Reminder: ScheduledReminder{
@ -145,21 +146,21 @@ func Test_ScheduledReminder_Validate(t *testing.T) {
Scale: ScaleType("exponential"),
Schedule: time.Now().Add(time.Hour),
},
ExpectedErr: ErrInvalidScaleType,
Err: ErrInvalidScaleType,
},
"Missing schedule is rejected": {
Reminder: ScheduledReminder{
Scale: ScaleTypeLinear,
},
ExpectedErr: ErrMissingSchedule,
Err: ErrMissingSchedule,
},
}
for name, tc := range tests {
t.Run(name, func(t *testing.T) {
for cn, c := range cc {
t.Run(cn, func(t *testing.T) {
t.Parallel()
assert.Equal(t, tc.ExpectedErr, tc.Reminder.Validate())
testutil.AssertEqualError(t, c.Err, c.Reminder.Validate())
})
}
}

410
server/core/internal/document/hook/processor/url_watcher_test.go
View File

@ -8,6 +8,7 @@ import (
"github.com/guregu/null/v5"
"github.com/oxynote/oxynote/server/core/internal/apps/webchange"
"github.com/oxynote/oxynote/server/core/pkg/testutil"
"github.com/shopspring/decimal"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
@ -50,12 +51,12 @@ func (f *fakeChangeDetection) DeleteWatcher(_ context.Context, watchID string) e
return f.deleteErr
}
// watcherState marshals a URL-watcher state for the watcher "w-1".
func watcherState(t *testing.T, lastChangedAt null.Time) State {
// watcherState marshals a URL-watcher state.
func watcherState(t *testing.T, watcherID string, lastChangedAt null.Time) State {
t.Helper()
raw, err := json.Marshal(URLWatcherState{
WatcherID: "w-1",
WatcherID: watcherID,
LastChangedAt: lastChangedAt,
})
require.NoError(t, err)
@ -66,23 +67,23 @@ func watcherState(t *testing.T, lastChangedAt null.Time) State {
func Test_URLWatcher_Validate(t *testing.T) {
t.Parallel()
tests := map[string]struct {
URL string
ExpectedErr error
cc := map[string]struct {
URL string
Err error
}{
"Https URL is valid": {URL: "https://example.com/page"},
"Http URL is valid": {URL: "http://example.com"},
"Other scheme is rejected": {URL: "ftp://example.com", ExpectedErr: ErrInvalidURL},
"Relative URL is rejected": {URL: "/page", ExpectedErr: ErrInvalidURL},
"Missing host is rejected": {URL: "https://", ExpectedErr: ErrInvalidURL},
"Unparsable URL is rejected": {URL: "https://exa mple.com/%zz", ExpectedErr: ErrInvalidURL},
"Other scheme is rejected": {URL: "ftp://example.com", Err: ErrInvalidURL},
"Relative URL is rejected": {URL: "/page", Err: ErrInvalidURL},
"Missing host is rejected": {URL: "https://", Err: ErrInvalidURL},
"Unparsable URL is rejected": {URL: "https://exa mple.com/%zz", Err: ErrInvalidURL},
}
for name, tc := range tests {
t.Run(name, func(t *testing.T) {
for cn, c := range cc {
t.Run(cn, func(t *testing.T) {
t.Parallel()
assert.Equal(t, tc.ExpectedErr, (&URLWatcher{URL: tc.URL}).Validate())
testutil.AssertEqualError(t, c.Err, (&URLWatcher{URL: c.URL}).Validate())
})
}
}
@ -92,103 +93,101 @@ func Test_URLWatcher_Process(t *testing.T) {
changed := time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC)
tests := map[string]struct {
CD *fakeChangeDetection
State State
ExpectErr bool
ExpectedScore decimal.Decimal
ExpectedStatus Status
Updated [][2]string
cc := map[string]struct {
CD *fakeChangeDetection
State State
Score decimal.Decimal
Status Status
Updated [][2]string
Err error
}{
"Unchanged watch keeps the full score": {
CD: &fakeChangeDetection{
watch: &webchange.Watch{URL: "https://example.com", LastChangedAt: changed},
},
State: watcherState(t, null.TimeFrom(changed)),
ExpectedScore: decimal.NewFromInt(100),
ExpectedStatus: StatusActive,
State: watcherState(t, "w-1", null.TimeFrom(changed)),
Score: decimal.NewFromInt(100),
Status: StatusActive,
},
"Newer change drops the score to zero": {
CD: &fakeChangeDetection{
watch: &webchange.Watch{URL: "https://example.com", LastChangedAt: changed.Add(time.Hour)},
},
State: watcherState(t, null.TimeFrom(changed)),
ExpectedScore: decimal.Zero,
ExpectedStatus: StatusActive,
State: watcherState(t, "w-1", null.TimeFrom(changed)),
Score: decimal.Zero,
Status: StatusActive,
},
"Missing baseline adopts the watch timestamp": {
CD: &fakeChangeDetection{
watch: &webchange.Watch{URL: "https://example.com", LastChangedAt: changed},
},
State: watcherState(t, null.Time{}),
ExpectedScore: decimal.NewFromInt(100),
ExpectedStatus: StatusActive,
State: watcherState(t, "w-1", null.Time{}),
Score: decimal.NewFromInt(100),
Status: StatusActive,
},
"Unreachable watch is a status": {
CD: &fakeChangeDetection{
watch: &webchange.Watch{URL: "https://example.com", Unreachable: true},
},
State: watcherState(t, null.Time{}),
ExpectedStatus: StatusUnreachableURL,
State: watcherState(t, "w-1", null.Time{}),
Status: StatusUnreachableURL,
},
"Watcher on another URL is pointed back": {
CD: &fakeChangeDetection{
watch: &webchange.Watch{URL: "https://old.example.com", LastChangedAt: changed},
},
State: watcherState(t, null.TimeFrom(changed)),
ExpectedScore: decimal.NewFromInt(100),
ExpectedStatus: StatusActive,
Updated: [][2]string{{"w-1", "https://example.com"}},
State: watcherState(t, "w-1", null.TimeFrom(changed)),
Score: decimal.NewFromInt(100),
Status: StatusActive,
Updated: [][2]string{{"w-1", "https://example.com"}},
},
"Removed watcher is created again": {
CD: &fakeChangeDetection{
fetchErr: webchange.ErrWatcherNotFound,
createdID: "w-new",
},
State: watcherState(t, null.Time{}),
ExpectedScore: decimal.NewFromInt(100),
ExpectedStatus: StatusActive,
State: watcherState(t, "w-1", null.Time{}),
Score: decimal.NewFromInt(100),
Status: StatusActive,
},
"Unconfigured changedetection is a status": {
CD: &fakeChangeDetection{fetchErr: webchange.ErrNotConfigured},
State: watcherState(t, null.Time{}),
ExpectedStatus: StatusUnconfigured,
CD: &fakeChangeDetection{fetchErr: webchange.ErrNotConfigured},
State: watcherState(t, "w-1", null.Time{}),
Status: StatusUnconfigured,
},
"Fetch failure is propagated": {
CD: &fakeChangeDetection{fetchErr: assert.AnError},
State: watcherState(t, null.Time{}),
ExpectErr: true,
CD: &fakeChangeDetection{fetchErr: assert.AnError},
State: watcherState(t, "w-1", null.Time{}),
Err: assert.AnError,
},
"Malformed state fails": {
CD: &fakeChangeDetection{},
State: State(`{not json`),
ExpectErr: true,
CD: &fakeChangeDetection{},
State: State(`{not json`),
Err: assert.AnError,
},
}
for name, tc := range tests {
t.Run(name, func(t *testing.T) {
for cn, c := range cc {
t.Run(cn, func(t *testing.T) {
t.Parallel()
uw := URLWatcher{URL: "https://example.com"}
res, err := uw.Process(context.Background(), stubInput{state: tc.State, cd: tc.CD})
if tc.ExpectErr {
require.Error(t, err)
res, err := uw.Process(context.Background(), stubInput{state: c.State, cd: c.CD})
testutil.AssertEqualError(t, c.Err, err)
if err != nil {
return
}
require.NoError(t, err)
assert.Equal(t, tc.ExpectedStatus, res.Status)
assert.True(t, res.Score.Equal(tc.ExpectedScore), "score %s", res.Score)
assert.Equal(t, c.Status, res.Status)
assert.True(t, res.Score.Equal(c.Score), "score %s", res.Score)
if tc.ExpectedStatus != StatusActive {
if c.Status != StatusActive {
assert.Nil(t, res.State)
}
assert.Equal(t, tc.Updated, tc.CD.updated)
assert.Equal(t, c.Updated, c.CD.updated)
})
}
}
@ -198,183 +197,150 @@ func Test_URLWatcher_Reset(t *testing.T) {
changed := time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC)
t.Run("Missing watcher is created", func(t *testing.T) {
t.Parallel()
cc := map[string]struct {
CD *fakeChangeDetection
State State
Created []string
Updated [][2]string
Result Result
Err error
}{
"Missing watcher is created": {
CD: &fakeChangeDetection{createdID: "w-new"},
Created: []string{"https://example.com"},
Result: Result{
Status: StatusActive,
Score: decimal.NewFromInt(100),
State: watcherState(t, "w-new", null.Time{}),
},
},
"Existing watcher adopts the watch timestamp": {
CD: &fakeChangeDetection{
watch: &webchange.Watch{URL: "https://example.com", LastChangedAt: changed},
},
State: watcherState(t, "w-1", null.Time{}),
Result: Result{
Status: StatusActive,
Score: decimal.NewFromInt(100),
State: watcherState(t, "w-1", null.TimeFrom(changed)),
},
},
"Changed URL updates the watcher and clears the baseline": {
CD: &fakeChangeDetection{
watch: &webchange.Watch{URL: "https://old.example.com", LastChangedAt: changed},
},
State: watcherState(t, "w-1", null.Time{}),
Updated: [][2]string{{"w-1", "https://example.com"}},
Result: Result{
Status: StatusActive,
Score: decimal.NewFromInt(100),
State: watcherState(t, "w-1", null.Time{}),
},
},
"Removed watcher is created again": {
CD: &fakeChangeDetection{
fetchErr: webchange.ErrWatcherNotFound,
createdID: "w-new",
},
State: watcherState(t, "w-1", null.Time{}),
Created: []string{"https://example.com"},
Result: Result{
Status: StatusActive,
Score: decimal.NewFromInt(100),
State: watcherState(t, "w-new", null.Time{}),
},
},
"Unconfigured changedetection is a status": {
CD: &fakeChangeDetection{createErr: webchange.ErrNotConfigured},
Created: []string{"https://example.com"},
Result: Result{Status: StatusUnconfigured},
},
"Error returned by ChangeDetection.UpdateWatcher": {
CD: &fakeChangeDetection{
watch: &webchange.Watch{URL: "https://old.example.com"},
updateErr: assert.AnError,
},
State: watcherState(t, "w-1", null.Time{}),
Updated: [][2]string{{"w-1", "https://example.com"}},
Err: assert.AnError,
},
"Error returned by ChangeDetection.CreateWatcher": {
CD: &fakeChangeDetection{createErr: assert.AnError},
Created: []string{"https://example.com"},
Err: assert.AnError,
},
"Malformed state": {
CD: &fakeChangeDetection{},
State: State(`{not json`),
Err: assert.AnError,
},
}
cd := &fakeChangeDetection{createdID: "w-new"}
uw := URLWatcher{URL: "https://example.com"}
for cn, c := range cc {
t.Run(cn, func(t *testing.T) {
t.Parallel()
res, err := uw.Reset(context.Background(), stubInput{cd: cd})
require.NoError(t, err)
uw := URLWatcher{URL: "https://example.com"}
assert.Equal(t, StatusActive, res.Status)
assert.True(t, res.Score.Equal(decimal.NewFromInt(100)))
assert.Equal(t, []string{"https://example.com"}, cd.createdURLs)
res, err := uw.Reset(context.Background(), stubInput{state: c.State, cd: c.CD})
testutil.AssertEqualError(t, c.Err, err)
var uws URLWatcherState
assert.Equal(t, c.Created, c.CD.createdURLs)
assert.Equal(t, c.Updated, c.CD.updated)
require.NoError(t, json.Unmarshal(res.State, &uws))
assert.Equal(t, "w-new", uws.WatcherID)
assert.False(t, uws.LastChangedAt.Valid)
})
if err != nil {
return
}
t.Run("Existing watcher adopts the watch timestamp", func(t *testing.T) {
t.Parallel()
cd := &fakeChangeDetection{
watch: &webchange.Watch{URL: "https://example.com", LastChangedAt: changed},
}
uw := URLWatcher{URL: "https://example.com"}
res, err := uw.Reset(context.Background(), stubInput{
state: watcherState(t, null.Time{}),
cd: cd,
assert.Equal(t, c.Result.Status, res.Status)
assert.True(t, res.Score.Equal(c.Result.Score), "score %s", res.Score)
assert.Equal(t, c.Result.State, res.State)
})
require.NoError(t, err)
var uws URLWatcherState
require.NoError(t, json.Unmarshal(res.State, &uws))
assert.Equal(t, null.TimeFrom(changed), uws.LastChangedAt)
assert.Empty(t, cd.updated)
})
t.Run("Changed URL updates the watcher and clears the baseline", func(t *testing.T) {
t.Parallel()
cd := &fakeChangeDetection{
watch: &webchange.Watch{URL: "https://old.example.com", LastChangedAt: changed},
}
uw := URLWatcher{URL: "https://example.com"}
res, err := uw.Reset(context.Background(), stubInput{
state: watcherState(t, null.Time{}),
cd: cd,
})
require.NoError(t, err)
assert.Equal(t, [][2]string{{"w-1", "https://example.com"}}, cd.updated)
var uws URLWatcherState
require.NoError(t, json.Unmarshal(res.State, &uws))
assert.Equal(t, "w-1", uws.WatcherID)
assert.False(t, uws.LastChangedAt.Valid)
})
t.Run("Removed watcher is created again", func(t *testing.T) {
t.Parallel()
cd := &fakeChangeDetection{
fetchErr: webchange.ErrWatcherNotFound,
createdID: "w-new",
}
res, err := (&URLWatcher{URL: "https://example.com"}).Reset(context.Background(), stubInput{
state: watcherState(t, null.Time{}),
cd: cd,
})
require.NoError(t, err)
var uws URLWatcherState
require.NoError(t, json.Unmarshal(res.State, &uws))
assert.Equal(t, "w-new", uws.WatcherID)
})
t.Run("Unconfigured changedetection is a status", func(t *testing.T) {
t.Parallel()
cd := &fakeChangeDetection{createErr: webchange.ErrNotConfigured}
res, err := (&URLWatcher{URL: "https://example.com"}).Reset(context.Background(), stubInput{cd: cd})
require.NoError(t, err)
assert.Equal(t, StatusUnconfigured, res.Status)
assert.Nil(t, res.State)
})
t.Run("Update failure is propagated", func(t *testing.T) {
t.Parallel()
cd := &fakeChangeDetection{
watch: &webchange.Watch{URL: "https://old.example.com"},
updateErr: assert.AnError,
}
_, err := (&URLWatcher{URL: "https://example.com"}).Reset(context.Background(), stubInput{
state: watcherState(t, null.Time{}),
cd: cd,
})
require.Error(t, err)
})
t.Run("Create failure is propagated", func(t *testing.T) {
t.Parallel()
cd := &fakeChangeDetection{createErr: assert.AnError}
_, err := (&URLWatcher{URL: "https://example.com"}).Reset(context.Background(), stubInput{cd: cd})
require.Error(t, err)
})
t.Run("Malformed state fails", func(t *testing.T) {
t.Parallel()
_, err := (&URLWatcher{}).Reset(context.Background(), stubInput{
state: State(`{not json`),
cd: &fakeChangeDetection{},
})
require.Error(t, err)
})
}
}
func Test_URLWatcher_Delete(t *testing.T) {
t.Parallel()
t.Run("Existing watcher is deleted", func(t *testing.T) {
t.Parallel()
cc := map[string]struct {
CD *fakeChangeDetection
State State
Deleted []string
Err error
}{
"Missing state is a no-op": {
CD: &fakeChangeDetection{},
},
"State without a watcher is a no-op": {
CD: &fakeChangeDetection{},
State: State(`{}`),
},
"Malformed state": {
CD: &fakeChangeDetection{},
State: State(`{not json`),
Err: assert.AnError,
},
"Error returned by ChangeDetection.DeleteWatcher": {
CD: &fakeChangeDetection{deleteErr: assert.AnError},
State: watcherState(t, "w-1", null.Time{}),
Deleted: []string{"w-1"},
Err: assert.AnError,
},
"Existing watcher is deleted": {
CD: &fakeChangeDetection{},
State: watcherState(t, "w-1", null.Time{}),
Deleted: []string{"w-1"},
},
}
cd := &fakeChangeDetection{}
for cn, c := range cc {
t.Run(cn, func(t *testing.T) {
t.Parallel()
err := (&URLWatcher{}).Delete(context.Background(), stubInput{
state: watcherState(t, null.Time{}),
cd: cd,
err := (&URLWatcher{}).Delete(context.Background(), stubInput{state: c.State, cd: c.CD})
testutil.AssertEqualError(t, c.Err, err)
assert.Equal(t, c.Deleted, c.CD.deletedIDs)
})
require.NoError(t, err)
assert.Equal(t, []string{"w-1"}, cd.deletedIDs)
})
t.Run("Missing state is a no-op", func(t *testing.T) {
t.Parallel()
cd := &fakeChangeDetection{}
require.NoError(t, (&URLWatcher{}).Delete(context.Background(), stubInput{cd: cd}))
assert.Empty(t, cd.deletedIDs)
})
t.Run("State without a watcher is a no-op", func(t *testing.T) {
t.Parallel()
cd := &fakeChangeDetection{}
require.NoError(t, (&URLWatcher{}).Delete(context.Background(), stubInput{
state: State(`{}`),
cd: cd,
}))
assert.Empty(t, cd.deletedIDs)
})
t.Run("Delete failure is propagated", func(t *testing.T) {
t.Parallel()
cd := &fakeChangeDetection{deleteErr: assert.AnError}
err := (&URLWatcher{}).Delete(context.Background(), stubInput{
state: watcherState(t, null.Time{}),
cd: cd,
})
require.Error(t, err)
})
}
}

27
server/core/internal/notification/interpreter/interpreter_test.go
View File

@ -250,8 +250,10 @@ func Test_Interpreter_interpretDocumentReviewRequestNotification(t *testing.T) {
func Test_Interpreter_interpretDocumentHookNotification(t *testing.T) {
cc := map[string]struct {
DB *DBMock
N notification.Notification
DB *DBMock
N notification.Notification
// Format defaults to _hookTriggeredFormat.
Format string
Result *Message
Err error
}{
@ -362,6 +364,20 @@ func Test_Interpreter_interpretDocumentHookNotification(t *testing.T) {
Text: fmt.Sprintf("<%s#blk1|a block in My Doc> may be outdated — Scheduled Reminder", _testDocURL),
},
},
"Successful interpretation with the needs attention format": {
DB: stubDB(),
N: stubNotification(notification.NewDocumentHookNeedsAttentionNotification(
_testDocID,
hook.TypeURLWatcher,
null.StringFrom("blk1"),
_testBranchID,
processor.StatusUnreachableURL,
)),
Format: _hookNeedsAttentionFormat,
Result: &Message{
Text: fmt.Sprintf("<%s#blk1|a block in My Doc> can no longer be checked — Website Changes", _testDocURL),
},
},
}
for cn, c := range cc {
@ -370,7 +386,12 @@ func Test_Interpreter_interpretDocumentHookNotification(t *testing.T) {
i := NewInterpreter(c.DB, NewSlackFormatter(), "https://app.test")
res, err := i.interpretDocumentHookNotification(context.Background(), c.N, _hookTriggeredFormat)
format := c.Format
if format == "" {
format = _hookTriggeredFormat
}
res, err := i.interpretDocumentHookNotification(context.Background(), c.N, format)
testutil.AssertEqualError(t, c.Err, err)
if err != nil {

6
server/core/internal/server/internal/document/branch_http.go
View File

@ -192,9 +192,9 @@ func (h *Handler) UpdateDocumentBranchByIDUnsafe(w http.ResponseWriter, r *http.
}
// a persist that changes nothing, such as the Yjs seed Hocuspocus
// sends on first load, leaves history alone. Only a persist that names
// its editors is attributed. A system write has no user, and without
// maintainers the branch's author is whoever edited before.
// sends on first load, leaves history alone. Only a user persist
// naming its maintainers is attributed. Otherwise LastUpdatedBy still
// names an earlier editor.
if !doc.SnapshotEqual(ndoc) {
var lastUpdatedBy null.String
if !ui.System && len(ui.Maintainers) > 0 {

68
server/core/internal/server/internal/document/branch_http_test.go
View File

@ -17,8 +17,7 @@ import (
"github.com/stretchr/testify/require"
)
// storedHook builds a stored hook whose processor needs no external
// dependencies.
// storedHook builds a stored hook on the document's main branch.
func storedHook(typ hookCore.Type) hookCore.Hook {
return hookCore.Hook{
ID: xid.New(),
@ -588,7 +587,7 @@ func Test_Handler_MergeBranches(t *testing.T) {
RespCode: http.StatusInternalServerError,
History: 1,
},
"Error returned by tx.FetchDocumentHooksByBranchID": {
"Error returned by Tx.FetchDocumentHooksByBranchID": {
DB: &DBMock{FetchDocumentByBranchIDFunc: fetchByBranch},
Tx: &TxMock{
FetchDocumentHooksByBranchIDFunc: func(context.Context, xid.ID, string) ([]hookCore.Hook, error) {
@ -598,6 +597,19 @@ func Test_Handler_MergeBranches(t *testing.T) {
Body: validBody,
RespCode: http.StatusInternalServerError,
},
"Error returned by Tx.InsertDocumentHook": {
DB: &DBMock{FetchDocumentByBranchIDFunc: fetchByBranch},
Tx: &TxMock{
FetchDocumentHooksByBranchIDFunc: func(context.Context, xid.ID, string) ([]hookCore.Hook, error) {
return []hookCore.Hook{storedHook(hookCore.TypeScheduledReminder)}, nil
},
InsertDocumentHookFunc: func(context.Context, hookCore.Hook) error {
return errors.New("boom")
},
},
Body: validBody,
RespCode: http.StatusInternalServerError,
},
"Commit error": {
DB: &DBMock{FetchDocumentByBranchIDFunc: fetchByBranch},
Tx: &TxMock{
@ -611,8 +623,15 @@ func Test_Handler_MergeBranches(t *testing.T) {
History: 1,
},
"Successful merge": {
DB: &DBMock{FetchDocumentByBranchIDFunc: fetchByBranch},
Tx: &TxMock{},
DB: &DBMock{FetchDocumentByBranchIDFunc: fetchByBranch},
Tx: &TxMock{
FetchDocumentHooksByBranchIDFunc: func(context.Context, xid.ID, string) ([]hookCore.Hook, error) {
hk := storedHook(hookCore.TypeScheduledReminder)
hk.BranchID = null.ValueFrom(_branchID2)
return []hookCore.Hook{hk}, nil
},
},
Body: validBody,
RespCode: http.StatusOK,
Committed: 1,
@ -666,6 +685,13 @@ func Test_Handler_MergeBranches(t *testing.T) {
assert.Equal(t, _branchID2, hh[0].BranchID)
assert.Equal(t, "org1", hh[0].OrganizationID)
ii := c.Tx.InsertDocumentHookCalls()
require.Len(t, ii, 1)
assert.Equal(t, null.ValueFrom(_branchID), ii[0].Hk.BranchID)
assert.Equal(t, null.ValueFrom(_documentID), ii[0].Hk.DocumentID)
assert.Equal(t, processor.StatusInitializing, ii[0].Hk.Status)
assert.False(t, ii[0].Hk.State.Valid)
pp := man.SetUpBranchCalls()
require.Len(t, pp, 1)
assert.Equal(t, _branchID, pp[0].BranchID)
@ -840,7 +866,7 @@ func Test_Handler_CreateDocumentBranch(t *testing.T) {
Body: validBody,
RespCode: http.StatusInternalServerError,
},
"Error returned by tx.FetchDocumentHooksByBranchID": {
"Error returned by Tx.FetchDocumentHooksByBranchID": {
DB: &DBMock{
FetchDocumentByBranchIDFunc: func(context.Context, xid.ID, string) (*documentCore.Document, error) {
return storedDoc(), nil
@ -854,6 +880,23 @@ func Test_Handler_CreateDocumentBranch(t *testing.T) {
Body: validBody,
RespCode: http.StatusInternalServerError,
},
"Error returned by Tx.InsertDocumentHook": {
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 []hookCore.Hook{storedHook(hookCore.TypeScheduledReminder)}, nil
},
InsertDocumentHookFunc: func(context.Context, hookCore.Hook) error {
return errors.New("boom")
},
},
Body: validBody,
RespCode: http.StatusInternalServerError,
},
"Commit error": {
DB: &DBMock{
FetchDocumentByBranchIDFunc: func(context.Context, xid.ID, string) (*documentCore.Document, error) {
@ -891,7 +934,11 @@ func Test_Handler_CreateDocumentBranch(t *testing.T) {
return storedDoc(), nil
},
},
Tx: &TxMock{},
Tx: &TxMock{
FetchDocumentHooksByBranchIDFunc: func(context.Context, xid.ID, string) ([]hookCore.Hook, error) {
return []hookCore.Hook{storedHook(hookCore.TypeScheduledReminder)}, nil
},
},
Body: validBody,
RespCode: http.StatusCreated,
Committed: 1,
@ -951,6 +998,13 @@ func Test_Handler_CreateDocumentBranch(t *testing.T) {
assert.Equal(t, _branchID, hh[0].BranchID)
assert.Equal(t, "org1", hh[0].OrganizationID)
ii := c.Tx.InsertDocumentHookCalls()
require.Len(t, ii, 1)
assert.Equal(t, null.ValueFrom(forked.BranchID), ii[0].Hk.BranchID)
assert.Equal(t, null.ValueFrom(_documentID), ii[0].Hk.DocumentID)
assert.Equal(t, processor.StatusInitializing, ii[0].Hk.Status)
assert.False(t, ii[0].Hk.State.Valid)
pp := man.SetUpBranchCalls()
require.Len(t, pp, 1)
assert.Equal(t, forked.BranchID, pp[0].BranchID)

5
server/core/internal/server/internal/document/document_http.go
View File

@ -841,8 +841,9 @@ func (h *Handler) upsertBranchReviewer(
// 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. A
// document duplicated from a branch also takes that branch's tags and its
// hooks, re-anchored through newBlockIDs. The tree-change notification is
// left to the caller, since it must not fire before the commit.
// hooks, re-anchored through newBlockIDs. Copied hooks are set up after
// the commit. 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,

29
server/core/internal/server/internal/document/document_http_test.go
View File

@ -1402,6 +1402,10 @@ func Test_Handler_CreateDocument(t *testing.T) {
assert.Equal(t, "org1", entry.OrganizationID)
assert.True(t, entry.Boundary)
assert.Equal(t, null.StringFrom("u1"), entry.By)
// a new document has no source branch to copy hooks from.
assert.Empty(t, c.Tx.FetchDocumentHooksByBranchIDCalls())
assert.Empty(t, hdl.hookMan.(*HookManagerMock).SetUpBranchCalls())
}
})
}
@ -1606,8 +1610,8 @@ func Test_Handler_DuplicateDocument(t *testing.T) {
Committed int
TreeCbs int
Copies int
// CopiedUID is a source block uid the hook copy is told to
// re-anchor.
// CopiedUID is the source block uid the case's hook is anchored
// to.
CopiedUID string
}{
"No session in context": {
@ -1703,8 +1707,15 @@ func Test_Handler_DuplicateDocument(t *testing.T) {
Copies: 1,
},
"Hooks follow the regenerated block uids": {
DB: imageDB(),
Tx: insertAwareTx(),
DB: imageDB(),
Tx: func() *TxMock {
tx := insertAwareTx()
tx.FetchDocumentHooksByBranchIDFunc = func(context.Context, xid.ID, string) ([]hookCore.Hook, error) {
return []hookCore.Hook{{BlockID: null.StringFrom("img-1-aaaaaaaaaaaaaaa")}}, nil
}
return tx
}(),
Storer: &StorerMock{},
RespCode: http.StatusCreated,
Committed: 1,
@ -1712,7 +1723,7 @@ func Test_Handler_DuplicateDocument(t *testing.T) {
Copies: 1,
CopiedUID: "img-1-aaaaaaaaaaaaaaa",
},
"Error returned by tx.FetchDocumentHooksByBranchID": {
"Error returned by Tx.FetchDocumentHooksByBranchID": {
DB: &DBMock{FetchDocumentFunc: fetchStored},
Tx: func() *TxMock {
tx := insertAwareTx()
@ -1739,14 +1750,6 @@ func Test_Handler_DuplicateDocument(t *testing.T) {
man := &HookManagerMock{}
hdl.hookMan = man
// a hook anchored to the copied block is re-anchored to its new
// uid.
if c.CopiedUID != "" {
c.Tx.FetchDocumentHooksByBranchIDFunc = func(context.Context, xid.ID, string) ([]hookCore.Hook, error) {
return []hookCore.Hook{{BlockID: null.StringFrom(c.CopiedUID)}}, nil
}
}
rec := httptest.NewRecorder()
hdl.DuplicateDocument(rec, newRequest(http.MethodPost, "", c.NoSession, c.OmitDoc, true))

3
server/core/internal/server/internal/document/hook/http.go
View File

@ -24,8 +24,7 @@ type Handler struct {
}
}
// NewHandler creates a new handler instance with the provided logger,
// database and hook manager.
// NewHandler creates a hook handler.
func NewHandler(log *slog.Logger, db DB, hookMan Manager) *Handler {
return &Handler{
log: log,

42
server/core/internal/server/internal/document/hook/http_test.go
View File

@ -40,8 +40,7 @@ func addSession(ctx context.Context) context.Context {
})
}
// scheduledHook builds a stored hook whose processor needs no external
// dependencies.
// scheduledHook builds a stored hook of the request's document.
func scheduledHook() *hookCore.Hook {
return &hookCore.Hook{
ID: _hookID,
@ -65,33 +64,6 @@ func storedHookManager() *ManagerMock {
}
}
// notified is one hooks-change announcement a handler made.
type notified struct {
organizationID string
documentID xid.ID
branchID xid.ID
}
// recordNotifications binds the handler's hooks-change callback to a
// list the test reads back.
func recordNotifications(hdl *Handler) *[]notified {
var out []notified
hdl.hooks.changeCallback = func(organizationID string, documentID, branchID xid.ID) {
out = append(out, notified{organizationID, documentID, branchID})
}
return &out
}
// assertNotified checks that a write announced nothing: the hook manager
// announces the hooks it changes.
func assertNotified(t *testing.T, got []notified) {
t.Helper()
assert.Empty(t, got)
}
func Test_NewHandler(t *testing.T) {
t.Parallel()
@ -238,7 +210,6 @@ func Test_Handler_FetchDocumentHooks(t *testing.T) {
log: slog.New(slog.DiscardHandler),
db: c.DB,
}
got := recordNotifications(&hdl)
req := httptest.NewRequest(http.MethodGet, "http://test.com/"+c.Query, http.NoBody)
@ -256,9 +227,6 @@ func Test_Handler_FetchDocumentHooks(t *testing.T) {
hdl.FetchDocumentHooks(rec, req.WithContext(ctx))
// a read announces nothing.
assert.Empty(t, *got)
for _, ch := range c.Checks {
ch(t, c.DB, rec)
}
@ -358,14 +326,12 @@ func Test_Handler_CreateDocumentHook(t *testing.T) {
t.Parallel()
hdl := NewHandler(slog.New(slog.DiscardHandler), &DBMock{}, c.Man)
got := recordNotifications(hdl)
rec := httptest.NewRecorder()
hdl.CreateDocumentHook(rec, hookRequest(http.MethodPost, c.Body, c.NoSession, c.OmitDoc, true))
assert.Equal(t, c.RespCode, rec.Code)
assertNotified(t, *got)
if c.RespBody != "" {
assert.JSONEq(t, c.RespBody, rec.Body.String())
@ -456,14 +422,12 @@ func Test_Handler_UpdateDocumentHook(t *testing.T) {
t.Parallel()
hdl := NewHandler(slog.New(slog.DiscardHandler), &DBMock{}, c.Man)
got := recordNotifications(hdl)
rec := httptest.NewRecorder()
hdl.UpdateDocumentHook(rec, hookRequest(http.MethodPut, c.Body, c.NoSession, c.OmitDoc, c.OmitHook))
assert.Equal(t, c.RespCode, rec.Code)
assertNotified(t, *got)
if c.RespBody != "" {
assert.JSONEq(t, c.RespBody, rec.Body.String())
@ -537,14 +501,12 @@ func Test_Handler_ResetDocumentHook(t *testing.T) {
t.Parallel()
hdl := NewHandler(slog.New(slog.DiscardHandler), &DBMock{}, c.Man)
got := recordNotifications(hdl)
rec := httptest.NewRecorder()
hdl.ResetDocumentHook(rec, hookRequest(http.MethodPut, "", c.NoSession, c.OmitDoc, c.OmitHook))
assert.Equal(t, c.RespCode, rec.Code)
assertNotified(t, *got)
ff := c.Man.ResetHookCalls()
require.Len(t, ff, c.Resets)
@ -612,14 +574,12 @@ func Test_Handler_DeleteDocumentHook(t *testing.T) {
t.Parallel()
hdl := NewHandler(slog.New(slog.DiscardHandler), &DBMock{}, c.Man)
got := recordNotifications(hdl)
rec := httptest.NewRecorder()
hdl.DeleteDocumentHook(rec, hookRequest(http.MethodDelete, "", c.NoSession, c.OmitDoc, c.OmitHook))
assert.Equal(t, c.RespCode, rec.Code)
assertNotified(t, *got)
ff := c.Man.DeleteHookCalls()
require.Len(t, ff, c.Deletes)

5
server/core/internal/server/server_test.go
View File

@ -153,6 +153,8 @@ func Test_NewServer(t *testing.T) {
t.Run("Successful construction", func(t *testing.T) {
t.Parallel()
hookMan := &HookManagerMock{}
srv, err := NewServer(
log,
Options{
@ -174,7 +176,7 @@ func Test_NewServer(t *testing.T) {
githubMan,
slackMan,
webchange.NewClient("", ""),
&HookManagerMock{},
hookMan,
stubSearcher{},
stubSearchTrigger{},
notifier,
@ -209,6 +211,7 @@ func Test_NewServer(t *testing.T) {
assert.NotNil(t, srv.handlers.email)
assert.NotNil(t, srv.handlers.ai)
assert.NotNil(t, srv.handlers.mcp)
assert.Len(t, hookMan.BindHookChangeCalls(), 1)
// nothing above is configured, so every capability reports false;
// the search gateway mock is the one switched on.

63
server/core/pkg/syncutil/keyed_mutex_test.go
View File

@ -4,6 +4,7 @@ import (
"context"
"testing"
"github.com/oxynote/oxynote/server/core/pkg/testutil"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.uber.org/goleak"
@ -24,30 +25,54 @@ func Test_NewKeyedMutex(t *testing.T) {
func Test_KeyedMutex_Lock(t *testing.T) {
t.Parallel()
km := NewKeyedMutex[string]()
unlock, err := km.Lock(context.Background(), "a")
require.NoError(t, err)
// a second caller gives up once its context ends.
ctx, cancel := context.WithCancel(context.Background())
ended, cancel := context.WithCancel(context.Background())
cancel()
_, err = km.Lock(ctx, "a")
assert.Equal(t, context.Canceled, err)
cc := map[string]struct {
Context context.Context
// Held is the key another caller holds during the lock.
Held string
Err error
}{
"Free key is locked": {
Context: context.Background(),
},
"Held key gives up when the context ends": {
Context: ended,
Held: "a",
Err: context.Canceled,
},
"Other key is independent": {
Context: context.Background(),
Held: "b",
},
}
// another key is not held by the first one's mutex.
other, err := km.Lock(context.Background(), "b")
require.NoError(t, err)
for cn, c := range cc {
t.Run(cn, func(t *testing.T) {
t.Parallel()
other()
unlock()
km := NewKeyedMutex[string]()
// every mutex is dropped once no one uses it.
assert.Empty(t, km.locks)
held := func() {}
unlock, err = km.Lock(context.Background(), "a")
require.NoError(t, err)
if c.Held != "" {
var err error
unlock()
held, err = km.Lock(context.Background(), c.Held)
require.NoError(t, err)
}
unlock, err := km.Lock(c.Context, "a")
testutil.AssertEqualError(t, c.Err, err)
if err == nil {
unlock()
}
held()
assert.Empty(t, km.locks)
})
}
}

132
web/app/components/NotificationBox.nuxt.test.ts
View File

@ -203,35 +203,33 @@ describe("<NotificationBox>", { concurrent: false }, () => {
)
})
it("names the hook that fired", async ({ expect }) => {
seedNotifications([
makeNotification({
it.for([
{
name: "names the hook that fired",
input: {
code: NotificationCode.DocumentHookTrigerred,
metadata: {
documentId: DOC_ID,
branchId: "b",
blockId: null,
type: DocumentHookType.URLWatcher,
},
}),
])
const wrapper = await mountBox()
expect(wrapper.text()).toContain("hook was triggered")
})
it("names the hook that can no longer check its target", async ({
expect,
}) => {
type: DocumentHookType.URLWatcher,
},
expected: "notification.messages.document-hook-triggered-description",
},
{
name: "names the hook that needs attention",
input: {
code: NotificationCode.DocumentHookNeedsAttention,
type: DocumentHookType.GitHubTracking,
},
expected:
"notification.messages.document-hook-needs-attention-description",
},
])("$name", async ({ input, expected }, { expect }) => {
seedNotifications([
makeNotification({
code: NotificationCode.DocumentHookNeedsAttention,
code: input.code,
metadata: {
documentId: DOC_ID,
branchId: "b",
blockId: null,
type: DocumentHookType.GitHubTracking,
type: input.type,
},
}),
])
@ -239,9 +237,7 @@ describe("<NotificationBox>", { concurrent: false }, () => {
const wrapper = await mountBox()
expect(wrapper.text()).toContain(
t("notification.messages.document-hook-needs-attention-description", {
hook: t("editor.hooks.github-tracking.title"),
}),
t(expected, { hook: t(`editor.hooks.${input.type}.title`) }),
)
})
@ -333,36 +329,63 @@ describe("<NotificationBox>", { concurrent: false }, () => {
})
it.for([
{ type: DocumentHookType.URLWatcher, expected: "mingcute:earth-2-line" },
{
type: DocumentHookType.GitHubTracking,
name: "marks a url watcher trigger with a globe icon",
input: {
code: NotificationCode.DocumentHookTrigerred,
type: DocumentHookType.URLWatcher,
},
expected: "mingcute:earth-2-line",
},
{
name: "marks a github tracking trigger with a github icon",
input: {
code: NotificationCode.DocumentHookTrigerred,
type: DocumentHookType.GitHubTracking,
},
expected: "simple-icons:github",
},
{ type: DocumentHookType.ScheduledReminder, expected: "lucide:timer" },
{
type: DocumentHookType.ContainerImageWatcher,
name: "marks a scheduled reminder trigger with a timer icon",
input: {
code: NotificationCode.DocumentHookTrigerred,
type: DocumentHookType.ScheduledReminder,
},
expected: "lucide:timer",
},
{
name: "marks a container image watcher trigger with a container icon",
input: {
code: NotificationCode.DocumentHookTrigerred,
type: DocumentHookType.ContainerImageWatcher,
},
expected: "lucide:container",
},
])(
"marks a $type hook trigger with its own icon",
async ({ type, expected }, { expect }) => {
seedNotifications([
makeNotification({
code: NotificationCode.DocumentHookTrigerred,
metadata: {
documentId: DOC_ID,
branchId: "b",
blockId: null,
type: type,
},
}),
])
const wrapper = await mountBox()
expect(renderedIconNames(wrapper)).toContain(expected)
{
name: "marks a hook that needs attention with its own icon",
input: {
code: NotificationCode.DocumentHookNeedsAttention,
type: DocumentHookType.ContainerImageWatcher,
},
expected: "lucide:container",
},
)
])("$name", async ({ input, expected }, { expect }) => {
seedNotifications([
makeNotification({
code: input.code,
metadata: {
documentId: DOC_ID,
branchId: "b",
blockId: null,
type: input.type,
},
}),
])
const wrapper = await mountBox()
expect(renderedIconNames(wrapper)).toContain(expected)
})
it("marks a hook type this build does not know with a warning icon", async ({
expect,
@ -553,6 +576,19 @@ describe("<NotificationBox>", { concurrent: false }, () => {
},
expected: `/acme/Runbook-${DOC_ID}`,
},
{
name: "opens a hook that needs attention at the block it watches",
notification: {
code: NotificationCode.DocumentHookNeedsAttention,
metadata: {
documentId: DOC_ID,
branchId: "b",
blockId: "block-7",
type: DocumentHookType.URLWatcher,
},
},
expected: `/acme/Runbook-${DOC_ID}#block-7`,
},
{
name: "opens a new comment at the block it is anchored to",
notification: {

2
web/app/components/editor/hooks/HookConfigPanel.nuxt.test.ts
View File

@ -179,7 +179,7 @@ describe("<HookConfigPanel>", { concurrent: false }, () => {
expected: false,
},
{
name: "says nothing of setup for a copy that cannot check its target",
name: "says nothing of setup for a copy whose integration is missing",
input: { state: null, status: "unconfigured" },
expected: false,
},

70
web/app/components/editor/hooks/container-image-watcher/ConfigMenu.nuxt.test.ts
View File

@ -89,30 +89,30 @@ describe("<ContainerImageWatcherConfigMenu>", { concurrent: false }, () => {
expect(menuText()).toContain(IMAGE)
})
it("says the image of a failing hook is unreachable", async ({ expect }) => {
await mountMenu({
hook: imageHook({
state: { digest: "sha256:old" },
status: "unauthorized",
}),
})
it.for([
{
name: "an unreachable image",
input: "unauthorized",
expected: "editor.hooks.container-image-watcher.detail-unreachable",
},
{
name: "an image gone from its registry",
input: "image_not_found",
expected: "editor.hooks.container-image-watcher.detail-missing",
},
] as const)(
"names $name on the hook's row",
async ({ input, expected }, { expect }) => {
await mountMenu({ hook: imageHook({ status: input }) })
const detail = t("editor.hooks.container-image-watcher.detail-unreachable")
expect(menuText()).toContain(
t("editor.hooks.subtext-detail", { subtext: IMAGE, detail: detail }),
)
})
it("says the image of a hook is gone from its registry", async ({
expect,
}) => {
await mountMenu({ hook: imageHook({ status: "image_not_found" }) })
const detail = t("editor.hooks.container-image-watcher.detail-missing")
expect(menuText()).toContain(
t("editor.hooks.subtext-detail", { subtext: IMAGE, detail: detail }),
)
})
expect(menuText()).toContain(
t("editor.hooks.subtext-detail", {
subtext: IMAGE,
detail: t(expected),
}),
)
},
)
it("prefills the image reference of the hook it edits", async ({
expect,
@ -177,20 +177,24 @@ describe("<ContainerImageWatcherConfigMenu>", { concurrent: false }, () => {
expect(hookNotice().querySelector(".font-semibold")).toBeNull()
})
it("warns about an image it cannot reach", async ({ expect }) => {
await mountMenu({
hook: imageHook({
state: { digest: "sha256:old" },
status: "unauthorized",
}),
})
it.for([
{
name: "an image it cannot reach",
input: "unauthorized",
expected: "editor.hooks.container-image-watcher.unreachable-image",
},
{
name: "an image gone from its registry",
input: "image_not_found",
expected: "editor.hooks.container-image-watcher.missing-image",
},
] as const)("warns about $name", async ({ input, expected }, { expect }) => {
await mountMenu({ hook: imageHook({ status: input }) })
await openHookSubMenu(t(TITLE))
expect(hookNotice().dataset.hookStatus).toBe("needs-attention")
expect(hookNotice().textContent.trim()).toBe(
t("editor.hooks.container-image-watcher.unreachable-image"),
)
expect(hookNotice().textContent.trim()).toBe(t(expected))
})
it("keeps the create button out of reach until an image is typed", async ({

4
web/app/components/editor/hooks/github-tracking/ConfigMenu.nuxt.test.ts
View File

@ -276,8 +276,8 @@ describe("<GitHubTrackingConfigMenu>", { concurrent: false }, () => {
},
)
// a server without github cannot be fixed from the settings, so the
// notice does not send the reader there
// github is also disconnected here. The notice still names the server,
// because no setting can fix it.
it("warns that github is not set up on this server", async ({ expect }) => {
mockGitHub({ connected: false })
await mountMenu({ hook: githubHook({ status: "unconfigured" }) })

5
web/app/components/editor/hooks/github-tracking/ConfigMenu.vue
View File

@ -98,9 +98,8 @@ const invalidData = computed(() => {
const failure = computed(() =>
checkStatus.value ? checkFailure(checkStatus.value) : null,
)
// a server without GitHub comes first, since no setup an editor does can
// fix it. Then the setup an editor still has to do, then what the last
// check found.
// a server without GitHub comes first, because no editor can fix it. Then
// the setup an editor still has to do, then what the last check found.
const notice = computed(() => {
if (failure.value?.notice === "unconfigured") {
return "unconfigured"

58
web/app/components/editor/hooks/hook-actions.nuxt.test.ts
View File

@ -87,34 +87,33 @@ describe("useHookActions", { concurrent: false }, () => {
])
})
it.for([
{ name: "creating", update: false },
{ name: "updating", update: true },
])(
"names the reason core refused $name the hook for",
async ({ update }, { expect }) => {
const refused = (_call: RecordedCall, event: H3Event) => {
setResponseStatus(event, 422)
it("shows core's reason when it refuses to create the hook", async ({
expect,
}) => {
mockEndpoint("POST", `/api/documents/${DOCUMENT_ID}/hooks`, refused)
const { actions } = await mountActions(null)
return {
code: "document_hook.unconfigured",
message: "the hook cannot check its target: unconfigured",
}
}
mockEndpoint("POST", `/api/documents/${DOCUMENT_ID}/hooks`, refused)
mockEndpoint("PUT", HOOK_PATH, refused)
const { actions } = await mountActions(update ? undefined : null)
const done = await actions.create(SETTINGS)
const done = update
? await actions.update(SETTINGS)
: await actions.create(SETTINGS)
expect(done).toBe(false)
expect(raisedToasts()).toMatchObject([
{ type: "error", title: t("editor.hooks.errors.codes.unconfigured") },
])
})
expect(done).toBe(false)
expect(raisedToasts()).toMatchObject([
{ type: "error", title: t("editor.hooks.errors.codes.unconfigured") },
])
},
)
it("shows core's reason when it refuses to update the hook", async ({
expect,
}) => {
mockEndpoint("PUT", HOOK_PATH, refused)
const { actions } = await mountActions()
const done = await actions.update(SETTINGS)
expect(done).toBe(false)
expect(raisedToasts()).toMatchObject([
{ type: "error", title: t("editor.hooks.errors.codes.unconfigured") },
])
})
it("updates the hook's settings after closing the menu", async ({
expect,
@ -276,3 +275,12 @@ function failing(_call: RecordedCall, event: H3Event) {
return { message: "boom" }
}
function refused(_call: RecordedCall, event: H3Event) {
setResponseStatus(event, 422)
return {
code: "document_hook.unconfigured",
message: "the hook cannot check its target: unconfigured",
}
}

4
web/app/components/editor/hooks/hook-actions.ts
View File

@ -3,8 +3,8 @@ import { hookErrorMessage } from "./hook-errors"
// useHookActions sends the requests a hook's menu makes. Each one closes
// the menu before it goes out, raises a toast when it fails, and resolves
// to whether it went through. A refusal core gives a reason for shows that
// reason.
// to whether it went through. When core names why it refused, the toast
// shows that reason.
export function useHookActions(options: {
type: DocumentHookType
nodeId: () => string | null

48
web/app/components/editor/hooks/hook-errors.test.ts
View File

@ -4,33 +4,37 @@ import { hookErrorMessage } from "./hook-errors"
describe("hookErrorMessage", () => {
it.for([
{
name: "a check that failed",
input: "document_hook.missing_repository",
expected: "editor.hooks.errors.codes.missing-repository",
name: "names the message of a check that failed",
input: { data: { code: "document_hook.missing_repository" } },
expected: "message:editor.hooks.errors.codes.missing-repository",
},
{
name: "settings the processor refused",
input: "document_hook.invalid_image",
expected: "editor.hooks.errors.codes.invalid-image",
name: "names the message of settings the processor refused",
input: { data: { code: "document_hook.invalid_image" } },
expected: "message:editor.hooks.errors.codes.invalid-image",
},
{
name: "a service the hook could not reach",
input: "document_hook.upstream_unavailable",
expected: "editor.hooks.errors.codes.upstream-unavailable",
name: "names the message of a service the hook could not reach",
input: { data: { code: "document_hook.upstream_unavailable" } },
expected: "message:editor.hooks.errors.codes.upstream-unavailable",
},
])("names the message of $name", ({ input, expected }, { expect }) => {
expect(hookErrorMessage({ data: { code: input } }, t)).toBe(
`message:${expected}`,
)
})
it("names nothing for an unknown code", ({ expect }) => {
expect(hookErrorMessage({ data: { code: "general" } }, t)).toBeUndefined()
})
it("names nothing for an error without a body", ({ expect }) => {
expect(hookErrorMessage(new Error("boom"), t)).toBeUndefined()
expect(hookErrorMessage(undefined, t)).toBeUndefined()
{
name: "names nothing for an unknown code",
input: { data: { code: "general" } },
expected: undefined,
},
{
name: "names nothing for an error without a body",
input: new Error("boom"),
expected: undefined,
},
{
name: "names nothing for a missing error",
input: undefined,
expected: undefined,
},
])("$name", ({ input, expected }, { expect }) => {
expect(hookErrorMessage(input, t)).toBe(expected)
})
})

2
web/app/components/editor/hooks/hook-errors.ts
View File

@ -1,4 +1,4 @@
// the refusals core answers a hook write with, keyed by error code.
// the message key for each code core refuses a hook write with.
const HOOK_ERROR_KEYS: Record<string, string> = {
"document_hook.unconfigured": "editor.hooks.errors.codes.unconfigured",
"document_hook.missing_installation":

11
web/app/components/editor/hooks/hook-status.test.ts
View File

@ -1,5 +1,10 @@
import { describe, it } from "vitest"
import { hookGroupStatus, hookStatus } from "./hook-status"
import {
hookGroupStatus,
hookStatus,
type HookGroupStatus,
type HookStatus,
} from "./hook-status"
import {
DocumentHookType,
type DocumentHook,
@ -14,7 +19,7 @@ describe("hookStatus", () => {
it.for<{
name: string
input: { score: string; status: DocumentHookStatus }
expected: string
expected: HookStatus
}>([
{
name: "shows a waiting hook as fresh",
@ -75,7 +80,7 @@ describe("hookGroupStatus", () => {
it.for<{
name: string
input: { score: string; status: DocumentHookStatus }[]
expected: string | null
expected: HookGroupStatus | null
}>([
{
name: "has no status without hooks",

2
web/app/components/editor/hooks/hook-status.ts
View File

@ -40,7 +40,7 @@ export const HOOK_BAR_STATUS_CLASS: Record<
// hookStatus puts a failed check first. A hook that cannot check what it
// watches needs fixing before its trigger means anything. A copy still
// being set up keeps the score its source had.
// being set up has not failed. It shows the score its source had.
export function hookStatus(hook: DocumentHook): HookStatus {
if (hook.status !== "active" && hook.status !== "initializing") {
return "needs-attention"

64
web/app/components/editor/hooks/url-watcher/ConfigMenu.nuxt.test.ts
View File

@ -94,27 +94,30 @@ describe("<URLWatcherConfigMenu>", { concurrent: false }, () => {
)
})
it("says the page of a failing hook is unreachable", async ({ expect }) => {
await mountMenu({
hook: urlHook({ state: {}, status: "unreachable_url" }),
})
it.for([
{
name: "an unreachable page",
input: "unreachable_url",
expected: "editor.hooks.url-watcher.detail-unreachable",
},
{
name: "missing change detection",
input: "unconfigured",
expected: "editor.hooks.url-watcher.detail-unconfigured",
},
] as const)(
"names $name on the hook's row",
async ({ input, expected }, { expect }) => {
await mountMenu({ hook: urlHook({ state: {}, status: input }) })
const detail = t("editor.hooks.url-watcher.detail-unreachable")
expect(menuText()).toContain(
t("editor.hooks.subtext-detail", { subtext: SHOWN_URL, detail: detail }),
)
})
it("says change detection is not set up for a hook that needs it", async ({
expect,
}) => {
await mountMenu({ hook: urlHook({ status: "unconfigured" }) })
const detail = t("editor.hooks.url-watcher.detail-unconfigured")
expect(menuText()).toContain(
t("editor.hooks.subtext-detail", { subtext: SHOWN_URL, detail: detail }),
)
})
expect(menuText()).toContain(
t("editor.hooks.subtext-detail", {
subtext: SHOWN_URL,
detail: t(expected),
}),
)
},
)
it("prefills the address of the hook it edits", async ({ expect }) => {
await mountMenu({ hook: urlHook() })
@ -180,17 +183,24 @@ describe("<URLWatcherConfigMenu>", { concurrent: false }, () => {
expect(hookNotice().querySelector(".font-semibold")).toBeNull()
})
it("warns about a site it cannot reach", async ({ expect }) => {
await mountMenu({
hook: urlHook({ state: {}, status: "unreachable_url" }),
})
it.for([
{
name: "a site it cannot reach",
input: "unreachable_url",
expected: "editor.hooks.url-watcher.unreachable-url",
},
{
name: "a server without change detection",
input: "unconfigured",
expected: "editor.hooks.url-watcher.unconfigured",
},
] as const)("warns about $name", async ({ input, expected }, { expect }) => {
await mountMenu({ hook: urlHook({ state: {}, status: input }) })
await openHookSubMenu(t(TITLE))
expect(hookNotice().dataset.hookStatus).toBe("needs-attention")
expect(hookNotice().textContent.trim()).toBe(
t("editor.hooks.url-watcher.unreachable-url"),
)
expect(hookNotice().textContent.trim()).toBe(t(expected))
})
it("keeps the create button out of reach until an address is typed", async ({

7
web/app/utils/api/document.ts
View File

@ -67,8 +67,8 @@ export interface DocumentHook {
settings: DocumentHookSettings
// null until the hook is set up
state: DocumentHookState | null
// whether the last check of what the hook watches worked. Score and
// state keep their last values while it is not active.
// active, initializing, or why the last check failed. Score and state
// keep their last values while a check fails.
status: DocumentHookStatus
score: string // decimal; between 0 and 100 (default: 100)
createdAt: Date | string
@ -120,8 +120,7 @@ export type DocumentHookStatus =
| DocumentHookStatusURLWatcher
| DocumentHookStatusContainerImageWatcher
// initializing marks a copy its first run has not set up yet. Any other
// status than active names why the last check failed.
// initializing marks a copied hook before its first run.
export type DocumentHookStatusScheduledReminder = "active" | "initializing"
export type DocumentHookStatusGitHubTracking =
| "active"

4
web/i18n/locales/en/editor.json
View File

@ -398,7 +398,7 @@
"reset-failed": "Failed to reset the hook. Please try again.",
"renew-failed": "Failed to renew the hook. Please try again."
},
"initializing": "Initializing this hook. It will be ready in a moment.",
"initializing": "Setting up this hook. It will be ready in a moment.",
"scheduled-reminder": {
"title": "Scheduled Reminder",
"description": "Nudge maintainers on a date",
@ -508,7 +508,7 @@
"triggered-notice": "{url} changed, so maintainers were notified.",
"url-fallback": "the webpage",
"unreachable-url": "The selected webpage cannot be reached. They may be temporarily down, no longer exist, or have anti-scraping measures in place.",
"unconfigured": "Website change detection is not set up on this server, so changes cannot be detected.",
"unconfigured": "Website change detection is not set up on this server, so this hook cannot check the page.",
"url-input-label": "Link to Website",
"url-input-placeholder": "https://example.com"
},

2
web/i18n/locales/en/notification.json
View File

@ -12,7 +12,7 @@
"document-new-comment-reply-description": "{user} has posted a reply to a comment",
"document-review-request-description": "Review requested for the draft version",
"document-hook-triggered-description": "{hook} hook was triggered",
"document-hook-needs-attention-description": "{hook} hook can no longer check its target",
"document-hook-needs-attention-description": "{hook} hook needs attention",
"default-description": "You have a new notification"
},
"actions": {