-
Notifications
You must be signed in to change notification settings - Fork 19
[Azure Search] Add initial query parsing #482
Conversation
src/NuGet.Services.AzureSearch/SearchService/SearchTextBuilder.cs
Outdated
Show resolved
Hide resolved
src/NuGet.Services.AzureSearch/SearchService/SearchTextBuilder.cs
Outdated
Show resolved
Hide resolved
src/NuGet.Services.AzureSearch/SearchService/AzureSearchQuery.cs
Outdated
Show resolved
Hide resolved
src/NuGet.Services.AzureSearch/SearchService/AzureSearchQuery.cs
Outdated
Show resolved
Hide resolved
src/NuGet.Services.AzureSearch/SearchService/AzureSearchQuery.cs
Outdated
Show resolved
Hide resolved
tests/NuGet.Services.AzureSearch.Tests/SearchService/SearchTextBuilderFacts.cs
Show resolved
Hide resolved
// See: https://github.com/NuGet/NuGetGallery/issues/6920 | ||
// See: https://github.com/NuGet/NuGetGallery/issues/6922 | ||
[Theory] | ||
[InlineData(false, "packageId:hello")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should only happen if luceneQuery
is true
right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's correct. I'm unit testing the current behavior, but this will be addressed by the TODO issues that are linked in the comment above.
src/NuGet.Services.AzureSearch/SearchService/AzureSearchQuery.cs
Outdated
Show resolved
Hide resolved
tests/NuGet.Services.AzureSearch.Tests/SearchService/SearchTextBuilderFacts.cs
Show resolved
Hide resolved
} | ||
|
||
private static void AppendEscapedString(StringBuilder result, string input) | ||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What the legacy service's behavior on surrogate pairs? This is kind of a nasty rabbit hole be we should know what the story is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😈 you can punt this till later but they shouldn't like cause an exception or give a backdoor
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved to separate issue: NuGet/NuGetGallery#6939
src/NuGet.Services.AzureSearch/SearchService/SearchTextBuilder.cs
Outdated
Show resolved
Hide resolved
src/NuGet.Services.AzureSearch/SearchService/SearchTextBuilder.cs
Outdated
Show resolved
Hide resolved
tests/NuGet.Services.AzureSearch.Tests/SearchService/SearchTextBuilderFacts.cs
Outdated
Show resolved
Hide resolved
{ "tag:a tags:a", "tags:a" }, | ||
|
||
// Single word query terms are unquoted. | ||
{ @"""a""", "+(a)" }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
⌚️
/// The type used to generate Azure Search Service queries. Used by <see cref="SearchTextBuilder"/>. | ||
/// Given the query "fieldA:value1 value2": | ||
/// | ||
/// * "value1" is a field-scoped value |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
XML documentation has markup for lists.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd rather avoid verbose XML syntax as we're not generating docs from these comments. Let me know if you feel strongly about this.
src/NuGet.Services.AzureSearch/SearchService/AzureSearchQuery.cs
Outdated
Show resolved
Hide resolved
🔔 All feedback should be addressed |
Parses NuGet queries and generates an Azure Search query. You can find documentation for Azure Search's query syntax here. You can find documentation for NuGet's query syntax here.
This is missing:
id
query field. Tracked by [Azure Search] Port the identifier analyzer NuGetGallery#6922packageId
,version
, andowners
query fields. Tracked by [Azure Search] Update index to support exact match semantics NuGetGallery#6920This change has been deployed to DEV. You can test this here: https://nuget-dev-usnc-azuresearch-staging.azurewebsites.net/query?q=packageId:Newtonsoft.Json
Addresses NuGet/NuGetGallery#6456