NPE fix for Acorn cluster stream undo handling 17/1517/2
authorTuukka Lehtonen <tuukka.lehtonen@semantum.fi>
Thu, 1 Mar 2018 22:35:49 +0000 (00:35 +0200)
committerJani Simomaa <jani.simomaa@semantum.fi>
Fri, 2 Mar 2018 06:13:40 +0000 (08:13 +0200)
refs #7792

Change-Id: If6e7e3d89db26bb7c4aeed06507b02ae34554d00

bundles/org.simantics.acorn/src/org/simantics/acorn/internal/ClusterChange2.java [deleted file]
bundles/org.simantics.acorn/src/org/simantics/acorn/internal/ClusterUpdateProcessor.java
bundles/org.simantics.acorn/src/org/simantics/acorn/internal/ClusterUpdateProcessor2.java [deleted file]
bundles/org.simantics.acorn/src/org/simantics/acorn/internal/ClusterUpdateProcessorBase.java
bundles/org.simantics.acorn/src/org/simantics/acorn/internal/ClusterUpdateProcessorBase2.java [deleted file]
bundles/org.simantics.acorn/src/org/simantics/acorn/internal/UndoClusterUpdateProcessor.java
bundles/org.simantics.acorn/src/org/simantics/acorn/lru/ClusterStreamChunk.java
bundles/org.simantics.acorn/src/org/simantics/acorn/lru/ClusterUpdateOperation.java
bundles/org.simantics.db.procore/src/fi/vtt/simantics/procore/internal/ClusterChange2.java

diff --git a/bundles/org.simantics.acorn/src/org/simantics/acorn/internal/ClusterChange2.java b/bundles/org.simantics.acorn/src/org/simantics/acorn/internal/ClusterChange2.java
deleted file mode 100644 (file)
index 472b4d7..0000000
+++ /dev/null
@@ -1,70 +0,0 @@
-package org.simantics.acorn.internal;
-
-
-public class ClusterChange2 {
-    public static final int VERSION = 2;
-    public static final byte SET_IMMUTABLE_OPERATION = 1; // <byte : 0 = false>
-    public static final byte UNDO_VALUE_OPERATION = 2; // <int : resource index>
-    private static final int INCREMENT = 1<<10;
-//    private boolean dirty = false;
-//    private byte[] bytes;
-//    private int byteIndex;
-//    private ClusterUID clusterUID;
-//    private ClusterImpl cluster;
-//    ClusterChange2(ClusterUID clusterUID, ClusterImpl cluster) {
-//        this.clusterUID = clusterUID;
-//        this.cluster = cluster;
-//        init();
-//    }
-//    void init() {
-////        System.err.println("clusterChange2 dirty " + cluster.clusterId);
-//        dirty = false;
-//        bytes = new byte[INCREMENT];
-//        byteIndex = 0;
-//        addInt(0); // Size of byte vector. Set by flush.
-//        addInt(VERSION);
-//        byteIndex = clusterUID.toByte(bytes, 8);
-//    }
-//    boolean isDirty() {
-//        return dirty;
-//    }
-//    void flush(GraphSession graphSession) {
-////        System.err.println("flush2 clusterChange2 " + dirty + this);
-//        if (!dirty)
-//            return;
-//        Bytes.writeLE(bytes, 0, byteIndex - 4);
-//        byte[] ops = Arrays.copyOf(bytes, byteIndex);
-////        System.err.println("flush2 clusterChange2 " + cluster.clusterId + " " + ops.length + " bytes.");
-//        graphSession.updateCluster(new UpdateClusterFunction(ops));
-//        init();
-//    }
-//    void setImmutable(boolean immutable) {
-//        dirty = true;
-//        addByte(SET_IMMUTABLE_OPERATION);
-//        addByte((byte)(immutable ? -1 : 0));
-//    }
-//    void undoValueEx(int resourceIndex) {
-//        dirty = true;
-//        addByte(UNDO_VALUE_OPERATION);
-//        addInt(resourceIndex);
-//    }
-//    private final void checkSpace(int len) {
-//        if (bytes.length - byteIndex > len)
-//            return;
-//       bytes = Arrays.copyOf(bytes, bytes.length + len + INCREMENT);
-//    }
-//    private final void addByte(byte value) {
-//        checkSpace(1);
-//        bytes[byteIndex++] = value;
-//    }
-//    private final void addInt(int value) {
-//        checkSpace(4);
-//        Bytes.writeLE(bytes, byteIndex, value);
-//        byteIndex += 4;
-//    }
-////    private void addLong(long value) {
-////        checkSpace(8);
-////        Bytes.writeLE(bytes, byteIndex, value);
-////        byteIndex += 8;
-////    }
-}
index d4381aeca4e9610f1ee4e9fdf138e3f4cea1f99c..2885338c706d469207ef5c27a7494907365f4bc5 100644 (file)
@@ -76,11 +76,16 @@ public class ClusterUpdateProcessor extends ClusterUpdateProcessorBase {
 
        }
 
+       @Override
+       void setImmutable(boolean value) {
+               cluster.setImmutable(value, support);
+       }
+
        public ClusterImpl process(ClusterImpl cluster) throws IllegalAcornStateException {
                this.cluster = cluster;
                process();
                info.finish();
                return this.cluster;
        }
-       
+
 }
diff --git a/bundles/org.simantics.acorn/src/org/simantics/acorn/internal/ClusterUpdateProcessor2.java b/bundles/org.simantics.acorn/src/org/simantics/acorn/internal/ClusterUpdateProcessor2.java
deleted file mode 100644 (file)
index 61a8a8a..0000000
+++ /dev/null
@@ -1,31 +0,0 @@
-package org.simantics.acorn.internal;
-
-import org.simantics.acorn.cluster.ClusterImpl;
-import org.simantics.acorn.exception.IllegalAcornStateException;
-import org.simantics.acorn.lru.ClusterUpdateOperation;
-import org.simantics.db.impl.ClusterSupport;
-
-public class ClusterUpdateProcessor2 extends ClusterUpdateProcessorBase2 {
-
-       final ClusterSupport support;
-       final ClusterUpdateOperation info;
-       private ClusterImpl cluster;
-
-       public ClusterUpdateProcessor2(ClusterSupport support, byte[] operations, ClusterUpdateOperation info) {
-               super(operations);
-               this.support = support;
-               this.info = info;
-       }
-
-       public void process(ClusterImpl cluster) throws IllegalAcornStateException {
-               this.cluster = cluster;
-               process();
-               info.finish();
-       }
-
-       @Override
-       void setImmutable(boolean value) {
-               cluster.setImmutable(value, support);
-       }
-       
-}
index cd8130d9c51357902b2cea7ef3a2f9be2117f2a8..507db172126963611a7bdba7da7f4009957afd0b 100644 (file)
@@ -12,9 +12,15 @@ import org.simantics.acorn.internal.ClusterStream.StmEnum;
 import org.simantics.db.exception.DatabaseException;
 import org.simantics.db.service.Bytes;
 import org.simantics.db.service.ClusterUID;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import fi.vtt.simantics.procore.internal.ClusterChange2;
 
 abstract public class ClusterUpdateProcessorBase {
-       
+
+       private static final Logger LOGGER = LoggerFactory.getLogger(ClusterUpdateProcessorBase.class);
+
        public final static boolean DEBUG = false;
 
        final protected ClusterManager manager;
@@ -24,9 +30,9 @@ abstract public class ClusterUpdateProcessorBase {
        final private ClusterUID uid;
        final private int clusterKey;
        final public int version;
-       
-       final Map<ClusterUID, Integer> clusterKeyCache = new HashMap<ClusterUID, Integer>();
-       
+
+       final Map<ClusterUID, Integer> clusterKeyCache = new HashMap<>();
+
        public int getResourceKey(ClusterUID uid, int index) throws IllegalAcornStateException {
                Integer match = clusterKeyCache.get(uid);
                if(match != null) return match+index;
@@ -40,7 +46,7 @@ abstract public class ClusterUpdateProcessorBase {
                this.manager = client;
                this.bytes = operations;
                this.len = Bytes.readLE4(bytes, 0)+4; // whatta?
-               version = Bytes.readLE4(bytes, 4);
+               this.version = Bytes.readLE4(bytes, 4);
                long cuid1 = Bytes.readLE8(bytes, 8);
                long cuid2 = Bytes.readLE8(bytes, 16);
                uid = ClusterUID.make(cuid1, cuid2);
@@ -66,24 +72,21 @@ abstract public class ClusterUpdateProcessorBase {
                try {
                        create();
                } catch (DatabaseException e) {
-                       e.printStackTrace();
+                       LOGGER.error("resource create failed", e);
                }
-               
        }
-       
+
        private void processDelete() {
-               
                int ri = Bytes.readLE2(bytes, pos);
                pos += 2;
-               
+
                if(DEBUG) System.err.println("DEBUG: Delete " + ri);
-               
+
                try {
                        delete(ri);
                } catch (DatabaseException e) {
-                       e.printStackTrace();
+                       LOGGER.error("resource {} value delete failed", ri, e);
                }
-               
        }
 
        private void processModify(int op) {
@@ -110,7 +113,8 @@ abstract public class ClusterUpdateProcessorBase {
                try {
                        modify(clusterKey + ri, offset, size, bytes, pos);
                } catch (DatabaseException e) {
-                       e.printStackTrace();
+                       LOGGER.error("resource value modify(clusterKey: {}, ri: {}, offset: {}, size: {}, pos: {}) failed",
+                                       clusterKey, ri, offset, size, pos, e);
                }
 
                pos += size;
@@ -134,7 +138,8 @@ abstract public class ClusterUpdateProcessorBase {
                try {
                        set(clusterKey+r, valueBuffer, length);
                } catch (DatabaseException e) {
-                       e.printStackTrace();
+                       LOGGER.error("resource value set(clusterKey: {}, r: {}, length: {}) failed",
+                                       clusterKey, r, length, e);
                }
                
        }
@@ -160,7 +165,8 @@ abstract public class ClusterUpdateProcessorBase {
                try {
                        set(clusterKey+r, valueBuffer, length);
                } catch (DatabaseException e) {
-                       e.printStackTrace();
+                       LOGGER.error("resource value setShort(clusterKey: {}, r: {}, length: {}) failed",
+                                       clusterKey, r, length, e);
                }
                
        }
@@ -288,7 +294,8 @@ abstract public class ClusterUpdateProcessorBase {
                        try {
                                claim(clusterKey+ri, predicateKey, objectKey, puid, ouid);
                        } catch (DatabaseException e) {
-                               e.printStackTrace();
+                               LOGGER.error("statement add(clusterKey: {}, ri: {}, predicateKey: {}, objectKey: {}, puid: {}, ouid: {}) failed",
+                                               clusterKey, ri, predicateKey, objectKey, puid.toString(), ouid.toString(), e);
                        }
                        
                } else {
@@ -301,7 +308,8 @@ abstract public class ClusterUpdateProcessorBase {
                        try {
                                deny(clusterKey+ri, predicateKey, objectKey, puid, ouid);
                        } catch (DatabaseException e) {
-                               e.printStackTrace();
+                               LOGGER.error("statement deny(clusterKey: {}, ri: {}, predicateKey: {}, objectKey: {}, puid: {}, ouid: {}) failed",
+                                               clusterKey, ri, predicateKey, objectKey, puid.toString(), ouid.toString(), e);
                        }
                        
                }
@@ -309,6 +317,14 @@ abstract public class ClusterUpdateProcessorBase {
        }
 
        public void process() throws IllegalAcornStateException {
+               if (version == ClusterChange.VERSION) {
+                       process1();
+               } else if (version == ClusterChange2.VERSION) {
+                       process2();
+               }
+       }
+
+       private void process1() throws IllegalAcornStateException {
                
                foreignPos = 0;
 
@@ -463,14 +479,50 @@ abstract public class ClusterUpdateProcessorBase {
                }
                
        }
-       
-       
+
+       private void process2() throws IllegalAcornStateException {
+
+               while(pos < len) {
+
+                       int op = bytes[pos++]&0xff;
+
+                       switch(op) {
+
+                       case ClusterChange2.SET_IMMUTABLE_OPERATION:
+                               processSetImmutable(op);
+                               break;
+                       case ClusterChange2.UNDO_VALUE_OPERATION:
+                               processUndoValue(op);
+                               break;
+                       case ClusterChange2.SET_DELETED_OPERATION:
+                               // TODO: implement?
+                               break;
+                       default:
+                               throw new IllegalAcornStateException("Can not process operation " + op + " for cluster " + uid);
+
+                       }
+               }
+
+       }
+
+       private void processSetImmutable(int op) {
+               int value = bytes[pos++]&0xff;
+               setImmutable(value > 0);
+       }
+
+       private void processUndoValue(int op) {
+               Bytes.readLE4(bytes, pos);
+               pos+=4;
+       }
+
        abstract void create() throws DatabaseException;
        abstract void delete(int resourceIndex) throws DatabaseException;
        abstract void modify(int resourceKey, long offset, int size, byte[] bytes, int pos) throws DatabaseException;
        abstract void set(int resourceKey, byte[] bytes, int length) throws DatabaseException;
-       
+
        abstract void claim(int resourceKey, int predicateKey, int objectKey, ClusterUID puid, ClusterUID ouid) throws DatabaseException;
        abstract void deny(int resourceKey, int predicateKey, int objectKey, ClusterUID puid, ClusterUID ouid) throws DatabaseException;
-       
+
+       abstract void setImmutable(boolean value);
+
 }
diff --git a/bundles/org.simantics.acorn/src/org/simantics/acorn/internal/ClusterUpdateProcessorBase2.java b/bundles/org.simantics.acorn/src/org/simantics/acorn/internal/ClusterUpdateProcessorBase2.java
deleted file mode 100644 (file)
index 502729c..0000000
+++ /dev/null
@@ -1,62 +0,0 @@
-package org.simantics.acorn.internal;
-
-import org.simantics.acorn.exception.IllegalAcornStateException;
-import org.simantics.db.service.Bytes;
-import org.simantics.db.service.ClusterUID;
-
-public abstract class ClusterUpdateProcessorBase2 {
-
-       final private byte[] bytes;
-       private int pos = 0;
-       final private int len;
-       final private ClusterUID uid;
-       
-       public ClusterUpdateProcessorBase2(byte[] operations) {
-               this.bytes = operations;
-               this.len = Bytes.readLE4(bytes, 0) + 4; // whatta?
-               int version = Bytes.readLE4(bytes, 4);
-               assert(version == ClusterChange2.VERSION);
-               long cuid1 = Bytes.readLE8(bytes, 8);
-               long cuid2 = Bytes.readLE8(bytes, 16);
-               pos = 24;
-               uid = ClusterUID.make(cuid1, cuid2);
-       }
-       
-       public ClusterUID getClusterUID() {
-               return uid;
-       }
-
-       private void processSetImmutable(int op) {
-               int value = bytes[pos++]&0xff;
-               setImmutable(value > 0);
-       }
-
-       private void processUndoValue(int op) {
-               Bytes.readLE4(bytes, pos);
-               pos+=4;
-       }
-
-       public void process() throws IllegalAcornStateException {
-               
-               while(pos < len) {
-               
-                       int op = bytes[pos++]&0xff;
-                       
-                       switch(op) {
-                       
-                       case ClusterChange2.SET_IMMUTABLE_OPERATION:
-                               processSetImmutable(op);
-                               break;
-                       case ClusterChange2.UNDO_VALUE_OPERATION:
-                               processUndoValue(op);
-                               break;
-                       default:
-                               throw new IllegalAcornStateException("Can not process cluster " + uid);
-                               
-                       }
-               }
-       }
-       
-       abstract void setImmutable(boolean value);
-       
-}
index 8b3e4f066a4e53a6033bc4c9fc8892aacee3f6eb..e75e2e0ceae71ad77da44d013442b6f10a996e11 100644 (file)
@@ -13,17 +13,21 @@ import org.simantics.acorn.lru.ClusterChangeSet.Entry;
 import org.simantics.acorn.lru.ClusterChangeSet.Type;
 import org.simantics.db.exception.DatabaseException;
 import org.simantics.db.service.ClusterUID;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 public class UndoClusterUpdateProcessor extends ClusterUpdateProcessorBase {
-       
+
+       private static final Logger LOGGER = LoggerFactory.getLogger(UndoClusterUpdateProcessor.class);
+
        public final static boolean DEBUG = false;
 
        final private ClusterChangeSet ccs;
-       
+
        private int oldValuesIndex = 0;
        private int statementMaskIndex = 0;
-       
-       final public List<Entry> entries = new ArrayList<Entry>();
+
+       final public List<Entry> entries = new ArrayList<>();
        
        public UndoClusterUpdateProcessor(ClusterManager client, ClusterStreamChunk chunk, ClusterChangeSet ccs) throws DatabaseException {
                super(client, readOperation(client, chunk, ccs));
@@ -31,27 +35,8 @@ public class UndoClusterUpdateProcessor extends ClusterUpdateProcessorBase {
        }
        
        private static byte[] readOperation(ClusterManager manager, ClusterStreamChunk chunk, ClusterChangeSet ccs) throws AcornAccessVerificationException, IllegalAcornStateException {
-               
-//             ClusterStreamChunk chunk;
-//             manager.streamLRU.acquireMutex();
-//             try {
-//                     chunk = ccs.getChunk(manager);
-//             } catch (Throwable t) {
-//                     throw new IllegalStateException(t);
-//             } finally {
-//                     manager.streamLRU.releaseMutex();
-//             }
-//
-//             chunk.acquireMutex();
-//             try {
-//             chunk.ve
-                       chunk.makeResident();
-                       return chunk.getOperation(ccs.chunkOffset);
-//             } catch (Throwable t) {
-//                     throw new IllegalStateException(t);
-//             } finally {
-//                     chunk.releaseMutex();
-//             }
+               chunk.makeResident();
+               return chunk.getOperation(ccs.chunkOffset);
        }
        
        @Override
@@ -110,5 +95,10 @@ public class UndoClusterUpdateProcessor extends ClusterUpdateProcessorBase {
                }
 
        }
-       
+
+       @Override
+       void setImmutable(boolean value) {
+               LOGGER.error("Attempted to undo `setImmutable({})` cluster operation for cluster {} which is not supported.", value, ccs.cuid);
+       }
+
 }
index 12d4d55b8cb20f92b79721d76d8df5ba3061ab3b..767746eb005864c2046a5e1b50237a1e03006030 100644 (file)
@@ -9,7 +9,6 @@ import org.simantics.acorn.ClusterManager;
 import org.simantics.acorn.Persistable;
 import org.simantics.acorn.exception.AcornAccessVerificationException;
 import org.simantics.acorn.exception.IllegalAcornStateException;
-import org.simantics.acorn.internal.ClusterChange;
 import org.simantics.acorn.internal.UndoClusterUpdateProcessor;
 import org.simantics.compressions.CompressionCodec;
 import org.simantics.compressions.Compressions;
@@ -60,8 +59,6 @@ public class ClusterStreamChunk extends LRUObject<String, ClusterStreamChunk> im
                if(op.ccs == null) throw new IllegalAcornStateException("Cluster ChangeSet " + ccsId + " was not found.");
 
                UndoClusterUpdateProcessor proc = new UndoClusterUpdateProcessor(clusters, this, op.ccs);
-               if(proc.version != ClusterChange.VERSION)
-                       return null;
 
                // This cluster and CCS can still be under preparation => wait
                clusters.clusterLRU.ensureUpdates(proc.getClusterUID());
index c23821b7973c51b8e0a8828c7fc66b3e894c4d08..254fbf55c10cf121f16479261534894d86e4f51b 100644 (file)
@@ -4,10 +4,7 @@ import org.simantics.acorn.ClusterManager;
 import org.simantics.acorn.cluster.ClusterImpl;
 import org.simantics.acorn.exception.AcornAccessVerificationException;
 import org.simantics.acorn.exception.IllegalAcornStateException;
-import org.simantics.acorn.internal.ClusterChange;
-import org.simantics.acorn.internal.ClusterChange2;
 import org.simantics.acorn.internal.ClusterUpdateProcessor;
-import org.simantics.acorn.internal.ClusterUpdateProcessor2;
 import org.simantics.db.service.Bytes;
 import org.simantics.db.service.ClusterUID;
 
@@ -59,22 +56,11 @@ final public class ClusterUpdateOperation {
        }
        
        public void runWithData(byte[] data) throws IllegalAcornStateException, AcornAccessVerificationException {
-               
                try {
-                       int version = Bytes.readLE4(data, 4);
-                       if(version == ClusterChange.VERSION) {
-                               ClusterUpdateProcessor processor = new ClusterUpdateProcessor(manager, manager.support, data, this);
-                               ClusterImpl cluster = info.getForUpdate();
-                               cluster = processor.process(cluster);
-                               manager.update(uid, cluster);
-                       } else if (version == ClusterChange2.VERSION) {
-                               ClusterUpdateProcessor2 processor = new ClusterUpdateProcessor2(manager.support, data, this);
-                               ClusterImpl cluster = info.getForUpdate();
-                               processor.process(cluster);
-                               manager.update(uid, cluster);
-                       } else {
-                               throw new IllegalAcornStateException("unsupported clusterChange version " + version);
-                       }
+                       ClusterUpdateProcessor processor = new ClusterUpdateProcessor(manager, manager.support, data, this);
+                       ClusterImpl cluster = info.getForUpdate();
+                       cluster = processor.process(cluster);
+                       manager.update(uid, cluster);
                } catch (IllegalAcornStateException | AcornAccessVerificationException e) {
                    throw e;
                } catch (Throwable t) {
index 57bbd44049052c696bc627ea7e7bc816634a3db4..cebcdcf4294f270b8a17a79a12632fa52a9dfda2 100644 (file)
@@ -6,7 +6,7 @@ import org.simantics.db.procore.cluster.ClusterImpl;
 import org.simantics.db.service.Bytes;
 import org.simantics.db.service.ClusterUID;
 
-class ClusterChange2 {
+public class ClusterChange2 {
     public static final int VERSION = 2;
     public static final byte SET_IMMUTABLE_OPERATION = 1; // <byte : 0 = false>
     public static final byte UNDO_VALUE_OPERATION = 2; // <int : resource index>