Set the temp file extension based on the content type (#208)

* Set the temp file extension based on the content type

* Making RealFileSystem public so we can remove an IVT; adding another content type for json

* Modifying RealFileSystem's GetTempFileName implementation to always require an extension; pass in .tmp if we can't find a more appropriate one. Also moved a couple integration tests to be unit tests now that there was no file system dependency.

* Fixing a namespace and making a public class internal again

* Changing RealFileSystem to disallow period-only extensions. Adding tests for the failure conditions, renaming the success conditions

src/Microsoft.HttpRepl.Fakes/FileSystemStub.cs

@@ -18,7 +18,7 @@ public bool FileExists(string path)
             return default;
         }
 
-        public string GetTempFileName()
+        public string GetTempFileName(string fileExtension)
         {
             return default;
         }

src/Microsoft.HttpRepl.Fakes/MockedFileSystem.cs

@@ -74,7 +74,7 @@ public void WriteAllTextToFile(string path, string contents)
             _files[path] = contents;
         }
 
-        public string GetTempFileName()
+        public string GetTempFileName(string fileExtension)
         {
             string path = GetRandomFileName();
             _files[path] = "";

src/Microsoft.HttpRepl.Tests/FileSystem/RealFileSystemTests.cs

@@ -0,0 +1,78 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using System;
+using System.IO;
+using Microsoft.HttpRepl.FileSystem;
+using Xunit;
+
+namespace Microsoft.HttpRepl.Tests.FileSystem
+{
+    public class RealFileSystemTests
+    {
+        [Theory]
+        [InlineData(".json")]
+        [InlineData(".xml")]
+        [InlineData(".tmp")]
+        [InlineData(".a")]
+        public void GetTempFileName_WithValidInput_ReturnsFileNameWithExtension(string extension)
+        {
+            RealFileSystem realFileSystem = new RealFileSystem();
+
+            string fullName = realFileSystem.GetTempFileName(extension);
+
+            Assert.NotNull(fullName);
+            Assert.EndsWith(extension, fullName, StringComparison.OrdinalIgnoreCase);
+        }
+
+        [Theory]
+        [InlineData(".json")]
+        [InlineData(".xml")]
+        [InlineData(".tmp")]
+        [InlineData(".a")]
+        public void GetTempFileName_WithValidInput_ReturnsFileInTempPath(string extension)
+        {
+            RealFileSystem realFileSystem = new RealFileSystem();
+            string expectedPath = Path.GetTempPath();
+
+            string actualPath = realFileSystem.GetTempFileName(extension);
+
+            Assert.StartsWith(expectedPath, actualPath, StringComparison.OrdinalIgnoreCase);
+        }
+
+        [Theory]
+        [InlineData(".json")]
+        [InlineData(".xml")]
+        [InlineData(".tmp")]
+        [InlineData(".a")]
+        public void GetTempFileName_WithValidInput_ReturnsFileThatStartsWithHttpRepl(string extension)
+        {
+            RealFileSystem realFileSystem = new RealFileSystem();
+            string expectedStart = "HttpRepl.";
+
+            string fullName = realFileSystem.GetTempFileName(extension);
+            string actualFileName = Path.GetFileName(fullName);
+
+            Assert.StartsWith(expectedStart, actualFileName, StringComparison.OrdinalIgnoreCase);
+        }
+
+        [Fact]
+        public void GetTempFileName_WithNullExtension_ThrowsArgumentNullException()
+        {
+            RealFileSystem realFileSystem = new RealFileSystem();
+
+            Assert.Throws<ArgumentNullException>(() => realFileSystem.GetTempFileName(null));
+        }
+
+        [Theory]
+        [InlineData("")]
+        [InlineData(".")]
+        [InlineData(",a")]
+        public void GetTEmpFileName_WithInvalidInput_ThrowsArgumentException(string extension)
+        {
+            RealFileSystem realFileSystem = new RealFileSystem();
+
+            Assert.Throws<ArgumentException>(() => realFileSystem.GetTempFileName(extension));
+        }
+    }
+}

src/Microsoft.HttpRepl/Commands/BaseHttpCommand.cs

@@ -37,6 +37,13 @@ public abstract class BaseHttpCommand : CommandWithStructuredInputBase<HttpState
         private const string StreamingOption = nameof(StreamingOption);
         private const string BodyContentOption = nameof(BodyContentOption);
         private static readonly char[] HeaderSeparatorChars = new[] { '=', ':' };
+        private static readonly Dictionary<string, string> FileExtensionLookup = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase)
+        {
+            { "application/json", ".json" },
+            { "text/json", ".json" },
+            { "application/xml", ".xml" },
+            { "text/xml", ".xml" },
+        };
 
         private CommandInputSpecification _inputSpec;
 
@@ -199,7 +206,7 @@ private void HandleRequiresBody(DefaultCommandInput<ICoreParseResult> commandInp
                     }
 
                     deleteFile = true;
-                    filePath = _fileSystem.GetTempFileName();
+                    filePath = _fileSystem.GetTempFileName(GetFileExtensionFromContentType(contentType));
 
                     string exampleBody = GetExampleBody(commandInput.Arguments.Count > 0 ? commandInput.Arguments[0].Text : string.Empty, ref contentType, Verb, programState);
 
@@ -248,6 +255,15 @@ private void HandleRequiresBody(DefaultCommandInput<ICoreParseResult> commandInp
             AddHttpContentHeaders(content, programState, requestHeaders);
         }
 
+        private static string GetFileExtensionFromContentType(string contentType)
+        {
+            if (FileExtensionLookup.TryGetValue(contentType, out string extension))
+            {
+                return extension;
+            }
+            return ".tmp";
+        }
+
         private void AddHttpContentHeaders(HttpContent content, HttpState programState, Dictionary<string, string> requestHeaders)
         {
             foreach (KeyValuePair<string, IEnumerable<string>> header in programState.Headers)

src/Microsoft.HttpRepl/FileSystem/IFileSystem.cs

@@ -9,7 +9,7 @@ public interface IFileSystem
     {
         void DeleteFile(string path);
         bool FileExists(string path);
-        string GetTempFileName();
+        string GetTempFileName(string fileExtension);
         byte[] ReadAllBytesFromFile(string path);
         string[] ReadAllLinesFromFile(string path);
         void WriteAllTextToFile(string path, string contents);

src/Microsoft.HttpRepl/FileSystem/RealFileSystem.cs

@@ -1,8 +1,10 @@
 // Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
+using System;
 using System.Collections.Generic;
 using System.IO;
+using Microsoft.HttpRepl.Resources;
 
 namespace Microsoft.HttpRepl.FileSystem
 {
@@ -40,9 +42,28 @@ public void DeleteFile(string path)
             File.Delete(path);
         }
 
-        public string GetTempFileName()
+        public string GetTempFileName(string fileExtension)
         {
-            return Path.GetTempFileName();
+            if (fileExtension is null)
+            {
+                throw new ArgumentNullException(nameof(fileExtension));
+            }
+
+            if (!fileExtension.StartsWith(".", StringComparison.Ordinal) || fileExtension.Length < 2)
+            {
+                throw new ArgumentException(string.Format(Strings.RealFileSystem_Error_InvalidExtension, nameof(fileExtension)), nameof(fileExtension));
+            }
+
+            string tempFileName = Path.Combine(Path.GetTempPath(), GetRandomFileName(fileExtension));
+
+            return tempFileName;
+        }
+
+        private static string GetRandomFileName(string fileExtension)
+        {
+            // Start it with HttpRepl so we can easily find it if necessary for debugging, etc
+            // Use a GUID to make it unique enough
+            return "HttpRepl." + Guid.NewGuid().ToString() + fileExtension;
         }
 
         private void VerifyDirectoryExists(string path)

src/Microsoft.HttpRepl/Resources/Strings.resx

@@ -350,6 +350,10 @@ When +history option is specified, commands specified in the text file will be a
   <data name="SetBaseCommand_MustSpecifyServerError" xml:space="preserve">
     <value>Must specify a server</value>
   </data>
+  <data name="RealFileSystem_Error_InvalidExtension" xml:space="preserve">
+    <value>If specified, {0} must begin with a period and have at least one character after the period.</value>
+    <comment>{0} is the parameter name</comment>
+  </data>
   <data name="UICommand_InvalidParameter" xml:space="preserve">
     <value>The parameter '{0}' could not be converted into a valid uri.</value>
     <comment>{0} is a string parameter to the UI command</comment>