From c61179d038ff3e9b865906a9c001f6b720fb7d8a Mon Sep 17 00:00:00 2001
From: Harald Musum <musum@yahooinc.com>
Date: Sun, 17 Sep 2023 18:50:30 +0200
Subject: Minor cleanup

---
 .../filedistribution/UrlDownloadRpcServer.java     | 42 +++++++++++++---------
 1 file changed, 26 insertions(+), 16 deletions(-)

diff --git a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/filedistribution/UrlDownloadRpcServer.java b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/filedistribution/UrlDownloadRpcServer.java
index 35c5bb14755..41ca5ceab82 100644
--- a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/filedistribution/UrlDownloadRpcServer.java
+++ b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/filedistribution/UrlDownloadRpcServer.java
@@ -9,6 +9,7 @@ import com.yahoo.jrt.Supervisor;
 import com.yahoo.security.tls.Capability;
 import com.yahoo.text.Utf8;
 import com.yahoo.vespa.defaults.Defaults;
+import com.yahoo.yolean.Exceptions;
 import net.jpountz.xxhash.XXHashFactory;
 
 import java.io.File;
@@ -20,6 +21,7 @@ import java.nio.ByteBuffer;
 import java.nio.channels.Channels;
 import java.nio.channels.ReadableByteChannel;
 import java.nio.file.Files;
+import java.util.Optional;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.TimeUnit;
 import java.util.logging.Level;
@@ -30,6 +32,7 @@ import static com.yahoo.vespa.config.UrlDownloader.HTTP_ERROR;
 import static com.yahoo.vespa.config.UrlDownloader.INTERNAL_ERROR;
 import static java.lang.Runtime.getRuntime;
 import static java.util.concurrent.Executors.newFixedThreadPool;
+import static java.util.logging.Level.WARNING;
 
 /**
  * An RPC server that handles URL download requests.
@@ -58,7 +61,8 @@ class UrlDownloadRpcServer {
     void close() {
         executor.shutdownNow();
         try {
-            executor.awaitTermination(10, TimeUnit.SECONDS);
+            if ( ! executor.awaitTermination(10, TimeUnit.SECONDS))
+                log.log(WARNING, "Failed to shut down url download rpc server within timeout");
         } catch (InterruptedException e) {
             throw new RuntimeException(e);
         }
@@ -80,25 +84,31 @@ class UrlDownloadRpcServer {
         }
 
         try {
-            URL website = new URL(url);
-            HttpURLConnection connection = (HttpURLConnection) website.openConnection();
-            if (connection.getResponseCode() == 200) {
-                log.log(Level.INFO, "Downloading URL '" + url + "'");
-                downloadFile(req, connection, downloadDir);
-            } else {
-                log.log(Level.SEVERE, "Download of URL '" + url + "' got server response: " + connection.getResponseCode());
-                req.setError(HTTP_ERROR, String.valueOf(connection.getResponseCode()));
-            }
+            Optional<File> file = downloadFile(url, downloadDir);
+            if (file.isPresent())
+                req.returnValues().add(new StringValue(file.get().getAbsolutePath()));
+            else
+                req.setError(DOES_NOT_EXIST, "URL '" + url + "' not found");
+        } catch (RuntimeException e) {
+            String message = "Download of '" + url + "' failed: " + Exceptions.toMessageString(e);
+            log.log(Level.SEVERE, message);
+            req.setError(HTTP_ERROR, e.getMessage());
         } catch (Throwable e) {
-            log.log(Level.SEVERE, "Download of URL '" + url + "' failed, got exception: " + e.getMessage());
-            req.setError(INTERNAL_ERROR, "Download of URL '" + url + "' internal error: " + e.getMessage());
+            String message = "Download of '" + url + "' failed: " + Exceptions.toMessageString(e);
+            log.log(Level.SEVERE, message);
+            req.setError(INTERNAL_ERROR, message);
         }
         req.returnRequest();
     }
 
-    private static void downloadFile(Request req, HttpURLConnection connection, File downloadDir) throws IOException {
+    private static Optional<File> downloadFile(String url, File downloadDir) throws IOException {
         long start = System.currentTimeMillis();
-        String url = connection.getURL().toString();
+        HttpURLConnection connection = (HttpURLConnection) new URL(url).openConnection();
+        if (connection.getResponseCode() == 200)
+            log.log(Level.INFO, "Downloading URL '" + url + "'");
+        else
+            throw new RuntimeException("Download of URL '" + url + "' failed, got response code " + connection.getResponseCode());
+
         Files.createDirectories(downloadDir.toPath());
         File contentsPath = new File(downloadDir, CONTENTS_FILE_NAME);
         try (ReadableByteChannel rbc = Channels.newChannel(connection.getInputStream())) {
@@ -107,13 +117,13 @@ class UrlDownloadRpcServer {
 
                 if (contentsPath.exists() && contentsPath.length() > 0) {
                     new RequestTracker().trackRequest(downloadDir);
-                    req.returnValues().add(new StringValue(contentsPath.getAbsolutePath()));
                     log.log(Level.FINE, () -> "URL '" + url + "' available at " + contentsPath);
                     log.log(Level.INFO, String.format("Download of URL '%s' done in %.3f seconds",
                                                       url, (System.currentTimeMillis() - start) / 1000.0));
+                    return Optional.of(contentsPath);
                 } else {
                     log.log(Level.SEVERE, "Downloaded URL '" + url + "' not found, returning error");
-                    req.setError(DOES_NOT_EXIST, "Downloaded '" + url + "' not found");
+                    return Optional.empty();
                 }
             }
         }
-- 
cgit v1.2.3