Wizard file upload fix (#3762)

Removing manual FilePart openapi mapping - using default generator. File upload test added
provectus · May 2, 2023 · 690dcd3 · 690dcd3
1 parent 7857bd5
commit 690dcd3
Show file tree

Hide file tree

Showing 7 changed files with 80 additions and 15 deletions.
diff --git a/...a-ui-api/src/main/java/com/provectus/kafka/ui/controller/ApplicationConfigController.java b/...a-ui-api/src/main/java/com/provectus/kafka/ui/controller/ApplicationConfigController.java
@@ -27,6 +27,7 @@
 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 Mono<ResponseEntity<Void>> restartWithConfig(Mono<RestartRequestDTO> rest
   }
 
   @Override
-  public Mono<ResponseEntity<UploadedFileInfoDTO>> uploadConfigRelatedFile(FilePart file, ServerWebExchange exchange) {
+  public Mono<ResponseEntity<UploadedFileInfoDTO>> uploadConfigRelatedFile(Flux<Part> 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
@@ -90,6 +90,7 @@ public Optional<PropertySource<?>> loadDynamicPropertySource() {
   }
 
   public PropertiesStructure getCurrentProperties() {
+    checkIfDynamicConfigEnabled();
     return PropertiesStructure.builder()
         .kafka(getNullableBean(ClustersProperties.class))
         .rbac(getNullableBean(RoleBasedAccessControlProperties.class))
@@ -112,20 +113,17 @@ private <T> T getNullableBean(Class<T> clazz) {
   }
 
   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);
     writeYamlToFile(yaml, dynamicConfigFilePath());
   }
 
   public Mono<Path> 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 Mono<Path> uploadConfigRelatedFile(FilePart file) {
         .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
@@ -2,13 +2,15 @@
 
 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;
 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 void initialize(@NotNull ConfigurableApplicationContext context) {
       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/...-api/src/test/java/com/provectus/kafka/ui/controller/ApplicationConfigControllerTest.java b/...-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<String, HttpEntity<?>> 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
@@ -0,0 +1 @@
+some content goes here
diff --git a/kafka-ui-contract/pom.xml b/kafka-ui-contract/pom.xml
@@ -101,9 +101,6 @@
                                         <useSpringBoot3>true</useSpringBoot3>
                                         <dateLibrary>java8</dateLibrary>
                                     </configOptions>
-                                    <typeMappings>
-                                        <mapping>filepart=org.springframework.http.codec.multipart.FilePart</mapping>
-                                    </typeMappings>
                                 </configuration>
                             </execution>
                             <execution>

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
@@ -1819,7 +1819,7 @@ paths:
               properties:
                 file:
                   type: string
-                  format: filepart
+                  format: binary
       responses:
         200:
           description: OK