fix: correct the getEvents, getColliNo and getDocument request envelopes - #2
Merged
Merged
Conversation
All three read methods were built with an envelope SUUS does not accept, so every
one of them answered PRJ000001 ("no order found for the given waybill number or
reference") for orders that demonstrably exist. addOrder was unaffected, which is
what hid this: shipments booked correctly and only the read side was dead. Labels
could never be downloaded, tracking returned zero events for every shipment, and
getColliNumbers() returned an empty array.
Reported against 1.0.0 from a live production integration, confirmed here against
the WS PK 1.0 specification (sections 5.2-5.4) and verified end to end on the
sandbox.
Envelope fixes
* getEvents and getColliNo now send the documented <shipments><shipment> wrapper.
Both take an ArrayOfShipments, never a bare <shipmentNo>; sent flat the shipment
list arrives empty and SUUS reports the order as not found.
* getDocument now names the document symbol <document>, not <documentType>. SUUS
saw no document symbol and answered PRJ000001, which reads as "order not found"
and points every investigation at the order rather than at the request.
* ResponseParser::colliNumbers() read one level too high. The <colliNo> element in
a getColliNo response is an ArrayOfColli wrapper holding <colli><colliNo> leaves;
reading the wrapper concatenated every child, so a six-package shipment yielded
one run-together string instead of six numbers. A single-package shipment came
out correct by accident, which kept this latent.
Error surfacing
* fetchStatus() and getColliNumbers() no longer swallow SUUS errors. A success=false
response raised nothing and came back as an empty result, so a rejected request
was indistinguishable from a shipment with no events yet. Both now raise
SuusApiException with the return code and description, as fetchDocument() already
did. All read methods share one assertSuccess() path.
New capability
* fetchDocument() accepts an array of colli numbers to request the label for one or
several specific packages rather than the shipment's whole set. Verified on the
sandbox: 3 colli = 132469 B, 2 colli = 88589 B, 1 colli = 44705 B.
* fetchLoadingList(string $masterNo) - loadingList is the one document keyed by the
master waybill number rather than by shipment, and could not be requested at all.
* Reference-keyed variants of every read call, since an integration usually holds
its own reference rather than the SUUS waybill number and the spec treats the two
as interchangeable: fetchStatusByReference(), fetchDocumentByReference(),
getColliNumbersByReference().
Tests
* Regression tests for all three envelopes and for the colli leaf parsing.
* tests/Fixtures/get_colli_numbers_response.xml rewritten to the real nested shape
from the spec; the previous fixture was invented flat and masked the parser bug.
* Integration coverage for the read side: the suite creates a three-package order
and reads its colli numbers and documents back. That is the only way to tell
"SUUS cannot find this order" apart from "SUUS could not read the request", since
both answer PRJ000001. This is the test whose absence let the bug ship.
Two API behaviours found while verifying and now documented
* getEvents lags addOrder. SUUS registers the first event (J_CR) asynchronously a
few minutes later, so a just-created order legitimately answers PRJ000001 for a
while. Combined with the envelope bug this is why the failure looked like a
sandbox limitation rather than a defect.
* Colli numbers do not come back in a stable order between calls, so the result of
getColliNumbers() is a set. Never map a colli number to a package by index.
No signature is broken: every new parameter is optional and appended.
Verified: 370 unit tests / 587 assertions green; 6 sandbox integration tests /
29 assertions green; PHPStan level 8 clean.
The additions (fetchLoadingList, the colli-number argument on fetchDocument and the reference-keyed read variants) are all optional and appended, so nothing downstream has to change to take this.
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
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.
All three read methods were built with an envelope SUUS does not accept, so every one of
them answered
PRJ000001("no order found for the given waybill number or reference") fororders that demonstrably exist.
addOrderwas unaffected, which is what hid this:shipments booked correctly and only the read side was dead. Labels could never be
downloaded, tracking returned zero events for every shipment, and
getColliNumbers()returned an empty array.
Reported against 1.0.0 from a live production integration, confirmed against the WS PK 1.0
specification (sections 5.2-5.4) and verified end to end on the sandbox.
Evidence
Old vs new envelope, same shipment, same account, seconds apart:
getEvents<shipmentNo>PRJ000001getEvents<shipments><shipment>CWS0001, eventJ_CRgetColliNo<shipmentNo>returnCodegetColliNo<shipments><shipment>CWS0001, 3 colligetDocument<documentType>PRJ000001getDocument<document>CWS0001, 176 628 B base64, valid PDFEnvelope fixes
getEventsandgetColliNonow send the documented<shipments><shipment>wrapper.Both take an
ArrayOfShipments, never a bare<shipmentNo>; sent flat the shipment listarrives empty and SUUS reports the order as not found.
getDocumentnow names the document symbol<document>, not<documentType>. SUUSsaw no document symbol and answered
PRJ000001, which reads as "order not found" andpoints every investigation at the order rather than at the request.
ResponseParser::colliNumbers()read one level too high. The<colliNo>element in agetColliNoresponse is anArrayOfColliwrapper holding<colli><colliNo>leaves;reading the wrapper concatenated every child, so a six-package shipment yielded one
run-together string instead of six numbers. A single-package shipment came out correct by
accident, which kept this latent.
Error surfacing
fetchStatus()andgetColliNumbers()no longer swallow SUUS errors. Asuccess=falseresponse raised nothing and came back as an empty result, so a rejected request was
indistinguishable from a shipment with no events yet. Both now raise
SuusApiExceptionwith the return code and description, as
fetchDocument()already did.New capability
fetchDocument()accepts an array of colli numbers, for the label of one or severalspecific packages rather than the shipment's whole set. Verified on the sandbox:
3 colli = 132 469 B, 2 colli = 88 589 B, 1 colli = 44 705 B.
fetchLoadingList(string $masterNo)-loadingListis the one document keyed by themaster waybill number rather than by shipment, and could not be requested at all.
reference rather than the SUUS waybill number and the spec treats the two as
interchangeable:
fetchStatusByReference(),fetchDocumentByReference(),getColliNumbersByReference().Tests
tests/Fixtures/get_colli_numbers_response.xmlrewritten to the real nested shape fromthe spec; the previous fixture was invented flat and masked the parser bug.
its colli numbers and documents back. That is the only way to tell "SUUS cannot find this
order" apart from "SUUS could not read the request", since both answer
PRJ000001. Thisis the test whose absence let the bug ship.
Two API behaviours found while verifying, now documented
getEventslagsaddOrder. SUUS registers the first event (J_CR) asynchronously afew minutes later, so a just-created order legitimately answers
PRJ000001for a while.Combined with the envelope bug, this is why the failure looked like a sandbox limitation
rather than a defect.
getColliNumbers()is a set. Never map a colli number to a package by index.Compatibility
No signature is broken: every new parameter is optional and appended.
Verification
370 unit tests / 587 assertions green; 6 sandbox integration tests / 29 assertions green;
PHPStan level 8 clean.