From commits-return-7947-archive-asf-public=cust-asf.ponee.io@zookeeper.apache.org Tue Sep 10 13:21:15 2019 Return-Path: X-Original-To: archive-asf-public@cust-asf.ponee.io Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [207.244.88.153]) by mx-eu-01.ponee.io (Postfix) with SMTP id 255CF180642 for ; Tue, 10 Sep 2019 15:21:15 +0200 (CEST) Received: (qmail 83151 invoked by uid 500); 10 Sep 2019 13:21:14 -0000 Mailing-List: contact commits-help@zookeeper.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@zookeeper.apache.org Delivered-To: mailing list commits@zookeeper.apache.org Received: (qmail 83139 invoked by uid 99); 10 Sep 2019 13:21:14 -0000 Received: from ec2-52-202-80-70.compute-1.amazonaws.com (HELO gitbox.apache.org) (52.202.80.70) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 10 Sep 2019 13:21:14 +0000 Received: by gitbox.apache.org (ASF Mail Server at gitbox.apache.org, from userid 33) id 514C7812E2; Tue, 10 Sep 2019 13:21:14 +0000 (UTC) Date: Tue, 10 Sep 2019 13:21:14 +0000 To: "commits@zookeeper.apache.org" Subject: [zookeeper] branch master updated: ZOOKEEPER-3495: fix SnapshotDigestTest to work with JDK12+ MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Message-ID: <156812167419.10007.9444262128472410325@gitbox.apache.org> From: andor@apache.org X-Git-Host: gitbox.apache.org X-Git-Repo: zookeeper X-Git-Refname: refs/heads/master X-Git-Reftype: branch X-Git-Oldrev: 692ea8bd681d741845c77040c1d991e34d2e91fe X-Git-Newrev: f01d01ce3ba86baf9bd623f91d97a560703260e4 X-Git-Rev: f01d01ce3ba86baf9bd623f91d97a560703260e4 X-Git-NotificationType: ref_changed_plus_diff X-Git-Multimail-Version: 1.5.dev Auto-Submitted: auto-generated 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 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 Author: Mate Szalay-Beko 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 digestLog = new LinkedList<>(); + private final DigestCalculator digestCalculator; + @SuppressWarnings("unchecked") public Set getEphemerals(long sessionId) { HashSet 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 nodes = new ConcurrentHashMap(); 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(), 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); } }