diff --git a/tools/cfl/README.md b/tools/cfl/README.md index 69dc92da..aa5c19dc 100644 --- a/tools/cfl/README.md +++ b/tools/cfl/README.md @@ -352,6 +352,8 @@ Content can be provided via: **Markdown is the default format.** It is converted to ADF, or to storage XHTML with `--legacy`. +**Writes are read back.** With `--body-format adf` or `xhtml` the page is fetched again after a write and the stored body compared against what was sent, because Confluence can store something other than what it was given — it drops `__confluenceMetadata` from link marks, for example. Losing content is an error and exits non-zero; attributes the server normalizes away are reported on stderr and tolerated. Markdown is converted before sending, so there is nothing to compare it against and the check is skipped. Use `--no-verify` to skip the extra read. + ```bash # Open markdown editor cfl page create --space DEV --title "My Page" @@ -384,6 +386,7 @@ cfl page create -s DEV -t "Legacy Page" --file content.md --legacy | `--editor` | | `false` | Force open in $EDITOR | | `--body-format` | | `markdown` | Input format: `markdown`, exact `adf` JSON, or exact storage `xhtml` | | `--legacy` | | `false` | Convert Markdown to storage XHTML instead of ADF; invalid with `adf` or `xhtml` | +| `--no-verify` | | `false` | Skip reading the page back after writing to confirm what Confluence stored (`adf`/`xhtml` only) | The selected format applies equally to files, stdin, and editor input; file extensions do not override it. @@ -400,6 +403,8 @@ Content can be provided via: **Markdown is the default format.** It is converted to ADF, or to storage XHTML with `--legacy`. +**Writes are read back.** With `--body-format adf` or `xhtml` the page is fetched again after a write and the stored body compared against what was sent, because Confluence can store something other than what it was given — it drops `__confluenceMetadata` from link marks, for example. Losing content is an error and exits non-zero; attributes the server normalizes away are reported on stderr and tolerated. Markdown is converted before sending, so there is nothing to compare it against and the check is skipped. Use `--no-verify` to skip the extra read. + ```bash # Open editor with existing page content cfl page edit 12345 @@ -438,6 +443,7 @@ cfl page view 12345 --body-format xhtml --content-only | \ | `--editor` | | `false` | Force open in $EDITOR | | `--body-format` | | `markdown` | Input/editor format: `markdown`, exact `adf` JSON, or exact storage `xhtml` | | `--legacy` | | `false` | Convert Markdown to storage XHTML instead of ADF; invalid with `adf` or `xhtml` | +| `--no-verify` | | `false` | Skip reading the page back after writing to confirm what Confluence stored (`adf`/`xhtml` only) | **Arguments:** - `` - The page ID (**required**) diff --git a/tools/cfl/internal/cmd/OUTPUT_SPEC.md b/tools/cfl/internal/cmd/OUTPUT_SPEC.md index b683c35b..dcfafa83 100644 --- a/tools/cfl/internal/cmd/OUTPUT_SPEC.md +++ b/tools/cfl/internal/cmd/OUTPUT_SPEC.md @@ -245,6 +245,10 @@ ID: URL: ``` +With `--body-format adf` or `xhtml` the page is read back after the write and +compared with what was sent, exactly as `page edit` does. See that section for +the stderr blocks emitted on normalization and on content loss. + ## `page edit ` Success: @@ -256,6 +260,53 @@ Version: URL: ``` +With `--body-format adf` or `xhtml` the page is read back and compared with +what was sent. When the stored body differs, a warning is written to stderr +after the success block. Attribute normalization is reported and the command +still succeeds: + +```text +Confluence normalized the stored body. Content is intact; these attributes were dropped: + - .attrs. (→) +``` + +Attributes the server added rather than dropped are reported the same way: + +```text +Confluence added attributes that were not sent: + + .attrs. (→) +``` + +Content loss is reported the same way and the command exits non-zero. The +first line names what moved in the document — one of three shapes: + +```text +Stored body does not match what was sent: visible text went from to characters. +Stored body does not match what was sent: visible text is unchanged at characters, but embedded content differs. +Stored body does not match what was sent: content differs at the same length of characters. +``` + +followed by: + +```text +The page was updated, but it does not hold the content supplied. Re-read the page before treating the change as applied. + first difference at offset — sent "", stored "" + embedded content changed: + ~ (→) + attributes dropped: + - .attrs. (→) + attributes added: + + .attrs. (→) +``` + +The offset line is omitted when the visible text is identical and only +embedded content changed. The embedded-content lines identify what moved in +that case, including for node types such as `hardBreak` and `rule` that carry +no attributes of their own. + +Counts are characters a reader sees and the offset is a character position, +both measured on the document rather than on any internal representation. + ## `page copy ` Success: diff --git a/tools/cfl/internal/cmd/page/create.go b/tools/cfl/internal/cmd/page/create.go index f6b911db..622aafb4 100644 --- a/tools/cfl/internal/cmd/page/create.go +++ b/tools/cfl/internal/cmd/page/create.go @@ -16,6 +16,7 @@ import ( ) type createOptions struct { + noVerify bool *root.Options space string title string @@ -95,6 +96,7 @@ without conversion.`, cmd.Flags().BoolVar(&opts.editor, "editor", false, "Open editor for content") cmd.Flags().StringVar(&opts.bodyFormat, "body-format", bodyFormatMarkdown, "Input format: markdown, adf, or xhtml") cmd.Flags().BoolVar(&opts.legacy, "legacy", false, "Create page in legacy editor format (Markdown input only)") + cmd.Flags().BoolVar(&opts.noVerify, "no-verify", false, "Skip reading the page back to confirm what Confluence stored (adf/xhtml input)") _ = cmd.MarkFlagRequired("title") @@ -128,6 +130,7 @@ func runCreate(ctx context.Context, opts *createOptions) error { if strings.TrimSpace(content) == "" { return fmt.Errorf("page content cannot be empty") } + sentContent := content body, err := bodyForInput(content, bodyFormat, opts.legacy) if err != nil { return err @@ -173,7 +176,20 @@ func runCreate(ctx context.Context, opts *createOptions) error { return err } - return cflpresent.Emit(opts.Options, cflpresent.PagePresenter{}.PresentCreate(page, cfg.URL)) + if err := cflpresent.Emit(opts.Options, cflpresent.PagePresenter{}.PresentCreate(page, cfg.URL)); err != nil { + return err + } + + // A create writes the same verbatim body an edit does, so it can suffer + // the same unseen server-side loss. + return verifyStoredBody(ctx, verifyRequest{ + opts: opts.Options, + client: client, + pageID: page.ID, + bodyFormat: bodyFormat, + sentContent: sentContent, + enabled: !opts.noVerify, + }) } func getContent(opts *createOptions, bodyFormat string) (string, error) { diff --git a/tools/cfl/internal/cmd/page/create_test.go b/tools/cfl/internal/cmd/page/create_test.go index 31564e05..1e20a899 100644 --- a/tools/cfl/internal/cmd/page/create_test.go +++ b/tools/cfl/internal/cmd/page/create_test.go @@ -100,6 +100,9 @@ func TestRunCreate_HTMLFile_Legacy(t *testing.T) { case r.Method == "GET" && strings.Contains(r.URL.Path, "/spaces"): w.WriteHeader(http.StatusOK) _, _ = w.Write([]byte(`{"results": [{"id": "123456", "key": "DEV"}]}`)) + case r.Method == "GET" && strings.Contains(r.URL.Path, "/pages/"): + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(storedCreatedPageJSON(receivedBody))) case r.Method == "POST" && strings.Contains(r.URL.Path, "/pages"): body, _ := io.ReadAll(r.Body) _ = json.Unmarshal(body, &receivedBody) @@ -147,6 +150,9 @@ func TestRunCreate_NoMarkdownFlag_Legacy(t *testing.T) { case r.Method == "GET" && strings.Contains(r.URL.Path, "/spaces"): w.WriteHeader(http.StatusOK) _, _ = w.Write([]byte(`{"results": [{"id": "123456", "key": "DEV"}]}`)) + case r.Method == "GET" && strings.Contains(r.URL.Path, "/pages/"): + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(storedCreatedPageJSON(receivedBody))) case r.Method == "POST" && strings.Contains(r.URL.Path, "/pages"): body, _ := io.ReadAll(r.Body) _ = json.Unmarshal(body, &receivedBody) @@ -304,6 +310,9 @@ func TestRunCreate_WithParent(t *testing.T) { case r.Method == "GET" && strings.Contains(r.URL.Path, "/spaces"): w.WriteHeader(http.StatusOK) _, _ = w.Write([]byte(`{"results": [{"id": "123456", "key": "DEV"}]}`)) + case r.Method == "GET" && strings.Contains(r.URL.Path, "/pages/"): + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(storedCreatedPageJSON(receivedBody))) case r.Method == "POST" && strings.Contains(r.URL.Path, "/pages"): body, _ := io.ReadAll(r.Body) _ = json.Unmarshal(body, &receivedBody) @@ -347,6 +356,9 @@ func TestRunCreate_MarkdownConversion_Legacy(t *testing.T) { case r.Method == "GET" && strings.Contains(r.URL.Path, "/spaces"): w.WriteHeader(http.StatusOK) _, _ = w.Write([]byte(`{"results": [{"id": "123456", "key": "DEV"}]}`)) + case r.Method == "GET" && strings.Contains(r.URL.Path, "/pages/"): + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(storedCreatedPageJSON(receivedBody))) case r.Method == "POST" && strings.Contains(r.URL.Path, "/pages"): body, _ := io.ReadAll(r.Body) _ = json.Unmarshal(body, &receivedBody) @@ -396,6 +408,9 @@ func TestRunCreate_MarkdownToADF(t *testing.T) { case r.Method == "GET" && strings.Contains(r.URL.Path, "/spaces"): w.WriteHeader(http.StatusOK) _, _ = w.Write([]byte(`{"results": [{"id": "123456", "key": "DEV"}]}`)) + case r.Method == "GET" && strings.Contains(r.URL.Path, "/pages/"): + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(storedCreatedPageJSON(receivedBody))) case r.Method == "POST" && strings.Contains(r.URL.Path, "/pages"): body, _ := io.ReadAll(r.Body) _ = json.Unmarshal(body, &receivedBody) @@ -462,6 +477,9 @@ func TestRunCreate_Stdin_ADF(t *testing.T) { case r.Method == "GET" && strings.Contains(r.URL.Path, "/spaces"): w.WriteHeader(http.StatusOK) _, _ = w.Write([]byte(`{"results": [{"id": "123456", "key": "DEV"}]}`)) + case r.Method == "GET" && strings.Contains(r.URL.Path, "/pages/"): + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(storedCreatedPageJSON(receivedBody))) case r.Method == "POST" && strings.Contains(r.URL.Path, "/pages"): body, _ := io.ReadAll(r.Body) _ = json.Unmarshal(body, &receivedBody) @@ -505,6 +523,9 @@ func TestRunCreate_Stdin_Legacy(t *testing.T) { case r.Method == "GET" && strings.Contains(r.URL.Path, "/spaces"): w.WriteHeader(http.StatusOK) _, _ = w.Write([]byte(`{"results": [{"id": "123456", "key": "DEV"}]}`)) + case r.Method == "GET" && strings.Contains(r.URL.Path, "/pages/"): + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(storedCreatedPageJSON(receivedBody))) case r.Method == "POST" && strings.Contains(r.URL.Path, "/pages"): body, _ := io.ReadAll(r.Body) _ = json.Unmarshal(body, &receivedBody) @@ -548,6 +569,9 @@ func TestRunCreate_Stdin_NoMarkdown_Legacy(t *testing.T) { case r.Method == "GET" && strings.Contains(r.URL.Path, "/spaces"): w.WriteHeader(http.StatusOK) _, _ = w.Write([]byte(`{"results": [{"id": "123456", "key": "DEV"}]}`)) + case r.Method == "GET" && strings.Contains(r.URL.Path, "/pages/"): + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(storedCreatedPageJSON(receivedBody))) case r.Method == "POST" && strings.Contains(r.URL.Path, "/pages"): body, _ := io.ReadAll(r.Body) _ = json.Unmarshal(body, &receivedBody) @@ -590,6 +614,9 @@ func TestRunCreate_StorageFlag_Stdin(t *testing.T) { case r.Method == "GET" && strings.Contains(r.URL.Path, "/spaces"): w.WriteHeader(http.StatusOK) _, _ = w.Write([]byte(`{"results": [{"id": "123456", "key": "DEV"}]}`)) + case r.Method == "GET" && strings.Contains(r.URL.Path, "/pages/"): + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(storedCreatedPageJSON(receivedBody))) case r.Method == "POST" && strings.Contains(r.URL.Path, "/pages"): body, _ := io.ReadAll(r.Body) _ = json.Unmarshal(body, &receivedBody) @@ -639,6 +666,9 @@ func TestRunCreate_StorageFlag_File(t *testing.T) { case r.Method == "GET" && strings.Contains(r.URL.Path, "/spaces"): w.WriteHeader(http.StatusOK) _, _ = w.Write([]byte(`{"results": [{"id": "123456", "key": "DEV"}]}`)) + case r.Method == "GET" && strings.Contains(r.URL.Path, "/pages/"): + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(storedCreatedPageJSON(receivedBody))) case r.Method == "POST" && strings.Contains(r.URL.Path, "/pages"): body, _ := io.ReadAll(r.Body) _ = json.Unmarshal(body, &receivedBody) @@ -681,6 +711,9 @@ func TestRunCreate_ComplexMarkdown_ADF(t *testing.T) { case r.Method == "GET" && strings.Contains(r.URL.Path, "/spaces"): w.WriteHeader(http.StatusOK) _, _ = w.Write([]byte(`{"results": [{"id": "123456", "key": "DEV"}]}`)) + case r.Method == "GET" && strings.Contains(r.URL.Path, "/pages/"): + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(storedCreatedPageJSON(receivedBody))) case r.Method == "POST" && strings.Contains(r.URL.Path, "/pages"): body, _ := io.ReadAll(r.Body) _ = json.Unmarshal(body, &receivedBody) @@ -831,6 +864,9 @@ func mockCreateBodyServer(t *testing.T, received *map[string]any) *httptest.Serv case r.Method == "GET" && strings.Contains(r.URL.Path, "/spaces"): w.WriteHeader(http.StatusOK) _, _ = w.Write([]byte(`{"results": [{"id": "123456", "key": "DEV"}]}`)) + case r.Method == "GET" && strings.Contains(r.URL.Path, "/pages/"): + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(storedCreatedPageJSON(*received))) case r.Method == "POST" && strings.Contains(r.URL.Path, "/pages"): body, _ := io.ReadAll(r.Body) _ = json.Unmarshal(body, received) @@ -987,3 +1023,20 @@ func TestRunCreate_FileDash_Stdin_Legacy(t *testing.T) { testutil.Contains(t, content, "bold") testutil.Nil(t, bodyMap["atlas_doc_format"]) } + +// storedCreatedPageJSON reports a created page the way Confluence does: a GET +// after the write returns the body that was stored. runCreate now reads the +// page back to verify the write, so a fake without this branch answers 404 +// for a create that in fact succeeded. +func storedCreatedPageJSON(received map[string]any) string { + fallback := `{"id":"99999","title":"Test","version":{"number":1}}` + body, ok := received["body"].(map[string]any) + if !ok { + return fallback + } + raw, err := json.Marshal(body) + if err != nil { + return fallback + } + return `{"id":"99999","title":"Test","version":{"number":1},"body":` + string(raw) + `}` +} diff --git a/tools/cfl/internal/cmd/page/edit.go b/tools/cfl/internal/cmd/page/edit.go index 0165a6f2..f7030216 100644 --- a/tools/cfl/internal/cmd/page/edit.go +++ b/tools/cfl/internal/cmd/page/edit.go @@ -26,6 +26,7 @@ type editOptions struct { bodyFormatExplicit bool legacy bool parent string + noVerify bool } func newEditCmd(rootOpts *root.Options) *cobra.Command { @@ -46,7 +47,14 @@ Content can be provided via: Content format is selected with --body-format markdown|adf|xhtml. Omitting --body-format means Markdown. ADF and XHTML are validated or sent -without conversion.`, +without conversion. + +For --body-format adf or xhtml the page is read back after writing and the +stored body compared against what was sent, because Confluence can store +something other than what it was given. Losing text is an error; attributes +the server normalizes away are reported and tolerated. Markdown is converted +before sending, so there is nothing to compare it against and the check is +skipped. Use --no-verify to skip the read.`, Example: ` # Edit a page in the editor with current content cfl page edit 12345 --editor @@ -103,6 +111,7 @@ without conversion.`, cmd.Flags().BoolVar(&opts.editor, "editor", false, "Open editor for content") cmd.Flags().StringVar(&opts.bodyFormat, "body-format", bodyFormatMarkdown, "Input format: markdown, adf, or xhtml") cmd.Flags().BoolVar(&opts.legacy, "legacy", false, "Edit page in legacy editor format (Markdown input only)") + cmd.Flags().BoolVar(&opts.noVerify, "no-verify", false, "Skip reading the page back to confirm what Confluence stored (adf/xhtml input)") return cmd } @@ -132,6 +141,7 @@ func runEdit(ctx context.Context, opts *editOptions) error { hasStdinData := (opts.Stdin != nil && opts.Stdin != os.Stdin) || hasPipedOSStdin(opts.Options) hasNewContent := opts.file != "" || opts.editor || hasStdinData var newBody *api.Body + var sentContent string if hasNewContent && !opts.editor { content, err := getEditContent(opts, nil, bodyFormat) if err != nil { @@ -140,6 +150,7 @@ func runEdit(ctx context.Context, opts *editOptions) error { if strings.TrimSpace(content) == "" { return fmt.Errorf("page content cannot be empty") } + sentContent = content newBody, err = bodyForInput(content, bodyFormat, opts.legacy) if err != nil { return err @@ -181,6 +192,7 @@ func runEdit(ctx context.Context, opts *editOptions) error { return fmt.Errorf("page content cannot be empty") } + sentContent = content newBody, err = bodyForInput(content, bodyFormat, opts.legacy) if err != nil { return err @@ -214,7 +226,18 @@ func runEdit(ctx context.Context, opts *editOptions) error { } } - return cflpresent.Emit(opts.Options, cflpresent.PagePresenter{}.PresentEdit(page, cfg.URL, opts.legacy && hasNewContent)) + if err := cflpresent.Emit(opts.Options, cflpresent.PagePresenter{}.PresentEdit(page, cfg.URL, opts.legacy && hasNewContent)); err != nil { + return err + } + + return verifyStoredBody(ctx, verifyRequest{ + opts: opts.Options, + client: client, + pageID: opts.pageID, + bodyFormat: bodyFormat, + sentContent: sentContent, + enabled: hasNewContent && !opts.noVerify, + }) } func getEditContent(opts *editOptions, existingPage *api.Page, bodyFormat string) (string, error) { diff --git a/tools/cfl/internal/cmd/page/edit_test.go b/tools/cfl/internal/cmd/page/edit_test.go index 5a2c1ff3..ece8c473 100644 --- a/tools/cfl/internal/cmd/page/edit_test.go +++ b/tools/cfl/internal/cmd/page/edit_test.go @@ -288,13 +288,13 @@ func TestRunEdit_HTMLFile(t *testing.T) { switch r.Method { case "GET": w.WriteHeader(http.StatusOK) - _, _ = w.Write([]byte(`{ + _, _ = w.Write([]byte(storedPageJSON(receivedBody, `{ "id": "12345", "title": "Test", "version": {"number": 1}, "body": {"storage": {"value": "

Old

"}}, "_links": {"webui": "/pages/12345"} - }`)) + }`))) case "PUT": body, _ := io.ReadAll(r.Body) _ = json.Unmarshal(body, &receivedBody) @@ -344,13 +344,13 @@ func TestRunEdit_NoMarkdownFlag(t *testing.T) { switch r.Method { case "GET": w.WriteHeader(http.StatusOK) - _, _ = w.Write([]byte(`{ + _, _ = w.Write([]byte(storedPageJSON(receivedBody, `{ "id": "12345", "title": "Test", "version": {"number": 1}, "body": {"storage": {"value": "

Old

"}}, "_links": {"webui": "/pages/12345"} - }`)) + }`))) case "PUT": body, _ := io.ReadAll(r.Body) _ = json.Unmarshal(body, &receivedBody) @@ -1265,13 +1265,13 @@ func TestRunEdit_StorageFlag_Stdin(t *testing.T) { switch r.Method { case "GET": w.WriteHeader(http.StatusOK) - _, _ = w.Write([]byte(`{ + _, _ = w.Write([]byte(storedPageJSON(receivedBody, `{ "id": "12345", "title": "Test", "version": {"number": 1}, "body": {"storage": {"value": "

Old

"}}, "_links": {"webui": "/pages/12345"} - }`)) + }`))) case "PUT": body, _ := io.ReadAll(r.Body) _ = json.Unmarshal(body, &receivedBody) @@ -1326,13 +1326,13 @@ func TestRunEdit_StorageFlag_File(t *testing.T) { switch r.Method { case "GET": w.WriteHeader(http.StatusOK) - _, _ = w.Write([]byte(`{ + _, _ = w.Write([]byte(storedPageJSON(receivedBody, `{ "id": "12345", "title": "Test", "version": {"number": 1}, "body": {"storage": {"value": "

Old

"}}, "_links": {"webui": "/pages/12345"} - }`)) + }`))) case "PUT": body, _ := io.ReadAll(r.Body) _ = json.Unmarshal(body, &receivedBody) @@ -1566,13 +1566,13 @@ func mockEditBodyServer(t *testing.T, received *map[string]any) *httptest.Server switch r.Method { case "GET": w.WriteHeader(http.StatusOK) - _, _ = w.Write([]byte(`{ + _, _ = w.Write([]byte(storedPageJSON(*received, `{ "id": "12345", "title": "Test", "version": {"number": 1}, "body": {"storage": {"value": "

Old

"}}, "_links": {"webui": "/pages/12345"} - }`)) + }`))) case "PUT": body, _ := io.ReadAll(r.Body) _ = json.Unmarshal(body, received) @@ -1821,3 +1821,20 @@ func TestRunEdit_EditorBodyFormats(t *testing.T) { }) } } + +// storedPageJSON reports the page the way Confluence would after a write: +// a GET reflects what was stored, not the body the page had beforehand. The +// post-write verification in runEdit compares against this, so a fake that +// kept returning its original body would report drift on every exact-format +// write. +func storedPageJSON(received map[string]any, before string) string { + body, ok := received["body"].(map[string]any) + if !ok { + return before + } + raw, err := json.Marshal(body) + if err != nil { + return before + } + return `{"id":"12345","title":"Test","version":{"number":2},"body":` + string(raw) + `,"_links":{"webui":"/pages/12345"}}` +} diff --git a/tools/cfl/internal/cmd/page/verify.go b/tools/cfl/internal/cmd/page/verify.go new file mode 100644 index 00000000..a56481f8 --- /dev/null +++ b/tools/cfl/internal/cmd/page/verify.go @@ -0,0 +1,465 @@ +package page + +import ( + "context" + "encoding/json" + "fmt" + "sort" + "strings" + + "github.com/open-cli-collective/confluence-cli/api" + "github.com/open-cli-collective/confluence-cli/internal/cmd/root" + cflpresent "github.com/open-cli-collective/confluence-cli/internal/present" +) + +// Confluence normalizes what it stores. A write can land with parts of the +// submitted document silently removed — observed with the +// __confluenceMetadata attributes Confluence strips from link marks — and +// the update response reports success either way. For an exact body format +// cfl transmits the caller's content verbatim, so any difference between +// what was sent and what came back is the server's doing and is precisely +// what the caller cannot otherwise see. +// +// Two kinds of difference carry different weight, so they are reported +// separately: losing text means the content did not land, while losing +// attributes means the document was normalized around content that did. + +// writeDrift describes how a stored body differs from the body that was sent. +type writeDrift struct { + // VisibleSent and VisibleStored count the characters a reader sees, so + // the numbers reported are facts about the document rather than + // positions in the internal fingerprint. + VisibleSent int + VisibleStored int + // AtomsChanged reports that embedded content (cards, images, mentions, + // breaks) differs even where the visible text does not. + AtomsChanged bool + // SentVisible and StoredVisible are the reader-visible text, which is + // where a reported position has to be measured for it to mean anything + // to the operator. + SentVisible string + StoredVisible string + // AtomChanges names embedded node types whose counts moved. Attribute + // diffs cannot cover this: hardBreak and rule carry no attributes, so + // without it a change confined to them has nothing to report. + AtomChanges []string + + // TextChanged reports that the document's text content differs. This is + // a failed write, not a normalization. + TextChanged bool + SentText string + StoredText string + + // DroppedAttrs and AddedAttrs are "nodeType.attrName" keys whose + // occurrence counts fell or rose, in sorted order. + DroppedAttrs []string + AddedAttrs []string +} + +// Clean reports whether the stored body matched what was sent. +func (d writeDrift) Clean() bool { + return !d.TextChanged && len(d.DroppedAttrs) == 0 && len(d.AddedAttrs) == 0 +} + +// compareStoredBody diffs a submitted body against the one Confluence stored. +// Bodies that cannot be parsed are reported as an error rather than silently +// treated as matching — an unverifiable write is not a verified one. +func compareStoredBody(sent, stored, bodyFormat string) (writeDrift, error) { + switch bodyFormat { + case bodyFormatADF: + return compareADF(sent, stored) + case bodyFormatXHTML: + sentText, storedText := xhtmlText(sent), xhtmlText(stored) + return writeDrift{ + TextChanged: sentText != storedText, + SentText: sentText, + StoredText: storedText, + VisibleSent: len([]rune(sentText)), + VisibleStored: len([]rune(storedText)), + SentVisible: sentText, + StoredVisible: storedText, + }, nil + default: + return writeDrift{}, fmt.Errorf("cannot verify %s input: it is converted before sending, so the stored body is not comparable to what was supplied", bodyFormat) + } +} + +func compareADF(sent, stored string) (writeDrift, error) { + var sentDoc, storedDoc any + if err := json.Unmarshal([]byte(sent), &sentDoc); err != nil { + return writeDrift{}, fmt.Errorf("parsing submitted ADF: %w", err) + } + if err := json.Unmarshal([]byte(stored), &storedDoc); err != nil { + return writeDrift{}, fmt.Errorf("parsing stored ADF: %w", err) + } + + sentText, storedText := adfContent(sentDoc), adfContent(storedDoc) + sentVisible, storedVisible := visibleText(sentDoc), visibleText(storedDoc) + dropped, added := diffAttrProfiles(adfAttrProfile(sentDoc), adfAttrProfile(storedDoc)) + return writeDrift{ + TextChanged: sentText != storedText, + SentText: sentText, + StoredText: storedText, + VisibleSent: len([]rune(sentVisible)), + VisibleStored: len([]rune(storedVisible)), + AtomsChanged: sentText != storedText && sentVisible == storedVisible, + SentVisible: sentVisible, + StoredVisible: storedVisible, + AtomChanges: diffAtomProfiles(atomProfile(sentDoc), atomProfile(storedDoc)), + DroppedAttrs: dropped, + AddedAttrs: added, + }, nil +} + +// contentAtoms are ADF leaf nodes that carry content the reader sees but +// hold no text of their own. They must count toward the content fingerprint: +// otherwise dropping an entire card, image or mention leaves the text +// identical and the loss is reported as harmless attribute normalization. +var contentAtoms = map[string]bool{ + "inlineCard": true, + "blockCard": true, + "embedCard": true, + "media": true, + "mediaInline": true, + "mention": true, + "emoji": true, + "status": true, + "date": true, + "extension": true, + "inlineExtension": true, + "bodiedExtension": true, + // Attribute-less atoms: dropping one moves neither the text nor the + // attribute profile, so without naming them here the loss is invisible. + "hardBreak": true, + "rule": true, +} + +// adfContent renders a document's content fingerprint in document order: +// text as itself, and each atom named in contentAtoms as a marker carrying +// its identifying attributes. +// +// The atom set is an allowlist, so the fingerprint is a sound signal and not +// a complete one: a differing fingerprint always means the content changed, +// while an identical one means nothing outside the allowlist moved. A node +// type ADF gains later goes unnoticed until it is added here. +func adfContent(node any) string { + var b strings.Builder + var walk func(any) + walk = func(n any) { + switch v := n.(type) { + case map[string]any: + nodeType, _ := v["type"].(string) + switch { + case nodeType == "text": + if s, ok := v["text"].(string); ok { + b.WriteString(s) + } + case contentAtoms[nodeType]: + // Identity, not just presence: swapping one card for another + // keeps the count identical. + b.WriteString("\x00" + nodeType + "(" + atomIdentity(v) + ")") + } + // Content order is what makes this comparable, so walk it + // explicitly rather than ranging over the map. + if content, ok := v["content"].([]any); ok { + for _, c := range content { + walk(c) + } + } + case []any: + for _, c := range v { + walk(c) + } + } + } + walk(node) + return b.String() +} + +// atomProfile counts content-bearing atoms by type. +func atomProfile(node any) map[string]int { + profile := map[string]int{} + var walk func(any) + walk = func(n any) { + switch v := n.(type) { + case map[string]any: + if t, _ := v["type"].(string); contentAtoms[t] { + profile[t]++ + } + if content, ok := v["content"].([]any); ok { + for _, c := range content { + walk(c) + } + } + case []any: + for _, c := range v { + walk(c) + } + } + } + walk(node) + return profile +} + +// diffAtomProfiles names atom types whose counts moved, in sorted order. +func diffAtomProfiles(sent, stored map[string]int) []string { + seen := map[string]bool{} + var out []string + for k := range sent { + seen[k] = true + } + for k := range stored { + seen[k] = true + } + keys := make([]string, 0, len(seen)) + for k := range seen { + keys = append(keys, k) + } + sort.Strings(keys) + for _, k := range keys { + if sent[k] != stored[k] { + out = append(out, fmt.Sprintf("%s (%d→%d)", k, sent[k], stored[k])) + } + } + return out +} + +// visibleText is only the characters a reader sees, with no atom markers, so +// a length taken from it is a statement about the document. +func visibleText(node any) string { + var b strings.Builder + var walk func(any) + walk = func(n any) { + switch v := n.(type) { + case map[string]any: + if v["type"] == "text" { + if s, ok := v["text"].(string); ok { + b.WriteString(s) + } + } + if content, ok := v["content"].([]any); ok { + for _, c := range content { + walk(c) + } + } + case []any: + for _, c := range v { + walk(c) + } + } + } + walk(node) + return b.String() +} + +// atomIdentity summarizes an atom's identifying attributes so a substitution +// is visible, using a stable order. +func atomIdentity(node map[string]any) string { + attrs, ok := node["attrs"].(map[string]any) + if !ok { + return "" + } + keys := make([]string, 0, len(attrs)) + for k := range attrs { + keys = append(keys, k) + } + sort.Strings(keys) + parts := make([]string, 0, len(keys)) + for _, k := range keys { + parts = append(parts, fmt.Sprintf("%s=%v", k, attrs[k])) + } + return strings.Join(parts, ",") +} + +// adfAttrProfile counts "nodeType.attrName" occurrences, including the +// attributes carried by marks, which is where Confluence's link metadata +// lives. +func adfAttrProfile(node any) map[string]int { + profile := map[string]int{} + var walk func(any, string) + walk = func(n any, parentType string) { + switch v := n.(type) { + case map[string]any: + nodeType, _ := v["type"].(string) + if nodeType == "" { + nodeType = parentType + } + if attrs, ok := v["attrs"].(map[string]any); ok { + for k := range attrs { + profile[nodeType+".attrs."+k]++ + } + } + if marks, ok := v["marks"].([]any); ok { + for _, m := range marks { + walk(m, nodeType) + } + } + if content, ok := v["content"].([]any); ok { + for _, c := range content { + walk(c, nodeType) + } + } + case []any: + for _, c := range v { + walk(c, parentType) + } + } + } + walk(node, "") + return profile +} + +func diffAttrProfiles(sent, stored map[string]int) (dropped, added []string) { + for k, n := range sent { + if stored[k] < n { + dropped = append(dropped, fmt.Sprintf("%s (%d→%d)", k, n, stored[k])) + } + } + for k, n := range stored { + if sent[k] < n { + added = append(added, fmt.Sprintf("%s (%d→%d)", k, sent[k], n)) + } + } + sort.Strings(dropped) + sort.Strings(added) + return dropped, added +} + +// xhtmlText strips tags so storage-format bodies compare on their text. +// Confluence rewrites storage markup freely, so element-level equality would +// report drift on every write. +func xhtmlText(s string) string { + var b strings.Builder + depth := 0 + for _, r := range s { + switch { + case r == '<': + depth++ + case r == '>': + if depth > 0 { + depth-- + } + case depth == 0: + b.WriteRune(r) + } + } + return strings.Join(strings.Fields(b.String()), " ") +} + +// verifyRequest carries what a post-write readback needs, so page edit and +// page create share one verification path. +type verifyRequest struct { + opts *root.Options + client *api.Client + pageID string + bodyFormat string + sentContent string + enabled bool +} + +// verifyStoredBody re-reads a page after a write and reports what Confluence +// actually stored. It only runs for exact body formats: markdown input is +// converted before sending, so the stored body is not comparable to it. +// +// Losing text is an error — the caller's content did not land. Normalization +// is reported and tolerated, because it is the server's prerogative and the +// content survived it. +func verifyStoredBody(ctx context.Context, req verifyRequest) error { + if !req.enabled || req.sentContent == "" { + return nil + } + if req.bodyFormat != bodyFormatADF && req.bodyFormat != bodyFormatXHTML { + return nil + } + + stored, err := getPageWithBodyFormat(ctx, req.client, req.pageID, req.bodyFormat) + if err != nil { + return fmt.Errorf("verifying stored page: %w — the write may have succeeded; re-read the page to confirm", err) + } + storedContent := bodyValue(stored, req.bodyFormat) + if storedContent == "" { + return fmt.Errorf("verifying stored page: no %s body returned — re-read the page to confirm the write", req.bodyFormat) + } + + drift, err := compareStoredBody(req.sentContent, storedContent, req.bodyFormat) + if err != nil { + return fmt.Errorf("verifying stored page: %w", err) + } + if drift.Clean() { + return nil + } + + off := diffOffset(drift.SentVisible, drift.StoredVisible) + finding := cflpresent.WriteDrift{ + BodyFormat: req.bodyFormat, + TextChanged: drift.TextChanged, + VisibleSent: drift.VisibleSent, + VisibleStored: drift.VisibleStored, + AtomsChanged: drift.AtomsChanged, + DiffOffset: off, + SentExcerpt: readableExcerpt(drift.SentVisible, off), + StoredExcerpt: readableExcerpt(drift.StoredVisible, off), + AtomChanges: drift.AtomChanges, + DroppedAttrs: drift.DroppedAttrs, + AddedAttrs: drift.AddedAttrs, + } + if emitErr := cflpresent.Emit(req.opts, cflpresent.PagePresenter{}.PresentWriteDrift(finding)); emitErr != nil { + return emitErr + } + if drift.TextChanged { + return fmt.Errorf("stored page content does not match what was sent") + } + return nil +} + +// bodyValue returns the page body in the requested representation. +func bodyValue(page *api.Page, bodyFormat string) string { + if page == nil || page.Body == nil { + return "" + } + switch bodyFormat { + case bodyFormatADF: + if page.Body.AtlasDocFormat != nil { + return page.Body.AtlasDocFormat.Value + } + case bodyFormatXHTML: + if page.Body.Storage != nil { + return page.Body.Storage.Value + } + } + return "" +} + +// diffOffset reports the character position where two reader-visible texts +// first differ, or -1 when they match. Measuring on the visible text is what +// makes the number a position in the document rather than in the compared +// representation. +func diffOffset(sent, stored string) int { + a, b := []rune(sent), []rune(stored) + limit := len(a) + if len(b) < limit { + limit = len(b) + } + i := 0 + for i < limit && a[i] == b[i] { + i++ + } + if i == limit && len(a) == len(b) { + return -1 + } + return i +} + +// readableExcerpt returns part of the reader-visible text, starting at a +// character position, for quoting back in a report. +func readableExcerpt(s string, at int) string { + r := []rune(s) + if at < 0 || at > len(r) { + return "" + } + end := at + 40 + if end > len(r) { + end = len(r) + } + return string(r[at:end]) +} diff --git a/tools/cfl/internal/cmd/page/verify_test.go b/tools/cfl/internal/cmd/page/verify_test.go new file mode 100644 index 00000000..1e2c99d1 --- /dev/null +++ b/tools/cfl/internal/cmd/page/verify_test.go @@ -0,0 +1,546 @@ +package page + +import ( + "context" + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/open-cli-collective/confluence-cli/api" + + sharedpresent "github.com/open-cli-collective/atlassian-go/present" + + cflpresent "github.com/open-cli-collective/confluence-cli/internal/present" +) + +// adfDoc builds a minimal ADF document around one paragraph's content JSON. +func adfDoc(paragraphContent string) string { + return `{"type":"doc","version":1,"content":[{"type":"paragraph","content":[` + paragraphContent + `]}]}` +} + +func TestCompareStoredBodyADF(t *testing.T) { + // The case this exists for: Confluence stores the text but drops the + // __confluenceMetadata it will not accept back on a link mark. + sentLink := adfDoc(`{"type":"text","text":"see A11","marks":[{"type":"link","attrs":{"href":"https://example.test#a11","__confluenceMetadata":{"linkType":"page"}}}]}`) + storedLink := adfDoc(`{"type":"text","text":"see A11","marks":[{"type":"link","attrs":{"href":"https://example.test#a11"}}]}`) + + tests := []struct { + name string + sent, stored string + wantTextChanged bool + wantDropped int + wantClean bool + }{ + { + name: "identical documents", + sent: adfDoc(`{"type":"text","text":"hello"}`), + stored: adfDoc(`{"type":"text","text":"hello"}`), + wantClean: true, + }, + { + name: "text silently changed", + sent: adfDoc(`{"type":"text","text":"hello"}`), + stored: adfDoc(`{"type":"text","text":"hell"}`), + wantTextChanged: true, + }, + { + name: "server dropped a link attribute but kept the text", + sent: sentLink, + stored: storedLink, + wantDropped: 1, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + d, err := compareStoredBody(tc.sent, tc.stored, bodyFormatADF) + if err != nil { + t.Fatalf("compareStoredBody: %v", err) + } + if d.TextChanged != tc.wantTextChanged { + t.Errorf("TextChanged = %v, want %v", d.TextChanged, tc.wantTextChanged) + } + if len(d.DroppedAttrs) != tc.wantDropped { + t.Errorf("DroppedAttrs = %v, want %d", d.DroppedAttrs, tc.wantDropped) + } + if d.Clean() != tc.wantClean { + t.Errorf("Clean() = %v, want %v", d.Clean(), tc.wantClean) + } + }) + } +} + +// Dropping an attribute must not be reported as losing content: the two +// carry different consequences, and conflating them would either cry wolf on +// every normalized write or hide a real one. +func TestDroppedAttrIsNotTextLoss(t *testing.T) { + sent := adfDoc(`{"type":"text","text":"same text","marks":[{"type":"link","attrs":{"href":"h","__confluenceMetadata":{"linkType":"page"}}}]}`) + stored := adfDoc(`{"type":"text","text":"same text","marks":[{"type":"link","attrs":{"href":"h"}}]}`) + d, err := compareStoredBody(sent, stored, bodyFormatADF) + if err != nil { + t.Fatal(err) + } + if d.TextChanged { + t.Error("attribute normalization reported as text loss") + } + lines := driftReport(t, d) + if !strings.Contains(lines, "Content is intact") { + t.Errorf("report should say the text survived, got:\n%s", lines) + } + if !strings.Contains(lines, "__confluenceMetadata") { + t.Errorf("report should name the dropped attribute, got:\n%s", lines) + } +} + +// Text order matters: two documents with the same words in different order +// are not the same document. +func TestADFTextRespectsOrder(t *testing.T) { + a := adfDoc(`{"type":"text","text":"one "},{"type":"text","text":"two"}`) + b := adfDoc(`{"type":"text","text":"two"},{"type":"text","text":"one "}`) + d, err := compareStoredBody(a, b, bodyFormatADF) + if err != nil { + t.Fatal(err) + } + if !d.TextChanged { + t.Error("reordered text reported as unchanged") + } +} + +func TestCompareStoredBodyXHTMLComparesText(t *testing.T) { + tests := []struct { + name string + sent, stored string + wantChanged bool + }{ + { + name: "markup rewritten, text preserved", + sent: "

hello world

", + stored: `

hello world

`, + }, + { + name: "text actually lost", + sent: "

hello world

", + stored: "

hello

", + wantChanged: true, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + d, err := compareStoredBody(tc.sent, tc.stored, bodyFormatXHTML) + if err != nil { + t.Fatal(err) + } + if d.TextChanged != tc.wantChanged { + t.Errorf("TextChanged = %v, want %v (sent %q stored %q)", d.TextChanged, tc.wantChanged, xhtmlText(tc.sent), xhtmlText(tc.stored)) + } + }) + } +} + +// Markdown is converted before it is sent, so there is nothing to compare it +// against; saying so is better than reporting a false mismatch. +func TestCompareStoredBodyRejectsMarkdown(t *testing.T) { + _, err := compareStoredBody("# hi", "{}", bodyFormatMarkdown) + if err == nil { + t.Fatal("expected an error for markdown input") + } + if !strings.Contains(err.Error(), "converted before sending") { + t.Errorf("error should explain why markdown is not comparable, got: %v", err) + } +} + +// An unparseable body means the write could not be verified, which must not +// be reported as a verified write. +func TestCompareStoredBodyUnparseable(t *testing.T) { + if _, err := compareStoredBody(`{"type":"doc"}`, "not json", bodyFormatADF); err == nil { + t.Fatal("expected an error when the stored body cannot be parsed") + } +} + +func TestDescribeDriftPointsAtTheDifference(t *testing.T) { + d, err := compareStoredBody( + adfDoc(`{"type":"text","text":"alpha bravo charlie"}`), + adfDoc(`{"type":"text","text":"alpha bravo"}`), + bodyFormatADF) + if err != nil { + t.Fatal(err) + } + lines := driftReport(t, d) + if !strings.Contains(lines, "does not hold the content supplied") { + t.Errorf("report should state the content did not land, got:\n%s", lines) + } + if !strings.Contains(lines, "offset") { + t.Errorf("report should locate the difference, got:\n%s", lines) + } +} + +// driftReport renders a finding through the presenter that owns the wording, +// so the tests assert on what an operator actually reads. +func driftReport(t *testing.T, d writeDrift) string { + t.Helper() + model := cflpresent.PagePresenter{}.PresentWriteDrift(cflpresent.WriteDrift{ + BodyFormat: bodyFormatADF, + TextChanged: d.TextChanged, + VisibleSent: d.VisibleSent, + VisibleStored: d.VisibleStored, + AtomsChanged: d.AtomsChanged, + DiffOffset: diffOffset(d.SentVisible, d.StoredVisible), + SentExcerpt: readableExcerpt(d.SentVisible, diffOffset(d.SentVisible, d.StoredVisible)), + StoredExcerpt: readableExcerpt(d.StoredVisible, diffOffset(d.SentVisible, d.StoredVisible)), + AtomChanges: d.AtomChanges, + DroppedAttrs: d.DroppedAttrs, + AddedAttrs: d.AddedAttrs, + }) + var b strings.Builder + for _, sec := range model.Sections { + if msg, ok := sec.(*sharedpresent.MessageSection); ok { + b.WriteString(msg.Message) + } + } + return b.String() +} + +// A dropped content atom is content loss, not normalization: the text is +// unchanged when an entire card, image or mention disappears. +func TestContentAtomLossCountsAsContentChange(t *testing.T) { + sent := adfDoc(`{"type":"text","text":"see "},{"type":"inlineCard","attrs":{"url":"https://example.test/x"}}`) + stored := adfDoc(`{"type":"text","text":"see "}`) + d, err := compareStoredBody(sent, stored, bodyFormatADF) + if err != nil { + t.Fatal(err) + } + if !d.TextChanged { + t.Error("a dropped inlineCard was not reported as content loss") + } +} + +// Swapping one atom for another keeps every count identical, so identity has +// to be part of the fingerprint. +func TestContentAtomSubstitutionDetected(t *testing.T) { + sent := adfDoc(`{"type":"inlineCard","attrs":{"url":"https://example.test/a"}}`) + stored := adfDoc(`{"type":"inlineCard","attrs":{"url":"https://example.test/b"}}`) + d, err := compareStoredBody(sent, stored, bodyFormatADF) + if err != nil { + t.Fatal(err) + } + if !d.TextChanged { + t.Error("a substituted inlineCard was not reported as content loss") + } +} + +// driftServer serves a page whose GET body is whatever storedBody returns, +// so a test can make Confluence "store" something other than what was sent. +func driftServer(t *testing.T, storedBody func(sent map[string]any) string) (*httptest.Server, *int) { + t.Helper() + gets := 0 + var received map[string]any + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.Method { + case "GET": + gets++ + w.WriteHeader(http.StatusOK) + body := `{"storage":{"value":"

Old

"}}` + if received != nil { + body = storedBody(received) + } + _, _ = w.Write([]byte(`{"id":"12345","title":"Test","version":{"number":1},"body":` + body + `,"_links":{"webui":"/pages/12345"}}`)) + case "PUT": + raw, _ := io.ReadAll(r.Body) + _ = json.Unmarshal(raw, &received) + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`{"id":"12345","title":"Test","version":{"number":2},"_links":{"webui":"/pages/12345"}}`)) + default: + w.WriteHeader(http.StatusNotFound) + } + })) + return srv, &gets +} + +func editOptsFor(t *testing.T, srv *httptest.Server, content string, noVerify bool) *editOptions { + t.Helper() + rootOpts := newEditTestRootOptions() + rootOpts.SetAPIClient(api.NewClient(srv.URL, "test@example.com", "token")) + rootOpts.Stdin = strings.NewReader(content) + return &editOptions{ + Options: rootOpts, + pageID: "12345", + file: "-", + bodyFormat: bodyFormatXHTML, + noVerify: noVerify, + } +} + +// The point of the feature: a write the server quietly altered must fail +// rather than report the version number and exit zero. +func TestRunEditFailsWhenStoredContentDiffers(t *testing.T) { + srv, _ := driftServer(t, func(map[string]any) string { + return `{"storage":{"value":"

something else entirely

"}}` + }) + defer srv.Close() + + err := runEdit(context.Background(), editOptsFor(t, srv, "

what we sent

", false)) + if err == nil { + t.Fatal("expected an error when the stored body does not match what was sent") + } + if !strings.Contains(err.Error(), "does not match what was sent") { + t.Errorf("unexpected error: %v", err) + } +} + +// --no-verify keeps the old behavior, including skipping the extra read. +func TestRunEditNoVerifySkipsReadback(t *testing.T) { + srv, gets := driftServer(t, func(map[string]any) string { + return `{"storage":{"value":"

something else entirely

"}}` + }) + defer srv.Close() + + before := *gets + if err := runEdit(context.Background(), editOptsFor(t, srv, "

what we sent

", true)); err != nil { + t.Fatalf("--no-verify should not fail on drift: %v", err) + } + if after := *gets - before; after != 1 { + t.Errorf("GET count = %d, want 1 (the pre-write fetch only)", after) + } +} + +// Normalization is not failure: the write stands and the drift is reported. +func TestRunEditToleratesNormalization(t *testing.T) { + srv, _ := driftServer(t, func(map[string]any) string { + // Same text, markup rewritten — what storage-format normalization + // looks like. + return `{"storage":{"value":"

what we sent

"}}` + }) + defer srv.Close() + + if err := runEdit(context.Background(), editOptsFor(t, srv, "

what we sent

", false)); err != nil { + t.Fatalf("normalized markup should not fail the write: %v", err) + } +} + +// Markdown is converted before sending, so verification must not run and +// must not invent a mismatch. +func TestRunEditSkipsVerificationForMarkdown(t *testing.T) { + srv, gets := driftServer(t, func(map[string]any) string { + return `{"storage":{"value":"

totally different

"}}` + }) + defer srv.Close() + + opts := editOptsFor(t, srv, "# heading", false) + opts.bodyFormat = bodyFormatMarkdown + before := *gets + if err := runEdit(context.Background(), opts); err != nil { + t.Fatalf("markdown edit should not be verified: %v", err) + } + if after := *gets - before; after != 1 { + t.Errorf("GET count = %d, want 1 (no verification read)", after) + } +} + +// The fingerprint's NUL atom separators are an internal detail; an operator +// must never be shown them. +func TestExcerptHidesFingerprintSeparators(t *testing.T) { + sent := adfDoc(`{"type":"text","text":"see "},{"type":"inlineCard","attrs":{"url":"https://example.test/x"}}`) + stored := adfDoc(`{"type":"text","text":"see "}`) + d, err := compareStoredBody(sent, stored, bodyFormatADF) + if err != nil { + t.Fatal(err) + } + report := driftReport(t, d) + if strings.Contains(report, "\x00") || strings.ContainsRune(report, 0) { + t.Errorf("report leaked the fingerprint separator:\n%s", report) + } + if !strings.Contains(report, "inlineCard") { + t.Errorf("report should still name the lost atom via its attributes:\n%s", report) + } +} + +// Attribute-less atoms move neither the text nor the attribute profile, so +// they have to be named in the fingerprint or their loss is invisible. +func TestAttributelessAtomLossDetected(t *testing.T) { + for _, atom := range []string{"hardBreak", "rule"} { + t.Run(atom, func(t *testing.T) { + sent := adfDoc(`{"type":"text","text":"a"},{"type":"` + atom + `"},{"type":"text","text":"b"}`) + stored := adfDoc(`{"type":"text","text":"a"},{"type":"text","text":"b"}`) + d, err := compareStoredBody(sent, stored, bodyFormatADF) + if err != nil { + t.Fatal(err) + } + if !d.TextChanged { + t.Errorf("a dropped %s was not reported as content loss", atom) + } + }) + } +} + +// createDriftServer answers a create and then serves whatever storedBody +// returns on the readback, so a test can make Confluence "store" something +// other than what was posted. +func createDriftServer(t *testing.T, storedBody func() string) (*httptest.Server, *int) { + t.Helper() + gets := 0 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case r.Method == "GET" && strings.Contains(r.URL.Path, "/spaces"): + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`{"results":[{"id":"123456","key":"DEV"}]}`)) + case r.Method == "GET" && strings.Contains(r.URL.Path, "/pages/"): + gets++ + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`{"id":"99999","title":"Test","version":{"number":1},"body":` + storedBody() + `}`)) + case r.Method == "POST" && strings.Contains(r.URL.Path, "/pages"): + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`{"id":"99999","title":"Test","version":{"number":1}}`)) + default: + w.WriteHeader(http.StatusNotFound) + } + })) + return srv, &gets +} + +func createOptsFor(t *testing.T, srv *httptest.Server, content string, noVerify bool) *createOptions { + t.Helper() + rootOpts := newCreateTestRootOptions() + rootOpts.SetAPIClient(api.NewClient(srv.URL, "test@example.com", "token")) + rootOpts.Stdin = strings.NewReader(content) + return &createOptions{ + Options: rootOpts, + space: "DEV", + title: "Test Page", + file: "-", + bodyFormat: bodyFormatXHTML, + noVerify: noVerify, + } +} + +// A create writes the same verbatim body an edit does, so it must fail the +// same way when the server stores something else. +func TestRunCreateFailsWhenStoredContentDiffers(t *testing.T) { + srv, _ := createDriftServer(t, func() string { + return `{"storage":{"value":"

something else entirely

"}}` + }) + defer srv.Close() + + err := runCreate(context.Background(), createOptsFor(t, srv, "

what we sent

", false)) + if err == nil { + t.Fatal("expected an error when the created page does not hold what was sent") + } + if !strings.Contains(err.Error(), "does not match what was sent") { + t.Errorf("unexpected error: %v", err) + } +} + +func TestRunCreateNoVerifySkipsReadback(t *testing.T) { + srv, gets := createDriftServer(t, func() string { + return `{"storage":{"value":"

something else entirely

"}}` + }) + defer srv.Close() + + if err := runCreate(context.Background(), createOptsFor(t, srv, "

what we sent

", true)); err != nil { + t.Fatalf("--no-verify should not fail on drift: %v", err) + } + if *gets != 0 { + t.Errorf("readback GET count = %d, want 0", *gets) + } +} + +func TestRunCreateToleratesNormalization(t *testing.T) { + srv, _ := createDriftServer(t, func() string { + return `{"storage":{"value":"

what we sent

"}}` + }) + defer srv.Close() + + if err := runCreate(context.Background(), createOptsFor(t, srv, "

what we sent

", false)); err != nil { + t.Fatalf("normalized markup should not fail the create: %v", err) + } +} + +// The numbers reported must describe the document, not the internal +// fingerprint: an atom marker must not inflate a character count, and a +// multibyte change must not report equal lengths as if nothing moved. +func TestReportedCountsDescribeTheDocument(t *testing.T) { + t.Run("atom does not inflate the character count", func(t *testing.T) { + sent := adfDoc(`{"type":"text","text":"hello ."},{"type":"inlineCard","attrs":{"url":"https://example.test/a-very-long-url"}}`) + stored := adfDoc(`{"type":"text","text":"hello ."}`) + d, err := compareStoredBody(sent, stored, bodyFormatADF) + if err != nil { + t.Fatal(err) + } + if d.VisibleSent != 7 { + t.Errorf("VisibleSent = %d, want 7 (the characters a reader sees)", d.VisibleSent) + } + if !d.AtomsChanged { + t.Error("AtomsChanged = false, but only the embedded card differs") + } + if r := driftReport(t, d); !strings.Contains(r, "embedded content differs") { + t.Errorf("report should say the embedded content changed:\n%s", r) + } + }) + + t.Run("multibyte text counts runes", func(t *testing.T) { + sent := adfDoc(`{"type":"text","text":"日本語テキスト"}`) + stored := adfDoc(`{"type":"text","text":"日本語"}`) + d, err := compareStoredBody(sent, stored, bodyFormatADF) + if err != nil { + t.Fatal(err) + } + if d.VisibleSent != 7 || d.VisibleStored != 3 { + t.Errorf("visible counts = %d/%d, want 7/3 runes", d.VisibleSent, d.VisibleStored) + } + if off := diffOffset(d.SentVisible, d.StoredVisible); off != 3 { + t.Errorf("diffOffset = %d, want 3 (runes, not bytes)", off) + } + }) +} + +// The XHTML branch must measure the document too, or the report states a +// length of zero characters for a body it never counted. +func TestXHTMLDriftReportsRealCounts(t *testing.T) { + d, err := compareStoredBody("

hello world

", "

hello

", bodyFormatXHTML) + if err != nil { + t.Fatal(err) + } + if d.VisibleSent != len("hello world") || d.VisibleStored != len("hello") { + t.Errorf("visible counts = %d/%d, want %d/%d", d.VisibleSent, d.VisibleStored, len("hello world"), len("hello")) + } + report := driftReport(t, d) + if strings.Contains(report, "0 characters") { + t.Errorf("report claimed zero characters for a measured body:\n%s", report) + } +} + +// An atoms-only change has no position in the visible text, so none is +// claimed. +func TestAtomOnlyChangeReportsNoTextOffset(t *testing.T) { + sent := adfDoc(`{"type":"text","text":"hello"},{"type":"inlineCard","attrs":{"url":"https://example.test/a"}}`) + stored := adfDoc(`{"type":"text","text":"hello"},{"type":"inlineCard","attrs":{"url":"https://example.test/b"}}`) + d, err := compareStoredBody(sent, stored, bodyFormatADF) + if err != nil { + t.Fatal(err) + } + if off := diffOffset(d.SentVisible, d.StoredVisible); off != -1 { + t.Errorf("diffOffset = %d, want -1: the visible text is identical", off) + } + if r := driftReport(t, d); strings.Contains(r, "first difference at offset") { + t.Errorf("report claimed a text position for an atoms-only change:\n%s", r) + } +} + +// hardBreak and rule carry no attributes, so a change confined to them has +// no attribute lines to identify it. The atom names have to carry that. +func TestAttributelessAtomChangeIsNamed(t *testing.T) { + sent := adfDoc(`{"type":"text","text":"a"},{"type":"hardBreak"},{"type":"text","text":"b"}`) + stored := adfDoc(`{"type":"text","text":"a"},{"type":"text","text":"b"}`) + d, err := compareStoredBody(sent, stored, bodyFormatADF) + if err != nil { + t.Fatal(err) + } + if len(d.DroppedAttrs) != 0 { + t.Fatalf("precondition: hardBreak should contribute no attributes, got %v", d.DroppedAttrs) + } + report := driftReport(t, d) + if !strings.Contains(report, "hardBreak") { + t.Errorf("report must name the changed atom when no attributes can:\n%s", report) + } +} diff --git a/tools/cfl/internal/present/mutation.go b/tools/cfl/internal/present/mutation.go index 612a1b5b..0f95a39c 100644 --- a/tools/cfl/internal/present/mutation.go +++ b/tools/cfl/internal/present/mutation.go @@ -2,6 +2,7 @@ package present import ( "fmt" + "strings" sharedpresent "github.com/open-cli-collective/atlassian-go/present" @@ -122,3 +123,106 @@ func pageVersionValue(v *api.Version) string { } return fmt.Sprintf("%d", v.Number) } + +// WriteDrift describes how a stored page body differed from the body that +// was submitted. Commands supply the finding; the wording is owned here. +type WriteDrift struct { + BodyFormat string + // TextChanged reports that content differs, not merely formatting. + TextChanged bool + // VisibleSent and VisibleStored count characters a reader sees. + VisibleSent int + VisibleStored int + // AtomsChanged reports embedded content differing where the text does not. + AtomsChanged bool + // DiffOffset is where the two bodies first diverge, or -1 when they do + // not. SentExcerpt and StoredExcerpt are the text from that point. + DiffOffset int + SentExcerpt string + StoredExcerpt string + // AtomChanges names embedded node types whose counts moved. + AtomChanges []string + DroppedAttrs []string + AddedAttrs []string +} + +// attrLines lists attribute changes, which identify the embedded content +// involved when the visible text cannot. +func attrLines(d WriteDrift) []string { + var lines []string + if len(d.AtomChanges) > 0 { + lines = append(lines, " embedded content changed:") + for _, a := range d.AtomChanges { + lines = append(lines, " ~ "+a) + } + } + if len(d.DroppedAttrs) > 0 { + lines = append(lines, " attributes dropped:") + for _, a := range d.DroppedAttrs { + lines = append(lines, " - "+a) + } + } + if len(d.AddedAttrs) > 0 { + lines = append(lines, " attributes added:") + for _, a := range d.AddedAttrs { + lines = append(lines, " + "+a) + } + } + return lines +} + +// describeContentChange states what moved in terms of the document: the +// characters a reader sees, and whether embedded content changed underneath +// unchanged text. +func describeContentChange(d WriteDrift) string { + if d.AtomsChanged { + return fmt.Sprintf("visible text is unchanged at %d characters, but embedded content differs", d.VisibleSent) + } + if d.VisibleSent == d.VisibleStored { + return fmt.Sprintf("content differs at the same length of %d characters", d.VisibleSent) + } + return fmt.Sprintf("visible text went from %d to %d characters", d.VisibleSent, d.VisibleStored) +} + +// PresentWriteDrift reports what Confluence stored versus what was sent. +// Losing content and normalizing attributes are worded differently because +// they oblige the reader differently: one means the change did not land, the +// other means it landed in a document the server tidied. +func (PagePresenter) PresentWriteDrift(d WriteDrift) *sharedpresent.OutputModel { + var lines []string + if d.TextChanged { + lines = append(lines, + fmt.Sprintf("Stored %s body does not match what was sent: %s.", d.BodyFormat, describeContentChange(d)), + "The page was updated, but it does not hold the content supplied. Re-read the page before treating the change as applied.", + ) + if d.DiffOffset >= 0 { + lines = append(lines, fmt.Sprintf(" first difference at offset %d — sent %q, stored %q", d.DiffOffset, d.SentExcerpt, d.StoredExcerpt)) + } + // Name what vanished. When only embedded content moved there is no + // text position to point at, so this is the only detail available. + lines = append(lines, attrLines(d)...) + } else { + if len(d.DroppedAttrs) > 0 { + lines = append(lines, fmt.Sprintf("Confluence normalized the stored %s body. Content is intact; these attributes were dropped:", d.BodyFormat)) + for _, a := range d.DroppedAttrs { + lines = append(lines, " - "+a) + } + } + if len(d.AddedAttrs) > 0 { + lines = append(lines, "Confluence added attributes that were not sent:") + for _, a := range d.AddedAttrs { + lines = append(lines, " + "+a) + } + } + } + if len(lines) == 0 { + return &sharedpresent.OutputModel{} + } + return &sharedpresent.OutputModel{Sections: []sharedpresent.Section{ + &sharedpresent.MessageSection{ + Kind: sharedpresent.MessageWarning, + Message: strings.Join(lines, "\n"), + Stream: sharedpresent.StreamStderr, + }, + }} +}