mimaison commented on code in PR #16065:
URL: https://github.com/apache/kafka/pull/16065#discussion_r1622393809
##########
metadata/src/test/java/org/apache/kafka/metadata/RecordTestUtils.java:
##########
@@ -373,12 +373,12 @@ public static RegisterControllerRecord
createTestControllerRegistration(
).iterator()
)).
setFeatures(new
RegisterControllerRecord.ControllerFeatureCollection(
- Arrays.asList(
- new RegisterControllerRecord.ControllerFeature().
- setName(MetadataVersion.FEATURE_NAME).
-
setMinSupportedVersion(MetadataVersion.MINIMUM_KRAFT_VERSION.featureLevel()).
-
setMaxSupportedVersion(MetadataVersion.IBP_3_6_IV1.featureLevel())
- ).iterator()
+ Collections.singletonList(
Review Comment:
Can you undo the indentation changes?
##########
metadata/src/test/java/org/apache/kafka/image/ImageDowngradeTest.java:
##########
@@ -83,18 +83,18 @@ static ApiMessageAndVersion
metadataVersionRecord(MetadataVersion metadataVersio
@Test
public void testPremodernVersion() {
writeWithExpectedLosses(MetadataVersion.IBP_3_2_IV0,
- Arrays.asList(
- "feature flag(s): foo.feature"),
- Arrays.asList(
- metadataVersionRecord(MetadataVersion.IBP_3_3_IV0),
- TEST_RECORDS.get(0),
- TEST_RECORDS.get(1),
- new ApiMessageAndVersion(new FeatureLevelRecord().
- setName("foo.feature").
- setFeatureLevel((short) 4), (short) 0)),
- Arrays.asList(
- TEST_RECORDS.get(0),
- TEST_RECORDS.get(1)));
+ Collections.singletonList(
Review Comment:
Can we undo the indentation changes?
##########
metadata/src/test/java/org/apache/kafka/controller/QuorumControllerIntegrationTestUtils.java:
##########
@@ -100,11 +99,11 @@ static Map<Integer, Long> registerBrokersAndUnfence(
Uuid.fromString("TESTBROKER" + Integer.toString(100000
+ brokerId).substring(1) + "DIRAAAA")
))
.setListeners(new ListenerCollection(
- Arrays.asList(
- new Listener()
- .setName("PLAINTEXT")
- .setHost("localhost")
- .setPort(9092 + brokerId)
+ Collections.singletonList(
Review Comment:
Can we undo the indentation changes?
##########
metadata/src/main/java/org/apache/kafka/metadata/FinalizedControllerFeatures.java:
##########
@@ -66,11 +66,9 @@ public boolean equals(Object o) {
@Override
public String toString() {
- StringBuilder bld = new StringBuilder();
- bld.append("{");
- bld.append("featureMap=").append(featureMap.toString());
- bld.append(", epoch=").append(epoch);
- bld.append("}");
- return bld.toString();
+ return "{" +
Review Comment:
If we bother updating this method, let's make it consistent with the other
ones.
##########
metadata/src/test/java/org/apache/kafka/controller/ClusterControlManagerTest.java:
##########
@@ -327,19 +327,19 @@ public void
testRegisterBrokerRecordVersion(MetadataVersion metadataVersion) {
short expectedVersion = metadataVersion.registerBrokerRecordVersion();
assertEquals(
- asList(new ApiMessageAndVersion(new RegisterBrokerRecord().
- setBrokerEpoch(123L).
- setBrokerId(0).
- setRack(null).
- setIncarnationId(Uuid.fromString("0H4fUu1xQEKXFYwB1aBjhg")).
- setFenced(true).
- setLogDirs(logDirs).
- setFeatures(new
RegisterBrokerRecord.BrokerFeatureCollection(asList(
- new RegisterBrokerRecord.BrokerFeature().
- setName(MetadataVersion.FEATURE_NAME).
- setMinSupportedVersion((short) 1).
- setMaxSupportedVersion((short) 1)).iterator())).
- setInControlledShutdown(false), expectedVersion)),
+ Collections.singletonList(new ApiMessageAndVersion(new
RegisterBrokerRecord().
Review Comment:
Let's undo the indentation changes
##########
metadata/src/test/java/org/apache/kafka/controller/PartitionReassignmentRevertTest.java:
##########
@@ -78,7 +79,7 @@ public void testSomeAdding() {
setAddingReplicas(new int[]{4,
5}).setLeader(3).setLeaderRecoveryState(LeaderRecoveryState.RECOVERED).setLeaderEpoch(100).setPartitionEpoch(200).build();
PartitionReassignmentRevert revert = new
PartitionReassignmentRevert(registration);
assertEquals(Arrays.asList(3, 2, 1), revert.replicas());
- assertEquals(Arrays.asList(2), revert.isr());
+ assertEquals(Collections.singletonList(2), revert.isr());
Review Comment:
I think keeping the multiple assert similar is more readable so you may want
to keep `Arrays.asList()` here
##########
metadata/src/main/java/org/apache/kafka/controller/QuorumController.java:
##########
@@ -1405,7 +1404,7 @@ private void maybeScheduleNextWriteNoOpRecord() {
maybeScheduleNextWriteNoOpRecord();
return ControllerResult.of(
- Arrays.asList(new ApiMessageAndVersion(new
NoOpRecord(), (short) 0)),
+ Collections.singletonList(new
ApiMessageAndVersion(new NoOpRecord(), (short) 0)),
Review Comment:
Let's undo the indentation changes
##########
metadata/src/test/java/org/apache/kafka/controller/FeatureControlManagerTest.java:
##########
@@ -169,8 +169,8 @@ public void testUpdateFeaturesErrorCases() {
setQuorumFeatures(features("foo", 1, 5, "bar", 0, 3)).
setSnapshotRegistry(snapshotRegistry).
setClusterFeatureSupportDescriber(createFakeClusterFeatureSupportDescriber(
- Arrays.asList(new SimpleImmutableEntry<>(5,
Collections.singletonMap("bar", VersionRange.of(0, 3)))),
- Arrays.asList())).
+ Collections.singletonList(new SimpleImmutableEntry<>(5,
singletonMap("bar", VersionRange.of(0, 3)))),
Review Comment:
Let's undo the indentation changes
##########
metadata/src/test/java/org/apache/kafka/controller/ReplicationControlManagerTest.java:
##########
@@ -1157,11 +1157,11 @@ public void
testAlterPartitionHandleUnknownTopicIdOrName(short version) {
AlterPartitionRequestData request = new AlterPartitionRequestData()
.setBrokerId(0)
.setBrokerEpoch(100)
- .setTopics(asList(new AlterPartitionRequestData.TopicData()
- .setTopicName(version <= 1 ? topicName : "")
- .setTopicId(version > 1 ? topicId : Uuid.ZERO_UUID)
- .setPartitions(asList(new PartitionData()
- .setPartitionIndex(0)))));
+ .setTopics(singletonList(new TopicData()
+ .setTopicName(version <= 1 ? topicName : "")
Review Comment:
Let's undo the indentation changes
##########
metadata/src/test/java/org/apache/kafka/metadata/KafkaConfigSchemaTest.java:
##########
@@ -61,9 +62,9 @@ public class KafkaConfigSchemaTest {
public static final Map<String, List<ConfigSynonym>> SYNONYMS = new
HashMap<>();
static {
- SYNONYMS.put("abc", Arrays.asList(new ConfigSynonym("foo.bar")));
- SYNONYMS.put("def", Arrays.asList(new ConfigSynonym("quux",
HOURS_TO_MILLISECONDS)));
- SYNONYMS.put("ghi", Arrays.asList(new ConfigSynonym("ghi")));
+ SYNONYMS.put("abc", Collections.singletonList(new
ConfigSynonym("foo.bar")));
Review Comment:
I wonder if it's more readable to actually keep `Arrays.asList()` here so
all lines are similar
##########
metadata/src/test/java/org/apache/kafka/image/loader/MetadataLoaderTest.java:
##########
@@ -305,11 +305,11 @@ public void testRemovePublisher() throws Exception {
loader.installPublishers(publishers.subList(0, 2)).get();
loader.removeAndClosePublisher(publishers.get(1)).get();
MockSnapshotReader snapshotReader =
MockSnapshotReader.fromRecordLists(
- new MetadataProvenance(100, 50, 2000),
- asList(asList(new ApiMessageAndVersion(
- new FeatureLevelRecord().
- setName(MetadataVersion.FEATURE_NAME).
- setFeatureLevel(IBP_3_3_IV2.featureLevel()), (short)
0))));
+ new MetadataProvenance(100, 50, 2000),
Review Comment:
Can we undo the indentation changes?
Same multiple times below
##########
metadata/src/test/java/org/apache/kafka/controller/ConfigurationControlManagerTest.java:
##########
@@ -382,14 +381,14 @@ expectedRecords1, toMap(entry(MYTOPIC, ApiError.NONE))),
for (ApiMessageAndVersion message : expectedRecords1) {
manager.replay((ConfigRecord) message.message());
}
- assertEquals(ControllerResult.atomicOf(asList(
- new ApiMessageAndVersion(
- new ConfigRecord()
- .setResourceType(TOPIC.id())
- .setResourceName("mytopic")
- .setName("abc")
- .setValue(null),
- CONFIG_RECORD.highestSupportedVersion())),
+ assertEquals(ControllerResult.atomicOf(Collections.singletonList(
+ new ApiMessageAndVersion(
Review Comment:
Let's undo the indentation changes
##########
metadata/src/test/java/org/apache/kafka/controller/ClientQuotaControlManagerTest.java:
##########
@@ -228,20 +228,20 @@ public void testEntityTypes() throws Exception {
new EntityData().setEntityType("user").setEntityName("user-3"),
new
EntityData().setEntityType("client-id").setEntityName(null))).
setKey("request_percentage").setValue(55.55).setRemove(false), (short) 0),
- new ApiMessageAndVersion(new
ClientQuotaRecord().setEntity(Arrays.asList(
- new
EntityData().setEntityType("user").setEntityName("user-1"))).
+ new ApiMessageAndVersion(new
ClientQuotaRecord().setEntity(Collections.singletonList(
Review Comment:
Let's undo the indentation changes
##########
metadata/src/test/java/org/apache/kafka/controller/QuorumControllerTest.java:
##########
@@ -1279,8 +1279,8 @@ public void testFatalMetadataReplayErrorOnActive() throws
Throwable {
@Test
public void testFatalMetadataErrorDuringSnapshotLoading() throws Exception
{
- InitialSnapshot invalidSnapshot = new
InitialSnapshot(Collections.unmodifiableList(Arrays.asList(
- new ApiMessageAndVersion(new PartitionRecord(), (short) 0)))
+ InitialSnapshot invalidSnapshot = new
InitialSnapshot(Collections.unmodifiableList(singletonList(
+ new ApiMessageAndVersion(new PartitionRecord(), (short) 0)))
Review Comment:
Let's undo the indentation changes
##########
metadata/src/test/java/org/apache/kafka/controller/QuorumControllerTest.java:
##########
@@ -270,7 +270,7 @@ private void testDelayedConfigurationOperations(
@Test
public void testFenceMultipleBrokers() throws Throwable {
List<Integer> allBrokers = Arrays.asList(1, 2, 3, 4, 5);
- List<Integer> brokersToKeepUnfenced = Arrays.asList(1);
+ List<Integer> brokersToKeepUnfenced = singletonList(1);
Review Comment:
I wonder if keeping all lines similar would be more readable
##########
metadata/src/test/java/org/apache/kafka/controller/QuorumControllerTest.java:
##########
@@ -798,21 +798,21 @@ public void testSnapshotSaveAndLoad() throws Throwable {
setIncarnationId(new Uuid(3465346L, i)).
setZkMigrationReady(false).
setListeners(new
ControllerRegistrationRequestData.ListenerCollection(
- Arrays.asList(
- new
ControllerRegistrationRequestData.Listener().
- setName("CONTROLLER").
- setHost("localhost").
- setPort(8000 + i).
-
setSecurityProtocol(SecurityProtocol.PLAINTEXT.id)
+ singletonList(
Review Comment:
Let's undo the indentation changes
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
