Merge pull request graphql-dotnet#434 from graphql-dotnet/bugs/doubles

joemcbride · web-flow · commit fd864e25f607 · 2017-09-22T08:33:43.000-07:00
Another attempt at fixing the decimal number parsing/printing
diff --git a/src/GraphQL.Tests/CultureTestHelper.cs b/src/GraphQL.Tests/CultureTestHelper.cs
@@ -0,0 +1,44 @@
+using System;
+using System.Collections.Generic;
+using System.Globalization;
+
+namespace GraphQL.Tests
+{
+    public class CultureTestHelper
+    {
+        public static IEnumerable<CultureInfo> Cultures => new[]
+        {
+            new CultureInfo("fi-FI"),
+            new CultureInfo("en-US"),
+            CultureInfo.InvariantCulture,
+            new CultureInfo(CultureInfo.CurrentUICulture.Name),
+            new CultureInfo(CultureInfo.CurrentCulture.Name)
+        };
+
+        public static void UseCultures(Action scope)
+        {
+            foreach (var culture in Cultures)
+            {
+                UseCulture(culture, scope);
+            }
+        }
+
+        public static void UseCulture(CultureInfo culture, Action scope)
+        {
+            var before = CultureInfo.CurrentCulture;
+            var beforeUi = CultureInfo.CurrentUICulture;
+            CultureInfo.DefaultThreadCurrentCulture = culture;
+            CultureInfo.DefaultThreadCurrentUICulture = culture;
+
+            try
+            {
+                scope();
+            }
+            finally
+            {
+                CultureInfo.DefaultThreadCurrentCulture = before;
+                CultureInfo.DefaultThreadCurrentUICulture = beforeUi;
+            }
+        }
+    }
+}
diff --git a/src/GraphQL.Tests/ObjectExtensionsTest.cs b/src/GraphQL.Tests/ObjectExtensionsTest.cs
@@ -0,0 +1,77 @@
+using System;
+using System.Collections.Generic;
+using System.Globalization;
+using System.Text;
+using GraphQL.Types;
+using Shouldly;
+using Xunit;
+
+namespace GraphQL.Tests
+{
+    public class ObjectExtensionsTests
+    {
+        [Fact]
+        public void convert_double_using_cultures() 
+        {
+            CultureTestHelper.UseCultures(convert_double);
+        } 
+
+        [Fact]
+        public void convert_double()
+        {
+            /* Given */
+            double value = 123.123d;
+            Type floatType = typeof(double);
+
+
+            /* When */
+            var actual = ObjectExtensions.ConvertValue(value, floatType);
+
+            /* Then */
+            actual.ShouldBe(value);
+        }
+
+        [Fact]
+        public void convert_decimal_using_cultures()
+        {
+            CultureTestHelper.UseCultures(convert_decimal);
+        }
+
+        [Fact]
+        public void convert_decimal()
+        {
+            /* Given */
+            decimal value = 123.123m;
+            Type floatType = typeof(decimal);
+
+
+            /* When */
+            var actual = ObjectExtensions.ConvertValue(value, floatType);
+
+            /* Then */
+            actual.ShouldBe(value);
+        }
+
+        [Fact]
+        public void convert_single_using_cultures()
+        {
+            CultureTestHelper.UseCultures(convert_single);
+        }
+
+        [Fact]
+        public void convert_single()
+        { 
+
+            /* Given */
+            float value = 123.123f;
+            Type floatType = typeof(float);
+
+
+            /* When */
+            var actual = ObjectExtensions.ConvertValue(value, floatType);
+
+            /* Then */
+            actual.ShouldBe(value);
+        }
+    }
+}
diff --git a/src/GraphQL.Tests/Types/DecimalGraphTypeTests.cs b/src/GraphQL.Tests/Types/DecimalGraphTypeTests.cs
@@ -1,4 +1,5 @@
-﻿using GraphQL.Language.AST;
+using System.Globalization;
+using GraphQL.Language.AST;
 using GraphQL.Types;
 using Shouldly;
 using Xunit;
@@ -18,7 +19,7 @@ public void coerces_null_to_null()
         [Fact]
         public void coerces_integer_to_decimal()
         {
-            type.ParseValue(0).ShouldBe((decimal)0);
+            type.ParseValue(0).ShouldBe((decimal) 0);
         }
 
         [Fact]
@@ -27,12 +28,24 @@ public void coerces_invalid_string_to_null()
             type.ParseValue("abcd").ShouldBe(null);
         }
 
+        [Fact]
+        public void coerces_numeric_string_to_decimal_using_cultures()
+        {
+            CultureTestHelper.UseCultures(coerces_numeric_string_to_decimal);
+        }
+
         [Fact]
         public void coerces_numeric_string_to_decimal()
         {
             type.ParseValue("12345.6579").ShouldBe((decimal)12345.6579);
         }
 
+        [Fact]
+        public void converts_DecimalValue_to_decimal_using_cultures()
+        {
+            CultureTestHelper.UseCultures(converts_DecimalValue_to_decimal);
+        }
+
         [Fact]
         public void converts_DecimalValue_to_decimal()
         {
diff --git a/src/GraphQL.Tests/Types/FloatGraphTypeTests.cs b/src/GraphQL.Tests/Types/FloatGraphTypeTests.cs
@@ -1,4 +1,5 @@
-﻿using GraphQL.Types;
+using System.Globalization;
+using GraphQL.Types;
 using Shouldly;
 using Xunit;
 
@@ -20,6 +21,12 @@ public void coerces_invalid_string_to_null()
             type.ParseValue("abcd").ShouldBe(null);
         }
 
+        [Fact]
+        public void coerces_double_to_value_using_cultures()
+        {
+            CultureTestHelper.UseCultures(coerces_double_to_value);
+        }
+
         [Fact]
         public void coerces_double_to_value()
         {
diff --git a/src/GraphQL.Tests/Utilities/AstPrinterTests.cs b/src/GraphQL.Tests/Utilities/AstPrinterTests.cs
@@ -1,4 +1,5 @@
-﻿using GraphQL.Execution;
+using System.Globalization;
+using GraphQL.Execution;
 using GraphQL.Language.AST;
 using GraphQL.Utilities;
 using Shouldly;
@@ -64,14 +65,20 @@ public void prints_long_value()
             result.ShouldBe(value);
         }
 
+        [Fact]
+        public void prints_float_value_using_cultures()
+        {
+            CultureTestHelper.UseCultures(prints_float_value);
+        }
+
         [Fact]
         public void prints_float_value()
         {
             double value = 3.33;
 
             var val = new FloatValue(value);
             var result = _printer.Visit(val);
-            result.ShouldBe($"{value, 0:0.0##}");
+            result.ShouldBe(value.ToString("0.0##", NumberFormatInfo.InvariantInfo));
         }
 
         private static string MonetizeLineBreaks(string input)
diff --git a/src/GraphQL.Tests/Validation/ArgumentsOfCorrectTypeTests.cs b/src/GraphQL.Tests/Validation/ArgumentsOfCorrectTypeTests.cs
@@ -1,4 +1,5 @@
-﻿using System.Collections.Generic;
+using System.Collections.Generic;
+using System.Globalization;
 using GraphQL.Validation.Rules;
 using Xunit;
 
@@ -122,6 +123,12 @@ public void int_into_string()
             });
         }
 
+        [Fact]
+        public void float_into_string_using_cultures()
+        {
+            CultureTestHelper.UseCultures(float_into_string);
+        }
+
         [Fact]
         public void float_into_string()
         {
@@ -426,6 +433,12 @@ public void int_into_enum()
             });
         }
 
+        [Fact]
+        public void float_into_enum_with_cultures()
+        {
+            CultureTestHelper.UseCultures(float_into_enum);
+        }
+
         [Fact]
         public void float_into_enum()
         {
diff --git a/src/GraphQL/Conversion/Conversions.cs b/src/GraphQL/Conversion/Conversions.cs
@@ -1,5 +1,6 @@
-﻿using System;
+using System;
 using System.Collections.Generic;
+using System.Globalization;
 using System.Linq;
 
 namespace GraphQL.Conversion
@@ -28,9 +29,9 @@ public Conversions()
                 char.TryParse(x, out c);
                 return c;
             });
-            RegisterConversion(decimal.Parse);
-            RegisterConversion(double.Parse);
-            RegisterConversion(float.Parse);
+            RegisterConversion(ParseDecimal);
+            RegisterConversion(ParseDouble);
+            RegisterConversion(ParseFloat);
             RegisterConversion(short.Parse);
             RegisterConversion(int.Parse);
             RegisterConversion(long.Parse);
@@ -48,6 +49,21 @@ public Conversions()
             });
         }
 
+        public static float ParseFloat(string value)
+        {
+            return System.Convert.ToSingle(value, NumberFormatInfo.InvariantInfo);
+        }
+
+        public static double ParseDouble(string value)
+        {
+            return System.Convert.ToDouble(value, NumberFormatInfo.InvariantInfo);
+        }
+
+        public static decimal ParseDecimal(string value)
+        {
+            return System.Convert.ToDecimal(value, NumberFormatInfo.InvariantInfo);
+        }
+
         private IEnumerable<IConversionProvider> providers()
         {
             foreach (var provider in _providers)
diff --git a/src/GraphQL/Language/AST/IntValue.cs b/src/GraphQL/Language/AST/IntValue.cs
@@ -1,5 +1,6 @@
-﻿using System;
+using System;
 using System.Collections.Generic;
+using System.Globalization;
 using System.Linq;
 
 namespace GraphQL.Language.AST
@@ -99,7 +100,8 @@ public FloatValue(double value)
 
         public override string ToString()
         {
-            return "FloatValue{{value={0}}}".ToFormat(Value);
+            var valueStr = Value.ToString("G", NumberFormatInfo.InvariantInfo);
+            return "FloatValue{{value={0}}}".ToFormat(valueStr);
         }
 
         protected bool Equals(FloatValue other)
diff --git a/src/GraphQL/Language/CoreToVanillaConverter.cs b/src/GraphQL/Language/CoreToVanillaConverter.cs
@@ -1,5 +1,6 @@
-﻿using System.Collections.Generic;
+using System.Collections.Generic;
 using System.Linq;
+using GraphQL.Conversion;
 using GraphQL.Language.AST;
 using GraphQLParser;
 using GraphQLParser.AST;
@@ -202,7 +203,8 @@ public IValue Value(GraphQLValue source)
                 case ASTNodeKind.FloatValue:
                 {
                     var str = source as GraphQLScalarValue;
-                    return new FloatValue(double.Parse(str.Value)).WithLocation(str, _body);
+                    var value = Conversions.ParseDouble(str.Value);
+                    return new FloatValue(value).WithLocation(str, _body);
                 }
                 case ASTNodeKind.BooleanValue:
                 {
diff --git a/src/GraphQL/ObjectExtensions.cs b/src/GraphQL/ObjectExtensions.cs
@@ -1,6 +1,7 @@
-﻿using System;
+using System;
 using System.Collections;
 using System.Collections.Generic;
+using System.Globalization;
 using System.Linq;
 using System.Reflection;
 using GraphQL.Conversion;
@@ -135,7 +136,16 @@ public static object ConvertValue(object value, Type fieldType)
                 return value;
             }
 
-            var text = value.ToString();
+            string text;
+            if (value is float)
+                text = ((float)value).ToString(CultureInfo.InvariantCulture);
+            else if (value is double)
+                text = ((double)value).ToString(CultureInfo.InvariantCulture);
+            else if (value is decimal)
+                text = ((decimal)value).ToString(CultureInfo.InvariantCulture);
+            else
+                text = value.ToString();
+
             return _conversions.Value.Convert(fieldType, text);
         }
 
diff --git a/src/GraphQL/Types/DecimalGraphType.cs b/src/GraphQL/Types/DecimalGraphType.cs
@@ -1,3 +1,6 @@
+using System;
+using System.Globalization;
+using GraphQL.Conversion;
 using GraphQL.Language.AST;
 
 namespace GraphQL.Types
@@ -16,12 +19,20 @@ public override object Serialize(object value)
 
         public override object ParseValue(object value)
         {
-            decimal result;
-            if (decimal.TryParse(value?.ToString() ?? string.Empty, out result))
+            if (value == null)
+                return null;
+
+            try
             {
+                var result = Convert.ToDecimal(value, NumberFormatInfo.InvariantInfo);
+
                 return result;
             }
-            return null;
+            catch (FormatException e)
+            {
+                //todo: should log or something?
+                return null;
+            }
         }
 
         public override object ParseLiteral(IValue value)
diff --git a/src/GraphQL/Types/FloatGraphType.cs b/src/GraphQL/Types/FloatGraphType.cs
diff --git a/src/GraphQL/Utilities/AstPrinter.cs b/src/GraphQL/Utilities/AstPrinter.cs
diff --git a/src/GraphQL/Utilities/SchemaBuilder.cs b/src/GraphQL/Utilities/SchemaBuilder.cs

Original file line number	Diff line number	Diff line change
`@@ -1,5 +1,6 @@`
`1`		`-using System;`
	`1`	`+using System;`
`2`	`2`	`using System.Collections.Generic;`
	`3`	`+using System.Globalization;`
`3`	`4`	`using System.Linq;`
`4`	`5`
`5`	`6`	`namespace GraphQL.Language.AST`
`@@ -99,7 +100,8 @@ public FloatValue(double value)`
`99`	`100`
`100`	`101`	`public override string ToString()`
`101`	`102`	`{`
`102`		`- return "FloatValue{{value={0}}}".ToFormat(Value);`
	`103`	`+ var valueStr = Value.ToString("G", NumberFormatInfo.InvariantInfo);`
	`104`	`+ return "FloatValue{{value={0}}}".ToFormat(valueStr);`
`103`	`105`	`}`
`104`	`106`
`105`	`107`	`protected bool Equals(FloatValue other)`
Original file line number	Diff line number	Diff line change
`@@ -1,5 +1,6 @@`
`1`		`-using System.Collections.Generic;`
	`1`	`+using System.Collections.Generic;`
`2`	`2`	`using System.Linq;`
	`3`	`+using GraphQL.Conversion;`
`3`	`4`	`using GraphQL.Language.AST;`
`4`	`5`	`using GraphQLParser;`
`5`	`6`	`using GraphQLParser.AST;`
`@@ -202,7 +203,8 @@ public IValue Value(GraphQLValue source)`
`202`	`203`	`case ASTNodeKind.FloatValue:`
`203`	`204`	`{`
`204`	`205`	`var str = source as GraphQLScalarValue;`
`205`		`- return new FloatValue(double.Parse(str.Value)).WithLocation(str, _body);`
	`206`	`+ var value = Conversions.ParseDouble(str.Value);`
	`207`	`+ return new FloatValue(value).WithLocation(str, _body);`
`206`	`208`	`}`
`207`	`209`	`case ASTNodeKind.BooleanValue:`
`208`	`210`	`{`