Settle the three decisions a 1.0 tag would freeze - #119
Merged
Conversation
`Operation` was half an input type and half an output type: its constructor is public, but `Contract` is built only from documents and the validators are internal, so a hand-built operation could be read back and asked for `responseFor()` and nothing else. Mark the constructor `@internal` and keep the class `@api` as the compiled read model consumers actually use, and declare the variance of `CompiledParameter` while it is still free — read-only for consumers, open to new keys in a minor. `allowReserved` is documented as what it is: a hand-off to consumers that render query values and cannot derive it from the schema, not an annotation nobody reads. The byte budgets were constants with no way to raise them, and a body over one was reported as a violation — accusing a message the validator had merely declined to read, so a gate on `isValid()` rejected traffic it never judged. `Limits` makes the policy the caller's, on the factories rather than on three signatures, so later knobs land on the object; `too_large` keeps its code and its two-state result and is documented as a refusal to look.
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: Warning Your free Security trial is over. An organization admin can upgrade to Advanced for continuous pull request security review or dismiss this notice. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Fixes #116
Fixes #117
Fixes #118
Three decisions that cost nothing today and a major release after 1.0.
Operationis an output type (#116). Its constructor was public, butContractis built only from documents and the validators are internal — a hand-built operation could be read back and asked forresponseFor(), and nothing else. The constructor is now@internal; the class stays@apias the compiled read model consumers actually use. The alternative (a public entry taking aMatchedOperation) was rejected:requestBody/responsesarearray<array-key, mixed>, so it would promise fail-closed behaviour over input the package never validated.CompiledParameterdeclares its variance (#117). Read-only for consumers, open to new keys in a minor.allowReservedis documented as what it is — a hand-off to consumers that render query values and cannot derive it from the schema — rather than as an annotation nobody reads. Validation still never reads it, and the docs now say why: after a message is parsed, an encoded reserved character cannot be told from an unencoded one.Budgets belong to the caller (#118).
Limits(documentBytes, messageBodyBytes, documentFiles)is accepted by all three factories as an optional last argument;Contract::MAX_DOCUMENT_BYTES/MAX_MESSAGE_BODY_BYTESgive way toLimits::DEFAULT_*at the same values. A budget below 1 is refused.The semantic half matters more than the knob.
*.body.too_largeaccused a message the validator had merely declined to read, so a gate onisValid()rejected traffic it never judged — and a thirdValidationResultstate cannot be added after 1.0 without changing whatisValid() === falsemeans for every consumer. This keeps the two-state result and the codes, and makes the other half honest: the budget is the caller's, and the code says "we did not look".roavereports four breaks (two constants, the@internalconstructor twice), all deliberate; CHANGELOG declares0.9.0overv0.8.0, which is the boundary the BC job checks against.Verification:
composer build,composer rector,bin/package-auditgreen; mutation 2149 killed / 148 escaped, MSI 93% against a gate of 92.