From 8d2f929a52109281559e588f77e708e6f1bd60cc Mon Sep 17 00:00:00 2001 From: iliax Date: Thu, 25 Mar 2021 11:24:51 +0300 Subject: [PATCH] ISSUE-309: Topic create & update models split (#312) * ISSUE-309: Topic create & update models split --- .../kafka/ui/controller/TopicsController.java | 11 +++--- .../GlobalErrorWebExceptionHandler.java | 4 +- .../kafka/ui/service/ClusterService.java | 11 +++--- .../kafka/ui/service/KafkaService.java | 25 ++++++------ .../kafka/ui/KafkaConsumerTests.java | 4 +- .../provectus/kafka/ui/ReadOnlyModeTests.java | 19 ++++----- .../main/resources/swagger/kafka-ui-api.yaml | 18 +++++++-- .../src/redux/actions/thunks/topics.ts | 39 +++++++++++++++++-- .../src/redux/interfaces/topic.ts | 4 +- 9 files changed, 88 insertions(+), 47 deletions(-) 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 bb1c123141..be76f78c46 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 @@ package com.provectus.kafka.ui.controller; 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> createTopic( - String clusterName, @Valid Mono topicFormData, ServerWebExchange exchange) { - return clusterService.createTopic(clusterName, topicFormData) + String clusterName, @Valid Mono 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 class TopicsController implements TopicsApi { @Override public Mono> updateTopic( - String clusterId, String topicName, @Valid Mono topicFormData, + String clusterId, String topicName, @Valid Mono 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 3bded1837c..6bfdfeab7c 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 @@ public class GlobalErrorWebExceptionHandler extends AbstractErrorWebExceptionHan private Mono 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 @@ public class GlobalErrorWebExceptionHandler extends AbstractErrorWebExceptionHan 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 18afa02d49..6f07f070ce 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.InternalTopic; 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 class ClusterService { .collect(Collectors.toList())); } - public Mono createTopic(String clusterName, Mono topicFormData) { + public Mono createTopic(String clusterName, Mono 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 class ClusterService { @SneakyThrows public Mono updateTopic(String clusterName, String topicName, - Mono topicFormData) { + Mono 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 3483218725..4ceb965d41 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.InternalTopicConfig; 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 @@ public class KafkaService { @SneakyThrows public Mono createTopic(AdminClient adminClient, - Mono topicFormData) { - return topicFormData.flatMap( + Mono topicCreation) { + return topicCreation.flatMap( topicData -> { NewTopic newTopic = new NewTopic(topicData.getName(), topicData.getPartitions(), topicData.getReplicationFactor().shortValue()); @@ -242,9 +243,9 @@ public class KafkaService { ); } - public Mono createTopic(KafkaCluster cluster, Mono topicFormData) { + public Mono createTopic(KafkaCluster cluster, Mono topicCreation) { return getOrCreateAdminClient(cluster) - .flatMap(ac -> createTopic(ac.getAdminClient(), topicFormData)); + .flatMap(ac -> createTopic(ac.getAdminClient(), topicCreation)); } public Mono deleteTopic(KafkaCluster cluster, String topicName) { @@ -320,16 +321,16 @@ public class KafkaService { @SneakyThrows public Mono 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 @@ public class KafkaService { .filter(t -> t.getName().equals(topicName)).findFirst().orElseThrow()); } - private Mono incrementalAlterConfig(TopicFormData topicFormData, ConfigResource topicCr, + private Mono incrementalAlterConfig(TopicUpdate topicUpdate, ConfigResource topicCr, ExtendedAdminClient ac) { - List listOp = topicFormData.getConfigs().entrySet().stream() + List 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 @@ public class KafkaService { } @SuppressWarnings("deprecation") - private Mono alterConfig(TopicFormData topicFormData, ConfigResource topicCr, + private Mono alterConfig(TopicUpdate topicUpdate, ConfigResource topicCr, ExtendedAdminClient ac) { - List configEntries = topicFormData.getConfigs().entrySet().stream() + List 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-api/src/test/java/com/provectus/kafka/ui/KafkaConsumerTests.java b/kafka-ui-api/src/test/java/com/provectus/kafka/ui/KafkaConsumerTests.java index ee4ebb67a0..0e46ed9e25 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 class KafkaConsumerTests extends AbstractBaseTest { 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 3e2ee1c5f8..3ef65ce75f 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 class ReadOnlyModeTests extends AbstractBaseTest { 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 class ReadOnlyModeTests extends AbstractBaseTest { 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 class ReadOnlyModeTests extends AbstractBaseTest { 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 class ReadOnlyModeTests extends AbstractBaseTest { .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 class ReadOnlyModeTests extends AbstractBaseTest { 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() 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 e50ab98fcc..c1531a62ae 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 0b3e024182..50942d410d 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 16968241f6..a4b348a7a7 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'; @@ -50,7 +50,7 @@ export interface TopicsState { messages: TopicMessage[]; } -export type TopicFormFormattedParams = TopicFormData['configs']; +export type TopicFormFormattedParams = TopicCreation['configs']; export interface TopicFormDataRaw { name: string;