Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
105 changes: 105 additions & 0 deletions src/main/java/io/stargate/sgv2/jsonapi/util/YamlMerger.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
package io.stargate.sgv2.jsonapi.util;

import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.node.ArrayNode;
import com.fasterxml.jackson.databind.node.ObjectNode;
import com.fasterxml.jackson.databind.node.ValueNode;
import com.fasterxml.jackson.dataformat.yaml.YAMLMapper;
import java.io.IOException;
import java.io.InputStream;
import java.util.Iterator;
import java.util.Map;

/**
* Utility for deep-merging YAML documents with simple, predictable semantics:
*
* <ul>
* <li>Objects: recursively merged; fields from patch override or extend base
* <li>Arrays: replaced entirely when present in patch
* <li>Scalars (string/number/boolean/null): replaced by patch value
* </ul>
*/
public final class YamlMerger {

private final ObjectMapper yamlMapper;

public YamlMerger() {
this.yamlMapper = new YAMLMapper();
}

/** Merge two YAML strings and return the merged YAML string. */
public String mergeYamlStrings(String baseYaml, String patchYaml) {
Copy link
Contributor

Choose a reason for hiding this comment

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

thinking about this - remove the two mergeYaml* functions and rename to JsonMerger. Can also make the mergeNodes method static.

the caller will be responsble for reading the YAML and putting back to YAML.

the reason is we will want to have better control of errors when reading the yaml, e.g. what if the patch YAML is invalid ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Merging functionality is format-agnostic, too, (even if type is called JsonNode) which makes it usable for JSON, YAML and other Jackson-supported formats (XML, Avro, Protobuf...) should those ever be needed.

try {
JsonNode base = yamlMapper.readTree(baseYaml);
JsonNode patch = yamlMapper.readTree(patchYaml);
JsonNode merged = mergeNodes(base, patch);
return yamlMapper.writeValueAsString(merged);
} catch (IOException e) {
throw new IllegalArgumentException("Failed to merge YAML", e);
}
}

/** Merge two YAML input streams and return the merged YAML string. */
public String mergeYamlStreams(InputStream baseYaml, InputStream patchYaml) {
try {
JsonNode base = yamlMapper.readTree(baseYaml);
JsonNode patch = yamlMapper.readTree(patchYaml);
JsonNode merged = mergeNodes(base, patch);
return yamlMapper.writeValueAsString(merged);
} catch (IOException e) {
throw new IllegalArgumentException("Failed to merge YAML streams", e);
}
}

/** Core merge logic following the documented semantics. */
public JsonNode mergeNodes(JsonNode base, JsonNode patch) {
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we have our own logic and not using something like https://github.com/java-json-tools/json-patch ?

should add some comments.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think use JSON Patch would not help here -- it's standard for defining stand-alone patch definitions to use for merging changes (diffs) across documents, bit like unix diff and patch commands. But it would not help here where task is updating one document with another
(not sure I am explaining this well).

Renaming of second document as overrides or something (instead of patch) would help understand the difference.

if (base == null || base.isNull()) {
return deepCopy(patch);
}
if (patch == null) {
return deepCopy(base);
}

// If both are objects, merge field-by-field
if (base.isObject() && patch.isObject()) {
ObjectNode result = base.deepCopy();
Iterator<Map.Entry<String, JsonNode>> fields = patch.fields();
while (fields.hasNext()) {
Map.Entry<String, JsonNode> entry = fields.next();
String fieldName = entry.getKey();
JsonNode patchValue = entry.getValue();
JsonNode baseValue = result.get(fieldName);
if (baseValue != null) {
JsonNode mergedChild = mergeNodes(baseValue, patchValue);
result.set(fieldName, mergedChild);
} else {
result.set(fieldName, deepCopy(patchValue));
}
}
return result;
}

// If both are arrays, replace entirely with patch (no element-wise merging)
if (base.isArray() && patch.isArray()) {
return ((ArrayNode) patch).deepCopy();
Copy link
Contributor

Choose a reason for hiding this comment

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

for consistency, can we call our deepCopy() below ?

but we may not even need this check, because the code below calls deepCopy() anyway

}

// Otherwise scalars or differing types: patch overrides
return deepCopy(patch);
}

private JsonNode deepCopy(JsonNode node) {
Copy link
Contributor

Choose a reason for hiding this comment

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

this can be cleaner using a modern switch on node.

case for null, case for ValueNode, and default is call deepCopy()

if (node == null) {
return null;
}
if (node.isObject() || node.isArray()) {
return node.deepCopy();
}
if (node instanceof ValueNode) {
Copy link
Contributor

Choose a reason for hiding this comment

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

this feels like a future bug, when if the caller changed the vlue of the returned ValueNode ? we shoud not return any of the same objects.

Copy link
Contributor

Choose a reason for hiding this comment

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

ValueNodes are fully immutable, cannot be changed and meant to be shared/reused.
(I agree with not sharing mutable ArrayNodes and ObjectNodes of course)

return node;
}
// Default path
return node.deepCopy();
}
}
Loading