Fix B7: render deduplication and RenderJob cleanup#154
Conversation
|
This PR addresses the B7 feedback from PR #150. I am an AI assistant working on behalf of @torian257x.
Fixed. The class_exists guard was removed from RenderJob::render(). Dedup is now handled at the correct level in RenderQueue::addRenderJob(), which uses Schema object identity to distinguish cached reuse from genuine duplicates.
Added testTransferredPropertyJsonPointerIsCorrect in B7RenderDedupTest.php, which asserts the transferred property has #[JsonPointer("/allOf/0/properties/age")]. |
|
Updated with tests that actually fail on master and pass on this branch. Bug #1 discovered: Proving test:
Bug #2 discovered: RenderQueue had no dedup, and RenderJob's All 2741 tests pass with these fixes applied (8 pre-existing failures unrelated to these changes). |
…ueue dedup PropertyAttributeSynthesizer now uses the composition branch pointer + property name (instead of the original branch property's JsonSchema pointer, which points to the $defs location for $ref'd branches) when generating branch pointers. This fixes transferred properties from $ref'd allOf/anyOf branches getting the wrong JsonPointer (e.g. /$defs/Extra/properties/age instead of /allOf/0/properties/age). RenderQueue::addRenderJob() deduplicates by target filename + content signature. Same Schema object via cache → skip silently. Different Schema, same content (e.g. multiple code paths reaching the same schema file) → skip silently. Different Schema + different content with same filename → throw FileSystemException (duplicate $id). RenderJob exposes getSchema() so the queue-level dedup can compare signatures; the class_exists guard remains removed.
c09786e to
65e5e2a
Compare
Summary
How addRenderJob dedup works
What the maintainer asked
"B7 silent skip in RenderJob looks more like working on symptoms rather than fixing issues on the root."
Response: The class_exists guard was symptom-masking. Dedup is now at the correct level (addRenderJob). The file_exists check in render() remains as safety net.
"The patch of cloneTransferredProperty looks reasonable but it's not covered by a test case."
Response: Added testTransferredPropertyJsonPointerIsCorrect in B7RenderDedupTest.php.
Changes