Culture model binding #871

Merged
merged 2 commits into from Jan 3, 2013

3 participants

@jchannon
Custodians of the Super-Duper-Happy-Path member

Using the Culture on the NancyContext it is used for model binding so that when 4,50 is posted to the server it is bound to 4.50 for example. I have also included a DateTime converter to bind dates that are culturally sound.

@thecodejunkie thecodejunkie commented on the diff Dec 19, 2012
src/Nancy/Extensions/TypeExtensions.cs
@@ -34,5 +34,40 @@ public static bool IsEnumerable(this Type source)
return source.IsGenericType && source.GetGenericTypeDefinition() == enumerableType;
}
+
+ /// <summary>
+ /// Determines if a type is numeric. Nullable numeric types are considered numeric.
+ /// </summary>
+ /// <remarks>
+ /// Boolean is not considered numeric.
+ /// </remarks>
+ public static bool IsNumeric(this Type source)
@thecodejunkie
Custodians of the Super-Duper-Happy-Path member
thecodejunkie added a line comment Dec 19, 2012

This could be replace with return t.IsValueType && !t.IsEnum && (t.IsPrimitive || t == typeof(decimal) ); which is a lot more compact. Of course your'd have to add in the generic stuff as well, but it covers most of the fugly switch statement ;)

@thecodejunkie
Custodians of the Super-Duper-Happy-Path member
thecodejunkie added a line comment Dec 27, 2012

Hmm did you miss the "of course you'll have to add in the generic stuff as well" ? You had some code before that checked the generic argumetn type of generic types. My code does not cover that. Not covered by a test?

@jchannon
Custodians of the Super-Duper-Happy-Path member
jchannon added a line comment Dec 27, 2012

Had to put back the switch statement ,sorry,your code didn't cut it 😄 Added extensive tests

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@thecodejunkie thecodejunkie commented on an outdated diff Dec 19, 2012
...y/ModelBinding/DefaultConverters/DateTimeConverter.cs
@@ -0,0 +1,41 @@
+using System;
@thecodejunkie
Custodians of the Super-Duper-Happy-Path member
thecodejunkie added a line comment Dec 19, 2012

Using statements should be places inside namespace declarations

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@thecodejunkie thecodejunkie commented on the diff Dec 19, 2012
...y/ModelBinding/DefaultConverters/DateTimeConverter.cs
+
+ /// <summary>
+ /// Convert the string representation to the destination type
+ /// </summary>
+ /// <param name="input">Input string</param>
+ /// <param name="destinationType">Destination type</param>
+ /// <param name="context">Current context</param>
+ /// <returns>Converted object of the destination type</returns>
+ public object Convert(string input, Type destinationType, BindingContext context)
+ {
+ if (string.IsNullOrEmpty(input))
+ {
+ return null;
+ }
+
+ return System.Convert.ChangeType(input, destinationType, context.Context.Culture);
@thecodejunkie
Custodians of the Super-Duper-Happy-Path member
thecodejunkie added a line comment Dec 19, 2012

Remove System prefix

@jchannon
Custodians of the Super-Duper-Happy-Path member
jchannon added a line comment Dec 21, 2012

Can't, look at the method name ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@thecodejunkie thecodejunkie commented on an outdated diff Dec 19, 2012
...cy/ModelBinding/DefaultConverters/NumericConverter.cs
+
+ /// <summary>
+ /// Convert the string representation to the destination type
+ /// </summary>
+ /// <param name="input">Input string</param>
+ /// <param name="destinationType">Destination type</param>
+ /// <param name="context">Current context</param>
+ /// <returns>Converted object of the destination type</returns>
+ public object Convert(string input, Type destinationType, BindingContext context)
+ {
+ if (string.IsNullOrEmpty(input))
+ {
+ return null;
+ }
+
+ return System.Convert.ChangeType(input, destinationType, context.Context.Culture);
@thecodejunkie
Custodians of the Super-Duper-Happy-Path member
thecodejunkie added a line comment Dec 19, 2012

Remove System prefix

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@thecodejunkie thecodejunkie and 1 other commented on an outdated diff Dec 19, 2012
...Nancy.Tests/Unit/ModelBinding/DefaultBinderFixture.cs
@@ -529,6 +531,45 @@ public void Should_be_able_to_bind_from_form_and_request_simultaneously()
}
[Fact]
+ public void Form_properties_should_be_bound_culturally_aware_if_numeric()
+ {
+
@thecodejunkie
Custodians of the Super-Duper-Happy-Path member
thecodejunkie added a line comment Dec 19, 2012

Missing // Given

@jchannon
Custodians of the Super-Duper-Happy-Path member
jchannon added a line comment Dec 21, 2012

Umm, there's plenty in there that don't :P

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@thecodejunkie thecodejunkie commented on an outdated diff Dec 19, 2012
...Nancy.Tests/Unit/ModelBinding/DefaultBinderFixture.cs
+ context.Culture = new CultureInfo("de-DE");
+ context.Request.Form["DoubleProperty"] = "4,50";
+
+ // When
+ var result = (TestModel)binder.Bind(context, typeof(TestModel), null, new BindingConfig());
+ // Then
+ result.StringProperty.ShouldEqual("Test");
+ result.DoubleProperty.ShouldEqual(4.50);
+ }
+
+ [Theory]
+ [InlineData("12/25/2012", 12, 25, 2012)]
+ [InlineData("12/12/2012", 12, 12, 2012)]
+ public void Form_properties_should_be_bound_culturally_aware_if_datetime(string date, int month, int day, int year)
+ {
+
@thecodejunkie
Custodians of the Super-Duper-Happy-Path member
thecodejunkie added a line comment Dec 19, 2012

Missing // Given

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@thecodejunkie thecodejunkie commented on an outdated diff Dec 19, 2012
...Nancy.Tests/Unit/ModelBinding/DefaultBinderFixture.cs
+
+ var context = CreateContextWithHeader("Content-Type", new[] { "application/xml" });
+ context.Culture = new CultureInfo("de-DE");
+ context.Request.Form["DoubleProperty"] = "4,50";
+
+ // When
+ var result = (TestModel)binder.Bind(context, typeof(TestModel), null, new BindingConfig());
+ // Then
+ result.StringProperty.ShouldEqual("Test");
+ result.DoubleProperty.ShouldEqual(4.50);
+ }
+
+ [Theory]
+ [InlineData("12/25/2012", 12, 25, 2012)]
+ [InlineData("12/12/2012", 12, 12, 2012)]
+ public void Form_properties_should_be_bound_culturally_aware_if_datetime(string date, int month, int day, int year)
@thecodejunkie
Custodians of the Super-Duper-Happy-Path member
thecodejunkie added a line comment Dec 19, 2012

Test name should start with Should_xxxxx

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@thecodejunkie thecodejunkie and 1 other commented on an outdated diff Dec 19, 2012
...Nancy.Tests/Unit/ModelBinding/DefaultBinderFixture.cs
@@ -529,6 +531,45 @@ public void Should_be_able_to_bind_from_form_and_request_simultaneously()
}
[Fact]
+ public void Form_properties_should_be_bound_culturally_aware_if_numeric()
@thecodejunkie
Custodians of the Super-Duper-Happy-Path member
thecodejunkie added a line comment Dec 19, 2012

Test name should start with Should_xxxxx

@jchannon
Custodians of the Super-Duper-Happy-Path member
jchannon added a line comment Dec 21, 2012

Umm, there's plenty in there that don't :P

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@thecodejunkie thecodejunkie commented on an outdated diff Dec 19, 2012
...Nancy.Tests/Unit/ModelBinding/DefaultBinderFixture.cs
+ var result = (TestModel)binder.Bind(context, typeof(TestModel), null, new BindingConfig());
+ // Then
+ result.StringProperty.ShouldEqual("Test");
+ result.DoubleProperty.ShouldEqual(4.50);
+ }
+
+ [Theory]
+ [InlineData("12/25/2012", 12, 25, 2012)]
+ [InlineData("12/12/2012", 12, 12, 2012)]
+ public void Form_properties_should_be_bound_culturally_aware_if_datetime(string date, int month, int day, int year)
+ {
+
+ var binder = this.GetBinder();
+
+ var context = CreateContextWithHeader("Content-Type", new[] { "application/xml" });
+ context.Culture = new CultureInfo("en-US");
@thecodejunkie
Custodians of the Super-Duper-Happy-Path member
thecodejunkie added a line comment Dec 19, 2012

Perhaps the culture should be specified in the InlineData so we can add in a couple of more test cases for different cultures

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@thecodejunkie thecodejunkie commented on an outdated diff Dec 19, 2012
...Nancy.Tests/Unit/ModelBinding/DefaultBinderFixture.cs
@@ -529,6 +531,45 @@ public void Should_be_able_to_bind_from_form_and_request_simultaneously()
}
[Fact]
+ public void Form_properties_should_be_bound_culturally_aware_if_numeric()
+ {
+
+ var binder = this.GetBinder();
+
+ var context = CreateContextWithHeader("Content-Type", new[] { "application/xml" });
+ context.Culture = new CultureInfo("de-DE");
@thecodejunkie
Custodians of the Super-Duper-Happy-Path member
thecodejunkie added a line comment Dec 19, 2012

Perhaps the culture should be specified in the InlineData so we can add in a couple of more test cases for different cultures

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@thecodejunkie thecodejunkie commented on an outdated diff Dec 19, 2012
...Nancy.Tests/Unit/ModelBinding/DefaultBinderFixture.cs
@@ -1,3 +1,5 @@
+using Xunit.Extensions;
@thecodejunkie
Custodians of the Super-Duper-Happy-Path member
thecodejunkie added a line comment Dec 19, 2012

Using statements should be places inside namespace declarations

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@thecodejunkie thecodejunkie commented on an outdated diff Dec 27, 2012
...cy/ModelBinding/DefaultConverters/NumericConverter.cs
@@ -0,0 +1,42 @@
+using System.Globalization;
@thecodejunkie
Custodians of the Super-Duper-Happy-Path member
thecodejunkie added a line comment Dec 27, 2012

tssk-tssk slap on wrist

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@thecodejunkie thecodejunkie commented on an outdated diff Dec 27, 2012
...Nancy.Tests/Unit/ModelBinding/DefaultBinderFixture.cs
@@ -528,6 +529,51 @@ public void Should_be_able_to_bind_from_form_and_request_simultaneously()
result.IntProperty.ShouldEqual(12);
}
+ [Theory]
+ [InlineData("de-DE", 4.50)]
+ [InlineData("en-GB", 450)]
+ [InlineData("en-US", 450)]
+ [InlineData("se-SE", 4.50)]
+ public void Form_properties_should_be_bound_culturally_aware_if_numeric(string culture, double expected)
@thecodejunkie
Custodians of the Super-Duper-Happy-Path member
thecodejunkie added a line comment Dec 27, 2012

Test naming convention is Should_xxxxxx

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@thecodejunkie thecodejunkie commented on an outdated diff Dec 27, 2012
...Nancy.Tests/Unit/ModelBinding/DefaultBinderFixture.cs
+ context.Culture = new CultureInfo(culture);
+ context.Request.Form["DoubleProperty"] = "4,50";
+
+ // When
+ var result = (TestModel)binder.Bind(context, typeof(TestModel), null, new BindingConfig());
+
+ // Then
+ result.DoubleProperty.ShouldEqual(expected);
+ }
+
+ [Theory]
+ [InlineData("12/25/2012", 12, 25, 2012, "en-US")]
+ [InlineData("12/12/2012", 12, 12, 2012, "en-US")]
+ [InlineData("25/12/2012", 12, 25, 2012, "en-GB")]
+ [InlineData("12/12/2012", 12, 12, 2012, "en-GB")]
+ public void Form_properties_should_be_bound_culturally_aware_if_datetime(string date, int month, int day, int year, string culture)
@thecodejunkie
Custodians of the Super-Duper-Happy-Path member
thecodejunkie added a line comment Dec 27, 2012

Test naming convention is Should_xxxxxx

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@thecodejunkie thecodejunkie commented on the diff Dec 27, 2012
...Nancy.Tests/Unit/ModelBinding/DefaultBinderFixture.cs
+ var context = CreateContextWithHeader("Content-Type", new[] { "application/xml" });
+ context.Culture = new CultureInfo(culture);
+ context.Request.Form["DoubleProperty"] = "4,50";
+
+ // When
+ var result = (TestModel)binder.Bind(context, typeof(TestModel), null, new BindingConfig());
+
+ // Then
+ result.DoubleProperty.ShouldEqual(expected);
+ }
+
+ [Theory]
+ [InlineData("12/25/2012", 12, 25, 2012, "en-US")]
+ [InlineData("12/12/2012", 12, 12, 2012, "en-US")]
+ [InlineData("25/12/2012", 12, 25, 2012, "en-GB")]
+ [InlineData("12/12/2012", 12, 12, 2012, "en-GB")]
@thecodejunkie
Custodians of the Super-Duper-Happy-Path member
thecodejunkie added a line comment Dec 27, 2012

Wonder if we should throw in a couple of more test cases that aren't English based. Perhaps pick a couple that are not so equal.. Something like Russian, Asian etc

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@thecodejunkie thecodejunkie commented on the diff Dec 27, 2012
...Nancy.Tests/Unit/ModelBinding/DefaultBinderFixture.cs
@@ -528,6 +529,51 @@ public void Should_be_able_to_bind_from_form_and_request_simultaneously()
result.IntProperty.ShouldEqual(12);
}
+ [Theory]
+ [InlineData("de-DE", 4.50)]
+ [InlineData("en-GB", 450)]
+ [InlineData("en-US", 450)]
+ [InlineData("se-SE", 4.50)]
@thecodejunkie
Custodians of the Super-Duper-Happy-Path member
thecodejunkie added a line comment Dec 27, 2012

Wonder if we should throw in a couple of more test cases that aren't English based. Perhaps pick a couple that are not so equal.. Something like Russian, Asian etc

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@thecodejunkie
Custodians of the Super-Duper-Happy-Path member

Needs rebasing

@jchannon
Custodians of the Super-Duper-Happy-Path member

Rebased

@grumpydev grumpydev merged commit 9bbe5b3 into NancyFx:master Jan 3, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment