Skip to content

Problem with accessing fields via dynamic object #620

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
sporty81 opened this issue Apr 24, 2014 · 3 comments
Closed

Problem with accessing fields via dynamic object #620

sporty81 opened this issue Apr 24, 2014 · 3 comments

Comments

@sporty81
Copy link

The library makes extensive use of dynamic objects. There is a problem with this in the current implementation. When doing something like a search that can return various fields depending on if a document has the field or not, it does not work. Take the example below. Let's pretend sometimes "testfield" is in the document and sometime it is not. There is no way to do hit["testfield"] != null or hit.ContainsKey("testfield") to see if the field exists.

        Elasticsearch.Net.ElasticsearchClient rawclient = new ElasticsearchClient(settings);
        var response = rawclient.Search("indexname", "{\"query\": {\"query_string\": { \"query\": \"host=test\"}}}");
        var hit = response.Response["Hits"]["hits"][0];
        var field = hit["testfield"]; //throws exception

Ultimately it is throwing an exception in here (ElasticDynamicValue) when it tries to call the underlying SimpleJson.JObject

    static object GetDynamicMember(object obj, string memberName)
    {
        var binder = Binder.GetMember(CSharpBinderFlags.None, memberName, null, new[] { CSharpArgumentInfo.Create(CSharpArgumentInfoFlags.None, null) });
        var callsite = CallSite<Func<CallSite, object, object>>.Create(binder);
        return callsite.Target(callsite, obj);
    }

Possible fixes

  1. Change SimpleJson.JObject to be public by removing SIMPLE_JSON_OBJARRAYINTERNAL. This would at least allow us to cast the object to this type and use its ContainsKey function. This is not very intuitive IMHO. (This was my workaround)
  2. Modify SimpleJson.JsonObject.TryGetIndex method to see if the field exists and return null if it doesn't. I would assume nobody wants to essential fork SimpleJson to do that.
  3. Change GetDynamicMember to better handle how it interfaces with SimpleJson to return null values when the dynamic properties do not exist. This seems like the best option.

Something like this would fix it but it sort of feels like a hack. Hopefully whoever designed all this dynamic stuff has a better understanding and other thoughts on the right way to do this.

    static object GetDynamicMember(object obj, string memberName)
    {
        if (obj is Elasticsearch.Net.Serialization.JsonObject)
        {
            if (!((Elasticsearch.Net.Serialization.JsonObject) obj).ContainsKey(memberName))
            {
                return null;
            }
        }

        var binder = Binder.GetMember(CSharpBinderFlags.None, memberName, null, new[] { CSharpArgumentInfo.Create(CSharpArgumentInfoFlags.None, null) });
        var callsite = CallSite<Func<CallSite, object, object>>.Create(binder);
        return callsite.Target(callsite, obj);
    }
@Mpdreamz
Copy link
Member

Hi @sporty81

A hit actually looks like this so

 "hits" : {
    "total" : 436,
    "max_score" : 1.0,
    "hits" : [ {
          "_index" : "nest_test_data-3192",
          "_type" : "elasticsearchprojects",
          "_id" : "0",
          "_score" : 1.0, 
          "_source" : {"id":0,"name": ......},
       } ]
   ....

So the right lookup should be:

var hit = response.Response["hits"]["hits"][0];
var source = hit["_source"]
var field = source["testfield"]; 

Also note the lowercase H on the first hits

I've attached a commit to this issue with two tests to try and reproduce the exception but I'm not able to reproduce it. Can you have a look ?

@Mpdreamz Mpdreamz added the Bug label Apr 24, 2014
@sporty81
Copy link
Author

So in my fork of the code it throws an error. In my copy of your repository with these tests it does not. I tried to get my fork to update to latest and thought I had it but I guess not. So maybe it is working fine. Something in my fork is out of sync with the code base. But these are good unit tests to have for sure.

@Mpdreamz
Copy link
Member

They are :) Elasticsearch.Net.Integration.Yaml has 600+ of them dealing with the deserialization of the dynamic types 👍

Let me know if you find out what it was in your fork.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants