You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-commits@jackrabbit.apache.org by st...@apache.org on 2021/09/15 08:36:57 UTC

[jackrabbit-oak] branch trunk updated: OAK-9567 : Avoid NullPointerException in ReadWriteVersionManager.removeVersion (#363)

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 089cd52  OAK-9567 : Avoid NullPointerException in ReadWriteVersionManager.removeVersion (#363)
089cd52 is described below

commit 089cd52d07abaccd17f2868279eaa6928d072d5b
Author: stefan-egli <st...@apache.org>
AuthorDate: Wed Sep 15 10:36:15 2021 +0200

    OAK-9567 : Avoid NullPointerException in ReadWriteVersionManager.removeVersion (#363)
    
    * OAK-9535 related : trying to increase mvn memory
    
    * OAK-9535 related : cleanup of erroneously pushed change
    
    * OAK-9567 : avoid NullPointerException if successor or predecessor references are not properly set - also log more details when this happens to help narrow down the root cause of this apparent inconsistency
    
    * revert of 8c5bb742c056e0a150caff8f5ce910bb5c92391e to have things separated properly : OAK-9535 related : cleanup of erroneously pushed change
    
    * OAK-9567 : more log details
    
    * OAK-9567 : log beautify
    
    * OAK-9567 : minor rename
    
    * OAK-9567 : log beautify
    
    * OAK-9567 : log beautify
    
    * OAK-9567 : log beautify
---
 .../plugins/version/ReadWriteVersionManager.java   | 52 ++++++++++++++++++++--
 1 file changed, 49 insertions(+), 3 deletions(-)

diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/version/ReadWriteVersionManager.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/version/ReadWriteVersionManager.java
index 544b4b6..e5f533f 100644
--- a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/version/ReadWriteVersionManager.java
+++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/version/ReadWriteVersionManager.java
@@ -49,6 +49,8 @@ import org.apache.jackrabbit.util.ISO8601;
 import org.apache.jackrabbit.util.Text;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
@@ -78,6 +80,8 @@ import static org.apache.jackrabbit.oak.spi.version.VersionConstants.VERSION_STO
  */
 public class ReadWriteVersionManager extends ReadOnlyVersionManager {
 
+    private static final Logger LOG = LoggerFactory.getLogger(ReadWriteVersionManager.class);
+
     private final NodeBuilder versionStorageNode;
     private final NodeBuilder workspaceRoot;
     private final TypePredicate isVersion;
@@ -192,9 +196,20 @@ public class ReadWriteVersionManager extends ReadOnlyVersionManager {
 
         for (String succId :  successorIds.getValue(Type.REFERENCES)) {
             NodeBuilder successor = getVersionById(vh, succId);
-
+            if (successor == null) {
+                LOG.info("removeVersion : successor not found with uuid: {}, historyRelPath: {}, versionNode: {}, versionHistory: {}",
+                        succId, historyRelPath, asLoggableString(versionNode), asLoggableString(vh));
+                continue;
+            }
             PropertyBuilder<String> pb = PropertyBuilder.array(Type.REFERENCE);
-            pb.setName(JCR_PREDECESSORS).setValues(successor.getProperty(JCR_PREDECESSORS).getValue(Type.REFERENCES));
+            pb.setName(JCR_PREDECESSORS);
+            PropertyState successorsPredecessors = successor.getProperty(JCR_PREDECESSORS);
+            if (successorsPredecessors == null) {
+                LOG.info("removeVersion : successor has no jcr:predecessors property, uuid: {}, historyRelPath: {}, versionNode: {}, successor: {}, versionHistory: {}",
+                        succId, historyRelPath, asLoggableString(versionNode), asLoggableString(successor), asLoggableString(vh));
+            } else {
+                pb.setValues(successorsPredecessors.getValue(Type.REFERENCES));
+            }
 
             pb.removeValue(versionId);
             pb.addValues(predecessorIds.getValue(Type.REFERENCES));
@@ -204,8 +219,20 @@ public class ReadWriteVersionManager extends ReadOnlyVersionManager {
 
         for (String predId :  predecessorIds.getValue(Type.REFERENCES)) {
             NodeBuilder predecessor = getVersionById(vh, predId);
+            if (predecessor == null) {
+                LOG.info("removeVersion : predecessor not found with uuid: {}, historyRelPath: {}, versionNode: {}, versionHistory: {}",
+                        predId, historyRelPath, asLoggableString(versionNode), asLoggableString(vh));
+                continue;
+            }
             PropertyBuilder<String> pb = PropertyBuilder.array(Type.REFERENCE);
-            pb.setName(JCR_SUCCESSORS).setValues(predecessor.getProperty(JCR_SUCCESSORS).getValue(Type.REFERENCES));
+            pb.setName(JCR_SUCCESSORS);
+            PropertyState predecessorsSuccessors = predecessor.getProperty(JCR_SUCCESSORS);
+            if (predecessorsSuccessors == null) {
+                LOG.info("removeVersion : predecessor has no jcr:successors property, uuid: {}, historyRelPath: {}, versionNode: {}, predecessor: {}, versionHistory: {}",
+                        predId, historyRelPath, asLoggableString(versionNode), asLoggableString(predecessor), asLoggableString(vh));
+            } else {
+                pb.setValues(predecessorsSuccessors.getValue(Type.REFERENCES));
+            }
 
             pb.removeValue(versionId);
             pb.addValues(successorIds.getValue(Type.REFERENCES));
@@ -215,6 +242,25 @@ public class ReadWriteVersionManager extends ReadOnlyVersionManager {
         versionNode.remove();
     }
 
+    /** small helper to log a node - useful for later debugging **/
+    private static String asLoggableString(NodeBuilder nb) {
+        try {
+            final StringBuilder sb = new StringBuilder();
+            boolean empty = true;
+            for (PropertyState p : nb.getProperties()) {
+                if (empty) {
+                    empty = false;
+                } else {
+                    sb.append(", ");
+                }
+                sb.append(p);
+            }
+            return "{ " + sb + " }";
+        } catch (Exception e) {
+            return "{ exception: " + e + ", message: " + e.getMessage() + " }";
+        }
+    }
+
     public void checkout(NodeBuilder versionable) {
         versionable.setProperty(JCR_ISCHECKEDOUT, true, Type.BOOLEAN);
         PropertyState baseVersion = versionable.getProperty(JCR_BASEVERSION);