zookeeper-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From an...@apache.org
Subject zookeeper git commit: ZOOKEEPER-3113: EphemeralType.get() fails to verify ephemeralOwner when currentElapsedTime() is small enough
Date Thu, 18 Oct 2018 09:18:46 GMT
Repository: zookeeper
Updated Branches:
  refs/heads/branch-3.5 f5c27607e -> 26012313d


ZOOKEEPER-3113: EphemeralType.get() fails to verify ephemeralOwner when currentElapsedTime()
is small enough

I refactored the unit test `testServerIds` to verify server id verification code explicitly
instead of through `EphemeralType.get()` method.

Reasons:
- The original test doesn't work on machines which booted recently, because the generated
`Time.currentElapsedTime()` value is not high enough and it's possible to generate a valid
ephemeralOwner even with high 0xff byte. Server ID cannot be verified reliably this way.
- EphemeralType.get() is already covered in other unit tests,
- Unit tests should test the smallest piece of logic and call the method under testing directly.

Author: Andor Molnar <andor@apache.org>

Reviewers: fangmin@apache.org, eolivelli@gmail.com, hanm@apache.org, andor@apache.org

Closes #651 from anmolnar/ZOOKEEPER-3113 and squashes the following commits:

7454d7e7 [Andor Molnar] ZOOKEEPER-3113. Added unit tests to cover EphemeralType.get edge cases
47dece7a [Andor Molnar] ZOOKEEPER-3113. Refactored unit test to validate server Id verification
code explicitly

(cherry picked from commit 5c85a236c9eb0805ea8389a52dab3b1bc0efadac)
Signed-off-by: Andor Molnar <andor@apache.org>


Project: http://git-wip-us.apache.org/repos/asf/zookeeper/repo
Commit: http://git-wip-us.apache.org/repos/asf/zookeeper/commit/26012313
Tree: http://git-wip-us.apache.org/repos/asf/zookeeper/tree/26012313
Diff: http://git-wip-us.apache.org/repos/asf/zookeeper/diff/26012313

Branch: refs/heads/branch-3.5
Commit: 26012313d3cb020ee3097a3b4cf46c9f6ae109c7
Parents: f5c2760
Author: Andor Molnar <andor@apache.org>
Authored: Thu Oct 18 11:18:25 2018 +0200
Committer: Andor Molnar <andor@apache.org>
Committed: Thu Oct 18 11:18:39 2018 +0200

----------------------------------------------------------------------
 .../zookeeper/server/Emulate353TTLTest.java     | 12 +++++++++
 .../zookeeper/server/EphemeralTypeTest.java     | 27 +++++++++++++++-----
 2 files changed, 33 insertions(+), 6 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/zookeeper/blob/26012313/zookeeper-server/src/test/java/org/apache/zookeeper/server/Emulate353TTLTest.java
----------------------------------------------------------------------
diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/server/Emulate353TTLTest.java
b/zookeeper-server/src/test/java/org/apache/zookeeper/server/Emulate353TTLTest.java
index c601a27..286fe4e 100644
--- a/zookeeper-server/src/test/java/org/apache/zookeeper/server/Emulate353TTLTest.java
+++ b/zookeeper-server/src/test/java/org/apache/zookeeper/server/Emulate353TTLTest.java
@@ -29,6 +29,8 @@ import org.junit.Test;
 
 import java.util.concurrent.atomic.AtomicLong;
 
+import static org.hamcrest.CoreMatchers.equalTo;
+
 public class Emulate353TTLTest extends ClientBase {
     private TestableZooKeeper zk;
 
@@ -83,6 +85,16 @@ public class Emulate353TTLTest extends ClientBase {
         Assert.assertNull("Ttl node should have been deleted", zk.exists("/foo", false));
     }
 
+    @Test
+    public void testEphemeralOwner_emulationTTL() {
+        Assert.assertThat(EphemeralType.get(-1), equalTo(EphemeralType.TTL));
+    }
+
+    @Test
+    public void testEphemeralOwner_emulationContainer() {
+        Assert.assertThat(EphemeralType.get(EphemeralType.CONTAINER_EPHEMERAL_OWNER), equalTo(EphemeralType.CONTAINER));
+    }
+
     private ContainerManager newContainerManager(final AtomicLong fakeElapsed) {
         return new ContainerManager(serverFactory.getZooKeeperServer()
                 .getZKDatabase(), serverFactory.getZooKeeperServer().firstProcessor, 1, 100)
{

http://git-wip-us.apache.org/repos/asf/zookeeper/blob/26012313/zookeeper-server/src/test/java/org/apache/zookeeper/server/EphemeralTypeTest.java
----------------------------------------------------------------------
diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/server/EphemeralTypeTest.java
b/zookeeper-server/src/test/java/org/apache/zookeeper/server/EphemeralTypeTest.java
index 40863f5..5c61ffc 100644
--- a/zookeeper-server/src/test/java/org/apache/zookeeper/server/EphemeralTypeTest.java
+++ b/zookeeper-server/src/test/java/org/apache/zookeeper/server/EphemeralTypeTest.java
@@ -24,6 +24,8 @@ import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
 
+import static org.hamcrest.CoreMatchers.equalTo;
+
 public class EphemeralTypeTest {
     @Before
     public void setUp() {
@@ -70,15 +72,28 @@ public class EphemeralTypeTest {
 
     @Test
     public void testServerIds() {
-        for ( int i = 0; i < 255; ++i ) {
-            Assert.assertEquals(EphemeralType.NORMAL, EphemeralType.get(SessionTrackerImpl.initializeNextSession(i)));
+        for ( int i = 0; i <= EphemeralType.MAX_EXTENDED_SERVER_ID; ++i ) {
+            EphemeralType.validateServerId(i);
         }
         try {
-            Assert.assertEquals(EphemeralType.TTL, EphemeralType.get(SessionTrackerImpl.initializeNextSession(255)));
-            Assert.fail("Should have thrown IllegalArgumentException");
-        } catch (IllegalArgumentException e) {
+            EphemeralType.validateServerId(EphemeralType.MAX_EXTENDED_SERVER_ID + 1);
+            Assert.fail("Should have thrown RuntimeException");
+        } catch (RuntimeException e) {
             // expected
         }
-        Assert.assertEquals(EphemeralType.NORMAL, EphemeralType.get(SessionTrackerImpl.initializeNextSession(EphemeralType.MAX_EXTENDED_SERVER_ID)));
+    }
+
+    @Test
+    public void testEphemeralOwner_extendedFeature_TTL() {
+        // 0xff = Extended feature is ON
+        // 0x0000 = Extended type id TTL (0)
+        Assert.assertThat(EphemeralType.get(0xff00000000000000L), equalTo(EphemeralType.TTL));
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void testEphemeralOwner_extendedFeature_extendedTypeUnsupported() {
+        // 0xff = Extended feature is ON
+        // 0x0001 = Unsupported extended type id (1)
+        EphemeralType.get(0xff00010000000000L);
     }
 }


Mime
View raw message