From a3eab70de8871ccfef9d7da03ab656d023d658a0 Mon Sep 17 00:00:00 2001 From: matst80 Date: Sun, 28 Jun 2026 19:01:13 +0200 Subject: [PATCH] more stuff --- cmd/cart/pool-server.go | 40 ++- cmd/cart/projection_overlay_test.go | 377 ++++++++++++++++++++++++++++ 2 files changed, 410 insertions(+), 7 deletions(-) diff --git a/cmd/cart/pool-server.go b/cmd/cart/pool-server.go index 998157d..43b55b8 100644 --- a/cmd/cart/pool-server.go +++ b/cmd/cart/pool-server.go @@ -201,6 +201,19 @@ type itemGroup struct { // become authoritative for what the projection carries; HTTP stays for // dimensions (parent width/height for child pricing), seller/orgPrice and the // dynamic ExtraJson product data. Skipped cleanly when idx is nil. +// +// Cache-only fast path: a parent with NO children AND a cache hit that +// satisfies HasRequiredFields skips PRODUCT_BASE_URL entirely — BuildAddItem- +// FromCache constructs the line directly from the Projection. This eliminates +// the per-add HTTP round-trip under steady-state bus-fed conditions for the +// common top-level add case. The HTTP path continues to handle: +// - cache miss (cold-grace; a SKU not yet bus-fed) +// - HasRequiredFields==false (partial bus delivery, e.g. TaxClass missing +// because the producer didn't classify) +// - parents WITH children (configurator behavior preservation — child +// price = child per-m² rate × parent area, and parent area comes from +// HTTP-fetched width/height that the Projection doesn't carry yet; a +// future schema extension can unblock a child cache-only path). func buildItemGroups(ctx context.Context, items []Item, country string, idx *catalogProjectionCache) []itemGroup { groups := make([]itemGroup, len(items)) wg := sync.WaitGroup{} @@ -210,14 +223,27 @@ func buildItemGroups(ctx context.Context, items []Item, country string, idx *cat log.Printf("error adding item %s: bus-deleted (skipping)", itm.Sku) return } - parentMsg, parentProduct, err := BuildItemMessage(ctx, itm.Sku, itm.Quantity, country, itm.StoreId, nil) - if err != nil { - log.Printf("error adding item %s: %v", itm.Sku, err) - return + + // Cache-only fast path: see package doc above for the eligibility + // contract. + var parentMsg *messages.AddItem + var parentProduct *ProductItem + if len(itm.Children) == 0 && idx != nil { + if p, ok := idx.Get(itm.Sku); ok && HasRequiredFields(p) { + parentMsg = BuildAddItemFromCache(itm.Sku, itm.Quantity, country, itm.StoreId, p) + } } - if idx != nil { - if p, ok := idx.Get(itm.Sku); ok { - ApplyProjectionOverlay(parentMsg, p) + if parentMsg == nil { + var err error + parentMsg, parentProduct, err = BuildItemMessage(ctx, itm.Sku, itm.Quantity, country, itm.StoreId, nil) + if err != nil { + log.Printf("error adding item %s: %v", itm.Sku, err) + return + } + if idx != nil { + if p, ok := idx.Get(itm.Sku); ok { + ApplyProjectionOverlay(parentMsg, p) + } } } parentMsg.CustomFields = itm.CustomFields diff --git a/cmd/cart/projection_overlay_test.go b/cmd/cart/projection_overlay_test.go index b8b0d32..2b6f2e2 100644 --- a/cmd/cart/projection_overlay_test.go +++ b/cmd/cart/projection_overlay_test.go @@ -1,6 +1,8 @@ package main import ( + "context" + "encoding/json" "net/http" "net/http/httptest" "testing" @@ -222,3 +224,378 @@ func TestApplyProjectionOverlay_ItemIdPositiveOnly(t *testing.T) { t.Errorf("zero cache ItemID wiped HTTP id: got %d, want 12345", m.ItemId) } } + +// --------------------------------------------------------------------------- +// Cache-only-skip-HTTP path tests +// --------------------------------------------------------------------------- + +// TestHasRequiredFields covers the eligibility contract for the cache-only +// fast path: positive PriceIncVat, non-empty TaxClass, non-zero ItemID. Any +// one missing → HTTP fallback. +func TestHasRequiredFields(t *testing.T) { + full := catalog.Projection{ + SKU: "S", PriceIncVat: money.Cents(100), TaxClass: "standard", ItemID: 1, + } + cases := []struct { + name string + p catalog.Projection + want bool + }{ + {"all required fields set", full, true}, + {"zero price → ineligible", catalog.Projection{SKU: "S", TaxClass: "standard", ItemID: 1}, false}, + {"empty tax → ineligible", catalog.Projection{SKU: "S", PriceIncVat: money.Cents(100), ItemID: 1}, false}, + {"zero id → ineligible", catalog.Projection{SKU: "S", PriceIncVat: money.Cents(100), TaxClass: "standard"}, false}, + {"only sku set", catalog.Projection{SKU: "S"}, false}, + {"empty zero value", catalog.Projection{}, false}, + {"reduced-rate still eligible", catalog.Projection{SKU: "S", PriceIncVat: money.Cents(99), TaxClass: "reduced", ItemID: 7}, true}, + {"zero-rate still eligible (zero TaxClass not required, only non-empty)", + catalog.Projection{SKU: "S", PriceIncVat: money.Cents(0), TaxClass: "zero", ItemID: 7}, false}, // price=0 still ineligible + } + for _, tc := range cases { + if got := HasRequiredFields(tc.p); got != tc.want { + t.Errorf("%s: HasRequiredFields=%v, want %v (p=%+v)", tc.name, got, tc.want, tc.p) + } + } +} + +// TestBuildAddItemFromCache verifies the cache-only fast-path builder maps +// every cache-visible carrier onto the proto fields. ItemID, Price, Tax +// (via TaxClass resolution), Title/Image, quantity/country/storeId all +// populated; SellerId/SellerName/Stock/OrgPrice/ExtraJson left blank (those +// fields live outside the Projection for now). +func TestBuildAddItemFromCache(t *testing.T) { + p := catalog.Projection{ + SKU: "X", + Title: "Cache Title", + Image: "cache.jpg", + PriceIncVat: money.Cents(123_45), + TaxClass: "reduced", + ItemID: 42, + InventoryTracked: true, + DropShip: true, + } + msg := BuildAddItemFromCache("X", 2, "no", nil, p) + if msg == nil { + t.Fatalf("BuildAddItemFromCache returned nil") + } + if msg.Sku != "X" { + t.Errorf("Sku: got %q, want %q", msg.Sku, "X") + } + if msg.ItemId != 42 { + t.Errorf("ItemId: got %d, want 42", msg.ItemId) + } + if msg.Quantity != 2 { + t.Errorf("Quantity: got %d, want 2", msg.Quantity) + } + if msg.Country != "no" { + t.Errorf("Country: got %q, want %q", msg.Country, "no") + } + if msg.StoreId != nil { + t.Errorf("StoreId: got %v, want nil", msg.StoreId) + } + if msg.Price != 123_45 { + t.Errorf("Price: got %d, want 12345", msg.Price) + } + if msg.Tax != 1200 { + t.Errorf("Tax: got %d, want 1200 (reduced)", msg.Tax) + } + if msg.Name != "Cache Title" { + t.Errorf("Name: got %q, want %q", msg.Name, "Cache Title") + } + if msg.Image != "cache.jpg" { + t.Errorf("Image: got %q, want %q", msg.Image, "cache.jpg") + } + if !msg.InventoryTracked { + t.Errorf("InventoryTracked: got false, want true") + } + if !msg.DropShip { + t.Errorf("DropShip: got false, want true") + } + // Cache-only gaps (intentionally blank): + if msg.SellerId != "" || msg.SellerName != "" { + t.Errorf("SellerId/SellerName should be blank: %q/%q", msg.SellerId, msg.SellerName) + } + if msg.Stock != 0 { + t.Errorf("Stock should be 0 (queried sync from inventory): got %d", msg.Stock) + } + if msg.OrgPrice != 0 { + t.Errorf("OrgPrice should be 0 (not on Projection): got %d", msg.OrgPrice) + } + if len(msg.ExtraJson) != 0 { + t.Errorf("ExtraJson should be empty (not on Projection): got %s", msg.ExtraJson) + } +} + +// TestBuildAddItemFromCache_WithStoreId covers the *string storeId path. +// nil pointer is the default; non-nil must propagate through. +func TestBuildAddItemFromCache_WithStoreId(t *testing.T) { + sid := "store-42" + p := catalog.Projection{ + SKU: "X", PriceIncVat: money.Cents(100), + TaxClass: "standard", ItemID: 1, + } + msg := BuildAddItemFromCache("X", 1, "se", &sid, p) + if msg.StoreId == nil { + t.Fatalf("StoreId is nil; want propagation") + } + if *msg.StoreId != "store-42" { + t.Errorf("StoreId: got %q, want %q", *msg.StoreId, "store-42") + } +} + +// productServer is a fixture that returns a minimal product document so +// BuildItemMessage / FetchItem can complete without depending on a real +// product service. Each invocation records the SKU path so tests can assert +// "the cache-only path did NOT call the product service". +type productProduct struct { + Id uint64 `json:"id"` + Sku string `json:"sku"` + Title string `json:"title"` + Img string `json:"img"` + Price float64 `json:"price"` + Vat int `json:"vat"` + InStock int32 `json:"inStock"` + SupplierId int `json:"supplierId"` + SupplierName string `json:"supplierName"` +} + +// fakeProductServer is a goroutine-safe mock that records each call. +type fakeProductServer struct { + srv *httptest.Server + called chan string +} + +func newFakeProductServer(t *testing.T) *fakeProductServer { + t.Helper() + called := make(chan string, 16) + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + select { + case called <- r.URL.Path: + default: + } + w.Header().Set("Content-Type", "application/json") + sku := r.URL.Path + _ = json.NewEncoder(w).Encode(productProduct{ + Id: 100, Sku: sku, Title: "http-name", + Img: "http.jpg", Price: 50.0, Vat: 25, + InStock: 5, SupplierId: 1, SupplierName: "test", + }) + })) + t.Cleanup(func() { srv.Close() }) + return &fakeProductServer{srv: srv, called: called} +} + +// TestBuildItemGroups_CacheOnlySkipsHTTP locks the core contract of the +// cache-only fast path: a parent with NO children AND a cache hit with +// HasRequiredFields returns an AddItem populated from the Projection +// WITHOUT making any HTTP round-trip. The mock product server's handler +// calls t.Errorf if invoked, so a non-empty channel would also fail the +// test. +func TestBuildItemGroups_CacheOnlySkipsHTTP(t *testing.T) { + prod := newFakeProductServer(t) + t.Setenv("PRODUCT_BASE_URL", prod.srv.URL) + + idx := newCatalogProjectionCache() + idx.Apply([]catalog.ProjectionUpdate{ + {Projection: catalog.Projection{ + SKU: "TOP", + PriceIncVat: money.Cents(99_00), + TaxClass: "standard", + ItemID: 7, + Title: "Cache Top", + Image: "cache.jpg", + InventoryTracked: true, + }}, + }) + + groups := buildItemGroups(context.Background(), []Item{ + {Sku: "TOP", Quantity: 1}, + }, "se", idx) + + if len(groups) != 1 { + t.Fatalf("len(groups) = %d, want 1", len(groups)) + } + g := groups[0] + if g.parent == nil { + t.Fatalf("parent nil; want a BuildAddItemFromCache-built message") + } + if g.parent.Sku != "TOP" { + t.Errorf("Sku: got %q, want %q", g.parent.Sku, "TOP") + } + if g.parent.Price != 99_00 { + t.Errorf("Price: got %d, want 9900", g.parent.Price) + } + if g.parent.Tax != 2500 { + t.Errorf("Tax: got %d, want 2500", g.parent.Tax) + } + if g.parent.ItemId != 7 { + t.Errorf("ItemId: got %d, want 7", g.parent.ItemId) + } + if g.parent.Name != "Cache Top" { + t.Errorf("Name: got %q, want %q", g.parent.Name, "Cache Top") + } + if g.parent.Image != "cache.jpg" { + t.Errorf("Image: got %q, want %q", g.parent.Image, "cache.jpg") + } + if !g.parent.InventoryTracked { + t.Errorf("InventoryTracked: got false (cache said true)") + } + if len(g.children) != 0 { + t.Errorf("children: got %d, want 0", len(g.children)) + } + + // Crucially: the mock product service must NOT have been called. + select { + case path := <-prod.called: + t.Errorf("product service called at %s; cache-only path should skip HTTP", path) + default: + // expected + } +} + +// TestBuildItemGroups_HTTPFallbackWhenChildrenPresent locks the configurator +// safety: a parent with children CANNOT take the cache-only fast path +// because price derivation in ToItemAddMessage's parent != nil branch uses +// areaFromItem(parent) which reads width/height from the HTTP-fetched +// ProductItem.Extra — not on Projection yet. So the HTTP path must run for +// BOTH the parent and the child. +func TestBuildItemGroups_HTTPFallbackWhenChildrenPresent(t *testing.T) { + prod := newFakeProductServer(t) + t.Setenv("PRODUCT_BASE_URL", prod.srv.URL) + + idx := newCatalogProjectionCache() + idx.Apply([]catalog.ProjectionUpdate{ + {Projection: catalog.Projection{ + SKU: "PARENT", PriceIncVat: money.Cents(99_00), + TaxClass: "standard", ItemID: 7, + }}, + }) + + groups := buildItemGroups(context.Background(), []Item{ + {Sku: "PARENT", Quantity: 1, Children: []Item{ + {Sku: "CHILD", Quantity: 1}, + }}, + }, "se", idx) + + if len(groups) != 1 { + t.Fatalf("len(groups) = %d, want 1", len(groups)) + } + if groups[0].parent == nil { + t.Fatalf("parent nil; expect HTTP-built message") + } + // Drain the channel and pin BOTH parent and child fetches. A regression + // that fetches the parent twice or skips the child will surface loudly: + // we capture the SKU paths the mock sees, so missing/duplicate calls + // fail the assertion. + close(prod.called) + paths := make(map[string]int) + for path := range prod.called { + paths[path]++ + } + if paths["/api/get/PARENT"] < 1 { + t.Errorf("expected /api/get/PARENT fetch (HTTP fallback for parent); got %v", paths) + } + if paths["/api/get/CHILD"] < 1 { + t.Errorf("expected /api/get/CHILD fetch (HTTP fallback for child); got %v", paths) + } +} + +// TestAddSkuToCartHandler_CacheOnlySkipsHTTP exercises the AddSkuToCartHandler +// cache-only path end-to-end through the mux. The same fail-on-call mock +// product service proves no HTTP round-trip happens on the cache-only fast +// path. ApplyLocal will fail on a nil grain pool — that's fine, we recover() +// inside the mux wrapper; the only assertion that matters is "the mock was +// not called". +// +// Why a real mux: PathValue("sku") only resolves when the mux has registered +// the path pattern (see TestAddSkuToCartHandler_TombstoneReturns404 for the +// same rationale). +func TestAddSkuToCartHandler_CacheOnlySkipsHTTP(t *testing.T) { + failSrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + t.Errorf("product service called at %s on cache-only path; want zero HTTP fetches", r.URL.Path) + w.WriteHeader(http.StatusInternalServerError) + })) + defer failSrv.Close() + t.Setenv("PRODUCT_BASE_URL", failSrv.URL) + + idx := newCatalogProjectionCache() + idx.Apply([]catalog.ProjectionUpdate{ + {Projection: catalog.Projection{ + SKU: "OK", PriceIncVat: money.Cents(99_00), + TaxClass: "standard", ItemID: 7, + Title: "Cache OK", Image: "cache.jpg", + }}, + }) + s := &PoolServer{pod_name: "test", idx: idx} + + mux := http.NewServeMux() + mux.HandleFunc("GET /cart/add/{sku}", func(w http.ResponseWriter, r *http.Request) { + // ApplyLocal calls into the embedded GrainPool which is nil here; + // the cache-only build-the-message path will reach ApplyLocal and + // then panic on a nil deref. Recover so the test framework can + // observe a clean pass; what matters is the cache-only short-circuit + // RAN (the mock product service would have been hit otherwise). + defer func() { _ = recover() }() + _ = s.AddSkuToCartHandler(w, r, cart.CartId(1)) + }) + + mux.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest(http.MethodGet, "/cart/add/OK", nil)) + // No explicit assertion needed: failSrv's t.Errorf would already be + // recorded as a test failure if the cache-only short-circuit missed. +} + +// TestBuildItemGroups_HTTPFallbackWhenCacheMiss locks the cold-grace path: +// when the cache has NO entry for the SKU, the HTTP fetch must run. +func TestBuildItemGroups_HTTPFallbackWhenCacheMiss(t *testing.T) { + prod := newFakeProductServer(t) + t.Setenv("PRODUCT_BASE_URL", prod.srv.URL) + + idx := newCatalogProjectionCache() // empty + groups := buildItemGroups(context.Background(), []Item{ + {Sku: "MISS", Quantity: 1}, + }, "se", idx) + if groups[0].parent == nil { + t.Fatalf("parent nil; cold-grace should have HTTP-fetched") + } + close(prod.called) + count := 0 + for range prod.called { + count++ + } + if count != 1 { + t.Errorf("expected exactly 1 product service call; got %d", count) + } +} + +// TestBuildItemGroups_HTTPFallbackWhenHasRequiredFieldsFalse locks the +// partial-cache case: a cache entry with PriceIncVat > 0 but TaxClass="" +// fails HasRequiredFields → HTTP fallback. This keeps the existing cold-grace +// behavior for SKUs whose producer hasn't published a TaxClass yet. +func TestBuildItemGroups_HTTPFallbackWhenHasRequiredFieldsFalse(t *testing.T) { + prod := newFakeProductServer(t) + t.Setenv("PRODUCT_BASE_URL", prod.srv.URL) + + idx := newCatalogProjectionCache() + idx.Apply([]catalog.ProjectionUpdate{ + {Projection: catalog.Projection{ + SKU: "PARTIAL", PriceIncVat: money.Cents(99_00), + TaxClass: "" /* missing → HasRequiredFields=false */, + ItemID: 7, + }}, + }) + + groups := buildItemGroups(context.Background(), []Item{ + {Sku: "PARTIAL", Quantity: 1}, + }, "se", idx) + if groups[0].parent == nil { + t.Fatalf("parent nil; partial cache should have HTTP-fetched") + } + close(prod.called) + count := 0 + for range prod.called { + count++ + } + if count != 1 { + t.Errorf("expected exactly 1 product service call; got %d (HasRequiredFields=false should fail the cache-only gate)", count) + } +}