diff --git a/CLAUDE.md b/CLAUDE.md index 1194903..27a4afe 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -64,4 +64,4 @@ The design target is **semantic** (not byte-for-byte) equivalence to valid Confl ### Frontmatter-driven publishing -Each markdown file is one page. Frontmatter carries `title`, `page_id`, `space` (a key), `parent` (`null` / a `.md` path / a page id), and `page_width`. `update` requires a `page_id` (from frontmatter or `--page-id`) and errors without one; `--title`/`--page-id` override the frontmatter (single FILE only), `--page-width` overrides too (batch allowed), and `update` never writes back to files. It asserts `page_width` only when set via flag or frontmatter (otherwise the live width is left alone), and skips a file whose mtime predates the page's last version unless `--force`. `create` takes `--title`/`--page-width` overrides (`--title` single-FILE only; `--page-width` batch-ok, default `max`) and, unless `--no-persist` is given, writes `title`/`space`/`parent`/`page_id`/`page_width` back after creating; `fix` reconciles all of these (plus `page_width`) from the live page. Commands process multiple files (except `info`, single-arg) and exit non-zero if any fail. +Each markdown file is one page. Frontmatter carries `title`, `page_id`, `space` (a key), `parent` (`null` / a `.md` path / a page id), and `page_width`. `update` requires a `page_id` (from frontmatter or `--page-id`) and errors without one; `--title`/`--page-id` override the frontmatter (single FILE only), `--page-width` overrides too (batch allowed), and `update` never writes back to files. It asserts `page_width` only when set via flag or frontmatter (otherwise the live width is left alone), and skips a file whose mtime predates the page's last version unless `--force`. `create` takes `--title`/`--page-width` overrides (`--title` single-FILE only; `--page-width` batch-ok, default `max`) and, unless `--no-persist` is given, writes `title`/`space`/`parent`/`page_id`/`page_width` back after creating; `fix` reconciles all of these (plus `page_width`) from the live page. `update`, `create`, and `fix` all take `--dry-run`, which previews the actions (pages, attachment uploads, width changes, and — for `create` — frontmatter write-backs) without writing to Confluence or files; per-file human output is identical to a real run, distinguished only by a leading `DRY RUN` banner and a `dry_run: true` field in `--json`. In a `create` dry-run a previewed page has no id/URL yet, and an in-set child's `parent` is null; the parent's source `.md` is always reported in the `parent_file` output field. Commands process multiple files (except `info`, single-arg) and exit non-zero if any fail. diff --git a/README.md b/README.md index 2445a7e..777da17 100644 --- a/README.md +++ b/README.md @@ -92,12 +92,20 @@ A whole tree can be created in one pass: give each child a `parent:` that points its parent's `.md` file, and `create` orders creation parents-first and fills in the real ids (see the `parent` field below). +`--dry-run` validates every file (the same checks a real run makes, so it exits +non-zero on the same failures) and previews what would be created — pages, +attachment uploads, page widths, and frontmatter write-backs — without writing to +Confluence or to any file. Because nothing is created, a previewed page has no id +or URL yet; an in-set child's `parent` is unresolved, but its source file is +reported in the `parent_file` output field (present in every run, in `--json`). + ```sh markfluence create docs/new_page.md --space ENG markfluence create docs/child.md --space ENG --parent 123456 markfluence create docs/*.md --space ENG # hierarchy via parent: paths markfluence create note.md --space ENG --title "Ad-hoc note" --page-width wide markfluence create note.md --space ENG --no-persist # create without touching the file +markfluence create docs/*.md --space ENG --dry-run # preview; write nothing ``` ### `update` @@ -120,6 +128,11 @@ Updates are skipped when a file hasn't changed since the page's last version (compared by mtime) unless `--force` is given. Each file is processed independently; the command exits non-zero if any file fails. +`--dry-run` previews what would be published — the version bump, attachment +uploads, and any page-width change — without writing to Confluence. It honors the +mtime skip and `--force` just like a real run, so its forecast matches what a real +run would do. + ```sh markfluence update docs/managing_an_incident.md markfluence update docs/*.md --message "Bulk update" @@ -127,6 +140,7 @@ markfluence update docs/foo.md --force # ignore the mtime check markfluence update page.md --page-id 123456 # override the target page markfluence update page.md --title "New Title" # override / rename markfluence update docs/*.md --page-width wide # set width across a batch +markfluence update docs/*.md --dry-run # preview; write nothing ``` ### `fix` diff --git a/_plans/016_dry-run.md b/_plans/016_dry-run.md new file mode 100644 index 0000000..f14dc8e --- /dev/null +++ b/_plans/016_dry-run.md @@ -0,0 +1,133 @@ +# Plan: `--dry-run` for `update` and `create` + +Add `--dry-run` to `update` and `create` so they preview what a real run would do +— pages created/updated (and version bumps), attachments that would upload, +page-width changes, and (for `create`) frontmatter write-backs — **without writing +to Confluence or to files**. `fix` already has `--dry-run`; this brings the two +publishing commands to parity (issue #11). + +Exit-non-zero semantics must match a real run's validation failures. + +## Decisions locked (from the interview) + +### Status model — reuse the verb, add `dry_run` + +- **No new status verbs.** A dry-run keeps the existing per-file status + (`published`/`skipped`/`created`/`failed`); the machine-readable signal is a + required, always-emitted `dry_run` boolean on `updateResult`/`createResult` + (mirroring the `dry_run` already on `fixResult`). Consumers key off `dry_run` to + know nothing was written. `summarize()` is untouched. + +### Human output — identical per-file lines + one banner + +- **Per-file lines are byte-identical between dry-run and a real run.** We do *not* + reword them (no "would publish" per line). The only human cue is a single leading + `ui.Warn` banner emitted once per invocation: + + ``` + DRY RUN — no changes will be written. + ``` + + The banner is **suppressed under `--json`** (it would corrupt the JSON document; + `dry_run: true` is the signal there). +- **`fix` is converted to this same rule**: drop its `renderHuman` "would set" verb + swap (verb is always "set") and add the banner. Update `fix`'s affected tests. +- The one **unavoidable exception** is `create`'s final success line. A real run + prints `Created page : `, but a dry-run has created nothing, so no id and + no URL exist. `create` dry-run prints a distinct line instead: + + ``` + Would create page 'Title' in SPACE + ``` + + (`update` has no such problem: its URL comes from the read-only `GetPage` and its + new version is `prev+1`, so `Published v4: ` prints identically and truly.) + +### `update --dry-run` + +All of `update`'s pre-write work is already read-only, so the flow is unchanged +through conversion; only the writes are swapped for previews. + +1. Parse, resolve title/page_id/width, `GetPage` — unchanged (reads). +2. mtime skip (unless `--force`) — **honored**. A file that would be skipped reports + `status: "skipped"` with the identical `Skipping -- no changes` line and short- + circuits before any attachment/width preview, exactly as a real run. +3. `convert` — unchanged; surfaces `broken`/`warnings` and the attachment list. +4. Attachments: previewed via a **new read-only `client.PlanAttachments`** that + shares the per-file decision logic with `SyncAttachments` (`ListAttachments` + + checksum compare) but performs **no uploads** → accurate + `created`/`updated`/`skipped`. Real run still calls `SyncAttachments`. +5. Version: reported as `{previous, new: previous+1}` (the same value a real run + would send); no `UpdatePage` call. +6. Width: previewed via `pagewidth.Read`; reported **only when it differs** from the + live width. No `pagewidth.Apply` call. (`fix` already reads live width in its + dry-run — precedent.) + +### `create --dry-run` + +`create`'s **phase 1 (validation) runs unchanged** — it performs no writes, so all +validation errors, their codes, and the abort→exit-1 behavior (including +`topoSort` cycle detection) match a real run for free. Only **phase 2** is swapped +from create to preview: + +- `convert` surfaces `broken`/`warnings` and the attachment list. +- Attachments: a not-yet-created page has nothing to read, so every attachment is + synthesized as `created` (no `PlanAttachments` call, no server read). +- `page_id`/`url` stay `null`; `status: "created"`, `dry_run: true`. +- Width shown from the record (no `Apply`). +- `persisted` reflects **intent**: `true` when `--persist` is in effect; `dry_run: + true` conveys nothing was actually written. No extra human line — a real run + prints nothing about persistence either, and the identical-messages rule keeps it + that way; the write-back intent lives in the JSON (`persisted` + `dry_run`). + +### New `parent_file` field (create only, always present) + +To give an automated consumer the parent→child relationship **without corrupting** +`parent` (whose contract stays "a page id or null"), add a separate +`parent_file` (`stringOrNull`) to `createResult`, **emitted in every run** (not +just dry-run): + +| parent kind | `parent` (id) | `parent_file` | +|---|---|---| +| top-level | `null` | `null` | +| external (raw id) | `` | `null` | +| published (`.md` w/ page_id) | `` | `foo.md` | +| in-set (sibling being created) | real: `` · **dry-run: `null`** | `foo.md` | + +Value = `parentInfo.display` (already populated for in-set and published parents). +In a dry-run an in-set child shows `parent: null` **and** `parent_file: "foo.md"`, +so the relationship is explicit; results are still emitted in topo order too. + +**Rationale:** a synthesized/fake parent id is a footgun for an agent (it could feed +the fake id into a later real call); `null` is unambiguous and safe, and a +separately-named field beats overloading `parent` with a `.md` path. + +### Flag + +Each command gets its own `--dry-run` bool flag (as `fix` has), default false. + +## Schema (`schema/json-output/v1.json`) — amend in place + +- `updateResult`: add `dry_run` to properties + `required`. +- `createResult`: add `dry_run` **and** `parent_file` to properties + `required`. +- **`abortedResult()` must emit both new fields** (it builds `createResult`s; a + dry-run that aborts in phase 1 would otherwise fail conformance): `dry_run` = + the run's flag, `parent_file` = null. +- Amending `v1` (not minting `v2`) matches the precedent `fix` set when it added + `dry_run` to `v1`. + +## Testing + +- `client.PlanAttachments`: unit test the created/updated/skipped decision (shared + helper with `SyncAttachments`, no uploads). +- `update`/`create` dry-run: human + `--json` result-shape tests, including the + stale-file skip (update) and an in-set-parent dry-run (create → `parent: null`, + `parent_file` set). +- `fix`: fix up tests that asserted the "would set" wording. +- `TestSchemaConformance`: extend to cover the new required fields. + +## Docs + +- CLAUDE.md: note `--dry-run` on `update`/`create` (parity with `fix`) and the new + `parent_file` output field. +- README: document `--dry-run` for both commands. diff --git a/cmd/create/create.go b/cmd/create/create.go index 306f626..7f9e888 100644 --- a/cmd/create/create.go +++ b/cmd/create/create.go @@ -27,6 +27,7 @@ var ( pageWidthOpt string persistOpt bool noPersistOpt bool + dryRunOpt bool ) // Cmd is the create command. @@ -54,6 +55,8 @@ func init() { "Write title/space/parent/page_id/page_width back into the frontmatter.") Cmd.Flags().BoolVar(&noPersistOpt, "no-persist", false, "Do not write anything back into the frontmatter.") + Cmd.Flags().BoolVar(&dryRunOpt, "dry-run", false, + "Preview what would be created without writing to Confluence or files.") } // parentInfo describes a resolved parent. kind is top|inset|published|external. @@ -93,6 +96,10 @@ func run(cmd *cobra.Command, args []string) error { return fatalFail(err.Error(), jsonout.CodeConfig) } + if dryRunOpt { + ui.Warn("DRY RUN — no changes will be written.") + } + inSetAbs := map[string]bool{} for _, f := range args { if abs, err := filepath.Abs(f); err == nil { @@ -184,7 +191,10 @@ func createInOrder( parentID := r.parent.id if r.parent.kind == "inset" { parentID = created[r.parent.abs] - if parentID == "" { + // In a dry-run nothing is created, so an in-set parent has no id yet; + // that is not a failure (the parent would have been created first). The + // relationship is still reported via parent_file. + if parentID == "" && !dryRunOpt { res := newResult(r) return res.fail(errors.New("parent page was not created; skipping"), jsonout.CodeValidation) } @@ -380,6 +390,22 @@ func createOne(r record, parentID string, c *client.ConfluenceClient, persist bo res.broken = append(res.broken, pageContent.Broken...) res.warnings = append(res.warnings, pageContent.Warnings...) + // --dry-run: preview without creating. The page has no id/URL yet (they stay + // null); every attachment would be a fresh upload, and a new page always has + // its width set. persisted reflects intent — dry_run signals nothing was + // actually written. + if dryRunOpt { + for _, a := range pageContent.Attachments { + res.attachments = append(res.attachments, jsonout.Attachment{Action: "created", Filename: a.Filename}) + } + res.width = &jsonout.PageWidth{Value: string(r.width), Default: false} + res.widthSet = true + res.persisted = persist + res.ok = true + res.status = statusCreated + return res + } + result, err := c.CreatePage(r.spaceID, r.title, pageContent.HTML, parentID) if err != nil { return res.fail(err, jsonout.CodeFor(err)) diff --git a/cmd/create/json.go b/cmd/create/json.go index 0238ead..4085ea9 100644 --- a/cmd/create/json.go +++ b/cmd/create/json.go @@ -21,10 +21,12 @@ type createResult struct { file string ok bool status string + dryRun bool pageID string title string space string parent *string + parentFile *string url string width *jsonout.PageWidth widthSet bool @@ -38,7 +40,10 @@ type createResult struct { // newResult seeds a result with the fields known before creation is attempted. func newResult(r record) *createResult { - return &createResult{file: r.filename, title: r.title, space: r.spaceKey} + return &createResult{ + file: r.filename, title: r.title, space: r.spaceKey, + dryRun: dryRunOpt, parentFile: nullableStr(r.parent.display), + } } func (r *createResult) fail(err error, code jsonout.Code) *createResult { @@ -68,6 +73,12 @@ func (r *createResult) renderHuman() { if r.widthSet && r.width != nil { ui.Info(prefix + " page width: " + r.width.Value) } + // A dry-run has created no page, so there is no id or URL to print; name the + // title and space instead. Every other line above is identical to a real run. + if r.dryRun { + ui.Success(fmt.Sprintf("%s Would create page '%s' in %s", prefix, r.title, r.space)) + return + } ui.Success(fmt.Sprintf("%s Created page %s: %s", prefix, r.pageID, r.url)) } @@ -75,11 +86,13 @@ func (r *createResult) renderHuman() { type jsonCreateResult struct { OK bool `json:"ok"` Status string `json:"status"` + DryRun bool `json:"dry_run"` File string `json:"file"` PageID *string `json:"page_id"` Title *string `json:"title"` Space *string `json:"space"` Parent *string `json:"parent"` + ParentFile *string `json:"parent_file"` URL *string `json:"url"` PageWidth *jsonout.PageWidth `json:"page_width"` Persisted bool `json:"persisted"` @@ -94,11 +107,13 @@ func (r *createResult) jsonResult() jsonCreateResult { res := jsonCreateResult{ OK: r.ok, Status: r.status, + DryRun: r.dryRun, File: r.file, PageID: nullableStr(r.pageID), Title: nullableStr(r.title), Space: nullableStr(r.space), Parent: r.parent, + ParentFile: r.parentFile, URL: nullableStr(r.url), PageWidth: r.width, Persisted: r.persisted, @@ -191,6 +206,7 @@ func abortedResult(file, status, errMsg string, code jsonout.Code) jsonCreateRes res := jsonCreateResult{ OK: false, Status: status, + DryRun: dryRunOpt, File: file, Attachments: []jsonout.Attachment{}, Warnings: []string{}, diff --git a/cmd/create/json_test.go b/cmd/create/json_test.go index a7da2d3..c4cabc6 100644 --- a/cmd/create/json_test.go +++ b/cmd/create/json_test.go @@ -12,6 +12,7 @@ import ( func TestSchemaConformance(t *testing.T) { // A normal (non-aborted) create batch. parent := "123" + parentFile := "index.md" results := []*createResult{ { file: "child.md", ok: true, status: statusCreated, @@ -19,6 +20,14 @@ func TestSchemaConformance(t *testing.T) { width: &jsonout.PageWidth{Value: "max", Default: false}, persisted: true, }, + { + // A dry-run of an in-set child: no page id/url yet, parent unresolved + // (null), but the relationship is reported via parent_file. + file: "grandchild.md", ok: true, status: statusCreated, dryRun: true, + title: "Grandchild", space: "ENG", parentFile: &parentFile, + width: &jsonout.PageWidth{Value: "max", Default: false}, + persisted: true, + }, (&createResult{file: "bad.md"}).fail(errString("boom"), jsonout.CodeConvert), } items := make([]any, len(results)) @@ -66,11 +75,13 @@ func TestJSONResultCreated(t *testing.T) { want := `{ "ok": true, "status": "created", + "dry_run": false, "file": "child.md", "page_id": "456", "title": "Child", "space": "ENG", "parent": "123", + "parent_file": null, "url": "https://wiki.example.net/wiki/spaces/ENG/pages/456/Child", "page_width": { "value": "max", @@ -88,6 +99,35 @@ func TestJSONResultCreated(t *testing.T) { } } +func TestJSONResultDryRunInSetParent(t *testing.T) { + parentFile := "index.md" + r := &createResult{ + file: "child.md", ok: true, status: statusCreated, dryRun: true, + title: "Child", space: "ENG", parentFile: &parentFile, + width: &jsonout.PageWidth{Value: "max", Default: false}, + persisted: true, + } + j := r.jsonResult() + if !j.DryRun { + t.Errorf("dry_run = false, want true") + } + // No page was created, so id/url are null; parent is unresolved but the + // source file is reported so the relationship is not lost. + if j.PageID != nil || j.URL != nil { + t.Errorf("page_id=%v url=%v, want both nil in a dry-run", j.PageID, j.URL) + } + if j.Parent != nil { + t.Errorf("parent = %v, want nil (in-set parent has no id yet)", j.Parent) + } + if j.ParentFile == nil || *j.ParentFile != "index.md" { + t.Errorf("parent_file = %v, want index.md", j.ParentFile) + } + // persisted reflects intent even though nothing was written. + if !j.Persisted { + t.Errorf("persisted = false, want true (intent)") + } +} + func TestAbortedResultShapes(t *testing.T) { // A validation-failed file. failed := abortedResult("bad.md", statusFailed, "no title given", jsonout.CodeValidation) diff --git a/cmd/fix/fix.go b/cmd/fix/fix.go index 3073a26..9d2c344 100644 --- a/cmd/fix/fix.go +++ b/cmd/fix/fix.go @@ -50,6 +50,10 @@ func run(cmd *cobra.Command, args []string) error { return ui.SilentExit(2) } + if dryRun { + ui.Warn("DRY RUN — no changes will be written.") + } + failures := 0 results := make([]*fixResult, 0, len(args)) for _, filename := range args { diff --git a/cmd/fix/json.go b/cmd/fix/json.go index 4095f53..1c94c74 100644 --- a/cmd/fix/json.go +++ b/cmd/fix/json.go @@ -55,12 +55,10 @@ func (r *fixResult) renderHuman() { ui.Info(prefix + " already consistent") return } - verb := "set" - if r.dryRun { - verb = "would set" - } + // The per-field lines are identical in a dry-run; the leading DRY RUN banner + // (and dry_run in --json) is the only signal nothing was written. for _, ch := range r.changes { - ui.Info(fmt.Sprintf("%s %s %s: %s -> %s", prefix, verb, ch.field, ch.oldDisplay, ch.newValue)) + ui.Info(fmt.Sprintf("%s set %s: %s -> %s", prefix, ch.field, ch.oldDisplay, ch.newValue)) } } diff --git a/cmd/update/json.go b/cmd/update/json.go index c05afef..7fc1006 100644 --- a/cmd/update/json.go +++ b/cmd/update/json.go @@ -21,6 +21,7 @@ type updateResult struct { file string ok bool status string + dryRun bool pageID string title string space string @@ -79,6 +80,7 @@ func (r *updateResult) renderHuman() { type jsonUpdateResult struct { OK bool `json:"ok"` Status string `json:"status"` + DryRun bool `json:"dry_run"` File string `json:"file"` PageID *string `json:"page_id"` Title *string `json:"title"` @@ -102,6 +104,7 @@ func (r *updateResult) jsonResult() jsonUpdateResult { res := jsonUpdateResult{ OK: r.ok, Status: r.status, + DryRun: r.dryRun, File: r.file, PageID: strOrNil(r.pageID), Title: strOrNil(r.title), diff --git a/cmd/update/json_test.go b/cmd/update/json_test.go index 818561b..4b7d5b9 100644 --- a/cmd/update/json_test.go +++ b/cmd/update/json_test.go @@ -18,6 +18,11 @@ func TestSchemaConformance(t *testing.T) { width: &jsonout.PageWidth{Value: "max", Default: false}, attachments: []jsonout.Attachment{{Action: "updated", Filename: "d.png"}}, }, + { + file: "docs/bar.md", ok: true, status: statusPublished, dryRun: true, + pageID: "456", title: "Bar", space: "ENG", url: "https://x/456", + versionPrev: 1, versionNew: 2, + }, (&updateResult{file: "bad.md"}).fail(errTest("boom"), jsonout.CodeValidation), } items := make([]any, len(results)) @@ -48,6 +53,7 @@ func TestJSONResultPublished(t *testing.T) { want := `{ "ok": true, "status": "published", + "dry_run": false, "file": "docs/foo.md", "page_id": "123", "title": "Foo", @@ -77,6 +83,25 @@ func TestJSONResultPublished(t *testing.T) { } } +func TestJSONResultDryRun(t *testing.T) { + r := &updateResult{ + file: "docs/foo.md", ok: true, status: statusPublished, dryRun: true, + pageID: "123", title: "Foo", space: "ENG", url: "https://x/123", + versionPrev: 3, versionNew: 4, + } + j := r.jsonResult() + if !j.DryRun { + t.Errorf("dry_run = false, want true") + } + // A dry-run still forecasts the version bump the real run would make. + if j.Version == nil || j.Version.Previous != 3 || j.Version.New != 4 { + t.Errorf("version = %+v, want previous=3 new=4", j.Version) + } + if j.Status != statusPublished { + t.Errorf("status = %q, want %q (dry-run reuses the verb)", j.Status, statusPublished) + } +} + func TestJSONResultSkipped(t *testing.T) { r := &updateResult{ file: "f.md", ok: true, status: statusSkipped, diff --git a/cmd/update/update.go b/cmd/update/update.go index 5379864..5a551fa 100644 --- a/cmd/update/update.go +++ b/cmd/update/update.go @@ -22,6 +22,7 @@ import ( var ( message string force bool + dryRun bool titleFlag string pageIDFlag string pageWidthFlag string @@ -44,6 +45,8 @@ var Cmd = &cobra.Command{ func init() { Cmd.Flags().StringVar(&message, "message", "Updated via markfluence", "Version message.") Cmd.Flags().BoolVar(&force, "force", false, "Skip the file-mtime check and always update the page.") + Cmd.Flags().BoolVar(&dryRun, "dry-run", false, + "Preview what would be published without writing to Confluence.") Cmd.Flags().StringVar(&titleFlag, "title", "", "Override the page title (requires a single FILE).") Cmd.Flags().StringVar(&pageIDFlag, "page-id", "", @@ -71,6 +74,10 @@ func run(cmd *cobra.Command, args []string) error { return ui.SilentExit(2) } + if dryRun { + ui.Warn("DRY RUN — no changes will be written.") + } + failures := 0 results := make([]*updateResult, 0, len(args)) for _, filename := range args { @@ -109,7 +116,7 @@ func run(cmd *cobra.Command, args []string) error { // processFile publishes one file and returns a result describing the outcome. It // performs no output itself; the caller renders the result (human lines or JSON). func processFile(filename string, c *client.ConfluenceClient) *updateResult { - r := &updateResult{file: filename} + r := &updateResult{file: filename, dryRun: dryRun} mf, err := frontmatter.ParseFile(filename) if err != nil { return r.fail(err, jsonout.CodeValidation) @@ -156,6 +163,26 @@ func processFile(filename string, c *client.ConfluenceClient) *updateResult { r.broken = append(r.broken, pageContent.Broken...) r.warnings = append(r.warnings, pageContent.Warnings...) + next := page.Version.Number + 1 + + // --dry-run: preview attachments (read-only) and the width change, but make + // no writes. The version bump and page URL are the same values a real run + // would produce, so the human output lines are identical. + if dryRun { + actions, err := c.PlanAttachments(pageID, toLocalAttachments(pageContent.Attachments)) + if err != nil { + return r.fail(err, jsonout.CodeFor(err)) + } + for _, a := range actions { + r.attachments = append(r.attachments, jsonout.Attachment{Action: a.Action, Filename: a.Filename}) + } + r.versionNew = next + r.previewWidth(c, pageID, width, applyWidth) + r.ok = true + r.status = statusPublished + return r + } + actions, err := c.SyncAttachments(pageID, toLocalAttachments(pageContent.Attachments)) if err != nil { return r.fail(err, jsonout.CodeFor(err)) @@ -164,7 +191,6 @@ func processFile(filename string, c *client.ConfluenceClient) *updateResult { r.attachments = append(r.attachments, jsonout.Attachment{Action: a.Action, Filename: a.Filename}) } - next := page.Version.Number + 1 result, err := c.UpdatePage(pageID, title, pageContent.HTML, next, message) if err != nil { return r.fail(err, jsonout.CodeFor(err)) @@ -194,6 +220,28 @@ func processFile(filename string, c *client.ConfluenceClient) *updateResult { return r } +// previewWidth reports the width change a dry-run update would make. It reads the +// live width (read-only) and marks a change only when it differs from the intended +// width — mirroring fix's dry-run, and matching the real run's "page width:" line +// only when there is something to change. A read failure is a warning, not fatal. +func (r *updateResult) previewWidth( + c *client.ConfluenceClient, pageID string, width pagewidth.Width, applyWidth bool, +) { + if !applyWidth { + return + } + live, _, err := pagewidth.Read(c, pageID) + if err != nil { + r.warnings = append(r.warnings, "could not read page width: "+err.Error()) + return + } + if live == width { + return + } + r.width = &jsonout.PageWidth{Value: string(width), Default: false} + r.widthSet = true +} + // overrideNeedsSingleFile reports whether a per-page override (--title/--page-id) // was given with anything other than exactly one FILE. --page-width is exempt (a // uniform width change across a batch is sensible). diff --git a/internal/client/client.go b/internal/client/client.go index 069bb49..be62c05 100644 --- a/internal/client/client.go +++ b/internal/client/client.go @@ -463,10 +463,21 @@ func (c *ConfluenceClient) ListAttachments(pageID string) ([]Attachment, error) return out.Results, nil } -// SyncAttachments creates, updates, or skips attachments so the page matches the -// local files, using a SHA-256 stored in each attachment's comment to detect -// changes. Returns one action per file. -func (c *ConfluenceClient) SyncAttachments(pageID string, attachments []LocalAttachment) ([]SyncAction, error) { +// attachmentPlan is the decision for one local attachment: the action to take +// plus the parameters an upload would need. It is what planAttachments computes +// and both SyncAttachments (executes) and PlanAttachments (reports) consume. +type attachmentPlan struct { + att LocalAttachment + action string // "created", "updated", or "skipped" + comment string + contentType string + existingID string // the current attachment id, for an "updated" upload +} + +// planAttachments decides, per file, whether it would be created, updated, or +// skipped — reading the page's existing attachments and each local file's +// checksum, but performing no uploads. Preserves input order. +func (c *ConfluenceClient) planAttachments(pageID string, attachments []LocalAttachment) ([]attachmentPlan, error) { if len(attachments) == 0 { return nil, nil } @@ -479,7 +490,7 @@ func (c *ConfluenceClient) SyncAttachments(pageID string, attachments []LocalAtt remote[a.Title] = a } - var actions []SyncAction + plans := make([]attachmentPlan, 0, len(attachments)) for _, att := range attachments { sum, err := fileChecksum(att.Path) if err != nil { @@ -490,26 +501,61 @@ func (c *ConfluenceClient) SyncAttachments(pageID string, attachments []LocalAtt if contentType == "" { contentType = "application/octet-stream" } - + p := attachmentPlan{att: att, comment: comment, contentType: contentType} cur, ok := remote[att.Filename] switch { case !ok: + p.action = "created" + case cur.Metadata.Comment == comment: + p.action = "skipped" + default: + p.action = "updated" + p.existingID = cur.ID + } + plans = append(plans, p) + } + return plans, nil +} + +// PlanAttachments reports what SyncAttachments would do — created/updated/skipped +// per file — without uploading anything. Used by --dry-run. +func (c *ConfluenceClient) PlanAttachments(pageID string, attachments []LocalAttachment) ([]SyncAction, error) { + plans, err := c.planAttachments(pageID, attachments) + if err != nil { + return nil, err + } + actions := make([]SyncAction, 0, len(plans)) + for _, p := range plans { + actions = append(actions, SyncAction{p.att.Filename, p.action}) + } + return actions, nil +} + +// SyncAttachments creates, updates, or skips attachments so the page matches the +// local files, using a SHA-256 stored in each attachment's comment to detect +// changes. Returns one action per file. +func (c *ConfluenceClient) SyncAttachments(pageID string, attachments []LocalAttachment) ([]SyncAction, error) { + plans, err := c.planAttachments(pageID, attachments) + if err != nil { + return nil, err + } + var actions []SyncAction + for _, p := range plans { + switch p.action { + case "created": if err := c.uploadAttachment( c.baseURL+"/wiki/rest/api/content/"+pageID+"/child/attachment", - att.Filename, comment, att.Path, contentType); err != nil { + p.att.Filename, p.comment, p.att.Path, p.contentType); err != nil { return nil, err } - actions = append(actions, SyncAction{att.Filename, "created"}) - case cur.Metadata.Comment == comment: - actions = append(actions, SyncAction{att.Filename, "skipped"}) - default: + case "updated": if err := c.uploadAttachment( - c.baseURL+"/wiki/rest/api/content/"+pageID+"/child/attachment/"+cur.ID+"/data", - att.Filename, comment, att.Path, contentType); err != nil { + c.baseURL+"/wiki/rest/api/content/"+pageID+"/child/attachment/"+p.existingID+"/data", + p.att.Filename, p.comment, p.att.Path, p.contentType); err != nil { return nil, err } - actions = append(actions, SyncAction{att.Filename, "updated"}) } + actions = append(actions, SyncAction{p.att.Filename, p.action}) } return actions, nil } diff --git a/internal/client/client_test.go b/internal/client/client_test.go index b1a19a0..f2d1e71 100644 --- a/internal/client/client_test.go +++ b/internal/client/client_test.go @@ -379,6 +379,37 @@ func TestSyncAttachmentsUpdatesWhenChecksumDiffers(t *testing.T) { } } +func TestPlanAttachmentsClassifiesWithoutUploading(t *testing.T) { + path, sum := writeTempImage(t) + // same.png matches (skip), stale.png differs (update), new.png is absent (create). + list := `{"results":[` + + `{"id":"a1","title":"same.png","metadata":{"comment":"` + attachmentChecksumPrefix + sum + `"}},` + + `{"id":"a2","title":"stale.png","metadata":{"comment":"` + attachmentChecksumPrefix + `stale"}}` + + `]}` + c, s := newServer(t, resp{200, list}) + actions, err := c.PlanAttachments("1", []LocalAttachment{ + {Path: path, Filename: "same.png"}, + {Path: path, Filename: "stale.png"}, + {Path: path, Filename: "new.png"}, + }) + if err != nil { + t.Fatal(err) + } + want := []SyncAction{{"same.png", "skipped"}, {"stale.png", "updated"}, {"new.png", "created"}} + if len(actions) != len(want) { + t.Fatalf("actions = %v, want %v", actions, want) + } + for i := range want { + if actions[i] != want[i] { + t.Errorf("actions[%d] = %v, want %v", i, actions[i], want[i]) + } + } + // A plan reads only; it must never upload. + if !eqStrings(s.calls, []string{"GET"}) { + t.Errorf("calls = %v, want [GET] (no uploads)", s.calls) + } +} + // --- misc -------------------------------------------------------------------- func TestLoadDotenv(t *testing.T) { diff --git a/schema/json-output/v1.json b/schema/json-output/v1.json index 4c228f6..6a1cf19 100644 --- a/schema/json-output/v1.json +++ b/schema/json-output/v1.json @@ -184,12 +184,13 @@ "type": "object", "additionalProperties": false, "required": [ - "ok", "status", "file", "page_id", "title", "space", "url", + "ok", "status", "dry_run", "file", "page_id", "title", "space", "url", "version", "page_width", "attachments", "warnings", "broken", "error", "code" ], "properties": { "ok": { "type": "boolean" }, "status": { "enum": ["published", "skipped", "failed"] }, + "dry_run": { "type": "boolean" }, "file": { "type": "string" }, "page_id": { "$ref": "#/$defs/stringOrNull" }, "title": { "$ref": "#/$defs/stringOrNull" }, @@ -218,17 +219,19 @@ "type": "object", "additionalProperties": false, "required": [ - "ok", "status", "file", "page_id", "title", "space", "parent", "url", + "ok", "status", "dry_run", "file", "page_id", "title", "space", "parent", "parent_file", "url", "page_width", "persisted", "attachments", "warnings", "broken", "error", "code" ], "properties": { "ok": { "type": "boolean" }, "status": { "enum": ["created", "not_created", "failed"] }, + "dry_run": { "type": "boolean" }, "file": { "type": "string" }, "page_id": { "$ref": "#/$defs/stringOrNull" }, "title": { "$ref": "#/$defs/stringOrNull" }, "space": { "$ref": "#/$defs/stringOrNull" }, "parent": { "$ref": "#/$defs/stringOrNull" }, + "parent_file": { "$ref": "#/$defs/stringOrNull" }, "url": { "$ref": "#/$defs/stringOrNull" }, "page_width": { "$ref": "#/$defs/pageWidthOrNull" }, "persisted": { "type": "boolean" },