Omit data for relationship objects not existing as property in the original model

[bjornharrtell]

Related to https://github.com/joukevandermaas/saule/issues/159.

[bjornharrtell]

Rebased and cleaned up. How to handle the remaining test case failure for "Handles recursive properties on resource objects" still eludes me...

[bjornharrtell]

In my opinion the problem is that an ApiResource has no explicit connection to the underlying model. I can deduce it for the requested root ApiResource as the serializer knows about it (_value) but for recursive properties I see no such possibility.

To me it would make sense to have an explicit connection beteween ApiResource and underlying model but not sure if/how that could be added to Saule at this point.

I'll keep at it but would appreciate you opinion on this @joukevandermaas.

[bjornharrtell]

Ah! I was mistaken, the information did exist in the ResourceGraphNode instance. Not entirely sure how it's wired up but it seems to do the trick so now this PR is ready for review. :)

[joukevandermaas]

Glad you managed to get it working. I'll try to review asap.

FWIW I see it as a benefit of Saule that the resources (public API of your server) are decoupled from the models (private API of your server) a bit. It allows you to serialize different objects into the same resource. That might seem crazy, but it's actually really convenient if you use CQRS. For an example of this, see this blog post.

In any case if your app is different and only uses one model for any particular resource, you can already build a generic ApiResource using only public API:

public class MyApiResource<T> : ApiResource
{
    public MyApiResource()
    {
        var resourceType = typeof(T);
        OfType(resourceType.Name);

        var props = resourceType.GetProperties();

        var attrs = props.Where(p => p.GetFirstAttributeOrDefault<AttrAttribute>() != null);
        var belongsTo = props.Where(p => p.GetFirstAttributeOrDefault<BelongsToAttribute>() != null);
        var hasMany = props.Where(p => p.GetFirstAttributeOrDefault<HasManyAttribute>() != null);

        // these are generic, so we need to use reflection for this
        var belongsToMethod = GetType().GetMethods()
                  .Single(m => m.GetParameters().Length == 1 && m.Name == nameof(BelongsTo))
                  .GetGenericMethodDefinition();
        var hasManyMethod = GetType().GetMethods()
                  .Single(m => m.GetParameters().Length == 1 && m.Name == nameof(HasMany))
                  .GetGenericMethodDefinition();
        var genericThisType = GetType().GetGenericTypeDefinition();

        foreach (var prop in attrs)
        {
            Attribute(prop.Name);
        }

        foreach (var prop in belongsTo)
        {
            var genericType = genericThisType.MakeGenericType(prop.PropertyType);
            belongsToMethod.MakeGenericMethod(genericType).Invoke(this, new object [] { prop.Name });
        }

        foreach (var prop in hasMany)
        {
            var genericType = genericThisType.MakeGenericType(prop.PropertyType.GetGenericTypeParameterOfCollection());
            hasManyMethod.MakeGenericMethod(genericType).Invoke(this, new object[] { prop.Name });
        }
    }
}

You can use in controllers like this:

[HttpGet]
[ReturnsResource(typeof(MyApiResource<MyModel>))]
public IEnumerable<MyModel> GetMyModel()
{
    return GetMyModels();
}

for models like this:

public class MyModel
{
    [Attr]
    public string MyName { get; set; }

    [BelongsTo]
    public MyOtherModel OtherThing { get; set; }

    [HasMany]
    public IEnumerable<MyOtherModel> MoreOtherThings { get; set; }
}

I didn't test this code but it should work :smile:. I could see a potential future where this is the default behavior for Saule if you do not specify a ReturnsResource attribute, but I'm generally not a big fan of adding serialization-specific attributes to my domain models. As long as it is possible to do this, I'm not sure the code needs to be part of Saule itself.

[bjornharrtell]

Thanks the detailed answer, very informative and after reading it I think I agree on your view of decoupling. :)

[joukevandermaas]

Sorry for the late review. Thanks!