This is an automated email from the ASF dual-hosted git repository.
stefanegli pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/jackrabbit-oak.git
The following commit(s) were added to refs/heads/trunk by this push:
new 84b9c5999a OAK-10542 : getLiveRevision must inspect split docs if
local deleted … (#1198)
84b9c5999a is described below
commit 84b9c5999a712b297b12e7ab51d6061d224109a8
Author: stefan-egli <[email protected]>
AuthorDate: Mon Nov 20 16:52:50 2023 +0100
OAK-10542 : getLiveRevision must inspect split docs if local deleted …
(#1198)
* OAK-10542 : getLiveRevision must inspect split docs if local deleted does
not contain most recent committed revision
* OAK-10542 : TODO added about refactoring possibility
* OAK-10542 : refactor code duplication
* OAK-10542 : unignore the tests as OAK-10542 was merged in the meantime
* OAK-10542 : method renamed for usability
---
.../oak/plugins/document/NodeDocument.java | 48 +++++++++++++++-------
.../document/VersionGarbageCollectorIT.java | 4 --
2 files changed, 33 insertions(+), 19 deletions(-)
diff --git
a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java
b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java
index c79c6adfc4..6e7afeaf62 100644
---
a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java
+++
b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java
@@ -971,20 +971,7 @@ public final class NodeDocument extends Document {
readRevision, validRevisions, lastRevs);
// check if there may be more recent values in a previous document
- if (value != null
- && !getPreviousRanges().isEmpty()
- && !isMostRecentCommitted(local, value.revision,
nodeStore)) {
- // not reading the most recent value, we may need to
- // consider previous documents as well
- for (Revision prev : getPreviousRanges().keySet()) {
- if (prev.compareRevisionTimeThenClusterId(value.revision)
> 0) {
- // a previous document has more recent changes
- // than value.revision
- value = null;
- break;
- }
- }
- }
+ value = requiresCompleteMapCheck(value, local, nodeStore) ? null :
value;
if (value == null && !getPreviousRanges().isEmpty()) {
// check revision history
@@ -1067,8 +1054,11 @@ public final class NodeDocument extends Document {
RevisionVector readRevision,
Map<Revision, String> validRevisions,
LastRevs lastRevs) {
+ final SortedMap<Revision, String> local = getLocalDeleted();
// check local deleted map first
- Value value = getLatestValue(context, getLocalDeleted().entrySet(),
readRevision, validRevisions, lastRevs);
+ Value value = getLatestValue(context, local.entrySet(), readRevision,
validRevisions, lastRevs);
+ // check if there may be more recent values in a previous document
+ value = requiresCompleteMapCheck(value, local, context) ? null : value;
if (value == null && !getPreviousRanges().isEmpty()) {
// need to check complete map
value = getLatestValue(context, getDeleted().entrySet(),
readRevision, validRevisions, lastRevs);
@@ -2181,6 +2171,34 @@ public final class NodeDocument extends Document {
return value;
}
+ /**
+ * Check if there may be more recent values in a previous document and
thus a
+ * complete map check is required.
+ *
+ * @param localValue value as resolved from local value map
+ * @param local local value map
+ * @param context the revision context
+ * @return false if it is most recent, true otherwise
+ */
+ private boolean requiresCompleteMapCheck(@Nullable Value localValue,
+ @NotNull SortedMap<Revision, String> local,
+ @NotNull RevisionContext context) {
+ if (localValue != null
+ && !getPreviousRanges().isEmpty()
+ && !isMostRecentCommitted(local, localValue.revision,
context)) {
+ // not reading the most recent value, we may need to
+ // consider previous documents as well
+ for (Revision prev : getPreviousRanges().keySet()) {
+ if (prev.compareRevisionTimeThenClusterId(localValue.revision)
> 0) {
+ // a previous document has more recent changes
+ // than localValue.revision
+ return true;
+ }
+ }
+ }
+ return false;
+ }
+
/**
* Get the latest property value smaller or equal the readRevision
revision.
*
diff --git
a/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollectorIT.java
b/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollectorIT.java
index 61f9ffbf9a..f46d4cdd32 100644
---
a/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollectorIT.java
+++
b/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollectorIT.java
@@ -82,7 +82,6 @@ import org.apache.jackrabbit.oak.stats.Clock;
import org.jetbrains.annotations.NotNull;
import org.junit.After;
import org.junit.Before;
-import org.junit.Ignore;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
@@ -295,7 +294,6 @@ public class VersionGarbageCollectorIT {
* tests a checkpoint when /t/target is deleted.
*/
@Test
- @Ignore(value = "requires fix for OAK-10526 and OAK-10542")
public void gcSplitDocWithReferencedDeleted_combined() throws Exception {
// step 1 : create a _delete entry with clusterId 2, plus do a GC
final DocumentNodeStore store2 = createSecondary();
@@ -361,7 +359,6 @@ public class VersionGarbageCollectorIT {
* checkpoint when /t/target is deleted.
*/
@Test
- @Ignore(value = "requires fix for OAK-10542 first")
public void gcSplitDocWithReferencedDeleted_true() throws Exception {
// step 1 : create some _deleted entries with clusterId 2
final DocumentNodeStore store2 = createSecondary();
@@ -402,7 +399,6 @@ public class VersionGarbageCollectorIT {
* checkpoint when /t/target exists.
*/
@Test
- @Ignore(value = "requires fix for OAK-10542 first")
public void gcSplitDocWithReferencedDeleted_false() throws Exception {
// step 1 : create a _delete entry with clusterId 2
final DocumentNodeStore store2 = createSecondary();
