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

prepare document history table for reinstantiating documents

Browse Source
This commit is contained in:
david 2026-09-22 19:38:57 +03:00
parent cdc6cd2204
commit 74389d81f6
33 changed files with 1416 additions and 456 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
server/AGENTS.md
View File

@ -96,11 +96,10 @@ The README lists the public routes; update it when changing handlers.
## Database
Postgres 18. Migrations are embedded from `core/internal/db/migrations/` and
applied by `db.New` (`rubenv/sql-migrate`). **Until the first release there
is only `001_initial.sql` and schema changes go into it in place**;
afterwards, add `NNN_<name>.sql`. An existing dev volume keeps the old
schema, so reset it with `docker compose -p oxynote -f
docker/docker-compose.dev.yaml down -v` after editing.
applied by `db.New` (`rubenv/sql-migrate`). **The system is live: every
schema change is a new `NNN_<name>.sql`; an applied migration is never
edited.** A dev volume behind on migrations can be reset with `docker
compose -p oxynote -f docker/docker-compose.dev.yaml down -v`.
**Core migrations own the Better Auth tables** (snake_case columns matching
the `fields` mappings in `auth-realtime/src/auth.ts`).

6
server/core/internal/db/db.go
View File

@ -236,6 +236,12 @@ func DetectError(err error) error {
)
case "tags_fk_organization_id_tag_name_key":
return tag.ErrDuplicateTagName
case "document_files_pkey":
return errutil.New(
http.StatusConflict,
"document_file.exists",
"file id is already in use",
)
}
}

146
server/core/internal/db/db_test.go
View File

@ -124,80 +124,104 @@ func Test_DB_Close(t *testing.T) {
}
func Test_DetectError(t *testing.T) {
cc := map[string]struct {
type tcase struct {
Err error
Result error
}{
"Not found": {
Err: sql.ErrNoRows,
Result: errutil.ErrNotFound,
}
cc := map[string]func(*testing.T) tcase{
"Not found": func(*testing.T) tcase {
return tcase{
Err: sql.ErrNoRows,
Result: errutil.ErrNotFound,
}
},
"Error pass through": {
Err: sql.ErrConnDone,
Result: sql.ErrConnDone,
"Error pass through": func(*testing.T) tcase {
return tcase{
Err: sql.ErrConnDone,
Result: sql.ErrConnDone,
}
},
"No error": {
Err: nil,
Result: nil,
"No error": func(*testing.T) tcase {
return tcase{}
},
"Unmatched pgx constraint": {
Err: &pgconn.PgError{
Code: "123",
},
Result: &pgconn.PgError{
Code: "123",
},
"Unmatched pgx constraint": func(*testing.T) tcase {
return tcase{
Err: &pgconn.PgError{
Code: "123",
},
Result: &pgconn.PgError{
Code: "123",
},
}
},
"Unmatched constraint": {
Err: errors.New("b"),
Result: errors.New("b"),
"Unmatched constraint": func(*testing.T) tcase {
return tcase{
Err: errors.New("b"),
Result: errors.New("b"),
}
},
"Duplicate branch name": func(t *testing.T) tcase {
db := prepTempDB(t)
// renaming a branch onto a sibling's name trips the unique key
// that DetectError has to recognise.
branches := prepDocumentBranches(t, db, 2, nil)
second := branches[1]
second.BranchName = branches[0].BranchName
err := db.UpdateDocumentBranchMetadata(context.Background(), *second)
require.Error(t, err)
return tcase{
Err: err,
Result: errutil.New(http.StatusBadRequest, "document_branch.duplicate_name", "branch name is already in use"),
}
},
"Duplicate file id": func(t *testing.T) tcase {
db := prepTempDB(t)
f := prepDocumentFiles(t, db, 1, nil)[0]
err := db.InsertDocumentFile(context.Background(), f)
require.Error(t, err)
return tcase{
Err: err,
Result: errutil.New(http.StatusConflict, "document_file.exists", "file id is already in use"),
}
},
"Duplicate tag name": func(t *testing.T) tcase {
db := prepTempDB(t)
existing := prepTags(t, db, 1, nil)[0]
dup := tag.NewTag(
tag.CreateInput{TagName: existing.TagName, Color: "#000000"},
existing.OrganizationID,
prepUsers(t, db, 1)[0],
)
err := db.InsertTag(context.Background(), dup)
require.Error(t, err)
return tcase{
Err: err,
Result: tag.ErrDuplicateTagName,
}
},
}
for cn, c := range cc {
for cn, cfn := range cc {
t.Run(cn, func(t *testing.T) {
t.Parallel()
c := cfn(t)
assert.Equal(t, c.Result, DetectError(c.Err))
})
}
t.Run("Duplicate branch name", func(t *testing.T) {
t.Parallel()
db := prepTempDB(t)
// renaming a branch onto a sibling's name trips the unique key that
// DetectError has to recognise.
branches := prepDocumentBranches(t, db, 2, nil)
second := branches[1]
second.BranchName = branches[0].BranchName
err := db.UpdateDocumentBranchMetadata(context.Background(), *second)
require.Error(t, err)
mapped := DetectError(err)
assert.Equal(t, http.StatusBadRequest, errutil.StatusCode(mapped, false))
assert.Contains(t, mapped.Error(), "branch name is already in use")
})
t.Run("Duplicate tag name", func(t *testing.T) {
t.Parallel()
db := prepTempDB(t)
existing := prepTags(t, db, 1, nil)[0]
dup := tag.NewTag(
tag.CreateInput{TagName: existing.TagName, Color: "#000000"},
existing.OrganizationID,
prepUsers(t, db, 1)[0],
)
err := db.InsertTag(context.Background(), dup)
require.Error(t, err)
assert.Equal(t, tag.ErrDuplicateTagName, DetectError(err))
})
}
func Test_DB_BeginTx(t *testing.T) {

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

@ -252,11 +252,7 @@ func (a *agent) UpdateDocument(ctx context.Context, doc document.Document) error
return err
}
if err := a.upsertDocumentBranch(ctx, tx, doc); err != nil {
return err
}
return a.insertDocumentBranchHistoryEntry(ctx, tx, doc.ID, doc.BranchID, doc.HistoryEntry())
return a.upsertDocumentBranch(ctx, tx, doc)
})
}

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

@ -2,13 +2,22 @@ package db
import (
"context"
"database/sql"
"errors"
"time"
sq "github.com/Masterminds/squirrel"
"github.com/jmoiron/sqlx"
"github.com/oxynote/oxynote/server/core/internal/document"
"github.com/oxynote/oxynote/server/core/internal/document/history"
"github.com/oxynote/oxynote/server/core/pkg/sqlutil"
"github.com/rs/xid"
)
// _historyAggregationDuration is the length of the time bucket that
// ordinary history entries of one branch collapse into.
var _historyAggregationDuration = 30 * time.Minute
// insertDocumentBranch inserts a branch row for a newly created document.
// Called inside the same transaction as InsertDocument.
func (a *agent) insertDocumentBranch(ctx context.Context, tx *sqlx.Tx, doc document.Document) error {
@ -250,38 +259,91 @@ func (a *agent) selectBranchSummary(b sq.SelectBuilder) sq.SelectBuilder {
).From("document_branches")
}
// insertDocumentBranchHistoryEntry inserts a history entry for a branch update
// through the given executor. The entry is keyed by branch id rather than
// branch name so that renaming a branch cannot break the reference.
func (a *agent) insertDocumentBranchHistoryEntry(
ctx context.Context,
ex sqlx.ExecerContext,
docID, branchID xid.ID,
entry document.HistoryEntry,
) error {
q, args := a.builder.Insert("document_branch_history_entries").
SetMap(map[string]any{
"id": entry.ID,
"fk_document_id": docID,
"fk_branch_id": branchID,
"content": entry.Content,
"raw_content": entry.RawContent,
"created_at": entry.CreatedAt,
}).
Suffix("ON CONFLICT (id) DO UPDATE SET " +
"content = excluded.content, raw_content = excluded.raw_content, created_at = excluded.created_at").
// InsertDocumentBranchHistoryEntry records a snapshot of a branch. Ordinary
// entries aggregate: when the branch's newest entry is an ordinary one from
// the same time bucket, that entry is updated in place instead of a row
// being added. A boundary entry (merge, fork) is always its own row and
// closes the bucket, so the state right before it survives and the next
// edit starts a new entry. The count trim runs afterwards; age trimming
// lives in the file manager, since it has to reach branches that stopped
// inserting altogether.
func (a *agent) InsertDocumentBranchHistoryEntry(ctx context.Context, entry history.Entry) error {
return sqlutil.WrapTx(ctx, a.sql, func(tx *sqlx.Tx) error {
newest, err := a.fetchNewestDocumentBranchHistoryEntry(ctx, tx, entry.BranchID)
if err != nil {
return err
}
aggregates := newest != nil &&
!newest.Boundary &&
!entry.Boundary &&
newest.CreatedAt.Truncate(_historyAggregationDuration).Equal(entry.CreatedAt.Truncate(_historyAggregationDuration))
if aggregates {
q, args := a.builder.Update("document_branch_history_entries").
SetMap(map[string]any{
"document_name": entry.DocumentName,
"icon": entry.Icon,
"content": entry.Content,
"hooks": entry.Hooks,
"fk_last_updated_by": entry.LastUpdatedBy,
"created_at": entry.CreatedAt,
}).
Where(sq.Eq{"id": newest.ID}).
MustSql()
_, err := tx.ExecContext(ctx, q, args...)
return err
}
q, args := a.builder.Insert("document_branch_history_entries").
SetMap(map[string]any{
"id": entry.ID,
"fk_document_id": entry.DocumentID,
"fk_branch_id": entry.BranchID,
"document_name": entry.DocumentName,
"icon": entry.Icon,
"content": entry.Content,
"hooks": entry.Hooks,
"fk_last_updated_by": entry.LastUpdatedBy,
"boundary": entry.Boundary,
"created_at": entry.CreatedAt,
}).
MustSql()
if _, err := tx.ExecContext(ctx, q, args...); err != nil {
return err
}
return a.trimDocumentBranchHistoryEntries(ctx, tx, entry.BranchID)
})
}
// fetchNewestDocumentBranchHistoryEntry retrieves the branch's newest
// history entry, or nil when the branch has none.
func (a *agent) fetchNewestDocumentBranchHistoryEntry(ctx context.Context, q sqlx.QueryerContext, branchID xid.ID) (*history.Entry, error) {
sqlq, args := a.selectDocumentBranchHistoryEntry(a.builder.Select()).
Where(sq.Eq{"fk_branch_id": branchID}).
OrderBy("created_at DESC", "id DESC").
Limit(1).
MustSql()
if _, err := ex.ExecContext(ctx, q, args...); err != nil {
return err
var entry history.Entry
err := sqlx.GetContext(ctx, q, &entry, sqlq, args...)
if err != nil {
if errors.Is(err, sql.ErrNoRows) {
return nil, nil //nolint:nilnil // no entry is a regular outcome, not an error
}
return nil, err
}
return a.trimDocumentBranchHistoryEntries(ctx, ex, branchID)
return &entry, nil
}
// trimDocumentBranchHistoryEntries keeps only the newest entries of a branch.
// Age trimming lives in the file manager instead, since it has to reach
// branches that stopped inserting altogether.
func (a *agent) trimDocumentBranchHistoryEntries(ctx context.Context, ex sqlx.ExecerContext, branchID xid.ID) error {
// a zero limit means unlimited retention; without this guard the
// subquery below would emit LIMIT 0 and the delete would drop every
@ -293,7 +355,7 @@ func (a *agent) trimDocumentBranchHistoryEntries(ctx context.Context, ex sqlx.Ex
b := a.builder.Select("id").
From("document_branch_history_entries").
Where(sq.Eq{"fk_branch_id": branchID}).
OrderBy("created_at DESC").
OrderBy("created_at DESC", "id DESC").
Limit(a.opts.MaxDocumentHistoryEntries).
Prefix("id NOT IN (").
Suffix(")")
@ -308,6 +370,23 @@ func (a *agent) trimDocumentBranchHistoryEntries(ctx context.Context, ex sqlx.Ex
return err
}
// selectDocumentBranchHistoryEntry prepares a sql select statement for
// fetching history entries.
func (a *agent) selectDocumentBranchHistoryEntry(b sq.SelectBuilder) sq.SelectBuilder {
return b.Columns(
`id AS "id"`,
`fk_document_id AS "fk_document_id"`,
`fk_branch_id AS "fk_branch_id"`,
`document_name AS "document_name"`,
`icon AS "icon"`,
`content AS "content"`,
`hooks AS "hooks"`,
`fk_last_updated_by AS "fk_last_updated_by"`,
`boundary AS "boundary"`,
`created_at AS "created_at"`,
).From("document_branch_history_entries")
}
// documentBranchRow maps a document's branch onto the document_branches
// columns.
func documentBranchRow(doc document.Document) map[string]any {

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

@ -4,6 +4,7 @@ import (
"context"
"database/sql"
"net/http"
"slices"
"strconv"
"testing"
"time"
@ -12,7 +13,9 @@ import (
"github.com/guregu/null/v5"
"github.com/jmoiron/sqlx"
"github.com/oxynote/oxynote/server/core/internal/document"
"github.com/oxynote/oxynote/server/core/internal/document/history"
"github.com/oxynote/oxynote/server/core/internal/document/hook"
"github.com/oxynote/oxynote/server/core/internal/document/hook/processor"
"github.com/oxynote/oxynote/server/core/pkg/errutil"
"github.com/oxynote/oxynote/server/core/pkg/sqlutil"
"github.com/oxynote/oxynote/server/core/pkg/testutil"
@ -653,25 +656,23 @@ func Test_agent_FetchDocumentBranchesAfter(t *testing.T) {
testutil.AssertFilterEqual(t, all[2:], res, null.Value[xid.ID]{})
}
// prepHistoryEntries writes count entries of the document's branch, one per
// aggregation bucket so each lands as its own entry, oldest first.
func prepHistoryEntries(t *testing.T, db *DB, doc *document.Document, count int) []document.HistoryEntry {
// prepHistoryEntries writes count ordinary entries of the document's branch,
// one per aggregation bucket so each lands as its own row, oldest first.
func prepHistoryEntries(t *testing.T, db *DB, doc *document.Document, count int) []history.Entry {
t.Helper()
res := make([]document.HistoryEntry, count)
res := make([]history.Entry, count)
base := timeutil.Now().Truncate(time.Second)
for i := range count {
doc.UpdatedAt = base.Add(-time.Duration(count-i) * time.Hour)
entry := doc.HistoryEntry()
entry := history.NewEntry(*doc,
base.Add(-time.Duration(count-i)*time.Hour),
null.String{},
history.Hooks{},
false,
)
require.NoError(t, db.insertDocumentBranchHistoryEntry(
context.Background(),
db.sql,
doc.ID,
doc.BranchID,
entry,
))
require.NoError(t, db.InsertDocumentBranchHistoryEntry(context.Background(), entry))
res[i] = entry
}
@ -679,6 +680,28 @@ func prepHistoryEntries(t *testing.T, db *DB, doc *document.Document, count int)
return res
}
// assertHistoryEntryEqual compares two entries, matching hook settings as
// JSON since jsonb normalises their spacing.
func assertHistoryEntryEqual(t *testing.T, exp, got history.Entry) {
t.Helper()
require.Len(t, got.Hooks, len(exp.Hooks))
// the hook slices are cloned: cases share their definitions and the
// subtests run in parallel.
exp.Hooks = slices.Clone(exp.Hooks)
got.Hooks = slices.Clone(got.Hooks)
for i := range exp.Hooks {
assert.JSONEq(t, string(exp.Hooks[i].Settings), string(got.Hooks[i].Settings))
exp.Hooks[i].Settings = nil
got.Hooks[i].Settings = nil
}
assert.Equal(t, exp, got)
}
// countHistoryEntries returns how many entries the branch currently has.
func countHistoryEntries(t *testing.T, db *DB, branchID xid.ID) int {
t.Helper()
@ -733,7 +756,7 @@ func Test_agent_trimDocumentBranchHistoryEntries(t *testing.T) {
}
// the survivors are the newest ones.
var ids []string
var ids []xid.ID
q, args := db.builder.Select("id").
From("document_branch_history_entries").
@ -741,7 +764,7 @@ func Test_agent_trimDocumentBranchHistoryEntries(t *testing.T) {
MustSql()
require.NoError(t, db.sql.Select(&ids, q, args...))
assert.ElementsMatch(t, []string{entries[2].ID, entries[3].ID}, ids)
assert.ElementsMatch(t, []xid.ID{entries[2].ID, entries[3].ID}, ids)
})
}
}
@ -770,34 +793,110 @@ func Test_agent_DeleteExpiredDocumentBranchHistoryEntries(t *testing.T) {
assert.Equal(t, 1, countHistoryEntries(t, db, doc.BranchID))
}
func Test_agent_insertDocumentBranchHistoryEntry(t *testing.T) {
func Test_agent_InsertDocumentBranchHistoryEntry(t *testing.T) {
type tcase struct {
DocumentID xid.ID
BranchID xid.ID
HistoryEntry document.HistoryEntry
Trimmed int
Err error
Entry history.Entry
// Rows is how many entries the branch holds afterwards.
Rows int
// Newest is the entry the branch's newest row is expected to
// equal afterwards.
Newest history.Entry
Err error
}
base := time.Date(2026, 1, 1, 10, 0, 0, 0, time.UTC)
hooks := history.Hooks{
{
Type: hook.TypeURLWatcher,
BlockID: null.StringFrom("b1"),
Settings: processor.Settings(`{"url":"https://example.com"}`),
},
}
cc := map[string]func(*testing.T, *DB) tcase{
"Non-existent branch": func(t *testing.T, db *DB) tcase {
doc := prepDocuments(t, db, 1, nil)[0]
entry := doc.HistoryEntry()
doc.BranchID = xid.New()
return tcase{
DocumentID: doc.ID,
BranchID: xid.New(),
HistoryEntry: entry,
Err: assert.AnError,
Entry: history.NewEntry(*doc, base, null.String{}, hooks, false),
Err: assert.AnError,
}
},
"Successful insert": func(t *testing.T, db *DB) tcase {
"First entry of a branch": func(t *testing.T, db *DB) tcase {
doc := prepDocuments(t, db, 1, nil)[0]
user := prepUsers(t, db, 1)[0]
entry := history.NewEntry(*doc, base.Add(5*time.Minute), null.StringFrom(user), hooks, false)
return tcase{
DocumentID: doc.ID,
BranchID: doc.BranchID,
HistoryEntry: doc.HistoryEntry(),
Entry: entry,
Rows: 1,
Newest: entry,
}
},
"Edit in the same bucket updates the entry in place": 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.String{}, history.Hooks{}, false)
require.NoError(t, db.InsertDocumentBranchHistoryEntry(context.Background(), first))
doc.DocumentName = "Renamed"
doc.Icon = "📕"
doc.Content.Content[0].Text = "Edited."
entry := history.NewEntry(*doc, base.Add(29*time.Minute), null.StringFrom(user), hooks, false)
newest := entry
newest.ID = first.ID
return tcase{
Entry: entry,
Rows: 1,
Newest: newest,
}
},
"Edit in the next bucket inserts": func(t *testing.T, db *DB) tcase {
doc := prepDocuments(t, db, 1, nil)[0]
first := history.NewEntry(*doc, base.Add(5*time.Minute), null.String{}, history.Hooks{}, false)
require.NoError(t, db.InsertDocumentBranchHistoryEntry(context.Background(), first))
entry := history.NewEntry(*doc, base.Add(31*time.Minute), null.String{}, hooks, false)
return tcase{
Entry: entry,
Rows: 2,
Newest: entry,
}
},
"Boundary entry in an occupied bucket inserts": func(t *testing.T, db *DB) tcase {
doc := prepDocuments(t, db, 1, nil)[0]
first := history.NewEntry(*doc, base.Add(5*time.Minute), null.String{}, history.Hooks{}, false)
require.NoError(t, db.InsertDocumentBranchHistoryEntry(context.Background(), first))
entry := history.NewEntry(*doc, base.Add(6*time.Minute), null.String{}, hooks, true)
return tcase{
Entry: entry,
Rows: 2,
Newest: entry,
}
},
"Edit after a boundary in the same bucket inserts": func(t *testing.T, db *DB) tcase {
doc := prepDocuments(t, db, 1, nil)[0]
boundary := history.NewEntry(*doc, base.Add(5*time.Minute), null.String{}, history.Hooks{}, true)
require.NoError(t, db.InsertDocumentBranchHistoryEntry(context.Background(), boundary))
entry := history.NewEntry(*doc, base.Add(6*time.Minute), null.String{}, hooks, false)
return tcase{
Entry: entry,
Rows: 2,
Newest: entry,
}
},
"Insert trims the branch down to the retained entries": func(t *testing.T, db *DB) tcase {
@ -806,32 +905,13 @@ func Test_agent_insertDocumentBranchHistoryEntry(t *testing.T) {
prepHistoryEntries(t, db, doc, 3)
db.opts.MaxDocumentHistoryEntries = 2
doc.UpdatedAt = timeutil.Now().Truncate(time.Second)
entry := history.NewEntry(*doc, timeutil.Now().Truncate(time.Second), null.String{}, hooks, false)
return tcase{
DocumentID: doc.ID,
BranchID: doc.BranchID,
HistoryEntry: doc.HistoryEntry(),
Trimmed: 2,
}
},
"Successful update of an existing entry": func(t *testing.T, db *DB) tcase {
doc := prepDocuments(t, db, 1, nil)[0]
require.NoError(t, db.insertDocumentBranchHistoryEntry(
context.Background(),
db.sql,
doc.ID,
doc.BranchID,
doc.HistoryEntry(),
))
doc.RawContent = []byte("updated raw content")
return tcase{
DocumentID: doc.ID,
BranchID: doc.BranchID,
HistoryEntry: doc.HistoryEntry(),
Entry: entry,
Rows: 2,
Newest: entry,
}
},
}
@ -843,33 +923,39 @@ func Test_agent_insertDocumentBranchHistoryEntry(t *testing.T) {
db := prepTempDB(t)
c := cfn(t, db)
err := db.insertDocumentBranchHistoryEntry(context.Background(), db.sql, c.DocumentID, c.BranchID, c.HistoryEntry)
err := db.InsertDocumentBranchHistoryEntry(context.Background(), c.Entry)
testutil.RequireEqualError(t, c.Err, err)
if c.Trimmed != 0 {
assert.Equal(t, c.Trimmed, countHistoryEntries(t, db, c.BranchID))
}
if err != nil {
return
}
var entry document.HistoryEntry
assert.Equal(t, c.Rows, countHistoryEntries(t, db, c.Entry.BranchID))
q, args := db.builder.Select(
`id AS "id"`,
`fk_document_id AS "fk_document_id"`,
`content AS "content"`,
`raw_content AS "raw_content"`,
`created_at AS "created_at"`,
).From("document_branch_history_entries").
Where(sq.Eq{
"id": c.HistoryEntry.ID,
}).MustSql()
err = db.sql.Get(&entry, q, args...)
newest, err := db.fetchNewestDocumentBranchHistoryEntry(context.Background(), db.sql, c.Entry.BranchID)
require.NoError(t, err)
testutil.AssertFilterEqual(t, c.HistoryEntry, entry)
require.NotNil(t, newest)
assertHistoryEntryEqual(t, c.Newest, *newest)
})
}
}
func Test_agent_fetchNewestDocumentBranchHistoryEntry(t *testing.T) {
t.Parallel()
db := prepTempDB(t)
doc := prepDocuments(t, db, 1, nil)[0]
// no entries
res, err := db.fetchNewestDocumentBranchHistoryEntry(context.Background(), db.sql, doc.BranchID)
require.NoError(t, err)
assert.Nil(t, res)
// the newest of several
entries := prepHistoryEntries(t, db, doc, 3)
res, err = db.fetchNewestDocumentBranchHistoryEntry(context.Background(), db.sql, doc.BranchID)
require.NoError(t, err)
require.NotNil(t, res)
assertHistoryEntryEqual(t, entries[2], *res)
}

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

@ -8,17 +8,13 @@ import (
"github.com/guregu/null/v5"
"github.com/jmoiron/sqlx"
"github.com/oxynote/oxynote/server/core/internal/document/file"
"github.com/oxynote/oxynote/server/core/pkg/errutil"
"github.com/rs/xid"
)
// InsertDocumentFile inserts a new document file into the database.
// Re-uploading into the same block reuses the file id, so the insert
// refreshes the existing row instead of failing: created_at is bumped
// to re-arm the retention grace period for what is a new object, and
// the unreferenced mark is cleared. A row owned by another organization
// is left alone and reported as errutil.ErrNotFound, since the id is
// client-chosen and must not reassign a file across organizations.
// InsertDocumentFile inserts a new document file into the database. The id
// is client-chosen and unique across organizations: an id that already has
// a row is refused, so an object a history entry points at is never
// overwritten in place.
func (a *agent) InsertDocumentFile(ctx context.Context, f file.File) error {
q, args := a.builder.Insert("document_files").
SetMap(map[string]any{
@ -33,29 +29,11 @@ func (a *agent) InsertDocumentFile(ctx context.Context, f file.File) error {
"created_at": f.CreatedAt,
"unreferenced_at": f.UnreferencedAt,
}).
Suffix("ON CONFLICT (id) DO UPDATE SET " +
"location = excluded.location, storage_key = excluded.storage_key, " +
"fk_document_id = excluded.fk_document_id, fk_organization_id = excluded.fk_organization_id, " +
"name = excluded.name, size = excluded.size, content_type = excluded.content_type, " +
"created_at = excluded.created_at, unreferenced_at = NULL " +
"WHERE document_files.fk_organization_id = excluded.fk_organization_id").
MustSql()
res, err := a.sql.ExecContext(ctx, q, args...)
if err != nil {
return err
}
_, err := a.sql.ExecContext(ctx, q, args...)
n, err := res.RowsAffected()
if err != nil {
return err
}
if n == 0 {
return errutil.ErrNotFound
}
return nil
return err
}
// FetchDocumentFile retrieves a document file by its block ID from the database.

28
server/core/internal/db/document_file_test.go
View File

@ -3,6 +3,7 @@ package db
import (
"context"
"database/sql"
"net/http"
"strconv"
"testing"
"time"
@ -11,6 +12,7 @@ import (
"github.com/oxynote/oxynote/server/core/internal/document"
"github.com/oxynote/oxynote/server/core/internal/document/comment"
"github.com/oxynote/oxynote/server/core/internal/document/file"
"github.com/oxynote/oxynote/server/core/internal/document/history"
"github.com/oxynote/oxynote/server/core/pkg/errutil"
"github.com/oxynote/oxynote/server/core/pkg/testutil"
"github.com/oxynote/oxynote/server/core/pkg/timeutil"
@ -77,29 +79,20 @@ func Test_agent_InsertDocumentFile(t *testing.T) {
}
cc := map[string]func(*testing.T, *DB) tcase{
"Re-upload refreshes the existing row": func(t *testing.T, db *DB) tcase {
f := prepDocumentFiles(t, db, 1, func(_ int, f *file.File) {
f.UnreferencedAt = null.TimeFrom(timeutil.Now().Truncate(time.Second))
})[0]
f.Location = file.LocationComment
f.StorageKey = "organizations/org/documents/doc/files/replaced"
"Existing id": func(t *testing.T, db *DB) tcase {
f := prepDocumentFiles(t, db, 1, nil)[0]
f.Name = "notes.zip"
f.Size = 2048
f.ContentType = "application/zip"
f.CreatedAt = timeutil.Now().Truncate(time.Second)
f.UnreferencedAt = null.Time{}
return tcase{File: f}
return tcase{File: f, Err: errutil.New(http.StatusConflict, "document_file.exists", "file id is already in use")}
},
"Re-upload from another organization": func(t *testing.T, db *DB) tcase {
"Existing id from another organization": func(t *testing.T, db *DB) tcase {
f := prepDocumentFiles(t, db, 1, nil)[0]
doc := prepDocuments(t, db, 1, nil)[0]
f.DocumentID = null.ValueFrom(doc.ID)
f.OrganizationID = null.StringFrom(doc.OrganizationID)
return tcase{File: f, Err: errutil.ErrNotFound}
return tcase{File: f, Err: errutil.New(http.StatusConflict, "document_file.exists", "file id is already in use")}
},
"Missing document": func(_ *testing.T, _ *DB) tcase {
return tcase{
@ -250,12 +243,9 @@ func Test_agent_CheckDocumentFileReferenced(t *testing.T) {
doc := prepDocuments(t, db, 1, nil)[0]
doc.Content.Content[0].Attrs = document.Attributes{"uid": "file-ref"}
require.NoError(t, db.insertDocumentBranchHistoryEntry(
require.NoError(t, db.InsertDocumentBranchHistoryEntry(
context.Background(),
db.sql,
doc.ID,
doc.BranchID,
doc.HistoryEntry(),
history.NewEntry(*doc, timeutil.Now(), null.String{}, nil, false),
))
return "file-ref", doc.ID, true

39
server/core/internal/db/document_test.go
View File

@ -711,33 +711,6 @@ func Test_agent_UpdateDocument(t *testing.T) {
assert.Equal(t, assert.AnError, err)
})
t.Run("HistoryEntries are kept per branch", func(t *testing.T) {
t.Parallel()
db := prepTempDB(t)
// two branches of one document, updated inside the same
// aggregation window.
branches := prepDocumentBranches(t, db, 2, nil)
now := timeutil.Now().Truncate(time.Second)
for _, b := range branches {
b.UpdatedAt = now
require.NoError(t, db.UpdateDocument(context.Background(), *b))
}
var logs uint64
q, args := db.builder.Select("COUNT(*)").From("document_branch_history_entries").
Where(sq.Eq{
"fk_document_id": branches[0].ID,
}).MustSql()
require.NoError(t, db.sql.Get(&logs, q, args...))
assert.Equal(t, uint64(2), logs)
})
type tcase struct {
Document document.Document
Err error
@ -783,18 +756,6 @@ func Test_agent_UpdateDocument(t *testing.T) {
err := db.sql.Get(&doc, q, args...)
require.NoError(t, err)
assert.Equal(t, c.Document, doc)
var logs uint64
q, args = db.builder.Select("COUNT(*)").From("document_branch_history_entries").
Where(sq.Eq{
"fk_document_id": c.Document.ID,
}).MustSql()
err = db.sql.Get(&logs, q, args...)
require.NoError(t, err)
assert.Equal(t, c.Document, doc)
assert.Equal(t, uint64(1), logs)
})
}
}

39
server/core/internal/db/migrations/002_history_entry_snapshot.sql Normal file
View File

@ -0,0 +1,39 @@
-- +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
DROP COLUMN raw_content,
ADD COLUMN document_name TEXT NULL,
ADD COLUMN icon TEXT NULL,
ADD COLUMN fk_last_updated_by TEXT REFERENCES users ON DELETE SET NULL,
ADD COLUMN hooks JSONB NOT NULL,
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);
-- +migrate Down
DROP INDEX document_branch_history_entries_created_at_idx;
DROP INDEX document_branch_history_entries_fk_branch_id_created_at_idx;
CREATE INDEX document_branch_history_entries_fk_branch_id_idx ON document_branch_history_entries (fk_branch_id);
DROP INDEX document_branch_history_entries_fk_last_updated_by_idx;
TRUNCATE document_branch_history_entries;
ALTER TABLE document_branch_history_entries
DROP COLUMN boundary,
DROP COLUMN hooks,
DROP COLUMN fk_last_updated_by,
DROP COLUMN icon,
DROP COLUMN document_name,
ADD COLUMN raw_content BYTEA NULL;

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

@ -15,18 +15,35 @@ Cross-service storage rules (Hocuspocus, Yjs) live in
who edited in that persist; `UpsertDocumentMaintainers` only adds. There
is no removal path.
- **Duplication copies, never shares**: `Document.Duplicate` regenerates
every block uid, rewrites image/file `src` paths to the new document, and
returns old→new maps for files (copied server-side) and block uids (hooks
re-anchored; unmapped ones dropped).
- History entries pin the files they reference, so
`DB_MAX_DOCUMENT_HISTORY_ENTRIES` and `DB_DOCUMENT_HISTORY_RETENTION` also
decide how long a removed image survives.
every block uid, rewrites image/file `src` paths to the new document (the
file id is read off the src), and returns old→new maps for files (copied
server-side) and block uids (hooks re-anchored; unmapped ones dropped).
## History entries
`document_branch_history_entries` are restorable snapshots: name, icon,
content, the branch's live hook definitions (type, block, settings; never
watcher state) and the author, written in the same transaction as the
branch update through `InsertDocumentBranchHistoryEntry`. Rules:
- Ordinary edits within one 30-minute bucket update the branch's newest
entry in place. A persist that changes nothing writes no entry; a system
write is unattributed (null author) and still takes the time of the
write.
- Merge and fork write a **boundary** entry that never aggregates and closes
its bucket: the next edit starts a new entry. The merge entry lists the
source's hooks.
- Entries pin the files they reference, so `DB_MAX_DOCUMENT_HISTORY_ENTRIES`
and `DB_DOCUMENT_HISTORY_RETENTION` also decide how long a removed image
survives.
## Files and hooks
Files live at `organizations/{org}/documents/{doc}/files/{blockUID}` (the
file id is the block uid, so a re-upload overwrites in place), tracked in
`document_files` with name, size and sniffed content type. `GET
Files live at `organizations/{org}/documents/{doc}/files/{fileId}`. The id
is a nanoid the editor mints per upload, not the block uid, and an id that
already has a row is refused with 409, so a history entry keeps its object
when a block's file is replaced. Rows are `document_files` with name, size
and sniffed content type. `GET
/api/documents/{id}/files/{fileId}-{fileName}` cuts the 21-char nanoid off
the front and ignores the name; `Content-Disposition` is `inline` only for
`file.Viewable`. The key is an S3 object key or a path under the storage

64
server/core/internal/document/document.go
View File

@ -2,6 +2,7 @@
package document
import (
"bytes"
_ "embed"
"encoding/json"
"fmt"
@ -27,16 +28,6 @@ var (
// DefaultBranch is the name of the default branch for a document.
const DefaultBranch = "main"
const (
// _aggregationDuration is the duration for which document changes are aggregated.
_aggregationDuration = 30 * time.Minute
// _historyEntryTimeLayout is the layout used for timestamps in history
// entry IDs.
_historyEntryTimeLayout = "2006-01-02T15:04:05"
)
// Branch holds branch-specific content and metadata for a document.
type Branch struct {
// BranchID is the unique identifier for this branch row.
@ -276,46 +267,27 @@ func (d Document) Title() string {
return d.DocumentName + " on branch " + d.BranchName
}
// HistoryEntry returns a history entry for the current state of the document.
// Entries aggregate per branch and per time bucket: the branch is part of the
// ID so that two branches of one document edited within the same bucket keep
// separate entries instead of overwriting each other.
func (d Document) HistoryEntry() HistoryEntry {
id := fmt.Sprintf(
"%s-%s-%s",
d.ID,
d.BranchID,
d.UpdatedAt.Truncate(_aggregationDuration).
Format(_historyEntryTimeLayout),
)
return HistoryEntry{
ID: id,
DocumentID: d.ID,
Content: d.Content,
RawContent: d.RawContent,
CreatedAt: d.UpdatedAt,
// SnapshotEqual reports whether the other document would produce the same
// snapshot: the same name, icon and content. Content is compared as
// JSON, which encoding/json emits with sorted keys.
func (d Document) SnapshotEqual(o Document) bool {
if d.DocumentName != o.DocumentName || d.Icon != o.Icon {
return false
}
}
// HistoryEntry represents one entry in a document's history.
type HistoryEntry struct {
// ID is the unique identifier for the entry.
ID string `json:"id" db:"id"`
dc, err := json.Marshal(d.Content)
if err != nil {
// NOCOV: a block tree of plain values always marshals.
return false
}
// DocumentID is the identifier for the document associated
// with this entry.
DocumentID xid.ID `json:"documentId" db:"fk_document_id"`
oc, err := json.Marshal(o.Content)
if err != nil {
// NOCOV: a block tree of plain values always marshals.
return false
}
// Content is the content of the document at the time of the entry.
Content RootBlock `json:"content" db:"content"`
// RawContent is the raw content of the document, typically in
// a text format.
RawContent []byte `json:"rawContent" db:"raw_content"`
// CreatedAt is the timestamp when the entry was created.
CreatedAt time.Time `json:"createdAt" db:"created_at"`
return bytes.Equal(dc, oc)
}
// CreateInput is the input structure for document operations.

69
server/core/internal/document/document_test.go
View File

@ -262,27 +262,58 @@ func Test_Document_ApplyBranchUpdate(t *testing.T) {
}
}
func Test_Document_HistoryEntry(t *testing.T) {
func Test_Document_SnapshotEqual(t *testing.T) {
t.Parallel()
doc := stubDocument()
doc.UpdatedAt = time.Date(2026, 1, 1, 10, 44, 30, 0, time.UTC)
cc := map[string]struct {
Mutate func(d *Document)
Equal bool
}{
"Identical": {
Mutate: func(_ *Document) {},
Equal: true,
},
"Different name": {
Mutate: func(d *Document) {
d.DocumentName = "Other"
},
},
"Different icon": {
Mutate: func(d *Document) {
d.Icon = "📕"
},
},
"Different block": {
Mutate: func(d *Document) {
d.Content.Content = d.Content.Content[1:]
},
},
"Different attribute": {
Mutate: func(d *Document) {
d.Content.Content[0].Attrs = map[string]any{"uid": "changed"}
},
},
"Different raw content only": {
Mutate: func(d *Document) {
d.RawContent = []byte("other")
},
Equal: true,
},
}
entry := doc.HistoryEntry()
for cn, c := range cc {
t.Run(cn, func(t *testing.T) {
t.Parallel()
// the timestamp is truncated to the 30-minute aggregation window.
assert.Equal(t, doc.ID.String()+"-"+doc.BranchID.String()+"-2026-01-01T10:30:00", entry.ID)
assert.Equal(t, doc.ID, entry.DocumentID)
assert.Equal(t, doc.Content, entry.Content)
assert.Equal(t, doc.RawContent, entry.RawContent)
assert.Equal(t, doc.UpdatedAt, entry.CreatedAt)
doc := stubDocument()
other := stubDocument()
other.ID = doc.ID
other.BranchID = doc.BranchID
c.Mutate(&other)
// a second branch of the same document updated within the same window
// keeps an entry of its own.
other := doc
other.BranchID = xid.New()
assert.NotEqual(t, entry.ID, other.HistoryEntry().ID)
assert.Equal(t, c.Equal, doc.SnapshotEqual(other))
})
}
}
func Test_Document_Duplicate(t *testing.T) {
@ -327,8 +358,8 @@ func Test_Document_Duplicate(t *testing.T) {
doc.Content.Content[0] = Block{
Type: BlockNodeImageBlock,
Attrs: Attributes{
"uid": "img1",
"src": "https://app.test/core" + FilePath(doc.ID, "img1", "shot.png"),
"uid": "img-1-aaaaaaaaaaaaaaa",
"src": "https://app.test/core" + FilePath(doc.ID, "img-1-aaaaaaaaaaaaaaa", "shot.png"),
},
}
@ -337,7 +368,7 @@ func Test_Document_Duplicate(t *testing.T) {
Check: func(t *testing.T, _, dup Document, files map[string]string) {
require.Len(t, files, 1)
newID, ok := files["img1"]
newID, ok := files["img-1-aaaaaaaaaaaaaaa"]
require.True(t, ok)
// the duplicate refers to its own copy under its own document.

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

@ -21,9 +21,8 @@ func Folder(organizationID string, documentID xid.ID) string {
return fmt.Sprintf(_folderFormat, organizationID, documentID)
}
// Key returns the storage key of the given document's file. The key
// is the file id alone: a re-upload into the same block then overwrites
// its object in place, so no object is ever left without a row naming it.
// Key returns the storage key of the given document's file: the file id
// under the document's folder.
func Key(organizationID string, documentID xid.ID, id string) string {
return path.Join(Folder(organizationID, documentID), id)
}

134
server/core/internal/document/history/history.go Normal file
View File

@ -0,0 +1,134 @@
// Package history defines the restorable history entries of a document branch.
package history
import (
"database/sql/driver"
"encoding/json"
"errors"
"time"
"github.com/guregu/null/v5"
"github.com/oxynote/oxynote/server/core/internal/document"
"github.com/oxynote/oxynote/server/core/internal/document/hook"
"github.com/oxynote/oxynote/server/core/internal/document/hook/processor"
"github.com/rs/xid"
)
// Entry is a snapshot of a branch that a restore can put back.
type Entry struct {
// ID is the unique identifier for the entry.
ID xid.ID `json:"id" db:"id"`
// DocumentID is the identifier of the document the entry belongs to.
DocumentID xid.ID `json:"documentId" db:"fk_document_id"`
// BranchID is the identifier of the branch the entry belongs to.
BranchID xid.ID `json:"branchId" db:"fk_branch_id"`
// DocumentName is the name of the document at the time of the entry.
DocumentName string `json:"documentName" db:"document_name"`
// Icon is the icon of the document at the time of the entry.
Icon string `json:"icon" db:"icon"`
// Content is the content of the branch at the time of the entry.
Content document.RootBlock `json:"content" db:"content"`
// Hooks are the hooks the branch carried at the time of the entry.
Hooks Hooks `json:"hooks" db:"hooks"`
// LastUpdatedBy is the user whose change the entry records. Null
// for a system write, which no user made.
LastUpdatedBy null.String `json:"lastUpdatedBy" db:"fk_last_updated_by"`
// Boundary indicates whether the entry was written by an operation
// that replaced the whole content and therefore never aggregates.
Boundary bool `json:"boundary" db:"boundary"`
// CreatedAt is the timestamp when the entry was taken.
CreatedAt time.Time `json:"createdAt" db:"created_at"`
}
// NewEntry takes a snapshot of the branch as it is now, at the given time.
// Aggregation into time buckets is decided where the entry is stored, so
// every call mints a fresh id. A boundary entry is one written by an
// operation that replaced the whole content (merge, fork); it never joins
// a bucket and closes the one it lands in.
func NewEntry(doc document.Document, at time.Time, by null.String, hooks Hooks, boundary bool) Entry {
return Entry{
ID: xid.New(),
DocumentID: doc.ID,
BranchID: doc.BranchID,
DocumentName: doc.DocumentName,
Icon: doc.Icon,
Content: doc.Content,
Hooks: hooks,
LastUpdatedBy: by,
Boundary: boundary,
CreatedAt: at,
}
}
// Hook is what it takes to re-create a hook: its type, the block it is
// anchored to and its settings. Watcher state is not part of it.
type Hook struct {
// Type is the type of the hook.
Type hook.Type `json:"type"`
// BlockID is the block the hook is anchored to; null for a document
// level hook.
BlockID null.String `json:"blockId"`
// Settings are the hook's settings in JSON format.
Settings processor.Settings `json:"settings"`
}
// Hooks is the list of hooks stored on a history entry.
type Hooks []Hook
// NewHooks reduces hooks to what a history entry records of them.
func NewHooks(hooks []hook.Hook) Hooks {
res := make(Hooks, 0, len(hooks))
for _, hk := range hooks {
res = append(res, Hook{
Type: hk.Type,
BlockID: hk.BlockID,
Settings: hk.Settings,
})
}
return res
}
// Value transforms the hooks into a database entry.
func (hh Hooks) Value() (driver.Value, error) {
if hh == nil {
return json.Marshal(Hooks{})
}
return json.Marshal(hh)
}
// Scan transforms a database entry into hooks.
func (hh *Hooks) Scan(src any) error {
var pv []byte
switch v := src.(type) {
case []byte:
pv = v
case string:
pv = []byte(v)
default:
return errors.New("invalid hooks type")
}
data := Hooks{}
if err := json.Unmarshal(pv, &data); err != nil {
return err
}
*hh = data
return nil
}

193
server/core/internal/document/history/history_test.go Normal file
View File

@ -0,0 +1,193 @@
package history
import (
"testing"
"time"
"github.com/guregu/null/v5"
"github.com/oxynote/oxynote/server/core/internal/document"
"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/testutil"
"github.com/rs/xid"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.uber.org/goleak"
)
func TestMain(m *testing.M) {
goleak.VerifyTestMain(m)
}
func Test_NewEntry(t *testing.T) {
t.Parallel()
doc := document.Document{
ID: xid.New(),
OrganizationID: "org-1",
BranchID: xid.New(),
BranchName: document.DefaultBranch,
DocumentName: "Runbook",
Icon: "📘",
Content: document.RootBlock{
Type: "doc",
Content: []document.Block{
{Type: document.BlockNodeParagraph, Text: "Hello"},
},
},
}
at := time.Date(2026, 1, 1, 10, 44, 30, 0, time.UTC)
hooks := Hooks{
{
Type: hook.TypeURLWatcher,
BlockID: null.StringFrom("b1"),
Settings: processor.Settings(`{"url":"https://example.com"}`),
},
}
entry := NewEntry(doc, at, null.StringFrom("user-1"), hooks, true)
assert.False(t, entry.ID.IsNil())
assert.Equal(t, doc.ID, entry.DocumentID)
assert.Equal(t, doc.BranchID, entry.BranchID)
assert.Equal(t, doc.DocumentName, entry.DocumentName)
assert.Equal(t, doc.Icon, entry.Icon)
assert.Equal(t, doc.Content, entry.Content)
assert.Equal(t, hooks, entry.Hooks)
assert.Equal(t, null.StringFrom("user-1"), entry.LastUpdatedBy)
assert.True(t, entry.Boundary)
assert.Equal(t, at, entry.CreatedAt)
// every call mints its own id.
assert.NotEqual(t, entry.ID, NewEntry(doc, at, null.String{}, nil, false).ID)
}
func Test_NewHooks(t *testing.T) {
t.Parallel()
assert.Equal(t, Hooks{}, NewHooks(nil))
assert.Equal(t, Hooks{
{
Type: hook.TypeURLWatcher,
BlockID: null.StringFrom("b1"),
Settings: processor.Settings(`{"url":"https://example.com"}`),
},
{
Type: hook.TypeScheduledReminder,
Settings: processor.Settings(`{"cron":"0 9 * * 1"}`),
},
}, NewHooks([]hook.Hook{
{
ID: xid.New(),
Type: hook.TypeURLWatcher,
BlockID: null.StringFrom("b1"),
Settings: processor.Settings(`{"url":"https://example.com"}`),
State: processor.State(`{"watcher":"w1"}`),
},
{
ID: xid.New(),
Type: hook.TypeScheduledReminder,
Settings: processor.Settings(`{"cron":"0 9 * * 1"}`),
},
}))
}
func Test_Hooks_Value(t *testing.T) {
t.Parallel()
cc := map[string]struct {
Hooks Hooks
JSON string
}{
"Nil hooks": {
JSON: `[]`,
},
"Hooks": {
Hooks: Hooks{
{
Type: hook.TypeURLWatcher,
BlockID: null.StringFrom("b1"),
Settings: processor.Settings(`{"url":"https://example.com"}`),
},
{
Type: hook.TypeScheduledReminder,
Settings: processor.Settings(`{"cron":"0 9 * * 1"}`),
},
},
JSON: `[{"type":"url-watcher","blockId":"b1","settings":{"url":"https://example.com"}},` +
`{"type":"scheduled-reminder","blockId":null,"settings":{"cron":"0 9 * * 1"}}]`,
},
}
for cn, c := range cc {
t.Run(cn, func(t *testing.T) {
t.Parallel()
v, err := c.Hooks.Value()
require.NoError(t, err)
data, ok := v.([]byte)
require.True(t, ok)
assert.JSONEq(t, c.JSON, string(data))
})
}
}
func Test_Hooks_Scan(t *testing.T) {
t.Parallel()
cc := map[string]struct {
Src any
Result Hooks
Err error
}{
"Invalid type": {
Src: 42,
Err: assert.AnError,
},
"Invalid JSON": {
Src: []byte(`{`),
Err: assert.AnError,
},
"Bytes": {
Src: []byte(`[{"type":"url-watcher","blockId":"b1","settings":{"url":"https://example.com"}}]`),
Result: Hooks{
{
Type: hook.TypeURLWatcher,
BlockID: null.StringFrom("b1"),
Settings: processor.Settings(`{"url":"https://example.com"}`),
},
},
},
"String": {
Src: `[{"type":"scheduled-reminder","blockId":null,"settings":{}}]`,
Result: Hooks{
{
Type: hook.TypeScheduledReminder,
Settings: processor.Settings(`{}`),
},
},
},
"Empty list": {
Src: []byte(`[]`),
Result: Hooks{},
},
}
for cn, c := range cc {
t.Run(cn, func(t *testing.T) {
t.Parallel()
var hh Hooks
err := hh.Scan(c.Src)
testutil.AssertEqualError(t, c.Err, err)
if err != nil {
return
}
assert.Equal(t, c.Result, hh)
})
}
}

46
server/core/internal/document/node.go
View File

@ -17,9 +17,14 @@ import (
// router mounts the file routes on this very format.
const FilePathFormat = "/api/documents/%s/files/%s"
// FileIDLength is the exact length of a file id: a 21-character nanoid.
// The id's alphabet includes the dash, so a "<id>-<file name>" segment is
// split by length, never at the first dash.
const FileIDLength = 21
// FilePath returns the URL path a document's file is served under: the
// file route with the id followed by a dash and the file name, so the
// address reads as the file it is. Every uid is a 21-character nanoid,
// address reads as the file it is. Every file id is a 21-character nanoid,
// which is what lets the server cut the id off the front again.
func FilePath(documentID xid.ID, id, name string) string {
return fmt.Sprintf(FilePathFormat, documentID, id+"-"+url.PathEscape(name))
@ -351,21 +356,11 @@ func (d *duplication) regenerateUID(old any) string {
return uid
}
// rewriteFileRef points a duplicated image or file block at the
// duplicate's own copy of the file and records the pair. Files served
// from anywhere but the source document's file route are left as they
// are: an externally hosted image has no object to copy.
// rewriteFileRef points a duplicated block's src at the duplicate's own
// copy of the file and records the pair. The file id is the fixed-length
// segment after the source document's file route; a src served from
// anywhere else has no object to copy and is left as it is.
func (d *duplication) rewriteFileRef(attrs, newAttrs Attributes) {
oldID, ok := attrs[AttrUID].(string)
if !ok {
return
}
newID, ok := newAttrs[AttrUID].(string)
if !ok {
return
}
src, ok := attrs[AttrSrc].(string)
if !ok {
return
@ -376,24 +371,27 @@ func (d *duplication) rewriteFileRef(attrs, newAttrs Attributes) {
return
}
// the id is followed by the dash and the file name.
oldPath := fmt.Sprintf(FilePathFormat, d.oldDocumentID, oldID)
idx := strings.Index(u.Path, oldPath)
if idx < 0 {
prefix, rest, ok := strings.Cut(u.Path, fmt.Sprintf(FilePathFormat, d.oldDocumentID, ""))
if !ok {
return
}
rest := u.Path[idx+len(oldPath):]
if !strings.HasPrefix(rest, "-") {
if len(rest) <= FileIDLength || rest[FileIDLength] != '-' {
return
}
oldID, name := rest[:FileIDLength], rest[FileIDLength+1:]
newID, ok := d.files[oldID]
if !ok {
newID = strutil.NanoID()
d.files[oldID] = newID
}
// only the path is swapped: the src was built by the frontend from its
// own api base url, which need not match this server's public url.
u.Path = u.Path[:idx] + fmt.Sprintf(FilePathFormat, d.newDocumentID, newID) + rest
u.Path = prefix + fmt.Sprintf(FilePathFormat, d.newDocumentID, newID) + "-" + name
u.RawPath = ""
newAttrs[AttrSrc] = u.String()
d.files[oldID] = newID
}

31
server/core/internal/document/node_test.go
View File

@ -342,8 +342,8 @@ func Test_RootBlock_Duplicate(t *testing.T) {
{
Type: BlockNodeImageBlock,
Attrs: Attributes{
"uid": "img1",
"src": "https://app.test/core" + FilePath(oldDocumentID, "img1", "shot.png"),
"uid": "img-1-aaaaaaaaaaaaaaa",
"src": "https://app.test/core" + FilePath(oldDocumentID, "img-1-aaaaaaaaaaaaaaa", "shot.png"),
},
},
{
@ -356,8 +356,8 @@ func Test_RootBlock_Duplicate(t *testing.T) {
{
Type: BlockNodeImageBlock,
Attrs: Attributes{
"uid": "img3",
"src": "https://app.test/core" + FilePath(xid.New(), "img3", "shot.png"),
"uid": "img-3-aaaaaaaaaaaaaaa",
"src": "https://app.test/core" + FilePath(xid.New(), "img-3-aaaaaaaaaaaaaaa", "shot.png"),
},
},
{
@ -367,8 +367,8 @@ func Test_RootBlock_Duplicate(t *testing.T) {
{
Type: BlockNodeFileBlock,
Attrs: Attributes{
"uid": "file1",
"src": "https://app.test/core" + FilePath(oldDocumentID, "file1", "my notes.zip"),
"uid": "file-1-aaaaaaaaaaaaaa",
"src": "https://app.test/core" + FilePath(oldDocumentID, "file-1-aaaaaaaaaaaaaa", "my notes.zip"),
"name": "notes.zip",
"size": 2048,
"contentType": "application/zip",
@ -381,12 +381,16 @@ func Test_RootBlock_Duplicate(t *testing.T) {
// document are remapped.
require.Len(t, files, 2)
newID, ok := files["img1"]
newID, ok := files["img-1-aaaaaaaaaaaaaaa"]
require.True(t, ok)
assert.Len(t, newID, 21)
// the file gets an id of its own; the block's uid is
// regenerated separately.
uid, ok := dup.Content[0].UID()
require.True(t, ok)
assert.Equal(t, newID, uid)
assert.NotEqual(t, newID, uid)
assert.NotEqual(t, "img-1-aaaaaaaaaaaaaaa", uid)
assert.Equal(
t,
"https://app.test/core"+FilePath(newDocumentID, newID, "shot.png"),
@ -403,12 +407,9 @@ func Test_RootBlock_Duplicate(t *testing.T) {
// the file block is remapped the way the image is, keeps
// the file name segment of its address, and keeps what
// it shows.
newFileID, ok := files["file1"]
newFileID, ok := files["file-1-aaaaaaaaaaaaaa"]
require.True(t, ok)
fileUID, ok := dup.Content[4].UID()
require.True(t, ok)
assert.Equal(t, newFileID, fileUID)
assert.Len(t, newFileID, 21)
assert.Equal(
t,
"https://app.test/core"+FilePath(newDocumentID, newFileID, "my notes.zip"),
@ -419,8 +420,8 @@ func Test_RootBlock_Duplicate(t *testing.T) {
assert.Equal(t, "application/zip", dup.Content[4].Attrs["contentType"])
// the original is untouched.
assert.Equal(t, "img1", orig.Content[0].Attrs["uid"])
assert.Equal(t, "file1", orig.Content[4].Attrs["uid"])
assert.Equal(t, "img-1-aaaaaaaaaaaaaaa", orig.Content[0].Attrs["uid"])
assert.Equal(t, "file-1-aaaaaaaaaaaaaa", orig.Content[4].Attrs["uid"])
},
}
}(),

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

@ -13,6 +13,7 @@ import (
"github.com/oxynote/oxynote/server/core/internal/document"
"github.com/oxynote/oxynote/server/core/internal/document/comment"
"github.com/oxynote/oxynote/server/core/internal/document/file"
"github.com/oxynote/oxynote/server/core/internal/document/history"
"github.com/oxynote/oxynote/server/core/internal/document/hook"
notificationCore "github.com/oxynote/oxynote/server/core/internal/notification"
"github.com/oxynote/oxynote/server/core/internal/search"
@ -208,6 +209,9 @@ var _ DB = &DBMock{}
// InsertDocumentBranchFunc: func(ctx context.Context, doc document.Document) error {
// panic("mock out the InsertDocumentBranch method")
// },
// InsertDocumentBranchHistoryEntryFunc: func(ctx context.Context, entry history.Entry) error {
// panic("mock out the InsertDocumentBranchHistoryEntry method")
// },
// InsertDocumentCommentFunc: func(ctx context.Context, c comment.Comment) error {
// panic("mock out the InsertDocumentComment method")
// },
@ -494,6 +498,9 @@ type DBMock struct {
// InsertDocumentBranchFunc mocks the InsertDocumentBranch method.
InsertDocumentBranchFunc func(ctx context.Context, doc document.Document) error
// InsertDocumentBranchHistoryEntryFunc mocks the InsertDocumentBranchHistoryEntry method.
InsertDocumentBranchHistoryEntryFunc func(ctx context.Context, entry history.Entry) error
// InsertDocumentCommentFunc mocks the InsertDocumentComment method.
InsertDocumentCommentFunc func(ctx context.Context, c comment.Comment) error
@ -1115,6 +1122,13 @@ type DBMock struct {
// Doc is the doc argument value.
Doc document.Document
}
// InsertDocumentBranchHistoryEntry holds details about calls to the InsertDocumentBranchHistoryEntry method.
InsertDocumentBranchHistoryEntry []struct {
// Ctx is the ctx argument value.
Ctx context.Context
// Entry is the entry argument value.
Entry history.Entry
}
// InsertDocumentComment holds details about calls to the InsertDocumentComment method.
InsertDocumentComment []struct {
// Ctx is the ctx argument value.
@ -1457,6 +1471,7 @@ type DBMock struct {
lockInsertDataSource sync.RWMutex
lockInsertDocument sync.RWMutex
lockInsertDocumentBranch sync.RWMutex
lockInsertDocumentBranchHistoryEntry sync.RWMutex
lockInsertDocumentComment sync.RWMutex
lockInsertDocumentCommentReply sync.RWMutex
lockInsertDocumentFile sync.RWMutex
@ -4039,6 +4054,45 @@ func (mock *DBMock) InsertDocumentBranchCalls() []struct {
return calls
}
// InsertDocumentBranchHistoryEntry calls InsertDocumentBranchHistoryEntryFunc.
func (mock *DBMock) InsertDocumentBranchHistoryEntry(ctx context.Context, entry history.Entry) error {
callInfo := struct {
Ctx context.Context
Entry history.Entry
}{
Ctx: ctx,
Entry: entry,
}
mock.lockInsertDocumentBranchHistoryEntry.Lock()
mock.calls.InsertDocumentBranchHistoryEntry = append(mock.calls.InsertDocumentBranchHistoryEntry, callInfo)
mock.lockInsertDocumentBranchHistoryEntry.Unlock()
if mock.InsertDocumentBranchHistoryEntryFunc == nil {
var (
errOut error
)
return errOut
}
return mock.InsertDocumentBranchHistoryEntryFunc(ctx, entry)
}
// InsertDocumentBranchHistoryEntryCalls gets all the calls that were made to InsertDocumentBranchHistoryEntry.
// Check the length with:
//
// len(mockedDB.InsertDocumentBranchHistoryEntryCalls())
func (mock *DBMock) InsertDocumentBranchHistoryEntryCalls() []struct {
Ctx context.Context
Entry history.Entry
} {
var calls []struct {
Ctx context.Context
Entry history.Entry
}
mock.lockInsertDocumentBranchHistoryEntry.RLock()
calls = mock.calls.InsertDocumentBranchHistoryEntry
mock.lockInsertDocumentBranchHistoryEntry.RUnlock()
return calls
}
// InsertDocumentComment calls InsertDocumentCommentFunc.
func (mock *DBMock) InsertDocumentComment(ctx context.Context, c comment.Comment) error {
callInfo := struct {

54
server/core/internal/server/internal/document/0mock_db_test.go
View File

@ -10,6 +10,7 @@ import (
"github.com/guregu/null/v5"
documentCore "github.com/oxynote/oxynote/server/core/internal/document"
"github.com/oxynote/oxynote/server/core/internal/document/file"
"github.com/oxynote/oxynote/server/core/internal/document/history"
"github.com/oxynote/oxynote/server/core/internal/document/hook"
"github.com/oxynote/oxynote/server/core/internal/search"
"github.com/rs/xid"
@ -106,6 +107,9 @@ var _ DB = &DBMock{}
// InsertDocumentBranchFunc: func(ctx context.Context, doc documentCore.Document) error {
// panic("mock out the InsertDocumentBranch method")
// },
// InsertDocumentBranchHistoryEntryFunc: func(ctx context.Context, entry history.Entry) error {
// panic("mock out the InsertDocumentBranchHistoryEntry method")
// },
// InsertDocumentFileFunc: func(ctx context.Context, f file.File) error {
// panic("mock out the InsertDocumentFile method")
// },
@ -227,6 +231,9 @@ type DBMock struct {
// InsertDocumentBranchFunc mocks the InsertDocumentBranch method.
InsertDocumentBranchFunc func(ctx context.Context, doc documentCore.Document) error
// InsertDocumentBranchHistoryEntryFunc mocks the InsertDocumentBranchHistoryEntry method.
InsertDocumentBranchHistoryEntryFunc func(ctx context.Context, entry history.Entry) error
// InsertDocumentFileFunc mocks the InsertDocumentFile method.
InsertDocumentFileFunc func(ctx context.Context, f file.File) error
@ -501,6 +508,13 @@ type DBMock struct {
// Doc is the doc argument value.
Doc documentCore.Document
}
// InsertDocumentBranchHistoryEntry holds details about calls to the InsertDocumentBranchHistoryEntry method.
InsertDocumentBranchHistoryEntry []struct {
// Ctx is the ctx argument value.
Ctx context.Context
// Entry is the entry argument value.
Entry history.Entry
}
// InsertDocumentFile holds details about calls to the InsertDocumentFile method.
InsertDocumentFile []struct {
// Ctx is the ctx argument value.
@ -624,6 +638,7 @@ type DBMock struct {
lockInsertBranchReviewer sync.RWMutex
lockInsertDocument sync.RWMutex
lockInsertDocumentBranch sync.RWMutex
lockInsertDocumentBranchHistoryEntry sync.RWMutex
lockInsertDocumentFile sync.RWMutex
lockInsertDocumentHook sync.RWMutex
lockInsertSearchJob sync.RWMutex
@ -1807,6 +1822,45 @@ func (mock *DBMock) InsertDocumentBranchCalls() []struct {
return calls
}
// InsertDocumentBranchHistoryEntry calls InsertDocumentBranchHistoryEntryFunc.
func (mock *DBMock) InsertDocumentBranchHistoryEntry(ctx context.Context, entry history.Entry) error {
callInfo := struct {
Ctx context.Context
Entry history.Entry
}{
Ctx: ctx,
Entry: entry,
}
mock.lockInsertDocumentBranchHistoryEntry.Lock()
mock.calls.InsertDocumentBranchHistoryEntry = append(mock.calls.InsertDocumentBranchHistoryEntry, callInfo)
mock.lockInsertDocumentBranchHistoryEntry.Unlock()
if mock.InsertDocumentBranchHistoryEntryFunc == nil {
var (
errOut error
)
return errOut
}
return mock.InsertDocumentBranchHistoryEntryFunc(ctx, entry)
}
// InsertDocumentBranchHistoryEntryCalls gets all the calls that were made to InsertDocumentBranchHistoryEntry.
// Check the length with:
//
// len(mockedDB.InsertDocumentBranchHistoryEntryCalls())
func (mock *DBMock) InsertDocumentBranchHistoryEntryCalls() []struct {
Ctx context.Context
Entry history.Entry
} {
var calls []struct {
Ctx context.Context
Entry history.Entry
}
mock.lockInsertDocumentBranchHistoryEntry.RLock()
calls = mock.calls.InsertDocumentBranchHistoryEntry
mock.lockInsertDocumentBranchHistoryEntry.RUnlock()
return calls
}
// InsertDocumentFile calls InsertDocumentFileFunc.
func (mock *DBMock) InsertDocumentFile(ctx context.Context, f file.File) error {
callInfo := struct {

54
server/core/internal/server/internal/document/0mock_tx_test.go
View File

@ -10,6 +10,7 @@ import (
"github.com/guregu/null/v5"
documentCore "github.com/oxynote/oxynote/server/core/internal/document"
"github.com/oxynote/oxynote/server/core/internal/document/file"
"github.com/oxynote/oxynote/server/core/internal/document/history"
"github.com/oxynote/oxynote/server/core/internal/document/hook"
"github.com/oxynote/oxynote/server/core/internal/search"
"github.com/rs/xid"
@ -106,6 +107,9 @@ var _ Tx = &TxMock{}
// InsertDocumentBranchFunc: func(ctx context.Context, doc documentCore.Document) error {
// panic("mock out the InsertDocumentBranch method")
// },
// InsertDocumentBranchHistoryEntryFunc: func(ctx context.Context, entry history.Entry) error {
// panic("mock out the InsertDocumentBranchHistoryEntry method")
// },
// InsertDocumentFileFunc: func(ctx context.Context, f file.File) error {
// panic("mock out the InsertDocumentFile method")
// },
@ -230,6 +234,9 @@ type TxMock struct {
// InsertDocumentBranchFunc mocks the InsertDocumentBranch method.
InsertDocumentBranchFunc func(ctx context.Context, doc documentCore.Document) error
// InsertDocumentBranchHistoryEntryFunc mocks the InsertDocumentBranchHistoryEntry method.
InsertDocumentBranchHistoryEntryFunc func(ctx context.Context, entry history.Entry) error
// InsertDocumentFileFunc mocks the InsertDocumentFile method.
InsertDocumentFileFunc func(ctx context.Context, f file.File) error
@ -503,6 +510,13 @@ type TxMock struct {
// Doc is the doc argument value.
Doc documentCore.Document
}
// InsertDocumentBranchHistoryEntry holds details about calls to the InsertDocumentBranchHistoryEntry method.
InsertDocumentBranchHistoryEntry []struct {
// Ctx is the ctx argument value.
Ctx context.Context
// Entry is the entry argument value.
Entry history.Entry
}
// InsertDocumentFile holds details about calls to the InsertDocumentFile method.
InsertDocumentFile []struct {
// Ctx is the ctx argument value.
@ -629,6 +643,7 @@ type TxMock struct {
lockInsertBranchReviewer sync.RWMutex
lockInsertDocument sync.RWMutex
lockInsertDocumentBranch sync.RWMutex
lockInsertDocumentBranchHistoryEntry sync.RWMutex
lockInsertDocumentFile sync.RWMutex
lockInsertDocumentHook sync.RWMutex
lockInsertSearchJob sync.RWMutex
@ -1804,6 +1819,45 @@ func (mock *TxMock) InsertDocumentBranchCalls() []struct {
return calls
}
// InsertDocumentBranchHistoryEntry calls InsertDocumentBranchHistoryEntryFunc.
func (mock *TxMock) InsertDocumentBranchHistoryEntry(ctx context.Context, entry history.Entry) error {
callInfo := struct {
Ctx context.Context
Entry history.Entry
}{
Ctx: ctx,
Entry: entry,
}
mock.lockInsertDocumentBranchHistoryEntry.Lock()
mock.calls.InsertDocumentBranchHistoryEntry = append(mock.calls.InsertDocumentBranchHistoryEntry, callInfo)
mock.lockInsertDocumentBranchHistoryEntry.Unlock()
if mock.InsertDocumentBranchHistoryEntryFunc == nil {
var (
errOut error
)
return errOut
}
return mock.InsertDocumentBranchHistoryEntryFunc(ctx, entry)
}
// InsertDocumentBranchHistoryEntryCalls gets all the calls that were made to InsertDocumentBranchHistoryEntry.
// Check the length with:
//
// len(mockedTx.InsertDocumentBranchHistoryEntryCalls())
func (mock *TxMock) InsertDocumentBranchHistoryEntryCalls() []struct {
Ctx context.Context
Entry history.Entry
} {
var calls []struct {
Ctx context.Context
Entry history.Entry
}
mock.lockInsertDocumentBranchHistoryEntry.RLock()
calls = mock.calls.InsertDocumentBranchHistoryEntry
mock.lockInsertDocumentBranchHistoryEntry.RUnlock()
return calls
}
// InsertDocumentFile calls InsertDocumentFileFunc.
func (mock *TxMock) InsertDocumentFile(ctx context.Context, f file.File) error {
callInfo := struct {

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

@ -10,12 +10,14 @@ import (
"github.com/guregu/null/v5"
"github.com/oxynote/oxynote/server/core/internal/apps/webchange"
documentCore "github.com/oxynote/oxynote/server/core/internal/document"
"github.com/oxynote/oxynote/server/core/internal/document/history"
"github.com/oxynote/oxynote/server/core/internal/document/hook"
"github.com/oxynote/oxynote/server/core/internal/search"
"github.com/oxynote/oxynote/server/core/internal/server/internal/auth"
"github.com/oxynote/oxynote/server/core/pkg/errutil"
"github.com/oxynote/oxynote/server/core/pkg/httpserver"
"github.com/oxynote/oxynote/server/core/pkg/logutil"
"github.com/oxynote/oxynote/server/core/pkg/timeutil"
"github.com/rs/xid"
)
@ -200,6 +202,31 @@ func (h *Handler) UpdateDocumentBranchByIDUnsafe(w http.ResponseWriter, r *http.
return
}
// a persist that changes nothing, such as the Yjs seed Hocuspocus
// sends on first load, leaves history alone. A system write is not
// attributed: no user made that change.
if !doc.SnapshotEqual(ndoc) {
var hooks []hook.Hook
hooks, err = tx.FetchDocumentHooksByBranchID(r.Context(), doc.BranchID, doc.OrganizationID)
if err != nil {
httpserver.RespondError(h.log, w, err)
return
}
var lastUpdatedBy null.String
if !ui.System {
lastUpdatedBy = ndoc.LastUpdatedBy
}
entry := history.NewEntry(ndoc, timeutil.Now(), lastUpdatedBy, history.NewHooks(hooks), false)
if err = tx.InsertDocumentBranchHistoryEntry(r.Context(), entry); err != nil {
httpserver.RespondError(h.log, w, err)
return
}
}
maintainers, err := tx.FetchDocumentMaintainers(r.Context(), doc.ID, doc.OrganizationID)
if err != nil {
httpserver.RespondError(h.log, w, err)
@ -336,6 +363,11 @@ func (h *Handler) MergeBranches(w http.ResponseWriter, r *http.Request) {
return
}
if err := h.insertBoundaryEntry(r.Context(), tx, ndoc, fromDoc.BranchID, session); err != nil {
httpserver.RespondError(h.log, w, err)
return
}
if err := tx.ReplaceBranchTags(r.Context(), session.ActiveOrganizationID, fromDoc.BranchID, toDoc.BranchID); err != nil {
httpserver.RespondError(h.log, w, err)
return
@ -466,6 +498,11 @@ func (h *Handler) CreateDocumentBranch(w http.ResponseWriter, r *http.Request) {
return
}
if err := h.insertBoundaryEntry(r.Context(), tx, newDoc, sourceDoc.BranchID, session); err != nil {
httpserver.RespondError(h.log, w, err)
return
}
if err := tx.CopyBranchTags(r.Context(), session.ActiveOrganizationID, sourceDoc.BranchID, newDoc.BranchID); err != nil {
httpserver.RespondError(h.log, w, err)
return
@ -669,3 +706,18 @@ func (h *Handler) copyHooksToBranch(
}
}
}
// insertBoundaryEntry records a boundary history entry of the document's
// branch, attributed to the session user. The entry lists the hooks of the
// branch the content came from, which are what the branch carries once
// copyHooksToBranch has run after the commit.
func (h *Handler) insertBoundaryEntry(ctx context.Context, tx Tx, doc documentCore.Document, sourceBranchID xid.ID, session auth.Session) error {
hooks, err := tx.FetchDocumentHooksByBranchID(ctx, sourceBranchID, session.ActiveOrganizationID)
if err != nil {
return err
}
entry := history.NewEntry(doc, timeutil.Now(), null.StringFrom(session.UserID), history.NewHooks(hooks), true)
return tx.InsertDocumentBranchHistoryEntry(ctx, entry)
}

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

@ -9,9 +9,12 @@ import (
"github.com/guregu/null/v5"
documentCore "github.com/oxynote/oxynote/server/core/internal/document"
"github.com/oxynote/oxynote/server/core/internal/document/history"
hookCore "github.com/oxynote/oxynote/server/core/internal/document/hook"
"github.com/oxynote/oxynote/server/core/internal/document/hook/processor"
"github.com/oxynote/oxynote/server/core/internal/search"
"github.com/oxynote/oxynote/server/core/internal/server/internal/auth"
"github.com/oxynote/oxynote/server/core/pkg/testutil"
"github.com/rs/xid"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
@ -146,7 +149,78 @@ func Test_Handler_UpdateDocumentBranchByIDUnsafe(t *testing.T) {
Maintainers int
SearchJobs int
BranchID xid.ID
// History is how many history entries the persist records.
History int
// HistoryCheck inspects the recorded entry.
HistoryCheck func(t *testing.T, entry history.Entry)
}{
"Unchanged persist writes no history entry": {
DB: &DBMock{FetchDocumentUnsafeByBranchIDFunc: fetchStored},
Tx: &TxMock{},
Body: `{"name":"Doc","maintainers":["u1","u9"]}`,
RespCode: http.StatusOK,
Committed: 1,
Metadata: 1,
Maintainers: 1,
SearchJobs: 1,
},
"History hook fetch error": {
DB: &DBMock{FetchDocumentUnsafeByBranchIDFunc: fetchStored},
Tx: &TxMock{
FetchDocumentHooksByBranchIDFunc: func(context.Context, xid.ID, string) ([]hookCore.Hook, error) {
return nil, errors.New("boom")
},
},
Body: validBody,
RespCode: http.StatusInternalServerError,
},
"History entry insert error": {
DB: &DBMock{FetchDocumentUnsafeByBranchIDFunc: fetchStored},
Tx: &TxMock{
InsertDocumentBranchHistoryEntryFunc: func(context.Context, history.Entry) error {
return errors.New("boom")
},
},
Body: validBody,
RespCode: http.StatusInternalServerError,
History: 1,
},
"System write records an unattributed entry": {
DB: &DBMock{
FetchDocumentUnsafeByBranchIDFunc: func(context.Context, xid.ID) (*documentCore.Document, error) {
doc := storedDoc()
doc.LastUpdatedBy = null.StringFrom("u5")
return doc, nil
},
},
Tx: &TxMock{
FetchDocumentHooksByBranchIDFunc: func(context.Context, xid.ID, string) ([]hookCore.Hook, error) {
return []hookCore.Hook{storedHook(hookCore.TypeScheduledReminder)}, nil
},
FetchDocumentMaintainersFunc: func(context.Context, xid.ID, string) ([]string, error) {
return []string{"u1", "u9"}, nil
},
},
Body: `{"name":"Renamed","system":true}`,
RespCode: http.StatusOK,
Committed: 1,
Metadata: 1,
SearchJobs: 1,
History: 1,
HistoryCheck: func(t *testing.T, entry history.Entry) {
assert.Equal(t, _branchID, entry.BranchID)
assert.Equal(t, "Renamed", entry.DocumentName)
assert.False(t, entry.LastUpdatedBy.Valid)
assert.False(t, entry.Boundary)
assert.Equal(t, history.Hooks{
{
Type: hookCore.TypeScheduledReminder,
Settings: processor.Settings(`{"scale":"linear"}`),
},
}, entry.Hooks)
},
},
"Missing branch ID parameter": {
DB: &DBMock{},
Tx: &TxMock{},
@ -209,6 +283,7 @@ func Test_Handler_UpdateDocumentBranchByIDUnsafe(t *testing.T) {
},
Body: validBody,
RespCode: http.StatusInternalServerError,
History: 1,
},
"Maintainer upsert error": {
DB: &DBMock{FetchDocumentUnsafeByBranchIDFunc: fetchStored},
@ -219,6 +294,7 @@ func Test_Handler_UpdateDocumentBranchByIDUnsafe(t *testing.T) {
},
Body: validBody,
RespCode: http.StatusInternalServerError,
History: 1,
},
"Search job insertion error on default branch": {
DB: &DBMock{FetchDocumentUnsafeByBranchIDFunc: fetchStored},
@ -233,6 +309,7 @@ func Test_Handler_UpdateDocumentBranchByIDUnsafe(t *testing.T) {
Body: validBody,
RespCode: http.StatusInternalServerError,
SearchJobs: 1,
History: 1,
},
"Commit error": {
DB: &DBMock{FetchDocumentUnsafeByBranchIDFunc: fetchStored},
@ -248,6 +325,7 @@ func Test_Handler_UpdateDocumentBranchByIDUnsafe(t *testing.T) {
RespCode: http.StatusInternalServerError,
Committed: 1,
SearchJobs: 1,
History: 1,
},
"Successful update with new maintainers": {
DB: &DBMock{FetchDocumentUnsafeByBranchIDFunc: fetchStored},
@ -258,6 +336,7 @@ func Test_Handler_UpdateDocumentBranchByIDUnsafe(t *testing.T) {
Metadata: 1,
Maintainers: 1,
SearchJobs: 1,
History: 1,
},
"Successful update without new maintainers": {
DB: &DBMock{FetchDocumentUnsafeByBranchIDFunc: fetchStored},
@ -271,6 +350,7 @@ func Test_Handler_UpdateDocumentBranchByIDUnsafe(t *testing.T) {
Committed: 1,
Metadata: 1,
SearchJobs: 1,
History: 1,
},
"Non-default branch queues its own search job": {
DB: &DBMock{
@ -289,6 +369,7 @@ func Test_Handler_UpdateDocumentBranchByIDUnsafe(t *testing.T) {
Metadata: 1,
SearchJobs: 1,
BranchID: _branchID2,
History: 1,
},
}
@ -307,6 +388,17 @@ func Test_Handler_UpdateDocumentBranchByIDUnsafe(t *testing.T) {
assert.Equal(t, c.Metadata, cnt.metadata)
assert.Equal(t, c.Maintainers, cnt.maintainers)
assert.Len(t, c.Tx.InsertSearchJobCalls(), c.SearchJobs)
assert.Len(t, c.Tx.InsertDocumentBranchHistoryEntryCalls(), c.History)
if c.HistoryCheck != nil {
c.HistoryCheck(t, c.Tx.InsertDocumentBranchHistoryEntryCalls()[0].Entry)
}
if cn == "Successful update with new maintainers" {
entry := c.Tx.InsertDocumentBranchHistoryEntryCalls()[0].Entry
assert.Equal(t, null.StringFrom("u9"), entry.LastUpdatedBy)
assert.Equal(t, history.Hooks{}, entry.Hooks)
}
// the job is scoped to the persisted branch, whichever branch
// it is.
@ -356,7 +448,30 @@ func Test_Handler_MergeBranches(t *testing.T) {
Committed int
Metadata int
Reviewers int
// History is how many history entries the merge records.
History int
}{
"History hook fetch error": {
DB: &DBMock{FetchDocumentByBranchIDFunc: fetchByBranch},
Tx: &TxMock{
FetchDocumentHooksByBranchIDFunc: func(context.Context, xid.ID, string) ([]hookCore.Hook, error) {
return nil, errors.New("boom")
},
},
Body: validBody,
RespCode: http.StatusInternalServerError,
},
"History entry insert error": {
DB: &DBMock{FetchDocumentByBranchIDFunc: fetchByBranch},
Tx: &TxMock{
InsertDocumentBranchHistoryEntryFunc: func(context.Context, history.Entry) error {
return errors.New("boom")
},
},
Body: validBody,
RespCode: http.StatusInternalServerError,
History: 1,
},
"No session in context": {
DB: &DBMock{},
Tx: &TxMock{},
@ -461,6 +576,7 @@ func Test_Handler_MergeBranches(t *testing.T) {
Committed: 1,
Metadata: 1,
Reviewers: 1,
History: 1,
},
"Hook re-creation error": {
DB: &DBMock{
@ -475,6 +591,7 @@ func Test_Handler_MergeBranches(t *testing.T) {
Committed: 1,
Metadata: 1,
Reviewers: 1,
History: 1,
},
"Hook insertion error": {
DB: &DBMock{
@ -492,6 +609,7 @@ func Test_Handler_MergeBranches(t *testing.T) {
Committed: 1,
Metadata: 1,
Reviewers: 1,
History: 1,
},
"Document update error": {
DB: &DBMock{FetchDocumentByBranchIDFunc: fetchByBranch},
@ -512,6 +630,7 @@ func Test_Handler_MergeBranches(t *testing.T) {
},
Body: validBody,
RespCode: http.StatusInternalServerError,
History: 1,
},
"Tag replacement error": {
DB: &DBMock{FetchDocumentByBranchIDFunc: fetchByBranch},
@ -522,6 +641,7 @@ func Test_Handler_MergeBranches(t *testing.T) {
},
Body: validBody,
RespCode: http.StatusInternalServerError,
History: 1,
},
"Commit error": {
DB: &DBMock{FetchDocumentByBranchIDFunc: fetchByBranch},
@ -533,6 +653,7 @@ func Test_Handler_MergeBranches(t *testing.T) {
Body: validBody,
RespCode: http.StatusInternalServerError,
Committed: 1,
History: 1,
},
"Successful merge": {
DB: &DBMock{
@ -547,6 +668,7 @@ func Test_Handler_MergeBranches(t *testing.T) {
Committed: 1,
Metadata: 1,
Reviewers: 1,
History: 1,
},
}
@ -569,8 +691,20 @@ func Test_Handler_MergeBranches(t *testing.T) {
assert.Len(t, c.Tx.CommitCalls(), c.Committed)
assert.Equal(t, c.Metadata, cnt.metadata)
assert.Equal(t, c.Reviewers, cnt.reviewers)
assert.Len(t, c.Tx.InsertDocumentBranchHistoryEntryCalls(), c.History)
if c.RespCode == http.StatusOK {
// the merge is a boundary entry of the target, listing the
// source's hooks and attributed to the merging user.
entry := c.Tx.InsertDocumentBranchHistoryEntryCalls()[0].Entry
assert.Equal(t, _branchID, entry.BranchID)
assert.True(t, entry.Boundary)
assert.Equal(t, null.StringFrom("u1"), entry.LastUpdatedBy)
hh := c.Tx.FetchDocumentHooksByBranchIDCalls()
require.Len(t, hh, 1)
assert.Equal(t, _branchID2, hh[0].BranchID)
// hooks are soft-deleted on the target inside the
// transaction and re-created from the source branch once it
// commits, since creating one creates its watcher.
@ -666,7 +800,38 @@ func Test_Handler_CreateDocumentBranch(t *testing.T) {
HookFetchErr error
RespCode int
Committed int
// History is how many history entries the fork records.
History int
}{
"History hook fetch error": {
DB: &DBMock{
FetchDocumentByBranchIDFunc: func(context.Context, xid.ID, string) (*documentCore.Document, error) {
return storedDoc(), nil
},
},
Tx: &TxMock{
FetchDocumentHooksByBranchIDFunc: func(context.Context, xid.ID, string) ([]hookCore.Hook, error) {
return nil, errors.New("boom")
},
},
Body: validBody,
RespCode: http.StatusInternalServerError,
},
"History entry insert error": {
DB: &DBMock{
FetchDocumentByBranchIDFunc: func(context.Context, xid.ID, string) (*documentCore.Document, error) {
return storedDoc(), nil
},
},
Tx: &TxMock{
InsertDocumentBranchHistoryEntryFunc: func(context.Context, history.Entry) error {
return errors.New("boom")
},
},
Body: validBody,
RespCode: http.StatusInternalServerError,
History: 1,
},
"No session in context": {
DB: &DBMock{},
Tx: &TxMock{},
@ -754,6 +919,7 @@ func Test_Handler_CreateDocumentBranch(t *testing.T) {
// leaves the branch itself standing.
RespCode: http.StatusCreated,
Committed: 1,
History: 1,
},
"Commit error": {
DB: &DBMock{
@ -769,6 +935,7 @@ func Test_Handler_CreateDocumentBranch(t *testing.T) {
Body: validBody,
RespCode: http.StatusInternalServerError,
Committed: 1,
History: 1,
},
"Search job insert error": {
DB: &DBMock{
@ -783,6 +950,7 @@ func Test_Handler_CreateDocumentBranch(t *testing.T) {
},
Body: validBody,
RespCode: http.StatusInternalServerError,
History: 1,
},
"Successful branch creation": {
DB: &DBMock{
@ -795,6 +963,7 @@ func Test_Handler_CreateDocumentBranch(t *testing.T) {
Body: validBody,
RespCode: http.StatusCreated,
Committed: 1,
History: 1,
},
"Tag copy error": {
DB: &DBMock{FetchDocumentByBranchIDFunc: func(context.Context, xid.ID, string) (*documentCore.Document, error) {
@ -807,6 +976,7 @@ func Test_Handler_CreateDocumentBranch(t *testing.T) {
},
Body: validBody,
RespCode: http.StatusInternalServerError,
History: 1,
},
}
@ -827,6 +997,7 @@ func Test_Handler_CreateDocumentBranch(t *testing.T) {
assert.Equal(t, c.RespCode, rec.Code)
assert.Len(t, c.Tx.CommitCalls(), c.Committed)
assert.Empty(t, c.Tx.InsertDocumentHookCalls())
assert.Len(t, c.Tx.InsertDocumentBranchHistoryEntryCalls(), c.History)
if c.RespCode == http.StatusCreated {
require.Len(t, c.Tx.InsertDocumentBranchCalls(), 1)
@ -836,6 +1007,15 @@ func Test_Handler_CreateDocumentBranch(t *testing.T) {
assert.False(t, forked.Protected)
assert.Nil(t, forked.RawContent)
// the fork starts with a boundary entry of its own content,
// listing the source's hooks.
entry := c.Tx.InsertDocumentBranchHistoryEntryCalls()[0].Entry
assert.Equal(t, forked.BranchID, entry.BranchID)
assert.True(t, entry.Boundary)
assert.Equal(t, null.StringFrom("u1"), entry.LastUpdatedBy)
require.Len(t, c.Tx.FetchDocumentHooksByBranchIDCalls(), 1)
assert.Equal(t, _branchID, c.Tx.FetchDocumentHooksByBranchIDCalls()[0].BranchID)
// the fork takes the source's tags before the commit.
require.Len(t, c.Tx.CopyBranchTagsCalls(), 1)
assert.Equal(t, _branchID, c.Tx.CopyBranchTagsCalls()[0].FromBranchID)
@ -1274,3 +1454,79 @@ func Test_Handler_UpdateDocumentBranch(t *testing.T) {
})
}
}
func Test_Handler_insertBoundaryEntry(t *testing.T) {
cc := map[string]struct {
Tx *TxMock
Err error
Hooks history.Hooks
}{
"Error returned by Tx.FetchDocumentHooksByBranchID": {
Tx: &TxMock{
FetchDocumentHooksByBranchIDFunc: func(context.Context, xid.ID, string) ([]hookCore.Hook, error) {
return nil, assert.AnError
},
},
Err: assert.AnError,
},
"Error returned by Tx.InsertDocumentBranchHistoryEntry": {
Tx: &TxMock{
InsertDocumentBranchHistoryEntryFunc: func(context.Context, history.Entry) error {
return assert.AnError
},
},
Err: assert.AnError,
},
"Successful insert": {
Tx: &TxMock{
FetchDocumentHooksByBranchIDFunc: func(context.Context, xid.ID, string) ([]hookCore.Hook, error) {
return []hookCore.Hook{storedHook(hookCore.TypeScheduledReminder)}, nil
},
},
Hooks: history.Hooks{
{
Type: hookCore.TypeScheduledReminder,
Settings: processor.Settings(`{"scale":"linear"}`),
},
},
},
}
for cn, c := range cc {
t.Run(cn, func(t *testing.T) {
t.Parallel()
hdl, _ := newTestHandler(&DBMock{}, &fakePublisher{})
doc := branchDoc(_branchID2)
err := hdl.insertBoundaryEntry(
context.Background(),
c.Tx,
*doc,
_branchID,
auth.Session{UserID: "u1", ActiveOrganizationID: "org1"},
)
testutil.AssertEqualError(t, c.Err, err)
ff := c.Tx.FetchDocumentHooksByBranchIDCalls()
require.Len(t, ff, 1)
assert.Equal(t, _branchID, ff[0].BranchID)
assert.Equal(t, "org1", ff[0].OrganizationID)
if err != nil && len(c.Tx.InsertDocumentBranchHistoryEntryCalls()) == 0 {
return
}
require.Len(t, c.Tx.InsertDocumentBranchHistoryEntryCalls(), 1)
entry := c.Tx.InsertDocumentBranchHistoryEntryCalls()[0].Entry
assert.Equal(t, _branchID2, entry.BranchID)
assert.Equal(t, doc.DocumentName, entry.DocumentName)
assert.True(t, entry.Boundary)
assert.Equal(t, null.StringFrom("u1"), entry.LastUpdatedBy)
if c.Hooks != nil {
assert.Equal(t, c.Hooks, entry.Hooks)
}
})
}
}

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

@ -14,6 +14,7 @@ import (
"github.com/oxynote/oxynote/server/core/internal/apps/webchange"
documentCore "github.com/oxynote/oxynote/server/core/internal/document"
"github.com/oxynote/oxynote/server/core/internal/document/file"
"github.com/oxynote/oxynote/server/core/internal/document/history"
"github.com/oxynote/oxynote/server/core/internal/document/hook"
"github.com/oxynote/oxynote/server/core/internal/notification"
"github.com/oxynote/oxynote/server/core/internal/search"
@ -1016,6 +1017,9 @@ type DocumentsDBAgent interface {
// UpdateDocument should update the document.
UpdateDocument(ctx context.Context, doc documentCore.Document) error
// InsertDocumentBranchHistoryEntry should record a snapshot of a branch.
InsertDocumentBranchHistoryEntry(ctx context.Context, entry history.Entry) error
// DeleteDocument should delete the document and report the ids of
// the document and of every cascade-deleted descendant.
DeleteDocument(ctx context.Context, id xid.ID, organizationID string) ([]xid.ID, error)

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

@ -1552,8 +1552,8 @@ func Test_Handler_DuplicateDocument(t *testing.T) {
{
Type: documentCore.BlockNodeImageBlock,
Attrs: documentCore.Attributes{
"uid": "img1",
"src": "https://app.test/core" + documentCore.FilePath(_documentID, "img1", "shot.png"),
"uid": "img-1-aaaaaaaaaaaaaaa",
"src": "https://app.test/core" + documentCore.FilePath(_documentID, "img-1-aaaaaaaaaaaaaaa", "shot.png"),
},
},
},
@ -1683,7 +1683,7 @@ func Test_Handler_DuplicateDocument(t *testing.T) {
"Hooks are copied along with the document": {
DB: func() *DBMock {
blockHook := storedHook(hookCore.TypeScheduledReminder)
blockHook.BlockID = null.StringFrom("img1")
blockHook.BlockID = null.StringFrom("img-1-aaaaaaaaaaaaaaa")
db := imageDB()
db.FetchDocumentHooksByBranchIDFunc = func(context.Context, xid.ID, string) ([]hookCore.Hook, error) {
@ -1761,7 +1761,7 @@ func Test_Handler_DuplicateDocument(t *testing.T) {
// uid rather than pointing at the source's.
for _, call := range c.DB.InsertDocumentHookCalls() {
assert.Equal(t, null.ValueFrom(dupl.BranchID), call.Hk.BranchID)
assert.NotEqual(t, null.StringFrom("img1"), call.Hk.BlockID)
assert.NotEqual(t, null.StringFrom("img-1-aaaaaaaaaaaaaaa"), call.Hk.BlockID)
assert.True(t, call.Hk.BlockID.Valid)
}
}

11
server/core/internal/server/internal/document/files/http.go
View File

@ -19,12 +19,7 @@ import (
"github.com/rs/xid"
)
// _fileIDLength is the length of a block uid: every editor mints
// 21-character nanoids, and the retrieval route relies on it to cut the
// id off the front of "<id>-<file name>".
const _fileIDLength = 21
// _fileIDPattern matches a block uid. The charset shuts out the path
// _fileIDPattern matches a file id. The charset shuts out the path
// separators and dot segments that would otherwise let an id escape its
// storage folder once joined into an object key.
var _fileIDPattern = regexp.MustCompile(`^[A-Za-z0-9_-]{21}$`)
@ -172,11 +167,11 @@ func (h *Handler) UploadDocumentFile(w http.ResponseWriter, r *http.Request) {
// fileID cuts the id off a "<id>-<file name>" route segment, reporting
// whether the segment has that shape.
func fileID(ref string) (string, bool) {
if len(ref) <= _fileIDLength || ref[_fileIDLength] != '-' {
if len(ref) <= document.FileIDLength || ref[document.FileIDLength] != '-' {
return "", false
}
id := ref[:_fileIDLength]
id := ref[:document.FileIDLength]
if !_fileIDPattern.MatchString(id) {
return "", false
}

16
server/core/internal/server/internal/document/files/http_test.go
View File

@ -16,6 +16,7 @@ import (
"github.com/oxynote/oxynote/server/core/internal/document/file"
"github.com/oxynote/oxynote/server/core/internal/server/internal/auth"
"github.com/oxynote/oxynote/server/core/internal/storage"
"github.com/oxynote/oxynote/server/core/pkg/errutil"
"github.com/oxynote/oxynote/server/core/pkg/testutil"
"github.com/rs/xid"
"github.com/stretchr/testify/assert"
@ -322,6 +323,21 @@ func Test_Handler_UploadDocumentFile(t *testing.T) {
wasUploadCalled(0),
),
},
"Existing file id": {
DB: &DBMock{
InsertDocumentFileFunc: func(context.Context, file.File) error {
return errutil.New(http.StatusConflict, "document_file.exists", "file id is already in use")
},
},
Storer: &StorerMock{},
Query: "?id=f1xxxxxxxxxxxxxxxxxxx&location=document&kind=image",
Checks: checks(
hasResp(http.StatusConflict, `{"code":"document_file.exists","message":"file id is already in use"}`),
wasInsertCalled(1),
wasUploadCalled(0),
wasDeleteCalled(0),
),
},
"DB insert error": {
DB: &DBMock{
InsertDocumentFileFunc: func(context.Context, file.File) error {

33
web/app/components/editor/blocks/file/FileBlock.nuxt.test.ts
View File

@ -457,8 +457,12 @@ describe("<FileBlock>", { concurrent: false }, () => {
await vi.waitFor(() => {
expect(calls).toHaveLength(1)
}, WAIT_FOR_OPTIONS)
// the upload carries an id of its own, never the block's uid, so a
// replacement leaves the previous object in place
const fileId = String(calls[0]?.query.id)
expect(fileId).toMatch(/^[A-Za-z0-9_-]{21}$/)
expect(calls[0]?.query).toEqual({
id: "file-1",
id: fileId,
location: "document",
kind: "file",
})
@ -470,9 +474,8 @@ describe("<FileBlock>", { concurrent: false }, () => {
})
expect(updateAttributes).toHaveBeenNthCalledWith(2, {
src: matchingString(
new RegExp(`/api/documents/${DOCUMENT_ID}/files/file-1-notes.zip$`),
new RegExp(`/api/documents/${DOCUMENT_ID}/files/${fileId}-notes.zip$`),
),
uid: "file-1",
name: "notes.zip",
size: 6,
contentType: "application/zip",
@ -481,30 +484,6 @@ describe("<FileBlock>", { concurrent: false }, () => {
expect(toast.custom).toHaveBeenCalledTimes(0)
})
it("uploads under a generated id when the node has none", async ({
expect,
}) => {
const calls = mockEndpoint(
"POST",
`/api/documents/${DOCUMENT_ID}/files`,
(_call, event) => {
setResponseHeader(event, "location", "https://cdn.test/stored.zip")
return uploadedZip()
},
)
const updateAttributes = vi.fn()
const wrapper = await mountFile({ uid: null }, updateAttributes)
await pickFile(wrapper, zipFile())
await vi.waitFor(() => {
expect(calls).toHaveLength(1)
}, WAIT_FOR_OPTIONS)
expect(calls[0]?.query.id).toEqual(expect.any(String))
expect(calls[0]?.query.id).not.toBe("")
})
it("does nothing when the file dialog is dismissed", async ({ expect }) => {
const calls = mockEndpoint(
"POST",

7
web/app/components/editor/blocks/file/FileBlock.vue
View File

@ -99,21 +99,20 @@ async function handleFileChange(event: Event) {
return
}
const blockId = (props.node.attrs.uid as string) || nanoid()
const fileId = nanoid()
props.updateAttributes({ uploading: true, name: file.name, size: file.size })
try {
const uploaded = await uploadDocumentFile.mutateAsync({
documentId: documentId.value,
id: blockId,
id: fileId,
loc: DocumentFileLocation.Document,
kind: DocumentFileKind.File,
file,
})
props.updateAttributes({
src: buildDocumentFileSrc(documentId.value, blockId, uploaded.name),
uid: blockId,
src: buildDocumentFileSrc(documentId.value, fileId, uploaded.name),
name: uploaded.name,
size: uploaded.size,
contentType: uploaded.contentType,

33
web/app/components/editor/blocks/image/ImageBlock.nuxt.test.ts
View File

@ -425,8 +425,12 @@ describe("<ImageBlock>", { concurrent: false }, () => {
await vi.waitFor(() => {
expect(calls).toHaveLength(1)
}, WAIT_FOR_OPTIONS)
// the upload carries an id of its own, never the block's uid, so a
// replacement leaves the previous object in place
const fileId = String(calls[0]?.query.id)
expect(fileId).toMatch(/^[A-Za-z0-9_-]{21}$/)
expect(calls[0]?.query).toEqual({
id: "image-1",
id: fileId,
location: "document",
kind: "image",
})
@ -434,38 +438,13 @@ describe("<ImageBlock>", { concurrent: false }, () => {
expect(updateAttributes).toHaveBeenNthCalledWith(1, { uploading: true })
expect(updateAttributes).toHaveBeenNthCalledWith(2, {
src: matchingString(
new RegExp(`/api/documents/${DOCUMENT_ID}/files/image-1-shot.png$`),
new RegExp(`/api/documents/${DOCUMENT_ID}/files/${fileId}-shot.png$`),
),
uid: "image-1",
uploading: false,
})
expect(toast.custom).toHaveBeenCalledTimes(0)
})
it("uploads under a generated id when the node has none", async ({
expect,
}) => {
const calls = mockEndpoint(
"POST",
`/api/documents/${DOCUMENT_ID}/files`,
(_call, event) => {
setResponseHeader(event, "location", "https://cdn.test/stored.png")
return uploadedImage()
},
)
const updateAttributes = vi.fn()
const wrapper = await mountImage({ uid: null }, updateAttributes)
await pickFile(wrapper, pngFile())
await vi.waitFor(() => {
expect(calls).toHaveLength(1)
}, WAIT_FOR_OPTIONS)
expect(calls[0]?.query.id).toEqual(expect.any(String))
expect(calls[0]?.query.id).not.toBe("")
})
it("does nothing when the file dialog is dismissed", async ({ expect }) => {
const calls = mockEndpoint(
"POST",

8
web/app/components/editor/blocks/image/ImageBlock.vue
View File

@ -176,24 +176,24 @@ async function handleFileChange(event: Event) {
return
}
const blockId = (props.node.attrs.uid as string) || nanoid()
const fileId = nanoid()
props.updateAttributes({ uploading: true })
try {
const uploaded = await uploadDocumentFile.mutateAsync({
documentId: documentId.value,
id: blockId,
id: fileId,
loc: DocumentFileLocation.Document,
kind: DocumentFileKind.Image,
file,
})
const nextSrc = buildDocumentFileSrc(
documentId.value,
blockId,
fileId,
uploaded.name,
)
props.updateAttributes({ src: nextSrc, uid: blockId, uploading: false })
props.updateAttributes({ src: nextSrc, uploading: false })
} catch (error) {
props.updateAttributes({ uploading: false })
if (!handleStorageError(error, t)) {

30
web/app/components/editor/blocks/upload-handler.test.ts
View File

@ -27,8 +27,8 @@ vi.mock("~/components/toast", () => ({
// buildDocumentFileSrc is pulled in through a nuxt auto-import, so the
// module itself is the only seam: the real one reaches for the nuxt app
vi.mock("~/composables/api/useDocumentFileAPI", () => ({
buildDocumentFileSrc: (documentId: string, blockId: string, name: string) =>
`/files/${documentId}/${blockId}-${name}`,
buildDocumentFileSrc: (documentId: string, fileId: string, name: string) =>
`/files/${documentId}/${fileId}-${name}`,
}))
const extensions = [
@ -135,6 +135,13 @@ function blockUid(doc: PMNode, typeName: string): string {
return blockAttrs(doc, typeName).uid as string
}
// the id every upload is sent under: a 21-character nanoid
const FILE_ID_PATTERN = /^[A-Za-z0-9_-]{21}$/
function uploadedFileId(): string {
return (uploadDocumentFile.mock.calls[0]?.[0] as { id: string }).id
}
// the upload chain is pure microtasks, so one macrotask boundary is
// enough to settle it — no timers involved
function flushUpload(): Promise<void> {
@ -177,11 +184,15 @@ describe("createUploadFileHandler", { concurrent: false }, () => {
expect(uploadDocumentFile).toHaveBeenCalledTimes(1)
expect(uploadDocumentFile).toHaveBeenCalledWith({
documentId: "d1",
id: blockUid(editor.state.doc, IMAGE_BLOCK_NAME),
id: expect.stringMatching(FILE_ID_PATTERN) as string,
loc: DocumentFileLocation.Document,
kind: DocumentFileKind.Image,
file,
})
// the file id is the upload's own, not the block's uid
expect(uploadedFileId()).not.toBe(
blockUid(editor.state.doc, IMAGE_BLOCK_NAME),
)
expect(showToastMessage).toHaveBeenCalledTimes(0)
})
@ -211,11 +222,14 @@ describe("createUploadFileHandler", { concurrent: false }, () => {
expect(uploadDocumentFile).toHaveBeenCalledTimes(1)
expect(uploadDocumentFile).toHaveBeenCalledWith({
documentId: "d1",
id: blockUid(editor.state.doc, FILE_BLOCK_NAME),
id: expect.stringMatching(FILE_ID_PATTERN) as string,
loc: DocumentFileLocation.Document,
kind: DocumentFileKind.File,
file,
})
expect(uploadedFileId()).not.toBe(
blockUid(editor.state.doc, FILE_BLOCK_NAME),
)
expect(showToastMessage).toHaveBeenCalledTimes(0)
})
@ -250,13 +264,13 @@ describe("createUploadFileHandler", { concurrent: false }, () => {
const editor = makeEditor([paragraph("one")])
editor.commands.setTextSelection(2)
fileCallbacks({ documentId: "d1" }).onPaste(editor, [pngFile("a.png")])
const uid = blockUid(editor.state.doc, IMAGE_BLOCK_NAME)
const fileId = uploadedFileId()
await flushUpload()
expect(shape(editor.state.doc)).toEqual([
"paragraph",
`imageBlock:/files/d1/${uid}-a.zip:false`,
`imageBlock:/files/d1/${fileId}-a.zip:false`,
])
expect(showToastMessage).toHaveBeenCalledTimes(0)
})
@ -274,13 +288,13 @@ describe("createUploadFileHandler", { concurrent: false }, () => {
fileCallbacks({ documentId: "d1" }).onPaste(editor, [
zipFile("notes.zip"),
])
const uid = blockUid(editor.state.doc, FILE_BLOCK_NAME)
const fileId = uploadedFileId()
await flushUpload()
expect(shape(editor.state.doc)).toEqual([
"paragraph",
`fileBlock:/files/d1/${uid}-renamed.zip:false`,
`fileBlock:/files/d1/${fileId}-renamed.zip:false`,
])
expect(blockAttrs(editor.state.doc, FILE_BLOCK_NAME)).toMatchObject({
name: "renamed.zip",

5
web/app/components/editor/blocks/upload-handler.ts
View File

@ -78,6 +78,7 @@ function insertBlockWithUpload(
const isImage = IMAGE_MIME_TYPES.has(file.type)
const nodeName = isImage ? IMAGE_BLOCK_NAME : FILE_BLOCK_NAME
const blockId = nanoid()
const fileId = nanoid()
const { state, view } = editor
const { schema } = state
@ -109,13 +110,13 @@ function insertBlockWithUpload(
uploadDocumentFile
.mutateAsync({
documentId,
id: blockId,
id: fileId,
loc: DocumentFileLocation.Document,
kind: isImage ? DocumentFileKind.Image : DocumentFileKind.File,
file,
})
.then((uploaded) => {
const src = buildDocumentFileSrc(documentId, blockId, uploaded.name)
const src = buildDocumentFileSrc(documentId, fileId, uploaded.name)
updateAttrsByUid(
editor,