Repository: provectus/kafka-ui
Issue: Split models for topic creation & update endpoints
Labels: scope/backend
scope/frontend

Currently the same model "TopicFormData" is used for topic creation and update. This is not convenient because some fields (name, partitions, replicationFactor) are marked as required, which is only suitable for create.

Issue comments:

Pull request review comments:

Patch that resolved the issue:
diff --git a/kafka-ui-api/src/main/java/com/provectus/kafka/ui/controller/TopicsController.java b/kafka-ui-api/src/main/java/com/provectus/kafka/ui/controller/TopicsController.java
index bb1c123141d..be76f78c46a 100644
--- a/kafka-ui-api/src/main/java/com/provectus/kafka/ui/controller/TopicsController.java
+++ b/kafka-ui-api/src/main/java/com/provectus/kafka/ui/controller/TopicsController.java
@@ -3,8 +3,9 @@
 import com.provectus.kafka.ui.api.TopicsApi;
 import com.provectus.kafka.ui.model.Topic;
 import com.provectus.kafka.ui.model.TopicConfig;
+import com.provectus.kafka.ui.model.TopicCreation;
 import com.provectus.kafka.ui.model.TopicDetails;
-import com.provectus.kafka.ui.model.TopicFormData;
+import com.provectus.kafka.ui.model.TopicUpdate;
 import com.provectus.kafka.ui.model.TopicsResponse;
 import com.provectus.kafka.ui.service.ClusterService;
 import java.util.Optional;
@@ -26,8 +27,8 @@ public class TopicsController implements TopicsApi {
 
   @Override
   public Mono<ResponseEntity<Topic>> createTopic(
-      String clusterName, @Valid Mono<TopicFormData> topicFormData, ServerWebExchange exchange) {
-    return clusterService.createTopic(clusterName, topicFormData)
+      String clusterName, @Valid Mono<TopicCreation> topicCreation, ServerWebExchange exchange) {
+    return clusterService.createTopic(clusterName, topicCreation)
         .map(s -> new ResponseEntity<>(s, HttpStatus.OK))
         .switchIfEmpty(Mono.just(ResponseEntity.notFound().build()));
   }
@@ -70,8 +71,8 @@ public Mono<ResponseEntity<TopicsResponse>> getTopics(String clusterName, @Valid
 
   @Override
   public Mono<ResponseEntity<Topic>> updateTopic(
-      String clusterId, String topicName, @Valid Mono<TopicFormData> topicFormData,
+      String clusterId, String topicName, @Valid Mono<TopicUpdate> topicUpdate,
       ServerWebExchange exchange) {
-    return clusterService.updateTopic(clusterId, topicName, topicFormData).map(ResponseEntity::ok);
+    return clusterService.updateTopic(clusterId, topicName, topicUpdate).map(ResponseEntity::ok);
   }
 }
diff --git a/kafka-ui-api/src/main/java/com/provectus/kafka/ui/exception/GlobalErrorWebExceptionHandler.java b/kafka-ui-api/src/main/java/com/provectus/kafka/ui/exception/GlobalErrorWebExceptionHandler.java
index 3bded1837c7..6bfdfeab7c8 100644
--- a/kafka-ui-api/src/main/java/com/provectus/kafka/ui/exception/GlobalErrorWebExceptionHandler.java
+++ b/kafka-ui-api/src/main/java/com/provectus/kafka/ui/exception/GlobalErrorWebExceptionHandler.java
@@ -71,7 +71,7 @@ private Mono<ServerResponse> renderErrorResponse(ServerRequest request) {
   private Mono<ServerResponse> renderDefault(Throwable throwable, ServerRequest request) {
     var response = new ErrorResponse()
         .code(ErrorCode.UNEXPECTED.code())
-        .message(throwable.getMessage())
+        .message(coalesce(throwable.getMessage(), "Unexpected internal error"))
         .requestId(requestId(request))
         .timestamp(currentTimestamp());
     return ServerResponse
@@ -84,7 +84,7 @@ private Mono<ServerResponse> render(CustomBaseException baseException, ServerReq
     ErrorCode errorCode = baseException.getErrorCode();
     var response = new ErrorResponse()
         .code(errorCode.code())
-        .message(baseException.getMessage())
+        .message(coalesce(baseException.getMessage(), "Internal error"))
         .requestId(requestId(request))
         .timestamp(currentTimestamp());
     return ServerResponse
diff --git a/kafka-ui-api/src/main/java/com/provectus/kafka/ui/service/ClusterService.java b/kafka-ui-api/src/main/java/com/provectus/kafka/ui/service/ClusterService.java
index 18afa02d494..6f07f070ce0 100644
--- a/kafka-ui-api/src/main/java/com/provectus/kafka/ui/service/ClusterService.java
+++ b/kafka-ui-api/src/main/java/com/provectus/kafka/ui/service/ClusterService.java
@@ -15,9 +15,10 @@
 import com.provectus.kafka.ui.model.KafkaCluster;
 import com.provectus.kafka.ui.model.Topic;
 import com.provectus.kafka.ui.model.TopicConfig;
+import com.provectus.kafka.ui.model.TopicCreation;
 import com.provectus.kafka.ui.model.TopicDetails;
-import com.provectus.kafka.ui.model.TopicFormData;
 import com.provectus.kafka.ui.model.TopicMessage;
+import com.provectus.kafka.ui.model.TopicUpdate;
 import com.provectus.kafka.ui.model.TopicsResponse;
 import com.provectus.kafka.ui.util.ClusterUtil;
 import java.util.Collection;
@@ -125,9 +126,9 @@ public Optional<List<TopicConfig>> getTopicConfigs(String name, String topicName
             .collect(Collectors.toList()));
   }
 
-  public Mono<Topic> createTopic(String clusterName, Mono<TopicFormData> topicFormData) {
+  public Mono<Topic> createTopic(String clusterName, Mono<TopicCreation> topicCreation) {
     return clustersStorage.getClusterByName(clusterName).map(cluster ->
-        kafkaService.createTopic(cluster, topicFormData)
+        kafkaService.createTopic(cluster, topicCreation)
             .doOnNext(t -> updateCluster(t, clusterName, cluster))
             .map(clusterMapper::toTopic)
     ).orElse(Mono.empty());
@@ -200,9 +201,9 @@ public Flux<Broker> getBrokers(String clusterName) {
 
   @SneakyThrows
   public Mono<Topic> updateTopic(String clusterName, String topicName,
-                                 Mono<TopicFormData> topicFormData) {
+                                 Mono<TopicUpdate> topicUpdate) {
     return clustersStorage.getClusterByName(clusterName).map(cl ->
-        topicFormData
+        topicUpdate
             .flatMap(t -> kafkaService.updateTopic(cl, topicName, t))
             .doOnNext(t -> updateCluster(t, clusterName, cl))
             .map(clusterMapper::toTopic)
diff --git a/kafka-ui-api/src/main/java/com/provectus/kafka/ui/service/KafkaService.java b/kafka-ui-api/src/main/java/com/provectus/kafka/ui/service/KafkaService.java
index 34832187250..4ceb965d412 100644
--- a/kafka-ui-api/src/main/java/com/provectus/kafka/ui/service/KafkaService.java
+++ b/kafka-ui-api/src/main/java/com/provectus/kafka/ui/service/KafkaService.java
@@ -12,7 +12,8 @@
 import com.provectus.kafka.ui.model.KafkaCluster;
 import com.provectus.kafka.ui.model.Metric;
 import com.provectus.kafka.ui.model.ServerStatus;
-import com.provectus.kafka.ui.model.TopicFormData;
+import com.provectus.kafka.ui.model.TopicCreation;
+import com.provectus.kafka.ui.model.TopicUpdate;
 import com.provectus.kafka.ui.util.ClusterUtil;
 import com.provectus.kafka.ui.util.JmxClusterUtil;
 import com.provectus.kafka.ui.util.JmxMetricsName;
@@ -223,8 +224,8 @@ private Mono<String> createTopic(AdminClient adminClient, NewTopic newTopic) {
 
   @SneakyThrows
   public Mono<InternalTopic> createTopic(AdminClient adminClient,
-                                         Mono<TopicFormData> topicFormData) {
-    return topicFormData.flatMap(
+                                         Mono<TopicCreation> topicCreation) {
+    return topicCreation.flatMap(
         topicData -> {
           NewTopic newTopic = new NewTopic(topicData.getName(), topicData.getPartitions(),
               topicData.getReplicationFactor().shortValue());
@@ -242,9 +243,9 @@ public Mono<InternalTopic> createTopic(AdminClient adminClient,
         );
   }
 
-  public Mono<InternalTopic> createTopic(KafkaCluster cluster, Mono<TopicFormData> topicFormData) {
+  public Mono<InternalTopic> createTopic(KafkaCluster cluster, Mono<TopicCreation> topicCreation) {
     return getOrCreateAdminClient(cluster)
-        .flatMap(ac -> createTopic(ac.getAdminClient(), topicFormData));
+        .flatMap(ac -> createTopic(ac.getAdminClient(), topicCreation));
   }
 
   public Mono<Void> deleteTopic(KafkaCluster cluster, String topicName) {
@@ -320,16 +321,16 @@ public KafkaConsumer<Bytes, Bytes> createConsumer(KafkaCluster cluster) {
 
   @SneakyThrows
   public Mono<InternalTopic> updateTopic(KafkaCluster cluster, String topicName,
-                                         TopicFormData topicFormData) {
+                                         TopicUpdate topicUpdate) {
     ConfigResource topicCr = new ConfigResource(ConfigResource.Type.TOPIC, topicName);
     return getOrCreateAdminClient(cluster)
         .flatMap(ac -> {
           if (ac.getSupportedFeatures()
               .contains(ExtendedAdminClient.SupportedFeature.INCREMENTAL_ALTER_CONFIGS)) {
-            return incrementalAlterConfig(topicFormData, topicCr, ac)
+            return incrementalAlterConfig(topicUpdate, topicCr, ac)
                 .flatMap(c -> getUpdatedTopic(ac, topicName));
           } else {
-            return alterConfig(topicFormData, topicCr, ac)
+            return alterConfig(topicUpdate, topicCr, ac)
                 .flatMap(c -> getUpdatedTopic(ac, topicName));
           }
         });
@@ -341,9 +342,9 @@ private Mono<InternalTopic> getUpdatedTopic(ExtendedAdminClient ac, String topic
             .filter(t -> t.getName().equals(topicName)).findFirst().orElseThrow());
   }
 
-  private Mono<String> incrementalAlterConfig(TopicFormData topicFormData, ConfigResource topicCr,
+  private Mono<String> incrementalAlterConfig(TopicUpdate topicUpdate, ConfigResource topicCr,
                                               ExtendedAdminClient ac) {
-    List<AlterConfigOp> listOp = topicFormData.getConfigs().entrySet().stream()
+    List<AlterConfigOp> listOp = topicUpdate.getConfigs().entrySet().stream()
         .flatMap(cfg -> Stream.of(new AlterConfigOp(new ConfigEntry(cfg.getKey(), cfg.getValue()),
             AlterConfigOp.OpType.SET))).collect(Collectors.toList());
     return ClusterUtil.toMono(
@@ -352,9 +353,9 @@ private Mono<String> incrementalAlterConfig(TopicFormData topicFormData, ConfigR
   }
 
   @SuppressWarnings("deprecation")
-  private Mono<String> alterConfig(TopicFormData topicFormData, ConfigResource topicCr,
+  private Mono<String> alterConfig(TopicUpdate topicUpdate, ConfigResource topicCr,
                                    ExtendedAdminClient ac) {
-    List<ConfigEntry> configEntries = topicFormData.getConfigs().entrySet().stream()
+    List<ConfigEntry> configEntries = topicUpdate.getConfigs().entrySet().stream()
         .flatMap(cfg -> Stream.of(new ConfigEntry(cfg.getKey(), cfg.getValue())))
         .collect(Collectors.toList());
     Config config = new Config(configEntries);
diff --git a/kafka-ui-contract/src/main/resources/swagger/kafka-ui-api.yaml b/kafka-ui-contract/src/main/resources/swagger/kafka-ui-api.yaml
index e50ab98fccb..c1531a62aed 100644
--- a/kafka-ui-contract/src/main/resources/swagger/kafka-ui-api.yaml
+++ b/kafka-ui-contract/src/main/resources/swagger/kafka-ui-api.yaml
@@ -162,7 +162,7 @@ paths:
         content:
           application/json:
             schema:
-              $ref: '#/components/schemas/TopicFormData'
+              $ref: '#/components/schemas/TopicCreation'
       responses:
         201:
           description: Created
@@ -215,7 +215,7 @@ paths:
         content:
           application/json:
             schema:
-              $ref: '#/components/schemas/TopicFormData'
+              $ref: '#/components/schemas/TopicUpdate'
       responses:
         200:
           description: Updated
@@ -1281,7 +1281,7 @@ components:
       required:
         - name
 
-    TopicFormData:
+    TopicCreation:
       type: object
       properties:
         name:
@@ -1296,6 +1296,18 @@ components:
             type: string
       required:
         - name
+        - partitions
+        - replicationFactor
+
+    TopicUpdate:
+      type: object
+      properties:
+        configs:
+          type: object
+          additionalProperties:
+            type: string
+      required:
+        - configs
 
     Broker:
       type: object
diff --git a/kafka-ui-react-app/src/redux/actions/thunks/topics.ts b/kafka-ui-react-app/src/redux/actions/thunks/topics.ts
index 0b3e024182c..50942d410dd 100644
--- a/kafka-ui-react-app/src/redux/actions/thunks/topics.ts
+++ b/kafka-ui-react-app/src/redux/actions/thunks/topics.ts
@@ -4,7 +4,8 @@ import {
   MessagesApi,
   Configuration,
   Topic,
-  TopicFormData,
+  TopicCreation,
+  TopicUpdate,
   TopicConfig,
 } from 'generated-sources';
 import {
@@ -136,7 +137,7 @@ export const fetchTopicConfig = (
   }
 };
 
-const formatTopicFormData = (form: TopicFormDataRaw): TopicFormData => {
+const formatTopicCreation = (form: TopicFormDataRaw): TopicCreation => {
   const {
     name,
     partitions,
@@ -172,6 +173,36 @@ const formatTopicFormData = (form: TopicFormDataRaw): TopicFormData => {
   };
 };
 
+const formatTopicUpdate = (form: TopicFormDataRaw): TopicUpdate => {
+  const {
+    cleanupPolicy,
+    retentionBytes,
+    retentionMs,
+    maxMessageBytes,
+    minInSyncReplicas,
+    customParams,
+  } = form;
+
+  return {
+    configs: {
+      'cleanup.policy': cleanupPolicy,
+      'retention.ms': retentionMs,
+      'retention.bytes': retentionBytes,
+      'max.message.bytes': maxMessageBytes,
+      'min.insync.replicas': minInSyncReplicas,
+      ...Object.values(customParams || {}).reduce(
+        (result: TopicFormFormattedParams, customParam: TopicConfig) => {
+          return {
+            ...result,
+            [customParam.name]: customParam.value,
+          };
+        },
+        {}
+      ),
+    },
+  };
+};
+
 export const createTopic = (
   clusterName: ClusterName,
   form: TopicFormDataRaw
@@ -180,7 +211,7 @@ export const createTopic = (
   try {
     const topic: Topic = await topicsApiClient.createTopic({
       clusterName,
-      topicFormData: formatTopicFormData(form),
+      topicCreation: formatTopicCreation(form),
     });
 
     const state = getState().topics;
@@ -210,7 +241,7 @@ export const updateTopic = (
     const topic: Topic = await topicsApiClient.updateTopic({
       clusterName,
       topicName: form.name,
-      topicFormData: formatTopicFormData(form),
+      topicUpdate: formatTopicUpdate(form),
     });
 
     const state = getState().topics;
diff --git a/kafka-ui-react-app/src/redux/interfaces/topic.ts b/kafka-ui-react-app/src/redux/interfaces/topic.ts
index d7c2ee99fcb..a45f1daabca 100644
--- a/kafka-ui-react-app/src/redux/interfaces/topic.ts
+++ b/kafka-ui-react-app/src/redux/interfaces/topic.ts
@@ -3,7 +3,7 @@ import {
   TopicDetails,
   TopicMessage,
   TopicConfig,
-  TopicFormData,
+  TopicCreation,
   GetTopicMessagesRequest,
 } from 'generated-sources';
 
@@ -55,7 +55,7 @@ export interface TopicsState {
   messages: TopicMessage[];
 }
 
-export type TopicFormFormattedParams = TopicFormData['configs'];
+export type TopicFormFormattedParams = TopicCreation['configs'];
 
 export interface TopicFormDataRaw {
   name: string;


Test patch:
diff --git a/kafka-ui-api/src/test/java/com/provectus/kafka/ui/KafkaConsumerTests.java b/kafka-ui-api/src/test/java/com/provectus/kafka/ui/KafkaConsumerTests.java
index ee4ebb67a0b..0e46ed9e25b 100644
--- a/kafka-ui-api/src/test/java/com/provectus/kafka/ui/KafkaConsumerTests.java
+++ b/kafka-ui-api/src/test/java/com/provectus/kafka/ui/KafkaConsumerTests.java
@@ -1,6 +1,6 @@
 package com.provectus.kafka.ui;
 
-import com.provectus.kafka.ui.model.TopicFormData;
+import com.provectus.kafka.ui.model.TopicCreation;
 import com.provectus.kafka.ui.model.TopicMessage;
 import com.provectus.kafka.ui.producer.KafkaTestProducer;
 import java.util.Map;
@@ -27,7 +27,7 @@ public void shouldDeleteRecords() {
     var topicName = UUID.randomUUID().toString();
     webTestClient.post()
         .uri("/api/clusters/{clusterName}/topics", LOCAL)
-        .bodyValue(new TopicFormData()
+        .bodyValue(new TopicCreation()
             .name(topicName)
             .partitions(1)
             .replicationFactor(1)
diff --git a/kafka-ui-api/src/test/java/com/provectus/kafka/ui/ReadOnlyModeTests.java b/kafka-ui-api/src/test/java/com/provectus/kafka/ui/ReadOnlyModeTests.java
index 3e2ee1c5f8b..3ef65ce75fb 100644
--- a/kafka-ui-api/src/test/java/com/provectus/kafka/ui/ReadOnlyModeTests.java
+++ b/kafka-ui-api/src/test/java/com/provectus/kafka/ui/ReadOnlyModeTests.java
@@ -1,6 +1,7 @@
 package com.provectus.kafka.ui;
 
-import com.provectus.kafka.ui.model.TopicFormData;
+import com.provectus.kafka.ui.model.TopicCreation;
+import com.provectus.kafka.ui.model.TopicUpdate;
 import java.util.Map;
 import java.util.UUID;
 import lombok.extern.log4j.Log4j2;
@@ -24,7 +25,7 @@ public void shouldCreateTopicForNonReadonlyCluster() {
     var topicName = UUID.randomUUID().toString();
     webTestClient.post()
         .uri("/api/clusters/{clusterName}/topics", LOCAL)
-        .bodyValue(new TopicFormData()
+        .bodyValue(new TopicCreation()
             .name(topicName)
             .partitions(1)
             .replicationFactor(1)
@@ -40,7 +41,7 @@ public void shouldNotCreateTopicForReadonlyCluster() {
     var topicName = UUID.randomUUID().toString();
     webTestClient.post()
         .uri("/api/clusters/{clusterName}/topics", SECOND_LOCAL)
-        .bodyValue(new TopicFormData()
+        .bodyValue(new TopicCreation()
             .name(topicName)
             .partitions(1)
             .replicationFactor(1)
@@ -56,7 +57,7 @@ public void shouldUpdateTopicForNonReadonlyCluster() {
     var topicName = UUID.randomUUID().toString();
     webTestClient.post()
         .uri("/api/clusters/{clusterName}/topics", LOCAL)
-        .bodyValue(new TopicFormData()
+        .bodyValue(new TopicCreation()
             .name(topicName)
             .partitions(1)
             .replicationFactor(1)
@@ -67,10 +68,7 @@ public void shouldUpdateTopicForNonReadonlyCluster() {
         .isOk();
     webTestClient.patch()
         .uri("/api/clusters/{clusterName}/topics/{topicName}", LOCAL, topicName)
-        .bodyValue(new TopicFormData()
-            .name(topicName)
-            .partitions(2)
-            .replicationFactor(1)
+        .bodyValue(new TopicUpdate()
             .configs(Map.of())
         )
         .exchange()
@@ -83,10 +81,7 @@ public void shouldNotUpdateTopicForReadonlyCluster() {
     var topicName = UUID.randomUUID().toString();
     webTestClient.patch()
         .uri("/api/clusters/{clusterName}/topics/{topicName}", SECOND_LOCAL, topicName)
-        .bodyValue(new TopicFormData()
-            .name(topicName)
-            .partitions(1)
-            .replicationFactor(1)
+        .bodyValue(new TopicUpdate()
             .configs(Map.of())
         )
         .exchange()