Cannot replace ContractResolver, even with a passthrough!

[bytenik]

Here's an example test case that DOES work:

void Main()
{
    var settings = new ConnectionSettings(new Uri("http://ffazva-es001-01.internal.funnelfire.com:9200/"));
    settings.SetJsonSerializerSettingsModifier(x => { });
    var client = new ElasticClient(settings);
    var results = client.GetMany<JObject>(new[] { "Metabase/17783019068" }, "feed", "feedItem");
    if (results.Count() == 0) throw Exception("Uh oh."); // does not throw
}

Now here's two different test cases that do NOT work:

void Main()
{
    var settings = new ConnectionSettings(new Uri("http://ffazva-es001-01.internal.funnelfire.com:9200/"));
    settings.SetJsonSerializerSettingsModifier(x => { x.ContractResolver = new ExampleContractResolver(x.ContractResolver); });
    var client = new ElasticClient(settings);
    var results = client.GetMany<JObject>(new[] { "Metabase/17783019068" }, "feed", "feedItem");
    if (results.Count() == 0) throw Exception("Uh oh."); // DOES throw every time, even though it exists
}

class ExampleContractResolver : IContractResolver
{
    private readonly IContractResolver _inner;
    public ExampleContractResolver(IContractResolver inner)
    {
        _inner = inner;
    }

    public JsonContract ResolveContract(Type type)
    {
        return _inner.ResolveContract(type);
    }
}

and the second example:

void Main()
{
    var settings = new ConnectionSettings(new Uri("http://ffazva-es001-01.internal.funnelfire.com:9200/"));
    settings.SetJsonSerializerSettingsModifier(x => { x.ContractResolver = new ExampleContractResolver(settings); });
    var client = new ElasticClient(settings);
    var results = client.GetMany<JObject>(new[] { "Metabase/17783019068" }, "feed", "feedItem");
    if (results.Count() == 0) throw Exception("Uh oh."); // DOES throw every time, even though it exists
}

public class ExampleContractResolver : Nest.Resolvers.ElasticContractResolver
{
    public ExampleContractResolver(IConnectionSettingsValues cs) : base(cs)
    {
    }
}

[bytenik]

In order to make this work, I had to do:

settings.SetJsonSerializerSettingsModifier(x => {
    var old = x.ContractResolver;
    x.ContractResolver = new ExampleContractResolver(settings);
    var propHack = typeof(ElasticContractResolver).GetProperty("PiggyBackState", BindingFlags.NonPublic | BindingFlags.Instance);
    propHack.SetValue(x.ContractResolver, propHack.GetValue(old));
});

That code smells pretty bad. :)

[bytenik]

Also, honestly, I need that first example to work. I have an alternate contract resolver from a third party that isn't going to be able to derive from ElasticContractResolver, but will passthrough any requests it doesn't know how to handle to your contract resolver.

[Mpdreamz]

Some json converters need the original C# request to piece to response back together (_msearch, _mget, others). This is the reason I need to inject state that I can pull out inside of the converter and the contractresolver is the only place, which is the reason that NEST only works (or should) with a subclass of ElasticContractResolver registered.

Im at a conference today and tomorrow but will look into seeing how we can make this smell a little less 👍

[bytenik]

Awesome, thanks!

By the way, the second example is a subclass of ElasticContractResolver, but I still have to hack that internal property.

[bytenik]

Can't you pass something into the ctor of the converters that would then let it get at state without going through the resolver?

[Mpdreamz]

I could but you want resolvers to be cached aggressively. If you use .Converters JSON.NET will check for every node in the json graph if the converter wants to come into play. This is why almost all converters are registered through attributes or in the contract resolver. This way the check is only done once and the converters become part of the type's cached contract and it no longer has to check 60 odd converters on every node in the json graph.

This is why i.e a stateless multisearch converter is registered in the contract that tries to get the state out of the piggybackconverter attached to the contract resolver.

All the deserialization uses the same json settings object unless you call deserialize with a stateful deserializer in which case deserialize is called with a new JsonSettings and new ElasticContractResolver. This scopes that deserialization but because the contract uses shared cache state if I pass in a stateful MultiSearchConverter it won't have precedence over the already cached MultiSearchConver which is why it needs to PiggyBack on the contract resolver.

This scoped / stateful deserialization is also why doing Deserialization with something other then JSON.NET in NEST is not possible.

Its not pretty but its a major performance boost.

[bytenik]

I'm not arguing against using a custom contract resolver; the performance improvement is totally worth it. I'm just arguing against storing some of this information in that resolver, vs. in some other location that you can then pass into the ctor of the converters as you hook them up to your custom resolver.

By the time I replace your resolver with my own (which still hands off 99% of the work to yours) your converters should already be hooked into the original contract resolver and I should be able to proxy contract creation requests to it.

[Mpdreamz]

So the actual method that needs piggyback state is;

public override object ReadJson(
    JsonReader reader, Type objectType, object existingValue, JsonSerializer serializer)

right now I can just do a property check in this method:

var elasticContractResolver = serializer.ContractResolver as ElasticContractResolver;
if (elasticContractResolver == null)
     return new MultiSearchResponse { IsValid = false };

The only other place I can think of is to have a static IDictionary<JsonSerializer, JsonConverter> lookup in the client where its removed afterwards. The only problem then is that deserializing the same object in multiple threads will no longer work.

I'm all ears for suggestions ! :)

[bytenik]

I wouldn't go with a static dictionary, but perhaps a ConcurrentDictionary in the client and a reference to the client passed into each converter's constructor?

[bytenik]

Actually, I don't know that this would work, but, could you subclass Json.NET's JsonSerializer and add your own properties? Since users cannot replace the serializer, only customize it, you wouldn't have to worry about ending up with a user replacing it.

[Mpdreamz]

That sounds so simple its probably going to work! (I cant remember actually going that route).

[bytenik]

I found an easier way to make this work without having to completely change the way you do this. Take a look at #705.