From c5109bf216750ca26e4c340af93d7b6e43d796d1 Mon Sep 17 00:00:00 2001 From: abhishek Date: Sat, 29 Aug 2026 11:29:15 +0530 Subject: [PATCH] catalogue images --- repositories/catalogueColumns_test.go | 71 ++++++++++++++++++++++++ repositories/catalogueRepository.go | 80 ++++++++++++++++++++++++++- 2 files changed, 150 insertions(+), 1 deletion(-) diff --git a/repositories/catalogueColumns_test.go b/repositories/catalogueColumns_test.go index 7830e80..c7cc986 100644 --- a/repositories/catalogueColumns_test.go +++ b/repositories/catalogueColumns_test.go @@ -83,3 +83,74 @@ func TestCoreColumnsAreAlwaysSelectedPlainly(t *testing.T) { t.Errorf("core columns should lead the select list, got:\n%s", got) } } + +/* +Where product photos come from. + +The owning team records what its image search found, per product, in +`image_url` and `image_urls`. Most of those are EXTERNAL — bigbasket, amazon, a +shop's own CDN — and only a subset were ever mirrored into our bucket. Reading +the bucket alone found photos for 2 of britannia's 6 products while they had +URLs for all 6. +*/ + +func TestJsonImageListIsRead(t *testing.T) { + // The shape their API returns today. + raw := `["https://www.bigbasket.com/media/uploads/p/l/270729_21-britannia.jpg",` + + `"https://m.media-amazon.com/images/I/71n1Q3cQL3L.jpg"]` + got := parseImageList(raw) + if len(got) != 2 || got[0] != "https://www.bigbasket.com/media/uploads/p/l/270729_21-britannia.jpg" { + t.Fatalf("json list not parsed: %#v", got) + } +} + +// The same column stored as a Postgres text[] rather than jsonb. Which one it +// is belongs to the owning team, and this side should not break when it moves. +func TestPostgresArrayImageListIsRead(t *testing.T) { + got := parseImageList(`{"https://a.example/1.jpg","https://b.example/2.jpg"}`) + if len(got) != 2 || got[1] != "https://b.example/2.jpg" { + t.Fatalf("pg array not parsed: %#v", got) + } +} + +func TestEmptyImageListsAreNotPhotos(t *testing.T) { + for _, raw := range []string{"", " ", "{}", "[]", "null"} { + if got := parseImageList(raw); len(got) != 0 { + t.Errorf("%q should yield no photos, got %#v", raw, got) + } + } +} + +// A malformed value must cost the photos, not the product. +func TestMalformedImageListDoesNotBreakTheRow(t *testing.T) { + if got := parseImageList(`["unterminated`); len(got) != 0 { + t.Errorf("expected no photos from malformed json, got %#v", got) + } +} + +func TestTheListedUrlWinsAndDuplicatesCollapse(t *testing.T) { + row := catalogueProductRow{ + ImageID: "britannia_britannia_marie_gold_250g", + ImageURL: "https://a.example/1.jpg", + ImageURLs: `["https://a.example/1.jpg","https://a.example/2.jpg"]`, + } + got := imagesFor("britannia", row) + if len(got) != 2 { + t.Fatalf("the primary url repeated in the list should appear once: %#v", got) + } + if got[0] != "https://a.example/1.jpg" || got[1] != "https://a.example/2.jpg" { + t.Errorf("order should follow image_urls: %#v", got) + } +} + +// With no URLs recorded, the bucket listing is still consulted — it is the only +// source for anything ingested before these columns existed. +func TestNoRecordedUrlsFallsBackToTheBucket(t *testing.T) { + row := catalogueProductRow{ImageID: "cadbury_cadbury_5_star_5_gm"} + // db.GetImages returns nil when the image store was never initialised, + // which is the case here — the point is that it does not panic and does not + // invent a photo. + if got := imagesFor("cadbury", row); len(got) != 0 { + t.Errorf("expected no photos without an image store, got %#v", got) + } +} diff --git a/repositories/catalogueRepository.go b/repositories/catalogueRepository.go index 8ac7f00..99764e0 100644 --- a/repositories/catalogueRepository.go +++ b/repositories/catalogueRepository.go @@ -1,6 +1,7 @@ package repositories import ( + "encoding/json" "errors" "fmt" "log" @@ -69,6 +70,24 @@ var catalogueOptionalColumns = []struct{ Name, Present, Absent string }{ {"search_query", "search_query", "NULL::text AS search_query"}, {"created_at", "created_at", "NULL::timestamptz AS created_at"}, {"updated_at", "updated_at", "NULL::timestamptz AS updated_at"}, + + // Where the photos actually are. + // + // The catalogue pipeline records what it found per product: `image_url` for + // the primary and `image_urls` for the rest. Most are EXTERNAL — bigbasket, + // amazon, a shop's own CDN — because stage 6 searches the web and only some + // results get mirrored into our bucket. + // + // This side ignored both columns and instead listed bucket objects under + // `daily/brands/{brand}/{image_id}/`, so a product showed a photo only if it + // happened to have been mirrored. Measured on britannia: 2 of 6 products had + // images here while the owning team had URLs for all 6. + // + // Cast to text and parsed by hand: the column is an array, and GORM's raw + // scan silently drops slice-kind destination fields — the same reason + // providers, highlights and nutrients are read this way. + {"image_url", "image_url", "NULL::text AS image_url"}, + {"image_urls", "image_urls::text AS image_urls", "NULL::text AS image_urls"}, } // columnsFor builds the SELECT list for one table from the columns it has. @@ -118,6 +137,65 @@ type catalogueProductRow struct { SearchQuery string CreatedAt time.Time UpdatedAt time.Time + ImageURL string + ImageURLs string +} + +// imagesFor decides which photos a catalogue product carries. +// +// The pipeline's own `image_urls` first, then its single `image_url`, and the +// bucket listing last. That order is deliberate: the owning team records what +// it found for every product, and only a subset of those files were ever +// mirrored into our bucket — so listing the bucket alone finds photos for a +// minority of the catalogue and reports the rest as having none. +// +// The bucket stays as the fallback rather than being dropped. It is the only +// source for anything ingested before these columns existed, and it is the one +// source that cannot rot: a external URL is somebody else's CDN and will +// eventually 404, at which point the card falls through to the next photo. +func imagesFor(brand string, row catalogueProductRow) []string { + seen := make(map[string]bool) + var out []string + add := func(url string) { + url = strings.TrimSpace(url) + if url == "" || seen[url] { + return + } + seen[url] = true + out = append(out, url) + } + + for _, url := range parseImageList(row.ImageURLs) { + add(url) + } + add(row.ImageURL) + + if len(out) == 0 { + return db.GetImages(brand, row.ImageID) + } + return out +} + +// parseImageList reads the list whichever way it was stored. +// +// A Postgres text[] casts to `{a,b}` and a jsonb column to `["a","b"]`, and the +// column type is the owning team's to change. Handling both here costs four +// lines and removes a whole class of "it worked until they migrated". +func parseImageList(raw string) []string { + raw = strings.TrimSpace(raw) + if raw == "" || raw == "{}" || raw == "[]" || raw == "null" { + return nil + } + if strings.HasPrefix(raw, "[") { + var urls []string + if err := json.Unmarshal([]byte(raw), &urls); err == nil { + return urls + } + // Falls through on a malformed value rather than dropping the row. + log.Printf("catalogue: could not parse image_urls as JSON: %.120s", raw) + return nil + } + return models.ParsePGArray(raw) } func (row catalogueProductRow) toModel(brand string) models.CatalogueProduct { @@ -129,7 +207,7 @@ func (row catalogueProductRow) toModel(brand string) models.CatalogueProduct { Description: row.Description, Category: row.Category, ImageID: row.ImageID, - Images: db.GetImages(brand, row.ImageID), + Images: imagesFor(brand, row), Size: row.Size, VariantKey: row.VariantKey, ProductSKU: row.ProductSKU,