Merge pull request #272 from msgpack/fix/issue-269-2

Fix ext type type error related issues

Author: yfakariya

CHANGES.txt

@@ -741,4 +741,7 @@ Release 1.0.0-rc1 T.B.D.
 
   BUG FIXES
   * Fix NRE in .NET Standard 1.1/1.3 build (this issue got mixed in beta2).
-  * Fix built-in Guid/BigInteger always output raw type even if PackerCompatibilityOptions.PackBinaryAsRaw is not specified. #270
+  * Fix built-in Guid/BigInteger always output raw type even if PackerCompatibilityOptions.PackBinaryAsRaw is not specified. #270
+  * Fix MessagePackObject.UnderlyingType reports wrong type for ext types. Part of #269.
+    This bug also caused misleading error message for incompatible type conversion.
+  * Fix exceptions thrown by MessagePackObject.AsBinary()/AsString() reports internal type name. Part of #269.

src/MsgPack/MessagePackObject.Utilities.cs

@@ -1037,6 +1037,14 @@ public Type UnderlyingType
 					{
 						return asMps.GetUnderlyingType();
 					}
+					else if ( this._handleOrTypeCode is byte[] )
+					{
+						// It should be MPETO
+#if DEBUG
+						Contract.Assert( ( this._value & 0xFFFFFFFFFFFFFF00 ) == 0, "( " + this._value.ToString( "X16" ) + " & 0xFFFFFFFFFFFFFF00 ) != 0" );
+#endif // DEBUG
+						return typeof( MessagePackExtendedTypeObject );
+					}
 					else
 					{
 						return this._handleOrTypeCode.GetType();
@@ -1355,7 +1363,7 @@ public string AsString( Encoding encoding )
 				return null;
 			}
 
-			VerifyUnderlyingType<MessagePackString>( this, null );
+			VerifyUnderlyingRawType<string>( this, null );
 
 			try
 			{
@@ -1395,7 +1403,7 @@ public string AsStringUtf8()
 		/// </remarks>
 		public string AsStringUtf16()
 		{
-			VerifyUnderlyingType<byte[]>( this, null );
+			VerifyUnderlyingRawType<string>( this, null );
 			Contract.EndContractBlock();
 
 			if ( this.IsNil )
@@ -1509,6 +1517,10 @@ public MessagePackObjectDictionary AsDictionary()
 
 		private static void VerifyUnderlyingType<T>( MessagePackObject instance, string parameterName )
 		{
+#if DEBUG
+			Contract.Assert( typeof( T ) != typeof( MessagePackString ), "Should use VerifyUnderlyingRawType()" );
+#endif // DEBUG
+
 			if ( instance.IsNil )
 			{
 				if ( !typeof( T ).GetIsValueType() || Nullable.GetUnderlyingType( typeof( T ) ) != null )
@@ -1540,6 +1552,24 @@ private static void VerifyUnderlyingType<T>( MessagePackObject instance, string
 			}
 		}
 
+		private static void VerifyUnderlyingRawType<T>( MessagePackObject instance, string parameterName )
+		{
+			if ( instance._handleOrTypeCode == null || instance._handleOrTypeCode is MessagePackString )
+			{
+				// nil or MPS (eventually string or byte[])
+				return;
+			}
+
+			if ( parameterName != null )
+			{
+				throw new ArgumentException( String.Format( CultureInfo.CurrentCulture, "Do not convert {0} MessagePackObject to {1}.", instance.UnderlyingType, typeof( T ) ), parameterName );
+			}
+			else
+			{
+				ThrowInvalidTypeAs<T>( instance );
+			}
+		}
+
 		private static void ThrowCannotBeNilAs<T>()
 		{
 			throw new InvalidOperationException( String.Format( CultureInfo.CurrentCulture, "Do not convert nil MessagePackObject to {0}.", typeof( T ) ) );
@@ -1845,7 +1875,7 @@ public static implicit operator MessagePackObject( MessagePackObject[] value )
 			return new MessagePackObject( value, false );
 		}
 
-		#endregion -- Conversion Operator Overloads --
+#endregion -- Conversion Operator Overloads --
 
 #if DEBUG
 		internal string DebugDump()

src/MsgPack/MessagePackObject.cs

@@ -774,7 +774,7 @@ public Double AsDouble()
 		/// <returns><see cref="String" /> instance corresponds to this instance.</returns>
 		public String AsString()
 		{
-			VerifyUnderlyingType<MessagePackString>( this, null );
+			VerifyUnderlyingRawType<string>( this, null );
 
 			if( this._handleOrTypeCode == null )
 			{
@@ -793,7 +793,7 @@ public String AsString()
 		/// <returns><see cref="Byte" />[] instance corresponds to this instance.</returns>
 		public Byte[] AsBinary()
 		{
-			VerifyUnderlyingType<MessagePackString>( this, null  );
+			VerifyUnderlyingRawType<byte[]>( this, null  );
 
 			if( this._handleOrTypeCode == null )
 			{
@@ -1603,7 +1603,7 @@ public static explicit operator Double( MessagePackObject value )
 		/// <returns><see cref="String" /> instance corresponds to <paramref name="value"/>.</returns>
 		public static explicit operator String( MessagePackObject value )
 		{
-			VerifyUnderlyingType<MessagePackString>( value, "value" );
+			VerifyUnderlyingRawType<string>( value, "value" );
 
 			if( value._handleOrTypeCode == null )
 			{
@@ -1623,7 +1623,7 @@ public static explicit operator String( MessagePackObject value )
 		/// <returns><see cref="Byte" />[] instance corresponds to <paramref name="value"/>.</returns>
 		public static explicit operator Byte[]( MessagePackObject value )
 		{
-			VerifyUnderlyingType<MessagePackString>( value, "value"  );
+			VerifyUnderlyingRawType<byte[]>( value, "value"  );
 
 			if( value._handleOrTypeCode == null )
 			{

src/MsgPack/MessagePackObject.tt

@@ -408,7 +408,7 @@ private void GenerateAsT( string val, Object typeOrTypeName, bool passParameterN
 	else if ( t == typeof( byte[] ) )
 	{
 #>
-			VerifyUnderlyingType<MessagePackString>( <#= val #>, <#= passParameterName ? "\"" + val + "\"" : "null" #>  );
+			VerifyUnderlyingRawType<byte[]>( <#= val #>, <#= passParameterName ? "\"" + val + "\"" : "null" #>  );
 
 			if( <#= val #>._handleOrTypeCode == null )
 			{
@@ -424,7 +424,7 @@ private void GenerateAsT( string val, Object typeOrTypeName, bool passParameterN
 	else if ( t == typeof( string ) )
 	{
 #>
-			VerifyUnderlyingType<MessagePackString>( <#= val #>, <#= passParameterName ? "\"" + val + "\"" : "null" #> );
+			VerifyUnderlyingRawType<string>( <#= val #>, <#= passParameterName ? "\"" + val + "\"" : "null" #> );
 
 			if( <#= val #>._handleOrTypeCode == null )
 			{

test/MsgPack.UnitTest/Serialization/RegressionTests.cs

@@ -531,5 +531,21 @@ public async Task TestIssue270Async_AsRaw()
 		}
 
 #endif // FEATURE_TAP
+
+		[Test]
+		public void TestIssue269()
+		{
+			var input = new MessagePackObject( Timestamp.UtcNow.Encode() );
+			var target = MessagePackSerializer.UnpackMessagePackObject( MessagePackSerializer.Get<MessagePackObject>().PackSingleObject( input ) );
+			Assert.That( target.UnderlyingType, Is.EqualTo( typeof( MessagePackExtendedTypeObject ) ) );
+			Assert.That( target.IsTypeOf<byte[]>(), Is.False );
+			Assert.That( target.IsTypeOf<MessagePackExtendedTypeObject>(), Is.True );
+
+			var forBinary = Assert.Throws<InvalidOperationException>( () => target.AsBinary() );
+			Assert.That( forBinary.Message, Is.EqualTo( "Do not convert MsgPack.MessagePackExtendedTypeObject MessagePackObject to System.Byte[]." ) );
+
+			var forString = Assert.Throws<InvalidOperationException>( () => target.AsString() );
+			Assert.That( forString.Message, Is.EqualTo( "Do not convert MsgPack.MessagePackExtendedTypeObject MessagePackObject to System.String." ) );
+		}
 	}
 }