hachikuji commented on a change in pull request #10892:
URL: https://github.com/apache/kafka/pull/10892#discussion_r660160050
##########
File path:
clients/src/main/java/org/apache/kafka/clients/admin/DeleteTopicsResult.java
##########
@@ -30,24 +31,62 @@
*/
@InterfaceStability.Evolving
public class DeleteTopicsResult {
- final Map<String, KafkaFuture<Void>> futures;
+ private Map<String, KafkaFuture<Void>> nameFutures;
+ private Map<Uuid, KafkaFuture<Void>> topicIdFutures;
- protected DeleteTopicsResult(Map<String, KafkaFuture<Void>> futures) {
- this.futures = futures;
+ protected DeleteTopicsResult() {}
+
+ private void setNameFutures(Map<String, KafkaFuture<Void>> nameFutures) {
+ this.nameFutures = nameFutures;
+ }
+
+ private void setTopicIdFutures(Map<Uuid, KafkaFuture<Void>>
topicIdFutures) {
+ this.topicIdFutures = topicIdFutures;
+ }
+
+ protected static DeleteTopicsResult ofTopicNames(Map<String,
KafkaFuture<Void>> nameFutures) {
+ DeleteTopicsResult result = new DeleteTopicsResult();
+ result.setNameFutures(nameFutures);
+ return result;
+ }
+
+ protected static DeleteTopicsResult ofTopicIds(Map<Uuid,
KafkaFuture<Void>> topicIdFutures) {
+ DeleteTopicsResult result = new DeleteTopicsResult();
+ result.setTopicIdFutures(topicIdFutures);
+ return result;
+ }
+
+ /**
+ * Return a map from topic names to futures which can be used to check the
status of
+ * individual deletions if the deleteTopics request used topic names.
Otherwise return null.
+ */
+ public Map<String, KafkaFuture<Void>> topicNameValues() {
+ return nameFutures;
+ }
+
Review comment:
nit: extra newline
##########
File path:
clients/src/main/java/org/apache/kafka/clients/admin/DeleteTopicsResult.java
##########
@@ -30,24 +31,62 @@
*/
@InterfaceStability.Evolving
public class DeleteTopicsResult {
- final Map<String, KafkaFuture<Void>> futures;
+ private Map<String, KafkaFuture<Void>> nameFutures;
+ private Map<Uuid, KafkaFuture<Void>> topicIdFutures;
- protected DeleteTopicsResult(Map<String, KafkaFuture<Void>> futures) {
- this.futures = futures;
+ protected DeleteTopicsResult() {}
Review comment:
Could this be private? I think it would be a little cleaner to take
`nameFutures` and `topicIdFutures` in the constructor. That would allow us to
mark those fields as final.
##########
File path:
clients/src/main/java/org/apache/kafka/clients/admin/DeleteTopicsResult.java
##########
@@ -30,24 +31,62 @@
*/
@InterfaceStability.Evolving
public class DeleteTopicsResult {
- final Map<String, KafkaFuture<Void>> futures;
+ private Map<String, KafkaFuture<Void>> nameFutures;
+ private Map<Uuid, KafkaFuture<Void>> topicIdFutures;
- protected DeleteTopicsResult(Map<String, KafkaFuture<Void>> futures) {
- this.futures = futures;
+ protected DeleteTopicsResult() {}
+
+ private void setNameFutures(Map<String, KafkaFuture<Void>> nameFutures) {
+ this.nameFutures = nameFutures;
+ }
+
+ private void setTopicIdFutures(Map<Uuid, KafkaFuture<Void>>
topicIdFutures) {
+ this.topicIdFutures = topicIdFutures;
+ }
+
+ protected static DeleteTopicsResult ofTopicNames(Map<String,
KafkaFuture<Void>> nameFutures) {
+ DeleteTopicsResult result = new DeleteTopicsResult();
+ result.setNameFutures(nameFutures);
+ return result;
+ }
+
+ protected static DeleteTopicsResult ofTopicIds(Map<Uuid,
KafkaFuture<Void>> topicIdFutures) {
+ DeleteTopicsResult result = new DeleteTopicsResult();
+ result.setTopicIdFutures(topicIdFutures);
+ return result;
+ }
+
+ /**
+ * Return a map from topic names to futures which can be used to check the
status of
+ * individual deletions if the deleteTopics request used topic names.
Otherwise return null.
Review comment:
nit: can we use javadoc `@return`?
##########
File path: clients/src/main/java/org/apache/kafka/common/TopicCollection.java
##########
@@ -0,0 +1,68 @@
+/*
+ * 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;
+
+import java.util.Collection;
+
+/**
+ * A class used to represent a collection of topics. This collection may
define topics by topic name
+ * or topic ID. Subclassing this class beyond the classes provided here is not
supported.
+ */
+public abstract class TopicCollection {
+
+ private TopicCollection() {}
+
+ /**
+ * A class used to represent a collection of topics defined by their topic
ID.
+ * Subclassing this class beyond the classes provided here is not
supported.
+ */
+ public static class TopicIdCollection extends TopicCollection {
+ private final Collection<Uuid> topicIds;
+
+ public TopicIdCollection(Collection<Uuid> topicIds) {
+ super();
+ this.topicIds = topicIds;
+ }
+
+ /**
+ * @return A collection of topic IDs
+ */
+ public Collection<Uuid> topicIds() {
+ return topicIds;
+ }
+ }
+
+ /**
+ * A class used to represent a collection of topics defined by their topic
name.
+ * Subclassing this class beyond the classes provided here is not
supported.
+ */
+ public static class TopicNameCollection extends TopicCollection {
+ private final Collection<String> topicNames;
+
+ public TopicNameCollection(Collection<String> topicNames) {
+ super();
+ this.topicNames = topicNames;
+ }
+
+ /**
+ * @return A collection of topic names
+ */
+ public Collection<String> topicNames() {
+ return topicNames;
+ }
+ }
+}
Review comment:
nit: newline
##########
File path: clients/src/main/java/org/apache/kafka/common/TopicCollection.java
##########
@@ -0,0 +1,68 @@
+/*
+ * 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;
+
+import java.util.Collection;
+
+/**
+ * A class used to represent a collection of topics. This collection may
define topics by topic name
+ * or topic ID. Subclassing this class beyond the classes provided here is not
supported.
+ */
+public abstract class TopicCollection {
+
+ private TopicCollection() {}
+
+ /**
+ * A class used to represent a collection of topics defined by their topic
ID.
+ * Subclassing this class beyond the classes provided here is not
supported.
+ */
+ public static class TopicIdCollection extends TopicCollection {
+ private final Collection<Uuid> topicIds;
+
+ public TopicIdCollection(Collection<Uuid> topicIds) {
+ super();
+ this.topicIds = topicIds;
+ }
+
+ /**
+ * @return A collection of topic IDs
+ */
+ public Collection<Uuid> topicIds() {
+ return topicIds;
Review comment:
Is it worth using `Collections.unmodifiableCollection` here? Same below.
##########
File path:
clients/src/main/java/org/apache/kafka/clients/admin/KafkaAdminClient.java
##########
@@ -1688,15 +1691,27 @@ void handleFailure(Throwable throwable) {
}
@Override
- public DeleteTopicsResult deleteTopics(final Collection<String> topicNames,
+ public DeleteTopicsResult deleteTopics(final TopicCollection topics,
final DeleteTopicsOptions options) {
+ DeleteTopicsResult result;
+ if (topics instanceof TopicIdCollection)
+ result = DeleteTopicsResult.ofTopicIds(new
HashMap<>(handleDeleteTopicsUsingIds(((TopicIdCollection) topics).topicIds(),
options)));
+ else if (topics instanceof TopicNameCollection)
+ result = DeleteTopicsResult.ofTopicNames(new
HashMap<>(handleDeleteTopicsUsingNames(((TopicNameCollection)
topics).topicNames(), options)));
+ else
+ throw new UnsupportedOperationException("The TopicCollection
provided did not match any supported classes for deleteTopics.");
Review comment:
Maybe `IllegalArgumentException` instead?
##########
File path: clients/src/main/java/org/apache/kafka/common/TopicCollection.java
##########
@@ -0,0 +1,68 @@
+/*
+ * 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;
+
+import java.util.Collection;
+
+/**
+ * A class used to represent a collection of topics. This collection may
define topics by topic name
+ * or topic ID. Subclassing this class beyond the classes provided here is not
supported.
+ */
+public abstract class TopicCollection {
Review comment:
Would it be helpful to have a couple static factories? For example:
```java
public static TopicIdCollection ofTopicIds(Collection<Uuid> uuids);
public static TopicNameCollection ofTopicNames(Collection<String> names);
```
--
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
