From c442ebea2db8dcbc153131fcc11536749c328293 Mon Sep 17 00:00:00 2001 From: Matthias Andreas Benkard Date: Sun, 10 Dec 2023 17:58:38 +0100 Subject: Add OSTree encoding roundtrip tests and fix the bugs discovered. Change-Id: I4c81329c5381d6ae843fee5da2bed035941011e3 --- .../java/eu/mulk/jgvariant/ostree/Checksum.java | 14 ++++++++--- .../eu/mulk/jgvariant/ostree/DeltaMetaEntry.java | 4 +-- .../eu/mulk/jgvariant/ostree/DeltaOperation.java | 17 ++++++------- .../eu/mulk/jgvariant/ostree/DeltaPartPayload.java | 2 +- .../eu/mulk/jgvariant/ostree/DeltaSuperblock.java | 29 +--------------------- 5 files changed, 22 insertions(+), 44 deletions(-) (limited to 'jgvariant-ostree/src/main/java/eu') diff --git a/jgvariant-ostree/src/main/java/eu/mulk/jgvariant/ostree/Checksum.java b/jgvariant-ostree/src/main/java/eu/mulk/jgvariant/ostree/Checksum.java index 829664e..e9bdf84 100644 --- a/jgvariant-ostree/src/main/java/eu/mulk/jgvariant/ostree/Checksum.java +++ b/jgvariant-ostree/src/main/java/eu/mulk/jgvariant/ostree/Checksum.java @@ -18,7 +18,11 @@ public record Checksum(ByteString byteString) { private static final int SIZE = 32; private static final Decoder DECODER = - ByteString.decoder().map(Checksum::new, Checksum::byteString); + ByteString.decoder().map(Checksum::new, Checksum::optimizedByteString); + + private static final ByteString NULL_BYTESTRING = new ByteString(new byte[0]); + + private static final Checksum ZERO = new Checksum(new ByteString(new byte[SIZE])); public Checksum { if (byteString.size() == 0) { @@ -47,7 +51,11 @@ public record Checksum(ByteString byteString) { * @return a checksum whose bits are all zero. */ public static Checksum zero() { - return new Checksum(new ByteString(new byte[SIZE])); + return ZERO; + } + + public ByteString optimizedByteString() { + return isEmpty() ? NULL_BYTESTRING : byteString; } /** @@ -56,7 +64,7 @@ public record Checksum(ByteString byteString) { * @return {@code true} if the byte string is equal to {@link #zero()}, {@code false} otherwise. */ public boolean isEmpty() { - return equals(zero()); + return equals(ZERO); } /** diff --git a/jgvariant-ostree/src/main/java/eu/mulk/jgvariant/ostree/DeltaMetaEntry.java b/jgvariant-ostree/src/main/java/eu/mulk/jgvariant/ostree/DeltaMetaEntry.java index 2be0426..71419c6 100644 --- a/jgvariant-ostree/src/main/java/eu/mulk/jgvariant/ostree/DeltaMetaEntry.java +++ b/jgvariant-ostree/src/main/java/eu/mulk/jgvariant/ostree/DeltaMetaEntry.java @@ -67,8 +67,8 @@ public record DeltaMetaEntry( var output = new ByteArrayOutputStream(); for (var deltaObject : deltaObjects) { - output.write(deltaObject.objectType.byteValue()); - output.writeBytes(deltaObject.checksum.byteString().bytes()); + output.write(deltaObject.objectType().byteValue()); + output.writeBytes(deltaObject.checksum().byteString().bytes()); } return output.toByteArray(); diff --git a/jgvariant-ostree/src/main/java/eu/mulk/jgvariant/ostree/DeltaOperation.java b/jgvariant-ostree/src/main/java/eu/mulk/jgvariant/ostree/DeltaOperation.java index d753a48..42b7056 100644 --- a/jgvariant-ostree/src/main/java/eu/mulk/jgvariant/ostree/DeltaOperation.java +++ b/jgvariant-ostree/src/main/java/eu/mulk/jgvariant/ostree/DeltaOperation.java @@ -69,16 +69,16 @@ public sealed interface DeltaOperation { } default void writeTo(ByteArrayOutputStream output) { - if (this instanceof OpenSpliceAndCloseMeta openSpliceAndCloseMeta) { - output.write(DeltaOperationType.OPEN_SPLICE_AND_CLOSE.byteValue()); - writeVarint64(output, openSpliceAndCloseMeta.offset); - writeVarint64(output, openSpliceAndCloseMeta.size); - } else if (this instanceof OpenSpliceAndCloseReal openSpliceAndCloseReal) { + if (this instanceof OpenSpliceAndCloseReal openSpliceAndCloseReal) { output.write(DeltaOperationType.OPEN_SPLICE_AND_CLOSE.byteValue()); writeVarint64(output, openSpliceAndCloseReal.modeOffset); writeVarint64(output, openSpliceAndCloseReal.xattrOffset); writeVarint64(output, openSpliceAndCloseReal.size); writeVarint64(output, openSpliceAndCloseReal.offset); + } else if (this instanceof OpenSpliceAndCloseMeta openSpliceAndCloseMeta) { + output.write(DeltaOperationType.OPEN_SPLICE_AND_CLOSE.byteValue()); + writeVarint64(output, openSpliceAndCloseMeta.size); + writeVarint64(output, openSpliceAndCloseMeta.offset); } else if (this instanceof Open open) { output.write(DeltaOperationType.OPEN.byteValue()); writeVarint64(output, open.modeOffset); @@ -130,16 +130,13 @@ public sealed interface DeltaOperation { * @see #readVarint64 */ private static void writeVarint64(ByteArrayOutputStream output, long value) { - while (value != 0) { + do { byte b = (byte) (value & 0x7F); value >>= 7; if (value != 0) { b |= (byte) 0x80; } output.write(b); - if (value == 0) { - break; - } - } + } while (value != 0); } } diff --git a/jgvariant-ostree/src/main/java/eu/mulk/jgvariant/ostree/DeltaPartPayload.java b/jgvariant-ostree/src/main/java/eu/mulk/jgvariant/ostree/DeltaPartPayload.java index 31c192d..ccd5630 100644 --- a/jgvariant-ostree/src/main/java/eu/mulk/jgvariant/ostree/DeltaPartPayload.java +++ b/jgvariant-ostree/src/main/java/eu/mulk/jgvariant/ostree/DeltaPartPayload.java @@ -156,7 +156,7 @@ public record DeltaPartPayload( Decoder.ofByteArray() .map( bytes -> parseDeltaOperationList(bytes, objectTypes), - deltaOperations -> serializeDeltaOperationList(deltaOperations))) + DeltaPartPayload::serializeDeltaOperationList)) .contramap(DeltaPartPayload::decompress, DeltaPartPayload::compress); } } diff --git a/jgvariant-ostree/src/main/java/eu/mulk/jgvariant/ostree/DeltaSuperblock.java b/jgvariant-ostree/src/main/java/eu/mulk/jgvariant/ostree/DeltaSuperblock.java index 9513fa0..b8143af 100644 --- a/jgvariant-ostree/src/main/java/eu/mulk/jgvariant/ostree/DeltaSuperblock.java +++ b/jgvariant-ostree/src/main/java/eu/mulk/jgvariant/ostree/DeltaSuperblock.java @@ -5,16 +5,11 @@ package eu.mulk.jgvariant.ostree; import eu.mulk.jgvariant.core.Decoder; -import eu.mulk.jgvariant.core.Signature; -import eu.mulk.jgvariant.core.Variant; import java.io.ByteArrayOutputStream; import java.nio.ByteBuffer; import java.nio.ByteOrder; -import java.text.ParseException; import java.util.ArrayList; -import java.util.HashMap; import java.util.List; -import java.util.Map; /** * A static delta. @@ -78,29 +73,7 @@ public record DeltaSuperblock( DeltaSuperblock::parseDeltaNameList, DeltaSuperblock::serializeDeltaNameList), Decoder.ofArray(DeltaMetaEntry.decoder()).withByteOrder(ByteOrder.LITTLE_ENDIAN), Decoder.ofArray(DeltaFallback.decoder()).withByteOrder(ByteOrder.LITTLE_ENDIAN)) - .map(DeltaSuperblock::byteSwappedIfBigEndian, DeltaSuperblock::withSpecifiedByteOrder); - - private DeltaSuperblock withSpecifiedByteOrder() { - Map extendedMetadataMap = new HashMap<>(metadata().fields()); - - try { - extendedMetadataMap.putIfAbsent( - "ostree.endianness", new Variant(Signature.parse("y"), (byte) 'l')); - } catch (ParseException e) { - // impossible - throw new IllegalStateException(e); - } - - return new DeltaSuperblock( - new Metadata(extendedMetadataMap), - timestamp, - fromChecksum, - toChecksum, - commit, - dependencies, - entries, - fallbacks); - } + .map(DeltaSuperblock::byteSwappedIfBigEndian, DeltaSuperblock::byteSwappedIfBigEndian); private DeltaSuperblock byteSwappedIfBigEndian() { // Fix up the endianness of the 'entries' and 'fallbacks' fields, which have -- cgit v1.2.3