diff --git a/src/NRedisStack/Search/SearchResult.cs b/src/NRedisStack/Search/SearchResult.cs index 162e5828..786792ce 100644 --- a/src/NRedisStack/Search/SearchResult.cs +++ b/src/NRedisStack/Search/SearchResult.cs @@ -1,4 +1,5 @@ -using StackExchange.Redis; +using System.Diagnostics; +using StackExchange.Redis; namespace NRedisStack.Search; @@ -22,64 +23,59 @@ internal SearchResult(RedisResult[] resp, bool hasContent, bool hasScores, bool { // Calculate the step distance to walk over the results. // The order of results is id, score (if withScore), payLoad (if hasPayloads), fields - int step = 1; - int scoreOffset = 0; - int contentOffset = 1; - int payloadOffset = 0; - if (hasScores) - { - step++; - scoreOffset = 1; - contentOffset++; + int stride = 1; + if (hasScores) stride++; + if (hasPayloads) stride++; + if (hasContent) stride++; - } - if (hasContent) + // the first element is always the number of results + if (resp is not { Length: > 0 }) { - step++; - if (hasPayloads) - { - payloadOffset = scoreOffset + 1; - step++; - contentOffset++; - } + // unexpected empty case + TotalResults = 0; + Documents = []; + Debug.Assert(false, "Empty result from FT.SEARCH"); // debug only, flag as a problem } - - // the first element is always the number of results - TotalResults = (long)resp[0]; - var docs = new List((resp.Length - 1) / step); - Documents = docs; - for (int i = 1; i < resp.Length; i += step) + else { - var id = resp[i].ToString(); - double score = 1.0; - byte[]? payload = null; - RedisValue[]? fields = null; - string[]? scoreExplained = null; - if (hasScores) + TotalResults = (long)resp[0]; + int count = checked((int)(resp.Length - 1) / stride); + var docs = Documents = new List(count); + int offset = 1; // skip the first element which is the number of results + for (int docIndex = 0; docIndex < count; docIndex++) { - // if (shouldExplainScore) - // { - // var scoreResult = (RedisResult[])resp[i + scoreOffset]; - // score = (double) scoreResult[0]; - // var redisResultsScoreExplained = (RedisResult[]) scoreResult[1]; - // scoreExplained = FlatRedisResultArray(redisResultsScoreExplained).ToArray(); - // } - //else - //{ - score = (double)resp[i + scoreOffset]; - //} - } - if (hasPayloads) - { - payload = (byte[]?)resp[i + payloadOffset]; - } + var id = resp[offset++].ToString(); + double score = 1.0; + byte[]? payload = null; + RedisValue[]? fields = null; + string[]? scoreExplained = null; + if (hasScores) + { + // if (shouldExplainScore) + // { + // var scoreResult = (RedisResult[])resp[offset++]; + // score = (double) scoreResult[0]; + // var redisResultsScoreExplained = (RedisResult[]) scoreResult[1]; + // scoreExplained = FlatRedisResultArray(redisResultsScoreExplained).ToArray(); + // } + //else + //{ + score = (double)resp[offset++]; + //} + } - if (hasContent) - { - fields = (RedisValue[]?)resp[i + contentOffset]; - } + if (hasPayloads) // match logic from setup + { + payload = (byte[]?)resp[offset++]; + } + + if (hasContent) + { + fields = (RedisValue[]?)resp[offset++]; + } - docs.Add(Document.Load(id, score, payload, fields, scoreExplained)); + docs.Add(Document.Load(id, score, payload, fields, scoreExplained)); + } } } } \ No newline at end of file diff --git a/tests/NRedisStack.Tests/Search/SearchTests.cs b/tests/NRedisStack.Tests/Search/SearchTests.cs index ca37c673..154763ab 100644 --- a/tests/NRedisStack.Tests/Search/SearchTests.cs +++ b/tests/NRedisStack.Tests/Search/SearchTests.cs @@ -2732,45 +2732,63 @@ public void TestQueryAddParam_DefaultDialect(string endpointId) } [SkippableTheory] - [MemberData(nameof(EndpointsFixture.Env.AllEnvironments), MemberType = typeof(EndpointsFixture.Env))] - public async Task TestQueryAddParam_DefaultDialectAsync(string endpointId) + [MemberData(nameof(TestQueryParamsWithParams_Args))] + public async Task TestQueryAddParam_DefaultDialectAsync(string endpointId, bool withScores, bool withPayloads, bool withContent) { SkipClusterPre8(endpointId); IDatabase db = GetCleanDatabase(endpointId); var ft = db.FT(2); - var sc = new Schema().AddNumericField("numval"); - Assert.True(await ft.CreateAsync("idx", new(), sc)); + var sc = new Schema().AddNumericField("numval").AddTextField("sval"); + Assert.True(await ft.CreateAsync("idx", new FTCreateParams().PayloadField("sval"), sc)); - db.HashSet("1", "numval", 1); - db.HashSet("2", "numval", 2); - db.HashSet("3", "numval", 3); + await db.HashSetAsync("1", [new("numval", 1), new("sval", "a")]); + await db.HashSetAsync("2", [new("numval", 2), new("sval", "b")]); + await db.HashSetAsync("3", [new("numval", 3), new("sval", "c")]); await AssertIndexSizeAsync(ft, "idx", 3); Query query = new Query("@numval:[$min $max]").AddParam("min", 1).AddParam("max", 2); + ConfigureQuery(withScores, withPayloads, withContent, query); var res = await ft.SearchAsync("idx", query); Assert.Equal(2, res.TotalResults); + CheckQueryResults(res, withScores, withPayloads, withContent); + } + + public static IEnumerable TestQueryParamsWithParams_Args() + { + // for each environment, test each permutation of scores, payloads and content + foreach (var outer in EndpointsFixture.Env.AllEnvironments()) + { + var env = outer.Single(); + for (int i = 0; i < 8; i++) + { + yield return [env, (i & 4) != 0, (i & 2) != 0, (i & 1) != 0]; + } + } } [SkippableTheory] - [MemberData(nameof(EndpointsFixture.Env.AllEnvironments), MemberType = typeof(EndpointsFixture.Env))] - public void TestQueryParamsWithParams_DefaultDialect(string endpointId) + [MemberData(nameof(TestQueryParamsWithParams_Args))] + public void TestQueryParamsWithParams_DefaultDialect(string endpointId, bool withScores, bool withPayloads, bool withContent) { SkipClusterPre8(endpointId); IDatabase db = GetCleanDatabase(endpointId); var ft = db.FT(2); var sc = new Schema().AddNumericField("numval"); - Assert.True(ft.Create("idx", new(), sc)); + Assert.True(ft.Create("idx", new FTCreateParams().PayloadField("numval"), sc)); - db.HashSet("1", "numval", 1); - db.HashSet("2", "numval", 2); - db.HashSet("3", "numval", 3); + db.HashSet("1", [new("numval", 1), new("sval", "a")]); + db.HashSet("2", [new("numval", 2), new("sval", "b")]); + db.HashSet("3", [new("numval", 3), new("sval", "c")]); AssertIndexSize(ft, "idx", 3); Query query = new Query("@numval:[$min $max]").AddParam("min", 1).AddParam("max", 2); + ConfigureQuery(withScores, withPayloads, withContent, query); + var res = ft.Search("idx", query); Assert.Equal(2, res.TotalResults); + CheckQueryResults(res, withScores, withPayloads, withContent); var paramValue = new Dictionary() { @@ -2778,8 +2796,35 @@ public void TestQueryParamsWithParams_DefaultDialect(string endpointId) ["max"] = 2 }; query = new("@numval:[$min $max]"); + ConfigureQuery(withScores, withPayloads, withContent, query); res = ft.Search("idx", query.Params(paramValue)); Assert.Equal(2, res.TotalResults); + CheckQueryResults(res, withScores, withPayloads, withContent); + } + + private static void ConfigureQuery(bool withScores, bool withPayloads, bool withContent, Query query) + { + if (withScores) query.SetWithScores(); + if (withPayloads) query.SetWithPayloads(); + if (!withContent) query.SetNoContent(); + } + + private void CheckQueryResults(SearchResult result, bool withScores, bool withPayloads, bool withContent) + { + Assert.NotNull(result); + Assert.NotNull(result.Documents); + Assert.Equal(result.TotalResults, result.Documents.Count); + foreach (var doc in result.Documents) + { + _ = withScores; + // (we can't validate scores properly, due to calculation differences between v7 and v8) + + if (withPayloads) Assert.NotNull(doc.Payload); + else Assert.Null(doc.Payload); + + if (withContent) Assert.NotEmpty(doc._properties); + else Assert.Empty(doc._properties); + } } [SkippableTheory]