From bef3fbeb4ad38813679891ee4e8c90dceec245c8 Mon Sep 17 00:00:00 2001 From: Shaopeng <81775155+shaopeng-gh@users.noreply.github.com> Date: Wed, 8 Dec 2021 21:59:02 -0800 Subject: [PATCH 1/8] change TypedPropertiesDictionary to use ConcurrentDictionary --- src/Sarif/ExtensionMethods.cs | 11 ++++++ src/Sarif/PropertiesDictionary.cs | 6 --- src/Sarif/TypedPropertiesDictionary.cs | 9 +---- .../PropertiesDictionaryTests.cs | 39 +++++++++++++++++++ 4 files changed, 52 insertions(+), 13 deletions(-) diff --git a/src/Sarif/ExtensionMethods.cs b/src/Sarif/ExtensionMethods.cs index dc0400693..aff477fe5 100644 --- a/src/Sarif/ExtensionMethods.cs +++ b/src/Sarif/ExtensionMethods.cs @@ -3,6 +3,7 @@ using System; using System.Collections; +using System.Collections.Concurrent; using System.Collections.Generic; using System.Globalization; using System.IO; @@ -585,5 +586,15 @@ internal static void ConsumeElementOfDepth(this XmlReader xmlReader, int endElem xmlReader.Read(); } } + + public static bool Add(this ConcurrentDictionary concurrentDictionary, string key, T value) + { + return concurrentDictionary.TryAdd(key, value); + } + + public static bool Remove(this ConcurrentDictionary concurrentDictionary, string key) + { + return concurrentDictionary.TryRemove(key, out _); + } } } diff --git a/src/Sarif/PropertiesDictionary.cs b/src/Sarif/PropertiesDictionary.cs index 1a6f8d3e0..6d4dc90ab 100644 --- a/src/Sarif/PropertiesDictionary.cs +++ b/src/Sarif/PropertiesDictionary.cs @@ -6,7 +6,6 @@ using System.Collections.Immutable; using System.ComponentModel; using System.IO; -using System.Runtime.Serialization; using System.Xml; using Newtonsoft.Json; @@ -37,11 +36,6 @@ public PropertiesDictionary( { } - protected PropertiesDictionary(SerializationInfo info, StreamingContext context) - : base(info, context) - { - } - public string Name { get; set; } public virtual T GetProperty(PerLanguageOption setting) diff --git a/src/Sarif/TypedPropertiesDictionary.cs b/src/Sarif/TypedPropertiesDictionary.cs index 5e8360c23..848b67a11 100644 --- a/src/Sarif/TypedPropertiesDictionary.cs +++ b/src/Sarif/TypedPropertiesDictionary.cs @@ -2,8 +2,8 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System; +using System.Collections.Concurrent; using System.Collections.Generic; -using System.Runtime.Serialization; using Newtonsoft.Json; @@ -13,7 +13,7 @@ public interface IMarker { } [Serializable] [JsonConverter(typeof(TypedPropertiesDictionaryConverter))] - public class TypedPropertiesDictionary : Dictionary, IMarker where T : new() + public class TypedPropertiesDictionary : ConcurrentDictionary, IMarker where T : new() { public TypedPropertiesDictionary() : this(null, StringComparer.Ordinal) { @@ -30,11 +30,6 @@ public TypedPropertiesDictionary(PropertiesDictionary initializer, IEqualityComp } } - protected TypedPropertiesDictionary(SerializationInfo info, StreamingContext context) - : base(info, context) - { - } - [System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Usage", "CA2227:CollectionPropertiesShouldBeReadOnly")] protected Dictionary SettingNameToDescriptionsMap { get; set; } diff --git a/src/Test.UnitTests.Sarif/PropertiesDictionaryTests.cs b/src/Test.UnitTests.Sarif/PropertiesDictionaryTests.cs index 40abc5cb7..37680b9d1 100644 --- a/src/Test.UnitTests.Sarif/PropertiesDictionaryTests.cs +++ b/src/Test.UnitTests.Sarif/PropertiesDictionaryTests.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.IO; +using System.Threading.Tasks; using FluentAssertions; @@ -124,6 +125,40 @@ public void PropertiesDictionary_RoundTripStringToVersionMap() ((TypedPropertiesDictionary)properties[MapKey])[ValueKey].Should().Be(version); } + [Fact] + public void PropertiesDictionary_ConcurrentReadAndWrite() + { + var properties = new PropertiesDictionary(); + + Exception exception = Record.Exception(() => + { + List taskList = new List(); + + for (int i = 0; i < 1000; i++) + { + // repeatedly set to same field + taskList.Add(Task.Factory.StartNew(() => + properties.SetProperty(StringSetProperty, new StringSet(new string[] { i.ToString() })))); + + // repeatedly set to different fields + taskList.Add(Task.Factory.StartNew(() => + properties.SetProperty(GetStringSetProperty(i), new StringSet(new string[] { i.ToString() })))); + + // repeatedly read from same field + taskList.Add(Task.Factory.StartNew(() => + properties.GetProperty(StringSetProperty))); + + // repeatedly read from different fields + taskList.Add(Task.Factory.StartNew(() => + properties.GetProperty(GetStringSetProperty(i)))); + } + + Task.WaitAll(taskList.ToArray()); + }); + Assert.Null(exception); + } + + private void ValidateProperties(PropertiesDictionary actual, PropertiesDictionary expected) { actual.Keys.Count.Should().Be(expected.Keys.Count); @@ -192,6 +227,10 @@ private PropertiesDictionary RoundTripThroughJson(PropertiesDictionary propertie new PerLanguageOption( FEATURE, nameof(StringSetProperty), defaultValue: () => { return STRINGSET_DEFAULT; }); + public static PerLanguageOption GetStringSetProperty(int i) => + new PerLanguageOption( + FEATURE, nameof(StringSetProperty) + i, defaultValue: () => { return STRINGSET_DEFAULT; }); + public static PerLanguageOption IntegerSetProperty { get; } = new PerLanguageOption( FEATURE, nameof(IntegerSetProperty), defaultValue: () => { return INTEGERSET_DEFAULT; }); From 73779127b3420ab42005a9d15704d4f76fa00d56 Mon Sep 17 00:00:00 2001 From: Shaopeng <81775155+shaopeng-gh@users.noreply.github.com> Date: Thu, 9 Dec 2021 16:14:53 -0800 Subject: [PATCH 2/8] fix SettingNameToDescriptionsMap as well --- src/Sarif/PropertiesDictionary.cs | 3 ++- src/Sarif/PropertiesDictionaryExtensionMethods.cs | 3 ++- src/Sarif/TypedPropertiesDictionary.cs | 4 ++-- src/Test.UnitTests.Sarif/PropertiesDictionaryTests.cs | 7 +++---- 4 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/Sarif/PropertiesDictionary.cs b/src/Sarif/PropertiesDictionary.cs index 6d4dc90ab..56342d7c2 100644 --- a/src/Sarif/PropertiesDictionary.cs +++ b/src/Sarif/PropertiesDictionary.cs @@ -2,6 +2,7 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System; +using System.Collections.Concurrent; using System.Collections.Generic; using System.Collections.Immutable; using System.ComponentModel; @@ -85,7 +86,7 @@ public void SetProperty(IOption setting, object value, bool cacheDescription, bo if (cacheDescription) { - SettingNameToDescriptionsMap = SettingNameToDescriptionsMap ?? new Dictionary(); + SettingNameToDescriptionsMap ??= new ConcurrentDictionary(); SettingNameToDescriptionsMap[setting.Name] = setting.Description; } diff --git a/src/Sarif/PropertiesDictionaryExtensionMethods.cs b/src/Sarif/PropertiesDictionaryExtensionMethods.cs index d27384e70..60070d452 100644 --- a/src/Sarif/PropertiesDictionaryExtensionMethods.cs +++ b/src/Sarif/PropertiesDictionaryExtensionMethods.cs @@ -3,6 +3,7 @@ using System; using System.Collections; +using System.Collections.Concurrent; using System.Collections.Generic; using System.Collections.Specialized; using System.ComponentModel; @@ -27,7 +28,7 @@ public static void SavePropertiesToXmlStream( XmlWriter writer, XmlWriterSettings settings, string name, - Dictionary settingNameToDescriptionMap) + ConcurrentDictionary settingNameToDescriptionMap) { if (propertyBag == null) { diff --git a/src/Sarif/TypedPropertiesDictionary.cs b/src/Sarif/TypedPropertiesDictionary.cs index 848b67a11..11745107f 100644 --- a/src/Sarif/TypedPropertiesDictionary.cs +++ b/src/Sarif/TypedPropertiesDictionary.cs @@ -31,7 +31,7 @@ public TypedPropertiesDictionary(PropertiesDictionary initializer, IEqualityComp } [System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Usage", "CA2227:CollectionPropertiesShouldBeReadOnly")] - protected Dictionary SettingNameToDescriptionsMap { get; set; } + protected ConcurrentDictionary SettingNameToDescriptionsMap { get; set; } public virtual T GetProperty(PerLanguageOption setting) { @@ -69,7 +69,7 @@ public virtual void SetProperty(IOption setting, T value, bool cacheDescription) if (cacheDescription) { - SettingNameToDescriptionsMap = SettingNameToDescriptionsMap ?? new Dictionary(); + SettingNameToDescriptionsMap ??= new ConcurrentDictionary(); SettingNameToDescriptionsMap[setting.Name] = setting.Description; } diff --git a/src/Test.UnitTests.Sarif/PropertiesDictionaryTests.cs b/src/Test.UnitTests.Sarif/PropertiesDictionaryTests.cs index 37680b9d1..a59455ed0 100644 --- a/src/Test.UnitTests.Sarif/PropertiesDictionaryTests.cs +++ b/src/Test.UnitTests.Sarif/PropertiesDictionaryTests.cs @@ -142,7 +142,7 @@ public void PropertiesDictionary_ConcurrentReadAndWrite() // repeatedly set to different fields taskList.Add(Task.Factory.StartNew(() => - properties.SetProperty(GetStringSetProperty(i), new StringSet(new string[] { i.ToString() })))); + properties.SetProperty(GenerateStringSetProperty(i), new StringSet(new string[] { i.ToString() })))); // repeatedly read from same field taskList.Add(Task.Factory.StartNew(() => @@ -150,7 +150,7 @@ public void PropertiesDictionary_ConcurrentReadAndWrite() // repeatedly read from different fields taskList.Add(Task.Factory.StartNew(() => - properties.GetProperty(GetStringSetProperty(i)))); + properties.GetProperty(GenerateStringSetProperty(i)))); } Task.WaitAll(taskList.ToArray()); @@ -158,7 +158,6 @@ public void PropertiesDictionary_ConcurrentReadAndWrite() Assert.Null(exception); } - private void ValidateProperties(PropertiesDictionary actual, PropertiesDictionary expected) { actual.Keys.Count.Should().Be(expected.Keys.Count); @@ -227,7 +226,7 @@ private PropertiesDictionary RoundTripThroughJson(PropertiesDictionary propertie new PerLanguageOption( FEATURE, nameof(StringSetProperty), defaultValue: () => { return STRINGSET_DEFAULT; }); - public static PerLanguageOption GetStringSetProperty(int i) => + public static PerLanguageOption GenerateStringSetProperty(int i) => new PerLanguageOption( FEATURE, nameof(StringSetProperty) + i, defaultValue: () => { return STRINGSET_DEFAULT; }); From 725f201499deb91fd79a5783efcc2e8137a835b4 Mon Sep 17 00:00:00 2001 From: Shaopeng <81775155+shaopeng-gh@users.noreply.github.com> Date: Fri, 10 Dec 2021 01:36:14 -0800 Subject: [PATCH 3/8] fix SaveSettingNameToDescriptionMap --- .../PropertiesDictionaryExtensionMethods.cs | 26 ++-- .../PropertiesDictionaryTests.cs | 116 ++++++++++++++---- 2 files changed, 108 insertions(+), 34 deletions(-) diff --git a/src/Sarif/PropertiesDictionaryExtensionMethods.cs b/src/Sarif/PropertiesDictionaryExtensionMethods.cs index 60070d452..76c0a244f 100644 --- a/src/Sarif/PropertiesDictionaryExtensionMethods.cs +++ b/src/Sarif/PropertiesDictionaryExtensionMethods.cs @@ -74,6 +74,7 @@ public static void SavePropertiesToXmlStream( StringSet stringSet = property as StringSet; if (stringSet != null) { + SaveSettingNameToDescriptionMap(writer, key, settingNameToDescriptionMap); SaveSet(writer, stringSet, key); continue; } @@ -81,6 +82,7 @@ public static void SavePropertiesToXmlStream( IntegerSet integerSet = property as IntegerSet; if (integerSet != null) { + SaveSettingNameToDescriptionMap(writer, key, settingNameToDescriptionMap); SaveSet(writer, integerSet, key); continue; } @@ -88,19 +90,12 @@ public static void SavePropertiesToXmlStream( IDictionary pb = property as IDictionary; if (pb != null) { + SaveSettingNameToDescriptionMap(writer, key, settingNameToDescriptionMap); ((IDictionary)pb).SavePropertiesToXmlStream(writer, settings, key, settingNameToDescriptionMap); continue; } - if (settingNameToDescriptionMap != null) - { - string description; - - if (settingNameToDescriptionMap.TryGetValue(key, out description)) - { - writer.WriteComment(description); - } - } + SaveSettingNameToDescriptionMap(writer, key, settingNameToDescriptionMap); writer.WriteStartElement(PROPERTY_ID); writer.WriteAttributeString(KEY_ID, key); @@ -120,6 +115,19 @@ public static void SavePropertiesToXmlStream( writer.WriteEndElement(); // Properties } + private static void SaveSettingNameToDescriptionMap(XmlWriter writer, string key, ConcurrentDictionary settingNameToDescriptionMap) + { + if (settingNameToDescriptionMap != null) + { + string description; + + if (settingNameToDescriptionMap.TryGetValue(key, out description)) + { + writer.WriteComment(description); + } + } + } + private static string NormalizeTypeName(string typeName) { // This helper currently assumes that namespaces in diff --git a/src/Test.UnitTests.Sarif/PropertiesDictionaryTests.cs b/src/Test.UnitTests.Sarif/PropertiesDictionaryTests.cs index a59455ed0..d5882781a 100644 --- a/src/Test.UnitTests.Sarif/PropertiesDictionaryTests.cs +++ b/src/Test.UnitTests.Sarif/PropertiesDictionaryTests.cs @@ -18,80 +18,85 @@ public class PropertiesDictionaryTests private const string OPTIONS = FEATURE + ".Options"; private static readonly bool BOOL_DEFAULT = true; + private static readonly bool BOOL_NONDEFAULT = false; private static readonly StringSet STRINGSET_DEFAULT = new StringSet(new string[] { "a", "b", "c" }); + private static readonly StringSet STRINGSET_NONDEFAULT = new StringSet(new string[] { "d", "e", "f" }); private static readonly IntegerSet INTEGERSET_DEFAULT = new IntegerSet(new int[] { -1, 0, 1, 2 }); + private static readonly IntegerSet INTEGERSET_NONDEFAULT = new IntegerSet(new int[] { -3, 0, 6, 9 }); private static readonly PropertiesDictionary PROPERTIES_DEFAULT = new PropertiesDictionary { { "TestKey", "TestValue" } }; + private static readonly PropertiesDictionary PROPERTIES_NONDEFAULT = new PropertiesDictionary + { + { "TestNonDefaultKey", "TestNonDefaultValue" }, + { "NewKey", 1337 }, + { "AnotherKey", true } + }; [Fact] - public void PropertiesDictionary_RoundTripBoolean() + public void PropertiesDictionary_RoundTrip_Boolean() { var properties = new PropertiesDictionary(); properties.GetProperty(BooleanProperty).Should().Be(BOOL_DEFAULT); - bool nonDefaultValue = false; - properties.SetProperty(BooleanProperty, nonDefaultValue); - properties.GetProperty(BooleanProperty).Should().Be(nonDefaultValue); + properties.SetProperty(BooleanProperty, BOOL_NONDEFAULT); + properties.GetProperty(BooleanProperty).Should().Be(BOOL_NONDEFAULT); properties = RoundTripThroughXml(properties); - properties.GetProperty(BooleanProperty).Should().Be(nonDefaultValue); + properties.GetProperty(BooleanProperty).Should().Be(BOOL_NONDEFAULT); properties = RoundTripThroughJson(properties); - properties.GetProperty(BooleanProperty).Should().Be(nonDefaultValue); + properties.GetProperty(BooleanProperty).Should().Be(BOOL_NONDEFAULT); } [Fact] - public void PropertiesDictionary_RoundTripStringSet() + public void PropertiesDictionary_RoundTrip_StringSet() { var properties = new PropertiesDictionary(); ValidateSets(properties.GetProperty(StringSetProperty), STRINGSET_DEFAULT); - var nonDefaultValue = new StringSet(new string[] { "d", "e" }); - properties.SetProperty(StringSetProperty, nonDefaultValue); + properties.SetProperty(StringSetProperty, STRINGSET_NONDEFAULT); properties = RoundTripThroughXml(properties); - ValidateSets(properties.GetProperty(StringSetProperty), nonDefaultValue); + ValidateSets(properties.GetProperty(StringSetProperty), STRINGSET_NONDEFAULT); properties = RoundTripThroughJson(properties); - ValidateSets(properties.GetProperty(StringSetProperty), nonDefaultValue); + ValidateSets(properties.GetProperty(StringSetProperty), STRINGSET_NONDEFAULT); } [Fact] - public void PropertiesDictionary_RoundTripIntegerSet() + public void PropertiesDictionary_RoundTrip_IntegerSet() { var properties = new PropertiesDictionary(); ValidateSets(properties.GetProperty(IntegerSetProperty), INTEGERSET_DEFAULT); - var nonDefaultValue = new IntegerSet(new int[] { 3, 4 }); - properties.SetProperty(IntegerSetProperty, nonDefaultValue); + properties.SetProperty(IntegerSetProperty, INTEGERSET_NONDEFAULT); properties = RoundTripThroughXml(properties); - ValidateSets(properties.GetProperty(IntegerSetProperty), nonDefaultValue); + ValidateSets(properties.GetProperty(IntegerSetProperty), INTEGERSET_NONDEFAULT); properties = RoundTripThroughJson(properties); - ValidateSets(properties.GetProperty(IntegerSetProperty), nonDefaultValue); + ValidateSets(properties.GetProperty(IntegerSetProperty), INTEGERSET_NONDEFAULT); } [Fact] - public void PropertiesDictionary_RoundTripNestedPropertiesDictionary() + public void PropertiesDictionary_RoundTrip_NestedPropertiesDictionary() { var properties = new PropertiesDictionary(); ValidateProperties(properties.GetProperty(PropertiesDictionaryProperty), PROPERTIES_DEFAULT); - var nonDefaultValue = new PropertiesDictionary { { "NewKey", 1337 }, { "AnotherKey", true } }; - properties.SetProperty(PropertiesDictionaryProperty, nonDefaultValue); + properties.SetProperty(PropertiesDictionaryProperty, PROPERTIES_NONDEFAULT); properties = RoundTripThroughXml(properties); - ValidateProperties(properties.GetProperty(PropertiesDictionaryProperty), nonDefaultValue); + ValidateProperties(properties.GetProperty(PropertiesDictionaryProperty), PROPERTIES_NONDEFAULT); properties = RoundTripThroughJson(properties); - ValidateProperties(properties.GetProperty(PropertiesDictionaryProperty), nonDefaultValue); + ValidateProperties(properties.GetProperty(PropertiesDictionaryProperty), PROPERTIES_NONDEFAULT); } [Fact] - public void PropertiesDictionary_RoundTripEmptyStringToVersionMap() + public void PropertiesDictionary_RoundTrip_EmptyStringToVersionMap() { const string MapKey = "MapKey"; const string ValueKey = "NewKey"; @@ -109,7 +114,7 @@ public void PropertiesDictionary_RoundTripEmptyStringToVersionMap() } [Fact] - public void PropertiesDictionary_RoundTripStringToVersionMap() + public void PropertiesDictionary_RoundTrip_StringToVersionMap() { const string MapKey = "MapKey"; const string ValueKey = "NewKey"; @@ -158,6 +163,35 @@ public void PropertiesDictionary_ConcurrentReadAndWrite() Assert.Null(exception); } + [Fact] + public void PropertiesDictionary_TestCacheDescription() + { + PropertiesDictionary_TestCacheDescription_Helper(GenerateBooleanProperty, BOOL_NONDEFAULT); + PropertiesDictionary_TestCacheDescription_Helper(GenerateStringSetProperty, STRINGSET_NONDEFAULT); + PropertiesDictionary_TestCacheDescription_Helper(GenerateIntegerSetProperty, INTEGERSET_NONDEFAULT); + PropertiesDictionary_TestCacheDescription_Helper(GeneratePropertiesDictionaryProperty, PROPERTIES_NONDEFAULT); + } + private void PropertiesDictionary_TestCacheDescription_Helper(Func GeneratePropertyMethod, object value) + { + string textLoaded = string.Empty; + string desc = "desc to save, no: "; + + var properties = new PropertiesDictionary(); + + properties.SetProperty(GeneratePropertyMethod(1, desc + 1), value, true); + properties.SetProperty(GeneratePropertyMethod(2, null), value, true); + properties.SetProperty(GeneratePropertyMethod(3, desc + 3), value, true); + properties.SetProperty(GeneratePropertyMethod(4, desc + 4), value, false); + properties.SetProperty(GeneratePropertyMethod(5, desc + 5), value, true); + + textLoaded = RoundTripWriteToXmlAndReadAsText(properties); + textLoaded.Should().Contain(desc + 1); + textLoaded.Should().NotContain(desc + 2); + textLoaded.Should().Contain(desc + 3); + textLoaded.Should().NotContain(desc + 4); + textLoaded.Should().Contain(desc + 5); + } + private void ValidateProperties(PropertiesDictionary actual, PropertiesDictionary expected) { actual.Keys.Count.Should().Be(expected.Keys.Count); @@ -198,6 +232,26 @@ private PropertiesDictionary RoundTripThroughXml(PropertiesDictionary properties return properties; } + private string RoundTripWriteToXmlAndReadAsText(PropertiesDictionary properties) + { + string temp = Path.GetTempFileName(); + string textLoaded = string.Empty; + + try + { + properties.SaveToXml(temp); + textLoaded = File.ReadAllText(temp); + } + finally + { + if (File.Exists(temp)) + { + File.Delete(temp); + } + } + return textLoaded; + } + private PropertiesDictionary RoundTripThroughJson(PropertiesDictionary properties) { string temp = Path.GetTempFileName(); @@ -222,20 +276,32 @@ private PropertiesDictionary RoundTripThroughJson(PropertiesDictionary propertie new PerLanguageOption( FEATURE, nameof(BooleanProperty), defaultValue: () => { return BOOL_DEFAULT; }); + public static PerLanguageOption GenerateBooleanProperty(int i, string description = null) => + new PerLanguageOption( + FEATURE, nameof(BooleanProperty) + i, defaultValue: () => { return BOOL_DEFAULT; }, description); + public static PerLanguageOption StringSetProperty { get; } = new PerLanguageOption( FEATURE, nameof(StringSetProperty), defaultValue: () => { return STRINGSET_DEFAULT; }); - public static PerLanguageOption GenerateStringSetProperty(int i) => + public static PerLanguageOption GenerateStringSetProperty(int i, string description = null) => new PerLanguageOption( - FEATURE, nameof(StringSetProperty) + i, defaultValue: () => { return STRINGSET_DEFAULT; }); + FEATURE, nameof(StringSetProperty) + i, defaultValue: () => { return STRINGSET_DEFAULT; }, description); public static PerLanguageOption IntegerSetProperty { get; } = new PerLanguageOption( FEATURE, nameof(IntegerSetProperty), defaultValue: () => { return INTEGERSET_DEFAULT; }); + public static PerLanguageOption GenerateIntegerSetProperty(int i, string description = null) => + new PerLanguageOption( + FEATURE, nameof(IntegerSetProperty) + i, defaultValue: () => { return INTEGERSET_DEFAULT; }, description); + public static PerLanguageOption PropertiesDictionaryProperty { get; } = new PerLanguageOption( FEATURE, nameof(PropertiesDictionaryProperty), defaultValue: () => { return PROPERTIES_DEFAULT; }); + + public static PerLanguageOption GeneratePropertiesDictionaryProperty(int i, string description = null) => + new PerLanguageOption( + FEATURE, nameof(PropertiesDictionaryProperty) + i, defaultValue: () => { return PROPERTIES_DEFAULT; }, description); } } From d43c31e51b5bd08fc9487952e6f73179c5112d87 Mon Sep 17 00:00:00 2001 From: Shaopeng <81775155+shaopeng-gh@users.noreply.github.com> Date: Fri, 10 Dec 2021 16:17:07 -0800 Subject: [PATCH 4/8] do not return bool for ConcurrentDictionary.Add() --- src/Sarif/ExtensionMethods.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Sarif/ExtensionMethods.cs b/src/Sarif/ExtensionMethods.cs index aff477fe5..46692a139 100644 --- a/src/Sarif/ExtensionMethods.cs +++ b/src/Sarif/ExtensionMethods.cs @@ -587,14 +587,14 @@ internal static void ConsumeElementOfDepth(this XmlReader xmlReader, int endElem } } - public static bool Add(this ConcurrentDictionary concurrentDictionary, string key, T value) + public static void Add(this ConcurrentDictionary concurrentDictionary, string key, T value) { - return concurrentDictionary.TryAdd(key, value); + ((IDictionary)concurrentDictionary).Add(key, value); } - public static bool Remove(this ConcurrentDictionary concurrentDictionary, string key) + public static void Remove(this ConcurrentDictionary concurrentDictionary, string key) { - return concurrentDictionary.TryRemove(key, out _); + ((IDictionary)concurrentDictionary).Remove(key); } } } From 8eb304d1bee931243c956f8b83ee1264eb583948 Mon Sep 17 00:00:00 2001 From: Shaopeng <81775155+shaopeng-gh@users.noreply.github.com> Date: Sun, 12 Dec 2021 01:28:26 -0800 Subject: [PATCH 5/8] fix comments --- src/ReleaseHistory.md | 4 +++ .../PropertiesDictionaryExtensionMethods.cs | 28 +++++++------------ .../PropertiesDictionaryTests.cs | 1 + 3 files changed, 15 insertions(+), 18 deletions(-) diff --git a/src/ReleaseHistory.md b/src/ReleaseHistory.md index 83e4fe219..4d021f919 100644 --- a/src/ReleaseHistory.md +++ b/src/ReleaseHistory.md @@ -1,5 +1,9 @@ # SARIF Package Release History (SDK, Driver, Converters, and Multitool) +## Unreleased + +* BUGFIX: Fix `InvalidOperationException` when using PropertiesDictionary in a multithreaded application. [#2415](https://github.com/microsoft/sarif-sdk/pull/2415) + ## **v2.4.12** [Sdk](https://www.nuget.org/packages/Sarif.Sdk/2.4.12) | [Driver](https://www.nuget.org/packages/Sarif.Driver/2.4.12) | [Converters](https://www.nuget.org/packages/Sarif.Converters/2.4.12) | [Multitool](https://www.nuget.org/packages/Sarif.Multitool/2.4.12) | [Multitool Library](https://www.nuget.org/packages/Sarif.Multitool.Library/2.4.12) * FEATURE: `MultithreadCommandBase` will use cache when hashing is enabled. [#2388](https://github.com/microsoft/sarif-sdk/pull/2388) diff --git a/src/Sarif/PropertiesDictionaryExtensionMethods.cs b/src/Sarif/PropertiesDictionaryExtensionMethods.cs index 76c0a244f..d41adbeed 100644 --- a/src/Sarif/PropertiesDictionaryExtensionMethods.cs +++ b/src/Sarif/PropertiesDictionaryExtensionMethods.cs @@ -71,10 +71,19 @@ public static void SavePropertiesToXmlStream( { object property = propertyBag[key]; + if (settingNameToDescriptionMap != null) + { + string description; + + if (settingNameToDescriptionMap.TryGetValue(key, out description)) + { + writer.WriteComment(description); + } + } + StringSet stringSet = property as StringSet; if (stringSet != null) { - SaveSettingNameToDescriptionMap(writer, key, settingNameToDescriptionMap); SaveSet(writer, stringSet, key); continue; } @@ -82,7 +91,6 @@ public static void SavePropertiesToXmlStream( IntegerSet integerSet = property as IntegerSet; if (integerSet != null) { - SaveSettingNameToDescriptionMap(writer, key, settingNameToDescriptionMap); SaveSet(writer, integerSet, key); continue; } @@ -90,13 +98,10 @@ public static void SavePropertiesToXmlStream( IDictionary pb = property as IDictionary; if (pb != null) { - SaveSettingNameToDescriptionMap(writer, key, settingNameToDescriptionMap); ((IDictionary)pb).SavePropertiesToXmlStream(writer, settings, key, settingNameToDescriptionMap); continue; } - SaveSettingNameToDescriptionMap(writer, key, settingNameToDescriptionMap); - writer.WriteStartElement(PROPERTY_ID); writer.WriteAttributeString(KEY_ID, key); @@ -115,19 +120,6 @@ public static void SavePropertiesToXmlStream( writer.WriteEndElement(); // Properties } - private static void SaveSettingNameToDescriptionMap(XmlWriter writer, string key, ConcurrentDictionary settingNameToDescriptionMap) - { - if (settingNameToDescriptionMap != null) - { - string description; - - if (settingNameToDescriptionMap.TryGetValue(key, out description)) - { - writer.WriteComment(description); - } - } - } - private static string NormalizeTypeName(string typeName) { // This helper currently assumes that namespaces in diff --git a/src/Test.UnitTests.Sarif/PropertiesDictionaryTests.cs b/src/Test.UnitTests.Sarif/PropertiesDictionaryTests.cs index d5882781a..241fbc29d 100644 --- a/src/Test.UnitTests.Sarif/PropertiesDictionaryTests.cs +++ b/src/Test.UnitTests.Sarif/PropertiesDictionaryTests.cs @@ -171,6 +171,7 @@ public void PropertiesDictionary_TestCacheDescription() PropertiesDictionary_TestCacheDescription_Helper(GenerateIntegerSetProperty, INTEGERSET_NONDEFAULT); PropertiesDictionary_TestCacheDescription_Helper(GeneratePropertiesDictionaryProperty, PROPERTIES_NONDEFAULT); } + private void PropertiesDictionary_TestCacheDescription_Helper(Func GeneratePropertyMethod, object value) { string textLoaded = string.Empty; From bc850ac356b4448215f6104b716b97e99d0eee8f Mon Sep 17 00:00:00 2001 From: Shaopeng <81775155+shaopeng-gh@users.noreply.github.com> Date: Mon, 3 Jan 2022 19:30:21 -0800 Subject: [PATCH 6/8] remove [Serializable] from class PropertiesDictionary --- src/ReleaseHistory.md | 2 +- src/Sarif/PropertiesDictionary.cs | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/ReleaseHistory.md b/src/ReleaseHistory.md index 4d021f919..077ed1ba4 100644 --- a/src/ReleaseHistory.md +++ b/src/ReleaseHistory.md @@ -2,7 +2,7 @@ ## Unreleased -* BUGFIX: Fix `InvalidOperationException` when using PropertiesDictionary in a multithreaded application. [#2415](https://github.com/microsoft/sarif-sdk/pull/2415) +* BREAKING: Fix `InvalidOperationException` when using PropertiesDictionary in a multithreaded application, and remove `[Serializable]` from it. Now use of BinaryFormatter on it will result in `SerializationException`: Type `PropertiesDictionary` is not marked as serializable. [#2415](https://github.com/microsoft/sarif-sdk/pull/2415) ## **v2.4.12** [Sdk](https://www.nuget.org/packages/Sarif.Sdk/2.4.12) | [Driver](https://www.nuget.org/packages/Sarif.Driver/2.4.12) | [Converters](https://www.nuget.org/packages/Sarif.Converters/2.4.12) | [Multitool](https://www.nuget.org/packages/Sarif.Multitool/2.4.12) | [Multitool Library](https://www.nuget.org/packages/Sarif.Multitool.Library/2.4.12) diff --git a/src/Sarif/PropertiesDictionary.cs b/src/Sarif/PropertiesDictionary.cs index 56342d7c2..62623402d 100644 --- a/src/Sarif/PropertiesDictionary.cs +++ b/src/Sarif/PropertiesDictionary.cs @@ -15,7 +15,6 @@ namespace Microsoft.CodeAnalysis.Sarif { - [Serializable] [JsonConverter(typeof(TypedPropertiesDictionaryConverter))] public class PropertiesDictionary : TypedPropertiesDictionary { From 649f09e621580e463d4c44a052ac4b68d9840395 Mon Sep 17 00:00:00 2001 From: "Michael C. Fanning" Date: Tue, 4 Jan 2022 10:59:26 -0800 Subject: [PATCH 7/8] Add 'Add' and 'Remove' methods from from TypedPropertiesDictionary, so that it can be used where IDictionary is expected. --- src/Sarif/PropertiesDictionaryExtensionMethods.cs | 2 +- src/Sarif/TypedPropertiesDictionary.cs | 12 +++++++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/Sarif/PropertiesDictionaryExtensionMethods.cs b/src/Sarif/PropertiesDictionaryExtensionMethods.cs index d41adbeed..bea097440 100644 --- a/src/Sarif/PropertiesDictionaryExtensionMethods.cs +++ b/src/Sarif/PropertiesDictionaryExtensionMethods.cs @@ -28,7 +28,7 @@ public static void SavePropertiesToXmlStream( XmlWriter writer, XmlWriterSettings settings, string name, - ConcurrentDictionary settingNameToDescriptionMap) + IDictionary settingNameToDescriptionMap) { if (propertyBag == null) { diff --git a/src/Sarif/TypedPropertiesDictionary.cs b/src/Sarif/TypedPropertiesDictionary.cs index 11745107f..9f1d0341f 100644 --- a/src/Sarif/TypedPropertiesDictionary.cs +++ b/src/Sarif/TypedPropertiesDictionary.cs @@ -30,6 +30,16 @@ public TypedPropertiesDictionary(PropertiesDictionary initializer, IEqualityComp } } + public void Add(string key, T value) + { + ((IDictionary)this).Add(key, value); + } + + public void Remove(string key) + { + ((IDictionary)this).Remove(key); + } + [System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Usage", "CA2227:CollectionPropertiesShouldBeReadOnly")] protected ConcurrentDictionary SettingNameToDescriptionsMap { get; set; } @@ -63,7 +73,7 @@ public virtual void SetProperty(IOption setting, T value, bool cacheDescription) if (value == null && this.ContainsKey(setting.Name)) { - this.Remove(setting.Name); + this.TryRemove(setting.Name, out T Value); return; } From 74d24510e47774eef845bf541d620d33b5512d8c Mon Sep 17 00:00:00 2001 From: "Michael C. Fanning" Date: Tue, 4 Jan 2022 11:38:05 -0800 Subject: [PATCH 8/8] Reset call to Remove in TypedPropertiesDictionary. --- src/Sarif/TypedPropertiesDictionary.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Sarif/TypedPropertiesDictionary.cs b/src/Sarif/TypedPropertiesDictionary.cs index 9f1d0341f..344c2a4c0 100644 --- a/src/Sarif/TypedPropertiesDictionary.cs +++ b/src/Sarif/TypedPropertiesDictionary.cs @@ -73,7 +73,7 @@ public virtual void SetProperty(IOption setting, T value, bool cacheDescription) if (value == null && this.ContainsKey(setting.Name)) { - this.TryRemove(setting.Name, out T Value); + this.Remove(setting.Name); return; }