Merge pull request #6975 from vespa-engine/freva/operate-on-byte-array

Operate on byte array

7 files changed, 63 insertions, 40 deletions

diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/FileContentCache.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/FileContentCache.java
index ca79e8bb113..43c9f7729ef 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/FileContentCache.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/FileContentCache.java
@@ -13,23 +13,23 @@ import java.util.Optional;
 class FileContentCache {
     private final UnixPath path;
 
-    private Optional<String> value = Optional.empty();
+    private Optional<byte[]> value = Optional.empty();
     private Optional<Instant> modifiedTime = Optional.empty();
 
     FileContentCache(UnixPath path) {
         this.path = path;
     }
 
-    String get(Instant lastModifiedTime) {
+    byte[] get(Instant lastModifiedTime) {
         if (!value.isPresent() || lastModifiedTime.compareTo(modifiedTime.get()) > 0) {
-            value = Optional.of(path.readUtf8File());
+            value = Optional.of(path.readBytes());
             modifiedTime = Optional.of(lastModifiedTime);
         }
 
         return value.get();
     }
 
-    void updateWith(String content, Instant modifiedTime) {
+    void updateWith(byte[] content, Instant modifiedTime) {
         this.value = Optional.of(content);
         this.modifiedTime = Optional.of(modifiedTime);
     }
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/FileSync.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/FileSync.java
index 7f11707c1cc..78f720074dc 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/FileSync.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/FileSync.java
@@ -5,7 +5,7 @@ package com.yahoo.vespa.hosted.node.admin.task.util.file;
 import com.yahoo.vespa.hosted.node.admin.component.TaskContext;
 
 import java.nio.file.Path;
-import java.util.Objects;
+import java.util.Arrays;
 import java.util.Optional;
 import java.util.logging.Logger;
 
@@ -46,7 +46,7 @@ public class FileSync {
     }
 
     private boolean maybeUpdateContent(TaskContext taskContext,
-                                       Optional<String> content,
+                                       Optional<byte[]> content,
                                        FileAttributesCache currentAttributes) {
         if (!content.isPresent()) {
             return false;
@@ -55,16 +55,16 @@ public class FileSync {
         if (!currentAttributes.exists()) {
             taskContext.recordSystemModification(logger, "Creating file " + path);
             path.createParents();
-            path.writeUtf8File(content.get());
+            path.writeBytes(content.get());
             contentCache.updateWith(content.get(), currentAttributes.forceGet().lastModifiedTime());
             return true;
         }
 
-        if (Objects.equals(content.get(), contentCache.get(currentAttributes.get().lastModifiedTime()))) {
+        if (Arrays.equals(content.get(), contentCache.get(currentAttributes.get().lastModifiedTime()))) {
             return false;
         } else {
             taskContext.recordSystemModification(logger, "Patching file " + path);
-            path.writeUtf8File(content.get());
+            path.writeBytes(content.get());
             contentCache.updateWith(content.get(), currentAttributes.forceGet().lastModifiedTime());
             return true;
         }
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/FileWriter.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/FileWriter.java
index c41fd71c62c..f7eba68e455 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/FileWriter.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/FileWriter.java
@@ -3,6 +3,7 @@ package com.yahoo.vespa.hosted.node.admin.task.util.file;
 
 import com.yahoo.vespa.hosted.node.admin.component.TaskContext;
 
+import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.function.Supplier;
@@ -16,11 +17,15 @@ public class FileWriter {
     private final Path path;
     private final FileSync fileSync;
     private final PartialFileData.Builder fileDataBuilder = PartialFileData.builder();
-    private final Supplier<String> contentProducer;
+    private final Supplier<byte[]> contentProducer;
 
     private boolean overwriteExistingFile = true;
 
     public FileWriter(Path path, Supplier<String> contentProducer) {
+        this(path, () -> contentProducer.get().getBytes(StandardCharsets.UTF_8));
+    }
+
+    public FileWriter(Path path, ByteArraySupplier contentProducer) {
         this.path = path;
         this.fileSync = new FileSync(path);
         this.contentProducer = contentProducer;
@@ -55,4 +60,7 @@ public class FileWriter {
         PartialFileData fileData = fileDataBuilder.create();
         return fileSync.convergeTo(context, fileData);
     }
+
+    @FunctionalInterface
+    public interface ByteArraySupplier extends Supplier<byte[]> { }
 }
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/PartialFileData.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/PartialFileData.java
index b931a374230..0fae9b17eba 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/PartialFileData.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/PartialFileData.java
@@ -2,6 +2,8 @@
 
 package com.yahoo.vespa.hosted.node.admin.task.util.file;
 
+import java.nio.charset.Charset;
+import java.nio.charset.StandardCharsets;
 import java.util.Optional;
 
 /**
@@ -11,7 +13,7 @@ import java.util.Optional;
  */
 // @Immutable
 public class PartialFileData {
-    private final Optional<String> content;
+    private final Optional<byte[]> content;
     private final Optional<String> owner;
     private final Optional<String> group;
     private final Optional<String> permissions;
@@ -20,17 +22,17 @@ public class PartialFileData {
         return new Builder();
     }
 
-    public PartialFileData(Optional<String> content,
-                           Optional<String> owner,
-                           Optional<String> group,
-                           Optional<String> permissions) {
+    private PartialFileData(Optional<byte[]> content,
+                            Optional<String> owner,
+                            Optional<String> group,
+                            Optional<String> permissions) {
         this.content = content;
         this.owner = owner;
         this.group = group;
         this.permissions = permissions;
     }
 
-    public Optional<String> getContent() {
+    public Optional<byte[]> getContent() {
         return content;
     }
 
@@ -47,12 +49,14 @@ public class PartialFileData {
     }
 
     public static class Builder {
-        private Optional<String> content = Optional.empty();
+        private Optional<byte[]> content = Optional.empty();
         private Optional<String> owner = Optional.empty();
         private Optional<String> group = Optional.empty();
         private Optional<String> permissions = Optional.empty();
 
-        public Builder withContent(String content) { this.content = Optional.of(content); return this; }
+        public Builder withContent(byte[] content) { this.content = Optional.of(content); return this; }
+        public Builder withContent(String content, Charset charset) { return withContent(content.getBytes(charset)); }
+        public Builder withContent(String content) { return withContent(content, StandardCharsets.UTF_8); }
         public Builder withOwner(String owner) { this.owner = Optional.of(owner); return this; }
         public Builder withGroup(String group) { this.group = Optional.of(group); return this; }
         public Builder withPermissions(String permissions) { this.permissions = Optional.of(permissions); return this; }
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/UnixPath.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/UnixPath.java
index 90404bb596e..4baba9acb4e 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/UnixPath.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/UnixPath.java
@@ -52,13 +52,19 @@ public class UnixPath {
     }
 
     public String readUtf8File() {
-        byte[] byteContent = uncheck(() -> Files.readAllBytes(path));
-        return new String(byteContent, StandardCharsets.UTF_8);
+        return new String(readBytes(), StandardCharsets.UTF_8);
+    }
+
+    public byte[] readBytes() {
+        return uncheck(() -> Files.readAllBytes(path));
     }
 
     public void writeUtf8File(String content, OpenOption... options) {
-        byte[] contentInUtf8 = content.getBytes(StandardCharsets.UTF_8);
-        uncheck(() -> Files.write(path, contentInUtf8, options));
+        writeBytes(content.getBytes(StandardCharsets.UTF_8), options);
+    }
+
+    public void writeBytes(byte[] content, OpenOption... options) {
+        uncheck(() -> Files.write(path, content, options));
     }
 
     public String getPermissions() {
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/file/FileContentCacheTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/file/FileContentCacheTest.java
index 83ab5462fb6..f499d8b46ad 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/file/FileContentCacheTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/file/FileContentCacheTest.java
@@ -4,9 +4,10 @@ package com.yahoo.vespa.hosted.node.admin.task.util.file;
 
 import org.junit.Test;
 
+import java.nio.charset.StandardCharsets;
 import java.time.Instant;
 
-import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertArrayEquals;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
@@ -17,41 +18,44 @@ public class FileContentCacheTest {
     private final UnixPath unixPath = mock(UnixPath.class);
     private final FileContentCache cache = new FileContentCache(unixPath);
 
+    private final byte[] content = "content".getBytes(StandardCharsets.UTF_8);
+    private final byte[] newContent = "new-content".getBytes(StandardCharsets.UTF_8);
+
     @Test
     public void get() {
-        when(unixPath.readUtf8File()).thenReturn("content");
-        assertEquals("content", cache.get(Instant.ofEpochMilli(0)));
-        verify(unixPath, times(1)).readUtf8File();
+        when(unixPath.readBytes()).thenReturn(content);
+        assertArrayEquals(content, cache.get(Instant.ofEpochMilli(0)));
+        verify(unixPath, times(1)).readBytes();
         verifyNoMoreInteractions(unixPath);
 
         // cache hit
-        assertEquals("content", cache.get(Instant.ofEpochMilli(0)));
-        verify(unixPath, times(1)).readUtf8File();
+        assertArrayEquals(content, cache.get(Instant.ofEpochMilli(0)));
+        verify(unixPath, times(1)).readBytes();
         verifyNoMoreInteractions(unixPath);
 
         // cache miss
-        when(unixPath.readUtf8File()).thenReturn("new-content");
-        assertEquals("new-content", cache.get(Instant.ofEpochMilli(1)));
-        verify(unixPath, times(1 + 1)).readUtf8File();
+        when(unixPath.readBytes()).thenReturn(newContent);
+        assertArrayEquals(newContent, cache.get(Instant.ofEpochMilli(1)));
+        verify(unixPath, times(1 + 1)).readBytes();
         verifyNoMoreInteractions(unixPath);
 
         // cache hit both at times 0 and 1
-        assertEquals("new-content", cache.get(Instant.ofEpochMilli(0)));
-        verify(unixPath, times(1 + 1)).readUtf8File();
+        assertArrayEquals(newContent, cache.get(Instant.ofEpochMilli(0)));
+        verify(unixPath, times(1 + 1)).readBytes();
         verifyNoMoreInteractions(unixPath);
-        assertEquals("new-content", cache.get(Instant.ofEpochMilli(1)));
-        verify(unixPath, times(1 + 1)).readUtf8File();
+        assertArrayEquals(newContent, cache.get(Instant.ofEpochMilli(1)));
+        verify(unixPath, times(1 + 1)).readBytes();
         verifyNoMoreInteractions(unixPath);
     }
 
     @Test
     public void updateWith() {
-        cache.updateWith("content", Instant.ofEpochMilli(2));
-        assertEquals("content", cache.get(Instant.ofEpochMilli(2)));
+        cache.updateWith(content, Instant.ofEpochMilli(2));
+        assertArrayEquals(content, cache.get(Instant.ofEpochMilli(2)));
         verifyNoMoreInteractions(unixPath);
 
-        cache.updateWith("new-content", Instant.ofEpochMilli(4));
-        assertEquals("new-content", cache.get(Instant.ofEpochMilli(4)));
+        cache.updateWith(newContent, Instant.ofEpochMilli(4));
+        assertArrayEquals(newContent, cache.get(Instant.ofEpochMilli(4)));
         verifyNoMoreInteractions(unixPath);
     }
 
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/file/FileSyncTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/file/FileSyncTest.java
index dbc2cc9a5d5..a141faf290b 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/file/FileSyncTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/file/FileSyncTest.java
@@ -14,6 +14,7 @@ import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
 
+import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
@@ -64,7 +65,7 @@ public class FileSyncTest {
         assertTrue(fileSync.convergeTo(taskContext, fileData));
 
         assertTrue(Files.isRegularFile(path));
-        fileData.getContent().ifPresent(content -> assertEquals(content, unixPath.readUtf8File()));
+        fileData.getContent().ifPresent(content -> assertArrayEquals(content, unixPath.readBytes()));
         fileData.getOwner().ifPresent(owner -> assertEquals(owner, unixPath.getOwner()));
         fileData.getGroup().ifPresent(group -> assertEquals(group, unixPath.getGroup()));
         fileData.getPermissions().ifPresent(permissions -> assertEquals(permissions, unixPath.getPermissions()));