zookeeper-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From an...@apache.org
Subject [zookeeper] branch master updated: ZOOKEEPER-3495: fix SnapshotDigestTest to work with JDK12+
Date Tue, 10 Sep 2019 13:21:14 GMT
This is an automated email from the ASF dual-hosted git repository.

andor pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/zookeeper.git


The following commit(s) were added to refs/heads/master by this push:
     new f01d01c  ZOOKEEPER-3495: fix SnapshotDigestTest to work with JDK12+
f01d01c is described below

commit f01d01ce3ba86baf9bd623f91d97a560703260e4
Author: Mate Szalay-Beko <szalay.beko.mate@gmail.com>
AuthorDate: Tue Sep 10 15:21:05 2019 +0200

    ZOOKEEPER-3495: fix SnapshotDigestTest to work with JDK12+
    
    The problem with the test `SnapshotDigestTest.testDifferentDigestVersion` was that it
used reflection to change a final static value in `DigestCalculator`, what is no longer supported
with JDK 12+ (see [JDK-8210522](https://bugs.openjdk.java.net/browse/JDK-8210522))
    
    I think there are still some hacky solutions to go behind these limitations (with [PowerMock](https://github.com/powermock/powermock)
maybe or using [VarHandle as suggested here](https://stackoverflow.com/questions/56039341/get-declared-fields-of-java-lang-reflect-fields-in-jdk12)),
but I think the best is to avoid the situation when we need to poke the static final field.
    
    I changed the fully static `DigestCalculator`, converting it to a static singleton object
with non-static methods / fields. The fields of the singleton object can not be modified from
production code, but we can change them from the tests even in JDK 12 / 13 (as they are not
static final fields).
    
    I tested it locally using openjdk, I hope it will also work in jenkins.
    
    Author: Mate Szalay-Beko <szalay.beko.mate@gmail.com>
    Author: Mate Szalay-Beko <mszalay@cloudera.com>
    
    Reviewers: fangmin@apache.org, eolivelli@apache.org, andor@apache.org
    
    Closes #1056 from symat/ZOOKEEPER-3495 and squashes the following commits:
    
    fa2a3b3c4 [Mate Szalay-Beko] Merge remote-tracking branch 'apache/master' into HEAD
    67c5532c8 [Mate Szalay-Beko] ZOOKEEPER-3495 move digestEnabled to the ZooKeeperServer
class
    c58a88c4f [Mate Szalay-Beko] ZOOKEEPER-3495: implement code-review comments
    1a8cc2b96 [Mate Szalay-Beko] Merge remote-tracking branch 'origin/master' into ZOOKEEPER-3495
    90da4c41e [Mate Szalay-Beko] ZOOKEEPER-3495: NodeHashMap use the same DigestCalculator
instance as DataTree
    e1f659cc8 [Mate Szalay-Beko] Merge remote-tracking branch 'origin/master' into ZOOKEEPER-3495
    dbbf38fc6 [Mate Szalay-Beko] ZOOKEEPER-3495: fix checkstyle errors
    07741ae18 [Mate Szalay-Beko] Merge remote-tracking branch 'origin/master' into ZOOKEEPER-3495
    d0d886dc6 [Mate Szalay-Beko] ZOOKEEPER-3495: change implementation according to code review
comments
    2a80c3216 [Mate Szalay-Beko] ZOOKEEPER-3495: fix SnapshotDigestTest to work with JDK12+
---
 .../java/org/apache/zookeeper/server/DataTree.java | 29 +++++++++++-------
 .../server/{util => }/DigestCalculator.java        | 33 ++++++---------------
 .../apache/zookeeper/server/NodeHashMapImpl.java   | 14 +++++----
 .../apache/zookeeper/server/ZooKeeperServer.java   | 15 ++++++++++
 .../org/apache/zookeeper/server/DataTreeTest.java  | 34 +++++++++-------------
 .../zookeeper/server/NodeHashMapImplTest.java      |  7 ++---
 .../zookeeper/server/SnapshotDigestTest.java       | 34 +++++++++-------------
 .../zookeeper/test/LoadFromLogNoServerTest.java    |  6 ++--
 8 files changed, 83 insertions(+), 89 deletions(-)

diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/DataTree.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/DataTree.java
index 0c4f223..9d552f0 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/DataTree.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/DataTree.java
@@ -56,7 +56,6 @@ import org.apache.zookeeper.common.PathTrie;
 import org.apache.zookeeper.data.ACL;
 import org.apache.zookeeper.data.Stat;
 import org.apache.zookeeper.data.StatPersisted;
-import org.apache.zookeeper.server.util.DigestCalculator;
 import org.apache.zookeeper.server.watch.IWatchManager;
 import org.apache.zookeeper.server.watch.WatchManagerFactory;
 import org.apache.zookeeper.server.watch.WatcherOrBitSet;
@@ -96,7 +95,7 @@ public class DataTree {
      * This map provides a fast lookup to the datanodes. The tree is the
      * source of truth and is where all the locking occurs
      */
-    private final NodeHashMap nodes = new NodeHashMapImpl();
+    private final NodeHashMap nodes;
 
     private IWatchManager dataWatches;
 
@@ -179,6 +178,8 @@ public class DataTree {
     // The historical digests list.
     private LinkedList<ZxidDigest> digestLog = new LinkedList<>();
 
+    private final DigestCalculator digestCalculator;
+
     @SuppressWarnings("unchecked")
     public Set<String> getEphemerals(long sessionId) {
         HashSet<String> retv = ephemerals.get(sessionId);
@@ -269,6 +270,13 @@ public class DataTree {
     private final DataNode quotaDataNode = new DataNode(new byte[0], -1L, new StatPersisted());
 
     public DataTree() {
+        this(new DigestCalculator());
+    }
+
+    DataTree(DigestCalculator digestCalculator) {
+        this.digestCalculator = digestCalculator;
+        nodes = new NodeHashMapImpl(digestCalculator);
+
         /* Rather than fight it, let root have an alias */
         nodes.put("", root);
         nodes.putWithoutDigest(rootZookeeper, root);
@@ -1594,7 +1602,7 @@ public class DataTree {
      * Add the digest to the historical list, and update the latest zxid digest.
      */
     private void logZxidDigest(long zxid, long digest) {
-        ZxidDigest zxidDigest = new ZxidDigest(zxid, DigestCalculator.DIGEST_VERSION, digest);
+        ZxidDigest zxidDigest = new ZxidDigest(zxid, digestCalculator.getDigestVersion(),
digest);
         lastProcessedZxidDigest = zxidDigest;
         if (zxidDigest.zxid % DIGEST_LOG_INTERVAL == 0) {
             synchronized (digestLog) {
@@ -1615,7 +1623,7 @@ public class DataTree {
      * @return true if the digest is serialized successfully
      */
     public boolean serializeZxidDigest(OutputArchive oa) throws IOException {
-        if (!DigestCalculator.digestEnabled()) {
+        if (!ZooKeeperServer.isDigestEnabled()) {
             return false;
         }
 
@@ -1636,7 +1644,7 @@ public class DataTree {
      * @return the true if it deserialized successfully
      */
     public boolean deserializeZxidDigest(InputArchive ia) throws IOException {
-        if (!DigestCalculator.digestEnabled()) {
+        if (!ZooKeeperServer.isDigestEnabled()) {
             return false;
         }
 
@@ -1661,9 +1669,9 @@ public class DataTree {
      */
     public void compareSnapshotDigests(long zxid) {
         if (zxid == digestFromLoadedSnapshot.zxid) {
-            if (DigestCalculator.DIGEST_VERSION != digestFromLoadedSnapshot.digestVersion)
{
+            if (digestCalculator.getDigestVersion() != digestFromLoadedSnapshot.digestVersion)
{
                 LOG.info("Digest version changed, local: {}, new: {}, "
-                         + "skip comparing digest now.", digestFromLoadedSnapshot.digestVersion,
DigestCalculator.DIGEST_VERSION);
+                         + "skip comparing digest now.", digestFromLoadedSnapshot.digestVersion,
digestCalculator.getDigestVersion());
                 digestFromLoadedSnapshot = null;
                 return;
             }
@@ -1722,10 +1730,9 @@ public class DataTree {
     }
 
     /**
-     * A helper class to maintain the digest meta associated with specific
-     * zxid.
+     * A helper class to maintain the digest meta associated with specific zxid.
      */
-    public static class ZxidDigest {
+    public class ZxidDigest {
 
         long zxid;
         // the digest value associated with this zxid
@@ -1734,7 +1741,7 @@ public class DataTree {
         int digestVersion;
 
         ZxidDigest() {
-            this(0, DigestCalculator.DIGEST_VERSION, 0);
+            this(0, digestCalculator.getDigestVersion(), 0);
         }
 
         ZxidDigest(long zxid, int digestVersion, long digest) {
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/util/DigestCalculator.java
b/zookeeper-server/src/main/java/org/apache/zookeeper/server/DigestCalculator.java
similarity index 79%
rename from zookeeper-server/src/main/java/org/apache/zookeeper/server/util/DigestCalculator.java
rename to zookeeper-server/src/main/java/org/apache/zookeeper/server/DigestCalculator.java
index aae68de..ca5041f 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/util/DigestCalculator.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/DigestCalculator.java
@@ -16,36 +16,26 @@
  * limitations under the License.
  */
 
-package org.apache.zookeeper.server.util;
+package org.apache.zookeeper.server;
 
 import java.nio.ByteBuffer;
 import java.util.zip.CRC32;
 import org.apache.zookeeper.ZooDefs;
 import org.apache.zookeeper.data.StatPersisted;
-import org.apache.zookeeper.server.DataNode;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 /**
  * Defines how to calculate the digest for a given node.
  */
-public class DigestCalculator {
+class DigestCalculator {
 
     private static final Logger LOG = LoggerFactory.getLogger(DigestCalculator.class);
 
     // The hardcoded digest version, should bump up this version whenever
     // we changed the digest method or fields.
-    //
-    // Defined it as Integer to make it able to be changed in test via reflection
-    public static final Integer DIGEST_VERSION = 2;
+    private static final int DIGEST_VERSION = 2;
 
-    public static final String ZOOKEEPER_DIGEST_ENABLED = "zookeeper.digest.enabled";
-    private static boolean digestEnabled;
-
-    static {
-        digestEnabled = Boolean.parseBoolean(System.getProperty(ZOOKEEPER_DIGEST_ENABLED,
"true"));
-        LOG.info("{} = {}", ZOOKEEPER_DIGEST_ENABLED, digestEnabled);
-    }
 
     /**
      * Calculate the digest based on the given params.
@@ -68,9 +58,9 @@ public class DigestCalculator {
      * @param stat the stat associated with the node
      * @return the digest calculated from the given params
      */
-    public static long calculateDigest(String path, byte[] data, StatPersisted stat) {
+    long calculateDigest(String path, byte[] data, StatPersisted stat) {
 
-        if (!digestEnabled()) {
+        if (!ZooKeeperServer.isDigestEnabled()) {
             return 0;
         }
 
@@ -120,7 +110,7 @@ public class DigestCalculator {
     /**
      * Calculate the digest based on the given path and data node.
      */
-    public static long calculateDigest(String path, DataNode node) {
+    long calculateDigest(String path, DataNode node) {
         if (!node.isDigestCached()) {
             node.setDigest(calculateDigest(path, node.getData(), node.stat));
             node.setDigestCached(true);
@@ -129,15 +119,10 @@ public class DigestCalculator {
     }
 
     /**
-     * Return true if the digest is enabled.
+     * Returns with the current digest version.
      */
-    public static boolean digestEnabled() {
-        return digestEnabled;
-    }
-
-    // Visible for test purpose
-    public static void setDigestEnabled(boolean enabled) {
-        digestEnabled = enabled;
+    int getDigestVersion() {
+        return DIGEST_VERSION;
     }
 
 }
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/NodeHashMapImpl.java
b/zookeeper-server/src/main/java/org/apache/zookeeper/server/NodeHashMapImpl.java
index 6ce8bf5..b28130b 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/NodeHashMapImpl.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/NodeHashMapImpl.java
@@ -22,7 +22,6 @@ import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import org.apache.zookeeper.server.util.AdHash;
-import org.apache.zookeeper.server.util.DigestCalculator;
 
 /**
  * a simple wrapper to ConcurrentHashMap that recalculates a digest after
@@ -33,6 +32,11 @@ public class NodeHashMapImpl implements NodeHashMap {
     private final ConcurrentHashMap<String, DataNode> nodes = new ConcurrentHashMap<String,
DataNode>();
 
     private AdHash hash = new AdHash();
+    private final DigestCalculator digestCalculator;
+
+    public NodeHashMapImpl(DigestCalculator digestCalculator) {
+        this.digestCalculator = digestCalculator;
+    }
 
     @Override
     public DataNode put(String path, DataNode node) {
@@ -98,14 +102,14 @@ public class NodeHashMapImpl implements NodeHashMap {
     }
 
     private void addDigest(String path, DataNode node) {
-        if (DigestCalculator.digestEnabled()) {
-            hash.addDigest(DigestCalculator.calculateDigest(path, node));
+        if (ZooKeeperServer.isDigestEnabled()) {
+            hash.addDigest(digestCalculator.calculateDigest(path, node));
         }
     }
 
     private void removeDigest(String path, DataNode node) {
-        if (DigestCalculator.digestEnabled()) {
-            hash.removeDigest(DigestCalculator.calculateDigest(path, node));
+        if (ZooKeeperServer.isDigestEnabled()) {
+            hash.removeDigest(digestCalculator.calculateDigest(path, node));
         }
     }
 
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServer.java
b/zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServer.java
index c623b87..b8e0bd2 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServer.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServer.java
@@ -107,6 +107,9 @@ public class ZooKeeperServer implements SessionExpirer, ServerStats.Provider
{
     public static final String SESSION_REQUIRE_CLIENT_SASL_AUTH = "zookeeper.sessionRequireClientSASLAuth";
     public static final String SASL_AUTH_SCHEME = "sasl";
 
+    public static final String ZOOKEEPER_DIGEST_ENABLED = "zookeeper.digest.enabled";
+    private static boolean digestEnabled;
+
     static {
         LOG = LoggerFactory.getLogger(ZooKeeperServer.class);
 
@@ -121,6 +124,9 @@ public class ZooKeeperServer implements SessionExpirer, ServerStats.Provider
{
         if (skipACL) {
             LOG.info(SKIP_ACL + "==\"yes\", ACL checks will be skipped");
         }
+
+        digestEnabled = Boolean.parseBoolean(System.getProperty(ZOOKEEPER_DIGEST_ENABLED,
"true"));
+        LOG.info("{} = {}", ZOOKEEPER_DIGEST_ENABLED, digestEnabled);
     }
 
     protected ZooKeeperServerBean jmxServerBean;
@@ -1743,6 +1749,15 @@ public class ZooKeeperServer implements SessionExpirer, ServerStats.Provider
{
         throw new KeeperException.NoAuthException();
     }
 
+    public static boolean isDigestEnabled() {
+        return digestEnabled;
+    }
+
+    public static void setDigestEnabled(boolean digestEnabled) {
+        LOG.info("{} = {}", ZOOKEEPER_DIGEST_ENABLED, digestEnabled);
+        ZooKeeperServer.digestEnabled = digestEnabled;
+    }
+
     /**
      * Trim a path to get the immediate predecessor.
      *
diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/server/DataTreeTest.java
b/zookeeper-server/src/test/java/org/apache/zookeeper/server/DataTreeTest.java
index 9ca5686..facacf3 100644
--- a/zookeeper-server/src/test/java/org/apache/zookeeper/server/DataTreeTest.java
+++ b/zookeeper-server/src/test/java/org/apache/zookeeper/server/DataTreeTest.java
@@ -46,11 +46,8 @@ import org.apache.zookeeper.ZooDefs;
 import org.apache.zookeeper.common.PathTrie;
 import org.apache.zookeeper.data.Stat;
 import org.apache.zookeeper.metrics.MetricsUtils;
-import org.apache.zookeeper.server.util.DigestCalculator;
 import org.apache.zookeeper.txn.CreateTxn;
 import org.apache.zookeeper.txn.TxnHeader;
-import org.junit.After;
-import org.junit.Before;
 import org.junit.Test;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -59,18 +56,6 @@ public class DataTreeTest extends ZKTestCase {
 
     protected static final Logger LOG = LoggerFactory.getLogger(DataTreeTest.class);
 
-    private DataTree dt;
-
-    @Before
-    public void setUp() throws Exception {
-        dt = new DataTree();
-    }
-
-    @After
-    public void tearDown() throws Exception {
-        dt = null;
-    }
-
     /**
      * For ZOOKEEPER-1755 - Test race condition when taking dumpEphemerals and
      * removing the session related ephemerals from DataTree structure
@@ -133,6 +118,7 @@ public class DataTreeTest extends ZKTestCase {
         }
         MyWatcher watcher = new MyWatcher();
         // set a watch on the root node
+        DataTree dt = new DataTree();
         dt.getChildren("/", new Stat(), watcher);
         // add a new node, should trigger a watch
         dt.createNode("/xyz", new byte[0], null, 0, dt.getNode("/").stat.getCversion() +
1, 1, 1);
@@ -145,7 +131,8 @@ public class DataTreeTest extends ZKTestCase {
     @Test(timeout = 60000)
     public void testIncrementCversion() throws Exception {
         try {
-            DigestCalculator.setDigestEnabled(true);
+            // digestCalculator gets initialized for the new DataTree constructor based on
the system property
+            ZooKeeperServer.setDigestEnabled(true);
             DataTree dt = new DataTree();
             dt.createNode("/test", new byte[0], null, 0, dt.getNode("/").stat.getCversion()
+ 1, 1, 1);
             DataNode zk = dt.getNode("/test");
@@ -166,12 +153,13 @@ public class DataTreeTest extends ZKTestCase {
                                       + ">", (newCversion == prevCversion + 1 &&
newPzxid == prevPzxid + 1));
             assertNotEquals(digestBefore, dt.getTreeDigest());
         } finally {
-            DigestCalculator.setDigestEnabled(false);
+            ZooKeeperServer.setDigestEnabled(false);
         }
     }
 
     @Test
     public void testNoCversionRevert() throws Exception {
+        DataTree dt = new DataTree();
         DataNode parent = dt.getNode("/");
         dt.createNode("/test", new byte[0], null, 0, parent.stat.getCversion() + 1, 1, 1);
         int currentCversion = parent.stat.getCversion();
@@ -193,6 +181,7 @@ public class DataTreeTest extends ZKTestCase {
 
     @Test
     public void testPzxidUpdatedWhenDeletingNonExistNode() throws Exception {
+        DataTree dt = new DataTree();
         DataNode root = dt.getNode("/");
         long currentPzxid = root.stat.getPzxid();
 
@@ -219,7 +208,10 @@ public class DataTreeTest extends ZKTestCase {
     @Test
     public void testDigestUpdatedWhenReplayCreateTxnForExistNode() {
         try {
-            DigestCalculator.setDigestEnabled(true);
+            // digestCalculator gets initialized for the new DataTree constructor based on
the system property
+            ZooKeeperServer.setDigestEnabled(true);
+            DataTree dt = new DataTree();
+
             dt.processTxn(new TxnHeader(13, 1000, 1, 30, ZooDefs.OpCode.create), new CreateTxn("/foo",
"".getBytes(), ZooDefs.Ids.OPEN_ACL_UNSAFE, false, 1));
 
             // create the same node with a higher cversion to simulate the
@@ -230,7 +222,7 @@ public class DataTreeTest extends ZKTestCase {
             // check the current digest value
             assertEquals(dt.getTreeDigest(), dt.getLastProcessedZxidDigest().digest);
         } finally {
-            DigestCalculator.setDigestEnabled(false);
+            ZooKeeperServer.setDigestEnabled(false);
         }
     }
 
@@ -456,7 +448,7 @@ public class DataTreeTest extends ZKTestCase {
     public void testDigest() throws Exception {
         try {
             // enable diegst check
-            DigestCalculator.setDigestEnabled(true);
+            ZooKeeperServer.setDigestEnabled(true);
 
             DataTree dt = new DataTree();
 
@@ -487,7 +479,7 @@ public class DataTreeTest extends ZKTestCase {
             dt.deleteNode("/digesttest/1", 5);
             assertNotEquals(dt.getTreeDigest(), previousDigest);
         } finally {
-            DigestCalculator.setDigestEnabled(false);
+            ZooKeeperServer.setDigestEnabled(false);
         }
     }
 
diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/server/NodeHashMapImplTest.java
b/zookeeper-server/src/test/java/org/apache/zookeeper/server/NodeHashMapImplTest.java
index 4a2fb69..b34dc90 100644
--- a/zookeeper-server/src/test/java/org/apache/zookeeper/server/NodeHashMapImplTest.java
+++ b/zookeeper-server/src/test/java/org/apache/zookeeper/server/NodeHashMapImplTest.java
@@ -24,7 +24,6 @@ import java.util.Map;
 import java.util.Set;
 import org.apache.zookeeper.ZKTestCase;
 import org.apache.zookeeper.data.StatPersisted;
-import org.apache.zookeeper.server.util.DigestCalculator;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
@@ -33,12 +32,12 @@ public class NodeHashMapImplTest extends ZKTestCase {
 
     @Before
     public void setUp() {
-        DigestCalculator.setDigestEnabled(true);
+        ZooKeeperServer.setDigestEnabled(true);
     }
 
     @After
     public void tearDown() {
-        DigestCalculator.setDigestEnabled(false);
+        ZooKeeperServer.setDigestEnabled(false);
     }
 
     /**
@@ -46,7 +45,7 @@ public class NodeHashMapImplTest extends ZKTestCase {
      */
     @Test
     public void testOperations() {
-        NodeHashMapImpl nodes = new NodeHashMapImpl();
+        NodeHashMapImpl nodes = new NodeHashMapImpl(new DigestCalculator());
 
         assertEquals(0, nodes.size());
         assertEquals(0L, nodes.getDigest());
diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/server/SnapshotDigestTest.java
b/zookeeper-server/src/test/java/org/apache/zookeeper/server/SnapshotDigestTest.java
index b3459a1..40f07a8 100644
--- a/zookeeper-server/src/test/java/org/apache/zookeeper/server/SnapshotDigestTest.java
+++ b/zookeeper-server/src/test/java/org/apache/zookeeper/server/SnapshotDigestTest.java
@@ -20,8 +20,6 @@ package org.apache.zookeeper.server;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNull;
-import java.lang.reflect.Field;
-import java.lang.reflect.Modifier;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.concurrent.ConcurrentHashMap;
@@ -33,7 +31,6 @@ import org.apache.zookeeper.ZooKeeper.States;
 import org.apache.zookeeper.server.metric.SimpleCounter;
 import org.apache.zookeeper.server.persistence.FileTxnSnapLog;
 import org.apache.zookeeper.server.quorum.QuorumPeerMainTest;
-import org.apache.zookeeper.server.util.DigestCalculator;
 import org.apache.zookeeper.test.ClientBase;
 import org.junit.After;
 import org.junit.Before;
@@ -68,13 +65,13 @@ public class SnapshotDigestTest extends ClientBase {
 
     @Override
     public void setupCustomizedEnv() {
-        DigestCalculator.setDigestEnabled(true);
+        ZooKeeperServer.setDigestEnabled(true);
         System.setProperty(ZooKeeperServer.SNAP_COUNT, "100");
     }
 
     @Override
     public void cleanUpCustomizedEnv() {
-        DigestCalculator.setDigestEnabled(false);
+        ZooKeeperServer.setDigestEnabled(false);
         System.clearProperty(ZooKeeperServer.SNAP_COUNT);
     }
 
@@ -129,7 +126,7 @@ public class SnapshotDigestTest extends ClientBase {
     @Test
     public void testDifferentDigestVersion() throws Exception {
         // check the current digest version
-        int currentVersion = DigestCalculator.DIGEST_VERSION;
+        int currentVersion = new DigestCalculator().getDigestVersion();
 
         // create a node
         String path = "/testDifferentDigestVersion";
@@ -138,23 +135,18 @@ public class SnapshotDigestTest extends ClientBase {
         // take a full snapshot
         server.takeSnapshot();
 
-        // using reflection to change the final static DIGEST_VERSION
+        //increment the digest version
         int newVersion = currentVersion + 1;
-        Field field = DigestCalculator.class.getDeclaredField("DIGEST_VERSION");
-        field.setAccessible(true);
-        Field modifiersField = Field.class.getDeclaredField("modifiers");
-        modifiersField.setAccessible(true);
-        modifiersField.setInt(field, field.getModifiers() & ~Modifier.FINAL);
-        field.set(null, newVersion);
-
-        assertEquals(newVersion, (int) DigestCalculator.DIGEST_VERSION);
+        DigestCalculator newVersionDigestCalculator = Mockito.spy(DigestCalculator.class);
+        Mockito.when(newVersionDigestCalculator.getDigestVersion()).thenReturn(newVersion);
+        assertEquals(newVersion, newVersionDigestCalculator.getDigestVersion());
 
         // using mock to return different digest value when the way we
         // calculate digest changed
         FileTxnSnapLog txnSnapLog = new FileTxnSnapLog(tmpDir, tmpDir);
-        DataTree dataTree = Mockito.spy(new DataTree());
+        DataTree dataTree = Mockito.spy(new DataTree(newVersionDigestCalculator));
         Mockito.when(dataTree.getTreeDigest()).thenReturn(0L);
-        txnSnapLog.restore(dataTree, new ConcurrentHashMap<Long, Integer>(), Mockito.mock(FileTxnSnapLog.PlayBackListener.class));
+        txnSnapLog.restore(dataTree, new ConcurrentHashMap<>(), Mockito.mock(FileTxnSnapLog.PlayBackListener.class));
 
         // make sure the reportDigestMismatch function is never called
         Mockito.verify(dataTree, Mockito.never()).reportDigestMismatch(Mockito.anyLong());
@@ -171,10 +163,10 @@ public class SnapshotDigestTest extends ClientBase {
         testCompatibleHelper(true, false);
     }
 
-    private void testCompatibleHelper(
-            boolean enabledBefore, boolean enabledAfter) throws Exception {
+    private void testCompatibleHelper(Boolean enabledBefore, Boolean enabledAfter) throws
Exception {
+
+        ZooKeeperServer.setDigestEnabled(enabledBefore);
 
-        DigestCalculator.setDigestEnabled(enabledBefore);
 
         // restart the server to cache the option change
         reloadSnapshotAndCheckDigest();
@@ -186,7 +178,7 @@ public class SnapshotDigestTest extends ClientBase {
         // take a full snapshot
         server.takeSnapshot();
 
-        DigestCalculator.setDigestEnabled(enabledAfter);
+        ZooKeeperServer.setDigestEnabled(enabledAfter);
 
         reloadSnapshotAndCheckDigest();
 
diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/test/LoadFromLogNoServerTest.java
b/zookeeper-server/src/test/java/org/apache/zookeeper/test/LoadFromLogNoServerTest.java
index 6cb79c6..5a19324 100644
--- a/zookeeper-server/src/test/java/org/apache/zookeeper/test/LoadFromLogNoServerTest.java
+++ b/zookeeper-server/src/test/java/org/apache/zookeeper/test/LoadFromLogNoServerTest.java
@@ -34,10 +34,10 @@ import org.apache.zookeeper.ZooDefs;
 import org.apache.zookeeper.common.Time;
 import org.apache.zookeeper.server.DataNode;
 import org.apache.zookeeper.server.DataTree;
+import org.apache.zookeeper.server.ZooKeeperServer;
 import org.apache.zookeeper.server.persistence.FileHeader;
 import org.apache.zookeeper.server.persistence.FileTxnLog;
 import org.apache.zookeeper.server.persistence.FileTxnSnapLog;
-import org.apache.zookeeper.server.util.DigestCalculator;
 import org.apache.zookeeper.txn.CreateTxn;
 import org.apache.zookeeper.txn.DeleteTxn;
 import org.apache.zookeeper.txn.MultiTxn;
@@ -58,7 +58,7 @@ public class LoadFromLogNoServerTest extends ZKTestCase {
     @Test
     public void testTxnFailure() throws Exception {
         try {
-            DigestCalculator.setDigestEnabled(true);
+            ZooKeeperServer.setDigestEnabled(true);
 
             long count = 1;
             File tmpDir = ClientBase.createTmpDir();
@@ -97,7 +97,7 @@ public class LoadFromLogNoServerTest extends ZKTestCase {
             // LOG.info("Attempting to delete " + "/test/" + (count + 1));
             // doOp(logFile, OpCode.delete, "/test/" + (count + 1), dt, zk);
         } finally {
-            DigestCalculator.setDigestEnabled(false);
+            ZooKeeperServer.setDigestEnabled(false);
         }
     }
 


Mime
View raw message