mirror of https://github.com/apache/kafka.git
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 <jolshan@confluent.io>, dengziming <dengziming1993@gmail.com>, David Jacot <djacot@confluent.io>
This commit is contained in:
parent
0924fd3f9f
commit
be4ef3df42
|
@ -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.");
|
||||
});
|
||||
|
|
|
@ -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) {
|
||||
|
|
|
@ -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 -> {
|
||||
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));
|
||||
}));
|
||||
}
|
||||
}
|
||||
|
|
|
@ -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));
|
||||
}
|
||||
}
|
|
@ -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
|
||||
|
|
Loading…
Reference in New Issue