diff --git a/clients/dotnet/WebClient/MemoryWebClient.cs b/clients/dotnet/WebClient/MemoryWebClient.cs index 76b398831..56dc79245 100644 --- a/clients/dotnet/WebClient/MemoryWebClient.cs +++ b/clients/dotnet/WebClient/MemoryWebClient.cs @@ -320,8 +320,8 @@ private async Task ImportInternalAsync( using (StringContent documentIdContent = new(uploadRequest.DocumentId)) { List disposables = new(); - formData.Add(documentIdContent, Constants.WebServiceDocumentIdField); formData.Add(indexContent, Constants.WebServiceIndexField); + formData.Add(documentIdContent, Constants.WebServiceDocumentIdField); // Add steps to the form foreach (string? step in uploadRequest.Steps) @@ -336,9 +336,9 @@ private async Task ImportInternalAsync( // Add tags to the form foreach (KeyValuePair tag in uploadRequest.Tags.Pairs) { - var tagContent = new StringContent(tag.Value); + var tagContent = new StringContent($"{tag.Key}{Constants.ReservedEqualsChar}{tag.Value}"); disposables.Add(tagContent); - formData.Add(tagContent, tag.Key); + formData.Add(tagContent, Constants.WebServiceTagsField); } // Add files to the form @@ -360,7 +360,6 @@ private async Task ImportInternalAsync( try { HttpResponseMessage? response = await this._client.PostAsync("/upload", formData, cancellationToken).ConfigureAwait(false); - formData.Dispose(); response.EnsureSuccessStatusCode(); } catch (HttpRequestException e) when (e.Data.Contains("StatusCode")) @@ -373,6 +372,7 @@ private async Task ImportInternalAsync( } finally { + formData.Dispose(); foreach (var disposable in disposables) { disposable.Dispose(); diff --git a/nuget-package.props b/nuget-package.props index 5929991cc..673ed930a 100644 --- a/nuget-package.props +++ b/nuget-package.props @@ -1,7 +1,7 @@  - 0.24.0 + 0.25.0 false diff --git a/service/Abstractions/Constants.cs b/service/Abstractions/Constants.cs index adff1fd05..2fb236eb8 100644 --- a/service/Abstractions/Constants.cs +++ b/service/Abstractions/Constants.cs @@ -13,6 +13,9 @@ public static class Constants // Form field containing the Document ID public const string WebServiceDocumentIdField = "documentId"; + // Form field containing the list of tags + public const string WebServiceTagsField = "tags"; + // Form field containing the list of pipeline steps public const string WebServiceStepsField = "steps"; diff --git a/service/Abstractions/Models/TagCollection.cs b/service/Abstractions/Models/TagCollection.cs index 481c665f0..789a1399c 100644 --- a/service/Abstractions/Models/TagCollection.cs +++ b/service/Abstractions/Models/TagCollection.cs @@ -155,5 +155,11 @@ private static void ValidateKey(string key) { throw new KernelMemoryException("A tag name cannot contain the '=' char"); } + + // ':' is reserved for backward/forward compatibility + if (key.Contains(':')) + { + throw new KernelMemoryException("A tag name cannot contain the ':' char"); + } } } diff --git a/service/Core/WebService/HttpDocumentUploadRequest.cs b/service/Core/WebService/HttpDocumentUploadRequest.cs index 2efee3e43..cb5fa6e86 100644 --- a/service/Core/WebService/HttpDocumentUploadRequest.cs +++ b/service/Core/WebService/HttpDocumentUploadRequest.cs @@ -29,10 +29,6 @@ public class HttpDocumentUploadRequest public static async Task<(HttpDocumentUploadRequest model, bool isValid, string errMsg)> BindHttpRequestAsync( HttpRequest httpRequest, CancellationToken cancellationToken = default) { - const string IndexField = Constants.WebServiceIndexField; - const string DocumentIdField = Constants.WebServiceDocumentIdField; - const string StepsField = Constants.WebServiceStepsField; - var result = new HttpDocumentUploadRequest(); // Content format validation @@ -50,71 +46,75 @@ public class HttpDocumentUploadRequest return (result, false, "No file was uploaded"); } - if (form.TryGetValue(IndexField, out StringValues indexes) && indexes.Count > 1) + // Only one index can be defined + if (form.TryGetValue(Constants.WebServiceIndexField, out StringValues indexes) && indexes.Count > 1) { - return (result, false, $"Invalid index name, '{IndexField}', multiple values provided"); + return (result, false, $"Invalid index name, '{Constants.WebServiceIndexField}', multiple values provided"); } - if (form.TryGetValue(DocumentIdField, out StringValues documentIds) && documentIds.Count > 1) + // Only one document ID can be defined + if (form.TryGetValue(Constants.WebServiceDocumentIdField, out StringValues documentIds) && documentIds.Count > 1) { - return (result, false, $"Invalid document ID, '{DocumentIdField}' must be a single value, not a list"); + return (result, false, $"Invalid document ID, '{Constants.WebServiceDocumentIdField}' must be a single value, not a list"); } - // Document Id is optional, e.g. used if the client wants to retry the same upload, otherwise we generate a random/unique one - var documentId = documentIds.FirstOrDefault(); + // Document Id is optional, e.g. used if the client wants to retry the same upload, otherwise a random/unique one is generated + string? documentId = documentIds.FirstOrDefault(); if (string.IsNullOrWhiteSpace(documentId)) { documentId = DateTimeOffset.Now.ToString("yyyyMMdd.HHmmss.", CultureInfo.InvariantCulture) + Guid.NewGuid().ToString("N"); } + // Optional document tags. Tags are passed in as "key:value", where a key can have multiple values. See TagCollection. + if (form.TryGetValue(Constants.WebServiceTagsField, out StringValues tags)) + { + foreach (string? tag in tags) + { + if (tag == null) { continue; } + + var keyValue = tag.Split(Constants.ReservedEqualsChar, 2); + string key = keyValue[0]; + ValidateTagName(key); + string? value = keyValue.Length == 1 ? null : keyValue[1]; + result.Tags.Add(key, value); + } + } + // Optional pipeline steps. The user can pass a custom list or leave it to the system to use the default. - if (form.TryGetValue(StepsField, out StringValues steps)) + if (form.TryGetValue(Constants.WebServiceStepsField, out StringValues steps)) { foreach (string? step in steps) { if (string.IsNullOrWhiteSpace(step)) { continue; } + // Allow step names to be separated by space, comma, semicolon var list = step.Replace(' ', ';').Replace(',', ';').Split(';'); result.Steps.AddRange(from s in list where !string.IsNullOrWhiteSpace(s) select s.Trim()); } } - result.DocumentId = documentId; result.Index = indexes[0]!; + result.DocumentId = documentId; result.Files = form.Files; - // Store any extra field as a tag - foreach (string key in form.Keys) - { - if (key == DocumentIdField - || key == IndexField - || key == StepsField - || !form.TryGetValue(key, out StringValues values)) { continue; } - - ValidateTagName(key); - foreach (string? x in values) - { - result.Tags.Add(key, x); - } - } - return (result, true, string.Empty); } - private static void ValidateTagName(string key) + private static void ValidateTagName(string tagName) { - if (key.Contains('=', StringComparison.Ordinal)) + if (tagName.StartsWith(Constants.ReservedTagsPrefix, StringComparison.Ordinal)) { - throw new KernelMemoryException("A tag name cannot contain the '=' char"); + throw new KernelMemoryException( + $"The tag name prefix '{Constants.ReservedTagsPrefix}' is reserved for internal use."); } - if (key is - Constants.ReservedDocumentIdTag + if (tagName is Constants.ReservedDocumentIdTag or Constants.ReservedFileIdTag or Constants.ReservedFilePartitionTag - or Constants.ReservedFileTypeTag) + or Constants.ReservedFileTypeTag + or Constants.ReservedSyntheticTypeTag) { - throw new KernelMemoryException($"The tag name '{key}' is reserved for internal use."); + throw new KernelMemoryException($"The tag name '{tagName}' is reserved for internal use."); } } } diff --git a/service/Service/Service.csproj b/service/Service/Service.csproj index 58ee865d8..eda2a5be6 100644 --- a/service/Service/Service.csproj +++ b/service/Service/Service.csproj @@ -36,6 +36,13 @@ + + + + + + + diff --git a/service/tests/ServiceFunctionalTests/DocumentUploadTest.cs b/service/tests/ServiceFunctionalTests/DocumentUploadTest.cs index 56be1ba28..44433c03f 100644 --- a/service/tests/ServiceFunctionalTests/DocumentUploadTest.cs +++ b/service/tests/ServiceFunctionalTests/DocumentUploadTest.cs @@ -46,4 +46,38 @@ await this._memory.ImportDocumentAsync( this.Log("Deleting memories extracted from the document"); await this._memory.DeleteDocumentAsync(Id); } + + [Fact] + [Trait("Category", "ServiceFunctionalTest")] + public async Task ItSupportTags() + { + // Arrange + const string Id = "ItSupportTags-file1-NASA-news.pdf"; + await this._memory.ImportDocumentAsync( + "file1-NASA-news.pdf", + documentId: Id, + tags: new TagCollection + { + { "type", "news" }, + { "type", "test" }, + { "ext", "pdf" } + }, + steps: Constants.PipelineWithoutSummary); + + // Act + var answer1 = await this._memory.AskAsync("What is Orion?", filter: MemoryFilters.ByTag("type", "news")); + this.Log(answer1.Result); + var answer2 = await this._memory.AskAsync("What is Orion?", filter: MemoryFilters.ByTag("type", "test")); + this.Log(answer2.Result); + var answer3 = await this._memory.AskAsync("What is Orion?", filter: MemoryFilters.ByTag("ext", "pdf")); + this.Log(answer3.Result); + var answer4 = await this._memory.AskAsync("What is Orion?", filter: MemoryFilters.ByTag("foo", "bar")); + this.Log(answer4.Result); + + // Assert + Assert.Contains("spacecraft", answer1.Result, StringComparison.OrdinalIgnoreCase); + Assert.Contains("spacecraft", answer2.Result, StringComparison.OrdinalIgnoreCase); + Assert.Contains("spacecraft", answer3.Result, StringComparison.OrdinalIgnoreCase); + Assert.Contains("NOT FOUND", answer4.Result, StringComparison.OrdinalIgnoreCase); + } }