From 61e4f1665221626cfd31a5826bff527ef7ce5719 Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Thu, 15 Mar 2018 08:25:43 +0900 Subject: [PATCH] ObjectIdSerializer: Support serialization of known non-null ObjectId The implementation of ObjectIdSerializer, added in change I7599cf8bd, is not equivalent to the original implementation in Gerrit [1]. The Gerrit implementation provides separate methods to (de)serialize instances of ObjectId that are known to be non-null. In these methods, no "marker" is written to the stream. Replacing Gerrit's implementation with ObjectIdSerializer [2] broke persistent caches because it started writing markers where they were not expected [3]. Since ObjectIdSerializer is included in JGit 4.11 we can't change the existing #write and #read methods. Keep those as-is, but extend the Javadoc to clarify that they support possibly null ObjectId instances. Add new methods #writeWithoutMarker and #readWithoutMarker to support the cases where the ObjectId is known to be non-null and the marker should not be written to the serialization stream. Also: - Replace the hard-coded `0` and `1` markers with constants that can be linked from the Javadocs. - Include the marker value in the "Invalid flag before ObjectId" exception message. [1] https://gerrit-review.googlesource.com/c/gerrit/+/9792 [2] https://gerrit-review.googlesource.com/c/gerrit/+/165851 [3] https://gerrit-review.googlesource.com/c/gerrit/+/165952 Change-Id: Iaf84c3ec32ecf83efffb306fdb4940cc85740f3f Signed-off-by: David Pursehouse --- .../jgit/lib/ObjectIdSerializerTest.java | 12 ++- .../eclipse/jgit/lib/ObjectIdSerializer.java | 83 +++++++++++++++---- 2 files changed, 78 insertions(+), 17 deletions(-) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ObjectIdSerializerTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ObjectIdSerializerTest.java index 24bc40eee..d98b792d7 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ObjectIdSerializerTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ObjectIdSerializerTest.java @@ -79,10 +79,18 @@ public class ObjectIdSerializerTest { throws Exception { File file = File.createTempFile("ObjectIdSerializerTest_", ""); try (OutputStream out = new FileOutputStream(file)) { - ObjectIdSerializer.write(out, objectId); + if (objectId == null) { + ObjectIdSerializer.write(out, objectId); + } else { + ObjectIdSerializer.writeWithoutMarker(out, objectId); + } } try (InputStream in = new FileInputStream(file)) { - return ObjectIdSerializer.read(in); + if (objectId == null) { + return ObjectIdSerializer.read(in); + } else { + return ObjectIdSerializer.readWithoutMarker(in); + } } } } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectIdSerializer.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectIdSerializer.java index 59da40830..96c7cee1c 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectIdSerializer.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectIdSerializer.java @@ -51,57 +51,110 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import org.eclipse.jgit.annotations.NonNull; import org.eclipse.jgit.annotations.Nullable; import org.eclipse.jgit.util.IO; /** * Helper to serialize {@link ObjectId} instances. {@link ObjectId} is already - * serializable, but this class provides a more optimal implementation. It - * writes out a flag (0 or 1) followed by the object's words, or nothing if it - * was a null id. + * serializable, but this class provides methods to handle null and non-null + * instances. * * @since 4.11 */ public class ObjectIdSerializer { + /* + * Marker to indicate a null ObjectId instance. + */ + private static final byte NULL_MARKER = 0; + + /* + * Marker to indicate a non-null ObjectId instance. + */ + private static final byte NON_NULL_MARKER = 1; + /** + * Write a possibly null {@link ObjectId} to the stream, using markers to + * differentiate null and non-null instances. + * + *

+ * If the id is non-null, writes a {@link #NON_NULL_MARKER} followed by the + * id's words. If it is null, writes a {@link #NULL_MARKER} and nothing + * else. + * * @param out * the output stream * @param id - * the object id to serialize + * the object id to serialize; may be null * @throws IOException * the stream writing failed */ public static void write(OutputStream out, @Nullable AnyObjectId id) throws IOException { if (id != null) { - out.write((byte) 1); - id.copyRawTo(out); + out.write(NON_NULL_MARKER); + writeWithoutMarker(out, id); } else { - out.write((byte) 0); + out.write(NULL_MARKER); } } /** + * Write a non-null {@link ObjectId} to the stream. + * + * @param out + * the output stream + * @param id + * the object id to serialize; never null + * @throws IOException + * the stream writing failed + */ + public static void writeWithoutMarker(OutputStream out, @NonNull AnyObjectId id) + throws IOException { + id.copyRawTo(out); + } + + /** + * Read a possibly null {@link ObjectId} from the stream. + * + * Reads the first byte of the stream, which is expected to be either + * {@link #NON_NULL_MARKER} or {@link #NULL_MARKER}. + * * @param in * the input stream - * @return the object id + * @return the object id, or null * @throws IOException * there was an error reading the stream */ @Nullable public static ObjectId read(InputStream in) throws IOException { - switch (in.read()) { - case 0: + byte marker = (byte) in.read(); + switch (marker) { + case NULL_MARKER: return null; - case 1: - final byte[] b = new byte[OBJECT_ID_LENGTH]; - IO.readFully(in, b, 0, OBJECT_ID_LENGTH); - return ObjectId.fromRaw(b); + case NON_NULL_MARKER: + return readWithoutMarker(in); default: - throw new IOException("Invalid flag before ObjectId"); //$NON-NLS-1$ + throw new IOException("Invalid flag before ObjectId: " + marker); //$NON-NLS-1$ } } + /** + * Read a non-null {@link ObjectId} from the stream. + * + * @param in + * the input stream + * @return the object id; never null + * @throws IOException + * there was an error reading the stream + */ + @NonNull + public static ObjectId readWithoutMarker(InputStream in) throws IOException { + final byte[] b = new byte[OBJECT_ID_LENGTH]; + IO.readFully(in, b, 0, OBJECT_ID_LENGTH); + return ObjectId.fromRaw(b); + } + private ObjectIdSerializer() { } }