Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 3 additions & 3 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -73,20 +73,20 @@ linters:
- forbidigo
# errs-typed-only enforced on paths already migrated to errs.NewXxxError.
# Add a path when its migration is complete.
- path-except: (internal/auth/|internal/errcompat/|internal/errclass/|internal/client/|internal/cmdutil/factory\.go|cmd/auth/|cmd/config/|cmd/service/|shortcuts/common/mcp_client\.go|shortcuts/base/|shortcuts/calendar/|shortcuts/contact/|shortcuts/doc/|shortcuts/drive/|shortcuts/im/|shortcuts/mail/|shortcuts/minutes/|shortcuts/okr/|shortcuts/sheets/|shortcuts/task/|shortcuts/vc/|shortcuts/whiteboard/|internal/event/consume/|cmd/event/|events/|shortcuts/event/)
- path-except: (internal/auth/|internal/errcompat/|internal/errclass/|internal/client/|internal/cmdutil/factory\.go|cmd/auth/|cmd/config/|cmd/service/|shortcuts/common/mcp_client\.go|shortcuts/base/|shortcuts/calendar/|shortcuts/contact/|shortcuts/doc/|shortcuts/drive/|shortcuts/im/|shortcuts/mail/|shortcuts/minutes/|shortcuts/okr/|shortcuts/sheets/|shortcuts/slides/|shortcuts/task/|shortcuts/vc/|shortcuts/whiteboard/|internal/event/consume/|cmd/event/|events/|shortcuts/event/)
text: errs-typed-only
linters:
- forbidigo
# errs-no-bare-wrap enforced on paths fully migrated to typed final
# errors. Scoped separately from errs-typed-only because cmd/auth/,
# cmd/config/ still have residual fmt.Errorf and must not be caught.
- path-except: (shortcuts/base/|shortcuts/calendar/|shortcuts/contact/|shortcuts/doc/|shortcuts/drive/|shortcuts/im/|shortcuts/mail/|shortcuts/minutes/|shortcuts/okr/|shortcuts/sheets/|shortcuts/task/|shortcuts/vc/|shortcuts/whiteboard/|shortcuts/common/mcp_client\.go|cmd/event/|events/|shortcuts/event/)
- path-except: (shortcuts/base/|shortcuts/calendar/|shortcuts/contact/|shortcuts/doc/|shortcuts/drive/|shortcuts/im/|shortcuts/mail/|shortcuts/minutes/|shortcuts/okr/|shortcuts/sheets/|shortcuts/slides/|shortcuts/task/|shortcuts/vc/|shortcuts/whiteboard/|shortcuts/common/mcp_client\.go|cmd/event/|events/|shortcuts/event/)
text: errs-no-bare-wrap
linters:
- forbidigo
# errs-no-legacy-helper enforced on domains whose shared validation/save
# helpers have migrated to typed final errors.
- path-except: (shortcuts/base/|shortcuts/calendar/|shortcuts/contact/|shortcuts/doc/|shortcuts/drive/|shortcuts/im/|shortcuts/mail/|shortcuts/minutes/|shortcuts/okr/|shortcuts/sheets/|shortcuts/task/|shortcuts/vc/|shortcuts/whiteboard/|cmd/event/|events/|shortcuts/event/)
- path-except: (shortcuts/base/|shortcuts/calendar/|shortcuts/contact/|shortcuts/doc/|shortcuts/drive/|shortcuts/im/|shortcuts/mail/|shortcuts/minutes/|shortcuts/okr/|shortcuts/sheets/|shortcuts/slides/|shortcuts/task/|shortcuts/vc/|shortcuts/whiteboard/|cmd/event/|events/|shortcuts/event/)
text: errs-no-legacy-helper
linters:
- forbidigo
Expand Down
1 change: 1 addition & 0 deletions lint/errscontract/rule_no_legacy_common_helper_call.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ var migratedCommonHelperPaths = []string{
"shortcuts/minutes/",
"shortcuts/okr/",
"shortcuts/sheets/",
"shortcuts/slides/",
"shortcuts/task/",
"shortcuts/vc/",
"shortcuts/whiteboard/",
Expand Down
1 change: 1 addition & 0 deletions lint/errscontract/rule_no_legacy_envelope_literal.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ var migratedEnvelopePaths = []string{
"shortcuts/minutes/",
"shortcuts/okr/",
"shortcuts/sheets/",
"shortcuts/slides/",
"shortcuts/task/",
"shortcuts/vc/",
"shortcuts/whiteboard/",
Expand Down
18 changes: 18 additions & 0 deletions lint/errscontract/rules_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -955,6 +955,7 @@ func TestCheckNoLegacyCommonHelperCall_RejectsLegacyHelpersOnMigratedPath(t *tes
"shortcuts/mail/mail_send.go",
"shortcuts/okr/okr_progress_create.go",
"shortcuts/sheets/helpers.go",
"shortcuts/slides/slides_create.go",
"shortcuts/task/task_update.go",
"shortcuts/whiteboard/whiteboard_query.go",
}
Expand Down Expand Up @@ -1039,6 +1040,23 @@ func boom() {
}
}

func TestCheckNoLegacyCommonHelperCall_CoversSlidesPathWithAliasAndFunctionValue(t *testing.T) {
src := `package migrated

import c "github.com/larksuite/cli/shortcuts/common"

func boom() {
f := c.FlagErrorf
_ = f
c.WrapInputStatError(nil)
}
`
v := CheckNoLegacyCommonHelperCall("shortcuts/slides/slides_create.go", src)
if len(v) != 2 {
t.Fatalf("expected 2 violations for aliased/function-value legacy helpers on slides path, got %d: %+v", len(v), v)
}
}

func TestCheckNoLegacyCommonHelperCall_AllowsNonMigratedPath(t *testing.T) {
src := `package contact

Expand Down
24 changes: 14 additions & 10 deletions shortcuts/slides/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
"regexp"
"strings"

"github.com/larksuite/cli/internal/output"
"github.com/larksuite/cli/errs"
"github.com/larksuite/cli/shortcuts/common"
)

Expand All @@ -30,29 +30,29 @@
func parsePresentationRef(input string) (presentationRef, error) {
raw := strings.TrimSpace(input)
if raw == "" {
return presentationRef{}, output.ErrValidation("--presentation cannot be empty")
return presentationRef{}, errs.NewValidationError(errs.SubtypeInvalidArgument, "--presentation cannot be empty").WithParam("--presentation")
}
// URL inputs: parse properly and only honor /slides/ or /wiki/ when they
// appear as a prefix of the URL path. Substring matching previously let
// e.g. `https://x/docx/foo?next=/slides/abc` resolve to token "abc".
if strings.Contains(raw, "://") {
u, err := url.Parse(raw)
if err != nil || u.Path == "" {
return presentationRef{}, output.ErrValidation("unsupported --presentation input %q: use an xml_presentation_id, a /slides/ URL, or a /wiki/ URL", raw)
return presentationRef{}, errs.NewValidationError(errs.SubtypeInvalidArgument, "unsupported --presentation input %q: use an xml_presentation_id, a /slides/ URL, or a /wiki/ URL", raw).WithParam("--presentation")

Check warning on line 41 in shortcuts/slides/helpers.go

View check run for this annotation

Codecov / codecov/patch

shortcuts/slides/helpers.go#L41

Added line #L41 was not covered by tests
}
if token, ok := tokenAfterPathPrefix(u.Path, "/slides/"); ok {
return presentationRef{Kind: "slides", Token: token}, nil
}
if token, ok := tokenAfterPathPrefix(u.Path, "/wiki/"); ok {
return presentationRef{Kind: "wiki", Token: token}, nil
}
return presentationRef{}, output.ErrValidation("unsupported --presentation input %q: use an xml_presentation_id, a /slides/ URL, or a /wiki/ URL", raw)
return presentationRef{}, errs.NewValidationError(errs.SubtypeInvalidArgument, "unsupported --presentation input %q: use an xml_presentation_id, a /slides/ URL, or a /wiki/ URL", raw).WithParam("--presentation")
}
// Non-URL input must be a bare token — anything with path/query/fragment
// chars is rejected so partial-path inputs like `tmp/wiki/wikcn123` don't
// get silently accepted.
if strings.ContainsAny(raw, "/?#") {
return presentationRef{}, output.ErrValidation("unsupported --presentation input %q: use an xml_presentation_id, a /slides/ URL, or a /wiki/ URL", raw)
return presentationRef{}, errs.NewValidationError(errs.SubtypeInvalidArgument, "unsupported --presentation input %q: use an xml_presentation_id, a /slides/ URL, or a /wiki/ URL", raw).WithParam("--presentation")
}
return presentationRef{Kind: "slides", Token: raw}, nil
}
Expand Down Expand Up @@ -82,7 +82,7 @@
case "slides":
return ref.Token, nil
case "wiki":
data, err := runtime.CallAPI(
data, err := runtime.CallAPITyped(
"GET",
"/open-apis/wiki/v2/spaces/get_node",
map[string]interface{}{"token": ref.Token},
Expand All @@ -95,14 +95,18 @@
objType := common.GetString(node, "obj_type")
objToken := common.GetString(node, "obj_token")
if objType == "" || objToken == "" {
return "", output.Errorf(output.ExitAPI, "api_error", "wiki get_node returned incomplete node data")
return "", errs.NewInternalError(errs.SubtypeInvalidResponse, "wiki get_node returned incomplete node data")

Check warning on line 98 in shortcuts/slides/helpers.go

View check run for this annotation

Codecov / codecov/patch

shortcuts/slides/helpers.go#L98

Added line #L98 was not covered by tests
}
if objType != "slides" {
return "", output.ErrValidation("wiki resolved to %q, but slides shortcuts require a slides presentation", objType)
return "", errs.NewValidationError(errs.SubtypeInvalidArgument, "wiki resolved to %q, but slides shortcuts require a slides presentation", objType).WithParam("--presentation")
}
return objToken, nil
default:
return "", output.ErrValidation("unsupported presentation ref kind %q", ref.Kind)
// Unreachable: ref.Kind is set only by parsePresentationRef, which
// emits exclusively "slides" or "wiki". A hit here means an internal
// invariant broke (e.g. a new kind added without updating this switch),
// not bad user input — classify as internal, not validation.
return "", errs.NewInternalError(errs.SubtypeUnknown, "unsupported presentation ref kind %q", ref.Kind)

Check warning on line 109 in shortcuts/slides/helpers.go

View check run for this annotation

Codecov / codecov/patch

shortcuts/slides/helpers.go#L109

Added line #L109 was not covered by tests
}
}

Expand Down Expand Up @@ -191,7 +195,7 @@
func ensureXMLRootID(xmlFragment, want string) (string, error) {
m := xmlRootOpenTagRegex.FindStringSubmatchIndex(xmlFragment)
if m == nil {
return "", fmt.Errorf("no root element found in XML fragment")
return "", errs.NewValidationError(errs.SubtypeInvalidArgument, "no root element found in XML fragment")
}
prefix := xmlFragment[m[2]:m[3]]
tagName := xmlFragment[m[4]:m[5]]
Expand Down
34 changes: 15 additions & 19 deletions shortcuts/slides/slides_create.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
"path/filepath"
"strings"

"github.com/larksuite/cli/internal/output"
"github.com/larksuite/cli/errs"
"github.com/larksuite/cli/internal/validate"
"github.com/larksuite/cli/shortcuts/common"
)
Expand Down Expand Up @@ -45,24 +45,24 @@
if slidesStr := runtime.Str("slides"); slidesStr != "" {
var slides []string
if err := json.Unmarshal([]byte(slidesStr), &slides); err != nil {
return common.FlagErrorf("--slides invalid JSON, must be an array of XML strings")
return errs.NewValidationError(errs.SubtypeInvalidArgument, "--slides invalid JSON, must be an array of XML strings").WithParam("--slides")
}
if len(slides) > maxSlidesPerCreate {
return common.FlagErrorf("--slides array exceeds maximum of %d slides; create the presentation first, then add slides via xml_presentation.slide.create", maxSlidesPerCreate)
return errs.NewValidationError(errs.SubtypeInvalidArgument, "--slides array exceeds maximum of %d slides; create the presentation first, then add slides via xml_presentation.slide.create", maxSlidesPerCreate).WithParam("--slides")
}
// Validate placeholder paths up front so we don't create a presentation
// only to fail mid-way on a missing local file.
for _, path := range extractImagePlaceholderPaths(slides) {
stat, err := runtime.FileIO().Stat(path)
if err != nil {
return common.WrapInputStatError(err, fmt.Sprintf("--slides @%s: file not found", path))
return slidesInputStatError(err, "--slides", fmt.Sprintf("--slides @%s: file not found", path))
}
if !stat.Mode().IsRegular() {
return common.FlagErrorf("--slides @%s: must be a regular file", path)
return errs.NewValidationError(errs.SubtypeInvalidArgument, "--slides @%s: must be a regular file", path).WithParam("--slides")

Check warning on line 61 in shortcuts/slides/slides_create.go

View check run for this annotation

Codecov / codecov/patch

shortcuts/slides/slides_create.go#L61

Added line #L61 was not covered by tests
}
if stat.Size() > common.MaxDriveMediaUploadSinglePartSize {
return common.FlagErrorf("--slides @%s: file size %s exceeds 20 MB limit for slides image upload",
path, common.FormatSize(stat.Size()))
return errs.NewValidationError(errs.SubtypeInvalidArgument, "--slides @%s: file size %s exceeds 20 MB limit for slides image upload",
path, common.FormatSize(stat.Size())).WithParam("--slides")

Check warning on line 65 in shortcuts/slides/slides_create.go

View check run for this annotation

Codecov / codecov/patch

shortcuts/slides/slides_create.go#L64-L65

Added lines #L64 - L65 were not covered by tests
}
}
}
Expand Down Expand Up @@ -128,7 +128,7 @@
slidesStr := runtime.Str("slides")

// Step 1: Create presentation
data, err := runtime.CallAPI(
data, err := runtime.CallAPITyped(
"POST",
"/open-apis/slides_ai/v1/xml_presentations",
nil,
Expand All @@ -144,7 +144,7 @@

presentationID := common.GetString(data, "xml_presentation_id")
if presentationID == "" {
return output.Errorf(output.ExitAPI, "api_error", "slides create returned no xml_presentation_id")
return errs.NewInternalError(errs.SubtypeInvalidResponse, "slides create returned no xml_presentation_id")
}

result := map[string]interface{}{
Expand All @@ -168,9 +168,7 @@
if len(placeholders) > 0 {
tokens, uploaded, err := uploadSlidesPlaceholders(runtime, presentationID, placeholders)
if err != nil {
return output.Errorf(output.ExitAPI, "api_error",
"image upload failed: %v (presentation %s was created; %d image(s) uploaded before failure)",
err, presentationID, uploaded)
return appendSlidesProgressHint(err, fmt.Sprintf("presentation %s was created; %d image(s) uploaded before failure", presentationID, uploaded))

Check warning on line 171 in shortcuts/slides/slides_create.go

View check run for this annotation

Codecov / codecov/patch

shortcuts/slides/slides_create.go#L171

Added line #L171 was not covered by tests
}
for i := range slides {
slides[i] = replaceImagePlaceholders(slides[i], tokens)
Expand All @@ -185,7 +183,7 @@

var slideIDs []string
for i, slideXML := range slides {
slideData, err := runtime.CallAPI(
slideData, err := runtime.CallAPITyped(
"POST",
slideURL,
map[string]interface{}{"revision_id": -1},
Expand All @@ -194,9 +192,7 @@
},
)
if err != nil {
return output.Errorf(output.ExitAPI, "api_error",
"slide %d/%d failed: %v (presentation %s was created; %d slide(s) added before failure)",
i+1, len(slides), err, presentationID, i)
return appendSlidesProgressHint(err, fmt.Sprintf("adding slide %d/%d failed; presentation %s was created, %d slide(s) added before failure", i+1, len(slides), presentationID, i))
}
if sid := common.GetString(slideData, "slide_id"); sid != "" {
slideIDs = append(slideIDs, sid)
Expand Down Expand Up @@ -256,18 +252,18 @@
for i, path := range paths {
stat, err := runtime.FileIO().Stat(path)
if err != nil {
return tokens, i, common.WrapInputStatError(err, fmt.Sprintf("@%s: file not found", path))
return tokens, i, slidesInputStatError(err, "--slides", fmt.Sprintf("@%s: file not found", path))

Check warning on line 255 in shortcuts/slides/slides_create.go

View check run for this annotation

Codecov / codecov/patch

shortcuts/slides/slides_create.go#L255

Added line #L255 was not covered by tests
}
if !stat.Mode().IsRegular() {
return tokens, i, output.ErrValidation("@%s: must be a regular file", path)
return tokens, i, errs.NewValidationError(errs.SubtypeInvalidArgument, "@%s: must be a regular file", path).WithParam("--slides")

Check warning on line 258 in shortcuts/slides/slides_create.go

View check run for this annotation

Codecov / codecov/patch

shortcuts/slides/slides_create.go#L258

Added line #L258 was not covered by tests
}
fileName := filepath.Base(path)
fmt.Fprintf(runtime.IO().ErrOut, "Uploading image %d/%d: %s (%s)\n",
i+1, len(paths), fileName, common.FormatSize(stat.Size()))

token, err := uploadSlidesMedia(runtime, path, fileName, stat.Size(), presentationID)
if err != nil {
return tokens, i, fmt.Errorf("@%s: %w", path, err)
return tokens, i, fmt.Errorf("@%s: %w", path, err) //nolint:forbidigo // intermediate; preserves typed cause via %w, reclassified by appendSlidesProgressHint at the call site

Check warning on line 266 in shortcuts/slides/slides_create.go

View check run for this annotation

Codecov / codecov/patch

shortcuts/slides/slides_create.go#L266

Added line #L266 was not covered by tests
}
tokens[path] = token
}
Expand Down
87 changes: 80 additions & 7 deletions shortcuts/slides/slides_create_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,14 @@ package slides
import (
"bytes"
"encoding/json"
"errors"
"os"
"strings"
"testing"

"github.com/spf13/cobra"

"github.com/larksuite/cli/errs"
"github.com/larksuite/cli/internal/cmdutil"
"github.com/larksuite/cli/internal/core"
"github.com/larksuite/cli/internal/httpmock"
Expand Down Expand Up @@ -400,15 +402,21 @@ func TestSlidesCreateWithSlidesPartialFailure(t *testing.T) {
if err == nil {
t.Fatal("expected error for partial failure, got nil")
}
errMsg := err.Error()
if !strings.Contains(errMsg, "pres_partial") {
t.Fatalf("error should contain presentation ID, got: %s", errMsg)
p, ok := errs.ProblemOf(err)
if !ok {
t.Fatalf("expected a typed errs.* error, got %v", err)
}
if !strings.Contains(errMsg, "slide 2/2") {
t.Fatalf("error should indicate slide 2/2 failed, got: %s", errMsg)
// The presentation was created but a slide add failed; the recovery hint
// carries the partial-progress context (which presentation exists, how many
// slides landed) so the caller can resume without recreating.
if !strings.Contains(p.Hint, "pres_partial") {
t.Fatalf("hint should contain presentation ID, got: %s", p.Hint)
}
if !strings.Contains(errMsg, "1 slide(s) added") {
t.Fatalf("error should report 1 slide added before failure, got: %s", errMsg)
if !strings.Contains(p.Hint, "slide 2/2") {
t.Fatalf("hint should indicate slide 2/2 failed, got: %s", p.Hint)
}
if !strings.Contains(p.Hint, "1 slide(s) added") {
t.Fatalf("hint should report 1 slide added before failure, got: %s", p.Hint)
}
}

Expand Down Expand Up @@ -457,6 +465,71 @@ func TestSlidesCreateWithSlidesExceedsMax(t *testing.T) {
}
}

// TestSlidesCreateValidationParam locks Param=="--slides" on the pure
// validation rejections, so callers route on the typed field rather than the
// message.
func TestSlidesCreateValidationParam(t *testing.T) {
t.Parallel()

elems := make([]string, 11)
for i := range elems {
elems[i] = `"<slide/>"`
}
exceedsMax := "[" + strings.Join(elems, ",") + "]"

tests := []struct {
name string
slides string
}{
{"invalid JSON", "not json"},
{"exceeds max", exceedsMax},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
f, stdout, _, _ := cmdutil.TestFactory(t, slidesTestConfig(t, ""))
err := runSlidesCreateShortcut(t, f, stdout, []string{
"+create",
"--slides", tt.slides,
"--as", "user",
})

var ve *errs.ValidationError
if !errors.As(err, &ve) {
t.Fatalf("err = %v, want *errs.ValidationError", err)
}
if ve.Param != "--slides" {
t.Fatalf("Param = %q, want --slides", ve.Param)
}
})
}
}

// TestSlidesCreatePlaceholderMissingParam guards the create.go caller wiring:
// a missing @-placeholder file must surface a --slides-tagged validation error
// through the shared slidesInputStatError helper.
func TestSlidesCreatePlaceholderMissingParam(t *testing.T) {
dir := t.TempDir()
withSlidesTestWorkingDir(t, dir)

f, stdout, _, _ := cmdutil.TestFactory(t, slidesTestConfig(t, ""))
slidesJSON := `["<slide><data><img src=\"@./missing.png\"/></data></slide>"]`
err := runSlidesCreateShortcut(t, f, stdout, []string{
"+create",
"--slides", slidesJSON,
"--as", "user",
})

var ve *errs.ValidationError
if !errors.As(err, &ve) {
t.Fatalf("err = %v, want *errs.ValidationError", err)
}
if ve.Param != "--slides" {
t.Fatalf("Param = %q, want --slides", ve.Param)
}
}

// TestSlidesCreateWithSlidesEmptyArray verifies that --slides '[]' behaves like no --slides.
func TestSlidesCreateWithSlidesEmptyArray(t *testing.T) {
t.Parallel()
Expand Down
Loading
Loading