Disable Debug Assertion for Parsing NPM Person objects

Nodejs/Product/Npm/NodeModuleBuilder.cs

@@ -71,7 +71,7 @@ public void AddAuthor(string text) {
         public IPerson Author {
             get {
                 var text = _authorBuff.ToString().Trim();
-                return string.IsNullOrEmpty(text) ? null : new Person(text);
+                return string.IsNullOrEmpty(text) ? null : Person.CreateFromJsonSource(text);
             }
         }
 

Nodejs/Product/Npm/SPI/PackageJson.cs

@@ -194,7 +194,7 @@ public IScripts Scripts {
         public IPerson Author {
             get {
                 var author = _package.author;
-                return null == author ? null : new Person(author.ToString());
+                return null == author ? null : Person.CreateFromJsonSource(author.ToString());
             }
         }
 

Nodejs/Product/Npm/SPI/Person.cs

@@ -20,77 +20,82 @@
 using System.Text.RegularExpressions;
 using Newtonsoft.Json;
 
-#if DEBUG
-using Newtonsoft.Json.Linq;
-#endif
-
 namespace Microsoft.NodejsTools.Npm.SPI {
     internal class Person : IPerson {
 
         // We cannot rely on the ordering of any of these fields,
         // so we should match them separately.
-        private static readonly Regex RegexPerson = new Regex(
-        "\"name\":\\s*\"(?<name>[^<]+?)\"" +
-        "|" +
-        "\"email\":\\s*\"(?<email>[^<]+?)\"" +
-        "|" +
-        "\"url\":\\s*\"(?<url>[^<]+?)\"",
-        RegexOptions.Singleline);
-    
+        private static readonly Regex ObjectPersonRegex = new Regex(
+            "\"name\":\\s*\"(?<name>[^<]+?)\"" +
+            "|" +
+            "\"email\":\\s*\"(?<email>[^<]+?)\"" +
+            "|" +
+            "\"url\":\\s*\"(?<url>[^<]+?)\"",
+            RegexOptions.Singleline);
+
         [JsonConstructor]
         private Person() {
             // Enables Json deserialization
         }
 
-        public Person(string source) {
-            InitFromString(source);
+        private Person(string name, string email = null, string url = null) {
+            Name = name;
+            Email = email;
+            Url = url;
         }
 
-        private void InitFromString(string source) {
-            if (source == null) {
-                Name = string.Empty;
-                return;
-            }
+        public static Person CreateFromJsonSource(string source) {
+            if (source == null)
+                return new Person(string.Empty);
+            return TryCreatePersonFromObject(source) ?? CreatePersonFromString(source);
+        }
+
+        /// <summary>
+        /// Try to create a person object from a json object.
+        /// 
+        /// This can either be a json object or a string: https://docs.npmjs.com/files/package.json#people-fields-author-contributors
+        /// </summary>
+        /// <param name="source">Json source</param>
+        private static Person TryCreatePersonFromObject(string source) {
+            string name = null;
+            string email = null;
+            string url = null;
 
             // We parse using a regex because JObject.Parse throws exceptions for malformatted json,
             // and simply handling them causes performance issues.
-            var matches = RegexPerson.Matches(source);
+            var matches = ObjectPersonRegex.Matches(source);
             if (matches.Count >= 1) {
                 foreach (Match match in matches) {
                     var group = match.Groups["name"];
                     if (group.Success) {
-                        Name = group.Value;
+                        name = group.Value;
                         continue;
                     }
 
                     group = match.Groups["email"];
                     if (group.Success) {
-                        Email = group.Value;
+                        email = group.Value;
                         continue;
                     }
 
                     group = match.Groups["url"];
                     if (group.Success) {
-                        Url = group.Value;
+                        url = group.Value;
                         continue;
                     }
                 }
-            } else {
-                Name = source;
             }
+            return name == null ? null : new Person(name, email, url);
+        }
 
-#if DEBUG
-            // Verify we are parsing correctly
-            try {
-                var jObject = JObject.Parse(source);
-                var name = (string)jObject["name"];
-                Debug.Assert(name != null ? name == Name : Name == source, string.Format("Failed to parse name from {0}", source));
-                Debug.Assert((string)jObject["email"] == Email, string.Format("Failed to parse email from {0}", source));
-                Debug.Assert((string)jObject["url"] == Url, string.Format("Failed to parse url from {0}", source));
-            } catch (Exception) {
-                Debug.Assert(source == Name);
-            }
-#endif
+        /// <summary>
+        /// Try to create a person object from a string.
+        /// 
+        /// TODO: currently does not try to parse the string to extract the email or url.
+        /// </summary>
+        /// <param name="source">Json source</param>
+        private static Person CreatePersonFromString(string source) {
+            return new Person(source);
         }
 
         [JsonProperty]
@@ -128,4 +133,4 @@ public override string ToString() {
             return buff.ToString();
         }
     }
-}
+}

Nodejs/Tests/NpmTests/NpmTests.csproj

@@ -1,4 +1,4 @@
-<?xml version="1.0" encoding="utf-8"?>
+<?xml version="1.0" encoding="utf-8"?>
 <Project ToolsVersion="4.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
   <Choose>
     <When Condition=" '$(VisualStudioVersion)'=='14.0'  Or '$(TargetVisualStudioVersion)'=='VS140' ">
@@ -94,6 +94,7 @@
     <Compile Include="NpmSearchTests.cs" />
     <Compile Include="PackageJsonDependencyTests.cs" />
     <Compile Include="PackageJsonTests.cs" />
+    <Compile Include="PersonTests.cs" />
     <Compile Include="ProblematicPackageJsonTests.cs" />
     <Compile Include="Properties\AssemblyInfo.cs" />
     <Compile Include="SemverVersionTestHelper.cs" />

Nodejs/Tests/NpmTests/PersonTests.cs

@@ -0,0 +1,86 @@
+//    Copyright (c) Microsoft. All rights reserved.
+//    
+//    Apache 2.0 License
+//    
+//    You may obtain a copy of the License at
+//    http://www.apache.org/licenses/LICENSE-2.0
+//    
+//    Unless required by applicable law or agreed to in writing, software 
+//    distributed under the License is distributed on an "AS IS" BASIS, 
+//    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or 
+//    implied. See the License for the specific language governing 
+//    permissions and limitations under the License.
+//
+//*********************************************************//
+
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+using Microsoft.NodejsTools.Npm.SPI;
+
+namespace NpmTests {
+    [TestClass]
+    public class PersonTests {
+        [TestMethod, TestCategory("NpmIntegration")]
+        public void ShouldReturnEmptyPersonForNullOrEmptyInput() {
+            var sources = new[] { null, "", " ", "    " };
+            foreach (var emptySource in sources) {
+                var person = Person.CreateFromJsonSource(null);
+                Assert.IsNotNull(person);
+                Assert.AreEqual("", person.Name);
+                Assert.IsNull(person.Email);
+                Assert.IsNull(person.Url);
+            }
+        }
+
+        [TestMethod, TestCategory("NpmIntegration")]
+        public void ShouldReturnNameForObjectWithNameOnly() {
+            var name = "J Scripter";
+            var sources = new[] {
+                @"{{""name"": ""{0}""}}",
+                @"{{""name"":""{0}""}}",
+                @"{{""name"":    ""{0}""}}",
+                @"{{      ""name"":     ""{0}""      }}",
+            };
+            foreach (var source in sources) {
+                var json = string.Format(source, name);
+                var person = Person.CreateFromJsonSource(json);
+                Assert.IsNotNull(person);
+                Assert.AreEqual(name, person.Name);
+                Assert.IsNull(person.Email);
+                Assert.IsNull(person.Url);
+            }
+        }
+
+        [TestMethod, TestCategory("NpmIntegration")]
+        public void ShouldSetNameEmailAndUrlForObjectWithTheseProperties() {
+            var name = "J Scripter";
+            var email = "j@contoso.com";
+            var url = "http://contoso.com";
+
+            var sources = new[] {
+                @"{{""name"": ""{0}"", ""email"": ""{1}"", ""url"": ""{2}""}}",
+                @"{{""url"": ""{2}"", ""email"": ""{1}"", ""name"": ""{0}""}}",
+                // Ignore other properties
+                @"{{""handle"": ""@code"", ""url"": ""{2}"", ""email"": ""{1}"", ""office"": ""1337"", ""name"": ""{0}""}}",
+            };
+
+            foreach (var source in sources) {
+                var json = string.Format(source, name, email, url);
+                var person = Person.CreateFromJsonSource(json);
+                Assert.IsNotNull(person);
+                Assert.AreEqual(name, person.Name);
+                Assert.AreEqual(email, person.Email);
+                Assert.AreEqual(url, person.Url);
+            }
+        }
+
+        [TestMethod, TestCategory("NpmIntegration")]
+        public void ShouldReturnInputAsNameIfInputIsObject() {
+            var name = "J Scripter";
+            var person = Person.CreateFromJsonSource(name);
+            Assert.IsNotNull(person);
+            Assert.AreEqual(name, person.Name);
+            Assert.IsNull(person.Email);
+            Assert.IsNull(person.Url);
+        }
+    }
+}