Repository navigation
Conversation
A filter declared on a property inside a root-level then/else branch is copied onto the root property by cloneTransferredProperty() and runs unconditionally, even while the branch is inactive. The test asserts the correct behaviour and currently fails (an inactive else filter still trims the value). The fix direction is still to be decided. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GXn5VCTemp5rnjGrXMMvJs
… anyOf/allOf rejection The filter of a property declared inside a root-level composition branch currently runs for every input because the transferred copy on the parent class keeps the FilterValidator. The new FilterCompositionBranchScopeTest asserts the intended behaviour: a filter only affects the value while its branch is active (if/then/else, oneOf, including a transforming filter), and a filtered property declared directly in an anyOf or allOf branch is rejected at generation time. These tests are expected to fail until the implementation lands. Tests that pinned the old behaviour are adjusted: - the playlist test now expects the title to stay untouched while its then branch is inactive - the nested-compositions schema no longer filters the anyOf branch property (the original shape is covered as a rejection) - the accepted row for a filtered property in a root allOf branch moves to the rejection tests Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GXn5VCTemp5rnjGrXMMvJs
Pins what works today as regression guards: a property-level anyOf with a single object branch (the shape accepted by the static tests), and property-level anyOf (merged class) and oneOf with several object branches, where only the matching branch may filter the value. Adds the property-level conditional case, which goes through the same branch transfer into a parent class as a root-level conditional and therefore shows the same leak; it fails until the implementation lands. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GXn5VCTemp5rnjGrXMMvJs
A single filtered property in an allOf branch works today and keeps working (new regression test). allOf is rejected only when a property is filtered more than once (several branches, or the parent schema plus a branch), because the first filter wins and the others are silently dropped. A filtered property in an anyOf branch is rejected. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GXn5VCTemp5rnjGrXMMvJs
A oneOf branch could own a filtered property, but its value would have to be forwarded through the enclosing anyOf, where several branches can match. It is rejected with the same message as a filter declared in an anyOf branch. Fails until the implementation lands. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GXn5VCTemp5rnjGrXMMvJs
…arker and branch key map FilterValidator can now return a copy which still describes the filter but is not rendered into the validation code, so a property which receives the filtered value from the branch owning the filter still exposes the filter to the output-type and serialization post processors. Composed validators expose getBranchFilteredKeyMap(), the properties for which the active branch of a oneOf or if/then/else (or a value forwarded through an allOf) hands the filtered value over to the containing schema. No generated code changes yet; the runtime consumption follows. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GXn5VCTemp5rnjGrXMMvJs
…ranch A filter on a property of the if condition keeps running inside the condition but must not be applied to the parent. A property which has its own filter and is filtered again in a then or oneOf branch is rejected like the allOf duplicates; the duplicate-filter message is unified. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GXn5VCTemp5rnjGrXMMvJs
The filter of a property declared in a then, else or oneOf branch ran on the parent class for every input because the transferred copy kept the executing FilterValidator. The branch class now owns the filter and the active branch hands the filtered value to the parent: - the transferred copy keeps a non-executing description of the filter - the compositions publish the filtered values of the active branch from the modified values they already compute (also through an allOf) - a substitution validator takes the published value in front of the other validators of the property; without a published value the value stays as provided - when a setter switches the active branch, dependent properties are validated again - a filter on a property of the if condition keeps running inside the condition but is neither applied to the parent nor published Transforming filters (serializer type guard) and the generation time rejections follow. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GXn5VCTemp5rnjGrXMMvJs
A property whose transforming filter is owned by a then, else or oneOf branch may hold the value as provided (branch inactive) or the transformed value: - the generated serializer only serializes a value of the output type of a filter and returns any other value as it is - the output type of the property is the raw type plus the types returned by the filters of its branches, also when then and else transform the same property with different filters - the transferred copy of a branch property carries the widened output type so merging the copies of several branches keeps the raw type Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GXn5VCTemp5rnjGrXMMvJs
A filtered property declared in a branch of an anyOf is rejected: several branches can match, so the filtered value cannot be attributed to a single branch. This includes a oneOf branch below an anyOf. A property which is filtered more than once by declarations that can apply to the same value is rejected as well: the schema itself plus a branch, several allOf branches or the branches of different compositions. The filters have no defined order and only the first declaration used to take effect while the others were silently dropped. The branches of one oneOf or if/then/else exclude each other and may filter the same property differently. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GXn5VCTemp5rnjGrXMMvJs
Replaces the note that a filter of a conditional branch property also runs while the branch is inactive with the rules of the filter scope: which compositions can own a filtered property, the if condition, the rejection of anyOf and of a property which is filtered more than once, and transforming filters (type hints and serialization). Adds notes to the anyOf, allOf and if pages. Adds tests for a conditional nested in a oneOf branch, a oneOf nested in a then branch and two compositions which filter the same property. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GXn5VCTemp5rnjGrXMMvJs
Coverage Report for CI Build 38052883874Coverage increased (+0.02%) to 98.907%Details
Uncovered Changes
Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
- Keep the filter declarations of the branches in a WeakMap keyed by schema and read the filters of the schema itself fresh on every check (deferred while the property is unresolved), so a property declared via $ref is covered and a recycled object id can't inherit foreign declarations - Share the output type of a branch-owned transforming filter between the transfer step and the output type post processor - Compute the filtered key map once per render step - Try the serializer of a filter without a return type last so it can't hide the serializers of the other filters - Add isAllOf()/isAnyOf() and FilterValidator::of() instead of repeating the checks Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GXn5VCTemp5rnjGrXMMvJs
- A filtered property in an anyOf is accepted when every branch declares it with the same filters: the outcome doesn't depend on the matching branch. Any other filtered property in an anyOf branch is still rejected. - The compositions stage the filtered values of the active branches and the constructor, setters and populate() adopt them only after the update succeeded (dependent properties are validated again at that point). A failing setter or populate() no longer leaves the values of the branch it tried to activate behind. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GXn5VCTemp5rnjGrXMMvJs
Drop the then/oneOf-overrides-root rows of the filtered-more-than-once provider (the same assertion path as the allOf rows), the positive branch-filtered key map tests (covered by the runtime tests) and two FilterValidator unit tests that the integration tests already cover. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GXn5VCTemp5rnjGrXMMvJs
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.
Summary
Stacked on #192 (base branch
claude/sweet-wright-0c9raw, GitHub retargets tomasteronce #192 is merged).A filter on a property declared in a
then,elseoroneOfbranch ran on the generated parent class for every input, because the transferred copy of the branch property kept the executingFilterValidator. #192 documents this as a known limitation; this PR fixes it.Behaviour
if/then/else,oneOf: the branch class owns the filter. The active branch hands its filtered value to the parent (also through anallOfand through nested compositions); while the branch is inactive the property keeps the value as provided. A setter which switches the active branch validates the dependent properties again.populate()). A failing setter orpopulate()leaves the model, including the properties which depend on the active branch, untouched (also in collect-errors mode).ifcondition: the filter keeps running inside the condition, but it is neither applied to the parent nor handed over.thenandelsemay use different transforming filters on one property.anyOf: several branches can match, so a filtered property is only accepted when every branch declares the property with the same filter (the outcome does not depend on the matching branch, the filter keeps being applied to the property as before). Every other filtered property in ananyOfbranch is rejected with aSchemaException, including aoneOfbranch nested below ananyOf.SchemaException): a property which is filtered more than once by declarations that can apply to the same value (the schema itself plus a branch, severalallOfbranches, or the branches of different compositions). Until now only the first filter took effect and the others were silently dropped.then/elseand the branches of oneoneOfexclude each other and may filter the same property differently.allOfbranch and filters on object branches of property level compositions that are not merged into a parent class (singleanyOfobject branch, merged class,oneOf) are unchanged and pinned by tests.Implementation
FilterValidator::withoutExecution()is a non-executing copy that still describes the filter, so the output type and serialization post processors keep working.AbstractComposedPropertyValidator::getBranchFilteredKeyMap()/publishesFilteredValueOf()decide which branch properties hand a value over. The compositions publish the filtered values from the$modifiedValuesthey already compute and stage them (_stagedBranchFilteredValues);_commitBranchFilteredValues()moves them into the internalbranchFilteredValuesholder (keyed by property name) from the constructor / setter /populate()after hooks. ABranchFilteredValueValidatortakes the value in front of the other validators of the property.SchemaProcessorconverts the transferred copy (marker for publishing branches, filter dropped for theifcondition), adds the substitution to the property which is registered after the merge (sincePropertyMergerdrops the validators of a later copy) and rejects ambiguous filters.Behaviour change
A value in an inactive branch is no longer filtered, and schemas which filter a property in an
anyOfbranch (unless every branch filters it in the same way) or more than once now fail at generation time instead of silently dropping filters. Worth a release note.Changes to tests of #192
thenbranch is inactive (it previously pinned the leak).anyOfbranch property (now rejected); the original shape is covered as a rejection test.allOfbranch moved to the rejection tests (parent filter plus branch filter).Documentation
filter.rst("Filters on properties of object branches" rewritten, the "also runs while inactive" warning removed), notes with links on theanyOf,allOfandifpages. The docs were not built locally (Sphinx is not installed in the sandbox).Test plan
Developed test-first (the commits show it): the failing tests came first, then building blocks, runtime, transforming filters, rejections and docs; a review round added the atomicity and
anyOffollow-ups.ReferencePropertyTestcases which fetchraw.githubusercontent.com(blocked in the sandbox)phpcs --standard=phpcs.xmlon all changed PHP files: cleanFilterCompositionBranchScopeTest(31 tests),BranchFilteredKeyMapTest,FilterValidatorTest🤖 Generated with Claude Code
https://claude.ai/code/session_01GXn5VCTemp5rnjGrXMMvJs