additional feedback

Author: KyleAure

modules/application-server-commons/src/main/java/org/testcontainers/applicationserver/ApplicationServerContainer.java

@@ -23,8 +23,6 @@
 import java.util.Map;
 import java.util.Objects;
 import java.util.concurrent.Future;
-import java.util.function.Function;
-import java.util.function.Predicate;
 import java.util.function.Supplier;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
@@ -137,10 +135,10 @@ public void setExposedPorts(List<Integer> exposedPorts) {
      * @param archives - An archive created using shrinkwrap and test runtime
      * @return self
      */
-    public ApplicationServerContainer withArchvies(@NonNull Archive<?>... archives) {
+    public ApplicationServerContainer withArchives(@NonNull Archive<?>... archives) {
         Stream.of(archives).forEach(archive -> {
             String name = archive.getName();
-            Path target = Paths.get(getTempDirectory().toString(), name);
+            Path target = Paths.get(createTempDirectory().toString(), name);
             archive.as(ZipExporter.class).exportTo(target.toFile(), true);
             this.archives.add(MountableFile.forHostPath(target));
         });
@@ -310,17 +308,6 @@ public String getBaseURL() {
         return "http://" + getHost() + ':' + getMappedPort(this.httpPort);
     }
 
-    /**
-     * Gets the current state of the HTTP port.
-     * Helpful during the configure step for implementations to set a default port
-     * if none was configured by the user.
-     *
-     * @return true if an httpPort was set, false otherwise.
-     */
-    protected boolean isHttpPortSet() {
-        return Objects.nonNull(httpPort);
-    }
-
     // Abstract
 
     /**
@@ -329,7 +316,7 @@ protected boolean isHttpPortSet() {
      *
      * @return - the application install directory
      */
-    abstract protected String getApplicationInstallDirectory();
+    protected abstract String getApplicationInstallDirectory();
 
     // Helpers
 
@@ -338,7 +325,7 @@ protected boolean isHttpPortSet() {
      *
      * @return - The temporary directory path
      */
-    protected static Path getTempDirectory() {
+    protected static Path createTempDirectory() {
         if( Objects.nonNull(tempDirectory) ) {
             return tempDirectory;
         }

modules/application-server-commons/src/test/java/org/testcontainers/applicationserver/ApplicationServerContainerTest.java

@@ -5,6 +5,7 @@
 import org.testcontainers.utility.DockerImageName;
 
 import java.time.Duration;
+import java.util.Arrays;
 import java.util.List;
 import java.util.concurrent.Future;
 
@@ -39,7 +40,7 @@ public void testNormalizePath() {
     public void httpPortMapping() {
         List<Integer> expected, actual;
 
-        expected = List.of(8080, 9080, 9443);
+        expected = Arrays.asList(8080, 9080, 9443);
 
         // Test expose ports, then add httpPort
         testContainer = new ApplicationServerContainerStub(DockerImageName.parse("open-liberty:kernel-slim-java11-openj9"));
@@ -60,7 +61,7 @@ public void httpPortMapping() {
         //Test httpPort then set exposed ports
         testContainer = new ApplicationServerContainerStub(DockerImageName.parse("open-liberty:kernel-slim-java11-openj9"));
         testContainer.withHttpPort(8080);
-        testContainer.setExposedPorts(List.of(9080, 9443));
+        testContainer.setExposedPorts(Arrays.asList(9080, 9443));
 
         actual = testContainer.getExposedPorts();
         assertThat(actual).containsExactlyElementsOf(expected);

modules/liberty/src/main/java/org/testcontainers/containers/LibertyServerContainer.java

@@ -1,16 +1,11 @@
 package org.testcontainers.containers;
 
 import lombok.NonNull;
+import org.testcontainers.images.builder.Transferable;
 import org.testcontainers.utility.DockerImageName;
 import org.testcontainers.utility.MountableFile;
 import org.testcontainers.applicationserver.ApplicationServerContainer;
 
-import java.io.IOException;
-import java.nio.charset.StandardCharsets;
-import java.nio.file.Files;
-import java.nio.file.Path;
-import java.nio.file.Paths;
-import java.nio.file.StandardOpenOption;
 import java.time.Duration;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -22,19 +17,17 @@
  */
 public class LibertyServerContainer extends ApplicationServerContainer {
 
-    public static final String NAME = "Liberty";
-
     // About the image
-    public static final String IMAGE = "open-liberty";
+    static final String IMAGE = "open-liberty";
 
-    private static final DockerImageName DEFAULT_IMAGE_NAME = DockerImageName.parse(IMAGE);
+    static final DockerImageName DEFAULT_IMAGE_NAME = DockerImageName.parse(IMAGE);
 
     // Container defaults
-    public static final int DEFAULT_HTTP_PORT = 9080;
+    static final int DEFAULT_HTTP_PORT = 9080;
 
-    public static final int DEFAULT_HTTPS_PORT = 9443;
+    static final int DEFAULT_HTTPS_PORT = 9443;
 
-    private static final Duration DEFAULT_WAIT_TIMEOUT = Duration.ofSeconds(30);
+    static final Duration DEFAULT_WAIT_TIMEOUT = Duration.ofSeconds(30);
 
     private static final String SERVER_CONFIG_DIR = "/config/";
 
@@ -43,7 +36,7 @@ public class LibertyServerContainer extends ApplicationServerContainer {
     private static final List<String> DEFAULT_FEATURES = Arrays.asList("webProfile-10.0");
 
     // Container fields
-    private MountableFile serverConfiguration;
+    private Transferable serverConfiguration;
 
     private List<String> features = new ArrayList<>();
 
@@ -74,16 +67,16 @@ public void configure() {
 
         // Copy server configuration
         if( Objects.nonNull(serverConfiguration) ) {
-            withCopyFileToContainer(serverConfiguration, SERVER_CONFIG_DIR + "server.xml");
+            withCopyToContainer(serverConfiguration, SERVER_CONFIG_DIR + "server.xml");
             return;
         }
 
         if ( ! features.isEmpty() ) {
-            withCopyFileToContainer(generateServerConfiguration(features), SERVER_CONFIG_DIR + "server.xml");
+            withCopyToContainer(generateServerConfiguration(features), SERVER_CONFIG_DIR + "server.xml");
             return;
         }
 
-        withCopyFileToContainer(generateServerConfiguration(DEFAULT_FEATURES), SERVER_CONFIG_DIR + "server.xml");
+        withCopyToContainer(generateServerConfiguration(DEFAULT_FEATURES), SERVER_CONFIG_DIR + "server.xml");
 
     }
 
@@ -122,7 +115,7 @@ public LibertyServerContainer withFeatures(String... features) {
 
     // Helpers
 
-    private static final MountableFile generateServerConfiguration(List<String> features) {
+    private static final Transferable generateServerConfiguration(List<String> features) {
         String configContents = "";
         configContents += "<server><featureManager>";
         for(String feature : features) {
@@ -131,15 +124,6 @@ private static final MountableFile generateServerConfiguration(List<String> feat
         configContents += "</featureManager></server>";
         configContents += System.lineSeparator();
 
-        Path generatedConfigPath = Paths.get(getTempDirectory().toString(), "generatedServer.xml");
-
-        try {
-            Files.write(generatedConfigPath, configContents.getBytes(StandardCharsets.UTF_8),
-                StandardOpenOption.CREATE, StandardOpenOption.WRITE);
-        } catch (IOException ioe) {
-            throw new RuntimeException("Unable to generate server configuration at runtime", ioe);
-        }
-
-        return MountableFile.forHostPath(generatedConfigPath);
+        return Transferable.of(configContents);
     }
 }

modules/liberty/src/test/java/org/testcontainers/containers/LibertyContainerTest.java

@@ -33,7 +33,7 @@ public class LibertyContainerTest {
         .withTag("23.0.0.3-full-java17-openj9");
 
     private static ApplicationServerContainer testContainer = new LibertyServerContainer(libertyImage)
-        .withArchvies(createDeployment())
+        .withArchives(createDeployment())
         .withAppContextRoot("test/app/service/")
         .withLazyEnv("mock.port", () ->  "" + dependantService.getMappedPort(80))
         .dependsOn(dependantService);