Epinova / Epinova.Elasticsearch

A search-plugin for Episerver CMS and Commerce
MIT License
29 stars 20 forks source link

Custom Fields Not Properly Mapped to Desired Types #133

Open ryanduffing opened 3 years ago

ryanduffing commented 3 years ago

Custom fields are not being mapped properly when pulling from the returned set of search results. Complex objects don't come back at all and result in the error: Unable to cast object of type 'Newtonsoft.Json.Linq.JObject' to type 'Newtonsoft.Json.Linq.JValue. Because of this it requires developers using the library to map these custom fields on their own.

I did some quick tests in Epinova.ElasticSearch.Core.Engine.SearchEngine.cs, and it seems a small change to the Map function could fix this. My proposed changes for Epinova.ElasticSearch.Core.Engine.SearchEngine.cs is below

private static SearchHit Map(Hit hit)
{
    var searchHit = new SearchHit(hit);

    CustomProperty[] customPropertiesForType = GetCustomPropertiesForType(hit);

    if(!IsValidCustomProperty())
    {
        return searchHit;
    }

    foreach(CustomProperty property in customPropertiesForType)
    {
        if(!hit.Source.UnmappedFields.ContainsKey(property.Name) || property.Type == null)
        {
            continue;
        }

        JToken unmappedField = hit.Source.UnmappedFields[property.Name];

        if(unmappedField == null)
        {
            break;
        }

        searchHit.CustomProperties[property.Name] = unmappedField.ToObject(property.Type);
    }

    return searchHit;

    bool IsValidCustomProperty()
    {
        return customPropertiesForType.Length > 0
            && hit.Source?.UnmappedFields != null
            && hit.Source.UnmappedFields.Any(u => customPropertiesForType.Any(c => c.Name == u.Key));
    }
}

The unit test Query_ReturnsCustomProperties() in Core.Tests.Engine.SearchEngineTests.cs will have to be updated slightly as the casting fails to convert the arrays to IEnumerable\<object>. Here is the slight modification to that unit test to make it pass.

[Fact]
public void Query_ReturnsCustomProperties()
{
    SetupEngineMock("Results_With_Custom_Properties.json");
    var date = DateTime.Parse("2015-03-31T23:01:04.2493062+02:00");
    const string text = "Lorem text";
    const double dec1 = 42.1;
    const long lng1 = 42;
    var arr1 = new long[] { 1, 2, 3 };
    var arr2 = new[] { "Foo", "Bar" };

    Indexing.Instance
        .ForType<TestPage>().IncludeField("Date", _ => date)
        .ForType<TestPage>().IncludeField("Text", _ => text)
        .ForType<TestPage>().IncludeField("Int", _ => lng1)
        .ForType<TestPage>().IncludeField("Dec", _ => dec1)
        .ForType<TestPage>().IncludeField("Array1", _ => arr1)
        .ForType<TestPage>().IncludeField("Array2", _ => arr2);

    SearchResult result = _engine.Query(_dummyQuery, CultureInfo.InvariantCulture);

    Assert.Equal(date, result.Hits.First().CustomProperties["Date"]);
    Assert.Equal(text, result.Hits.First().CustomProperties["Text"]);
    Assert.Equal(lng1, result.Hits.First().CustomProperties["Int"]);
    Assert.Equal(dec1, result.Hits.First().CustomProperties["Dec"]);
    Assert.True(Factory.ArrayEquals(arr1, result.Hits.First().CustomProperties["Array1"] as IEnumerable<long>));
    Assert.True(Factory.ArrayEquals(arr2, result.Hits.First().CustomProperties["Array2"] as IEnumerable<string>));
}
otanum commented 2 years ago

I have added some additional test data and tests. Merged to master. Please validate @ryanduffing.

Fixed: 11.7.4.233+ Pull request: https://github.com/Epinova/Epinova.Elasticsearch/pull/163