Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Serializer will now ignore: indexer properties, and properties with their name listed in a JsonIgnore above the declaring class #299

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
64 changes: 64 additions & 0 deletions nanoFramework.Json/JsonIgnoreAttribute.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
using System;
using System.Collections;
using System.Text;

namespace nanoFramework.Json
{

icy3141 marked this conversation as resolved.
Show resolved Hide resolved



/// <summary>
/// Hides properties from the json serializer
icy3141 marked this conversation as resolved.
Show resolved Hide resolved
/// </summary>
[System.AttributeUsage(AttributeTargets.Class, Inherited = false, AllowMultiple = true)]
public sealed class JsonIgnoreAttribute : Attribute
{
// See the attribute guidelines at
// http://go.microsoft.com/fwlink/?LinkId=85236

icy3141 marked this conversation as resolved.
Show resolved Hide resolved
/// <summary>
/// Hides properties from the json serializer
/// </summary>
public JsonIgnoreAttribute() {

PropertyNames = new string[0];
}
/// <summary>
icy3141 marked this conversation as resolved.
Show resolved Hide resolved
/// array of property names for json serializer to ignore
icy3141 marked this conversation as resolved.
Show resolved Hide resolved
/// </summary>
public string[] PropertyNames { get; set; }

/// <summary>
/// Hides properties from the json serializer
/// </summary>
/// <param name="getterNamesToIgnore">a comma separated list of property names to ignore in json</param>
public JsonIgnoreAttribute(string getterNamesToIgnore)
{
//split by commas, then load array
PropertyNames = getterNamesToIgnore.Split(',');
for(int i = 0; i < PropertyNames.Length; i++)
{
PropertyNames[i] = PropertyNames[i].Trim();
}
}
}

//implementation to place above individual properties
icy3141 marked this conversation as resolved.
Show resolved Hide resolved
///// <summary>
///// Hides a property from the json serializer
///// </summary>
//[System.AttributeUsage(AttributeTargets.Property | AttributeTargets.Method,
// Inherited = false, AllowMultiple = true)]
//public sealed class JsonIgnoreAttribute : Attribute
//{
// // See the attribute guidelines at
// // http://go.microsoft.com/fwlink/?LinkId=85236

// /// <summary>
// /// Hides a property from the json serializer
// /// </summary>
// public JsonIgnoreAttribute() { }
//}

}
47 changes: 47 additions & 0 deletions nanoFramework.Json/JsonSerializer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -125,9 +125,56 @@ private static bool ShouldSerializeMethod(MethodInfo method)
return false;
}

//Ignore property getters with at least one parameter (this[index] operator overloads)
icy3141 marked this conversation as resolved.
Show resolved Hide resolved
ParameterInfo[] parameters = method.GetParameters();
if (parameters.Length > 0)
{
return false;
}
parameters = null;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing enter before the line

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and why setting parameters to null?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My thought was, this ParameterInfo[] could take up memory needed for other things, and if it's set to null before the end of the function, the garbage collector might get it sooner. TBH, I don't know enough about how the garbage collector works. If my assumption was untrue, then my bad. Thinking about it more, I guess the function would at least try to finish in a time slice if it can, and then the parameters variable wouldn't exist anymore anyway.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While benchmarking, removed the call to GetParameters() altogether and replaced with method.Name == "get_Item". Much better performance with the same effect!
For example, ComplexArrayObject benchmark got a mean of 446 ms with GetParameters(), and 382ms with string check.
image

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I love when optimizations are coming all along with this benchmark tool and some good thinking! Great work on this!


// Ignore properties listed in [JsonIgnore()] attribute
if (ShouldIgnorePropertyFromClassAttribute(method))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For performance reasons, please add a new option in the sterilizer so you call this only if the ignore option is on. The cost is VERY high in terms of performances when this is calls every time.
Also, please cache this because as you place it on the class level, you should call it only once, not for every property.
By having those 2 things implemented, you can have a decent performance.
And please run the performance tool without your code and with your code, so we can compare.

return false;
icy3141 marked this conversation as resolved.
Show resolved Hide resolved

// per property [JsonIgnore()] attribute - not working due to method.GetCustomAttributes returning empty
icy3141 marked this conversation as resolved.
Show resolved Hide resolved
//var attributes = method.GetCustomAttributes(false);
//foreach (var attributeInfo in attributes)
//{
// if(typeof(JsonIgnoreAttribute).IsInstanceOfType(attributeInfo))
// {
// return false;
// }
//}

return true;
}

/// <summary>
/// split out method to check for ignore attribute
icy3141 marked this conversation as resolved.
Show resolved Hide resolved
/// </summary>
private static bool ShouldIgnorePropertyFromClassAttribute(MethodInfo method)
{
string[] gettersToIgnore = null;
object[] classAttributes = method.DeclaringType.GetCustomAttributes(true);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that should be cashed for a specific class

foreach (object attribute in classAttributes)
{
if (attribute is JsonIgnoreAttribute ignoreAttribute)
{
gettersToIgnore = ignoreAttribute.PropertyNames;
break;
}
}
classAttributes = null;
icy3141 marked this conversation as resolved.
Show resolved Hide resolved
if (gettersToIgnore == null) return false;
icy3141 marked this conversation as resolved.
Show resolved Hide resolved
foreach (string propertyName in gettersToIgnore)
{
if (propertyName.Equals(method.Name.Substring(4)))
icy3141 marked this conversation as resolved.
Show resolved Hide resolved
return true;
icy3141 marked this conversation as resolved.
Show resolved Hide resolved
}
return false;
icy3141 marked this conversation as resolved.
Show resolved Hide resolved
}

/// <summary>
/// Convert an IEnumerable to a JSON string.
/// </summary>
Expand Down
1 change: 1 addition & 0 deletions nanoFramework.Json/nanoFramework.Json.nfproj
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
<Compile Include="Converters\UIntConverter.cs" />
<Compile Include="Converters\ULongConverter.cs" />
<Compile Include="Converters\UShortConverter.cs" />
<Compile Include="JsonIgnoreAttribute.cs" />
<Compile Include="JsonSerializer.cs" />
<Compile Include="Resolvers\IMemberResolver.cs" />
<Compile Include="Resolvers\MemberResolver.cs" />
Expand Down