From 08a873eae57f24e74b7c829fd2cfb3d9f5e96127 Mon Sep 17 00:00:00 2001 From: DumboJetEngine Date: Thu, 11 Jan 2024 00:14:34 +0200 Subject: [PATCH] Fix for #375 - Added `.Strict(bool)` and `.Verbatim(bool)` methods on `QueryContainer` that recursively apply the respective attribute to all subqueries. Also added a `.Name(string)` method on `QueryContainer` that names the (root) contained query. Signed-off-by: Kostas --- CHANGELOG.md | 6 ++ .../Mapping/Visitor/PropertyWalker.cs | 20 ++++- .../Container/QueryContainer-Dsl.cs | 32 +++++++ .../Visitor/QueryNodeModifierVisitor.cs | 38 +++++++++ tests/Tests/Mapping/PropertyWalkerTests.cs | 84 +++++++++++++++++++ .../QueryDsl/Container/QueryContainerTests.cs | 72 ++++++++++++++++ 6 files changed, 249 insertions(+), 3 deletions(-) create mode 100644 src/OpenSearch.Client/QueryDsl/Visitor/QueryNodeModifierVisitor.cs create mode 100644 tests/Tests/Mapping/PropertyWalkerTests.cs create mode 100644 tests/Tests/QueryDsl/Container/QueryContainerTests.cs diff --git a/CHANGELOG.md b/CHANGELOG.md index 92b97d81c5..c8ca0a7ac7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,12 +6,18 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - Removed support for the `net461` target ([#256](https://github.com/opensearch-project/opensearch-net/pull/256)) - Fixed naming of `ClusterManagerTimeout` and `MasterTimeout` properties from `*TimeSpanout` in the low-level client ([#332](https://github.com/opensearch-project/opensearch-net/pull/332)) + +### Added +- Added `.Strict(bool)` and `.Verbatim(bool)` methods on `QueryContainer` that recursively apply the respective attribute to all subqueries. Also added a `.Name(string)` method on `QueryContainer` that names the (root) contained query. + ### Removed - Removed the `Features` API which is not supported by OpenSearch from the low-level client ([#331](https://github.com/opensearch-project/opensearch-net/pull/331)) - Removed the deprecated low-level `IndexTemplateV2` APIs in favour of the `ComposableIndexTemplate` APIs ([#437](https://github.com/opensearch-project/opensearch-net/pull/437)) ### Fixed - Fix `HttpConnection.ConvertHttpMethod` to support `Patch` method ([#489](https://github.com/opensearch-project/opensearch-net/pull/489)) +- Fixed `IEnumerable` property mapping. ([#503](https://github.com/opensearch-project/opensearch-net/pull/503)) + ### Dependencies - Bumps `Microsoft.CodeAnalysis.CSharp` from 4.2.0 to 4.6.0 diff --git a/src/OpenSearch.Client/Mapping/Visitor/PropertyWalker.cs b/src/OpenSearch.Client/Mapping/Visitor/PropertyWalker.cs index fcd72b7415..5734b2d21b 100644 --- a/src/OpenSearch.Client/Mapping/Visitor/PropertyWalker.cs +++ b/src/OpenSearch.Client/Mapping/Visitor/PropertyWalker.cs @@ -205,11 +205,25 @@ private static Type GetUnderlyingType(Type type) if (type.IsArray) return type.GetElementType(); - if (type.IsGenericType && type.GetGenericArguments().Length == 1 - && (type.GetInterfaces().HasAny(t => t == typeof(IEnumerable)) || Nullable.GetUnderlyingType(type) != null)) - return type.GetGenericArguments()[0]; + if (ShouldUnwrapType(type)) + { + var returnType = type.GetGenericArguments()[0]; + if (ShouldUnwrapType(returnType)) // This is needed for types like IEnumerable. + { + return returnType.GetGenericArguments()[0]; + } + return returnType; + } return type; } + + private static bool ShouldUnwrapType(Type ty) => + ty.IsGenericType && + ty.GetGenericArguments().Length == 1 && + ( + ty.GetInterfaces().HasAny(t => t == typeof(IEnumerable)) || + Nullable.GetUnderlyingType(ty) != null + ); } } diff --git a/src/OpenSearch.Client/QueryDsl/Abstractions/Container/QueryContainer-Dsl.cs b/src/OpenSearch.Client/QueryDsl/Abstractions/Container/QueryContainer-Dsl.cs index 2d4221d6e4..625296d768 100644 --- a/src/OpenSearch.Client/QueryDsl/Abstractions/Container/QueryContainer-Dsl.cs +++ b/src/OpenSearch.Client/QueryDsl/Abstractions/Container/QueryContainer-Dsl.cs @@ -28,6 +28,7 @@ using System; using System.Runtime.Serialization; +using OpenSearch.Client.QueryDsl.Visitor; using OpenSearch.Net.Utf8Json; namespace OpenSearch.Client @@ -131,5 +132,36 @@ out QueryContainer queryContainer // ReSharper disable once UnusedMember.Global internal bool ShouldSerialize(IJsonFormatterResolver formatterResolver) => IsWritable; + + + public QueryContainer Name(string name) + { + ContainedQuery.Name = name; + return this; + } + + /// + /// Applies or removes the `strict` attribute to the query container and all child sub-queries. + /// + /// + /// + public QueryContainer Strict(bool strict) + { + var visitor = new QueryNodeModifierVisitor((query, ctx) => query.IsStrict = strict); + Accept(visitor); + return this; + } + + /// + /// Applies or removes the `verbatim` attribute to the query container and all child sub-queries. + /// + /// + /// + public QueryContainer Verbatim(bool verbatim) + { + var visitor = new QueryNodeModifierVisitor((query, ctx) => query.IsVerbatim = verbatim); + Accept(visitor); + return this; + } } } diff --git a/src/OpenSearch.Client/QueryDsl/Visitor/QueryNodeModifierVisitor.cs b/src/OpenSearch.Client/QueryDsl/Visitor/QueryNodeModifierVisitor.cs new file mode 100644 index 0000000000..93f0936f7a --- /dev/null +++ b/src/OpenSearch.Client/QueryDsl/Visitor/QueryNodeModifierVisitor.cs @@ -0,0 +1,38 @@ +/* SPDX-License-Identifier: Apache-2.0 +* +* The OpenSearch Contributors require contributions made to +* this file be licensed under the Apache-2.0 license or a +* compatible open source license. +*/ + +using System; + +namespace OpenSearch.Client.QueryDsl.Visitor +{ + public class QueryNodeModifierVisitor : QueryVisitor + { + private readonly Action action; + private Context context; + + public struct Context + { + public int Depth { get; internal set; } + public VisitorScope Scope { get; internal set; } + } + + public QueryNodeModifierVisitor(Action action) + { + this.action = action; + context = new Context(); + } + + public override void Visit(IQuery query) + { + context.Depth = Depth; + context.Scope = Scope; + + action(query, context); + base.Visit(query); + } + } +} diff --git a/tests/Tests/Mapping/PropertyWalkerTests.cs b/tests/Tests/Mapping/PropertyWalkerTests.cs new file mode 100644 index 0000000000..4ea85b68dd --- /dev/null +++ b/tests/Tests/Mapping/PropertyWalkerTests.cs @@ -0,0 +1,84 @@ +/* SPDX-License-Identifier: Apache-2.0 +* +* The OpenSearch Contributors require contributions made to +* this file be licensed under the Apache-2.0 license or a +* compatible open source license. +*/ + +using System; +using System.Collections.Generic; +using System.Linq; +using FluentAssertions; +using OpenSearch.Client; +using OpenSearch.OpenSearch.Xunit.XunitPlumbing; + +namespace Tests.Mapping +{ + public class PropertyWalkerTests + { + [U] + public void BoolGetsMappedCorrectly() + { + var result = TestPropertyType(); + result.Should().Be("boolean"); + } + + [U] + public void StringGetsMappedCorrectly() + { + var result = TestPropertyType(); + result.Should().Be("text"); + } + + + [U] + public void DateTimeGetsMappedCorrectly() + { + var result = TestPropertyType(); + result.Should().Be("date"); + } + + [U] + public void IEnumerableOfNullableGetsMappedCorrectly() + { + var result = TestPropertyType>(); + result.Should().Be("integer"); + } + + [U] + public void IListOfNullableGetsMappedCorrectly() + { + var result = TestPropertyType>(); + result.Should().Be("integer"); + } + + [U] + public void IEnumerableOfValueTypesGetsMappedCorrectly() + { + var result = TestPropertyType>(); + result.Should().Be("integer"); + } + + [U] + public void IListOfValueTypesGetsMappedCorrectly() + { + var result = TestPropertyType>(); + result.Should().Be("integer"); + } + + private static string TestPropertyType() + { + var walker = new PropertyWalker(typeof(Test), null, 0); + var properties = walker.GetProperties(); + var result = properties.SingleOrDefault(); + return result.Value?.Type; + } + + + private class Test + { + public T Values { get; set; } + } + + } +} diff --git a/tests/Tests/QueryDsl/Container/QueryContainerTests.cs b/tests/Tests/QueryDsl/Container/QueryContainerTests.cs new file mode 100644 index 0000000000..7db7980770 --- /dev/null +++ b/tests/Tests/QueryDsl/Container/QueryContainerTests.cs @@ -0,0 +1,72 @@ +/* SPDX-License-Identifier: Apache-2.0 +* +* The OpenSearch Contributors require contributions made to +* this file be licensed under the Apache-2.0 license or a +* compatible open source license. +*/ + +using FluentAssertions; +using OpenSearch.Client; +using OpenSearch.OpenSearch.Xunit.XunitPlumbing; +using Xunit; + +namespace Tests.QueryDsl.Container +{ + public class QueryContainerTests + { + [TU] + [InlineData(false, false)] + [InlineData(false, true)] + [InlineData(true, false)] + [InlineData(true, true)] + public void StrictAndVerbatimAttributesAreRecursivelySetCorrectly(bool targetStrict, bool targetVerbatim) + { + // Arrange + var query0 = new TermQuery { Field = "field", Value = 1, IsStrict = !targetStrict, IsVerbatim = !targetVerbatim }; + var query1 = new BoolQuery { MustNot = new QueryContainer[] { query0 } }; + var query2 = new TermQuery { Field = "field2", Value = 7, IsStrict = !targetStrict, IsVerbatim = !targetVerbatim }; + var query3 = new BoolQuery { Must = new QueryContainer[] { query1, query2 } }; + var queryContainer = new QueryContainer(query3); + + // Act + queryContainer.Strict(targetStrict); + queryContainer.Verbatim(targetVerbatim); + + // Assert + query0.IsStrict.Should().Be(targetStrict); + query0.IsVerbatim.Should().Be(targetVerbatim); + query1.IsStrict.Should().Be(targetStrict); + query1.IsVerbatim.Should().Be(targetVerbatim); + query2.IsStrict.Should().Be(targetStrict); + query2.IsVerbatim.Should().Be(targetVerbatim); + query3.IsStrict.Should().Be(targetStrict); + query3.IsVerbatim.Should().Be(targetVerbatim); + } + + [TU] + [InlineData("name1")] + [InlineData("a name")] + [InlineData(null)] + public void SettingTheNameOnTheQueryContainerSetTheNameOnTheContainedQuery(string name) + { + // Arrange + var query0 = new TermQuery { Name = "a", Field = "field", Value = 1 }; + var query1 = new BoolQuery { Name = "b", MustNot = new QueryContainer[] { query0 } }; + var query2 = new TermQuery { Name = "c", Field = "field2", Value = 7 }; + var query3 = new BoolQuery { Name = "d", Must = new QueryContainer[] { query1, query2 } }; + var queryContainer = new QueryContainer(query3); + + // Act + queryContainer.Name(name); + + // Assert + query3.Name.Should().Be(name); + queryContainer.ContainedQuery.Name.Should().Be(name); + query0.Name.Should().Be("a"); + query1.Name.Should().Be("b"); + query2.Name.Should().Be("c"); + } + + + } +}