From 49d45e4b66ada0f4515822cbf1473ee6758f350a Mon Sep 17 00:00:00 2001 From: S'pht'Kr Date: Fri, 23 Jan 2015 10:04:21 +0100 Subject: [PATCH 1/8] Initial implementation of GetPropertyMap, no tests yet. --- JSONAPI/Core/ModelManager.cs | 32 +++++++++++++++++++++++++++++++- JSONAPI/Json/JsonApiFormatter.cs | 19 ++++--------------- 2 files changed, 35 insertions(+), 16 deletions(-) diff --git a/JSONAPI/Core/ModelManager.cs b/JSONAPI/Core/ModelManager.cs index 9cd8da1a..09d32a6a 100644 --- a/JSONAPI/Core/ModelManager.cs +++ b/JSONAPI/Core/ModelManager.cs @@ -1,4 +1,5 @@ -using System; +using JSONAPI.Json; +using System; using System.Collections.Generic; using System.Linq; using System.Reflection; @@ -32,6 +33,11 @@ private Lazy> _idProperties () => new Dictionary() ); + private Lazy>> _propertyMaps + = new Lazy>>( + () => new Dictionary>() + ); + #endregion #region Id property determination @@ -57,5 +63,29 @@ public PropertyInfo GetIdProperty(Type type) } #endregion + + #region Property Maps + + public Dictionary GetPropertyMap(Type type) + { + Dictionary propMap = null; + + var propMapCache = _propertyMaps.Value; + + if (propMapCache.TryGetValue(type, out propMap)) return propMap; + + propMap = new Dictionary(); + PropertyInfo[] props = type.GetProperties(); + foreach (PropertyInfo prop in props) + { + propMap[JsonApiFormatter.FormatPropertyName(prop.Name)] = prop; + } + + propMapCache.Add(type, propMap); + + return propMap; + } + + #endregion } } diff --git a/JSONAPI/Json/JsonApiFormatter.cs b/JSONAPI/Json/JsonApiFormatter.cs index 5e8fb41e..59090f8a 100644 --- a/JSONAPI/Json/JsonApiFormatter.cs +++ b/JSONAPI/Json/JsonApiFormatter.cs @@ -549,15 +549,9 @@ private object ReadFromStream(Type type, Stream readStream, HttpContent content, public object Deserialize(Type objectType, Stream readStream, JsonReader reader, JsonSerializer serializer) { object retval = Activator.CreateInstance(objectType); - PropertyInfo[] props = objectType.GetProperties(); - - //TODO: This could get expensive...cache these maps per type, so we only build the map once? - IDictionary propMap = new Dictionary(); - foreach (PropertyInfo prop in props) - { - propMap[FormatPropertyName(prop.Name)] = prop; - } + IDictionary propMap = ModelManager.Instance.GetPropertyMap(objectType); + if (reader.TokenType != JsonToken.StartObject) throw new JsonReaderException(String.Format("Expected JsonToken.StartObject, got {0}", reader.TokenType.ToString())); reader.Read(); // Burn the StartObject token do @@ -640,13 +634,7 @@ private void DeserializeLinkedResources(object obj, Stream readStream, JsonReade //reader.Read(); if (reader.TokenType != JsonToken.StartObject) throw new JsonSerializationException("'links' property is not an object!"); - //TODO: Redundant, already done in Deserialize method...optimize? - PropertyInfo[] props = obj.GetType().GetProperties(); - IDictionary propMap = new Dictionary(); - foreach (PropertyInfo prop in props) - { - propMap[FormatPropertyName(prop.Name)] = prop; - } + IDictionary propMap = ModelManager.Instance.GetPropertyMap(obj.GetType()); while (reader.Read()) { @@ -809,6 +797,7 @@ private Type GetSingleType(Type type)//dynamic value = null) return type; } + //TODO: Should this move to ModelManager? Could be cached there to improve performance? public static string FormatPropertyName(string propertyName) { string result = propertyName.Substring(0, 1).ToLower() + propertyName.Substring(1); From 47339fd17053ca3de7e95fd47e50ac3cead573d0 Mon Sep 17 00:00:00 2001 From: S'pht'Kr Date: Fri, 23 Jan 2015 13:17:00 +0100 Subject: [PATCH 2/8] Additional optimizations, tests added now. --- JSONAPI.Tests/Core/ModelManagerTests.cs | 29 ++++++++++++ JSONAPI/Core/ModelManager.cs | 61 +++++++++++++++++++++---- JSONAPI/Json/JsonApiFormatter.cs | 37 ++++++--------- 3 files changed, 95 insertions(+), 32 deletions(-) diff --git a/JSONAPI.Tests/Core/ModelManagerTests.cs b/JSONAPI.Tests/Core/ModelManagerTests.cs index e88d5ad6..af563db3 100644 --- a/JSONAPI.Tests/Core/ModelManagerTests.cs +++ b/JSONAPI.Tests/Core/ModelManagerTests.cs @@ -36,5 +36,34 @@ public void DoesntFindMissingId() // Assert Assert.Fail("An InvalidOperationException should be thrown and we shouldn't get here!"); } + + [TestMethod] + public void GetPropertyMapTest() + { + // Arrange + // Act + var propMap = ModelManager.Instance.GetPropertyMap(typeof(Post)); + + // Assert + Assert.AreSame(typeof(Post).GetProperty("Id"), propMap["id"]); + Assert.AreSame(typeof(Post).GetProperty("Author"), propMap["author"]); + } + + [TestMethod] + public void GetJsonKeyForTypeTest() + { + // Arrange + var pluralizationService = new PluralizationService(); + + // Act + var postKey = ModelManager.Instance.GetJsonKeyForType(typeof(Post), pluralizationService); + var authorKey = ModelManager.Instance.GetJsonKeyForType(typeof(Author), pluralizationService); + var commentKey = ModelManager.Instance.GetJsonKeyForType(typeof(Comment), pluralizationService); + + // Assert + Assert.AreEqual("posts", postKey); + Assert.AreEqual("authors", authorKey); + Assert.AreEqual("comments", commentKey); + } } } diff --git a/JSONAPI/Core/ModelManager.cs b/JSONAPI/Core/ModelManager.cs index d8b9a1e9..ed3ca38f 100644 --- a/JSONAPI/Core/ModelManager.cs +++ b/JSONAPI/Core/ModelManager.cs @@ -8,7 +8,7 @@ namespace JSONAPI.Core { - class ModelManager + internal class ModelManager { #region Singleton pattern @@ -38,6 +38,11 @@ private Lazy>> _propertyMaps () => new Dictionary>() ); + private Lazy> _jsonKeysForType + = new Lazy>( + () => new Dictionary() + ); + #endregion #region Id property determination @@ -75,20 +80,58 @@ public Dictionary GetPropertyMap(Type type) var propMapCache = _propertyMaps.Value; - if (propMapCache.TryGetValue(type, out propMap)) return propMap; - - propMap = new Dictionary(); - PropertyInfo[] props = type.GetProperties(); - foreach (PropertyInfo prop in props) + lock (propMapCache) { - propMap[JsonApiFormatter.FormatPropertyName(prop.Name)] = prop; - } + if (propMapCache.TryGetValue(type, out propMap)) return propMap; + + propMap = new Dictionary(); + PropertyInfo[] props = type.GetProperties(); + foreach (PropertyInfo prop in props) + { + propMap[JsonApiFormatter.FormatPropertyName(prop.Name)] = prop; + } - propMapCache.Add(type, propMap); + propMapCache.Add(type, propMap); + } return propMap; } #endregion + + //TODO: This has been "moved" here so we can cache the results and improve performance...but + // it raises the question of whether the various methods called within here should belong + // to JsonApiFormatter at all...should they move here also? Should the IPluralizationService + // instance belong to ModelManager instead? + internal string GetJsonKeyForType(Type type, IPluralizationService pluralizationService) + { + string key = null; + + var keyCache = _jsonKeysForType.Value; + + lock (keyCache) + { + if (keyCache.TryGetValue(type, out key)) return key; + + if (JsonApiFormatter.IsMany(type)) + type = JsonApiFormatter.GetSingleType(type); + + var attrs = type.CustomAttributes.Where(x => x.AttributeType == typeof(Newtonsoft.Json.JsonObjectAttribute)).ToList(); + + string title = type.Name; + if (attrs.Any()) + { + var titles = attrs.First().NamedArguments.Where(arg => arg.MemberName == "Title") + .Select(arg => arg.TypedValue.Value.ToString()).ToList(); + if (titles.Any()) title = titles.First(); + } + + key = JsonApiFormatter.FormatPropertyName(pluralizationService.Pluralize(title)); + + keyCache.Add(type, key); + } + + return key; + } } } diff --git a/JSONAPI/Json/JsonApiFormatter.cs b/JSONAPI/Json/JsonApiFormatter.cs index 59090f8a..4d701084 100644 --- a/JSONAPI/Json/JsonApiFormatter.cs +++ b/JSONAPI/Json/JsonApiFormatter.cs @@ -106,7 +106,7 @@ public override Task WriteToStreamAsync(System.Type type, object value, Stream w //writer.Formatting = Formatting.Indented; - var root = GetPropertyName(type, value); + var root = GetJsonKeyForType(type, value); writer.WriteStartObject(); writer.WritePropertyName(root); @@ -155,7 +155,9 @@ protected void Serialize(object value, Stream writeStream, JsonWriter writer, Js var idProp = ModelManager.Instance.GetIdProperty(value.GetType()); writer.WriteValue(GetValueForIdProperty(idProp, value)); - PropertyInfo[] props = value.GetType().GetProperties(); + // Leverage the cached map to avoid another costly call to GetProperties() + PropertyInfo[] props = ModelManager.Instance.GetPropertyMap(value.GetType()).Values.ToArray(); + // Do non-model properties first, everything else goes in "links" //TODO: Unless embedded??? IList modelProps = new List(); @@ -293,8 +295,9 @@ protected void Serialize(object value, Stream writeStream, JsonWriter writer, Js if (lt == null) throw new JsonSerializationException( "A property was decorated with SerializeAs(SerializeAsOptions.Link) but no LinkTemplate attribute was provided."); - string link = String.Format(lt, objId, - value.GetType().GetProperty("Id").GetValue(value, null)); + string link = String.Format(lt, objId, + GetIdFor(value)); //value.GetType().GetProperty("Id").GetValue(value, null)); + //writer.WritePropertyName(ContractResolver.FormatPropertyName(prop.Name)); writer.WriteStartObject(); writer.WritePropertyName("href"); @@ -399,7 +402,7 @@ protected void SerializeLinkedResources(Stream writeStream, JsonWriter writer, J foreach (KeyValuePair> apair in writers) { apair.Value.Key.WriteEnd(); // close off the array - writer.WritePropertyName(GetPropertyName(apair.Key)); + writer.WritePropertyName(GetJsonKeyForType(apair.Key)); writer.WriteRawValue(apair.Value.Value.ToString()); // write the contents of the type JsonWriter's StringWriter to the main JsonWriter } @@ -422,7 +425,7 @@ private object ReadFromStream(Type type, Stream readStream, HttpContent content, { object retval = null; Type singleType = GetSingleType(type); - var pripropname = GetPropertyName(type); + var pripropname = GetJsonKeyForType(type); var contentHeaders = content == null ? null : content.Headers; // If content length is 0 then return default value for this type @@ -749,22 +752,10 @@ private object DeserializePrimitive(Type type, JsonReader reader) #endregion - private string GetPropertyName(Type type, dynamic value = null) + //TODO: Could be expensive, and is called often...move to ModelManager and cache result? + private string GetJsonKeyForType(Type type, dynamic value = null) { - if (IsMany(type)) - type = GetSingleType(type); - - var attrs = type.CustomAttributes.Where(x => x.AttributeType == typeof(Newtonsoft.Json.JsonObjectAttribute)).ToList(); - - string title = type.Name; - if (attrs.Any()) - { - var titles = attrs.First().NamedArguments.Where(arg => arg.MemberName == "Title") - .Select(arg => arg.TypedValue.Value.ToString()).ToList(); - if (titles.Any()) title = titles.First(); - } - - return FormatPropertyName(this.PluralizationService.Pluralize(title)); + return ModelManager.Instance.GetJsonKeyForType(type, this.PluralizationService); } //private string GetPropertyName(Type type) @@ -780,14 +771,14 @@ private bool IsMany(dynamic value = null) return false; } - private bool IsMany(Type type) + internal static bool IsMany(Type type) { return type.IsArray || (type.GetInterfaces().Contains(typeof(IEnumerable)) && type.IsGenericType); } - private Type GetSingleType(Type type)//dynamic value = null) + internal static Type GetSingleType(Type type)//dynamic value = null) { if (IsMany(type)) if (type.IsGenericType) From 721f98a48f5b0109595d27cf7e7138483cd534db Mon Sep 17 00:00:00 2001 From: S'pht'Kr Date: Mon, 26 Jan 2015 12:00:17 +0100 Subject: [PATCH 3/8] Much more refactoring. Moved PluralizationService, still have to move IsMany and GetSingleType. --- .../EntityConverterTests.cs | 9 ++-- JSONAPI.Tests/Core/MetadataManagerTests.cs | 3 +- JSONAPI.Tests/Core/ModelManagerTests.cs | 11 ++-- .../Json/JsonApiMediaFormaterTests.cs | 19 +++---- JSONAPI.Tests/Json/LinkTemplateTests.cs | 6 +-- JSONAPI/Core/IModelManager.cs | 8 +++ JSONAPI/Core/ModelManager.cs | 45 ++++++++++++++-- JSONAPI/Json/JsonApiFormatter.cs | 54 ++++++++++--------- 8 files changed, 98 insertions(+), 57 deletions(-) diff --git a/JSONAPI.EntityFramework.Tests/EntityConverterTests.cs b/JSONAPI.EntityFramework.Tests/EntityConverterTests.cs index 33f2412b..3c8df580 100644 --- a/JSONAPI.EntityFramework.Tests/EntityConverterTests.cs +++ b/JSONAPI.EntityFramework.Tests/EntityConverterTests.cs @@ -106,8 +106,7 @@ public void SetupEntities() public void SerializeTest() { // Arrange - JsonApiFormatter formatter = new JSONAPI.Json.JsonApiFormatter(); - formatter.PluralizationService = new JSONAPI.Core.PluralizationService(); + JsonApiFormatter formatter = new JSONAPI.Json.JsonApiFormatter(new JSONAPI.Core.PluralizationService()); MemoryStream stream = new MemoryStream(); // Act @@ -124,8 +123,7 @@ public void SerializeTest() public void DeserializePostIntegrationTest() { // Arrange - JsonApiFormatter formatter = new JSONAPI.Json.JsonApiFormatter(); - formatter.PluralizationService = new JSONAPI.Core.PluralizationService(); + JsonApiFormatter formatter = new JSONAPI.Json.JsonApiFormatter(new JSONAPI.Core.PluralizationService()); MemoryStream stream = new MemoryStream(); EntityFrameworkMaterializer materializer = new EntityFrameworkMaterializer(context); @@ -159,8 +157,7 @@ public void DeserializePostIntegrationTest() public void UnderpostingTest() { // Arrange - JsonApiFormatter formatter = new JSONAPI.Json.JsonApiFormatter(); - formatter.PluralizationService = new JSONAPI.Core.PluralizationService(); + JsonApiFormatter formatter = new JSONAPI.Json.JsonApiFormatter(new JSONAPI.Core.PluralizationService()); MemoryStream stream = new MemoryStream(); EntityFrameworkMaterializer materializer = new EntityFrameworkMaterializer(context); diff --git a/JSONAPI.Tests/Core/MetadataManagerTests.cs b/JSONAPI.Tests/Core/MetadataManagerTests.cs index 562a7efc..e4a0d259 100644 --- a/JSONAPI.Tests/Core/MetadataManagerTests.cs +++ b/JSONAPI.Tests/Core/MetadataManagerTests.cs @@ -15,8 +15,7 @@ public void PropertyWasPresentTest() { // Arrange - JsonApiFormatter formatter = new JSONAPI.Json.JsonApiFormatter(); - formatter.PluralizationService = new JSONAPI.Core.PluralizationService(); + JsonApiFormatter formatter = new JSONAPI.Json.JsonApiFormatter(new JSONAPI.Core.PluralizationService()); MemoryStream stream = new MemoryStream(); stream = new MemoryStream(System.Text.Encoding.ASCII.GetBytes(@"{""posts"":{""id"":42,""links"":{""author"":""18""}}}")); diff --git a/JSONAPI.Tests/Core/ModelManagerTests.cs b/JSONAPI.Tests/Core/ModelManagerTests.cs index ae8bfc8b..1fa02477 100644 --- a/JSONAPI.Tests/Core/ModelManagerTests.cs +++ b/JSONAPI.Tests/Core/ModelManagerTests.cs @@ -45,8 +45,10 @@ public void DoesntFindMissingId() public void GetPropertyMapTest() { // Arrange + var mm = new ModelManager(); + // Act - var propMap = ModelManager.Instance.GetPropertyMap(typeof(Post)); + var propMap = mm.GetPropertyMap(typeof(Post)); // Assert Assert.AreSame(typeof(Post).GetProperty("Id"), propMap["id"]); @@ -58,11 +60,12 @@ public void GetJsonKeyForTypeTest() { // Arrange var pluralizationService = new PluralizationService(); + var mm = new ModelManager(pluralizationService); // Act - var postKey = ModelManager.Instance.GetJsonKeyForType(typeof(Post), pluralizationService); - var authorKey = ModelManager.Instance.GetJsonKeyForType(typeof(Author), pluralizationService); - var commentKey = ModelManager.Instance.GetJsonKeyForType(typeof(Comment), pluralizationService); + var postKey = mm.GetJsonKeyForType(typeof(Post)); + var authorKey = mm.GetJsonKeyForType(typeof(Author)); + var commentKey = mm.GetJsonKeyForType(typeof(Comment)); // Assert Assert.AreEqual("posts", postKey); diff --git a/JSONAPI.Tests/Json/JsonApiMediaFormaterTests.cs b/JSONAPI.Tests/Json/JsonApiMediaFormaterTests.cs index 1a572d10..e2074b0e 100644 --- a/JSONAPI.Tests/Json/JsonApiMediaFormaterTests.cs +++ b/JSONAPI.Tests/Json/JsonApiMediaFormaterTests.cs @@ -133,8 +133,7 @@ public void SerializerIntegrationTest() //ModelConverter mc = new ModelConverter(); //ContractResolver.PluralizationService = new PluralizationService(); - JsonApiFormatter formatter = new JSONAPI.Json.JsonApiFormatter(); - formatter.PluralizationService = new JSONAPI.Core.PluralizationService(); + JsonApiFormatter formatter = new JSONAPI.Json.JsonApiFormatter(new JSONAPI.Core.PluralizationService()); MemoryStream stream = new MemoryStream(); // Act @@ -158,8 +157,7 @@ public void SerializeArrayIntegrationTest() //ModelConverter mc = new ModelConverter(); //ContractResolver.PluralizationService = new PluralizationService(); - JsonApiFormatter formatter = new JSONAPI.Json.JsonApiFormatter(); - formatter.PluralizationService = new JSONAPI.Core.PluralizationService(); + JsonApiFormatter formatter = new JSONAPI.Json.JsonApiFormatter(new JSONAPI.Core.PluralizationService()); MemoryStream stream = new MemoryStream(); // Act @@ -180,7 +178,6 @@ public void Should_serialize_error() { // Arrange var formatter = new JSONAPI.Json.JsonApiFormatter(new MockErrorSerializer()); - formatter.PluralizationService = new JSONAPI.Core.PluralizationService(); var stream = new MemoryStream(); // Act @@ -199,8 +196,7 @@ public void Should_serialize_error() public void SerializeErrorIntegrationTest() { // Arrange - JsonApiFormatter formatter = new JSONAPI.Json.JsonApiFormatter(); - formatter.PluralizationService = new JSONAPI.Core.PluralizationService(); + JsonApiFormatter formatter = new JSONAPI.Json.JsonApiFormatter(new JSONAPI.Core.PluralizationService()); MemoryStream stream = new MemoryStream(); // Act @@ -224,8 +220,7 @@ public void SerializeErrorIntegrationTest() public void DeserializeCollectionIntegrationTest() { // Arrange - JsonApiFormatter formatter = new JSONAPI.Json.JsonApiFormatter(); - formatter.PluralizationService = new JSONAPI.Core.PluralizationService(); + JsonApiFormatter formatter = new JSONAPI.Json.JsonApiFormatter(new JSONAPI.Core.PluralizationService()); MemoryStream stream = new MemoryStream(); formatter.WriteToStreamAsync(typeof(Post), new List {p, p2}, stream, (System.Net.Http.HttpContent)null, (System.Net.TransportContext)null); @@ -246,8 +241,7 @@ public void DeserializeCollectionIntegrationTest() [TestMethod(), Timeout(1000)] public void DeserializeExtraPropertyTest() { - JsonApiFormatter formatter = new JSONAPI.Json.JsonApiFormatter(); - formatter.PluralizationService = new JSONAPI.Core.PluralizationService(); + JsonApiFormatter formatter = new JSONAPI.Json.JsonApiFormatter(new JSONAPI.Core.PluralizationService()); MemoryStream stream = new MemoryStream(); stream = new MemoryStream(System.Text.Encoding.ASCII.GetBytes(@"{""authors"":{""id"":13,""name"":""Jason Hater"",""bogus"":""PANIC!"",""links"":{""posts"":[]}}}")); @@ -264,8 +258,7 @@ public void DeserializeExtraPropertyTest() [TestMethod(), Timeout(1000)] public void DeserializeExtraRelationshipTest() { - JsonApiFormatter formatter = new JSONAPI.Json.JsonApiFormatter(); - formatter.PluralizationService = new JSONAPI.Core.PluralizationService(); + JsonApiFormatter formatter = new JSONAPI.Json.JsonApiFormatter(new JSONAPI.Core.PluralizationService()); MemoryStream stream = new MemoryStream(); stream = new MemoryStream(System.Text.Encoding.ASCII.GetBytes(@"{""authors"":{""id"":13,""name"":""Jason Hater"",""links"":{""posts"":[],""bogus"":[""PANIC!""]}}}")); diff --git a/JSONAPI.Tests/Json/LinkTemplateTests.cs b/JSONAPI.Tests/Json/LinkTemplateTests.cs index c74c4943..e5eb0d83 100644 --- a/JSONAPI.Tests/Json/LinkTemplateTests.cs +++ b/JSONAPI.Tests/Json/LinkTemplateTests.cs @@ -50,9 +50,9 @@ public void SetupModels() public void GetResourceWithLinkTemplateRelationship() { var formatter = new JsonApiFormatter - { - PluralizationService = new JSONAPI.Core.PluralizationService() - }; + ( + new JSONAPI.Core.PluralizationService() + ); var stream = new MemoryStream(); formatter.WriteToStreamAsync(typeof(Post), ThePost, stream, null, null); diff --git a/JSONAPI/Core/IModelManager.cs b/JSONAPI/Core/IModelManager.cs index 78c21d83..6faecf9c 100644 --- a/JSONAPI/Core/IModelManager.cs +++ b/JSONAPI/Core/IModelManager.cs @@ -9,6 +9,14 @@ namespace JSONAPI.Core { public interface IModelManager { + IPluralizationService PluralizationService { get; } + PropertyInfo GetIdProperty(Type type); + string GetJsonKeyForType(Type type); + string GetJsonKeyForProperty(PropertyInfo propInfo); //TODO: Do we need to have a type parameter here, in case propInfo is inherited? + PropertyInfo GetPropertyForJsonKey(Type type, string jsonKey); + + [Obsolete] + IDictionary GetPropertyMap(Type type); } } diff --git a/JSONAPI/Core/ModelManager.cs b/JSONAPI/Core/ModelManager.cs index 6b1f511f..1dcacdd9 100644 --- a/JSONAPI/Core/ModelManager.cs +++ b/JSONAPI/Core/ModelManager.cs @@ -10,7 +10,23 @@ namespace JSONAPI.Core { public class ModelManager : IModelManager { - public ModelManager() { } + public ModelManager() { + _pluralizationService = new PluralizationService(); + } + + public ModelManager(IPluralizationService pluralizationService) + { + _pluralizationService = pluralizationService; + } + + private IPluralizationService _pluralizationService = null; + public IPluralizationService PluralizationService + { + get + { + return _pluralizationService; + } + } #region Cache storage @@ -60,7 +76,7 @@ public PropertyInfo GetIdProperty(Type type) #region Property Maps - public Dictionary GetPropertyMap(Type type) + public IDictionary GetPropertyMap(Type type) //FIXME: Will become protected { Dictionary propMap = null; @@ -74,7 +90,7 @@ public Dictionary GetPropertyMap(Type type) PropertyInfo[] props = type.GetProperties(); foreach (PropertyInfo prop in props) { - propMap[JsonApiFormatter.FormatPropertyName(prop.Name)] = prop; + propMap[GetJsonKeyForProperty(prop)] = prop; } propMapCache.Add(type, propMap); @@ -83,13 +99,20 @@ public Dictionary GetPropertyMap(Type type) return propMap; } + public PropertyInfo GetPropertyForJsonKey(Type type, string jsonKey) + { + PropertyInfo propInfo; + if (GetPropertyMap(type).TryGetValue(jsonKey, out propInfo)) return propInfo; + else return null; // Or, throw an exception here?? + } + #endregion //TODO: This has been "moved" here so we can cache the results and improve performance...but // it raises the question of whether the various methods called within here should belong // to JsonApiFormatter at all...should they move here also? Should the IPluralizationService // instance belong to ModelManager instead? - internal string GetJsonKeyForType(Type type, IPluralizationService pluralizationService) + public string GetJsonKeyForType(Type type) { string key = null; @@ -112,12 +135,24 @@ internal string GetJsonKeyForType(Type type, IPluralizationService pluralization if (titles.Any()) title = titles.First(); } - key = JsonApiFormatter.FormatPropertyName(pluralizationService.Pluralize(title)); + key = FormatPropertyName(PluralizationService.Pluralize(title)); keyCache.Add(type, key); } return key; } + + public string GetJsonKeyForProperty(PropertyInfo propInfo) + { + return FormatPropertyName(propInfo.Name); + //TODO: Respect [JsonProperty(PropertyName = "FooBar")], and probably cache the result. + } + + protected static string FormatPropertyName(string propertyName) + { + string result = propertyName.Substring(0, 1).ToLower() + propertyName.Substring(1); + return result; + } } } diff --git a/JSONAPI/Json/JsonApiFormatter.cs b/JSONAPI/Json/JsonApiFormatter.cs index 32d7dd66..ab259ff1 100644 --- a/JSONAPI/Json/JsonApiFormatter.cs +++ b/JSONAPI/Json/JsonApiFormatter.cs @@ -26,8 +26,10 @@ public JsonApiFormatter() SupportedMediaTypes.Add(new MediaTypeHeaderValue("application/vnd.api+json")); } + // Currently for tests only. internal JsonApiFormatter(IErrorSerializer errorSerializer) { + _modelManager = new ModelManager(new PluralizationService()); _errorSerializer = errorSerializer; } @@ -36,7 +38,18 @@ public JsonApiFormatter(IModelManager modelManager) : this() _modelManager = modelManager; } - public IPluralizationService PluralizationService { get; set; } //FIXME: Deprecated, will be removed shortly + public JsonApiFormatter(IPluralizationService pluralizationService) : this() + { + _modelManager = new ModelManager(pluralizationService); + } + + public IPluralizationService PluralizationService //FIXME: Deprecated, will be removed shortly + { + get + { + return _modelManager.PluralizationService; + } + } private readonly IErrorSerializer _errorSerializer; private readonly IModelManager _modelManager; @@ -163,7 +176,7 @@ protected void Serialize(object value, Stream writeStream, JsonWriter writer, Js writer.WriteValue(GetValueForIdProperty(idProp, value)); // Leverage the cached map to avoid another costly call to GetProperties() - PropertyInfo[] props = ModelManager.Instance.GetPropertyMap(value.GetType()).Values.ToArray(); + PropertyInfo[] props = _modelManager.GetPropertyMap(value.GetType()).Values.ToArray(); // Do non-model properties first, everything else goes in "links" //TODO: Unless embedded??? @@ -179,7 +192,7 @@ protected void Serialize(object value, Stream writeStream, JsonWriter writer, Js continue; // numbers, strings, dates... - writer.WritePropertyName(FormatPropertyName(prop.Name)); + writer.WritePropertyName(_modelManager.GetJsonKeyForProperty(prop)); serializer.Serialize(writer, prop.GetValue(value, null)); } else @@ -220,7 +233,7 @@ protected void Serialize(object value, Stream writeStream, JsonWriter writer, Js } if (skip) continue; - writer.WritePropertyName(FormatPropertyName(prop.Name)); + writer.WritePropertyName(_modelManager.GetJsonKeyForProperty(prop)); // Look out! If we want to SerializeAs a link, computing the property is probably // expensive...so don't force it just to check for null early! @@ -236,7 +249,7 @@ protected void Serialize(object value, Stream writeStream, JsonWriter writer, Js switch (sa) { case SerializeAsOptions.Ids: - //writer.WritePropertyName(ContractResolver.FormatPropertyName(prop.Name)); + //writer.WritePropertyName(ContractResolver._modelManager.GetJsonKeyForProperty(prop)); IEnumerable items = (IEnumerable)prop.GetValue(value, null); if (items == null) { @@ -263,7 +276,7 @@ protected void Serialize(object value, Stream writeStream, JsonWriter writer, Js if (lt == null) throw new JsonSerializationException("A property was decorated with SerializeAs(SerializeAsOptions.Link) but no LinkTemplate attribute was provided."); //TODO: Check for "{0}" in linkTemplate and (only) if it's there, get the Ids of all objects and "implode" them. string href = String.Format(lt, null, GetIdFor(value)); - //writer.WritePropertyName(ContractResolver.FormatPropertyName(prop.Name)); + //writer.WritePropertyName(ContractResolver._modelManager.GetJsonKeyForProperty(prop)); //TODO: Support ids and type properties in "link" object writer.WriteStartObject(); writer.WritePropertyName("href"); @@ -272,7 +285,7 @@ protected void Serialize(object value, Stream writeStream, JsonWriter writer, Js break; case SerializeAsOptions.Embedded: // Not really supported by Ember Data yet, incidentally...but easy to implement here. - //writer.WritePropertyName(ContractResolver.FormatPropertyName(prop.Name)); + //writer.WritePropertyName(ContractResolver._modelManager.GetJsonKeyForProperty(prop)); //serializer.Serialize(writer, prop.GetValue(value, null)); this.Serialize(prop.GetValue(value, null), writeStream, writer, serializer, aggregator); break; @@ -292,7 +305,7 @@ protected void Serialize(object value, Stream writeStream, JsonWriter writer, Js switch (sa) { case SerializeAsOptions.Ids: - //writer.WritePropertyName(ContractResolver.FormatPropertyName(prop.Name)); + //writer.WritePropertyName(ContractResolver._modelManager.GetJsonKeyForProperty(prop)); serializer.Serialize(writer, objId); if (iip) if (aggregator != null) @@ -305,7 +318,7 @@ protected void Serialize(object value, Stream writeStream, JsonWriter writer, Js string link = String.Format(lt, objId, GetIdFor(value)); //value.GetType().GetProperty("Id").GetValue(value, null)); - //writer.WritePropertyName(ContractResolver.FormatPropertyName(prop.Name)); + //writer.WritePropertyName(ContractResolver._modelManager.GetJsonKeyForProperty(prop)); writer.WriteStartObject(); writer.WritePropertyName("href"); writer.WriteValue(link); @@ -313,7 +326,7 @@ protected void Serialize(object value, Stream writeStream, JsonWriter writer, Js break; case SerializeAsOptions.Embedded: // Not really supported by Ember Data yet, incidentally...but easy to implement here. - //writer.WritePropertyName(ContractResolver.FormatPropertyName(prop.Name)); + //writer.WritePropertyName(ContractResolver._modelManager.GetJsonKeyForProperty(prop)); //serializer.Serialize(writer, prop.GetValue(value, null)); this.Serialize(prop.GetValue(value, null), writeStream, writer, serializer, aggregator); break; @@ -560,7 +573,7 @@ public object Deserialize(Type objectType, Stream readStream, JsonReader reader, { object retval = Activator.CreateInstance(objectType); - IDictionary propMap = ModelManager.Instance.GetPropertyMap(objectType); + IDictionary propMap = _modelManager.GetPropertyMap(objectType); if (reader.TokenType != JsonToken.StartObject) throw new JsonReaderException(String.Format("Expected JsonToken.StartObject, got {0}", reader.TokenType.ToString())); reader.Read(); // Burn the StartObject token @@ -620,7 +633,7 @@ select prop { if (links == null) break; // Well, apparently we don't have any data for the relationships! - string btId = (string)links[FormatPropertyName(prop.Name)]; + string btId = (string)links[_modelManager.GetJsonKeyForProperty(prop)]; if (btId == null) { prop.SetValue(retval, null, null); // Important that we set--the value may have been cleared! @@ -644,7 +657,7 @@ private void DeserializeLinkedResources(object obj, Stream readStream, JsonReade //reader.Read(); if (reader.TokenType != JsonToken.StartObject) throw new JsonSerializationException("'links' property is not an object!"); - IDictionary propMap = ModelManager.Instance.GetPropertyMap(obj.GetType()); + IDictionary propMap = _modelManager.GetPropertyMap(obj.GetType()); while (reader.Read()) { @@ -759,10 +772,10 @@ private object DeserializePrimitive(Type type, JsonReader reader) #endregion - //TODO: Could be expensive, and is called often...move to ModelManager and cache result? + //TODO: Remove shell function, call _modelManager.GetJsonKeyForType directly. private string GetJsonKeyForType(Type type, dynamic value = null) { - return ModelManager.Instance.GetJsonKeyForType(type, this.PluralizationService); + return _modelManager.GetJsonKeyForType(type); } //private string GetPropertyName(Type type) @@ -770,14 +783,6 @@ private string GetJsonKeyForType(Type type, dynamic value = null) // return FormatPropertyName(PluralizationService.Pluralize(type.Name)); //} - private bool IsMany(dynamic value = null) - { - if (value != null && (value is IEnumerable || value.GetType().IsArray)) - return true; - else - return false; - } - internal static bool IsMany(Type type) { return @@ -795,12 +800,13 @@ internal static Type GetSingleType(Type type)//dynamic value = null) return type; } - //TODO: Should this move to ModelManager? Could be cached there to improve performance? + /* public static string FormatPropertyName(string propertyName) { string result = propertyName.Substring(0, 1).ToLower() + propertyName.Substring(1); return result; } + */ protected object GetById(Type type, string id) { From eece47c13a4bad1118d5b4d31aa9a7c620f694d6 Mon Sep 17 00:00:00 2001 From: S'pht'Kr Date: Wed, 28 Jan 2015 11:32:01 +0100 Subject: [PATCH 4/8] Finished removing `GetPropertyMap`, adds `GetProperties` to accomodate one of the use cases. Added tests. --- JSONAPI.Tests/Core/ModelManagerTests.cs | 51 ++++++++++++++++++------- JSONAPI/Core/IModelManager.cs | 15 ++++++-- JSONAPI/Core/ModelManager.cs | 7 +++- JSONAPI/Json/JsonApiFormatter.cs | 14 +++---- 4 files changed, 62 insertions(+), 25 deletions(-) diff --git a/JSONAPI.Tests/Core/ModelManagerTests.cs b/JSONAPI.Tests/Core/ModelManagerTests.cs index 1fa02477..1e7f6c74 100644 --- a/JSONAPI.Tests/Core/ModelManagerTests.cs +++ b/JSONAPI.Tests/Core/ModelManagerTests.cs @@ -42,35 +42,60 @@ public void DoesntFindMissingId() } [TestMethod] - public void GetPropertyMapTest() + public void GetJsonKeyForTypeTest() { // Arrange - var mm = new ModelManager(); + var pluralizationService = new PluralizationService(); + var mm = new ModelManager(pluralizationService); // Act - var propMap = mm.GetPropertyMap(typeof(Post)); - + var postKey = mm.GetJsonKeyForType(typeof(Post)); + var authorKey = mm.GetJsonKeyForType(typeof(Author)); + var commentKey = mm.GetJsonKeyForType(typeof(Comment)); + // Assert - Assert.AreSame(typeof(Post).GetProperty("Id"), propMap["id"]); - Assert.AreSame(typeof(Post).GetProperty("Author"), propMap["author"]); + Assert.AreEqual("posts", postKey); + Assert.AreEqual("authors", authorKey); + Assert.AreEqual("comments", commentKey); } [TestMethod] - public void GetJsonKeyForTypeTest() + public void GetJsonKeyForPropertyTest() { // Arrange var pluralizationService = new PluralizationService(); var mm = new ModelManager(pluralizationService); // Act - var postKey = mm.GetJsonKeyForType(typeof(Post)); - var authorKey = mm.GetJsonKeyForType(typeof(Author)); - var commentKey = mm.GetJsonKeyForType(typeof(Comment)); + var idKey = mm.GetJsonKeyForProperty(typeof(Author).GetProperty("Id")); + var nameKey = mm.GetJsonKeyForProperty(typeof(Author).GetProperty("Name")); + var postsKey = mm.GetJsonKeyForProperty(typeof(Author).GetProperty("Posts")); // Assert - Assert.AreEqual("posts", postKey); - Assert.AreEqual("authors", authorKey); - Assert.AreEqual("comments", commentKey); + Assert.AreEqual("id", idKey); + Assert.AreEqual("name", nameKey); + Assert.AreEqual("posts", postsKey); + + } + + [TestMethod] + public void GetPropertyForJsonKeyTest() + { + // Arrange + var pluralizationService = new PluralizationService(); + var mm = new ModelManager(pluralizationService); + Type authorType = typeof(Author).GetType(); + + // Act + var idProp = mm.GetPropertyForJsonKey(authorType, "id"); + var nameProp = mm.GetPropertyForJsonKey(authorType, "name"); + var postsProp = mm.GetPropertyForJsonKey(authorType, "posts"); + + // Assert + Assert.AreSame(authorType.GetProperty("Id"), idProp); + Assert.AreSame(authorType.GetProperty("Name"), nameProp); + Assert.AreSame(authorType.GetProperty("Posts"), postsProp); + } } } diff --git a/JSONAPI/Core/IModelManager.cs b/JSONAPI/Core/IModelManager.cs index 6faecf9c..e15674da 100644 --- a/JSONAPI/Core/IModelManager.cs +++ b/JSONAPI/Core/IModelManager.cs @@ -13,10 +13,19 @@ public interface IModelManager PropertyInfo GetIdProperty(Type type); string GetJsonKeyForType(Type type); - string GetJsonKeyForProperty(PropertyInfo propInfo); //TODO: Do we need to have a type parameter here, in case propInfo is inherited? + string GetJsonKeyForProperty(PropertyInfo propInfo); //TODO: Do we need to have a type parameter here, in case the property is inherited? PropertyInfo GetPropertyForJsonKey(Type type, string jsonKey); - [Obsolete] - IDictionary GetPropertyMap(Type type); + /// + /// Analogue to System.Type.GetProperties(), but made available so that any caching done + /// by an IModelManager can be leveraged to return the results faster. + /// + /// The type to get properties from + /// All properties recognized by the IModelManager. + //TODO: This needs to include JsonIgnore'd properties, so that they can be found and explicitly included at runtime...confusing? Add another method that excludes these? + PropertyInfo[] GetProperties(Type type); + + //[Obsolete] + //IDictionary GetPropertyMap(Type type); } } diff --git a/JSONAPI/Core/ModelManager.cs b/JSONAPI/Core/ModelManager.cs index 1dcacdd9..d86bb946 100644 --- a/JSONAPI/Core/ModelManager.cs +++ b/JSONAPI/Core/ModelManager.cs @@ -76,7 +76,7 @@ public PropertyInfo GetIdProperty(Type type) #region Property Maps - public IDictionary GetPropertyMap(Type type) //FIXME: Will become protected + protected IDictionary GetPropertyMap(Type type) //FIXME: Will become protected { Dictionary propMap = null; @@ -99,6 +99,11 @@ public IDictionary GetPropertyMap(Type type) //FIXME: Will return propMap; } + public PropertyInfo[] GetProperties(Type type) + { + return GetPropertyMap(type).Values.ToArray(); + } + public PropertyInfo GetPropertyForJsonKey(Type type, string jsonKey) { PropertyInfo propInfo; diff --git a/JSONAPI/Json/JsonApiFormatter.cs b/JSONAPI/Json/JsonApiFormatter.cs index ab259ff1..10fd3e25 100644 --- a/JSONAPI/Json/JsonApiFormatter.cs +++ b/JSONAPI/Json/JsonApiFormatter.cs @@ -175,8 +175,8 @@ protected void Serialize(object value, Stream writeStream, JsonWriter writer, Js var idProp = _modelManager.GetIdProperty(value.GetType()); writer.WriteValue(GetValueForIdProperty(idProp, value)); - // Leverage the cached map to avoid another costly call to GetProperties() - PropertyInfo[] props = _modelManager.GetPropertyMap(value.GetType()).Values.ToArray(); + // Leverage the cached map to avoid another costly call to System.Type.GetProperties() + PropertyInfo[] props = _modelManager.GetProperties(value.GetType()); // Do non-model properties first, everything else goes in "links" //TODO: Unless embedded??? @@ -573,8 +573,6 @@ public object Deserialize(Type objectType, Stream readStream, JsonReader reader, { object retval = Activator.CreateInstance(objectType); - IDictionary propMap = _modelManager.GetPropertyMap(objectType); - if (reader.TokenType != JsonToken.StartObject) throw new JsonReaderException(String.Format("Expected JsonToken.StartObject, got {0}", reader.TokenType.ToString())); reader.Read(); // Burn the StartObject token do @@ -589,7 +587,7 @@ public object Deserialize(Type objectType, Stream readStream, JsonReader reader, //TODO: linked resources (Done??) DeserializeLinkedResources(retval, readStream, reader, serializer); } - else if (propMap.TryGetValue(value, out prop)) + else if ((prop = _modelManager.GetPropertyForJsonKey(objectType, value)) != null) { reader.Read(); // burn the PropertyName token //TODO: Embedded would be dropped here! @@ -657,7 +655,7 @@ private void DeserializeLinkedResources(object obj, Stream readStream, JsonReade //reader.Read(); if (reader.TokenType != JsonToken.StartObject) throw new JsonSerializationException("'links' property is not an object!"); - IDictionary propMap = _modelManager.GetPropertyMap(obj.GetType()); + Type objectType = obj.GetType(); while (reader.Read()) { @@ -665,8 +663,8 @@ private void DeserializeLinkedResources(object obj, Stream readStream, JsonReade { string value = (string)reader.Value; reader.Read(); // burn the PropertyName token - PropertyInfo prop; - if (propMap.TryGetValue(value, out prop) && !CanWriteTypeAsPrimitive(prop.PropertyType)) + PropertyInfo prop = _modelManager.GetPropertyForJsonKey(objectType, value); + if (prop != null && !CanWriteTypeAsPrimitive(prop.PropertyType)) { //FIXME: We're really assuming they're ICollections...but testing for that doesn't work for some reason. Break prone! if (prop.PropertyType.GetInterfaces().Contains(typeof(IEnumerable)) && prop.PropertyType.IsGenericType) From 547de42da82266192f184c15bf9cb622505fddea Mon Sep 17 00:00:00 2001 From: S'pht'Kr Date: Wed, 28 Jan 2015 13:31:15 +0100 Subject: [PATCH 5/8] Finished refactoring ModelManager, now there is no circular dependency with JsonApiFormatter, and the interface boundary is much more clear. --- JSONAPI.Tests/Core/ModelManagerTests.cs | 53 +++++++++++++++++++++++- JSONAPI/Core/IModelManager.cs | 51 ++++++++++++++++++++++- JSONAPI/Core/ModelManager.cs | 29 ++++++++++--- JSONAPI/Json/JsonApiFormatter.cs | 54 ++++--------------------- 4 files changed, 132 insertions(+), 55 deletions(-) diff --git a/JSONAPI.Tests/Core/ModelManagerTests.cs b/JSONAPI.Tests/Core/ModelManagerTests.cs index 1e7f6c74..0348ef55 100644 --- a/JSONAPI.Tests/Core/ModelManagerTests.cs +++ b/JSONAPI.Tests/Core/ModelManagerTests.cs @@ -3,13 +3,15 @@ using JSONAPI.Core; using JSONAPI.Tests.Models; using System.Reflection; +using System.Collections.Generic; +using System.Collections; namespace JSONAPI.Tests.Core { [TestClass] public class ModelManagerTests { - private class InvalidModel + private class InvalidModel // No Id discernable! { public string Data { get; set; } } @@ -97,5 +99,54 @@ public void GetPropertyForJsonKeyTest() Assert.AreSame(authorType.GetProperty("Posts"), postsProp); } + + [TestMethod] + public void IsSerializedAsManyTest() + { + // Arrange + var mm = new ModelManager(); + + // Act + bool isArray = mm.IsSerializedAsMany(typeof(Post[])); + bool isGenericEnumerable = mm.IsSerializedAsMany(typeof(IEnumerable)); + bool isString = mm.IsSerializedAsMany(typeof(string)); + bool isAuthor = mm.IsSerializedAsMany(typeof(Author)); + bool isNonGenericEnumerable = mm.IsSerializedAsMany(typeof(IEnumerable)); + + // Assert + Assert.IsTrue(isArray); + Assert.IsTrue(isGenericEnumerable); + Assert.IsFalse(isString); + Assert.IsFalse(isAuthor); + Assert.IsFalse(isNonGenericEnumerable); + } + + [TestMethod] + public void GetElementTypeTest() + { + // Arrange + var mm = new ModelManager(); + + // Act + Type postTypeFromArray = mm.GetElementType(typeof(Post[])); + Type postTypeFromEnumerable = mm.GetElementType(typeof(IEnumerable)); + + // Assert + Assert.AreSame(typeof(Post), postTypeFromArray); + Assert.AreSame(typeof(Post), postTypeFromEnumerable); + } + + [TestMethod] + public void GetElementTypeInvalidArgumentTest() + { + // Arrange + var mm = new ModelManager(); + + // Act + Type x = mm.GetElementType(typeof(Author)); + + // Assert + Assert.IsNull(x, "Return value of GetElementType should be null for a non-Many type argument!"); + } } } diff --git a/JSONAPI/Core/IModelManager.cs b/JSONAPI/Core/IModelManager.cs index e15674da..6eae86c7 100644 --- a/JSONAPI/Core/IModelManager.cs +++ b/JSONAPI/Core/IModelManager.cs @@ -11,9 +11,42 @@ public interface IModelManager { IPluralizationService PluralizationService { get; } + /// + /// Returns the property that is treated as the unique identifier in a given class. + /// This is used most importantly by JsonApiFormatter to determine what value to + /// write when serializing a "Many" relationship as an array of Ids. It is also + /// used to make dummy related objects (with only the Id property set) when + /// deserializing a JSON payload that specifies a related object only by Id. + /// + /// Rules for determining this may vary by implementation. + /// + /// + /// The property determined to represent the Id. PropertyInfo GetIdProperty(Type type); + + /// + /// Returns the key that will be used to represent a collection of objects of a + /// given type, for example in the top-level of a JSON API document or within + /// the "linked" objects section of a payload. + /// + /// The serializable Type + /// The string denoting the given type in JSON documents. string GetJsonKeyForType(Type type); + + /// + /// Returns the key that will be used to represent the given property in serialized + /// JSON. Inverse of GetPropertyForJsonKey. + /// + /// The serializable property + /// The string denoting the given property within a JSON document. string GetJsonKeyForProperty(PropertyInfo propInfo); //TODO: Do we need to have a type parameter here, in case the property is inherited? + + /// + /// Returns the property corresponding to a given JSON Key. Inverse of GetJsonKeyForType. + /// + /// The Type to find the property on + /// The JSON key representing a property + /// PropertyInfo GetPropertyForJsonKey(Type type, string jsonKey); /// @@ -25,7 +58,21 @@ public interface IModelManager //TODO: This needs to include JsonIgnore'd properties, so that they can be found and explicitly included at runtime...confusing? Add another method that excludes these? PropertyInfo[] GetProperties(Type type); - //[Obsolete] - //IDictionary GetPropertyMap(Type type); + /// + /// Determines whether or not the given type will be treated as a "Many" relationship. + /// + /// The serializable Type + /// True for Array and IEnumerable<T> types, false otherwise. + bool IsSerializedAsMany(Type type); + + /// + /// Analogue for System.Type.GetElementType, but works for arrays or IEnumerable<T>, + /// and provides a capture point to cache potentially expensive reflection operations that + /// have to occur repeatedly in JsonApiFormatter. + /// + /// A type which must be either an Array type or implement IEnumerable<T>. + /// The element type of an Array, or the first generic parameter of an IEnumerable<T>. + Type GetElementType(Type manyType); + } } diff --git a/JSONAPI/Core/ModelManager.cs b/JSONAPI/Core/ModelManager.cs index d86bb946..66584701 100644 --- a/JSONAPI/Core/ModelManager.cs +++ b/JSONAPI/Core/ModelManager.cs @@ -113,10 +113,6 @@ public PropertyInfo GetPropertyForJsonKey(Type type, string jsonKey) #endregion - //TODO: This has been "moved" here so we can cache the results and improve performance...but - // it raises the question of whether the various methods called within here should belong - // to JsonApiFormatter at all...should they move here also? Should the IPluralizationService - // instance belong to ModelManager instead? public string GetJsonKeyForType(Type type) { string key = null; @@ -127,8 +123,8 @@ public string GetJsonKeyForType(Type type) { if (keyCache.TryGetValue(type, out key)) return key; - if (JsonApiFormatter.IsMany(type)) - type = JsonApiFormatter.GetSingleType(type); + if (IsSerializedAsMany(type)) + type = GetElementType(type); var attrs = type.CustomAttributes.Where(x => x.AttributeType == typeof(Newtonsoft.Json.JsonObjectAttribute)).ToList(); @@ -159,5 +155,26 @@ protected static string FormatPropertyName(string propertyName) string result = propertyName.Substring(0, 1).ToLower() + propertyName.Substring(1); return result; } + + public bool IsSerializedAsMany(Type type) + { + bool isMany = + type.IsArray || + (type.GetInterfaces().Contains(typeof(System.Collections.IEnumerable)) && type.IsGenericType); + + return isMany; + } + + public Type GetElementType(Type manyType) + { + Type etype = null; + if (manyType.IsGenericType) + etype = manyType.GetGenericArguments()[0]; + else + etype = manyType.GetElementType(); + + return etype; + } + } } diff --git a/JSONAPI/Json/JsonApiFormatter.cs b/JSONAPI/Json/JsonApiFormatter.cs index 10fd3e25..b47c67a0 100644 --- a/JSONAPI/Json/JsonApiFormatter.cs +++ b/JSONAPI/Json/JsonApiFormatter.cs @@ -119,18 +119,18 @@ public override Task WriteToStreamAsync(System.Type type, object value, Stream w else { Type valtype = GetSingleType(value.GetType()); - if (IsMany(value.GetType())) + if (_modelManager.IsSerializedAsMany(value.GetType())) aggregator.AddPrimary(valtype, (IEnumerable) value); else aggregator.AddPrimary(valtype, value); //writer.Formatting = Formatting.Indented; - var root = GetJsonKeyForType(type, value); + var root = _modelManager.GetJsonKeyForType(type); writer.WriteStartObject(); writer.WritePropertyName(root); - if (IsMany(value.GetType())) + if (_modelManager.IsSerializedAsMany(value.GetType())) this.SerializeMany(value, writeStream, writer, serializer, aggregator); else this.Serialize(value, writeStream, writer, serializer, aggregator); @@ -422,7 +422,7 @@ protected void SerializeLinkedResources(Stream writeStream, JsonWriter writer, J foreach (KeyValuePair> apair in writers) { apair.Value.Key.WriteEnd(); // close off the array - writer.WritePropertyName(GetJsonKeyForType(apair.Key)); + writer.WritePropertyName(_modelManager.GetJsonKeyForType(apair.Key)); writer.WriteRawValue(apair.Value.Value.ToString()); // write the contents of the type JsonWriter's StringWriter to the main JsonWriter } @@ -445,7 +445,7 @@ private object ReadFromStream(Type type, Stream readStream, HttpContent content, { object retval = null; Type singleType = GetSingleType(type); - var pripropname = GetJsonKeyForType(type); + var pripropname = _modelManager.GetJsonKeyForType(type); var contentHeaders = content == null ? null : content.Headers; // If content length is 0 then return default value for this type @@ -522,7 +522,7 @@ private object ReadFromStream(Type type, Stream readStream, HttpContent content, */ if (retval != null) { - if (!type.IsAssignableFrom(retval.GetType()) && IsMany(type)) + if (!type.IsAssignableFrom(retval.GetType()) && _modelManager.IsSerializedAsMany(type)) { IList list = (IList)Activator.CreateInstance(typeof(List<>).MakeGenericType(singleType)); list.Add(retval); @@ -770,42 +770,11 @@ private object DeserializePrimitive(Type type, JsonReader reader) #endregion - //TODO: Remove shell function, call _modelManager.GetJsonKeyForType directly. - private string GetJsonKeyForType(Type type, dynamic value = null) + private Type GetSingleType(Type type)//dynamic value = null) { - return _modelManager.GetJsonKeyForType(type); + return _modelManager.IsSerializedAsMany(type) ? _modelManager.GetElementType(type) : type; } - //private string GetPropertyName(Type type) - //{ - // return FormatPropertyName(PluralizationService.Pluralize(type.Name)); - //} - - internal static bool IsMany(Type type) - { - return - type.IsArray || - (type.GetInterfaces().Contains(typeof(IEnumerable)) && type.IsGenericType); - } - - internal static Type GetSingleType(Type type)//dynamic value = null) - { - if (IsMany(type)) - if (type.IsGenericType) - return type.GetGenericArguments()[0]; - else - return type.GetElementType(); - return type; - } - - /* - public static string FormatPropertyName(string propertyName) - { - string result = propertyName.Substring(0, 1).ToLower() + propertyName.Substring(1); - return result; - } - */ - protected object GetById(Type type, string id) { // Only good for creating dummy relationship objects... @@ -815,13 +784,6 @@ protected object GetById(Type type, string id) return retval; } - /* - protected PropertyInfo GetIdProperty(Type type) - { - return type.GetProperty("Id"); - } - */ - protected string GetValueForIdProperty(PropertyInfo idprop, object obj) { if (idprop != null) From dac651b5be8ebcf706902e595492a33717a7fc9c Mon Sep 17 00:00:00 2001 From: S'pht'Kr Date: Wed, 28 Jan 2015 13:47:51 +0100 Subject: [PATCH 6/8] We're going to need access to the IModelManager from EnableFilteringAttribute and elsewhere! --- JSONAPI/Json/JsonApiFormatter.cs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/JSONAPI/Json/JsonApiFormatter.cs b/JSONAPI/Json/JsonApiFormatter.cs index b47c67a0..47ae724b 100644 --- a/JSONAPI/Json/JsonApiFormatter.cs +++ b/JSONAPI/Json/JsonApiFormatter.cs @@ -43,6 +43,7 @@ public JsonApiFormatter(IPluralizationService pluralizationService) : this() _modelManager = new ModelManager(pluralizationService); } + [Obsolete] public IPluralizationService PluralizationService //FIXME: Deprecated, will be removed shortly { get @@ -50,8 +51,17 @@ public JsonApiFormatter(IPluralizationService pluralizationService) : this() return _modelManager.PluralizationService; } } + private readonly IErrorSerializer _errorSerializer; + private readonly IModelManager _modelManager; + public IModelManager ModelManager + { + get + { + return _modelManager; + } + } private Lazy> _relationAggregators = new Lazy>( From 73a5d90c6bbc78e4af49a14b28b5bf3c3bb2fc04 Mon Sep 17 00:00:00 2001 From: Chris Santero Date: Wed, 28 Jan 2015 11:53:01 -0500 Subject: [PATCH 7/8] rearrange constructors --- JSONAPI/Json/JsonApiFormatter.cs | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/JSONAPI/Json/JsonApiFormatter.cs b/JSONAPI/Json/JsonApiFormatter.cs index 47ae724b..a80fbbcc 100644 --- a/JSONAPI/Json/JsonApiFormatter.cs +++ b/JSONAPI/Json/JsonApiFormatter.cs @@ -20,27 +20,32 @@ namespace JSONAPI.Json public class JsonApiFormatter : JsonMediaTypeFormatter { public JsonApiFormatter() - : this(new ErrorSerializer()) + : this(new ModelManager(), new ErrorSerializer()) { - if (_modelManager == null) _modelManager = new ModelManager(); - SupportedMediaTypes.Add(new MediaTypeHeaderValue("application/vnd.api+json")); } - // Currently for tests only. - internal JsonApiFormatter(IErrorSerializer errorSerializer) + public JsonApiFormatter(IModelManager modelManager) : + this(modelManager, new ErrorSerializer()) { - _modelManager = new ModelManager(new PluralizationService()); - _errorSerializer = errorSerializer; } - public JsonApiFormatter(IModelManager modelManager) : this() + public JsonApiFormatter(IPluralizationService pluralizationService) : + this(new ModelManager(pluralizationService)) { - _modelManager = modelManager; } - public JsonApiFormatter(IPluralizationService pluralizationService) : this() + // Currently for tests only. + internal JsonApiFormatter(IErrorSerializer errorSerializer) + : this(new ModelManager(), errorSerializer) + { + + } + + internal JsonApiFormatter(IModelManager modelManager, IErrorSerializer errorSerializer) { - _modelManager = new ModelManager(pluralizationService); + _modelManager = modelManager; + _errorSerializer = errorSerializer; + SupportedMediaTypes.Add(new MediaTypeHeaderValue("application/vnd.api+json")); } [Obsolete] From 5558fcfd531e5cdc6fa28e7b725194ccea6dcb40 Mon Sep 17 00:00:00 2001 From: S'pht'Kr Date: Thu, 29 Jan 2015 10:00:17 +0100 Subject: [PATCH 8/8] Incorporated feedback from #25, including removing unneeded default constructors. Added caching to `IsSerializedAsMany` and `GetElementType`. Made `ModelManager` a little more subclassing-friendly. --- JSONAPI.Tests/Core/ModelManagerTests.cs | 10 ++-- .../Json/JsonApiMediaFormaterTests.cs | 2 +- JSONAPI/Core/IModelManager.cs | 2 +- JSONAPI/Core/ModelManager.cs | 59 ++++++++++++++----- JSONAPI/Json/JsonApiFormatter.cs | 9 +-- 5 files changed, 52 insertions(+), 30 deletions(-) diff --git a/JSONAPI.Tests/Core/ModelManagerTests.cs b/JSONAPI.Tests/Core/ModelManagerTests.cs index 0348ef55..5bbe7198 100644 --- a/JSONAPI.Tests/Core/ModelManagerTests.cs +++ b/JSONAPI.Tests/Core/ModelManagerTests.cs @@ -20,7 +20,7 @@ private class InvalidModel // No Id discernable! public void FindsIdNamedId() { // Arrange - var mm = new ModelManager(); + var mm = new ModelManager(new PluralizationService()); // Act PropertyInfo idprop = mm.GetIdProperty(typeof(Author)); @@ -34,7 +34,7 @@ public void FindsIdNamedId() public void DoesntFindMissingId() { // Arrange - var mm = new ModelManager(); + var mm = new ModelManager(new PluralizationService()); // Act PropertyInfo idprop = mm.GetIdProperty(typeof(InvalidModel)); @@ -104,7 +104,7 @@ public void GetPropertyForJsonKeyTest() public void IsSerializedAsManyTest() { // Arrange - var mm = new ModelManager(); + var mm = new ModelManager(new PluralizationService()); // Act bool isArray = mm.IsSerializedAsMany(typeof(Post[])); @@ -125,7 +125,7 @@ public void IsSerializedAsManyTest() public void GetElementTypeTest() { // Arrange - var mm = new ModelManager(); + var mm = new ModelManager(new PluralizationService()); // Act Type postTypeFromArray = mm.GetElementType(typeof(Post[])); @@ -140,7 +140,7 @@ public void GetElementTypeTest() public void GetElementTypeInvalidArgumentTest() { // Arrange - var mm = new ModelManager(); + var mm = new ModelManager(new PluralizationService()); // Act Type x = mm.GetElementType(typeof(Author)); diff --git a/JSONAPI.Tests/Json/JsonApiMediaFormaterTests.cs b/JSONAPI.Tests/Json/JsonApiMediaFormaterTests.cs index e2074b0e..1fd8a6e8 100644 --- a/JSONAPI.Tests/Json/JsonApiMediaFormaterTests.cs +++ b/JSONAPI.Tests/Json/JsonApiMediaFormaterTests.cs @@ -107,7 +107,7 @@ private enum TestEnum public void CanWritePrimitiveTest() { // Arrange - JsonApiFormatter formatter = new JSONAPI.Json.JsonApiFormatter(); + JsonApiFormatter formatter = new JSONAPI.Json.JsonApiFormatter(new PluralizationService()); // Act // Assert Assert.IsTrue(formatter.CanWriteTypeAsPrimitive(typeof(Int32)), "CanWriteTypeAsPrimitive returned wrong answer for Integer!"); diff --git a/JSONAPI/Core/IModelManager.cs b/JSONAPI/Core/IModelManager.cs index 6eae86c7..8f5a0305 100644 --- a/JSONAPI/Core/IModelManager.cs +++ b/JSONAPI/Core/IModelManager.cs @@ -42,7 +42,7 @@ public interface IModelManager string GetJsonKeyForProperty(PropertyInfo propInfo); //TODO: Do we need to have a type parameter here, in case the property is inherited? /// - /// Returns the property corresponding to a given JSON Key. Inverse of GetJsonKeyForType. + /// Returns the property corresponding to a given JSON Key. Inverse of GetJsonKeyForProperty. /// /// The Type to find the property on /// The JSON key representing a property diff --git a/JSONAPI/Core/ModelManager.cs b/JSONAPI/Core/ModelManager.cs index 66584701..5dc49bcc 100644 --- a/JSONAPI/Core/ModelManager.cs +++ b/JSONAPI/Core/ModelManager.cs @@ -10,16 +10,12 @@ namespace JSONAPI.Core { public class ModelManager : IModelManager { - public ModelManager() { - _pluralizationService = new PluralizationService(); - } - public ModelManager(IPluralizationService pluralizationService) { _pluralizationService = pluralizationService; } - private IPluralizationService _pluralizationService = null; + protected IPluralizationService _pluralizationService = null; public IPluralizationService PluralizationService { get @@ -30,21 +26,31 @@ public IPluralizationService PluralizationService #region Cache storage - private Lazy> _idProperties + protected Lazy> _idProperties = new Lazy>( () => new Dictionary() ); - private Lazy>> _propertyMaps + protected Lazy>> _propertyMaps = new Lazy>>( () => new Dictionary>() ); - private Lazy> _jsonKeysForType + protected Lazy> _jsonKeysForType = new Lazy>( () => new Dictionary() ); + protected Lazy> _isSerializedAsMany + = new Lazy>( + () => new Dictionary() + ); + + protected Lazy> _getElementType + = new Lazy>( + () => new Dictionary() + ); + #endregion #region Id property determination @@ -76,7 +82,7 @@ public PropertyInfo GetIdProperty(Type type) #region Property Maps - protected IDictionary GetPropertyMap(Type type) //FIXME: Will become protected + protected IDictionary GetPropertyMap(Type type) { Dictionary propMap = null; @@ -158,9 +164,20 @@ protected static string FormatPropertyName(string propertyName) public bool IsSerializedAsMany(Type type) { - bool isMany = - type.IsArray || - (type.GetInterfaces().Contains(typeof(System.Collections.IEnumerable)) && type.IsGenericType); + bool isMany; + + var isManyCache = _isSerializedAsMany.Value; + + lock (isManyCache) + { + if (isManyCache.TryGetValue(type, out isMany)) return isMany; + + isMany = + type.IsArray || + (type.GetInterfaces().Contains(typeof(System.Collections.IEnumerable)) && type.IsGenericType); + + isManyCache.Add(type, isMany); + } return isMany; } @@ -168,10 +185,20 @@ public bool IsSerializedAsMany(Type type) public Type GetElementType(Type manyType) { Type etype = null; - if (manyType.IsGenericType) - etype = manyType.GetGenericArguments()[0]; - else - etype = manyType.GetElementType(); + + var etypeCache = _getElementType.Value; + + lock (etypeCache) + { + if (etypeCache.TryGetValue(manyType, out etype)) return etype; + + if (manyType.IsGenericType) + etype = manyType.GetGenericArguments()[0]; + else + etype = manyType.GetElementType(); + + etypeCache.Add(manyType, etype); + } return etype; } diff --git a/JSONAPI/Json/JsonApiFormatter.cs b/JSONAPI/Json/JsonApiFormatter.cs index a80fbbcc..c678b402 100644 --- a/JSONAPI/Json/JsonApiFormatter.cs +++ b/JSONAPI/Json/JsonApiFormatter.cs @@ -19,11 +19,6 @@ namespace JSONAPI.Json { public class JsonApiFormatter : JsonMediaTypeFormatter { - public JsonApiFormatter() - : this(new ModelManager(), new ErrorSerializer()) - { - } - public JsonApiFormatter(IModelManager modelManager) : this(modelManager, new ErrorSerializer()) { @@ -36,7 +31,7 @@ public JsonApiFormatter(IPluralizationService pluralizationService) : // Currently for tests only. internal JsonApiFormatter(IErrorSerializer errorSerializer) - : this(new ModelManager(), errorSerializer) + : this(new ModelManager(new PluralizationService()), errorSerializer) { } @@ -48,7 +43,7 @@ internal JsonApiFormatter(IModelManager modelManager, IErrorSerializer errorSeri SupportedMediaTypes.Add(new MediaTypeHeaderValue("application/vnd.api+json")); } - [Obsolete] + [Obsolete("Use ModelManager.PluralizationService instead")] public IPluralizationService PluralizationService //FIXME: Deprecated, will be removed shortly { get