diff --git a/Nodejs/Product/Npm/NodeModuleBuilder.cs b/Nodejs/Product/Npm/NodeModuleBuilder.cs index c1c3e5de0..cd0118112 100644 --- a/Nodejs/Product/Npm/NodeModuleBuilder.cs +++ b/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); } } diff --git a/Nodejs/Product/Npm/SPI/PackageJson.cs b/Nodejs/Product/Npm/SPI/PackageJson.cs index 8f228c707..10f6f8082 100644 --- a/Nodejs/Product/Npm/SPI/PackageJson.cs +++ b/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()); } } diff --git a/Nodejs/Product/Npm/SPI/Person.cs b/Nodejs/Product/Npm/SPI/Person.cs index cdbe3f674..e68086879 100644 --- a/Nodejs/Product/Npm/SPI/Person.cs +++ b/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*\"(?[^<]+?)\"" + - "|" + - "\"email\":\\s*\"(?[^<]+?)\"" + - "|" + - "\"url\":\\s*\"(?[^<]+?)\"", - RegexOptions.Singleline); - + private static readonly Regex ObjectPersonRegex = new Regex( + "\"name\":\\s*\"(?[^<]+?)\"" + + "|" + + "\"email\":\\s*\"(?[^<]+?)\"" + + "|" + + "\"url\":\\s*\"(?[^<]+?)\"", + 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); + } + + /// + /// 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 + /// + /// Json source + 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 + /// + /// Try to create a person object from a string. + /// + /// TODO: currently does not try to parse the string to extract the email or url. + /// + /// Json source + private static Person CreatePersonFromString(string source) { + return new Person(source); } [JsonProperty] @@ -128,4 +133,4 @@ public override string ToString() { return buff.ToString(); } } -} \ No newline at end of file +} diff --git a/Nodejs/Tests/NpmTests/NpmTests.csproj b/Nodejs/Tests/NpmTests/NpmTests.csproj index a68fda04c..fefc3d791 100644 --- a/Nodejs/Tests/NpmTests/NpmTests.csproj +++ b/Nodejs/Tests/NpmTests/NpmTests.csproj @@ -1,4 +1,4 @@ - + @@ -94,6 +94,7 @@ + diff --git a/Nodejs/Tests/NpmTests/PersonTests.cs b/Nodejs/Tests/NpmTests/PersonTests.cs new file mode 100644 index 000000000..5103388f6 --- /dev/null +++ b/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); + } + } +}