Request - Client: Ignore JsonIgnore and readonly properties

[weitzhandler]

Hi,

Let's have the CTC (client) ignore any:

• Read only property
• Properties attributed as JsonIgnore

So the above conditions don't affect the client's entities' TrackingState and ModifiedProperties properties.

It's likely that addressing this issue will eliminate the source of the issue addressed by #170, I do still think both server-side (#165) and client-side are important, because there could be both scenarios (i.e. NotMapped ^ JsonIgnore).

Can I PR?

[weitzhandler]

Fixes #165

[tonysneed]

JsonIgnore has to do with serialization. Can you explain more about your scenario where you need this? If you like, you can create a PR that has just a failing test.

[weitzhandler]

I'm not sure I can come up with a failing test for it now, but the logic behind this is very simple:
If the client says JsonIgnore this property, meaning it doesn't want it to be serialized to the server, and in fact, this property may not even exist in the server (for example that property is only declared in a client-only partial class), thus, it shouldn't be in the ModifiedProperties either.

[weitzhandler]

Imaging this class would look like this in the client:

using System;
using Newtonsoft.Json;

namespace TrackableEntities.Client.Tests.Entities.FamilyModels
{
    [JsonObject]
    public partial class Child : EntityBase
    {
        public Child() { }
        public Child(string name)
        {
            _name = name;
        }

        private string _name;
        public string Name
        {
            get { return _name; }
            set
            {
                if (value == _name) return;
                _name = value;
                NotifyPropertyChanged(() => Name);
            }
        }

        private ChangeTrackingCollection<Child> _children;
        public ChangeTrackingCollection<Child> Children
        {
            get { return _children; }
            set
            {
                if (Equals(value, _children)) return;
                _children = value;
                NotifyPropertyChanged(() => Children);
            }
        }

        /******* Notice the following couple of properties *******/

        [JsonIgnore]
        public string DisplayName
        {
            get
            {
                return Name;
            }
            set 
            {
                Name = value;
            }
        }

        public string ShortName
        {
            get
            {
                return Name == null ? null : Name.Substring(0, Math.Max(Name.Length, 3));
            }
        }
    }
}

Now, the readonly property (ShortName) does need a property changed notification so to affect UI, but TE isn't supposed to care about it.
The DisplayName property, is a client-only property, and it's not even in the server. Why should it exist in the ModifiedProperties property. Now this has already been fixed with the non-mapped members issue, but still, the server isn't supposed to hear about these properties, if they were chosen to be readonly (meaning non-modifiable) or ignored from the serialization by JsonIgnoreing them.

[tonysneed]

Have you tried ExcludedProperties on CTC?
The issue is how to go about indicating that properties should be excluded on the client. JsonIgnore might not be the right path, because it's a serialization concern.

[weitzhandler]

Never heard of it... Lemme check.
But anyway, why don't we make this automatic? Common sense says if client clearly says he doesn't want this serialized (he JsonIgnore this, or made it readonly), data about its modification shouldn't be passed through, that's how I see it.

[tonysneed]

Yes, so I would suggest an extension method that accepts a lambda (strategy pattern). It can be a Func<T, bool>. Then the caller can apply whatever strategy they want. If the delegate returns true, then the method can add the property to ExcludedProperties. The method can be called ExcludeProperties. The T can be an expression where you can specify a property.

The caller can then apply whatever logic they want (read-only, JsonIgnore, no DataMember attribute, etc.

[weitzhandler]

Makes sense.
Should we make a default method that ignores readonly and JsonIgnore props, so that we don't have to call the default ignores for each CTC manually?

[tonysneed]

Let's start with the generic method and go from there, perhaps adding overloads for common patterns - maybe accepting a bit mask [Flags] enum.

[weitzhandler]

@tonysneed

The method can be called ExcludeProperties. The T can be an expression where you can specify a property.

We need a way that covers more than one property.
I'd say it should be Func<TEntity, IEnumerable<string>> InitExcludedProperties { get; set; }, that when is not null, fills the ExcludedProperties with the provided list, if we're going about adding that property.
Besides, the excluded properties in the CTC is for the root entity type. How will we access the entire graph's ExcludedProperties dynamically?
Which is why I'm personally still voting for adding readonly and JsonIgnored properties to the ExcludedProperties automatically, as a means of dynamically excluding properties, rather than rely on manually messing with the ExcludedProperties property of the CTC. Because really the default behavior should have been ignoring properties of these categories.

[tonysneed]

We need a way that covers more than one property.

So the way ExcludeProperties should work is that it iterates over all the properties of the CTC entity or entities. We can start with strings for property names. (We can also implement recursion if recursive parameter is true, by walking down the object graph.)

private void ExcludeProperties(TEntity entity, Func<string, bool> predicate, bool recursive)
{
    foreach (var prop in typeof(TEntity).GetRuntimeProperties())
    {
        if (predicate(prop.Name))
        {
            ExcludedProperties.Add(prop.Name);
        }
    }
}

Here is the public method that calls the private method.

public void ExcludeProperties(Func<string, bool> predicate, bool recursive)
{
    foreach (var item in Items)
    {
        ExcludeProperties(item, predicate, recursive);
    }
}

How does this look?

Cheers,
Tony

[weitzhandler]

The problem with setting up the excluded properties in the CTC is that it must be performed on each CTC individually throughout the entire graph, which doesn't really make sense.
There has to be a global way to configure excluded properties per entity type.

[tonysneed]

The recursive flag is there to allow you to do this. We just need to add the CTC type as an argument on the Func, so that the callback knows the CTC type. The approach here is to build out the low level API based on a Strategy pattern, then build a higher level of abstraction on top of that, so you can do things like look for Json attributes.

[tonysneed]

Don't worry, we'll get to where you want to be, with a method that does what you want. It's just that I'd like to try for a more extensible design. That way, other criteria can be applied if desired. If you like, I can flesh out the low level API first, and then you can layer your method on top of that.

[weitzhandler]

I'll try to implement during the week.
I really appreciate you kind and detailed responses, and your sympathy!

[weitzhandler]

It's just that I'd like to try for a more extensible design

The config can't sit in the CTC. It has to be configured in a global map, via attributes or other per-class/property excludes.