diff options
author | Jon Skeet <jonskeet@google.com> | 2016-01-15 10:13:56 +0000 |
---|---|---|
committer | Jon Skeet <jonskeet@google.com> | 2016-01-15 10:13:56 +0000 |
commit | f262611ff65d5a5d1b4665e2d339f108ef29fcf9 (patch) | |
tree | 64f71fc6bb7351df375afdecf799ceb23ed228ee /csharp/src/Google.Protobuf/FieldCodec.cs | |
parent | f2fe50bfc516cdab99f51fa2ca90ba0db1ef6637 (diff) | |
download | protobuf-f262611ff65d5a5d1b4665e2d339f108ef29fcf9.tar.gz protobuf-f262611ff65d5a5d1b4665e2d339f108ef29fcf9.tar.bz2 protobuf-f262611ff65d5a5d1b4665e2d339f108ef29fcf9.zip |
Fix handling of repeated wrappers
Previously we were incorrectly packing wrapper types.
This also refactors FieldCodec a bit as well, using more C# 6-ness.
Diffstat (limited to 'csharp/src/Google.Protobuf/FieldCodec.cs')
-rw-r--r-- | csharp/src/Google.Protobuf/FieldCodec.cs | 138 |
1 files changed, 66 insertions, 72 deletions
diff --git a/csharp/src/Google.Protobuf/FieldCodec.cs b/csharp/src/Google.Protobuf/FieldCodec.cs index c2b8d268..98313088 100644 --- a/csharp/src/Google.Protobuf/FieldCodec.cs +++ b/csharp/src/Google.Protobuf/FieldCodec.cs @@ -30,6 +30,7 @@ // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #endregion +using Google.Protobuf.Compatibility; using Google.Protobuf.WellKnownTypes; using System; using System.Collections.Generic; @@ -329,17 +330,24 @@ namespace Google.Protobuf } /// <summary> + /// <para> /// An encode/decode pair for a single field. This effectively encapsulates /// all the information needed to read or write the field value from/to a coded /// stream. + /// </para> + /// <para> + /// This class is public and has to be as it is used by generated code, but its public + /// API is very limited - just what the generated code needs to call directly. + /// </para> /// </summary> /// <remarks> - /// This never writes default values to the stream, and is not currently designed - /// to play well with packed arrays. + /// This never writes default values to the stream, and does not address "packedness" + /// in repeated fields itself, other than to know whether or not the field *should* be packed. /// </remarks> public sealed class FieldCodec<T> { private static readonly T DefaultDefault; + private static readonly bool TypeSupportsPacking = typeof(T).IsValueType() && Nullable.GetUnderlyingType(typeof(T)) == null; static FieldCodec() { @@ -354,75 +362,31 @@ namespace Google.Protobuf // Otherwise it's the default value of the CLR type } - private readonly Func<CodedInputStream, T> reader; - private readonly Action<CodedOutputStream, T> writer; - private readonly Func<T, int> sizeCalculator; - private readonly uint tag; - private readonly int tagSize; - private readonly int fixedSize; - // Default value for this codec. Usually the same for every instance of the same type, but - // for string/ByteString wrapper fields the codec's default value is null, whereas for - // other string/ByteString fields it's "" or ByteString.Empty. - private readonly T defaultValue; + internal static bool IsPackedRepeatedField(uint tag) => + TypeSupportsPacking && WireFormat.GetTagWireType(tag) == WireFormat.WireType.LengthDelimited; - internal FieldCodec( - Func<CodedInputStream, T> reader, - Action<CodedOutputStream, T> writer, - Func<T, int> sizeCalculator, - uint tag) : this(reader, writer, sizeCalculator, tag, DefaultDefault) - { - } - - internal FieldCodec( - Func<CodedInputStream, T> reader, - Action<CodedOutputStream, T> writer, - Func<T, int> sizeCalculator, - uint tag, - T defaultValue) - { - this.reader = reader; - this.writer = writer; - this.sizeCalculator = sizeCalculator; - this.fixedSize = 0; - this.tag = tag; - this.defaultValue = defaultValue; - tagSize = CodedOutputStream.ComputeRawVarint32Size(tag); - } - - internal FieldCodec( - Func<CodedInputStream, T> reader, - Action<CodedOutputStream, T> writer, - int fixedSize, - uint tag) - { - this.reader = reader; - this.writer = writer; - this.sizeCalculator = _ => fixedSize; - this.fixedSize = fixedSize; - this.tag = tag; - tagSize = CodedOutputStream.ComputeRawVarint32Size(tag); - } + internal bool PackedRepeatedField { get; } /// <summary> - /// Returns the size calculator for just a value. + /// Returns a delegate to write a value (unconditionally) to a coded output stream. /// </summary> - internal Func<T, int> ValueSizeCalculator { get { return sizeCalculator; } } + internal Action<CodedOutputStream, T> ValueWriter { get; } /// <summary> - /// Returns a delegate to write a value (unconditionally) to a coded output stream. + /// Returns the size calculator for just a value. /// </summary> - internal Action<CodedOutputStream, T> ValueWriter { get { return writer; } } + internal Func<T, int> ValueSizeCalculator { get; } /// <summary> /// Returns a delegate to read a value from a coded input stream. It is assumed that /// the stream is already positioned on the appropriate tag. /// </summary> - internal Func<CodedInputStream, T> ValueReader { get { return reader; } } + internal Func<CodedInputStream, T> ValueReader { get; } /// <summary> /// Returns the fixed size for an entry, or 0 if sizes vary. /// </summary> - internal int FixedSize { get { return fixedSize; } } + internal int FixedSize { get; } /// <summary> /// Gets the tag of the codec. @@ -430,15 +394,54 @@ namespace Google.Protobuf /// <value> /// The tag of the codec. /// </value> - public uint Tag { get { return tag; } } + internal uint Tag { get; } /// <summary> - /// Gets the default value of the codec's type. + /// Default value for this codec. Usually the same for every instance of the same type, but + /// for string/ByteString wrapper fields the codec's default value is null, whereas for + /// other string/ByteString fields it's "" or ByteString.Empty. /// </summary> /// <value> /// The default value of the codec's type. /// </value> - public T DefaultValue { get { return defaultValue; } } + internal T DefaultValue { get; } + + private readonly int tagSize; + + internal FieldCodec( + Func<CodedInputStream, T> reader, + Action<CodedOutputStream, T> writer, + int fixedSize, + uint tag) : this(reader, writer, _ => fixedSize, tag) + { + FixedSize = fixedSize; + } + + internal FieldCodec( + Func<CodedInputStream, T> reader, + Action<CodedOutputStream, T> writer, + Func<T, int> sizeCalculator, + uint tag) : this(reader, writer, sizeCalculator, tag, DefaultDefault) + { + } + + internal FieldCodec( + Func<CodedInputStream, T> reader, + Action<CodedOutputStream, T> writer, + Func<T, int> sizeCalculator, + uint tag, + T defaultValue) + { + ValueReader = reader; + ValueWriter = writer; + ValueSizeCalculator = sizeCalculator; + FixedSize = 0; + Tag = tag; + DefaultValue = defaultValue; + tagSize = CodedOutputStream.ComputeRawVarint32Size(tag); + // Detect packed-ness once, so we can check for it within RepeatedField<T>. + PackedRepeatedField = IsPackedRepeatedField(tag); + } /// <summary> /// Write a tag and the given value, *if* the value is not the default. @@ -447,8 +450,8 @@ namespace Google.Protobuf { if (!IsDefault(value)) { - output.WriteTag(tag); - writer(output, value); + output.WriteTag(Tag); + ValueWriter(output, value); } } @@ -457,23 +460,14 @@ namespace Google.Protobuf /// </summary> /// <param name="input">The input stream to read from.</param> /// <returns>The value read from the stream.</returns> - public T Read(CodedInputStream input) - { - return reader(input); - } + public T Read(CodedInputStream input) => ValueReader(input); /// <summary> /// Calculates the size required to write the given value, with a tag, /// if the value is not the default. /// </summary> - public int CalculateSizeWithTag(T value) - { - return IsDefault(value) ? 0 : sizeCalculator(value) + tagSize; - } + public int CalculateSizeWithTag(T value) => IsDefault(value) ? 0 : ValueSizeCalculator(value) + tagSize; - private bool IsDefault(T value) - { - return EqualityComparer<T>.Default.Equals(value, defaultValue); - } + private bool IsDefault(T value) => EqualityComparer<T>.Default.Equals(value, DefaultValue); } } |