From 59107607f6ed2e323f1086629ec0596002e7f836 Mon Sep 17 00:00:00 2001 From: Mark Brophy Date: Tue, 26 Nov 2019 14:07:59 +0000 Subject: [PATCH 01/15] Support for lazily serialized values in gRPC Metadata. First add a new a Metadata.BinaryStreamMarshaller interface which serializes to/from instances of InputStream, and a corresponding Key.of() factory method. Values set with this type of key will be kept unserialized internally, alongside a reference to the Marshaller.x A new method InternalMetadata.serializePartial(), returns values which are either byte[] or InputStream, and allows transport-specific handling of lazily-serialized values. For the regular serialize() method, stream-marshalled values will be converted to byte[] via an InputStreams. --- .../main/java/io/grpc/InternalMetadata.java | 16 ++ api/src/main/java/io/grpc/Metadata.java | 267 ++++++++++++++++-- api/src/test/java/io/grpc/MetadataTest.java | 152 +++++++++- 3 files changed, 412 insertions(+), 23 deletions(-) diff --git a/api/src/main/java/io/grpc/InternalMetadata.java b/api/src/main/java/io/grpc/InternalMetadata.java index cfd6fa5c0e6..51fb3a505ab 100644 --- a/api/src/main/java/io/grpc/InternalMetadata.java +++ b/api/src/main/java/io/grpc/InternalMetadata.java @@ -19,6 +19,7 @@ import com.google.common.io.BaseEncoding; import io.grpc.Metadata.AsciiMarshaller; import io.grpc.Metadata.Key; +import java.io.InputStream; import java.nio.charset.Charset; /** @@ -82,4 +83,19 @@ public static byte[][] serialize(Metadata md) { public static int headerCount(Metadata md) { return md.headerCount(); } + + @Internal + public static Object[] serializePartial(Metadata md) { + return md.serializePartial(); + } + + @Internal + public static Metadata.LazyValue parsedValue(Key key, T value) { + return Metadata.LazyValue.create(key, value); + } + + @Internal + public static Metadata newMetadataWithStreamValues(int usedNames, Object[] namesAndValues) { + return new Metadata(usedNames, namesAndValues); + } } diff --git a/api/src/main/java/io/grpc/Metadata.java b/api/src/main/java/io/grpc/Metadata.java index c7559d5d6b0..54802551889 100644 --- a/api/src/main/java/io/grpc/Metadata.java +++ b/api/src/main/java/io/grpc/Metadata.java @@ -23,6 +23,10 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import com.google.common.io.BaseEncoding; +import com.google.common.io.ByteStreams; +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.io.InputStream; import java.nio.ByteBuffer; import java.util.ArrayList; import java.util.Arrays; @@ -118,7 +122,7 @@ public String parseAsciiString(String serialized) { * Constructor called by the transport layer when it receives binary metadata. Metadata will * mutate the passed in array. * - * @param usedNames the number of + * @param usedNames the number of names */ Metadata(int usedNames, byte[]... binaryValues) { assert (binaryValues.length & 1) == 0 : "Odd number of key-value pairs " + binaryValues.length; @@ -126,19 +130,30 @@ public String parseAsciiString(String serialized) { namesAndValues = binaryValues; } - private byte[][] namesAndValues; + /** + * Constructor called by the transport layer when it receives partially-parsed metadata. + * Metadata will mutate the passed in array. + */ + Metadata(int usedNames, Object[] namesAndValues) { + assert (namesAndValues.length & 1) == 0 + : "Odd number of key-value pairs " + namesAndValues.length; + size = usedNames; + this.namesAndValues = namesAndValues; + } + + private Object[] namesAndValues; // The unscaled number of headers present. private int size; private byte[] name(int i) { - return namesAndValues[i * 2]; + return (byte[]) namesAndValues[i * 2]; } private void name(int i, byte[] name) { namesAndValues[i * 2] = name; } - private byte[] value(int i) { + private Object value(int i) { return namesAndValues[i * 2 + 1]; } @@ -146,6 +161,41 @@ private void value(int i, byte[] value) { namesAndValues[i * 2 + 1] = value; } + private void value(int i, Object value) { + if (namesAndValues instanceof byte[][]) { + // Reallocate an array of Object. + expand(cap()); + } + namesAndValues[i * 2 + 1] = value; + } + + private byte[] valueAsBytes(int i) { + Object value = value(i); + if (value instanceof byte[]) { + return (byte[]) value; + } else { + return ((LazyValue) value).toBytes(); + } + } + + private Object valueAsBytesOrStream(int i) { + Object value = value(i); + if (value instanceof byte[]) { + return value; + } else { + return ((LazyValue) value).toStream(); + } + } + + private T valueAsT(int i, Key key) { + Object value = value(i); + if (value instanceof byte[]) { + return key.parseBytes((byte[]) value(i)); + } else { + return ((LazyValue) value(i)).toObject(key); + } + } + private int cap() { return namesAndValues != null ? namesAndValues.length : 0; } @@ -192,7 +242,7 @@ public boolean containsKey(Key key) { public T get(Key key) { for (int i = size - 1; i >= 0; i--) { if (bytesEqual(key.asciiName(), name(i))) { - return key.parseBytes(value(i)); + return valueAsT(i, key); } } return null; @@ -231,7 +281,7 @@ public boolean hasNext() { public T next() { if (hasNext()) { hasNext = false; - return key.parseBytes(value(idx++)); + return valueAsT(idx++, key); } throw new NoSuchElementException(); } @@ -288,7 +338,11 @@ public void put(Key key, T value) { Preconditions.checkNotNull(value, "value"); maybeExpand(); name(size, key.asciiName()); - value(size, key.toBytes(value)); + if (key.serializeToStreams()) { + value(size, LazyValue.create(key, value)); + } else { + value(size, key.toBytes(value)); + } size++; } @@ -300,7 +354,7 @@ private void maybeExpand() { // Expands to exactly the desired capacity. private void expand(int newCapacity) { - byte[][] newNamesAndValues = new byte[newCapacity][]; + Object[] newNamesAndValues = new Object[newCapacity]; if (!isEmpty()) { System.arraycopy(namesAndValues, 0, newNamesAndValues, 0, len()); } @@ -322,8 +376,7 @@ public boolean remove(Key key, T value) { if (!bytesEqual(key.asciiName(), name(i))) { continue; } - @SuppressWarnings("unchecked") - T stored = key.parseBytes(value(i)); + T stored = valueAsT(i, key); if (!value.equals(stored)) { continue; } @@ -333,7 +386,7 @@ public boolean remove(Key key, T value) { System.arraycopy(namesAndValues, readIdx, namesAndValues, writeIdx, readLen); size -= 1; name(size, null); - value(size, null); + value(size, (byte[]) null); return true; } return false; @@ -350,7 +403,7 @@ public Iterable removeAll(Key key) { for (; readIdx < size; readIdx++) { if (bytesEqual(key.asciiName(), name(readIdx))) { ret = ret != null ? ret : new ArrayList(); - ret.add(key.parseBytes(value(readIdx))); + ret.add(valueAsT(readIdx, key)); continue; } name(writeIdx, name(readIdx)); @@ -406,11 +459,36 @@ public void discardAll(Key key) { */ @Nullable byte[][] serialize() { - if (len() == cap()) { - return namesAndValues; - } byte[][] serialized = new byte[len()][]; - System.arraycopy(namesAndValues, 0, serialized, 0, len()); + if (namesAndValues instanceof byte[][]) { + System.arraycopy(namesAndValues, 0, serialized, 0, len()); + } else { + for (int i = 0; i < size; i++) { + serialized[i * 2] = name(i); + serialized[i * 2 + 1] = valueAsBytes(i); + } + } + return serialized; + } + + /** + * Serialize all metadata entries, leaving some values as {@link InputStream}s. + * + *

Produces serialized names and values interleaved. result[i*2] are names, while + * result[i*2+1] are values. + * + *

Names are byte arrays as described according to the {@link #serialize}. Valees + * are either byte arrays or {@link InputStream}s. + * + *

This method is intended for transport use only. + */ + @Nullable + Object[] serializePartial() { + Object[] serialized = new Object[len()]; + for (int i = 0; i < size; i++) { + serialized[i * 2] = name(i); + serialized[i * 2 + 1] = valueAsBytesOrStream(i); + } return serialized; } @@ -467,9 +545,9 @@ public String toString() { String headerName = new String(name(i), US_ASCII); sb.append(headerName).append('='); if (headerName.endsWith(BINARY_HEADER_SUFFIX)) { - sb.append(BASE64_ENCODING_OMIT_PADDING.encode(value(i))); + sb.append(BASE64_ENCODING_OMIT_PADDING.encode(valueAsBytes(i))); } else { - String headerValue = new String(value(i), US_ASCII); + String headerValue = new String(valueAsBytes(i), US_ASCII); sb.append(headerValue); } } @@ -532,6 +610,25 @@ public interface AsciiMarshaller { T parseAsciiString(String serialized); } + /** Marshaller for metadata values that are serialized to an InputStream. */ + public interface BinaryStreamMarshaller { + /** + * Serialize a metadata value to an {@link InputStream}. + * + * @param value to serialize + * @return serialized version of value + */ + InputStream toStream(T value); + + /** + * Parse a serialized metadata value from an {@link InputStream}. + * + * @param stream of metadata to parse + * @return a parsed instance of type T + */ + T parseStream(InputStream stream); + } + /** * Key for metadata entries. Allows for parsing and serialization of metadata. * @@ -579,6 +676,16 @@ public static Key of(String name, BinaryMarshaller marshaller) { return new BinaryKey<>(name, marshaller); } + /** + * Creates a key for a binary header, serializing to input streams. + * + * @param name Must contain only the valid key characters as defined in the class comment. Must + * end with {@link #BINARY_HEADER_SUFFIX}. + */ + public static Key of(String name, BinaryStreamMarshaller marshaller) { + return new LazyStreamBinaryKey<>(name, marshaller); + } + /** * Creates a key for an ASCII header. * @@ -601,6 +708,7 @@ static Key of(String name, boolean pseudo, TrustedAsciiMarshaller mars private final String name; private final byte[] nameBytes; + private final Object marshaller; private static BitSet generateValidTChars() { BitSet valid = new BitSet(0x7f); @@ -632,10 +740,11 @@ private static String validateName(String n, boolean pseudo) { return n; } - private Key(String name, boolean pseudo) { + private Key(String name, boolean pseudo, Object marshaller) { this.originalName = checkNotNull(name, "name"); this.name = validateName(this.originalName.toLowerCase(Locale.ROOT), pseudo); this.nameBytes = this.name.getBytes(US_ASCII); + this.marshaller = marshaller; } /** @@ -706,6 +815,26 @@ public String toString() { * @return a parsed instance of type T */ abstract T parseBytes(byte[] serialized); + + /** @return whether this key should be serialized to bytes lazily. */ + boolean serializeToStreams() { + return false; + } + + /** + * Get this keys (implementation-specific) marshaller, or null if the + * marshaller is not of the given time. + * + * @param marshallerClass The type we expect the marshaller to be. + * @return the marshaller object for this key, or null. + */ + @Nullable + final M getMarshaller(Class marshallerClass) { + if (marshallerClass.isInstance(marshaller)) { + return marshallerClass.cast(marshaller); + } + return null; + } } private static class BinaryKey extends Key { @@ -713,7 +842,7 @@ private static class BinaryKey extends Key { /** Keys have a name and a binary marshaller used for serialization. */ private BinaryKey(String name, BinaryMarshaller marshaller) { - super(name, false /* not pseudo */); + super(name, false /* not pseudo */, marshaller); checkArgument( name.endsWith(BINARY_HEADER_SUFFIX), "Binary header is named %s. It must end with %s", @@ -734,12 +863,98 @@ T parseBytes(byte[] serialized) { } } + /** A binary key for values which should be serialized lazily to {@Link InputStream}s. */ + private static class LazyStreamBinaryKey extends Key { + + private final BinaryStreamMarshaller marshaller; + + /** Keys have a name and a stream marshaller used for serialization. */ + private LazyStreamBinaryKey(String name, BinaryStreamMarshaller marshaller) { + super(name, false /* not pseudo */, marshaller); + checkArgument( + name.endsWith(BINARY_HEADER_SUFFIX), + "Binary header is named %s. It must end with %s", + name, + BINARY_HEADER_SUFFIX); + checkArgument(name.length() > BINARY_HEADER_SUFFIX.length(), "empty key name"); + this.marshaller = checkNotNull(marshaller, "marshaller is null"); + } + + @Override + byte[] toBytes(T value) { + return streamToBytes(marshaller.toStream(value)); + } + + @Override + T parseBytes(byte[] serialized) { + return marshaller.parseStream(new ByteArrayInputStream(serialized)); + } + + @Override + boolean serializeToStreams() { + return true; + } + } + + /** Internal holder for values which are serialized/de-serialized lazily. */ + static final class LazyValue { + private final BinaryStreamMarshaller marshaller; + private final T value; + private volatile byte[] serialized; + + static LazyValue create(Key key, T value) { + return new LazyValue<>(checkNotNull(getBinaryStreamMarshaller(key)), value); + } + + /** A value set by the application. */ + private LazyValue(BinaryStreamMarshaller marshaller, T value) { + this.marshaller = marshaller; + this.value = value; + } + + InputStream toStream() { + return marshaller.toStream(value); + } + + byte[] toBytes() { + if (serialized == null) { + synchronized (this) { + if (serialized == null) { + serialized = streamToBytes(toStream()); + } + } + } + return serialized; + } + + @SuppressWarnings({"unchecked", "ReferenceEquality"}) + T2 toObject(Key key) { + if (key.serializeToStreams()) { + BinaryStreamMarshaller marshaller = getBinaryStreamMarshaller(key); + if (marshaller == this.marshaller) { + // The marshallers match so T2 == T. + return (T2) value; + } else { + return marshaller.parseStream(toStream()); + } + } else { + return key.parseBytes(toBytes()); + } + } + + @Nullable + @SuppressWarnings("unchecked") + private static BinaryStreamMarshaller getBinaryStreamMarshaller(Key key) { + return (BinaryStreamMarshaller) key.getMarshaller(BinaryStreamMarshaller.class); + } + } + private static class AsciiKey extends Key { private final AsciiMarshaller marshaller; /** Keys have a name and an ASCII marshaller used for serialization. */ private AsciiKey(String name, boolean pseudo, AsciiMarshaller marshaller) { - super(name, pseudo); + super(name, pseudo, marshaller); Preconditions.checkArgument( !name.endsWith(BINARY_HEADER_SUFFIX), "ASCII header is named %s. Only binary headers may end with %s", @@ -764,7 +979,7 @@ private static final class TrustedAsciiKey extends Key { /** Keys have a name and an ASCII marshaller used for serialization. */ private TrustedAsciiKey(String name, boolean pseudo, TrustedAsciiMarshaller marshaller) { - super(name, pseudo); + super(name, pseudo, marshaller); Preconditions.checkArgument( !name.endsWith(BINARY_HEADER_SUFFIX), "ASCII header is named %s. Only binary headers may end with %s", @@ -808,4 +1023,12 @@ interface TrustedAsciiMarshaller { */ T parseAsciiString(byte[] serialized); } + + private static byte[] streamToBytes(InputStream stream) { + try { + return ByteStreams.toByteArray(stream); + } catch (IOException ioe) { + throw new RuntimeException("failure reading serialized stream", ioe); + } + } } diff --git a/api/src/test/java/io/grpc/MetadataTest.java b/api/src/test/java/io/grpc/MetadataTest.java index e2bdf667ae0..9ae11d47ec4 100644 --- a/api/src/test/java/io/grpc/MetadataTest.java +++ b/api/src/test/java/io/grpc/MetadataTest.java @@ -22,13 +22,19 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertNotSame; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import com.google.common.collect.Lists; +import com.google.common.io.ByteStreams; import io.grpc.Metadata.Key; import io.grpc.internal.GrpcUtil; +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.io.InputStream; import java.util.Arrays; import java.util.Iterator; import java.util.Locale; @@ -59,9 +65,59 @@ public Fish parseBytes(byte[] serialized) { } }; + private static class FishStreamMarsaller implements Metadata.BinaryStreamMarshaller { + @Override + public InputStream toStream(Fish fish) { + return new ByteArrayInputStream(FISH_MARSHALLER.toBytes(fish)); + } + + @Override + public Fish parseStream(InputStream stream) { + try { + return FISH_MARSHALLER.parseBytes(ByteStreams.toByteArray(stream)); + } catch (IOException ioe) { + throw new AssertionError(); + } + } + } + + private static final Metadata.BinaryStreamMarshaller FISH_STREAM_MARSHALLER = + new FishStreamMarsaller(); + + /** A pattern commonly used to avoid unnecessary serialization of immutable objects. */ + private static final class FakeFishStream extends InputStream { + final Fish fish; + + FakeFishStream(Fish fish) { + this.fish = fish; + } + + @Override + public int read() throws IOException { + throw new IOException("Not actually a stream"); + } + } + + private static final Metadata.BinaryStreamMarshaller IMMUTABLE_FISH_MARSHALLER = + new Metadata.BinaryStreamMarshaller() { + @Override + public InputStream toStream(Fish fish) { + return new FakeFishStream(fish); + } + + @Override + public Fish parseStream(InputStream stream) { + return ((FakeFishStream) stream).fish; + } + }; + private static final String LANCE = "lance"; private static final byte[] LANCE_BYTES = LANCE.getBytes(US_ASCII); private static final Metadata.Key KEY = Metadata.Key.of("test-bin", FISH_MARSHALLER); + private static final Metadata.Key KEY_STREAMED = + Key.of("streamed-bin", FISH_STREAM_MARSHALLER); + private static final Metadata.Key KEY_IMMUTABLE = + Key.of("immutable-bin", IMMUTABLE_FISH_MARSHALLER); @Test public void noPseudoHeaders() { @@ -334,6 +390,96 @@ public void invalidKeyName() { } } + @Test + public void getSameKeyReturnsSameInstnce() { + Fish salmon = new Fish("salmon"); + Metadata h = new Metadata(); + h.put(KEY_STREAMED, salmon); + assertSame(salmon, h.get(KEY_STREAMED)); + } + + @Test + public void getSameMarshallerReturnsSameInstnce() { + Fish salmon = new Fish("salmon"); + Metadata h = new Metadata(); + h.put(KEY_STREAMED, salmon); + + // Get using a different key instance (but the same marshaller). + Fish fish = h.get(copyKey(KEY_STREAMED, FISH_STREAM_MARSHALLER)); + assertSame(salmon, fish); + } + + @Test + public void getDifferentMarshallerReturnsParsedInstance() { + Metadata h = new Metadata(); + Fish salmon = new Fish("salmon"); + h.put(KEY_STREAMED, salmon); + + // Get using a different marshaller instance. + Fish fish = h.get(copyKey(KEY_STREAMED, new FishStreamMarsaller())); + assertEquals(salmon, fish); + assertNotSame(salmon, fish); + } + + @Test + public void serializeParseMetadataWithStreams() { + Metadata h = new Metadata(); + Fish salmon = new Fish("salmon"); + h.put(KEY_STREAMED, salmon); + + Metadata parsed = new Metadata(h.serialize()); + assertEquals(salmon, parsed.get(KEY_STREAMED)); + } + + @Test + public void immutableMarshaller() { + Metadata h = new Metadata(KEY.asciiName(), LANCE_BYTES); + Fish salmon = new Fish("salmon"); + h.put(KEY_IMMUTABLE, salmon); + assertSame(salmon, h.get(KEY_IMMUTABLE)); + // Even though the key differs, the marshaller can chose to avoid serialization. + assertSame(salmon, h.get(copyKey(KEY_IMMUTABLE, IMMUTABLE_FISH_MARSHALLER))); + } + + @Test + public void partialSerialization() { + Metadata h = new Metadata(KEY.asciiName(), LANCE_BYTES); + Fish salmon = new Fish("salmon"); + h.put(KEY_STREAMED, salmon); + h.put(KEY_IMMUTABLE, salmon); + + Object[] serialized = InternalMetadata.serializePartial(h); + assertEquals(6, serialized.length); + assertEquals("test-bin", new String((byte[]) serialized[0], US_ASCII)); + assertArrayEquals(LANCE_BYTES, (byte[]) serialized[1]); + + assertEquals("streamed-bin", new String((byte[]) serialized[2], US_ASCII)); + assertEquals(salmon, FISH_STREAM_MARSHALLER.parseStream((InputStream) serialized[3])); + assertNotSame(salmon, FISH_STREAM_MARSHALLER.parseStream((InputStream) serialized[3])); + + assertEquals("immutable-bin", new String((byte[]) serialized[4], US_ASCII)); + assertSame(salmon, IMMUTABLE_FISH_MARSHALLER.parseStream((InputStream) serialized[5])); + } + + @Test + public void createFromPartial() { + Metadata h = new Metadata(KEY.asciiName(), LANCE_BYTES); + Fish salmon = new Fish("salmon"); + h.put(KEY_STREAMED, salmon); + h.put(KEY_IMMUTABLE, salmon); + + Fish anotherSalmon = new Fish("salmon"); + + Object[] partial = InternalMetadata.serializePartial(h); + partial[3] = InternalMetadata.parsedValue(KEY_STREAMED, anotherSalmon); + partial[5] = InternalMetadata.parsedValue(KEY_IMMUTABLE, anotherSalmon); + + Metadata h2 = new Metadata(3, partial); + assertEquals(new Fish(LANCE), h2.get(KEY)); + assertSame(anotherSalmon, h2.get(KEY_STREAMED)); + assertSame(anotherSalmon, h2.get(KEY_IMMUTABLE)); + } + private static final class Fish { private String name; @@ -358,7 +504,7 @@ public boolean equals(Object o) { @Override public int hashCode() { - return name.hashCode(); + return super.hashCode(); } @Override @@ -366,4 +512,8 @@ public String toString() { return "Fish(" + name + ")"; } } + + private static Key copyKey(Key key, Metadata.BinaryStreamMarshaller marshaller) { + return Key.of(key.originalName(), marshaller); + } } From 34cc46c4545bba114a89820706adc15967ea2c1c Mon Sep 17 00:00:00 2001 From: Mark Brophy Date: Tue, 26 Nov 2019 14:07:59 +0000 Subject: [PATCH 02/15] Support for lazily serialized values in gRPC Metadata. First add a new a Metadata.BinaryStreamMarshaller interface which serializes to/from instances of InputStream, and a corresponding Key.of() factory method. Values set with this type of key will be kept unserialized internally, alongside a reference to the Marshaller.x A new method InternalMetadata.serializePartial(), returns values which are either byte[] or InputStream, and allows transport-specific handling of lazily-serialized values. For the regular serialize() method, stream-marshalled values will be converted to byte[] via an InputStreams. --- .../main/java/io/grpc/InternalMetadata.java | 15 + api/src/main/java/io/grpc/Metadata.java | 267 ++++++++++++++++-- api/src/test/java/io/grpc/MetadataTest.java | 150 ++++++++++ 3 files changed, 410 insertions(+), 22 deletions(-) diff --git a/api/src/main/java/io/grpc/InternalMetadata.java b/api/src/main/java/io/grpc/InternalMetadata.java index cfd6fa5c0e6..3753bce16d3 100644 --- a/api/src/main/java/io/grpc/InternalMetadata.java +++ b/api/src/main/java/io/grpc/InternalMetadata.java @@ -82,4 +82,19 @@ public static byte[][] serialize(Metadata md) { public static int headerCount(Metadata md) { return md.headerCount(); } + + @Internal + public static Object[] serializePartial(Metadata md) { + return md.serializePartial(); + } + + @Internal + public static Metadata.LazyValue parsedValue(Key key, T value) { + return Metadata.LazyValue.create(key, value); + } + + @Internal + public static Metadata newMetadataWithStreamValues(int usedNames, Object[] namesAndValues) { + return new Metadata(usedNames, namesAndValues); + } } diff --git a/api/src/main/java/io/grpc/Metadata.java b/api/src/main/java/io/grpc/Metadata.java index c7559d5d6b0..54802551889 100644 --- a/api/src/main/java/io/grpc/Metadata.java +++ b/api/src/main/java/io/grpc/Metadata.java @@ -23,6 +23,10 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import com.google.common.io.BaseEncoding; +import com.google.common.io.ByteStreams; +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.io.InputStream; import java.nio.ByteBuffer; import java.util.ArrayList; import java.util.Arrays; @@ -118,7 +122,7 @@ public String parseAsciiString(String serialized) { * Constructor called by the transport layer when it receives binary metadata. Metadata will * mutate the passed in array. * - * @param usedNames the number of + * @param usedNames the number of names */ Metadata(int usedNames, byte[]... binaryValues) { assert (binaryValues.length & 1) == 0 : "Odd number of key-value pairs " + binaryValues.length; @@ -126,19 +130,30 @@ public String parseAsciiString(String serialized) { namesAndValues = binaryValues; } - private byte[][] namesAndValues; + /** + * Constructor called by the transport layer when it receives partially-parsed metadata. + * Metadata will mutate the passed in array. + */ + Metadata(int usedNames, Object[] namesAndValues) { + assert (namesAndValues.length & 1) == 0 + : "Odd number of key-value pairs " + namesAndValues.length; + size = usedNames; + this.namesAndValues = namesAndValues; + } + + private Object[] namesAndValues; // The unscaled number of headers present. private int size; private byte[] name(int i) { - return namesAndValues[i * 2]; + return (byte[]) namesAndValues[i * 2]; } private void name(int i, byte[] name) { namesAndValues[i * 2] = name; } - private byte[] value(int i) { + private Object value(int i) { return namesAndValues[i * 2 + 1]; } @@ -146,6 +161,41 @@ private void value(int i, byte[] value) { namesAndValues[i * 2 + 1] = value; } + private void value(int i, Object value) { + if (namesAndValues instanceof byte[][]) { + // Reallocate an array of Object. + expand(cap()); + } + namesAndValues[i * 2 + 1] = value; + } + + private byte[] valueAsBytes(int i) { + Object value = value(i); + if (value instanceof byte[]) { + return (byte[]) value; + } else { + return ((LazyValue) value).toBytes(); + } + } + + private Object valueAsBytesOrStream(int i) { + Object value = value(i); + if (value instanceof byte[]) { + return value; + } else { + return ((LazyValue) value).toStream(); + } + } + + private T valueAsT(int i, Key key) { + Object value = value(i); + if (value instanceof byte[]) { + return key.parseBytes((byte[]) value(i)); + } else { + return ((LazyValue) value(i)).toObject(key); + } + } + private int cap() { return namesAndValues != null ? namesAndValues.length : 0; } @@ -192,7 +242,7 @@ public boolean containsKey(Key key) { public T get(Key key) { for (int i = size - 1; i >= 0; i--) { if (bytesEqual(key.asciiName(), name(i))) { - return key.parseBytes(value(i)); + return valueAsT(i, key); } } return null; @@ -231,7 +281,7 @@ public boolean hasNext() { public T next() { if (hasNext()) { hasNext = false; - return key.parseBytes(value(idx++)); + return valueAsT(idx++, key); } throw new NoSuchElementException(); } @@ -288,7 +338,11 @@ public void put(Key key, T value) { Preconditions.checkNotNull(value, "value"); maybeExpand(); name(size, key.asciiName()); - value(size, key.toBytes(value)); + if (key.serializeToStreams()) { + value(size, LazyValue.create(key, value)); + } else { + value(size, key.toBytes(value)); + } size++; } @@ -300,7 +354,7 @@ private void maybeExpand() { // Expands to exactly the desired capacity. private void expand(int newCapacity) { - byte[][] newNamesAndValues = new byte[newCapacity][]; + Object[] newNamesAndValues = new Object[newCapacity]; if (!isEmpty()) { System.arraycopy(namesAndValues, 0, newNamesAndValues, 0, len()); } @@ -322,8 +376,7 @@ public boolean remove(Key key, T value) { if (!bytesEqual(key.asciiName(), name(i))) { continue; } - @SuppressWarnings("unchecked") - T stored = key.parseBytes(value(i)); + T stored = valueAsT(i, key); if (!value.equals(stored)) { continue; } @@ -333,7 +386,7 @@ public boolean remove(Key key, T value) { System.arraycopy(namesAndValues, readIdx, namesAndValues, writeIdx, readLen); size -= 1; name(size, null); - value(size, null); + value(size, (byte[]) null); return true; } return false; @@ -350,7 +403,7 @@ public Iterable removeAll(Key key) { for (; readIdx < size; readIdx++) { if (bytesEqual(key.asciiName(), name(readIdx))) { ret = ret != null ? ret : new ArrayList(); - ret.add(key.parseBytes(value(readIdx))); + ret.add(valueAsT(readIdx, key)); continue; } name(writeIdx, name(readIdx)); @@ -406,11 +459,36 @@ public void discardAll(Key key) { */ @Nullable byte[][] serialize() { - if (len() == cap()) { - return namesAndValues; - } byte[][] serialized = new byte[len()][]; - System.arraycopy(namesAndValues, 0, serialized, 0, len()); + if (namesAndValues instanceof byte[][]) { + System.arraycopy(namesAndValues, 0, serialized, 0, len()); + } else { + for (int i = 0; i < size; i++) { + serialized[i * 2] = name(i); + serialized[i * 2 + 1] = valueAsBytes(i); + } + } + return serialized; + } + + /** + * Serialize all metadata entries, leaving some values as {@link InputStream}s. + * + *

Produces serialized names and values interleaved. result[i*2] are names, while + * result[i*2+1] are values. + * + *

Names are byte arrays as described according to the {@link #serialize}. Valees + * are either byte arrays or {@link InputStream}s. + * + *

This method is intended for transport use only. + */ + @Nullable + Object[] serializePartial() { + Object[] serialized = new Object[len()]; + for (int i = 0; i < size; i++) { + serialized[i * 2] = name(i); + serialized[i * 2 + 1] = valueAsBytesOrStream(i); + } return serialized; } @@ -467,9 +545,9 @@ public String toString() { String headerName = new String(name(i), US_ASCII); sb.append(headerName).append('='); if (headerName.endsWith(BINARY_HEADER_SUFFIX)) { - sb.append(BASE64_ENCODING_OMIT_PADDING.encode(value(i))); + sb.append(BASE64_ENCODING_OMIT_PADDING.encode(valueAsBytes(i))); } else { - String headerValue = new String(value(i), US_ASCII); + String headerValue = new String(valueAsBytes(i), US_ASCII); sb.append(headerValue); } } @@ -532,6 +610,25 @@ public interface AsciiMarshaller { T parseAsciiString(String serialized); } + /** Marshaller for metadata values that are serialized to an InputStream. */ + public interface BinaryStreamMarshaller { + /** + * Serialize a metadata value to an {@link InputStream}. + * + * @param value to serialize + * @return serialized version of value + */ + InputStream toStream(T value); + + /** + * Parse a serialized metadata value from an {@link InputStream}. + * + * @param stream of metadata to parse + * @return a parsed instance of type T + */ + T parseStream(InputStream stream); + } + /** * Key for metadata entries. Allows for parsing and serialization of metadata. * @@ -579,6 +676,16 @@ public static Key of(String name, BinaryMarshaller marshaller) { return new BinaryKey<>(name, marshaller); } + /** + * Creates a key for a binary header, serializing to input streams. + * + * @param name Must contain only the valid key characters as defined in the class comment. Must + * end with {@link #BINARY_HEADER_SUFFIX}. + */ + public static Key of(String name, BinaryStreamMarshaller marshaller) { + return new LazyStreamBinaryKey<>(name, marshaller); + } + /** * Creates a key for an ASCII header. * @@ -601,6 +708,7 @@ static Key of(String name, boolean pseudo, TrustedAsciiMarshaller mars private final String name; private final byte[] nameBytes; + private final Object marshaller; private static BitSet generateValidTChars() { BitSet valid = new BitSet(0x7f); @@ -632,10 +740,11 @@ private static String validateName(String n, boolean pseudo) { return n; } - private Key(String name, boolean pseudo) { + private Key(String name, boolean pseudo, Object marshaller) { this.originalName = checkNotNull(name, "name"); this.name = validateName(this.originalName.toLowerCase(Locale.ROOT), pseudo); this.nameBytes = this.name.getBytes(US_ASCII); + this.marshaller = marshaller; } /** @@ -706,6 +815,26 @@ public String toString() { * @return a parsed instance of type T */ abstract T parseBytes(byte[] serialized); + + /** @return whether this key should be serialized to bytes lazily. */ + boolean serializeToStreams() { + return false; + } + + /** + * Get this keys (implementation-specific) marshaller, or null if the + * marshaller is not of the given time. + * + * @param marshallerClass The type we expect the marshaller to be. + * @return the marshaller object for this key, or null. + */ + @Nullable + final M getMarshaller(Class marshallerClass) { + if (marshallerClass.isInstance(marshaller)) { + return marshallerClass.cast(marshaller); + } + return null; + } } private static class BinaryKey extends Key { @@ -713,7 +842,7 @@ private static class BinaryKey extends Key { /** Keys have a name and a binary marshaller used for serialization. */ private BinaryKey(String name, BinaryMarshaller marshaller) { - super(name, false /* not pseudo */); + super(name, false /* not pseudo */, marshaller); checkArgument( name.endsWith(BINARY_HEADER_SUFFIX), "Binary header is named %s. It must end with %s", @@ -734,12 +863,98 @@ T parseBytes(byte[] serialized) { } } + /** A binary key for values which should be serialized lazily to {@Link InputStream}s. */ + private static class LazyStreamBinaryKey extends Key { + + private final BinaryStreamMarshaller marshaller; + + /** Keys have a name and a stream marshaller used for serialization. */ + private LazyStreamBinaryKey(String name, BinaryStreamMarshaller marshaller) { + super(name, false /* not pseudo */, marshaller); + checkArgument( + name.endsWith(BINARY_HEADER_SUFFIX), + "Binary header is named %s. It must end with %s", + name, + BINARY_HEADER_SUFFIX); + checkArgument(name.length() > BINARY_HEADER_SUFFIX.length(), "empty key name"); + this.marshaller = checkNotNull(marshaller, "marshaller is null"); + } + + @Override + byte[] toBytes(T value) { + return streamToBytes(marshaller.toStream(value)); + } + + @Override + T parseBytes(byte[] serialized) { + return marshaller.parseStream(new ByteArrayInputStream(serialized)); + } + + @Override + boolean serializeToStreams() { + return true; + } + } + + /** Internal holder for values which are serialized/de-serialized lazily. */ + static final class LazyValue { + private final BinaryStreamMarshaller marshaller; + private final T value; + private volatile byte[] serialized; + + static LazyValue create(Key key, T value) { + return new LazyValue<>(checkNotNull(getBinaryStreamMarshaller(key)), value); + } + + /** A value set by the application. */ + private LazyValue(BinaryStreamMarshaller marshaller, T value) { + this.marshaller = marshaller; + this.value = value; + } + + InputStream toStream() { + return marshaller.toStream(value); + } + + byte[] toBytes() { + if (serialized == null) { + synchronized (this) { + if (serialized == null) { + serialized = streamToBytes(toStream()); + } + } + } + return serialized; + } + + @SuppressWarnings({"unchecked", "ReferenceEquality"}) + T2 toObject(Key key) { + if (key.serializeToStreams()) { + BinaryStreamMarshaller marshaller = getBinaryStreamMarshaller(key); + if (marshaller == this.marshaller) { + // The marshallers match so T2 == T. + return (T2) value; + } else { + return marshaller.parseStream(toStream()); + } + } else { + return key.parseBytes(toBytes()); + } + } + + @Nullable + @SuppressWarnings("unchecked") + private static BinaryStreamMarshaller getBinaryStreamMarshaller(Key key) { + return (BinaryStreamMarshaller) key.getMarshaller(BinaryStreamMarshaller.class); + } + } + private static class AsciiKey extends Key { private final AsciiMarshaller marshaller; /** Keys have a name and an ASCII marshaller used for serialization. */ private AsciiKey(String name, boolean pseudo, AsciiMarshaller marshaller) { - super(name, pseudo); + super(name, pseudo, marshaller); Preconditions.checkArgument( !name.endsWith(BINARY_HEADER_SUFFIX), "ASCII header is named %s. Only binary headers may end with %s", @@ -764,7 +979,7 @@ private static final class TrustedAsciiKey extends Key { /** Keys have a name and an ASCII marshaller used for serialization. */ private TrustedAsciiKey(String name, boolean pseudo, TrustedAsciiMarshaller marshaller) { - super(name, pseudo); + super(name, pseudo, marshaller); Preconditions.checkArgument( !name.endsWith(BINARY_HEADER_SUFFIX), "ASCII header is named %s. Only binary headers may end with %s", @@ -808,4 +1023,12 @@ interface TrustedAsciiMarshaller { */ T parseAsciiString(byte[] serialized); } + + private static byte[] streamToBytes(InputStream stream) { + try { + return ByteStreams.toByteArray(stream); + } catch (IOException ioe) { + throw new RuntimeException("failure reading serialized stream", ioe); + } + } } diff --git a/api/src/test/java/io/grpc/MetadataTest.java b/api/src/test/java/io/grpc/MetadataTest.java index e2bdf667ae0..89199a20dee 100644 --- a/api/src/test/java/io/grpc/MetadataTest.java +++ b/api/src/test/java/io/grpc/MetadataTest.java @@ -22,13 +22,19 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertNotSame; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import com.google.common.collect.Lists; +import com.google.common.io.ByteStreams; import io.grpc.Metadata.Key; import io.grpc.internal.GrpcUtil; +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.io.InputStream; import java.util.Arrays; import java.util.Iterator; import java.util.Locale; @@ -59,9 +65,59 @@ public Fish parseBytes(byte[] serialized) { } }; + private static class FishStreamMarsaller implements Metadata.BinaryStreamMarshaller { + @Override + public InputStream toStream(Fish fish) { + return new ByteArrayInputStream(FISH_MARSHALLER.toBytes(fish)); + } + + @Override + public Fish parseStream(InputStream stream) { + try { + return FISH_MARSHALLER.parseBytes(ByteStreams.toByteArray(stream)); + } catch (IOException ioe) { + throw new AssertionError(); + } + } + } + + private static final Metadata.BinaryStreamMarshaller FISH_STREAM_MARSHALLER = + new FishStreamMarsaller(); + + /** A pattern commonly used to avoid unnecessary serialization of immutable objects. */ + private static final class FakeFishStream extends InputStream { + final Fish fish; + + FakeFishStream(Fish fish) { + this.fish = fish; + } + + @Override + public int read() throws IOException { + throw new IOException("Not actually a stream"); + } + } + + private static final Metadata.BinaryStreamMarshaller IMMUTABLE_FISH_MARSHALLER = + new Metadata.BinaryStreamMarshaller() { + @Override + public InputStream toStream(Fish fish) { + return new FakeFishStream(fish); + } + + @Override + public Fish parseStream(InputStream stream) { + return ((FakeFishStream) stream).fish; + } + }; + private static final String LANCE = "lance"; private static final byte[] LANCE_BYTES = LANCE.getBytes(US_ASCII); private static final Metadata.Key KEY = Metadata.Key.of("test-bin", FISH_MARSHALLER); + private static final Metadata.Key KEY_STREAMED = + Key.of("streamed-bin", FISH_STREAM_MARSHALLER); + private static final Metadata.Key KEY_IMMUTABLE = + Key.of("immutable-bin", IMMUTABLE_FISH_MARSHALLER); @Test public void noPseudoHeaders() { @@ -334,6 +390,96 @@ public void invalidKeyName() { } } + @Test + public void getSameKeyReturnsSameInstnce() { + Fish salmon = new Fish("salmon"); + Metadata h = new Metadata(); + h.put(KEY_STREAMED, salmon); + assertSame(salmon, h.get(KEY_STREAMED)); + } + + @Test + public void getSameMarshallerReturnsSameInstnce() { + Fish salmon = new Fish("salmon"); + Metadata h = new Metadata(); + h.put(KEY_STREAMED, salmon); + + // Get using a different key instance (but the same marshaller). + Fish fish = h.get(copyKey(KEY_STREAMED, FISH_STREAM_MARSHALLER)); + assertSame(salmon, fish); + } + + @Test + public void getDifferentMarshallerReturnsParsedInstance() { + Metadata h = new Metadata(); + Fish salmon = new Fish("salmon"); + h.put(KEY_STREAMED, salmon); + + // Get using a different marshaller instance. + Fish fish = h.get(copyKey(KEY_STREAMED, new FishStreamMarsaller())); + assertEquals(salmon, fish); + assertNotSame(salmon, fish); + } + + @Test + public void serializeParseMetadataWithStreams() { + Metadata h = new Metadata(); + Fish salmon = new Fish("salmon"); + h.put(KEY_STREAMED, salmon); + + Metadata parsed = new Metadata(h.serialize()); + assertEquals(salmon, parsed.get(KEY_STREAMED)); + } + + @Test + public void immutableMarshaller() { + Metadata h = new Metadata(KEY.asciiName(), LANCE_BYTES); + Fish salmon = new Fish("salmon"); + h.put(KEY_IMMUTABLE, salmon); + assertSame(salmon, h.get(KEY_IMMUTABLE)); + // Even though the key differs, the marshaller can chose to avoid serialization. + assertSame(salmon, h.get(copyKey(KEY_IMMUTABLE, IMMUTABLE_FISH_MARSHALLER))); + } + + @Test + public void partialSerialization() { + Metadata h = new Metadata(KEY.asciiName(), LANCE_BYTES); + Fish salmon = new Fish("salmon"); + h.put(KEY_STREAMED, salmon); + h.put(KEY_IMMUTABLE, salmon); + + Object[] serialized = InternalMetadata.serializePartial(h); + assertEquals(6, serialized.length); + assertEquals("test-bin", new String((byte[]) serialized[0], US_ASCII)); + assertArrayEquals(LANCE_BYTES, (byte[]) serialized[1]); + + assertEquals("streamed-bin", new String((byte[]) serialized[2], US_ASCII)); + assertEquals(salmon, FISH_STREAM_MARSHALLER.parseStream((InputStream) serialized[3])); + assertNotSame(salmon, FISH_STREAM_MARSHALLER.parseStream((InputStream) serialized[3])); + + assertEquals("immutable-bin", new String((byte[]) serialized[4], US_ASCII)); + assertSame(salmon, IMMUTABLE_FISH_MARSHALLER.parseStream((InputStream) serialized[5])); + } + + @Test + public void createFromPartial() { + Metadata h = new Metadata(KEY.asciiName(), LANCE_BYTES); + Fish salmon = new Fish("salmon"); + h.put(KEY_STREAMED, salmon); + h.put(KEY_IMMUTABLE, salmon); + + Fish anotherSalmon = new Fish("salmon"); + + Object[] partial = InternalMetadata.serializePartial(h); + partial[3] = InternalMetadata.parsedValue(KEY_STREAMED, anotherSalmon); + partial[5] = InternalMetadata.parsedValue(KEY_IMMUTABLE, anotherSalmon); + + Metadata h2 = new Metadata(3, partial); + assertEquals(new Fish(LANCE), h2.get(KEY)); + assertSame(anotherSalmon, h2.get(KEY_STREAMED)); + assertSame(anotherSalmon, h2.get(KEY_IMMUTABLE)); + } + private static final class Fish { private String name; @@ -366,4 +512,8 @@ public String toString() { return "Fish(" + name + ")"; } } + + private static Key copyKey(Key key, Metadata.BinaryStreamMarshaller marshaller) { + return Key.of(key.originalName(), marshaller); + } } From ec008fe5da6faa7d634c6f01f6f6b0bd4215303a Mon Sep 17 00:00:00 2001 From: Mark Brophy Date: Tue, 26 Nov 2019 14:24:52 +0000 Subject: [PATCH 03/15] minor cleanup. --- api/src/main/java/io/grpc/InternalMetadata.java | 1 - api/src/test/java/io/grpc/MetadataTest.java | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/api/src/main/java/io/grpc/InternalMetadata.java b/api/src/main/java/io/grpc/InternalMetadata.java index 51fb3a505ab..3753bce16d3 100644 --- a/api/src/main/java/io/grpc/InternalMetadata.java +++ b/api/src/main/java/io/grpc/InternalMetadata.java @@ -19,7 +19,6 @@ import com.google.common.io.BaseEncoding; import io.grpc.Metadata.AsciiMarshaller; import io.grpc.Metadata.Key; -import java.io.InputStream; import java.nio.charset.Charset; /** diff --git a/api/src/test/java/io/grpc/MetadataTest.java b/api/src/test/java/io/grpc/MetadataTest.java index 9ae11d47ec4..89199a20dee 100644 --- a/api/src/test/java/io/grpc/MetadataTest.java +++ b/api/src/test/java/io/grpc/MetadataTest.java @@ -504,7 +504,7 @@ public boolean equals(Object o) { @Override public int hashCode() { - return super.hashCode(); + return name.hashCode(); } @Override From 205eb7a8acd0f24925201f13785abe3bc4e5f122 Mon Sep 17 00:00:00 2001 From: Mark Brophy Date: Tue, 26 Nov 2019 15:04:54 +0000 Subject: [PATCH 04/15] Avoid a possible npe in LazyValue.toObject. --- api/src/main/java/io/grpc/Metadata.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/api/src/main/java/io/grpc/Metadata.java b/api/src/main/java/io/grpc/Metadata.java index 54802551889..acacb62fe4f 100644 --- a/api/src/main/java/io/grpc/Metadata.java +++ b/api/src/main/java/io/grpc/Metadata.java @@ -934,12 +934,11 @@ T2 toObject(Key key) { if (marshaller == this.marshaller) { // The marshallers match so T2 == T. return (T2) value; - } else { + } else if (marshaller != null) { return marshaller.parseStream(toStream()); } - } else { - return key.parseBytes(toBytes()); } + return key.parseBytes(toBytes()); } @Nullable From 87c40f3990238611bfaa4ec5241ccf24c6c563d6 Mon Sep 17 00:00:00 2001 From: Mark Brophy Date: Tue, 26 Nov 2019 15:59:56 +0000 Subject: [PATCH 05/15] fix comment. --- api/src/main/java/io/grpc/Metadata.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/api/src/main/java/io/grpc/Metadata.java b/api/src/main/java/io/grpc/Metadata.java index acacb62fe4f..dd918d7694a 100644 --- a/api/src/main/java/io/grpc/Metadata.java +++ b/api/src/main/java/io/grpc/Metadata.java @@ -816,7 +816,9 @@ public String toString() { */ abstract T parseBytes(byte[] serialized); - /** @return whether this key should be serialized to bytes lazily. */ + /** + * @return whether this key should be serialized to bytes lazily. + */ boolean serializeToStreams() { return false; } From 685884092f2c83a5954075d3aecdb51fbeb3c9f9 Mon Sep 17 00:00:00 2001 From: Mark Brophy Date: Tue, 26 Nov 2019 17:25:43 +0000 Subject: [PATCH 06/15] Rename new InternalMetadata method. --- api/src/main/java/io/grpc/InternalMetadata.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/src/main/java/io/grpc/InternalMetadata.java b/api/src/main/java/io/grpc/InternalMetadata.java index 3753bce16d3..d8734f9a9c9 100644 --- a/api/src/main/java/io/grpc/InternalMetadata.java +++ b/api/src/main/java/io/grpc/InternalMetadata.java @@ -94,7 +94,7 @@ public static Metadata.LazyValue parsedValue(Key key, T value) { } @Internal - public static Metadata newMetadataWithStreamValues(int usedNames, Object[] namesAndValues) { + public static Metadata newMetadataWithParsedValues(int usedNames, Object[] namesAndValues) { return new Metadata(usedNames, namesAndValues); } } From 5ed8cf787ad9da6c3be1b5b5cd7db3dea5a25064 Mon Sep 17 00:00:00 2001 From: Mark Brophy Date: Wed, 27 Nov 2019 14:28:47 +0000 Subject: [PATCH 07/15] Fix comment. --- api/src/main/java/io/grpc/Metadata.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/src/main/java/io/grpc/Metadata.java b/api/src/main/java/io/grpc/Metadata.java index dd918d7694a..0267f56db03 100644 --- a/api/src/main/java/io/grpc/Metadata.java +++ b/api/src/main/java/io/grpc/Metadata.java @@ -477,8 +477,8 @@ byte[][] serialize() { *

Produces serialized names and values interleaved. result[i*2] are names, while * result[i*2+1] are values. * - *

Names are byte arrays as described according to the {@link #serialize}. Valees - * are either byte arrays or {@link InputStream}s. + *

Names are byte arrays as described according to the {@link #serialize} + * method. Values are either byte arrays or {@link InputStream}s. * *

This method is intended for transport use only. */ From 8d151c77382faad62fa82c84477ce6b698298cb2 Mon Sep 17 00:00:00 2001 From: Mark Brophy Date: Thu, 28 Nov 2019 11:21:53 +0000 Subject: [PATCH 08/15] Update from review. Relax test guarantees for streamed values, and return Object from InternalMetadata.parsedValue --- api/src/main/java/io/grpc/InternalMetadata.java | 2 +- api/src/test/java/io/grpc/MetadataTest.java | 13 ++++++------- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/api/src/main/java/io/grpc/InternalMetadata.java b/api/src/main/java/io/grpc/InternalMetadata.java index d8734f9a9c9..6499c8d42a6 100644 --- a/api/src/main/java/io/grpc/InternalMetadata.java +++ b/api/src/main/java/io/grpc/InternalMetadata.java @@ -89,7 +89,7 @@ public static Object[] serializePartial(Metadata md) { } @Internal - public static Metadata.LazyValue parsedValue(Key key, T value) { + public static Object parsedValue(Key key, T value) { return Metadata.LazyValue.create(key, value); } diff --git a/api/src/test/java/io/grpc/MetadataTest.java b/api/src/test/java/io/grpc/MetadataTest.java index 89199a20dee..d771264a5db 100644 --- a/api/src/test/java/io/grpc/MetadataTest.java +++ b/api/src/test/java/io/grpc/MetadataTest.java @@ -391,34 +391,33 @@ public void invalidKeyName() { } @Test - public void getSameKeyReturnsSameInstnce() { + public void streamedValue() { Fish salmon = new Fish("salmon"); Metadata h = new Metadata(); h.put(KEY_STREAMED, salmon); - assertSame(salmon, h.get(KEY_STREAMED)); + assertEquals(salmon, h.get(KEY_STREAMED)); } @Test - public void getSameMarshallerReturnsSameInstnce() { + public void streamedValueDifferentKey() { Fish salmon = new Fish("salmon"); Metadata h = new Metadata(); h.put(KEY_STREAMED, salmon); // Get using a different key instance (but the same marshaller). Fish fish = h.get(copyKey(KEY_STREAMED, FISH_STREAM_MARSHALLER)); - assertSame(salmon, fish); + assertEquals(salmon, fish); } @Test - public void getDifferentMarshallerReturnsParsedInstance() { - Metadata h = new Metadata(); + public void streamedValueDifferentMarshaller() { Fish salmon = new Fish("salmon"); + Metadata h = new Metadata(); h.put(KEY_STREAMED, salmon); // Get using a different marshaller instance. Fish fish = h.get(copyKey(KEY_STREAMED, new FishStreamMarsaller())); assertEquals(salmon, fish); - assertNotSame(salmon, fish); } @Test From 84cd07bef8953d8dcfa2c1f4b74ff4e5d559a6bb Mon Sep 17 00:00:00 2001 From: Mark Brophy Date: Wed, 4 Dec 2019 11:33:56 +0000 Subject: [PATCH 09/15] Updates from review. --- .../main/java/io/grpc/InternalMetadata.java | 27 +++++++++++++++++++ api/src/main/java/io/grpc/Metadata.java | 24 ++++++++--------- 2 files changed, 38 insertions(+), 13 deletions(-) diff --git a/api/src/main/java/io/grpc/InternalMetadata.java b/api/src/main/java/io/grpc/InternalMetadata.java index 6499c8d42a6..1cef18f4814 100644 --- a/api/src/main/java/io/grpc/InternalMetadata.java +++ b/api/src/main/java/io/grpc/InternalMetadata.java @@ -83,16 +83,43 @@ public static int headerCount(Metadata md) { return md.headerCount(); } + /** + * Serializes all metadata entries, leaving some values as {@link InputStream}s. + * + *

Produces serialized names and values interleaved. result[i*2] are names, while + * result[i*2+1] are values. + * + *

Names are byte arrays as described according to the {@link Metadata#serialize} + * method. Values are either byte arrays or {@link InputStream}s. + */ @Internal public static Object[] serializePartial(Metadata md) { return md.serializePartial(); } + /** + * Creates a holder for a pre-parsed value read by the transport. + * + * @param key The key associated with this value. + * @param value The value to store. + * @return an object holding the pre-parsed value for this key. + */ @Internal public static Object parsedValue(Key key, T value) { return Metadata.LazyValue.create(key, value); } + /** + * Creates a new {@link Metadata} instance from serialized data, + * with some values pre-parsed. Metadata will mutate the passed in array. + * + * @param usedNames The number of names used. + * @param namesAndValues An array of iterleaved names and values, + * with each name (at even indices) represented as a byte array, + * and each value (at odd indices) represented as either a byte + * array or an object returned by the {@link #parsedValue} + * method. + */ @Internal public static Metadata newMetadataWithParsedValues(int usedNames, Object[] namesAndValues) { return new Metadata(usedNames, namesAndValues); diff --git a/api/src/main/java/io/grpc/Metadata.java b/api/src/main/java/io/grpc/Metadata.java index 0267f56db03..80e69cdba84 100644 --- a/api/src/main/java/io/grpc/Metadata.java +++ b/api/src/main/java/io/grpc/Metadata.java @@ -125,9 +125,7 @@ public String parseAsciiString(String serialized) { * @param usedNames the number of names */ Metadata(int usedNames, byte[]... binaryValues) { - assert (binaryValues.length & 1) == 0 : "Odd number of key-value pairs " + binaryValues.length; - size = usedNames; - namesAndValues = binaryValues; + this(usedNames, (Object[]) binaryValues); } /** @@ -338,7 +336,7 @@ public void put(Key key, T value) { Preconditions.checkNotNull(value, "value"); maybeExpand(); name(size, key.asciiName()); - if (key.serializeToStreams()) { + if (key.serializesToStreams()) { value(size, LazyValue.create(key, value)); } else { value(size, key.toBytes(value)); @@ -472,7 +470,7 @@ byte[][] serialize() { } /** - * Serialize all metadata entries, leaving some values as {@link InputStream}s. + * Serializes all metadata entries, leaving some values as {@link InputStream}s. * *

Produces serialized names and values interleaved. result[i*2] are names, while * result[i*2+1] are values. @@ -613,7 +611,7 @@ public interface AsciiMarshaller { /** Marshaller for metadata values that are serialized to an InputStream. */ public interface BinaryStreamMarshaller { /** - * Serialize a metadata value to an {@link InputStream}. + * Serializes a metadata value to an {@link InputStream}. * * @param value to serialize * @return serialized version of value @@ -621,7 +619,7 @@ public interface BinaryStreamMarshaller { InputStream toStream(T value); /** - * Parse a serialized metadata value from an {@link InputStream}. + * Parses a serialized metadata value from an {@link InputStream}. * * @param stream of metadata to parse * @return a parsed instance of type T @@ -817,15 +815,15 @@ public String toString() { abstract T parseBytes(byte[] serialized); /** - * @return whether this key should be serialized to bytes lazily. + * @return whether this key will be serialized to bytes lazily. */ - boolean serializeToStreams() { + boolean serializesToStreams() { return false; } /** - * Get this keys (implementation-specific) marshaller, or null if the - * marshaller is not of the given time. + * Gets this keys (implementation-specific) marshaller, or null if the + * marshaller is not of the given type. * * @param marshallerClass The type we expect the marshaller to be. * @return the marshaller object for this key, or null. @@ -893,7 +891,7 @@ T parseBytes(byte[] serialized) { } @Override - boolean serializeToStreams() { + boolean serializesToStreams() { return true; } } @@ -931,7 +929,7 @@ byte[] toBytes() { @SuppressWarnings({"unchecked", "ReferenceEquality"}) T2 toObject(Key key) { - if (key.serializeToStreams()) { + if (key.serializesToStreams()) { BinaryStreamMarshaller marshaller = getBinaryStreamMarshaller(key); if (marshaller == this.marshaller) { // The marshallers match so T2 == T. From 377dacdd1abf8b35581ed0622c07d895fc5912b5 Mon Sep 17 00:00:00 2001 From: Mark Brophy Date: Wed, 4 Dec 2019 11:53:47 +0000 Subject: [PATCH 10/15] fix comment. --- api/src/main/java/io/grpc/InternalMetadata.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/api/src/main/java/io/grpc/InternalMetadata.java b/api/src/main/java/io/grpc/InternalMetadata.java index 1cef18f4814..4391e465be5 100644 --- a/api/src/main/java/io/grpc/InternalMetadata.java +++ b/api/src/main/java/io/grpc/InternalMetadata.java @@ -115,10 +115,10 @@ public static Object parsedValue(Key key, T value) { * * @param usedNames The number of names used. * @param namesAndValues An array of iterleaved names and values, - * with each name (at even indices) represented as a byte array, - * and each value (at odd indices) represented as either a byte - * array or an object returned by the {@link #parsedValue} - * method. + * with each name (at even indices) represented as a byte array, + * and each value (at odd indices) represented as either a byte + * array or an object returned by the {@link #parsedValue} + * method. */ @Internal public static Metadata newMetadataWithParsedValues(int usedNames, Object[] namesAndValues) { From b4c7a79d2c8f9b5fdfcd2d30e61d90203ef56508 Mon Sep 17 00:00:00 2001 From: Mark Brophy Date: Thu, 12 Dec 2019 11:57:48 +0000 Subject: [PATCH 11/15] Update from review. --- api/src/main/java/io/grpc/InternalMetadata.java | 2 +- api/src/main/java/io/grpc/Metadata.java | 14 ++++++++------ api/src/test/java/io/grpc/MetadataTest.java | 2 +- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/api/src/main/java/io/grpc/InternalMetadata.java b/api/src/main/java/io/grpc/InternalMetadata.java index 4391e465be5..7e5a5038982 100644 --- a/api/src/main/java/io/grpc/InternalMetadata.java +++ b/api/src/main/java/io/grpc/InternalMetadata.java @@ -114,7 +114,7 @@ public static Object parsedValue(Key key, T value) { * with some values pre-parsed. Metadata will mutate the passed in array. * * @param usedNames The number of names used. - * @param namesAndValues An array of iterleaved names and values, + * @param namesAndValues An array of interleaved names and values, * with each name (at even indices) represented as a byte array, * and each value (at odd indices) represented as either a byte * array or an object returned by the {@link #parsedValue} diff --git a/api/src/main/java/io/grpc/Metadata.java b/api/src/main/java/io/grpc/Metadata.java index 80e69cdba84..8b1ea003542 100644 --- a/api/src/main/java/io/grpc/Metadata.java +++ b/api/src/main/java/io/grpc/Metadata.java @@ -131,6 +131,11 @@ public String parseAsciiString(String serialized) { /** * Constructor called by the transport layer when it receives partially-parsed metadata. * Metadata will mutate the passed in array. + * + * @param usedNames the number of names + * @param namesAndValues an array of interleaved names and values, with each name + * (at even indices) represented by a byte array, and values (at odd indices) as + * described by {@link InternalMetadata#newMetadataWithParsedValues}. */ Metadata(int usedNames, Object[] namesAndValues) { assert (namesAndValues.length & 1) == 0 @@ -188,9 +193,9 @@ private Object valueAsBytesOrStream(int i) { private T valueAsT(int i, Key key) { Object value = value(i); if (value instanceof byte[]) { - return key.parseBytes((byte[]) value(i)); + return key.parseBytes((byte[]) value); } else { - return ((LazyValue) value(i)).toObject(key); + return ((LazyValue) value).toObject(key); } } @@ -931,10 +936,7 @@ byte[] toBytes() { T2 toObject(Key key) { if (key.serializesToStreams()) { BinaryStreamMarshaller marshaller = getBinaryStreamMarshaller(key); - if (marshaller == this.marshaller) { - // The marshallers match so T2 == T. - return (T2) value; - } else if (marshaller != null) { + if (marshaller != null) { return marshaller.parseStream(toStream()); } } diff --git a/api/src/test/java/io/grpc/MetadataTest.java b/api/src/test/java/io/grpc/MetadataTest.java index d771264a5db..251286b1094 100644 --- a/api/src/test/java/io/grpc/MetadataTest.java +++ b/api/src/test/java/io/grpc/MetadataTest.java @@ -475,7 +475,7 @@ public void createFromPartial() { Metadata h2 = new Metadata(3, partial); assertEquals(new Fish(LANCE), h2.get(KEY)); - assertSame(anotherSalmon, h2.get(KEY_STREAMED)); + assertEquals(anotherSalmon, h2.get(KEY_STREAMED)); assertSame(anotherSalmon, h2.get(KEY_IMMUTABLE)); } From f0e42265207216c733b44917c7fec5b6b1dfe34a Mon Sep 17 00:00:00 2001 From: Mark Brophy Date: Thu, 12 Dec 2019 18:53:14 +0000 Subject: [PATCH 12/15] Pass BinaryStreamMarshaller to InternalMetadata.parseValue After an offline discussion it became clear we can do this now, which we think is preferable. --- api/src/main/java/io/grpc/InternalMetadata.java | 6 +++--- api/src/main/java/io/grpc/Metadata.java | 2 +- api/src/test/java/io/grpc/MetadataTest.java | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/api/src/main/java/io/grpc/InternalMetadata.java b/api/src/main/java/io/grpc/InternalMetadata.java index 7e5a5038982..1f08a8ee935 100644 --- a/api/src/main/java/io/grpc/InternalMetadata.java +++ b/api/src/main/java/io/grpc/InternalMetadata.java @@ -100,13 +100,13 @@ public static Object[] serializePartial(Metadata md) { /** * Creates a holder for a pre-parsed value read by the transport. * - * @param key The key associated with this value. + * @param marshaller The {@link Metadata#BinaryStreamMarshaller} associated with this value. * @param value The value to store. * @return an object holding the pre-parsed value for this key. */ @Internal - public static Object parsedValue(Key key, T value) { - return Metadata.LazyValue.create(key, value); + public static Object parsedValue(BinaryStreamMarshaller marshaller, T value) { + return new Metadata.LazyValue(marshaller, value); } /** diff --git a/api/src/main/java/io/grpc/Metadata.java b/api/src/main/java/io/grpc/Metadata.java index 8b1ea003542..dec8edb6fec 100644 --- a/api/src/main/java/io/grpc/Metadata.java +++ b/api/src/main/java/io/grpc/Metadata.java @@ -912,7 +912,7 @@ static LazyValue create(Key key, T value) { } /** A value set by the application. */ - private LazyValue(BinaryStreamMarshaller marshaller, T value) { + LazyValue(BinaryStreamMarshaller marshaller, T value) { this.marshaller = marshaller; this.value = value; } diff --git a/api/src/test/java/io/grpc/MetadataTest.java b/api/src/test/java/io/grpc/MetadataTest.java index 251286b1094..c9095a82d5a 100644 --- a/api/src/test/java/io/grpc/MetadataTest.java +++ b/api/src/test/java/io/grpc/MetadataTest.java @@ -470,8 +470,8 @@ public void createFromPartial() { Fish anotherSalmon = new Fish("salmon"); Object[] partial = InternalMetadata.serializePartial(h); - partial[3] = InternalMetadata.parsedValue(KEY_STREAMED, anotherSalmon); - partial[5] = InternalMetadata.parsedValue(KEY_IMMUTABLE, anotherSalmon); + partial[3] = InternalMetadata.parsedValue(FISH_STREAM_MARSHALLER, anotherSalmon); + partial[5] = InternalMetadata.parsedValue(IMMUTABLE_FISH_MARSHALLER, anotherSalmon); Metadata h2 = new Metadata(3, partial); assertEquals(new Fish(LANCE), h2.get(KEY)); From f7a80d1c5e87e21cf07613e20ac1275e50eefa33 Mon Sep 17 00:00:00 2001 From: Mark Brophy Date: Thu, 12 Dec 2019 19:04:37 +0000 Subject: [PATCH 13/15] Fix warning. --- api/src/main/java/io/grpc/InternalMetadata.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/src/main/java/io/grpc/InternalMetadata.java b/api/src/main/java/io/grpc/InternalMetadata.java index 1f08a8ee935..0af1552e670 100644 --- a/api/src/main/java/io/grpc/InternalMetadata.java +++ b/api/src/main/java/io/grpc/InternalMetadata.java @@ -106,7 +106,7 @@ public static Object[] serializePartial(Metadata md) { */ @Internal public static Object parsedValue(BinaryStreamMarshaller marshaller, T value) { - return new Metadata.LazyValue(marshaller, value); + return new Metadata.LazyValue<>(marshaller, value); } /** From 08606e7cc4060903bb0f3441bfb985a76052b8bc Mon Sep 17 00:00:00 2001 From: Mark Brophy Date: Thu, 12 Dec 2019 19:05:38 +0000 Subject: [PATCH 14/15] and fix import.. --- api/src/main/java/io/grpc/InternalMetadata.java | 1 + 1 file changed, 1 insertion(+) diff --git a/api/src/main/java/io/grpc/InternalMetadata.java b/api/src/main/java/io/grpc/InternalMetadata.java index 0af1552e670..d011eb9e8c7 100644 --- a/api/src/main/java/io/grpc/InternalMetadata.java +++ b/api/src/main/java/io/grpc/InternalMetadata.java @@ -18,6 +18,7 @@ import com.google.common.io.BaseEncoding; import io.grpc.Metadata.AsciiMarshaller; +import io.grpc.Metadata.BinaryStreamMarshaller; import io.grpc.Metadata.Key; import java.nio.charset.Charset; From 734183f04247a65dfb3eadf507e9cdad98b05ef2 Mon Sep 17 00:00:00 2001 From: Mark Brophy Date: Thu, 12 Dec 2019 20:56:42 +0000 Subject: [PATCH 15/15] Remove now-unnecessary warning suppression. --- api/src/main/java/io/grpc/Metadata.java | 1 - 1 file changed, 1 deletion(-) diff --git a/api/src/main/java/io/grpc/Metadata.java b/api/src/main/java/io/grpc/Metadata.java index dec8edb6fec..07446155432 100644 --- a/api/src/main/java/io/grpc/Metadata.java +++ b/api/src/main/java/io/grpc/Metadata.java @@ -932,7 +932,6 @@ byte[] toBytes() { return serialized; } - @SuppressWarnings({"unchecked", "ReferenceEquality"}) T2 toObject(Key key) { if (key.serializesToStreams()) { BinaryStreamMarshaller marshaller = getBinaryStreamMarshaller(key);