From b2a1343180c3702f0c4787555bd921f8e256ed8b Mon Sep 17 00:00:00 2001 From: Hiroaki KAWAI Date: Thu, 20 Aug 2026 19:01:43 +0900 Subject: [PATCH 1/2] Represent attachments as lazy materialization pointers --- README.md | 67 +++++++++++--- internal/confluence/attachments.go | 108 +++++++++++++++++++++- internal/confluence/attachments_test.go | 32 +++++++ internal/confluence/location.go | 12 ++- internal/confluence/location_test.go | 14 +++ internal/confluence/tree.go | 113 ++++++++++++++++++++++-- internal/fastimport/stream.go | 59 ++++++++++++- internal/fastimport/stream_test.go | 29 +++++- internal/remotehelper/helper.go | 29 +++++- internal/remotehelper/helper_test.go | 92 +++++++++++++++---- 10 files changed, 507 insertions(+), 48 deletions(-) diff --git a/README.md b/README.md index a953081..aeab175 100644 --- a/README.md +++ b/README.md @@ -2,8 +2,10 @@ `git-remote-confluence` is a Git remote helper that treats a Confluence page tree or space as a Git remote. It imports Confluence storage-format XML and -attachments into a Git repository and can push committed body updates for -existing pages back to Confluence. +attachment metadata pointers into a Git repository. The optional +`git-confluence` filter materializes page Markdown and attachment bytes in the +working tree. The helper can push committed body updates for existing pages +back to Confluence. The helper is intentionally narrow: Confluence remains the system that owns page identity, hierarchy, version numbers, and storage-format XML. Git becomes the @@ -23,17 +25,22 @@ content changed, checks the Confluence version and storage hash, and updates the existing Confluence page body. It refuses to overwrite a page when Confluence no longer matches the imported metadata. -Create, delete, move, title, and attachment changes are not pushed yet. +Git stores each attachment as a small text pointer containing its Confluence +identity and current version. Clone and fetch therefore do not download +attachment bytes. With the `git-confluence` filter configured, checkout +downloads the version named by the pointer and exposes it as a normal working +file. Create, delete, move, title, and attachment changes are not pushed yet. ## Working With Markdown Confluence storage-format XML is the synchronized content format. The companion -`git-confluence` clean/smudge filter makes that practical for day-to-day -editing: +`git-confluence` filter makes pages and attachments practical for day-to-day +use: - Git stores each page body as Confluence storage-format XML. - The working tree can show the same file as Markdown on checkout. - `git add` can convert the edited Markdown back to Confluence storage XML. +- Git stores attachment pointers and can materialize their bytes on checkout. That gives users Markdown editing while preserving the storage XML that Confluence needs for reliable import and push. @@ -41,10 +48,17 @@ Confluence needs for reliable import and push. The imported repository includes this `.gitattributes` entry: ```gitattributes -*.md filter=confluence-storage diff=markdown +*.md filter=confluence diff=markdown +**/attachments/** filter=confluence -text ``` -Configure the `git-confluence` filter before checking out imported files. +Without the filter, checkout leaves storage XML and attachment pointers intact. +This is the lightweight default for agents and offline use. Configure the +filter once with `git confluence install --global` to expose Markdown and +attachment bytes for normal human use. + +The `install` subcommand registers the unified clean/smudge driver in Git +configuration. It does not install the executable or download attachments. ## Repository Layout @@ -64,7 +78,7 @@ Child pages are placed under their parent's page-id directory: 123456789/123456790.yml ``` -Attachments are downloaded below the page ID that owns them: +Attachments use their normal filenames below the page ID that owns them: ```text 123456789/attachments/diagram.png @@ -73,6 +87,12 @@ Attachments are downloaded below the page ID that owns them: Path separators and control characters in attachment names are replaced with underscores so an attachment cannot escape its page's `attachments` directory. +Git stores a canonical text pointer at each attachment path. The pointer records +the source site, page ID, attachment ID, version, filename, size, media type, +and stable download path. It contains no token and no attachment bytes. The +configured filter replaces the pointer with that version's bytes in the working +tree and restores the same pointer on `git add`. Locally modified attachment +bytes are rejected because attachment push is not supported yet. The `.md` file is stored in Git as Confluence storage-format XML. With the `git-confluence` filter configured, it is checked out as Markdown and converted @@ -124,10 +144,11 @@ finds from: ## Clone -If the `git-confluence` filter is already configured globally, clone with Git's -explicit remote-helper syntax: +For human-oriented working trees, configure the unified page and attachment +filter once, then clone with Git's explicit remote-helper syntax: ```sh +git confluence install --global CONFLUENCE_PAT=... git clone \ 'confluence::https://confluence.example.com/pages/viewpage.action?pageId=123456789' ``` @@ -140,14 +161,32 @@ CONFLUENCE_PAT=... git clone --no-checkout \ 'confluence::https://confluence.example.com/pages/viewpage.action?pageId=123456789' \ pages cd pages -git config filter.confluence-storage.clean "/path/to/git-confluence/git-confluence clean" -git config filter.confluence-storage.smudge "/path/to/git-confluence/git-confluence smudge" -git config filter.confluence-storage.required true +git confluence install --local git checkout ``` +To keep attachment pointers during checkout while still converting pages, set +the skip-smudge variable for that command: + +```sh +GIT_CONFLUENCE_SKIP_SMUDGE=1 git checkout +``` + +Afterward, materialize all or selected attachments explicitly: + +```sh +git confluence pull +git confluence pull 123456789/attachments/diagram.png +``` + The remote URL may identify a page by `pageId`, a display page URL, or a -Confluence space. +Confluence space. A URL containing both `pageId` and `attachmentId` can still be +cloned directly when a standalone repository containing the attachment's full +binary version history is required. + +For Confluence Data Center, attachment history is read by following +`history.previousVersion` through historical content responses. The helper does +not require a `GET /content/{id}/version` listing endpoint. ## REST API Path diff --git a/internal/confluence/attachments.go b/internal/confluence/attachments.go index 7ae1298..06f7faa 100644 --- a/internal/confluence/attachments.go +++ b/internal/confluence/attachments.go @@ -5,12 +5,26 @@ import ( "net/url" "sort" "strconv" + + "github.com/hkwi/git-remote-confluence/internal/confluencetypes" ) type Attachment struct { - ID string `json:"id"` - Title string `json:"title"` - Links map[string]string `json:"_links"` + ID string `json:"id"` + Title string `json:"title"` + Version confluencetypes.Version `json:"version"` + History AttachmentHistory `json:"history"` + Extensions AttachmentExtensions `json:"extensions"` + Links map[string]string `json:"_links"` +} + +type AttachmentHistory struct { + PreviousVersion confluencetypes.Version `json:"previousVersion"` +} + +type AttachmentExtensions struct { + FileSize int64 `json:"fileSize"` + MediaType string `json:"mediaType"` } type attachmentListResponse struct { @@ -18,6 +32,22 @@ type attachmentListResponse struct { Links map[string]string `json:"_links"` } +func (c *Client) FetchAttachment(attachmentID string) (Attachment, error) { + if attachmentID == "" { + return Attachment{}, fmt.Errorf("Confluence attachment id is required") + } + values := url.Values{} + values.Set("expand", attachmentVersionExpand()) + var attachment Attachment + if err := c.getJSON(c.apiPath("content/"+url.PathEscape(attachmentID)), values, &attachment); err != nil { + return Attachment{}, err + } + if attachment.ID == "" { + return Attachment{}, fmt.Errorf("Confluence attachment response is missing id") + } + return attachment, nil +} + func (c *Client) FetchAttachments(pageID string) ([]Attachment, error) { var attachments []Attachment start := 0 @@ -27,6 +57,7 @@ func (c *Client) FetchAttachments(pageID string) ([]Attachment, error) { values := url.Values{} values.Set("start", strconv.Itoa(start)) values.Set("limit", strconv.Itoa(limit)) + values.Set("expand", attachmentVersionExpand()) var response attachmentListResponse path := c.apiPath("content/" + url.PathEscape(pageID) + "/child/attachment") @@ -50,6 +81,64 @@ func (c *Client) FetchAttachments(pageID string) ([]Attachment, error) { } func (c *Client) DownloadAttachment(attachment Attachment) ([]byte, error) { + return c.DownloadAttachmentVersion(attachment, 0) +} + +func (c *Client) FetchAttachmentVersions(attachment Attachment) ([]confluencetypes.Version, error) { + if attachment.ID == "" { + return nil, fmt.Errorf("Confluence attachment response is missing id") + } + if attachment.Version.Number <= 0 { + current, err := c.FetchAttachment(attachment.ID) + if err != nil { + return nil, err + } + attachment = current + } + + versions := []confluencetypes.Version{attachment.Version} + seen := map[int]bool{attachment.Version.Number: true} + previous := attachment.History.PreviousVersion + for previous.Number > 0 { + if seen[previous.Number] { + return nil, fmt.Errorf("Confluence attachment %s version history contains a cycle at version %d", attachment.ID, previous.Number) + } + historical, err := c.fetchHistoricalAttachment(attachment.ID, previous.Number) + if err != nil { + return nil, fmt.Errorf("fetch historical attachment %s version %d: %w", attachment.ID, previous.Number, err) + } + if historical.Version.Number != previous.Number { + return nil, fmt.Errorf("Confluence attachment %s requested version %d but returned version %d", attachment.ID, previous.Number, historical.Version.Number) + } + versions = append(versions, historical.Version) + seen[historical.Version.Number] = true + previous = historical.History.PreviousVersion + } + + sort.Slice(versions, func(i, j int) bool { return versions[i].Number < versions[j].Number }) + return versions, nil +} + +func (c *Client) fetchHistoricalAttachment(attachmentID string, version int) (Attachment, error) { + values := url.Values{} + values.Set("status", "historical") + values.Set("version", strconv.Itoa(version)) + values.Set("expand", attachmentVersionExpand()) + var attachment Attachment + if err := c.getJSON(c.apiPath("content/"+url.PathEscape(attachmentID)), values, &attachment); err != nil { + return Attachment{}, err + } + if attachment.ID == "" { + return Attachment{}, fmt.Errorf("Confluence historical attachment response is missing id") + } + return attachment, nil +} + +func attachmentVersionExpand() string { + return "version,version.by,history.previousVersion,history.previousVersion.by" +} + +func (c *Client) DownloadAttachmentVersion(attachment Attachment, version int) ([]byte, error) { if attachment.ID == "" { return nil, fmt.Errorf("Confluence attachment response is missing id") } @@ -57,5 +146,16 @@ func (c *Client) DownloadAttachment(attachment Attachment) ([]byte, error) { if download == "" { return nil, fmt.Errorf("Confluence attachment %s is missing download link", attachment.ID) } - return c.getBytes(resolveLink(c.BaseURL, download)) + downloadURL := resolveLink(c.BaseURL, download) + if version > 0 { + parsed, err := url.Parse(downloadURL) + if err != nil { + return nil, fmt.Errorf("invalid Confluence attachment download link: %w", err) + } + query := parsed.Query() + query.Set("version", strconv.Itoa(version)) + parsed.RawQuery = query.Encode() + downloadURL = parsed.String() + } + return c.getBytes(downloadURL) } diff --git a/internal/confluence/attachments_test.go b/internal/confluence/attachments_test.go index 792120e..8917d14 100644 --- a/internal/confluence/attachments_test.go +++ b/internal/confluence/attachments_test.go @@ -3,9 +3,41 @@ package confluence import ( "net/http" "net/http/httptest" + "strings" "testing" ) +func TestAttachmentPointerUsesStableMetadata(t *testing.T) { + pointer, err := attachmentPointer("https://cf.example.test/wiki", "123", Attachment{ + ID: "456", + Title: "diagram one.png", + Version: Version{Number: 7}, + Extensions: AttachmentExtensions{FileSize: 42, MediaType: "image/png"}, + Links: map[string]string{ + "download": "/wiki/download/attachments/123/diagram%20one.png?version=7&modificationDate=99&api=v2", + }, + }) + if err != nil { + t.Fatal(err) + } + text := string(pointer) + for _, want := range []string{ + "attachment-id 456\n", + "attachment-version 7\n", + "filename diagram one.png\n", + "size 42\n", + "media-type image/png\n", + "download-path /wiki/download/attachments/123/diagram%20one.png?api=v2\n", + } { + if !strings.Contains(text, want) { + t.Fatalf("pointer missing %q:\n%s", want, text) + } + } + if strings.Contains(text, "modificationDate") { + t.Fatalf("pointer retained volatile download query:\n%s", text) + } +} + func TestSafeAttachmentName(t *testing.T) { tests := []struct { title string diff --git a/internal/confluence/location.go b/internal/confluence/location.go index e187c26..ff97eea 100644 --- a/internal/confluence/location.go +++ b/internal/confluence/location.go @@ -32,6 +32,9 @@ func ParseLocation(rawURL string) (Location, error) { } query := parsed.Query() + if attachmentID := firstQuery(query, "attachmentId", "attachmentid"); attachmentID != "" && isDigits(attachmentID) { + return Location{BaseURL: baseURL(parsed), RootType: "attachment", RootValue: attachmentID, OriginalURL: original}, nil + } if pageID := firstQuery(query, "pageId", "pageid", "id"); pageID != "" && isDigits(pageID) { return Location{BaseURL: baseURL(parsed), RootType: "page", RootValue: pageID, OriginalURL: original}, nil } @@ -59,7 +62,14 @@ func ParseLocation(rawURL string) (Location, error) { return Location{BaseURL: baseURL(parsed), RootType: "space", RootValue: spaceKey, OriginalURL: original}, nil } - return Location{}, fmt.Errorf("remote URL must identify a pageId or a Confluence space") + return Location{}, fmt.Errorf("remote URL must identify a pageId, attachmentId, or a Confluence space") +} + +func AttachmentRemoteURL(baseURL, pageID, attachmentID string) string { + values := url.Values{} + values.Set("pageId", pageID) + values.Set("attachmentId", attachmentID) + return "confluence::" + strings.TrimRight(baseURL, "/") + "/pages/viewpage.action?" + values.Encode() } func ResolvePAT(remoteName string) string { diff --git a/internal/confluence/location_test.go b/internal/confluence/location_test.go index b93260a..09dc60c 100644 --- a/internal/confluence/location_test.go +++ b/internal/confluence/location_test.go @@ -31,6 +31,20 @@ func TestParseLocationSpaceURL(t *testing.T) { } } +func TestParseLocationAttachmentURL(t *testing.T) { + remoteURL := AttachmentRemoteURL("https://cf.example.test/wiki", "123", "456") + location, err := ParseLocation(remoteURL) + if err != nil { + t.Fatal(err) + } + if location.BaseURL != "https://cf.example.test/wiki" { + t.Fatalf("base URL = %q", location.BaseURL) + } + if location.RootType != "attachment" || location.RootValue != "456" { + t.Fatalf("root = %s %s", location.RootType, location.RootValue) + } +} + func TestParseLocationDisplayPageURL(t *testing.T) { location, err := ParseLocation("confluence:https://cf.example.test/display/ABC/%E5%90%BE%E8%BC%A9%E3%81%AF%E7%8C%AB%E3%81%A7%E3%81%82%E3%82%8B") if err != nil { diff --git a/internal/confluence/tree.go b/internal/confluence/tree.go index 6ccacf7..38c2286 100644 --- a/internal/confluence/tree.go +++ b/internal/confluence/tree.go @@ -2,6 +2,7 @@ package confluence import ( "fmt" + "net/url" "sort" "strings" "unicode" @@ -206,10 +207,6 @@ func fetchAttachments(client *Client, page fastimport.PageRecord, progress Progr result := make([]fastimport.AttachmentRecord, 0, len(attachments)) usedPaths := map[string]bool{} for _, attachment := range attachments { - data, err := client.DownloadAttachment(attachment) - if err != nil { - return nil, fmt.Errorf("download attachment %s (%q): %w", attachment.ID, attachment.Title, err) - } name := safeAttachmentName(attachment.Title, attachment.ID) path := joinPath(page.PathDir, page.PageID, "attachments", name) if usedPaths[path] { @@ -217,15 +214,117 @@ func fetchAttachments(client *Client, page fastimport.PageRecord, progress Progr path = joinPath(page.PathDir, page.PageID, "attachments", name) } usedPaths[path] = true + pointer, err := attachmentPointer(client.BaseURL, page.PageID, attachment) + if err != nil { + return nil, fmt.Errorf("build pointer for attachment %s (%q): %w", attachment.ID, attachment.Title, err) + } result = append(result, fastimport.AttachmentRecord{ - Path: path, - Data: data, + ID: attachment.ID, + Title: name, + Path: path, + Pointer: pointer, }) - report(progress, "downloaded attachment %s %s (%d bytes)", attachment.ID, attachment.Title, len(data)) + report(progress, "recorded attachment pointer %s %s version %d", attachment.ID, attachment.Title, attachment.Version.Number) } return result, nil } +const attachmentPointerVersion = "https://github.com/hkwi/git-remote-confluence/spec/attachment/v1" + +func attachmentPointer(baseURL, pageID string, attachment Attachment) ([]byte, error) { + if attachment.ID == "" { + return nil, fmt.Errorf("attachment id is required") + } + if attachment.Version.Number <= 0 { + return nil, fmt.Errorf("attachment version is required") + } + downloadPath, err := stableDownloadPath(baseURL, attachment.Links["download"]) + if err != nil { + return nil, err + } + filename := safeAttachmentName(attachment.Title, attachment.ID) + var pointer strings.Builder + fmt.Fprintf(&pointer, "version %s\n", attachmentPointerVersion) + fmt.Fprintf(&pointer, "source %s\n", strings.TrimRight(baseURL, "/")) + fmt.Fprintf(&pointer, "page-id %s\n", pageID) + fmt.Fprintf(&pointer, "attachment-id %s\n", attachment.ID) + fmt.Fprintf(&pointer, "attachment-version %d\n", attachment.Version.Number) + fmt.Fprintf(&pointer, "filename %s\n", filename) + fmt.Fprintf(&pointer, "size %d\n", attachment.Extensions.FileSize) + if attachment.Extensions.MediaType != "" { + fmt.Fprintf(&pointer, "media-type %s\n", attachment.Extensions.MediaType) + } + fmt.Fprintf(&pointer, "download-path %s\n", downloadPath) + return []byte(pointer.String()), nil +} + +func stableDownloadPath(baseURL, download string) (string, error) { + if download == "" { + return "", fmt.Errorf("download link is required") + } + base, err := url.Parse(strings.TrimRight(baseURL, "/") + "/") + if err != nil { + return "", fmt.Errorf("invalid Confluence base URL: %w", err) + } + link, err := url.Parse(download) + if err != nil { + return "", fmt.Errorf("invalid download link: %w", err) + } + resolved := base.ResolveReference(link) + if resolved.Scheme != base.Scheme || resolved.Host != base.Host { + return "", fmt.Errorf("download link points outside Confluence origin") + } + query := resolved.Query() + stableQuery := url.Values{} + if api := query.Get("api"); api != "" { + stableQuery.Set("api", api) + } + resolved.RawQuery = stableQuery.Encode() + resolved.Fragment = "" + path := resolved.EscapedPath() + if path == "" || !strings.HasPrefix(path, "/") { + return "", fmt.Errorf("download link has no absolute path") + } + if resolved.RawQuery != "" { + path += "?" + resolved.RawQuery + } + return path, nil +} + +func FetchAttachmentRepository(client *Client, attachmentID string, progress ProgressFunc) (fastimport.AttachmentRecord, error) { + report(progress, "fetching attachment %s", attachmentID) + attachment, err := client.FetchAttachment(attachmentID) + if err != nil { + return fastimport.AttachmentRecord{}, fmt.Errorf("fetch attachment %s: %w", attachmentID, err) + } + return fetchAttachmentRecord(client, attachment, "", progress) +} + +func fetchAttachmentRecord(client *Client, attachment Attachment, path string, progress ProgressFunc) (fastimport.AttachmentRecord, error) { + versions, err := client.FetchAttachmentVersions(attachment) + if err != nil { + return fastimport.AttachmentRecord{}, fmt.Errorf("fetch versions for attachment %s (%q): %w", attachment.ID, attachment.Title, err) + } + if len(versions) == 0 { + return fastimport.AttachmentRecord{}, fmt.Errorf("Confluence attachment %s (%q) has no versions", attachment.ID, attachment.Title) + } + + record := fastimport.AttachmentRecord{ + ID: attachment.ID, + Title: safeAttachmentName(attachment.Title, attachment.ID), + Path: path, + } + for _, version := range versions { + data, err := client.DownloadAttachmentVersion(attachment, version.Number) + if err != nil { + return fastimport.AttachmentRecord{}, fmt.Errorf("download attachment %s (%q) version %d: %w", attachment.ID, attachment.Title, version.Number, err) + } + record.Versions = append(record.Versions, fastimport.AttachmentVersionRecord{Version: version, Data: data}) + report(progress, "downloaded attachment %s %s version %d (%d bytes)", attachment.ID, attachment.Title, version.Number, len(data)) + } + return record, nil +} + func safeAttachmentName(title, id string) string { name := strings.Map(func(r rune) rune { if r == '/' || r == '\\' || unicode.IsControl(r) { diff --git a/internal/fastimport/stream.go b/internal/fastimport/stream.go index c2e6080..4571ad0 100644 --- a/internal/fastimport/stream.go +++ b/internal/fastimport/stream.go @@ -13,7 +13,7 @@ import ( const DefaultBranch = "refs/heads/main" const AttributesPath = ".gitattributes" -const AttributesContent = "*.md filter=confluence-storage diff=markdown\n" +const AttributesContent = "*.md filter=confluence diff=markdown\n**/attachments/** filter=confluence -text\n" type Location struct { RootType string @@ -35,8 +35,16 @@ type PageRecord struct { } type AttachmentRecord struct { - Path string - Data []byte + ID string + Title string + Path string + Pointer []byte + Versions []AttachmentVersionRecord +} + +type AttachmentVersionRecord struct { + Version confluencetypes.Version + Data []byte } func (p PageRecord) ContentPath() string { @@ -69,7 +77,7 @@ func BuildStreamWithProgress(branch string, location Location, pages []PageRecor appendFile(&out, page.ContentPath(), []byte(page.StorageXML)) appendFile(&out, page.MetadataPath(), []byte(PageMetadataYAML(location, page))) for _, attachment := range page.Attachments { - appendFile(&out, attachment.Path, attachment.Data) + appendFile(&out, attachment.Path, attachment.Pointer) } } @@ -81,6 +89,38 @@ func BuildStreamWithProgress(branch string, location Location, pages []PageRecor return out.Bytes() } +func BuildAttachmentStream(branch string, attachment AttachmentRecord) []byte { + var out bytes.Buffer + appendAttachmentCommits(&out, branch, attachment, 1) + out.WriteString("done\n") + return out.Bytes() +} + +func appendAttachmentCommits(out *bytes.Buffer, ref string, attachment AttachmentRecord, nextMark int) (int, int) { + previousMark := 0 + for _, record := range attachment.Versions { + mark := nextMark + nextMark++ + fmt.Fprintf(out, "commit %s\n", ref) + fmt.Fprintf(out, "mark :%d\n", mark) + fmt.Fprintf(out, "committer Confluence %d +0000\n", versionTimestamp(record.Version)) + appendData(out, []byte(attachmentCommitMessage(attachment, record.Version))) + if previousMark == 0 { + out.WriteString("deleteall\n") + } else { + fmt.Fprintf(out, "from :%d\n", previousMark) + } + appendFile(out, attachment.Title, record.Data) + out.WriteByte('\n') + previousMark = mark + } + return previousMark, nextMark +} + +func attachmentCommitMessage(attachment AttachmentRecord, version confluencetypes.Version) string { + return fmt.Sprintf("Import Confluence attachment %s version %d\n", attachment.ID, version.Number) +} + func SelectBranch(refs []string) string { for _, ref := range refs { if ref != "HEAD" && strings.HasPrefix(ref, "refs/heads/") { @@ -170,6 +210,17 @@ func commitTimestamp(pages []PageRecord) int64 { return max } +func versionTimestamp(version confluencetypes.Version) int64 { + if version.When == "" { + return 0 + } + parsed, err := time.Parse(time.RFC3339Nano, version.When) + if err != nil { + return 0 + } + return parsed.Unix() +} + func joinPath(parts ...string) string { var kept []string for _, part := range parts { diff --git a/internal/fastimport/stream_test.go b/internal/fastimport/stream_test.go index df4fa1f..7823b9b 100644 --- a/internal/fastimport/stream_test.go +++ b/internal/fastimport/stream_test.go @@ -17,7 +17,10 @@ func TestBuildStreamContainsPageFiles(t *testing.T) { Links: map[string]string{"webui": "https://cf.example.test/pages/viewpage.action?pageId=1"}, StorageXML: "

吾輩は猫である。名前はまだ無い。

", Attachments: []AttachmentRecord{{ - Path: "1/attachments/挿絵.png", Data: []byte("PNG DATA\x00"), + ID: "10", + Title: "挿絵.png", + Path: "1/attachments/挿絵.png", + Pointer: []byte("version pointer/v1\nattachment-id 10\nattachment-version 2\n"), }}, } @@ -28,7 +31,8 @@ func TestBuildStreamContainsPageFiles(t *testing.T) { []byte("M 100644 inline 1.md\n"), []byte("M 100644 inline 1.yml\n"), []byte("M 100644 inline 1/attachments/挿絵.png\n"), - []byte("data 9\nPNG DATA\x00\n"), + []byte("attachment-id 10\n"), + []byte("attachment-version 2\n"), []byte(`storage_xml: "1.md"`), []byte("number: 3\n"), } { @@ -37,3 +41,24 @@ func TestBuildStreamContainsPageFiles(t *testing.T) { } } } + +func TestBuildAttachmentStreamContainsVersionHistory(t *testing.T) { + attachment := AttachmentRecord{ + ID: "10", Title: "diagram.png", + Versions: []AttachmentVersionRecord{ + {Version: confluencetypes.Version{Number: 1, When: "2025-01-01T00:00:00Z"}, Data: []byte("one")}, + {Version: confluencetypes.Version{Number: 2, When: "2025-01-02T00:00:00Z"}, Data: []byte("two")}, + }, + } + stream := BuildAttachmentStream(DefaultBranch, attachment) + for _, expected := range [][]byte{ + []byte("Import Confluence attachment 10 version 1\n"), + []byte("Import Confluence attachment 10 version 2\n"), + []byte("from :1\n"), + []byte("M 100644 inline diagram.png\n"), + } { + if !bytes.Contains(stream, expected) { + t.Fatalf("stream did not contain %q\n%s", expected, stream) + } + } +} diff --git a/internal/remotehelper/helper.go b/internal/remotehelper/helper.go index d694a86..93ce269 100644 --- a/internal/remotehelper/helper.go +++ b/internal/remotehelper/helper.go @@ -55,7 +55,7 @@ func (h *helper) serve() error { switch { case line == "capabilities": - if _, err := io.WriteString(h.out, "import\npush\noption\nrefspec refs/heads/*:refs/heads/*\n\n"); err != nil { + if _, err := io.WriteString(h.out, h.capabilities()); err != nil { return err } case line == "list": @@ -88,6 +88,14 @@ func (h *helper) serve() error { } } +func (h *helper) capabilities() string { + location, err := confluence.ParseLocation(h.remoteURL) + if err == nil && location.RootType == "attachment" { + return "import\noption\nrefspec refs/heads/*:refs/heads/*\n\n" + } + return "import\npush\noption\nrefspec refs/heads/*:refs/heads/*\n\n" +} + func (h *helper) handleOption(line string) error { parts := strings.SplitN(line, " ", 3) if len(parts) != 3 { @@ -144,6 +152,9 @@ func (h *helper) runImport(refs []string) error { if err != nil { return err } + if location.RootType == "attachment" { + return h.runAttachmentImport(refs, location, client) + } location, err = confluence.ResolveLocation(client, location, h.reportProgress) if err != nil { return err @@ -173,6 +184,22 @@ func (h *helper) runImport(refs []string) error { return err } +func (h *helper) runAttachmentImport(refs []string, location confluence.Location, client *confluence.Client) error { + h.reportProgress("root attachment %s at %s", location.RootValue, location.BaseURL) + attachment, err := confluence.FetchAttachmentRepository(client, location.RootValue, h.reportProgress) + if err != nil { + return err + } + stream := fastimport.BuildAttachmentStream(fastimport.SelectBranch(refs), attachment) + h.reportProgress("importing attachment %s with %d versions", attachment.ID, len(attachment.Versions)) + h.reportProgress("writing %d bytes to git fast-import", len(stream)) + _, err = h.out.Write(stream) + if err == nil { + h.reportProgress("done") + } + return err +} + func (h *helper) confluenceClient() (confluence.Location, *confluence.Client, error) { location, err := confluence.ParseLocation(h.remoteURL) if err != nil { diff --git a/internal/remotehelper/helper_test.go b/internal/remotehelper/helper_test.go index 34cf291..e7154e4 100644 --- a/internal/remotehelper/helper_test.go +++ b/internal/remotehelper/helper_test.go @@ -9,11 +9,13 @@ import ( "os/exec" "path/filepath" "strings" + "sync/atomic" "testing" ) func TestGitCloneFromMockConfluence(t *testing.T) { - server := httptest.NewServer(mockConfluenceHandler(t)) + var attachmentDownloads atomic.Int32 + server := httptest.NewServer(mockConfluenceHandler(t, &attachmentDownloads)) defer server.Close() tmp := t.TempDir() @@ -23,18 +25,18 @@ func TestGitCloneFromMockConfluence(t *testing.T) { if err := os.Mkdir(binDir, 0o755); err != nil { t.Fatal(err) } - helperPath := filepath.Join(binDir, "git-remote-confluence-test") + helperPath := filepath.Join(binDir, "git-remote-confluence") build := exec.Command("go", "build", "-o", helperPath, ".") build.Dir = root build.Env = append(os.Environ(), "GOCACHE="+filepath.Join(tmp, "gocache")) if output, err := build.CombinedOutput(); err != nil { t.Fatalf("go build failed: %v\n%s", err, output) } - - cmd := exec.Command("git", "clone", "--progress", "--verbose", "confluence-test::"+server.URL+"/pages/viewpage.action?pageId=1", destination) + cmd := exec.Command("git", "clone", "--progress", "--verbose", "confluence::"+server.URL+"/pages/viewpage.action?pageId=1", destination) cmd.Dir = tmp cmd.Env = append(os.Environ(), "PATH="+binDir+string(os.PathListSeparator)+os.Getenv("PATH"), + "GIT_EXEC_PATH="+binDir, "CONFLUENCE_PAT=secret-token", "NO_PROXY=127.0.0.1,localhost", "no_proxy=127.0.0.1,localhost", @@ -56,8 +58,19 @@ func TestGitCloneFromMockConfluence(t *testing.T) { } attachment := readCloneFile(t, destination, "1", "attachments", "挿絵.png") - if string(attachment) != "PNG DATA\x00" { - t.Fatalf("attachment = %q", attachment) + for _, want := range []string{ + "attachment-id 10\n", + "attachment-version 2\n", + "size 9\n", + "media-type image/png\n", + "download-path /download/attachments/1/%E6%8C%BF%E7%B5%B5.png\n", + } { + if !strings.Contains(string(attachment), want) { + t.Fatalf("attachment pointer missing %q:\n%s", want, attachment) + } + } + if attachmentDownloads.Load() != 0 { + t.Fatalf("page clone downloaded attachment %d times", attachmentDownloads.Load()) } metadata := readCloneFile(t, destination, "1.yml") @@ -66,7 +79,7 @@ func TestGitCloneFromMockConfluence(t *testing.T) { } attributes := readCloneFile(t, destination, ".gitattributes") - if string(attributes) != "*.md filter=confluence-storage diff=markdown\n" { + if string(attributes) != "*.md filter=confluence diff=markdown\n**/attachments/** filter=confluence -text\n" { t.Fatalf("unexpected attributes:\n%s", attributes) } @@ -75,7 +88,7 @@ func TestGitCloneFromMockConfluence(t *testing.T) { if err != nil { t.Fatalf("git check-attr failed: %v\n%s", err, output) } - if !strings.Contains(string(output), "filter: confluence-storage") { + if !strings.Contains(string(output), "filter: confluence") { t.Fatalf("filter attribute missing:\n%s", output) } } @@ -121,7 +134,7 @@ func listWorktree(t *testing.T, root string) string { } func TestHelperProgressOutput(t *testing.T) { - server := httptest.NewServer(mockConfluenceHandler(t)) + server := httptest.NewServer(mockConfluenceHandler(t, nil)) defer server.Close() t.Setenv("CONFLUENCE_PAT", "secret-token") t.Setenv("NO_PROXY", "127.0.0.1,localhost") @@ -160,8 +173,27 @@ func TestHelperProgressOutput(t *testing.T) { } } +func TestAttachmentRemoteIsReadOnly(t *testing.T) { + var stdout bytes.Buffer + err := Main( + []string{"origin", "confluence::https://cf.example.test/pages/viewpage.action?attachmentId=10&pageId=1"}, + strings.NewReader("capabilities\n\n"), + &stdout, + &bytes.Buffer{}, + ) + if err != nil { + t.Fatal(err) + } + if strings.Contains(stdout.String(), "push\n") { + t.Fatalf("attachment remote advertised push:\n%s", stdout.String()) + } + if !strings.Contains(stdout.String(), "import\n") { + t.Fatalf("attachment remote did not advertise import:\n%s", stdout.String()) + } +} + func TestHelperResolvesDisplayPageURL(t *testing.T) { - server := httptest.NewServer(mockConfluenceHandler(t)) + server := httptest.NewServer(mockConfluenceHandler(t, nil)) defer server.Close() t.Setenv("CONFLUENCE_PAT", "secret-token") t.Setenv("NO_PROXY", "127.0.0.1,localhost") @@ -206,7 +238,7 @@ func TestHelperResolvesDisplayPageURL(t *testing.T) { } } -func mockConfluenceHandler(t *testing.T) http.Handler { +func mockConfluenceHandler(t *testing.T, attachmentDownloads *atomic.Int32) http.Handler { t.Helper() pages := map[string]map[string]any{ "1": { @@ -254,12 +286,42 @@ func mockConfluenceHandler(t *testing.T) http.Handler { case "/rest/api/content/1/child/attachment": writeJSON(t, w, map[string]any{"results": []any{map[string]any{ "id": "10", "title": "挿絵.png", - "extensions": map[string]any{"mediaType": "image/png"}, - "version": map[string]any{"number": 2}, - "_links": map[string]any{"download": "/download/attachments/1/%E6%8C%BF%E7%B5%B5.png"}, + "extensions": map[string]any{"mediaType": "image/png", "fileSize": 9}, + "version": map[string]any{ + "number": 2, "when": "2025-01-02T03:04:05.000Z", + }, + "history": map[string]any{ + "previousVersion": map[string]any{"number": 1, "when": "2025-01-01T03:04:05.000Z"}, + }, + "_links": map[string]any{"download": "/download/attachments/1/%E6%8C%BF%E7%B5%B5.png"}, }}, "size": 1, "limit": 100}) + case "/rest/api/content/10": + if r.URL.Query().Get("status") == "historical" && r.URL.Query().Get("version") == "1" { + writeJSON(t, w, map[string]any{ + "id": "10", "type": "attachment", "status": "historical", "title": "挿絵.png", + "version": map[string]any{"number": 1, "when": "2025-01-01T03:04:05.000Z"}, + "history": map[string]any{}, + "_links": map[string]any{"download": "/download/attachments/1/%E6%8C%BF%E7%B5%B5.png"}, + }) + return + } + writeJSON(t, w, map[string]any{ + "id": "10", "type": "attachment", "title": "挿絵.png", + "version": map[string]any{"number": 2, "when": "2025-01-02T03:04:05.000Z"}, + "history": map[string]any{ + "previousVersion": map[string]any{"number": 1, "when": "2025-01-01T03:04:05.000Z"}, + }, + "_links": map[string]any{"download": "/download/attachments/1/%E6%8C%BF%E7%B5%B5.png"}, + }) case "/download/attachments/1/挿絵.png": - _, _ = w.Write([]byte("PNG DATA\x00")) + if attachmentDownloads != nil { + attachmentDownloads.Add(1) + } + if r.URL.Query().Get("version") == "1" { + _, _ = w.Write([]byte("OLD PNG DATA\x00")) + } else { + _, _ = w.Write([]byte("PNG DATA\x00")) + } case "/rest/api/content/2": writeJSON(t, w, pages["2"]) case "/rest/api/content/2/child/page": From fd61d1e08e5fa97995df2714e0263295f978d303 Mon Sep 17 00:00:00 2001 From: Hiroaki KAWAI Date: Fri, 21 Aug 2026 11:53:44 +0900 Subject: [PATCH 2/2] Use YAML attachment pointers and logfmt logs --- README.md | 18 +++++++++-- go.mod | 2 ++ go.sum | 2 ++ internal/confluence/attachments_test.go | 13 ++++---- internal/confluence/tree.go | 42 +++++++++++++++++-------- internal/fastimport/stream.go | 12 ++++--- internal/fastimport/stream_test.go | 20 ++++++++++-- internal/logging/logging.go | 20 ++++++++++++ internal/logging/logging_test.go | 15 +++++++++ internal/remotehelper/helper.go | 10 +++--- internal/remotehelper/helper_test.go | 25 ++++++++------- main.go | 3 +- 12 files changed, 136 insertions(+), 46 deletions(-) create mode 100644 go.sum create mode 100644 internal/logging/logging.go create mode 100644 internal/logging/logging_test.go diff --git a/README.md b/README.md index aeab175..4b2d4af 100644 --- a/README.md +++ b/README.md @@ -25,7 +25,7 @@ content changed, checks the Confluence version and storage hash, and updates the existing Confluence page body. It refuses to overwrite a page when Confluence no longer matches the imported metadata. -Git stores each attachment as a small text pointer containing its Confluence +Git stores each attachment as a small YAML pointer containing its Confluence identity and current version. Clone and fetch therefore do not download attachment bytes. With the `git-confluence` filter configured, checkout downloads the version named by the pointer and exposes it as a normal working @@ -87,13 +87,27 @@ Attachments use their normal filenames below the page ID that owns them: Path separators and control characters in attachment names are replaced with underscores so an attachment cannot escape its page's `attachments` directory. -Git stores a canonical text pointer at each attachment path. The pointer records +Git stores a canonical YAML pointer at each attachment path. The pointer records the source site, page ID, attachment ID, version, filename, size, media type, and stable download path. It contains no token and no attachment bytes. The configured filter replaces the pointer with that version's bytes in the working tree and restores the same pointer on `git add`. Locally modified attachment bytes are rejected because attachment push is not supported yet. +Attachment pointers use the `attachment/v1` YAML schema. For example: + +```yaml +version: https://github.com/hkwi/git-remote-confluence/spec/attachment/v1 +source: https://confluence.example.com +page_id: "123456789" +attachment_id: "987654321" +attachment_version: 3 +filename: diagram.png +size: 42000 +media_type: image/png +download_path: /download/attachments/123456789/diagram.png +``` + The `.md` file is stored in Git as Confluence storage-format XML. With the `git-confluence` filter configured, it is checked out as Markdown and converted back to storage XML on `git add`. diff --git a/go.mod b/go.mod index cccafff..e429195 100644 --- a/go.mod +++ b/go.mod @@ -1,3 +1,5 @@ module github.com/hkwi/git-remote-confluence go 1.26 + +require go.yaml.in/yaml/v3 v3.0.5 diff --git a/go.sum b/go.sum new file mode 100644 index 0000000..8675e7f --- /dev/null +++ b/go.sum @@ -0,0 +1,2 @@ +go.yaml.in/yaml/v3 v3.0.5 h1:N6y/pJk8buWs9NY5ERU2HSMfm+IuD/OtfdAnq6kESPw= +go.yaml.in/yaml/v3 v3.0.5/go.mod h1:HVTZu1O7/Vkt2N+BFy8Zza+lnLsABggaTM2ZpNIGuKg= diff --git a/internal/confluence/attachments_test.go b/internal/confluence/attachments_test.go index 8917d14..8e43c14 100644 --- a/internal/confluence/attachments_test.go +++ b/internal/confluence/attachments_test.go @@ -22,12 +22,13 @@ func TestAttachmentPointerUsesStableMetadata(t *testing.T) { } text := string(pointer) for _, want := range []string{ - "attachment-id 456\n", - "attachment-version 7\n", - "filename diagram one.png\n", - "size 42\n", - "media-type image/png\n", - "download-path /wiki/download/attachments/123/diagram%20one.png?api=v2\n", + "version: https://github.com/hkwi/git-remote-confluence/spec/attachment/v1\n", + "attachment_id: \"456\"\n", + "attachment_version: 7\n", + "filename: diagram one.png\n", + "size: 42\n", + "media_type: image/png\n", + "download_path: /wiki/download/attachments/123/diagram%20one.png?api=v2\n", } { if !strings.Contains(text, want) { t.Fatalf("pointer missing %q:\n%s", want, text) diff --git a/internal/confluence/tree.go b/internal/confluence/tree.go index 38c2286..1d9789e 100644 --- a/internal/confluence/tree.go +++ b/internal/confluence/tree.go @@ -7,6 +7,8 @@ import ( "strings" "unicode" + "go.yaml.in/yaml/v3" + "github.com/hkwi/git-remote-confluence/internal/fastimport" ) @@ -231,6 +233,18 @@ func fetchAttachments(client *Client, page fastimport.PageRecord, progress Progr const attachmentPointerVersion = "https://github.com/hkwi/git-remote-confluence/spec/attachment/v1" +type attachmentPointerYAML struct { + Version string `yaml:"version"` + SourceURL string `yaml:"source"` + PageID string `yaml:"page_id"` + AttachmentID string `yaml:"attachment_id"` + AttachmentVersion int `yaml:"attachment_version"` + Filename string `yaml:"filename"` + Size int64 `yaml:"size"` + MediaType string `yaml:"media_type,omitempty"` + DownloadPath string `yaml:"download_path"` +} + func attachmentPointer(baseURL, pageID string, attachment Attachment) ([]byte, error) { if attachment.ID == "" { return nil, fmt.Errorf("attachment id is required") @@ -243,19 +257,21 @@ func attachmentPointer(baseURL, pageID string, attachment Attachment) ([]byte, e return nil, err } filename := safeAttachmentName(attachment.Title, attachment.ID) - var pointer strings.Builder - fmt.Fprintf(&pointer, "version %s\n", attachmentPointerVersion) - fmt.Fprintf(&pointer, "source %s\n", strings.TrimRight(baseURL, "/")) - fmt.Fprintf(&pointer, "page-id %s\n", pageID) - fmt.Fprintf(&pointer, "attachment-id %s\n", attachment.ID) - fmt.Fprintf(&pointer, "attachment-version %d\n", attachment.Version.Number) - fmt.Fprintf(&pointer, "filename %s\n", filename) - fmt.Fprintf(&pointer, "size %d\n", attachment.Extensions.FileSize) - if attachment.Extensions.MediaType != "" { - fmt.Fprintf(&pointer, "media-type %s\n", attachment.Extensions.MediaType) - } - fmt.Fprintf(&pointer, "download-path %s\n", downloadPath) - return []byte(pointer.String()), nil + pointer, err := yaml.Marshal(attachmentPointerYAML{ + Version: attachmentPointerVersion, + SourceURL: strings.TrimRight(baseURL, "/"), + PageID: pageID, + AttachmentID: attachment.ID, + AttachmentVersion: attachment.Version.Number, + Filename: filename, + Size: attachment.Extensions.FileSize, + MediaType: attachment.Extensions.MediaType, + DownloadPath: downloadPath, + }) + if err != nil { + return nil, fmt.Errorf("marshal attachment pointer YAML: %w", err) + } + return pointer, nil } func stableDownloadPath(baseURL, download string) (string, error) { diff --git a/internal/fastimport/stream.go b/internal/fastimport/stream.go index 4571ad0..f7f67be 100644 --- a/internal/fastimport/stream.go +++ b/internal/fastimport/stream.go @@ -9,6 +9,7 @@ import ( "time" "github.com/hkwi/git-remote-confluence/internal/confluencetypes" + "github.com/hkwi/git-remote-confluence/internal/logging" ) const DefaultBranch = "refs/heads/main" @@ -62,9 +63,9 @@ func BuildStream(branch string, location Location, pages []PageRecord) []byte { func BuildStreamWithProgress(branch string, location Location, pages []PageRecord, progress bool) []byte { var out bytes.Buffer if progress { - appendProgress(&out, "confluence: importing %d pages", len(pages)) + appendProgress(&out, "importing %d pages", len(pages)) for _, page := range pages { - appendProgress(&out, "confluence: importing page %s %s", page.PageID, page.Title) + appendProgress(&out, "importing page %s %s", page.PageID, page.Title) } } fmt.Fprintf(&out, "commit %s\n", branch) @@ -83,7 +84,7 @@ func BuildStreamWithProgress(branch string, location Location, pages []PageRecor out.WriteByte('\n') if progress { - appendProgress(&out, "confluence: done") + appendProgress(&out, "done") } out.WriteString("done\n") return out.Bytes() @@ -174,9 +175,10 @@ func appendData(out *bytes.Buffer, data []byte) { func appendProgress(out *bytes.Buffer, format string, args ...any) { message := fmt.Sprintf(format, args...) - message = strings.NewReplacer("\n", " ", "\r", " ").Replace(message) + var record bytes.Buffer + logging.New(&record).Info(message, "app", "git-remote-confluence") out.WriteString("progress ") - out.WriteString(message) + out.Write(bytes.TrimSuffix(record.Bytes(), []byte{'\n'})) out.WriteByte('\n') } diff --git a/internal/fastimport/stream_test.go b/internal/fastimport/stream_test.go index 7823b9b..5c69a8f 100644 --- a/internal/fastimport/stream_test.go +++ b/internal/fastimport/stream_test.go @@ -20,7 +20,7 @@ func TestBuildStreamContainsPageFiles(t *testing.T) { ID: "10", Title: "挿絵.png", Path: "1/attachments/挿絵.png", - Pointer: []byte("version pointer/v1\nattachment-id 10\nattachment-version 2\n"), + Pointer: []byte("version: pointer/v1\nattachment_id: \"10\"\nattachment_version: 2\n"), }}, } @@ -31,8 +31,8 @@ func TestBuildStreamContainsPageFiles(t *testing.T) { []byte("M 100644 inline 1.md\n"), []byte("M 100644 inline 1.yml\n"), []byte("M 100644 inline 1/attachments/挿絵.png\n"), - []byte("attachment-id 10\n"), - []byte("attachment-version 2\n"), + []byte("attachment_id: \"10\"\n"), + []byte("attachment_version: 2\n"), []byte(`storage_xml: "1.md"`), []byte("number: 3\n"), } { @@ -62,3 +62,17 @@ func TestBuildAttachmentStreamContainsVersionHistory(t *testing.T) { } } } + +func TestBuildStreamProgressUsesLogfmt(t *testing.T) { + page := PageRecord{PageID: "1", Title: "line one\nline two"} + stream := BuildStreamWithProgress(DefaultBranch, Location{}, []PageRecord{page}, true) + for _, expected := range [][]byte{ + []byte(`progress level=INFO msg="importing 1 pages" app=git-remote-confluence` + "\n"), + []byte(`progress level=INFO msg="importing page 1 line one\nline two" app=git-remote-confluence` + "\n"), + []byte(`progress level=INFO msg=done app=git-remote-confluence` + "\n"), + } { + if !bytes.Contains(stream, expected) { + t.Fatalf("stream did not contain %q\n%s", expected, stream) + } + } +} diff --git a/internal/logging/logging.go b/internal/logging/logging.go new file mode 100644 index 0000000..5352514 --- /dev/null +++ b/internal/logging/logging.go @@ -0,0 +1,20 @@ +package logging + +import ( + "io" + "log/slog" +) + +// New returns a logger whose text output is stable logfmt. Command-line logs +// omit timestamps because the surrounding process already provides ordering. +func New(output io.Writer) *slog.Logger { + handler := slog.NewTextHandler(output, &slog.HandlerOptions{ + ReplaceAttr: func(groups []string, attr slog.Attr) slog.Attr { + if attr.Key == slog.TimeKey { + return slog.Attr{} + } + return attr + }, + }) + return slog.New(handler) +} diff --git a/internal/logging/logging_test.go b/internal/logging/logging_test.go new file mode 100644 index 0000000..3233a21 --- /dev/null +++ b/internal/logging/logging_test.go @@ -0,0 +1,15 @@ +package logging + +import ( + "bytes" + "testing" +) + +func TestNewWritesLogfmt(t *testing.T) { + var output bytes.Buffer + New(&output).Warn("download failed", "path", "one two.png") + want := `level=WARN msg="download failed" path="one two.png"` + "\n" + if output.String() != want { + t.Fatalf("log output = %q, want %q", output.String(), want) + } +} diff --git a/internal/remotehelper/helper.go b/internal/remotehelper/helper.go index 93ce269..a0fb540 100644 --- a/internal/remotehelper/helper.go +++ b/internal/remotehelper/helper.go @@ -4,11 +4,13 @@ import ( "bufio" "fmt" "io" + "log/slog" "os" "strings" "github.com/hkwi/git-remote-confluence/internal/confluence" "github.com/hkwi/git-remote-confluence/internal/fastimport" + "github.com/hkwi/git-remote-confluence/internal/logging" ) func Main(args []string, stdin io.Reader, stdout, stderr io.Writer) error { @@ -24,7 +26,7 @@ func Main(args []string, stdin io.Reader, stdout, stderr io.Writer) error { remoteURL: remoteURL, in: bufio.NewReader(stdin), out: stdout, - err: progressOut, + logger: logging.New(progressOut), verbosity: 1, progress: true, }).serve() @@ -35,7 +37,7 @@ type helper struct { remoteURL string in *bufio.Reader out io.Writer - err io.Writer + logger *slog.Logger verbosity int progress bool } @@ -218,10 +220,10 @@ func (h *helper) confluenceClient() (confluence.Location, *confluence.Client, er } func (h *helper) reportProgress(format string, args ...any) { - if h.err == nil || !h.showProgress() { + if h.logger == nil || !h.showProgress() { return } - fmt.Fprintf(h.err, "confluence: "+format+"\n", args...) + h.logger.Info(fmt.Sprintf(format, args...), "app", "git-remote-confluence") } func (h *helper) showProgress() bool { diff --git a/internal/remotehelper/helper_test.go b/internal/remotehelper/helper_test.go index e7154e4..1ac9b0c 100644 --- a/internal/remotehelper/helper_test.go +++ b/internal/remotehelper/helper_test.go @@ -59,11 +59,12 @@ func TestGitCloneFromMockConfluence(t *testing.T) { attachment := readCloneFile(t, destination, "1", "attachments", "挿絵.png") for _, want := range []string{ - "attachment-id 10\n", - "attachment-version 2\n", - "size 9\n", - "media-type image/png\n", - "download-path /download/attachments/1/%E6%8C%BF%E7%B5%B5.png\n", + "version: https://github.com/hkwi/git-remote-confluence/spec/attachment/v1\n", + "attachment_id: \"10\"\n", + "attachment_version: 2\n", + "size: 9\n", + "media_type: image/png\n", + "download_path: /download/attachments/1/%E6%8C%BF%E7%B5%B5.png\n", } { if !strings.Contains(string(attachment), want) { t.Fatalf("attachment pointer missing %q:\n%s", want, attachment) @@ -159,10 +160,10 @@ func TestHelperProgressOutput(t *testing.T) { if err != nil { t.Fatal(err) } - if !strings.Contains(stderr.String(), "confluence: fetching page 1") { + if !strings.Contains(stderr.String(), `level=INFO msg="fetching page 1" app=git-remote-confluence`) { t.Fatalf("stderr progress missing:\n%s", stderr.String()) } - if !strings.Contains(stdout.String(), "progress confluence: importing page 1") { + if !strings.Contains(stdout.String(), `progress level=INFO msg="importing page 1 `) { t.Fatalf("fast-import progress missing:\n%s", stdout.String()) } if !strings.Contains(stdout.String(), "M 100644 inline 1.md\n") { @@ -221,10 +222,10 @@ func TestHelperResolvesDisplayPageURL(t *testing.T) { stderrText := stderr.String() for _, want := range []string{ - `confluence: resolving page "吾輩は猫である" in space ABC`, - `confluence: resolved page "吾輩は猫である" in space ABC to page 1`, - "confluence: root page 1 at " + server.URL, - "confluence: fetching page 1", + `level=INFO msg="resolving page \"吾輩は猫である\" in space ABC" app=git-remote-confluence`, + `level=INFO msg="resolved page \"吾輩は猫である\" in space ABC to page 1" app=git-remote-confluence`, + `level=INFO msg="root page 1 at ` + server.URL + `" app=git-remote-confluence`, + `level=INFO msg="fetching page 1" app=git-remote-confluence`, } { if !strings.Contains(stderrText, want) { t.Fatalf("stderr missing %q:\n%s", want, stderrText) @@ -233,7 +234,7 @@ func TestHelperResolvesDisplayPageURL(t *testing.T) { if strings.Contains(stderrText, "fetching space ABC") { t.Fatalf("display page URL was treated as a space:\n%s", stderrText) } - if !strings.Contains(stdout.String(), "progress confluence: importing page 1 吾輩は猫である") { + if !strings.Contains(stdout.String(), `progress level=INFO msg="importing page 1 吾輩は猫である" app=git-remote-confluence`) { t.Fatalf("fast-import progress missing resolved page:\n%s", stdout.String()) } } diff --git a/main.go b/main.go index 9481a62..ed9c199 100644 --- a/main.go +++ b/main.go @@ -6,6 +6,7 @@ import ( "runtime/debug" "github.com/hkwi/git-remote-confluence/internal/confluence" + "github.com/hkwi/git-remote-confluence/internal/logging" "github.com/hkwi/git-remote-confluence/internal/remotehelper" ) @@ -29,7 +30,7 @@ func main() { confluence.SetUserAgentVersion(releaseVersion()) if err := remotehelper.Main(args, os.Stdin, os.Stdout, os.Stderr); err != nil { - fmt.Fprintf(os.Stderr, "%s: %v\n", appName, err) + logging.New(os.Stderr).Error(err.Error(), "app", appName) os.Exit(1) } }