From fe9c8602bff10cb9d7b68c8860b4d2c6101c978f Mon Sep 17 00:00:00 2001 From: carlosscastro Date: Tue, 20 Oct 2020 10:52:33 -0700 Subject: [PATCH 1/3] Debugging: Move from first win to last wins, interface converter now has a non-concurrent dictionary and default source map is nullsourcemap --- .../DebuggerSourceMap.cs | 13 +++++++++++++ .../Converters/InterfaceConverter.cs | 2 +- .../Debugging/DebugSupport.cs | 2 +- 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/libraries/Microsoft.Bot.Builder.Dialogs.Debugging/DebuggerSourceMap.cs b/libraries/Microsoft.Bot.Builder.Dialogs.Debugging/DebuggerSourceMap.cs index 0a258c9f62..6cd8152ccb 100644 --- a/libraries/Microsoft.Bot.Builder.Dialogs.Debugging/DebuggerSourceMap.cs +++ b/libraries/Microsoft.Bot.Builder.Dialogs.Debugging/DebuggerSourceMap.cs @@ -73,6 +73,19 @@ void ISourceMap.Add(object item, SourceRange range) lock (_gate) { + // Map works on a last one wins basis. When we refresh the item for a range, + // just keep the last range + + // Remove old item instance for this range. + // Find the entry for the current range + var rangeItemEntry = _sourceByItem.FirstOrDefault(kv => kv.Value.Equals(range) && kv.Key.GetType().Equals(item.GetType())); + + if (rangeItemEntry.Key != null) + { + // If found, remove the outdated item from the map + _sourceByItem.Remove(rangeItemEntry.Key); + } + _sourceByItem[item] = range; _dirty = true; } diff --git a/libraries/Microsoft.Bot.Builder.Dialogs.Declarative/Converters/InterfaceConverter.cs b/libraries/Microsoft.Bot.Builder.Dialogs.Declarative/Converters/InterfaceConverter.cs index 9e636b687a..7869f8236d 100644 --- a/libraries/Microsoft.Bot.Builder.Dialogs.Declarative/Converters/InterfaceConverter.cs +++ b/libraries/Microsoft.Bot.Builder.Dialogs.Declarative/Converters/InterfaceConverter.cs @@ -24,7 +24,7 @@ public class InterfaceConverter : JsonConverter, IObservableConverter, IObser private readonly ResourceExplorer resourceExplorer; private readonly List observers = new List(); private readonly SourceContext sourceContext; - private readonly ConcurrentDictionary cachedRefDialogs = new ConcurrentDictionary(); + private readonly Dictionary cachedRefDialogs = new Dictionary(); /// /// Initializes a new instance of the class. diff --git a/libraries/Microsoft.Bot.Builder.Dialogs/Debugging/DebugSupport.cs b/libraries/Microsoft.Bot.Builder.Dialogs/Debugging/DebugSupport.cs index 9671f20c34..42af5cd45a 100644 --- a/libraries/Microsoft.Bot.Builder.Dialogs/Debugging/DebugSupport.cs +++ b/libraries/Microsoft.Bot.Builder.Dialogs/Debugging/DebugSupport.cs @@ -15,7 +15,7 @@ public static class DebugSupport /// Gets or sets the source map instance. /// /// The instance. - public static ISourceMap SourceMap { get; set; } = Debugging.SourceMap.Instance; + public static ISourceMap SourceMap { get; set; } = NullSourceMap.Instance; /// /// Extension method to get IDialogDebugger from TurnContext. From 042ed83c94b8698ffb277bc3b7040d378b359243 Mon Sep 17 00:00:00 2001 From: carlosscastro Date: Tue, 20 Oct 2020 15:11:15 -0700 Subject: [PATCH 2/3] Undo source map change --- .../Microsoft.Bot.Builder.Dialogs/Debugging/DebugSupport.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/Microsoft.Bot.Builder.Dialogs/Debugging/DebugSupport.cs b/libraries/Microsoft.Bot.Builder.Dialogs/Debugging/DebugSupport.cs index 42af5cd45a..9671f20c34 100644 --- a/libraries/Microsoft.Bot.Builder.Dialogs/Debugging/DebugSupport.cs +++ b/libraries/Microsoft.Bot.Builder.Dialogs/Debugging/DebugSupport.cs @@ -15,7 +15,7 @@ public static class DebugSupport /// Gets or sets the source map instance. /// /// The instance. - public static ISourceMap SourceMap { get; set; } = NullSourceMap.Instance; + public static ISourceMap SourceMap { get; set; } = Debugging.SourceMap.Instance; /// /// Extension method to get IDialogDebugger from TurnContext. From 84a8862882504b047a1caaf68952ec2da5339bb8 Mon Sep 17 00:00:00 2001 From: carlosscastro Date: Tue, 20 Oct 2020 15:15:25 -0700 Subject: [PATCH 3/3] Declarative: Remove JToken cloning due to JSOn.NET bug --- .../Converters/InterfaceConverter.cs | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/libraries/Microsoft.Bot.Builder.Dialogs.Declarative/Converters/InterfaceConverter.cs b/libraries/Microsoft.Bot.Builder.Dialogs.Declarative/Converters/InterfaceConverter.cs index 7869f8236d..914a7154ea 100644 --- a/libraries/Microsoft.Bot.Builder.Dialogs.Declarative/Converters/InterfaceConverter.cs +++ b/libraries/Microsoft.Bot.Builder.Dialogs.Declarative/Converters/InterfaceConverter.cs @@ -203,9 +203,6 @@ public void RegisterObserver(IJsonLoadObserver observer) private static JToken TryAssignId(JToken jToken, SourceContext sourceContext) { - // This is the jToken we'll use to build the concrete type. - var tokenToBuild = jToken; - // If our JToken does not have an id, try to get an id from the resource explorer // in a best-effort manner. if (jToken is JObject jObj && !jObj.ContainsKey("id")) @@ -213,15 +210,13 @@ private static JToken TryAssignId(JToken jToken, SourceContext sourceContext) // Check if we have an id registered for this token if (sourceContext is ResourceSourceContext rsc && rsc.DefaultIdMap.ContainsKey(jToken)) { - // Clone the token since we'll alter it from the file version. - // If we don't clone, future ranges will be calculated based on the altered token. - // which will end in a wrong source range. - tokenToBuild = jToken.DeepClone(); - tokenToBuild["id"] = rsc.DefaultIdMap[jToken]; + // Just assign. Avoid cloning JTokens since Json.NET does not clone line info + // Tracking item on Json.NET: https://github.com/JamesNK/Newtonsoft.Json/issues/2410 + jToken["id"] = rsc.DefaultIdMap[jToken]; } } - return tokenToBuild; + return jToken; } } }