diff options
author | Valerij Fredriksen <valerijf@verizonmedia.com> | 2021-09-09 13:16:13 +0200 |
---|---|---|
committer | Valerij Fredriksen <valerijf@verizonmedia.com> | 2021-09-09 21:41:10 +0200 |
commit | dacf6dc8fe7566123423aad9aa29481bb50f6f60 (patch) | |
tree | 30b0db26d7c5122b1160f46d5b08d57c59f71d9d /controller-server | |
parent | 4184153d3d9b790d53af99ccfe668692f735f508 (diff) |
Create class to generate ApplicationPackageDiff
Diffstat (limited to 'controller-server')
8 files changed, 628 insertions, 21 deletions
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackage.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackage.java index 216a1717bad..3fcf9fc41f2 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackage.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackage.java @@ -251,10 +251,11 @@ public class ApplicationPackage { private Map<Path, Optional<byte[]>> read(Collection<String> names) { var entries = new ZipStreamReader(new ByteArrayInputStream(zip), name -> names.contains(withoutLegacyDir(name)), - maxSize) + maxSize, + true) .entries().stream() .collect(toMap(entry -> Paths.get(withoutLegacyDir(entry.zipEntry().getName())).normalize(), - entry -> Optional.of(entry.content()))); + ZipStreamReader.ZipEntryWithContent::content)); names.stream().map(Paths::get).forEach(path -> entries.putIfAbsent(path.normalize(), Optional.empty())); return entries; } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackageDiff.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackageDiff.java new file mode 100644 index 00000000000..6b106085b70 --- /dev/null +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackageDiff.java @@ -0,0 +1,114 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.controller.application.pkg; + +import java.io.BufferedReader; +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.io.InputStreamReader; +import java.io.UncheckedIOException; +import java.nio.charset.StandardCharsets; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +import static com.yahoo.vespa.hosted.controller.application.pkg.ZipStreamReader.ZipEntryWithContent; + +/** + * @author freva + */ +public class ApplicationPackageDiff { + + public static String diffAgainstEmpty(ApplicationPackage right) { + byte[] emptyZip = new byte[]{80, 75, 5, 6, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}; + return diff(new ApplicationPackage(emptyZip), right); + } + + public static String diff(ApplicationPackage left, ApplicationPackage right) { + return diff(left, right, 10 << 20, 1 << 20, 10 << 20); + } + + static String diff(ApplicationPackage left, ApplicationPackage right, int maxFileSizeToDiff, int maxDiffSizePerFile, int maxTotalDiffSize) { + if (Arrays.equals(left.zippedContent(), right.zippedContent())) return "No diff\n"; + + Map<String, ZipEntryWithContent> leftContents = readContents(left, maxFileSizeToDiff); + Map<String, ZipEntryWithContent> rightContents = readContents(right, maxFileSizeToDiff); + + StringBuilder sb = new StringBuilder(); + List<String> files = Stream.of(leftContents, rightContents) + .flatMap(contents -> contents.keySet().stream()) + .sorted() + .distinct() + .collect(Collectors.toList()); + for (String file : files) { + if (sb.length() > maxTotalDiffSize) + sb.append("--- ").append(file).append('\n').append("Diff skipped: Total diff size >").append(maxTotalDiffSize).append("B)\n\n"); + else + diff(Optional.ofNullable(leftContents.get(file)), Optional.ofNullable(rightContents.get(file)), maxDiffSizePerFile) + .ifPresent(diff -> sb.append("--- ").append(file).append('\n').append(diff).append('\n')); + } + + return sb.length() == 0 ? "No diff\n" : sb.toString(); + } + + private static Optional<String> diff(Optional<ZipEntryWithContent> left, Optional<ZipEntryWithContent> right, int maxDiffSizePerFile) { + Optional<byte[]> leftContent = left.flatMap(ZipEntryWithContent::content); + Optional<byte[]> rightContent = right.flatMap(ZipEntryWithContent::content); + if (leftContent.isPresent() && rightContent.isPresent() && Arrays.equals(leftContent.get(), rightContent.get())) + return Optional.empty(); + + if (left.map(entry -> entry.content().isEmpty()).orElse(false) || right.map(entry -> entry.content().isEmpty()).orElse(false)) + return Optional.of(String.format("Diff skipped: File too large (%s -> %s)\n", + left.map(e -> e.size() + "B").orElse("new file"), right.map(e -> e.size() + "B").orElse("file deleted"))); + + if (leftContent.map(c -> isBinary(c)).or(() -> rightContent.map(c -> isBinary(c))).orElse(false)) + return Optional.of(String.format("Diff skipped: File is binary (%s -> %s)\n", + left.map(e -> e.size() + "B").orElse("new file"), right.map(e -> e.size() + "B").orElse("file deleted"))); + + return LinesComparator.diff( + leftContent.map(c -> lines(c)).orElseGet(List::of), + rightContent.map(c -> lines(c)).orElseGet(List::of)) + .map(diff -> diff.length() > maxDiffSizePerFile ? "Diff skipped: Diff too large (" + diff.length() + "B)\n" : diff); + } + + private static Map<String, ZipEntryWithContent> readContents(ApplicationPackage app, int maxFileSizeToDiff) { + return new ZipStreamReader(new ByteArrayInputStream(app.zippedContent()), entry -> true, maxFileSizeToDiff, false).entries().stream() + .collect(Collectors.toMap(entry -> entry.zipEntry().getName(), e -> e)); + } + + private static List<String> lines(byte[] data) { + List<String> lines = new ArrayList<>(Math.min(16, data.length / 100)); + try (ByteArrayInputStream stream = new ByteArrayInputStream(data); + InputStreamReader streamReader = new InputStreamReader(stream, StandardCharsets.UTF_8); + BufferedReader bufferedReader = new BufferedReader(streamReader)) { + String line; + while ((line = bufferedReader.readLine()) != null) { + lines.add(line); + } + } catch (IOException e) { + throw new UncheckedIOException(e); + } + return lines; + } + + private static boolean isBinary(byte[] data) { + if (data.length == 0) return false; + + int lengthToCheck = Math.min(data.length, 10000); + int ascii = 0; + + for (int i = 0; i < lengthToCheck; i++) { + byte b = data[i]; + if (b < 0x9) return true; + + // TAB, newline/line feed, carriage return + if (b == 0x9 || b == 0xA || b == 0xD) ascii++; + else if (b >= 0x20 && b <= 0x7E) ascii++; + } + + return (double) ascii / lengthToCheck < 0.95; + } +} diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/LinesComparator.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/LinesComparator.java new file mode 100644 index 00000000000..8b4791c6b1b --- /dev/null +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/LinesComparator.java @@ -0,0 +1,246 @@ +/* + * Line based variant of Apache commons-text StringComparator + * https://github.com/apache/commons-text/blob/3b1a0a5a47ee9fa2b36f99ca28e2e1d367a10a11/src/main/java/org/apache/commons/text/diff/StringsComparator.java + */ + +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.yahoo.vespa.hosted.controller.application.pkg; + +import com.yahoo.collections.Pair; + +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.stream.Collectors; +import java.util.stream.IntStream; + +/** + * <p> + * It is guaranteed that the comparisons will always be done as + * {@code o1.equals(o2)} where {@code o1} belongs to the first + * sequence and {@code o2} belongs to the second sequence. This can + * be important if subclassing is used for some elements in the first + * sequence and the {@code equals} method is specialized. + * </p> + * <p> + * Comparison can be seen from two points of view: either as giving the smallest + * modification allowing to transform the first sequence into the second one, or + * as giving the longest sequence which is a subsequence of both initial + * sequences. The {@code equals} method is used to compare objects, so any + * object can be put into sequences. Modifications include deleting, inserting + * or keeping one object, starting from the beginning of the first sequence. + * </p> + * <p> + * This class implements the comparison algorithm, which is the very efficient + * algorithm from Eugene W. Myers + * <a href="http://www.cis.upenn.edu/~bcpierce/courses/dd/papers/diff.ps"> + * An O(ND) Difference Algorithm and Its Variations</a>. + */ +public class LinesComparator { + + private final List<String> left; + private final List<String> right; + private final int[] vDown; + private final int[] vUp; + + private LinesComparator(List<String> left, List<String> right) { + this.left = left; + this.right = right; + + int size = left.size() + right.size() + 2; + vDown = new int[size]; + vUp = new int[size]; + } + + private void buildScript(int start1, int end1, int start2, int end2, List<Pair<LineOperation, String>> result) { + Snake middle = getMiddleSnake(start1, end1, start2, end2); + + if (middle == null + || middle.start == end1 && middle.diag == end1 - end2 + || middle.end == start1 && middle.diag == start1 - start2) { + + int i = start1; + int j = start2; + while (i < end1 || j < end2) { + if (i < end1 && j < end2 && left.get(i).equals(right.get(j))) { + result.add(new Pair<>(LineOperation.keep, left.get(i))); + ++i; + ++j; + } else { + if (end1 - start1 > end2 - start2) { + result.add(new Pair<>(LineOperation.delete, left.get(i))); + ++i; + } else { + result.add(new Pair<>(LineOperation.insert, right.get(j))); + ++j; + } + } + } + + } else { + buildScript(start1, middle.start, start2, middle.start - middle.diag, result); + for (int i = middle.start; i < middle.end; ++i) { + result.add(new Pair<>(LineOperation.keep, left.get(i))); + } + buildScript(middle.end, end1, middle.end - middle.diag, end2, result); + } + } + + private Snake buildSnake(final int start, final int diag, final int end1, final int end2) { + int end = start; + while (end - diag < end2 && end < end1 && left.get(end).equals(right.get(end - diag))) { + ++end; + } + return new Snake(start, end, diag); + } + + private Snake getMiddleSnake(final int start1, final int end1, final int start2, final int end2) { + final int m = end1 - start1; + final int n = end2 - start2; + if (m == 0 || n == 0) { + return null; + } + + final int delta = m - n; + final int sum = n + m; + final int offset = (sum % 2 == 0 ? sum : sum + 1) / 2; + vDown[1 + offset] = start1; + vUp[1 + offset] = end1 + 1; + + for (int d = 0; d <= offset; ++d) { + // Down + for (int k = -d; k <= d; k += 2) { + // First step + + final int i = k + offset; + if (k == -d || k != d && vDown[i - 1] < vDown[i + 1]) { + vDown[i] = vDown[i + 1]; + } else { + vDown[i] = vDown[i - 1] + 1; + } + + int x = vDown[i]; + int y = x - start1 + start2 - k; + + while (x < end1 && y < end2 && left.get(x).equals(right.get(y))) { + vDown[i] = ++x; + ++y; + } + // Second step + if (delta % 2 != 0 && delta - d <= k && k <= delta + d) { + if (vUp[i - delta] <= vDown[i]) { // NOPMD + return buildSnake(vUp[i - delta], k + start1 - start2, end1, end2); + } + } + } + + // Up + for (int k = delta - d; k <= delta + d; k += 2) { + // First step + final int i = k + offset - delta; + if (k == delta - d || k != delta + d && vUp[i + 1] <= vUp[i - 1]) { + vUp[i] = vUp[i + 1] - 1; + } else { + vUp[i] = vUp[i - 1]; + } + + int x = vUp[i] - 1; + int y = x - start1 + start2 - k; + while (x >= start1 && y >= start2 && left.get(x).equals(right.get(y))) { + vUp[i] = x--; + y--; + } + // Second step + if (delta % 2 == 0 && -d <= k && k <= d) { + if (vUp[i] <= vDown[i + delta]) { // NOPMD + return buildSnake(vUp[i], k + start1 - start2, end1, end2); + } + } + } + } + + // this should not happen + throw new RuntimeException("Internal Error"); + } + + private static class Snake { + private final int start; + private final int end; + private final int diag; + + private Snake(int start, int end, int diag) { + this.start = start; + this.end = end; + this.diag = diag; + } + } + + private enum LineOperation { + keep(" "), delete("- "), insert("+ "); + private final String prefix; + LineOperation(String prefix) { + this.prefix = prefix; + } + } + + /** @return line-based diff in unified format. Empty contents are identical. */ + public static Optional<String> diff(List<String> left, List<String> right) { + List<Pair<LineOperation, String>> changes = new ArrayList<>(Math.max(left.size(), right.size())); + new LinesComparator(left, right).buildScript(0, left.size(), 0, right.size(), changes); + + // After we have a list of keep, delete, insert for each line from left and right input, generate a unified + // diff by printing all delete and insert operations with contextLines of keep lines before and after. + // Make sure the change windows are non-overlapping by continuously growing the window + int contextLines = 3; + List<int[]> changeWindows = new ArrayList<>(); + int[] last = null; + for (int i = 0, leftIndex = 0, rightIndex = 0; i < changes.size(); i++) { + if (changes.get(i).getFirst() == LineOperation.keep) { + leftIndex++; + rightIndex++; + continue; + } + + // We found a new change and it is too far away from the previous change to be combined into the same window + if (last == null || i - last[1] > contextLines) { + last = new int[]{Math.max(i - contextLines, 0), Math.min(i + contextLines + 1, changes.size()), Math.max(leftIndex - contextLines, 0), Math.max(rightIndex - contextLines, 0)}; + changeWindows.add(last); + } else // otherwise, extend the previous change window + last[1] = Math.min(i + contextLines + 1, changes.size()); + + if (changes.get(i).getFirst() == LineOperation.delete) leftIndex++; + else rightIndex++; + } + if (changeWindows.isEmpty()) return Optional.empty(); + + StringBuilder sb = new StringBuilder(); + for (int[] changeWindow: changeWindows) { + int start = changeWindow[0], end = changeWindow[1], leftIndex = changeWindow[2], rightIndex = changeWindow[3]; + Map<LineOperation, Long> counts = IntStream.range(start, end) + .mapToObj(i -> changes.get(i).getFirst()) + .collect(Collectors.groupingBy(i -> i, Collectors.counting())); + sb.append("@@ -").append(leftIndex + 1).append(',').append(end - start - counts.getOrDefault(LineOperation.insert, 0L)) + .append(" +").append(rightIndex + 1).append(',').append(end - start - counts.getOrDefault(LineOperation.delete, 0L)).append(" @@\n"); + for (int i = start; i < end; i++) + sb.append(changes.get(i).getFirst().prefix).append(changes.get(i).getSecond()).append('\n'); + } + return Optional.of(sb.toString()); + } +} diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ZipStreamReader.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ZipStreamReader.java index d350d441fa4..7ddd0af7a7a 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ZipStreamReader.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ZipStreamReader.java @@ -10,6 +10,7 @@ import java.nio.file.Path; import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.Optional; import java.util.function.Predicate; import java.util.zip.ZipEntry; import java.util.zip.ZipInputStream; @@ -23,16 +24,15 @@ public class ZipStreamReader { private final List<ZipEntryWithContent> entries = new ArrayList<>(); private final int maxEntrySizeInBytes; - public ZipStreamReader(InputStream input, Predicate<String> entryNameMatcher, int maxEntrySizeInBytes) { + public ZipStreamReader(InputStream input, Predicate<String> entryNameMatcher, int maxEntrySizeInBytes, boolean throwIfEntryExceedsMaxSize) { this.maxEntrySizeInBytes = maxEntrySizeInBytes; try (ZipInputStream zipInput = new ZipInputStream(input)) { ZipEntry zipEntry; while (null != (zipEntry = zipInput.getNextEntry())) { if (!entryNameMatcher.test(requireName(zipEntry.getName()))) continue; - entries.add(new ZipEntryWithContent(zipEntry, readContent(zipInput))); + entries.add(readContent(zipEntry, zipInput, throwIfEntryExceedsMaxSize)); } - } catch (IOException e) { throw new UncheckedIOException("IO error reading zip content", e); } @@ -59,7 +59,7 @@ public class ZipStreamReader { } } - private byte[] readContent(ZipInputStream zipInput) { + private ZipEntryWithContent readContent(ZipEntry zipEntry, ZipInputStream zipInput, boolean throwIfEntryExceedsMaxSize) { try (ByteArrayOutputStream bis = new ByteArrayOutputStream()) { byte[] buffer = new byte[2048]; int read; @@ -67,12 +67,15 @@ public class ZipStreamReader { while ( -1 != (read = zipInput.read(buffer))) { size += read; if (size > maxEntrySizeInBytes) { - throw new IllegalArgumentException("Entry in zip content exceeded size limit of " + - maxEntrySizeInBytes + " bytes"); - } - bis.write(buffer, 0, read); + if (throwIfEntryExceedsMaxSize) throw new IllegalArgumentException( + "Entry in zip content exceeded size limit of " + maxEntrySizeInBytes + " bytes"); + } else bis.write(buffer, 0, read); } - return bis.toByteArray(); + + boolean hasContent = size <= maxEntrySizeInBytes; + return new ZipEntryWithContent(zipEntry, + Optional.of(bis).filter(__ -> hasContent).map(ByteArrayOutputStream::toByteArray), + size); } catch (IOException e) { throw new UncheckedIOException("Failed reading from zipped content", e); } @@ -96,16 +99,19 @@ public class ZipStreamReader { public static class ZipEntryWithContent { private final ZipEntry zipEntry; - private final byte[] content; + private final Optional<byte[]> content; + private final long size; - public ZipEntryWithContent(ZipEntry zipEntry, byte[] content) { + public ZipEntryWithContent(ZipEntry zipEntry, Optional<byte[]> content, long size) { this.zipEntry = zipEntry; this.content = content; + this.size = size; } public ZipEntry zipEntry() { return zipEntry; } - public byte[] content() { return content; } - + public byte[] contentOrThrow() { return content.orElseThrow(); } + public Optional<byte[]> content() { return content; } + public long size() { return size; } } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackageDiffTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackageDiffTest.java new file mode 100644 index 00000000000..101ca42761d --- /dev/null +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackageDiffTest.java @@ -0,0 +1,128 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.controller.application.pkg; + +import org.junit.Test; + +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.UncheckedIOException; +import java.util.Map; +import java.util.zip.Deflater; +import java.util.zip.ZipEntry; +import java.util.zip.ZipOutputStream; + +import static com.yahoo.vespa.hosted.controller.application.pkg.ApplicationPackageDiff.diff; + +import static com.yahoo.vespa.hosted.controller.application.pkg.ApplicationPackageDiff.diffAgainstEmpty; +import static java.nio.charset.StandardCharsets.UTF_8; +import static org.junit.Assert.assertEquals; + +/** + * @author freva + */ +public class ApplicationPackageDiffTest { + private static final ApplicationPackage app1 = applicationPackage(Map.of("file1", "contents of the\nfirst file", "dir/myfile", "Second file", "dir/binary", "øøøø")); + private static final ApplicationPackage app2 = applicationPackage(Map.of("file1", "updated contents\nof the\nfirst file\nafter some changes", "dir/myfile2", "Second file", "dir/binary", "øøøø")); + + @Test + public void no_diff() { + assertEquals("No diff\n", diff(app1, app1)); + } + + @Test + public void diff_against_empty() { + assertEquals("--- dir/binary\n" + + "Diff skipped: File is binary (new file -> 8B)\n" + + "\n" + + "--- dir/myfile\n" + + "@@ -1,0 +1,1 @@\n" + + "+ Second file\n" + + "\n" + + "--- file1\n" + + "@@ -1,0 +1,2 @@\n" + + "+ contents of the\n" + + "+ first file\n" + + "\n", diffAgainstEmpty(app1)); + } + + @Test + public void full_diff() { + // Even though dir/binary is binary file, we can see they are identical, so it should not print "Diff skipped" + assertEquals("--- dir/myfile\n" + + "@@ -1,1 +1,0 @@\n" + + "- Second file\n" + + "\n" + + "--- dir/myfile2\n" + + "@@ -1,0 +1,1 @@\n" + + "+ Second file\n" + + "\n" + + "--- file1\n" + + "@@ -1,2 +1,4 @@\n" + + "+ updated contents\n" + + "+ of the\n" + + "- contents of the\n" + + " first file\n" + + "+ after some changes\n" + + "\n", diff(app1, app2)); + } + + @Test + public void skips_diff_for_too_large_files() { + assertEquals("--- dir/myfile\n" + + "@@ -1,1 +1,0 @@\n" + + "- Second file\n" + + "\n" + + "--- dir/myfile2\n" + + "@@ -1,0 +1,1 @@\n" + + "+ Second file\n" + + "\n" + + "--- file1\n" + + "Diff skipped: File too large (26B -> 53B)\n" + + "\n", diff(app1, app2, 12, 1000, 1000)); + } + + @Test + public void skips_diff_if_file_diff_is_too_large() { + assertEquals("--- dir/myfile\n" + + "@@ -1,1 +1,0 @@\n" + + "- Second file\n" + + "\n" + + "--- dir/myfile2\n" + + "@@ -1,0 +1,1 @@\n" + + "+ Second file\n" + + "\n" + + "--- file1\n" + + "Diff skipped: Diff too large (96B)\n" + + "\n", diff(app1, app2, 1000, 50, 1000)); + } + + @Test + public void skips_diff_if_total_diff_is_too_large() { + assertEquals("--- dir/myfile\n" + + "@@ -1,1 +1,0 @@\n" + + "- Second file\n" + + "\n" + + "--- dir/myfile2\n" + + "Diff skipped: Total diff size >20B)\n" + + "\n" + + "--- file1\n" + + "Diff skipped: Total diff size >20B)\n" + + "\n", diff(app1, app2, 1000, 1000, 20)); + } + + private static ApplicationPackage applicationPackage(Map<String, String> files) { + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + try (ZipOutputStream out = new ZipOutputStream(baos)) { + out.setLevel(Deflater.NO_COMPRESSION); // This is for testing purposes so we skip compression for performance + for (Map.Entry<String, String> file : files.entrySet()) { + ZipEntry entry = new ZipEntry(file.getKey()); + out.putNextEntry(entry); + out.write(file.getValue().getBytes(UTF_8)); + out.closeEntry(); + } + } catch (IOException e) { + throw new UncheckedIOException(e); + } + return new ApplicationPackage(baos.toByteArray()); + } +} diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackageTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackageTest.java index 28f14b5580f..75e00e3434c 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackageTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackageTest.java @@ -109,10 +109,10 @@ public class ApplicationPackageTest { } private static Map<String, String> unzip(byte[] zip) { - return new ZipStreamReader(new ByteArrayInputStream(zip), __ -> true, 1 << 10) + return new ZipStreamReader(new ByteArrayInputStream(zip), __ -> true, 1 << 10, true) .entries().stream() .collect(Collectors.toMap(entry -> entry.zipEntry().getName(), - entry -> new String(entry.content(), UTF_8))); + entry -> new String(entry.contentOrThrow(), UTF_8))); } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/pkg/LinesComparatorTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/pkg/LinesComparatorTest.java new file mode 100644 index 00000000000..92137094f62 --- /dev/null +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/pkg/LinesComparatorTest.java @@ -0,0 +1,112 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.controller.application.pkg; + +import org.junit.Test; + +import java.util.Optional; +import java.util.stream.Collectors; + +import static org.junit.Assert.assertEquals; + +public class LinesComparatorTest { + private static final String text1 = "This part of the\n" + + "document has stayed the\n" + + "same from version to\n" + + "version. It shouldn't\n" + + "be shown if it doesn't\n" + + "change. Otherwise, that\n" + + "would not be helping to\n" + + "compress the size of the\n" + + "changes.\n" + + "\n" + + "This paragraph contains\n" + + "text that is outdated.\n" + + "It will be deleted in the\n" + + "near future.\n" + + "\n" + + "It is important to spell\n" + + "check this dokument. On\n" + + "the other hand, a\n" + + "misspelled word isn't\n" + + "the end of the world.\n" + + "Nothing in the rest of\n" + + "this paragraph needs to\n" + + "be changed. Things can\n" + + "be added after it."; + private static final String text2 = "This is an important\n" + + "notice! It should\n" + + "therefore be located at\n" + + "the beginning of this\n" + + "document!\n" + + "\n" + + "This part of the\n" + + "document has stayed the\n" + + "same from version to\n" + + "version. It shouldn't\n" + + "be shown if it doesn't\n" + + "change. Otherwise, that\n" + + "would not be helping to\n" + + "compress the size of the\n" + + "changes.\n" + + "\n" + + "It is important to spell\n" + + "check this document. On\n" + + "the other hand, a\n" + + "misspelled word isn't\n" + + "the end of the world.\n" + + "Nothing in the rest of\n" + + "this paragraph needs to\n" + + "be changed. Things can\n" + + "be added after it.\n" + + "\n" + + "This paragraph contains\n" + + "important new additions\n" + + "to this document."; + + @Test + public void diff_test() { + assertDiff(null, "", ""); + assertDiff(null, text1, text1); + assertDiff(text1.lines().map(line -> "- " + line).collect(Collectors.joining("\n", "@@ -1,24 +1,0 @@\n", "\n")), text1, ""); + assertDiff(text1.lines().map(line -> "+ " + line).collect(Collectors.joining("\n", "@@ -1,0 +1,24 @@\n", "\n")), "", text1); + assertDiff("@@ -1,3 +1,9 @@\n" + + "+ This is an important\n" + + "+ notice! It should\n" + + "+ therefore be located at\n" + + "+ the beginning of this\n" + + "+ document!\n" + + "+ \n" + + " This part of the\n" + + " document has stayed the\n" + + " same from version to\n" + + "@@ -7,14 +13,9 @@\n" + + " would not be helping to\n" + + " compress the size of the\n" + + " changes.\n" + + "- \n" + + "- This paragraph contains\n" + + "- text that is outdated.\n" + + "- It will be deleted in the\n" + + "- near future.\n" + + " \n" + + " It is important to spell\n" + + "+ check this document. On\n" + + "- check this dokument. On\n" + + " the other hand, a\n" + + " misspelled word isn't\n" + + " the end of the world.\n" + + "@@ -22,3 +23,7 @@\n" + + " this paragraph needs to\n" + + " be changed. Things can\n" + + " be added after it.\n" + + "+ \n" + + "+ This paragraph contains\n" + + "+ important new additions\n" + + "+ to this document.\n", text1, text2); + } + + private static void assertDiff(String expected, String left, String right) { + assertEquals(Optional.ofNullable(expected), + LinesComparator.diff(left.lines().collect(Collectors.toList()), right.lines().collect(Collectors.toList()))); + } +}
\ No newline at end of file diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/pkg/ZipStreamReaderTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/pkg/ZipStreamReaderTest.java index de0c17e9b95..afbd232f01c 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/pkg/ZipStreamReaderTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/pkg/ZipStreamReaderTest.java @@ -38,15 +38,15 @@ public class ZipStreamReaderTest { public void test_size_limit() { Map<String, String> entries = Map.of("foo.xml", "foobar"); try { - new ZipStreamReader(new ByteArrayInputStream(zip(entries)), "foo.xml"::equals, 1); + new ZipStreamReader(new ByteArrayInputStream(zip(entries)), "foo.xml"::equals, 1, true); fail("Expected exception"); } catch (IllegalArgumentException ignored) {} entries = Map.of("foo.xml", "foobar", "foo.jar", "0".repeat(100) // File not extracted and thus not subject to size limit ); - ZipStreamReader reader = new ZipStreamReader(new ByteArrayInputStream(zip(entries)), "foo.xml"::equals,10); - byte[] extracted = reader.entries().get(0).content(); + ZipStreamReader reader = new ZipStreamReader(new ByteArrayInputStream(zip(entries)), "foo.xml"::equals, 10, true); + byte[] extracted = reader.entries().get(0).contentOrThrow(); assertEquals("foobar", new String(extracted, StandardCharsets.UTF_8)); } @@ -65,7 +65,7 @@ public class ZipStreamReaderTest { ); tests.forEach((name, expectException) -> { try { - new ZipStreamReader(new ByteArrayInputStream(zip(Map.of(name, "foo"))), name::equals, 1024); + new ZipStreamReader(new ByteArrayInputStream(zip(Map.of(name, "foo"))), name::equals, 1024, true); assertFalse("Expected exception for '" + name + "'", expectException); } catch (IllegalArgumentException ignored) { assertTrue("Unexpected exception for '" + name + "'", expectException); |