You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kafka.apache.org by da...@apache.org on 2022/03/22 08:42:46 UTC
[kafka] branch 3.1 updated: KAFKA-13752: Uuid compare using equals in java (#11912)
This is an automated email from the ASF dual-hosted git repository.
dajac pushed a commit to branch 3.1
in repository https://gitbox.apache.org/repos/asf/kafka.git
The following commit(s) were added to refs/heads/3.1 by this push:
new 52910e4 KAFKA-13752: Uuid compare using equals in java (#11912)
52910e4 is described below
commit 52910e48f01ecb6f36bee4d7cab7d7588d79f57d
Author: Xiaobing Fang <bi...@qq.com>
AuthorDate: Tue Mar 22 16:31:46 2022 +0800
KAFKA-13752: Uuid compare using equals in java (#11912)
This patch fixes a few cases where we use `==` instead of `equals` to compare UUID. The impact of this bug is low because `Uuid.ZERO_UUID` is used by default everywhere.
Reviewers: Justine Olshan <jo...@confluent.io>, dengziming <de...@gmail.com>, David Jacot <dj...@confluent.io>
---
.../kafka/common/requests/MetadataRequest.java | 2 +-
.../kafka/common/requests/MetadataResponse.java | 2 +-
.../kafka/common/requests/MetadataRequestTest.java | 28 ++++++---
.../common/requests/MetadataResponseTest.java | 66 ++++++++++++++++++++++
.../common/requests/UpdateMetadataRequestTest.java | 2 +-
5 files changed, 90 insertions(+), 10 deletions(-)
diff --git a/clients/src/main/java/org/apache/kafka/common/requests/MetadataRequest.java b/clients/src/main/java/org/apache/kafka/common/requests/MetadataRequest.java
index aab5fc6..48609b1 100644
--- a/clients/src/main/java/org/apache/kafka/common/requests/MetadataRequest.java
+++ b/clients/src/main/java/org/apache/kafka/common/requests/MetadataRequest.java
@@ -112,7 +112,7 @@ public class MetadataRequest extends AbstractRequest {
if (topic.name() == null && version < 12)
throw new UnsupportedVersionException("MetadataRequest version " + version +
" does not support null topic names.");
- if (topic.topicId() != Uuid.ZERO_UUID && version < 12)
+ if (!Uuid.ZERO_UUID.equals(topic.topicId()) && version < 12)
throw new UnsupportedVersionException("MetadataRequest version " + version +
" does not support non-zero topic IDs.");
});
diff --git a/clients/src/main/java/org/apache/kafka/common/requests/MetadataResponse.java b/clients/src/main/java/org/apache/kafka/common/requests/MetadataResponse.java
index d539fa8..3696b04 100644
--- a/clients/src/main/java/org/apache/kafka/common/requests/MetadataResponse.java
+++ b/clients/src/main/java/org/apache/kafka/common/requests/MetadataResponse.java
@@ -151,7 +151,7 @@ public class MetadataResponse extends AbstractResponse {
if (metadata.error == Errors.NONE) {
if (metadata.isInternal)
internalTopics.add(metadata.topic);
- if (metadata.topicId() != null && metadata.topicId() != Uuid.ZERO_UUID) {
+ if (metadata.topicId() != null && !Uuid.ZERO_UUID.equals(metadata.topicId())) {
topicIds.put(metadata.topic, metadata.topicId());
}
for (PartitionMetadata partitionMetadata : metadata.partitionMetadata) {
diff --git a/clients/src/test/java/org/apache/kafka/common/requests/MetadataRequestTest.java b/clients/src/test/java/org/apache/kafka/common/requests/MetadataRequestTest.java
index 74c217d..84764c2 100644
--- a/clients/src/test/java/org/apache/kafka/common/requests/MetadataRequestTest.java
+++ b/clients/src/test/java/org/apache/kafka/common/requests/MetadataRequestTest.java
@@ -26,6 +26,7 @@ import java.util.Arrays;
import java.util.Collections;
import java.util.List;
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNull;
@@ -82,12 +83,25 @@ public class MetadataRequestTest {
// if version is 10 or 11, the invalid topic metadata should return an error
List<Short> invalidVersions = Arrays.asList((short) 10, (short) 11);
- invalidVersions.forEach(version ->
- topics.forEach(topic -> {
- MetadataRequestData metadataRequestData = new MetadataRequestData().setTopics(Collections.singletonList(topic));
- MetadataRequest.Builder builder = new MetadataRequest.Builder(metadataRequestData);
- assertThrows(UnsupportedVersionException.class, () -> builder.build(version));
- })
- );
+ invalidVersions.forEach(version -> topics.forEach(topic -> {
+ MetadataRequestData metadataRequestData = new MetadataRequestData().setTopics(Collections.singletonList(topic));
+ MetadataRequest.Builder builder = new MetadataRequest.Builder(metadataRequestData);
+ assertThrows(UnsupportedVersionException.class, () -> builder.build(version));
+ }));
+ }
+
+ @Test
+ public void testTopicIdWithZeroUuid() {
+ List<MetadataRequestData.MetadataRequestTopic> topics = Arrays.asList(
+ new MetadataRequestData.MetadataRequestTopic().setName("topic").setTopicId(Uuid.ZERO_UUID),
+ new MetadataRequestData.MetadataRequestTopic().setName("topic").setTopicId(new Uuid(0L, 0L)),
+ new MetadataRequestData.MetadataRequestTopic().setName("topic"));
+
+ List<Short> invalidVersions = Arrays.asList((short) 10, (short) 11);
+ invalidVersions.forEach(version -> topics.forEach(topic -> {
+ MetadataRequestData metadataRequestData = new MetadataRequestData().setTopics(Collections.singletonList(topic));
+ MetadataRequest.Builder builder = new MetadataRequest.Builder(metadataRequestData);
+ assertDoesNotThrow(() -> builder.build(version));
+ }));
}
}
diff --git a/clients/src/test/java/org/apache/kafka/common/requests/MetadataResponseTest.java b/clients/src/test/java/org/apache/kafka/common/requests/MetadataResponseTest.java
new file mode 100644
index 0000000..37f7356
--- /dev/null
+++ b/clients/src/test/java/org/apache/kafka/common/requests/MetadataResponseTest.java
@@ -0,0 +1,66 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.kafka.common.requests;
+
+import org.apache.kafka.common.Cluster;
+import org.apache.kafka.common.Uuid;
+import org.apache.kafka.common.message.MetadataResponseData;
+import org.apache.kafka.common.protocol.ApiKeys;
+import org.apache.kafka.common.protocol.Errors;
+import org.junit.jupiter.api.Test;
+
+import static java.util.Collections.emptyList;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
+
+public class MetadataResponseTest {
+
+ @Test
+ void buildClusterTest() {
+ Uuid zeroUuid = new Uuid(0L, 0L);
+ Uuid randomUuid = Uuid.randomUuid();
+ MetadataResponseData.MetadataResponseTopic topicMetadata1 = new MetadataResponseData.MetadataResponseTopic()
+ .setName("topic1")
+ .setErrorCode(Errors.NONE.code())
+ .setPartitions(emptyList())
+ .setIsInternal(false);
+ MetadataResponseData.MetadataResponseTopic topicMetadata2 = new MetadataResponseData.MetadataResponseTopic()
+ .setName("topic2")
+ .setErrorCode(Errors.NONE.code())
+ .setTopicId(zeroUuid)
+ .setPartitions(emptyList())
+ .setIsInternal(false);
+ MetadataResponseData.MetadataResponseTopic topicMetadata3 = new MetadataResponseData.MetadataResponseTopic()
+ .setName("topic3")
+ .setErrorCode(Errors.NONE.code())
+ .setTopicId(randomUuid)
+ .setPartitions(emptyList())
+ .setIsInternal(false);
+
+ MetadataResponseData.MetadataResponseTopicCollection topics =
+ new MetadataResponseData.MetadataResponseTopicCollection();
+ topics.add(topicMetadata1);
+ topics.add(topicMetadata2);
+ topics.add(topicMetadata3);
+ MetadataResponse metadataResponse = new MetadataResponse(new MetadataResponseData().setTopics(topics),
+ ApiKeys.METADATA.latestVersion());
+ Cluster cluster = metadataResponse.buildCluster();
+ assertNull(cluster.topicName(Uuid.ZERO_UUID));
+ assertNull(cluster.topicName(zeroUuid));
+ assertEquals("topic3", cluster.topicName(randomUuid));
+ }
+}
diff --git a/clients/src/test/java/org/apache/kafka/common/requests/UpdateMetadataRequestTest.java b/clients/src/test/java/org/apache/kafka/common/requests/UpdateMetadataRequestTest.java
index 6f9d5c2..2dd17f7 100644
--- a/clients/src/test/java/org/apache/kafka/common/requests/UpdateMetadataRequestTest.java
+++ b/clients/src/test/java/org/apache/kafka/common/requests/UpdateMetadataRequestTest.java
@@ -203,7 +203,7 @@ public class UpdateMetadataRequestTest {
long topicIdCount = deserializedRequest.data().topicStates().stream()
.map(UpdateMetadataRequestData.UpdateMetadataTopicState::topicId)
- .filter(topicId -> topicId != Uuid.ZERO_UUID).count();
+ .filter(topicId -> !Uuid.ZERO_UUID.equals(topicId)).count();
if (version >= 7)
assertEquals(2, topicIdCount);
else