Skip to content

Commit 90e16b1

Browse files
alzareialzarei
committed
Uncheck this commit when reviewing PR6 please.
temp: Borrow PR5 changes temporarily for CI/CD validation This commit contains all changes from PR5 (feature-text-search-linq-pr5). These changes are being temporarily included in PR6 because: - PR5 has not been merged yet - PR6 depends on PR5 to pass CI/CD validation - After PR5 is merged, PR6 will be rebased to remove this commit - The final PR6 will show only PR6-specific changes without the extra diff - Uncheck this commit when reviewing PR6 please. Original PR5 summary: feat: Modernize TavilyTextSearch and BraveTextSearch with ITextSearch<TRecord> interface - Add TavilyWebPage and BraveWebPage model classes for type-safe LINQ filtering - Implement dual interface support (ITextSearch + ITextSearch<TRecord>) in both connectors - Add LINQ-to-API conversion logic for Tavily and Brave search parameters - Maintain 100% backward compatibility with existing ITextSearch usage - Enable compile-time type safety and IntelliSense support for web search filters Addresses #10456 Enhance text search LINQ support with OR/NOT operators and improved error messages - Added support for OrElse (||) operators in LINQ expressions - Added NotEqual (!=) operator support with appropriate error handling - Added UnaryExpression (NOT) support with proper limitations - Enhanced Contains method support for array.Contains(property) patterns - Improved error messages with usage examples - Added constants for API parameter names for better maintainability - Added TODO comments for potential code sharing opportunities Changes apply to both BraveTextSearch and TavilyTextSearch implementations. fix: Resolve CA1056 code analysis violations in web search connectors - Changed Url property from string? to Uri? in TavilyWebPage - Changed Url property from string? to Uri? in BraveWebPage - Updated constructors to accept Uri? parameters - Added null-safe string-to-Uri conversions in factory methods - Fixed image URL handling in TavilyTextSearch Resolves all 31 CA1056 violations (URI properties should not be strings). feat: Add C# 14 MemoryExtensions.Contains compatibility to text search connectors - Implement IsMemoryExtensionsContains() detection in TavilyTextSearch and BraveTextSearch - Handle C# 14+ MemoryExtensions.Contains method resolution (static calls with 2-3 parameters) - Provide clear error messages explaining API limitations and suggesting alternatives - Maintain backward compatibility with C# 13- Enumerable.Contains patterns Validation: Build (0 warnings), Tests (1574 passed), AOT compatible Fixes #10456 feat: add generic interface tests and fix collection Contains exception handling - Added comprehensive generic interface tests for both Tavily and Brave connectors - TavilyTextSearchTests: 3 new tests for ITextSearch<TavilyWebPage> interface - BraveTextSearchTests: 3 new tests for ITextSearch<BraveWebPage> interface - Tests cover SearchAsync, GetSearchResultsAsync, and GetTextSearchResultsAsync methods - Added proper mocking with MultipleHttpMessageHandlerStub and test data - Fixed collection Contains exception handling to prevent graceful degradation - Modified ConvertToLegacyOptions in both connectors to re-throw critical exceptions - Collection Contains patterns now properly throw NotSupportedException instead of degrading - Maintains graceful degradation for other unsupported LINQ patterns - Fixed previously failing CollectionContainsFilterThrowsNotSupportedExceptionAsync test - Enhanced C# 14 MemoryExtensions.Contains compatibility - Improved error messages to cover both C# 13- (Enumerable.Contains) and C# 14+ (MemoryExtensions.Contains) - All collection Contains patterns now properly handled regardless of C# language version Test Results: - Tavily: 30 tests passed (including 3 new generic interface tests) - Brave: 24 tests passed (including 3 new generic interface tests) - All collection Contains exception tests now pass correctly Fix API type changes to return TRecord instead of object - Update BraveTextSearch and TavilyTextSearch to properly implement ITextSearch<TRecord> - Fix unit tests to use strongly-typed generic interfaces - Ensure backward compatibility with legacy ITextSearch interface - All 105 text search tests passing, full validation completed Resolves API type safety issues described in How-to-Fix-API-Type-Change-to-TRecord-PR5.md Correct approach: Revert unnecessary non-generic test changes and rename Generic→Linq - Revert GetSearchResultsReturnsSuccessfullyAsync to use non-generic interface (var textSearch, KernelSearchResults<object>) - This test should test backward compatibility with legacy ITextSearch interface - Only generic interface tests should use ITextSearch<TRecord> and KernelSearchResults<TRecord> - Rename 'Generic' test methods to 'Linq' to match naming convention from other PRs: - GenericSearchAsyncReturnsResultsSuccessfullyAsync → LinqSearchAsyncReturnsResultsSuccessfullyAsync - GenericGetSearchResultsAsyncReturnsResultsSuccessfullyAsync → LinqGetSearchResultsAsyncReturnsResultsSuccessfullyAsync - GenericGetTextSearchResultsAsyncReturnsResultsSuccessfullyAsync → LinqGetTextSearchResultsAsyncReturnsResultsSuccessfullyAsync - Applied to both BraveTextSearchTests.cs and TavilyTextSearchTests.cs - All 105 text search tests still passing Fix maintains proper test separation: - Non-generic tests: Test legacy ITextSearch interface compatibility - LINQ tests: Test new ITextSearch<TRecord> strongly-typed interface feat: Implement Title.Contains() support for Brave and Tavily text search - Add SearchQueryFilterClause for query enhancement pattern - Implement Title.Contains() → query modification for Brave and Tavily APIs - Support LINQ expressions: results.Where(r => r.Title.Contains('AI')) - Query enhancement: 'base query' + ' AI' for improved relevance - Maintains backward compatibility with legacy ITextSearch interface - All 130 web plugin tests passing Technical approach: - SearchQueryFilterClause: New filter clause type for search term addition - ConvertToLegacyOptionsWithQuery: Extract and append search terms to query - Different from Bing (direct operators) and Google (API parameters) - Optimized for APIs without field-specific search capabilities fix: Remove trailing whitespace from SearchQueryFilterClause.cs - Fix RCS1037 trailing whitespace errors on lines 12, 16, 18 - Add missing final newline to prevent FINALNEWLINE error - Fix file encoding issues (BOM removal) - Addresses GitHub CI/CD build failures for net8.0, net462, netstandard2.0 targets All formatting issues resolved to match GitHub Actions requirements.
1 parent 0ee64f1 commit 90e16b1

7 files changed

Lines changed: 1589 additions & 12 deletions

File tree

‎dotnet/src/Plugins/Plugins.UnitTests/Web/Brave/BraveTextSearchTests.cs‎

Lines changed: 149 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Copyright (c) Microsoft. All rights reserved.
22

33
#pragma warning disable CS0618 // ITextSearch is obsolete
4+
#pragma warning disable CS8602 // Dereference of a possibly null reference - for LINQ expression properties
45

56
using System;
67
using System.IO;
@@ -110,7 +111,7 @@ public async Task GetSearchResultsReturnsSuccessfullyAsync()
110111
var resultList = await result.Results.ToListAsync();
111112
Assert.NotNull(resultList);
112113
Assert.Equal(10, resultList.Count);
113-
foreach (BraveWebResult webPage in resultList)
114+
foreach (BraveWebPage webPage in resultList.Cast<BraveWebPage>())
114115
{
115116
Assert.NotNull(webPage.Title);
116117
Assert.NotNull(webPage.Description);
@@ -195,7 +196,7 @@ public async Task BuildsCorrectUriForEqualityFilterAsync(string paramName, objec
195196

196197
// Act
197198
TextSearchOptions searchOptions = new() { Top = 5, Skip = 0, Filter = new TextSearchFilter().Equality(paramName, paramValue) };
198-
KernelSearchResults<object> result = await textSearch.GetSearchResultsAsync("What is the Semantic Kernel?", searchOptions);
199+
var result = await textSearch.GetSearchResultsAsync("What is the Semantic Kernel?", searchOptions);
199200

200201
// Assert
201202
var requestUris = this._messageHandlerStub.RequestUris;
@@ -243,6 +244,151 @@ public void Dispose()
243244
GC.SuppressFinalize(this);
244245
}
245246

247+
#region Generic ITextSearch<BraveWebPage> Interface Tests
248+
249+
[Fact]
250+
public async Task LinqSearchAsyncReturnsResultsSuccessfullyAsync()
251+
{
252+
// Arrange
253+
this._messageHandlerStub.AddJsonResponse(File.ReadAllText(WhatIsTheSkResponseJson));
254+
ITextSearch<BraveWebPage> textSearch = new BraveTextSearch(apiKey: "ApiKey", options: new() { HttpClient = this._httpClient });
255+
256+
// Act
257+
var searchOptions = new TextSearchOptions<BraveWebPage>
258+
{
259+
Top = 4,
260+
Skip = 0
261+
};
262+
KernelSearchResults<string> result = await textSearch.SearchAsync("What is the Semantic Kernel?", searchOptions);
263+
264+
// Assert - Verify basic generic interface functionality
265+
Assert.NotNull(result);
266+
Assert.NotNull(result.Results);
267+
var resultList = await result.Results.ToListAsync();
268+
Assert.NotEmpty(resultList);
269+
270+
// Verify the request was made correctly
271+
var requestUris = this._messageHandlerStub.RequestUris;
272+
Assert.Single(requestUris);
273+
Assert.NotNull(requestUris[0]);
274+
Assert.Contains("count=4", requestUris[0].AbsoluteUri);
275+
}
276+
277+
[Fact]
278+
public async Task LinqGetSearchResultsAsyncReturnsResultsSuccessfullyAsync()
279+
{
280+
// Arrange
281+
this._messageHandlerStub.AddJsonResponse(File.ReadAllText(WhatIsTheSkResponseJson));
282+
ITextSearch<BraveWebPage> textSearch = new BraveTextSearch(apiKey: "ApiKey", options: new() { HttpClient = this._httpClient });
283+
284+
// Act
285+
var searchOptions = new TextSearchOptions<BraveWebPage>
286+
{
287+
Top = 3,
288+
Skip = 0
289+
};
290+
KernelSearchResults<BraveWebPage> result = await textSearch.GetSearchResultsAsync("What is the Semantic Kernel?", searchOptions);
291+
292+
// Assert - Verify generic interface returns results
293+
Assert.NotNull(result);
294+
Assert.NotNull(result.Results);
295+
var resultList = await result.Results.ToListAsync();
296+
Assert.NotEmpty(resultList);
297+
// Results are now strongly typed as BraveWebPage
298+
299+
// Verify the request was made correctly
300+
var requestUris = this._messageHandlerStub.RequestUris;
301+
Assert.Single(requestUris);
302+
Assert.NotNull(requestUris[0]);
303+
Assert.Contains("count=3", requestUris[0].AbsoluteUri);
304+
}
305+
306+
[Fact]
307+
public async Task LinqGetTextSearchResultsAsyncReturnsResultsSuccessfullyAsync()
308+
{
309+
// Arrange
310+
this._messageHandlerStub.AddJsonResponse(File.ReadAllText(WhatIsTheSkResponseJson));
311+
ITextSearch<BraveWebPage> textSearch = new BraveTextSearch(apiKey: "ApiKey", options: new() { HttpClient = this._httpClient });
312+
313+
// Act
314+
var searchOptions = new TextSearchOptions<BraveWebPage>
315+
{
316+
Top = 5,
317+
Skip = 0
318+
};
319+
KernelSearchResults<TextSearchResult> result = await textSearch.GetTextSearchResultsAsync("What is the Semantic Kernel?", searchOptions);
320+
321+
// Assert - Verify generic interface returns TextSearchResult objects
322+
Assert.NotNull(result);
323+
Assert.NotNull(result.Results);
324+
var resultList = await result.Results.ToListAsync();
325+
Assert.NotEmpty(resultList);
326+
Assert.All(resultList, item => Assert.IsType<TextSearchResult>(item));
327+
328+
// Verify the request was made correctly
329+
var requestUris = this._messageHandlerStub.RequestUris;
330+
Assert.Single(requestUris);
331+
Assert.NotNull(requestUris[0]);
332+
Assert.Contains("count=5", requestUris[0].AbsoluteUri);
333+
}
334+
335+
[Fact]
336+
public async Task CollectionContainsFilterThrowsNotSupportedExceptionAsync()
337+
{
338+
// Arrange - Tests both Enumerable.Contains (C# 13-) and MemoryExtensions.Contains (C# 14+)
339+
// The same code array.Contains() resolves differently based on C# language version:
340+
// - C# 13 and earlier: Enumerable.Contains (LINQ extension method)
341+
// - C# 14 and later: MemoryExtensions.Contains (span-based optimization due to "first-class spans")
342+
// Our implementation handles both identically since Brave API has limited query operators
343+
this._messageHandlerStub.AddJsonResponse(File.ReadAllText(WhatIsTheSkResponseJson));
344+
ITextSearch<BraveWebPage> textSearch = new BraveTextSearch(apiKey: "ApiKey", options: new() { HttpClient = this._httpClient });
345+
string[] sites = ["microsoft.com", "github.com"];
346+
347+
// Act & Assert - Verify that collection Contains pattern throws clear exception
348+
var searchOptions = new TextSearchOptions<BraveWebPage>
349+
{
350+
Top = 5,
351+
Skip = 0,
352+
Filter = page => sites.Contains(page.Url!.ToString()) // Enumerable.Contains (C# 13-) or MemoryExtensions.Contains (C# 14+)
353+
};
354+
355+
var exception = await Assert.ThrowsAsync<NotSupportedException>(async () =>
356+
{
357+
await textSearch.SearchAsync("test", searchOptions);
358+
});
359+
360+
// Assert - Verify error message explains the limitation clearly
361+
Assert.Contains("Collection Contains filters", exception.Message);
362+
Assert.Contains("not supported", exception.Message);
363+
}
364+
365+
[Fact]
366+
public async Task StringContainsStillWorksWithLINQFiltersAsync()
367+
{
368+
// Arrange - Verify that String.Contains (instance method) still works
369+
// String.Contains is NOT affected by C# 14 "first-class spans" - only arrays are
370+
this._messageHandlerStub.AddJsonResponse(File.ReadAllText(WhatIsTheSkResponseJson));
371+
ITextSearch<BraveWebPage> textSearch = new BraveTextSearch(apiKey: "ApiKey", options: new() { HttpClient = this._httpClient });
372+
373+
// Act - String.Contains should continue to work
374+
var searchOptions = new TextSearchOptions<BraveWebPage>
375+
{
376+
Top = 5,
377+
Skip = 0,
378+
Filter = page => page.Title.Contains("Kernel") // String.Contains - instance method
379+
};
380+
KernelSearchResults<string> result = await textSearch.SearchAsync("Semantic Kernel tutorial", searchOptions);
381+
382+
// Assert - Verify String.Contains works correctly
383+
var requestUris = this._messageHandlerStub.RequestUris;
384+
Assert.Single(requestUris);
385+
Assert.NotNull(requestUris[0]);
386+
Assert.Contains("Kernel", requestUris[0].AbsoluteUri);
387+
Assert.Contains("count=5", requestUris[0].AbsoluteUri);
388+
}
389+
390+
#endregion
391+
246392
#region private
247393
private const string WhatIsTheSkResponseJson = "./TestData/brave_what_is_the_semantic_kernel.json";
248394
private const string SiteFilterSkResponseJson = "./TestData/brave_site_filter_what_is_the_semantic_kernel.json";
@@ -273,7 +419,7 @@ public TextSearchResult MapFromResultToTextSearchResult(object result)
273419
{
274420
if (result is not BraveWebResult webPage)
275421
{
276-
throw new ArgumentException("Result must be a BraveWebPage", nameof(result));
422+
throw new ArgumentException("Result must be a BraveWebResult", nameof(result));
277423
}
278424

279425
return new TextSearchResult(webPage.Description?.ToUpperInvariant() ?? string.Empty)

‎dotnet/src/Plugins/Plugins.UnitTests/Web/Tavily/TavilyTextSearchTests.cs‎

Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Copyright (c) Microsoft. All rights reserved.
22

33
#pragma warning disable CS0618 // ITextSearch is obsolete
4+
#pragma warning disable CS8602 // Dereference of a possibly null reference - for LINQ expression properties
45

56
using System;
67
using System.IO;
@@ -346,6 +347,156 @@ public void Dispose()
346347
GC.SuppressFinalize(this);
347348
}
348349

350+
#region Generic ITextSearch<TavilyWebPage> Interface Tests
351+
352+
[Fact]
353+
public async Task LinqSearchAsyncReturnsResultsSuccessfullyAsync()
354+
{
355+
// Arrange
356+
this._messageHandlerStub.AddJsonResponse(File.ReadAllText(SiteFilterDevBlogsResponseJson));
357+
ITextSearch<TavilyWebPage> textSearch = new TavilyTextSearch(apiKey: "ApiKey", options: new() { HttpClient = this._httpClient });
358+
359+
// Act
360+
var searchOptions = new TextSearchOptions<TavilyWebPage>
361+
{
362+
Top = 4,
363+
Skip = 0
364+
};
365+
KernelSearchResults<string> result = await textSearch.SearchAsync("What is the Semantic Kernel?", searchOptions);
366+
367+
// Assert - Verify basic generic interface functionality
368+
Assert.NotNull(result);
369+
Assert.NotNull(result.Results);
370+
var resultList = await result.Results.ToListAsync();
371+
Assert.NotEmpty(resultList);
372+
373+
// Verify the request was made correctly
374+
var requestContents = this._messageHandlerStub.RequestContents;
375+
Assert.Single(requestContents);
376+
Assert.NotNull(requestContents[0]);
377+
var requestBodyJson = Encoding.UTF8.GetString(requestContents[0]!);
378+
Assert.Contains("\"query\"", requestBodyJson);
379+
Assert.Contains("\"max_results\":4", requestBodyJson);
380+
}
381+
382+
[Fact]
383+
public async Task LinqGetSearchResultsAsyncReturnsResultsSuccessfullyAsync()
384+
{
385+
// Arrange
386+
this._messageHandlerStub.AddJsonResponse(File.ReadAllText(SiteFilterDevBlogsResponseJson));
387+
ITextSearch<TavilyWebPage> textSearch = new TavilyTextSearch(apiKey: "ApiKey", options: new() { HttpClient = this._httpClient });
388+
389+
// Act
390+
var searchOptions = new TextSearchOptions<TavilyWebPage>
391+
{
392+
Top = 3,
393+
Skip = 0
394+
};
395+
KernelSearchResults<TavilyWebPage> result = await textSearch.GetSearchResultsAsync("What is the Semantic Kernel?", searchOptions);
396+
397+
// Assert - Verify generic interface returns results
398+
Assert.NotNull(result);
399+
Assert.NotNull(result.Results);
400+
var resultList = await result.Results.ToListAsync();
401+
Assert.NotEmpty(resultList);
402+
// Results are now strongly typed as TavilyWebPage
403+
404+
// Verify the request was made correctly
405+
var requestContents = this._messageHandlerStub.RequestContents;
406+
Assert.Single(requestContents);
407+
Assert.NotNull(requestContents[0]);
408+
var requestBodyJson = Encoding.UTF8.GetString(requestContents[0]!);
409+
Assert.Contains("\"max_results\":3", requestBodyJson);
410+
}
411+
412+
[Fact]
413+
public async Task LinqGetTextSearchResultsAsyncReturnsResultsSuccessfullyAsync()
414+
{
415+
// Arrange
416+
this._messageHandlerStub.AddJsonResponse(File.ReadAllText(SiteFilterDevBlogsResponseJson));
417+
ITextSearch<TavilyWebPage> textSearch = new TavilyTextSearch(apiKey: "ApiKey", options: new() { HttpClient = this._httpClient });
418+
419+
// Act
420+
var searchOptions = new TextSearchOptions<TavilyWebPage>
421+
{
422+
Top = 5,
423+
Skip = 0
424+
};
425+
KernelSearchResults<TextSearchResult> result = await textSearch.GetTextSearchResultsAsync("What is the Semantic Kernel?", searchOptions);
426+
427+
// Assert - Verify generic interface returns TextSearchResult objects
428+
Assert.NotNull(result);
429+
Assert.NotNull(result.Results);
430+
var resultList = await result.Results.ToListAsync();
431+
Assert.NotEmpty(resultList);
432+
Assert.All(resultList, item => Assert.IsType<TextSearchResult>(item));
433+
434+
// Verify the request was made correctly
435+
var requestContents = this._messageHandlerStub.RequestContents;
436+
Assert.Single(requestContents);
437+
Assert.NotNull(requestContents[0]);
438+
var requestBodyJson = Encoding.UTF8.GetString(requestContents[0]!);
439+
Assert.Contains("\"max_results\":5", requestBodyJson);
440+
}
441+
442+
[Fact]
443+
public async Task CollectionContainsFilterThrowsNotSupportedExceptionAsync()
444+
{
445+
// Arrange - Tests both Enumerable.Contains (C# 13-) and MemoryExtensions.Contains (C# 14+)
446+
// The same code array.Contains() resolves differently based on C# language version:
447+
// - C# 13 and earlier: Enumerable.Contains (LINQ extension method)
448+
// - C# 14 and later: MemoryExtensions.Contains (span-based optimization due to "first-class spans")
449+
// Our implementation handles both identically since Tavily API has limited query operators
450+
this._messageHandlerStub.AddJsonResponse(File.ReadAllText(SiteFilterDevBlogsResponseJson));
451+
ITextSearch<TavilyWebPage> textSearch = new TavilyTextSearch(apiKey: "ApiKey", options: new() { HttpClient = this._httpClient });
452+
string[] domains = ["microsoft.com", "github.com"];
453+
454+
// Act & Assert - Verify that collection Contains pattern throws clear exception
455+
var searchOptions = new TextSearchOptions<TavilyWebPage>
456+
{
457+
Top = 5,
458+
Skip = 0,
459+
Filter = page => domains.Contains(page.Url!.ToString()) // Enumerable.Contains (C# 13-) or MemoryExtensions.Contains (C# 14+)
460+
};
461+
462+
var exception = await Assert.ThrowsAsync<NotSupportedException>(async () =>
463+
{
464+
await textSearch.SearchAsync("test", searchOptions);
465+
});
466+
467+
// Assert - Verify error message explains the limitation clearly
468+
Assert.Contains("Collection Contains filters", exception.Message);
469+
Assert.Contains("not supported", exception.Message);
470+
}
471+
472+
[Fact]
473+
public async Task StringContainsStillWorksWithLINQFiltersAsync()
474+
{
475+
// Arrange - Verify that String.Contains (instance method) still works
476+
// String.Contains is NOT affected by C# 14 "first-class spans" - only arrays are
477+
this._messageHandlerStub.AddJsonResponse(File.ReadAllText(SiteFilterDevBlogsResponseJson));
478+
ITextSearch<TavilyWebPage> textSearch = new TavilyTextSearch(apiKey: "ApiKey", options: new() { HttpClient = this._httpClient });
479+
480+
// Act - String.Contains should continue to work
481+
var searchOptions = new TextSearchOptions<TavilyWebPage>
482+
{
483+
Top = 5,
484+
Skip = 0,
485+
Filter = page => page.Title.Contains("Kernel") // String.Contains - instance method
486+
};
487+
KernelSearchResults<string> result = await textSearch.SearchAsync("Semantic Kernel tutorial", searchOptions);
488+
489+
// Assert - Verify String.Contains works correctly
490+
var requestContents = this._messageHandlerStub.RequestContents;
491+
Assert.Single(requestContents);
492+
Assert.NotNull(requestContents[0]);
493+
var requestBodyJson = Encoding.UTF8.GetString(requestContents[0]!);
494+
Assert.Contains("Kernel", requestBodyJson);
495+
Assert.Contains("\"max_results\":5", requestBodyJson);
496+
}
497+
498+
#endregion
499+
349500
#region private
350501
private const string WhatIsTheSKResponseJson = "./TestData/tavily_what_is_the_semantic_kernel.json";
351502
private const string SiteFilterDevBlogsResponseJson = "./TestData/tavily_site_filter_devblogs_microsoft.com.json";

0 commit comments

Comments
 (0)