4
\$\begingroup\$

I have a controller action that takes JSON bodies. I know, that the JSON is either of type A or B, where both classes are of interface IModel.

    [HttpPost]
    [Route("api/post/")]
    public IHttpActionResult MyPost([ModelBinder(typeof(MyModelBinder))]IModel viewModel)
    {
        doSomething(viewModel);
        return Ok("ok");
    }

In order to map the JSON, I created my own model binder, like this one

public class MyModelBinder : IModelBinder
{
    public bool BindModel(HttpActionContext actionContext, ModelBindingContext bindingContext)
    {
        var content = actionContext.Request.Content;
        string json = content.ReadAsStringAsync().Result;

        try
        {
            var DTO = JsonConvert.DeserializeObject<A>(json);
            IModel viewModel = new A()
            {
                // mapping here
            }; 
            bindingContext.Model = viewModel;
            return true;
        }
        catch(Exception e)
        {

        }
        try
        {
            var DTO = JsonConvert.DeserializeObject<B>(json);
            IModel viewModel = new B()
            {
                // mapping here
            };
            bindingContext.Model = viewModel;
            return true;
        }
        catch (Exception e)
        {

        }
        return false;
    }
}

Edit: It doesn't work, because Newtonsoft Json can apparently always deserialize, so it never jumps out of the first try block. I was wondering, how to improve the code. Right now I have only 2 possible types (A and B), but in the near future I expect to get much more possible types...

\$\endgroup\$
6
  • 1
    \$\begingroup\$ Your discovery made your question off-topic because now we know for sure that the code is broken. \$\endgroup\$
    – t3chb0t
    Jun 18 '18 at 10:32
  • \$\begingroup\$ I updated the code \$\endgroup\$
    – rst
    Jun 18 '18 at 10:35
  • \$\begingroup\$ I'm not sure whether this is such a good idea as this probably invalidates the answer. If it does then unless it's deleted/updated this is a dead end and your question needs to be rolled-back. \$\endgroup\$
    – t3chb0t
    Jun 18 '18 at 10:36
  • 2
    \$\begingroup\$ I think all you can do know is to accept the answer and ask a follow-up. \$\endgroup\$
    – t3chb0t
    Jun 18 '18 at 10:38
  • \$\begingroup\$ @t3chb0t: Yes you are right. I don't know exactly what to do. Just delete the whole question? \$\endgroup\$
    – rst
    Jun 18 '18 at 10:38
4
\$\begingroup\$

The first thing to do is to remove duplicated code. If you do not need further processing DeserializeObject<T>() returns T and you do not need any mapping, you simply can return its result. That code is also shared between A and B then you can simply introduce a separate function:

private static bool TryDeserializeObject<T>(string json, out T result)
    where T : IModel
{
    result = default(T);

    try
    {
        result = JsonConvert.DeserializeObject<T>(json);
        return true;
    }
    catch (JsonReaderException)
    {
        return false;
    }
}

Note that I'm not catching Exception but a more specific one. OutOfMemoryException (for example) it's not intended to signal that I'm trying to deserialize the wrong type. This code is, however, sub-optimal because calling point is like this:

if (TryDeserializeObject<A>(json, out var a))
    bindingContext.Model = a;
else if (TryDeserializeObject<B>(json, out var b))
    bindingContext.Model = b;
else
    return false;

return true;

It does not scale well if I have more than few types. For that I'd love to write this:

private static bool TryDeserializeObject(string json, Type type, out IModel result)
{
    result = null;

    try
    {
        result = (IModel)JsonConvert.DeserializeObject(json, type);
        return true;
    }
    catch (JsonReaderException)
    {
        return false;
    }
}

We can now introduce another function:

private static IModel DeserializeObjectOrNull(string json, params Type[] types)
{
    foreach (var type in types)
    {
        if (TryDeserializeObject(json, type, out var model))
            return model;
    }

    return null;
}

Now calling point might be (it may be more terse but I try to keep it clear):

var model = DeserializeObjectOrNull(json, typeof(A), typeof(B));
if (model == null)
    return false;

bindingContext.Model = model;

return true;

List may be specified here or come from configuration or an hard-coded list somewhere else. This will scale much better.

We have another issue to solve: how to detect we're trying to deserialize the wrong type? Newtonsoft JSON by default simply ignores unknown members then we'll always get an object with only the known property correctly initialised. You need to use an overload which accepts JsonSerializerSettings where you specified MissingMemberHandling.Error for JSonSerializerSettings.MissingMemberHandling property.

Is there a better way? If you can change your JSON then you may add a _type property (for example, also consider the @A and @B variants). If not then you may consider to deserialize to dynamic and to check for well-known properties (using OOM mapper to copy properties). I'd do this only if performance hit is significant enough because it'll make your code much more convoluted.

Solved our problem with JSON we may take a look to the first lines...getting the result of an async method with .Result is seldom what you want. Just refer to MSDN for details and differences but probably what you want is .GetAwaiter().GetResult(). Even better you should make your method async and take advantage of it (there are alternatives ModelBinder implementation which supports async):

public async Task<bool> BindModel(HttpActionContext actionContext,
    ModelBindingContext bindingContext)
{
    var content = actionContext.Request.Content;
    string json = await content.ReadAsStringAsync();

    // ...
}
\$\endgroup\$
4
  • \$\begingroup\$ I didn't read you answer yet, but I just realized that Newtonsoft really ignores the deserialization if it doesn't match. I thought it worked but I had another issue later in the code so I missed that. \$\endgroup\$
    – rst
    Jun 18 '18 at 10:27
  • \$\begingroup\$ Do you mind elaborate the part with @A? Check my updated question with regard to the _type \$\endgroup\$
    – rst
    Jun 18 '18 at 10:30
  • \$\begingroup\$ @rst if your code didn't handle that somewhere else then it's actually off-topic (please fix it and adding the minimum necessary code). @A is just another convention to include type information in JSON (few examples in tech.signavio.com/2017/json-type-information, it's just my very first Google search result then there is for sure something better) \$\endgroup\$ Jun 18 '18 at 10:34
  • 1
    \$\begingroup\$ @rst adding JSonSerializerSettings.MissingMemberHandling was enough to make your code working! \$\endgroup\$ Jun 18 '18 at 10:48

Not the answer you're looking for? Browse other questions tagged or ask your own question.