Skip to content
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

MultiGet ignores _source field (reopen #849) #858

Closed
lakario opened this issue Aug 6, 2014 · 9 comments
Closed

MultiGet ignores _source field (reopen #849) #858

lakario opened this issue Aug 6, 2014 · 9 comments

Comments

@lakario
Copy link

lakario commented Aug 6, 2014

Issue #849 reported that MultiGet was ignoring _source fields. The issue has since been closed with a supposed fix pushed to develop. Having tested that fix, I can confirm that functionality is still missing.

Here's a sample which demonstrates the issue (references latest develop sources):

class Program
{
    static void Main(string[] args)
    {
        const string serverUrl = "http://localhost:9200";
        const string indexName = "test";
        const bool resetAll = false;

        var guids = new[] {
            new Guid("16b006aa-416f-42e4-801c-7f61f6d74294"),
            new Guid("eb2e4f76-a697-4690-b339-cf923eda2c92"),
            new Guid("494d59fa-965d-4c84-96e3-ac617a7536ca"),
        };

        var client = new ElasticClient(new ConnectionSettings(new Uri(serverUrl))
            .SetTimeout(15000)
            .SetDefaultIndex(indexName));

        if (resetAll || !client.IndexExists(x => x.Index(indexName)).Exists)
        {
            ResetIndex(client, indexName, guids);
        }

        var response = client.MultiGet(s => s
            .GetMany<Thing>(guids.Select(x => x.ToString()))
            .SourceInclude<Thing>(x => x.Id, x => x.Name));

        DisplayThings(response.Documents.Select(x => x.Source).Cast<Thing>());

        Console.ReadKey();
    }

    static void DisplayThings(IEnumerable<Thing> things)
    {
        foreach (var item in things)
        {
            Console.WriteLine("{1}{0}{1}{1}{2}", item, Environment.NewLine, new String('*', 50));
        }
    } 

    static void ResetIndex(IElasticClient client, string indexName, IList<Guid> guids)
    {
        Action fnCreate = () =>
        {
            client.CreateIndex(x => x.Index(indexName));
            client.Map<Thing>(x => x.MapFromAttributes());
        };

        if (client.IndexExists(x => x.Index(indexName)).Exists)
        {
            client.DeleteIndex(x => x.Index(indexName));
            fnCreate();
        }
        else
        {
            fnCreate();
        }

        var items = new[] {
            new Thing() {
                Id = guids[0],
                Name = "Thing 1",
                Description = "This is a description for thing 1"
            },
            new Thing() {
                Id = guids[1],
                Name = "Thing 2",
                Description = "This is a description for thing 2"
            },
            new Thing() {
                Id = guids[2],
                Name = "Thing 3",
                Description = "This is a description for thing 3"
            },
        };

        client.IndexMany(items);
    }

    [ElasticType(IdProperty = "Id", Name = "thing")]
    public class Thing
    {
        public Guid Id { get; set; }
        public string Name { get; set; }
        public string Description { get; set; }

        public override string ToString()
        {
            return String.Format("Id: {0}{3}" +
                                 "Name: {1}{3}" +
                                 "Description: {2}",
                                 Name, Id, Description ?? "<null>", Environment.NewLine);
        }
    }
}   
@gmarz
Copy link
Contributor

gmarz commented Aug 6, 2014

@lakario Yup, I just confirmed this as well. I'm looking into it, thanks for following up.

@Mpdreamz
Copy link
Member

Mpdreamz commented Aug 6, 2014

Yeah huge props for the code to reproduce the issue 👍, we should have confirmed that we saw your comment on #849 quicker :)

@lakario
Copy link
Author

lakario commented Aug 6, 2014

Thanks guys. I wasn't sure if notifications stopped on closed issues so I opened a new one just in case.

@gmarz
Copy link
Contributor

gmarz commented Aug 7, 2014

@Mpdreamz so I think the issue here lies in ApiGenerator, where we are explicitly renaming the source query params from the REST spec.

var globalQueryStringRenames = new Dictionary<string, string>
{
    {"_source", "source_enabled"},
    {"_source_include", "source_include"},
    {"_source_exclude", "source_exclude"},
};

I'm not sure of the nature or history of why we're doing this, but it's causing the API calls to be incorrect.

@Mpdreamz
Copy link
Member

Mpdreamz commented Aug 7, 2014

That bit is to be able to override the generated C# method names.

There was a bug though that caused them to send the wrong querystring params that I though I fixed in this commit:

1cd084e

@gmarz
Copy link
Contributor

gmarz commented Aug 7, 2014

Ah, I see. The problem is actually with the RequestParametersExtensions generated class still using the wrong param names. If you use the methods that take params string[] you're fine, but if you use the overloaded methods that take expressions then you'll run into this bug.

@gmarz gmarz closed this as completed in ff207b8 Aug 7, 2014
@gmarz
Copy link
Contributor

gmarz commented Aug 7, 2014

Pushed the above fix. @lakario feel free to get latest and confirm. Thanks again for reporting.

@lakario
Copy link
Author

lakario commented Aug 7, 2014

Fix looks good. Thanks guys.

@lakario
Copy link
Author

lakario commented Feb 5, 2015

Just verified locally under the exact same conditions as originally.

New issue opened: #1235.

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

3 participants