chia7712 commented on code in PR #16666:
URL: https://github.com/apache/kafka/pull/16666#discussion_r1689586395
##########
core/src/test/java/kafka/testkit/TestKitNode.java:
##########
@@ -28,7 +42,140 @@ default String metadataDirectory() {
return initialMetaPropertiesEnsemble().metadataLogDir().get();
}
+ default Set<String> logDataDirectories() {
+ return initialMetaPropertiesEnsemble().logDirProps().keySet();
+ }
+
MetaPropertiesEnsemble initialMetaPropertiesEnsemble();
- boolean combined();
+ Map<String, String> propertyOverrides();
+
+
+ class Builder {
+ private int id = -1;
+ private String baseDirectory;
+ private String clusterId;
+ private int numLogDirectories = 1;
+ private Map<String, String> propertyOverrides = Collections.emptyMap();
+ private boolean combined;
+
+ protected Builder() {}
+
+ public int id() {
+ return id;
+ }
+
+ public Builder setId(int id) {
+ this.id = id;
+ return this;
+ }
+
+ public Builder setNumLogDirectories(int numLogDirectories) {
+ this.numLogDirectories = numLogDirectories;
+ return this;
+ }
+
+ public Builder setClusterId(String clusterId) {
+ this.clusterId = clusterId;
+ return this;
+ }
+
+ public Builder setBaseDirectory(String baseDirectory) {
+ this.baseDirectory = baseDirectory;
+ return this;
+ }
+
+ public Builder setCombined(boolean combined) {
+ this.combined = combined;
+ return this;
+ }
+
+ public Builder setPropertyOverrides(Map<String, String>
propertyOverrides) {
+ this.propertyOverrides = Collections.unmodifiableMap(new
HashMap<>(propertyOverrides));
+ return this;
+ }
+
+ public TestKitNode buildBrokerNode() {
+ Objects.requireNonNull(baseDirectory);
+ Objects.requireNonNull(clusterId);
+ if (id == -1) {
+ throw new IllegalArgumentException("You must set the node
id.");
+ }
+ if (numLogDirectories < 1) {
+ throw new IllegalArgumentException("The value of
numLogDirectories should be at least 1.");
+ }
+ List<String> logDataDirectories = IntStream
+ .range(0, numLogDirectories)
+ .mapToObj(i -> {
+ if (combined) {
+ return String.format("combined_%d_%d", id, i);
+ }
+ return String.format("broker_%d_data%d", id, i);
+ })
+ .map(logDir -> {
+ if (Paths.get(logDir).isAbsolute()) {
+ return logDir;
+ }
+ return new File(baseDirectory, logDir).getAbsolutePath();
+ })
+ .collect(Collectors.toList());
+ MetaPropertiesEnsemble.Copier copier =
+ new
MetaPropertiesEnsemble.Copier(MetaPropertiesEnsemble.EMPTY);
+ copier.setMetaLogDir(Optional.of(logDataDirectories.get(0)));
+ for (String logDir : logDataDirectories) {
+ copier.setLogDirProps(logDir, new MetaProperties.Builder().
+ setVersion(MetaPropertiesVersion.V1).
+ setClusterId(clusterId).
+ setNodeId(id).
+ setDirectoryId(copier.generateValidDirectoryId()).
+ build());
+ }
+ return new TestKitNode() {
+ private final MetaPropertiesEnsemble ensemble = copier.copy();
+
+ @Override
+ public MetaPropertiesEnsemble initialMetaPropertiesEnsemble() {
+ return ensemble;
+ }
+
+ @Override
+ public Map<String, String> propertyOverrides() {
+ return propertyOverrides;
+ }
+ };
+ }
+
+ public TestKitNode buildControllerNode() {
+ if (id == -1) {
+ throw new IllegalArgumentException("You must set the node
id.");
+ }
+ if (baseDirectory == null) {
+ throw new IllegalArgumentException("You must set the base
directory.");
+ }
+ String metadataDirectory = new File(baseDirectory,
+ combined ? String.format("combined_%d_0", id) :
String.format("controller_%d", id)).getAbsolutePath();
+ MetaPropertiesEnsemble.Copier copier =
+ new
MetaPropertiesEnsemble.Copier(MetaPropertiesEnsemble.EMPTY);
+ copier.setMetaLogDir(Optional.of(metadataDirectory));
+ copier.setLogDirProps(metadataDirectory, new
MetaProperties.Builder().
+ setVersion(MetaPropertiesVersion.V1).
+ setClusterId(clusterId).
+ setNodeId(id).
+ setDirectoryId(copier.generateValidDirectoryId()).
+ build());
+ return new TestKitNode() {
+ private final MetaPropertiesEnsemble ensemble = copier.copy();
+
+ @Override
+ public MetaPropertiesEnsemble initialMetaPropertiesEnsemble() {
+ return ensemble;
+ }
+
+ @Override
+ public Map<String, String> propertyOverrides() {
+ return propertyOverrides;
Review Comment:
please get a copy of `propertyOverrides` to make it immutable
##########
core/src/test/java/kafka/testkit/TestKitNode.java:
##########
@@ -28,7 +42,140 @@ default String metadataDirectory() {
return initialMetaPropertiesEnsemble().metadataLogDir().get();
}
+ default Set<String> logDataDirectories() {
+ return initialMetaPropertiesEnsemble().logDirProps().keySet();
+ }
+
MetaPropertiesEnsemble initialMetaPropertiesEnsemble();
- boolean combined();
+ Map<String, String> propertyOverrides();
+
+
+ class Builder {
Review Comment:
Do we really need this builder? It seems `TestKitNodes#Builder` is almost
same to this builder. Could we move helper method `buildBrokerNode` and
`buildControllerNode` to `TestKitNodes`?
##########
core/src/test/java/kafka/testkit/TestKitNode.java:
##########
@@ -28,7 +42,140 @@ default String metadataDirectory() {
return initialMetaPropertiesEnsemble().metadataLogDir().get();
}
+ default Set<String> logDataDirectories() {
+ return initialMetaPropertiesEnsemble().logDirProps().keySet();
+ }
+
MetaPropertiesEnsemble initialMetaPropertiesEnsemble();
- boolean combined();
+ Map<String, String> propertyOverrides();
+
+
+ class Builder {
+ private int id = -1;
+ private String baseDirectory;
+ private String clusterId;
+ private int numLogDirectories = 1;
+ private Map<String, String> propertyOverrides = Collections.emptyMap();
+ private boolean combined;
+
+ protected Builder() {}
+
+ public int id() {
+ return id;
+ }
+
+ public Builder setId(int id) {
+ this.id = id;
+ return this;
+ }
+
+ public Builder setNumLogDirectories(int numLogDirectories) {
+ this.numLogDirectories = numLogDirectories;
+ return this;
+ }
+
+ public Builder setClusterId(String clusterId) {
+ this.clusterId = clusterId;
+ return this;
+ }
+
+ public Builder setBaseDirectory(String baseDirectory) {
+ this.baseDirectory = baseDirectory;
+ return this;
+ }
+
+ public Builder setCombined(boolean combined) {
+ this.combined = combined;
+ return this;
+ }
+
+ public Builder setPropertyOverrides(Map<String, String>
propertyOverrides) {
+ this.propertyOverrides = Collections.unmodifiableMap(new
HashMap<>(propertyOverrides));
+ return this;
+ }
+
+ public TestKitNode buildBrokerNode() {
+ Objects.requireNonNull(baseDirectory);
+ Objects.requireNonNull(clusterId);
+ if (id == -1) {
+ throw new IllegalArgumentException("You must set the node
id.");
+ }
+ if (numLogDirectories < 1) {
+ throw new IllegalArgumentException("The value of
numLogDirectories should be at least 1.");
+ }
+ List<String> logDataDirectories = IntStream
+ .range(0, numLogDirectories)
+ .mapToObj(i -> {
+ if (combined) {
+ return String.format("combined_%d_%d", id, i);
+ }
+ return String.format("broker_%d_data%d", id, i);
+ })
+ .map(logDir -> {
+ if (Paths.get(logDir).isAbsolute()) {
+ return logDir;
+ }
+ return new File(baseDirectory, logDir).getAbsolutePath();
+ })
+ .collect(Collectors.toList());
+ MetaPropertiesEnsemble.Copier copier =
+ new
MetaPropertiesEnsemble.Copier(MetaPropertiesEnsemble.EMPTY);
+ copier.setMetaLogDir(Optional.of(logDataDirectories.get(0)));
+ for (String logDir : logDataDirectories) {
+ copier.setLogDirProps(logDir, new MetaProperties.Builder().
+ setVersion(MetaPropertiesVersion.V1).
+ setClusterId(clusterId).
+ setNodeId(id).
+ setDirectoryId(copier.generateValidDirectoryId()).
+ build());
+ }
+ return new TestKitNode() {
+ private final MetaPropertiesEnsemble ensemble = copier.copy();
+
+ @Override
+ public MetaPropertiesEnsemble initialMetaPropertiesEnsemble() {
+ return ensemble;
+ }
+
+ @Override
+ public Map<String, String> propertyOverrides() {
+ return propertyOverrides;
Review Comment:
please get a copy of `propertyOverrides` to make it immutable
--
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]
