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

improve hooks error handling

Browse Source
This commit is contained in:
david 2026-10-04 18:19:07 +03:00
parent fddc2c99b7
commit 95a7c344b3
30 changed files with 519 additions and 527 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
server/auth-realtime/src/core.test.ts
View File

@ -363,6 +363,30 @@ describe("createCoreClient", () => {
{ headers },
)
})
it.for([
{ name: "the current", input: "." },
{ name: "the parent", input: ".." },
])(
"refuses an id naming $name path",
async ({ input }, { expect }) => {
const http = stubHttp({ status: 200, data: {} })
await expect(
createCoreClient(
BASE_URL,
http,
).updateBranch(
"doc-1",
input,
{ body: {} },
{ headers: new AxiosHeaders() },
),
).rejects.toThrow(`invalid path id "${input}"`)
expect(http.put).not.toHaveBeenCalled()
},
)
})
describe("deleteBranch", () => {

24
server/auth-realtime/src/core.ts
View File

@ -238,10 +238,24 @@ export function createCoreClient(
}
}
// urlOn returns a template tag that appends a path to base, encoding each
// interpolated id so one holding "/" or ".." stays in its own segment.
// urlOn returns a template tag that appends a path to base. Each
// interpolated id is encoded, so one holding "/" stays in its own segment.
// Encoding keeps dots, so an id of "." or ".." would name another path and
// is refused.
function urlOn(base: string) {
return (strings: TemplateStringsArray, ...ids: string[]): string =>
base +
String.raw({ raw: strings }, ...ids.map(encodeURIComponent))
return (strings: TemplateStringsArray, ...ids: string[]): string => {
for (const id of ids) {
if (id === "." || id === "..") {
throw new Error(`invalid path id "${id}"`)
}
}
return (
base +
String.raw(
{ raw: strings },
...ids.map(encodeURIComponent),
)
)
}
}

6
server/auth-realtime/src/routes.ts
View File

@ -663,14 +663,14 @@ export function createRoutes({
app.post(
"/internal/documents/:documentId/branches/:branchId/flush",
async (c) => {
const branch = await flushed(c, () =>
const flush = await flushed(c, () =>
flushBranch(
c.req.param("documentId"),
c.req.param("branchId"),
),
)
if (branch instanceof Response) {
return branch
if (flush instanceof Response) {
return flush
}
return c.body(null, 204)

5
server/core/internal/apps/registry/client.go
View File

@ -21,9 +21,6 @@ var (
// ErrNotFound is returned when the registry has no such image or tag.
ErrNotFound = errutil.New(http.StatusNotFound, "registry.not_found", "container image not found")
// ErrInvalidReference is returned when an image reference cannot be parsed.
ErrInvalidReference = errutil.New(http.StatusBadRequest, "registry.invalid_reference", "invalid container image reference")
)
// digestOptions holds options for configuring the Digest function.
@ -130,7 +127,7 @@ func parseReference(image string) (name.Reference, error) {
name.WithDefaultTag("latest"),
)
if err != nil {
return nil, ErrInvalidReference
return nil, fmt.Errorf("parsing image reference %q: %w", image, err)
}
return ref, nil

37
server/core/internal/apps/registry/client_test.go
View File

@ -13,6 +13,7 @@ import (
"github.com/google/go-containerregistry/pkg/registry"
"github.com/google/go-containerregistry/pkg/v1/random"
"github.com/google/go-containerregistry/pkg/v1/remote"
"github.com/oxynote/oxynote/server/core/pkg/testutil"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
@ -82,11 +83,12 @@ func Test_Digest(t *testing.T) {
Path string
Opts []DigestOption
ExpectedErr string
ErrContains string
}{
"Anonymous fetch returns the seeded digest": {},
"Invalid image reference": {
Reference: "UPPERCASE not allowed",
ExpectedErr: ErrInvalidReference.Error(),
ErrContains: "parsing image reference",
},
"Missing tag maps to ErrNotFound": {
Path: "/test/img:v2",
@ -138,6 +140,13 @@ func Test_Digest(t *testing.T) {
digest, err := Digest(context.Background(), reference, tc.Opts...)
if tc.ErrContains != "" {
require.Error(t, err)
assert.Contains(t, err.Error(), tc.ErrContains)
return
}
if tc.ExpectedErr != "" {
require.Error(t, err)
assert.EqualError(t, err, tc.ExpectedErr)
@ -154,7 +163,27 @@ func Test_Digest(t *testing.T) {
func Test_ValidateReference(t *testing.T) {
t.Parallel()
assert.NoError(t, ValidateReference("nginx"))
assert.NoError(t, ValidateReference("ghcr.io/owner/image:v1"))
assert.ErrorIs(t, ValidateReference("UPPERCASE not allowed"), ErrInvalidReference)
cc := map[string]struct {
Image string
Err error
}{
"Invalid reference": {
Image: "UPPERCASE not allowed",
Err: assert.AnError,
},
"Docker Hub image": {
Image: "nginx",
},
"Image of another registry": {
Image: "ghcr.io/owner/image:v1",
},
}
for cn, c := range cc {
t.Run(cn, func(t *testing.T) {
t.Parallel()
testutil.AssertEqualError(t, c.Err, ValidateReference(c.Image))
})
}
}

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

@ -36,11 +36,9 @@ for free.
tag-only writes record none).
- **Tag and hook writes notify**: tag tools end with
`Input.NotifyTagTreeChange` (`assign_tag`/`unassign_tag` also
`NotifyBranchTagsChange`), hook writes call `NotifyHooksChange`, wired in
`server.NewServer` beside the tree notifier. Hook writes drive the same
`hook.NewHook`/`ApplyUpdate`/`Reset`/`Delete` lifecycle as the HTTP
handler; `decodeHookSettings` switches on type into the processor's own
struct.
`NotifyBranchTagsChange`). Hook writes go through `HookManager`, which
records history and announces the change, as for the HTTP handler;
`decodeHookSettings` switches on type into the processor's own struct.
- **A block write reports from the operation, never a re-read** (the
realtime service persists on a debounce): `sanitizeBlock` once, ship the
canonical uids, return `blockRows` of the expanded tree.

42
server/core/internal/assistant/tools/0mock_hook_manager_test.go
View File

@ -24,13 +24,13 @@ var _ HookManager = &HookManagerMock{}
// CreateHookFunc: func(ctx context.Context, ci hook.CreateInput, documentID xid.ID, organizationID string, updatedBy string) (*hook.Hook, error) {
// panic("mock out the CreateHook method")
// },
// DeleteHookFunc: func(ctx context.Context, id xid.ID, organizationID string, updatedBy string) error {
// DeleteHookFunc: func(ctx context.Context, id xid.ID, documentID xid.ID, organizationID string, updatedBy string) error {
// panic("mock out the DeleteHook method")
// },
// ResetHookFunc: func(ctx context.Context, id xid.ID, organizationID string) (*hook.Hook, error) {
// ResetHookFunc: func(ctx context.Context, id xid.ID, documentID xid.ID, organizationID string) (*hook.Hook, error) {
// panic("mock out the ResetHook method")
// },
// UpdateHookFunc: func(ctx context.Context, id xid.ID, organizationID string, ui hook.UpdateInput, updatedBy string) (*hook.Hook, error) {
// UpdateHookFunc: func(ctx context.Context, id xid.ID, documentID xid.ID, organizationID string, ui hook.UpdateInput, updatedBy string) (*hook.Hook, error) {
// panic("mock out the UpdateHook method")
// },
// }
@ -44,13 +44,13 @@ type HookManagerMock struct {
CreateHookFunc func(ctx context.Context, ci hook.CreateInput, documentID xid.ID, organizationID string, updatedBy string) (*hook.Hook, error)
// DeleteHookFunc mocks the DeleteHook method.
DeleteHookFunc func(ctx context.Context, id xid.ID, organizationID string, updatedBy string) error
DeleteHookFunc func(ctx context.Context, id xid.ID, documentID xid.ID, organizationID string, updatedBy string) error
// ResetHookFunc mocks the ResetHook method.
ResetHookFunc func(ctx context.Context, id xid.ID, organizationID string) (*hook.Hook, error)
ResetHookFunc func(ctx context.Context, id xid.ID, documentID xid.ID, organizationID string) (*hook.Hook, error)
// UpdateHookFunc mocks the UpdateHook method.
UpdateHookFunc func(ctx context.Context, id xid.ID, organizationID string, ui hook.UpdateInput, updatedBy string) (*hook.Hook, error)
UpdateHookFunc func(ctx context.Context, id xid.ID, documentID xid.ID, organizationID string, ui hook.UpdateInput, updatedBy string) (*hook.Hook, error)
// calls tracks calls to the methods.
calls struct {
@ -73,6 +73,8 @@ type HookManagerMock struct {
Ctx context.Context
// ID is the id argument value.
ID xid.ID
// DocumentID is the documentID argument value.
DocumentID xid.ID
// OrganizationID is the organizationID argument value.
OrganizationID string
// UpdatedBy is the updatedBy argument value.
@ -84,6 +86,8 @@ type HookManagerMock struct {
Ctx context.Context
// ID is the id argument value.
ID xid.ID
// DocumentID is the documentID argument value.
DocumentID xid.ID
// OrganizationID is the organizationID argument value.
OrganizationID string
}
@ -93,6 +97,8 @@ type HookManagerMock struct {
Ctx context.Context
// ID is the id argument value.
ID xid.ID
// DocumentID is the documentID argument value.
DocumentID xid.ID
// OrganizationID is the organizationID argument value.
OrganizationID string
// UI is the ui argument value.
@ -160,15 +166,17 @@ func (mock *HookManagerMock) CreateHookCalls() []struct {
}
// DeleteHook calls DeleteHookFunc.
func (mock *HookManagerMock) DeleteHook(ctx context.Context, id xid.ID, organizationID string, updatedBy string) error {
func (mock *HookManagerMock) DeleteHook(ctx context.Context, id xid.ID, documentID xid.ID, organizationID string, updatedBy string) error {
callInfo := struct {
Ctx context.Context
ID xid.ID
DocumentID xid.ID
OrganizationID string
UpdatedBy string
}{
Ctx: ctx,
ID: id,
DocumentID: documentID,
OrganizationID: organizationID,
UpdatedBy: updatedBy,
}
@ -181,7 +189,7 @@ func (mock *HookManagerMock) DeleteHook(ctx context.Context, id xid.ID, organiza
)
return errOut
}
return mock.DeleteHookFunc(ctx, id, organizationID, updatedBy)
return mock.DeleteHookFunc(ctx, id, documentID, organizationID, updatedBy)
}
// DeleteHookCalls gets all the calls that were made to DeleteHook.
@ -191,12 +199,14 @@ func (mock *HookManagerMock) DeleteHook(ctx context.Context, id xid.ID, organiza
func (mock *HookManagerMock) DeleteHookCalls() []struct {
Ctx context.Context
ID xid.ID
DocumentID xid.ID
OrganizationID string
UpdatedBy string
} {
var calls []struct {
Ctx context.Context
ID xid.ID
DocumentID xid.ID
OrganizationID string
UpdatedBy string
}
@ -207,14 +217,16 @@ func (mock *HookManagerMock) DeleteHookCalls() []struct {
}
// ResetHook calls ResetHookFunc.
func (mock *HookManagerMock) ResetHook(ctx context.Context, id xid.ID, organizationID string) (*hook.Hook, error) {
func (mock *HookManagerMock) ResetHook(ctx context.Context, id xid.ID, documentID xid.ID, organizationID string) (*hook.Hook, error) {
callInfo := struct {
Ctx context.Context
ID xid.ID
DocumentID xid.ID
OrganizationID string
}{
Ctx: ctx,
ID: id,
DocumentID: documentID,
OrganizationID: organizationID,
}
mock.lockResetHook.Lock()
@ -227,7 +239,7 @@ func (mock *HookManagerMock) ResetHook(ctx context.Context, id xid.ID, organizat
)
return hookOut, errOut
}
return mock.ResetHookFunc(ctx, id, organizationID)
return mock.ResetHookFunc(ctx, id, documentID, organizationID)
}
// ResetHookCalls gets all the calls that were made to ResetHook.
@ -237,11 +249,13 @@ func (mock *HookManagerMock) ResetHook(ctx context.Context, id xid.ID, organizat
func (mock *HookManagerMock) ResetHookCalls() []struct {
Ctx context.Context
ID xid.ID
DocumentID xid.ID
OrganizationID string
} {
var calls []struct {
Ctx context.Context
ID xid.ID
DocumentID xid.ID
OrganizationID string
}
mock.lockResetHook.RLock()
@ -251,16 +265,18 @@ func (mock *HookManagerMock) ResetHookCalls() []struct {
}
// UpdateHook calls UpdateHookFunc.
func (mock *HookManagerMock) UpdateHook(ctx context.Context, id xid.ID, organizationID string, ui hook.UpdateInput, updatedBy string) (*hook.Hook, error) {
func (mock *HookManagerMock) UpdateHook(ctx context.Context, id xid.ID, documentID xid.ID, organizationID string, ui hook.UpdateInput, updatedBy string) (*hook.Hook, error) {
callInfo := struct {
Ctx context.Context
ID xid.ID
DocumentID xid.ID
OrganizationID string
UI hook.UpdateInput
UpdatedBy string
}{
Ctx: ctx,
ID: id,
DocumentID: documentID,
OrganizationID: organizationID,
UI: ui,
UpdatedBy: updatedBy,
@ -275,7 +291,7 @@ func (mock *HookManagerMock) UpdateHook(ctx context.Context, id xid.ID, organiza
)
return hookOut, errOut
}
return mock.UpdateHookFunc(ctx, id, organizationID, ui, updatedBy)
return mock.UpdateHookFunc(ctx, id, documentID, organizationID, ui, updatedBy)
}
// UpdateHookCalls gets all the calls that were made to UpdateHook.
@ -285,6 +301,7 @@ func (mock *HookManagerMock) UpdateHook(ctx context.Context, id xid.ID, organiza
func (mock *HookManagerMock) UpdateHookCalls() []struct {
Ctx context.Context
ID xid.ID
DocumentID xid.ID
OrganizationID string
UI hook.UpdateInput
UpdatedBy string
@ -292,6 +309,7 @@ func (mock *HookManagerMock) UpdateHookCalls() []struct {
var calls []struct {
Ctx context.Context
ID xid.ID
DocumentID xid.ID
OrganizationID string
UI hook.UpdateInput
UpdatedBy string

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

@ -58,7 +58,7 @@ func stubHookManager(stored ...*hook.Hook) *HookManagerMock {
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) {
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
@ -66,10 +66,10 @@ func stubHookManager(stored ...*hook.Hook) *HookManagerMock {
return hk, nil
},
DeleteHookFunc: func(ctx context.Context, _ xid.ID, organizationID, _ string) error {
DeleteHookFunc: func(ctx context.Context, _, _ xid.ID, organizationID, _ string) error {
return lookup().Delete(ctx, inp(organizationID))
},
ResetHookFunc: func(ctx context.Context, _ xid.ID, organizationID string) (*hook.Hook, error) {
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
@ -703,7 +703,7 @@ func Test_updateHook_Execute(t *testing.T) {
t.Parallel()
failing := stubHookManager()
failing.UpdateHookFunc = func(context.Context, xid.ID, string, hook.UpdateInput, string) (*hook.Hook, error) {
failing.UpdateHookFunc = func(context.Context, xid.ID, xid.ID, string, hook.UpdateInput, string) (*hook.Hook, error) {
return nil, assert.AnError
}
@ -863,7 +863,7 @@ func Test_resetHook_Execute(t *testing.T) {
t.Parallel()
failing := stubHookManager()
failing.ResetHookFunc = func(context.Context, xid.ID, string) (*hook.Hook, error) {
failing.ResetHookFunc = func(context.Context, xid.ID, xid.ID, string) (*hook.Hook, error) {
return nil, assert.AnError
}
@ -976,7 +976,7 @@ func Test_deleteHook_Execute(t *testing.T) {
t.Parallel()
failing := stubHookManager()
failing.DeleteHookFunc = func(context.Context, xid.ID, string, string) error {
failing.DeleteHookFunc = func(context.Context, xid.ID, xid.ID, string, string) error {
return assert.AnError
}

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

@ -1033,11 +1033,9 @@ func (i *input) unknownHook(documentID, hookID xid.ID) error {
}
// CreateHook creates a hook on the branch branchID names, anchored to the
// block blockUID names when one is given. The branch and block are
// checked here rather than by the caller, so a hook cannot land on a
// branch of another document or on a block the branch does not hold. A
// type whose integration the deployment lacks is refused before anything
// is created.
// block blockUID names when one is given. The branch is checked here, so
// a hook cannot land on a branch of another document. A type whose
// integration the deployment lacks is refused before anything is created.
func (i *input) CreateHook(documentID, branchID xid.ID, blockUID string, tp hook.Type, settings processor.Settings) (*hook.Hook, error) {
doc, err := i.FetchBranch(documentID, branchID)
if err != nil {
@ -1081,7 +1079,7 @@ func (i *input) CreateHook(documentID, branchID xid.ID, blockUID string, tp hook
// UpdateHook replaces the hook's settings and resets its score and state,
// as a fresh hook with those settings would have them.
func (i *input) UpdateHook(hk *hook.Hook, settings processor.Settings) error {
updated, err := i.hookMan.UpdateHook(i.ctx, hk.ID, i.orgID, hook.UpdateInput{Settings: settings}, i.userID)
updated, err := i.hookMan.UpdateHook(i.ctx, hk.ID, hk.DocumentID.V, i.orgID, hook.UpdateInput{Settings: settings}, i.userID)
if err != nil {
return err
}
@ -1095,7 +1093,7 @@ func (i *input) UpdateHook(hk *hook.Hook, settings processor.Settings) error {
// ResetHook restores the hook's score and state, keeping its settings.
func (i *input) ResetHook(hk *hook.Hook) error {
reset, err := i.hookMan.ResetHook(i.ctx, hk.ID, i.orgID)
reset, err := i.hookMan.ResetHook(i.ctx, hk.ID, hk.DocumentID.V, i.orgID)
if err != nil {
return err
}
@ -1111,7 +1109,7 @@ func (i *input) ResetHook(hk *hook.Hook) error {
// removes its row. The branch stays, and the editor showing it has to
// redraw, so the delete records the branch as touched.
func (i *input) DeleteHook(hk *hook.Hook) error {
if err := i.hookMan.DeleteHook(i.ctx, hk.ID, i.orgID, i.userID); err != nil {
if err := i.hookMan.DeleteHook(i.ctx, hk.ID, hk.DocumentID.V, i.orgID, i.userID); err != nil {
return err
}
@ -1289,17 +1287,17 @@ type HookManager interface {
// credited to updatedBy.
CreateHook(ctx context.Context, ci hook.CreateInput, documentID xid.ID, organizationID, updatedBy string) (*hook.Hook, error)
// UpdateHook should replace the hook's settings, credited to
// updatedBy, and return the stored hook.
UpdateHook(ctx context.Context, id xid.ID, organizationID string, ui hook.UpdateInput, updatedBy string) (*hook.Hook, error)
// UpdateHook should replace the settings of the document's hook,
// credited to updatedBy, and return the stored hook.
UpdateHook(ctx context.Context, id, documentID xid.ID, organizationID string, ui hook.UpdateInput, updatedBy string) (*hook.Hook, error)
// DeleteHook should tear the hook down and remove it, credited to
// updatedBy.
DeleteHook(ctx context.Context, id xid.ID, organizationID, updatedBy string) error
// DeleteHook should tear the document's hook down and remove it,
// credited to updatedBy.
DeleteHook(ctx context.Context, id, documentID xid.ID, organizationID, updatedBy string) error
// ResetHook should restore the hook's score and state and return the
// stored hook.
ResetHook(ctx context.Context, id xid.ID, organizationID string) (*hook.Hook, error)
// ResetHook should reset the document's hook and return the stored
// hook.
ResetHook(ctx context.Context, id, documentID xid.ID, organizationID string) (*hook.Hook, error)
}
// SearchTrigger runs the search-job worker once a job has committed.

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

@ -2530,7 +2530,7 @@ func Test_input_UpdateHook(t *testing.T) {
t.Parallel()
failing := stubHookManager()
failing.UpdateHookFunc = func(context.Context, xid.ID, string, hook.UpdateInput, string) (*hook.Hook, error) {
failing.UpdateHookFunc = func(context.Context, xid.ID, xid.ID, string, hook.UpdateInput, string) (*hook.Hook, error) {
return nil, assert.AnError
}
@ -2602,7 +2602,7 @@ func Test_input_ResetHook(t *testing.T) {
t.Parallel()
failing := stubHookManager()
failing.ResetHookFunc = func(context.Context, xid.ID, string) (*hook.Hook, error) {
failing.ResetHookFunc = func(context.Context, xid.ID, xid.ID, string) (*hook.Hook, error) {
return nil, assert.AnError
}
@ -2669,7 +2669,7 @@ func Test_input_DeleteHook(t *testing.T) {
t.Parallel()
failing := stubHookManager()
failing.DeleteHookFunc = func(context.Context, xid.ID, string, string) error {
failing.DeleteHookFunc = func(context.Context, xid.ID, xid.ID, string, string) error {
return assert.AnError
}

6
server/core/internal/db/migrations/002_history_entry_snapshot.sql
View File

@ -1,5 +1,9 @@
-- +migrate Up
-- History entries become restorable snapshots: they carry the branch
-- name, icon, author and hook definitions and drop the Yjs binary. The
-- existing rows predate that shape and nothing reads them, so they are
-- dropped rather than backfilled.
TRUNCATE document_branch_history_entries;
ALTER TABLE document_branch_history_entries
@ -11,6 +15,8 @@ ALTER TABLE document_branch_history_entries
ADD COLUMN boundary BOOLEAN NOT NULL;
CREATE INDEX document_branch_history_entries_fk_last_updated_by_idx ON document_branch_history_entries (fk_last_updated_by);
-- Entries are read per branch, newest first, and expired by age; the
-- composite index also covers the plain branch lookup it replaces.
DROP INDEX document_branch_history_entries_fk_branch_id_idx;
CREATE INDEX document_branch_history_entries_fk_branch_id_created_at_idx ON document_branch_history_entries (fk_branch_id, created_at DESC, id DESC);
CREATE INDEX document_branch_history_entries_created_at_idx ON document_branch_history_entries (created_at);

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

@ -31,6 +31,10 @@ var ErrUpstreamUnavailable = errutil.New(http.StatusFailedDependency, "document_
// another document.
var ErrBranchMismatch = errutil.New(http.StatusNotFound, "document.branch_mismatch", "branch does not belong to the document")
// ErrHookMismatch is returned when a hook is addressed under a document it
// does not belong to.
var ErrHookMismatch = errutil.New(http.StatusNotFound, "document.hook_mismatch", "hook does not belong to the document")
// ErrBlockNotFound is returned when a hook is to be anchored to a block the
// branch's content does not hold.
var ErrBlockNotFound = errutil.New(http.StatusNotFound, "document.hook_block_not_found", "block not found in the branch")
@ -317,12 +321,7 @@ func (h *Hook) setUp(ctx context.Context, inp *Input) error {
return nil
}
return errutil.New(
http.StatusUnprocessableEntity,
"document_hook."+string(h.Status),
"the hook cannot check its target: %s",
h.Status,
)
return newStatusError(h.Status)
}
// ensurePrepared prepares the hook for processing.
@ -403,6 +402,31 @@ type UpdateInput struct {
Settings processor.Settings `json:"settings"`
}
// newStatusError returns the error a hook is refused with when its check
// failed with the given status.
func newStatusError(status processor.Status) error {
switch status {
case processor.StatusUnconfigured:
return errutil.New(http.StatusUnprocessableEntity, "document_hook.unconfigured", "the integration the hook needs is not configured")
case processor.StatusMissingInstallation:
return errutil.New(http.StatusUnprocessableEntity, "document_hook.missing_installation", "the organization has no github app installation")
case processor.StatusMissingRepository:
return errutil.New(http.StatusUnprocessableEntity, "document_hook.missing_repository", "the github repository was not found")
case processor.StatusMissingBranch:
return errutil.New(http.StatusUnprocessableEntity, "document_hook.missing_branch", "the github branch was not found")
case processor.StatusTreeTruncated:
return errutil.New(http.StatusUnprocessableEntity, "document_hook.tree_truncated", "the github repository is too large to compare")
case processor.StatusUnreachableURL:
return errutil.New(http.StatusUnprocessableEntity, "document_hook.unreachable_url", "the url cannot be reached")
case processor.StatusUnauthorized:
return errutil.New(http.StatusUnprocessableEntity, "document_hook.unauthorized", "the container registry refused access")
case processor.StatusImageNotFound:
return errutil.New(http.StatusUnprocessableEntity, "document_hook.image_not_found", "the container image was not found")
default:
return errutil.New(http.StatusUnprocessableEntity, "document_hook.cannot_check", "the hook cannot check its target")
}
}
// runner is an interface that defines the methods for applying a freshness hook.
type runner interface {
// Validate checks the settings without reaching any service.

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

@ -123,7 +123,7 @@ func Test_NewHook(t *testing.T) {
}, 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 hook cannot check its target: unconfigured")
assert.EqualError(t, err, "the integration the hook needs is not configured")
})
}
@ -417,6 +417,60 @@ func Test_Hook_ensurePrepared(t *testing.T) {
}
}
func Test_newStatusError(t *testing.T) {
t.Parallel()
cc := map[string]struct {
Status processor.Status
Err error
}{
"Unconfigured": {
Status: processor.StatusUnconfigured,
Err: errutil.New(http.StatusUnprocessableEntity, "document_hook.unconfigured", "the integration the hook needs is not configured"),
},
"Missing installation": {
Status: processor.StatusMissingInstallation,
Err: errutil.New(http.StatusUnprocessableEntity, "document_hook.missing_installation", "the organization has no github app installation"),
},
"Missing repository": {
Status: processor.StatusMissingRepository,
Err: errutil.New(http.StatusUnprocessableEntity, "document_hook.missing_repository", "the github repository was not found"),
},
"Missing branch": {
Status: processor.StatusMissingBranch,
Err: errutil.New(http.StatusUnprocessableEntity, "document_hook.missing_branch", "the github branch was not found"),
},
"Tree truncated": {
Status: processor.StatusTreeTruncated,
Err: errutil.New(http.StatusUnprocessableEntity, "document_hook.tree_truncated", "the github repository is too large to compare"),
},
"Unreachable url": {
Status: processor.StatusUnreachableURL,
Err: errutil.New(http.StatusUnprocessableEntity, "document_hook.unreachable_url", "the url cannot be reached"),
},
"Unauthorized": {
Status: processor.StatusUnauthorized,
Err: errutil.New(http.StatusUnprocessableEntity, "document_hook.unauthorized", "the container registry refused access"),
},
"Image not found": {
Status: processor.StatusImageNotFound,
Err: errutil.New(http.StatusUnprocessableEntity, "document_hook.image_not_found", "the container image was not found"),
},
"Unknown status": {
Status: processor.Status("bogus"),
Err: errutil.New(http.StatusUnprocessableEntity, "document_hook.cannot_check", "the hook cannot check its target"),
},
}
for cn, c := range cc {
t.Run(cn, func(t *testing.T) {
t.Parallel()
testutil.AssertEqualError(t, c.Err, newStatusError(c.Status))
})
}
}
func Test_Type_Validate(t *testing.T) {
t.Parallel()

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

@ -251,7 +251,7 @@ func (m *Manager) runHook(ctx context.Context, doc *document.Document, h *hook.H
// The mark is still stored, since it starts the retention clock.
if !h.SoftDeletedAt.Valid {
if err := h.Process(ctx, m.input(h.OrganizationID.String)); err != nil {
// a transient failure keeps the stored state.
// a failed run keeps the stored score and state.
m.log.With("hook_id", h.ID).
With("error", err).
Error("processing document hook")

104
server/core/internal/document/hook/manager/write.go
View File

@ -2,11 +2,14 @@ package manager
import (
"context"
"errors"
"fmt"
"log/slog"
"net/http"
"github.com/guregu/null/v5"
"github.com/oxynote/oxynote/server/core/internal/document/hook"
"github.com/oxynote/oxynote/server/core/internal/document/hook/processor"
"github.com/oxynote/oxynote/server/core/pkg/errutil"
"github.com/rs/xid"
)
@ -50,16 +53,7 @@ func (m *Manager) CreateHook(
hk, err := hook.NewHook(ctx, ci, documentID, ci.BranchID, organizationID, m.input(organizationID))
if err != nil {
// a refusal the hook reports passes through. A service it could
// not reach is logged here, since the caller only sees the
// sentinel.
if errutil.StatusCode(err, false) >= http.StatusInternalServerError {
m.log.With("error", err).Warn("cannot reach the service a hook checks")
return nil, hook.ErrUpstreamUnavailable
}
return nil, err
return nil, m.hookErr(err)
}
defer func() {
@ -95,12 +89,13 @@ func (m *Manager) CreateHook(
// UpdateHook applies the new settings, which resets the hook's state, then
// stores the hook and records its branch in history, credited to
// updatedBy. It fails unless the hook can check its target with the new
// settings. The outside calls run before the transaction opens, so no
// connection waits on them.
// updatedBy. It fails unless the hook belongs to the document and can
// check its target with the new settings. The outside calls run before the
// transaction opens, so no connection waits on them.
func (m *Manager) UpdateHook(
ctx context.Context,
id xid.ID,
documentID xid.ID,
organizationID string,
ui hook.UpdateInput,
updatedBy string,
@ -120,7 +115,13 @@ func (m *Manager) UpdateHook(
return nil, err
}
if hk.DocumentID.Valid && hk.BranchID.Valid {
// a row whose document was deleted carries no document id and is not
// addressable through any document.
if !hk.DocumentID.Valid || hk.DocumentID.V != documentID {
return nil, hook.ErrHookMismatch
}
if hk.BranchID.Valid {
if err = m.flusher.Flush(ctx, hk.DocumentID.V, hk.BranchID.V); err != nil {
return nil, fmt.Errorf("storing the branch's pending edits: %w", err)
}
@ -135,16 +136,7 @@ func (m *Manager) UpdateHook(
}()
if err = hk.ApplyUpdate(ctx, ui, m.input(organizationID)); err != nil {
// a refusal the hook reports passes through. A service it could
// not reach is logged here, since the caller only sees the
// sentinel.
if errutil.StatusCode(err, false) >= http.StatusInternalServerError {
m.log.With("error", err).Warn("cannot reach the service a hook checks")
return nil, hook.ErrUpstreamUnavailable
}
return nil, err
return nil, m.hookErr(err)
}
var tx Tx
@ -175,8 +167,9 @@ func (m *Manager) UpdateHook(
}
// DeleteHook tears down the hook's external resource, then removes the row
// and records its branch in history, credited to updatedBy.
func (m *Manager) DeleteHook(ctx context.Context, id xid.ID, organizationID, updatedBy string) error {
// and records its branch in history, credited to updatedBy. It fails unless
// the hook belongs to the document.
func (m *Manager) DeleteHook(ctx context.Context, id, documentID xid.ID, organizationID, updatedBy string) error {
ctx, cancel := context.WithTimeout(ctx, _hookTimeout)
defer cancel()
@ -192,23 +185,20 @@ func (m *Manager) DeleteHook(ctx context.Context, id xid.ID, organizationID, upd
return err
}
if hk.DocumentID.Valid && hk.BranchID.Valid {
// a row whose document was deleted carries no document id and is not
// addressable through any document.
if !hk.DocumentID.Valid || hk.DocumentID.V != documentID {
return hook.ErrHookMismatch
}
if hk.BranchID.Valid {
if err = m.flusher.Flush(ctx, hk.DocumentID.V, hk.BranchID.V); err != nil {
return fmt.Errorf("storing the branch's pending edits: %w", err)
}
}
if err = hk.Delete(ctx, m.input(organizationID)); err != nil {
// a refusal the hook reports passes through. A service it could
// not reach is logged here, since the caller only sees the
// sentinel.
if errutil.StatusCode(err, false) >= http.StatusInternalServerError {
m.log.With("error", err).Warn("cannot reach the service a hook checks")
return hook.ErrUpstreamUnavailable
}
return err
return m.hookErr(err)
}
var tx Tx
@ -238,10 +228,11 @@ func (m *Manager) DeleteHook(ctx context.Context, id xid.ID, organizationID, upd
return nil
}
// ResetHook restores the hook's score and state and stores them. History
// holds no watcher state, so nothing is recorded. A hook that cannot check
// its target is stored with that status, and its maintainers are told.
func (m *Manager) ResetHook(ctx context.Context, id xid.ID, organizationID string) (_ *hook.Hook, err error) {
// ResetHook takes the target as it is now as the hook's new baseline and
// stores it. History holds no watcher state, so nothing is recorded. A hook
// that cannot check its target is stored with that status, and its
// maintainers are told. It fails unless the hook belongs to the document.
func (m *Manager) ResetHook(ctx context.Context, id, documentID xid.ID, organizationID string) (_ *hook.Hook, err error) {
ctx, cancel := context.WithTimeout(ctx, _hookTimeout)
defer cancel()
@ -257,6 +248,12 @@ func (m *Manager) ResetHook(ctx context.Context, id xid.ID, organizationID strin
return nil, err
}
// a row whose document was deleted carries no document id and is not
// addressable through any document.
if !hk.DocumentID.Valid || hk.DocumentID.V != documentID {
return nil, hook.ErrHookMismatch
}
prev := *hk
defer func() {
@ -266,16 +263,7 @@ func (m *Manager) ResetHook(ctx context.Context, id xid.ID, organizationID strin
}()
if err = hk.Reset(ctx, m.input(organizationID)); err != nil {
// a refusal the hook reports passes through. A service it could
// not reach is logged here, since the caller only sees the
// sentinel.
if errutil.StatusCode(err, false) >= http.StatusInternalServerError {
m.log.With("error", err).Warn("cannot reach the service a hook checks")
return nil, hook.ErrUpstreamUnavailable
}
return nil, err
return nil, m.hookErr(err)
}
if err = m.db.UpdateDocumentHook(ctx, *hk); err != nil {
@ -287,3 +275,17 @@ func (m *Manager) ResetHook(ctx context.Context, id xid.ID, organizationID strin
return hk, nil
}
// hookErr returns a hook's error as a write's caller sees it. A service
// the hook could not reach becomes ErrUpstreamUnavailable, logged here
// since the caller sees only the sentinel. A refusal with its own status
// and any other error pass through.
func (m *Manager) hookErr(err error) error {
if !errors.Is(err, processor.ErrUnreachable) || errutil.StatusCode(err, false) < http.StatusInternalServerError {
return err
}
m.log.Warn("cannot reach the service a hook checks", slog.String("error", err.Error()))
return hook.ErrUpstreamUnavailable
}

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

@ -111,7 +111,7 @@ func Test_Manager_CreateHook(t *testing.T) {
Type: hook.TypeURLWatcher,
Settings: processor.Settings(`{"url":"https://example.com"}`),
Tx: &TxMock{},
Err: errutil.New(http.StatusUnprocessableEntity, "document_hook.unconfigured", "the hook cannot check its target: %s", processor.StatusUnconfigured),
Err: errutil.New(http.StatusUnprocessableEntity, "document_hook.unconfigured", "the integration the hook needs is not configured"),
},
"Service the hook checks is unreachable": {
Type: hook.TypeURLWatcher,
@ -156,8 +156,7 @@ func Test_Manager_CreateHook(t *testing.T) {
Watchers: []string{"w-new"},
Err: assert.AnError,
},
// the insert is rolled back, so the watcher the hook created is torn
// down again.
// the hook was not stored, so the watcher it created is torn down.
"Error returned by tx.InsertDocumentHook": {
Type: hook.TypeURLWatcher,
Settings: processor.Settings(`{"url":"https://example.com"}`),
@ -290,20 +289,21 @@ func Test_Manager_UpdateHook(t *testing.T) {
cc := map[string]struct {
// Held has another write hold the hook's lock until the update
// gives up.
Held bool
Branchless bool
Unreachable bool
Settings processor.Settings
FetchErr error
FlushErr error
BeginErr error
UpdateErr error
RecordErr error
CommitErr error
Updates int
Records int
Watchers []string
Err error
Held bool
Branchless bool
OtherDocument bool
Unreachable bool
Settings processor.Settings
FetchErr error
FlushErr error
BeginErr error
UpdateErr error
RecordErr error
CommitErr error
Updates int
Records int
Watchers []string
Err error
}{
"Hook another write holds": {
Held: true,
@ -315,6 +315,11 @@ func Test_Manager_UpdateHook(t *testing.T) {
FetchErr: sql.ErrNoRows,
Err: sql.ErrNoRows,
},
"Hook of another document": {
Settings: settings,
OtherDocument: true,
Err: hook.ErrHookMismatch,
},
"Error returned by flusher.Flush": {
Settings: settings,
FlushErr: assert.AnError,
@ -424,7 +429,12 @@ func Test_Manager_UpdateHook(t *testing.T) {
defer cancel()
}
hk, err := man.UpdateHook(ctx, stored.ID, "org-1", hook.UpdateInput{Settings: c.Settings}, "u1")
documentID := stored.DocumentID.V
if c.OtherDocument {
documentID = xid.New()
}
hk, err := man.UpdateHook(ctx, stored.ID, documentID, "org-1", hook.UpdateInput{Settings: c.Settings}, "u1")
testutil.AssertEqualError(t, c.Err, err)
assert.Len(t, tx.UpdateDocumentHookCalls(), c.Updates)
@ -468,7 +478,10 @@ func Test_Manager_DeleteHook(t *testing.T) {
branchID := xid.New()
cc := map[string]struct {
Hook func(*testing.T) hook.Hook
Hook func(*testing.T) hook.Hook
OtherDocument bool
// CDURL points changedetection at this address.
CDURL string
FetchErr error
FlushErr error
BeginErr error
@ -486,6 +499,13 @@ func Test_Manager_DeleteHook(t *testing.T) {
FetchErr: sql.ErrNoRows,
Err: sql.ErrNoRows,
},
"Hook of another document": {
Hook: func(t *testing.T) hook.Hook {
return stubHook(t, branchID, time.Now().Add(time.Hour), time.Now())
},
OtherDocument: true,
Err: hook.ErrHookMismatch,
},
"Error returned by flusher.Flush": {
Hook: func(t *testing.T) hook.Hook {
return stubHook(t, branchID, time.Now().Add(time.Hour), time.Now())
@ -495,14 +515,22 @@ func Test_Manager_DeleteHook(t *testing.T) {
},
// the row goes only once the resource it describes is gone, so a
// failed teardown keeps it.
"External teardown fails": {
"Unreachable changedetection keeps the hook": {
Hook: func(*testing.T) hook.Hook {
return urlWatcherHook(branchID)
},
CDURL: "http://127.0.0.1:1",
Err: hook.ErrUpstreamUnavailable,
},
// a stored state that does not decode is no service's failure.
"Malformed state keeps the hook": {
Hook: func(*testing.T) hook.Hook {
hk := urlWatcherHook(branchID)
hk.State = null.ValueFrom(processor.State(`{`))
return hk
},
Err: hook.ErrUpstreamUnavailable,
Err: assert.AnError,
},
"Error returned by db.BeginTx": {
Hook: func(t *testing.T) hook.Hook {
@ -583,7 +611,13 @@ func Test_Manager_DeleteHook(t *testing.T) {
},
}
man := newTestManager(t, stubStoredHook(stubDB(&DBMock{}, tx, c.BeginErr), hk, c.FetchErr), &fakePublisher{}, nil)
var wc *webchange.Client
if c.CDURL != "" {
wc = webchange.NewClient(c.CDURL, "key")
}
man := newTestManager(t, stubStoredHook(stubDB(&DBMock{}, tx, c.BeginErr), hk, c.FetchErr), &fakePublisher{}, wc)
if c.FlushErr != nil {
man.flusher = &FlusherMock{
@ -596,9 +630,19 @@ func Test_Manager_DeleteHook(t *testing.T) {
changes := &changeRecorder{}
man.BindHookChange(changes.record)
err := man.DeleteHook(context.Background(), hk.ID, "org-1", "u1")
documentID := hk.DocumentID.V
if c.OtherDocument {
documentID = xid.New()
}
err := man.DeleteHook(context.Background(), hk.ID, documentID, "org-1", "u1")
testutil.AssertEqualError(t, c.Err, err)
// only a service the hook could not reach is reported as one.
if c.Err != hook.ErrUpstreamUnavailable { //nolint:err113,errorlint // the sentinel is returned as is
assert.NotEqual(t, hook.ErrUpstreamUnavailable, err)
}
ff := tx.DeleteDocumentHookCalls()
require.Len(t, ff, c.Deletes)
assert.Len(t, tx.RecordDocumentBranchHistoryEntryCalls(), c.Records)
@ -627,16 +671,17 @@ func Test_Manager_ResetHook(t *testing.T) {
branchID := xid.New()
cc := map[string]struct {
Hook func(*testing.T) hook.Hook
CD bool
Unreachable bool
FetchErr error
UpdateErr error
Updates int
Status processor.Status
Published []notification.Code
Watchers []string
Err error
Hook func(*testing.T) hook.Hook
OtherDocument bool
CD bool
Unreachable bool
FetchErr error
UpdateErr error
Updates int
Status processor.Status
Published []notification.Code
Watchers []string
Err error
}{
"Error returned by db.FetchDocumentHook": {
Hook: func(t *testing.T) hook.Hook {
@ -645,6 +690,13 @@ func Test_Manager_ResetHook(t *testing.T) {
FetchErr: sql.ErrNoRows,
Err: sql.ErrNoRows,
},
"Hook of another document": {
Hook: func(t *testing.T) hook.Hook {
return stubHook(t, branchID, time.Now().Add(time.Hour), time.Now())
},
OtherDocument: true,
Err: hook.ErrHookMismatch,
},
"Service the hook checks is unreachable": {
Hook: func(*testing.T) hook.Hook {
hk := urlWatcherHook(branchID)
@ -722,7 +774,12 @@ func Test_Manager_ResetHook(t *testing.T) {
changes := &changeRecorder{}
man.BindHookChange(changes.record)
hk, err := man.ResetHook(context.Background(), stored.ID, "org-1")
documentID := stored.DocumentID.V
if c.OtherDocument {
documentID = xid.New()
}
hk, err := man.ResetHook(context.Background(), stored.ID, documentID, "org-1")
testutil.AssertEqualError(t, c.Err, err)
ff := db.UpdateDocumentHookCalls()

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

@ -87,7 +87,7 @@ func (ciw *ContainerImageWatcher) digest(ctx context.Context) (string, Status, e
case errors.Is(err, registry.ErrNotFound):
return "", StatusImageNotFound, nil
default:
return "", "", fmt.Errorf("fetching container image digest: %w", err)
return "", "", unreachable(fmt.Errorf("fetching container image digest: %w", err))
}
}

4
server/core/internal/document/hook/processor/github_tracking.go
View File

@ -131,7 +131,7 @@ func (gt *GithubTracking) fetchTree(ctx context.Context, inp Input) (github.Tree
case errors.Is(err, github.ErrInstallationNotFound):
return nil, StatusMissingInstallation, nil
default:
return nil, "", fmt.Errorf("getting github client: %w", err)
return nil, "", unreachable(fmt.Errorf("getting github client: %w", err))
}
tree, err := client.FetchRepositoryTree(ctx, gt.Repository, gt.Branch)
@ -146,7 +146,7 @@ func (gt *GithubTracking) fetchTree(ctx context.Context, inp Input) (github.Tree
case errors.Is(err, github.ErrTreeTruncated):
return nil, StatusTreeTruncated, nil
default:
return nil, "", fmt.Errorf("fetching repository tree: %w", err)
return nil, "", unreachable(fmt.Errorf("fetching repository tree: %w", err))
}
}

9
server/core/internal/document/hook/processor/processor.go
View File

@ -12,6 +12,10 @@ import (
"github.com/shopspring/decimal"
)
// ErrUnreachable is returned when a processor cannot reach the service it
// checks.
var ErrUnreachable = errors.New("cannot reach the service the hook checks")
// All available Status constants.
const (
// StatusActive indicates that the processor checked what it watches.
@ -211,3 +215,8 @@ type ChangeDetection interface {
// DeleteWatcher deletes the watcher with the given watch ID.
DeleteWatcher(ctx context.Context, watchID string) error
}
// unreachable marks err, a failed call to the service a processor checks.
func unreachable(err error) error {
return fmt.Errorf("%w: %w", ErrUnreachable, err)
}

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

@ -56,7 +56,7 @@ func (uw *URLWatcher) Process(ctx context.Context, inp Input) (Result, error) {
case errors.Is(err, webchange.ErrNotConfigured):
return inactive(StatusUnconfigured), nil
default:
return Result{}, fmt.Errorf("fetching url watcher: %w", err)
return Result{}, unreachable(fmt.Errorf("fetching url watcher: %w", err))
}
// a write that failed after moving the watcher leaves it on a URL the
@ -117,18 +117,18 @@ func (uw *URLWatcher) syncWatcher(ctx context.Context, cd ChangeDetection, watch
}, nil
case err == nil:
if err = cd.UpdateWatcher(ctx, watcherID, uw.URL); err != nil {
return URLWatcherState{}, fmt.Errorf("updating url watcher: %w", err)
return URLWatcherState{}, unreachable(fmt.Errorf("updating url watcher: %w", err))
}
return URLWatcherState{WatcherID: watcherID}, nil
case !errors.Is(err, webchange.ErrWatcherNotFound):
return URLWatcherState{}, fmt.Errorf("fetching url watcher: %w", err)
return URLWatcherState{}, unreachable(fmt.Errorf("fetching url watcher: %w", err))
}
}
watcherID, err := cd.CreateWatcher(ctx, uw.URL)
if err != nil {
return URLWatcherState{}, fmt.Errorf("creating url watcher: %w", err)
return URLWatcherState{}, unreachable(fmt.Errorf("creating url watcher: %w", err))
}
return URLWatcherState{WatcherID: watcherID}, nil
@ -152,7 +152,7 @@ func (uw *URLWatcher) Delete(ctx context.Context, inp Input) error {
err := inp.ChangeDetection().DeleteWatcher(ctx, uws.WatcherID)
if err != nil {
return fmt.Errorf("deleting url watcher: %w", err)
return unreachable(fmt.Errorf("deleting url watcher: %w", err))
}
return nil

60
server/core/internal/server/0mock_db_test.go
View File

@ -136,9 +136,6 @@ var _ DB = &DBMock{}
// FetchDocumentFileFunc: func(ctx context.Context, id string, organizationID string) (*file.File, error) {
// panic("mock out the FetchDocumentFile method")
// },
// FetchDocumentHookFunc: func(ctx context.Context, id xid.ID, organizationID string) (*hookCore.Hook, error) {
// panic("mock out the FetchDocumentHook method")
// },
// FetchDocumentHooksByBranchIDFunc: func(ctx context.Context, branchID xid.ID, organizationID string) ([]hookCore.Hook, error) {
// panic("mock out the FetchDocumentHooksByBranchID method")
// },
@ -419,9 +416,6 @@ type DBMock struct {
// FetchDocumentFileFunc mocks the FetchDocumentFile method.
FetchDocumentFileFunc func(ctx context.Context, id string, organizationID string) (*file.File, error)
// FetchDocumentHookFunc mocks the FetchDocumentHook method.
FetchDocumentHookFunc func(ctx context.Context, id xid.ID, organizationID string) (*hookCore.Hook, error)
// FetchDocumentHooksByBranchIDFunc mocks the FetchDocumentHooksByBranchID method.
FetchDocumentHooksByBranchIDFunc func(ctx context.Context, branchID xid.ID, organizationID string) ([]hookCore.Hook, error)
@ -917,15 +911,6 @@ type DBMock struct {
// OrganizationID is the organizationID argument value.
OrganizationID string
}
// FetchDocumentHook holds details about calls to the FetchDocumentHook method.
FetchDocumentHook []struct {
// Ctx is the ctx argument value.
Ctx context.Context
// ID is the id argument value.
ID xid.ID
// OrganizationID is the organizationID argument value.
OrganizationID string
}
// FetchDocumentHooksByBranchID holds details about calls to the FetchDocumentHooksByBranchID method.
FetchDocumentHooksByBranchID []struct {
// Ctx is the ctx argument value.
@ -1426,7 +1411,6 @@ type DBMock struct {
lockFetchDocumentCommentReply sync.RWMutex
lockFetchDocumentCommentsByBranchID sync.RWMutex
lockFetchDocumentFile sync.RWMutex
lockFetchDocumentHook sync.RWMutex
lockFetchDocumentHooksByBranchID sync.RWMutex
lockFetchDocumentHooksByDocumentID sync.RWMutex
lockFetchDocumentHooksByOrganizationID sync.RWMutex
@ -3027,50 +3011,6 @@ func (mock *DBMock) FetchDocumentFileCalls() []struct {
return calls
}
// FetchDocumentHook calls FetchDocumentHookFunc.
func (mock *DBMock) FetchDocumentHook(ctx context.Context, id xid.ID, organizationID string) (*hookCore.Hook, error) {
callInfo := struct {
Ctx context.Context
ID xid.ID
OrganizationID string
}{
Ctx: ctx,
ID: id,
OrganizationID: organizationID,
}
mock.lockFetchDocumentHook.Lock()
mock.calls.FetchDocumentHook = append(mock.calls.FetchDocumentHook, callInfo)
mock.lockFetchDocumentHook.Unlock()
if mock.FetchDocumentHookFunc == nil {
var (
hookOut *hookCore.Hook
errOut error
)
return hookOut, errOut
}
return mock.FetchDocumentHookFunc(ctx, id, organizationID)
}
// FetchDocumentHookCalls gets all the calls that were made to FetchDocumentHook.
// Check the length with:
//
// len(mockedDB.FetchDocumentHookCalls())
func (mock *DBMock) FetchDocumentHookCalls() []struct {
Ctx context.Context
ID xid.ID
OrganizationID string
} {
var calls []struct {
Ctx context.Context
ID xid.ID
OrganizationID string
}
mock.lockFetchDocumentHook.RLock()
calls = mock.calls.FetchDocumentHook
mock.lockFetchDocumentHook.RUnlock()
return calls
}
// FetchDocumentHooksByBranchID calls FetchDocumentHooksByBranchIDFunc.
func (mock *DBMock) FetchDocumentHooksByBranchID(ctx context.Context, branchID xid.ID, organizationID string) ([]hookCore.Hook, error) {
callInfo := struct {

42
server/core/internal/server/0mock_hook_manager_test.go
View File

@ -27,16 +27,16 @@ var _ HookManager = &HookManagerMock{}
// CreateHookFunc: func(ctx context.Context, ci hookCore.CreateInput, documentID xid.ID, organizationID string, updatedBy string) (*hookCore.Hook, error) {
// panic("mock out the CreateHook method")
// },
// DeleteHookFunc: func(ctx context.Context, id xid.ID, organizationID string, updatedBy string) error {
// DeleteHookFunc: func(ctx context.Context, id xid.ID, documentID xid.ID, organizationID string, updatedBy string) error {
// panic("mock out the DeleteHook method")
// },
// ResetHookFunc: func(ctx context.Context, id xid.ID, organizationID string) (*hookCore.Hook, error) {
// ResetHookFunc: func(ctx context.Context, id xid.ID, documentID xid.ID, organizationID string) (*hookCore.Hook, error) {
// panic("mock out the ResetHook method")
// },
// SetUpBranchFunc: func(ctx context.Context, branchID xid.ID, organizationID string) {
// panic("mock out the SetUpBranch method")
// },
// UpdateHookFunc: func(ctx context.Context, id xid.ID, organizationID string, ui hookCore.UpdateInput, updatedBy string) (*hookCore.Hook, error) {
// UpdateHookFunc: func(ctx context.Context, id xid.ID, documentID xid.ID, organizationID string, ui hookCore.UpdateInput, updatedBy string) (*hookCore.Hook, error) {
// panic("mock out the UpdateHook method")
// },
// }
@ -53,16 +53,16 @@ type HookManagerMock struct {
CreateHookFunc func(ctx context.Context, ci hookCore.CreateInput, documentID xid.ID, organizationID string, updatedBy string) (*hookCore.Hook, error)
// DeleteHookFunc mocks the DeleteHook method.
DeleteHookFunc func(ctx context.Context, id xid.ID, organizationID string, updatedBy string) error
DeleteHookFunc func(ctx context.Context, id xid.ID, documentID xid.ID, organizationID string, updatedBy string) error
// ResetHookFunc mocks the ResetHook method.
ResetHookFunc func(ctx context.Context, id xid.ID, organizationID string) (*hookCore.Hook, error)
ResetHookFunc func(ctx context.Context, id xid.ID, documentID xid.ID, organizationID string) (*hookCore.Hook, error)
// SetUpBranchFunc mocks the SetUpBranch method.
SetUpBranchFunc func(ctx context.Context, branchID xid.ID, organizationID string)
// UpdateHookFunc mocks the UpdateHook method.
UpdateHookFunc func(ctx context.Context, id xid.ID, organizationID string, ui hookCore.UpdateInput, updatedBy string) (*hookCore.Hook, error)
UpdateHookFunc func(ctx context.Context, id xid.ID, documentID xid.ID, organizationID string, ui hookCore.UpdateInput, updatedBy string) (*hookCore.Hook, error)
// calls tracks calls to the methods.
calls struct {
@ -90,6 +90,8 @@ type HookManagerMock struct {
Ctx context.Context
// ID is the id argument value.
ID xid.ID
// DocumentID is the documentID argument value.
DocumentID xid.ID
// OrganizationID is the organizationID argument value.
OrganizationID string
// UpdatedBy is the updatedBy argument value.
@ -101,6 +103,8 @@ type HookManagerMock struct {
Ctx context.Context
// ID is the id argument value.
ID xid.ID
// DocumentID is the documentID argument value.
DocumentID xid.ID
// OrganizationID is the organizationID argument value.
OrganizationID string
}
@ -119,6 +123,8 @@ type HookManagerMock struct {
Ctx context.Context
// ID is the id argument value.
ID xid.ID
// DocumentID is the documentID argument value.
DocumentID xid.ID
// OrganizationID is the organizationID argument value.
OrganizationID string
// UI is the ui argument value.
@ -220,15 +226,17 @@ func (mock *HookManagerMock) CreateHookCalls() []struct {
}
// DeleteHook calls DeleteHookFunc.
func (mock *HookManagerMock) DeleteHook(ctx context.Context, id xid.ID, organizationID string, updatedBy string) error {
func (mock *HookManagerMock) DeleteHook(ctx context.Context, id xid.ID, documentID xid.ID, organizationID string, updatedBy string) error {
callInfo := struct {
Ctx context.Context
ID xid.ID
DocumentID xid.ID
OrganizationID string
UpdatedBy string
}{
Ctx: ctx,
ID: id,
DocumentID: documentID,
OrganizationID: organizationID,
UpdatedBy: updatedBy,
}
@ -241,7 +249,7 @@ func (mock *HookManagerMock) DeleteHook(ctx context.Context, id xid.ID, organiza
)
return errOut
}
return mock.DeleteHookFunc(ctx, id, organizationID, updatedBy)
return mock.DeleteHookFunc(ctx, id, documentID, organizationID, updatedBy)
}
// DeleteHookCalls gets all the calls that were made to DeleteHook.
@ -251,12 +259,14 @@ func (mock *HookManagerMock) DeleteHook(ctx context.Context, id xid.ID, organiza
func (mock *HookManagerMock) DeleteHookCalls() []struct {
Ctx context.Context
ID xid.ID
DocumentID xid.ID
OrganizationID string
UpdatedBy string
} {
var calls []struct {
Ctx context.Context
ID xid.ID
DocumentID xid.ID
OrganizationID string
UpdatedBy string
}
@ -267,14 +277,16 @@ func (mock *HookManagerMock) DeleteHookCalls() []struct {
}
// ResetHook calls ResetHookFunc.
func (mock *HookManagerMock) ResetHook(ctx context.Context, id xid.ID, organizationID string) (*hookCore.Hook, error) {
func (mock *HookManagerMock) ResetHook(ctx context.Context, id xid.ID, documentID xid.ID, organizationID string) (*hookCore.Hook, error) {
callInfo := struct {
Ctx context.Context
ID xid.ID
DocumentID xid.ID
OrganizationID string
}{
Ctx: ctx,
ID: id,
DocumentID: documentID,
OrganizationID: organizationID,
}
mock.lockResetHook.Lock()
@ -287,7 +299,7 @@ func (mock *HookManagerMock) ResetHook(ctx context.Context, id xid.ID, organizat
)
return hookOut, errOut
}
return mock.ResetHookFunc(ctx, id, organizationID)
return mock.ResetHookFunc(ctx, id, documentID, organizationID)
}
// ResetHookCalls gets all the calls that were made to ResetHook.
@ -297,11 +309,13 @@ func (mock *HookManagerMock) ResetHook(ctx context.Context, id xid.ID, organizat
func (mock *HookManagerMock) ResetHookCalls() []struct {
Ctx context.Context
ID xid.ID
DocumentID xid.ID
OrganizationID string
} {
var calls []struct {
Ctx context.Context
ID xid.ID
DocumentID xid.ID
OrganizationID string
}
mock.lockResetHook.RLock()
@ -351,16 +365,18 @@ func (mock *HookManagerMock) SetUpBranchCalls() []struct {
}
// UpdateHook calls UpdateHookFunc.
func (mock *HookManagerMock) UpdateHook(ctx context.Context, id xid.ID, organizationID string, ui hookCore.UpdateInput, updatedBy string) (*hookCore.Hook, error) {
func (mock *HookManagerMock) UpdateHook(ctx context.Context, id xid.ID, documentID xid.ID, organizationID string, ui hookCore.UpdateInput, updatedBy string) (*hookCore.Hook, error) {
callInfo := struct {
Ctx context.Context
ID xid.ID
DocumentID xid.ID
OrganizationID string
UI hookCore.UpdateInput
UpdatedBy string
}{
Ctx: ctx,
ID: id,
DocumentID: documentID,
OrganizationID: organizationID,
UI: ui,
UpdatedBy: updatedBy,
@ -375,7 +391,7 @@ func (mock *HookManagerMock) UpdateHook(ctx context.Context, id xid.ID, organiza
)
return hookOut, errOut
}
return mock.UpdateHookFunc(ctx, id, organizationID, ui, updatedBy)
return mock.UpdateHookFunc(ctx, id, documentID, organizationID, ui, updatedBy)
}
// UpdateHookCalls gets all the calls that were made to UpdateHook.
@ -385,6 +401,7 @@ func (mock *HookManagerMock) UpdateHook(ctx context.Context, id xid.ID, organiza
func (mock *HookManagerMock) UpdateHookCalls() []struct {
Ctx context.Context
ID xid.ID
DocumentID xid.ID
OrganizationID string
UI hookCore.UpdateInput
UpdatedBy string
@ -392,6 +409,7 @@ func (mock *HookManagerMock) UpdateHookCalls() []struct {
var calls []struct {
Ctx context.Context
ID xid.ID
DocumentID xid.ID
OrganizationID string
UI hookCore.UpdateInput
UpdatedBy string

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

@ -556,56 +556,6 @@ func Test_Handler_MergeBranches(t *testing.T) {
Body: validBody,
RespCode: http.StatusInternalServerError,
},
// the hooks are copied after the commit, so a failure there leaves
// the merge itself standing.
"Hook copy fetch error": {
DB: &DBMock{
FetchDocumentByBranchIDFunc: fetchByBranch,
FetchDocumentHooksByBranchIDFunc: func(context.Context, xid.ID, string) ([]hookCore.Hook, error) {
return nil, errors.New("boom")
},
},
Tx: &TxMock{},
Body: validBody,
RespCode: http.StatusOK,
Committed: 1,
Metadata: 1,
Reviewers: 1,
History: 1,
},
"Hook re-creation error": {
DB: &DBMock{
FetchDocumentByBranchIDFunc: fetchByBranch,
FetchDocumentHooksByBranchIDFunc: func(context.Context, xid.ID, string) ([]hookCore.Hook, error) {
return []hookCore.Hook{storedHook("bogus")}, nil
},
},
Tx: &TxMock{},
Body: validBody,
RespCode: http.StatusOK,
Committed: 1,
Metadata: 1,
Reviewers: 1,
History: 1,
},
"Hook insertion error": {
DB: &DBMock{
FetchDocumentByBranchIDFunc: fetchByBranch,
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")
},
},
Tx: &TxMock{},
Body: validBody,
RespCode: http.StatusOK,
Committed: 1,
Metadata: 1,
Reviewers: 1,
History: 1,
},
"Document update error": {
DB: &DBMock{FetchDocumentByBranchIDFunc: fetchByBranch},
Tx: &TxMock{

60
server/core/internal/server/internal/document/hook/0mock_db_test.go
View File

@ -25,9 +25,6 @@ var _ DB = &DBMock{}
// FetchDocumentByBranchIDFunc: func(ctx context.Context, branchID xid.ID, organizationID string) (*document.Document, error) {
// panic("mock out the FetchDocumentByBranchID method")
// },
// FetchDocumentHookFunc: func(ctx context.Context, id xid.ID, organizationID string) (*hookCore.Hook, error) {
// panic("mock out the FetchDocumentHook method")
// },
// FetchDocumentHooksByBranchIDFunc: func(ctx context.Context, branchID xid.ID, organizationID string) ([]hookCore.Hook, error) {
// panic("mock out the FetchDocumentHooksByBranchID method")
// },
@ -41,9 +38,6 @@ type DBMock struct {
// FetchDocumentByBranchIDFunc mocks the FetchDocumentByBranchID method.
FetchDocumentByBranchIDFunc func(ctx context.Context, branchID xid.ID, organizationID string) (*document.Document, error)
// FetchDocumentHookFunc mocks the FetchDocumentHook method.
FetchDocumentHookFunc func(ctx context.Context, id xid.ID, organizationID string) (*hookCore.Hook, error)
// FetchDocumentHooksByBranchIDFunc mocks the FetchDocumentHooksByBranchID method.
FetchDocumentHooksByBranchIDFunc func(ctx context.Context, branchID xid.ID, organizationID string) ([]hookCore.Hook, error)
@ -58,15 +52,6 @@ type DBMock struct {
// OrganizationID is the organizationID argument value.
OrganizationID string
}
// FetchDocumentHook holds details about calls to the FetchDocumentHook method.
FetchDocumentHook []struct {
// Ctx is the ctx argument value.
Ctx context.Context
// ID is the id argument value.
ID xid.ID
// OrganizationID is the organizationID argument value.
OrganizationID string
}
// FetchDocumentHooksByBranchID holds details about calls to the FetchDocumentHooksByBranchID method.
FetchDocumentHooksByBranchID []struct {
// Ctx is the ctx argument value.
@ -78,7 +63,6 @@ type DBMock struct {
}
}
lockFetchDocumentByBranchID sync.RWMutex
lockFetchDocumentHook sync.RWMutex
lockFetchDocumentHooksByBranchID sync.RWMutex
}
@ -126,50 +110,6 @@ func (mock *DBMock) FetchDocumentByBranchIDCalls() []struct {
return calls
}
// FetchDocumentHook calls FetchDocumentHookFunc.
func (mock *DBMock) FetchDocumentHook(ctx context.Context, id xid.ID, organizationID string) (*hookCore.Hook, error) {
callInfo := struct {
Ctx context.Context
ID xid.ID
OrganizationID string
}{
Ctx: ctx,
ID: id,
OrganizationID: organizationID,
}
mock.lockFetchDocumentHook.Lock()
mock.calls.FetchDocumentHook = append(mock.calls.FetchDocumentHook, callInfo)
mock.lockFetchDocumentHook.Unlock()
if mock.FetchDocumentHookFunc == nil {
var (
hookOut *hookCore.Hook
errOut error
)
return hookOut, errOut
}
return mock.FetchDocumentHookFunc(ctx, id, organizationID)
}
// FetchDocumentHookCalls gets all the calls that were made to FetchDocumentHook.
// Check the length with:
//
// len(mockedDB.FetchDocumentHookCalls())
func (mock *DBMock) FetchDocumentHookCalls() []struct {
Ctx context.Context
ID xid.ID
OrganizationID string
} {
var calls []struct {
Ctx context.Context
ID xid.ID
OrganizationID string
}
mock.lockFetchDocumentHook.RLock()
calls = mock.calls.FetchDocumentHook
mock.lockFetchDocumentHook.RUnlock()
return calls
}
// FetchDocumentHooksByBranchID calls FetchDocumentHooksByBranchIDFunc.
func (mock *DBMock) FetchDocumentHooksByBranchID(ctx context.Context, branchID xid.ID, organizationID string) ([]hookCore.Hook, error) {
callInfo := struct {

42
server/core/internal/server/internal/document/hook/0mock_manager_test.go
View File

@ -24,13 +24,13 @@ var _ Manager = &ManagerMock{}
// CreateHookFunc: func(ctx context.Context, ci hookCore.CreateInput, documentID xid.ID, organizationID string, updatedBy string) (*hookCore.Hook, error) {
// panic("mock out the CreateHook method")
// },
// DeleteHookFunc: func(ctx context.Context, id xid.ID, organizationID string, updatedBy string) error {
// DeleteHookFunc: func(ctx context.Context, id xid.ID, documentID xid.ID, organizationID string, updatedBy string) error {
// panic("mock out the DeleteHook method")
// },
// ResetHookFunc: func(ctx context.Context, id xid.ID, organizationID string) (*hookCore.Hook, error) {
// ResetHookFunc: func(ctx context.Context, id xid.ID, documentID xid.ID, organizationID string) (*hookCore.Hook, error) {
// panic("mock out the ResetHook method")
// },
// UpdateHookFunc: func(ctx context.Context, id xid.ID, organizationID string, ui hookCore.UpdateInput, updatedBy string) (*hookCore.Hook, error) {
// UpdateHookFunc: func(ctx context.Context, id xid.ID, documentID xid.ID, organizationID string, ui hookCore.UpdateInput, updatedBy string) (*hookCore.Hook, error) {
// panic("mock out the UpdateHook method")
// },
// }
@ -44,13 +44,13 @@ type ManagerMock struct {
CreateHookFunc func(ctx context.Context, ci hookCore.CreateInput, documentID xid.ID, organizationID string, updatedBy string) (*hookCore.Hook, error)
// DeleteHookFunc mocks the DeleteHook method.
DeleteHookFunc func(ctx context.Context, id xid.ID, organizationID string, updatedBy string) error
DeleteHookFunc func(ctx context.Context, id xid.ID, documentID xid.ID, organizationID string, updatedBy string) error
// ResetHookFunc mocks the ResetHook method.
ResetHookFunc func(ctx context.Context, id xid.ID, organizationID string) (*hookCore.Hook, error)
ResetHookFunc func(ctx context.Context, id xid.ID, documentID xid.ID, organizationID string) (*hookCore.Hook, error)
// UpdateHookFunc mocks the UpdateHook method.
UpdateHookFunc func(ctx context.Context, id xid.ID, organizationID string, ui hookCore.UpdateInput, updatedBy string) (*hookCore.Hook, error)
UpdateHookFunc func(ctx context.Context, id xid.ID, documentID xid.ID, organizationID string, ui hookCore.UpdateInput, updatedBy string) (*hookCore.Hook, error)
// calls tracks calls to the methods.
calls struct {
@ -73,6 +73,8 @@ type ManagerMock struct {
Ctx context.Context
// ID is the id argument value.
ID xid.ID
// DocumentID is the documentID argument value.
DocumentID xid.ID
// OrganizationID is the organizationID argument value.
OrganizationID string
// UpdatedBy is the updatedBy argument value.
@ -84,6 +86,8 @@ type ManagerMock struct {
Ctx context.Context
// ID is the id argument value.
ID xid.ID
// DocumentID is the documentID argument value.
DocumentID xid.ID
// OrganizationID is the organizationID argument value.
OrganizationID string
}
@ -93,6 +97,8 @@ type ManagerMock struct {
Ctx context.Context
// ID is the id argument value.
ID xid.ID
// DocumentID is the documentID argument value.
DocumentID xid.ID
// OrganizationID is the organizationID argument value.
OrganizationID string
// UI is the ui argument value.
@ -160,15 +166,17 @@ func (mock *ManagerMock) CreateHookCalls() []struct {
}
// DeleteHook calls DeleteHookFunc.
func (mock *ManagerMock) DeleteHook(ctx context.Context, id xid.ID, organizationID string, updatedBy string) error {
func (mock *ManagerMock) DeleteHook(ctx context.Context, id xid.ID, documentID xid.ID, organizationID string, updatedBy string) error {
callInfo := struct {
Ctx context.Context
ID xid.ID
DocumentID xid.ID
OrganizationID string
UpdatedBy string
}{
Ctx: ctx,
ID: id,
DocumentID: documentID,
OrganizationID: organizationID,
UpdatedBy: updatedBy,
}
@ -181,7 +189,7 @@ func (mock *ManagerMock) DeleteHook(ctx context.Context, id xid.ID, organization
)
return errOut
}
return mock.DeleteHookFunc(ctx, id, organizationID, updatedBy)
return mock.DeleteHookFunc(ctx, id, documentID, organizationID, updatedBy)
}
// DeleteHookCalls gets all the calls that were made to DeleteHook.
@ -191,12 +199,14 @@ func (mock *ManagerMock) DeleteHook(ctx context.Context, id xid.ID, organization
func (mock *ManagerMock) DeleteHookCalls() []struct {
Ctx context.Context
ID xid.ID
DocumentID xid.ID
OrganizationID string
UpdatedBy string
} {
var calls []struct {
Ctx context.Context
ID xid.ID
DocumentID xid.ID
OrganizationID string
UpdatedBy string
}
@ -207,14 +217,16 @@ func (mock *ManagerMock) DeleteHookCalls() []struct {
}
// ResetHook calls ResetHookFunc.
func (mock *ManagerMock) ResetHook(ctx context.Context, id xid.ID, organizationID string) (*hookCore.Hook, error) {
func (mock *ManagerMock) ResetHook(ctx context.Context, id xid.ID, documentID xid.ID, organizationID string) (*hookCore.Hook, error) {
callInfo := struct {
Ctx context.Context
ID xid.ID
DocumentID xid.ID
OrganizationID string
}{
Ctx: ctx,
ID: id,
DocumentID: documentID,
OrganizationID: organizationID,
}
mock.lockResetHook.Lock()
@ -227,7 +239,7 @@ func (mock *ManagerMock) ResetHook(ctx context.Context, id xid.ID, organizationI
)
return hookOut, errOut
}
return mock.ResetHookFunc(ctx, id, organizationID)
return mock.ResetHookFunc(ctx, id, documentID, organizationID)
}
// ResetHookCalls gets all the calls that were made to ResetHook.
@ -237,11 +249,13 @@ func (mock *ManagerMock) ResetHook(ctx context.Context, id xid.ID, organizationI
func (mock *ManagerMock) ResetHookCalls() []struct {
Ctx context.Context
ID xid.ID
DocumentID xid.ID
OrganizationID string
} {
var calls []struct {
Ctx context.Context
ID xid.ID
DocumentID xid.ID
OrganizationID string
}
mock.lockResetHook.RLock()
@ -251,16 +265,18 @@ func (mock *ManagerMock) ResetHookCalls() []struct {
}
// UpdateHook calls UpdateHookFunc.
func (mock *ManagerMock) UpdateHook(ctx context.Context, id xid.ID, organizationID string, ui hookCore.UpdateInput, updatedBy string) (*hookCore.Hook, error) {
func (mock *ManagerMock) UpdateHook(ctx context.Context, id xid.ID, documentID xid.ID, organizationID string, ui hookCore.UpdateInput, updatedBy string) (*hookCore.Hook, error) {
callInfo := struct {
Ctx context.Context
ID xid.ID
DocumentID xid.ID
OrganizationID string
UI hookCore.UpdateInput
UpdatedBy string
}{
Ctx: ctx,
ID: id,
DocumentID: documentID,
OrganizationID: organizationID,
UI: ui,
UpdatedBy: updatedBy,
@ -275,7 +291,7 @@ func (mock *ManagerMock) UpdateHook(ctx context.Context, id xid.ID, organization
)
return hookOut, errOut
}
return mock.UpdateHookFunc(ctx, id, organizationID, ui, updatedBy)
return mock.UpdateHookFunc(ctx, id, documentID, organizationID, ui, updatedBy)
}
// UpdateHookCalls gets all the calls that were made to UpdateHook.
@ -285,6 +301,7 @@ func (mock *ManagerMock) UpdateHook(ctx context.Context, id xid.ID, organization
func (mock *ManagerMock) UpdateHookCalls() []struct {
Ctx context.Context
ID xid.ID
DocumentID xid.ID
OrganizationID string
UI hookCore.UpdateInput
UpdatedBy string
@ -292,6 +309,7 @@ func (mock *ManagerMock) UpdateHookCalls() []struct {
var calls []struct {
Ctx context.Context
ID xid.ID
DocumentID xid.ID
OrganizationID string
UI hookCore.UpdateInput
UpdatedBy string

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

@ -9,19 +9,10 @@ import (
"github.com/oxynote/oxynote/server/core/internal/document"
hookCore "github.com/oxynote/oxynote/server/core/internal/document/hook"
"github.com/oxynote/oxynote/server/core/internal/server/internal/auth"
"github.com/oxynote/oxynote/server/core/pkg/errutil"
"github.com/oxynote/oxynote/server/core/pkg/httpserver"
"github.com/rs/xid"
)
// ErrBranchMismatch is returned when the requested branch does not belong to
// the document identified by the request path.
var ErrBranchMismatch = errutil.New(http.StatusNotFound, "document.branch_mismatch", "branch does not belong to the document")
// ErrHookMismatch is returned when the requested hook does not belong to the
// document identified by the request path.
var ErrHookMismatch = errutil.New(http.StatusNotFound, "document.hook_mismatch", "hook does not belong to the document")
// Handler holds dependencies required for document hook operations.
type Handler struct {
log *slog.Logger
@ -70,7 +61,7 @@ func (h *Handler) FetchDocumentHooks(w http.ResponseWriter, r *http.Request) {
}
if branchDoc.ID != documentID {
httpserver.RespondError(h.log, w, ErrBranchMismatch)
httpserver.RespondError(h.log, w, hookCore.ErrBranchMismatch)
return
}
@ -146,19 +137,6 @@ func (h *Handler) UpdateDocumentHook(w http.ResponseWriter, r *http.Request) {
return
}
hk, err := h.db.FetchDocumentHook(r.Context(), id, session.ActiveOrganizationID)
if err != nil {
httpserver.RespondError(h.log, w, err)
return
}
// a row whose document was deleted carries no document id at all and is
// not addressable through any document path.
if !hk.DocumentID.Valid || hk.DocumentID.V != documentID {
httpserver.RespondError(h.log, w, ErrHookMismatch)
return
}
var ui hookCore.UpdateInput
if err = httpserver.DecodeJSON(r, &ui); err != nil {
@ -166,7 +144,7 @@ func (h *Handler) UpdateDocumentHook(w http.ResponseWriter, r *http.Request) {
return
}
hk, err = h.hookMan.UpdateHook(r.Context(), hk.ID, session.ActiveOrganizationID, ui, session.UserID)
hk, err := h.hookMan.UpdateHook(r.Context(), id, documentID, session.ActiveOrganizationID, ui, session.UserID)
if err != nil {
httpserver.RespondError(h.log, w, err)
return
@ -199,20 +177,7 @@ func (h *Handler) ResetDocumentHook(w http.ResponseWriter, r *http.Request) {
return
}
hk, err := h.db.FetchDocumentHook(r.Context(), id, session.ActiveOrganizationID)
if err != nil {
httpserver.RespondError(h.log, w, err)
return
}
// a row whose document was deleted carries no document id at all and is
// not addressable through any document path.
if !hk.DocumentID.Valid || hk.DocumentID.V != documentID {
httpserver.RespondError(h.log, w, ErrHookMismatch)
return
}
hk, err = h.hookMan.ResetHook(r.Context(), hk.ID, session.ActiveOrganizationID)
hk, err := h.hookMan.ResetHook(r.Context(), id, documentID, session.ActiveOrganizationID)
if err != nil {
httpserver.RespondError(h.log, w, err)
return
@ -245,20 +210,7 @@ func (h *Handler) DeleteDocumentHook(w http.ResponseWriter, r *http.Request) {
return
}
hk, err := h.db.FetchDocumentHook(r.Context(), id, session.ActiveOrganizationID)
if err != nil {
httpserver.RespondError(h.log, w, err)
return
}
// a row whose document was deleted carries no document id at all and is
// not addressable through any document path.
if !hk.DocumentID.Valid || hk.DocumentID.V != documentID {
httpserver.RespondError(h.log, w, ErrHookMismatch)
return
}
if err = h.hookMan.DeleteHook(r.Context(), hk.ID, session.ActiveOrganizationID, session.UserID); err != nil {
if err = h.hookMan.DeleteHook(r.Context(), id, documentID, session.ActiveOrganizationID, session.UserID); err != nil {
httpserver.RespondError(h.log, w, err)
return
}
@ -279,9 +231,6 @@ type DB interface {
// FetchDocumentByBranchID should fetch the document joined against the branch identified by branchID.
FetchDocumentByBranchID(ctx context.Context, branchID xid.ID, organizationID string) (*document.Document, error)
// FetchDocumentHook should fetch the document hook for the given id.
FetchDocumentHook(ctx context.Context, id xid.ID, organizationID string) (*hookCore.Hook, error)
// FetchDocumentHooksByBranchID should fetch all hooks for a specific branch.
FetchDocumentHooksByBranchID(ctx context.Context, branchID xid.ID, organizationID string) ([]hookCore.Hook, error)
}
@ -294,15 +243,15 @@ type Manager interface {
// credited to updatedBy.
CreateHook(ctx context.Context, ci hookCore.CreateInput, documentID xid.ID, organizationID, updatedBy string) (*hookCore.Hook, error)
// UpdateHook should replace the hook's settings, credited to
// updatedBy, and return the stored hook.
UpdateHook(ctx context.Context, id xid.ID, organizationID string, ui hookCore.UpdateInput, updatedBy string) (*hookCore.Hook, error)
// UpdateHook should replace the settings of the document's hook,
// credited to updatedBy, and return the stored hook.
UpdateHook(ctx context.Context, id, documentID xid.ID, organizationID string, ui hookCore.UpdateInput, updatedBy string) (*hookCore.Hook, error)
// DeleteHook should tear the hook down and remove it, credited to
// updatedBy.
DeleteHook(ctx context.Context, id xid.ID, organizationID, updatedBy string) error
// DeleteHook should tear the document's hook down and remove it,
// credited to updatedBy.
DeleteHook(ctx context.Context, id, documentID xid.ID, organizationID, updatedBy string) error
// ResetHook should restore the hook's score and state and return the
// stored hook.
ResetHook(ctx context.Context, id xid.ID, organizationID string) (*hookCore.Hook, error)
// ResetHook should reset the document's hook and return the stored
// hook.
ResetHook(ctx context.Context, id, documentID xid.ID, organizationID string) (*hookCore.Hook, error)
}

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

@ -56,10 +56,10 @@ func scheduledHook() *hookCore.Hook {
// storedHookManager answers an update or reset with the stored hook.
func storedHookManager() *ManagerMock {
return &ManagerMock{
UpdateHookFunc: func(context.Context, xid.ID, string, hookCore.UpdateInput, string) (*hookCore.Hook, error) {
UpdateHookFunc: func(context.Context, xid.ID, xid.ID, string, hookCore.UpdateInput, string) (*hookCore.Hook, error) {
return scheduledHook(), nil
},
ResetHookFunc: func(context.Context, xid.ID, string) (*hookCore.Hook, error) {
ResetHookFunc: func(context.Context, xid.ID, xid.ID, string) (*hookCore.Hook, error) {
return scheduledHook(), nil
},
}
@ -288,15 +288,6 @@ func hookRequest(method, body string, noSession, omitDoc, omitHook bool) *http.R
return req.WithContext(ctx)
}
// storedHookDB answers the hook lookup with the given hook.
func storedHookDB(hk *hookCore.Hook) *DBMock {
return &DBMock{
FetchDocumentHookFunc: func(context.Context, xid.ID, string) (*hookCore.Hook, error) {
return hk, nil
},
}
}
func Test_Handler_CreateDocumentHook(t *testing.T) {
validBody := `{"type":"scheduled-reminder","branchId":"` + _branchID.String() + `","settings":{"scale":"linear"}}`
@ -397,11 +388,7 @@ func Test_Handler_CreateDocumentHook(t *testing.T) {
func Test_Handler_UpdateDocumentHook(t *testing.T) {
validBody := `{"settings":{"scale":"linear","duration":"48h"}}`
otherDocument := scheduledHook()
otherDocument.DocumentID = null.ValueFrom(xid.New())
cc := map[string]struct {
DB *DBMock
Man *ManagerMock
NoSession bool
OmitDoc bool
@ -412,53 +399,42 @@ func Test_Handler_UpdateDocumentHook(t *testing.T) {
Updates int
}{
"No session in context": {
DB: storedHookDB(scheduledHook()),
Man: &ManagerMock{},
NoSession: true,
Body: validBody,
RespCode: http.StatusUnauthorized,
},
"Missing document ID parameter": {
DB: storedHookDB(scheduledHook()),
Man: &ManagerMock{},
OmitDoc: true,
Body: validBody,
RespCode: http.StatusNotFound,
},
"Missing hook ID parameter": {
DB: storedHookDB(scheduledHook()),
Man: &ManagerMock{},
OmitHook: true,
Body: validBody,
RespCode: http.StatusNotFound,
},
"Hook fetch error": {
DB: &DBMock{
FetchDocumentHookFunc: func(context.Context, xid.ID, string) (*hookCore.Hook, error) {
return nil, errors.New("boom")
"Hook of another document": {
Man: &ManagerMock{
UpdateHookFunc: func(context.Context, xid.ID, xid.ID, string, hookCore.UpdateInput, string) (*hookCore.Hook, error) {
return nil, hookCore.ErrHookMismatch
},
},
Man: &ManagerMock{},
Body: validBody,
RespCode: http.StatusInternalServerError,
},
"Hook of another document": {
DB: storedHookDB(otherDocument),
Man: &ManagerMock{},
Body: validBody,
RespCode: http.StatusNotFound,
RespBody: `{"code":"document.hook_mismatch","message":"hook does not belong to the document"}`,
Updates: 1,
},
"Invalid JSON body": {
DB: storedHookDB(scheduledHook()),
Man: &ManagerMock{},
Body: "{",
RespCode: http.StatusBadRequest,
},
"Error returned by man.UpdateHook": {
DB: storedHookDB(scheduledHook()),
Man: &ManagerMock{
UpdateHookFunc: func(context.Context, xid.ID, string, hookCore.UpdateInput, string) (*hookCore.Hook, error) {
UpdateHookFunc: func(context.Context, xid.ID, xid.ID, string, hookCore.UpdateInput, string) (*hookCore.Hook, error) {
return nil, hookCore.ErrUpstreamUnavailable
},
},
@ -468,7 +444,6 @@ func Test_Handler_UpdateDocumentHook(t *testing.T) {
Updates: 1,
},
"Successful update": {
DB: storedHookDB(scheduledHook()),
Man: storedHookManager(),
Body: validBody,
RespCode: http.StatusOK,
@ -480,7 +455,7 @@ func Test_Handler_UpdateDocumentHook(t *testing.T) {
t.Run(cn, func(t *testing.T) {
t.Parallel()
hdl := NewHandler(slog.New(slog.DiscardHandler), c.DB, c.Man)
hdl := NewHandler(slog.New(slog.DiscardHandler), &DBMock{}, c.Man)
got := recordNotifications(hdl)
rec := httptest.NewRecorder()
@ -499,6 +474,7 @@ func Test_Handler_UpdateDocumentHook(t *testing.T) {
if c.Updates > 0 {
assert.Equal(t, _hookID, ff[0].ID)
assert.Equal(t, _documentID, ff[0].DocumentID)
assert.Equal(t, "org1", ff[0].OrganizationID)
assert.JSONEq(t, `{"scale":"linear","duration":"48h"}`, string(ff[0].UI.Settings))
assert.Equal(t, "u1", ff[0].UpdatedBy)
@ -508,11 +484,7 @@ func Test_Handler_UpdateDocumentHook(t *testing.T) {
}
func Test_Handler_ResetDocumentHook(t *testing.T) {
otherDocument := scheduledHook()
otherDocument.DocumentID = null.ValueFrom(xid.New())
cc := map[string]struct {
DB *DBMock
Man *ManagerMock
NoSession bool
OmitDoc bool
@ -521,41 +493,32 @@ func Test_Handler_ResetDocumentHook(t *testing.T) {
Resets int
}{
"No session in context": {
DB: storedHookDB(scheduledHook()),
Man: &ManagerMock{},
NoSession: true,
RespCode: http.StatusUnauthorized,
},
"Missing document ID parameter": {
DB: storedHookDB(scheduledHook()),
Man: &ManagerMock{},
OmitDoc: true,
RespCode: http.StatusNotFound,
},
"Missing hook ID parameter": {
DB: storedHookDB(scheduledHook()),
Man: &ManagerMock{},
OmitHook: true,
RespCode: http.StatusNotFound,
},
"Hook fetch error": {
DB: &DBMock{
FetchDocumentHookFunc: func(context.Context, xid.ID, string) (*hookCore.Hook, error) {
return nil, errors.New("boom")
"Hook of another document": {
Man: &ManagerMock{
ResetHookFunc: func(context.Context, xid.ID, xid.ID, string) (*hookCore.Hook, error) {
return nil, hookCore.ErrHookMismatch
},
},
Man: &ManagerMock{},
RespCode: http.StatusInternalServerError,
},
"Hook of another document": {
DB: storedHookDB(otherDocument),
Man: &ManagerMock{},
RespCode: http.StatusNotFound,
Resets: 1,
},
"Error returned by man.ResetHook": {
DB: storedHookDB(scheduledHook()),
Man: &ManagerMock{
ResetHookFunc: func(context.Context, xid.ID, string) (*hookCore.Hook, error) {
ResetHookFunc: func(context.Context, xid.ID, xid.ID, string) (*hookCore.Hook, error) {
return nil, errors.New("boom")
},
},
@ -563,7 +526,6 @@ func Test_Handler_ResetDocumentHook(t *testing.T) {
Resets: 1,
},
"Successful reset": {
DB: storedHookDB(scheduledHook()),
Man: storedHookManager(),
RespCode: http.StatusOK,
Resets: 1,
@ -574,7 +536,7 @@ func Test_Handler_ResetDocumentHook(t *testing.T) {
t.Run(cn, func(t *testing.T) {
t.Parallel()
hdl := NewHandler(slog.New(slog.DiscardHandler), c.DB, c.Man)
hdl := NewHandler(slog.New(slog.DiscardHandler), &DBMock{}, c.Man)
got := recordNotifications(hdl)
rec := httptest.NewRecorder()
@ -589,6 +551,7 @@ func Test_Handler_ResetDocumentHook(t *testing.T) {
if c.Resets > 0 {
assert.Equal(t, _hookID, ff[0].ID)
assert.Equal(t, _documentID, ff[0].DocumentID)
assert.Equal(t, "org1", ff[0].OrganizationID)
}
})
@ -596,11 +559,7 @@ func Test_Handler_ResetDocumentHook(t *testing.T) {
}
func Test_Handler_DeleteDocumentHook(t *testing.T) {
otherDocument := scheduledHook()
otherDocument.DocumentID = null.ValueFrom(xid.New())
cc := map[string]struct {
DB *DBMock
Man *ManagerMock
NoSession bool
OmitDoc bool
@ -609,41 +568,32 @@ func Test_Handler_DeleteDocumentHook(t *testing.T) {
Deletes int
}{
"No session in context": {
DB: storedHookDB(scheduledHook()),
Man: &ManagerMock{},
NoSession: true,
RespCode: http.StatusUnauthorized,
},
"Missing document ID parameter": {
DB: storedHookDB(scheduledHook()),
Man: &ManagerMock{},
OmitDoc: true,
RespCode: http.StatusNotFound,
},
"Missing hook ID parameter": {
DB: storedHookDB(scheduledHook()),
Man: &ManagerMock{},
OmitHook: true,
RespCode: http.StatusNotFound,
},
"Hook fetch error": {
DB: &DBMock{
FetchDocumentHookFunc: func(context.Context, xid.ID, string) (*hookCore.Hook, error) {
return nil, errors.New("boom")
"Hook of another document": {
Man: &ManagerMock{
DeleteHookFunc: func(context.Context, xid.ID, xid.ID, string, string) error {
return hookCore.ErrHookMismatch
},
},
Man: &ManagerMock{},
RespCode: http.StatusInternalServerError,
},
"Hook of another document": {
DB: storedHookDB(otherDocument),
Man: &ManagerMock{},
RespCode: http.StatusNotFound,
Deletes: 1,
},
"Error returned by man.DeleteHook": {
DB: storedHookDB(scheduledHook()),
Man: &ManagerMock{
DeleteHookFunc: func(context.Context, xid.ID, string, string) error {
DeleteHookFunc: func(context.Context, xid.ID, xid.ID, string, string) error {
return errors.New("boom")
},
},
@ -651,7 +601,6 @@ func Test_Handler_DeleteDocumentHook(t *testing.T) {
Deletes: 1,
},
"Successful deletion": {
DB: storedHookDB(scheduledHook()),
Man: &ManagerMock{},
RespCode: http.StatusNoContent,
Deletes: 1,
@ -662,7 +611,7 @@ func Test_Handler_DeleteDocumentHook(t *testing.T) {
t.Run(cn, func(t *testing.T) {
t.Parallel()
hdl := NewHandler(slog.New(slog.DiscardHandler), c.DB, c.Man)
hdl := NewHandler(slog.New(slog.DiscardHandler), &DBMock{}, c.Man)
got := recordNotifications(hdl)
rec := httptest.NewRecorder()
@ -677,6 +626,7 @@ func Test_Handler_DeleteDocumentHook(t *testing.T) {
if c.Deletes > 0 {
assert.Equal(t, _hookID, ff[0].ID)
assert.Equal(t, _documentID, ff[0].DocumentID)
assert.Equal(t, "org1", ff[0].OrganizationID)
assert.Equal(t, "u1", ff[0].UpdatedBy)
}

13
server/core/pkg/syncutil/keyed_mutex.go
View File

@ -27,7 +27,10 @@ func (km *KeyedMutex[K]) Lock(ctx context.Context, key K) (func(), error) {
select {
case l.ch <- struct{}{}:
return km.unlocker(key, l), nil
return func() {
<-l.ch
km.unref(key, l)
}, nil
case <-ctx.Done():
km.unref(key, l)
@ -64,14 +67,6 @@ func (km *KeyedMutex[K]) unref(key K, l *keyedLock) {
}
}
// unlocker returns the function that unlocks a held lock.
func (km *KeyedMutex[K]) unlocker(key K, l *keyedLock) func() {
return func() {
<-l.ch
km.unref(key, l)
}
}
// keyedLock is one key's lock: holding it is a value in ch.
type keyedLock struct {
ch chan struct{}

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

@ -62,10 +62,6 @@ function githubHook(overrides: Partial<DocumentHook> = {}) {
})
}
function failingGithubHook(status: DocumentHookStatusGitHubTracking) {
return githubHook({ status: status })
}
function mockGitHub(
options: { connected?: boolean; repositories?: string[] } = {},
) {
@ -183,7 +179,7 @@ describe("<GitHubTrackingConfigMenu>", { concurrent: false }, () => {
async ({ status, problem }, { expect }) => {
mockGitHub()
await mountMenu({ hook: failingGithubHook(status) })
await mountMenu({ hook: githubHook({ status: status }) })
expect(menuText()).toContain(
t("editor.hooks.subtext-detail", {
@ -222,7 +218,7 @@ describe("<GitHubTrackingConfigMenu>", { concurrent: false }, () => {
it("warns that a hook lost its github connection", async ({ expect }) => {
mockGitHub()
await mountMenu({ hook: failingGithubHook("missing_installation") })
await mountMenu({ hook: githubHook({ status: "missing_installation" }) })
await openHookSubMenu(t(TITLE))
@ -269,7 +265,7 @@ describe("<GitHubTrackingConfigMenu>", { concurrent: false }, () => {
"warns about $name it can no longer reach",
async ({ status, key }, { expect }) => {
mockGitHub()
await mountMenu({ hook: failingGithubHook(status) })
await mountMenu({ hook: githubHook({ status: status }) })
await openHookSubMenu(t(TITLE))
@ -284,7 +280,7 @@ describe("<GitHubTrackingConfigMenu>", { concurrent: false }, () => {
// notice does not send the reader there
it("warns that github is not set up on this server", async ({ expect }) => {
mockGitHub({ connected: false })
await mountMenu({ hook: failingGithubHook("unconfigured") })
await mountMenu({ hook: githubHook({ status: "unconfigured" }) })
await openHookSubMenu(t(TITLE))

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

@ -50,6 +50,24 @@ const targetBranchSettings = computed(
const checkStatus = computed(
() => props.hook?.status as DocumentHookStatusGitHubTracking | undefined,
)
// how many of the repository, branch and files fields an editor can change.
// A failed check leaves those up to the one at fault.
const editableFields = computed(() => {
switch (checkStatus.value) {
case "unconfigured":
case "missing_installation":
return 0
case "missing_repository":
return 1
case "missing_branch":
case "tree_truncated":
return 2
case "active":
case "initializing":
case undefined:
return 3
}
})
const status = computed(() => (props.hook ? hookStatus(props.hook) : null))
const selectedRepository = ref<string | undefined>(
activeBranchSettings.value?.repository,
@ -354,8 +372,7 @@ function close() {
cn(
'flex flex-col gap-1 opacity-100 transition-opacity duration-200',
(!fetchGitHubConnectionStatus.data.value?.connected ||
checkStatus === 'missing_installation' ||
checkStatus === 'unconfigured') &&
editableFields === 0) &&
'pointer-events-none opacity-60',
)
"
@ -366,12 +383,7 @@ function close() {
!fetchGitHubConnectionStatus.data.value?.connected ||
fetchGitHubRepositories.isLoading.value ||
fetchGitHubRepositories.state.value.data?.length === 0 ||
(!!checkStatus &&
checkStatus !== 'active' &&
checkStatus !== 'initializing' &&
checkStatus !== 'missing_repository' &&
checkStatus !== 'missing_branch' &&
checkStatus !== 'tree_truncated')
editableFields < 1
"
>
<ShadcnUiSelectLabel>
@ -416,11 +428,7 @@ function close() {
:disabled="
!selectedRepository ||
fetchGitHubBranches.isLoading.value ||
(!!checkStatus &&
checkStatus !== 'active' &&
checkStatus !== 'initializing' &&
checkStatus !== 'missing_branch' &&
checkStatus !== 'tree_truncated')
editableFields < 2
"
>
<ShadcnUiSelectLabel>
@ -468,9 +476,7 @@ function close() {
!selectedRepository ||
!selectedBranch ||
fetchGitHubPaths.isLoading.value ||
(!!checkStatus &&
checkStatus !== 'active' &&
checkStatus !== 'initializing')
editableFields < 3
"
:options="fetchGitHubPaths.state.value.data || []"
:placeholder="