Fixed bad logical bug from Acorn's MainState.load rollback 82/482/3
authorTuukka Lehtonen <tuukka.lehtonen@semantum.fi>
Thu, 4 May 2017 21:52:29 +0000 (00:52 +0300)
committerjsimomaa <jani.simomaa@gmail.com>
Fri, 5 May 2017 04:53:15 +0000 (07:53 +0300)
The major bug was the logical not in MainState.load rollback which
caused the database revisioning to be started from 0 when the database
was *not* empty. It should have been the other way around.

Also cleaned up the database head.state validation code by not using
exceptions for flow control in validating head.state files.

refs #7124

Change-Id: I7cd57fa73d39a637c71159df63566aed5063fc40

bundles/org.simantics.acorn/src/org/simantics/acorn/HeadState.java
bundles/org.simantics.acorn/src/org/simantics/acorn/MainState.java

index fd38bc98542052bbdb2e762c65d49a4cad4f3585..a6a1622c80027d68b11b05a7de6b6d1891bb26f5 100644 (file)
@@ -16,9 +16,13 @@ import org.simantics.databoard.adapter.AdapterConstructionException;
 import org.simantics.databoard.binding.mutable.MutableVariant;
 import org.simantics.databoard.serialization.Serializer;
 import org.simantics.databoard.util.binary.BinaryMemory;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 public class HeadState {
 
+    private static transient final Logger LOGGER = LoggerFactory.getLogger(HeadState.class);
+    
     public static final String HEAD_STATE = "head.state";
     public static final String SHA_1 = "SHA-1";
     
@@ -41,7 +45,6 @@ public class HeadState {
             byte[] bytes = Files.readAllBytes(f);
             MessageDigest sha1 = MessageDigest.getInstance(SHA_1);
             int digestLength = sha1.getDigestLength();
-            
             sha1.update(bytes, digestLength, bytes.length - digestLength);
             byte[] newChecksum = sha1.digest();
             if (!Arrays.equals(newChecksum, Arrays.copyOfRange(bytes, 0, digestLength))) {
@@ -96,7 +99,7 @@ public class HeadState {
         }
     }
 
-    public static void validateHeadStateIntegrity(Path headState) throws InvalidHeadStateException, IOException {
+    public static boolean validateHeadStateIntegrity(Path headState) {
         try {
             byte[] bytes = Files.readAllBytes(headState);
             MessageDigest sha1 = MessageDigest.getInstance(SHA_1);
@@ -104,12 +107,17 @@ public class HeadState {
             sha1.update(bytes, digestLength, bytes.length - digestLength);
             byte[] newChecksum = sha1.digest();
             if (!Arrays.equals(newChecksum, Arrays.copyOfRange(bytes, 0, digestLength))) {
-                throw new InvalidHeadStateException(
-                        "Checksum " + Arrays.toString(newChecksum) + " does not match excpected "
-                                + Arrays.toString(Arrays.copyOfRange(bytes, 0, digestLength)) + " for " + headState.toAbsolutePath());
+                LOGGER.error("Checksum " + Arrays.toString(newChecksum) + " does not match excpected " + Arrays.toString(Arrays.copyOfRange(bytes, 0, digestLength)) + " for " + headState.toAbsolutePath());
+                return false;
             }
+            return true;
+        } catch (IOException e) {
+            LOGGER.error("An I/O error occured while validating integrity of head.state", e);
+            return false;
         } catch (NoSuchAlgorithmException e) {
+            LOGGER.error("SHA-1 digest not found, should not happen", e);
             throw new Error("SHA-1 digest not found, should not happen", e);
         }
     }
+
 }
index 63fc5d4937b06cd0aff10b571506e66830741535..ebc4079d51c64ccf1c2b40e372b89c3602e399cf 100644 (file)
@@ -15,7 +15,6 @@ import java.util.function.Predicate;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
-import org.simantics.acorn.exception.InvalidHeadStateException;
 import org.simantics.databoard.Bindings;
 import org.simantics.databoard.binding.mutable.MutableVariant;
 import org.simantics.databoard.util.binary.BinaryMemory;
@@ -49,10 +48,10 @@ public class MainState implements Serializable {
                     Bindings.getBindingUnchecked(MainState.class));
             int latestRevision = state.headDir - 1;
             try {
-                HeadState.validateHeadStateIntegrity(directory.resolve(latestRevision + "/" + HeadState.HEAD_STATE));
-                archiveRevisionDirectories(directory, latestRevision, rollbackCallback);
-                return state;
-            } catch (InvalidHeadStateException e) {
+                if (HeadState.validateHeadStateIntegrity(directory.resolve(latestRevision + "/" + HeadState.HEAD_STATE))) {
+                    archiveRevisionDirectories(directory, latestRevision, rollbackCallback);
+                    return state;
+                }
                 LOGGER.warn("Failed to start database from revision " + latestRevision + " stored in " + mainState + ". " + HeadState.HEAD_STATE + " is invalid.");
                 return rollback(directory, rollbackCallback);
             } catch (FileNotFoundException e) {
@@ -61,7 +60,7 @@ public class MainState implements Serializable {
             }
         } catch (FileNotFoundException e) {
             // The database may also be totally empty at this point
-            if (!listRevisionDirs(directory, true, MainState::isInteger).isEmpty())
+            if (listRevisionDirs(directory, true, MainState::isInteger).isEmpty())
                 return new MainState(0);
 
             LOGGER.warn("Unclean exit detected, " + mainState + " not found. Initiating automatic rollback.");
@@ -77,7 +76,7 @@ public class MainState implements Serializable {
     private static MainState rollback(Path directory, Runnable rollbackCallback) throws IOException {
         LOGGER.warn("Database rollback initiated for " + directory);
         rollbackCallback.run();
-        Path latest = findNewHeadStateDir(directory, rollbackCallback);
+        Path latest = findNewHeadStateDir(directory);
         int latestRevision = latest != null ? safeParseInt(-1, latest) : -1;
         // +1 because we want to return the next head version to use,
         // not the latest existing version.
@@ -126,16 +125,10 @@ public class MainState implements Serializable {
      * @return
      * @throws IOException
      */
-    private static Path findNewHeadStateDir(Path directory, Runnable rollbackCallback) throws IOException {
-        for (Path last : listRevisionDirs(directory, true, MainState::isInteger)) {
-            try {
-                HeadState.validateHeadStateIntegrity(last.resolve(HeadState.HEAD_STATE));
+    private static Path findNewHeadStateDir(Path directory) throws IOException {
+        for (Path last : listRevisionDirs(directory, true, MainState::isInteger))
+            if (HeadState.validateHeadStateIntegrity(last.resolve(HeadState.HEAD_STATE)))
                 return last;
-            } catch (IOException | InvalidHeadStateException e) {
-                // Cleanup is done in {@link cleanRevisionDirectories} method
-                rollbackCallback.run();
-            }
-        }
         return null;
     }