Skip to content

Commit affcc42

Browse files
JohannesLichtenbergerJohannes Lichtenberger
andauthored
Feature/diff path support (#821)
* feat: add path field to diff response using PathSummary - Add path field to JsonDiffSerializer.java using PathSummary (enabled by default) - Add resolveArrayPositions method to convert path like /arr/[] to /arr/[3] - Add include-data query param to DiffHandler.kt (default false for compact mode) - Add generateDiff overload to BasicJsonDiff.java that accepts includeData param This enables the frontend to display diffs with full paths without requiring DeweyIDs. * fix: update tests for new path field in diff serialization - Update expected JSON files in basicJsonDiffTest to include path field - Fix JsonResourceCopy to handle delete as object instead of primitive (delete now returns {nodeKey, path, ...} instead of just nodeKey) * feat: implement delete() in JsonDBItem for sdb:select-item support Implements the delete() method from UpdatableJsonItem interface, enabling JSONiq delete expressions to work with sdb:select-item(). Changes: - Add default delete() implementation in JsonDBItem interface - Add test testRemoveFromObjectViaSdbSelectItem() to verify functionality This allows queries like: delete json sdb:select-item(jn:doc('db','res'), nodeKey) Requires Brackit feature/sdb-select-item-delete-support branch. * feat: implement delete() in JsonDBItem for sdb:select-item support Implements the delete() method from UpdatableJsonItem interface, enabling JSONiq delete expressions to work with sdb:select-item(). Changes: - Add default delete() implementation in JsonDBItem interface - Add test testRemoveFromObjectViaSdbSelectItem() to verify functionality This allows queries like: delete json sdb:select-item(jn:doc('db','res'), nodeKey) Requires Brackit feature/sdb-select-item-delete-support branch. * Update expected diff output to include path fields The JsonDiffSerializer now includes path information in the diff output for insert, delete, replace, and update operations. Updated the expected test output file to match the new format with path fields. * Fix test * feat: enable branching from historical revisions When editing a document opened at a historical revision (e.g., revision 1), the write transaction is now automatically reverted to that revision before applying changes. This enables true branching from any point in history. Changes: - JsonDBItem.getOrCreateWriteTrx(): New method that gets or creates a write transaction and calls revertTo(sourceRevision) if the source document is from an older revision than the most recent - JsonDBItem.delete() and replaceValue(): Now use getOrCreateWriteTrx() - AbstractJsonDBArray.getReadWriteTrx(): Added revertTo logic - JsonDBObject.getReadWriteTrx(): Added revertTo logic This allows users to: 1. View any historical revision in the Explorer 2. Make edits (insert, update, delete) 3. Commit - creates a new revision branching from that historical point 4. All intermediate revisions remain preserved (SirixDB's append-only model) * Add reference counting to MMStorage to prevent premature arena closure When MMStorage remaps memory-mapped segments due to file growth, it was previously closing the old arena immediately, which invalidated any existing readers still using those segments. This change: - Adds ArenaGeneration class to track arena/segments with reference count - Readers increment ref count when created, decrement when closed - Old arenas are only closed when their ref count reaches zero - Fixes 'Already closed' errors in JsonDiffSerializer path resolution Also reverts the exception-catching workaround in JsonDiffSerializer since the root cause is now properly addressed. * fix: use int for revision references length instead of byte - Changed from writeByte/readByte to writeInt/readInt for the length field in REVISION_REFERENCES_NODE serialization - This supports nodes with more than 255 revision references - Previously, a byte could only hold 0-255, and due to signed byte interpretation, values > 127 caused IndexOutOfBoundsException * Fix test * Fix JSONiq bug, replacing two values... --------- Co-authored-by: Johannes Lichtenberger <johannes.lichtenberger@sirix.io>
1 parent f6a04fe commit affcc42

20 files changed

Lines changed: 461 additions & 71 deletions

File tree

bundles/sirix-core/src/main/java/io/sirix/diff/JsonDiffSerializer.java

Lines changed: 142 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,18 @@
33
import com.google.api.client.util.Objects;
44
import com.google.gson.JsonArray;
55
import com.google.gson.JsonObject;
6+
import io.brackit.query.atomic.QNm;
7+
import io.brackit.query.util.path.Path;
68
import io.sirix.api.json.JsonNodeReadOnlyTrx;
79
import io.sirix.api.json.JsonResourceSession;
10+
import io.sirix.index.path.summary.PathSummaryReader;
811
import io.sirix.node.NodeKind;
912
import io.sirix.service.json.serialize.JsonSerializer;
1013

1114
import java.io.IOException;
1215
import java.io.StringWriter;
1316
import java.io.UncheckedIOException;
17+
import java.util.ArrayDeque;
1418
import java.util.Collection;
1519

1620
public final class JsonDiffSerializer {
@@ -78,6 +82,9 @@ public String serialize(boolean emitFromDiffAlgorithm) {
7882

7983
insertBasedOnNewRtx(newRtx, jsonInsertDiff);
8084

85+
// Add path using PathSummary (always available by default)
86+
addPathIfAvailable(jsonInsertDiff, newRtx, newRevisionNumber);
87+
8188
if (resourceManager.getResourceConfig().areDeweyIDsStored) {
8289
final var deweyId = newRtx.getDeweyID();
8390
jsonInsertDiff.addProperty("deweyID", deweyId.toString());
@@ -92,21 +99,20 @@ public String serialize(boolean emitFromDiffAlgorithm) {
9299
break;
93100
case DELETED:
94101
final var deletedJson = new JsonObject();
102+
final var jsonDeletedDiff = new JsonObject();
95103

96-
if (resourceManager.getResourceConfig().areDeweyIDsStored) {
97-
final var jsonDeletedDiff = new JsonObject();
104+
jsonDeletedDiff.addProperty("nodeKey", diffTuple.getOldNodeKey());
98105

99-
jsonDeletedDiff.addProperty("nodeKey", diffTuple.getOldNodeKey());
106+
// Add path using PathSummary (always available by default)
107+
addPathIfAvailable(jsonDeletedDiff, oldRtx, oldRevisionNumber);
100108

109+
if (resourceManager.getResourceConfig().areDeweyIDsStored) {
101110
final var deweyId = oldRtx.getDeweyID();
102111
jsonDeletedDiff.addProperty("deweyID", deweyId.toString());
103112
jsonDeletedDiff.addProperty("depth", deweyId.getLevel());
104-
105-
deletedJson.add("delete", jsonDeletedDiff);
106-
} else {
107-
deletedJson.addProperty("delete", diffTuple.getOldNodeKey());
108113
}
109114

115+
deletedJson.add("delete", jsonDeletedDiff);
110116
jsonDiffs.add(deletedJson);
111117
break;
112118
case REPLACEDNEW:
@@ -118,6 +124,9 @@ public String serialize(boolean emitFromDiffAlgorithm) {
118124
jsonReplaceDiff.addProperty("oldNodeKey", diffTuple.getOldNodeKey());
119125
jsonReplaceDiff.addProperty("newNodeKey", diffTuple.getNewNodeKey());
120126

127+
// Add path using PathSummary (always available by default)
128+
addPathIfAvailable(jsonReplaceDiff, newRtx, newRevisionNumber);
129+
121130
if (resourceManager.getResourceConfig().areDeweyIDsStored) {
122131
final var deweyId = newRtx.getDeweyID();
123132
jsonReplaceDiff.addProperty("deweyID", deweyId.toString());
@@ -134,6 +143,9 @@ public String serialize(boolean emitFromDiffAlgorithm) {
134143

135144
jsonUpdateDiff.addProperty("nodeKey", diffTuple.getOldNodeKey());
136145

146+
// Add path using PathSummary (always available by default)
147+
addPathIfAvailable(jsonUpdateDiff, newRtx, newRevisionNumber);
148+
137149
if (resourceManager.getResourceConfig().areDeweyIDsStored) {
138150
final var deweyId = newRtx.getDeweyID();
139151
jsonUpdateDiff.addProperty("deweyID", deweyId.toString());
@@ -227,4 +239,127 @@ public static void serialize(int newRevision, JsonResourceSession resourceManage
227239
throw new UncheckedIOException(e);
228240
}
229241
}
242+
243+
/**
244+
* Get the path for a node using PathSummary.
245+
* Returns null if PathSummary is not enabled or if the path cannot be retrieved.
246+
*
247+
* @param rtx the read-only transaction positioned at the node
248+
* @param revisionNumber the revision number
249+
* @return the path string, or null if unavailable
250+
*/
251+
private String getNodePath(JsonNodeReadOnlyTrx rtx, int revisionNumber) {
252+
if (!resourceManager.getResourceConfig().withPathSummary) {
253+
return null;
254+
}
255+
256+
try (final PathSummaryReader pathReader = resourceManager.openPathSummary(revisionNumber)) {
257+
if (!pathReader.moveTo(rtx.getPathNodeKey())) {
258+
return null;
259+
}
260+
261+
final var pathNode = pathReader.getPathNode();
262+
if (pathNode == null) {
263+
return null;
264+
}
265+
266+
final Path<QNm> path = pathReader.getPath();
267+
if (path == null) {
268+
return null;
269+
}
270+
271+
// Resolve array positions like sdb:path() does
272+
return resolveArrayPositions(rtx, path);
273+
} catch (final IllegalStateException e) {
274+
// Resource may have been closed (e.g., memory-mapped file reader)
275+
// This can happen during concurrent operations or cleanup
276+
return null;
277+
}
278+
}
279+
280+
/**
281+
* Resolve array indices in the path to concrete positions.
282+
* Converts "/arr/[]" to "/arr/[3]" based on actual sibling position.
283+
*
284+
* @param rtx the transaction positioned at the node
285+
* @param path the path with unresolved array indices
286+
* @return the path with resolved array indices
287+
*/
288+
private String resolveArrayPositions(JsonNodeReadOnlyTrx rtx, Path<QNm> path) {
289+
final String pathString = path.toString();
290+
291+
if (!pathString.contains("[]")) {
292+
return pathString;
293+
}
294+
295+
// We need to walk up the tree to resolve array positions
296+
final var steps = path.steps();
297+
final var positions = new ArrayDeque<Integer>();
298+
299+
// Save current position
300+
final long originalNodeKey = rtx.getNodeKey();
301+
302+
try {
303+
for (int i = steps.size() - 1; i >= 0; i--) {
304+
final var step = steps.get(i);
305+
306+
if (step.getAxis() == Path.Axis.CHILD_ARRAY) {
307+
positions.addFirst(getArrayPosition(rtx));
308+
rtx.moveToParent();
309+
} else {
310+
rtx.moveToParent();
311+
}
312+
}
313+
314+
var result = pathString;
315+
for (Integer pos : positions) {
316+
if (pos == -1) {
317+
// Keep as [] for arrays that are direct children of object keys
318+
continue;
319+
}
320+
result = result.replaceFirst("/\\[]", "/[" + pos + "]");
321+
}
322+
323+
// Replace remaining unresolved positions with []
324+
result = result.replaceAll("/\\[-1]", "/[]");
325+
326+
return result;
327+
} finally {
328+
// Restore original position
329+
rtx.moveTo(originalNodeKey);
330+
}
331+
}
332+
333+
/**
334+
* Get the array position (index) of the current node among its siblings.
335+
*
336+
* @param rtx the transaction positioned at the node
337+
* @return the 0-based index, or -1 if this is an array directly under an object key
338+
*/
339+
private int getArrayPosition(JsonNodeReadOnlyTrx rtx) {
340+
if (rtx.getParentKind() == NodeKind.OBJECT_KEY && rtx.isArray()) {
341+
return -1;
342+
}
343+
344+
int index = 0;
345+
while (rtx.hasLeftSibling()) {
346+
rtx.moveToLeftSibling();
347+
index++;
348+
}
349+
return index;
350+
}
351+
352+
/**
353+
* Add path to a diff JSON object if PathSummary is available.
354+
*
355+
* @param json the JSON object to add the path to
356+
* @param rtx the transaction positioned at the node
357+
* @param revisionNumber the revision number
358+
*/
359+
private void addPathIfAvailable(JsonObject json, JsonNodeReadOnlyTrx rtx, int revisionNumber) {
360+
final String path = getNodePath(rtx, revisionNumber);
361+
if (path != null) {
362+
json.addProperty("path", path);
363+
}
364+
}
230365
}

bundles/sirix-core/src/main/java/io/sirix/io/memorymapped/MMFileReader.java

Lines changed: 39 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,18 @@ public final class MMFileReader extends AbstractReader {
6969
private final Arena arena;
7070

7171
/**
72-
* Constructor.
72+
* Arena generation for reference counting. Used when managed by MMStorage.
73+
*/
74+
private final MMStorage.@Nullable ArenaGeneration generation;
75+
76+
/**
77+
* Reference to the storage for releasing the generation. Used when managed by MMStorage.
78+
*/
79+
@Nullable
80+
private final MMStorage storage;
81+
82+
/**
83+
* Constructor for standalone reader with its own arena.
7384
*
7485
* @param dataFileSegment memory-mapped segment for the data file
7586
* @param revisionFileSegment memory-mapped segment for the revisions file
@@ -82,11 +93,32 @@ public final class MMFileReader extends AbstractReader {
8293
public MMFileReader(final MemorySegment dataFileSegment, final MemorySegment revisionFileSegment,
8394
final ByteHandler byteHandler, final SerializationType type, final PagePersister pagePersister,
8495
final Cache<Integer, RevisionFileData> cache, @Nullable final Arena arena) {
96+
this(dataFileSegment, revisionFileSegment, byteHandler, type, pagePersister, cache, null, null);
97+
}
98+
99+
/**
100+
* Constructor for reader managed by MMStorage with reference counting.
101+
*
102+
* @param dataFileSegment memory-mapped segment for the data file
103+
* @param revisionFileSegment memory-mapped segment for the revisions file
104+
* @param byteHandler {@link ByteHandler} instance
105+
* @param type serialization type
106+
* @param pagePersister page persister
107+
* @param cache revision file data cache
108+
* @param generation arena generation for reference counting
109+
* @param storage storage for releasing the generation
110+
*/
111+
public MMFileReader(final MemorySegment dataFileSegment, final MemorySegment revisionFileSegment,
112+
final ByteHandler byteHandler, final SerializationType type, final PagePersister pagePersister,
113+
final Cache<Integer, RevisionFileData> cache, final MMStorage.@Nullable ArenaGeneration generation,
114+
@Nullable final MMStorage storage) {
85115
super(byteHandler, pagePersister, type);
86116
this.dataFileSegment = requireNonNull(dataFileSegment);
87117
this.revisionsOffsetFileSegment = requireNonNull(revisionFileSegment);
88118
this.cache = requireNonNull(cache);
89-
this.arena = arena; // May be null if managed externally (by MMStorage)
119+
this.arena = null; // Not used when managed by MMStorage
120+
this.generation = generation;
121+
this.storage = storage;
90122
}
91123

92124
@Override
@@ -161,9 +193,13 @@ public RevisionFileData getRevisionFileData(int revision) {
161193

162194
@Override
163195
public void close() {
196+
// If managed by MMStorage with reference counting, release the generation
197+
if (generation != null && storage != null) {
198+
storage.releaseGeneration(generation);
199+
}
164200
// Only close the arena if we own it (not null).
165201
// When arena is null, the storage (MMStorage) owns and manages the shared arena.
166-
if (arena != null) {
202+
else if (arena != null) {
167203
arena.close();
168204
}
169205
}

0 commit comments

Comments
 (0)