--force message updates. handle 422 code from log post endpoint. (#2656)

* Update text to reflect --force obsoletion. * Update code comment. * Update release notes.: * Update release notes with PR details. * Remove erroneous log statement. * Remove erroneous console write.
microsoft · Apr 14, 2023 · e5f6770 · e5f6770
1 parent 36b4792
commit e5f6770
Show file tree

Hide file tree

Showing 8 changed files with 32 additions and 24 deletions.
diff --git a/ReleaseHistory.md b/ReleaseHistory.md
@@ -9,8 +9,10 @@
 * BRK: `fileRegionsCache` parameter is now required for the `InsertOptionalDataVisitor`. [#2642](https://github.com/microsoft/sarif-sdk/pull/2642)
 * BRK: Add `IAnalysisLogger.TargetAnalysisComplete` method. [#2637](https://github.com/microsoft/sarif-sdk/pull/2637)
 * BRK: Remove unused `quiet` parameter from `SarifLogger`. [#2639]https://github.com/microsoft/sarif-sdk/pull/2639
-* BRK: Remove `CompuateHashData` and `AnalysisTargetToHashDataMap` properties from `SarifLogger` (in preference of new `fileRegionsCache` parameter. [#2639](https://github.com/microsoft/sarif-sdk/pull/2639)
+* BRK: Remove `ComputeHashData` and `AnalysisTargetToHashDataMap` properties from `SarifLogger` (in preference of new `fileRegionsCache` parameter. [#2639](https://github.com/microsoft/sarif-sdk/pull/2639)
 * BRK: Eliminate proactive hashing of artifacts in `SarifLogger` constructor when `OptionallyEmittedData.Hashes` is specified. [#2639](https://github.com/microsoft/sarif-sdk/pull/2639)
+* BUG: Update user messages and code comments that refer to `--force` (replaced by `--log ForceOverwrite`). [#2656](https://github.com/microsoft/sarif-sdk/pull/2656)
+* BUG: Handle return code 422 `UnprocessableEntity` when validating that log file POST endpoint is available. [#2656](https://github.com/microsoft/sarif-sdk/pull/2656)
 * BUG: Eliminate erroneous `Posted log file successfully` message when context `PostUri` is non-null but empty. [#2655](https://github.com/microsoft/sarif-sdk/pull/2655)
 * BUG: Resolves `IOException` raised by calling `FileSystem.ReadAllText` on file locked for write (but not read). [#2655](https://github.com/microsoft/sarif-sdk/pull/2655)
 * BUG: Correct `toolComponent.language` regex in JSON schema. [#2653]https://github.com/microsoft/sarif-sdk/pull/2653

diff --git a/src/Sarif.Driver/DriverExtensionMethods.cs b/src/Sarif.Driver/DriverExtensionMethods.cs
@@ -130,14 +130,15 @@ private static void ReportInvalidOutputFormatOptions()
         /// absent, because by default each transformed file is written to the path containing
         /// corresponding input file.
         ///
-        /// However, similarly to the case of SingleFileOptionsBase, we _do_ want to set --force
-        /// whenever --inline is true, because there's no reason to force the user to type
-        /// "--force" when they've already said that they want to overwrite the input file
+        /// However, similarly to the case of SingleFileOptionsBase, we _do_ want to set 
+        /// --log ForceOverwrite whenever --log Inline is true, because there's no reason
+        /// to force the user to type "--log ForceOverwrite;Inline" when they've said
+        /// that they want to overwrite the input file
         /// (see https://github.com/microsoft/sarif-sdk/issues/1642).
         ///
         /// So we introduce this method for three reasons:
         /// 1) For symmetry with the SingleFileOptionsBase,
-        /// 2) To DRY out the logic for making --inline and --force consistent, and
+        /// 2) To DRY out the logic for making --log Inline and --log ForceOverwrite consistent, and
         /// 3) To leave an obvious place to put output file option consistency logic if it's
         ///    needed in future.
         /// </remarks>

diff --git a/src/Sarif.Driver/DriverUtilities.cs b/src/Sarif.Driver/DriverUtilities.cs
@@ -21,7 +21,7 @@ public static class DriverUtilities
         /// A list of the paths to the output files.
         /// </param>
         /// <param name="force">
-        /// true if the --force option was specified.
+        /// true if the --log ForceOverwrite option was specified.
         /// </param>
         /// <param name="fileSystem">
         /// An object that provides access to the file system.
@@ -49,7 +49,7 @@ public static bool ReportWhetherOutputFilesCanBeCreated(IEnumerable<string> outp
         /// The path to the output file.
         /// </param>
         /// <param name="force">
-        /// true if the --force option was specified.
+        /// true if the --log ForceOverwrite option was specified.
         /// </param>
         /// <param name="fileSystem">
         /// An object that provides access to the file system.
@@ -79,7 +79,7 @@ public static bool ReportWhetherOutputFileCanBeCreated(string outputFilePath, bo
         /// The path to the output file.
         /// </param>
         /// <param name="force">
-        /// true if the --force option was specified.
+        /// true if the --log ForceOverwrite option was specified.
         /// </param>
         /// <param name="fileSystem">
         /// An object that provides access to the file system.

diff --git a/src/Sarif.Driver/Sdk/MultithreadedAnalyzeCommandBase.cs b/src/Sarif.Driver/Sdk/MultithreadedAnalyzeCommandBase.cs
@@ -8,6 +8,7 @@
 using System.Diagnostics;
 using System.IO;
 using System.Linq;
+using System.Net;
 using System.Net.Http;
 using System.Runtime.InteropServices;
 using System.Threading.Channels;
@@ -286,6 +287,7 @@ public virtual TContext ValidateContext(TContext globalContext)
                                        globalContext.PluginFilePaths,
                                        shouldExist: required ? true : (bool?)null);
 
+
             if (!string.IsNullOrEmpty(globalContext.PostUri))
             {
                 try
@@ -294,9 +296,11 @@ public virtual TContext ValidateContext(TContext globalContext)
                     var content = new StringContent(string.Empty);
                     HttpResponseMessage httpResponseMessage = httpClient.PostAsync(globalContext.PostUri, content).GetAwaiter().GetResult();
 
-                    // Internal server error means we found our server but it didn't like our malformed payload.
-                    // That means we're all good! i.e., if we provide a good SARIF file we should succeed.
-                    if (httpResponseMessage.StatusCode != System.Net.HttpStatusCode.InternalServerError)
+                    // Internal server error means we found our server but it didn't like our malformed payload
+                    // (in first implementation). In a server update, this condition returns 422 (unprocessable
+                    // payload). We treat either return value as good, i.e. posting a valid SARIF file should work.
+                    if (httpResponseMessage.StatusCode != HttpStatusCode.InternalServerError &&
+                        httpResponseMessage.StatusCode != (HttpStatusCode)422)
                     {
                         globalContext.PostUri = null;
                         succeeded = false;
@@ -305,6 +309,7 @@ public virtual TContext ValidateContext(TContext globalContext)
                 catch (Exception e)
                 {
                     globalContext.PostUri = null;
+                    succeeded = false;
                     globalContext.RuntimeErrors |= RuntimeConditions.ExceptionPostingLogFile;
                     globalContext.RuntimeExceptions ??= new List<Exception>();
                     globalContext.RuntimeExceptions.Add(e);

diff --git a/src/Sarif/Errors.cs b/src/Sarif/Errors.cs
@@ -381,7 +381,7 @@ public static void LogFileAlreadyExists(IAnalysisContext context, string filePat
 
             context.RuntimeErrors |= RuntimeConditions.FileAlreadyExists;
 
-            // The output file '{0}' already exists. Use --force to overwrite.
+            // The output file '{0}' already exists. Use --log ForceOverwrite to overwrite.
             context.Logger.LogConfigurationNotification(
                 CreateNotification(
                     uri: null,

diff --git a/src/Sarif/SdkResources.Designer.cs b/src/Sarif/SdkResources.Designer.cs
diff --git a/src/Sarif/SdkResources.resx b/src/Sarif/SdkResources.resx
@@ -244,7 +244,7 @@
     <value>All rules were explicitly disabled so there is no work to do.</value>
   </data>
   <data name="ERR997_FileAlreadyExists" xml:space="preserve">
-    <value>The output file '{0}' already exists. Use --force to overwrite.</value>
+    <value>The output file '{0}' already exists. Use --log ForceOverwrite to overwrite.</value>
   </data>
   <data name="PartioningVisitHappensAtSarifLogLevel" xml:space="preserve">
     <value>Could not identify the log being partitioned. Call VisitSarifLog and provide the log to partition. This class is designed to create log files on a per-run basis (i.e., all partioned logs will contain a single run only).</value>

diff --git a/src/Test.UnitTests.Sarif.Driver/DriverExtensionMethodsTests.cs b/src/Test.UnitTests.Sarif.Driver/DriverExtensionMethodsTests.cs
@@ -33,7 +33,7 @@ private class ValidateSingleFileOutputOptionsTestCase
             {
                 new ValidateSingleFileOutputOptionsTestCase
                 {
-                    Title = "--inline and not --force",
+                    Title = "--log Inline and not --log ForceOverwrite",
                     Options = new SingleFileOptionsBase
                     {
                         OutputFilePath = null,
@@ -46,7 +46,7 @@ private class ValidateSingleFileOutputOptionsTestCase
 
                 new ValidateSingleFileOutputOptionsTestCase
                 {
-                    Title = "--inline and superfluous --force",
+                    Title = "--log Inline and superfluous --log ForceOverwrite",
                     Options = new SingleFileOptionsBase
                     {
                         OutputFileOptions = new[] { FilePersistenceOptions.ForceOverwrite, FilePersistenceOptions.Inline },
@@ -58,7 +58,7 @@ private class ValidateSingleFileOutputOptionsTestCase
 
                 new ValidateSingleFileOutputOptionsTestCase
                 {
-                    Title = "Output path with --force",
+                    Title = "Output path with --log ForceOverwrite",
                     Options = new SingleFileOptionsBase
                     {
                         OutputFileOptions = new[] { FilePersistenceOptions.ForceOverwrite },
@@ -70,7 +70,7 @@ private class ValidateSingleFileOutputOptionsTestCase
 
                 new ValidateSingleFileOutputOptionsTestCase
                 {
-                    Title = "Output path without --force",
+                    Title = "Output path without --log ForceOverwrite",
                     Options = new SingleFileOptionsBase
                     {
                         OutputFileOptions = new[] { FilePersistenceOptions.None },
@@ -82,7 +82,7 @@ private class ValidateSingleFileOutputOptionsTestCase
 
                 new ValidateSingleFileOutputOptionsTestCase
                 {
-                    Title = "Neither --inline nor output path",
+                    Title = "Neither --log Inline nor output path",
                     Options = new SingleFileOptionsBase
                     {
 
@@ -94,7 +94,7 @@ private class ValidateSingleFileOutputOptionsTestCase
 
                 new ValidateSingleFileOutputOptionsTestCase
                 {
-                    Title = "Both --inline and output path",
+                    Title = "Both --log Inline and output path",
                     Options = new SingleFileOptionsBase
                     {
                         OutputFileOptions = new[] { FilePersistenceOptions.Inline },
@@ -141,7 +141,7 @@ private class ValidateMultipleFilesOutputOptionsTestCase
             {
                 new ValidateMultipleFilesOutputOptionsTestCase
                 {
-                    Title = "--force and not --inline",
+                    Title = "--log ForceOverwrite and not --log Inline",
                     Options = new MultipleFilesOptionsBase
                     {
                         OutputFileOptions = new[] { FilePersistenceOptions.ForceOverwrite },
@@ -152,7 +152,7 @@ private class ValidateMultipleFilesOutputOptionsTestCase
 
                 new ValidateMultipleFilesOutputOptionsTestCase
                 {
-                    Title = "--inline and not --force",
+                    Title = "--log Inline and not --log ForceOverwrite",
                     Options = new MultipleFilesOptionsBase
                     {
                         OutputFileOptions = new[] { FilePersistenceOptions.Inline },
@@ -163,7 +163,7 @@ private class ValidateMultipleFilesOutputOptionsTestCase
 
                 new ValidateMultipleFilesOutputOptionsTestCase
                 {
-                    Title = "--inline and superfluous --force",
+                    Title = "--log Inline and superfluous --log ForceOverwrite",
                     Options = new MultipleFilesOptionsBase
                     {
                         OutputFileOptions = new[] { FilePersistenceOptions.ForceOverwrite, FilePersistenceOptions.Inline },