diff --git a/cmd/block.go b/cmd/block.go index 6b1b367..fe7a68a 100644 --- a/cmd/block.go +++ b/cmd/block.go @@ -191,8 +191,12 @@ var blockAppendCmd = &cobra.Command{ Short: "Append blocks to a page", Long: `Append content to a Notion page or block. -Supports plain text, block types, and markdown files. Large markdown files -are handled transparently: +Supports plain text, block types, markdown files, and media blocks. +Media blocks accept one of: + --image-url/--image-file/--image-upload (and the same pattern for + file/video/audio/pdf). See 'notion block append --help' for the full list. + +Large markdown files are handled transparently: - >100 children are auto-batched into sequential PATCHes. - code / rich_text exceeding Notion's 2000-char limit are split by default (override with --on-oversize=truncate|fail). @@ -203,7 +207,9 @@ Examples: notion block append --type code --lang go "fmt.Println()" notion block append --file notes.md notion block append --file big.md --on-oversize=truncate - notion block append --image-url https://example.com/a.png --caption "图 1-1"`, + notion block append --image-url https://example.com/a.png --caption "图 1-1" + notion block append --image-file ./chart.png --caption "heap usage" + notion block append --pdf-upload 351d45fb-... --caption "spec v2"`, Args: cobra.MinimumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { token, err := getToken() @@ -214,8 +220,6 @@ Examples: parentID := util.ResolveID(args[0]) blockType, _ := cmd.Flags().GetString("type") filePath, _ := cmd.Flags().GetString("file") - imageURL, _ := cmd.Flags().GetString("image-url") - caption, _ := cmd.Flags().GetString("caption") onOversizeRaw, _ := cmd.Flags().GetString("on-oversize") mode, err := parseOversizeMode(onOversizeRaw) if err != nil { @@ -227,7 +231,8 @@ Examples: text = args[1] } - if err := validateMediaFlags(imageURL, filePath, text); err != nil { + mediaSrc, err := resolveMediaSource(cmd, filePath, text) + if err != nil { return err } @@ -240,8 +245,12 @@ Examples: var children []map[string]interface{} - if imageURL != "" { - children = append(children, buildExternalImageBlock(imageURL, caption)) + if mediaSrc.IsActive() { + block, err := mediaSrc.Build(c) + if err != nil { + return err + } + children = append(children, block) } else if filePath != "" { // Read file and parse markdown to blocks data, err := os.ReadFile(filePath) @@ -251,7 +260,7 @@ Examples: children = parseMarkdownToBlocks(string(data)) } else { if text == "" { - return fmt.Errorf("text content, --file, or --image-url is required") + return fmt.Errorf("text content, --file, or a media source (--image-url, --image-file, --image-upload, ...) is required") } notionType := mapBlockType(blockType) @@ -343,7 +352,8 @@ Examples: notion block insert "New paragraph" --after notion block insert "Section" --after --type h2 notion block insert --file notes.md --after - notion block insert --after --image-url https://example.com/a.png --caption "图 1-1"`, + notion block insert --after --image-url https://example.com/a.png --caption "图 1-1" + notion block insert --after --image-file ./chart.png`, Args: cobra.MinimumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { token, err := getToken() @@ -355,15 +365,14 @@ Examples: afterID, _ := cmd.Flags().GetString("after") blockType, _ := cmd.Flags().GetString("type") filePath, _ := cmd.Flags().GetString("file") - imageURL, _ := cmd.Flags().GetString("image-url") - caption, _ := cmd.Flags().GetString("caption") text := "" if len(args) > 1 { text = args[1] } - if err := validateMediaFlags(imageURL, filePath, text); err != nil { + mediaSrc, err := resolveMediaSource(cmd, filePath, text) + if err != nil { return err } @@ -387,8 +396,12 @@ Examples: return err } - if imageURL != "" { - children = append(children, buildExternalImageBlock(imageURL, caption)) + if mediaSrc.IsActive() { + block, err := mediaSrc.Build(c) + if err != nil { + return err + } + children = append(children, block) } else if filePath != "" { data, err := os.ReadFile(filePath) if err != nil { @@ -397,7 +410,7 @@ Examples: children = parseMarkdownToBlocks(string(data)) } else { if text == "" { - return fmt.Errorf("text content, --file, or --image-url is required") + return fmt.Errorf("text content, --file, or a media source (--image-url, --image-file, --image-upload, ...) is required") } notionType := mapBlockType(blockType) @@ -591,16 +604,14 @@ func init() { blockAppendCmd.Flags().StringP("type", "t", "paragraph", "Block type: paragraph, h1, h2, h3, todo, bullet, numbered, quote, code, callout, divider") blockAppendCmd.Flags().String("lang", "plain text", "Language for code blocks (e.g. go, python, bash)") blockAppendCmd.Flags().String("file", "", "Read content from a file (each double-newline-separated section becomes a block)") - blockAppendCmd.Flags().String("image-url", "", "Append an external image block by URL (http/https)") - blockAppendCmd.Flags().String("caption", "", "Caption for --image-url (optional)") blockAppendCmd.Flags().String("on-oversize", "split", "Behavior for rich_text >2000 chars: split|truncate|fail") + registerMediaFlags(blockAppendCmd) blockInsertCmd.Flags().String("after", "", "Block ID to insert after (required)") blockInsertCmd.Flags().StringP("type", "t", "paragraph", "Block type") blockInsertCmd.Flags().String("lang", "plain text", "Language for code blocks") blockInsertCmd.Flags().String("file", "", "Read content from a file") - blockInsertCmd.Flags().String("image-url", "", "Insert an external image block by URL (http/https)") - blockInsertCmd.Flags().String("caption", "", "Caption for --image-url (optional)") blockInsertCmd.Flags().String("on-oversize", "split", "Behavior for rich_text >2000 chars: split|truncate|fail") + registerMediaFlags(blockInsertCmd) blockListCmd.Flags().String("cursor", "", "Pagination cursor") blockListCmd.Flags().Bool("all", false, "Fetch all pages of results") blockListCmd.Flags().Int("depth", 1, "Depth of nested blocks to fetch (default 1)") @@ -621,23 +632,13 @@ func init() { } func buildExternalImageBlock(url, caption string) map[string]interface{} { - image := map[string]interface{}{ - "type": "external", - "external": map[string]interface{}{"url": url}, - } - if caption != "" { - image["caption"] = []map[string]interface{}{ - {"type": "text", "text": map[string]interface{}{"content": caption}}, - } - } - return map[string]interface{}{ - "object": "block", - "type": "image", - "image": image, - } + // Back-compat wrapper; new code should use buildExternalMediaBlock. + return buildExternalMediaBlock("image", url, caption) } func validateMediaFlags(imageURL, filePath, text string) error { + // Back-compat shim preserved for existing tests; new flows use + // resolveMediaSource(). if imageURL == "" { return nil } diff --git a/cmd/media_source.go b/cmd/media_source.go new file mode 100644 index 0000000..e876f78 --- /dev/null +++ b/cmd/media_source.go @@ -0,0 +1,170 @@ +package cmd + +import ( + "fmt" + "strings" + + "github.com/spf13/cobra" +) + +// Media block kinds supported by Notion. The string is both the +// user-facing flag prefix ("image", "file", "video", "audio", "pdf") and +// the Notion block type on the wire. +var mediaKinds = []string{"image", "file", "video", "audio", "pdf"} + +// mediaClient is the minimal client surface mediaBuilder needs. Kept as an +// interface so media flag resolution is testable without a real HTTP client. +type mediaClient interface { + fileUploadAPI +} + +// mediaSource describes a user-chosen way to produce a media block. +// Exactly one source is active per CLI invocation. +type mediaSource struct { + kind string // "image" | "file" | "video" | "audio" | "pdf" + mode string // "external" | "file" | "upload" + value string // URL / local path / file_upload id + caption string +} + +// IsActive reports whether any media flag was set. +func (m *mediaSource) IsActive() bool { + return m != nil && m.kind != "" +} + +// Build performs any required upload step and returns the assembled block. +// For "external" sources there's no network I/O; for "file" sources we +// upload the local path and then reference the returned file_upload id. +func (m *mediaSource) Build(c mediaClient) (map[string]interface{}, error) { + switch m.mode { + case "external": + return buildExternalMediaBlock(m.kind, m.value, m.caption), nil + case "upload": + return buildFileUploadMediaBlock(m.kind, m.value, m.caption), nil + case "file": + outcome, err := uploadFile(c, m.value, "") + if err != nil { + return nil, fmt.Errorf("upload %s: %w", m.value, err) + } + return buildFileUploadMediaBlock(m.kind, outcome.UploadID, m.caption), nil + default: + return nil, fmt.Errorf("internal: unknown media mode %q", m.mode) + } +} + +// registerMediaFlags adds all media source flags to a cobra command. +// The flag names follow a strict pattern: +// +// --image-url http(s) URL (external image) +// --image-file local path (upload + embed) +// --image-upload existing file_upload id (embed only) +// +// The same triple exists for file / video / audio / pdf, minus the -url +// variant for non-image types (image is the only one that previously +// shipped --image-url, and the Notion API accepts external URLs for every +// media type — we keep parity by also adding -url for the others, so +// e.g. `--video-url https://…/clip.mp4` works). +func registerMediaFlags(cmd *cobra.Command) { + cmd.Flags().String("caption", "", "Caption for the media block (applies to any of the --*-url/--*-file/--*-upload flags)") + for _, kind := range mediaKinds { + cmd.Flags().String(kind+"-url", "", fmt.Sprintf("External %s URL (http/https)", kind)) + cmd.Flags().String(kind+"-file", "", fmt.Sprintf("Local %s file to upload and embed", kind)) + cmd.Flags().String(kind+"-upload", "", fmt.Sprintf("Existing file_upload ID to embed as %s", kind)) + } +} + +// resolveMediaSource inspects every media flag on the command and returns +// the single active source, or nil if none were used. It returns an error +// when more than one source is set, or when a source collides with --file +// (markdown) or positional text. +func resolveMediaSource(cmd *cobra.Command, filePath, text string) (*mediaSource, error) { + caption, _ := cmd.Flags().GetString("caption") + + var picked []string + var active *mediaSource + + for _, kind := range mediaKinds { + for _, mode := range []string{"url", "file", "upload"} { + flag := kind + "-" + mode + v, _ := cmd.Flags().GetString(flag) + v = strings.TrimSpace(v) + if v == "" { + continue + } + picked = append(picked, "--"+flag) + normalizedMode := mode + if mode == "url" { + normalizedMode = "external" + } + active = &mediaSource{ + kind: kind, + mode: normalizedMode, + value: v, + caption: caption, + } + } + } + + if len(picked) == 0 { + if caption != "" && filePath == "" && text == "" { + return nil, fmt.Errorf("--caption requires one of ---url/---file/---upload") + } + return nil, nil + } + if len(picked) > 1 { + return nil, fmt.Errorf("at most one media source may be set, got: %s", strings.Join(picked, ", ")) + } + if filePath != "" { + return nil, fmt.Errorf("%s cannot be combined with --file", picked[0]) + } + if text != "" { + return nil, fmt.Errorf("%s cannot be combined with a positional text argument", picked[0]) + } + + if active.mode == "external" { + if !strings.HasPrefix(active.value, "http://") && !strings.HasPrefix(active.value, "https://") { + return nil, fmt.Errorf("%s must be an http:// or https:// URL", picked[0]) + } + } + return active, nil +} + +// buildExternalMediaBlock produces a block whose media source is an +// external URL (no upload needed). +func buildExternalMediaBlock(kind, url, caption string) map[string]interface{} { + media := map[string]interface{}{ + "type": "external", + "external": map[string]interface{}{"url": url}, + } + if caption != "" { + media["caption"] = captionRichText(caption) + } + return map[string]interface{}{ + "object": "block", + "type": kind, + kind: media, + } +} + +// buildFileUploadMediaBlock produces a block that references a previously +// created file_upload by id. +func buildFileUploadMediaBlock(kind, uploadID, caption string) map[string]interface{} { + media := map[string]interface{}{ + "type": "file_upload", + "file_upload": map[string]interface{}{"id": uploadID}, + } + if caption != "" { + media["caption"] = captionRichText(caption) + } + return map[string]interface{}{ + "object": "block", + "type": kind, + kind: media, + } +} + +func captionRichText(caption string) []map[string]interface{} { + return []map[string]interface{}{ + {"type": "text", "text": map[string]interface{}{"content": caption}}, + } +} diff --git a/cmd/media_source_test.go b/cmd/media_source_test.go new file mode 100644 index 0000000..b5a198f --- /dev/null +++ b/cmd/media_source_test.go @@ -0,0 +1,218 @@ +package cmd + +import ( + "fmt" + "os" + "strings" + "testing" + + "github.com/spf13/cobra" +) + +func newMediaCmd() *cobra.Command { + c := &cobra.Command{Use: "test"} + registerMediaFlags(c) + return c +} + +type fakeMediaClient struct { + mockFileUploadClient +} + +func (f *fakeMediaClient) Post(path string, body interface{}) ([]byte, error) { + return f.mockFileUploadClient.Post(path, body) +} +func (f *fakeMediaClient) Patch(path string, body interface{}) ([]byte, error) { + return f.mockFileUploadClient.Patch(path, body) +} +func (f *fakeMediaClient) UploadFileContent(uploadID, fileName, contentType string, fileBytes []byte) ([]byte, error) { + return f.mockFileUploadClient.UploadFileContent(uploadID, fileName, contentType, fileBytes) +} + +func TestResolveMediaSource_NoneSet(t *testing.T) { + cmd := newMediaCmd() + got, err := resolveMediaSource(cmd, "", "") + if err != nil { + t.Fatal(err) + } + if got.IsActive() { + t.Error("no flags set should produce inactive source") + } +} + +func TestResolveMediaSource_EveryKindAndMode(t *testing.T) { + cases := []struct { + flag string + value string + wantKind string + wantMode string + }{ + {"image-url", "https://x/y.png", "image", "external"}, + {"image-file", "/tmp/a.png", "image", "file"}, + {"image-upload", "abc-123", "image", "upload"}, + {"video-url", "https://x/y.mp4", "video", "external"}, + {"video-file", "/tmp/a.mp4", "video", "file"}, + {"video-upload", "abc-123", "video", "upload"}, + {"audio-url", "https://x/y.mp3", "audio", "external"}, + {"audio-file", "/tmp/a.mp3", "audio", "file"}, + {"audio-upload", "abc-123", "audio", "upload"}, + {"file-url", "https://x/y.zip", "file", "external"}, + {"file-file", "/tmp/a.zip", "file", "file"}, + {"file-upload", "abc-123", "file", "upload"}, + {"pdf-url", "https://x/y.pdf", "pdf", "external"}, + {"pdf-file", "/tmp/a.pdf", "pdf", "file"}, + {"pdf-upload", "abc-123", "pdf", "upload"}, + } + for _, tc := range cases { + t.Run(tc.flag, func(t *testing.T) { + cmd := newMediaCmd() + if err := cmd.Flags().Set(tc.flag, tc.value); err != nil { + t.Fatal(err) + } + got, err := resolveMediaSource(cmd, "", "") + if err != nil { + t.Fatal(err) + } + if got.kind != tc.wantKind || got.mode != tc.wantMode || got.value != tc.value { + t.Errorf("got %+v, want {kind:%s mode:%s value:%s}", got, tc.wantKind, tc.wantMode, tc.value) + } + }) + } +} + +func TestResolveMediaSource_MutualExclusion(t *testing.T) { + cmd := newMediaCmd() + cmd.Flags().Set("image-url", "https://x/y.png") + cmd.Flags().Set("pdf-upload", "abc-123") + _, err := resolveMediaSource(cmd, "", "") + if err == nil { + t.Fatal("expected error for two active sources") + } + if !strings.Contains(err.Error(), "at most one media source") { + t.Errorf("unexpected error text: %v", err) + } +} + +func TestResolveMediaSource_ConflictsWithTextAndFile(t *testing.T) { + cmd := newMediaCmd() + cmd.Flags().Set("image-file", "/tmp/x.png") + if _, err := resolveMediaSource(cmd, "notes.md", ""); err == nil || !strings.Contains(err.Error(), "--file") { + t.Errorf("should conflict with --file, got: %v", err) + } + cmd2 := newMediaCmd() + cmd2.Flags().Set("image-upload", "abc") + if _, err := resolveMediaSource(cmd2, "", "hello"); err == nil || !strings.Contains(err.Error(), "positional") { + t.Errorf("should conflict with text, got: %v", err) + } +} + +func TestResolveMediaSource_RejectsNonHTTPExternal(t *testing.T) { + cmd := newMediaCmd() + cmd.Flags().Set("image-url", "ftp://x/y.png") + _, err := resolveMediaSource(cmd, "", "") + if err == nil || !strings.Contains(err.Error(), "http://") { + t.Errorf("ftp url should be rejected, got: %v", err) + } +} + +func TestResolveMediaSource_CaptionWithoutSource(t *testing.T) { + cmd := newMediaCmd() + cmd.Flags().Set("caption", "fig.1") + if _, err := resolveMediaSource(cmd, "", ""); err == nil { + t.Error("standalone --caption should error") + } +} + +func TestBuildExternalMediaBlock_Types(t *testing.T) { + for _, kind := range mediaKinds { + b := buildExternalMediaBlock(kind, "https://x/y", "c") + if b["type"] != kind { + t.Errorf("type=%v, want %s", b["type"], kind) + } + m := b[kind].(map[string]interface{}) + if m["type"] != "external" { + t.Errorf("%s.type=%v, want external", kind, m["type"]) + } + if _, ok := m["caption"]; !ok { + t.Errorf("%s caption missing", kind) + } + } +} + +func TestBuildFileUploadMediaBlock_Types(t *testing.T) { + for _, kind := range mediaKinds { + b := buildFileUploadMediaBlock(kind, "u-1", "") + if b["type"] != kind { + t.Errorf("type=%v, want %s", b["type"], kind) + } + m := b[kind].(map[string]interface{}) + if m["type"] != "file_upload" { + t.Errorf("%s.type=%v, want file_upload", kind, m["type"]) + } + fu := m["file_upload"].(map[string]interface{}) + if fu["id"] != "u-1" { + t.Errorf("%s file_upload.id=%v", kind, fu["id"]) + } + if _, ok := m["caption"]; ok { + t.Errorf("%s caption should be omitted when empty", kind) + } + } +} + +func TestMediaSource_Build_Upload(t *testing.T) { + src := &mediaSource{kind: "pdf", mode: "upload", value: "upload-xyz", caption: "spec"} + block, err := src.Build(&fakeMediaClient{}) + if err != nil { + t.Fatal(err) + } + if block["type"] != "pdf" { + t.Errorf("type = %v", block["type"]) + } + m := block["pdf"].(map[string]interface{}) + if m["file_upload"].(map[string]interface{})["id"] != "upload-xyz" { + t.Errorf("wrong upload id: %v", m) + } +} + +func TestMediaSource_Build_External(t *testing.T) { + src := &mediaSource{kind: "image", mode: "external", value: "https://x/y.png"} + block, err := src.Build(&fakeMediaClient{}) + if err != nil { + t.Fatal(err) + } + m := block["image"].(map[string]interface{}) + if m["external"].(map[string]interface{})["url"] != "https://x/y.png" { + t.Errorf("wrong url: %v", m) + } +} + +// Regression: --image-file should upload the local file and then reference +// the returned file_upload id in a single block, without attaching +// anywhere else (attaching is the caller's job via appendChildrenBatched). +func TestMediaSource_Build_File_UploadsAndReferences(t *testing.T) { + tmp := t.TempDir() + localPath := fmt.Sprintf("%s/chart.png", tmp) + if err := writeTempFile(localPath, "not-a-png"); err != nil { + t.Fatal(err) + } + mock := &fakeMediaClient{} + src := &mediaSource{kind: "image", mode: "file", value: localPath} + block, err := src.Build(mock) + if err != nil { + t.Fatal(err) + } + if mock.postPath != "/v1/file_uploads" { + t.Errorf("expected file_uploads POST, got %q", mock.postPath) + } + if mock.patchPath != "" { + t.Errorf("media source should NOT patch a parent; got path=%q", mock.patchPath) + } + m := block["image"].(map[string]interface{}) + if m["file_upload"].(map[string]interface{})["id"] != "upload-123" { + t.Errorf("expected upload-123, got %v", m["file_upload"]) + } +} + +func writeTempFile(path, contents string) error { + return os.WriteFile(path, []byte(contents), 0600) +}