From 33c5f5f975f5a859c00c4db493d83037537668bd Mon Sep 17 00:00:00 2001 From: iliax Date: Tue, 2 May 2023 13:26:41 +0400 Subject: [PATCH] Removing manual FilePart type mapping --- .../ApplicationConfigController.java | 12 +++-- .../ui/util/DynamicConfigOperations.java | 20 +++++--- .../kafka/ui/AbstractIntegrationTest.java | 8 +++ .../ApplicationConfigControllerTest.java | 49 +++++++++++++++++++ .../src/test/resources/fileForUploadTest.txt | 1 + kafka-ui-contract/pom.xml | 3 -- .../main/resources/swagger/kafka-ui-api.yaml | 2 +- 7 files changed, 80 insertions(+), 15 deletions(-) create mode 100644 kafka-ui-api/src/test/java/com/provectus/kafka/ui/controller/ApplicationConfigControllerTest.java create mode 100644 kafka-ui-api/src/test/resources/fileForUploadTest.txt diff --git a/kafka-ui-api/src/main/java/com/provectus/kafka/ui/controller/ApplicationConfigController.java b/kafka-ui-api/src/main/java/com/provectus/kafka/ui/controller/ApplicationConfigController.java index 571250ba94..df04b40fab 100644 --- a/kafka-ui-api/src/main/java/com/provectus/kafka/ui/controller/ApplicationConfigController.java +++ b/kafka-ui-api/src/main/java/com/provectus/kafka/ui/controller/ApplicationConfigController.java @@ -27,6 +27,7 @@ import org.mapstruct.Mapper; import org.mapstruct.factory.Mappers; import org.springframework.http.ResponseEntity; import org.springframework.http.codec.multipart.FilePart; +import org.springframework.http.codec.multipart.Part; import org.springframework.web.bind.annotation.RestController; import org.springframework.web.server.ServerWebExchange; import reactor.core.publisher.Flux; @@ -92,16 +93,19 @@ public class ApplicationConfigController implements ApplicationConfigApi { } @Override - public Mono> uploadConfigRelatedFile(FilePart file, ServerWebExchange exchange) { + public Mono> uploadConfigRelatedFile(Flux fileFlux, + ServerWebExchange exchange) { return accessControlService .validateAccess( AccessContext.builder() .applicationConfigActions(EDIT) .build() ) - .then(dynamicConfigOperations.uploadConfigRelatedFile(file)) - .map(path -> new UploadedFileInfoDTO().location(path.toString())) - .map(ResponseEntity::ok); + .then(fileFlux.single()) + .flatMap(file -> + dynamicConfigOperations.uploadConfigRelatedFile((FilePart) file) + .map(path -> new UploadedFileInfoDTO().location(path.toString())) + .map(ResponseEntity::ok)); } @Override diff --git a/kafka-ui-api/src/main/java/com/provectus/kafka/ui/util/DynamicConfigOperations.java b/kafka-ui-api/src/main/java/com/provectus/kafka/ui/util/DynamicConfigOperations.java index 75c6d25f95..83822e1ddf 100644 --- a/kafka-ui-api/src/main/java/com/provectus/kafka/ui/util/DynamicConfigOperations.java +++ b/kafka-ui-api/src/main/java/com/provectus/kafka/ui/util/DynamicConfigOperations.java @@ -90,6 +90,7 @@ public class DynamicConfigOperations { } public PropertiesStructure getCurrentProperties() { + checkIfDynamicConfigEnabled(); return PropertiesStructure.builder() .kafka(getNullableBean(ClustersProperties.class)) .rbac(getNullableBean(RoleBasedAccessControlProperties.class)) @@ -112,11 +113,7 @@ public class DynamicConfigOperations { } public void persist(PropertiesStructure properties) { - if (!dynamicConfigEnabled()) { - throw new ValidationException( - "Dynamic config change is not allowed. " - + "Set dynamic.config.enabled property to 'true' to enabled it."); - } + checkIfDynamicConfigEnabled(); properties.initAndValidate(); String yaml = serializeToYaml(properties); @@ -124,8 +121,9 @@ public class DynamicConfigOperations { } public Mono uploadConfigRelatedFile(FilePart file) { - String targetDirStr = (String) ctx.getEnvironment().getSystemEnvironment() - .getOrDefault(CONFIG_RELATED_UPLOADS_DIR_PROPERTY, CONFIG_RELATED_UPLOADS_DIR_DEFAULT); + checkIfDynamicConfigEnabled(); + String targetDirStr = ctx.getEnvironment() + .getProperty(CONFIG_RELATED_UPLOADS_DIR_PROPERTY, CONFIG_RELATED_UPLOADS_DIR_DEFAULT); Path targetDir = Path.of(targetDirStr); if (!Files.exists(targetDir)) { @@ -149,6 +147,14 @@ public class DynamicConfigOperations { .onErrorMap(th -> new FileUploadException(targetFilePath, th)); } + private void checkIfDynamicConfigEnabled(){ + if (!dynamicConfigEnabled()) { + throw new ValidationException( + "Dynamic config change is not allowed. " + + "Set dynamic.config.enabled property to 'true' to enabled it."); + } + } + @SneakyThrows private void writeYamlToFile(String yaml, Path path) { if (Files.isDirectory(path)) { diff --git a/kafka-ui-api/src/test/java/com/provectus/kafka/ui/AbstractIntegrationTest.java b/kafka-ui-api/src/test/java/com/provectus/kafka/ui/AbstractIntegrationTest.java index dbdfb67fd5..1938f93044 100644 --- a/kafka-ui-api/src/test/java/com/provectus/kafka/ui/AbstractIntegrationTest.java +++ b/kafka-ui-api/src/test/java/com/provectus/kafka/ui/AbstractIntegrationTest.java @@ -2,6 +2,7 @@ package com.provectus.kafka.ui; import com.provectus.kafka.ui.container.KafkaConnectContainer; import com.provectus.kafka.ui.container.SchemaRegistryContainer; +import java.nio.file.Path; import java.util.List; import java.util.Properties; import org.apache.kafka.clients.admin.AdminClient; @@ -9,6 +10,7 @@ import org.apache.kafka.clients.admin.AdminClientConfig; import org.apache.kafka.clients.admin.NewTopic; import org.jetbrains.annotations.NotNull; import org.junit.jupiter.api.function.ThrowingConsumer; +import org.junit.jupiter.api.io.TempDir; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.autoconfigure.web.reactive.AutoConfigureWebTestClient; import org.springframework.boot.test.context.SpringBootTest; @@ -47,6 +49,9 @@ public abstract class AbstractIntegrationTest { .dependsOn(kafka) .dependsOn(schemaRegistry); + @TempDir + public static Path tmpDir; + static { kafka.start(); schemaRegistry.start(); @@ -76,6 +81,9 @@ public abstract class AbstractIntegrationTest { System.setProperty("kafka.clusters.1.schemaRegistry", schemaRegistry.getUrl()); System.setProperty("kafka.clusters.1.kafkaConnect.0.name", "kafka-connect"); System.setProperty("kafka.clusters.1.kafkaConnect.0.address", kafkaConnect.getTarget()); + + System.setProperty("dynamic.config.enabled", "true"); + System.setProperty("config.related.uploads.dir", tmpDir.toString()); } } diff --git a/kafka-ui-api/src/test/java/com/provectus/kafka/ui/controller/ApplicationConfigControllerTest.java b/kafka-ui-api/src/test/java/com/provectus/kafka/ui/controller/ApplicationConfigControllerTest.java new file mode 100644 index 0000000000..7840760868 --- /dev/null +++ b/kafka-ui-api/src/test/java/com/provectus/kafka/ui/controller/ApplicationConfigControllerTest.java @@ -0,0 +1,49 @@ +package com.provectus.kafka.ui.controller; + +import static org.assertj.core.api.Assertions.assertThat; + +import com.provectus.kafka.ui.AbstractIntegrationTest; +import com.provectus.kafka.ui.model.UploadedFileInfoDTO; +import java.io.IOException; +import java.nio.file.Path; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.core.io.ClassPathResource; +import org.springframework.http.HttpEntity; +import org.springframework.http.client.MultipartBodyBuilder; +import org.springframework.test.web.reactive.server.WebTestClient; +import org.springframework.util.MultiValueMap; + +class ApplicationConfigControllerTest extends AbstractIntegrationTest { + + @Autowired + private WebTestClient webTestClient; + + @Test + public void testUpload() throws IOException { + var fileToUpload = new ClassPathResource("/fileForUploadTest.txt", this.getClass()); + + UploadedFileInfoDTO result = webTestClient + .post() + .uri("/api/config/relatedfiles") + .bodyValue(generateBody(fileToUpload)) + .exchange() + .expectStatus() + .isOk() + .expectBody(UploadedFileInfoDTO.class) + .returnResult() + .getResponseBody(); + + assertThat(result).isNotNull(); + assertThat(result.getLocation()).isNotNull(); + assertThat(Path.of(result.getLocation())) + .hasSameBinaryContentAs(fileToUpload.getFile().toPath()); + } + + private MultiValueMap> generateBody(ClassPathResource resource) { + MultipartBodyBuilder builder = new MultipartBodyBuilder(); + builder.part("file", resource); + return builder.build(); + } + +} diff --git a/kafka-ui-api/src/test/resources/fileForUploadTest.txt b/kafka-ui-api/src/test/resources/fileForUploadTest.txt new file mode 100644 index 0000000000..cc58280d07 --- /dev/null +++ b/kafka-ui-api/src/test/resources/fileForUploadTest.txt @@ -0,0 +1 @@ +some content goes here diff --git a/kafka-ui-contract/pom.xml b/kafka-ui-contract/pom.xml index f99f20d3d8..0d8e238368 100644 --- a/kafka-ui-contract/pom.xml +++ b/kafka-ui-contract/pom.xml @@ -101,9 +101,6 @@ true java8 - - filepart=org.springframework.http.codec.multipart.FilePart - 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 2bafb05faa..b589198b5a 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 @@ -1819,7 +1819,7 @@ paths: properties: file: type: string - format: filepart + format: binary responses: 200: description: OK