aspnet / JsonPatch

[Archived] JSON PATCH library. Project moved to https://github.com/aspnet/AspNetCore
Apache License 2.0
103 stars 48 forks source link

Patch an aggregated DTO #104

Closed Mardoxx closed 7 years ago

Mardoxx commented 7 years ago

A somewhat contrived, but nonetheless important, example.

Assume the following case whereby UserDetails is an aggregate DTO (not sure of correct terminology, but basically a model of collected information from different stores/services) which is used by a RESTful web service. It does not necessarily have the same property names as the objects it collects together.

public class UserDetails
{
    public int UserId { get;set; }
    public string GivenName { get; set; }
    public string Surname { get; set; }
    public int? UserGroupId { get;set; } // FK in a different database
}

Let our stores persist the following models:

public class User
{
    public int Id { get; set; }
    public string GivenName { get; set; }
    public string Surname { get; set; }
}
public class UserGroup
{
    public int UserId { get; set; }
    public int GroupId { get; set; }
}

Let the UserDetails object be populated thusly:

User user = _userService.GetUser(userId) ?? throw new Exception();
UserGroup userGroup = _userGroupService.GetUserGroup(user.Id);

UserDetails userDetails = new UserDetails {
    UserId = user.Id,
    GivenName = user.GivenName,
    Surname = user.Surname,
    UserGroupId = userGroup?.GroupId
};

That is, setting FirstName or Surname should delegate to the UserService, and UserGroupId to the GroupService.

This UserDetails object is used for GET and PUT, the logic here is pretty trivial, however a JSONPatch document for this object is sent for PATCH requests. This is apparently much more complicated.

How can we go about changing the user's group? The best ('best' being used very loosely) I came up with is this:

JsonPatchDocument<UserDetails> patch;

// This likely works fine, because the properties in `UserDetails`
// are named the same as those in `User`
List<string> userProps = new List<string> {"/givenName", "/surname"};
if (patch.Operations.Any(x => userProps.Contains(x.path))) {
    User user = _userService.GetUserByUserId(userId);
    patch.ApplyTo(user);
    _userService.SetUser(userId, user);
}

// Do specialised stuff for UserGroup
// Can't do ApplyTo() because `UserDetails.UserGroupId` is not named the same as `UserGroup.GroupId`
IEnumerable<Operation<UserDetails>> groupOps = patch.Operations.Where(x => x.path == "/userGroupId");
foreach (Operation<UserDetails> op in groupOps)
{
    switch (op.OperationType)
    {
        case OperationType.Add:
        case OperationType.Replace:
            _groupService.SetOrUpdateUserGroup(userId, (int?)(op.value));
            break;

        case OperationType.Remove:
            _groupService.RemoveUserGroup(userId);
            break;
    }
}

Which is pretty garishly awful. It's a lot of boilerplate, and relies on a magic string.

Something like patch.Path(x => x.UserGroupId ) which returns "/userGroupId" would at least get rid of the magic strings.

JsonPatch seems pretty limited in this regard, seems more tailored towards systems where there is a 1:1 mapping between DAO (entities) and DTO (model). Anyone got any good ideas? Can't be hard to beat the tripe I came up with!!

Eilon commented 7 years ago

Thanks for the report, we will take a look at this and see what we can do.

Eilon commented 7 years ago

This seems like a good place to use a library such as AutoMapper to modify the objects and apply the data from the original User/UserGroup objects to the DTO.

Closing because there are no plans to implement this.

Mardoxx commented 7 years ago

@Eilon This is one of the solutions I ended up at, but I didn't really like it because it involves a lot of boilerplate, and the naive way I implemented it involved 3(!) calls to the db - which is obviously terrible! Really felt like I was forcing it to do something it doesn't want to do -- using the wrong tool for the job I think!

Thanks for looking in to this though! :)