Indexing JToken objects ignores DefaultFieldNameInferrer

[LordMike]

NEST/Elasticsearch.Net version:
2.4.4

Elasticsearch version:
2.3.3

Description of the problem including expected versus actual behavior:
When we Index any object, we normally run through the DefaultFieldNameInferrer - using that we can control the property names in ES. We've used this to fix all names to be lowercase in our project.

But when we index anything containing a JToken, the DefaultFieldNameInferrer is ignored.

Steps to reproduce:

Use the following code.

class Program
{
    static void Main(string[] args)
    {
        ConnectionSettings settings = new ConnectionSettings(new SingleNodeConnectionPool(new Uri("http://127.0.0.1:9200/")));
        settings.DefaultFieldNameInferrer(s => s.ToLower());
        settings.DefaultIndex("testindex");
        settings.ThrowExceptions();

        ElasticClient client = new ElasticClient(settings);
        var item = new MyClass { MyInteger = 5, MyString = "Hello world" };
        var itemAsJtoken = JToken.FromObject(item);

        client.Index(itemAsJtoken);
    }
}

public class MyClass
{
    public string MyString { get; set; }

    public int MyInteger { get; set; }
}

Observe that the auto-created mapping on testindex is as follows:

{
  "testindex" : {
    "mappings" : {
      "jtoken" : {
        "properties" : {
          "MyInteger" : {
            "type" : "long"
          },
          "MyString" : {
            "type" : "string"
          }
        }
      }
    }
  }
}

If we index the item instead of itemAsJtoken, we get the following map:

{
  "testindex" : {
    "mappings" : {
      "myclass" : {
        "properties" : {
          "myinteger" : {
            "type" : "long"
          },
          "mystring" : {
            "type" : "string"
          }
        }
      }
    }
  }
}

Relevant sources:
We found an issue in Newtonsoft.NET, where they've changed the way the contract resolvers are called. It seems that the DefaultContractResolver in Newtonsoft no longer calls its ResolvePropertyName method, which is used internally by NEST. The issue for that is here:

JamesNK/Newtonsoft.Json/issues/950

We can reproduce this by creating the following code:

class Program
{
    static void Main(string[] args)
    {
        var item = new MyClass { MyInteger = 5, MyString = "Hello world" };
        var itemAsJtoken = JToken.FromObject(item);

        var serializerSettings = new JsonSerializerSettings();
        serializerSettings.ContractResolver = new MyContractResolver();

        var serializer = JsonSerializer.Create(serializerSettings);

        StringBuilder sb = new StringBuilder();
        using (var sw = new StringWriter(sb))
            serializer.Serialize(sw, itemAsJtoken);

        Console.WriteLine(sb.ToString());
    }
}

public class MyClass
{
    public string MyString { get; set; }

    public int MyInteger { get; set; }
}

internal class MyContractResolver : DefaultContractResolver
{
    protected override string ResolvePropertyName(string propertyName)
    {
        return propertyName.ToLower();
    }
}

Relevant people (on my end): @Genbox

[russcam]

Thanks for raising this, @LordMike; will look into

[russcam]

I've looked further into this.

JToken.FromObject(object o) will use the default Json.NET conventions for how property names are serialized at serialization time; the NamingStrategy is used at conversion time from object to JToken, using the ResolvePropertyName() method of the Contract resolver (delegating to the NamingStrategy) on the serializer passed to JToken.FromObject(object o, JsonSerializer serializer). In the case of the method that does not take a JsonSerializer instance, an instance will be constructed using JsonSerializer.CreateDefault() and passed to the overloaded version that does.

With the above in mind, in order for serialization of a JObject to adhere to the default field inference, an instance of a JsonSerializer needs to be passed for the conversion that uses the same rules as the contract resolver used by NEST. The ElasticContractResolver overrides ResolvePropertyName(string name) and doesn't call the base method, so we don't need a custom NamingStrategy, NEST will work as is with the DefaultFieldNameInferrer, so long as the JsonSerializer used for conversion uses ElasticConstractResolver.

Here is where things are a little tricky though; because NEST defines an interface for a serializer, IElasticsearchSerializer, it doesn't directly expose the Json.NET serializer that is used under the covers in the default case (which may not be Json.NET at all for someone using a custom serializer). The contract resolver is exposed however, but as a protected property, so we can get at it with a derived type. We can use the contract resolver in the construction of a JsonSerializer to pass to JToken.FromObject(object o, JsonSerializer serializer).

Here's an example that will work

public class GithubIssue2224
{
    [U]
    public void DefaultFieldNameInferrerAppliesToJObjects()
    {
        var pool = new SingleNodeConnectionPool(new Uri("http://localhost:9200"));
        var settings = new ConnectionSettings(pool, new InMemoryConnection(), new CustomSerializerFactory());
        var client = new ElasticClient(settings);

        var project = new Project
        {
            Name = "First Project"
        };

        var jObject = JObject.FromObject(project, ((CustomSerializer)client.Serializer).Serializer);
        var json = client.Serializer.SerializeToString(jObject);

        json.Should().NotContain("\"Name\"").And.Contain("\"name\"");
    }

    private class CustomSerializerFactory : ISerializerFactory
    {
        public IElasticsearchSerializer Create(IConnectionSettingsValues settings)
        {
            return new CustomSerializer(settings);
        }

        public IElasticsearchSerializer CreateStateful(IConnectionSettingsValues settings, JsonConverter converter)
        {
            return new CustomSerializer(settings, converter);
        }
    }

    private class CustomSerializer : JsonNetSerializer
    {
        public CustomSerializer(IConnectionSettingsValues settings) : base(settings)
        {
        }

        public CustomSerializer(IConnectionSettingsValues settings, JsonConverter statefulConverter) :
            base(settings, statefulConverter)
        {
        }

                // get a JsonSerializer out that uses the configured ContractResolver
        public JsonSerializer Serializer => JsonSerializer.Create(new JsonSerializerSettings
        {
            Formatting = Formatting.None,
            ContractResolver = this.ContractResolver,
            DefaultValueHandling = DefaultValueHandling.Include,
            NullValueHandling = NullValueHandling.Ignore
        });
    }
}

[Mpdreamz]

@russcam @gmarz what do you reckon is the best course of action here?

Code that used to rely on us doing the right thing when just calling JObject.FromObject(obj) will now fail albeit due to a breaking change in json.net.

Do we push the requirement to call JObject.FromObject(obj, serializer) to the user or do we need to implement a custom NamingStrategy?

Also, we used to throw if ContractResolver on our serializers was not a subclass of ElasticContractResolver. Need to double check we still do.

[russcam]

@Mpdreamz I just tried this with latest 1.x and get

"MyString":"Hello world"
"MyInteger":5

the same behaviour is exhibited i.e. not lowercased property names for a JObject. It doesn't appear to be a breaking change in NEST behaviour.

NOTE:
JObject doesn't serialize correctly in 1.x; it gets treated as an IEnumerable<object> (this is a separate issue; thought it was addressed with #1804, but there's a difference in how index requests are serialized in 1.x and 2.x; I'll open a PR now).

I also don't think we need a custom NamingStrategy; looking at Json.NET, the places where it is called are:

• ResolvePropertyName
• ResolveDictionaryKey

in ElasticContractResolver however, we override ResolvePropertyName but don't call into the base method and ResolveDictionaryKey delegates to ResolvePropertyName when NamingStrategy is null.

JObject defines its own WriteTo() method so it doesn't look like there's a hook to lowercase JProperty names on the JObject downstream.

I'm not totally convinced it's an issue though 😄 to clarify, property name casing happens when creating a JObject from T with JObject.FromObject(), and this is controlled by the serializer passed for that conversion (else it uses the default).

[Mpdreamz]

Ok I'm good with closing with a known workaround of calling JObject.FromObject(obj, serializer)

@LordMike closing this but please reply if you feel strongly that this a client concern that we should handle!