Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add package block for content-aware JSON config diffing and patching #259

Merged
merged 23 commits into from
Aug 29, 2024

Conversation

hexaglow
Copy link
Contributor

@hexaglow hexaglow commented Aug 21, 2024

New package block can be used for diffing JSON-like config structures. Function Diff produces a slice of Patches, which can be applied to the original A structure to transform it into the B structure. Diff takes a list of Blocks, which define the boundaries withing the structure between changes.

By designing the Blocks to match the logical structure of the config, the user can apply patches to different config structures than were originally used to produce the Diff, with limited chance of conflict.

Package split implements diffing and patching of JSON-like structures,
where the boundaries between logical objects
are defined by schema.
Arrays can be diffed if they are arrays of maps. The splits must identify a key for the array, to track element identity.
There was a bug where if a nested field/element was deleted, then the top-level field containing it would be deleted instead.

Now a field/element is only deleted if it matches the final path element.
To make testing easier, output Patches from Diff sorted by their Path.
Perform a round-trip JSON encoding to convert arbitrary Go types into the working format for Diff and ApplyPatches.

ApplyPatch changed into ApplyPatches - because of the JSON encode/decode above, the operation is now more expensive and no longer in-place.
Therefore, allow providing multiple patches to apply at the same time.
Due to a shadowed variable, nil was being incorrectly returned as Patch.Value.

Add more tests to improve coverage.

Add testable examples to demonstrate usage of the package.
@hexaglow
Copy link
Contributor Author

@mattnathan I've separated this out from the original PR (#256) as discussed

mattnathan and others added 5 commits August 22, 2024 09:56
We decided it's a less confusing name, overall.

Rename type Split to Block.

Rename fields and parameters accordingly.
Add new string-based format for representing paths in Patch.

Old array-based format is still supported for parsing in JSON, but output will be in the new string-based format.
@hexaglow
Copy link
Contributor Author

@mattnathan I've added the new string-based XMLPath-like path format as discussed.

Use the Path wrapper type in Patch, so that the new string JSON serialisation format is used.
Make Path.MarshalJSON a value-receiver method,
so Patch.Path is serialised correctly.

Change ExampleDiff to show new path format.
Copy link
Contributor

@mattnathan mattnathan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm about half way through a review - most of path is reviewed and some of Diff. As there are a few bugs it'd be good to get them fixed and I'll finish the review later today or early next week.

pkg/block/block.go Outdated Show resolved Hide resolved
pkg/block/block.go Show resolved Hide resolved
pkg/block/block.go Outdated Show resolved Hide resolved
pkg/block/block.go Outdated Show resolved Hide resolved
pkg/block/block.go Outdated Show resolved Hide resolved
pkg/block/block.go Outdated Show resolved Hide resolved
pkg/block/block.go Outdated Show resolved Hide resolved
pkg/block/block.go Show resolved Hide resolved
pkg/block/block.go Outdated Show resolved Hide resolved
pkg/block/path.go Outdated Show resolved Hide resolved
The slices package doesn't exist yet in Go 1.20 (added in Go 1.21).

Use golang.org/x/exp/slices which contains the required functions.
Creates a sort order for Paths.

Remove sorting of patches from Diff - document that the user can do it
using ComparePaths if required.

Sort patches in TestDiff, ExampleDiff so we have deterministic output
to check against.
move compareAny (block.go) -> comparePrimitives (path.go) to better reflect its purpose and usage site.

diffMap: rename variables to match type names

diffPages -> diffBlockValues: rename to match type name.
Adjust local variable names too.

prefixPatches, fieldPathSegments: use Path instead of []PathSegment

fieldTree -> blockTree: rename type to better reflect what it is
- a tree-based representation of a []Block.
Also add an example of the transformation performed by buildBlockTree.

splitMap: document purpose in more detail, with example

Block: reorder fields for better grouping.

Changes based on review feedback (#259).
Allow parsing paths that start with an array access.

Allow multiple contiguous array subscripts.
@hexaglow hexaglow changed the title Add package split for content-aware JSON config diffing and patching Add package block for content-aware JSON config diffing and patching Aug 29, 2024
pkg/block/block.go Outdated Show resolved Hide resolved
pkg/block/block.go Show resolved Hide resolved
pkg/block/block.go Outdated Show resolved Hide resolved
… sub-block

maps.Clone(nil) returns nil, which can't be written to.
In splitMap, we require a writeable map so add special-case handling.

Add regression test which can detect this issue.
If a patch references a sub-field (or sub-array, at any nesting depth) of an array element that doesn't exist, the patch failed to apply.

In applyPatch, when referencing an array element that doesn't exist, create a placeholder with the right key so we have something to apply the patch to.

This could be triggered with patches from Diff, if the patch to the sub-field came before the patch creating the array element.
Use global ignoreSentinal object in Ignore.MarshalJSON

In equalKeys, second loop could only return false if the first loop had, so it's redundant.
Comparing length and the keys from a to b is sufficient.
@hexaglow hexaglow merged commit 385a2e1 into main Aug 29, 2024
2 checks passed
hexaglow added a commit that referenced this pull request Aug 29, 2024
move compareAny (block.go) -> comparePrimitives (path.go) to better reflect its purpose and usage site.

diffMap: rename variables to match type names

diffPages -> diffBlockValues: rename to match type name.
Adjust local variable names too.

prefixPatches, fieldPathSegments: use Path instead of []PathSegment

fieldTree -> blockTree: rename type to better reflect what it is
- a tree-based representation of a []Block.
Also add an example of the transformation performed by buildBlockTree.

splitMap: document purpose in more detail, with example

Block: reorder fields for better grouping.

Changes based on review feedback (#259).
@hexaglow hexaglow deleted the feat/split branch August 29, 2024 15:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants