Json patch, initial pr, structures only#1525
Json patch, initial pr, structures only#1525suarezrominajulieta wants to merge 7 commits intomasterfrom
Conversation
|
Hi @suarezrominajulieta, first ask for my review and then, when I aprove the PR, I will request @arcuri82 's review. |
|
Hi @jgaleotti ! Okey, done :) |
| * Provides heuristics that extract valid JSON Pointer paths from a resource schema | ||
| * and group them by field type to construct type-safe path-value pairs. | ||
| */ | ||
| object JsonPatchGeneBuilder { |
There was a problem hiding this comment.
Why it is saying that it provides "heuristics"? This class deterministically creates an JsonPathGene from a given schema
| resourceSchema: Gene? | ||
| ): ArrayGene<ChoiceGene<JsonPatchOperationGene>> { | ||
|
|
||
| val allPaths = JsonPatchGeneBuilder.extractAllPaths(resourceSchema) |
There was a problem hiding this comment.
what happens if resourceSchema is null?
| val choices = mutableListOf<JsonPatchOperationGene>() | ||
|
|
||
| // Operations that only need a path | ||
| choices.add(JsonPatchPathOnlyGene("remove", pathEnum.copy() as EnumGene<String>)) |
There was a problem hiding this comment.
replace "remove" and other strings with constants
|
|
||
| // Operations that need path + value; pairs are grouped by schema field type | ||
| val pathValueEntries = JsonPatchGeneBuilder.buildPathValueEntries(allPaths) | ||
| val effectiveEntries: List<PairGene<EnumGene<String>, Gene>> = |
There was a problem hiding this comment.
what does effectiveEntries mean? Shouldn't this list be called pathValuePairGenes?
| val fromGene: EnumGene<String>, | ||
| val pathGene: EnumGene<String>, | ||
| geneName: String = "${operationName}Op" | ||
| ) : JsonPatchOperationGene(geneName, operationName, listOf(fromGene, pathGene)) { |
There was a problem hiding this comment.
check that the operation name is move or copy. Why does it add the "Op" ?
| operationsArr: ArrayGene<ChoiceGene<JsonPatchOperationGene>> | ||
| ) : CompositeFixedGene(name, listOf(operationsArr)) { | ||
|
|
||
| constructor(name: String, resourceSchema: Gene? = null) |
There was a problem hiding this comment.
move this logic to JsonPatchDocumentGeneBuilder
| * Provides heuristics that extract valid JSON Pointer paths from a resource schema | ||
| * and group them by field type to construct type-safe path-value pairs. | ||
| */ | ||
| object JsonPatchGeneBuilder { |
There was a problem hiding this comment.
Do not include the schema extraction heuristics in this PR
| class JsonPatchPathOnlyGene( | ||
| operationName: String, | ||
| val pathGene: EnumGene<String>, | ||
| geneName: String = "${operationName}Op" |
There was a problem hiding this comment.
why do we have operationName and geneName?
There was a problem hiding this comment.
I'm not sure what I should implement. Could you explain again what we were expecting here?
I created the operationName property, which inherits from the abstract class, to verify and distinguish between a pathfrom that is a move and a pathfrom that is a copy, and to ensure that each is printed correctly.
And then there's the geneName or name, which should be the name of the gene, which can be anything, like "Gene1".
I'm wondering if the child classes should only have operationName and not name, or if we should keep both, but make them exactly the same as those defined by the parent class in the inheritance.
| targetFormat: OutputFormat?, | ||
| extraCheck: Boolean | ||
| ): String { | ||
| val activePair = pathValueChoice.activeGene() |
There was a problem hiding this comment.
same as previous comments
2 participants
Summary
Introduces the foundational gene structure and builder needed to model JSON Patch
New files
Gene model (
core/src/main/kotlin/.../search/gene/patch/)JsonPatchOperationGene— enum-backed gene representing a single RFC 6902 op (add,remove,replace,move,copy,test)JsonPatchPathOnlyGene— composite gene for ops that only require apathfield (remove)JsonPatchPathValueGene— composite gene for ops that requirepath+value(add,replace,test)JsonPatchFromPathGene— composite gene for ops that requirepath+from(move,copy)JsonPatchDocumentGene— top-level gene representing a full JSON Patch document (array of operation objects); handles serialization to JSON and mutation logicBuilder (
core/src/main/kotlin/.../problem/rest/builder/)JsonPatchGeneBuilder— constructs aJsonPatchDocumentGenefrom an OpenAPI schema, mappingPATCHrequest body fields to the appropriate sub-gene variantTests (
core/src/test/kotlin/...)GeneSamplerForTestsandGeneNumberOfGenesTestupdated to include new gene typesWhat's NOT included yet
RestActionBuilderV3(wiring the builder into the main action pipeline)Notes
This is a structural PR — all new classes compile and their unit tests pass, but the genes are not yet invoked during actual fuzzing runs.