Skip to content

refactor: migrate mail shortcuts to FileIO #356

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
tuxedomm merged 5 commits into from
Apr 9, 2026
Merged

refactor: migrate mail shortcuts to FileIO #356

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f8b08f2
refactor: migrate mail shortcuts to FileIO
tuxedomm Apr 8, 2026
e2158b2
fix: preserve original error messages in mail draft patch and helpers
tuxedomm Apr 8, 2026
0785663
fix: restore validation order and fix misleading comments in mail mig…
tuxedomm Apr 8, 2026
ed1c3ee
refactor: extract DraftCtx from DraftSnapshot for runtime dependencies
tuxedomm Apr 9, 2026
1c3be98
fix: restore original validation order in addAttachment
tuxedomm Apr 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions shortcuts/mail/draft/acceptance_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ func TestAcceptanceReplyDraftSubjectOnly(t *testing.T) {
originalInline := findPart(snapshot.Body, "1.2")
originalAttachment := findPart(snapshot.Body, "1.3")

if err := Apply(snapshot, Patch{
if err := Apply(&DraftCtx{FIO: testFIO}, snapshot, Patch{
Ops: []PatchOp{{Op: "set_subject", Value: "Reply updated"}},
}); err != nil {
t.Fatalf("Apply() error = %v", err)
Expand Down Expand Up @@ -46,7 +46,7 @@ func TestAcceptanceReplyDraftSubjectOnly(t *testing.T) {

func TestAcceptanceHTMLInlineReplaceHTML(t *testing.T) {
snapshot := mustParseFixtureDraft(t, mustReadFixture(t, "testdata/html_inline_draft.eml"))
if err := Apply(snapshot, Patch{
if err := Apply(&DraftCtx{FIO: testFIO}, snapshot, Patch{
Ops: []PatchOp{{Op: "replace_body", BodyKind: "text/html", Selector: "primary", Value: `<div>updated<img src="cid:logo"></div>`}},
}); err != nil {
t.Fatalf("Apply() error = %v", err)
Expand All @@ -70,7 +70,7 @@ func TestAcceptanceHTMLInlineReplaceHTML(t *testing.T) {

func TestAcceptanceAlternativeSetBodyUpdatesHTMLAndSummary(t *testing.T) {
snapshot := mustParseFixtureDraft(t, mustReadFixture(t, "testdata/alternative_draft.eml"))
if err := Apply(snapshot, Patch{
if err := Apply(&DraftCtx{FIO: testFIO}, snapshot, Patch{
Ops: []PatchOp{{Op: "set_body", Value: "<div>updated <strong>body</strong></div>"}},
}); err != nil {
t.Fatalf("Apply() error = %v", err)
Expand All @@ -97,7 +97,7 @@ func TestAcceptanceCalendarDraftAppendPlainPreservesCalendar(t *testing.T) {
if originalCalendar == nil {
t.Fatalf("calendar part missing")
}
if err := Apply(snapshot, Patch{
if err := Apply(&DraftCtx{FIO: testFIO}, snapshot, Patch{
Ops: []PatchOp{{Op: "append_body", BodyKind: "text/plain", Selector: "primary", Value: "\nagenda"}},
}); err != nil {
t.Fatalf("Apply() error = %v", err)
Expand All @@ -122,7 +122,7 @@ func TestAcceptanceCalendarDraftAppendPlainPreservesCalendar(t *testing.T) {
func TestAcceptanceSignedDraftSubjectOnlyPreservesSignedEntity(t *testing.T) {
snapshot := mustParseFixtureDraft(t, mustReadFixture(t, "testdata/multipart_signed_draft.eml"))
originalBodyEntity := string(snapshot.Body.RawEntity)
if err := Apply(snapshot, Patch{
if err := Apply(&DraftCtx{FIO: testFIO}, snapshot, Patch{
Ops: []PatchOp{{Op: "set_subject", Value: "Signed updated"}},
}); err != nil {
t.Fatalf("Apply() error = %v", err)
Expand All @@ -144,7 +144,7 @@ func TestAcceptanceDirtyMultipartAppendPlainPreservesOuterNoise(t *testing.T) {
snapshot := mustParseFixtureDraft(t, mustReadFixture(t, "testdata/dirty_multipart_preamble.eml"))
originalPreamble := string(snapshot.Body.Preamble)
originalEpilogue := string(snapshot.Body.Epilogue)
if err := Apply(snapshot, Patch{
if err := Apply(&DraftCtx{FIO: testFIO}, snapshot, Patch{
Ops: []PatchOp{{Op: "append_body", BodyKind: "text/plain", Selector: "primary", Value: "\nworld"}},
}); err != nil {
t.Fatalf("Apply() error = %v", err)
Expand Down
8 changes: 8 additions & 0 deletions shortcuts/mail/draft/model.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import (
"mime"
"net/mail"
"strings"

"github.com/larksuite/cli/extension/fileio"
)

type DraftRaw struct {
Expand Down Expand Up @@ -98,6 +100,12 @@ func (p *Part) FileName() string {
return ""
}

// DraftCtx carries runtime dependencies for draft operations.
// It is separate from DraftSnapshot to keep the snapshot a pure data model.
type DraftCtx struct {
FIO fileio.FileIO
}

type DraftSnapshot struct {
Comment thread
greptile-apps[bot] marked this conversation as resolved.
DraftID string
Headers []Header
Expand Down
75 changes: 39 additions & 36 deletions shortcuts/mail/draft/patch.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,14 @@ package draft

import (
"fmt"
"io"
"mime"
"path/filepath"
"regexp"
"strings"

"github.com/google/uuid"
"github.com/larksuite/cli/internal/validate"
"github.com/larksuite/cli/internal/vfs"
"github.com/larksuite/cli/shortcuts/mail/filecheck"
)

Expand All @@ -39,26 +39,26 @@ var bodyChangingOps = map[string]bool{
"append_body": true,
}

func Apply(snapshot *DraftSnapshot, patch Patch) error {
func Apply(dctx *DraftCtx, snapshot *DraftSnapshot, patch Patch) error {
if err := patch.Validate(); err != nil {
return err
}
hasBodyChange := false
for _, op := range patch.Ops {
if err := applyOp(snapshot, op, patch.Options); err != nil {
if err := applyOp(dctx, snapshot, op, patch.Options); err != nil {
return err
}
if bodyChangingOps[op.Op] {
hasBodyChange = true
}
}
if err := postProcessInlineImages(snapshot, hasBodyChange); err != nil {
if err := postProcessInlineImages(dctx, snapshot, hasBodyChange); err != nil {
return err
}
return refreshSnapshot(snapshot)
}

func applyOp(snapshot *DraftSnapshot, op PatchOp, options PatchOptions) error {
func applyOp(dctx *DraftCtx, snapshot *DraftSnapshot, op PatchOp, options PatchOptions) error {
switch op.Op {
case "set_subject":
if strings.ContainsAny(op.Value, "\r\n") {
Expand Down Expand Up @@ -100,21 +100,21 @@ func applyOp(snapshot *DraftSnapshot, op PatchOp, options PatchOptions) error {
}
removeHeader(&snapshot.Headers, op.Name)
case "add_attachment":
return addAttachment(snapshot, op.Path)
return addAttachment(dctx, snapshot, op.Path)
case "remove_attachment":
partID, err := resolveTarget(snapshot, op.Target)
if err != nil {
return fmt.Errorf("remove_attachment: %w", err)
}
return removeAttachment(snapshot, partID)
case "add_inline":
return addInline(snapshot, op.Path, op.CID, op.FileName, op.ContentType)
return addInline(dctx, snapshot, op.Path, op.CID, op.FileName, op.ContentType)
case "replace_inline":
partID, err := resolveTarget(snapshot, op.Target)
if err != nil {
return fmt.Errorf("replace_inline: %w", err)
}
return replaceInline(snapshot, partID, op.Path, op.CID, op.FileName, op.ContentType)
return replaceInline(dctx, snapshot, partID, op.Path, op.CID, op.FileName, op.ContentType)
case "remove_inline":
partID, err := resolveTarget(snapshot, op.Target)
if err != nil {
Expand Down Expand Up @@ -478,22 +478,23 @@ func newMultipartContainer(mediaType string) *Part {
}
}

func addAttachment(snapshot *DraftSnapshot, path string) error {
safePath, err := validate.SafeInputPath(path)
if err != nil {
return fmt.Errorf("attachment %q: %w", path, err)
}
func addAttachment(dctx *DraftCtx, snapshot *DraftSnapshot, path string) error {
if err := checkBlockedExtension(filepath.Base(path)); err != nil {
return err
}
info, err := vfs.Stat(safePath)
info, err := dctx.FIO.Stat(path)
if err != nil {
return err
}
if err := checkSnapshotAttachmentLimit(snapshot, info.Size(), nil); err != nil {
return err
}
content, err := vfs.ReadFile(safePath)
f, err := dctx.FIO.Open(path)
if err != nil {
return err
}
defer f.Close()
content, err := io.ReadAll(f)
if err != nil {
return err
}
Expand Down Expand Up @@ -543,19 +544,20 @@ func addAttachment(snapshot *DraftSnapshot, path string) error {
// creates a MIME inline part, and attaches it to the snapshot's
// multipart/related container. If container is non-nil it is reused;
// otherwise the container is resolved from the snapshot.
func loadAndAttachInline(snapshot *DraftSnapshot, path, cid, fileName string, container *Part) (*Part, error) {
safePath, err := validate.SafeInputPath(path)
if err != nil {
return nil, fmt.Errorf("inline image %q: %w", path, err)
}
info, err := vfs.Stat(safePath)
func loadAndAttachInline(dctx *DraftCtx, snapshot *DraftSnapshot, path, cid, fileName string, container *Part) (*Part, error) {
info, err := dctx.FIO.Stat(path)
if err != nil {
return nil, fmt.Errorf("inline image %q: %w", path, err)
}
if err := checkSnapshotAttachmentLimit(snapshot, info.Size(), nil); err != nil {
return nil, err
}
content, err := vfs.ReadFile(safePath)
f, err := dctx.FIO.Open(path)
if err != nil {
return nil, fmt.Errorf("inline image %q: %w", path, err)
}
defer f.Close()
content, err := io.ReadAll(f)
if err != nil {
return nil, fmt.Errorf("inline image %q: %w", path, err)
}
Expand All @@ -567,7 +569,7 @@ func loadAndAttachInline(snapshot *DraftSnapshot, path, cid, fileName string, co
if err != nil {
return nil, fmt.Errorf("inline image %q: %w", path, err)
}
inline, err := newInlinePart(safePath, content, cid, name, detectedCT)
inline, err := newInlinePart(path, content, cid, name, detectedCT)
if err != nil {
return nil, fmt.Errorf("inline image %q: %w", path, err)
}
Expand All @@ -586,31 +588,32 @@ func loadAndAttachInline(snapshot *DraftSnapshot, path, cid, fileName string, co
return container, nil
}

func addInline(snapshot *DraftSnapshot, path, cid, fileName, contentType string) error {
_, err := loadAndAttachInline(snapshot, path, cid, fileName, nil)
func addInline(dctx *DraftCtx, snapshot *DraftSnapshot, path, cid, fileName, contentType string) error {
_, err := loadAndAttachInline(dctx, snapshot, path, cid, fileName, nil)
return err
}

func replaceInline(snapshot *DraftSnapshot, partID, path, cid, fileName, contentType string) error {
func replaceInline(dctx *DraftCtx, snapshot *DraftSnapshot, partID, path, cid, fileName, contentType string) error {
part := findPart(snapshot.Body, partID)
if part == nil {
return fmt.Errorf("inline part %q not found", partID)
}
if !isInlinePart(part) {
return fmt.Errorf("part %q is not an inline MIME part", partID)
}
safePath, err := validate.SafeInputPath(path)
if err != nil {
return fmt.Errorf("inline image %q: %w", path, err)
}
info, err := vfs.Stat(safePath)
info, err := dctx.FIO.Stat(path)
if err != nil {
return err
}
if err := checkSnapshotAttachmentLimit(snapshot, info.Size(), part); err != nil {
return err
}
content, err := vfs.ReadFile(safePath)
f, err := dctx.FIO.Open(path)
if err != nil {
return err
}
defer f.Close()
content, err := io.ReadAll(f)
if err != nil {
return err
}
Expand Down Expand Up @@ -990,7 +993,7 @@ func ResolveLocalImagePaths(html string) (string, []LocalImageRef, error) {
// resolveLocalImgSrc scans HTML for <img src="local/path"> references,
// creates MIME inline parts for each local file, and returns the HTML
// with those src attributes replaced by cid: URIs.
func resolveLocalImgSrc(snapshot *DraftSnapshot, html string) (string, error) {
func resolveLocalImgSrc(dctx *DraftCtx, snapshot *DraftSnapshot, html string) (string, error) {
resolved, refs, err := ResolveLocalImagePaths(html)
if err != nil {
return "", err
Expand All @@ -999,7 +1002,7 @@ func resolveLocalImgSrc(snapshot *DraftSnapshot, html string) (string, error) {
var container *Part
for _, ref := range refs {
fileName := filepath.Base(ref.FilePath)
container, err = loadAndAttachInline(snapshot, ref.FilePath, ref.CID, fileName, container)
container, err = loadAndAttachInline(dctx, snapshot, ref.FilePath, ref.CID, fileName, container)
if err != nil {
return "", err
}
Expand Down Expand Up @@ -1092,7 +1095,7 @@ func FindOrphanedCIDs(html string, addedCIDs []string) []string {
// NOTE: The EML builder path has an equivalent function processInlineImagesForEML
// in shortcuts/mail/helpers.go. When adding new validation or processing logic here,
// update processInlineImagesForEML as well (or extract a shared function).
func postProcessInlineImages(snapshot *DraftSnapshot, resolveLocal bool) error {
func postProcessInlineImages(dctx *DraftCtx, snapshot *DraftSnapshot, resolveLocal bool) error {
htmlPart := findPrimaryBodyPart(snapshot.Body, "text/html")
if htmlPart == nil {
return nil
Expand All @@ -1102,7 +1105,7 @@ func postProcessInlineImages(snapshot *DraftSnapshot, resolveLocal bool) error {
html := origHTML
if resolveLocal {
var err error
html, err = resolveLocalImgSrc(snapshot, origHTML)
html, err = resolveLocalImgSrc(dctx, snapshot, origHTML)
if err != nil {
return err
}
Expand Down
Loading
Loading