adds fail fast when import URL has no path

zamzar · Mar 19, 2024 · 96fd2e3 · 96fd2e3
1 parent a322995
commit 96fd2e3
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 17 deletions.
diff --git a/src/main/java/com/zamzar/api/JobBuilder.java b/src/main/java/com/zamzar/api/JobBuilder.java
@@ -85,8 +85,8 @@ public JobBuilder exportingTo(URI destination) {
         return this;
     }
 
-    protected Source getSource() {
-        return source;
+    protected Integer prepareSource(ZamzarClient zamzar) throws ApiException {
+        return source.prepare(this, zamzar);
     }
 
     protected String getTargetFormat() {
@@ -115,7 +115,7 @@ static Modifier identity() {
     }
 
     protected interface Source {
-        Integer prepare(ZamzarClient zamzar) throws ApiException;
+        Integer prepare(JobBuilder builder, ZamzarClient zamzar) throws ApiException;
     }
 
     protected static class LocalFile implements Source {
@@ -125,7 +125,7 @@ protected LocalFile(File file) {
             this.file = file;
         }
 
-        public Integer prepare(ZamzarClient zamzar) throws ApiException {
+        public Integer prepare(JobBuilder builder, ZamzarClient zamzar) throws ApiException {
             return zamzar.upload(file).getId();
         }
     }
@@ -137,7 +137,7 @@ protected ExistingFile(Integer id) {
             this.id = id;
         }
 
-        public Integer prepare(ZamzarClient zamzar) {
+        public Integer prepare(JobBuilder builder, ZamzarClient zamzar) {
             return id;
         }
     }
@@ -149,18 +149,22 @@ protected RemoteFile(URI url) {
             this.url = url;
         }
 
-        public Integer prepare(ZamzarClient zamzar) throws ApiException {
-            final ImportManager _import = zamzar.imports().start(this.url.toString(), this.getFilename(null)).awaitOrThrow();
+        public Integer prepare(JobBuilder builder, ZamzarClient zamzar) throws ApiException {
+            final String url = this.url.toString();
+            final String filename = this.getFilename(builder.getSourceFormat());
+            final ImportManager _import = zamzar.imports().start(url, filename).awaitOrThrow();
             return _import.getImportedFile().getId();
         }
 
-        protected String getFilename(String extension) {
+        protected String getFilename(String extension) throws ApiException {
             final File file = new File(url.getPath());
 
             if (file.getName().isEmpty()) {
-                return null;
-            } else if (file.getName().contains(".") || extension == null) {
+                throw new ApiException("Could not infer filename from URL (" + url + "). Provide a URL that contains a path.");
+            } else if (file.getName().contains(".")) {
                 return file.getName();
+            } else if (extension == null || extension.isEmpty()) {
+                throw new ApiException("Could not infer filename from URL (" + url + "). Provide an extension to disambiguate.");
             } else {
                 return file.getName() + "." + extension;
             }

diff --git a/src/main/java/com/zamzar/api/JobsService.java b/src/main/java/com/zamzar/api/JobsService.java
@@ -100,7 +100,7 @@ public JobManager create(URI source, String targetFormat, JobBuilder.Modifier mo
     }
 
     protected JobManager create(JobBuilder builder) throws ApiException {
-        final Integer sourceId = builder.getSource().prepare(zamzar);
+        final Integer sourceId = builder.prepareSource(zamzar);
         final String targetFormat = builder.getTargetFormat();
         final String sourceFormat = builder.getSourceFormat();
         final String exportUrl = builder.getDestination() == null ? null : builder.getDestination().toString();

diff --git a/src/test/java/com/zamzar/api/JobBuilderTest.java b/src/test/java/com/zamzar/api/JobBuilderTest.java
@@ -1,36 +1,52 @@
 package com.zamzar.api;
 
+import com.zamzar.api.invoker.ApiException;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.Arguments;
 import org.junit.jupiter.params.provider.MethodSource;
 
 import java.net.URI;
-import java.net.URISyntaxException;
 import java.util.stream.Stream;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 
 public class JobBuilderTest {
 
     @ParameterizedTest
-    @MethodSource("urlsAndExtensions")
-    void remoteFileSetsReasonableFilename(String url, String extension, String expected) throws URISyntaxException {
+    @MethodSource("filenameCanBeInferred")
+    void remoteFileSetsReasonableFilename(String url, String extension, String expected) throws Exception {
         assertEquals(
             expected,
             new JobBuilder.RemoteFile(new URI(url)).getFilename(extension)
         );
     }
 
-    private static Stream<Arguments> urlsAndExtensions() {
+    @ParameterizedTest
+    @MethodSource("filenameCannotBeInferred")
+    void remoteFileThrowsWhenFilenameCannotBeInferred(String url, String extension) {
+        assertThrows(
+            ApiException.class,
+            () -> new JobBuilder.RemoteFile(new URI(url)).getFilename(extension)
+        );
+    }
+
+    private static Stream<Arguments> filenameCanBeInferred() {
         return Stream.of(
             // URL / extension / expected filename
             Arguments.of("https://example.com/file.txt", null, "file.txt"),
             Arguments.of("https://example.com/file.txt", "unused", "file.txt"),
             Arguments.of("https://example.com/file", "txt", "file.txt"),
-            Arguments.of("https://example.com/file", null, "file"),
             Arguments.of("https://example.com/file.txt?query=param", null, "file.txt"),
             Arguments.of("https://example.com/file.txt?query=param", "unused", "file.txt"),
-            Arguments.of("https://example.com/file?query=param", "txt", "file.txt"),
+            Arguments.of("https://example.com/file?query=param", "txt", "file.txt")
+        );
+    }
+
+    private static Stream<Arguments> filenameCannotBeInferred() {
+        return Stream.of(
+            // URL / extension
+            Arguments.of("https://example.com/file", null),
             Arguments.of("https://example.com/file?query=param", null, "file"),
             Arguments.of("https://example.com/", "txt", null),
             Arguments.of("https://example.com/", null, null),