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/core/DecoderTest.java | 1 - .../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 +---- .../mulk/jgvariant/ostree/OstreeDecoderTest.java | 117 ++++++++++++++------- 7 files changed, 100 insertions(+), 84 deletions(-) diff --git a/jgvariant-core/src/test/java/eu/mulk/jgvariant/core/DecoderTest.java b/jgvariant-core/src/test/java/eu/mulk/jgvariant/core/DecoderTest.java index d97cf88..d8fa6a5 100644 --- a/jgvariant-core/src/test/java/eu/mulk/jgvariant/core/DecoderTest.java +++ b/jgvariant-core/src/test/java/eu/mulk/jgvariant/core/DecoderTest.java @@ -163,7 +163,6 @@ class DecoderTest { entity.put("hi", -2); entity.put("bye", -1); var roundtripData = decoder.encode(entity); - System.out.println(HexFormat.of().formatHex(roundtripData.array())); assertArrayEquals(data, roundtripData.array()); } 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 diff --git a/jgvariant-ostree/src/test/java/eu/mulk/jgvariant/ostree/OstreeDecoderTest.java b/jgvariant-ostree/src/test/java/eu/mulk/jgvariant/ostree/OstreeDecoderTest.java index 75c5ea4..4465d02 100644 --- a/jgvariant-ostree/src/test/java/eu/mulk/jgvariant/ostree/OstreeDecoderTest.java +++ b/jgvariant-ostree/src/test/java/eu/mulk/jgvariant/ostree/OstreeDecoderTest.java @@ -12,18 +12,16 @@ import com.adelean.inject.resources.junit.jupiter.TestWithResources; import eu.mulk.jgvariant.core.Signature; import eu.mulk.jgvariant.core.Variant; import java.nio.ByteBuffer; -import java.util.Arrays; -import java.util.HexFormat; -import java.util.List; -import java.util.Map; +import java.util.*; import org.junit.jupiter.api.Test; @TestWithResources @SuppressWarnings({ + "DoubleBraceInitialization", "ImmutableListOf1", "ImmutableMapOf1", "initialization.field.uninitialized", - "NullAway" + "NullAway", }) class OstreeDecoderTest { @@ -51,7 +49,8 @@ class OstreeDecoderTest { @Test void summaryDecoder() { var decoder = Summary.decoder(); - var summary = decoder.decode(ByteBuffer.wrap(summaryBytes)); + var input = ByteBuffer.wrap(summaryBytes); + var summary = decoder.decode(input); assertAll( () -> @@ -70,65 +69,102 @@ class OstreeDecoderTest { summary.entries()), () -> assertEquals( - Map.of( - "ostree.summary.last-modified", - new Variant(Signature.parse("t"), 1640537300L), - "ostree.summary.tombstone-commits", - new Variant(Signature.parse("b"), false), - "ostree.static-deltas", - new Variant( - Signature.parse("a{sv}"), - Map.of( - "3d3b3329dca38871f29aeda1bf5854d76c707fa269759a899d0985c91815fe6f-66ff167ff35ce87daac817447a9490a262ee75f095f017716a6eb1a9d9eb3350", - new Variant( - Signature.parse("ay"), - bytesOfHex( - "03738040e28e7662e9c9d2599c530ea974e642c9f87e6c00cbaa39a0cdac8d44")), - "31c8835d5c9d2c6687a50091c85142d1b2d853ff416a9fb81b4ee30754510d52", - new Variant( - Signature.parse("ay"), - bytesOfHex( - "f481144629474bd88c106e45ac405ebd75b324b0655af1aec14b31786ae1fd61")), - "31c8835d5c9d2c6687a50091c85142d1b2d853ff416a9fb81b4ee30754510d52-3d3b3329dca38871f29aeda1bf5854d76c707fa269759a899d0985c91815fe6f", - new Variant( - Signature.parse("ay"), - bytesOfHex( - "2c6a07bc1cf4d7ce7d00f82d7d2d6d156fd0e31d476851b46dc2306b181b064a")))), - "ostree.summary.mode", - new Variant(Signature.parse("s"), "bare"), - "ostree.summary.indexed-deltas", - new Variant(Signature.parse("b"), true)), + new LinkedHashMap() { + { + put("ostree.summary.mode", new Variant(Signature.parse("s"), "bare")); + put( + "ostree.summary.last-modified", + new Variant(Signature.parse("t"), 1640537300L)); + put( + "ostree.summary.tombstone-commits", + new Variant(Signature.parse("b"), false)); + put( + "ostree.static-deltas", + new Variant( + Signature.parse("a{sv}"), + new LinkedHashMap() { + { + put( + "3d3b3329dca38871f29aeda1bf5854d76c707fa269759a899d0985c91815fe6f-66ff167ff35ce87daac817447a9490a262ee75f095f017716a6eb1a9d9eb3350", + new Variant( + Signature.parse("ay"), + bytesOfHex( + "03738040e28e7662e9c9d2599c530ea974e642c9f87e6c00cbaa39a0cdac8d44"))); + put( + "31c8835d5c9d2c6687a50091c85142d1b2d853ff416a9fb81b4ee30754510d52", + new Variant( + Signature.parse("ay"), + bytesOfHex( + "f481144629474bd88c106e45ac405ebd75b324b0655af1aec14b31786ae1fd61"))); + put( + "31c8835d5c9d2c6687a50091c85142d1b2d853ff416a9fb81b4ee30754510d52-3d3b3329dca38871f29aeda1bf5854d76c707fa269759a899d0985c91815fe6f", + new Variant( + Signature.parse("ay"), + bytesOfHex( + "2c6a07bc1cf4d7ce7d00f82d7d2d6d156fd0e31d476851b46dc2306b181b064a"))); + } + })); + put("ostree.summary.indexed-deltas", new Variant(Signature.parse("b"), true)); + } + }, summary.metadata().fields())); + var encoded = decoder.encode(summary); + input.rewind(); + assertEquals(input, encoded); + System.out.println(summary); } @Test void commitDecoder() { var decoder = Commit.decoder(); - var commit = decoder.decode(ByteBuffer.wrap(commitBytes)); + var input = ByteBuffer.wrap(commitBytes); + var commit = decoder.decode(input); + + var encoded = decoder.encode(commit); + input.rewind(); + assertEquals(input, encoded); + System.out.println(commit); } @Test void dirTreeDecoder() { var decoder = DirTree.decoder(); - var dirTree = decoder.decode(ByteBuffer.wrap(dirTreeBytes)); + var input = ByteBuffer.wrap(dirTreeBytes); + var dirTree = decoder.decode(input); + + var encoded = decoder.encode(dirTree); + input.rewind(); + assertEquals(input, encoded); + System.out.println(dirTree); } @Test void dirMetaDecoder() { var decoder = DirMeta.decoder(); + var input = ByteBuffer.wrap(dirMetaBytes); var dirMeta = decoder.decode(ByteBuffer.wrap(dirMetaBytes)); + + var encoded = decoder.encode(dirMeta); + input.rewind(); + assertEquals(input, encoded); + System.out.println(dirMeta); } @Test void superblockDecoder() { var decoder = DeltaSuperblock.decoder(); - var deltaSuperblock = decoder.decode(ByteBuffer.wrap(deltaSuperblockBytes)); + var input = ByteBuffer.wrap(deltaSuperblockBytes); + var deltaSuperblock = decoder.decode(input); System.out.println(deltaSuperblock); + + var encoded = decoder.encode(deltaSuperblock); + input.rewind(); + assertEquals(input, encoded); } @Test @@ -137,9 +173,12 @@ class OstreeDecoderTest { var superblock = superblockDecoder.decode(ByteBuffer.wrap(deltaSuperblockBytes)); var decoder = DeltaPartPayload.decoder(superblock.entries().get(0)); - var deltaPartPayload = decoder.decode(ByteBuffer.wrap(deltaPartPayloadBytes)); + var input = ByteBuffer.wrap(deltaPartPayloadBytes); + var deltaPartPayload = decoder.decode(input); - System.out.println(deltaPartPayload); + var encoded = decoder.encode(deltaPartPayload); + var decodedAgain = decoder.decode(encoded); + assertEquals(deltaPartPayload, decodedAgain); } private static List bytesOfHex(String hex) { -- cgit v1.2.3