[FR] Update service version, TODO cleanup, and add missing xxOperatio…

…n tests (#17387)

* service version and easy TODO cleanup

* parameter validation avoiding breaking changes

* add missing tests

* update docstring

* remove checks that throw in other methods

sdk/formrecognizer/Azure.AI.FormRecognizer/api/Azure.AI.FormRecognizer.netstandard2.0.cs

@@ -44,11 +44,12 @@ public FormRecognizerClient(System.Uri endpoint, Azure.Core.TokenCredential cred
     }
     public partial class FormRecognizerClientOptions : Azure.Core.ClientOptions
     {
-        public FormRecognizerClientOptions(Azure.AI.FormRecognizer.FormRecognizerClientOptions.ServiceVersion version = Azure.AI.FormRecognizer.FormRecognizerClientOptions.ServiceVersion.V2_0) { }
+        public FormRecognizerClientOptions(Azure.AI.FormRecognizer.FormRecognizerClientOptions.ServiceVersion version = Azure.AI.FormRecognizer.FormRecognizerClientOptions.ServiceVersion.V2_1_Preview_2) { }
         public Azure.AI.FormRecognizer.FormRecognizerClientOptions.ServiceVersion Version { get { throw null; } }
         public enum ServiceVersion
         {
             V2_0 = 1,
+            V2_1_Preview_2 = 2,
         }
     }
     public static partial class OperationExtensions

sdk/formrecognizer/Azure.AI.FormRecognizer/src/CopyModelOperation.cs

@@ -86,6 +86,9 @@ public override CustomFormModelInfo Value
         /// <param name="client">The client used to check for completion.</param>
         public CopyModelOperation(string operationId, string targetModelId, FormTrainingClient client)
         {
+            Argument.AssertNotNullOrEmpty(operationId, nameof(operationId));
+            Argument.AssertNotNull(client, nameof(client));
+
             _serviceClient = client.ServiceClient;
             _diagnostics = client.Diagnostics;
             _targetModelId = targetModelId;

sdk/formrecognizer/Azure.AI.FormRecognizer/src/CreateCustomFormModelOperation.cs

@@ -123,6 +123,8 @@ internal CreateCustomFormModelOperation(string location, FormRecognizerRestClien
         /// <param name="client">The client used to check for completion.</param>
         public CreateCustomFormModelOperation(string operationId, FormTrainingClient client)
         {
+            Argument.AssertNotNull(client, nameof(client));
+
             Id = operationId;
             _diagnostics = client.Diagnostics;
             _serviceClient = client.ServiceClient;

sdk/formrecognizer/Azure.AI.FormRecognizer/src/FormField.cs

@@ -53,7 +53,6 @@ internal FormField(string name, FieldValue_internal fieldValue, IReadOnlyList<Re
             {
                 IReadOnlyList<FormElement> fieldElements = ConvertTextReferences(fieldValue.Elements, readResults);
 
-                // TODO: FormEnum<T> ?
                 FieldBoundingBox boundingBox = new FieldBoundingBox(fieldValue.BoundingBox);
 
                 ValueData = new FieldData(boundingBox, fieldValue.Page.Value, fieldValue.Text, fieldElements);
@@ -120,9 +119,6 @@ internal static IReadOnlyList<FormElement> ConvertTextReferences(IReadOnlyList<s
 
         private static FormElement ResolveTextReference(IReadOnlyList<ReadResult> readResults, string reference)
         {
-            // TODO: Add additional validations here.
-            // https://github.com/Azure/azure-sdk-for-net/issues/10363
-
             // Example: the following should result in PageIndex = 3, LineIndex = 7, WordIndex = 12
             // "#/analyzeResult/readResults/3/lines/7/words/12" from DocumentResult
             // "#/readResults/3/lines/7/words/12" from PageResult

sdk/formrecognizer/Azure.AI.FormRecognizer/src/FormRecognizerClientOptions.cs

@@ -13,7 +13,7 @@ namespace Azure.AI.FormRecognizer
     /// </summary>
     public class FormRecognizerClientOptions : ClientOptions
     {
-        internal const ServiceVersion LatestVersion = ServiceVersion.V2_0;
+        internal const ServiceVersion LatestVersion = ServiceVersion.V2_1_Preview_2;
 
         /// <summary>
         /// Initializes a new instance of the <see cref="FormRecognizerClientOptions"/> class which allows
@@ -35,7 +35,12 @@ public enum ServiceVersion
             /// The V2.0 of the service.
             /// </summary>
 #pragma warning disable CA1707 // Identifiers should not contain underscores
-            V2_0 = 1
+            V2_0 = 1,
+
+            /// <summary>
+            /// Version 2.1-preview.2
+            /// </summary>
+            V2_1_Preview_2 = 2,
 #pragma warning restore CA1707 // Identifiers should not contain underscores
         }
 
@@ -49,6 +54,7 @@ internal static string GetVersionString(ServiceVersion version)
             return version switch
             {
                 ServiceVersion.V2_0 => "v2.0",
+                ServiceVersion.V2_1_Preview_2 => "v2.1-preview.2",
                 _ => throw new NotSupportedException($"The service version {version} is not supported."),
             };
         }

sdk/formrecognizer/Azure.AI.FormRecognizer/src/FormTable.cs

@@ -62,10 +62,6 @@ internal FormTable(int pageNumber, int columnCount, int rowCount, IReadOnlyList<
         /// <summary> Bounding box of the table. </summary>
         public FieldBoundingBox BoundingBox { get; }
 
-        // TODO: implement table indexer
-        // TODO: Handling column-span?
-        // https://github.com/Azure/azure-sdk-for-net/issues/9975
-
         /// <summary>
         /// </summary>
 #pragma warning disable CA1822 // Mark as static

sdk/formrecognizer/Azure.AI.FormRecognizer/src/RecognizeBusinessCardsOperation.cs

@@ -85,7 +85,8 @@ public override RecognizedFormCollection Value
         /// <param name="client">The client used to check for completion.</param>
         public RecognizeBusinessCardsOperation(string operationId, FormRecognizerClient client)
         {
-            // TODO: Add argument validation here.
+            Argument.AssertNotNullOrEmpty(operationId, nameof(operationId));
+            Argument.AssertNotNull(client, nameof(client));
 
             Id = operationId;
             _serviceClient = client.ServiceClient;

sdk/formrecognizer/Azure.AI.FormRecognizer/src/RecognizeContentOperation.cs

@@ -77,7 +77,7 @@ public override FormPageCollection Value
         /// <param name="client">The client used to check for completion.</param>
         public RecognizeContentOperation(string operationId, FormRecognizerClient client)
         {
-            // TODO: Add argument validation here.
+            Argument.AssertNotNull(client, nameof(client));
 
             Id = operationId;
             _serviceClient = client.ServiceClient;

sdk/formrecognizer/Azure.AI.FormRecognizer/src/RecognizeCustomFormsOperation.cs

@@ -145,6 +145,9 @@ internal RecognizeCustomFormsOperation(FormRecognizerRestClient operations, Clie
         /// <param name="client">The client used to check for completion.</param>
         public RecognizeCustomFormsOperation(string operationId, FormRecognizerClient client)
         {
+            Argument.AssertNotNullOrEmpty(operationId, nameof(operationId));
+            Argument.AssertNotNull(client, nameof(client));
+
             _serviceClient = client.ServiceClient;
             _diagnostics = client.Diagnostics;
 
@@ -208,8 +211,6 @@ private async ValueTask<Response> UpdateStatusAsync(bool async, CancellationToke
                         ? await _serviceClient.GetAnalyzeFormResultAsync(new Guid(_modelId), new Guid(_resultId), cancellationToken).ConfigureAwait(false)
                         : _serviceClient.GetAnalyzeFormResult(new Guid(_modelId), new Guid(_resultId), cancellationToken);
 
-                    // TODO: Add reasonable null checks.
-
                     _response = update.GetRawResponse();
 
                     if (update.Value.Status == OperationStatus.Succeeded)

sdk/formrecognizer/Azure.AI.FormRecognizer/src/RecognizeInvoicesOperation.cs

@@ -85,7 +85,8 @@ public override RecognizedFormCollection Value
         /// <param name="client">The client used to check for completion.</param>
         public RecognizeInvoicesOperation(string operationId, FormRecognizerClient client)
         {
-            // TODO: Add argument validation here.
+            Argument.AssertNotNullOrEmpty(operationId, nameof(operationId));
+            Argument.AssertNotNull(client, nameof(client));
 
             Id = operationId;
             _serviceClient = client.ServiceClient;

sdk/formrecognizer/Azure.AI.FormRecognizer/src/RecognizeReceiptsOperation.cs

@@ -85,7 +85,7 @@ public override RecognizedFormCollection Value
         /// <param name="client">The client used to check for completion.</param>
         public RecognizeReceiptsOperation(string operationId, FormRecognizerClient client)
         {
-            // TODO: Add argument validation here.
+            Argument.AssertNotNull(client, nameof(client));
 
             Id = operationId;
             _serviceClient = client.ServiceClient;

sdk/formrecognizer/Azure.AI.FormRecognizer/src/RecognizedForm.cs

@@ -32,9 +32,6 @@ internal RecognizedForm(DocumentResult documentResult, IReadOnlyList<PageResult>
             // Recognized form from a model trained with labels.
             FormType = documentResult.DocType;
 
-            // TODO: validate that PageRange.Length == 2.
-            // https://github.com/Azure/azure-sdk-for-net/issues/10547
-
             PageRange = new FormPageRange(documentResult.PageRange[0], documentResult.PageRange[1]);
 
             // documentResult.Fields is required and not null, according to the swagger file, but it's not

sdk/formrecognizer/Azure.AI.FormRecognizer/tests/Models/OperationsLiveTests.cs

@@ -2,6 +2,7 @@
 // Licensed under the MIT License.
 
 using System;
+using System.Collections.Generic;
 using System.Threading.Tasks;
 using Azure.AI.FormRecognizer.Models;
 using Azure.AI.FormRecognizer.Training;
@@ -123,6 +124,26 @@ public async Task TrainingOperationCanPollFromNewObject()
             Assert.AreEqual(CustomFormModelStatus.Ready, sameOperation.Value.Status);
         }
 
+        [Test]
+        public async Task CreateComposedModelOperationCanPollFromNewObject()
+        {
+            // Skip instrumenting here because the internal service client passed to the operation object would be made null otherwise,
+            // making the test fail.
+
+            var client = CreateFormTrainingClient(skipInstrumenting: true);
+
+            await using var trainedModelA = await CreateDisposableTrainedModelAsync(useTrainingLabels: true);
+            await using var trainedModelB = await CreateDisposableTrainedModelAsync(useTrainingLabels: true);
+
+            var operation = await client.StartCreateComposedModelAsync(new List<string> { trainedModelA.ModelId, trainedModelB.ModelId });
+
+            var sameOperation = new CreateComposedModelOperation(operation.Id, client);
+            await sameOperation.WaitForCompletionAsync(PollingInterval);
+
+            Assert.IsTrue(sameOperation.HasValue);
+            Assert.AreEqual(CustomFormModelStatus.Ready, sameOperation.Value.Status);
+        }
+
         [Test]
         public async Task CopyModelOperationCanPollFromNewObject()
         {

sdk/formrecognizer/Azure.AI.FormRecognizer/tests/Models/OperationsMockTests.cs

@@ -77,6 +77,14 @@ public async Task RecognizeContentOperationCreatesDiagnosticScopeOnUpdate()
             testListener.AssertScope($"{nameof(RecognizeContentOperation)}.{nameof(RecognizeContentOperation.UpdateStatus)}");
         }
 
+        [Test]
+        public void RecognizeContentOperationRequiredParameters()
+        {
+            FormRecognizerClient client = CreateFormRecognizerClient();
+
+            Assert.Throws<ArgumentNullException>(() => new RecognizeContentOperation("00000000 - 0000 - 0000 - 0000 - 000000000000", null));
+        }
+
         [Test]
         public async Task RecognizeReceiptsOperationCreatesDiagnosticScopeOnUpdate()
         {
@@ -97,6 +105,74 @@ public async Task RecognizeReceiptsOperationCreatesDiagnosticScopeOnUpdate()
             testListener.AssertScope($"{nameof(RecognizeReceiptsOperation)}.{nameof(RecognizeReceiptsOperation.UpdateStatus)}");
         }
 
+        [Test]
+        public void RecognizeReceiptsOperationRequiredParameters()
+        {
+            FormRecognizerClient client = CreateFormRecognizerClient();
+
+            Assert.Throws<ArgumentNullException>(() => new RecognizeReceiptsOperation("00000000 - 0000 - 0000 - 0000 - 000000000000", null));
+        }
+
+        [Test]
+        public async Task RecognizeBusinessCardsOperationCreatesDiagnosticScopeOnUpdate()
+        {
+            using var testListener = new ClientDiagnosticListener(DiagnosticNamespace);
+            using var stream = new MemoryStream(Encoding.UTF8.GetBytes("{}"));
+
+            var mockResponse = new MockResponse(200);
+            mockResponse.ContentStream = stream;
+
+            var mockTransport = new MockTransport(new[] { mockResponse, mockResponse });
+            var options = new FormRecognizerClientOptions() { Transport = mockTransport };
+            var client = CreateFormRecognizerClient(options);
+
+            var operation = new RecognizeBusinessCardsOperation("00000000-0000-0000-0000-000000000000", client);
+
+            await operation.UpdateStatusAsync();
+
+            testListener.AssertScope($"{nameof(RecognizeBusinessCardsOperation)}.{nameof(RecognizeBusinessCardsOperation.UpdateStatus)}");
+        }
+
+        [Test]
+        public void RecognizeBusinessCardsOperationRequiredParameters()
+        {
+            FormRecognizerClient client = CreateFormRecognizerClient();
+
+            Assert.Throws<ArgumentNullException>(() => new RecognizeBusinessCardsOperation(null, client));
+            Assert.Throws<ArgumentException>(() => new RecognizeBusinessCardsOperation(string.Empty, client));
+            Assert.Throws<ArgumentNullException>(() => new RecognizeBusinessCardsOperation("00000000 - 0000 - 0000 - 0000 - 000000000000", null));
+        }
+
+        [Test]
+        public async Task RecognizeInvoicesOperationCreatesDiagnosticScopeOnUpdate()
+        {
+            using var testListener = new ClientDiagnosticListener(DiagnosticNamespace);
+            using var stream = new MemoryStream(Encoding.UTF8.GetBytes("{}"));
+
+            var mockResponse = new MockResponse(200);
+            mockResponse.ContentStream = stream;
+
+            var mockTransport = new MockTransport(new[] { mockResponse, mockResponse });
+            var options = new FormRecognizerClientOptions() { Transport = mockTransport };
+            var client = CreateFormRecognizerClient(options);
+
+            var operation = new RecognizeInvoicesOperation("00000000-0000-0000-0000-000000000000", client);
+
+            await operation.UpdateStatusAsync();
+
+            testListener.AssertScope($"{nameof(RecognizeInvoicesOperation)}.{nameof(RecognizeInvoicesOperation.UpdateStatus)}");
+        }
+
+        [Test]
+        public void RecognizeInvoicesOperationRequiredParameters()
+        {
+            FormRecognizerClient client = CreateFormRecognizerClient();
+
+            Assert.Throws<ArgumentNullException>(() => new RecognizeInvoicesOperation(null, client));
+            Assert.Throws<ArgumentException>(() => new RecognizeInvoicesOperation(string.Empty, client));
+            Assert.Throws<ArgumentNullException>(() => new RecognizeInvoicesOperation("00000000 - 0000 - 0000 - 0000 - 000000000000", null));
+        }
+
         [Test]
         public async Task RecognizeCustomFormsOperationCreatesDiagnosticScopeOnUpdate()
         {
@@ -117,6 +193,16 @@ public async Task RecognizeCustomFormsOperationCreatesDiagnosticScopeOnUpdate()
             testListener.AssertScope($"{nameof(RecognizeCustomFormsOperation)}.{nameof(RecognizeCustomFormsOperation.UpdateStatus)}");
         }
 
+        [Test]
+        public void RecognizeCustomFormsOperationRequiredParameters()
+        {
+            FormRecognizerClient client = CreateFormRecognizerClient();
+
+            Assert.Throws<ArgumentNullException>(() => new RecognizeCustomFormsOperation(null, client));
+            Assert.Throws<ArgumentException>(() => new RecognizeCustomFormsOperation(string.Empty, client));
+            Assert.Throws<ArgumentNullException>(() => new RecognizeCustomFormsOperation("00000000 - 0000 - 0000 - 0000 - 000000000000", null));
+        }
+
         [Test]
         public async Task TrainingOperationCreatesDiagnosticScopeOnUpdate()
         {
@@ -143,6 +229,48 @@ public async Task TrainingOperationCreatesDiagnosticScopeOnUpdate()
             testListener.AssertScope($"{nameof(CreateCustomFormModelOperation)}.{nameof(CreateCustomFormModelOperation.UpdateStatus)}");
         }
 
+        [Test]
+        public void TrainingOperationRequiredParameters()
+        {
+            FormTrainingClient client = CreateFormTrainingClient();
+
+            Assert.Throws<ArgumentNullException>(() => new TrainingOperation("00000000 - 0000 - 0000 - 0000 - 000000000000", null));
+        }
+
+        [Test]
+        public async Task CreateComposedModelOperationCreatesDiagnosticScopeOnUpdate()
+        {
+            using var testListener = new ClientDiagnosticListener(DiagnosticNamespace);
+            using var stream = new MemoryStream(Encoding.UTF8.GetBytes(@"
+                {
+                    ""modelInfo"": {
+                        ""status"": ""creating"",
+                        ""modelId"": ""00000000-0000-0000-0000-000000000000""
+                    }
+                }"));
+
+            var mockResponse = new MockResponse(200);
+            mockResponse.ContentStream = stream;
+
+            var mockTransport = new MockTransport(new[] { mockResponse, mockResponse });
+            var options = new FormRecognizerClientOptions() { Transport = mockTransport };
+            var client = CreateFormTrainingClient(options);
+
+            var operation = new CreateComposedModelOperation("00000000-0000-0000-0000-000000000000", client);
+
+            await operation.UpdateStatusAsync();
+
+            testListener.AssertScope($"{nameof(CreateCustomFormModelOperation)}.{nameof(CreateCustomFormModelOperation.UpdateStatus)}");
+        }
+
+        [Test]
+        public void CreateComposedModelOperationRequiredParameters()
+        {
+            FormTrainingClient client = CreateFormTrainingClient();
+
+            Assert.Throws<ArgumentNullException>(() => new CreateComposedModelOperation("00000000 - 0000 - 0000 - 0000 - 000000000000", null));
+        }
+
         [Test]
         public async Task CopyModelOperationCreatesDiagnosticScopeOnUpdate()
         {
@@ -162,5 +290,17 @@ public async Task CopyModelOperationCreatesDiagnosticScopeOnUpdate()
 
             testListener.AssertScope($"{nameof(CopyModelOperation)}.{nameof(CopyModelOperation.UpdateStatus)}");
         }
+
+        [Test]
+        public void CopyModelOperationRequiredParameters()
+        {
+            FormTrainingClient client = CreateFormTrainingClient();
+            string operationId = "00000000-0000-0000-0000-000000000000/copyresults/00000000-0000-0000-0000-000000000000";
+            string targetId = "00000000-0000-0000-0000-000000000000";
+
+            Assert.Throws<ArgumentNullException>(() => new CopyModelOperation(null, targetId, client));
+            Assert.Throws<ArgumentException>(() => new CopyModelOperation(string.Empty, targetId, client));
+            Assert.Throws<ArgumentNullException>(() => new CopyModelOperation(operationId, targetId, null));
+        }
     }
 }