From 51c32e6d612e9dfee6902f57e09387df03a02ff7 Mon Sep 17 00:00:00 2001 From: Steponas Kaminskas <132608893+SteponasK@users.noreply.github.com> Date: Fri, 3 Jul 2026 11:35:24 +0300 Subject: [PATCH] BRD-1127: Rewrite CLAUDE.md around V2 (public-safe); add ValueObjects child doc --- CLAUDE.md | 79 ++++++++++++++--------------------- src/V2/ValueObjects/CLAUDE.md | 34 +++++++++++++++ 2 files changed, 65 insertions(+), 48 deletions(-) create mode 100644 src/V2/ValueObjects/CLAUDE.md diff --git a/CLAUDE.md b/CLAUDE.md index aa1432e..d664781 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1,32 +1,27 @@ # CLAUDE.md -This file provides guidance to Claude Code when working with code in this repository (`bradsearch/search-sync-sdk`). Facts below verified against the working tree as of 2026-07-03. +Guidance for Claude Code in this repo. This file covers what rarely changes; deep ValueObject conventions live in `src/V2/ValueObjects/CLAUDE.md` — read it before touching a payload type. ## What this is -Pure-PHP (>= 8.4), zero runtime composer deps, client library for the Brad Search synchronization/admin HTTP API. Every payload it emits is an API contract: the server's OpenAPI v2 spec defines the shape, this repo's fixtures assert it byte-for-byte, and downstream consumers depend on it. +`bradsearch/search-sync-sdk` is a pure-PHP (>= 8.4), zero-runtime-dependency client library for the Brad Search synchronization/admin HTTP API. It never talks to the search backend directly — only to that HTTP API. Every payload it emits is a contract: the server's OpenAPI spec defines the shape, this repo's fixtures assert it byte-for-byte, and every consumer (a Laravel application, plus Shopify/Magento sync jobs) depends on it not drifting. -## V1 / V2 — two generations coexist +**V1 is deprecated.** `src/SynchronizationApiSdk.php` and its array/`DataValidator` payload style are legacy — kept alive only for customers not yet migrated, never extended. All new work targets V2. -**V2 is the current, active surface. Center new work here.** V1 is legacy and stays alive only while customers remain on it — neither generation may be deleted. +## V2 architecture -| | V1 (legacy) | V2 (current) | -|---|---|---| -| Facade | `src/SynchronizationApiSdk.php` | `src/SyncV2Sdk.php` | -| Config | `src/Config/SyncConfig.php` | `src/Config/SyncConfigV2.php` (appId **must be a UUID**; apiUrl, token, optional `targetIndex`) | -| Endpoints | `/api/v1/sync/...` | `/api/v2/applications/{appId}/...` | -| Payload style | raw arrays + `src/Validators/DataValidator.php` | **strict immutable readonly ValueObjects** in `src/V2/ValueObjects/` (BulkOperations, Index, Normalize, Product, Response, Search, SearchSettings, Synonym, Common) with constructor validation, builders, and `jsonSerialize()` asserted against fixtures | -| Adapters | `PrestaShopAdapter`, `MagentoAdapter` | `PrestaShopAdapterV2`, `MagentoAdapterV2`, `ShopifyAdapter` (V2-only) | +- **Facade**: `src/SyncV2Sdk.php`. +- **Config**: `src/Config/SyncConfigV2.php` — `appId` (must be a UUID), `apiUrl`, `token`, optional `targetIndex`. +- **Endpoints**: `/api/v2/applications/{appId}/...`. +- **Payloads**: strict immutable readonly ValueObjects in `src/V2/ValueObjects/` (BulkOperations, Index, Normalize, Product, Response, Search, SearchSettings, Synonym, Common), each with constructor validation, a builder, and a `jsonSerialize()` verified against a fixture. See `src/V2/ValueObjects/CLAUDE.md` for the conventions. +- **Adapters**: `PrestaShopAdapterV2`, `MagentoAdapterV2` (GraphQL-fed via `src/Magento/`), `ShopifyAdapter` — transform platform product data into V2 payloads. +- **Admin**: `src/AdminSdk.php` + `src/Client/AdminHttpClient.php` for `/api/v2/admin/indices` (raw physical index list/delete). -Also: `src/AdminSdk.php` + `src/Client/AdminHttpClient.php` for `/api/v2/admin/indices` (raw physical index list/delete). - -**A field/feature that both generations expose must land twice** — once in the V1 array path, once in the V2 ValueObject path. Confirm with the owner before duplicating; some features (synonyms, search settings, alias versioning) are V2-only. Which generation a given customer uses is decided server-side by the consuming application, not here. - -The V2 design contract lives in `tasks/prd-v2-valueobjects.md` — read it before changing VO conventions (immutability, `with*()` methods, builders, exact OpenAPI alignment). +The V2 design contract lives in `tasks/prd-v2-valueobjects.md` — read it before changing ValueObject conventions (immutability, `with*()` methods, builders, exact API alignment). ## OpenAPI golden-fixture parity (the centerpiece discipline) -`tests/fixtures/openapi-examples/*.json` are not sample data — they ARE the cross-repo contract test: +`tests/fixtures/openapi-examples/*.json` are not sample data — they ARE the contract test: ``` tests/fixtures/openapi-examples/ @@ -38,18 +33,18 @@ tests/fixtures/openapi-examples/ └── synonyms-ecommerce-en.json ``` -Each file is copied verbatim from an example payload in the server's OpenAPI v2 spec. `tests/V2/ApiPayloadVerificationTest.php` builds the same payload through the V2 ValueObjects/builders and asserts `jsonSerialize()` equals the decoded fixture — exact byte-level/structural alignment, not "close enough." `tests/V2/DarboDrabuziaiWorkflowTest.php` chains the fixtures into a full end-to-end simulation (create index v1 → configure → bulk-sync → create index v2 → sync → activate v2 → verify → cleanup, plus a rollback scenario). +Each file is copied verbatim from an example payload in the server's OpenAPI spec. `tests/V2/ApiPayloadVerificationTest.php` builds the same payload through the V2 ValueObjects/builders and asserts `jsonSerialize()` equals the decoded fixture — exact structural alignment, not "close enough." `tests/V2/DarboDrabuziaiWorkflowTest.php` chains the fixtures into a full end-to-end simulation (create index → configure → bulk-sync → create new version → sync → activate → verify → cleanup, plus a rollback scenario). **If you change a V2 payload shape**, do this in lockstep, in one PR (after the server-side API change lands first): -1. Confirm the field/shape exists in the server's OpenAPI v2 spec first — never invent a shape SDK-side. +1. Confirm the field/shape exists in the server's OpenAPI spec first — never invent a shape SDK-side. 2. Update the ValueObject in `src/V2/ValueObjects//`. 3. Update the matching builder and `with*()` methods. 4. Update the affected fixture — copied from the server's OpenAPI spec, never hand-authored from memory. -5. Update `ApiPayloadVerificationTest` (and `DarboDrabuziaiWorkflowTest` if index-create/bulk-operations shape moved). -6. Decide explicitly whether the V1 side also needs the change. +5. Update `ApiPayloadVerificationTest` (and `DarboDrabuziaiWorkflowTest` if the index-create/bulk-operations shape moved). +6. Decide explicitly whether the deprecated V1 side also needs the change (it usually doesn't). 7. Quality-gate triple green (below). -**Failure smell**: if a fixture test fails, do not "fix" it by editing the fixture to match your output. The fixture mirrors the API spec. Either copy the spec's new example verbatim, or fix your VO — the fixture is never adjusted just to silence a test. +**Failure smell**: if a fixture test fails, do not "fix" it by editing the fixture to match your output. The fixture mirrors the API spec. Either copy the spec's new example verbatim, or fix your ValueObject — the fixture is never adjusted just to silence a test. ## Locale-suffix contract @@ -60,11 +55,19 @@ Documented in `src/Adapters/README.md` ("Locale Handling"): 3. Every other locale gets a suffixed field: `name_lt-LT`, `description_en-US`. 4. Fallback: if a product is missing the default locale's value, adapters fall back to the first available locale. -Enforced by `src/V2/ValueObjects/Common/LocalizedField.php`, which builds `_` and validates the locale against `^[a-z]{2}(-[A-Z]{2})?$` (region part is optional — `lt` is as valid as `lt-LT`), throwing `InvalidLocaleException` otherwise. +Enforced by `src/V2/ValueObjects/Common/LocalizedField.php`, which builds `_` and validates the locale against `^[a-z]{2}(-[A-Z]{2})?$` (the region part is optional — `lt` is as valid as `lt-LT`), throwing `InvalidLocaleException` otherwise. + +Getting this wrong does not error — it silently breaks search relevance in one language (fields land under the wrong name; the backend's per-language analysis never sees them). Treat any locale-touching diff as high-risk; cover both the unsuffixed default and the suffixed path with tests. + +## targetIndex / alias semantics + +The API exposes versioned physical indices behind an alias named after the appId. `SyncConfigV2->targetIndex` defaults to `null`, so bulk ops normally hit the alias (the LIVE index). During a zero-downtime reindex, construct a second `SyncConfigV2` with `targetIndex` set to the new versioned index so bulk-loading targets the inactive version while search keeps serving the old one; only `activateIndexVersion()` flips traffic. If a sync appears to do nothing, check whether it wrote to a non-active version via `getIndexInfo()`. `AdminSdk` lists raw physical indices, not aliases. -Getting this wrong does not error — it silently breaks search relevance in one language (fields land under the wrong name; the engine's per-language analyzers never see them). Treat any locale-touching diff as high-risk; cover both the unsuffixed default and the suffixed path with tests. +## Price correctness (recurring bug class) -**Known debt**: V1's embeddable-fields builder hardcodes the locale pair — `src/SynchronizationApiSdk.php:348-349` (`$locales = ['en-US', 'lt-LT'];`). This blocks V1 customers outside that pair. It is a known gap; fixing it needs its own ticket (changes V1 index mappings) — do not fix it as a drive-by. +- **Shopify money math is bcmath-only, mandatorily**: `ShopifyAdapter` refuses to construct without `bccomp` and compares prices with `bccomp(..., 2)`. Never replace bcmath comparisons with float `>`/`==`. +- bcmath is Shopify-only — `PrestaShopAdapterV2`/`MagentoAdapterV2` use native numerics with explicit zero-price guards. This asymmetry is historical, not principled; keep zero-price guards intact if you touch non-Shopify price code. +- Zero/empty prices are legitimate inputs from every platform — treat as "no discount", never divide by them. ## Development commands @@ -92,30 +95,10 @@ composer install composer update ``` -## Code architecture - -### Key components -- **`SynchronizationApiSdk`** (V1) / **`SyncV2Sdk`** (V2) — main facades. -- **Field Configuration** (`src/Models/`) — `FieldConfig`, `FieldConfigBuilder`, `FieldType` enum (V1). -- **ValueObjects** (`src/V2/ValueObjects/`) — V2 payload types with constructor validation and builders. -- **Validation** (`src/Validators/DataValidator.php`) — V1 client-side validation before any API call. -- **HTTP** (`src/Client/`) — `HttpClient` (V1), `AdminHttpClient` (admin API). -- **Adapters** (`src/Adapters/`) — `PrestaShopAdapter`/`PrestaShopAdapterV2`, `MagentoAdapter`/`MagentoAdapterV2` (GraphQL-fed via `src/Magento/`), `ShopifyAdapter` (V2-only). - -### targetIndex / alias semantics - -The API exposes versioned physical indices behind an alias named after the appId. `SyncConfigV2->targetIndex` defaults to `null`, so bulk ops normally hit the alias (the LIVE index). During a zero-downtime reindex, construct a second `SyncConfigV2` with `targetIndex` set to the new versioned index so bulk-loading targets the inactive version while search keeps serving the old one; only `activateIndexVersion()` flips traffic. If a sync appears to do nothing, check whether it wrote to a non-active version via `getIndexInfo()`. `AdminSdk` lists raw physical indices, not aliases. - -### Price correctness (recurring bug class) - -- **Shopify money math is bcmath-only, mandatorily**: `ShopifyAdapter` refuses to construct without `bccomp` and compares prices with `bccomp(..., 2)`. Never replace bcmath comparisons with float `>`/`==`. -- bcmath is Shopify-only — `PrestaShopAdapterV2`/`MagentoAdapterV2` use native numerics with explicit zero-price guards. This asymmetry is historical, not principled; keep zero-price guards intact if you touch non-Shopify price code. -- Zero/empty prices are legitimate inputs from every platform — treat as "no discount", never divide by them. - -### Who consumes this SDK +## Who consumes this SDK -- The primary consumer is a **Laravel application**, resolving `bradsearch/search-sync-sdk` from packagist.org (no `repositories` block). Local dev uses a composer path-repository symlink to your checkout. -- The **PrestaShop module** does NOT depend on this SDK — it targets PHP >= 7.1 and owns its own product transformation. +- The primary consumer is a Laravel application, resolving `bradsearch/search-sync-sdk` from packagist.org (no `repositories` block). Local dev uses a composer path-repository symlink to your checkout. +- A separate PrestaShop module does NOT depend on this SDK — it targets an older PHP baseline and owns its own product transformation. - Releases are git tags (`vMAJOR.MINOR.PATCH`); there is no publish workflow in this repo. ## Dependencies diff --git a/src/V2/ValueObjects/CLAUDE.md b/src/V2/ValueObjects/CLAUDE.md new file mode 100644 index 0000000..8cfdce0 --- /dev/null +++ b/src/V2/ValueObjects/CLAUDE.md @@ -0,0 +1,34 @@ +# V2 ValueObjects + +The V2 payload layer. Every class here represents one shape from the server's OpenAPI spec — see the root `CLAUDE.md` for the fixture-parity discipline these exist to satisfy. + +## Conventions (design contract: `tasks/prd-v2-valueobjects.md`) + +- All ValueObjects extend `ValueObject` (this directory) and are declared `readonly`: immutable, constructed once, never mutated in place. +- `jsonSerialize()` (required by `ValueObject`) must return the exact API-compatible array — key names, nesting, and presence/omission of optional keys must match the OpenAPI example byte-for-byte. `toArray()` is a named alias for the same thing. +- Validate in the constructor, not in a separate step — an invalid ValueObject should be impossible to construct. +- Prefer a `with*()` method over a public setter for any change to an existing instance (see `LocalizedField::withLocale()` for the pattern) — it returns a new instance, the original is untouched. +- Non-trivial request types get a companion `*Builder` (e.g. `IndexCreateRequestBuilder`, `ProductBuilder`, `QueryConfigurationRequestBuilder`, `SearchSettingsRequestBuilder`, `FieldDefinitionBuilder`, `SearchFieldConfigBuilder`) so callers can assemble a payload incrementally instead of a single large constructor call. +- Response-side types (`Response/`) are parsed the other direction: a `fromArray()` (or equivalent named constructor) instead of `jsonSerialize()`. Harden these against malformed/partial API responses — server responses are not as tightly controlled as the requests we build ourselves, and a response type should degrade gracefully rather than throw on an unexpected shape. + +## Directory map + +| Directory | Covers | +|---|---| +| `BulkOperations/` | Index/update/delete product payloads sent to the bulk-sync endpoint | +| `Index/` | Index creation: field definitions, field types, variant attributes, search analysis | +| `Search/` | Query configuration (boost algorithm, match mode, multi-word operator, field config) | +| `SearchSettings/` | The larger per-application search-behavior document: query/scoring/response config, highlighting, multi-match, function-score, variant enrichment | +| `Synonym/` | Synonym configuration | +| `Normalize/` | Field-value normalization requests | +| `Product/` | Shared product-level value types (pricing, image URLs) | +| `Response/` | Parsed API responses for all of the above | +| `Common/` | Cross-cutting helpers — currently `LocalizedField` (see root `CLAUDE.md`'s locale-suffix contract) | + +## Known trap: dual-shape parsing + +At least one config type (synonym groups) can arrive from the API as either a comma-separated string or an array of strings, and both forms must normalize to the same internal representation. When a ValueObject accepts more than one input shape for the same concept, keep the normalization in one shared place (see `Synonym/NormalizesSynonymGroups.php`) rather than duplicating the branch logic — a fix applied to only one branch is a recurring source of bugs here. + +## Adding or changing a ValueObject + +Don't do this in isolation — follow the full lockstep checklist in the root `CLAUDE.md` (OpenAPI spec first, then ValueObject, builder, fixture, tests). A ValueObject change that isn't backed by a fixture is not verified, no matter how correct it looks.