diff --git a/README.md b/README.md
index 98b737a..ed8c969 100644
--- a/README.md
+++ b/README.md
@@ -31,4 +31,5 @@ make check-style
## Documentation
+
See the [Mattermost plugin development guide](https://developers.mattermost.com/integrate/plugins/) for plugin structure, server/webapp hooks, and the release process.
diff --git a/assets/i18n/en.json b/assets/i18n/en.json
index a18af74..7112364 100644
--- a/assets/i18n/en.json
+++ b/assets/i18n/en.json
@@ -43,10 +43,6 @@
"id": "app.page.create.invalid_user_id.app_error",
"translation": "Invalid user ID."
},
- {
- "id": "app.page.create.search_text_without_content.app_error",
- "translation": "Search text cannot be set without page content."
- },
{
"id": "app.page.create.space_not_found.app_error",
"translation": "The space could not be found."
@@ -103,6 +99,10 @@
"id": "app.page.get_children.invalid_id.app_error",
"translation": "Invalid page ID."
},
+ {
+ "id": "app.page.invalid_content.app_error",
+ "translation": "The page body is not valid content."
+ },
{
"id": "app.page.invalid_parent.app_error",
"translation": "The destination parent page does not exist."
@@ -151,6 +151,14 @@
"id": "app.page.not_found.app_error",
"translation": "The page could not be found."
},
+ {
+ "id": "app.page.presence.invalid_page_id.app_error",
+ "translation": "Invalid page ID."
+ },
+ {
+ "id": "app.page.presence.invalid_space_id.app_error",
+ "translation": "Invalid space ID."
+ },
{
"id": "app.page.restore.invalid_id.app_error",
"translation": "Invalid page ID."
@@ -199,6 +207,158 @@
"id": "app.page.update.store_error.app_error",
"translation": "An error occurred while updating the page."
},
+ {
+ "id": "app.page_draft.create.invalid_parent.app_error",
+ "translation": "The parent page or draft was not found."
+ },
+ {
+ "id": "app.page_draft.create.invalid_parent_id.app_error",
+ "translation": "The parent ID is not a valid ID."
+ },
+ {
+ "id": "app.page_draft.create.invalid_space_id.app_error",
+ "translation": "Invalid space ID."
+ },
+ {
+ "id": "app.page_draft.create.invalid_user_id.app_error",
+ "translation": "Invalid user ID."
+ },
+ {
+ "id": "app.page_draft.create.parent_cycle.app_error",
+ "translation": "Setting this parent would create a cycle in the draft hierarchy."
+ },
+ {
+ "id": "app.page_draft.create.parent_too_deep.app_error",
+ "translation": "The draft hierarchy is too deep to add another level."
+ },
+ {
+ "id": "app.page_draft.delete.invalid_page_id.app_error",
+ "translation": "Invalid page ID."
+ },
+ {
+ "id": "app.page_draft.delete.invalid_space_id.app_error",
+ "translation": "Invalid space ID."
+ },
+ {
+ "id": "app.page_draft.delete.invalid_user_id.app_error",
+ "translation": "Invalid user ID."
+ },
+ {
+ "id": "app.page_draft.delete.not_found.app_error",
+ "translation": "Draft not found."
+ },
+ {
+ "id": "app.page_draft.get.invalid_page_id.app_error",
+ "translation": "Invalid page ID."
+ },
+ {
+ "id": "app.page_draft.get.invalid_space_id.app_error",
+ "translation": "Invalid space ID."
+ },
+ {
+ "id": "app.page_draft.get.invalid_user_id.app_error",
+ "translation": "Invalid user ID."
+ },
+ {
+ "id": "app.page_draft.get.not_found.app_error",
+ "translation": "Draft not found."
+ },
+ {
+ "id": "app.page_draft.list.invalid_space_id.app_error",
+ "translation": "Invalid space ID."
+ },
+ {
+ "id": "app.page_draft.list.invalid_user_id.app_error",
+ "translation": "Invalid user ID."
+ },
+ {
+ "id": "app.page_draft.publish.baseline_required.app_error",
+ "translation": "Cannot publish: the draft is missing its edit baseline. Reopen the page and try again."
+ },
+ {
+ "id": "app.page_draft.publish.conflict.app_error",
+ "translation": "The page was modified by another writer. Refresh and try again."
+ },
+ {
+ "id": "app.page_draft.publish.draft_changed.app_error",
+ "translation": "This draft was saved again while it was being published. Nothing was published; try publishing again to include your latest changes."
+ },
+ {
+ "id": "app.page_draft.publish.draft_not_found.app_error",
+ "translation": "The draft could not be found; it may have already been published or discarded."
+ },
+ {
+ "id": "app.page_draft.publish.edit_conflict.app_error",
+ "translation": "Someone else edited this page while you were writing. Reopen the page to see their changes, then publish again."
+ },
+ {
+ "id": "app.page_draft.publish.invalid_page_id.app_error",
+ "translation": "Invalid page ID."
+ },
+ {
+ "id": "app.page_draft.publish.invalid_space_id.app_error",
+ "translation": "Invalid space ID."
+ },
+ {
+ "id": "app.page_draft.publish.invalid_user_id.app_error",
+ "translation": "Invalid user ID."
+ },
+ {
+ "id": "app.page_draft.publish.page_deleted.app_error",
+ "translation": "The page was deleted and can no longer be published."
+ },
+ {
+ "id": "app.page_draft.publish.page_not_found.app_error",
+ "translation": "The page does not exist or is not accessible in this space."
+ },
+ {
+ "id": "app.page_draft.publish.parent_unpublished.app_error",
+ "translation": "The parent page must be published before this page can be published."
+ },
+ {
+ "id": "app.page_draft.quota_exceeded.app_error",
+ "translation": "Draft limit reached. Publish or discard an existing draft to free up space."
+ },
+ {
+ "id": "app.page_draft.update.draft_changed.app_error",
+ "translation": "A concurrent autosave updated the draft. Reload it and try saving again."
+ },
+ {
+ "id": "app.page_draft.update.edit_conflict.app_error",
+ "translation": "A concurrent edit has been published; please reload the page to continue editing."
+ },
+ {
+ "id": "app.page_draft.update.invalid_page_id.app_error",
+ "translation": "Invalid page ID."
+ },
+ {
+ "id": "app.page_draft.update.invalid_parent_id.app_error",
+ "translation": "Invalid parent ID."
+ },
+ {
+ "id": "app.page_draft.update.invalid_space_id.app_error",
+ "translation": "Invalid space ID."
+ },
+ {
+ "id": "app.page_draft.update.invalid_user_id.app_error",
+ "translation": "Invalid user ID."
+ },
+ {
+ "id": "app.page_draft.update.nil_draft.app_error",
+ "translation": "Draft is required."
+ },
+ {
+ "id": "app.page_draft.update.page_not_found.app_error",
+ "translation": "The page does not exist or is not accessible in this space."
+ },
+ {
+ "id": "app.page_draft.update.parent_cycle.app_error",
+ "translation": "Setting this parent would create a cycle in the draft hierarchy."
+ },
+ {
+ "id": "app.page_draft.update.parent_too_deep.app_error",
+ "translation": "The draft hierarchy is too deep to add another level."
+ },
{
"id": "app.shared.description_too_long.app_error",
"translation": "The description exceeds the maximum length of {{.MaxLength}} characters."
@@ -367,6 +527,10 @@
"id": "app.store.too_large.app_error",
"translation": "The result set is too large; narrow your request (limit {{.Limit}})."
},
+ {
+ "id": "model.draft.is_valid.base_edit_at.app_error",
+ "translation": "Invalid draft baseline edit time."
+ },
{
"id": "model.draft.is_valid.body_size.app_error",
"translation": "The draft body is too long."
@@ -375,10 +539,18 @@
"id": "model.draft.is_valid.create_at.app_error",
"translation": "Invalid draft creation time."
},
+ {
+ "id": "model.draft.is_valid.file_id.app_error",
+ "translation": "One or more file IDs are invalid."
+ },
{
"id": "model.draft.is_valid.file_ids.app_error",
"translation": "The draft has too many file attachments."
},
+ {
+ "id": "model.draft.is_valid.last_active_at.app_error",
+ "translation": "Invalid draft last-active time."
+ },
{
"id": "model.draft.is_valid.page_id.app_error",
"translation": "Invalid page ID for the draft."
diff --git a/go.mod b/go.mod
index 4dca5b5..0b5ee62 100644
--- a/go.mod
+++ b/go.mod
@@ -2,19 +2,19 @@ module github.com/mattermost/mattermost-plugin-docs
go 1.26.4
-// Dev-only pins: server/public is pinned to the head of the paired core branch that adds the
-// Space backing-channel type — ChannelTypeSpace ("S"), pluginapi Channel.GetChannelOfType, and
-// pluginapi Channel.Restore are not yet in a released server/public.
+// Dev-only pins: server/public is pinned to the master commit that merged the Space
+// backing-channel type — ChannelTypeSpace ("S"), pluginapi Channel.GetChannelOfType, and
+// pluginapi Channel.Restore — which is not yet in a released server/public.
// server/v8 is the test harness only (storetest helpers); it does not contribute any runtime
// symbols and is pinned independently to an older commit. The two modules live in the same
// monorepo but are versioned independently, so their pseudo-version timestamps will always
// differ; what matters is that server/public has the APIs this plugin calls.
-// Bump both to a release tag once the core space-channel changes merge and ship.
+// Bump both to a release tag once the core space-channel changes ship.
require (
github.com/gorilla/mux v1.8.1
github.com/jmoiron/sqlx v1.4.0
github.com/lib/pq v1.12.3
- github.com/mattermost/mattermost/server/public v0.4.4-0.20260713131524-80d5b7966dc5
+ github.com/mattermost/mattermost/server/public v0.4.4-0.20260716203457-5f7f967a7dbf
github.com/mattermost/mattermost/server/v8 v8.0.0-20260623200446-ba033eae4704
github.com/mattermost/morph v1.1.0
github.com/mattermost/squirrel v0.5.0
diff --git a/go.sum b/go.sum
index 3a0cdde..5b7f020 100644
--- a/go.sum
+++ b/go.sum
@@ -140,8 +140,8 @@ github.com/mattermost/ldap v0.0.0-20231116144001-0f480c025956 h1:Y1Tu/swM31pVwwb
github.com/mattermost/ldap v0.0.0-20231116144001-0f480c025956/go.mod h1:SRl30Lb7/QoYyohYeVBuqYvvmXSZJxZgiV3Zf6VbxjI=
github.com/mattermost/logr/v2 v2.0.22 h1:npFkXlkAWR9J8payh8ftPcCZvLbHSI125mAM5/r/lP4=
github.com/mattermost/logr/v2 v2.0.22/go.mod h1:0sUKpO+XNMZApeumaid7PYaUZPBIydfuWZ0dqixXo+s=
-github.com/mattermost/mattermost/server/public v0.4.4-0.20260713131524-80d5b7966dc5 h1:L/o7nmoq4fv/wfcaen7tr85GxZ/V05hpzfpI3/TuOto=
-github.com/mattermost/mattermost/server/public v0.4.4-0.20260713131524-80d5b7966dc5/go.mod h1:rHFKFSnyNmyk1qieL00Fv+YuLb093Q7y8VTwBu/43ic=
+github.com/mattermost/mattermost/server/public v0.4.4-0.20260716203457-5f7f967a7dbf h1:Elv/2xLhHNauIlgA3EajAQYRzpF2OfCF77UTCmQdhX4=
+github.com/mattermost/mattermost/server/public v0.4.4-0.20260716203457-5f7f967a7dbf/go.mod h1:rHFKFSnyNmyk1qieL00Fv+YuLb093Q7y8VTwBu/43ic=
github.com/mattermost/mattermost/server/v8 v8.0.0-20260623200446-ba033eae4704 h1:vEw+u4m6mUrjHgpvDLXfCkuYcpUk8Q5CorgPRsqjW74=
github.com/mattermost/mattermost/server/v8 v8.0.0-20260623200446-ba033eae4704/go.mod h1:RBaqawSPsPB76XA4hfrIRFvLS0QWF1qJWHr2+w34+6s=
github.com/mattermost/morph v1.1.0 h1:Q9vrJbeM3s2jfweGheq12EFIzdNp9a/6IovcbvOQ6Cw=
diff --git a/server/api.go b/server/api.go
index 287f347..c128f36 100644
--- a/server/api.go
+++ b/server/api.go
@@ -23,8 +23,8 @@ import (
//
// Authorization: every route requires an authenticated user via MattermostAuthorizationRequired.
// All space- and page-scoped handlers additionally gate on backing-channel membership via
-// CheckSpaceMembership (implemented). Per-page role ACLs (author vs. editor within a space)
-// are not yet implemented and are deferred to a follow-up.
+// CheckSpaceMembership. Per-page role ACLs (author vs. editor within a space) are not yet
+// implemented.
func (p *Plugin) initRouter() *mux.Router {
router := mux.NewRouter()
router.Use(p.MattermostAuthorizationRequired)
@@ -60,6 +60,17 @@ func (p *Plugin) initRouter() *mux.Router {
api.HandleFunc("/spaces/{space_id}/pages/{page_id}/move-to-space", p.handleMovePageToSpace).Methods(http.MethodPatch)
api.HandleFunc("/spaces/{space_id}/pages/{page_id}/duplicate", p.handleDuplicatePage).Methods(http.MethodPost)
+ // Draft CRUD + publish.
+ api.HandleFunc("/spaces/{space_id}/drafts", p.handleCreateSpaceDraft).Methods(http.MethodPost)
+ api.HandleFunc("/spaces/{space_id}/drafts", p.handleGetPageDraftsForSpace).Methods(http.MethodGet)
+ api.HandleFunc("/spaces/{space_id}/pages/{page_id}/draft", p.handleUpdatePageDraft).Methods(http.MethodPatch)
+ api.HandleFunc("/spaces/{space_id}/pages/{page_id}/draft", p.handleGetPageDraft).Methods(http.MethodGet)
+ api.HandleFunc("/spaces/{space_id}/pages/{page_id}/draft", p.handleDeletePageDraft).Methods(http.MethodDelete)
+ api.HandleFunc("/spaces/{space_id}/pages/{page_id}/draft/publish", p.handlePublishPageDraft).Methods(http.MethodPost)
+
+ // Presence.
+ api.HandleFunc("/spaces/{space_id}/pages/{page_id}/active-editors", p.handleGetPageActiveEditors).Methods(http.MethodGet)
+
return router
}
@@ -118,11 +129,38 @@ func (p *Plugin) writeAppError(w http.ResponseWriter, appErr *mmmodel.AppError)
if appErr.StatusCode >= http.StatusInternalServerError {
p.API.LogError("Docs API request failed", "where", appErr.Where, "id", appErr.Id, "status_code", appErr.StatusCode, "err", appErr.Error())
}
+ if appErr.StatusCode == http.StatusConflict {
+ p.writeConflictWithPage(w, appErr, nil)
+ return
+ }
safe := *appErr
safe.WipeDetailed()
writeJSON(w, appErr.StatusCode, &safe)
}
+// conflictResponse is the body every 409 carries: the scrubbed AppError plus the current server
+// page. One shape across all conflicts means a client parses a 409 the same way whichever endpoint
+// produced it, rather than branching on the route.
+//
+// current_page is null when the handler has no page to offer — the conflict was not about a page, or
+// the re-read that would have produced it failed — so a client treats it as an optional shortcut and
+// falls back to a GET. Where it is populated (publish and page-update conflicts, which already read
+// the live page to build the error) it saves that round-trip: the client diffs and re-baselines
+// against the returned EditAt directly. The whole page is returned rather than a curated snapshot,
+// so the client renders whatever it needs.
+type conflictResponse struct {
+ Error *mmmodel.AppError `json:"error"`
+ CurrentPage *model.Page `json:"current_page"`
+}
+
+// writeConflictWithPage writes a conflictResponse using the AppError's own StatusCode (409) as the
+// HTTP status. DetailedError is scrubbed first, matching writeAppError.
+func (p *Plugin) writeConflictWithPage(w http.ResponseWriter, appErr *mmmodel.AppError, current *model.Page) {
+ safe := *appErr
+ safe.WipeDetailed()
+ writeJSON(w, appErr.StatusCode, conflictResponse{Error: &safe, CurrentPage: current})
+}
+
// writeJSON serialises v as a JSON body with the given status.
func writeJSON(w http.ResponseWriter, status int, v any) {
w.Header().Set("Content-Type", "application/json")
diff --git a/server/api_handler_test.go b/server/api_handler_test.go
index 3b558ef..fc0d569 100644
--- a/server/api_handler_test.go
+++ b/server/api_handler_test.go
@@ -110,7 +110,6 @@ func seedSpace(t *testing.T, s *store.Store, channelID string) *model.Space {
return seedSpaceInTeam(t, s, channelID, mmmodel.NewId())
}
-// seedSpaceInTeam mirrors testutil.MustCreateSpace's (channelID, teamID) parameter order.
func seedSpaceInTeam(t *testing.T, s *store.Store, channelID, teamID string) *model.Space {
t.Helper()
return testutil.MustCreateSpace(t, s, channelID, teamID)
@@ -224,16 +223,16 @@ func TestHandler_SpaceAndPageRoundTrip(t *testing.T) {
require.Equal(t, space.Id, page.SpaceId)
})
- t.Run("create page with content and search_text", func(t *testing.T) {
+ t.Run("create page derives search_text from body", func(t *testing.T) {
rec := h.do(t, http.MethodPost, "/api/v1/spaces/"+space.Id+"/pages", user, map[string]any{
"title": "Page B",
- "body": `{"type":"doc","content":[]}`,
- "search_text": "plain text projection",
+ "body": `{"type":"doc","content":[{"type":"paragraph","content":[{"type":"text","text":"searchable body"}]}]}`,
+ "search_text": "ignored client value",
})
require.Equal(t, http.StatusCreated, rec.Code)
var page model.Page
require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &page))
- require.Equal(t, "plain text projection", page.SearchText)
+ require.Equal(t, "searchable body", page.SearchText, "SearchText is derived from the body, not the caller-supplied value")
})
t.Run("get page in wrong space is 404", func(t *testing.T) {
@@ -545,11 +544,10 @@ func TestHandler_UpdatePage(t *testing.T) {
space := seedSpace(t, h.store, channelID)
page := seedPage(t, h.store, space.Id, channelID, "")
- // Body and search text must be patched together (search text is the body's plain-text
- // projection), so both are supplied.
+ // SearchText is derived from the body server-side, so only the body is supplied; a
+ // caller-supplied search_text is ignored.
body := map[string]any{
- "body": `{"type":"doc","content":[{"type":"paragraph"}]}`,
- "search_text": "updated text",
+ "body": `{"type":"doc","content":[{"type":"paragraph","content":[{"type":"text","text":"updated text"}]}]}`,
"base_edit_at": page.EditAt,
}
rec := h.do(t, http.MethodPatch, "/api/v1/spaces/"+space.Id+"/pages/"+page.Id, user, body)
@@ -557,12 +555,24 @@ func TestHandler_UpdatePage(t *testing.T) {
var updated model.Page
require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &updated))
- require.Equal(t, `{"type":"doc","content":[{"type":"paragraph"}]}`, updated.Body)
+ require.Contains(t, updated.Body, "updated text")
require.Equal(t, "updated text", updated.SearchText)
// The first update bumped EditAt, so the same baseline is now stale.
rec = h.do(t, http.MethodPatch, "/api/v1/spaces/"+space.Id+"/pages/"+page.Id, user, body)
require.Equal(t, http.StatusConflict, rec.Code)
+
+ // Every 409 carries the same shape, so a client parses it without branching on the route. This
+ // one populates current_page, letting the caller re-baseline without a follow-up read.
+ var conflict struct {
+ Error *mmmodel.AppError `json:"error"`
+ CurrentPage *model.Page `json:"current_page"`
+ }
+ require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &conflict))
+ require.NotNil(t, conflict.Error)
+ require.Empty(t, conflict.Error.DetailedError, "the conflict body must not carry internal error detail")
+ require.NotNil(t, conflict.CurrentPage, "the update conflict must carry the current server page")
+ require.Greater(t, conflict.CurrentPage.EditAt, page.EditAt, "current page must carry the advanced baseline")
}
// TestHandler_UpdatePage_BaselineRequired verifies a PATCH that omits base_edit_at without force
@@ -767,7 +777,7 @@ func TestHandler_MovePageToSpace_DepthExceeded(t *testing.T) {
require.NoError(t, err)
parentID := ""
- for range app.MaxPageDepth {
+ for range model.MaxPageDepth {
p := seedPage(t, h.store, spaceB.Id, channelB, parentID)
parentID = p.Id
}
@@ -837,7 +847,7 @@ func TestHandler_MovePage_MaxDepthExceeded(t *testing.T) {
space := seedSpace(t, h.store, channelID)
parentID := ""
- for range app.MaxPageDepth {
+ for range model.MaxPageDepth {
p := seedPage(t, h.store, space.Id, channelID, parentID)
parentID = p.Id
}
@@ -1097,6 +1107,14 @@ func TestHandler_SpaceMembershipRequired(t *testing.T) {
{http.MethodPatch, "/api/v1/spaces/" + space.Id + "/pages/" + page.Id + "/move", nil},
{http.MethodPatch, "/api/v1/spaces/" + space.Id + "/pages/" + page.Id + "/move-to-space", nil},
{http.MethodPost, "/api/v1/spaces/" + space.Id + "/pages/" + page.Id + "/duplicate", nil},
+ // Draft + presence handlers.
+ {http.MethodPost, "/api/v1/spaces/" + space.Id + "/drafts", map[string]any{"title": "D"}},
+ {http.MethodGet, "/api/v1/spaces/" + space.Id + "/drafts", nil},
+ {http.MethodPatch, "/api/v1/spaces/" + space.Id + "/pages/" + page.Id + "/draft", map[string]any{"title": "D"}},
+ {http.MethodGet, "/api/v1/spaces/" + space.Id + "/pages/" + page.Id + "/draft", nil},
+ {http.MethodDelete, "/api/v1/spaces/" + space.Id + "/pages/" + page.Id + "/draft", nil},
+ {http.MethodPost, "/api/v1/spaces/" + space.Id + "/pages/" + page.Id + "/draft/publish", nil},
+ {http.MethodGet, "/api/v1/spaces/" + space.Id + "/pages/" + page.Id + "/active-editors", nil},
}
for _, tc := range cases {
rec := h.do(t, tc.method, tc.path, stranger, tc.body)
diff --git a/server/api_page.go b/server/api_page.go
index d7f27f5..0328908 100644
--- a/server/api_page.go
+++ b/server/api_page.go
@@ -44,16 +44,16 @@ func (p *Plugin) handleCreatePage(w http.ResponseWriter, r *http.Request) {
return
}
+ // SearchText is not accepted: it is derived server-side from Body.
var req struct {
- Title string `json:"title"`
- ParentId *string `json:"parent_id,omitempty"`
- Body string `json:"body,omitempty"`
- SearchText string `json:"search_text,omitempty"`
+ Title string `json:"title"`
+ ParentId *string `json:"parent_id,omitempty"`
+ Body string `json:"body,omitempty"`
}
if !p.decodeJSONBody(w, r, maxPageBodyBytes, &req, "handleCreatePage", false) {
return
}
- page, appErr := p.service.CreatePage(vars["space_id"], mmmodel.SafeDereference(req.ParentId), req.Title, req.Body, req.SearchText, userID)
+ page, appErr := p.service.CreatePage(vars["space_id"], mmmodel.SafeDereference(req.ParentId), req.Title, req.Body, userID)
if appErr != nil {
p.writeAppError(w, appErr)
return
@@ -85,10 +85,10 @@ func (p *Plugin) handleUpdatePage(w http.ResponseWriter, r *http.Request) {
return
}
+ // SearchText is not accepted: it is derived server-side from Body (matches the create path).
var req struct {
Title *string `json:"title"`
Body *string `json:"body"`
- SearchText *string `json:"search_text"`
Props *mmmodel.StringInterface `json:"props"`
BaseEditAt *int64 `json:"base_edit_at"`
Force bool `json:"force"`
@@ -96,10 +96,14 @@ func (p *Plugin) handleUpdatePage(w http.ResponseWriter, r *http.Request) {
if !p.decodeJSONBody(w, r, maxPageBodyBytes, &req, "handleUpdatePage", false) {
return
}
- patch := &model.PagePatch{Title: req.Title, Body: req.Body, SearchText: req.SearchText, Props: req.Props}
+ patch := &model.PagePatch{Title: req.Title, Body: req.Body, Props: req.Props}
updated, appErr := p.service.UpdatePage(vars["page_id"], vars["space_id"], patch, req.BaseEditAt, req.Force, userID)
if appErr != nil {
+ if updated != nil {
+ p.writeConflictWithPage(w, appErr, updated)
+ return
+ }
p.writeAppError(w, appErr)
return
}
diff --git a/server/api_page_drafts.go b/server/api_page_drafts.go
new file mode 100644
index 0000000..38441e0
--- /dev/null
+++ b/server/api_page_drafts.go
@@ -0,0 +1,234 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See LICENSE.txt for license information.
+
+package main
+
+import (
+ "net/http"
+
+ "github.com/gorilla/mux"
+ mmmodel "github.com/mattermost/mattermost/server/public/model"
+
+ "github.com/mattermost/mattermost-plugin-docs/server/model"
+)
+
+const (
+ // maxDraftBodyBytes caps the autosave request, which carries the full document body. Body is JSON
+ // nested inside the request JSON, so its transport form can grow far beyond its decoded size once
+ // quotes, backslashes, and control characters are escaped (worst case ~6x for all-control-char
+ // input). Size the transport cap for that worst case plus headroom for the title/props/file-ids
+ // and JSON envelope; the decoded body stays capped at model.PageBodyMaxBytes during normalization.
+ draftBodyHeadroomBytes = 64 * 1024
+ maxDraftBodyBytes = 6*model.PageBodyMaxBytes + draftBodyHeadroomBytes
+)
+
+// handleUpdatePageDraft handles PATCH /api/v1/spaces/{space_id}/pages/{page_id}/draft
+// It upserts the calling user's draft for the page. PATCH is deliberate: the request merges rather
+// than replaces, so an omitted field means "unchanged", not "cleared" (autosave heartbeats carry
+// only the fields the editor touched). parent_id: null omits the field; parent_id: "" explicitly
+// clears it.
+//
+// For existing published pages, the first request creates the draft (open an edit session). The client
+// must send the top-level base_edit_at field — the page's EditAt at the moment the user opened it — on
+// every autosave, so a subsequent publish can detect a concurrent edit. It is not a props key. The
+// stored baseline is write-once: whichever request establishes the draft row freezes it, and values
+// sent on later autosaves are ignored (re-baselining requires discarding the draft and reopening).
+//
+// For new-page drafts (no page row yet), the draft must already exist via POST
+// /spaces/{space_id}/drafts. This prevents a space member who learns another user's pending page
+// ID from squatting on it via the autosave endpoint.
+func (p *Plugin) handleUpdatePageDraft(w http.ResponseWriter, r *http.Request) {
+ vars := mux.Vars(r)
+ spaceID := vars["space_id"]
+ pageID := vars["page_id"]
+ userID := userIDFromRequest(r)
+
+ space, ok := p.requireSpaceMembership(w, spaceID, userID, false)
+ if !ok {
+ return
+ }
+
+ var req struct {
+ ParentId *string `json:"parent_id"`
+ Title string `json:"title"`
+ Body string `json:"body"`
+ FileIds *mmmodel.StringArray `json:"file_ids"`
+ Props *mmmodel.StringInterface `json:"props"`
+ BaseEditAt *int64 `json:"base_edit_at"`
+ }
+ if !p.decodeJSONBody(w, r, maxDraftBodyBytes, &req, "handleUpdatePageDraft", false) {
+ return
+ }
+
+ // base_edit_at nil → 0 (no baseline). A new-page draft legitimately has no baseline. For an
+ // existing published page the client must send base_edit_at on every autosave (see the handler doc
+ // above): omitting it on the first autosave of an edit session is rejected with 409 — an
+ // edit-session draft cannot be opened against a live page without a baseline. Props flows via
+ // the pointer below (like file_ids), so it is not set on the struct here.
+ var baseEditAt int64
+ if req.BaseEditAt != nil {
+ baseEditAt = *req.BaseEditAt
+ }
+ draft := &model.Draft{
+ UserId: userID,
+ SpaceId: spaceID,
+ PageId: pageID,
+ Title: req.Title,
+ Body: req.Body,
+ BaseEditAt: baseEditAt,
+ }
+
+ // req.ParentId nil → preserve; pointer to "" → clear to root; pointer to ID → set parent.
+ // req.FileIds nil → preserve; pointer to [] → clear; pointer to [...] → replace.
+ // req.Props nil → preserve; non-nil → replace the whole map (an empty map clears all keys).
+ saved, appErr := p.service.UpdatePageDraft(draft, req.ParentId, req.FileIds, req.Props, space.ChannelId)
+ if appErr != nil {
+ p.writeAppError(w, appErr)
+ return
+ }
+
+ writeJSON(w, http.StatusOK, saved)
+}
+
+// handleGetPageDraft handles GET /api/v1/spaces/{space_id}/pages/{page_id}/draft
+func (p *Plugin) handleGetPageDraft(w http.ResponseWriter, r *http.Request) {
+ vars := mux.Vars(r)
+ spaceID := vars["space_id"]
+ pageID := vars["page_id"]
+ userID := userIDFromRequest(r)
+
+ if _, ok := p.requireSpaceMembership(w, spaceID, userID, false); !ok {
+ return
+ }
+
+ draft, appErr := p.service.GetPageDraft(userID, spaceID, pageID)
+ if appErr != nil {
+ p.writeAppError(w, appErr)
+ return
+ }
+
+ writeJSON(w, http.StatusOK, draft)
+}
+
+// handleDeletePageDraft handles DELETE /api/v1/spaces/{space_id}/pages/{page_id}/draft
+func (p *Plugin) handleDeletePageDraft(w http.ResponseWriter, r *http.Request) {
+ vars := mux.Vars(r)
+ spaceID := vars["space_id"]
+ pageID := vars["page_id"]
+ userID := userIDFromRequest(r)
+
+ space, ok := p.requireSpaceMembership(w, spaceID, userID, false)
+ if !ok {
+ return
+ }
+
+ if appErr := p.service.DeletePageDraft(userID, spaceID, pageID, space.ChannelId); appErr != nil {
+ p.writeAppError(w, appErr)
+ return
+ }
+
+ writeStatusOK(w)
+}
+
+// handleCreateSpaceDraft handles POST /api/v1/spaces/{space_id}/drafts
+// It creates a new-page draft (no Pages row) with a server-generated page id, reserving the id
+// before the page is published so a new page has a stable link from the start.
+//
+// The page id is generated server-side and returned only in the response body, so the caller
+// cannot address the page until this call completes: subsequent requests (autosave to
+// .../pages/{page_id}/draft, publish) all key off the returned id. A new page therefore costs
+// one round-trip before it is editable; the id is not caller-supplied.
+func (p *Plugin) handleCreateSpaceDraft(w http.ResponseWriter, r *http.Request) {
+ vars := mux.Vars(r)
+ spaceID := vars["space_id"]
+ userID := userIDFromRequest(r)
+
+ if _, ok := p.requireSpaceMembership(w, spaceID, userID, false); !ok {
+ return
+ }
+
+ var req struct {
+ Title string `json:"title"`
+ ParentId string `json:"parent_id"`
+ }
+ if !p.decodeJSONBody(w, r, maxPageStructBodyBytes, &req, "handleCreateSpaceDraft", false) {
+ return
+ }
+
+ saved, appErr := p.service.CreateSpaceDraft(userID, spaceID, req.Title, req.ParentId)
+ if appErr != nil {
+ p.writeAppError(w, appErr)
+ return
+ }
+
+ writeJSON(w, http.StatusCreated, saved)
+}
+
+// handlePublishPageDraft handles POST /api/v1/spaces/{space_id}/pages/{page_id}/draft/publish
+// It publishes the calling user's draft as a page, creating it on first publish and updating it
+// otherwise.
+//
+// The optimistic-lock baseline for an edit-publish is not a field on this request: it travels with
+// the draft, stored in its write-once BaseEditAt column (sent as the top-level base_edit_at field on
+// the autosave requests). This differs from the per-request base_edit_at on handleUpdatePage (and
+// expected_update_at on handleMovePage) because a publish ships whatever the draft already holds
+// rather than re-supplying a freshly-read baseline. Because that baseline is write-once, a 409
+// edit-conflict cannot be resolved by autosaving a newer base_edit_at: the client recovers by
+// republishing with force, or by discarding the draft and reopening the edit session.
+func (p *Plugin) handlePublishPageDraft(w http.ResponseWriter, r *http.Request) {
+ vars := mux.Vars(r)
+ spaceID := vars["space_id"]
+ pageID := vars["page_id"]
+ userID := userIDFromRequest(r)
+
+ if _, ok := p.requireSpaceMembership(w, spaceID, userID, false); !ok {
+ return
+ }
+
+ // Optional body: {force: bool}. force=true overrides first-write-wins.
+ var req struct {
+ Force bool `json:"force"`
+ }
+ // allowEmptyBody=true: an absent body means force=false. A present-but-malformed body is still
+ // an error, so the return value must be checked.
+ if !p.decodeJSONBody(w, r, maxPageStructBodyBytes, &req, "handlePublishPageDraft", true) {
+ return
+ }
+
+ page, wasCreated, appErr := p.service.PublishPageDraft(userID, spaceID, pageID, req.Force)
+ if appErr != nil {
+ if page != nil {
+ p.writeConflictWithPage(w, appErr, page)
+ return
+ }
+ p.writeAppError(w, appErr)
+ return
+ }
+
+ status := http.StatusOK
+ if wasCreated {
+ status = http.StatusCreated
+ }
+ writeJSON(w, status, page)
+}
+
+// handleGetPageDraftsForSpace handles GET /api/v1/spaces/{space_id}/drafts
+// It lists the calling user's unpublished page drafts for the space.
+func (p *Plugin) handleGetPageDraftsForSpace(w http.ResponseWriter, r *http.Request) {
+ vars := mux.Vars(r)
+ spaceID := vars["space_id"]
+ userID := userIDFromRequest(r)
+
+ if _, ok := p.requireSpaceMembership(w, spaceID, userID, false); !ok {
+ return
+ }
+
+ page, perPage := pageParam(r), perPageParam(r)
+ drafts, hasMore, appErr := p.service.GetPageDraftsForSpace(userID, spaceID, page, perPage)
+ if appErr != nil {
+ p.writeAppError(w, appErr)
+ return
+ }
+
+ writePaginatedJSON(w, drafts, page, perPage, hasMore)
+}
diff --git a/server/api_page_drafts_test.go b/server/api_page_drafts_test.go
new file mode 100644
index 0000000..ccc765a
--- /dev/null
+++ b/server/api_page_drafts_test.go
@@ -0,0 +1,534 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See LICENSE.txt for license information.
+
+package main
+
+import (
+ "bytes"
+ "encoding/json"
+ "net/http"
+ "net/http/httptest"
+ "testing"
+
+ "github.com/stretchr/testify/require"
+
+ "github.com/mattermost/mattermost/server/public/plugin"
+
+ mmmodel "github.com/mattermost/mattermost/server/public/model"
+
+ "github.com/mattermost/mattermost-plugin-docs/server/app"
+ "github.com/mattermost/mattermost-plugin-docs/server/model"
+)
+
+// TestHandler_DraftLifecycle drives the draft routes end to end over the real router: create a
+// new-page draft, autosave it, read it back, publish it (201, reserved id preserved), and confirm
+// the draft is gone afterwards.
+func TestHandler_DraftLifecycle(t *testing.T) {
+ h := openTestPlugin(t, nil)
+ channelID := mmmodel.NewId()
+ space := seedSpace(t, h.store, channelID)
+ userID := mmmodel.NewId()
+ base := "/api/v1/spaces/" + space.Id
+
+ rec := h.do(t, http.MethodPost, base+"/drafts", userID, map[string]any{"title": "New Doc"})
+ require.Equal(t, http.StatusCreated, rec.Code)
+ var draft model.Draft
+ require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &draft))
+ pageID := draft.PageId
+ require.True(t, mmmodel.IsValidId(pageID))
+
+ rec = h.do(t, http.MethodPatch, base+"/pages/"+pageID+"/draft", userID, map[string]any{
+ "title": "New Doc",
+ "body": `{"type":"doc","content":[{"type":"paragraph","content":[{"type":"text","text":"hello"}]}]}`,
+ })
+ require.Equal(t, http.StatusOK, rec.Code)
+
+ rec = h.do(t, http.MethodGet, base+"/pages/"+pageID+"/draft", userID, nil)
+ require.Equal(t, http.StatusOK, rec.Code)
+ require.Contains(t, rec.Body.String(), "hello")
+
+ rec = h.do(t, http.MethodPost, base+"/pages/"+pageID+"/draft/publish", userID, nil)
+ require.Equal(t, http.StatusCreated, rec.Code)
+ var page model.Page
+ require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &page))
+ require.Equal(t, pageID, page.Id, "publish preserves the reserved draft id")
+
+ rec = h.do(t, http.MethodGet, base+"/pages/"+pageID+"/draft", userID, nil)
+ require.Equal(t, http.StatusNotFound, rec.Code, "draft is deleted on publish")
+}
+
+// TestHandler_ListSpaceDrafts covers the drafts listing: it is paginated like every other list
+// endpoint, and it omits draft bodies so the listing does not ship whole documents.
+func TestHandler_ListSpaceDrafts(t *testing.T) {
+ h := openTestPlugin(t, nil)
+ space := seedSpace(t, h.store, mmmodel.NewId())
+ userID := mmmodel.NewId()
+ base := "/api/v1/spaces/" + space.Id
+
+ for _, title := range []string{"First", "Second", "Third"} {
+ rec := h.do(t, http.MethodPost, base+"/drafts", userID, map[string]any{"title": title})
+ require.Equal(t, http.StatusCreated, rec.Code)
+ }
+
+ var listed struct {
+ Items []map[string]any `json:"items"`
+ Page int `json:"page"`
+ PerPage int `json:"per_page"`
+ HasMore bool `json:"has_more"`
+ }
+
+ rec := h.do(t, http.MethodGet, base+"/drafts?page=0&per_page=2", userID, nil)
+ require.Equal(t, http.StatusOK, rec.Code)
+ require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &listed))
+ require.Len(t, listed.Items, 2)
+ require.True(t, listed.HasMore, "a third draft remains")
+ require.NotContains(t, listed.Items[0], "body", "the listing must not ship draft bodies")
+
+ rec = h.do(t, http.MethodGet, base+"/drafts?page=1&per_page=2", userID, nil)
+ require.Equal(t, http.StatusOK, rec.Code)
+ require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &listed))
+ require.Len(t, listed.Items, 1)
+ require.False(t, listed.HasMore, "the last page reports no more")
+}
+
+// TestHandler_PublishMalformedBodyReturns400 is the regression for the unchecked-decode bug: a
+// malformed publish body must be rejected with 400 and must NOT publish the draft.
+func TestHandler_PublishMalformedBodyReturns400(t *testing.T) {
+ h := openTestPlugin(t, nil)
+ channelID := mmmodel.NewId()
+ space := seedSpace(t, h.store, channelID)
+ userID := mmmodel.NewId()
+ base := "/api/v1/spaces/" + space.Id
+
+ rec := h.do(t, http.MethodPost, base+"/drafts", userID, map[string]any{"title": "Doc"})
+ require.Equal(t, http.StatusCreated, rec.Code)
+ var draft model.Draft
+ require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &draft))
+
+ // Raw, truncated JSON body — h.do would marshal a value, so issue the request directly.
+ req := httptest.NewRequest(http.MethodPost, base+"/pages/"+draft.PageId+"/draft/publish", bytes.NewReader([]byte(`{"force":tru`)))
+ req.Header.Set("Mattermost-User-ID", userID)
+ malformed := httptest.NewRecorder()
+ h.plugin.ServeHTTP(&plugin.Context{}, malformed, req)
+ require.Equal(t, http.StatusBadRequest, malformed.Code)
+
+ // The draft must still exist (publish did not run).
+ rec = h.do(t, http.MethodGet, base+"/pages/"+draft.PageId+"/draft", userID, nil)
+ require.Equal(t, http.StatusOK, rec.Code, "a malformed publish body must not publish or delete the draft")
+}
+
+// TestHandler_UpdatePageDraftRequiresExistingDraft confirms the update-only guard: PATCH on a page id
+// that has no existing draft must return 404 rather than silently creating one.
+func TestHandler_UpdatePageDraftRequiresExistingDraft(t *testing.T) {
+ h := openTestPlugin(t, nil)
+ space := seedSpace(t, h.store, mmmodel.NewId())
+ userID := mmmodel.NewId()
+
+ // No draft has been created for this page id — the PATCH must be rejected.
+ rec := h.do(t, http.MethodPatch, "/api/v1/spaces/"+space.Id+"/pages/"+mmmodel.NewId()+"/draft", userID, map[string]any{
+ "title": "ghost",
+ })
+ require.Equal(t, http.StatusNotFound, rec.Code)
+}
+
+// TestHandler_UpdatePageDraftClearsParentWhenParentIdIsEmptyString covers the null-vs-empty
+// distinction: sending parent_id: "" in the PATCH body must clear an existing parent (set it to
+// root), while omitting parent_id entirely must leave the parent unchanged.
+func TestHandler_UpdatePageDraftClearsParentWhenParentIdIsEmptyString(t *testing.T) {
+ h := openTestPlugin(t, nil)
+ space := seedSpace(t, h.store, mmmodel.NewId())
+ userID := mmmodel.NewId()
+ base := "/api/v1/spaces/" + space.Id
+
+ // Create the parent draft.
+ rec := h.do(t, http.MethodPost, base+"/drafts", userID, map[string]any{"title": "Parent"})
+ require.Equal(t, http.StatusCreated, rec.Code)
+ var parentDraft model.Draft
+ require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &parentDraft))
+
+ // Create the child draft under the parent.
+ rec = h.do(t, http.MethodPost, base+"/drafts", userID, map[string]any{
+ "title": "Child",
+ "parent_id": parentDraft.PageId,
+ })
+ require.Equal(t, http.StatusCreated, rec.Code)
+ var childDraft model.Draft
+ require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &childDraft))
+ require.Equal(t, parentDraft.PageId, childDraft.ParentId)
+
+ // PATCH with parent_id omitted must preserve the existing parent.
+ rec = h.do(t, http.MethodPatch, base+"/pages/"+childDraft.PageId+"/draft", userID, map[string]any{
+ "title": "Child updated",
+ })
+ require.Equal(t, http.StatusOK, rec.Code)
+ var saved model.Draft
+ require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &saved))
+ require.Equal(t, parentDraft.PageId, saved.ParentId, "omitting parent_id must preserve the existing parent")
+
+ // PATCH with parent_id: "" must clear the parent to root.
+ rec = h.do(t, http.MethodPatch, base+"/pages/"+childDraft.PageId+"/draft", userID, map[string]any{
+ "parent_id": "",
+ })
+ require.Equal(t, http.StatusOK, rec.Code)
+ require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &saved))
+ require.Empty(t, saved.ParentId, `parent_id: "" must clear the parent`)
+}
+
+// TestHandler_UpdatePageDraftCreatesForExistingPage drives the existing-page edit flow: publish a
+// new-page draft to get a live page, then open an edit session by PATCH .../draft with the page's
+// EditAt baseline in the top-level base_edit_at field. Verifies the draft is created, autosave
+// updates it, publish succeeds (edit path → 200), and the draft is gone afterwards.
+func TestHandler_UpdatePageDraftCreatesForExistingPage(t *testing.T) {
+ h := openTestPlugin(t, nil)
+ channelID := mmmodel.NewId()
+ space := seedSpace(t, h.store, channelID)
+ userID := mmmodel.NewId()
+ base := "/api/v1/spaces/" + space.Id
+
+ // Step 1: create a new-page draft and publish it to get a live page.
+ rec := h.do(t, http.MethodPost, base+"/drafts", userID, map[string]any{"title": "Original"})
+ require.Equal(t, http.StatusCreated, rec.Code)
+ var draft model.Draft
+ require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &draft))
+ pageID := draft.PageId
+
+ rec = h.do(t, http.MethodPost, base+"/pages/"+pageID+"/draft/publish", userID, nil)
+ require.Equal(t, http.StatusCreated, rec.Code)
+ var page model.Page
+ require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &page))
+ require.Equal(t, pageID, page.Id)
+
+ // Step 2: open an edit session — first PATCH creates the draft for an existing page.
+ rec = h.do(t, http.MethodPatch, base+"/pages/"+pageID+"/draft", userID, map[string]any{
+ "title": "Original",
+ "base_edit_at": page.EditAt,
+ })
+ require.Equal(t, http.StatusOK, rec.Code)
+ var editDraft model.Draft
+ require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &editDraft))
+ require.Equal(t, pageID, editDraft.PageId)
+ require.Equal(t, page.EditAt, editDraft.BaseEditAt, "draft must carry the base_edit_at baseline so publish can detect conflicts")
+
+ // GET must round-trip the same baseline.
+ rec = h.do(t, http.MethodGet, base+"/pages/"+pageID+"/draft", userID, nil)
+ require.Equal(t, http.StatusOK, rec.Code)
+ var fetched model.Draft
+ require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &fetched))
+ require.Equal(t, page.EditAt, fetched.BaseEditAt, "GET must return the base_edit_at baseline that was set via PATCH")
+
+ // Step 3: autosave new content.
+ rec = h.do(t, http.MethodPatch, base+"/pages/"+pageID+"/draft", userID, map[string]any{
+ "title": "Edited",
+ "body": `{"type":"doc","content":[{"type":"paragraph","content":[{"type":"text","text":"updated"}]}]}`,
+ })
+ require.Equal(t, http.StatusOK, rec.Code)
+
+ // Step 4: publish the edit — edit path returns 200 (not 201).
+ rec = h.do(t, http.MethodPost, base+"/pages/"+pageID+"/draft/publish", userID, nil)
+ require.Equal(t, http.StatusOK, rec.Code, "edit-path publish must return 200")
+ var updated model.Page
+ require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &updated))
+ require.Equal(t, pageID, updated.Id)
+ require.Equal(t, "Edited", updated.Title)
+
+ // Step 5: the draft must be gone after publish.
+ rec = h.do(t, http.MethodGet, base+"/pages/"+pageID+"/draft", userID, nil)
+ require.Equal(t, http.StatusNotFound, rec.Code, "draft must be deleted on publish")
+}
+
+// TestHandler_PublishConflict409 covers the stale-baseline 409: when user B publishes an edit
+// that advances the page's EditAt, user A's publish against the original baseline must return 409.
+func TestHandler_PublishConflict409(t *testing.T) {
+ h := openTestPlugin(t, nil)
+ channelID := mmmodel.NewId()
+ space := seedSpace(t, h.store, channelID)
+ userA := mmmodel.NewId()
+ userB := mmmodel.NewId()
+ base := "/api/v1/spaces/" + space.Id
+
+ // Create a new-page draft and publish it to get a live page.
+ rec := h.do(t, http.MethodPost, base+"/drafts", userA, map[string]any{"title": "Shared Doc"})
+ require.Equal(t, http.StatusCreated, rec.Code)
+ var draft model.Draft
+ require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &draft))
+ pageID := draft.PageId
+
+ rec = h.do(t, http.MethodPost, base+"/pages/"+pageID+"/draft/publish", userA, nil)
+ require.Equal(t, http.StatusCreated, rec.Code)
+ var page model.Page
+ require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &page))
+ editAt := page.EditAt
+
+ // User A and user B both open edit sessions against the same baseline.
+ rec = h.do(t, http.MethodPatch, base+"/pages/"+pageID+"/draft", userA, map[string]any{
+ "title": "Edit by A",
+ "base_edit_at": editAt,
+ })
+ require.Equal(t, http.StatusOK, rec.Code)
+
+ rec = h.do(t, http.MethodPatch, base+"/pages/"+pageID+"/draft", userB, map[string]any{
+ "title": "Edit by B",
+ "base_edit_at": editAt,
+ })
+ require.Equal(t, http.StatusOK, rec.Code)
+
+ // User B publishes first, advancing the page's EditAt.
+ rec = h.do(t, http.MethodPost, base+"/pages/"+pageID+"/draft/publish", userB, nil)
+ require.Equal(t, http.StatusOK, rec.Code, "user B edit-path publish must succeed")
+
+ // User A publishes against the now-stale baseline — must get 409.
+ rec = h.do(t, http.MethodPost, base+"/pages/"+pageID+"/draft/publish", userA, nil)
+ require.Equal(t, http.StatusConflict, rec.Code, "stale baseline must return 409 Conflict")
+
+ // The 409 body carries the current server page so the client can diff and re-baseline without a
+ // follow-up read: it reflects user B's winning edit and the advanced EditAt.
+ var conflict struct {
+ Error *mmmodel.AppError `json:"error"`
+ CurrentPage *model.Page `json:"current_page"`
+ }
+ require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &conflict))
+ require.NotNil(t, conflict.Error, "conflict body must include the error")
+ require.Empty(t, conflict.Error.DetailedError, "the conflict body must not carry internal error detail")
+ require.NotNil(t, conflict.CurrentPage, "conflict body must include the current server page")
+ require.Equal(t, "Edit by B", conflict.CurrentPage.Title, "current page must reflect the winning edit")
+ require.Greater(t, conflict.CurrentPage.EditAt, editAt, "current page must carry the advanced baseline")
+}
+
+// TestHandler_DeletePageDraft verifies the DELETE endpoint: 200 on success, draft is gone
+// afterwards, and 404 when no draft exists.
+func TestHandler_DeletePageDraft(t *testing.T) {
+ h := openTestPlugin(t, nil)
+ space := seedSpace(t, h.store, mmmodel.NewId())
+ userID := mmmodel.NewId()
+ base := "/api/v1/spaces/" + space.Id
+
+ // Create a new-page draft.
+ rec := h.do(t, http.MethodPost, base+"/drafts", userID, map[string]any{"title": "To Delete"})
+ require.Equal(t, http.StatusCreated, rec.Code)
+ var draft model.Draft
+ require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &draft))
+ pageID := draft.PageId
+
+ // DELETE the draft — must return 200.
+ rec = h.do(t, http.MethodDelete, base+"/pages/"+pageID+"/draft", userID, nil)
+ require.Equal(t, http.StatusOK, rec.Code)
+
+ // GET after delete must return 404.
+ rec = h.do(t, http.MethodGet, base+"/pages/"+pageID+"/draft", userID, nil)
+ require.Equal(t, http.StatusNotFound, rec.Code, "draft must be gone after delete")
+
+ // DELETE again on a missing draft must return 404.
+ rec = h.do(t, http.MethodDelete, base+"/pages/"+pageID+"/draft", userID, nil)
+ require.Equal(t, http.StatusNotFound, rec.Code, "deleting a non-existent draft must return 404")
+}
+
+// TestHandler_ActiveEditorsWrongSpaceReturns404 is the regression for the cross-space presence
+// leak: a page in one space must not be reachable through another space's active-editors route.
+func TestHandler_ActiveEditorsWrongSpaceReturns404(t *testing.T) {
+ h := openTestPlugin(t, nil)
+ spaceA := seedSpace(t, h.store, mmmodel.NewId())
+ spaceB := seedSpace(t, h.store, mmmodel.NewId())
+ pageInB := seedPage(t, h.store, spaceB.Id, spaceB.ChannelId, "")
+ userID := mmmodel.NewId()
+
+ rec := h.do(t, http.MethodGet, "/api/v1/spaces/"+spaceA.Id+"/pages/"+pageInB.Id+"/active-editors", userID, nil)
+ require.Equal(t, http.StatusNotFound, rec.Code, "a page must not resolve through another space's route")
+
+ rec = h.do(t, http.MethodGet, "/api/v1/spaces/"+spaceB.Id+"/pages/"+pageInB.Id+"/active-editors", userID, nil)
+ require.Equal(t, http.StatusOK, rec.Code)
+}
+
+// TestHandler_ActiveEditorsResponseBody verifies the JSON payload of the active-editors endpoint:
+// an empty list when no draft is open, and the editor's user ID when they have an active draft.
+func TestHandler_ActiveEditorsResponseBody(t *testing.T) {
+ h := openTestPlugin(t, nil)
+ space := seedSpace(t, h.store, mmmodel.NewId())
+ userID := mmmodel.NewId()
+ base := "/api/v1/spaces/" + space.Id
+
+ // Publish a new-page draft to get a live page.
+ rec := h.do(t, http.MethodPost, base+"/drafts", userID, map[string]any{"title": "Presence Test"})
+ require.Equal(t, http.StatusCreated, rec.Code)
+ var draft model.Draft
+ require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &draft))
+ pageID := draft.PageId
+
+ rec = h.do(t, http.MethodPost, base+"/pages/"+pageID+"/draft/publish", userID, nil)
+ require.Equal(t, http.StatusCreated, rec.Code)
+ var page model.Page
+ require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &page))
+
+ // No edit draft open yet — active_editors must be an empty list, not null. The response also
+ // carries snapshot_at and active_timeout_ms, mirroring the page_presence_updated WS payload so a client
+ // resyncing over REST can reason about snapshot staleness the same way.
+ rec = h.do(t, http.MethodGet, base+"/pages/"+pageID+"/active-editors", userID, nil)
+ require.Equal(t, http.StatusOK, rec.Code)
+ var resp struct {
+ ActiveEditors []string `json:"active_editors"`
+ SnapshotAt int64 `json:"snapshot_at"`
+ ActiveTimeoutMs int64 `json:"active_timeout_ms"`
+ }
+ require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &resp))
+ require.NotNil(t, resp.ActiveEditors)
+ require.Empty(t, resp.ActiveEditors)
+ require.Positive(t, resp.SnapshotAt, "response must carry the snapshot timestamp")
+ require.Equal(t, app.ActiveEditorTimeoutMs, resp.ActiveTimeoutMs, "response must carry the active-editor window")
+
+ // Open an edit draft — the user must now appear as an active editor.
+ rec = h.do(t, http.MethodPatch, base+"/pages/"+pageID+"/draft", userID, map[string]any{
+ "title": "Presence Test",
+ "base_edit_at": page.EditAt,
+ })
+ require.Equal(t, http.StatusOK, rec.Code)
+
+ rec = h.do(t, http.MethodGet, base+"/pages/"+pageID+"/active-editors", userID, nil)
+ require.Equal(t, http.StatusOK, rec.Code)
+ require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &resp))
+ require.Contains(t, resp.ActiveEditors, userID, "user with an open edit draft must appear as an active editor")
+}
+
+// TestHandler_UpdatePageDraftPropsPointerIntent covers the props pointer-intent semantics over the
+// HTTP boundary: an absent props field preserves the stored map, an explicit null behaves the same
+// as absent, an empty object clears all keys, and a populated object replaces the whole map.
+func TestHandler_UpdatePageDraftPropsPointerIntent(t *testing.T) {
+ h := openTestPlugin(t, nil)
+ space := seedSpace(t, h.store, mmmodel.NewId())
+ userID := mmmodel.NewId()
+ base := "/api/v1/spaces/" + space.Id
+
+ rec := h.do(t, http.MethodPost, base+"/drafts", userID, map[string]any{"title": "Props Test"})
+ require.Equal(t, http.StatusCreated, rec.Code)
+ var draft model.Draft
+ require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &draft))
+ draftPath := base + "/pages/" + draft.PageId + "/draft"
+
+ // Populate props.
+ rec = h.do(t, http.MethodPatch, draftPath, userID, map[string]any{
+ "title": "Props Test",
+ "props": map[string]any{"color": "red"},
+ })
+ require.Equal(t, http.StatusOK, rec.Code)
+ var populated model.Draft
+ require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &populated))
+ require.Equal(t, "red", populated.Props["color"])
+
+ // props absent must preserve the existing map. Each step below unmarshals into a fresh Draft
+ // value: reusing one across json.Unmarshal calls would merge into the existing non-nil map
+ // instead of replacing it, masking whether the server actually cleared/replaced the props.
+ rec = h.do(t, http.MethodPatch, draftPath, userID, map[string]any{
+ "title": "Props Test Updated",
+ })
+ require.Equal(t, http.StatusOK, rec.Code)
+ var afterAbsent model.Draft
+ require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &afterAbsent))
+ require.Equal(t, "red", afterAbsent.Props["color"], "omitting props must preserve the existing map")
+
+ // props: null must also preserve the existing map — it unmarshals to a nil pointer, same as
+ // omitting the field entirely.
+ rec = h.do(t, http.MethodPatch, draftPath, userID, map[string]any{
+ "props": nil,
+ })
+ require.Equal(t, http.StatusOK, rec.Code)
+ var afterNull model.Draft
+ require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &afterNull))
+ require.Equal(t, "red", afterNull.Props["color"], "props: null must preserve the existing map, same as omitting it")
+
+ // props: {} must clear all keys.
+ rec = h.do(t, http.MethodPatch, draftPath, userID, map[string]any{
+ "props": map[string]any{},
+ })
+ require.Equal(t, http.StatusOK, rec.Code)
+ var afterClear model.Draft
+ require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &afterClear))
+ require.Empty(t, afterClear.Props, "props: {} must clear all keys")
+
+ // props populated again must replace the (now-empty) map.
+ rec = h.do(t, http.MethodPatch, draftPath, userID, map[string]any{
+ "props": map[string]any{"size": "large"},
+ })
+ require.Equal(t, http.StatusOK, rec.Code)
+ var afterReplace model.Draft
+ require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &afterReplace))
+ require.Equal(t, "large", afterReplace.Props["size"])
+ require.NotContains(t, afterReplace.Props, "color", "props replace must drop keys not present in the new map")
+}
+
+// TestHandler_UpdatePageDraftPropsBaselineNoLongerHonored covers the removal of the old
+// props.original_page_edit_at baseline convention: placing the value only under that props key must
+// NOT establish an optimistic-lock baseline. Since a baseline-less edit-open against a live page is
+// itself rejected (the store refuses to create a baseline-less edit draft for an existing page — see
+// TestPublishRejectsNoBaselineOnExistingPageEdit in server/app/page_draft_test.go), the PATCH must
+// fail with the same conflict the client would get if it had supplied no baseline at all.
+func TestHandler_UpdatePageDraftPropsBaselineNoLongerHonored(t *testing.T) {
+ h := openTestPlugin(t, nil)
+ channelID := mmmodel.NewId()
+ space := seedSpace(t, h.store, channelID)
+ userID := mmmodel.NewId()
+ base := "/api/v1/spaces/" + space.Id
+
+ rec := h.do(t, http.MethodPost, base+"/drafts", userID, map[string]any{"title": "Doc"})
+ require.Equal(t, http.StatusCreated, rec.Code)
+ var draft model.Draft
+ require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &draft))
+ pageID := draft.PageId
+
+ rec = h.do(t, http.MethodPost, base+"/pages/"+pageID+"/draft/publish", userID, nil)
+ require.Equal(t, http.StatusCreated, rec.Code)
+ var page model.Page
+ require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &page))
+
+ // Open an edit session but put the baseline only under the old props key, not the top-level
+ // base_edit_at field. This must be rejected with 409, exactly as if base_edit_at had been
+ // omitted entirely (proving the props key carries no special meaning any more).
+ rec = h.do(t, http.MethodPatch, base+"/pages/"+pageID+"/draft", userID, map[string]any{
+ "title": "Edited",
+ "props": map[string]any{"original_page_edit_at": page.EditAt},
+ })
+ require.Equal(t, http.StatusConflict, rec.Code, "props.original_page_edit_at must not be honored as a baseline")
+}
+
+// TestHandler_PublishForceOverHTTP exercises the {"force": true} publish body through the real
+// router: a stale-baseline publish that 409s without it succeeds when the request carries force.
+func TestHandler_PublishForceOverHTTP(t *testing.T) {
+ h := openTestPlugin(t, nil)
+ space := seedSpace(t, h.store, mmmodel.NewId())
+ userA := mmmodel.NewId()
+ userB := mmmodel.NewId()
+ base := "/api/v1/spaces/" + space.Id
+
+ // Create a new-page draft and publish it to get a live page.
+ rec := h.do(t, http.MethodPost, base+"/drafts", userA, map[string]any{"title": "Shared Doc"})
+ require.Equal(t, http.StatusCreated, rec.Code)
+ var draft model.Draft
+ require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &draft))
+ pageID := draft.PageId
+
+ rec = h.do(t, http.MethodPost, base+"/pages/"+pageID+"/draft/publish", userA, nil)
+ require.Equal(t, http.StatusCreated, rec.Code)
+ var page model.Page
+ require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &page))
+ editAt := page.EditAt
+
+ // Both users open edit sessions at the same baseline; B publishes first, staling A's baseline.
+ rec = h.do(t, http.MethodPatch, base+"/pages/"+pageID+"/draft", userA, map[string]any{
+ "title": "Edit by A",
+ "base_edit_at": editAt,
+ })
+ require.Equal(t, http.StatusOK, rec.Code)
+ rec = h.do(t, http.MethodPatch, base+"/pages/"+pageID+"/draft", userB, map[string]any{
+ "title": "Edit by B",
+ "base_edit_at": editAt,
+ })
+ require.Equal(t, http.StatusOK, rec.Code)
+ rec = h.do(t, http.MethodPost, base+"/pages/"+pageID+"/draft/publish", userB, nil)
+ require.Equal(t, http.StatusOK, rec.Code)
+
+ // Without force the stale baseline 409s; with {"force": true} in the HTTP body it publishes.
+ rec = h.do(t, http.MethodPost, base+"/pages/"+pageID+"/draft/publish", userA, nil)
+ require.Equal(t, http.StatusConflict, rec.Code)
+ rec = h.do(t, http.MethodPost, base+"/pages/"+pageID+"/draft/publish", userA, map[string]any{"force": true})
+ require.Equal(t, http.StatusOK, rec.Code)
+ var published model.Page
+ require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &published))
+ require.Equal(t, "Edit by A", published.Title)
+}
diff --git a/server/api_page_presence.go b/server/api_page_presence.go
new file mode 100644
index 0000000..a04c050
--- /dev/null
+++ b/server/api_page_presence.go
@@ -0,0 +1,32 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See LICENSE.txt for license information.
+
+package main
+
+import (
+ "net/http"
+
+ "github.com/gorilla/mux"
+)
+
+// handleGetPageActiveEditors handles
+// GET /api/v1/spaces/{space_id}/pages/{page_id}/active-editors
+// Returns the active-editors snapshot for the page (active_editors, snapshot_at, active_timeout_ms).
+func (p *Plugin) handleGetPageActiveEditors(w http.ResponseWriter, r *http.Request) {
+ vars := mux.Vars(r)
+ spaceID := vars["space_id"]
+ pageID := vars["page_id"]
+ userID := userIDFromRequest(r)
+
+ if _, ok := p.requireSpaceMembership(w, spaceID, userID, false); !ok {
+ return
+ }
+
+ snapshot, appErr := p.service.GetPageActiveEditors(pageID, spaceID)
+ if appErr != nil {
+ p.writeAppError(w, appErr)
+ return
+ }
+
+ writeJSON(w, http.StatusOK, snapshot)
+}
diff --git a/server/app/page.go b/server/app/page.go
index c4173d2..77c703d 100644
--- a/server/app/page.go
+++ b/server/app/page.go
@@ -14,13 +14,9 @@ import (
"github.com/mattermost/mattermost-plugin-docs/server/store"
)
-// MaxPageDepth is the page hierarchy depth limit (root is depth 1).
-// store.MaxPageHierarchyDepth (50) is a separate, larger bound used by descendant/ancestor reads.
-const MaxPageDepth = 10
-
// CreatePage creates a new page in spaceID. ChannelId is derived from the space, not supplied by the caller.
// The page ID is always server-generated; callers must not supply one.
-func (s *Service) CreatePage(spaceID, parentID, title, body, searchText, userID string) (*model.Page, *mmmodel.AppError) {
+func (s *Service) CreatePage(spaceID, parentID, title, body, userID string) (*model.Page, *mmmodel.AppError) {
if !mmmodel.IsValidId(spaceID) {
return nil, mmmodel.NewAppError("CreatePage", "app.page.create.invalid_space_id.app_error", nil, "", http.StatusBadRequest)
}
@@ -31,10 +27,11 @@ func (s *Service) CreatePage(spaceID, parentID, title, body, searchText, userID
if titleErr != nil {
return nil, titleErr
}
- // SearchText is the body's plain-text projection, so it makes no sense without a body
- // (matches the update path's rule).
- if searchText != "" && body == "" {
- return nil, mmmodel.NewAppError("CreatePage", "app.page.create.search_text_without_content.app_error", nil, "", http.StatusBadRequest)
+ // Validate and normalize the TipTap body and derive SearchText from it. SearchText is the body's
+ // server-derived projection, so it is never taken from the caller.
+ normBody, normSearch, contentErr := normalizePageContent("CreatePage", body)
+ if contentErr != nil {
+ return nil, contentErr
}
// Space existence and liveness are validated by store.CreatePage itself (surfaced as
@@ -49,21 +46,20 @@ func (s *Service) CreatePage(spaceID, parentID, title, body, searchText, userID
}
}
- // Body is stored as-is (TipTap validation/normalization and SearchText deferred).
page := &model.Page{
SpaceId: spaceID,
ParentId: parentID,
Type: model.PageTypePage,
Title: title,
- Body: body,
- SearchText: searchText,
+ Body: normBody,
+ SearchText: normSearch,
UserId: userID,
LastModifiedBy: userID,
}
s.log.Debug("Creating page", "space_id", spaceID, "parent_id", parentID, "user_id", userID)
- created, storeErr := s.store.CreatePage(page, MaxPageDepth)
+ created, storeErr := s.store.CreatePage(page, model.MaxPageDepth)
if storeErr != nil {
if store.IsErrNotFound(storeErr) {
// The space is missing or soft-deleted.
@@ -99,6 +95,8 @@ func (s *Service) GetPage(pageID string) (*model.Page, *mmmodel.AppError) {
// UpdatePage patches a page, optimistic-locked on baseEditAt; a nil baseEditAt without force is
// rejected. spaceID scopes the write: a page moved to another space since the caller's last
// check returns not-found instead of updating the wrong copy.
+// On a 409 the returned page is the current server page (nil if the re-read failed), so the caller
+// can re-baseline without a follow-up read; on every other error it is nil.
func (s *Service) UpdatePage(pageID, spaceID string, patch *model.PagePatch, baseEditAt *int64, force bool, userID string) (*model.Page, *mmmodel.AppError) {
if !mmmodel.IsValidId(pageID) {
return nil, mmmodel.NewAppError("UpdatePage", "app.page.update.invalid_id.app_error", nil, "", http.StatusBadRequest)
@@ -112,6 +110,11 @@ func (s *Service) UpdatePage(pageID, spaceID string, patch *model.PagePatch, bas
if appErr := requireBaseline("UpdatePage", "base_edit_at", baseEditAt, force); appErr != nil {
return nil, appErr
}
+ // Validate/normalize the TipTap body (and recompute SearchText) before patch validation, so a
+ // body-only patch is valid and a direct edit is sanitized on the same content path as publish.
+ if contentErr := normalizePatchContent("UpdatePage", patch); contentErr != nil {
+ return nil, contentErr
+ }
if validErr := normalizeAndValidatePagePatch("UpdatePage", patch); validErr != nil {
return nil, validErr
}
@@ -137,7 +140,9 @@ func (s *Service) UpdatePage(pageID, spaceID string, patch *model.PagePatch, bas
return nil, mmmodel.NewAppError("UpdatePage", "app.page.update.conflict.app_error",
nil, "conflict", http.StatusConflict).Wrap(storeErr)
}
- return nil, mmmodel.NewAppError("UpdatePage", "app.page.update.conflict.app_error",
+ // The page travels back alongside the conflict so the caller can re-baseline without a
+ // follow-up read, matching PublishPageDraft's edit-conflict contract.
+ return fresh, mmmodel.NewAppError("UpdatePage", "app.page.update.conflict.app_error",
map[string]any{"ModifiedBy": fresh.LastModifiedBy, "ModifiedAt": fresh.EditAt},
"conflict", http.StatusConflict).Wrap(storeErr)
}
@@ -197,7 +202,7 @@ func (s *Service) RestorePage(pageID, spaceID, userID string) (*model.Page, *mmm
return nil, mmmodel.NewAppError("RestorePage", "app.page.restore.invalid_user_id.app_error", nil, "", http.StatusBadRequest)
}
s.log.Debug("Restoring page", "page_id", pageID, "user_id", userID)
- restored, restoreErr := s.store.RestorePage(pageID, spaceID, userID, MaxPageDepth)
+ restored, restoreErr := s.store.RestorePage(pageID, spaceID, userID, model.MaxPageDepth)
if restoreErr != nil {
if appErr := restoreReasonAppError(restoreErr, map[string]*mmmodel.AppError{
store.ReasonNotRestorable: mmmodel.NewAppError("RestorePage", "app.page.restore.not_restorable.app_error", nil, "", http.StatusBadRequest),
@@ -268,7 +273,7 @@ func (s *Service) DuplicatePage(pageID string, sourceSpace *model.Space, userID
s.log.Debug("Duplicating page", "page_id", pageID, "source_space_id", sourceSpace.Id, "user_id", userID)
- created, createErr := s.store.CreatePageSubtree(pages, MaxPageDepth)
+ created, createErr := s.store.CreatePageSubtree(pages, model.MaxPageDepth)
if createErr != nil {
if store.IsErrNotFound(createErr) {
return nil, mmmodel.NewAppError("DuplicatePage", "app.page.duplicate.dest_not_found.app_error", nil, "", http.StatusNotFound).Wrap(createErr)
@@ -277,7 +282,7 @@ func (s *Service) DuplicatePage(pageID string, sourceSpace *model.Space, userID
// plain placement-depth breach uses storeAppError's operation-neutral key.
var limErr *store.ErrLimitExceeded
if errors.As(createErr, &limErr) && limErr.Reason == store.ReasonSubtreeMaxDepthExceeded {
- return nil, mmmodel.NewAppError("DuplicatePage", "app.page.duplicate.subtree_max_depth_exceeded.app_error", map[string]any{"MaxDepth": MaxPageDepth}, "", http.StatusBadRequest).Wrap(createErr)
+ return nil, mmmodel.NewAppError("DuplicatePage", "app.page.duplicate.subtree_max_depth_exceeded.app_error", map[string]any{"MaxDepth": model.MaxPageDepth}, "", http.StatusBadRequest).Wrap(createErr)
}
return nil, storeAppError("DuplicatePage", createErr)
}
@@ -299,37 +304,30 @@ func buildDuplicatePages(source *model.Page, descendants []*model.Page, destSpac
rootID := mmmodel.NewId()
idMap := map[string]string{source.Id: rootID}
pages := make([]*model.Page, 0, 1+len(descendants))
- pages = append(pages, &model.Page{
- Id: rootID,
- SpaceId: destSpaceID,
- ParentId: destParentID,
- Type: source.Type,
- Title: copyTitle(source.Title),
- Body: source.Body,
- SearchText: source.SearchText,
- Props: maps.Clone(source.Props),
- UserId: userID,
- LastModifiedBy: userID,
- })
+ pages = append(pages, clonePageFields(source, rootID, destSpaceID, destParentID, copyTitle(source.Title), userID))
for _, d := range descendants {
newID := mmmodel.NewId()
- pages = append(pages, &model.Page{
- Id: newID,
- SpaceId: destSpaceID,
- ParentId: idMap[d.ParentId],
- Type: d.Type,
- Title: d.Title,
- Body: d.Body,
- SearchText: d.SearchText,
- Props: maps.Clone(d.Props),
- UserId: userID,
- LastModifiedBy: userID,
- })
+ pages = append(pages, clonePageFields(d, newID, destSpaceID, idMap[d.ParentId], d.Title, userID))
idMap[d.Id] = newID
}
return pages
}
+func clonePageFields(src *model.Page, id, spaceID, parentID, title, userID string) *model.Page {
+ return &model.Page{
+ Id: id,
+ SpaceId: spaceID,
+ ParentId: parentID,
+ Type: src.Type,
+ Title: title,
+ Body: src.Body,
+ SearchText: src.SearchText,
+ Props: maps.Clone(src.Props),
+ UserId: userID,
+ LastModifiedBy: userID,
+ }
+}
+
// copyTitle prefixes "Copy of " and truncates to the page-title cap so the duplicate's title
// always passes CreatePage validation.
func copyTitle(original string) string {
diff --git a/server/app/page_content.go b/server/app/page_content.go
new file mode 100644
index 0000000..56f4ca6
--- /dev/null
+++ b/server/app/page_content.go
@@ -0,0 +1,160 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See LICENSE.txt for license information.
+
+package app
+
+import (
+ "encoding/json"
+ "net/http"
+ "strings"
+ "unicode"
+
+ mmmodel "github.com/mattermost/mattermost/server/public/model"
+ "github.com/pkg/errors"
+
+ "github.com/mattermost/mattermost-plugin-docs/server/model"
+)
+
+// normalizePageContent normalizes a page body — validating it as TipTap content and deriving
+// SearchText from it. Returns a 400 AppError when the body is not valid TipTap content.
+//
+// An empty body ("") is returned as-is, representing "no content"; the draft path instead seeds new
+// pages with model.EmptyTipTapJSON (a rendered-empty document). These are two DISTINCT empty
+// representations, and consumers may assign them different meaning: the publish path treats "" as
+// "field not sent, preserve the existing page body" and EmptyTipTapJSON as "explicitly cleared".
+func normalizePageContent(where, body string) (normBody, searchText string, appErr *mmmodel.AppError) {
+ normBody, doc, empty, err := normalizeContent(body)
+ if err != nil {
+ return "", "", wrapContentError(where, err)
+ }
+ if !empty {
+ searchText = model.BuildSearchText(doc)
+ }
+ return normBody, searchText, nil
+}
+
+// wrapContentError renders a content validation/normalization failure as the shared invalid-content
+// AppError, so the error key and status stay defined in one place across content callers.
+func wrapContentError(where string, err error) *mmmodel.AppError {
+ return mmmodel.NewAppError(where, "app.page.invalid_content.app_error", nil, "", http.StatusBadRequest).Wrap(err)
+}
+
+// normalizePatchContent normalizes a page patch's Body in place when present, recomputing
+// SearchText from it (SearchText is the body's server-derived projection). A patch that sets Body
+// has its caller-supplied SearchText overwritten by the derived value; a patch that does not touch
+// Body is left unchanged (a SearchText-without-Body patch is rejected later by PagePatch.IsValid). A
+// nil patch is a no-op, matching PagePatch's own contract.
+func normalizePatchContent(where string, patch *model.PagePatch) *mmmodel.AppError {
+ if patch == nil || patch.Body == nil {
+ return nil
+ }
+ normBody, searchText, appErr := normalizePageContent(where, *patch.Body)
+ if appErr != nil {
+ return appErr
+ }
+ patch.Body = &normBody
+ patch.SearchText = &searchText
+ return nil
+}
+
+// normalizeContentBody normalizes a body without deriving SearchText, for callers (draft autosave)
+// that store only the body. It shares normalizeContent with normalizePageContent but discards the
+// parsed doc.
+func normalizeContentBody(where, body string) (string, *mmmodel.AppError) {
+ normBody, _, _, err := normalizeContent(body)
+ if err != nil {
+ return "", wrapContentError(where, err)
+ }
+ return normBody, nil
+}
+
+// normalizeContent normalizes TipTap/plain-text page content to its stored body form, returning the
+// parsed doc and empty flag so callers decide whether to derive SearchText (normalizePageContent
+// does; normalizeContentBody skips it). Returns a raw error; callers wrap it via wrapContentError.
+// An empty content string ("") is returned as-is (no-op), with a zero-value doc.
+func normalizeContent(content string) (normBody string, doc model.TipTapDocument, empty bool, err error) {
+ doc, empty, err = normalizeContentToDoc(content)
+ if err != nil {
+ return "", model.TipTapDocument{}, false, err
+ }
+ if empty {
+ return content, doc, true, nil
+ }
+ normBody, err = marshalTipTapDoc(doc)
+ if err != nil {
+ return "", model.TipTapDocument{}, false, err
+ }
+ return normBody, doc, false, nil
+}
+
+// normalizeContentToDoc normalizes content and returns its TipTap document, validating it and
+// rejecting invalid TipTap. empty is true for an empty content string ("") — a no-op the caller
+// returns as-is.
+func normalizeContentToDoc(content string) (doc model.TipTapDocument, empty bool, err error) {
+ if content == "" {
+ return model.TipTapDocument{}, true, nil
+ }
+ // Reject over-limit content up front for both input forms: ParseTipTapDocument enforces the
+ // same cap only on the JSON path.
+ if len(content) > model.PageBodyMaxBytes {
+ return model.TipTapDocument{}, false, errors.New("content exceeds the maximum body size")
+ }
+ // Treat the body as TipTap only when it is actually valid JSON: a plain-text body that merely
+ // starts with "{" (e.g. "{shrug}") is not JSON and must be wrapped, not rejected. A body that is
+ // valid JSON but not a "doc" is a genuine content error and ParseTipTapDocument rejects it.
+ idx := strings.IndexFunc(content, func(r rune) bool { return !unicode.IsSpace(r) })
+ if idx >= 0 && content[idx] == '{' {
+ parsed, parseErr := model.ParseTipTapDocument(content)
+ if parseErr == nil {
+ return parsed, false, nil
+ }
+ var syntaxErr *json.SyntaxError
+ if !errors.As(parseErr, &syntaxErr) {
+ return model.TipTapDocument{}, false, parseErr
+ }
+ // SyntaxError → not valid JSON → fall through to plain-text wrapping.
+ }
+ // Non-JSON content: wrap in a minimal TipTap doc.
+ wrapped, err := convertPlainTextToTipTap(content)
+ if err != nil {
+ return model.TipTapDocument{}, false, err
+ }
+ return wrapped, false, nil
+}
+
+// marshalTipTapDoc serializes a TipTapDocument to its stored JSON form.
+func marshalTipTapDoc(doc model.TipTapDocument) (string, error) {
+ sanitized, err := json.Marshal(doc)
+ if err != nil {
+ return "", err
+ }
+ return string(sanitized), nil
+}
+
+// maxPlainTextParagraphs caps the number of paragraph nodes produced when converting plain text
+// to a TipTap document. convertPlainTextToTipTap splits on newlines and turns each line into its
+// own paragraph node (a map[string]any), so a newline-only body at PageBodyMaxBytes would
+// otherwise build ~2M such maps before the post-normalization body-size check could reject it.
+const maxPlainTextParagraphs = 10_000
+
+// convertPlainTextToTipTap wraps plain text in a minimal TipTap document. Returns an error when
+// the input has more than maxPlainTextParagraphs newlines so that over-limit bodies are rejected
+// with a clear error rather than silently truncated.
+func convertPlainTextToTipTap(plainText string) (model.TipTapDocument, error) {
+ paragraphs := strings.SplitN(plainText, "\n", maxPlainTextParagraphs+1)
+ if len(paragraphs) > maxPlainTextParagraphs {
+ return model.TipTapDocument{}, errors.New("plain text body has too many paragraphs")
+ }
+ nodes := make([]map[string]any, 0, len(paragraphs))
+ for _, para := range paragraphs {
+ node := map[string]any{"type": "paragraph"}
+ if strings.TrimSpace(para) != "" {
+ node["content"] = []any{map[string]any{"type": "text", "text": para}}
+ }
+ nodes = append(nodes, node)
+ }
+ return model.TipTapDocument{
+ Type: model.TipTapDocType,
+ Content: nodes,
+ }, nil
+}
diff --git a/server/app/page_content_test.go b/server/app/page_content_test.go
new file mode 100644
index 0000000..6c8da30
--- /dev/null
+++ b/server/app/page_content_test.go
@@ -0,0 +1,168 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See LICENSE.txt for license information.
+
+package app
+
+import (
+ "net/http"
+ "strings"
+ "testing"
+
+ mmmodel "github.com/mattermost/mattermost/server/public/model"
+ "github.com/stretchr/testify/require"
+
+ "github.com/mattermost/mattermost-plugin-docs/server/model"
+)
+
+func TestNormalizePageContent(t *testing.T) {
+ t.Run("empty content is a no-op", func(t *testing.T) {
+ body, search, appErr := normalizePageContent("Test", "")
+ require.Nil(t, appErr)
+ require.Equal(t, "", body)
+ require.Equal(t, "", search)
+ })
+
+ t.Run("TipTap JSON is normalized and search text derived", func(t *testing.T) {
+ body, search, appErr := normalizePageContent("Test", `{"type":"doc","content":[{"type":"paragraph","content":[{"type":"text","text":"hello world"}]}]}`)
+ require.Nil(t, appErr)
+ require.Contains(t, body, "hello world")
+ require.Equal(t, "hello world", search)
+ })
+
+ t.Run("plain text is wrapped into a TipTap doc", func(t *testing.T) {
+ body, search, appErr := normalizePageContent("Test", "just text")
+ require.Nil(t, appErr)
+ require.True(t, strings.HasPrefix(body, `{"type":"doc"`), "plain text should be wrapped: %s", body)
+ require.Contains(t, body, "just text")
+ require.Equal(t, "just text", search)
+ })
+
+ t.Run("malformed TipTap JSON is rejected", func(t *testing.T) {
+ _, _, appErr := normalizePageContent("Test", `{"type":"bogus"}`)
+ require.NotNil(t, appErr)
+ })
+
+ t.Run("javascript URL is stripped on normalization", func(t *testing.T) {
+ body, _, appErr := normalizePageContent("Test", `{"type":"doc","content":[{"type":"text","text":"x","marks":[{"type":"link","attrs":{"href":"javascript:alert(1)"}}]}]}`)
+ require.Nil(t, appErr)
+ require.NotContains(t, body, "javascript:alert")
+ })
+
+ t.Run("string starting with { but not valid JSON is wrapped as plain text", func(t *testing.T) {
+ body, search, appErr := normalizePageContent("Test", "{shrug}")
+ require.Nil(t, appErr)
+ require.True(t, strings.HasPrefix(body, `{"type":"doc"`), "brace-leading non-JSON must be wrapped as plain text: %s", body)
+ require.Equal(t, "{shrug}", search)
+ })
+
+ t.Run("multiline plain text becomes multiple paragraphs", func(t *testing.T) {
+ body, _, appErr := normalizePageContent("Test", "line one\nline two")
+ require.Nil(t, appErr)
+ require.Contains(t, body, "line one")
+ require.Contains(t, body, "line two")
+ })
+
+ t.Run("plain text preserves leading whitespace within lines", func(t *testing.T) {
+ body, _, appErr := normalizePageContent("Test", " indented line")
+ require.Nil(t, appErr)
+ require.Contains(t, body, `" indented line"`, "leading spaces must not be stripped from paragraph text")
+ })
+}
+
+// TestNormalizePageContentRejectsTooManyParagraphs verifies that a plain-text body exceeding
+// maxPlainTextParagraphs newlines is rejected with 400, rather than producing an oversized TipTap
+// document.
+func TestNormalizePageContentRejectsTooManyParagraphs(t *testing.T) {
+ body := strings.Repeat("x\n", maxPlainTextParagraphs+1)
+
+ _, _, appErr := normalizePageContent("test", body)
+ require.NotNil(t, appErr)
+ require.Equal(t, http.StatusBadRequest, appErr.StatusCode)
+ require.Contains(t, appErr.Error(), "too many paragraphs")
+}
+
+func TestNormalizePatchContent(t *testing.T) {
+ t.Run("nil patch is a no-op", func(t *testing.T) {
+ require.Nil(t, normalizePatchContent("test", nil))
+ })
+
+ t.Run("patch with nil Body is a no-op", func(t *testing.T) {
+ patch := &model.PagePatch{Title: mmmodel.NewPointer("title")}
+ require.Nil(t, normalizePatchContent("test", patch))
+ require.Nil(t, patch.SearchText, "SearchText must remain nil when Body is unset")
+ })
+
+ t.Run("patch with valid Body normalizes and derives SearchText", func(t *testing.T) {
+ body := `{"type":"doc","content":[{"type":"paragraph","content":[{"type":"text","text":"hello"}]}]}`
+ patch := &model.PagePatch{Body: mmmodel.NewPointer(body)}
+ require.Nil(t, normalizePatchContent("test", patch))
+ require.NotNil(t, patch.SearchText)
+ require.Equal(t, "hello", *patch.SearchText)
+ })
+
+ t.Run("patch with invalid Body returns 400 AppError", func(t *testing.T) {
+ patch := &model.PagePatch{Body: mmmodel.NewPointer(`{"type":"bogus"}`)}
+ appErr := normalizePatchContent("test", patch)
+ require.NotNil(t, appErr)
+ require.Equal(t, 400, appErr.StatusCode)
+ })
+
+ t.Run("patch with Body strips dangerous URLs", func(t *testing.T) {
+ body := `{"type":"doc","content":[{"type":"text","text":"x","marks":[{"type":"link","attrs":{"href":"javascript:alert(1)"}}]}]}`
+ patch := &model.PagePatch{Body: mmmodel.NewPointer(body)}
+ require.Nil(t, normalizePatchContent("test", patch))
+ require.NotContains(t, *patch.Body, "javascript:alert")
+ })
+}
+
+// TestNormalizeContentBody covers the draft autosave entry point, the only one that sanitizes a body
+// without deriving SearchText — publish derives it later from the same content.
+func TestNormalizeContentBody(t *testing.T) {
+ t.Run("an empty body is left alone", func(t *testing.T) {
+ body, appErr := normalizeContentBody("test", "")
+ require.Nil(t, appErr)
+ require.Empty(t, body)
+ })
+
+ t.Run("valid TipTap content is normalized", func(t *testing.T) {
+ body, appErr := normalizeContentBody("test",
+ `{"type":"doc","content":[{"type":"paragraph","content":[{"type":"text","text":"hello"}]}]}`)
+ require.Nil(t, appErr)
+ require.Contains(t, body, "hello")
+ })
+
+ t.Run("dangerous URLs are stripped", func(t *testing.T) {
+ body, appErr := normalizeContentBody("test",
+ `{"type":"doc","content":[{"type":"text","text":"x","marks":[{"type":"link","attrs":{"href":"javascript:alert(1)"}}]}]}`)
+ require.Nil(t, appErr)
+ require.NotContains(t, body, "javascript:alert")
+ })
+
+ t.Run("an unlisted node type is rejected", func(t *testing.T) {
+ _, appErr := normalizeContentBody("test", `{"type":"doc","content":[{"type":"script"}]}`)
+ require.NotNil(t, appErr)
+ require.Equal(t, http.StatusBadRequest, appErr.StatusCode)
+ })
+}
+
+func TestNormalizeContentRejectsOversizedBodyBeforeParsing(t *testing.T) {
+ // The size gate must run before json.Unmarshal, so the stored-body limit — not the larger
+ // request-transport cap — bounds the parse allocation.
+ oversized := `{"type":"doc","content":[{"type":"paragraph","content":[{"type":"text","text":"` +
+ strings.Repeat("a", model.PageBodyMaxBytes) + `"}]}]}`
+ _, _, appErr := normalizePageContent("TestNormalizeContent", oversized)
+ require.NotNil(t, appErr)
+ require.Equal(t, http.StatusBadRequest, appErr.StatusCode)
+}
+
+// TestNormalizePageContentAcceptsMaxParagraphs pins the paragraph cap's boundary: a body producing
+// exactly maxPlainTextParagraphs paragraphs is accepted — the cap rejects strictly more than the
+// documented limit, not "the limit or more".
+func TestNormalizePageContentAcceptsMaxParagraphs(t *testing.T) {
+ // "x\n" repeated N-1 times splits into N-1 "x" paragraphs plus one trailing empty paragraph:
+ // exactly maxPlainTextParagraphs.
+ body := strings.Repeat("x\n", maxPlainTextParagraphs-1)
+
+ _, _, appErr := normalizePageContent("test", body)
+ require.Nil(t, appErr)
+}
diff --git a/server/app/page_draft.go b/server/app/page_draft.go
new file mode 100644
index 0000000..c963d3b
--- /dev/null
+++ b/server/app/page_draft.go
@@ -0,0 +1,606 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See LICENSE.txt for license information.
+
+package app
+
+import (
+ "errors"
+ "maps"
+ "net/http"
+
+ mmmodel "github.com/mattermost/mattermost/server/public/model"
+
+ "github.com/mattermost/mattermost-plugin-docs/server/model"
+ "github.com/mattermost/mattermost-plugin-docs/server/store"
+)
+
+// UpdatePageDraft upserts the calling user's autosave draft for a page in a space. channelID is
+// the space's backing channel, used to scope the presence broadcast.
+//
+// draft.PageId is the unified page id, allocated up front and stable across the draft → publish
+// lifecycle. It is reserved before any published page row exists, so carrying a valid page id does
+// not imply the page is published — a draft may exist first. The space must exist and be live. The
+// caller owns the draft: draft.UserId is always sourced from the request, never the request body.
+//
+// An autosave may omit fields the editor didn't change, and an omitted field keeps its stored
+// value. parentID, fileIDs, and props signal omission with a nil pointer; the draft struct's own
+// fields signal it by being empty.
+func (s *Service) UpdatePageDraft(draft *model.Draft, parentID *string, fileIDs *mmmodel.StringArray, props *mmmodel.StringInterface, channelID string) (*model.Draft, *mmmodel.AppError) {
+ if draft == nil {
+ return nil, mmmodel.NewAppError("UpdatePageDraft", "app.page_draft.update.nil_draft.app_error", nil, "", http.StatusBadRequest)
+ }
+ if !mmmodel.IsValidId(draft.UserId) {
+ return nil, mmmodel.NewAppError("UpdatePageDraft", "app.page_draft.update.invalid_user_id.app_error", nil, "", http.StatusBadRequest)
+ }
+ if !mmmodel.IsValidId(draft.SpaceId) {
+ return nil, mmmodel.NewAppError("UpdatePageDraft", "app.page_draft.update.invalid_space_id.app_error", nil, "", http.StatusBadRequest)
+ }
+ if !mmmodel.IsValidId(draft.PageId) {
+ return nil, mmmodel.NewAppError("UpdatePageDraft", "app.page_draft.update.invalid_page_id.app_error", nil, "", http.StatusBadRequest)
+ }
+ if parentID != nil && *parentID != "" && !mmmodel.IsValidId(*parentID) {
+ return nil, mmmodel.NewAppError("UpdatePageDraft", "app.page_draft.update.invalid_parent_id.app_error", nil, "", http.StatusBadRequest)
+ }
+ if draft.Title != "" {
+ title, titleErr := validateTitle("UpdatePageDraft", draft.Title, model.PageTitleMaxRunes)
+ if titleErr != nil {
+ return nil, titleErr
+ }
+ draft.Title = title
+ }
+
+ // Normalize the draft body on the same content path as publish, so a stored draft never holds
+ // unsanitized markup (normalization parses the body through the TipTap sanitizer). Defense-in-depth:
+ // only the author can read a draft back today, but any future reader of Draft.Body inherits a
+ // sanitized value.
+ if draft.Body != "" {
+ normalizedBody, contentErr := normalizeContentBody("UpdatePageDraft", draft.Body)
+ if contentErr != nil {
+ return nil, contentErr
+ }
+ draft.Body = normalizedBody
+ }
+
+ // AutosaveDraft enforces the autosave-path guards itself (see its godoc), so no separate
+ // pre-check reads are needed here.
+ saved, savedPageWasLive, err := s.store.AutosaveDraft(draft, parentID, fileIDs, props)
+ if err != nil {
+ switch store.ConflictReason(err) {
+ case store.ReasonConcurrentEdit:
+ return nil, mmmodel.NewAppError("UpdatePageDraft", "app.page_draft.update.edit_conflict.app_error",
+ nil, "", http.StatusConflict).Wrap(err)
+ case store.ReasonConcurrentAutosave:
+ return nil, mmmodel.NewAppError("UpdatePageDraft", "app.page_draft.update.draft_changed.app_error",
+ nil, "", http.StatusConflict).Wrap(err)
+ }
+ if store.InvalidInputReason(err) == store.ReasonPageNotLive {
+ // The page is not addressable in this space — deleted, snapshotted, moved away, held as
+ // a draft in another space, or never reserved. All of these read as "no such page here",
+ // so report 404 rather than a generic 400.
+ return nil, mmmodel.NewAppError("UpdatePageDraft", "app.page_draft.update.page_not_found.app_error", nil, "", http.StatusNotFound).Wrap(err)
+ }
+ return nil, storeAppError("UpdatePageDraft", err)
+ }
+
+ // AutosaveDraft determined page liveness as part of the same call, so trust its result here.
+ s.maybeBroadcastDraftPresence(savedPageWasLive, saved.PageId, saved.UserId, saved.SpaceId, channelID)
+
+ return saved, nil
+}
+
+// CreateSpaceDraft creates a new-page draft with a server-generated, reserved page id, so a new
+// page has a stable link before it is published. title is required; pageParentID, when set, must be
+// a published page in the space or an existing draft of the caller.
+// The per-user-per-space draft quota is enforced atomically inside the store.
+func (s *Service) CreateSpaceDraft(userID, spaceID, title, pageParentID string) (*model.Draft, *mmmodel.AppError) {
+ if !mmmodel.IsValidId(userID) {
+ return nil, mmmodel.NewAppError("CreateSpaceDraft", "app.page_draft.create.invalid_user_id.app_error", nil, "", http.StatusBadRequest)
+ }
+ if !mmmodel.IsValidId(spaceID) {
+ return nil, mmmodel.NewAppError("CreateSpaceDraft", "app.page_draft.create.invalid_space_id.app_error", nil, "", http.StatusBadRequest)
+ }
+ s.log.Debug("Creating space draft", "space_id", spaceID, "parent_id", pageParentID, "user_id", userID)
+
+ title, titleErr := validateTitle("CreateSpaceDraft", title, model.PageTitleMaxRunes)
+ if titleErr != nil {
+ return nil, titleErr
+ }
+
+ if pageParentID != "" {
+ if parentErr := s.validateDraftParent(userID, spaceID, pageParentID); parentErr != nil {
+ return nil, parentErr
+ }
+ }
+
+ draft := &model.Draft{
+ UserId: userID,
+ SpaceId: spaceID,
+ PageId: mmmodel.NewId(),
+ Title: title,
+ Body: model.EmptyTipTapJSON,
+ }
+ var parentPtr *string
+ if pageParentID != "" {
+ parentPtr = &pageParentID
+ }
+
+ // Use UpsertDraft directly: the page row does not exist yet (new-page draft), so
+ // UpdatePageDraft's guard — which rejects drafts for non-existent pages on the autosave
+ // path — would incorrectly block this call. The page is never live here, so the liveness
+ // flag is discarded.
+ saved, _, err := s.store.UpsertDraft(draft, parentPtr, nil, nil)
+ if err != nil {
+ // Translate hierarchy errors with create-specific keys so the client receives an
+ // appropriate message. invalidInputAppError maps these to update.* keys, which don't
+ // apply to a create operation.
+ var invErr *store.ErrInvalidInput
+ if errors.As(err, &invErr) {
+ switch invErr.Reason {
+ case store.ReasonParentNotLive:
+ // The parent validated in validateDraftParent can disappear before UpsertDraft's
+ // locked check; surface the create-specific key rather than storeAppError's page.* key.
+ return nil, mmmodel.NewAppError("CreateSpaceDraft", "app.page_draft.create.invalid_parent.app_error", nil, "", http.StatusBadRequest).Wrap(err)
+ case store.ReasonDraftCycle:
+ return nil, mmmodel.NewAppError("CreateSpaceDraft", "app.page_draft.create.parent_cycle.app_error", nil, "", http.StatusBadRequest).Wrap(err)
+ case store.ReasonDraftTooDeep:
+ return nil, mmmodel.NewAppError("CreateSpaceDraft", "app.page_draft.create.parent_too_deep.app_error", nil, "", http.StatusBadRequest).Wrap(err)
+ }
+ }
+ return nil, storeAppError("CreateSpaceDraft", err)
+ }
+
+ return saved, nil
+}
+
+// validateDraftParent accepts a parent that is either a published page in spaceID or an existing
+// draft of the caller in spaceID (which allows child drafts under not-yet-published parents).
+func (s *Service) validateDraftParent(userID, spaceID, parentID string) *mmmodel.AppError {
+ if !mmmodel.IsValidId(parentID) {
+ return mmmodel.NewAppError("validateDraftParent", "app.page_draft.create.invalid_parent_id.app_error", nil, "", http.StatusBadRequest)
+ }
+ // Probe the parent space-scoped and collapse "not found" and "exists in another space" into one
+ // error, so a caller cannot use a distinct response to probe page ids in spaces it cannot read
+ // (matches validateParentExists).
+ pageExists, existsErr := s.store.PageExistsInSpace(parentID, spaceID)
+ if existsErr != nil {
+ return storeAppError("validateDraftParent", existsErr)
+ }
+ if pageExists {
+ return nil
+ }
+ // Not a published page in this space — accept only the caller's own draft in this space.
+ // GetDraftSpaceID probes existence (space-checked below) without hauling the draft body.
+ draftSpaceID, draftErr := s.store.GetDraftSpaceID(userID, parentID)
+ if draftErr == nil && draftSpaceID == spaceID {
+ return nil
+ }
+ if draftErr != nil && !store.IsErrNotFound(draftErr) {
+ return storeAppError("validateDraftParent", draftErr)
+ }
+ return mmmodel.NewAppError("validateDraftParent", "app.page_draft.create.invalid_parent.app_error", nil, "", http.StatusBadRequest)
+}
+
+// GetPageDraft returns the calling user's draft for the given page in the given space. Returns not-found
+// when no draft exists for that user.
+func (s *Service) GetPageDraft(userID, spaceID, pageID string) (*model.Draft, *mmmodel.AppError) {
+ if !mmmodel.IsValidId(userID) {
+ return nil, mmmodel.NewAppError("GetPageDraft", "app.page_draft.get.invalid_user_id.app_error", nil, "", http.StatusBadRequest)
+ }
+ if !mmmodel.IsValidId(spaceID) {
+ return nil, mmmodel.NewAppError("GetPageDraft", "app.page_draft.get.invalid_space_id.app_error", nil, "", http.StatusBadRequest)
+ }
+ if !mmmodel.IsValidId(pageID) {
+ return nil, mmmodel.NewAppError("GetPageDraft", "app.page_draft.get.invalid_page_id.app_error", nil, "", http.StatusBadRequest)
+ }
+
+ draft, err := s.store.GetDraft(userID, pageID)
+ if err != nil {
+ if store.IsErrNotFound(err) {
+ return nil, mmmodel.NewAppError("GetPageDraft", "app.page_draft.get.not_found.app_error", nil, "", http.StatusNotFound).Wrap(err)
+ }
+ return nil, storeAppError("GetPageDraft", err)
+ }
+
+ // Defend against a draft key collision across spaces: a draft is keyed by (UserId, PageId),
+ // so confirm it belongs to the space named in the request.
+ if draft.SpaceId != spaceID {
+ return nil, mmmodel.NewAppError("GetPageDraft", "app.page_draft.get.not_found.app_error", nil, "", http.StatusNotFound)
+ }
+
+ return draft, nil
+}
+
+// DeletePageDraft removes the calling user's draft for the given page — the discard path. (A
+// publish deletes the draft inside the store publish transaction without calling here.)
+// Returns not-found when no draft exists. channelID is the space's backing channel, used to scope
+// the presence broadcast.
+func (s *Service) DeletePageDraft(userID, spaceID, pageID, channelID string) *mmmodel.AppError {
+ if !mmmodel.IsValidId(userID) {
+ return mmmodel.NewAppError("DeletePageDraft", "app.page_draft.delete.invalid_user_id.app_error", nil, "", http.StatusBadRequest)
+ }
+ if !mmmodel.IsValidId(spaceID) {
+ return mmmodel.NewAppError("DeletePageDraft", "app.page_draft.delete.invalid_space_id.app_error", nil, "", http.StatusBadRequest)
+ }
+ if !mmmodel.IsValidId(pageID) {
+ return mmmodel.NewAppError("DeletePageDraft", "app.page_draft.delete.invalid_page_id.app_error", nil, "", http.StatusBadRequest)
+ }
+ s.log.Debug("Deleting page draft", "space_id", spaceID, "page_id", pageID, "user_id", userID)
+
+ // Discard is unconditional. Autosave and discard are separate HTTP requests in separate
+ // transactions, so an autosave request the same user dispatched just before the discard can
+ // still be in flight when the discard commits and then re-insert (resurrect) this draft — the
+ // server does not guarantee the autosave commits before the later-issued delete.
+ //
+ // UpsertDraft's staleness guard cannot catch this case: an unpublished new-page draft has no
+ // page row for the guard to key on (a published page is protected; only this case is not).
+ //
+ // The resulting zombie draft is harmless — it is visible only to its owning user (drafts are
+ // keyed by (UserId, PageId)), and that user removes it by discarding again — so a deletion
+ // tombstone to permanently block re-insertion is not warranted.
+ // A draft is keyed by (UserId, PageId) without SpaceId; DeleteDraftReparenting scopes its
+ // delete by (UserId, SpaceId, PageId) and requires a live space, so a member of another space
+ // cannot delete a draft here by passing this space's id with a foreign page id — no separate
+ // pre-check read is needed.
+ pageWasLive, delErr := s.store.DeleteDraftReparenting(userID, spaceID, pageID)
+ if delErr != nil {
+ // No matching draft — never existed, wrong space, dead space, or removed by a concurrent
+ // publish/delete. All read as "nothing to discard", so report 404 rather than a 500 that
+ // would also emit a spurious server-side error log.
+ if store.IsErrNotFound(delErr) {
+ return mmmodel.NewAppError("DeletePageDraft", "app.page_draft.delete.not_found.app_error", nil, "", http.StatusNotFound).Wrap(delErr)
+ }
+ return storeAppError("DeletePageDraft", delErr)
+ }
+
+ s.endDraftPresenceSession(pageWasLive, pageID, userID, spaceID, channelID)
+
+ return nil
+}
+
+// GetPageDraftsForSpace lists the user's unpublished drafts in a space, most-recently-updated
+// first, one pagination page at a time; the second return reports whether further pages exist.
+// Draft bodies are omitted from the listing.
+func (s *Service) GetPageDraftsForSpace(userID, spaceID string, page, perPage int) ([]*model.DraftSummary, bool, *mmmodel.AppError) {
+ if !mmmodel.IsValidId(userID) {
+ return nil, false, mmmodel.NewAppError("GetPageDraftsForSpace", "app.page_draft.list.invalid_user_id.app_error", nil, "", http.StatusBadRequest)
+ }
+ if !mmmodel.IsValidId(spaceID) {
+ return nil, false, mmmodel.NewAppError("GetPageDraftsForSpace", "app.page_draft.list.invalid_space_id.app_error", nil, "", http.StatusBadRequest)
+ }
+
+ // The store already excludes drafts in a soft-deleted space, so this need not re-check liveness.
+ offset, limit := paginationOffsetLimit(page, perPage)
+ drafts, err := s.store.GetDraftsForSpace(userID, spaceID, offset, limit)
+ if err != nil {
+ return nil, false, storeAppError("GetPageDraftsForSpace", err)
+ }
+ drafts, hasMore := trimPage(drafts, limit)
+ return drafts, hasMore, nil
+}
+
+// PublishPageDraft publishes the calling user's draft for pageID in spaceID as a page, creating the
+// page on first publish or updating it if it already exists. pageID is the id reserved when editing
+// began (see CreateSpaceDraft) and is stable across the draft → publish lifecycle, so its presence
+// does not imply a published page yet: whether this is a create or an edit is re-derived from the
+// database (no client trust). The draft is validated, and the page write and the draft's removal
+// either both take effect or neither does.
+// Returns (page, wasCreated, appErr):
+// - wasCreated=true → a new page was inserted by this call (handler should return 201)
+// - wasCreated=false → an existing page was updated, or a concurrent create was adopted (return 200)
+// - appErr is a 409 edit conflict → page is the current server page (or nil if the re-read failed),
+// so the caller can surface a diff without a follow-up read; on every other error page is nil.
+func (s *Service) PublishPageDraft(userID, spaceID, pageID string, force bool) (*model.Page, bool, *mmmodel.AppError) {
+ if !mmmodel.IsValidId(userID) {
+ return nil, false, mmmodel.NewAppError("PublishPageDraft", "app.page_draft.publish.invalid_user_id.app_error", nil, "", http.StatusBadRequest)
+ }
+ if !mmmodel.IsValidId(spaceID) {
+ return nil, false, mmmodel.NewAppError("PublishPageDraft", "app.page_draft.publish.invalid_space_id.app_error", nil, "", http.StatusBadRequest)
+ }
+ if !mmmodel.IsValidId(pageID) {
+ return nil, false, mmmodel.NewAppError("PublishPageDraft", "app.page_draft.publish.invalid_page_id.app_error", nil, "", http.StatusBadRequest)
+ }
+ s.log.Debug("Publishing page draft", "space_id", spaceID, "page_id", pageID, "user_id", userID, "force", force)
+ // 1. Fetch draft (idempotency guard: 404 = draft already published or discarded).
+ draft, appErr := s.GetPageDraft(userID, spaceID, pageID)
+ if appErr != nil {
+ if appErr.StatusCode == http.StatusNotFound {
+ return nil, false, mmmodel.NewAppError("PublishPageDraft", "app.page_draft.publish.draft_not_found.app_error",
+ nil, "", http.StatusNotFound).Wrap(appErr)
+ }
+ return nil, false, appErr
+ }
+
+ // 2. Derive isNewPage from the database; never trust client state.
+ existing, existingErr := s.GetPageWithDeleted(pageID)
+ isNewPage, targetErr := derivePublishTarget(existing, existingErr, spaceID)
+ if targetErr != nil {
+ return nil, false, targetErr
+ }
+
+ // 3. Parent guard (new-page path only): a new page's parent must be a published live page; a
+ // draft-only or non-live parent returns 409. The edit path never reparents, so a stale ParentId
+ // carried on the draft must not block a content-only edit-publish.
+ if isNewPage && draft.ParentId != "" {
+ // Existence-only probe, space-scoped: collapse "not a live page" and "lives in another space"
+ // into one error so the response can't be used to probe page ids in spaces the caller cannot
+ // read (matches validateDraftParent / validateParentExists).
+ parentExists, parentErr := s.store.PageExistsInSpace(draft.ParentId, spaceID)
+ if parentErr != nil {
+ return nil, false, storeAppError("PublishPageDraft", parentErr)
+ }
+ if !parentExists {
+ return nil, false, mmmodel.NewAppError("PublishPageDraft", "app.page_draft.publish.parent_unpublished.app_error",
+ nil, "", http.StatusConflict)
+ }
+ }
+
+ // 4/5/6. Validate & normalise the draft, build the store write (new-page insert vs edit patch;
+ // see helpers), and hand it to the store. draft.UpdateAt is passed through so a concurrent
+ // autosave rolls this publish back as a conflict rather than shipping older content — see
+ // store.deletePublishedDraftTx.
+ var page *model.Page
+ var storeErr error
+ if isNewPage {
+ pageForWrite, buildErr := s.buildNewPageForPublish(pageID, spaceID, userID, draft)
+ if buildErr != nil {
+ return nil, false, buildErr
+ }
+ page, storeErr = s.store.PublishNewPageDraft(pageForWrite, userID, spaceID, model.MaxPageDepth, draft.UpdateAt)
+ } else {
+ patch, buildErr := s.buildEditPatchForPublish(draft, force)
+ if buildErr != nil {
+ return nil, false, buildErr
+ }
+ // A "baseline-only" edit draft carries an optimistic-lock baseline but no populated field,
+ // so there is no page change to write; discard it rather than bumping EditAt for nothing
+ // (see helper).
+ if patch.Title == nil && patch.Body == nil && patch.Props == nil {
+ return s.discardBaselineOnlyDraft(userID, pageID, spaceID, draft.UpdateAt)
+ }
+ page, storeErr = s.store.PublishPageEditDraft(pageID, spaceID, patch, draft.BaseEditAt, force, userID, draft.UpdateAt)
+ }
+ if storeErr != nil {
+ return s.translatePublishStoreError(storeErr, isNewPage, userID, spaceID, pageID, draft.UpdateAt)
+ }
+
+ // 7. Broadcast the write with the same shape and channel scope as the direct-CRUD page events.
+ // A publish-via-draft edit reuses page_updated — a client handles it identically to a direct PATCH.
+ // page_created includes parent_id so clients can place the new node in the tree without a fetch.
+ if isNewPage {
+ s.publishToChannels(wsEventPageCreated, map[string]any{
+ "page_id": page.Id,
+ "space_id": page.SpaceId,
+ "parent_id": page.ParentId,
+ }, page.ChannelId)
+ } else {
+ s.publishToChannels(wsEventPageUpdated, map[string]any{
+ "page_id": page.Id,
+ "space_id": page.SpaceId,
+ }, page.ChannelId)
+ }
+ // The publish deleted the draft inside the store publish transaction (bypassing the app-level DeletePageDraft
+ // that normally broadcasts presence), so end the presence session now so the active-editors
+ // indicator clears on other clients. The page is live by definition at this point.
+ s.endDraftPresenceSession(true, pageID, userID, spaceID, page.ChannelId)
+
+ return page, isNewPage, nil
+}
+
+// translatePublishStoreError maps a failed publish write to the caller-facing result, so
+// PublishPageDraft stays a sequence of steps and every publish-failure condition lands in one place.
+// It returns the same triple as PublishPageDraft: the adopted page with a nil error when a concurrent
+// publish already created this page, the current server page alongside a 409 on an edit conflict, and
+// a nil page otherwise.
+func (s *Service) translatePublishStoreError(storeErr error, isNewPage bool, userID, spaceID, pageID string, draftUpdateAt int64) (*model.Page, bool, *mmmodel.AppError) {
+ switch {
+ // The draft moved under this publish: the caller's own editor autosaved after this call read it,
+ // so the whole write was rolled back rather than committing the older content. Distinct from the
+ // conflicts below — the client republishes to ship the newer draft, it does not re-baseline.
+ case store.ConflictReason(storeErr) == store.ReasonConcurrentAutosave:
+ return nil, false, mmmodel.NewAppError("PublishPageDraft", "app.page_draft.publish.draft_changed.app_error",
+ nil, "", http.StatusConflict).Wrap(storeErr)
+
+ // Someone else edited the page since the baseline was captured. The draft's baseline is
+ // write-once (see store.UpsertDraft), so the client cannot re-baseline the existing draft:
+ // it recovers by publishing with force, or by discarding the draft and reopening the edit
+ // session against the current page. Return the current server page alongside the conflict so
+ // the client can diff and choose in one round-trip rather than a follow-up GET.
+ // The pre-lock page snapshot is stale by definition here, so re-read the live page.
+ case store.ConflictReason(storeErr) == store.ReasonConcurrentEdit:
+ editConflictErr := mmmodel.NewAppError("PublishPageDraft", "app.page_draft.publish.edit_conflict.app_error",
+ nil, "", http.StatusConflict).Wrap(storeErr)
+ current, getErr := s.GetPageInSpace("PublishPageDraft", pageID, spaceID, false)
+ if getErr != nil {
+ // A concurrent delete or cross-space move can make the page unreadable here; fall
+ // back to a bare conflict and let the client GET the page itself.
+ s.log.Warn("failed to re-read page for edit-conflict body",
+ "page_id", pageID, "user_id", userID, "err", getErr)
+ return nil, false, editConflictErr
+ }
+ return current, false, editConflictErr
+
+ case store.IsErrConflict(storeErr):
+ // PK collision on the new-page path: a concurrent publish won this page id. Adopt the
+ // winner and return 200 without broadcasting; a winner that is not this caller's to read
+ // falls through to a plain conflict (see adoptPublishRaceWinner).
+ if isNewPage {
+ if raced, adopted := s.adoptPublishRaceWinner(userID, pageID, spaceID, draftUpdateAt); adopted {
+ return raced, false, nil
+ }
+ }
+ return nil, false, mmmodel.NewAppError("PublishPageDraft", "app.page_draft.publish.conflict.app_error",
+ nil, "", http.StatusConflict).Wrap(storeErr)
+ case store.IsErrNotFound(storeErr):
+ // A concurrent delete removed the page or its parent between the pre-checks and the lock.
+ return nil, false, mmmodel.NewAppError("PublishPageDraft", "app.page_draft.publish.page_deleted.app_error",
+ nil, "", http.StatusConflict).Wrap(storeErr)
+ default:
+ return nil, false, storeAppError("PublishPageDraft", storeErr)
+ }
+}
+
+// derivePublishTarget classifies a publish target from the GetPageWithDeleted read: no page means
+// a new-page publish, a live page in the caller's space means an edit-publish. A page in another
+// space reports 404 rather than confirming the id exists elsewhere. A deleted page reports 409:
+// the draft outlived its page and cannot be published.
+func derivePublishTarget(existing *model.Page, existingErr *mmmodel.AppError, spaceID string) (isNewPage bool, appErr *mmmodel.AppError) {
+ // The cross-space and deleted cases below are reachable only when the page moved or was deleted
+ // between the draft read and this classification — the draft read's liveness filter excludes
+ // drafts in either steady state.
+ switch {
+ case existingErr != nil && existingErr.StatusCode == http.StatusNotFound:
+ return true, nil
+ case existingErr != nil:
+ return false, existingErr
+ case existing == nil:
+ // GetPageWithDeleted never returns (nil, nil) today; classify it like not-found rather
+ // than dereferencing nil, matching the guard adoptPublishRaceWinner applies to the same read.
+ return true, nil
+ case existing.SpaceId != spaceID:
+ // GetPageWithDeleted is not space-scoped and the caller is only authorized for spaceID, so
+ // collapse to the same 404 the space-scoped reads return.
+ return false, mmmodel.NewAppError("PublishPageDraft", "app.page_draft.publish.page_not_found.app_error",
+ nil, "", http.StatusNotFound)
+ case existing.DeleteAt != 0:
+ return false, mmmodel.NewAppError("PublishPageDraft", "app.page_draft.publish.page_deleted.app_error",
+ nil, "", http.StatusConflict)
+ default: // live page in this space → edit path
+ return false, nil
+ }
+}
+
+// buildNewPageForPublish normalises the draft body and constructs the *model.Page inserted by
+// store.PublishNewPageDraft. The page adopts the draft's title, body, and props outright.
+func (s *Service) buildNewPageForPublish(pageID, spaceID, userID string, draft *model.Draft) (*model.Page, *mmmodel.AppError) {
+ body, searchText, contentErr := normalizePageContent("PublishPageDraft", draft.Body)
+ if contentErr != nil {
+ return nil, contentErr
+ }
+ title, titleErr := validateTitle("PublishPageDraft", draft.Title, model.PageTitleMaxRunes)
+ if titleErr != nil {
+ return nil, titleErr
+ }
+ // ChannelId is derived by the store from the space, matching CreatePage, so it is
+ // intentionally left unset here.
+ return &model.Page{
+ Id: pageID,
+ SpaceId: spaceID,
+ ParentId: draft.ParentId,
+ Title: title,
+ Body: body,
+ SearchText: searchText,
+ Props: maps.Clone(draft.Props),
+ UserId: userID,
+ LastModifiedBy: userID,
+ }, nil
+}
+
+// buildEditPatchForPublish normalises the draft body and translates the draft into the
+// *model.PagePatch applied by store.PublishPageEditDraft, carrying only the fields this draft
+// changed. A Draft has no per-field presence markers, so empty is its unset marker: an omitted
+// field stays nil in the patch and keeps the live page's current value, which means a partial
+// draft never wipes an untouched field and a force-publish cannot revert a concurrent edit to a
+// field this draft did not change. Body's "" reading is safe because a document the user cleared
+// is stored as EmptyTipTapJSON, never "". Props follow the same rule: a bare map cannot express
+// "clear to empty" distinctly from "unset", so an empty draft map means preserve.
+// The caller detects a baseline-only draft (every patch field nil) after the build.
+func (s *Service) buildEditPatchForPublish(draft *model.Draft, force bool) (*model.PagePatch, *mmmodel.AppError) {
+ body, searchText, contentErr := normalizePageContent("PublishPageDraft", draft.Body)
+ if contentErr != nil {
+ return nil, contentErr
+ }
+ // Require an optimistic-lock baseline unless force, so a client that never captured the
+ // page's EditAt cannot silently overwrite a concurrent edit.
+ if !force && draft.BaseEditAt == 0 {
+ return nil, mmmodel.NewAppError("PublishPageDraft", "app.page_draft.publish.baseline_required.app_error",
+ nil, "", http.StatusBadRequest)
+ }
+ patch := &model.PagePatch{}
+ if draft.Title != "" {
+ title, titleErr := validateTitle("PublishPageDraft", draft.Title, model.PageTitleMaxRunes)
+ if titleErr != nil {
+ return nil, titleErr
+ }
+ patch.Title = &title
+ }
+ if draft.Body != "" {
+ patch.Body = &body
+ patch.SearchText = &searchText
+ }
+ if len(draft.Props) > 0 {
+ // No clone here: the store merges via model.Page.Patch, which clones Props itself.
+ patch.Props = &draft.Props
+ }
+ return patch, nil
+}
+
+// discardBaselineOnlyDraft handles an edit-path publish whose draft carries an optimistic-lock
+// baseline but no content in any field — Title, Body, and Props were never populated (all empty).
+// This is NOT a user who cleared the document: a cleared doc is EmptyTipTapJSON, a non-empty Body
+// that publishes normally; empty here means "never sent". With every field empty there is no page
+// change to write. Publishing would still bump EditAt and emit page_updated with no actual change,
+// invalidating other editors' baselines for nothing. So delete the draft and return the page from a
+// fresh read — not the caller's pre-lock snapshot, which a concurrent edit may have outdated.
+func (s *Service) discardBaselineOnlyDraft(userID, pageID, spaceID string, draftUpdateAt int64) (*model.Page, bool, *mmmodel.AppError) {
+ deleted, delErr := s.store.DeleteDraftVersion(userID, pageID, draftUpdateAt)
+ if delErr != nil {
+ return nil, false, storeAppError("PublishPageDraft", delErr)
+ }
+ if !deleted {
+ // A concurrent autosave advanced the draft version between the read and the delete.
+ // The newer draft may have real content — the client must re-read and re-publish.
+ return nil, false, mmmodel.NewAppError("PublishPageDraft", "app.page_draft.publish.draft_changed.app_error",
+ nil, "", http.StatusConflict)
+ }
+ current, getErr := s.GetPageInSpace("PublishPageDraft", pageID, spaceID, false)
+ if getErr != nil {
+ // The draft is already discarded, so this publish's mutation succeeded; only the re-read
+ // failed. Mirror the edit-conflict branch: log and translate rather than forwarding the
+ // re-read error as a publish failure. Clear the rate-limit entry so a later session starts
+ // unthrottled — with no readable page there is no channel to broadcast the session end to.
+ s.log.Warn("failed to re-read page after discarding baseline-only draft",
+ "page_id", pageID, "user_id", userID, "err", getErr)
+ s.clearPresenceThrottle(pageID, userID)
+ if getErr.StatusCode == http.StatusNotFound {
+ // A concurrent delete or cross-space move made the page unreadable: the draft outlived
+ // its page, which is the defined page_deleted conflict.
+ return nil, false, mmmodel.NewAppError("PublishPageDraft", "app.page_draft.publish.page_deleted.app_error",
+ nil, "", http.StatusConflict).Wrap(getErr)
+ }
+ return nil, false, getErr
+ }
+ s.endDraftPresenceSession(true, pageID, userID, spaceID, current.ChannelId)
+ return current, false, nil
+}
+
+// adoptPublishRaceWinner handles the PK-collision case on the new-page publish path: a concurrent
+// publish already created this page id. It returns the winner and true when this caller should adopt
+// it (return 200 without broadcasting — the winner already broadcast wsEventPageCreated). The winner
+// must be a live page in this space; a different-space or already-deleted winner is not this caller's
+// to read, so it returns (nil, false) and the caller surfaces a plain conflict.
+func (s *Service) adoptPublishRaceWinner(userID, pageID, spaceID string, draftUpdateAt int64) (*model.Page, bool) {
+ raced, rErr := s.GetPageWithDeleted(pageID)
+ if rErr == nil && raced != nil && raced.SpaceId == spaceID && raced.DeleteAt == 0 {
+ // Discard this caller's now-orphaned draft so it does not linger pointing at a
+ // published page — but only if it still holds the version this publish read. A fresh
+ // autosave after the race winner committed bumps UpdateAt, so the CAS matches no row
+ // and that newer draft is left intact rather than dropped.
+ //
+ // Best-effort: the page is already published by the winner, so a failure here is
+ // logged (a stray draft the user can discard), never surfaced as a publish failure.
+ if _, delErr := s.store.DeleteDraftVersion(userID, pageID, draftUpdateAt); delErr != nil {
+ s.log.Warn("failed to delete orphaned draft after adopting race winner",
+ "page_id", pageID, "user_id", userID, "err", delErr)
+ }
+ // The draft is consumed; end the presence session so the active-editors indicator drops
+ // this user, matching the non-conflict path. The adopted winner is live by definition.
+ s.endDraftPresenceSession(true, pageID, userID, spaceID, raced.ChannelId)
+ return raced, true
+ }
+ if rErr != nil {
+ // Log this: a real store failure here would otherwise look identical to losing the race.
+ s.log.Warn("failed to read the page that won the publish race",
+ "page_id", pageID, "user_id", userID, "err", rErr)
+ }
+ return nil, false
+}
diff --git a/server/app/page_draft_internal_test.go b/server/app/page_draft_internal_test.go
new file mode 100644
index 0000000..64acf27
--- /dev/null
+++ b/server/app/page_draft_internal_test.go
@@ -0,0 +1,126 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See LICENSE.txt for license information.
+
+package app
+
+import (
+ "net/http"
+ "testing"
+
+ mmmodel "github.com/mattermost/mattermost/server/public/model"
+ "github.com/stretchr/testify/require"
+
+ "github.com/mattermost/mattermost-plugin-docs/server/internal/testutil"
+ "github.com/mattermost/mattermost-plugin-docs/server/model"
+ "github.com/mattermost/mattermost-plugin-docs/server/store"
+)
+
+// TestAdoptPublishRaceWinner exercises the PK-collision adoption path directly: PublishPageDraft
+// reaches it only when a concurrent publish inserts the page between the target classification and
+// the store insert, a window that cannot be held open through the public service surface.
+func TestAdoptPublishRaceWinner(t *testing.T) {
+ s, _ := testutil.OpenTestStore(t)
+ svc := New(s, nil, nil)
+ space := testutil.MustCreateSpace(t, s, mmmodel.NewId(), mmmodel.NewId())
+ userID := mmmodel.NewId()
+
+ // newDraftAt reserves pageID as the user's new-page draft, mirroring CreateSpaceDraft.
+ newDraftAt := func(t *testing.T, pageID string) *model.Draft {
+ t.Helper()
+ d, _, err := s.UpsertDraft(&model.Draft{UserId: userID, SpaceId: space.Id, PageId: pageID, Title: "Doc"}, nil, nil, nil)
+ require.NoError(t, err)
+ return d
+ }
+
+ // winnerAt commits a live page at pageID, simulating the concurrent publish that won the id.
+ winnerAt := func(t *testing.T, pageID string) *model.Page {
+ t.Helper()
+ winner := testutil.NewPage(space.Id, space.ChannelId, userID, "")
+ winner.Id = pageID
+ created, err := s.CreatePage(winner, testutil.UncappedMaxDepth)
+ require.NoError(t, err)
+ return created
+ }
+
+ t.Run("live winner in the same space is adopted and the orphaned draft is consumed", func(t *testing.T) {
+ pageID := mmmodel.NewId()
+ draft := newDraftAt(t, pageID)
+ created := winnerAt(t, pageID)
+
+ raced, adopted := svc.adoptPublishRaceWinner(userID, pageID, space.Id, draft.UpdateAt)
+ require.True(t, adopted)
+ require.Equal(t, created.Id, raced.Id)
+
+ _, err := s.GetDraft(userID, pageID)
+ require.True(t, store.IsErrNotFound(err), "the orphaned draft must be deleted, got %v", err)
+ })
+
+ t.Run("deleted winner is not adopted", func(t *testing.T) {
+ pageID := mmmodel.NewId()
+ draft := newDraftAt(t, pageID)
+ winnerAt(t, pageID)
+ _, err := s.DeletePage(pageID, space.Id, userID)
+ require.NoError(t, err)
+
+ raced, adopted := svc.adoptPublishRaceWinner(userID, pageID, space.Id, draft.UpdateAt)
+ require.False(t, adopted, "a deleted page is not a publishable winner")
+ require.Nil(t, raced)
+ })
+
+ t.Run("winner in another space is not adopted", func(t *testing.T) {
+ pageID := mmmodel.NewId()
+ draft := newDraftAt(t, pageID)
+ winnerAt(t, pageID)
+
+ raced, adopted := svc.adoptPublishRaceWinner(userID, pageID, mmmodel.NewId(), draft.UpdateAt)
+ require.False(t, adopted, "a winner outside the caller's space is not adoptable")
+ require.Nil(t, raced)
+
+ _, err := s.GetDraft(userID, pageID)
+ require.NoError(t, err, "a non-adopted draft must be left in place")
+ })
+}
+
+// TestDerivePublishTarget covers the publish-target classification directly: the cross-space and
+// deleted branches guard races (the page moved or was deleted between the draft read and the
+// classification) that cannot be constructed through the public service surface — the draft
+// read's liveness filter excludes both steady states.
+func TestDerivePublishTarget(t *testing.T) {
+ spaceID := mmmodel.NewId()
+
+ t.Run("missing page is a new-page publish", func(t *testing.T) {
+ notFound := mmmodel.NewAppError("GetPageWithDeleted", "app.page.get.not_found.app_error", nil, "", http.StatusNotFound)
+ isNewPage, appErr := derivePublishTarget(nil, notFound, spaceID)
+ require.Nil(t, appErr)
+ require.True(t, isNewPage)
+ })
+
+ t.Run("read failure is passed through", func(t *testing.T) {
+ readErr := mmmodel.NewAppError("GetPageWithDeleted", "app.store.not_found.app_error", nil, "", http.StatusInternalServerError)
+ isNewPage, appErr := derivePublishTarget(nil, readErr, spaceID)
+ require.False(t, isNewPage)
+ require.Equal(t, readErr, appErr)
+ })
+
+ t.Run("page in another space reports not-found", func(t *testing.T) {
+ isNewPage, appErr := derivePublishTarget(&model.Page{SpaceId: mmmodel.NewId()}, nil, spaceID)
+ require.False(t, isNewPage)
+ require.NotNil(t, appErr)
+ require.Equal(t, http.StatusNotFound, appErr.StatusCode)
+ require.Equal(t, "app.page_draft.publish.page_not_found.app_error", appErr.Id)
+ })
+
+ t.Run("deleted page conflicts", func(t *testing.T) {
+ isNewPage, appErr := derivePublishTarget(&model.Page{SpaceId: spaceID, DeleteAt: 1}, nil, spaceID)
+ require.False(t, isNewPage)
+ require.NotNil(t, appErr)
+ require.Equal(t, http.StatusConflict, appErr.StatusCode)
+ require.Equal(t, "app.page_draft.publish.page_deleted.app_error", appErr.Id)
+ })
+
+ t.Run("live page in the caller's space is an edit publish", func(t *testing.T) {
+ isNewPage, appErr := derivePublishTarget(&model.Page{SpaceId: spaceID}, nil, spaceID)
+ require.Nil(t, appErr)
+ require.False(t, isNewPage)
+ })
+}
diff --git a/server/app/page_draft_test.go b/server/app/page_draft_test.go
new file mode 100644
index 0000000..d8db064
--- /dev/null
+++ b/server/app/page_draft_test.go
@@ -0,0 +1,1095 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See LICENSE.txt for license information.
+
+package app_test
+
+import (
+ "fmt"
+ "net/http"
+ "strings"
+ "sync"
+ "testing"
+
+ "github.com/stretchr/testify/mock"
+ "github.com/stretchr/testify/require"
+
+ mmmodel "github.com/mattermost/mattermost/server/public/model"
+ "github.com/mattermost/mattermost/server/public/plugin/plugintest"
+
+ "github.com/mattermost/mattermost-plugin-docs/server/app"
+ "github.com/mattermost/mattermost-plugin-docs/server/model"
+)
+
+// docWith returns a minimal TipTap document whose paragraph contains text.
+func docWith(text string) string {
+ return fmt.Sprintf(`{"type":"doc","content":[{"type":"paragraph","content":[{"type":"text","text":%q}]}]}`, text)
+}
+
+// publishNewPage creates a new-page draft, autosaves the given body, and publishes it, returning
+// the live page. It asserts the reserved draft id is preserved through publish.
+func publishNewPage(t *testing.T, h *testHarness, spaceID, userID, title, bodyText string) *model.Page {
+ t.Helper()
+ draft, appErr := h.svc.CreateSpaceDraft(userID, spaceID, title, "")
+ require.Nil(t, appErr)
+ reservedID := draft.PageId
+
+ _, appErr = h.svc.UpdatePageDraft(&model.Draft{UserId: userID, SpaceId: spaceID, PageId: reservedID, Title: title, Body: docWith(bodyText)}, nil, nil, nil, "")
+ require.Nil(t, appErr)
+
+ page, wasCreated, appErr := h.svc.PublishPageDraft(userID, spaceID, reservedID, false)
+ require.Nil(t, appErr)
+ require.True(t, wasCreated, "publishing a brand-new page must report wasCreated=true")
+ require.Equal(t, reservedID, page.Id, "publish must preserve the reserved draft id")
+ require.Contains(t, page.Body, bodyText)
+ return page
+}
+
+func TestUpdatePageDraftPreservesBodyOnTitleOnlyAutosave(t *testing.T) {
+ h := openTestService(t)
+ space := mustCreateSpace(t, h.store, mmmodel.NewId())
+ userID := mmmodel.NewId()
+
+ draft, appErr := h.svc.CreateSpaceDraft(userID, space.Id, "Doc", "")
+ require.Nil(t, appErr)
+ pageID := draft.PageId
+
+ // Autosave real content.
+ _, appErr = h.svc.UpdatePageDraft(&model.Draft{UserId: userID, SpaceId: space.Id, PageId: pageID, Title: "Doc", Body: docWith("keep me")}, nil, nil, nil, "")
+ require.Nil(t, appErr)
+
+ // A heartbeat that sends only the title (empty body) must not wipe the stored draft body.
+ saved, appErr := h.svc.UpdatePageDraft(&model.Draft{UserId: userID, SpaceId: space.Id, PageId: pageID, Title: "Doc"}, nil, nil, nil, "")
+ require.Nil(t, appErr)
+ require.Contains(t, saved.Body, "keep me", "a title-only autosave must not clear the draft body")
+}
+
+func TestPublishEmptyDraftBodyDoesNotWipePage(t *testing.T) {
+ h := openTestService(t)
+ space := mustCreateSpace(t, h.store, mmmodel.NewId())
+ userID := mmmodel.NewId()
+
+ page := publishNewPage(t, h, space.Id, userID, "Important", "ORIGINAL")
+
+ // Start an edit session whose first (and only) autosave carries the title but no body, exactly
+ // the heartbeat case that previously wiped the page on publish.
+ _, appErr := h.svc.UpdatePageDraft(&model.Draft{
+ UserId: userID, SpaceId: space.Id, PageId: page.Id, Title: "Important",
+ BaseEditAt: page.EditAt,
+ }, nil, nil, nil, "")
+ require.Nil(t, appErr)
+
+ republished, _, appErr := h.svc.PublishPageDraft(userID, space.Id, page.Id, false)
+ require.Nil(t, appErr)
+ require.Contains(t, republished.Body, "ORIGINAL", "publishing a title-only edit must preserve the page body")
+}
+
+// TestPublishNoOpDraftDiscardsAndReturnsExistingPage verifies that publishing a draft that carries
+// no content change — only the optimistic-lock baseline, no Title, no Body — is treated as a
+// discard rather than a no-op page write: the draft is deleted, the existing page comes back
+// unchanged with its EditAt intact, no page_updated event fires, and wasCreated is false.
+func TestPublishNoOpDraftDiscardsAndReturnsExistingPage(t *testing.T) {
+ mockAPI := &plugintest.API{}
+ h := openTestServiceWithAPI(t, mockAPI)
+ space := mustCreateSpace(t, h.store, mmmodel.NewId())
+ userID := mmmodel.NewId()
+
+ page := publishNewPage(t, h, space.Id, userID, "Doc", "original")
+
+ // Start an edit session whose only autosave carries the baseline — no Title, no Body.
+ _, appErr := h.svc.UpdatePageDraft(&model.Draft{
+ UserId: userID, SpaceId: space.Id, PageId: page.Id,
+ BaseEditAt: page.EditAt,
+ }, nil, nil, nil, "")
+ require.Nil(t, appErr)
+
+ result, wasCreated, appErr := h.svc.PublishPageDraft(userID, space.Id, page.Id, false)
+ require.Nil(t, appErr)
+ require.False(t, wasCreated, "a no-content publish must not report a creation")
+ require.Equal(t, page.Title, result.Title, "the page's title must be unchanged")
+ require.Contains(t, result.Body, "original", "the page's body must be unchanged")
+ require.Equal(t, page.EditAt, result.EditAt, "a no-content publish must not bump EditAt")
+ mockAPI.AssertNotCalled(t, "PublishWebSocketEvent", "page_updated", mock.Anything, mock.Anything)
+
+ // The draft was consumed: it was converted into a discard rather than left in place.
+ _, appErr = h.svc.GetPageDraft(userID, space.Id, page.Id)
+ require.NotNil(t, appErr)
+ require.Equal(t, http.StatusNotFound, appErr.StatusCode)
+}
+
+func TestPublishRejectsMissingBaselineOnEdit(t *testing.T) {
+ h := openTestService(t)
+ space := mustCreateSpace(t, h.store, mmmodel.NewId())
+ userID := mmmodel.NewId()
+
+ page := publishNewPage(t, h, space.Id, userID, "Doc", "v1")
+
+ // UpsertDraft guards the first edit-draft save: attempting to create a draft for an existing
+ // page without a base_edit_at baseline is rejected at the store layer, so the client must send
+ // the baseline on the very first autosave (the response tells it to reload and set it).
+ _, appErr := h.svc.UpdatePageDraft(&model.Draft{UserId: userID, SpaceId: space.Id, PageId: page.Id, Title: "Doc", Body: docWith("v2")}, nil, nil, nil, "")
+ require.NotNil(t, appErr)
+ require.Equal(t, http.StatusConflict, appErr.StatusCode)
+
+ // With a proper baseline the draft is created; force=true publishes regardless of baseline.
+ _, appErr = h.svc.UpdatePageDraft(&model.Draft{
+ UserId: userID, SpaceId: space.Id, PageId: page.Id, Title: "Doc", Body: docWith("v2"),
+ BaseEditAt: page.EditAt,
+ }, nil, nil, nil, "")
+ require.Nil(t, appErr)
+
+ forced, _, appErr := h.svc.PublishPageDraft(userID, space.Id, page.Id, true)
+ require.Nil(t, appErr)
+ require.Contains(t, forced.Body, "v2")
+}
+
+// TestPublishRejectsNoBaselineOnExistingPageEdit covers the app-level baseline_required guard in
+// PublishPageDraft: a stored edit-draft with BaseEditAt still 0 (no baseline was ever captured) must
+// be rejected with 400 unless force=true. The store's UpsertDraft guard normally blocks a
+// baseline-less draft from ever being created against a live page (see
+// TestPublishRejectsMissingBaselineOnEdit), so this simulates the race where the draft's first
+// autosave lands as a new-page draft and the underlying page becomes live afterward without the
+// draft ever acquiring a baseline — reached here by inserting the draft row directly.
+func TestPublishRejectsNoBaselineOnExistingPageEdit(t *testing.T) {
+ h := openTestService(t)
+ channelID := mmmodel.NewId()
+ space := mustCreateSpace(t, h.store, channelID)
+ userID := mmmodel.NewId()
+
+ page := mustCreatePage(t, h.store, space.Id, channelID, userID, "")
+
+ now := mmmodel.GetMillis()
+ _, err := h.db.Exec(
+ `INSERT INTO docs_draft (userid,spaceid,pageid,parentid,title,body,fileids,props,createat,updateat,lastactiveat,baseeditat)
+ VALUES ($1,$2,$3,'',$4,$5,'[]','{}', $6,$6,$6,0)`,
+ userID, space.Id, page.Id, "Edited", docWith("v2"), now,
+ )
+ require.NoError(t, err)
+
+ // Without a baseline, a non-force publish of an edit must be rejected.
+ _, _, appErr := h.svc.PublishPageDraft(userID, space.Id, page.Id, false)
+ require.NotNil(t, appErr)
+ require.Equal(t, http.StatusBadRequest, appErr.StatusCode)
+ require.Equal(t, "app.page_draft.publish.baseline_required.app_error", appErr.Id)
+
+ // force=true bypasses the missing-baseline guard and publishes the edit.
+ forced, wasCreated, appErr := h.svc.PublishPageDraft(userID, space.Id, page.Id, true)
+ require.Nil(t, appErr)
+ require.False(t, wasCreated)
+ require.Equal(t, "Edited", forced.Title)
+ require.Contains(t, forced.Body, "v2")
+}
+
+func TestPublishStaleBaselineConflicts(t *testing.T) {
+ h := openTestService(t)
+ space := mustCreateSpace(t, h.store, mmmodel.NewId())
+ userID := mmmodel.NewId()
+
+ page := publishNewPage(t, h, space.Id, userID, "Doc", "v1")
+ staleEditAt := page.EditAt
+
+ // Start an edit session with a draft baselined at the current EditAt. The draft persists (it is
+ // not consumed by any publish), so the stale-baseline conflict surfaces at publish time.
+ _, appErr := h.svc.UpdatePageDraft(&model.Draft{
+ UserId: userID, SpaceId: space.Id, PageId: page.Id, Title: "Doc", Body: docWith("v3"),
+ BaseEditAt: staleEditAt,
+ }, nil, nil, nil, "")
+ require.Nil(t, appErr)
+
+ // A concurrent direct edit advances the page's EditAt out from under that baseline, without
+ // touching the draft.
+ concurrent := docWith("concurrent")
+ _, appErr = h.svc.UpdatePage(page.Id, space.Id, &model.PagePatch{Body: &concurrent}, new(staleEditAt), false, userID)
+ require.Nil(t, appErr)
+
+ // Publishing the draft against the now-stale baseline must 409, and return the current server
+ // page (the concurrent edit's content + advanced baseline) so the caller can diff without a
+ // follow-up read.
+ current, _, appErr := h.svc.PublishPageDraft(userID, space.Id, page.Id, false)
+ require.NotNil(t, appErr)
+ require.Equal(t, http.StatusConflict, appErr.StatusCode)
+ require.NotNil(t, current, "edit conflict must return the current server page")
+ require.Contains(t, current.Body, "concurrent", "current page must reflect the concurrent edit")
+ require.Greater(t, current.EditAt, staleEditAt, "current page must carry the advanced baseline")
+}
+
+// TestPublishAfterPageDeleteReturns404 verifies that deleting a page cascade-deletes its drafts,
+// so a later publish finds no draft (404) rather than writing to a tombstone. The page-deleted 409
+// path in PublishPageDraft guards only the concurrent-delete race, which this flow cannot produce.
+func TestPublishAfterPageDeleteReturns404(t *testing.T) {
+ h := openTestService(t)
+ space := mustCreateSpace(t, h.store, mmmodel.NewId())
+ userID := mmmodel.NewId()
+
+ page := publishNewPage(t, h, space.Id, userID, "Doomed", "x")
+ _, appErr := h.svc.UpdatePageDraft(&model.Draft{
+ UserId: userID, SpaceId: space.Id, PageId: page.Id, Title: "Doomed", Body: docWith("y"),
+ BaseEditAt: page.EditAt,
+ }, nil, nil, nil, "")
+ require.Nil(t, appErr)
+ requireStoreDeletePage(t, h.store, page.Id, space.Id, userID)
+
+ _, _, appErr = h.svc.PublishPageDraft(userID, space.Id, page.Id, false)
+ require.NotNil(t, appErr)
+ require.Equal(t, http.StatusNotFound, appErr.StatusCode)
+}
+
+func TestDeletePageDraftRejectsWrongSpace(t *testing.T) {
+ h := openTestService(t)
+ spaceA := mustCreateSpace(t, h.store, mmmodel.NewId())
+ spaceB := mustCreateSpace(t, h.store, mmmodel.NewId())
+ userID := mmmodel.NewId()
+
+ draft, appErr := h.svc.CreateSpaceDraft(userID, spaceA.Id, "In A", "")
+ require.Nil(t, appErr)
+
+ // Deleting the draft while naming the wrong space must 404, and must not delete it.
+ delErr := h.svc.DeletePageDraft(userID, spaceB.Id, draft.PageId, "")
+ require.NotNil(t, delErr)
+ require.Equal(t, http.StatusNotFound, delErr.StatusCode)
+
+ got, appErr := h.svc.GetPageDraft(userID, spaceA.Id, draft.PageId)
+ require.Nil(t, appErr, "draft must survive a delete attempt through the wrong space")
+ require.Equal(t, draft.PageId, got.PageId)
+}
+
+// TestPublishPageDraftConcurrentPublishesConverge races two publishes of the same new-page draft
+// (a double-clicked publish). The winner is not fixed, so the assertions are outcome invariants:
+// exactly one publish creates the page; the other either adopts the winner (success without
+// wasCreated — see adoptPublishRaceWinner) or reports a benign already-published outcome (404
+// draft gone / 409 conflict). Either way the page ends up live and the draft consumed.
+func TestPublishPageDraftConcurrentPublishesConverge(t *testing.T) {
+ h := openTestService(t)
+ space := mustCreateSpace(t, h.store, mmmodel.NewId())
+ userID := mmmodel.NewId()
+
+ draft, appErr := h.svc.CreateSpaceDraft(userID, space.Id, "Doc", "")
+ require.Nil(t, appErr)
+ _, appErr = h.svc.UpdatePageDraft(&model.Draft{UserId: userID, SpaceId: space.Id, PageId: draft.PageId, Title: "Doc", Body: docWith("race")}, nil, nil, nil, "")
+ require.Nil(t, appErr)
+
+ type result struct {
+ page *model.Page
+ created bool
+ appErr *mmmodel.AppError
+ }
+ results := make([]result, 2)
+ start := make(chan struct{})
+ var wg sync.WaitGroup
+ for i := range results {
+ wg.Go(func() {
+ <-start
+ p, created, pubErr := h.svc.PublishPageDraft(userID, space.Id, draft.PageId, false)
+ results[i] = result{page: p, created: created, appErr: pubErr}
+ })
+ }
+ close(start)
+ wg.Wait()
+
+ var created, benign int
+ for _, r := range results {
+ switch {
+ case r.appErr == nil && r.created:
+ created++
+ require.Equal(t, draft.PageId, r.page.Id)
+ case r.appErr == nil:
+ // Adopted the winner: same live page, without wasCreated.
+ require.Equal(t, draft.PageId, r.page.Id)
+ default:
+ require.Contains(t, []int{http.StatusNotFound, http.StatusConflict}, r.appErr.StatusCode,
+ "the losing publish may only fail benignly, got %v", r.appErr)
+ benign++
+ }
+ }
+ require.Equal(t, 1, created, "exactly one concurrent publish must create the page, got %+v", results)
+ require.LessOrEqual(t, benign, 1)
+
+ // Converged state: the draft is consumed and the page is live in the space.
+ _, appErr = h.svc.GetPageDraft(userID, space.Id, draft.PageId)
+ require.NotNil(t, appErr)
+ require.Equal(t, http.StatusNotFound, appErr.StatusCode, "the draft must be consumed by the publish")
+ snapshot, appErr := h.svc.GetPageActiveEditors(draft.PageId, space.Id)
+ require.Nil(t, appErr, "the published page must resolve in its space")
+ require.Empty(t, snapshot.ActiveEditors)
+}
+
+// TestGetPageActiveEditorsUnknownPageReturns404 pins the plain not-found case: a page id with no
+// row at all (as opposed to a page living in another space, covered below).
+func TestGetPageActiveEditorsUnknownPageReturns404(t *testing.T) {
+ h := openTestService(t)
+ space := mustCreateSpace(t, h.store, mmmodel.NewId())
+
+ _, appErr := h.svc.GetPageActiveEditors(mmmodel.NewId(), space.Id)
+ require.NotNil(t, appErr)
+ require.Equal(t, http.StatusNotFound, appErr.StatusCode)
+}
+
+func TestGetPageActiveEditorsRejectsWrongSpace(t *testing.T) {
+ h := openTestService(t)
+ spaceA := mustCreateSpace(t, h.store, mmmodel.NewId())
+ spaceB := mustCreateSpace(t, h.store, mmmodel.NewId())
+ userID := mmmodel.NewId()
+
+ page := publishNewPage(t, h, spaceA.Id, userID, "Doc", "x")
+
+ // The page lives in space A; querying its editors through space B must not resolve it.
+ _, appErr := h.svc.GetPageActiveEditors(page.Id, spaceB.Id)
+ require.NotNil(t, appErr)
+ require.Equal(t, http.StatusNotFound, appErr.StatusCode)
+
+ // Through its own space it resolves (empty set — no active drafts).
+ snapshot, appErr := h.svc.GetPageActiveEditors(page.Id, spaceA.Id)
+ require.Nil(t, appErr)
+ require.Empty(t, snapshot.ActiveEditors)
+}
+
+func TestActiveEditorsSurfacesHeartbeat(t *testing.T) {
+ h := openTestService(t)
+ space := mustCreateSpace(t, h.store, mmmodel.NewId())
+ userID := mmmodel.NewId()
+
+ page := publishNewPage(t, h, space.Id, userID, "Doc", "x")
+
+ // An autosave is the heartbeat; the editor must then appear as active.
+ _, appErr := h.svc.UpdatePageDraft(&model.Draft{
+ UserId: userID, SpaceId: space.Id, PageId: page.Id, Title: "Doc", Body: docWith("editing"),
+ BaseEditAt: page.EditAt,
+ }, nil, nil, nil, "")
+ require.Nil(t, appErr)
+
+ snapshot, appErr := h.svc.GetPageActiveEditors(page.Id, space.Id)
+ require.Nil(t, appErr)
+ require.Contains(t, snapshot.ActiveEditors, userID)
+ require.Positive(t, snapshot.SnapshotAt)
+ require.Equal(t, app.ActiveEditorTimeoutMs, snapshot.ActiveTimeoutMs)
+}
+
+func TestPublishSetsLastModifiedBy(t *testing.T) {
+ h := openTestService(t)
+ space := mustCreateSpace(t, h.store, mmmodel.NewId())
+ userID := mmmodel.NewId()
+
+ page := publishNewPage(t, h, space.Id, userID, "Doc", "x")
+ require.Equal(t, userID, page.LastModifiedBy, "a page published via draft must record its author as last modifier")
+}
+
+func TestPublishForceOverridesStaleBaseline(t *testing.T) {
+ h := openTestService(t)
+ space := mustCreateSpace(t, h.store, mmmodel.NewId())
+ userID := mmmodel.NewId()
+
+ page := publishNewPage(t, h, space.Id, userID, "Doc", "v1")
+ staleEditAt := page.EditAt
+
+ // Start an edit session with a draft baselined at the current EditAt; the draft persists.
+ _, appErr := h.svc.UpdatePageDraft(&model.Draft{
+ UserId: userID, SpaceId: space.Id, PageId: page.Id, Title: "Doc", Body: docWith("v3"),
+ BaseEditAt: staleEditAt,
+ }, nil, nil, nil, "")
+ require.Nil(t, appErr)
+
+ // A concurrent direct edit advances the page's EditAt, making the draft's baseline stale.
+ concurrent := docWith("concurrent")
+ concurrentPage, appErr := h.svc.UpdatePage(page.Id, space.Id, &model.PagePatch{Body: &concurrent}, new(staleEditAt), false, userID)
+ require.Nil(t, appErr)
+
+ // force=true must override the stale-baseline CAS and win with the draft's content.
+ forced, _, appErr := h.svc.PublishPageDraft(userID, space.Id, page.Id, true)
+ require.Nil(t, appErr)
+ require.Contains(t, forced.Body, "v3", "force must override a stale baseline")
+ require.Greater(t, forced.EditAt, concurrentPage.EditAt, "a force-publish must still advance the page's EditAt")
+}
+
+func TestPublishForceDoesNotRevertUntouchedField(t *testing.T) {
+ h := openTestService(t)
+ space := mustCreateSpace(t, h.store, mmmodel.NewId())
+ userID := mmmodel.NewId()
+
+ page := publishNewPage(t, h, space.Id, userID, "Original title", "original body")
+ baseEditAt := page.EditAt
+
+ // A title-only edit: the draft carries a new title but no body, baselined at the current EditAt.
+ _, appErr := h.svc.UpdatePageDraft(&model.Draft{
+ UserId: userID, SpaceId: space.Id, PageId: page.Id, Title: "New title",
+ BaseEditAt: baseEditAt,
+ }, nil, nil, nil, "")
+ require.Nil(t, appErr)
+
+ // A concurrent edit changes the BODY — a field the draft never touched — and advances EditAt.
+ concurrent := docWith("concurrent body")
+ _, appErr = h.svc.UpdatePage(page.Id, space.Id, &model.PagePatch{Body: &concurrent}, new(baseEditAt), false, userID)
+ require.Nil(t, appErr)
+
+ // Force-publish the title-only draft: force overrides the stale baseline, but only the title may
+ // be applied. The concurrent body edit must survive rather than be reverted to the pre-lock value.
+ forced, _, appErr := h.svc.PublishPageDraft(userID, space.Id, page.Id, true)
+ require.Nil(t, appErr)
+ require.Equal(t, "New title", forced.Title, "the draft's title must be applied")
+ require.Contains(t, forced.Body, "concurrent body", "a field the draft never set must not be reverted")
+}
+
+func TestUpdatePageDraftRejectsStaleBaselineAfterPublish(t *testing.T) {
+ h := openTestService(t)
+ space := mustCreateSpace(t, h.store, mmmodel.NewId())
+ userID := mmmodel.NewId()
+
+ page := publishNewPage(t, h, space.Id, userID, "Doc", "v1")
+ baseEditAt := page.EditAt
+
+ // Edit and publish: the draft is consumed and the page's EditAt advances.
+ _, appErr := h.svc.UpdatePageDraft(&model.Draft{
+ UserId: userID, SpaceId: space.Id, PageId: page.Id, Title: "Doc", Body: docWith("v2"),
+ BaseEditAt: baseEditAt,
+ }, nil, nil, nil, "")
+ require.Nil(t, appErr)
+ _, _, appErr = h.svc.PublishPageDraft(userID, space.Id, page.Id, false)
+ require.Nil(t, appErr)
+
+ // A late autosave carrying the pre-publish baseline must be rejected, not resurrect a phantom
+ // draft on the now-published page.
+ _, appErr = h.svc.UpdatePageDraft(&model.Draft{
+ UserId: userID, SpaceId: space.Id, PageId: page.Id, Title: "Doc", Body: docWith("late"),
+ BaseEditAt: baseEditAt,
+ }, nil, nil, nil, "")
+ require.NotNil(t, appErr)
+ require.Equal(t, http.StatusConflict, appErr.StatusCode)
+
+ // No phantom draft was created.
+ _, appErr = h.svc.GetPageDraft(userID, space.Id, page.Id)
+ require.NotNil(t, appErr)
+ require.Equal(t, http.StatusNotFound, appErr.StatusCode)
+}
+
+func TestUpdatePageDraftPreservesPropsOnOmit(t *testing.T) {
+ h := openTestService(t)
+ space := mustCreateSpace(t, h.store, mmmodel.NewId())
+ userID := mmmodel.NewId()
+
+ draft, appErr := h.svc.CreateSpaceDraft(userID, space.Id, "Doc", "")
+ require.Nil(t, appErr)
+ pageID := draft.PageId
+
+ // First autosave writes a props map (non-nil pointer → whole-value replace).
+ saved, appErr := h.svc.UpdatePageDraft(&model.Draft{
+ UserId: userID, SpaceId: space.Id, PageId: pageID, Title: "Doc", Body: docWith("v1"),
+ }, nil, nil, &mmmodel.StringInterface{"custom": "v"}, "")
+ require.Nil(t, appErr)
+ require.Equal(t, "v", saved.Props["custom"])
+
+ // A later autosave that omits props (nil pointer) must preserve the stored map.
+ saved, appErr = h.svc.UpdatePageDraft(&model.Draft{
+ UserId: userID, SpaceId: space.Id, PageId: pageID, Title: "Doc", Body: docWith("v2"),
+ }, nil, nil, nil, "")
+ require.Nil(t, appErr)
+ require.Equal(t, "v", saved.Props["custom"], "omitted props must preserve the stored map")
+
+ // An explicit empty map (non-nil pointer) must clear the stored props.
+ saved, appErr = h.svc.UpdatePageDraft(&model.Draft{
+ UserId: userID, SpaceId: space.Id, PageId: pageID, Title: "Doc", Body: docWith("v3"),
+ }, nil, nil, &mmmodel.StringInterface{}, "")
+ require.Nil(t, appErr)
+ require.Empty(t, saved.Props, "an explicit empty props map must clear the stored props")
+}
+
+// A draft autosave whose (valid TipTap) body exceeds PageBodyMaxBytes must be rejected with 400 —
+// the body is well-formed and small in node count, so it clears content normalization and is caught
+// by Draft.IsValid's size guard in the store, surfaced through the app layer.
+func TestUpdatePageDraftRejectsOversizedBody(t *testing.T) {
+ h := openTestService(t)
+ space := mustCreateSpace(t, h.store, mmmodel.NewId())
+ userID := mmmodel.NewId()
+
+ draft, appErr := h.svc.CreateSpaceDraft(userID, space.Id, "Doc", "")
+ require.Nil(t, appErr)
+
+ // One paragraph, one text node whose text alone exceeds PageBodyMaxBytes — clears the node/
+ // paragraph limits but overflows the serialized-body size cap.
+ oversized := docWith(strings.Repeat("x", model.PageBodyMaxBytes))
+ _, appErr = h.svc.UpdatePageDraft(&model.Draft{
+ UserId: userID, SpaceId: space.Id, PageId: draft.PageId, Title: "Doc", Body: oversized,
+ }, nil, nil, nil, "")
+ require.NotNil(t, appErr, "an oversized draft body must be rejected")
+ require.Equal(t, http.StatusBadRequest, appErr.StatusCode)
+}
+
+// Props follow PagePatch.Props write intent on publish: a new page adopts the draft's props, an
+// edit whose draft carries a non-empty props map replaces the live page's props, and an edit that
+// carries no props preserves them.
+func TestPublishCarriesDraftProps(t *testing.T) {
+ h := openTestService(t)
+ space := mustCreateSpace(t, h.store, mmmodel.NewId())
+ userID := mmmodel.NewId()
+
+ // New-page publish adopts the draft's props.
+ draft, appErr := h.svc.CreateSpaceDraft(userID, space.Id, "Doc", "")
+ require.Nil(t, appErr)
+ pageID := draft.PageId
+ _, appErr = h.svc.UpdatePageDraft(&model.Draft{
+ UserId: userID, SpaceId: space.Id, PageId: pageID, Title: "Doc", Body: docWith("v1"),
+ }, nil, nil, &mmmodel.StringInterface{"color": "blue"}, "")
+ require.Nil(t, appErr)
+
+ page, wasCreated, appErr := h.svc.PublishPageDraft(userID, space.Id, pageID, false)
+ require.Nil(t, appErr)
+ require.True(t, wasCreated)
+ require.Equal(t, "blue", page.Props["color"], "a new page must adopt the draft's props on publish")
+
+ // An edit whose draft carries a non-empty props map replaces the page's props.
+ _, appErr = h.svc.UpdatePageDraft(&model.Draft{
+ UserId: userID, SpaceId: space.Id, PageId: pageID, Title: "Doc", BaseEditAt: page.EditAt,
+ }, nil, nil, &mmmodel.StringInterface{"color": "red"}, "")
+ require.Nil(t, appErr)
+ edited, _, appErr := h.svc.PublishPageDraft(userID, space.Id, pageID, false)
+ require.Nil(t, appErr)
+ require.Equal(t, "red", edited.Props["color"], "an edit with non-empty draft props must replace the page's props")
+
+ // An edit that carries content but no props preserves the live page's props.
+ _, appErr = h.svc.UpdatePageDraft(&model.Draft{
+ UserId: userID, SpaceId: space.Id, PageId: pageID, Body: docWith("v2"), BaseEditAt: edited.EditAt,
+ }, nil, nil, nil, "")
+ require.Nil(t, appErr)
+ preserved, _, appErr := h.svc.PublishPageDraft(userID, space.Id, pageID, false)
+ require.Nil(t, appErr)
+ require.Equal(t, "red", preserved.Props["color"], "an edit carrying no props must preserve the page's props")
+ require.Contains(t, preserved.Body, "v2")
+}
+
+func TestPublishRejectsForeignSpacePage(t *testing.T) {
+ h := openTestService(t)
+ spaceA := mustCreateSpace(t, h.store, mmmodel.NewId())
+ spaceB := mustCreateSpace(t, h.store, mmmodel.NewId())
+ userA, userB := mmmodel.NewId(), mmmodel.NewId()
+
+ // userA reserves a page id in space A.
+ draftA, appErr := h.svc.CreateSpaceDraft(userA, spaceA.Id, "A doc", "")
+ require.Nil(t, appErr)
+ pageID := draftA.PageId
+
+ // userB cannot reserve a draft in space B against userA's not-yet-live page id —
+ // the cross-space reservation is now correctly rejected at the app layer.
+ _, appErr = h.svc.UpdatePageDraft(&model.Draft{UserId: userB, SpaceId: spaceB.Id, PageId: pageID, Title: "B doc", Body: docWith("b content")}, nil, nil, nil, "")
+ require.NotNil(t, appErr, "cross-space draft reservation must be rejected")
+ require.Equal(t, http.StatusNotFound, appErr.StatusCode, "cross-space draft reservation returns 404")
+
+ // userA autosaves content and publishes, so the page becomes live in space A.
+ _, appErr = h.svc.UpdatePageDraft(&model.Draft{UserId: userA, SpaceId: spaceA.Id, PageId: pageID, Title: "A doc", Body: docWith("a content")}, nil, nil, nil, "")
+ require.Nil(t, appErr)
+ pageA, wasCreated, appErr := h.svc.PublishPageDraft(userA, spaceA.Id, pageID, false)
+ require.Nil(t, appErr)
+ require.True(t, wasCreated)
+
+ // userB has no draft for pageID (the cross-space reservation was rejected above), so publishing
+ // from space B must fail with 404 (draft not found). force cannot bypass this.
+ for _, force := range []bool{false, true} {
+ _, _, appErr = h.svc.PublishPageDraft(userB, spaceB.Id, pageID, force)
+ require.NotNil(t, appErr, "cross-space publish (force=%v) must fail", force)
+ require.Equal(t, http.StatusNotFound, appErr.StatusCode,
+ "cross-space publish (force=%v) must be rejected", force)
+ }
+
+ // Space A's page is unchanged.
+ stillA, appErr := h.svc.GetPageInSpace("test", pageID, spaceA.Id, false)
+ require.Nil(t, appErr)
+ require.Contains(t, stillA.Body, "a content")
+ require.Equal(t, pageA.EditAt, stillA.EditAt, "cross-space publish must not have modified space A's page")
+}
+
+func TestUpdatePageDraftPreservesTitleOnBodyOnlyAutosave(t *testing.T) {
+ h := openTestService(t)
+ space := mustCreateSpace(t, h.store, mmmodel.NewId())
+ userID := mmmodel.NewId()
+
+ draft, appErr := h.svc.CreateSpaceDraft(userID, space.Id, "Keep Title", "")
+ require.Nil(t, appErr)
+ pageID := draft.PageId
+
+ _, appErr = h.svc.UpdatePageDraft(&model.Draft{UserId: userID, SpaceId: space.Id, PageId: pageID, Title: "Keep Title", Body: docWith("v1")}, nil, nil, nil, "")
+ require.Nil(t, appErr)
+
+ // A heartbeat that sends only the body (empty title) must not wipe the stored title.
+ saved, appErr := h.svc.UpdatePageDraft(&model.Draft{UserId: userID, SpaceId: space.Id, PageId: pageID, Body: docWith("v2")}, nil, nil, nil, "")
+ require.Nil(t, appErr)
+ require.Equal(t, "Keep Title", saved.Title, "a body-only autosave must not clear the draft title")
+}
+
+func TestUpdatePageDraftSanitizesBody(t *testing.T) {
+ h := openTestService(t)
+ space := mustCreateSpace(t, h.store, mmmodel.NewId())
+ userID := mmmodel.NewId()
+
+ draft, appErr := h.svc.CreateSpaceDraft(userID, space.Id, "Doc", "")
+ require.Nil(t, appErr)
+
+ // The autosave path must sanitize the body on the same content path as publish.
+ saved, appErr := h.svc.UpdatePageDraft(&model.Draft{
+ UserId: userID, SpaceId: space.Id, PageId: draft.PageId, Title: "Doc",
+ Body: `{"type":"doc","content":[{"type":"image","attrs":{"src":"x","onerror":"alert(document.cookie)"}}]}`,
+ }, nil, nil, nil, "")
+ require.Nil(t, appErr)
+ require.NotContains(t, saved.Body, "onerror", "autosave must sanitize the draft body")
+}
+
+// TestPublishEditIgnoresStaleParentGuard reproduces the edit-path parent-guard bug: a live child
+// page whose in-progress draft still carries a parent id that has since become non-live must still
+// publish a content-only edit, because the edit path never reparents.
+func TestPublishEditIgnoresStaleParentGuard(t *testing.T) {
+ h := openTestService(t)
+ space := mustCreateSpace(t, h.store, mmmodel.NewId())
+ userID := mmmodel.NewId()
+
+ parent := publishNewPage(t, h, space.Id, userID, "Parent", "p")
+
+ // Publish a child under the (live) parent.
+ childDraft, appErr := h.svc.CreateSpaceDraft(userID, space.Id, "Child", parent.Id)
+ require.Nil(t, appErr)
+ childID := childDraft.PageId
+ _, appErr = h.svc.UpdatePageDraft(&model.Draft{UserId: userID, SpaceId: space.Id, PageId: childID, Title: "Child", Body: docWith("c1")}, nil, nil, nil, "")
+ require.Nil(t, appErr)
+ child, _, appErr := h.svc.PublishPageDraft(userID, space.Id, childID, false)
+ require.Nil(t, appErr)
+
+ // Start an edit session whose draft still carries the (currently live) parent id.
+ parentID := parent.Id
+ _, appErr = h.svc.UpdatePageDraft(&model.Draft{
+ UserId: userID, SpaceId: space.Id, PageId: childID, Title: "Child", Body: docWith("c2"),
+ BaseEditAt: child.EditAt,
+ }, &parentID, nil, nil, "")
+ require.Nil(t, appErr)
+
+ // The parent is deleted mid-edit (its children are promoted), leaving the draft's parent id stale.
+ requireStoreDeletePage(t, h.store, parent.Id, space.Id, userID)
+
+ // A content-only edit-publish must still succeed: the edit path does not reparent.
+ republished, wasCreated, appErr := h.svc.PublishPageDraft(userID, space.Id, childID, false)
+ require.Nil(t, appErr, "edit-publish must not be blocked by a stale parent id")
+ require.False(t, wasCreated)
+ require.Contains(t, republished.Body, "c2")
+}
+
+func TestUpdatePageDraftRejectsInvalidPageID(t *testing.T) {
+ h := openTestService(t)
+ space := mustCreateSpace(t, h.store, mmmodel.NewId())
+ userID := mmmodel.NewId()
+
+ _, appErr := h.svc.UpdatePageDraft(&model.Draft{UserId: userID, SpaceId: space.Id, PageId: "not-a-valid-id", Title: "x"}, nil, nil, nil, "")
+ require.NotNil(t, appErr)
+ require.Equal(t, http.StatusBadRequest, appErr.StatusCode)
+}
+
+func TestCreateSpaceDraftRejectsForeignDraftParent(t *testing.T) {
+ h := openTestService(t)
+ space := mustCreateSpace(t, h.store, mmmodel.NewId())
+ userA, userB := mmmodel.NewId(), mmmodel.NewId()
+
+ parent, appErr := h.svc.CreateSpaceDraft(userA, space.Id, "Parent", "")
+ require.Nil(t, appErr)
+
+ // userB cannot parent a draft under userA's draft.
+ _, appErr = h.svc.CreateSpaceDraft(userB, space.Id, "Child", parent.PageId)
+ require.NotNil(t, appErr)
+ require.Equal(t, http.StatusBadRequest, appErr.StatusCode)
+
+ // userA can, and the child then publishes only after the parent does.
+ child, appErr := h.svc.CreateSpaceDraft(userA, space.Id, "Child", parent.PageId)
+ require.Nil(t, appErr)
+ _, _, appErr = h.svc.PublishPageDraft(userA, space.Id, child.PageId, false)
+ require.NotNil(t, appErr)
+ require.Equal(t, http.StatusConflict, appErr.StatusCode, "child cannot publish under an unpublished parent")
+}
+
+func TestUpdatePageDraftRejectsInvalidUserID(t *testing.T) {
+ h := openTestService(t)
+ space := mustCreateSpace(t, h.store, mmmodel.NewId())
+
+ _, appErr := h.svc.UpdatePageDraft(&model.Draft{UserId: "bad-id", SpaceId: space.Id, PageId: mmmodel.NewId()}, nil, nil, nil, "")
+ require.NotNil(t, appErr)
+ require.Equal(t, http.StatusBadRequest, appErr.StatusCode)
+}
+
+func TestUpdatePageDraftRejectsInvalidSpaceID(t *testing.T) {
+ h := openTestService(t)
+
+ _, appErr := h.svc.UpdatePageDraft(&model.Draft{UserId: mmmodel.NewId(), SpaceId: "bad-id", PageId: mmmodel.NewId()}, nil, nil, nil, "")
+ require.NotNil(t, appErr)
+ require.Equal(t, http.StatusBadRequest, appErr.StatusCode)
+}
+
+func TestCreateSpaceDraftRejectsEmptyTitle(t *testing.T) {
+ h := openTestService(t)
+ space := mustCreateSpace(t, h.store, mmmodel.NewId())
+
+ _, appErr := h.svc.CreateSpaceDraft(mmmodel.NewId(), space.Id, "", "")
+ require.NotNil(t, appErr)
+ require.Equal(t, http.StatusBadRequest, appErr.StatusCode)
+}
+
+func TestCreateSpaceDraftRejectsTitleOverCap(t *testing.T) {
+ h := openTestService(t)
+ space := mustCreateSpace(t, h.store, mmmodel.NewId())
+ longTitle := strings.Repeat("x", model.PageTitleMaxRunes+1)
+
+ _, appErr := h.svc.CreateSpaceDraft(mmmodel.NewId(), space.Id, longTitle, "")
+ require.NotNil(t, appErr)
+ require.Equal(t, http.StatusBadRequest, appErr.StatusCode)
+}
+
+func TestCreateSpaceDraftRejectsMalformedParentID(t *testing.T) {
+ h := openTestService(t)
+ space := mustCreateSpace(t, h.store, mmmodel.NewId())
+
+ _, appErr := h.svc.CreateSpaceDraft(mmmodel.NewId(), space.Id, "Doc", "not-a-valid-id")
+ require.NotNil(t, appErr)
+ require.Equal(t, http.StatusBadRequest, appErr.StatusCode)
+}
+
+// TestUpdatePageDraftRejectsResurrectionAfterNewPagePublish covers the else-if-!ok branch of the
+// resurrection guard: a late autosave with no edit baseline (new-page path) must be rejected after
+// the draft has already been consumed by publish, rather than recreating a phantom draft.
+func TestUpdatePageDraftRejectsResurrectionAfterNewPagePublish(t *testing.T) {
+ h := openTestService(t)
+ space := mustCreateSpace(t, h.store, mmmodel.NewId())
+ userID := mmmodel.NewId()
+
+ draft, appErr := h.svc.CreateSpaceDraft(userID, space.Id, "Doc", "")
+ require.Nil(t, appErr)
+ pageID := draft.PageId
+
+ _, appErr = h.svc.UpdatePageDraft(&model.Draft{UserId: userID, SpaceId: space.Id, PageId: pageID, Body: docWith("v1")}, nil, nil, nil, "")
+ require.Nil(t, appErr)
+ _, _, appErr = h.svc.PublishPageDraft(userID, space.Id, pageID, false)
+ require.Nil(t, appErr)
+
+ // Late autosave with no baseline: the draft is gone, so this must be rejected.
+ _, appErr = h.svc.UpdatePageDraft(&model.Draft{
+ UserId: userID, SpaceId: space.Id, PageId: pageID, Body: docWith("late"),
+ }, nil, nil, nil, "")
+ require.NotNil(t, appErr)
+ require.Equal(t, http.StatusConflict, appErr.StatusCode)
+
+ // No phantom draft must have been created.
+ _, appErr = h.svc.GetPageDraft(userID, space.Id, pageID)
+ require.NotNil(t, appErr)
+ require.Equal(t, http.StatusNotFound, appErr.StatusCode)
+}
+
+// TestUpdatePageDraftRejectsDraftParentCycle covers the cycle-detection guard: setting a draft's
+// parent to a page that already (transitively) points back to the draft must be rejected.
+func TestUpdatePageDraftRejectsDraftParentCycle(t *testing.T) {
+ h := openTestService(t)
+ space := mustCreateSpace(t, h.store, mmmodel.NewId())
+ userID := mmmodel.NewId()
+
+ draftA, appErr := h.svc.CreateSpaceDraft(userID, space.Id, "A", "")
+ require.Nil(t, appErr)
+ draftB, appErr := h.svc.CreateSpaceDraft(userID, space.Id, "B", "")
+ require.Nil(t, appErr)
+
+ // B → A is valid (A has no parent).
+ _, appErr = h.svc.UpdatePageDraft(&model.Draft{
+ UserId: userID, SpaceId: space.Id, PageId: draftB.PageId,
+ }, &draftA.PageId, nil, nil, "")
+ require.Nil(t, appErr)
+
+ // A → B would create A → B → A: a cycle. This must be rejected.
+ _, appErr = h.svc.UpdatePageDraft(&model.Draft{
+ UserId: userID, SpaceId: space.Id, PageId: draftA.PageId,
+ }, &draftB.PageId, nil, nil, "")
+ require.NotNil(t, appErr)
+ require.Equal(t, http.StatusBadRequest, appErr.StatusCode)
+}
+
+func TestUpdatePageDraftRejectsDraftHierarchyTooDeep(t *testing.T) {
+ h := openTestService(t)
+ space := mustCreateSpace(t, h.store, mmmodel.NewId())
+ userID := mmmodel.NewId()
+
+ // Build a chain of model.MaxPageDepth+1 drafts. The first draft is the root
+ // (no parent). Each subsequent draft sets its parent to the previous one.
+ // A chain of model.MaxPageDepth drafts fills the limit, so adding one more
+ // child is the first rejection.
+ const chainLen = model.MaxPageDepth
+ drafts := make([]*model.Draft, chainLen)
+ for i := range chainLen {
+ d, appErr := h.svc.CreateSpaceDraft(userID, space.Id, fmt.Sprintf("D%d", i), "")
+ require.Nil(t, appErr)
+ drafts[i] = d
+ }
+ for i := 1; i < chainLen; i++ {
+ _, appErr := h.svc.UpdatePageDraft(&model.Draft{
+ UserId: userID, SpaceId: space.Id, PageId: drafts[i].PageId,
+ }, &drafts[i-1].PageId, nil, nil, "")
+ require.Nil(t, appErr, "chaining draft %d under draft %d must succeed", i, i-1)
+ }
+
+ // Adding one more level (depth 11) must be rejected.
+ extra, appErr := h.svc.CreateSpaceDraft(userID, space.Id, "Extra", "")
+ require.Nil(t, appErr)
+ leaf := drafts[chainLen-1].PageId
+ _, appErr = h.svc.UpdatePageDraft(&model.Draft{
+ UserId: userID, SpaceId: space.Id, PageId: extra.PageId,
+ }, &leaf, nil, nil, "")
+ require.NotNil(t, appErr)
+ require.Equal(t, http.StatusBadRequest, appErr.StatusCode)
+}
+
+func TestDeletePageDraftReparentsChildren(t *testing.T) {
+ h := openTestService(t)
+ space := mustCreateSpace(t, h.store, mmmodel.NewId())
+ userID := mmmodel.NewId()
+
+ // Create a parent draft P and a child draft C that points to P.
+ draftP, appErr := h.svc.CreateSpaceDraft(userID, space.Id, "Parent", "")
+ require.Nil(t, appErr)
+
+ draftC, appErr := h.svc.CreateSpaceDraft(userID, space.Id, "Child", "")
+ require.Nil(t, appErr)
+
+ _, appErr = h.svc.UpdatePageDraft(&model.Draft{
+ UserId: userID, SpaceId: space.Id, PageId: draftC.PageId,
+ }, &draftP.PageId, nil, nil, "")
+ require.Nil(t, appErr)
+
+ // Discard the parent draft.
+ appErr = h.svc.DeletePageDraft(userID, space.Id, draftP.PageId, "")
+ require.Nil(t, appErr)
+
+ // The parent must be gone.
+ _, appErr = h.svc.GetPageDraft(userID, space.Id, draftP.PageId)
+ require.NotNil(t, appErr)
+ require.Equal(t, http.StatusNotFound, appErr.StatusCode)
+
+ // The child must still exist, reparented to the root (ParentId = "").
+ got, appErr := h.svc.GetPageDraft(userID, space.Id, draftC.PageId)
+ require.Nil(t, appErr)
+ require.Equal(t, "", got.ParentId, "child must be reparented to root after parent draft is discarded")
+}
+
+func TestGetPageDraftCrossSpaceReturns404(t *testing.T) {
+ h := openTestService(t)
+ spaceA := mustCreateSpace(t, h.store, mmmodel.NewId())
+ spaceB := mustCreateSpace(t, h.store, mmmodel.NewId())
+ userID := mmmodel.NewId()
+
+ draft, appErr := h.svc.CreateSpaceDraft(userID, spaceA.Id, "Doc", "")
+ require.Nil(t, appErr)
+
+ // GetPageDraft through spaceB must return 404, not expose the draft.
+ _, appErr = h.svc.GetPageDraft(userID, spaceB.Id, draft.PageId)
+ require.NotNil(t, appErr)
+ require.Equal(t, http.StatusNotFound, appErr.StatusCode)
+}
+
+func TestDeletePageDraftRejectsInvalidUserID(t *testing.T) {
+ h := openTestService(t)
+ space := mustCreateSpace(t, h.store, mmmodel.NewId())
+
+ appErr := h.svc.DeletePageDraft("bad-id", space.Id, mmmodel.NewId(), "")
+ require.NotNil(t, appErr)
+ require.Equal(t, http.StatusBadRequest, appErr.StatusCode)
+}
+
+func TestDeletePageDraftRejectsInvalidPageID(t *testing.T) {
+ h := openTestService(t)
+ space := mustCreateSpace(t, h.store, mmmodel.NewId())
+
+ appErr := h.svc.DeletePageDraft(mmmodel.NewId(), space.Id, "bad-id", "")
+ require.NotNil(t, appErr)
+ require.Equal(t, http.StatusBadRequest, appErr.StatusCode)
+}
+
+func TestPublishNewPageRejectsCrossSpaceParent(t *testing.T) {
+ h := openTestService(t)
+ spaceA := mustCreateSpace(t, h.store, mmmodel.NewId())
+ spaceB := mustCreateSpace(t, h.store, mmmodel.NewId())
+ userID := mmmodel.NewId()
+
+ // Publish a live parent in spaceA.
+ parentInA := publishNewPage(t, h, spaceA.Id, userID, "Parent", "p")
+
+ // The store rejects a draft whose ParentId points at a page in another space.
+ // To simulate the race (parent was in the same space when editing started, then moved),
+ // inject the draft row directly, bypassing the app-layer parent check.
+ pageID := mmmodel.NewId()
+ now := mmmodel.GetMillis()
+ _, err := h.db.Exec(
+ `INSERT INTO docs_draft (userid,spaceid,pageid,parentid,title,body,fileids,props,createat,updateat,lastactiveat)
+ VALUES ($1,$2,$3,$4,$5,$6,'[]','{}', $7,$7,$7)`,
+ userID, spaceB.Id, pageID, parentInA.Id, "Child", docWith("body"), now,
+ )
+ require.NoError(t, err)
+
+ // Publishing must reject: the parent lives in a different space than the draft's space. The guard
+ // collapses "cross-space" and "not a live page" into a single parent_unpublished 409 so the
+ // response can't be used to probe page ids in spaces the caller cannot read.
+ _, _, appErr := h.svc.PublishPageDraft(userID, spaceB.Id, pageID, false)
+ require.NotNil(t, appErr)
+ require.Equal(t, http.StatusConflict, appErr.StatusCode, "cross-space parent must be rejected: %v", appErr)
+ require.Equal(t, "app.page_draft.publish.parent_unpublished.app_error", appErr.Id)
+}
+
+func TestGetPageActiveEditorsRejectsInvalidPageID(t *testing.T) {
+ h := openTestService(t)
+ space := mustCreateSpace(t, h.store, mmmodel.NewId())
+
+ _, appErr := h.svc.GetPageActiveEditors("not-valid", space.Id)
+ require.NotNil(t, appErr)
+ require.Equal(t, http.StatusBadRequest, appErr.StatusCode)
+}
+
+func TestCreateSpaceDraftEnforcesQuota(t *testing.T) {
+ h := openTestService(t)
+ space := mustCreateSpace(t, h.store, mmmodel.NewId())
+ userID := mmmodel.NewId()
+
+ // Fill the user's quota by inserting rows directly; calling CreateSpaceDraft 100 times
+ // would be slow and the store enforcement path (inside UpsertDraft's transaction) is what
+ // we're testing.
+ now := mmmodel.GetMillis()
+ for i := range model.MaxDraftsPerUserPerSpace {
+ pageID := mmmodel.NewId()
+ title := fmt.Sprintf("draft-%d", i)
+ _, err := h.db.Exec(
+ `INSERT INTO docs_draft (userid,spaceid,pageid,parentid,title,body,fileids,props,createat,updateat,lastactiveat)
+ VALUES ($1,$2,$3,'',$4,'[]','[]','{}', $5,$5,$5)`,
+ userID, space.Id, pageID, title, now,
+ )
+ require.NoError(t, err)
+ }
+
+ _, appErr := h.svc.CreateSpaceDraft(userID, space.Id, "One Too Many", "")
+ require.NotNil(t, appErr)
+ // 422, not 429: the cap is a standing per-space quota, so retrying cannot clear it.
+ require.Equal(t, http.StatusUnprocessableEntity, appErr.StatusCode)
+ require.Equal(t, "app.page_draft.quota_exceeded.app_error", appErr.Id)
+}
+
+// TestGetPageDraftRejectsInvalidIDs exercises GetPageDraft's three input-validation branches at
+// the service layer, independent of HTTP routing.
+func TestGetPageDraftRejectsInvalidIDs(t *testing.T) {
+ h := openTestService(t)
+ space := mustCreateSpace(t, h.store, mmmodel.NewId())
+ userID, pageID := mmmodel.NewId(), mmmodel.NewId()
+
+ cases := []struct {
+ name string
+ userID, spaceID, pageID string
+ }{
+ {"invalid user id", "not-valid", space.Id, pageID},
+ {"invalid space id", userID, "not-valid", pageID},
+ {"invalid page id", userID, space.Id, "not-valid"},
+ }
+ for _, tc := range cases {
+ t.Run(tc.name, func(t *testing.T) {
+ _, appErr := h.svc.GetPageDraft(tc.userID, tc.spaceID, tc.pageID)
+ require.NotNil(t, appErr)
+ require.Equal(t, http.StatusBadRequest, appErr.StatusCode)
+ })
+ }
+}
+
+// TestGetPageDraftsForSpaceRejectsInvalidIDs exercises the list endpoint's service-level input
+// validation, which handler tests reach only through valid routes.
+func TestGetPageDraftsForSpaceRejectsInvalidIDs(t *testing.T) {
+ h := openTestService(t)
+ space := mustCreateSpace(t, h.store, mmmodel.NewId())
+
+ _, _, appErr := h.svc.GetPageDraftsForSpace("not-valid", space.Id, 0, 10)
+ require.NotNil(t, appErr)
+ require.Equal(t, http.StatusBadRequest, appErr.StatusCode)
+
+ _, _, appErr = h.svc.GetPageDraftsForSpace(mmmodel.NewId(), "not-valid", 0, 10)
+ require.NotNil(t, appErr)
+ require.Equal(t, http.StatusBadRequest, appErr.StatusCode)
+}
+
+func TestGetPageActiveEditorsRejectsInvalidSpaceID(t *testing.T) {
+ h := openTestService(t)
+
+ _, appErr := h.svc.GetPageActiveEditors(mmmodel.NewId(), "not-valid")
+ require.NotNil(t, appErr)
+ require.Equal(t, http.StatusBadRequest, appErr.StatusCode)
+}
+
+// TestPublishForceAppliesDraftFieldOverConcurrentEdit covers the force-publish interaction where
+// the concurrent edit touched BOTH a field the draft changed and one it did not: the draft's
+// value must win for the field it carries, while the concurrent value survives for the field the
+// draft left unset.
+func TestPublishForceAppliesDraftFieldOverConcurrentEdit(t *testing.T) {
+ h := openTestService(t)
+ space := mustCreateSpace(t, h.store, mmmodel.NewId())
+ userID := mmmodel.NewId()
+
+ page := publishNewPage(t, h, space.Id, userID, "Original title", "original body")
+ baseEditAt := page.EditAt
+
+ // A title-only edit draft baselined at the current EditAt.
+ _, appErr := h.svc.UpdatePageDraft(&model.Draft{
+ UserId: userID, SpaceId: space.Id, PageId: page.Id, Title: "Draft title",
+ BaseEditAt: baseEditAt,
+ }, nil, nil, nil, "")
+ require.Nil(t, appErr)
+
+ // A concurrent edit changes the title AND the body, advancing EditAt past the baseline.
+ concurrentTitle := "Concurrent title"
+ concurrentBody := docWith("concurrent body")
+ _, appErr = h.svc.UpdatePage(page.Id, space.Id,
+ &model.PagePatch{Title: &concurrentTitle, Body: &concurrentBody}, new(baseEditAt), false, userID)
+ require.Nil(t, appErr)
+
+ forced, _, appErr := h.svc.PublishPageDraft(userID, space.Id, page.Id, true)
+ require.Nil(t, appErr)
+ require.Equal(t, "Draft title", forced.Title, "the field the draft carries must win under force")
+ require.Contains(t, forced.Body, "concurrent body", "a field the draft never set must keep the concurrent value")
+}
+
+// TestUpdatePageDraftRejectsOversizedFileIds covers the inline fileIDs size guard: fileIDs travels
+// to the store as a separate write-intent pointer, so Draft.IsValid never sees it and this guard
+// is the only bound on the write path.
+func TestUpdatePageDraftRejectsOversizedFileIds(t *testing.T) {
+ h := openTestService(t)
+ space := mustCreateSpace(t, h.store, mmmodel.NewId())
+ userID := mmmodel.NewId()
+
+ // 12 valid ids serialize to ~349 runes, over the DraftFileIdsMaxRunes=300 cap.
+ ids := make(mmmodel.StringArray, 0, 12)
+ for range 12 {
+ ids = append(ids, mmmodel.NewId())
+ }
+
+ _, appErr := h.svc.UpdatePageDraft(&model.Draft{UserId: userID, SpaceId: space.Id, PageId: mmmodel.NewId(), Title: "x"}, nil, &ids, nil, "")
+ require.NotNil(t, appErr)
+ require.Equal(t, http.StatusBadRequest, appErr.StatusCode)
+ require.Equal(t, "model.draft.is_valid.file_ids.app_error", appErr.Id)
+}
+
+// TestUpdatePageDraftRejectsOversizedProps covers the inline props size guard: props travels to the
+// store as a separate write-intent pointer, so Draft.IsValid checks the (empty) draft.Props field
+// and this guard is the only bound on the written value.
+func TestUpdatePageDraftRejectsOversizedProps(t *testing.T) {
+ h := openTestService(t)
+ space := mustCreateSpace(t, h.store, mmmodel.NewId())
+ userID := mmmodel.NewId()
+
+ props := mmmodel.StringInterface{"k": strings.Repeat("x", model.PagePropsMaxBytes)}
+ _, appErr := h.svc.UpdatePageDraft(&model.Draft{UserId: userID, SpaceId: space.Id, PageId: mmmodel.NewId(), Title: "x"}, nil, nil, &props, "")
+ require.NotNil(t, appErr)
+ require.Equal(t, http.StatusBadRequest, appErr.StatusCode)
+ require.Equal(t, "model.shared.props_too_large.app_error", appErr.Id)
+}
+
+// TestUpdatePageDraftRejectsInvalidFileId covers the per-entry id check on the fileIDs write
+// intent, including the empty entry an empty-slice clear must not admit.
+func TestUpdatePageDraftRejectsInvalidFileId(t *testing.T) {
+ h := openTestService(t)
+ space := mustCreateSpace(t, h.store, mmmodel.NewId())
+ userID := mmmodel.NewId()
+
+ for name, entry := range map[string]string{"malformed id": "not-a-valid-id", "empty entry": ""} {
+ t.Run(name, func(t *testing.T) {
+ ids := mmmodel.StringArray{entry}
+ _, appErr := h.svc.UpdatePageDraft(&model.Draft{UserId: userID, SpaceId: space.Id, PageId: mmmodel.NewId(), Title: "x"}, nil, &ids, nil, "")
+ require.NotNil(t, appErr)
+ require.Equal(t, http.StatusBadRequest, appErr.StatusCode)
+ require.Equal(t, "model.draft.is_valid.file_id.app_error", appErr.Id)
+ })
+ }
+}
diff --git a/server/app/page_duplicate_test.go b/server/app/page_duplicate_test.go
index c7a8f5f..ffc81f6 100644
--- a/server/app/page_duplicate_test.go
+++ b/server/app/page_duplicate_test.go
@@ -12,7 +12,6 @@ import (
mmmodel "github.com/mattermost/mattermost/server/public/model"
- "github.com/mattermost/mattermost-plugin-docs/server/app"
"github.com/mattermost/mattermost-plugin-docs/server/model"
)
@@ -50,16 +49,17 @@ func TestServiceDuplicatePage(t *testing.T) {
}
// TestServiceDuplicatePage_CopiesSearchText verifies the duplicate carries the source's SearchText
-// verbatim (alongside the body) so it is immediately searchable like its source.
+// (alongside the body) so it is immediately searchable like its source. SearchText is derived
+// server-side from the body, so a caller-supplied value is ignored.
func TestServiceDuplicatePage_CopiesSearchText(t *testing.T) {
h := openTestService(t)
channelID := mmmodel.NewId()
space := mustCreateSpace(t, h.store, channelID)
userID := mmmodel.NewId()
- source, appErr := h.svc.CreatePage(space.Id, "", "Searchable", "body text", "search text", userID)
+ source, appErr := h.svc.CreatePage(space.Id, "", "Searchable", "body text", userID)
require.Nil(t, appErr)
- require.Equal(t, "search text", source.SearchText)
+ require.Equal(t, "body text", source.SearchText, "SearchText is derived from the body, not the caller-supplied value")
dup, appErr := h.svc.DuplicatePage(source.Id, space, userID, false, nil, nil)
require.Nil(t, appErr)
@@ -76,7 +76,7 @@ func TestServiceDuplicatePage_CopiesProps(t *testing.T) {
space := mustCreateSpace(t, h.store, channelID)
userID := mmmodel.NewId()
- source, appErr := h.svc.CreatePage(space.Id, "", "Has Props", "body text", "", userID)
+ source, appErr := h.svc.CreatePage(space.Id, "", "Has Props", "body text", userID)
require.Nil(t, appErr)
// "nested" round-trips through the store as a plain map[string]any (JSON decoding never
@@ -108,7 +108,7 @@ func TestServiceDuplicatePage_TruncatesLongTitle(t *testing.T) {
userID := mmmodel.NewId()
longTitle := strings.Repeat("x", model.PageTitleMaxRunes)
- source, appErr := h.svc.CreatePage(space.Id, "", longTitle, "", "", userID)
+ source, appErr := h.svc.CreatePage(space.Id, "", longTitle, "", userID)
require.Nil(t, appErr)
dup, appErr := h.svc.DuplicatePage(source.Id, space, userID, false, nil, nil)
@@ -205,7 +205,7 @@ func TestServiceDuplicatePage_MaxDepthExceeded(t *testing.T) {
// copied under it would land at (MaxPageDepth-1)+2 = MaxPageDepth+1 > MaxPageDepth.
var deepest *model.Page
parentID := ""
- for range app.MaxPageDepth {
+ for range model.MaxPageDepth {
deepest = mustCreatePage(t, h.store, space.Id, channelID, userID, parentID)
parentID = deepest.Id
}
@@ -343,7 +343,7 @@ func TestServiceDuplicatePage_IncludeChildren_MaxDepthExceeded(t *testing.T) {
// destinationDepth = (MaxPageDepth-2)+2 = MaxPageDepth — one level short of the cap.
var deepest *model.Page
parentID := ""
- for range app.MaxPageDepth - 1 {
+ for range model.MaxPageDepth - 1 {
deepest = mustCreatePage(t, h.store, space.Id, channelID, userID, parentID)
parentID = deepest.Id
}
diff --git a/server/app/page_hierarchy.go b/server/app/page_hierarchy.go
index 8c938e1..1314c06 100644
--- a/server/app/page_hierarchy.go
+++ b/server/app/page_hierarchy.go
@@ -140,7 +140,7 @@ func (s *Service) MovePage(pageID, spaceID string, newParentID *string, newIndex
// committed in between (surviving here via force) would make the earlier-read parent stale and
// point clients at the wrong subtree to invalidate.
func (s *Service) reparentWithinSpace(where, pageID, spaceID string, newParentID *string, newIndex *int64, expectedUpdateAt *int64, force bool) (*model.Page, *mmmodel.AppError) {
- moved, priorParentID, didMove, storeErr := s.store.MovePage(pageID, spaceID, newParentID, newIndex, mmmodel.SafeDereference(expectedUpdateAt), force, MaxPageDepth)
+ moved, priorParentID, didMove, storeErr := s.store.MovePage(pageID, spaceID, newParentID, newIndex, mmmodel.SafeDereference(expectedUpdateAt), force, model.MaxPageDepth)
if storeErr != nil {
return nil, storeAppError(where, storeErr)
}
@@ -161,9 +161,9 @@ func (s *Service) reparentWithinSpace(where, pageID, spaceID string, newParentID
// store.MovePageToSpace and surface through storeAppError's shared message keys.
// A nil expectedUpdateAt without force is rejected: the mutation must supply a baseline.
// sourceSpace and targetSpace are the caller's already-fetched records (from its membership
-// gates), so no re-read happens here. userID is the acting user, recorded in logs only — a
-// move does not change the page's LastModifiedBy. Per-page restrictions and redirects are not
-// handled yet.
+// gates), so no re-read happens here. userID is the acting user and must be a valid ID: the store
+// scopes the target-space draft quota to the drafts it owns. It does not change the page's
+// LastModifiedBy. Per-page restrictions and redirects are not handled yet.
func (s *Service) MovePageToSpace(pageID string, sourceSpace, targetSpace *model.Space, parentPageID *string, expectedUpdateAt *int64, force bool, userID string) (*model.Page, *mmmodel.AppError) {
if !mmmodel.IsValidId(pageID) {
return nil, mmmodel.NewAppError("MovePageToSpace", "app.page.move_to_space.invalid_id.app_error", nil, "", http.StatusBadRequest)
@@ -231,7 +231,7 @@ func (s *Service) MovePageToSpace(pageID string, sourceSpace, targetSpace *model
s.log.Debug("Moving page to space", "page_id", pageID, "source_space_id", sourceSpace.Id, "target_space_id", targetSpace.Id, "user_id", userID)
- moved, priorParentID, storeErr := s.store.MovePageToSpace(pageID, sourceSpace.Id, targetSpace.Id, parentPageID, mmmodel.SafeDereference(expectedUpdateAt), force, MaxPageDepth)
+ moved, priorParentID, storeErr := s.store.MovePageToSpace(pageID, sourceSpace.Id, targetSpace.Id, userID, parentPageID, mmmodel.SafeDereference(expectedUpdateAt), force, model.MaxPageDepth)
if storeErr != nil {
return nil, storeAppError("MovePageToSpace", storeErr)
}
diff --git a/server/app/page_move_test.go b/server/app/page_move_test.go
index 1151402..7cadf96 100644
--- a/server/app/page_move_test.go
+++ b/server/app/page_move_test.go
@@ -10,7 +10,6 @@ import (
mmmodel "github.com/mattermost/mattermost/server/public/model"
- "github.com/mattermost/mattermost-plugin-docs/server/app"
"github.com/mattermost/mattermost-plugin-docs/server/model"
"github.com/mattermost/mattermost-plugin-docs/server/store"
)
@@ -186,7 +185,7 @@ func TestServiceMovePage_ErrorPaths(t *testing.T) {
// A chain of MaxPageDepth pages: the deepest has MaxPageDepth-1 ancestors, so a leaf
// moved under it would land at depth (MaxPageDepth-1)+2 = MaxPageDepth+1 > MaxPageDepth.
- chain := buildPageChain(t, h.store, space.Id, channelID, userID, app.MaxPageDepth)
+ chain := buildPageChain(t, h.store, space.Id, channelID, userID, model.MaxPageDepth)
deepest := chain[len(chain)-1]
leaf := mustCreatePage(t, h.store, space.Id, channelID, userID, "")
@@ -206,7 +205,7 @@ func TestServiceMovePage_ErrorPaths(t *testing.T) {
// Anchor at depth MaxPageDepth-1 (MaxPageDepth-2 ancestors): a moved page lands at
// (MaxPageDepth-2)+2 = MaxPageDepth (passes the per-page check), but its one child would
// be one deeper, MaxPageDepth+1, tripping the subtree check.
- chain := buildPageChain(t, h.store, space.Id, channelID, userID, app.MaxPageDepth-1)
+ chain := buildPageChain(t, h.store, space.Id, channelID, userID, model.MaxPageDepth-1)
anchor := chain[len(chain)-1]
subtreeRoot := mustCreatePage(t, h.store, space.Id, channelID, userID, "")
diff --git a/server/app/page_move_to_space_test.go b/server/app/page_move_to_space_test.go
index c4e3a4a..a58c6cb 100644
--- a/server/app/page_move_to_space_test.go
+++ b/server/app/page_move_to_space_test.go
@@ -12,7 +12,6 @@ import (
mmmodel "github.com/mattermost/mattermost/server/public/model"
"github.com/mattermost/mattermost/server/public/plugin/plugintest"
- "github.com/mattermost/mattermost-plugin-docs/server/app"
"github.com/mattermost/mattermost-plugin-docs/server/internal/testutil"
"github.com/mattermost/mattermost-plugin-docs/server/model"
"github.com/mattermost/mattermost-plugin-docs/server/store"
@@ -206,7 +205,7 @@ func TestServiceMovePageToSpace_RejectsDepthExceeded(t *testing.T) {
// Build a chain in spaceB down to MaxPageDepth; a child under the deepest node would breach it.
parentID := ""
- for range app.MaxPageDepth {
+ for range model.MaxPageDepth {
p := mustCreatePage(t, h.store, spaceB.Id, chB, user, parentID)
parentID = p.Id
}
diff --git a/server/app/page_presence.go b/server/app/page_presence.go
new file mode 100644
index 0000000..5800e80
--- /dev/null
+++ b/server/app/page_presence.go
@@ -0,0 +1,207 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See LICENSE.txt for license information.
+
+package app
+
+import (
+ "net/http"
+
+ mmmodel "github.com/mattermost/mattermost/server/public/model"
+
+ "github.com/mattermost/mattermost-plugin-docs/server/model"
+)
+
+// ActiveEditorTimeoutMs is the window within which a draft autosave keeps a user counted as an
+// active editor. Presence is derived from the shared DOCS_Draft table: the editor's autosave is
+// the heartbeat, so an editor with a draft updated inside this window is "active". Because the
+// list comes from the master DB, it is correct across an HA cluster.
+const ActiveEditorTimeoutMs int64 = 5 * 60 * 1000
+
+// presenceBroadcastMinIntervalMs is the minimum time between autosave-triggered presence broadcasts
+// for the same page. Delete and publish paths always broadcast regardless of this interval.
+const presenceBroadcastMinIntervalMs int64 = 30 * 1000
+
+// presenceBroadcastSweepIntervalMs is the minimum time between sweeps of the broadcast rate-limit
+// map. Sweeping is opportunistic — it runs on the first autosave after the interval has elapsed — so
+// an autosave performs a full scan of the map at most once per interval.
+const presenceBroadcastSweepIntervalMs int64 = 5 * 60 * 1000
+
+func activeEditorSince() int64 {
+ return mmmodel.GetMillis() - ActiveEditorTimeoutMs
+}
+
+func presenceBroadcastKey(pageID, userID string) string {
+ return pageID + ":" + userID
+}
+
+// clearPresenceThrottle removes the (pageID, userID) broadcast rate-limit entry, so the next
+// broadcast for that editor is not suppressed and a later session starts unthrottled. Called
+// whenever a draft session ends.
+func (s *Service) clearPresenceThrottle(pageID, userID string) {
+ s.presenceBroadcastTimes.Delete(presenceBroadcastKey(pageID, userID))
+}
+
+// sweepPresenceBroadcastTimes removes stale entries from the broadcast rate-limit map to bound its
+// size. An entry is normally removed when its session ends (discard or publish); this sweep is the
+// fallback for sessions abandoned without either.
+//
+// It removes entries older than the active-editor window (ActiveEditorTimeoutMs). Removing such an
+// entry cannot change behavior: the map only suppresses a broadcast within
+// presenceBroadcastMinIntervalMs of the stored time, and an entry this old is already far past that
+// window, so the next autosave broadcasts whether or not the entry is still present. CompareAndDelete
+// removes only an entry whose value is unchanged, so one a concurrent autosave just refreshed is
+// left in place.
+func (s *Service) sweepPresenceBroadcastTimes(now int64) {
+ last := s.lastPresenceSweepAt.Load()
+ if now-last < presenceBroadcastSweepIntervalMs {
+ return
+ }
+ if !s.lastPresenceSweepAt.CompareAndSwap(last, now) {
+ return
+ }
+
+ s.presenceBroadcastTimes.Range(func(key, value any) bool {
+ if ts, ok := value.(int64); ok && now-ts >= ActiveEditorTimeoutMs {
+ s.presenceBroadcastTimes.CompareAndDelete(key, value)
+ }
+ return true
+ })
+}
+
+// getActiveEditors returns the user IDs currently editing pageID in spaceID — those with a draft
+// updated within the active-editor window. The bool is true on success and false on a store
+// failure; callers must skip the broadcast on failure to avoid publishing a spurious empty snapshot
+// that would wrongly clear a valid presence indicator. Use this only where a best-effort answer is
+// acceptable, not to back a REST read that must surface a store failure.
+func (s *Service) getActiveEditors(pageID, spaceID string) ([]string, bool) {
+ editors, err := s.store.GetPageActiveEditors(pageID, spaceID, activeEditorSince())
+ if err != nil {
+ s.log.Warn("failed to query active editors; skipping broadcast",
+ "page_id", pageID, "err", err)
+ return nil, false
+ }
+ return editors, true
+}
+
+// publishSelfPresence sends a presence snapshot to userID only. Used when the page is not yet
+// published (no channel to broadcast to), so only the author's own UI learns of the session:
+// editors is the author's own ID while the session is active, and empty when it ends.
+func (s *Service) publishSelfPresence(userID, pageID, spaceID string, editors []string) {
+ s.publishToUser(wsEventPagePresenceUpdated, map[string]any{
+ "page_id": pageID,
+ "space_id": spaceID,
+ "active_editors": editors,
+ "snapshot_at": mmmodel.GetMillis(),
+ "active_timeout_ms": ActiveEditorTimeoutMs,
+ }, userID)
+}
+
+// broadcastPagePresence fans a page_presence_updated event out to the space audience on channelID
+// (the space's backing channel), carrying the current presence snapshot.
+// Best-effort: failures are logged, never surfaced. The return value reports whether a snapshot was
+// actually published, so a throttling caller can release its claimed slot on failure; a nil client
+// (store-only unit tests) is a deliberate no-op, not a failure.
+//
+// Broadcasts fire only on user actions (autosave, discard, publish), never periodically, so a client
+// that receives no newer snapshot cannot distinguish a still-active editor from one whose session
+// ended abnormally. active_timeout_ms lets it expire the snapshot's editors on its own once
+// snapshot_at + active_timeout_ms has passed.
+func (s *Service) broadcastPagePresence(pageID, spaceID, channelID string) bool {
+ if s.client == nil {
+ return true
+ }
+ // Stamp snapshot_at before the editors query so it marks when the snapshot was taken, not when the
+ // broadcast finished assembling — clients use it to discard out-of-order snapshots.
+ snapshotAt := mmmodel.GetMillis()
+ editors, ok := s.getActiveEditors(pageID, spaceID)
+ if !ok {
+ return false
+ }
+ s.publishToChannels(wsEventPagePresenceUpdated, map[string]any{
+ "page_id": pageID,
+ "space_id": spaceID,
+ "active_editors": editors,
+ "snapshot_at": snapshotAt,
+ "active_timeout_ms": ActiveEditorTimeoutMs,
+ }, channelID)
+ return true
+}
+
+// maybeBroadcastDraftPresence performs the rate-limited presence broadcast that follows a
+// successful autosave. The rate-limit bucket is keyed per (page, user) so each editor gets an
+// independent limit — one editor's broadcast can't rate-limit another editor on the same page —
+// and it covers both audiences below, so autosave cadence cannot flood either.
+//
+// The claimed slot is released again when the channel broadcast fails (a store error while
+// assembling the snapshot), so the next autosave retries immediately instead of waiting out the
+// full throttle window on an attempt that published nothing.
+func (s *Service) maybeBroadcastDraftPresence(pageWasLive bool, pageID, userID, spaceID, channelID string) {
+ presenceKey := presenceBroadcastKey(pageID, userID)
+ now := mmmodel.GetMillis()
+ s.sweepPresenceBroadcastTimes(now)
+ existing, loaded := s.presenceBroadcastTimes.LoadOrStore(presenceKey, now)
+ if loaded {
+ lastTime, ok := existing.(int64)
+ if !ok || now-lastTime < presenceBroadcastMinIntervalMs {
+ return
+ }
+ if !s.presenceBroadcastTimes.CompareAndSwap(presenceKey, existing, now) {
+ return
+ }
+ }
+ // New-page drafts (no published page row yet) must not broadcast presence to the space channel:
+ // that would expose the reserved page ID and the author's identity to all space members before
+ // the page exists. Send the event only to the author so their own UI can track the session.
+ if !pageWasLive {
+ s.publishSelfPresence(userID, pageID, spaceID, []string{userID})
+ return
+ }
+ if !s.broadcastPagePresence(pageID, spaceID, channelID) {
+ s.presenceBroadcastTimes.CompareAndDelete(presenceKey, now)
+ }
+}
+
+// endDraftPresenceSession clears the (pageID, userID) rate-limit entry — so the announcement below
+// is never suppressed and a later session starts unthrottled — and announces the end of a draft
+// session: channel-wide when the page is live, to the author alone otherwise (an unpublished
+// new-page draft's session was never visible to the channel, so it ends the same way it was
+// announced, with an empty editor set).
+func (s *Service) endDraftPresenceSession(pageWasLive bool, pageID, userID, spaceID, channelID string) {
+ s.clearPresenceThrottle(pageID, userID)
+ if !pageWasLive {
+ s.publishSelfPresence(userID, pageID, spaceID, []string{})
+ return
+ }
+ s.broadcastPagePresence(pageID, spaceID, channelID)
+}
+
+// GetPageActiveEditors returns the editor-presence snapshot for the given page in the given space,
+// after confirming the page exists in that space. Returns 404 if the page is unknown or belongs to
+// another space; store failures are propagated (unlike the best-effort getActiveEditors, this backs
+// a REST read that must not report "nobody editing" when the query actually failed).
+func (s *Service) GetPageActiveEditors(pageID, spaceID string) (*model.PageActiveEditors, *mmmodel.AppError) {
+ if !mmmodel.IsValidId(pageID) {
+ return nil, mmmodel.NewAppError("GetPageActiveEditors", "app.page.presence.invalid_page_id.app_error", nil, "", http.StatusBadRequest)
+ }
+ if !mmmodel.IsValidId(spaceID) {
+ return nil, mmmodel.NewAppError("GetPageActiveEditors", "app.page.presence.invalid_space_id.app_error", nil, "", http.StatusBadRequest)
+ }
+ exists, err := s.store.PageExistsInSpace(pageID, spaceID)
+ if err != nil {
+ return nil, storeAppError("GetPageActiveEditors", err)
+ }
+ if !exists {
+ return nil, mmmodel.NewAppError("GetPageActiveEditors", "app.page.not_found.app_error", nil, "", http.StatusNotFound)
+ }
+ // Stamp snapshot_at before the query so it marks when the snapshot was taken, matching the WS event.
+ snapshotAt := mmmodel.GetMillis()
+ editors, storeErr := s.store.GetPageActiveEditors(pageID, spaceID, snapshotAt-ActiveEditorTimeoutMs)
+ if storeErr != nil {
+ return nil, storeAppError("GetPageActiveEditors", storeErr)
+ }
+ return &model.PageActiveEditors{
+ ActiveEditors: editors,
+ SnapshotAt: snapshotAt,
+ ActiveTimeoutMs: ActiveEditorTimeoutMs,
+ }, nil
+}
diff --git a/server/app/page_presence_test.go b/server/app/page_presence_test.go
new file mode 100644
index 0000000..4426d5b
--- /dev/null
+++ b/server/app/page_presence_test.go
@@ -0,0 +1,49 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See LICENSE.txt for license information.
+
+package app
+
+import (
+ "testing"
+
+ "github.com/stretchr/testify/require"
+)
+
+// TestActiveEditorTimeoutMsValue pins the active-editor window to its contract value. The other
+// presence tests assert response fields equal this const (verifying wiring), so this is the one
+// place that guards the value itself.
+func TestActiveEditorTimeoutMsValue(t *testing.T) {
+ require.Equal(t, int64(5*60*1000), ActiveEditorTimeoutMs)
+}
+
+// TestSweepPresenceBroadcastTimesEvictsStaleEntriesOncePerWindow verifies sweepPresenceBroadcastTimes's
+// two behaviors: it evicts entries older than ActiveEditorTimeoutMs while leaving fresh entries in
+// place, and it runs at most once per presenceBroadcastSweepIntervalMs — a second call with the same
+// `now` must be a no-op even if a new stale entry was added in between.
+func TestSweepPresenceBroadcastTimesEvictsStaleEntriesOncePerWindow(t *testing.T) {
+ svc := &Service{}
+
+ now := int64(1_000_000_000)
+ staleKey := "stale-page:stale-user"
+ freshKey := "fresh-page:fresh-user"
+ svc.presenceBroadcastTimes.Store(staleKey, now-2*ActiveEditorTimeoutMs)
+ svc.presenceBroadcastTimes.Store(freshKey, now)
+
+ // Open the gate: make the sweep consider itself overdue.
+ svc.lastPresenceSweepAt.Store(now - presenceBroadcastSweepIntervalMs)
+
+ svc.sweepPresenceBroadcastTimes(now)
+
+ _, staleStillPresent := svc.presenceBroadcastTimes.Load(staleKey)
+ require.False(t, staleStillPresent, "a stale entry must be evicted by the sweep")
+ _, freshStillPresent := svc.presenceBroadcastTimes.Load(freshKey)
+ require.True(t, freshStillPresent, "a fresh entry must survive the sweep")
+
+ // Re-seed a stale entry and call again with the same `now`: the gate must be closed (at most one
+ // sweep per ActiveEditorTimeoutMs), so this entry must NOT be evicted.
+ svc.presenceBroadcastTimes.Store(staleKey, now-2*ActiveEditorTimeoutMs)
+ svc.sweepPresenceBroadcastTimes(now)
+
+ _, stillThere := svc.presenceBroadcastTimes.Load(staleKey)
+ require.True(t, stillThere, "a second call within the sweep interval must be a no-op")
+}
diff --git a/server/app/pagination.go b/server/app/pagination.go
index afd9cb3..22f8c50 100644
--- a/server/app/pagination.go
+++ b/server/app/pagination.go
@@ -9,13 +9,7 @@ const PageMaximum = 1 << 20
// ClampPage normalizes a requested page index into [0, PageMaximum].
func ClampPage(page int) int {
- if page < 0 {
- return 0
- }
- if page > PageMaximum {
- return PageMaximum
- }
- return page
+ return min(max(page, 0), PageMaximum)
}
// PerPageDefault is the page size used when perPage is not a positive value, matching
diff --git a/server/app/service.go b/server/app/service.go
index 0ee955c..6091b95 100644
--- a/server/app/service.go
+++ b/server/app/service.go
@@ -10,6 +10,8 @@ import (
"errors"
"net/http"
"strings"
+ "sync"
+ "sync/atomic"
"unicode/utf8"
mmmodel "github.com/mattermost/mattermost/server/public/model"
@@ -37,6 +39,23 @@ type Service struct {
store *store.Store
log Logger
client *pluginapi.Client
+
+ // presenceBroadcastTimes records the last channel-wide presence broadcast time (ms) per
+ // (pageID, userID). Autosave cadence is client-driven and unbounded server-side, and every autosave
+ // on a live page would otherwise fan a presence event out to the whole channel, so this caps those
+ // broadcasts to at most one per presenceBroadcastMinIntervalMs per (page, user). Delete and publish
+ // paths bypass this and always broadcast.
+ //
+ // The map is per-process, so each node throttles independently: a user whose autosaves are
+ // spread across nodes can broadcast more often than the interval implies. That is acceptable —
+ // the payload is queried fresh from the shared DB on every broadcast, so the throttle only
+ // trades broadcast volume, never correctness. Entries are dropped on discard and publish, and
+ // swept by age via sweepPresenceBroadcastTimes for sessions abandoned without either.
+ presenceBroadcastTimes sync.Map
+
+ // lastPresenceSweepAt is the timestamp (ms) of the most recent presenceBroadcastTimes sweep, used
+ // to rate-limit the sweep itself to once per presenceBroadcastSweepIntervalMs.
+ lastPresenceSweepAt atomic.Int64
}
// New creates a Service wired to the given store, logger, and optional pluginapi client.
@@ -129,6 +148,11 @@ func storeAppError(where string, err error) *mmmodel.AppError {
return mmmodel.NewAppError(where, "app.page.max_depth_exceeded.app_error", map[string]any{"MaxDepth": limitErr.Limit}, "", http.StatusBadRequest).Wrap(err)
case store.ReasonSubtreeMaxDepthExceeded:
return mmmodel.NewAppError(where, "app.page.subtree_max_depth_exceeded.app_error", map[string]any{"MaxDepth": limitErr.Limit}, "", http.StatusBadRequest).Wrap(err)
+ case store.ReasonDraftQuotaExceeded:
+ // 422, not 429: the caller is over a standing per-space draft cap, not sending too many
+ // requests. Retrying the same request never succeeds until a draft is discarded, so a
+ // rate-limit code would send clients into a wait-and-retry loop.
+ return mmmodel.NewAppError(where, "app.page_draft.quota_exceeded.app_error", nil, "", http.StatusUnprocessableEntity).Wrap(err)
}
return mmmodel.NewAppError(where, "app.store.too_large.app_error", map[string]any{"Limit": limitErr.Limit}, "", http.StatusUnprocessableEntity).Wrap(err)
default:
@@ -143,11 +167,16 @@ func storeAppError(where string, err error) *mmmodel.AppError {
func invalidInputAppError(where string, err error) *mmmodel.AppError {
var invErr *store.ErrInvalidInput
if errors.As(err, &invErr) && invErr.Reason != "" {
- if invErr.Reason == store.ReasonParentNotLive {
+ switch invErr.Reason {
+ case store.ReasonParentNotLive:
// Same key as the app-layer parent pre-checks (validateParentExists), so a parent
// that disappears between the pre-check and the store's locked check reads
// identically to one that never existed — the contract is not race-dependent.
return mmmodel.NewAppError(where, "app.page.invalid_parent.app_error", nil, "", http.StatusBadRequest).Wrap(err)
+ case store.ReasonDraftCycle:
+ return mmmodel.NewAppError(where, "app.page_draft.update.parent_cycle.app_error", nil, "", http.StatusBadRequest).Wrap(err)
+ case store.ReasonDraftTooDeep:
+ return mmmodel.NewAppError(where, "app.page_draft.update.parent_too_deep.app_error", nil, "", http.StatusBadRequest).Wrap(err)
}
return mmmodel.NewAppError(where, invErr.Reason, nil, "", http.StatusBadRequest).Wrap(err)
}
diff --git a/server/app/service_test.go b/server/app/service_test.go
index d88abab..977ed02 100644
--- a/server/app/service_test.go
+++ b/server/app/service_test.go
@@ -106,7 +106,7 @@ func TestServiceCreatePageParentDifferentSpace(t *testing.T) {
otherSpace := mustCreateSpace(t, h.store, otherChannelID)
rogueParent := mustCreatePage(t, h.store, otherSpace.Id, otherChannelID, userID, "")
- _, err := h.svc.CreatePage(space.Id, rogueParent.Id, "Child", "", "", userID)
+ _, err := h.svc.CreatePage(space.Id, rogueParent.Id, "Child", "", userID)
require.NotNil(t, err)
require.Equal(t, http.StatusBadRequest, err.StatusCode)
require.Equal(t, "app.page.invalid_parent.app_error", err.Id)
@@ -206,10 +206,10 @@ func TestServiceUpdatePageSearchTextWithoutBody(t *testing.T) {
require.Equal(t, "model.page.patch.search_text_body_mismatch.app_error", err.Id)
}
-// TestServiceUpdatePageBodyWithoutSearchText covers the inverse: a Body change with no
-// accompanying SearchText would strand the GIN index on the page's old content, so the
-// update-path guard rejects it too (both or neither).
-func TestServiceUpdatePageBodyWithoutSearchText(t *testing.T) {
+// TestServiceUpdatePageBodyDerivesSearchText verifies that a Body-only patch succeeds and its
+// SearchText is derived server-side from the body, keeping the search index in sync without the
+// caller having to supply it.
+func TestServiceUpdatePageBodyDerivesSearchText(t *testing.T) {
h := openTestService(t)
channelID := mmmodel.NewId()
@@ -217,18 +217,16 @@ func TestServiceUpdatePageBodyWithoutSearchText(t *testing.T) {
userID := mmmodel.NewId()
created := mustCreatePage(t, h.store, space.Id, channelID, userID, "")
- _, err := h.svc.UpdatePage(
+ updated, err := h.svc.UpdatePage(
created.Id, created.SpaceId, &model.PagePatch{Body: mmmodel.NewPointer("new body")}, new(created.EditAt), false, userID,
)
- require.NotNil(t, err)
- require.Equal(t, http.StatusBadRequest, err.StatusCode)
- require.Equal(t, "model.page.patch.search_text_body_mismatch.app_error", err.Id)
+ require.Nil(t, err)
+ require.Equal(t, "new body", updated.SearchText)
}
-// TestServiceUpdatePageSearchTextWithBodyCleared verifies that clearing Body to "" while
-// setting a non-empty SearchText is rejected: SearchText is the body's plain-text projection
-// and must not survive an emptied body (mirrors the create-path rule).
-func TestServiceUpdatePageSearchTextWithBodyCleared(t *testing.T) {
+// TestServiceUpdatePageSearchTextIgnoredOnBodyClear verifies that clearing Body to "" derives an
+// empty SearchText regardless of the caller-supplied value — SearchText is the body's projection.
+func TestServiceUpdatePageSearchTextIgnoredOnBodyClear(t *testing.T) {
h := openTestService(t)
channelID := mmmodel.NewId()
@@ -236,14 +234,14 @@ func TestServiceUpdatePageSearchTextWithBodyCleared(t *testing.T) {
userID := mmmodel.NewId()
created := mustCreatePage(t, h.store, space.Id, channelID, userID, "")
- _, err := h.svc.UpdatePage(
+ updated, err := h.svc.UpdatePage(
created.Id, created.SpaceId,
- &model.PagePatch{Body: mmmodel.NewPointer(""), SearchText: mmmodel.NewPointer("some text")},
+ &model.PagePatch{Body: mmmodel.NewPointer(""), SearchText: mmmodel.NewPointer("ignored")},
new(created.EditAt), false, userID,
)
- require.NotNil(t, err)
- require.Equal(t, http.StatusBadRequest, err.StatusCode)
- require.Equal(t, "model.page.patch.search_text_without_content.app_error", err.Id)
+ require.Nil(t, err)
+ require.Equal(t, "", updated.SearchText)
+ require.Equal(t, "", updated.Body)
}
// TestServiceUpdatePageClearSearchTextAlone verifies the coupling rule: clearing SearchText
@@ -414,7 +412,7 @@ func TestServiceCreatePageDerivesChannelFromSpace(t *testing.T) {
space := mustCreateSpace(t, h.store, channelID)
userID := mmmodel.NewId()
- created, err := h.svc.CreatePage(space.Id, "", "My Page", "", "", userID)
+ created, err := h.svc.CreatePage(space.Id, "", "My Page", "", userID)
require.Nil(t, err)
require.Equal(t, "My Page", created.Title)
require.Equal(t, space.Id, created.SpaceId)
@@ -428,34 +426,34 @@ func TestServiceCreatePage(t *testing.T) {
userID := mmmodel.NewId()
t.Run("rejects invalid space id", func(t *testing.T) {
- _, err := h.svc.CreatePage("not-a-valid-id", "", "Title", "", "", userID)
+ _, err := h.svc.CreatePage("not-a-valid-id", "", "Title", "", userID)
require.NotNil(t, err)
require.Equal(t, http.StatusBadRequest, err.StatusCode)
require.Equal(t, "app.page.create.invalid_space_id.app_error", err.Id)
})
t.Run("rejects invalid user id", func(t *testing.T) {
- _, err := h.svc.CreatePage(space.Id, "", "Title", "", "", "not-a-valid-id")
+ _, err := h.svc.CreatePage(space.Id, "", "Title", "", "not-a-valid-id")
require.NotNil(t, err)
require.Equal(t, http.StatusBadRequest, err.StatusCode)
require.Equal(t, "app.page.create.invalid_user_id.app_error", err.Id)
})
t.Run("rejects empty title", func(t *testing.T) {
- _, err := h.svc.CreatePage(space.Id, "", " ", "", "", userID)
+ _, err := h.svc.CreatePage(space.Id, "", " ", "", userID)
require.NotNil(t, err)
require.Equal(t, 400, err.StatusCode)
})
t.Run("rejects title too long", func(t *testing.T) {
long := strings.Repeat("x", model.PageTitleMaxRunes+1)
- _, err := h.svc.CreatePage(space.Id, "", long, "", "", userID)
+ _, err := h.svc.CreatePage(space.Id, "", long, "", userID)
require.NotNil(t, err)
require.Equal(t, 400, err.StatusCode)
})
t.Run("rejects nonexistent parent", func(t *testing.T) {
- _, err := h.svc.CreatePage(space.Id, mmmodel.NewId(), "Title", "", "", userID)
+ _, err := h.svc.CreatePage(space.Id, mmmodel.NewId(), "Title", "", userID)
require.NotNil(t, err)
require.Equal(t, 400, err.StatusCode)
})
@@ -464,7 +462,7 @@ func TestServiceCreatePage(t *testing.T) {
otherChannelID := mmmodel.NewId()
otherSpace := mustCreateSpace(t, h.store, otherChannelID)
parent := mustCreatePage(t, h.store, otherSpace.Id, otherChannelID, userID, "")
- _, err := h.svc.CreatePage(space.Id, parent.Id, "Title", "", "", userID)
+ _, err := h.svc.CreatePage(space.Id, parent.Id, "Title", "", userID)
require.NotNil(t, err)
require.Equal(t, 400, err.StatusCode)
})
@@ -476,18 +474,18 @@ func TestServiceCreatePage(t *testing.T) {
// Build a full-depth chain (root at depth 1 up to MaxPageDepth); the next
// child would be at depth MaxPageDepth+1 and must be rejected.
parentID := ""
- for range app.MaxPageDepth {
- p, err := h.svc.CreatePage(depthSpace.Id, parentID, "d", "", "", userID)
+ for range model.MaxPageDepth {
+ p, err := h.svc.CreatePage(depthSpace.Id, parentID, "d", "", userID)
require.Nil(t, err)
parentID = p.Id
}
- _, err := h.svc.CreatePage(depthSpace.Id, parentID, "too deep", "", "", userID)
+ _, err := h.svc.CreatePage(depthSpace.Id, parentID, "too deep", "", userID)
require.NotNil(t, err)
require.Equal(t, 400, err.StatusCode)
})
t.Run("creates a valid page", func(t *testing.T) {
- created, err := h.svc.CreatePage(space.Id, "", "My Page", "", "", userID)
+ created, err := h.svc.CreatePage(space.Id, "", "My Page", "", userID)
require.Nil(t, err)
require.Equal(t, "My Page", created.Title)
})
@@ -542,12 +540,12 @@ func TestServiceUpdatePageOversizedBody(t *testing.T) {
_, err := h.svc.UpdatePage(created.Id, created.SpaceId, &model.PagePatch{Body: mmmodel.NewPointer(oversized), SearchText: mmmodel.NewPointer("")}, new(created.EditAt), false, mmmodel.NewId())
require.NotNil(t, err)
require.Equal(t, http.StatusBadRequest, err.StatusCode)
- require.Equal(t, "model.page.is_valid.body.app_error", err.Id)
+ require.Equal(t, "app.page.invalid_content.app_error", err.Id)
}
-// TestServiceUpdatePageOversizedSearchText verifies that the update path rejects oversized
-// searchText with 400.
-func TestServiceUpdatePageOversizedSearchText(t *testing.T) {
+// TestServiceUpdatePageOversizedSearchTextIgnored verifies a caller-supplied oversized SearchText
+// is harmless: it is ignored and SearchText is derived from the (small) body instead.
+func TestServiceUpdatePageOversizedSearchTextIgnored(t *testing.T) {
h := openTestService(t)
channelID := mmmodel.NewId()
@@ -555,10 +553,9 @@ func TestServiceUpdatePageOversizedSearchText(t *testing.T) {
created := mustCreatePage(t, h.store, space.Id, channelID, mmmodel.NewId(), "")
oversized := strings.Repeat("x", model.PageSearchTextMaxBytes+1)
- _, err := h.svc.UpdatePage(created.Id, created.SpaceId, &model.PagePatch{Title: mmmodel.NewPointer("Title"), Body: mmmodel.NewPointer("body"), SearchText: mmmodel.NewPointer(oversized)}, new(created.EditAt), false, mmmodel.NewId())
- require.NotNil(t, err)
- require.Equal(t, http.StatusBadRequest, err.StatusCode)
- require.Equal(t, "model.page.is_valid.search_text.app_error", err.Id)
+ updated, err := h.svc.UpdatePage(created.Id, created.SpaceId, &model.PagePatch{Title: mmmodel.NewPointer("Title"), Body: mmmodel.NewPointer("body"), SearchText: mmmodel.NewPointer(oversized)}, new(created.EditAt), false, mmmodel.NewId())
+ require.Nil(t, err)
+ require.Equal(t, "body", updated.SearchText)
}
// TestServiceCreatePageOversizedBody verifies that CreatePage rejects a body that exceeds
@@ -570,10 +567,10 @@ func TestServiceCreatePageOversizedBody(t *testing.T) {
space := mustCreateSpace(t, h.store, channelID)
oversized := strings.Repeat("x", model.PageBodyMaxBytes+1)
- _, err := h.svc.CreatePage(space.Id, "", "Title", oversized, "", mmmodel.NewId())
+ _, err := h.svc.CreatePage(space.Id, "", "Title", oversized, mmmodel.NewId())
require.NotNil(t, err)
require.Equal(t, http.StatusBadRequest, err.StatusCode)
- require.Equal(t, "model.page.is_valid.body.app_error", err.Id)
+ require.Equal(t, "app.page.invalid_content.app_error", err.Id)
}
// TestServiceUpdatePageCanSetEmptyBody verifies the patch contract: a non-nil empty Body
@@ -599,18 +596,17 @@ func TestServiceUpdatePageCanSetEmptyBody(t *testing.T) {
require.Equal(t, "", emptied.Body, "an explicit empty Body must clear the stored body")
}
-// TestServiceCreatePageSearchTextWithoutBody verifies CreatePage rejects searchText
-// supplied without a body, matching the update path's rule.
-func TestServiceCreatePageSearchTextWithoutBody(t *testing.T) {
+// TestServiceCreatePageSearchTextDerivedFromBody verifies CreatePage ignores a caller-supplied
+// SearchText and derives it from the body — so an empty body yields an empty SearchText.
+func TestServiceCreatePageSearchTextDerivedFromBody(t *testing.T) {
h := openTestService(t)
channelID := mmmodel.NewId()
space := mustCreateSpace(t, h.store, channelID)
- _, err := h.svc.CreatePage(space.Id, "", "Title", "", "searchtext", mmmodel.NewId())
- require.NotNil(t, err)
- require.Equal(t, http.StatusBadRequest, err.StatusCode)
- require.Equal(t, "app.page.create.search_text_without_content.app_error", err.Id)
+ created, err := h.svc.CreatePage(space.Id, "", "Title", "", mmmodel.NewId())
+ require.Nil(t, err)
+ require.Equal(t, "", created.SearchText)
}
// TestServiceGetPageWithDeleted verifies the with-deleted reader returns a soft-deleted
diff --git a/server/app/ws_events.go b/server/app/ws_events.go
index 3053b08..62b0f8a 100644
--- a/server/app/ws_events.go
+++ b/server/app/ws_events.go
@@ -34,6 +34,11 @@ const (
wsEventPageMoved = "page_moved"
wsEventPageDuplicated = "page_duplicated"
wsEventPageMovedToSpace = "page_moved_to_space"
+ // Unlike the page_* events above — refetch signals carrying only the ids the client needs to
+ // refetch — wsEventPagePresenceUpdated carries the full presence snapshot inline
+ // ({page_id, space_id, active_editors, snapshot_at, active_timeout_ms}), so clients need no
+ // follow-up fetch.
+ wsEventPagePresenceUpdated = "page_presence_updated"
wsEventSpaceCreated = "space_created"
wsEventSpaceUpdated = "space_updated"
@@ -50,12 +55,12 @@ func (s *Service) publishToChannels(event string, payload map[string]any, channe
if s.client == nil {
return
}
- seen := make(map[string]bool, len(channelIDs))
+ seen := make(map[string]struct{}, len(channelIDs))
for _, chID := range channelIDs {
- if chID == "" || seen[chID] {
+ if _, ok := seen[chID]; chID == "" || ok {
continue
}
- seen[chID] = true
+ seen[chID] = struct{}{}
s.client.Frontend.PublishWebSocketEvent(event, payload, &mmmodel.WebsocketBroadcast{ChannelId: chID})
}
}
diff --git a/server/app/ws_events_test.go b/server/app/ws_events_test.go
index afd7b33..262a4b9 100644
--- a/server/app/ws_events_test.go
+++ b/server/app/ws_events_test.go
@@ -8,6 +8,7 @@
package app_test
import (
+ "slices"
"testing"
"github.com/stretchr/testify/mock"
@@ -173,6 +174,300 @@ func TestServiceMovePageToSpace_NoOpPublishesNothing(t *testing.T) {
mockAPI.AssertNotCalled(t, "PublishWebSocketEvent", "page_moved_to_space", mock.Anything, mock.Anything)
}
+// TestServiceUpdatePageDraft_PublishesPresenceEvent pins page_presence_updated: the presence-snapshot
+// payload ({page_id, space_id, active_editors, snapshot_at}) — unlike the other page_* events, which carry
+// only {page_id, space_id} as a change signal — broadcast to the space's backing channel. An autosave
+// is the heartbeat, so the saving user appears in active_editors.
+func TestServiceUpdatePageDraft_PublishesPresenceEvent(t *testing.T) {
+ mockAPI := &plugintest.API{}
+ h := openTestServiceWithAPI(t, mockAPI)
+
+ channelID := mmmodel.NewId()
+ userID := mmmodel.NewId()
+ space := mustCreateSpace(t, h.store, channelID)
+
+ // Use a published page so UpdatePageDraft takes the channel-broadcast path.
+ page := publishNewPage(t, h, space.Id, userID, "Doc", "v1")
+
+ _, appErr := h.svc.UpdatePageDraft(&model.Draft{
+ UserId: userID, SpaceId: space.Id, PageId: page.Id, Title: "Doc",
+ BaseEditAt: page.EditAt,
+ }, nil, nil, nil, channelID)
+ require.Nil(t, appErr)
+
+ mockAPI.AssertCalled(t, "PublishWebSocketEvent", "page_presence_updated",
+ mock.MatchedBy(func(payload map[string]any) bool {
+ editors, ok := payload["active_editors"].([]string)
+ return ok &&
+ payload["page_id"] == page.Id &&
+ payload["space_id"] == space.Id &&
+ payload["snapshot_at"] != nil &&
+ payload["active_timeout_ms"] == app.ActiveEditorTimeoutMs &&
+ slices.Contains(editors, userID)
+ }),
+ &mmmodel.WebsocketBroadcast{ChannelId: channelID})
+}
+
+// TestServiceUpdatePageDraft_PresenceBroadcastThrottled pins the autosave presence rate limit
+// end-to-end: repeated autosaves for the same (page, user) inside presenceBroadcastMinIntervalMs
+// broadcast page_presence_updated exactly once — the first autosave claims the throttle slot and
+// the rest are suppressed. (Discard and publish bypass the throttle; they are pinned elsewhere.)
+func TestServiceUpdatePageDraft_PresenceBroadcastThrottled(t *testing.T) {
+ mockAPI := &plugintest.API{}
+ h := openTestServiceWithAPI(t, mockAPI)
+
+ channelID := mmmodel.NewId()
+ userID := mmmodel.NewId()
+ space := mustCreateSpace(t, h.store, channelID)
+ page := publishNewPage(t, h, space.Id, userID, "Doc", "v1")
+
+ presenceBroadcasts := func() int {
+ n := 0
+ for _, c := range mockAPI.Calls {
+ if c.Method == "PublishWebSocketEvent" && len(c.Arguments) > 0 && c.Arguments[0] == "page_presence_updated" {
+ n++
+ }
+ }
+ return n
+ }
+
+ // publishNewPage ends the draft session (broadcasting presence and clearing the throttle), so
+ // count deltas from here rather than absolute totals.
+ before := presenceBroadcasts()
+ for range 3 {
+ _, appErr := h.svc.UpdatePageDraft(&model.Draft{
+ UserId: userID, SpaceId: space.Id, PageId: page.Id, Title: "Doc",
+ BaseEditAt: page.EditAt,
+ }, nil, nil, nil, channelID)
+ require.Nil(t, appErr)
+ }
+ require.Equal(t, before+1, presenceBroadcasts(),
+ "autosaves within the throttle window must broadcast presence exactly once")
+}
+
+// TestServicePublishPageDraft_PublishesCreatedEvent pins that publishing a brand-new page's draft
+// reuses page_created (not a draft-specific event): {page_id, space_id, parent_id} payload,
+// broadcast to the new page's backing channel. Also pins the accompanying presence-clear broadcast.
+func TestServicePublishPageDraft_PublishesCreatedEvent(t *testing.T) {
+ mockAPI := &plugintest.API{}
+ h := openTestServiceWithAPI(t, mockAPI)
+
+ channelID := mmmodel.NewId()
+ userID := mmmodel.NewId()
+ space := mustCreateSpace(t, h.store, channelID)
+
+ page := publishNewPage(t, h, space.Id, userID, "Doc", "hello")
+
+ mockAPI.AssertCalled(t, "PublishWebSocketEvent", "page_created",
+ map[string]any{"page_id": page.Id, "space_id": space.Id, "parent_id": page.ParentId},
+ &mmmodel.WebsocketBroadcast{ChannelId: channelID})
+
+ // The store publish transaction deletes the draft itself, bypassing DeletePageDraft, so PublishPageDraft broadcasts presence directly.
+ mockAPI.AssertCalled(t, "PublishWebSocketEvent", "page_presence_updated",
+ mock.MatchedBy(func(payload map[string]any) bool {
+ editors, ok := payload["active_editors"].([]string)
+ return ok &&
+ payload["page_id"] == page.Id &&
+ payload["space_id"] == space.Id &&
+ payload["snapshot_at"] != nil &&
+ len(editors) == 0
+ }),
+ &mmmodel.WebsocketBroadcast{ChannelId: channelID})
+}
+
+// TestServicePublishPageDraft_PublishesUpdatedEvent pins that publishing an edit to an existing page
+// reuses page_updated: {page_id, space_id} payload, broadcast to the page's backing channel.
+// Also pins the accompanying presence-clear broadcast.
+func TestServicePublishPageDraft_PublishesUpdatedEvent(t *testing.T) {
+ mockAPI := &plugintest.API{}
+ h := openTestServiceWithAPI(t, mockAPI)
+
+ channelID := mmmodel.NewId()
+ userID := mmmodel.NewId()
+ space := mustCreateSpace(t, h.store, channelID)
+ page := publishNewPage(t, h, space.Id, userID, "Doc", "original")
+
+ // Start an edit session against the live page's baseline, then publish it.
+ _, appErr := h.svc.UpdatePageDraft(&model.Draft{
+ UserId: userID, SpaceId: space.Id, PageId: page.Id, Title: "Doc", Body: docWith("edited"),
+ BaseEditAt: page.EditAt,
+ }, nil, nil, nil, "")
+ require.Nil(t, appErr)
+
+ republished, wasCreated, appErr := h.svc.PublishPageDraft(userID, space.Id, page.Id, false)
+ require.Nil(t, appErr)
+ require.False(t, wasCreated, "publishing an edit to an existing page must report wasCreated=false")
+
+ mockAPI.AssertCalled(t, "PublishWebSocketEvent", "page_updated",
+ map[string]any{"page_id": republished.Id, "space_id": space.Id},
+ &mmmodel.WebsocketBroadcast{ChannelId: channelID})
+
+ // The store publish transaction deletes the draft itself, bypassing DeletePageDraft, so PublishPageDraft broadcasts presence directly.
+ mockAPI.AssertCalled(t, "PublishWebSocketEvent", "page_presence_updated",
+ mock.MatchedBy(func(payload map[string]any) bool {
+ editors, ok := payload["active_editors"].([]string)
+ return ok &&
+ payload["page_id"] == republished.Id &&
+ payload["space_id"] == space.Id &&
+ payload["snapshot_at"] != nil &&
+ payload["active_timeout_ms"] == app.ActiveEditorTimeoutMs &&
+ len(editors) == 0
+ }),
+ &mmmodel.WebsocketBroadcast{ChannelId: channelID})
+}
+
+// TestServiceDeletePageDraft_PublishesPresenceEvent pins that discarding a draft broadcasts
+// page_presence_updated so the active-editors indicator drops the user; after the discard the
+// snapshot is the empty set ([] not null).
+func TestServiceDeletePageDraft_PublishesPresenceEvent(t *testing.T) {
+ mockAPI := &plugintest.API{}
+ h := openTestServiceWithAPI(t, mockAPI)
+
+ channelID := mmmodel.NewId()
+ userID := mmmodel.NewId()
+ space := mustCreateSpace(t, h.store, channelID)
+
+ // Use a published page so the edit-draft delete takes the channel-broadcast path.
+ page := publishNewPage(t, h, space.Id, userID, "Doc", "v1")
+
+ _, appErr := h.svc.UpdatePageDraft(&model.Draft{
+ UserId: userID, SpaceId: space.Id, PageId: page.Id, Title: "Doc",
+ BaseEditAt: page.EditAt,
+ }, nil, nil, nil, channelID)
+ require.Nil(t, appErr)
+
+ require.Nil(t, h.svc.DeletePageDraft(userID, space.Id, page.Id, channelID))
+
+ mockAPI.AssertCalled(t, "PublishWebSocketEvent", "page_presence_updated",
+ mock.MatchedBy(func(payload map[string]any) bool {
+ editors, ok := payload["active_editors"].([]string)
+ return ok &&
+ payload["page_id"] == page.Id &&
+ payload["space_id"] == space.Id &&
+ payload["snapshot_at"] != nil &&
+ payload["active_timeout_ms"] == app.ActiveEditorTimeoutMs &&
+ len(editors) == 0
+ }),
+ &mmmodel.WebsocketBroadcast{ChannelId: channelID})
+}
+
+// TestServiceUpdatePageDraft_NewPageDraftPublishesToUserOnly pins that a draft for a not-yet-published
+// page broadcasts presence only to the author — never to the space channel, which would leak the
+// reserved page ID and author identity before the page exists.
+func TestServiceUpdatePageDraft_NewPageDraftPublishesToUserOnly(t *testing.T) {
+ mockAPI := &plugintest.API{}
+ h := openTestServiceWithAPI(t, mockAPI)
+
+ channelID := mmmodel.NewId()
+ userID := mmmodel.NewId()
+ space := mustCreateSpace(t, h.store, channelID)
+
+ // Create a new-page draft — no published page row exists yet.
+ draft, appErr := h.svc.CreateSpaceDraft(userID, space.Id, "Unpublished", "")
+ require.Nil(t, appErr)
+
+ // Reset call log so only the following UpdatePageDraft broadcast is observed.
+ mockAPI.Calls = nil
+
+ _, appErr = h.svc.UpdatePageDraft(&model.Draft{
+ UserId: userID, SpaceId: space.Id, PageId: draft.PageId, Title: "Unpublished",
+ }, nil, nil, nil, channelID)
+ require.Nil(t, appErr)
+
+ // The broadcast must be user-scoped: only the author learns about their own unreleased page.
+ mockAPI.AssertCalled(t, "PublishWebSocketEvent", "page_presence_updated",
+ mock.MatchedBy(func(payload map[string]any) bool {
+ return payload["page_id"] == draft.PageId && payload["space_id"] == space.Id &&
+ payload["active_timeout_ms"] == app.ActiveEditorTimeoutMs
+ }),
+ &mmmodel.WebsocketBroadcast{UserId: userID})
+
+ // Must not broadcast to the channel, which would expose the reserved page ID.
+ mockAPI.AssertNotCalled(t, "PublishWebSocketEvent", "page_presence_updated",
+ mock.Anything,
+ &mmmodel.WebsocketBroadcast{ChannelId: channelID})
+}
+
+// TestServiceDeletePageDraft_NewPageDraftClearsSelfPresenceOnly pins that discarding a draft for a
+// not-yet-published page clears presence the same way the session was announced: an empty snapshot
+// sent to the author only, never to the space channel (the session was never visible there).
+func TestServiceDeletePageDraft_NewPageDraftClearsSelfPresenceOnly(t *testing.T) {
+ mockAPI := &plugintest.API{}
+ h := openTestServiceWithAPI(t, mockAPI)
+
+ channelID := mmmodel.NewId()
+ userID := mmmodel.NewId()
+ space := mustCreateSpace(t, h.store, channelID)
+
+ // Create a new-page draft — no published page row exists yet.
+ draft, appErr := h.svc.CreateSpaceDraft(userID, space.Id, "Unpublished", "")
+ require.Nil(t, appErr)
+
+ // Reset call log so only the discard's broadcast is observed.
+ mockAPI.Calls = nil
+
+ require.Nil(t, h.svc.DeletePageDraft(userID, space.Id, draft.PageId, channelID))
+
+ // The clear must be user-scoped and carry the empty editor set ([] not null).
+ mockAPI.AssertCalled(t, "PublishWebSocketEvent", "page_presence_updated",
+ mock.MatchedBy(func(payload map[string]any) bool {
+ editors, ok := payload["active_editors"].([]string)
+ return ok &&
+ payload["page_id"] == draft.PageId &&
+ payload["space_id"] == space.Id &&
+ payload["snapshot_at"] != nil &&
+ payload["active_timeout_ms"] == app.ActiveEditorTimeoutMs &&
+ len(editors) == 0
+ }),
+ &mmmodel.WebsocketBroadcast{UserId: userID})
+
+ // Must not broadcast to the channel: the draft was never visible there.
+ mockAPI.AssertNotCalled(t, "PublishWebSocketEvent", "page_presence_updated",
+ mock.Anything,
+ &mmmodel.WebsocketBroadcast{ChannelId: channelID})
+}
+
+// TestServiceUpdatePageDraft_PresenceRateLimitSuppressesSecondBroadcast verifies that a second
+// autosave within presenceBroadcastMinIntervalMs does not trigger a second channel broadcast.
+// The rate-limit prevents flooding the channel on every keystroke.
+func TestServiceUpdatePageDraft_PresenceRateLimitSuppressesSecondBroadcast(t *testing.T) {
+ mockAPI := &plugintest.API{}
+ h := openTestServiceWithAPI(t, mockAPI)
+
+ channelID := mmmodel.NewId()
+ userID := mmmodel.NewId()
+ space := mustCreateSpace(t, h.store, channelID)
+ page := publishNewPage(t, h, space.Id, userID, "Doc", "v1")
+
+ // Reset call log so only the two autosaves below are counted.
+ mockAPI.Calls = nil
+
+ autosave := func() {
+ _, appErr := h.svc.UpdatePageDraft(&model.Draft{
+ UserId: userID, SpaceId: space.Id, PageId: page.Id, Title: "Doc",
+ BaseEditAt: page.EditAt,
+ }, nil, nil, nil, channelID)
+ require.Nil(t, appErr)
+ }
+ autosave() // first: should broadcast
+ autosave() // second: rate-limited, must not broadcast again
+
+ // Count channel-scoped page_presence_updated calls.
+ n := 0
+ for _, call := range mockAPI.Calls {
+ if call.Method != "PublishWebSocketEvent" || len(call.Arguments) < 3 {
+ continue
+ }
+ if call.Arguments[0] != "page_presence_updated" {
+ continue
+ }
+ if bc, ok := call.Arguments[2].(*mmmodel.WebsocketBroadcast); ok && bc.ChannelId != "" {
+ n++
+ }
+ }
+ require.Equal(t, 1, n, "second autosave within rate-limit window must not broadcast presence to channel again")
+}
+
// TestServiceCreateSpace_PublishesCreatedEvent pins space_created: space-id payload, broadcast
// scoped to the backing channel (the visibility boundary the REST API enforces; the team space
// list is filtered to the caller's channel memberships).
diff --git a/server/model/draft.go b/server/model/draft.go
index 592de7d..9ad4400 100644
--- a/server/model/draft.go
+++ b/server/model/draft.go
@@ -14,6 +14,10 @@ import (
// DraftFileIdsMaxRunes is the maximum rune length of the serialized FileIds JSON array.
const DraftFileIdsMaxRunes = 300
+// MaxDraftsPerUserPerSpace is the maximum number of draft rows a single user may hold in one
+// space.
+const MaxDraftsPerUserPerSpace = 100
+
// Draft is a per-user autosave draft for a space page, stored in DOCS_Draft.
//
// A draft is keyed by (UserId, PageId): PageId is the page id reserved when the
@@ -22,17 +26,46 @@ const DraftFileIdsMaxRunes = 300
// same key. An orphan draft — one whose PageId has no matching DOCS_Page row — is legal,
// since the page has not been published yet. ParentId carries the pending hierarchy
// parent for a new page; Body holds the raw (opaque) editor content.
+//
+// LastActiveAt is distinct from UpdateAt: it moves only when the user themselves saves the draft,
+// whereas UpdateAt also moves when a bulk maintenance write touches the row. Editor presence is
+// derived from LastActiveAt, so only real authoring activity counts as editing.
type Draft struct {
- UserId string `json:"user_id"`
- SpaceId string `json:"space_id"`
- PageId string `json:"page_id"`
- ParentId string `json:"parent_id"`
- Title string `json:"title"`
- Body string `json:"body"`
- FileIds mmmodel.StringArray `json:"file_ids"`
- Props mmmodel.StringInterface `json:"props"`
- CreateAt int64 `json:"create_at"`
- UpdateAt int64 `json:"update_at"`
+ UserId string `json:"user_id"`
+ SpaceId string `json:"space_id"`
+ PageId string `json:"page_id"`
+ ParentId string `json:"parent_id"`
+ Title string `json:"title"`
+ Body string `json:"body"`
+ FileIds mmmodel.StringArray `json:"file_ids"`
+ Props mmmodel.StringInterface `json:"props"`
+ CreateAt int64 `json:"create_at"`
+ UpdateAt int64 `json:"update_at"`
+ LastActiveAt int64 `json:"last_active_at"`
+ // BaseEditAt is the optimistic-lock (CAS) baseline: the page EditAt the client saw when it opened
+ // this page for editing, compared against the page's current EditAt on publish to reject a
+ // concurrent-edit conflict. Write-once — frozen at the SQL layer on conflict (see
+ // Store.UpsertDraft), so unlike the mutable Page.EditAt it never changes once established. 0 means
+ // no baseline: either a new-page draft or an existing-page edit whose baseline was never captured
+ // (fails closed to a forced publish). "New page" is decided by page existence, never by
+ // BaseEditAt == 0.
+ BaseEditAt int64 `json:"base_edit_at"`
+}
+
+// DraftSummary is the metadata projection returned by draft collection endpoints. It deliberately
+// omits Body and Props: Body can be up to PageBodyMaxBytes per draft, and Props is opaque and may
+// be up to PagePropsMaxBytes — matching the fields PageSummary omits for the same reason. Fetch a
+// Draft by page id when the content is required.
+type DraftSummary struct {
+ UserId string `json:"user_id"`
+ SpaceId string `json:"space_id"`
+ PageId string `json:"page_id"`
+ ParentId string `json:"parent_id"`
+ Title string `json:"title"`
+ FileIds mmmodel.StringArray `json:"file_ids"`
+ CreateAt int64 `json:"create_at"`
+ UpdateAt int64 `json:"update_at"`
+ LastActiveAt int64 `json:"last_active_at"`
}
// PreSave sanitizes Draft and defaults its Id-independent fields before insert.
@@ -53,20 +86,23 @@ func (d *Draft) PreSave() {
d.CreateAt = now
}
d.UpdateAt = now
+ d.LastActiveAt = now
}
// Auditable returns Draft's fields safe to include in an audit log, excluding Body.
func (d *Draft) Auditable() map[string]any {
return map[string]any{
- "user_id": d.UserId,
- "space_id": d.SpaceId,
- "page_id": d.PageId,
- "parent_id": d.ParentId,
- "title": d.Title,
- "file_ids": d.FileIds,
- "props": d.GetProps(),
- "create_at": d.CreateAt,
- "update_at": d.UpdateAt,
+ "user_id": d.UserId,
+ "space_id": d.SpaceId,
+ "page_id": d.PageId,
+ "parent_id": d.ParentId,
+ "title": d.Title,
+ "file_ids": d.FileIds,
+ "props": d.GetProps(),
+ "create_at": d.CreateAt,
+ "update_at": d.UpdateAt,
+ "last_active_at": d.LastActiveAt,
+ "base_edit_at": d.BaseEditAt,
}
}
@@ -75,11 +111,9 @@ func (d *Draft) IsValid() *mmmodel.AppError {
if !mmmodel.IsValidId(d.UserId) {
return mmmodel.NewAppError("Draft.IsValid", "model.draft.is_valid.user_id.app_error", nil, "user_id="+d.UserId, http.StatusBadRequest)
}
-
if !mmmodel.IsValidId(d.SpaceId) {
return mmmodel.NewAppError("Draft.IsValid", "model.draft.is_valid.space_id.app_error", nil, "space_id="+d.SpaceId, http.StatusBadRequest)
}
-
if !mmmodel.IsValidId(d.PageId) {
return mmmodel.NewAppError("Draft.IsValid", "model.draft.is_valid.page_id.app_error", nil, "page_id="+d.PageId, http.StatusBadRequest)
}
@@ -101,6 +135,20 @@ func (d *Draft) IsValid() *mmmodel.AppError {
return mmmodel.NewAppError("Draft.IsValid", "model.draft.is_valid.update_at.app_error", nil, "page_id="+d.PageId, http.StatusBadRequest)
}
+ // Write-path contract only: PreSave always stamps LastActiveAt, so a zero here means the
+ // caller skipped PreSave. Stored rows can still legitimately hold 0 — bulk maintenance writes
+ // (cross-space move) reset LastActiveAt in SQL to drop the owner from presence — so a read
+ // path must not treat a stored 0 as invalid.
+ if d.LastActiveAt == 0 {
+ return mmmodel.NewAppError("Draft.IsValid", "model.draft.is_valid.last_active_at.app_error", nil, "page_id="+d.PageId, http.StatusBadRequest)
+ }
+
+ // BaseEditAt is an optimistic-lock baseline (a page EditAt) or 0 for "no baseline"; a negative
+ // value is never legitimate. 0 is legal and must not be rejected.
+ if d.BaseEditAt < 0 {
+ return mmmodel.NewAppError("Draft.IsValid", "model.draft.is_valid.base_edit_at.app_error", nil, "page_id="+d.PageId, http.StatusBadRequest)
+ }
+
// A draft publishes into a page, so it is bound by the page content limits.
if utf8.RuneCountInString(d.Title) > PageTitleMaxRunes {
return mmmodel.NewAppError("Draft.IsValid", "model.draft.is_valid.title_length.app_error", nil, "page_id="+d.PageId, http.StatusBadRequest)
@@ -110,17 +158,51 @@ func (d *Draft) IsValid() *mmmodel.AppError {
return mmmodel.NewAppError("Draft.IsValid", "model.draft.is_valid.body_size.app_error", nil, "page_id="+d.PageId, http.StatusBadRequest)
}
- if utf8.RuneCountInString(mmmodel.ArrayToJSON(d.FileIds)) > DraftFileIdsMaxRunes {
- return mmmodel.NewAppError("Draft.IsValid", "model.draft.is_valid.file_ids.app_error", nil, "page_id="+d.PageId, http.StatusBadRequest)
+ if err := ValidateDraftFileIds("Draft.IsValid", "page_id="+d.PageId, d.FileIds); err != nil {
+ return err
}
- if err := validatePropsSize("Draft.IsValid", "page_id="+d.PageId, d.Props, PagePropsMaxBytes); err != nil {
+ if err := ValidatePropsSize("Draft.IsValid", "page_id="+d.PageId, d.Props, PagePropsMaxBytes); err != nil {
return err
}
return nil
}
+// ValidateDraftFileIds checks that the serialized file-id list stays within DraftFileIdsMaxRunes and
+// that every entry is a well-formed ID. An empty list is valid; an empty entry is not, since a blank
+// ID names no file and would be stored verbatim.
+func ValidateDraftFileIds(where, detail string, fileIDs mmmodel.StringArray) *mmmodel.AppError {
+ if utf8.RuneCountInString(mmmodel.ArrayToJSON(fileIDs)) > DraftFileIdsMaxRunes {
+ return mmmodel.NewAppError(where, "model.draft.is_valid.file_ids.app_error", nil, detail, http.StatusBadRequest)
+ }
+ for _, fileID := range fileIDs {
+ if !mmmodel.IsValidId(fileID) {
+ return mmmodel.NewAppError(where, "model.draft.is_valid.file_id.app_error", nil, detail, http.StatusBadRequest)
+ }
+ }
+ return nil
+}
+
+// ValidateDraftWriteIntent applies the FileIds and Props bounds to the pointer-carried write-intent
+// values a draft upsert takes alongside the Draft struct. Those values never reach the struct's own
+// fields — the upsert merges them in SQL — so IsValid cannot see them, and without this the bounds
+// would hold only for callers that remembered to check them separately. A nil pointer means the
+// field was omitted and the stored value is preserved, so there is nothing to bound.
+func ValidateDraftWriteIntent(where, detail string, fileIDs *mmmodel.StringArray, props *mmmodel.StringInterface) *mmmodel.AppError {
+ if fileIDs != nil {
+ if err := ValidateDraftFileIds(where, detail, *fileIDs); err != nil {
+ return err
+ }
+ }
+ if props != nil {
+ if err := ValidatePropsSize(where, detail, *props, PagePropsMaxBytes); err != nil {
+ return err
+ }
+ }
+ return nil
+}
+
// GetProps returns Props, or an empty map if Props is nil.
func (d *Draft) GetProps() mmmodel.StringInterface {
return ensureProps(d.Props)
diff --git a/server/model/draft_test.go b/server/model/draft_test.go
index 58ae14a..bc0aff8 100644
--- a/server/model/draft_test.go
+++ b/server/model/draft_test.go
@@ -127,6 +127,28 @@ func TestDraftIsValid(t *testing.T) {
require.NotNil(t, aerr)
require.Equal(t, "model.draft.is_valid.update_at.app_error", aerr.Id)
})
+
+ t.Run("negative BaseEditAt rejected", func(t *testing.T) {
+ d := validDraft()
+ d.BaseEditAt = -1
+ aerr := d.IsValid()
+ require.NotNil(t, aerr)
+ require.Equal(t, "model.draft.is_valid.base_edit_at.app_error", aerr.Id)
+ })
+
+ t.Run("zero BaseEditAt allowed", func(t *testing.T) {
+ d := validDraft()
+ d.BaseEditAt = 0
+ require.Nil(t, d.IsValid())
+ })
+}
+
+func TestDraftIsValidLastActiveAtZeroRejected(t *testing.T) {
+ d := validDraft()
+ d.LastActiveAt = 0
+ aerr := d.IsValid()
+ require.NotNil(t, aerr)
+ require.Equal(t, "model.draft.is_valid.last_active_at.app_error", aerr.Id)
}
func TestDraftPreSaveDefaults(t *testing.T) {
@@ -134,6 +156,32 @@ func TestDraftPreSaveDefaults(t *testing.T) {
d.PreSave()
require.NotZero(t, d.CreateAt)
require.NotZero(t, d.UpdateAt)
+ require.NotZero(t, d.LastActiveAt)
require.NotNil(t, d.FileIds)
require.NotNil(t, d.Props)
}
+
+func TestDraftPreSavePreservesExistingCreateAt(t *testing.T) {
+ d := &model.Draft{UserId: mmmodel.NewId(), SpaceId: mmmodel.NewId(), PageId: mmmodel.NewId(), CreateAt: 12345}
+ d.PreSave()
+ require.Equal(t, int64(12345), d.CreateAt, "PreSave must not overwrite an existing CreateAt")
+}
+
+func TestDraftPreSaveTrimsTitleWhitespace(t *testing.T) {
+ d := &model.Draft{UserId: mmmodel.NewId(), SpaceId: mmmodel.NewId(), PageId: mmmodel.NewId(), Title: " hello "}
+ d.PreSave()
+ require.Equal(t, "hello", d.Title)
+}
+
+func TestDraftGetPropsNilReturnsEmpty(t *testing.T) {
+ d := &model.Draft{Props: nil}
+ require.NotNil(t, d.GetProps(), "GetProps must return an empty map, not nil")
+ require.Empty(t, d.GetProps())
+}
+
+func TestDraftAuditable(t *testing.T) {
+ d := validDraft()
+ d.BaseEditAt = 42
+ auditable := d.Auditable()
+ require.Equal(t, int64(42), auditable["base_edit_at"])
+}
diff --git a/server/model/page.go b/server/model/page.go
index ab840ef..827d4fd 100644
--- a/server/model/page.go
+++ b/server/model/page.go
@@ -22,6 +22,11 @@ const (
// so this is a storage safety bound against unbounded writes.
PageBodyMaxBytes = 2 * 1024 * 1024
+ // MaxPageDepth is the page hierarchy depth limit (root is depth 1). It also bounds draft
+ // nesting: a draft chain publishes into a page chain of the same depth, so the store's
+ // draft-cycle walk is capped at the same value.
+ MaxPageDepth = 10
+
// PagePropsMaxBytes caps the serialized size of the opaque Props map.
PagePropsMaxBytes = 64 * 1024
@@ -108,9 +113,7 @@ func MaxDepthOfPages(pages []*Page, rootID string) int {
}
cur = parent
}
- if depth > maxDepth {
- maxDepth = depth
- }
+ maxDepth = max(maxDepth, depth)
}
return maxDepth
}
@@ -252,7 +255,7 @@ func (p *Page) IsValid() *mmmodel.AppError {
return mmmodel.NewAppError("Page.IsValid", "model.page.is_valid.search_text_without_body.app_error", nil, "id="+p.Id, http.StatusBadRequest)
}
- if err := validatePropsSize("Page.IsValid", "id="+p.Id, p.Props, PagePropsMaxBytes); err != nil {
+ if err := ValidatePropsSize("Page.IsValid", "id="+p.Id, p.Props, PagePropsMaxBytes); err != nil {
return err
}
diff --git a/server/model/page_content.go b/server/model/page_content.go
new file mode 100644
index 0000000..e4f45a9
--- /dev/null
+++ b/server/model/page_content.go
@@ -0,0 +1,608 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See LICENSE.txt for license information.
+
+package model
+
+import (
+ "encoding/base64"
+ "encoding/json"
+ "html"
+ "net/http"
+ "slices"
+ "strings"
+
+ "github.com/pkg/errors"
+)
+
+const (
+ TipTapDocType = "doc"
+ EmptyTipTapJSON = `{"type":"doc","content":[]}`
+)
+
+// TipTapDocument is the parsed form of a TipTap editor document. Content is left as an untyped node
+// tree because node attributes and nesting vary per editor extension; the permitted node and mark
+// type names are constrained by allowedNodeTypes/allowedMarkTypes during sanitization. Client-supplied
+// content must be produced by ParseTipTapDocument for the sanitization invariant to hold — a value
+// decoded from client JSON any other way has not passed sanitizeTipTapDocument and must not be
+// stored or rendered. The one exception is a document assembled internally from trusted parts (e.g.
+// convertPlainTextToTipTap, which emits only paragraph and text nodes with no attrs or marks): it is
+// safe by construction and needs no sanitization pass.
+type TipTapDocument struct {
+ Type string `json:"type"`
+ Content []map[string]any `json:"content"`
+}
+
+// BuildSearchText extracts searchable plain text from a TipTap document.
+func BuildSearchText(doc TipTapDocument) string {
+ var b strings.Builder
+ for _, node := range doc.Content {
+ appendNodeText(&b, node, 0)
+ }
+ return b.String()
+}
+
+// ParseTipTapDocument parses and sanitizes a TipTap JSON string into a TipTapDocument. Unlike the
+// model's field validators, it returns a plain error (not an *mmmodel.AppError): its failures are
+// parse-level, not per-field, so there is no single i18n key to attach per reason.
+func ParseTipTapDocument(contentJSON string) (TipTapDocument, error) {
+ if contentJSON == "" {
+ return TipTapDocument{
+ Type: TipTapDocType,
+ Content: []map[string]any{},
+ }, nil
+ }
+
+ // Reject over-limit content before parsing: json.Unmarshal materializes every node as a
+ // map[string]any at a large multiple of its encoded size, and the per-node budget inside the
+ // sanitize walk only applies after that allocation.
+ if len(contentJSON) > PageBodyMaxBytes {
+ return TipTapDocument{}, errors.New("content exceeds the maximum body size")
+ }
+
+ var doc TipTapDocument
+ if err := json.Unmarshal([]byte(contentJSON), &doc); err != nil {
+ return TipTapDocument{}, err
+ }
+
+ if doc.Type != TipTapDocType {
+ return TipTapDocument{}, errors.New("content must be valid TipTap JSON with type: doc")
+ }
+
+ // A document with no "content" key (or an explicit null) decodes to a nil slice, which would
+ // re-marshal to "content":null. Empty content is [] everywhere else, and a client walking the
+ // array would fault on null, so normalize to the one empty representation.
+ if doc.Content == nil {
+ doc.Content = []map[string]any{}
+ }
+
+ if err := sanitizeTipTapDocument(&doc); err != nil {
+ return TipTapDocument{}, err
+ }
+ return doc, nil
+}
+
+// appendNodeText walks a node subtree once, appending each text leaf and mention label to b. A
+// single shared builder keeps extraction O(total text) instead of re-joining every subtree at each
+// ancestor, which would copy a leaf once per level of nesting.
+func appendNodeText(b *strings.Builder, node map[string]any, depth int) {
+ if depth > maxTipTapDepth {
+ return
+ }
+
+ if textVal, ok := node["text"]; ok {
+ if text, ok := textVal.(string); ok && text != "" {
+ if normalized := strings.Join(strings.Fields(text), " "); normalized != "" {
+ writeSearchTextPart(b, normalized)
+ }
+ }
+ }
+
+ if nodeType, ok := node["type"].(string); ok && (nodeType == "mention" || nodeType == "channelMention") {
+ if attrs, ok := node["attrs"].(map[string]any); ok {
+ if label, ok := attrs["label"].(string); ok && label != "" {
+ writeSearchTextPart(b, "@"+label)
+ } else if id, ok := attrs["id"].(string); ok && id != "" {
+ writeSearchTextPart(b, "@"+id)
+ }
+ }
+ }
+
+ if contentVal, ok := node["content"]; ok {
+ if children, ok := contentVal.([]any); ok {
+ for _, child := range children {
+ if childNode, ok := child.(map[string]any); ok {
+ appendNodeText(b, childNode, depth+1)
+ }
+ }
+ }
+ }
+}
+
+// writeSearchTextPart appends part to b, separated from any prior content by a single space.
+func writeSearchTextPart(b *strings.Builder, part string) {
+ if b.Len() > 0 {
+ b.WriteByte(' ')
+ }
+ b.WriteString(part)
+}
+
+// maxTipTapDepth bounds recursion over client-supplied content. encoding/json already caps nesting,
+// but this rejects a pathologically deep document before the recursive walk and keeps stored content
+// within a sane depth.
+const maxTipTapDepth = 100
+
+// maxTipTapNodes caps the total number of content nodes in a TipTap document, bounding the
+// sanitize walk and the stored node count. It applies after json.Unmarshal has materialized the
+// document, so it does not bound the parse itself — the pre-parse body-size check at the top of
+// ParseTipTapDocument does that. The plain-text path is capped at maxPlainTextParagraphs
+// (10 000 paragraphs → ~20 000 nodes, since each non-empty line is a paragraph plus a text child);
+// rich documents with ~5 inline nodes per paragraph stay well under 50 000 for any sane document.
+const maxTipTapNodes = 50_000
+
+var errAttrDepthExceeded = errors.New("content attribute nesting exceeds the maximum depth")
+
+func sanitizeTipTapDocument(doc *TipTapDocument) error {
+ // count is a running node budget shared across the walk: sanitizeTipTapNode increments it per
+ // node and bails once it crosses maxTipTapNodes, so a crafted payload is rejected before the
+ // walk allocates without bound — without a separate counting pass over the whole tree.
+ count := 0
+ for i := range doc.Content {
+ if doc.Content[i] == nil {
+ return errors.New("content document nodes must be objects")
+ }
+ if err := sanitizeTipTapNode(doc.Content[i], 0, &count); err != nil {
+ return err
+ }
+ }
+ return nil
+}
+
+// urlAttrKeys are the attribute keys (matched case-insensitively) whose values are URLs and must
+// pass through sanitizeURL.
+var urlAttrKeys = map[string]struct{}{
+ "href": {},
+ "src": {},
+ "poster": {},
+ "xlink:href": {},
+ "xlinkhref": {},
+}
+
+// allowedNodeTypes is the allowlist of TipTap node type values permitted in stored content, keyed to
+// the core WysiwygEditor schema plus the extensions the page editor augments it with. A node type not
+// listed here is rejected outright: this pins the server's accepted node set to the client's schema so
+// the two cannot drift silently, and it fails closed for any script- or embed-bearing type the client
+// never emits. Matched case-sensitively, since TipTap schema names are case-sensitive camelCase.
+//
+// This is a hand-maintained mirror of the editor schema: when the page editor gains a node type — a
+// core WysiwygEditor/StarterKit change or a Docs-specific extension — add it here in the same change,
+// or the new content is rejected on save.
+var allowedNodeTypes = map[string]struct{}{
+ // Core WysiwygEditor schema (StarterKit + Link + CodeBlockLowlight + Table).
+ "doc": {},
+ "paragraph": {},
+ "text": {},
+ "heading": {},
+ "hardBreak": {},
+ "horizontalRule": {},
+ "blockquote": {},
+ "codeBlock": {},
+ "bulletList": {},
+ "orderedList": {},
+ "listItem": {},
+ "table": {},
+ "tableRow": {},
+ "tableCell": {},
+ "tableHeader": {},
+ // Page editor extensions layered on the core schema.
+ "taskList": {},
+ "taskItem": {},
+ "mention": {},
+ "channelMention": {},
+ "callout": {},
+ "image": {},
+ "imageResize": {},
+ "imagePlaceholder": {},
+ "video": {},
+ "fileAttachment": {},
+}
+
+// allowedMarkTypes is the allowlist of TipTap mark type values, keyed to the core WysiwygEditor
+// schema plus the page editor's extensions. Like allowedNodeTypes, a mark type not listed here is
+// rejected rather than passed through. "link" is a mark (TipTap's inline hyperlink); its href is
+// sanitized by sanitizeURL.
+var allowedMarkTypes = map[string]struct{}{
+ // Core WysiwygEditor marks.
+ "bold": {},
+ "italic": {},
+ "strike": {},
+ "code": {},
+ "underline": {},
+ "link": {},
+ // Page editor extensions.
+ "textStyle": {},
+ "commentAnchor": {},
+}
+
+// dangerousAttrKeys are attribute keys (matched case-insensitively) stripped outright regardless of
+// value: they can execute script or embed foreign markup, and no supported node needs them. This
+// denylist is layered on top of the URL-scheme allowlist.
+var dangerousAttrKeys = map[string]struct{}{
+ "style": {},
+ "formaction": {},
+ "action": {},
+ "srcdoc": {},
+ "srcset": {},
+ "background": {},
+ "dynsrc": {},
+ "lowsrc": {},
+ "ping": {},
+ "data": {},
+}
+
+// trimBrowserIgnoredChars trims leading/trailing ASCII space and control characters (r <= ' ') —
+// the characters an HTML tokenizer ignores around attribute names and a browser strips from the
+// ends of a URL — so a sanitizer match cannot be defeated by padding with them.
+func trimBrowserIgnoredChars(s string) string {
+ return strings.TrimFunc(s, func(r rune) bool { return r <= ' ' })
+}
+
+// stripDangerousKeys strips script-bearing keys (event handlers plus the dangerousAttrKeys set) and
+// neutralizes dangerous URL schemes on any URL-valued key, at the top level of m only. Key names are
+// matched case-insensitively, since HTML attribute names are case-insensitive.
+//
+// It is applied to attribute maps and also to the node and mark objects themselves, because a
+// lenient client renderer may read a dangerous or URL-valued key placed directly on the object
+// rather than nested under its "attrs".
+func stripDangerousKeys(m map[string]any) {
+ for key, val := range m {
+ // Trim leading/trailing whitespace and control characters before matching: an HTML
+ // tokenizer treats "\tonclick" as the onclick attribute, so a key that fails to match
+ // here because of such a prefix would carry its payload through every check below.
+ trimmed := trimBrowserIgnoredChars(key)
+ lower := strings.ToLower(trimmed)
+ if _, dangerous := dangerousAttrKeys[lower]; strings.HasPrefix(lower, "on") || dangerous {
+ delete(m, key)
+ continue
+ }
+ // A padded key is not a name any editor schema defines — the padding exists only to slip
+ // past a matcher that does not trim. Drop it outright instead of sanitizing its value in
+ // place, so a renderer that resolves the trimmed name cannot find both a padded and a
+ // canonical entry for the same attribute.
+ if trimmed != key {
+ delete(m, key)
+ continue
+ }
+ // A data-* attribute may carry a URL a lenient client renderer treats as a navigation
+ // target, but it is not necessarily one: an editor extension may store a ratio ("16:9"), a
+ // timestamp ("12:30"), or any other colon-bearing value under a data-* key. The strict URL
+ // allowlist would read the leading token as a scheme and blank every such value, so
+ // neutralize only the unambiguously dangerous schemes here (javascript:/vbscript:/non-image
+ // data:), matching the bare-array-string path. A non-string value (number, nested map/array)
+ // carries no scheme to strip and is left for the recursive attr walk to handle.
+ if strings.HasPrefix(lower, "data-") {
+ if v, ok := val.(string); ok {
+ m[key] = neutralizeAmbiguousURLScheme(v)
+ }
+ continue
+ }
+ // A designated URL key (href, src, ...) genuinely holds a URL, so apply the strict scheme
+ // allowlist. A non-string value (e.g. a JSON array) can be coerced back into a dangerous
+ // string by a client renderer, so drop it rather than leave it untouched.
+ if _, isURL := urlAttrKeys[lower]; isURL {
+ v, ok := val.(string)
+ if !ok {
+ delete(m, key)
+ continue
+ }
+ m[key] = sanitizeURL(v)
+ }
+ }
+}
+
+// sanitizeAttrs strips dangerous keys from an attribute map and descends into any nested maps and
+// arrays it holds, so a URL or handler buried under a sub-object (e.g. attrs.config.href, a shape
+// an extension's schema may define) is neutralized rather than passed through untouched. It fails
+// closed: attribute nesting past maxTipTapDepth returns an error (rejecting the whole document)
+// rather than silently leaving the over-deep subtree unsanitized.
+func sanitizeAttrs(attrs map[string]any, depth int) error {
+ if depth > maxTipTapDepth {
+ return errAttrDepthExceeded
+ }
+ stripDangerousKeys(attrs)
+ for _, val := range attrs {
+ if err := sanitizeAttrValue(val, depth); err != nil {
+ return err
+ }
+ }
+ return nil
+}
+
+// sanitizeAttrValue recurses into the containers an attribute value may hold, neutralizing dangerous
+// URL schemes on any bare string reached inside an array. A scalar string held directly under a map
+// key is left to stripDangerousKeys, which sanitizes it only under a URL-designated key; but a string
+// inside an array has no key to mark it, so a dangerous scheme there — e.g. ["javascript:alert(1)"]
+// under a non-URL-designated key an editor extension may define — would otherwise pass through
+// untouched. Like sanitizeAttrs, it fails closed past maxTipTapDepth.
+func sanitizeAttrValue(val any, depth int) error {
+ if depth > maxTipTapDepth {
+ return errAttrDepthExceeded
+ }
+ switch v := val.(type) {
+ case map[string]any:
+ return sanitizeAttrs(v, depth+1)
+ case []any:
+ for i, item := range v {
+ if s, ok := item.(string); ok {
+ v[i] = neutralizeAmbiguousURLScheme(s)
+ continue
+ }
+ if err := sanitizeAttrValue(item, depth+1); err != nil {
+ return err
+ }
+ }
+ }
+ return nil
+}
+
+// nodeSkipKeys are the top-level keys excluded from the flat-key sanitization pass in
+// sanitizeTipTapNode: "attrs" is handled separately, and "content"/"marks" get their own recursion.
+var nodeSkipKeys = map[string]struct{}{"attrs": {}, "content": {}, "marks": {}}
+
+// markSkipKeys are the top-level keys excluded from the flat-key sanitization pass for mark objects.
+var markSkipKeys = map[string]struct{}{"attrs": {}}
+
+// sanitizeObjAttrsAndFlatKeys sanitizes the "attrs" sub-object of a node or mark, and any flat keys
+// not in skipKeys. stripDangerousKeys must be called on obj before this.
+func sanitizeObjAttrsAndFlatKeys(obj map[string]any, attrsErrMsg string, skipKeys map[string]struct{}, depth int) error {
+ if attrsVal, ok := obj["attrs"]; ok && attrsVal != nil {
+ attrs, ok := attrsVal.(map[string]any)
+ if !ok {
+ return errors.New(attrsErrMsg)
+ }
+ // The attrs subtree restarts the depth budget at 0, while the flat-key values below inherit
+ // the node's depth and so get what remains of it. Both fail closed at maxTipTapDepth, and
+ // attrs nesting is a property of the attribute value rather than of the document tree, so
+ // charging it against the node's remaining depth would bound it by where the node happens
+ // to sit. The asymmetry is deliberate.
+ if err := sanitizeAttrs(attrs, 0); err != nil {
+ return err
+ }
+ }
+ for key, val := range obj {
+ if _, skip := skipKeys[key]; skip {
+ continue
+ }
+ if err := sanitizeAttrValue(val, depth); err != nil {
+ return err
+ }
+ }
+ return nil
+}
+
+func sanitizeTipTapNode(node map[string]any, depth int, count *int) error {
+ if node == nil {
+ return errors.New("content node must not be null")
+ }
+ if depth > maxTipTapDepth {
+ return errors.New("content nesting exceeds the maximum depth")
+ }
+ *count++
+ if *count > maxTipTapNodes {
+ return errors.Errorf("content exceeds the maximum of %d nodes", maxTipTapNodes)
+ }
+
+ // Strip dangerous/URL keys placed directly on the node object, then sanitize its attrs. The
+ // node's own keys need the same treatment as a mark's: "content" and "marks" are walked below,
+ // and no supported node key collides with the handler/URL sets.
+ stripDangerousKeys(node)
+
+ nodeType, ok := node["type"].(string)
+ if !ok || nodeType == "" {
+ return errors.New("content node must have a non-empty type field")
+ }
+ if _, allowed := allowedNodeTypes[nodeType]; !allowed {
+ return errors.Errorf("content node type %q is not allowed", nodeType)
+ }
+
+ if err := sanitizeObjAttrsAndFlatKeys(node, "content node attrs must be an object", nodeSkipKeys, depth); err != nil {
+ return err
+ }
+
+ if marksVal, ok := node["marks"]; ok && marksVal != nil {
+ marksArray, ok := marksVal.([]any)
+ if !ok {
+ return errors.New("content node marks must be an array")
+ }
+ for _, mark := range marksArray {
+ markNode, ok := mark.(map[string]any)
+ if !ok {
+ return errors.New("content mark must be an object")
+ }
+ // Sanitize both the mark's nested attrs and any dangerous/URL keys placed directly on
+ // the mark object (a non-standard shape a lenient renderer may read).
+ stripDangerousKeys(markNode)
+ markType, ok := markNode["type"].(string)
+ if !ok || markType == "" {
+ return errors.New("content mark must have a non-empty type field")
+ }
+ if _, allowed := allowedMarkTypes[markType]; !allowed {
+ return errors.Errorf("content mark type %q is not allowed", markType)
+ }
+ if err := sanitizeObjAttrsAndFlatKeys(markNode, "content mark attrs must be an object", markSkipKeys, depth); err != nil {
+ return err
+ }
+ }
+ }
+
+ if contentVal, ok := node["content"]; ok && contentVal != nil {
+ contentArray, ok := contentVal.([]any)
+ if !ok {
+ return errors.New("content node content must be an array")
+ }
+ for _, child := range contentArray {
+ childNode, ok := child.(map[string]any)
+ if !ok {
+ return errors.New("content child must be an object")
+ }
+ if err := sanitizeTipTapNode(childNode, depth+1, count); err != nil {
+ return err
+ }
+ }
+ }
+ return nil
+}
+
+// safeImageMIMETypes are the only image MIME types allowed through data: URIs. SVG is excluded
+// because it can carry script. safeImageDataPrefixes is derived from this slice so adding a new
+// type here keeps both in sync automatically.
+var safeImageMIMETypes = []string{
+ "image/png",
+ "image/jpeg",
+ "image/gif",
+ "image/webp",
+ "image/bmp",
+}
+
+var safeImageDataPrefixes = func() []string {
+ out := make([]string, len(safeImageMIMETypes))
+ for i, mt := range safeImageMIMETypes {
+ out[i] = "data:" + mt
+ }
+ return out
+}()
+
+// sniffBase64Chars is how many leading base64 characters are decoded for content sniffing. 24 chars
+// decode to 18 bytes, covering the longest signature http.DetectContentType matches on for the types
+// above (WebP, whose RIFF container marker runs to byte 14). 24 is a multiple of 4, so a truncated
+// (unpadded) prefix stays valid StdEncoding; a short payload that already carries "=" padding within
+// those 24 chars decodes too (unlike RawStdEncoding, which rejects padding).
+const sniffBase64Chars = 24
+
+// isBase64ImagePayload confirms a data:image/* URI actually carries base64-encoded bytes that sniff
+// as one of the allowed raster images, so a script-bearing payload cannot ride in under an allowed
+// MIME label (e.g. an SVG declared as image/png).
+func isBase64ImagePayload(url string) bool {
+ meta, payload, ok := strings.Cut(url, ",")
+ if !ok {
+ return false
+ }
+ if !strings.Contains(strings.ToLower(meta), ";base64") {
+ return false
+ }
+ trimmed := strings.TrimSpace(payload)
+ if len(trimmed) > sniffBase64Chars {
+ trimmed = trimmed[:sniffBase64Chars]
+ }
+ data, err := base64.StdEncoding.DecodeString(trimmed)
+ if err != nil {
+ return false
+ }
+ contentType, _, _ := strings.Cut(http.DetectContentType(data), ";")
+ return slices.Contains(safeImageMIMETypes, contentType)
+}
+
+// urlScheme returns the scheme of a URL and whether one is present. lower must already be
+// lowercased (decodeURLScheme passes its lowered form): the character check below accepts only a-z,
+// so an uppercase scheme would be reported as absent. A scheme must sit at the very start and
+// precede any '/', '?', or '#', matching how browsers parse schemes; a relative reference (no
+// scheme) returns ("", false).
+func urlScheme(lower string) (string, bool) {
+ for i, r := range lower {
+ if r == ':' {
+ if i == 0 {
+ return "", false
+ }
+ return lower[:i], true
+ }
+ if r == '/' || r == '?' || r == '#' {
+ return "", false
+ }
+ valid := (r >= 'a' && r <= 'z') || (r >= '0' && r <= '9') || r == '+' || r == '-' || r == '.'
+ if !valid {
+ return "", false
+ }
+ }
+ return "", false
+}
+
+// urlStripChars removes the ASCII tab/newline/CR that browsers strip from a URL before resolving
+// its scheme, so an obfuscated "java\tscript:" cannot slip past the scheme check.
+var urlStripChars = strings.NewReplacer("\t", "", "\n", "", "\r", "", "\x00", "")
+
+// decodeURLScheme extracts the scheme a browser would resolve from url, defeating the obfuscation a
+// dangerous scheme can hide behind. It decodes HTML entities and strips the tab/newline/CR/null
+// browsers ignore, so an entity-encoded ":" or an embedded control char is detected. Two strip
+// passes: the first removes literal control chars, the second removes any html.UnescapeString
+// re-introduces (e.g. " " → "\t"). Percent-encoded characters (%09, %0A, etc.) are intentionally
+// NOT stripped: browsers do not strip them from scheme names, so "java%09script:" never parses as
+// "javascript:". Returns the lowercased scheme, the lowercased cleaned string (for data: prefix
+// matching), and whether a scheme is present (false for a relative reference). The decode is used
+// only for detection; callers return the original string when they allow it.
+func decodeURLScheme(url string) (scheme, lower string, hasScheme bool) {
+ cleaned := urlStripChars.Replace(url)
+ cleaned = html.UnescapeString(cleaned)
+ cleaned = urlStripChars.Replace(cleaned)
+ cleaned = trimBrowserIgnoredChars(cleaned)
+ lower = strings.ToLower(cleaned)
+ scheme, hasScheme = urlScheme(lower)
+ return scheme, lower, hasScheme
+}
+
+// isSafeImageDataURL reports whether a data: URL carries a base64 payload sniffing as an allowed
+// raster image. lower is the lowercased, obfuscation-decoded form of url from decodeURLScheme.
+func isSafeImageDataURL(url, lower string) bool {
+ for _, prefix := range safeImageDataPrefixes {
+ if strings.HasPrefix(lower, prefix) && isBase64ImagePayload(url) {
+ return true
+ }
+ }
+ return false
+}
+
+// sanitizeURL returns the URL unchanged if its scheme is on the allowlist (or it is a relative
+// reference), and "" otherwise. It defends against control-character, leading-whitespace, and
+// HTML-entity obfuscation of dangerous schemes (e.g. "java script:alert(1)"). Applied to a
+// value under a URL-designated attribute key (href, src, poster, xlink:href). data-* keys are not
+// necessarily URLs and take the lenient neutralizeAmbiguousURLScheme path instead.
+func sanitizeURL(url string) string {
+ scheme, lower, hasScheme := decodeURLScheme(url)
+ if !hasScheme {
+ return url
+ }
+ switch scheme {
+ case "http", "https", "mailto", "tel":
+ return url
+ case "data":
+ if isSafeImageDataURL(url, lower) {
+ return url
+ }
+ return ""
+ default:
+ return ""
+ }
+}
+
+// neutralizeAmbiguousURLScheme blanks a string that carries a script-executing or foreign-content
+// URL scheme, for values that are not unambiguously URLs. It is reached two ways: a bare element of
+// an array attribute value (no key marks it a URL), and a data-* attribute value (whose key may mark
+// a URL but need not — an extension may store a "12:30" timestamp or "16:9" ratio there). Unlike
+// sanitizeURL (a strict allowlist applied where an attribute key designates the value a URL), only
+// the unambiguously dangerous schemes are neutralized: a plain colon-bearing string is preserved,
+// while a javascript:/vbscript:/non-image data: payload is dropped.
+func neutralizeAmbiguousURLScheme(s string) string {
+ scheme, lower, hasScheme := decodeURLScheme(s)
+ if !hasScheme {
+ return s
+ }
+ switch scheme {
+ case "javascript", "vbscript":
+ return ""
+ case "data":
+ if isSafeImageDataURL(s, lower) {
+ return s
+ }
+ return ""
+ default:
+ return s
+ }
+}
diff --git a/server/model/page_content_test.go b/server/model/page_content_test.go
new file mode 100644
index 0000000..edc00c0
--- /dev/null
+++ b/server/model/page_content_test.go
@@ -0,0 +1,753 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See LICENSE.txt for license information.
+
+package model_test
+
+import (
+ "encoding/json"
+ "strings"
+ "testing"
+
+ "github.com/stretchr/testify/require"
+
+ "github.com/mattermost/mattermost-plugin-docs/server/model"
+)
+
+// marshal round-trips a parsed document to JSON so tests can assert on the sanitized output.
+func marshal(t *testing.T, doc model.TipTapDocument) string {
+ t.Helper()
+ b, err := json.Marshal(doc)
+ require.NoError(t, err)
+ return string(b)
+}
+
+func TestParseTipTapDocument(t *testing.T) {
+ t.Run("empty string yields an empty doc", func(t *testing.T) {
+ doc, err := model.ParseTipTapDocument("")
+ require.NoError(t, err)
+ require.Equal(t, model.TipTapDocType, doc.Type)
+ require.Empty(t, doc.Content)
+ })
+
+ t.Run("valid doc round-trips", func(t *testing.T) {
+ doc, err := model.ParseTipTapDocument(`{"type":"doc","content":[{"type":"paragraph","content":[{"type":"text","text":"hello"}]}]}`)
+ require.NoError(t, err)
+ require.Equal(t, model.TipTapDocType, doc.Type)
+ require.Len(t, doc.Content, 1)
+ })
+
+ t.Run("non-doc top-level type is rejected", func(t *testing.T) {
+ _, err := model.ParseTipTapDocument(`{"type":"bogus","content":[]}`)
+ require.Error(t, err)
+ })
+
+ t.Run("malformed JSON is rejected", func(t *testing.T) {
+ _, err := model.ParseTipTapDocument(`{not json`)
+ require.Error(t, err)
+ })
+
+ t.Run("minimal doc without content does not panic", func(t *testing.T) {
+ doc, err := model.ParseTipTapDocument(`{"type":"doc"}`)
+ require.NoError(t, err)
+ require.Empty(t, doc.Content)
+ })
+}
+
+func TestParseTipTapDocumentSanitizesURLs(t *testing.T) {
+ // Each case is a link mark href; want=="" means the scheme must be stripped.
+ cases := []struct {
+ name string
+ href string
+ want string
+ }{
+ {"plain javascript", "javascript:alert(1)", ""},
+ {"tab-obfuscated javascript", "java\tscript:alert(1)", ""},
+ {"newline-obfuscated javascript", "java\nscript:alert(1)", ""},
+ {"leading-control javascript", "\x01javascript:alert(1)", ""},
+ {"uppercase javascript", "JAVASCRIPT:alert(1)", ""},
+ {"leading-space javascript", " javascript:alert(1)", ""},
+ {"entity-colon javascript", "javascript:alert(1)", ""},
+ {"entity-tab javascript", "java script:alert(1)", ""},
+ {"vbscript", "vbscript:msgbox(1)", ""},
+ {"data html", "data:text/html,", ""},
+ {"data svg", "data:image/svg+xml,