chia7712 commented on code in PR #15761:
URL: https://github.com/apache/kafka/pull/15761#discussion_r1575053494
##########
core/src/test/java/kafka/test/ClusterConfig.java:
##########
@@ -173,63 +199,104 @@ public static class Builder {
private String listenerName;
private File trustStoreFile;
private MetadataVersion metadataVersion;
+ private Map<String, String> serverProperties =
Collections.unmodifiableMap(new HashMap<>());
+ private Map<String, String> producerProperties =
Collections.unmodifiableMap(new HashMap<>());
+ private Map<String, String> consumerProperties =
Collections.unmodifiableMap(new HashMap<>());
+ private Map<String, String> adminClientProperties =
Collections.unmodifiableMap(new HashMap<>());
+ private Map<String, String> saslServerProperties =
Collections.unmodifiableMap(new HashMap<>());
+ private Map<String, String> saslClientProperties =
Collections.unmodifiableMap(new HashMap<>());
+ private Map<Integer, Map<String, String>> perBrokerOverrideProperties
= Collections.unmodifiableMap(new HashMap<>());
- Builder(Type type, int brokers, int controllers, boolean autoStart,
SecurityProtocol securityProtocol, MetadataVersion metadataVersion) {
- this.type = type;
- this.brokers = brokers;
- this.controllers = controllers;
- this.autoStart = autoStart;
- this.securityProtocol = securityProtocol;
- this.metadataVersion = metadataVersion;
- }
+ private Builder() {}
- public Builder type(Type type) {
+ public Builder setType(Type type) {
this.type = type;
return this;
}
- public Builder brokers(int brokers) {
+ public Builder setBrokers(int brokers) {
this.brokers = brokers;
return this;
}
- public Builder controllers(int controllers) {
+ public Builder setControllers(int controllers) {
this.controllers = controllers;
return this;
}
- public Builder name(String name) {
+ public Builder setName(String name) {
this.name = name;
return this;
}
- public Builder autoStart(boolean autoStart) {
+ public Builder setAutoStart(boolean autoStart) {
this.autoStart = autoStart;
return this;
}
- public Builder securityProtocol(SecurityProtocol securityProtocol) {
+ public Builder setSecurityProtocol(SecurityProtocol securityProtocol) {
this.securityProtocol = securityProtocol;
return this;
}
- public Builder listenerName(String listenerName) {
+ public Builder setListenerName(String listenerName) {
this.listenerName = listenerName;
return this;
}
- public Builder trustStoreFile(File trustStoreFile) {
+ public Builder setTrustStoreFile(File trustStoreFile) {
this.trustStoreFile = trustStoreFile;
return this;
}
- public Builder metadataVersion(MetadataVersion metadataVersion) {
+ public Builder setMetadataVersion(MetadataVersion metadataVersion) {
this.metadataVersion = metadataVersion;
return this;
}
+ public Builder setServerProperties(Map<String, String>
serverProperties) {
+ this.serverProperties =
Collections.unmodifiableMap(serverProperties);
+ return this;
+ }
+
+ public Builder setConsumerProperties(Map<String, String>
consumerProperties) {
+ this.consumerProperties =
Collections.unmodifiableMap(consumerProperties);
+ return this;
+ }
+
+ public Builder setProducerProperties(Map<String, String>
producerProperties) {
+ this.producerProperties =
Collections.unmodifiableMap(producerProperties);
+ return this;
+ }
+
+ public Builder setAdminClientProperties(Map<String, String>
adminClientProperties) {
+ this.adminClientProperties =
Collections.unmodifiableMap(adminClientProperties);
+ return this;
+ }
+
+ public Builder setSaslServerProperties(Map<String, String>
saslServerProperties) {
+ this.saslServerProperties =
Collections.unmodifiableMap(saslServerProperties);
+ return this;
+ }
+
+ public Builder setSaslClientProperties(Map<String, String>
saslClientProperties) {
+ this.saslClientProperties =
Collections.unmodifiableMap(saslClientProperties);
+ return this;
+ }
+
+ public Builder setPerBrokerProperties(Map<Integer, Map<String,
String>> perBrokerOverrideProperties) {
+ this.perBrokerOverrideProperties = Collections.unmodifiableMap(
Review Comment:
```java
this.perBrokerOverrideProperties = Collections.unmodifiableMap(
perBrokerOverrideProperties.entrySet().stream()
.collect(Collectors.toMap(Map.Entry::getKey, e
-> Collections.unmodifiableMap(new HashMap<>(e.getValue())))));
```
##########
core/src/test/java/kafka/test/ClusterConfig.java:
##########
@@ -173,63 +199,104 @@ public static class Builder {
private String listenerName;
private File trustStoreFile;
private MetadataVersion metadataVersion;
+ private Map<String, String> serverProperties =
Collections.unmodifiableMap(new HashMap<>());
Review Comment:
Is `Collections.emptyMap();` more suitable?
##########
core/src/test/java/kafka/testkit/BrokerNode.java:
##########
@@ -130,7 +154,7 @@ public BrokerNode build(
this.incarnationId = incarnationId;
this.initialMetaPropertiesEnsemble = initialMetaPropertiesEnsemble;
this.combined = combined;
- this.propertyOverrides = new HashMap<>(propertyOverrides);
+ this.propertyOverrides =
Collections.unmodifiableMap(propertyOverrides);
Review Comment:
`ClusterConfig` address the "immutable" by creating immutable objects in
Builder. Could we align that behavior?
##########
core/src/test/java/kafka/test/ClusterConfig.java:
##########
@@ -173,63 +199,104 @@ public static class Builder {
private String listenerName;
private File trustStoreFile;
private MetadataVersion metadataVersion;
+ private Map<String, String> serverProperties =
Collections.unmodifiableMap(new HashMap<>());
+ private Map<String, String> producerProperties =
Collections.unmodifiableMap(new HashMap<>());
+ private Map<String, String> consumerProperties =
Collections.unmodifiableMap(new HashMap<>());
+ private Map<String, String> adminClientProperties =
Collections.unmodifiableMap(new HashMap<>());
+ private Map<String, String> saslServerProperties =
Collections.unmodifiableMap(new HashMap<>());
+ private Map<String, String> saslClientProperties =
Collections.unmodifiableMap(new HashMap<>());
+ private Map<Integer, Map<String, String>> perBrokerOverrideProperties
= Collections.unmodifiableMap(new HashMap<>());
- Builder(Type type, int brokers, int controllers, boolean autoStart,
SecurityProtocol securityProtocol, MetadataVersion metadataVersion) {
- this.type = type;
- this.brokers = brokers;
- this.controllers = controllers;
- this.autoStart = autoStart;
- this.securityProtocol = securityProtocol;
- this.metadataVersion = metadataVersion;
- }
+ private Builder() {}
- public Builder type(Type type) {
+ public Builder setType(Type type) {
this.type = type;
return this;
}
- public Builder brokers(int brokers) {
+ public Builder setBrokers(int brokers) {
this.brokers = brokers;
return this;
}
- public Builder controllers(int controllers) {
+ public Builder setControllers(int controllers) {
this.controllers = controllers;
return this;
}
- public Builder name(String name) {
+ public Builder setName(String name) {
this.name = name;
return this;
}
- public Builder autoStart(boolean autoStart) {
+ public Builder setAutoStart(boolean autoStart) {
this.autoStart = autoStart;
return this;
}
- public Builder securityProtocol(SecurityProtocol securityProtocol) {
+ public Builder setSecurityProtocol(SecurityProtocol securityProtocol) {
this.securityProtocol = securityProtocol;
return this;
}
- public Builder listenerName(String listenerName) {
+ public Builder setListenerName(String listenerName) {
this.listenerName = listenerName;
return this;
}
- public Builder trustStoreFile(File trustStoreFile) {
+ public Builder setTrustStoreFile(File trustStoreFile) {
this.trustStoreFile = trustStoreFile;
return this;
}
- public Builder metadataVersion(MetadataVersion metadataVersion) {
+ public Builder setMetadataVersion(MetadataVersion metadataVersion) {
this.metadataVersion = metadataVersion;
return this;
}
+ public Builder setServerProperties(Map<String, String>
serverProperties) {
+ this.serverProperties =
Collections.unmodifiableMap(serverProperties);
+ return this;
+ }
+
+ public Builder setConsumerProperties(Map<String, String>
consumerProperties) {
+ this.consumerProperties =
Collections.unmodifiableMap(consumerProperties);
+ return this;
+ }
+
+ public Builder setProducerProperties(Map<String, String>
producerProperties) {
+ this.producerProperties =
Collections.unmodifiableMap(producerProperties);
+ return this;
+ }
+
+ public Builder setAdminClientProperties(Map<String, String>
adminClientProperties) {
+ this.adminClientProperties =
Collections.unmodifiableMap(adminClientProperties);
+ return this;
+ }
+
+ public Builder setSaslServerProperties(Map<String, String>
saslServerProperties) {
+ this.saslServerProperties =
Collections.unmodifiableMap(saslServerProperties);
+ return this;
+ }
+
+ public Builder setSaslClientProperties(Map<String, String>
saslClientProperties) {
+ this.saslClientProperties =
Collections.unmodifiableMap(saslClientProperties);
+ return this;
+ }
+
+ public Builder setPerBrokerProperties(Map<Integer, Map<String,
String>> perBrokerOverrideProperties) {
+ this.perBrokerOverrideProperties = Collections.unmodifiableMap(
+ perBrokerOverrideProperties.entrySet().stream()
+ .map(e -> new
AbstractMap.SimpleEntry<>(e.getKey(),
Collections.unmodifiableMap(e.getValue())))
+ .collect(Collectors.toMap(Map.Entry::getKey,
Map.Entry::getValue)));
+ return this;
+ }
+
public ClusterConfig build() {
- return new ClusterConfig(type, brokers, controllers, name,
autoStart, securityProtocol, listenerName, trustStoreFile, metadataVersion);
+ return new ClusterConfig(type, brokers, controllers, name,
autoStart, securityProtocol, listenerName,
Review Comment:
Could you please add null check for the non-null variables?
##########
core/src/test/java/kafka/testkit/BrokerNode.java:
##########
@@ -41,6 +46,9 @@ public static class Builder {
private int numLogDirectories = 1;
private String metadataDirectory = null;
private Map<String, String> propertyOverrides = new HashMap<>();
Review Comment:
`Collections.emptyMap()`
--
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: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
