more stuff

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,7 +223,19 @@ 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)
+
+			// 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 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
@@ -220,6 +245,7 @@ func buildItemGroups(ctx context.Context, items []Item, country string, idx *cat
 						ApplyProjectionOverlay(parentMsg, p)
 					}
 				}
+			}
 			parentMsg.CustomFields = itm.CustomFields
 			groups[i].parent = parentMsg
 

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)
+	}
+}