From e6f98c1bb5f8ddcf1f1cdfc76672e1844abb6819 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Wed, 17 Jun 2026 09:30:01 +0100 Subject: [PATCH] feat(slides): support public image URLs --- CHANGELOG.md | 4 + docs/commands.generated.md | 4 +- docs/commands/README.md | 4 +- docs/commands/gog-slides-insert-image.md | 7 +- docs/commands/gog-slides-replace-slide.md | 5 +- docs/commands/gog-slides.md | 4 +- internal/cmd/slides.go | 4 +- internal/cmd/slides_insert_image.go | 116 ++++++++-------- internal/cmd/slides_insert_image_test.go | 160 ++++++++++++++++++++++ internal/cmd/slides_replace_slide.go | 97 +++++++------ internal/cmd/slides_replace_slide_test.go | 95 +++++++++++++ internal/cmd/slides_shared.go | 39 ++++++ 12 files changed, 415 insertions(+), 124 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6ec2b8b69..ce3cd7ab6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## 0.28.1 - Unreleased +### Added + +- Slides: allow `insert-image` and `replace-slide` to use public HTTPS image URLs without temporary Drive sharing. (#825) — thanks @sebsnyk. + ### Fixed - Docs: update the Docker authentication example to persist file-keyring tokens with `GOG_HOME`. (#828, #830) — thanks @WadydX. diff --git a/docs/commands.generated.md b/docs/commands.generated.md index b6a8effec..d18a52725 100644 --- a/docs/commands.generated.md +++ b/docs/commands.generated.md @@ -593,12 +593,12 @@ Generated from `gog schema --json`. - [`gog slides (slide) delete-slide `](commands/gog-slides-delete-slide.md) - Delete a slide by object ID - [`gog slides (slide) export (download,dl) [flags]`](commands/gog-slides-export.md) - Export a Google Slides deck (pdf|pptx) - [`gog slides (slide) info (get,show) `](commands/gog-slides-info.md) - Get Google Slides presentation metadata - - [`gog slides (slide) insert-image --width=FLOAT-64 [flags]`](commands/gog-slides-insert-image.md) - Insert an image at a position and size on an existing slide + - [`gog slides (slide) insert-image --width=FLOAT-64 [] [flags]`](commands/gog-slides-insert-image.md) - Insert a local or public image at a position and size - [`gog slides (slide) insert-text [flags]`](commands/gog-slides-insert-text.md) - Insert text into an existing page element (shape or table) by objectId - [`gog slides (slide) list-slides `](commands/gog-slides-list-slides.md) - List all slides with their object IDs - [`gog slides (slide) raw [flags]`](commands/gog-slides-raw.md) - Dump raw Google Slides API response as JSON (Presentations.Get; lossless; for scripting and LLM consumption) - [`gog slides (slide) read-slide `](commands/gog-slides-read-slide.md) - Read slide content: speaker notes, text elements, and images - - [`gog slides (slide) replace-slide [flags]`](commands/gog-slides-replace-slide.md) - Replace the image on an existing slide in-place + - [`gog slides (slide) replace-slide [] [flags]`](commands/gog-slides-replace-slide.md) - Replace an existing slide image from a local file or public URL - [`gog slides (slide) replace-text [flags]`](commands/gog-slides-replace-text.md) - Find-and-replace text across a presentation - [`gog slides (slide) thumbnail (thumb) [flags]`](commands/gog-slides-thumbnail.md) - Get or download a rendered thumbnail for a slide - [`gog slides (slide) update-notes [flags]`](commands/gog-slides-update-notes.md) - Update speaker notes on an existing slide diff --git a/docs/commands/README.md b/docs/commands/README.md index 4793cf404..ff0a078c5 100644 --- a/docs/commands/README.md +++ b/docs/commands/README.md @@ -644,12 +644,12 @@ Generated pages: 646. - [gog slides delete-slide](gog-slides-delete-slide.md) - Delete a slide by object ID - [gog slides export](gog-slides-export.md) - Export a Google Slides deck (pdf|pptx) - [gog slides info](gog-slides-info.md) - Get Google Slides presentation metadata - - [gog slides insert-image](gog-slides-insert-image.md) - Insert an image at a position and size on an existing slide + - [gog slides insert-image](gog-slides-insert-image.md) - Insert a local or public image at a position and size - [gog slides insert-text](gog-slides-insert-text.md) - Insert text into an existing page element (shape or table) by objectId - [gog slides list-slides](gog-slides-list-slides.md) - List all slides with their object IDs - [gog slides raw](gog-slides-raw.md) - Dump raw Google Slides API response as JSON (Presentations.Get; lossless; for scripting and LLM consumption) - [gog slides read-slide](gog-slides-read-slide.md) - Read slide content: speaker notes, text elements, and images - - [gog slides replace-slide](gog-slides-replace-slide.md) - Replace the image on an existing slide in-place + - [gog slides replace-slide](gog-slides-replace-slide.md) - Replace an existing slide image from a local file or public URL - [gog slides replace-text](gog-slides-replace-text.md) - Find-and-replace text across a presentation - [gog slides thumbnail](gog-slides-thumbnail.md) - Get or download a rendered thumbnail for a slide - [gog slides update-notes](gog-slides-update-notes.md) - Update speaker notes on an existing slide diff --git a/docs/commands/gog-slides-insert-image.md b/docs/commands/gog-slides-insert-image.md index ae876b9ab..c08ec1959 100644 --- a/docs/commands/gog-slides-insert-image.md +++ b/docs/commands/gog-slides-insert-image.md @@ -2,12 +2,12 @@ > Generated from `gog schema --json`. Do not edit this page by hand; run `make docs-commands`. -Insert an image at a position and size on an existing slide +Insert a local or public image at a position and size ## Usage ```bash -gog slides (slide) insert-image --width=FLOAT-64 [flags] +gog slides (slide) insert-image --width=FLOAT-64 [] [flags] ``` ## Parent @@ -28,7 +28,7 @@ gog slides (slide) insert-image --width=FLOAT-64 `--force`
`--assume-yes`
`--yes` | `bool` | | Skip confirmations for destructive commands | | `--gmail-no-send` | `bool` | false | Block Gmail send operations (agent safety) | -| `--height` | `float64` | 0 | Image height, in --unit; omit to keep the image's aspect ratio | +| `--height` | `float64` | 0 | Image height, in --unit; required with --url, local files preserve aspect ratio when omitted | | `-h`
`--help` | `kong.helpFlag` | | Show context-sensitive help. | | `--home` | `string` | | Override gogcli config/data/state/cache root (equivalent to GOG_HOME) | | `-j`
`--json`
`--machine` | `bool` | false | Output JSON to stdout (best for scripting) | @@ -37,6 +37,7 @@ gog slides (slide) insert-image --width=FLOAT-64 `--pick`
`--project` | `string` | | In JSON mode, select comma-separated fields (best-effort; supports dot paths). Desire path: use --fields for most commands. | | `--unit` | `string` | PT | Measurement unit for x/y/width/height (PT or EMU) | +| `--url` | `string` | | Public HTTPS image URL to insert directly | | `-v`
`--verbose` | `bool` | | Enable verbose logging | | `--version` | `kong.VersionFlag` | | Print version and exit | | `--width` | `float64` | | Image width, in --unit | diff --git a/docs/commands/gog-slides-replace-slide.md b/docs/commands/gog-slides-replace-slide.md index abe4d909d..43b6fcad8 100644 --- a/docs/commands/gog-slides-replace-slide.md +++ b/docs/commands/gog-slides-replace-slide.md @@ -2,12 +2,12 @@ > Generated from `gog schema --json`. Do not edit this page by hand; run `make docs-commands`. -Replace the image on an existing slide in-place +Replace an existing slide image from a local file or public URL ## Usage ```bash -gog slides (slide) replace-slide [flags] +gog slides (slide) replace-slide [] [flags] ``` ## Parent @@ -37,6 +37,7 @@ gog slides (slide) replace-slide [flags] | `-p`
`--plain`
`--tsv` | `bool` | false | Output stable, parseable text to stdout (TSV; no colors) | | `--results-only` | `bool` | | In JSON mode, emit only the primary result (drops envelope fields like nextPageToken) | | `--select`
`--pick`
`--project` | `string` | | In JSON mode, select comma-separated fields (best-effort; supports dot paths). Desire path: use --fields for most commands. | +| `--url` | `string` | | Public HTTPS image URL to use directly | | `-v`
`--verbose` | `bool` | | Enable verbose logging | | `--version` | `kong.VersionFlag` | | Print version and exit | | `--wrap-untrusted` | `bool` | false | In JSON/raw output, wrap fetched text fields in external untrusted-content markers | diff --git a/docs/commands/gog-slides.md b/docs/commands/gog-slides.md index 50dea63bf..df72f94dd 100644 --- a/docs/commands/gog-slides.md +++ b/docs/commands/gog-slides.md @@ -24,12 +24,12 @@ gog slides (slide) [flags] - [gog slides delete-slide](gog-slides-delete-slide.md) - Delete a slide by object ID - [gog slides export](gog-slides-export.md) - Export a Google Slides deck (pdf|pptx) - [gog slides info](gog-slides-info.md) - Get Google Slides presentation metadata -- [gog slides insert-image](gog-slides-insert-image.md) - Insert an image at a position and size on an existing slide +- [gog slides insert-image](gog-slides-insert-image.md) - Insert a local or public image at a position and size - [gog slides insert-text](gog-slides-insert-text.md) - Insert text into an existing page element (shape or table) by objectId - [gog slides list-slides](gog-slides-list-slides.md) - List all slides with their object IDs - [gog slides raw](gog-slides-raw.md) - Dump raw Google Slides API response as JSON (Presentations.Get; lossless; for scripting and LLM consumption) - [gog slides read-slide](gog-slides-read-slide.md) - Read slide content: speaker notes, text elements, and images -- [gog slides replace-slide](gog-slides-replace-slide.md) - Replace the image on an existing slide in-place +- [gog slides replace-slide](gog-slides-replace-slide.md) - Replace an existing slide image from a local file or public URL - [gog slides replace-text](gog-slides-replace-text.md) - Find-and-replace text across a presentation - [gog slides thumbnail](gog-slides-thumbnail.md) - Get or download a rendered thumbnail for a slide - [gog slides update-notes](gog-slides-update-notes.md) - Update speaker notes on an existing slide diff --git a/internal/cmd/slides.go b/internal/cmd/slides.go index a4fdb94ae..2965f5ad9 100644 --- a/internal/cmd/slides.go +++ b/internal/cmd/slides.go @@ -27,8 +27,8 @@ type SlidesCmd struct { ReadSlide SlidesReadSlideCmd `cmd:"" name:"read-slide" help:"Read slide content: speaker notes, text elements, and images"` Thumbnail SlidesThumbnailCmd `cmd:"" name:"thumbnail" aliases:"thumb" help:"Get or download a rendered thumbnail for a slide"` UpdateNotes SlidesUpdateNotesCmd `cmd:"" name:"update-notes" help:"Update speaker notes on an existing slide"` - ReplaceSlide SlidesReplaceSlideCmd `cmd:"" name:"replace-slide" help:"Replace the image on an existing slide in-place"` - InsertImage SlidesInsertImageCmd `cmd:"" name:"insert-image" help:"Insert an image at a position and size on an existing slide"` + ReplaceSlide SlidesReplaceSlideCmd `cmd:"" name:"replace-slide" help:"Replace an existing slide image from a local file or public URL"` + InsertImage SlidesInsertImageCmd `cmd:"" name:"insert-image" help:"Insert a local or public image at a position and size"` InsertText SlidesInsertTextCmd `cmd:"" name:"insert-text" help:"Insert text into an existing page element (shape or table) by objectId"` ReplaceText SlidesReplaceTextCmd `cmd:"" name:"replace-text" help:"Find-and-replace text across a presentation"` Raw SlidesRawCmd `cmd:"" name:"raw" help:"Dump raw Google Slides API response as JSON (Presentations.Get; lossless; for scripting and LLM consumption)"` diff --git a/internal/cmd/slides_insert_image.go b/internal/cmd/slides_insert_image.go index d7ca2f462..86b66a3d1 100644 --- a/internal/cmd/slides_insert_image.go +++ b/internal/cmd/slides_insert_image.go @@ -23,17 +23,17 @@ import ( // existing slide. Unlike add-slide (which lays a full-bleed image on a new // slide), this places a sized element on a slide you already have, so callers // can build native decks via the Slides API and still drop in a logo, chart, -// or badge at a precise location. It reuses the same private-image flow as -// add-slide: upload to Drive, grant a temporary read permission so the Slides -// image fetcher can read it, create the image, then delete the temp file. +// or badge at a precise location. Local files use the same temporary Drive +// upload flow as add-slide; public HTTPS URLs are passed directly to Slides. type SlidesInsertImageCmd struct { PresentationID string `arg:"" name:"presentationId" help:"Presentation ID"` SlideID string `arg:"" name:"slideId" help:"Slide object ID to place the image on"` - Image string `arg:"" name:"image" help:"Local image file (PNG/JPG/GIF)" type:"existingfile"` + Image string `arg:"" optional:"" name:"image" help:"Local image file (PNG/JPG/GIF)" type:"existingfile"` + URL string `name:"url" help:"Public HTTPS image URL to insert directly"` X float64 `name:"x" default:"0" help:"Left position of the image, in --unit"` Y float64 `name:"y" default:"0" help:"Top position of the image, in --unit"` Width float64 `name:"width" required:"" help:"Image width, in --unit"` - Height float64 `name:"height" default:"0" help:"Image height, in --unit; omit to keep the image's aspect ratio"` + Height float64 `name:"height" default:"0" help:"Image height, in --unit; required with --url, local files preserve aspect ratio when omitted"` Unit string `name:"unit" enum:"PT,EMU" default:"PT" help:"Measurement unit for x/y/width/height (PT or EMU)"` } @@ -55,41 +55,40 @@ func (c *SlidesInsertImageCmd) Run(ctx context.Context, flags *RootFlags) error return usage("--height cannot be negative") } - // Validate image format. - ext := strings.ToLower(filepath.Ext(c.Image)) - var mimeType string - switch ext { - case extPNG: - mimeType = mimePNG - case imageExtJPG, imageExtJPEG: - mimeType = imageMimeJPEG - case imageExtGIF: - mimeType = imageMimeGIF - default: - return usagef("unsupported image format %q (use PNG, JPG, or GIF)", ext) + source, err := resolveSlidesImageSource(c.Image, c.URL) + if err != nil { + return err } // Resolve height from the image's aspect ratio when not supplied. height := c.Height if height == 0 { - ar, err := imageAspectRatio(c.Image) - if err != nil { - return fmt.Errorf("determine image aspect ratio (pass --height to skip): %w", err) + if source.imageURL != "" { + return usage("--height is required with --url") + } + ar, aspectErr := imageAspectRatio(source.localPath) + if aspectErr != nil { + return fmt.Errorf("determine image aspect ratio (pass --height to skip): %w", aspectErr) } height = c.Width * ar } - if dryRunErr := dryRunExit(ctx, flags, "slides.insert-image", map[string]any{ + dryRunPayload := map[string]any{ "presentation_id": presentationID, "slide_id": slideID, - "image": c.Image, - "mime_type": mimeType, "x": c.X, "y": c.Y, "width": c.Width, "height": height, "unit": c.Unit, - }); dryRunErr != nil { + } + if source.imageURL != "" { + dryRunPayload["url"] = source.imageURL + } else { + dryRunPayload["image"] = source.localPath + dryRunPayload["mime_type"] = source.mimeType + } + if dryRunErr := dryRunExit(ctx, flags, "slides.insert-image", dryRunPayload); dryRunErr != nil { return dryRunErr } @@ -113,46 +112,41 @@ func (c *SlidesInsertImageCmd) Run(ctx context.Context, flags *RootFlags) error return fmt.Errorf("slide %q not found in presentation", slideID) } - driveSvc, err := driveService(ctx, account) - if err != nil { - return err - } - - // Upload the image to Drive as a temporary file. - imgFile, err := os.Open(c.Image) - if err != nil { - return fmt.Errorf("open image: %w", err) - } - defer imgFile.Close() - - driveFile, err := driveSvc.Files.Create(&drive.File{ - Name: filepath.Base(c.Image), - MimeType: mimeType, - }).Media(imgFile).Fields("id, webContentLink").Context(ctx).Do() - if err != nil { - return fmt.Errorf("upload image to Drive: %w", err) - } - - // Clean up the temporary Drive file when done. Use a cancellation-immune - // context so the public temp file is still removed if the request context - // was canceled, and surface a loud warning if deletion fails (otherwise the - // uploaded image stays world-readable until someone removes it by hand). - defer func() { - if delErr := driveSvc.Files.Delete(driveFile.Id).Context(context.WithoutCancel(ctx)).Do(); delErr != nil { - u.Err().Linef("Warning: failed to delete temporary Drive image %s; it may remain publicly readable until removed: %v", driveFile.Id, delErr) + imageURL := source.imageURL + if imageURL == "" { + driveSvc, driveErr := driveService(ctx, account) + if driveErr != nil { + return driveErr } - }() - - // Make publicly readable so the Slides API can fetch it. - _, err = driveSvc.Permissions.Create(driveFile.Id, &drive.Permission{ - Type: "anyone", - Role: "reader", - }).Context(ctx).Do() - if err != nil { - return fmt.Errorf("set image permissions: %w", err) + imgFile, openErr := os.Open(source.localPath) + if openErr != nil { + return fmt.Errorf("open image: %w", openErr) + } + defer imgFile.Close() + + driveFile, uploadErr := driveSvc.Files.Create(&drive.File{ + Name: filepath.Base(source.localPath), + MimeType: source.mimeType, + }).Media(imgFile).Fields("id, webContentLink").Context(ctx).Do() + if uploadErr != nil { + return fmt.Errorf("upload image to Drive: %w", uploadErr) + } + defer func() { + if delErr := driveSvc.Files.Delete(driveFile.Id).Context(context.WithoutCancel(ctx)).Do(); delErr != nil { + u.Err().Linef("Warning: failed to delete temporary Drive image %s; it may remain publicly readable until removed: %v", driveFile.Id, delErr) + } + }() + + _, permissionErr := driveSvc.Permissions.Create(driveFile.Id, &drive.Permission{ + Type: "anyone", + Role: "reader", + }).Context(ctx).Do() + if permissionErr != nil { + return fmt.Errorf("set image permissions: %w", permissionErr) + } + imageURL = driveImageDownloadURL(driveFile.Id) } - imageURL := driveImageDownloadURL(driveFile.Id) imageID := fmt.Sprintf("img_%d", time.Now().UnixNano()) err = batchUpdateSlidesImageRequests(ctx, slidesSvc, presentationID, &slides.BatchUpdatePresentationRequest{ diff --git a/internal/cmd/slides_insert_image_test.go b/internal/cmd/slides_insert_image_test.go index ed9b025a7..c7efd4721 100644 --- a/internal/cmd/slides_insert_image_test.go +++ b/internal/cmd/slides_insert_image_test.go @@ -117,6 +117,166 @@ func TestSlidesInsertImage_PlacesSizedImageOnExistingSlide(t *testing.T) { } } +func TestResolveSlidesImageSource(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + file string + url string + want string + }{ + {name: "missing", want: "required: image argument or --url"}, + {name: "both", file: "image.png", url: "https://example.com/image.png", want: "mutually exclusive"}, + {name: "http", url: "http://example.com/image.png", want: "public HTTPS"}, + {name: "relative", url: "image.png", want: "public HTTPS"}, + {name: "credentials", url: "https://user:pass@example.com/image.png", want: "without embedded credentials"}, + {name: "unsupported file", file: "image.bmp", want: "unsupported image format"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + _, err := resolveSlidesImageSource(tt.file, tt.url) + if err == nil || !strings.Contains(err.Error(), tt.want) { + t.Fatalf("resolveSlidesImageSource() error = %v, want %q", err, tt.want) + } + }) + } + + source, err := resolveSlidesImageSource("", "https://example.com/image.png?sig=abc") + if err != nil { + t.Fatalf("resolve URL source: %v", err) + } + if source.imageURL != "https://example.com/image.png?sig=abc" || source.localPath != "" { + t.Fatalf("unexpected URL source: %#v", source) + } + + source, err = resolveSlidesImageSource(" image.png", "") + if err != nil { + t.Fatalf("resolve local source: %v", err) + } + if source.localPath != " image.png" || source.mimeType != mimePNG { + t.Fatalf("local source path was changed: %#v", source) + } +} + +func TestSlidesInsertImage_URLSkipsDrive(t *testing.T) { + t.Parallel() + + var captured slides.BatchUpdatePresentationRequest + slidesSrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + switch { + case strings.HasSuffix(r.URL.Path, ":batchUpdate") && r.Method == http.MethodPost: + if err := json.NewDecoder(r.Body).Decode(&captured); err != nil { + t.Fatalf("decode batchUpdate: %v", err) + } + _ = json.NewEncoder(w).Encode(map[string]any{"presentationId": "pres1", "replies": []any{map[string]any{}}}) + case strings.Contains(r.URL.Path, "/presentations/pres1") && r.Method == http.MethodGet: + _ = json.NewEncoder(w).Encode(slidesPresGetResponse("", false)) + default: + http.NotFound(w, r) + } + })) + defer slidesSrv.Close() + + slidesSvc, err := slides.NewService(context.Background(), + option.WithoutAuthentication(), option.WithHTTPClient(slidesSrv.Client()), option.WithEndpoint(slidesSrv.URL+"/")) + if err != nil { + t.Fatalf("slides.NewService: %v", err) + } + driveFactory := func(context.Context, string) (*drive.Service, error) { + t.Fatal("URL insertion must not create a Drive service") + return nil, errors.New("unexpected Drive service call") + } + + var stdout, stderr bytes.Buffer + ctx := withSlidesTestService( + withDriveTestServiceFactory(newCmdRuntimeJSONOutputContext(t, &stdout, &stderr), driveFactory), + slidesSvc, + ) + runErr := runKong(t, &SlidesInsertImageCmd{}, []string{ + "pres1", + "existing_slide_1", + "--url", "https://example.com/image.png?sig=abc", + "--width", "120", + "--height", "60", + }, ctx, &RootFlags{Account: "a@b.com"}) + if runErr != nil { + t.Fatalf("slides insert-image --url: %v", runErr) + } + if len(captured.Requests) != 1 || captured.Requests[0].CreateImage == nil { + t.Fatalf("unexpected requests: %#v", captured.Requests) + } + create := captured.Requests[0].CreateImage + if create.Url != "https://example.com/image.png?sig=abc" { + t.Fatalf("CreateImage URL = %q", create.Url) + } + if create.ElementProperties.Size.Width.Magnitude != 120 || create.ElementProperties.Size.Height.Magnitude != 60 { + t.Fatalf("unexpected image size: %#v", create.ElementProperties.Size) + } +} + +func TestSlidesInsertImage_URLRequiresHeight(t *testing.T) { + t.Parallel() + + err := (&SlidesInsertImageCmd{ + PresentationID: "pres1", + SlideID: "slide1", + URL: "https://example.com/image.png", + Width: 120, + }).Run(newCmdRuntimeOutputContext(t, io.Discard, io.Discard), &RootFlags{Account: "a@b.com"}) + if err == nil || !strings.Contains(err.Error(), "--height is required with --url") { + t.Fatalf("expected URL height error, got %v", err) + } + if ExitCode(err) != 2 { + t.Fatalf("expected usage exit code 2, got %d", ExitCode(err)) + } +} + +func TestSlidesInsertImage_URLDryRunSkipsServices(t *testing.T) { + t.Parallel() + + slidesFactory := func(context.Context, string) (*slides.Service, error) { + t.Fatal("dry-run must not create a Slides service") + return nil, errors.New("unexpected Slides service call") + } + driveFactory := func(context.Context, string) (*drive.Service, error) { + t.Fatal("dry-run must not create a Drive service") + return nil, errors.New("unexpected Drive service call") + } + + var stdout, stderr bytes.Buffer + ctx := withSlidesTestServiceFactory( + withDriveTestServiceFactory(newCmdRuntimeJSONOutputContext(t, &stdout, &stderr), driveFactory), + slidesFactory, + ) + err := runKong(t, &SlidesInsertImageCmd{}, []string{ + "pres1", + "slide1", + "--url", "https://example.com/image.png", + "--width", "120", + "--height", "60", + }, ctx, &RootFlags{Account: "a@b.com", DryRun: true, NoInput: true}) + var exitErr *ExitError + if !errors.As(err, &exitErr) || exitErr.Code != 0 { + t.Fatalf("dry-run error = %v", err) + } + var payload struct { + Op string `json:"op"` + Request struct { + URL string `json:"url"` + } `json:"request"` + } + if err := json.Unmarshal(stdout.Bytes(), &payload); err != nil { + t.Fatalf("decode dry-run output: %v\n%s", err, stdout.String()) + } + if payload.Op != "slides.insert-image" || payload.Request.URL != "https://example.com/image.png" { + t.Fatalf("unexpected dry-run output: %#v", payload) + } +} + func TestSlidesInsertImage_RejectsMissingSlide(t *testing.T) { slidesSrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") diff --git a/internal/cmd/slides_replace_slide.go b/internal/cmd/slides_replace_slide.go index f9bda63bb..dbaef4ecb 100644 --- a/internal/cmd/slides_replace_slide.go +++ b/internal/cmd/slides_replace_slide.go @@ -17,7 +17,8 @@ import ( type SlidesReplaceSlideCmd struct { PresentationID string `arg:"" name:"presentationId" help:"Presentation ID"` SlideID string `arg:"" name:"slideId" help:"Slide object ID to replace"` - Image string `arg:"" name:"image" help:"Local image file (PNG/JPG/GIF)" type:"existingfile"` + Image string `arg:"" optional:"" name:"image" help:"Local image file (PNG/JPG/GIF)" type:"existingfile"` + URL string `name:"url" help:"Public HTTPS image URL to use directly"` Notes *string `name:"notes" help:"New speaker notes text (omit to preserve existing notes; use --notes '' to clear)"` NotesFile string `name:"notes-file" help:"Path to file containing new speaker notes" type:"existingfile"` } @@ -39,28 +40,24 @@ func (c *SlidesReplaceSlideCmd) Run(ctx context.Context, flags *RootFlags) error return usage("empty slideId") } - // Validate image format. - ext := strings.ToLower(filepath.Ext(c.Image)) - var mimeType string - switch ext { - case extPNG: - mimeType = mimePNG - case imageExtJPG, imageExtJPEG: - mimeType = imageMimeJPEG - case imageExtGIF: - mimeType = imageMimeGIF - default: - return usagef("unsupported image format %q (use PNG, JPG, or GIF)", ext) + source, err := resolveSlidesImageSource(c.Image, c.URL) + if err != nil { + return err } - if dryRunErr := dryRunExit(ctx, flags, "slides.replace-slide", map[string]any{ + dryRunPayload := map[string]any{ "presentation_id": presentationID, "slide_id": slideID, - "image": c.Image, - "mime_type": mimeType, "update_notes": updateNotes, "notes": updateNotes && notes != "", - }); dryRunErr != nil { + } + if source.imageURL != "" { + dryRunPayload["url"] = source.imageURL + } else { + dryRunPayload["image"] = source.localPath + dryRunPayload["mime_type"] = source.mimeType + } + if dryRunErr := dryRunExit(ctx, flags, "slides.replace-slide", dryRunPayload); dryRunErr != nil { return dryRunErr } @@ -73,10 +70,6 @@ func (c *SlidesReplaceSlideCmd) Run(ctx context.Context, flags *RootFlags) error if err != nil { return err } - driveSvc, err := driveService(ctx, account) - if err != nil { - return err - } // Get presentation to find the slide and its image element. pres, err := slidesSvc.Presentations.Get(presentationID).Context(ctx).Do() @@ -101,37 +94,41 @@ func (c *SlidesReplaceSlideCmd) Run(ctx context.Context, flags *RootFlags) error return fmt.Errorf("no image found on slide %s", slideID) } - // Upload new image to Drive. - imgFile, err := os.Open(c.Image) - if err != nil { - return fmt.Errorf("open image: %w", err) - } - defer imgFile.Close() - - driveFile, err := driveSvc.Files.Create(&drive.File{ - Name: filepath.Base(c.Image), - MimeType: mimeType, - }).Media(imgFile).Fields("id, webContentLink").Context(ctx).Do() - if err != nil { - return fmt.Errorf("upload image to Drive: %w", err) - } - - // Clean up the temporary Drive file when done. - defer func() { - _ = driveSvc.Files.Delete(driveFile.Id).Context(ctx).Do() - }() - - // Make publicly readable so the Slides API can fetch it. - _, err = driveSvc.Permissions.Create(driveFile.Id, &drive.Permission{ - Type: "anyone", - Role: "reader", - }).Context(ctx).Do() - if err != nil { - return fmt.Errorf("set image permissions: %w", err) + imageURL := source.imageURL + if imageURL == "" { + driveSvc, driveErr := driveService(ctx, account) + if driveErr != nil { + return driveErr + } + imgFile, openErr := os.Open(source.localPath) + if openErr != nil { + return fmt.Errorf("open image: %w", openErr) + } + defer imgFile.Close() + + driveFile, uploadErr := driveSvc.Files.Create(&drive.File{ + Name: filepath.Base(source.localPath), + MimeType: source.mimeType, + }).Media(imgFile).Fields("id, webContentLink").Context(ctx).Do() + if uploadErr != nil { + return fmt.Errorf("upload image to Drive: %w", uploadErr) + } + defer func() { + if delErr := driveSvc.Files.Delete(driveFile.Id).Context(context.WithoutCancel(ctx)).Do(); delErr != nil { + u.Err().Linef("Warning: failed to delete temporary Drive image %s; it may remain publicly readable until removed: %v", driveFile.Id, delErr) + } + }() + + _, permissionErr := driveSvc.Permissions.Create(driveFile.Id, &drive.Permission{ + Type: "anyone", + Role: "reader", + }).Context(ctx).Do() + if permissionErr != nil { + return fmt.Errorf("set image permissions: %w", permissionErr) + } + imageURL = driveImageDownloadURL(driveFile.Id) } - imageURL := driveImageDownloadURL(driveFile.Id) - // Replace the image in-place. requests := []*slides.Request{ { diff --git a/internal/cmd/slides_replace_slide_test.go b/internal/cmd/slides_replace_slide_test.go index c057ae6db..cd80d7067 100644 --- a/internal/cmd/slides_replace_slide_test.go +++ b/internal/cmd/slides_replace_slide_test.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "encoding/json" + "errors" "io" "net/http" "net/http/httptest" @@ -159,6 +160,100 @@ func TestSlidesReplaceSlide(t *testing.T) { } } +func TestSlidesReplaceSlide_URLSkipsDrive(t *testing.T) { + t.Parallel() + + var capturedRequests []*slides.Request + slidesSrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + switch { + case strings.HasSuffix(r.URL.Path, ":batchUpdate") && r.Method == http.MethodPost: + var req slides.BatchUpdatePresentationRequest + if err := json.NewDecoder(r.Body).Decode(&req); err != nil { + t.Fatalf("decode batchUpdate: %v", err) + } + capturedRequests = req.Requests + _ = json.NewEncoder(w).Encode(map[string]any{"presentationId": "pres1", "replies": []any{}}) + case strings.Contains(r.URL.Path, "/presentations/pres1") && r.Method == http.MethodGet: + _ = json.NewEncoder(w).Encode(replaceSlidePresResponse()) + default: + http.NotFound(w, r) + } + })) + defer slidesSrv.Close() + + slidesSvc, err := slides.NewService(context.Background(), + option.WithoutAuthentication(), option.WithHTTPClient(slidesSrv.Client()), option.WithEndpoint(slidesSrv.URL+"/")) + if err != nil { + t.Fatalf("slides.NewService: %v", err) + } + driveFactory := func(context.Context, string) (*drive.Service, error) { + t.Fatal("URL replacement must not create a Drive service") + return nil, errors.New("unexpected Drive service call") + } + + var stdout, stderr bytes.Buffer + ctx := withSlidesTestService( + withDriveTestServiceFactory(newCmdRuntimeJSONOutputContext(t, &stdout, &stderr), driveFactory), + slidesSvc, + ) + runErr := runKong(t, &SlidesReplaceSlideCmd{}, []string{ + "pres1", + "slide_1", + "--url", "https://example.com/replacement.png?sig=abc", + }, ctx, &RootFlags{Account: "a@b.com"}) + if runErr != nil { + t.Fatalf("slides replace-slide --url: %v", runErr) + } + if len(capturedRequests) != 1 || capturedRequests[0].ReplaceImage == nil { + t.Fatalf("unexpected requests: %#v", capturedRequests) + } + replace := capturedRequests[0].ReplaceImage + if replace.ImageObjectId != "img_on_slide" || replace.Url != "https://example.com/replacement.png?sig=abc" { + t.Fatalf("unexpected ReplaceImage request: %#v", replace) + } +} + +func TestSlidesReplaceSlide_URLDryRunSkipsServices(t *testing.T) { + t.Parallel() + + slidesFactory := func(context.Context, string) (*slides.Service, error) { + t.Fatal("dry-run must not create a Slides service") + return nil, errors.New("unexpected Slides service call") + } + driveFactory := func(context.Context, string) (*drive.Service, error) { + t.Fatal("dry-run must not create a Drive service") + return nil, errors.New("unexpected Drive service call") + } + + var stdout, stderr bytes.Buffer + ctx := withSlidesTestServiceFactory( + withDriveTestServiceFactory(newCmdRuntimeJSONOutputContext(t, &stdout, &stderr), driveFactory), + slidesFactory, + ) + err := runKong(t, &SlidesReplaceSlideCmd{}, []string{ + "pres1", + "slide_1", + "--url", "https://example.com/replacement.png", + }, ctx, &RootFlags{Account: "a@b.com", DryRun: true, NoInput: true}) + var exitErr *ExitError + if !errors.As(err, &exitErr) || exitErr.Code != 0 { + t.Fatalf("dry-run error = %v", err) + } + var payload struct { + Op string `json:"op"` + Request struct { + URL string `json:"url"` + } `json:"request"` + } + if err := json.Unmarshal(stdout.Bytes(), &payload); err != nil { + t.Fatalf("decode dry-run output: %v\n%s", err, stdout.String()) + } + if payload.Op != "slides.replace-slide" || payload.Request.URL != "https://example.com/replacement.png" { + t.Fatalf("unexpected dry-run output: %#v", payload) + } +} + func TestSlidesReplaceSlide_WithNotes(t *testing.T) { var capturedRequests []*slides.Request diff --git a/internal/cmd/slides_shared.go b/internal/cmd/slides_shared.go index bc01ed619..0c98c54e3 100644 --- a/internal/cmd/slides_shared.go +++ b/internal/cmd/slides_shared.go @@ -4,7 +4,9 @@ import ( "context" "errors" "fmt" + "net/url" "os" + "path/filepath" "strings" gapi "google.golang.org/api/googleapi" @@ -21,6 +23,43 @@ const ( placeholderTypeBody = "BODY" ) +type slidesImageSource struct { + localPath string + mimeType string + imageURL string +} + +func resolveSlidesImageSource(localImage, imageURL string) (slidesImageSource, error) { + imageURL = strings.TrimSpace(imageURL) + if localImage == "" && imageURL == "" { + return slidesImageSource{}, usage("required: image argument or --url") + } + if localImage != "" && imageURL != "" { + return slidesImageSource{}, usage("image argument and --url are mutually exclusive") + } + if imageURL != "" { + parsed, err := url.ParseRequestURI(imageURL) + if err != nil || !strings.EqualFold(parsed.Scheme, "https") || parsed.Host == "" || parsed.User != nil { + return slidesImageSource{}, usage("--url must be a public HTTPS image URL without embedded credentials") + } + return slidesImageSource{imageURL: parsed.String()}, nil + } + + ext := strings.ToLower(filepath.Ext(localImage)) + var mimeType string + switch ext { + case extPNG: + mimeType = mimePNG + case imageExtJPG, imageExtJPEG: + mimeType = imageMimeJPEG + case imageExtGIF: + mimeType = imageMimeGIF + default: + return slidesImageSource{}, usagef("unsupported image format %q (use PNG, JPG, or GIF)", ext) + } + return slidesImageSource{localPath: localImage, mimeType: mimeType}, nil +} + func resolveSlidesNotesInput(notes *string, notesFile string) (string, bool, error) { if notesFile != "" { // The caller's Kong field uses type:"existingfile"; this helper only