From b6bbd5fac50f315e487ff2c65a4fcdd3efa2ab4c Mon Sep 17 00:00:00 2001
From: Romain Grecourt <romain.grecourt@oracle.com>
Date: Tue, 7 Feb 2023 13:57:53 -0800
Subject: [PATCH] Update BodyPart to return Optional instead of a nullable
 String (#6101)

* Update BodyPart to return Optional instead of a nullable String
Also add a new method isNamed to test the control name.

Fixes #2833
---
 .../FileService.java.multipart.mustache       | 10 +++++--
 examples/media/multipart/README.md            |  2 +-
 .../examples/media/multipart/FileService.java | 12 +++++---
 reactive/media/multipart/pom.xml              |  7 ++++-
 .../reactive/media/multipart/BodyPart.java    | 29 ++++++++++++++-----
 .../media/multipart/BodyPartTest.java         | 13 +++++----
 6 files changed, 51 insertions(+), 22 deletions(-)

diff --git a/archetypes/helidon/src/main/archetype/se/custom/files/src/main/java/__pkg__/FileService.java.multipart.mustache b/archetypes/helidon/src/main/archetype/se/custom/files/src/main/java/__pkg__/FileService.java.multipart.mustache
index c4cbcd21ed6..24798fa7ccf 100644
--- a/archetypes/helidon/src/main/archetype/se/custom/files/src/main/java/__pkg__/FileService.java.multipart.mustache
+++ b/archetypes/helidon/src/main/archetype/se/custom/files/src/main/java/__pkg__/FileService.java.multipart.mustache
@@ -7,6 +7,7 @@ import java.util.Map;
 import java.util.concurrent.ExecutorService;
 
 import io.helidon.common.configurable.ThreadPoolSupplier;
+import io.helidon.common.http.BadRequestException;
 import io.helidon.common.http.ContentDisposition;
 import io.helidon.common.http.DataChunk;
 import io.helidon.common.http.Http;
@@ -67,10 +68,13 @@ public final class FileService implements Service {
     private void upload(ServerRequest req, ServerResponse res) {
         req.content().asStream(ReadableBodyPart.class)
            .forEach(part -> {
-               if ("file[]".equals(part.name())) {
-                   part.content().map(DataChunk::data)
+               if (part.isNamed("file[]")) {
+                   String filename = part.filename()
+                                         .orElseThrow(() -> new BadRequestException("Missing filename"));
+                   part.content()
+                       .map(DataChunk::data)
                        .flatMapIterable(Arrays::asList)
-                       .to(IoMulti.writeToFile(storage.create(part.filename()))
+                       .to(IoMulti.writeToFile(storage.create(filename))
                                   .executor(executor)
                                   .build());
                } else {
diff --git a/examples/media/multipart/README.md b/examples/media/multipart/README.md
index cbe0e9fb682..206a630c045 100644
--- a/examples/media/multipart/README.md
+++ b/examples/media/multipart/README.md
@@ -17,7 +17,7 @@ mvn package
 First, start the server:
 
 ```
-java -jar target/helidon-examples-microprofile-multipart.jar
+java -jar target/helidon-examples-media-multipart.jar
 ```
 
 Then open <http://localhost:8080/ui> in your browser.
diff --git a/examples/media/multipart/src/main/java/io/helidon/examples/media/multipart/FileService.java b/examples/media/multipart/src/main/java/io/helidon/examples/media/multipart/FileService.java
index cc85a0fd113..b757d8b0741 100644
--- a/examples/media/multipart/src/main/java/io/helidon/examples/media/multipart/FileService.java
+++ b/examples/media/multipart/src/main/java/io/helidon/examples/media/multipart/FileService.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2020, 2022 Oracle and/or its affiliates.
+ * Copyright (c) 2020, 2023 Oracle and/or its affiliates.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -21,6 +21,7 @@
 import java.util.concurrent.ExecutorService;
 
 import io.helidon.common.configurable.ThreadPoolSupplier;
+import io.helidon.common.http.BadRequestException;
 import io.helidon.common.http.ContentDisposition;
 import io.helidon.common.http.DataChunk;
 import io.helidon.common.http.Http;
@@ -81,10 +82,13 @@ private void download(ServerRequest req, ServerResponse res) {
     private void upload(ServerRequest req, ServerResponse res) {
         req.content().asStream(ReadableBodyPart.class)
            .forEach(part -> {
-               if ("file[]".equals(part.name())) {
-                   part.content().map(DataChunk::data)
+               if (part.isNamed("file[]")) {
+                   String filename = part.filename()
+                                         .orElseThrow(() -> new BadRequestException("Missing filename"));
+                   part.content()
+                       .map(DataChunk::data)
                        .flatMapIterable(Arrays::asList)
-                       .to(IoMulti.writeToFile(storage.create(part.filename()))
+                       .to(IoMulti.writeToFile(storage.create(filename))
                                   .executor(executor)
                                   .build());
                } else {
diff --git a/reactive/media/multipart/pom.xml b/reactive/media/multipart/pom.xml
index 37f6a7646d7..b82d4991339 100644
--- a/reactive/media/multipart/pom.xml
+++ b/reactive/media/multipart/pom.xml
@@ -1,6 +1,6 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <!--
-    Copyright (c) 2020, 2022 Oracle and/or its affiliates.
+    Copyright (c) 2020, 2023 Oracle and/or its affiliates.
 
     Licensed under the Apache License, Version 2.0 (the "License");
     you may not use this file except in compliance with the License.
@@ -113,5 +113,10 @@
             <artifactId>reactive-streams-tck-flow</artifactId>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>io.helidon.common.testing</groupId>
+            <artifactId>helidon-common-testing-junit5</artifactId>
+            <scope>test</scope>
+        </dependency>
     </dependencies>
 </project>
diff --git a/reactive/media/multipart/src/main/java/io/helidon/reactive/media/multipart/BodyPart.java b/reactive/media/multipart/src/main/java/io/helidon/reactive/media/multipart/BodyPart.java
index 605b5e4d83d..33ee88a1388 100644
--- a/reactive/media/multipart/src/main/java/io/helidon/reactive/media/multipart/BodyPart.java
+++ b/reactive/media/multipart/src/main/java/io/helidon/reactive/media/multipart/BodyPart.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2020, 2022 Oracle and/or its affiliates.
+ * Copyright (c) 2020, 2023 Oracle and/or its affiliates.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -15,6 +15,8 @@
  */
 package io.helidon.reactive.media.multipart;
 
+import java.util.Optional;
+
 import io.helidon.reactive.media.common.MessageBodyContent;
 
 /**
@@ -27,33 +29,46 @@ public interface BodyPart {
 
     /**
      * Get the reactive representation of the part content.
+     *
      * @return {@link io.helidon.reactive.media.common.MessageBodyContent}, never {@code null}
      */
     MessageBodyContent content();
 
     /**
      * Returns HTTP part headers.
+     *
      * @return BodyPartHeaders, never {@code null}
      */
     BodyPartHeaders headers();
 
+    /**
+     * Test the control name.
+     *
+     * @param name the name to test
+     * @return {@code true} if the {@code name} parameter of the {@code Content-Disposition} header matches,
+     * {@code false} otherwise
+     */
+    default boolean isNamed(String name) {
+        return name().map(name::equals).orElse(false);
+    }
+
     /**
      * Get the control name.
      *
      * @return the {@code name} parameter of the {@code Content-Disposition}
-     * header, or {@code null} if not present.
+     * header.
      */
-    default String name() {
-        return headers().contentDisposition().contentName().orElse(null);
+    default Optional<String> name() {
+        return headers().contentDisposition().contentName();
     }
 
     /**
      * Get the file name.
      *
      * @return the {@code filename} parameter of the {@code Content-Disposition}
-     * header, or {@code null} if not present.
+     * header.
      */
-    default String filename() {
-        return headers().contentDisposition().filename().orElse(null);
+    default Optional<String> filename() {
+        return headers().contentDisposition().filename();
     }
 }
diff --git a/reactive/media/multipart/src/test/java/io/helidon/reactive/media/multipart/BodyPartTest.java b/reactive/media/multipart/src/test/java/io/helidon/reactive/media/multipart/BodyPartTest.java
index 1505bc132c2..5a271c62480 100644
--- a/reactive/media/multipart/src/test/java/io/helidon/reactive/media/multipart/BodyPartTest.java
+++ b/reactive/media/multipart/src/test/java/io/helidon/reactive/media/multipart/BodyPartTest.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2020, 2022 Oracle and/or its affiliates.
+ * Copyright (c) 2020, 2023 Oracle and/or its affiliates.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -30,10 +30,11 @@
 
 import org.junit.jupiter.api.Test;
 
+import static io.helidon.common.testing.junit5.OptionalMatcher.optionalEmpty;
+import static io.helidon.common.testing.junit5.OptionalMatcher.optionalValue;
 import static io.helidon.reactive.media.multipart.MultiPartDecoderTest.chunksPublisher;
 import static org.hamcrest.CoreMatchers.equalTo;
 import static org.hamcrest.CoreMatchers.is;
-import static org.hamcrest.CoreMatchers.nullValue;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.fail;
@@ -122,8 +123,8 @@ public void testName() {
                         .build())
                 .entity("abc")
                 .build();
-        assertThat(bodyPart.name(), is(equalTo("foo")));
-        assertThat(bodyPart.filename(), is(nullValue()));
+        assertThat(bodyPart.name(), optionalValue(is("foo")));
+        assertThat(bodyPart.filename(), is(optionalEmpty()));
     }
 
     @Test
@@ -136,8 +137,8 @@ public void testFilename() {
                         .build())
                 .entity("abc")
                 .build();
-        assertThat(bodyPart.filename(), is(equalTo("foo.txt")));
-        assertThat(bodyPart.name(), is(nullValue()));
+        assertThat(bodyPart.filename(), optionalValue(is("foo.txt")));
+        assertThat(bodyPart.name(), is(optionalEmpty()));
     }
 
     static MessageBodyReadableContent readableContent(Publisher<DataChunk> chunks) {