You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2020/11/05 20:56:58 UTC
[GitHub] [kafka] jolshan opened a new pull request #9566: KAFKA-10618: Update to Uuid class
jolshan opened a new pull request #9566:
URL: https://github.com/apache/kafka/pull/9566
As decided in KIP-516, the UUID class has been named Uuid. This PR changes all instances of org.apache.kafka.common.UUID to org.apache.kafka.common.Uuid.
It also modifies the Uuid class so that it no longer wraps a java.util.UUID object. Now it simply stores two longs.
### Committer Checklist (excluded from commit message)
- [ ] Verify design and implementation
- [ ] Verify test coverage and CI build status
- [ ] Verify documentation (including upgrade notes)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] jolshan commented on pull request #9566: KAFKA-10618: Update to Uuid class
Posted by GitBox <gi...@apache.org>.
jolshan commented on pull request #9566:
URL: https://github.com/apache/kafka/pull/9566#issuecomment-722639934
@cmccabe @ijuma
Let me know if this looks ok.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ijuma merged pull request #9566: KAFKA-10618: Update to Uuid class
Posted by GitBox <gi...@apache.org>.
ijuma merged pull request #9566:
URL: https://github.com/apache/kafka/pull/9566
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ijuma commented on a change in pull request #9566: KAFKA-10618: Update to Uuid class
Posted by GitBox <gi...@apache.org>.
ijuma commented on a change in pull request #9566:
URL: https://github.com/apache/kafka/pull/9566#discussion_r524722302
##########
File path: clients/src/test/java/org/apache/kafka/common/UuidTest.java
##########
@@ -21,50 +21,50 @@
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotEquals;
-public class UUIDTest {
+public class UuidTest {
@Test
public void testSignificantBits() {
- UUID id = new UUID(34L, 98L);
+ Uuid id = new Uuid(34L, 98L);
assertEquals(id.getMostSignificantBits(), 34L);
assertEquals(id.getLeastSignificantBits(), 98L);
}
@Test
- public void testUUIDEquality() {
- UUID id1 = new UUID(12L, 13L);
- UUID id2 = new UUID(12L, 13L);
- UUID id3 = new UUID(24L, 38L);
+ public void testUuidEquality() {
Review comment:
Can we add a test that verifies that the `hashCode` is the same for our `Uuid` and Java's `UUID`? Or do we have that already?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ijuma commented on a change in pull request #9566: KAFKA-10618: Update to Uuid class
Posted by GitBox <gi...@apache.org>.
ijuma commented on a change in pull request #9566:
URL: https://github.com/apache/kafka/pull/9566#discussion_r524738967
##########
File path: clients/src/test/java/org/apache/kafka/common/UuidTest.java
##########
@@ -21,50 +21,50 @@
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotEquals;
-public class UUIDTest {
+public class UuidTest {
@Test
public void testSignificantBits() {
- UUID id = new UUID(34L, 98L);
+ Uuid id = new Uuid(34L, 98L);
assertEquals(id.getMostSignificantBits(), 34L);
assertEquals(id.getLeastSignificantBits(), 98L);
}
@Test
- public void testUUIDEquality() {
- UUID id1 = new UUID(12L, 13L);
- UUID id2 = new UUID(12L, 13L);
- UUID id3 = new UUID(24L, 38L);
+ public void testUuidEquality() {
Review comment:
We don't have to specify it, but it would be good to ensure we have a test for the actual hashCode we're implementing. At the moment, we are only verifying that the hashCode is the same for two equal UUIDs and different for two unequal UUIDs. One option would be to have a few tests where we verify that the result is what we expect it to be.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] jolshan commented on a change in pull request #9566: KAFKA-10618: Update to Uuid class
Posted by GitBox <gi...@apache.org>.
jolshan commented on a change in pull request #9566:
URL: https://github.com/apache/kafka/pull/9566#discussion_r524727151
##########
File path: clients/src/test/java/org/apache/kafka/common/UuidTest.java
##########
@@ -21,50 +21,50 @@
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotEquals;
-public class UUIDTest {
+public class UuidTest {
@Test
public void testSignificantBits() {
- UUID id = new UUID(34L, 98L);
+ Uuid id = new Uuid(34L, 98L);
assertEquals(id.getMostSignificantBits(), 34L);
assertEquals(id.getLeastSignificantBits(), 98L);
}
@Test
- public void testUUIDEquality() {
- UUID id1 = new UUID(12L, 13L);
- UUID id2 = new UUID(12L, 13L);
- UUID id3 = new UUID(24L, 38L);
+ public void testUuidEquality() {
Review comment:
We don't have that yet. I also didn't specify this behavior. Should I?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ijuma commented on a change in pull request #9566: KAFKA-10618: Update to Uuid class
Posted by GitBox <gi...@apache.org>.
ijuma commented on a change in pull request #9566:
URL: https://github.com/apache/kafka/pull/9566#discussion_r523336595
##########
File path: clients/src/main/java/org/apache/kafka/common/protocol/types/Struct.java
##########
@@ -266,12 +266,12 @@ public Long getLong(String name) {
return (Long) get(name);
}
- public UUID getUUID(BoundField field) {
- return (UUID) get(field);
+ public Uuid getUUID(BoundField field) {
+ return (Uuid) get(field);
}
- public UUID getUUID(String name) {
- return (UUID) get(name);
+ public Uuid getUUID(String name) {
Review comment:
Should we update the method name too?
##########
File path: clients/src/main/java/org/apache/kafka/common/Uuid.java
##########
@@ -20,101 +20,100 @@
import java.util.Base64;
/**
- * This class defines an immutable universally unique identifier (UUID). It represents a 128-bit value.
- * More specifically, the random UUIDs generated by this class are variant 2 (Leach-Salz) version 4 UUIDs.
- * This is the same type of UUID as the ones generated by java.util.UUID except that the toString() method prints
+ * This class defines an immutable universally unique identifier (Uuid). It represents a 128-bit value.
+ * More specifically, the random Uuids generated by this class are variant 2 (Leach-Salz) version 4 Uuids.
+ * This is the same type of Uuid as the ones generated by java.util.UUID. The toString() method prints
* using the base64 string encoding. Likewise, the fromString method expects a base64 string encoding.
*/
-public class UUID {
+public class Uuid {
private static final java.util.UUID SENTINEL_ID_INTERNAL = new java.util.UUID(0L, 1L);
/**
- * A UUID that represents a null or empty UUID. Will never be returned by the randomUUID method
+ * A Uuid that represents a null or empty Uuid. Will never be returned by the randomUuid method.
*/
- public static final UUID ZERO_UUID = new UUID(new java.util.UUID(0L, 0L));
+ public static final Uuid ZERO_UUID = new Uuid(0L, 0L);
private static final java.util.UUID ZERO_ID_INTERNAL = new java.util.UUID(0L, 0L);
- private final java.util.UUID uuid;
+ private final long mostSignificantBits;
+ private final long leastSignificantBits;
/**
- * Constructs a 128-bit type 4 UUID where the first long represents the the most significant 64 bits
+ * Constructs a 128-bit type 4 Uuid where the first long represents the the most significant 64 bits
* and the second long represents the least significant 64 bits.
*/
- public UUID(long mostSigBits, long leastSigBits) {
- this.uuid = new java.util.UUID(mostSigBits, leastSigBits);
- }
-
- private UUID(java.util.UUID uuid) {
- this.uuid = uuid;
+ public Uuid(long mostSigBits, long leastSigBits) {
+ this.mostSignificantBits = mostSigBits;
+ this.leastSignificantBits = leastSigBits;
}
/**
- * Static factory to retrieve a type 4 (pseudo randomly generated) UUID.
+ * Static factory to retrieve a type 4 (pseudo randomly generated) Uuid.
*/
- public static UUID randomUUID() {
+ public static Uuid randomUuid() {
java.util.UUID uuid = java.util.UUID.randomUUID();
while (uuid.equals(SENTINEL_ID_INTERNAL) || uuid.equals(ZERO_ID_INTERNAL)) {
uuid = java.util.UUID.randomUUID();
}
- return new UUID(uuid);
+ return new Uuid(uuid.getMostSignificantBits(), uuid.getLeastSignificantBits());
}
/**
- * Returns the most significant bits of the UUID's 128 value.
+ * Returns the most significant bits of the Uuid's 128 value.
*/
public long getMostSignificantBits() {
- return uuid.getMostSignificantBits();
+ return this.mostSignificantBits;
}
/**
- * Returns the least significant bits of the UUID's 128 value.
+ * Returns the least significant bits of the Uuid's 128 value.
*/
public long getLeastSignificantBits() {
- return uuid.getLeastSignificantBits();
+ return this.leastSignificantBits;
}
/**
- * Returns true iff obj is another UUID represented by the same two long values.
+ * Returns true iff obj is another Uuid represented by the same two long values.
*/
@Override
public boolean equals(Object obj) {
if ((null == obj) || (obj.getClass() != this.getClass()))
return false;
- UUID id = (UUID) obj;
- return this.getMostSignificantBits() == id.getMostSignificantBits() &&
- this.getLeastSignificantBits() == id.getLeastSignificantBits();
+ Uuid id = (Uuid) obj;
+ return this.mostSignificantBits == id.mostSignificantBits &&
+ this.leastSignificantBits == id.leastSignificantBits;
}
/**
- * Returns a hash code for this UUID
+ * Returns a hash code for this Uuid
*/
@Override
public int hashCode() {
- return uuid.hashCode();
+ long xor = mostSignificantBits ^ leastSignificantBits;
+ return (int) (xor >> 32) ^ (int) xor;
}
/**
- * Returns a base64 string encoding of the UUID.
+ * Returns a base64 string encoding of the Uuid.
*/
@Override
public String toString() {
- return Base64.getUrlEncoder().withoutPadding().encodeToString(getBytesFromUuid(uuid));
+ return Base64.getUrlEncoder().withoutPadding().encodeToString(getBytesFromUuid());
}
/**
- * Creates a UUID based on a base64 string encoding used in the toString() method.
+ * Creates a Uuid based on a base64 string encoding used in the toString() method.
Review comment:
Isn't it better to keep `UUID` in javadoc when we're referring to the concept?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org