refactor(woocommerce): replace inline SDK with automattic/woocommerce composer dependency (#264)#267
Open
Avatarsia wants to merge 30 commits intoOpenXE-org:masterfrom
Open
Conversation
Avatarsia
pushed a commit
to Avatarsia/OpenXE
that referenced
this pull request
Apr 21, 2026
Adds the three WooCommerce PRs that are currently open against openxe-org/openxe to the nightly production manifest: - fix/woocommerce-order-import-pagination (PR OpenXE-org#265) — foundation - feature/woocommerce-batch-stock-sync (PR OpenXE-org#266) — independent - refactor/woocommerce-composer-sdk (PR OpenXE-org#267) — stacked on OpenXE-org#265, listed after it so the sequential merge in Pass 2 picks the dependency up first and the composer refactor applies on top cleanly. Grouped under a dedicated header block so the intent stays visible once the PRs merge upstream and can be removed from the manifest.
Adds getHeaders() / getHeader() accessors to the inline WCResponse class and captures HTTP response headers case-insensitively via CURLOPT_HEADERFUNCTION. Required foundation for pagination handling (Issue OpenXE-org#262).
Exposes the underlying WCResponse of the most recent request so callers can read response headers (X-WP-Total, X-WP-TotalPages) without changing the existing JSON-body return contract. Follow-up to 291197d, required by the pagination work in issue OpenXE-org#262.
Reads felder.letzter_import_timestamp from shopexport.einstellungen_json with a 30-day fallback for first runs, and adds a persistLastImportTimestamp() helper that does a read-modify-write via DatabaseService named params. Infrastructure for the pagination loop in issue OpenXE-org#262; not yet called here.
Replaces the fake greater-than-id filter (800 hardcoded IDs) with the WC v3 after=<iso-8601> parameter and walks X-WP-TotalPages up to MAX_PAGES_PER_RUN=5 pages per run (500 orders). Persists a progress timestamp via persistLastImportTimestamp() after each processed order so aborted runs resume cleanly. Adds a one-shot ab_nummer->timestamp translation for existing shops transitioning from the legacy cursor. Fixes silent data loss when more than 20 orders arrived between runs. Issue OpenXE-org#262.
Captures lastImportTimestamp into a local variable before the pagination
loop so progress persistence inside the loop does not mutate the GET
filter. Without this, after=\$lastTs moves forward each iteration while
page advances too, causing 100 orders per extra page to be skipped.
Also fixes two smaller issues:
- resolveAbNummerToTimestamp() returns ts-1 so the strictly-after
filter does not lose the transition order.
- explode(';', \$this->statusPending) is now PHP 8.1+ safe via (string)
cast.
Follow-up to abe58aa, addresses code review findings on issue OpenXE-org#262.
Captures scope, fix parameters (MAX_PAGES_PER_RUN=5, 30-day first-run fallback, UTC timestamps), implementation steps, integration test matrix T1-T10, rollout and rollback strategy, risks and mitigations. Companion doc to issue OpenXE-org#262 and the fix commits on this branch.
$this->app->DatabaseService is only lazy-bound in the web context. When the shopimporter runs through the cron trigger the service is not available, which breaks the timestamp persistence path. Falls back to $this->app->DB with real_escape_string when DatabaseService is absent. Discovered during the WC 8.9.3 + 10.7.0 integration test matrix on the .143 test instance.
WCHttpClient::authenticate() had isQueryStringAuth() smuggled into its SSL-gating during f12b09a. That changed the auth scheme for existing HTTP-configured shops from OAuth 1.0a to basic-auth-over-query-string (since query_string_auth=true is set at client construction site). Restores the pre-f12b09a4 behaviour. The CLI-context fallback for persistLastImportTimestamp from f12b09a is kept. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The caller in shopimport.php uses $result[0] per iteration of a for loop capped by ImportGetAuftraegeAnzahl() and maxmanuell. Returning 500 orders per call therefore silently dropped 499 of them while advancing the server-side after-cursor past them. Restores the historical 1-order contract; the after-filter still replaces the legacy 800-id include hack, and per-order persist gives us resume-after-crash semantics with at most one order lost per crash (consistent with pre-OpenXE-org#262 behaviour). MAX_PAGES_PER_RUN and ORDERS_PER_PAGE constants are removed; the caller loop (bounded by maxmanuell, default 100) now owns the batch size. Follow-up to review on OpenXE-org#262. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
After the external review of OpenXE-org#262 highlighted that shopimport.php expects $result[0] per RemoteGetAuftrag iteration, the internal pagination loop was dropped. Plan now reflects: single-order per call, caller loop bounded by maxmanuell, per-order progress persist. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… when nothing to import shopimport.php:1308 gates on is_array($result), which happily accepts [] and then crashes trying to dereference $result[0]['id']. The pre-OpenXE-org#262 behaviour returned null on empty — restore that. Spotted by review of 7af9edb.
The defensive getKonfig($data['shopid'] ?? null, $data) call inside
ImportGetAuftrag() is actively harmful: CatchRemoteCommand('data')
returns the getauftrag-payload, which does not carry a shopid (see
class.remote.php:194/241). The re-init therefore clears $this->shopid
and rebuilds the WCClient from empty preferences. RemoteCommand()
already initialises the importer with the real shop id before dispatch
(class.remote.php:2685), so the duplicate call is both redundant and
broken. Spotted by re-review of OpenXE-org#262.
shopimport.php calls RemoteGetAuftraegeAnzahl() before RemoteGetAuftrag() in its main flow. If the stored cursor is still the 30-day fallback and all pending orders are older than 30 days, the count returns 0 and ImportGetAuftrag() never runs, so the one-shot ab_nummer -> timestamp migration never fires and the shop stays on the fallback forever. Extract the migration into a private helper and invoke it from both count and fetch paths. Idempotent via lastImportTimestampIsFallback.
The after-filter is strictly-greater-than, so orders sharing an identical date_created_gmt with the last processed order were silently dropped. Move to a tuple cursor: persist both timestamp and order id, query with after=<ts-1s> plus exclude=[last_id]. Orders with the same GMT second now reach the caller in subsequent iterations without duplicating the already-processed one. Schema is additive (new felder.letzter_import_order_id key in shopexport.einstellungen_json). Persistence helper becomes persistLastImportCursor; the single-argument persistLastImportTimestamp remains as a wrapper so the ab_nummer migration path keeps working without a second rewrite.
…ration Reflects re-review findings: tuple cursor (ts, id) for same-second order resilience, migration helper called from both count and fetch paths, scope list updated to match current single-order design.
…s offset Bei identischem date_created_gmt mehrerer Orders hielt exclude nur die zuletzt importierte ID. Nach zwei Orders im selben Bucket wurde die erste wieder sichtbar und Count- wie Fetch-Pfad liefen in eine Endlosschleife. Cursor persistiert jetzt die komplette Liste aller IDs innerhalb des aktuellen Sekunden-Buckets (felder.letzter_import_order_ids als JSON- Array). Bei Bucket-Wechsel wird die Liste zurueckgesetzt; bei gleichem Bucket wird die neue ID angehaengt. Gleichzeitig wird die -1s-Korrektur am Query gated: nur wenn mindestens eine exclude-ID bekannt ist, wird der after-Filter um 1 Sekunde nach hinten verschoben. Dadurch entfaellt die Doppel-Subtraktion nach der ab_nummer-Migration (resolveAbNummerToTimestamp schon -1s, Query war nochmal -1s -> 2s zurueck in der Vergangenheit). Der Erstlauf nach Migration liefert jetzt exakt die ab_nummer-Order als Startpunkt. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Avatarsia
force-pushed
the
refactor/woocommerce-composer-sdk
branch
2 times, most recently
from
a096d34 to
ded8846
Compare
…ails If resolveAbNummerToTimestamp() cannot find the referenced legacy order (404, missing date_created_gmt, etc.) the migration previously left the importer on the volatile 30-day fallback, which is recomputed on every run as now()-30d. The cron cycle would then re-scan the same rolling window forever, multiplying API load and caller-dedup activity. On resolution failure we now explicitly persist the current fallback timestamp so the fallback flag flips to false and the lower bound stays stable across runs. Also emits a warning so the operator can spot the stale ab_nummer. Spotted by review of OpenXE-org#265.
Previously, stock sync ran two HTTP requests per article (SKU lookup +
update). For n articles this was 2n roundtrips; at 1000 articles that
is 2000 requests, easily tripping hoster rate limits and aborting the
sync partway through.
Switch to the official WC REST v3 batch endpoints:
- Collect all SKUs up front, resolve them in one or a few
products?sku=<csv>&per_page=100 lookups (map sku -> id).
- Send stock updates in chunks of up to 100 items via
POST products/batch.
- Variations go through POST products/{parent}/variations/batch,
grouped per parent product.
Partial errors in a batch response are logged per SKU without aborting
the rest of the sync. At 1000 articles this reduces request count
from 2000 to roughly 15-20.
WCClient::post() accepts array data and JSON-encodes it directly --
no new postBatch() helper needed.
Closes OpenXE-org#263.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Companion doc to issue OpenXE-org#263 and the batch-refactor commit on this branch. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… level processBatchResponse()-Aufrufe sind jetzt in try/catch eingeschlossen — ein einzelner WC-Batch mit HTTP 5xx/Timeout bricht den Sync nicht mehr ab, nachfolgende Chunks werden trotzdem verarbeitet. Der Erfolgs-Log in processBatchResponse() nutzt jetzt ->info() statt ->error() (war falscher Log-Level). Spotted by review of b96079b.
ImportSendListLager() wraps the per-chunk batch POSTs in try/catch but the upstream SKU-to-ID lookup was bare: a single failing lookup chunk (timeout, 5xx, rate-limit) aborted the whole sync before the unaffected remaining chunks could even be processed. That gave the batch refactor a larger failure domain than the pre-OpenXE-org#266 per-item path it replaced. Lookup exceptions are now logged at error level and the loop continues with the next chunk. Items missing from the SKU map are already handled downstream (logged as not-found, skipped from the update batches). Spotted by review of OpenXE-org#266.
…ency Adds automattic/woocommerce 3.1.1 via Composer. Vendor files included per project convention (vendor/ is tracked). Resolves issue OpenXE-org#264. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…m client The upstream automattic/woocommerce SDK lacks two accessors required by the OpenXE business logic: - Client::getLastResponse() (upstream only exposes $client->http->getResponse()) - Response::getHeader($name) with case-insensitive lookup Adds Xentral\Components\WooCommerce\ClientWrapper which wraps the upstream Client and exposes both methods. ResponseWrapper normalises all header keys to lowercase at construction time. ssl_ignore is mapped to the upstream verify_ssl option; logger is accepted but unused (no upstream hook). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Removes all inline SDK classes (WCClient, WCHttpClient, WCResponse, WCRequest, WCOAuth, WCBasicAuth, WCOptions, WCHttpClientException) from shopimporter_woocommerce.php — 1374 lines deleted. Business logic now uses: - new ClientWrapper(...) instead of new WCClient(...) - use Xentral\Components\WooCommerce\ClientWrapper - use Automattic\WooCommerce\HttpClient\HttpClientException All @throws and catch blocks updated to HttpClientException. Fixes OpenXE-org#264. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…diagnostics Constructor accepted a logger but discarded it, which removed the structured WooCommerce API error logs (4xx/5xx responses, failed requests) that the inline SDK used to produce. That was a regression for operational diagnostics. ClientWrapper now: - stores the PSR-3 logger passed in by the shopimporter, - logs HTTP >= 400 responses as warning (with a truncated body excerpt), - logs HttpClientException as error before rethrowing, - keeps the same public interface, no callsite changes needed. Spotted by review of e433b5a.
vendor/composer/installed.php referenced an outdated HEAD (84fdb97) from when the composer require was first run, not the final branch state. Regenerated via composer install so InstalledVersions:: getRootPackage() returns the correct revision and composer dump-autoload is a no-op on a fresh clone. Spotted by review of 1a61123.
added 3 commits
April 21, 2026 14:00
Upstream vendored code (e.g. vendor/automattic/woocommerce/composer.json) ships with formatting choices we do not control. Without this, git diff --check reports their trailing whitespace as if it were a review issue on our branches. Spotted by review of 1a61123.
This file's root-package reference is a post-facto artefact: it records whatever HEAD was when `composer install` ran, so any following commit puts it out of sync with the new HEAD. This sync-commit lets installed.php reference the last non-sync commit (f26fb82). Merging the branch will regenerate it against the merge commit automatically. There is no way to make it reference its own commit hash without a self-referencing cycle; if this becomes a CI concern later, vendor/composer/installed.php could be moved to .gitignore on the understanding that composer install runs at deploy time.
Post-rebase regeneration of the Composer root reference so InstalledVersions::getRootPackage() describes the current branch tip rather than the pre-rebase commit. Spotted by review of OpenXE-org#267.
Avatarsia
force-pushed
the
refactor/woocommerce-composer-sdk
branch
from
ded8846 to
2040194
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TL;DR
Ersetzt die eingefrorene 2019er-Inline-Kopie (1374 Zeilen) des WooCommerce-PHP-SDK durch
die offizielle Composer-Dependency
automattic/woocommerce:^3.1. Ein schmalerClientWrapperhält die in #265 genutzten Accessors (getLastResponse(), case-insensitives
getHeader()) verfügbar. Upstream-Patches — Security-Fixes,PHP-Kompatibilität — fließen künftig per
composer updateein.composer install --no-devauf dem Server ausgeführt werden, damit die neuenvendor/automattic/-Dateien und der Autoloader-Index auf den Merge-Commit-Standsynchronisiert werden.
Closes #264.
Detaillierte Analyse (LLM-generiert, Claude Opus 4.7)
Was geändert wurde
Hauptverschiebung: 1374 Zeilen Inline-SDK aus
www/pages/shopimporter_woocommerce.phpentfernt, ersetzt durch
automattic/woocommerceals Composer-Dependency.Neue Datei:
classes/Components/WooCommerce/ClientWrapper.php(NamespaceXentral\Components\WooCommerce) hält vier Aufgaben wach:getLastResponse()auf dem Client — upstream-SDK exponiert das nicht.getHeader($name)auf der Response-Wrapper-Klasse —upstream liefert nur
getHeaders()als Array.->warning(), Exception als->error(), Rethrow unverändert (damit Call-Sites ihren try/catch-Kontraktbehalten).
ssl_ignore-Mapping aufverify_ssl=falseder Upstream-Options.Die Businesslogik-Aufrufe (
$this->client->get/post/…,$this->client->getLastResponse()->getHeader(…)) bleiben unverändert — der Wrapperhält den Kontrakt zu allen Pagination- und Batch-Stock-Logiken aus #265/#266.
Commits (chronologisch auf dem Branch)
00707afe—composer require automattic/woocommercee433b5a7—ClientWrapper(207 Zeilen) angelegt1a611230— Inline-SDK entfernt, Namespace-Swapaf965010— Logger-Integration im ClientWrapper (Review-Fix Medium)ad804690,2cfc64c0— composer install metadata regenerate (Review-Fix Low)f26fb82b—.gitattributes:vendor/**vongit diff --check-Whitespaceausnehmen (Review-Fix Low)
Test-Ergebnisse
End-to-End gegen Docker-WC-10.7 + WP 6.9.4 + PHP 8.3 auf Testinstanz — alle 7 PASS:
date_created_gmt)Autoloader-Sanity verifiziert:
Automattic\WooCommerce\ClientundXentral\Components\WooCommerce\ClientWrapperbeide via Composer-PSR-4 auflösbar.Logger-Integration verifiziert:
Known Notes
vendor/composer/installed.php) referenzieren auf demBranch technisch immer nur den vorletzten Commit — das Merge-
composer installregeneriert die Datei auf den Merge-Commit. Selbstreferenz im Commit ist
mathematisch nicht möglich.
vendor/**ist via.gitattributesvongit diff --check-Whitespace-Prüfungenausgenommen (Upstream-Code, unter unserer Kontrolle nicht).
Abgrenzung zu parallelen PRs/Issues
Rollback
Revert in toto. Das Inline-SDK lebt in der Git-Historie weiter und kommt durch die
Reverts automatisch wieder rein. Einziger Nacharbeits-Step nach Rollback:
composer install --no-deverneut ausführen, um die altevendor/-Struktur zusynchronisieren.