Improve error message in wizard when installing an invalid library name (#651)

Addresses #508

src/LibraryManager.Vsix/Resources/Text.Designer.cs

@@ -69,6 +69,15 @@ namespace Microsoft.Web.LibraryManager.Vsix.Resources {
             }
         }
         
+        /// <summary>
+        ///   Looks up a localized string similar to The library {0} did not match the naming scheme for provider {1}. {2}.
+        /// </summary>
+        public static string BadLibraryId {
+            get {
+                return ResourceManager.GetString("BadLibraryId", resourceCulture);
+            }
+        }
+        
         /// <summary>
         ///   Looks up a localized string similar to _Cancel.
         /// </summary>

src/LibraryManager.Vsix/Resources/Text.resx

@@ -1,17 +1,17 @@
 <?xml version="1.0" encoding="utf-8"?>
 <root>
-  <!-- 
-    Microsoft ResX Schema 
-    
+  <!--
+    Microsoft ResX Schema
+
     Version 2.0
-    
-    The primary goals of this format is to allow a simple XML format 
-    that is mostly human readable. The generation and parsing of the 
-    various data types are done through the TypeConverter classes 
+
+    The primary goals of this format is to allow a simple XML format
+    that is mostly human readable. The generation and parsing of the
+    various data types are done through the TypeConverter classes
     associated with the data types.
-    
+
     Example:
-    
+
     ... ado.net/XML headers & schema ...
     <resheader name="resmimetype">text/microsoft-resx</resheader>
     <resheader name="version">2.0</resheader>
@@ -26,36 +26,36 @@
         <value>[base64 mime encoded string representing a byte array form of the .NET Framework object]</value>
         <comment>This is a comment</comment>
     </data>
-                
-    There are any number of "resheader" rows that contain simple 
+
+    There are any number of "resheader" rows that contain simple
     name/value pairs.
-    
-    Each data row contains a name, and value. The row also contains a 
-    type or mimetype. Type corresponds to a .NET class that support 
-    text/value conversion through the TypeConverter architecture. 
-    Classes that don't support this are serialized and stored with the 
+
+    Each data row contains a name, and value. The row also contains a
+    type or mimetype. Type corresponds to a .NET class that support
+    text/value conversion through the TypeConverter architecture.
+    Classes that don't support this are serialized and stored with the
     mimetype set.
-    
-    The mimetype is used for serialized objects, and tells the 
-    ResXResourceReader how to depersist the object. This is currently not 
+
+    The mimetype is used for serialized objects, and tells the
+    ResXResourceReader how to depersist the object. This is currently not
     extensible. For a given mimetype the value must be set accordingly:
-    
-    Note - application/x-microsoft.net.object.binary.base64 is the format 
-    that the ResXResourceWriter will generate, however the reader can 
+
+    Note - application/x-microsoft.net.object.binary.base64 is the format
+    that the ResXResourceWriter will generate, however the reader can
     read any of the formats listed below.
-    
+
     mimetype: application/x-microsoft.net.object.binary.base64
-    value   : The object must be serialized with 
+    value   : The object must be serialized with
             : System.Runtime.Serialization.Formatters.Binary.BinaryFormatter
             : and then encoded with base64 encoding.
-    
+
     mimetype: application/x-microsoft.net.object.soap.base64
-    value   : The object must be serialized with 
+    value   : The object must be serialized with
             : System.Runtime.Serialization.Formatters.Soap.SoapFormatter
             : and then encoded with base64 encoding.
 
     mimetype: application/x-microsoft.net.object.bytearray.base64
-    value   : The object must be serialized into a byte array 
+    value   : The object must be serialized into a byte array
             : using a System.ComponentModel.TypeConverter
             : and then encoded with base64 encoding.
     -->
@@ -271,4 +271,8 @@ Do you want to continue?</value>
   <data name="NoMatchesFound" xml:space="preserve">
     <value>No matches found</value>
   </data>
-</root>
+  <data name="BadLibraryId" xml:space="preserve">
+    <value>The library {0} did not match the naming scheme for provider {1}. {2}</value>
+    <comment>Used in an error message popup.  {2} is the provider's example text (already localized elsewhere).</comment>
+  </data>
+</root>

src/LibraryManager.Vsix/UI/InstallDialog.xaml.cs

@@ -53,7 +53,7 @@ namespace Microsoft.Web.LibraryManager.Vsix.UI
             {
                 DialogResult = res;
             }
-            catch(InvalidOperationException)
+            catch (InvalidOperationException)
             { }
 
             Close();

src/LibraryManager.Vsix/UI/Models/InstallDialogViewModel.cs

@@ -403,6 +403,12 @@ namespace Microsoft.Web.LibraryManager.Vsix.UI.Models
 
         public async Task<bool> IsLibraryInstallationStateValidAsync()
         {
+            if (!LibraryIdToNameAndVersionConverter.Instance.IsWellFormedLibraryId(LibraryId, SelectedProvider.Id))
+            {
+                ErrorMessage = string.Format(Text.BadLibraryId, LibraryId, SelectedProvider.Id, SelectedProvider.LibraryIdHintText);
+                return false;
+            }
+
             (string name, string version) = LibraryIdToNameAndVersionConverter.Instance.GetLibraryNameAndVersion(LibraryId, SelectedProvider.Id);
             LibraryInstallationState libraryInstallationState = new LibraryInstallationState
             {

src/LibraryManager/LibraryNaming/ILibraryNamingScheme.cs

@@ -8,6 +8,14 @@ namespace Microsoft.Web.LibraryManager.LibraryNaming
     /// </summary>
     internal interface ILibraryNamingScheme
     {
+        /// <summary>
+        /// Returns whether the given library identifier matches the naming scheme
+        /// </summary>
+        /// <param name="libraryId">The library ID to validate.</param>
+        /// <returns>Returns true if the library ID matches the naming scheme; false otherwise.</returns>
+        /// <remarks>This does not indicate that the library ID is valid, but only that it is well-formed.</remarks>
+        bool IsValidLibraryId(string libraryId);
+
         /// <summary>
         /// Splits libraryId into name and version.
         /// </summary>

src/LibraryManager/LibraryNaming/LibraryIdToNameAndVersionConverter.cs

@@ -110,6 +110,14 @@ namespace Microsoft.Web.LibraryManager.LibraryNaming
             return GetSchemeForProvider(providerId).GetLibraryId(name, version);
         }
 
+        /// <summary>
+        /// Returns whether the given library ID is of a valid form for the given provider
+        /// </summary>
+        public bool IsWellFormedLibraryId(string libraryId, string providerId)
+        {
+            return GetSchemeForProvider(providerId).IsValidLibraryId(libraryId);
+        }
+
         private ILibraryNamingScheme GetSchemeForProvider(string providerId)
         {
             lock (_syncObject)

src/LibraryManager/LibraryNaming/SimpleLibraryNamingScheme.cs

@@ -19,5 +19,11 @@ namespace Microsoft.Web.LibraryManager.LibraryNaming
         {
             return (libraryId ?? string.Empty, string.Empty);
         }
+
+        /// <inheritdoc />
+        public bool IsValidLibraryId(string libraryId)
+        {
+            return !string.IsNullOrEmpty(libraryId);
+        }
     }
 }

src/LibraryManager/LibraryNaming/VersionedLibraryNamingScheme.cs

@@ -66,5 +66,18 @@ namespace Microsoft.Web.LibraryManager.LibraryNaming
                 : $"{name}{Separator}{version}";
 
         }
+
+        /// <inheritdoc />
+        public bool IsValidLibraryId(string libraryId)
+        {
+            if (string.IsNullOrEmpty(libraryId))
+            {
+                return false;
+            }
+
+            int separator = libraryId.LastIndexOf(Separator);
+
+            return separator > 1 && separator < libraryId.Length;
+        }
     }
 }

test/LibraryManager.Test/SimpleLibraryNamingSchemeTest.cs

@@ -42,5 +42,20 @@ namespace Microsoft.Web.LibraryManager.Test
 
             Assert.AreEqual(expectedLibraryId, libraryId);
         }
+
+        [TestMethod]
+        [DataRow(null, false)]
+        [DataRow("", false)]
+        [DataRow("foobarbaz", true)]
+        [DataRow(":@#/\\|", true)]
+        [DataRow(" \t\r\n", true)]
+        public void IsValidLibraryId(string libraryId, bool expected)
+        {
+            var namingScheme = new SimpleLibraryNamingScheme();
+
+            bool result = namingScheme.IsValidLibraryId(libraryId);
+
+            Assert.AreEqual(expected, result);
+        }
     }
 }

test/LibraryManager.Test/VersionedLibraryNamingSchemeTest.cs

@@ -47,5 +47,25 @@ namespace Microsoft.Web.LibraryManager.Test
 
             Assert.AreEqual(expectedLibraryId, libraryId);
         }
+
+        [TestMethod]
+        [DataRow(null, false)]
+        [DataRow("", false)]
+        [DataRow("@", false)]
+        [DataRow("test", false)]
+        [DataRow("test@1", true)]
+        [DataRow("@test1", false)]
+        [DataRow("@test/1.0", false)]
+        [DataRow("@test1@1.0", true)]
+        [DataRow("test@input", true)]
+        [DataRow("@@@1.0", true)]     // kind of an odd case, but this translates to "@@" as the name
+        public void IsValidLibraryId(string libraryId, bool expected)
+        {
+            var namingScheme = new VersionedLibraryNamingScheme();
+
+            bool result = namingScheme.IsValidLibraryId(libraryId);
+
+            Assert.AreEqual(expected, result);
+        }
     }
 }