[incubator-baremaps] branch main updated: Make the behavior of replaceExisting more consistent (#727)

[bchapuis]

This is an automated email from the ASF dual-hosted git repository.

bchapuis pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/incubator-baremaps.git


The following commit(s) were added to refs/heads/main by this push:
     new 9ddeefd0 Make the behavior of replaceExisting more consistent (#727)
9ddeefd0 is described below

commit 9ddeefd0ae29c74b3a6f9778ec2efbf524d46051
Author: Perdjesk <perdj...@users.noreply.github.com>
AuthorDate: Sun Jul 30 20:12:08 2023 +0200

    Make the behavior of replaceExisting more consistent (#727)
    
    * Consistent behavior for replaceExisting file, only replace if set to true
    
    * Adapt test to replaceExisting behavior
---
 .../apache/baremaps/workflow/tasks/DownloadUrl.java | 21 +++++----------------
 .../baremaps/workflow/tasks/DownloadUrlTest.java    |  2 +-
 2 files changed, 6 insertions(+), 17 deletions(-)

diff --git 
a/baremaps-core/src/main/java/org/apache/baremaps/workflow/tasks/DownloadUrl.java
 
b/baremaps-core/src/main/java/org/apache/baremaps/workflow/tasks/DownloadUrl.java
index f22bb3a8..1472dfe8 100644
--- 
a/baremaps-core/src/main/java/org/apache/baremaps/workflow/tasks/DownloadUrl.java
+++ 
b/baremaps-core/src/main/java/org/apache/baremaps/workflow/tasks/DownloadUrl.java
@@ -41,29 +41,18 @@ public record DownloadUrl(String url, Path path, boolean 
replaceExisting) implem
     var targetUrl = new URL(url);
     var targetPath = path.toAbsolutePath();
 
-    if (isHttp(targetUrl)) {
-      if (Files.exists(targetPath) && !replaceExisting) {
-        var head = (HttpURLConnection) targetUrl.openConnection();
-        head.setInstanceFollowRedirects(true);
-        head.setRequestMethod("HEAD");
-        var contentLength = head.getContentLengthLong();
-        head.disconnect();
-        if (Files.size(targetPath) == contentLength) {
-          logger.info("Skipping download of {} to {}", url, path);
-          return;
-        }
-      }
+    if (Files.exists(targetPath) && !replaceExisting) {
+      logger.info("Skipping download of {} to {}", url, path);
+      return;
+    }
 
+    if (isHttp(targetUrl)) {
       var get = (HttpURLConnection) targetUrl.openConnection();
       get.setInstanceFollowRedirects(true);
       get.setRequestMethod("GET");
       urlDownloadToFile(get, targetPath);
       get.disconnect();
     } else if (isFtp(targetUrl)) {
-      if (Files.exists(targetPath) && !replaceExisting) {
-        logger.info("Skipping download of {} to {}", url, path);
-        return;
-      }
       urlDownloadToFile(targetUrl.openConnection(), targetPath);
     } else {
       throw new IllegalArgumentException("Unsupported URL protocol (supported: 
http(s)/ftp)");
diff --git 
a/baremaps-core/src/test/java/org/apache/baremaps/workflow/tasks/DownloadUrlTest.java
 
b/baremaps-core/src/test/java/org/apache/baremaps/workflow/tasks/DownloadUrlTest.java
index 00b78c35..dd99b9d4 100644
--- 
a/baremaps-core/src/test/java/org/apache/baremaps/workflow/tasks/DownloadUrlTest.java
+++ 
b/baremaps-core/src/test/java/org/apache/baremaps/workflow/tasks/DownloadUrlTest.java
@@ -31,7 +31,7 @@ class DownloadUrlTest {
     var file = File.createTempFile("test", ".tmp");
     file.deleteOnExit();
     var task = new 
DownloadUrl("https://raw.githubusercontent.com/baremaps/baremaps/main/README.md";,
-        file.toPath());
+        file.toPath(), true);
     task.execute(new WorkflowContext());
     assertTrue(Files.readString(file.toPath()).contains("Baremaps"));
   }