From ee8ad6e7fe632e0f32bdba2f7dd11d373c88c88a Mon Sep 17 00:00:00 2001 From: Sterling Hanenkamp Date: Wed, 5 Aug 2026 12:19:31 -0500 Subject: [PATCH 1/2] Carry the hotlink and download-location URLs on Descriptor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Unsplash's API guidelines require consumers to display photos from the URLs the API returns under "urls" rather than from a copy they host themselves, so that views are attributed to the photographer, and to call the download endpoint when a photo is actually used. Consumers could do neither: Source.Photo had both values in hand and discarded them, keeping only an unexported image with a Filename and a Reader. Descriptor gains ImageURL and DownloadLocation, filled in from the photo already fetched, so no extra request is made. ImageURL prefers the raw variant because it carries no size preset, leaving the caller free to append its own Imgix parameters, and falls back to full then regular when a response omits it. The two are deliberately separate fields. Displaying a photo is not a download, and a consumer wanting to follow the guidelines has to trigger them at different moments — the hotlink on every render, the download once when the photo is chosen. Both are omitted when empty, so photo metadata written by earlier versions still loads unchanged. Co-Authored-By: Claude Opus 5 --- Changes.md | 4 ++ cmd/version.txt | 2 +- pkg/photo/photo.go | 12 ++++ pkg/photo/unsplash/photo.go | 21 +++++++ pkg/photo/unsplash/photo_test.go | 100 ++++++++++++++++++++++++++++++- 5 files changed, 136 insertions(+), 3 deletions(-) diff --git a/Changes.md b/Changes.md index 7dd0fc0..126e1b7 100644 --- a/Changes.md +++ b/Changes.md @@ -1,3 +1,7 @@ +## 1.1.0 TBD + + * :sparkles: `photo.Descriptor` now carries `ImageURL` and `DownloadLocation`, and the Unsplash source fills them in from the photo it already fetches. `ImageURL` is the hotlink to display the photo from, taken from the API's `urls` (preferring `raw`, which carries no size preset, and falling back to `full` then `regular`). `DownloadLocation` is the endpoint to call when the photo is actually used. Unsplash's API guidelines require consumers to display photos from these URLs rather than from a self-hosted copy, and to trigger a download only on use; keeping the two apart lets a consumer do each at the right moment. Both fields are omitted from the serialized form when empty, so existing photo metadata still loads unchanged. + ## 1.0.0 2026-08-05 * :sparkles: First stable release. The reference, text, photo, and OpenScripture.Today APIs are settled enough to commit to. diff --git a/cmd/version.txt b/cmd/version.txt index 3eefcb9..9084fa2 100644 --- a/cmd/version.txt +++ b/cmd/version.txt @@ -1 +1 @@ -1.0.0 +1.1.0 diff --git a/pkg/photo/photo.go b/pkg/photo/photo.go index 7870c73..b78300f 100644 --- a/pkg/photo/photo.go +++ b/pkg/photo/photo.go @@ -20,6 +20,18 @@ type Descriptor struct { Color string `yaml:"color,omitempty" json:"color,omitempty"` Creator Creator `yaml:"creator" json:"creator"` + // ImageURL is the source's own hotlink for the image, suitable for use as an + // img src or a CSS background. Unsplash requires that consumers display + // photos from these URLs rather than from a copy they host themselves, so + // that views are attributed to the photographer. + ImageURL string `yaml:"image_url,omitempty" json:"image_url,omitempty"` + + // DownloadLocation is the endpoint to call when the image is actually used, + // which is how Unsplash counts a download. It is deliberately separate from + // ImageURL: displaying a photo is not a download, and the two are meant to + // be triggered at different moments. + DownloadLocation string `yaml:"download_location,omitempty" json:"download_location,omitempty"` + images map[string]ImageComplete } diff --git a/pkg/photo/unsplash/photo.go b/pkg/photo/unsplash/photo.go index fd0e5ab..2fa1b7b 100644 --- a/pkg/photo/unsplash/photo.go +++ b/pkg/photo/unsplash/photo.go @@ -14,6 +14,25 @@ import ( // stringValue is a helper for use with the Source Client to pull out strings // from responses. +// hotlinkURL picks the URL to display the photo from. Unsplash asks that photos +// be shown from the URLs it returns under "urls" rather than from a copy the +// consumer hosts. Raw is preferred because it carries no size preset, leaving +// the caller free to append its own Imgix parameters; full and regular stand in +// when a response omits it. +func hotlinkURL(image *unsplash.Photo) string { + if image.Urls == nil { + return "" + } + + for _, u := range []*unsplash.URL{image.Urls.Raw, image.Urls.Full, image.Urls.Regular} { + if s := urlValueString(u); s != "" { + return s + } + } + + return "" +} + func stringValue(str *string) string { if str == nil { return "" @@ -62,6 +81,8 @@ func (u *Source) Photo( Name: stringValue(image.Photographer.Name), Link: urlValueString(image.Photographer.Links.HTML), }, + ImageURL: hotlinkURL(image), + DownloadLocation: urlValueString(image.Links.DownloadLocation), } filename, err := IDFromURL(urlValueString(image.Links.Download)) diff --git a/pkg/photo/unsplash/photo_test.go b/pkg/photo/unsplash/photo_test.go index 2455d9d..dae97b6 100644 --- a/pkg/photo/unsplash/photo_test.go +++ b/pkg/photo/unsplash/photo_test.go @@ -28,8 +28,14 @@ func testServer() *httptest.Server { j := map[string]any{ "id": "abc123-_XYZ", "links": map[string]any{ - "html": baseUrl + "/photos/a-test-photo-with-title-that-does-not-matter-abc123-_XYZ", - "download": baseUrl + "/photos/abc123-_XYZ/download", + "html": baseUrl + "/photos/a-test-photo-with-title-that-does-not-matter-abc123-_XYZ", + "download": baseUrl + "/photos/abc123-_XYZ/download", + "download_location": baseUrl + "/photos/abc123-_XYZ/download", + }, + "urls": map[string]any{ + "raw": baseUrl + "/img/photo-abc123?ixid=raw", + "full": baseUrl + "/img/photo-abc123?ixid=full", + "regular": baseUrl + "/img/photo-abc123?ixid=regular", }, "user": map[string]any{ "name": "Test User", @@ -103,6 +109,8 @@ func TestSource(t *testing.T) { //nolint:paralleltest // unsplash client has glo Name: "Test User", Link: u.String() + "/testuser", }, + ImageURL: u.String() + "/img/photo-abc123?ixid=raw", + DownloadLocation: u.String() + "/photos/abc123-_XYZ/download", }, d) item := d.GetImage(photo.Original) @@ -115,3 +123,91 @@ func TestSource(t *testing.T) { //nolint:paralleltest // unsplash client has glo assert.Equal(t, "YZ/download", item.Filename()) } + +// hotlinkVariantServer serves a photo whose "urls" block contains only the +// variants given, so the preference order can be exercised. The variant values +// are opaque to the code under test, so they need not point anywhere real. +func hotlinkVariantServer(urls map[string]any) *httptest.Server { + baseUrl := "" + ts := httptest.NewServer(http.HandlerFunc( + func(w http.ResponseWriter, r *http.Request) { + // Source.Photo also resolves the download link, so that endpoint + // has to answer even though this test only inspects the hotlink. + if r.URL.Path == "/photos/abc123-_XYZ/download" { + if err := json.NewEncoder(w).Encode(map[string]any{ + "url": baseUrl + "/photos/abc123-_XYZ/download/actual-file", + }); err != nil { + w.WriteHeader(500) + } + return + } + + if r.URL.Path != "/photos/abc123-_XYZ" { + w.WriteHeader(404) + return + } + + j := map[string]any{ + "id": "abc123-_XYZ", + "links": map[string]any{ + "html": baseUrl + "/photos/a-test-photo-abc123-_XYZ", + "download": baseUrl + "/photos/abc123-_XYZ/download", + }, + "user": map[string]any{ + "name": "Test User", + "links": map[string]any{"html": baseUrl + "/testuser"}, + }, + } + if urls != nil { + j["urls"] = urls + } + + if err := json.NewEncoder(w).Encode(j); err != nil { + w.WriteHeader(500) + } + }, + )) + baseUrl = ts.URL + return ts +} + +// Unsplash asks that photos be displayed from the URLs under "urls". Raw is +// preferred because it carries no size preset, but a response omitting it must +// still yield a usable hotlink rather than none. +func TestSourcePhotoHotlinkURL(t *testing.T) { //nolint:paralleltest // unsplash client has globals that have to be set + const ( + raw = "https://images.example/photo-abc123?ixid=raw" + full = "https://images.example/photo-abc123?ixid=full" + regular = "https://images.example/photo-abc123?ixid=regular" + ) + + tests := []struct { + name string + urls map[string]any + want string + }{ + {"prefers raw", map[string]any{"raw": raw, "full": full, "regular": regular}, raw}, + {"falls back to full", map[string]any{"full": full, "regular": regular}, full}, + {"falls back to regular", map[string]any{"regular": regular}, regular}, + {"no urls block at all", nil, ""}, + {"empty urls block", map[string]any{}, ""}, + } + + //nolint:paralleltest // each case sets the client's global base URL + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + ts := hotlinkVariantServer(tc.urls) + defer ts.Close() + + u, err := url.Parse(ts.URL) + require.NoError(t, err) + unsp.SetupBaseUrl(u.String() + "/") + + src := &unsplash.Source{Client: unsp.New(ts.Client())} + d, err := src.Photo(context.Background(), "https://unsplash.com/photos/a-test-photo-abc123-_XYZ") + require.NoError(t, err) + + assert.Equal(t, tc.want, d.ImageURL) + }) + } +} From af593e0e097d4c501d76308c636ba83067b30411 Mon Sep 17 00:00:00 2001 From: Sterling Hanenkamp Date: Wed, 5 Aug 2026 12:26:15 -0500 Subject: [PATCH 2/2] fix: restore the stringValue doc comment and reorder hotlinkURL hotlinkURL was inserted between stringValue's doc comment and stringValue itself, so the comment described the wrong function and stringValue was left undocumented. Moves hotlinkURL below the two generic string helpers, which restores the file's original grouping -- the small pointer-dereference helpers first, then the domain functions -- and puts hotlinkURL after urlValueString, which it calls. Addresses Copilot review feedback on #92. Co-Authored-By: Claude Opus 5 --- pkg/photo/unsplash/photo.go | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/pkg/photo/unsplash/photo.go b/pkg/photo/unsplash/photo.go index 2fa1b7b..f2cf26b 100644 --- a/pkg/photo/unsplash/photo.go +++ b/pkg/photo/unsplash/photo.go @@ -14,6 +14,22 @@ import ( // stringValue is a helper for use with the Source Client to pull out strings // from responses. +func stringValue(str *string) string { + if str == nil { + return "" + } + return *str +} + +// urlValueString is a helper for use with the Source Client to pull out URL +// strings from responses. +func urlValueString(u *unsplash.URL) string { + if u == nil { + return "" + } + return u.String() +} + // hotlinkURL picks the URL to display the photo from. Unsplash asks that photos // be shown from the URLs it returns under "urls" rather than from a copy the // consumer hosts. Raw is preferred because it carries no size preset, leaving @@ -33,22 +49,6 @@ func hotlinkURL(image *unsplash.Photo) string { return "" } -func stringValue(str *string) string { - if str == nil { - return "" - } - return *str -} - -// urlValueString is a helper for use with the Source Client to pull out URL -// strings from responses. -func urlValueString(u *unsplash.URL) string { - if u == nil { - return "" - } - return u.String() -} - // IDFromURL extracts the photo ID from a URL. func IDFromURL(s string) (string, error) { u, err := url.Parse(s)