MapFromAttributes not working with Enum fields

[jayhilden]

The MapFromAttributes is not working when the class includes an enum. Here is a simple example:

    public void InitializeES()
    {
        //drop the index for debugging purposes, remove this later
        var deleteResponse = _client.DeleteIndex(f => f.Index(_client.Infer.DefaultIndex));
        VerifyResponse(deleteResponse);
        if (!_client.IndexExists(i => i.Index(_client.Infer.DefaultIndex)).Exists)
        {
            var response = _client.CreateIndex(_client.Infer.DefaultIndex);
            VerifyResponse(response);
        }
        VerifyResponse(_client.Map<TestClass>(m => m.MapFromAttributes()));
    }

    private static void VerifyResponse(IResponse response)
    {
        if (!response.IsValid)
        {
            throw new ElasticsearchServerException(response.ServerError);
        }
    }

    [ElasticType(Name = "testclass")]
    public class TestClass
    {
        public int Id { get; set; }

        [ElasticProperty(Index = FieldIndexOption.NotAnalyzed)]
        [JsonConverter(typeof(StringEnumConverter))]
        public EnumTester EnumTest { get; set; }
    }

    public enum EnumTester
    {
        Value1,
        Value2
    }

This is the resulting structure from calling GET testindex/_mapping:

{
"testindex": {
"mappings": {
"testclass": {
"properties": {
"id": {
"type": "integer"
}
}
}
}
}
}

The enum is missing from the mapped data structure.

I'm on ES 1.3.0, NEST 1.0.2

Am I doing something wrong or is this a bug?

Thanks in advance guys, I love ES and I love NEST!

[jayhilden]

As a workaround, if you decorate the Enum field with FieldType.String it will pick it up:

        [ElasticProperty(Type = FieldType.String, Index = FieldIndexOption.NotAnalyzed)]
        public EnumTester EnumHere { get; set; }

[Mpdreamz]

Hey @jayhilden,

Wondering wether this is a bug or a feature ... 😕 since enums default to being numbers having the type = string explicitly seems appropriate.

Thoughts? ping @gmarz

[gmarz]

@Mpdreamz I agree

[jayhilden]

@Mpdreamz @gmarz that makes sense to me, thank you.

[pickypg]

Related: http://stackoverflow.com/questions/29194750/attribute-based-index-hints-change-my-results

I noticed this question pop up over at StackOverflow. It sounds more like a bug to silently skip it just because the type is unset, especially because other attributes are being set.

The type should be mandatory, it should be defaulted, or it should at least log a very high level warning to the user.

[gmarz]

We can either default to integer, string, or throw an exception. I'm torn- defaulting to integer seems technically correct, since that's how enums are treated naturally. On the other hand, I think most people are expecting/want a string. Thoughts @Mpdreamz ?

[pickypg]

@gmarz I think that I like the exception because it covers cases more generally. Enums can perhaps be supported automatically later, but a good start would be failing fast here.

[bloritsch]

Weighing in here, as the guy who was blindsided by this issue and asked the question.

We are talking about documents, not POCO objects. How the runtime stores and works with enums is irrelevant from a document standpoint. What's important is what those numbers convey, and that meaning is expressed in words. I would argue for storing the enum as a string, the way that ElasticSearch 1.4.x does now.

I would go a step further, and when you automap based on types, all enums would be treated as strings that are not analyzed. The number of values an enum can hold is finite, and they should never be tokenized and split apart (i.e. with underscores, etc.). Feel free to prove me wrong, but I would argue enum types would be most often used with terms filters.

[bloritsch]

In the very near term, I agree with @pickypg. Throwing an exception at the client.Map(m => m.MapFromAttributes()) call if you come across an attribute you don't know what to do with is far better than silently ignoring the attribute. At least then I'm not spending a week trying to figure out why a query is suddenly broken.

[Mpdreamz]

@bloritsch first of all sorry this was such a nuisance to you, I have to admit that I totally misread this issue as enums defaulting to ints instead of strings. Not that they were skipped altogether, this is a real bug.

However in our 1.x releases we have to default to int since thats what Json.NET defaults to as well.

If you write a query/filter using enums right now, without setting the StringEnumConverter, it'll write them as integers as well. See this unit test and its expected output. The same when you index an enum without that converter in place it will also default to writing enums as ints.

I just submitted a PR so that MapFromAttributes() maps enums as int's here: #1290

To clarify/summarize:

MapFromAttributes() is documented to not be an exhaustive and fully inclusive mapping, it takes the attributes you set explicitly into account and automatically maps object types and known value types (long, int, double, datetime, etcetera) for you out of the box while still allowing you to override them afterwards.

With that in mind it should throw an exception only when an attribute is present on a property we cannot infer a FieldType for and no FieldType is explicitly set on the attribute.

Also the linked xml comment's english is dreadful and totally unclear, I will push an update for that as well.

Both of these are now also part of the PR and should go in the next release.

Thanks @bloritsch and @pickypg for chasing our asses on this!

[bloritsch]

@Mpdreamz, is it a bug then that when my record was serialized to ElasticSearch it was indeed a string? This is how it automapped without the explicit field type (using the _mapping?pretty URI):

"releasableTo" : {
        "type" : "string"
}

The records had the enum names in them. I just thought that was how it was supposed to be, and it seemed to be expected behavior to me. I was only surprised that the NotAnalyzed wasn't being applied.

When you have a new version pushed to NuGet I'll update my project. There's more attributes I have to add, but I haven't quite gotten to that part of the functionality yet.