fix: Use IStorage for skill's id factory

runtime/dotnet/core/SkillConversationIdFactory.cs

@@ -1,45 +1,78 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License.
 
-using System.Collections.Concurrent;
+using System;
+using System.Collections.Generic;
+using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
+using Microsoft.Bot.Builder;
 using Microsoft.Bot.Builder.Skills;
-using Microsoft.Bot.Schema;
-using Newtonsoft.Json;
+using Newtonsoft.Json.Linq;
 
 namespace Microsoft.BotFramework.Composer.Core
 {
     /// <summary>
-    /// A <see cref="SkillConversationIdFactory"/> that uses an in memory <see cref="ConcurrentDictionary{TKey,TValue}"/>
-    /// to store and retrieve <see cref="ConversationReference"/> instances.
+    /// A <see cref="SkillConversationIdFactory"/> that uses <see cref="IStorage"/> to store
+    /// and retrieve <see cref="SkillConversationReference"/> instances.
     /// </summary>
     public class SkillConversationIdFactory : SkillConversationIdFactoryBase
     {
-        private readonly ConcurrentDictionary<string, string> _conversationRefs = new ConcurrentDictionary<string, string>();
+        private readonly IStorage _storage;
 
-        public override Task<string> CreateSkillConversationIdAsync(SkillConversationIdFactoryOptions options, CancellationToken cancellationToken)
+        public SkillConversationIdFactory(IStorage storage)
         {
+            _storage = storage ?? throw new ArgumentNullException(nameof(storage));
+        }
+
+        public override async Task<string> CreateSkillConversationIdAsync(SkillConversationIdFactoryOptions options, CancellationToken cancellationToken)
+        {
+            if (options == null)
+            {
+                throw new ArgumentNullException(nameof(options));
+            }
+
+            // Create the storage key based on the SkillConversationIdFactoryOptions.
+            var conversationReference = options.Activity.GetConversationReference();
+            var skillConversationId = $"{options.FromBotId}-{options.BotFrameworkSkill.AppId}-{conversationReference.Conversation.Id}-{conversationReference.ChannelId}-skillconvo";
+
+            // Create the SkillConversationReference instance.
             var skillConversationReference = new SkillConversationReference
             {
-                ConversationReference = options.Activity.GetConversationReference(),
+                ConversationReference = conversationReference,
                 OAuthScope = options.FromBotOAuthScope
             };
-            var key = $"{options.FromBotId}-{options.BotFrameworkSkill.AppId}-{skillConversationReference.ConversationReference.Conversation.Id}-{skillConversationReference.ConversationReference.ChannelId}-skillconvo";
-            _conversationRefs.GetOrAdd(key, JsonConvert.SerializeObject(skillConversationReference));
-            return Task.FromResult(key);
+
+            // Store the SkillConversationReference using the skillConversationId as a key.
+            var skillConversationInfo = new Dictionary<string, object> { { skillConversationId, JObject.FromObject(skillConversationReference) } };
+            await _storage.WriteAsync(skillConversationInfo, cancellationToken).ConfigureAwait(false);
+
+            // Return the generated skillConversationId (that will be also used as the conversation ID to call the skill).
+            return skillConversationId;
         }
 
-        public override Task<SkillConversationReference> GetSkillConversationReferenceAsync(string skillConversationId, CancellationToken cancellationToken)
+        public override async Task<SkillConversationReference> GetSkillConversationReferenceAsync(string skillConversationId, CancellationToken cancellationToken)
         {
-            var conversationReference = JsonConvert.DeserializeObject<SkillConversationReference>(_conversationRefs[skillConversationId]);
-            return Task.FromResult(conversationReference);
+            if (string.IsNullOrWhiteSpace(skillConversationId))
+            {
+                throw new ArgumentNullException(nameof(skillConversationId));
+            }
+
+            // Get the SkillConversationReference from storage for the given skillConversationId.
+            var skillConversationInfo = await _storage.ReadAsync(new[] { skillConversationId }, cancellationToken).ConfigureAwait(false);
+            if (skillConversationInfo.Any())
+            {
+                var conversationInfo = ((JObject)skillConversationInfo[skillConversationId]).ToObject<SkillConversationReference>();
+                return conversationInfo;
+            }
+
+            return null;
         }
 
-        public override Task DeleteConversationReferenceAsync(string skillConversationId, CancellationToken cancellationToken)
+        public override async Task DeleteConversationReferenceAsync(string skillConversationId, CancellationToken cancellationToken)
         {
-            _conversationRefs.TryRemove(skillConversationId, out _);
-            return Task.CompletedTask;
+            // Delete the SkillConversationReference from storage.
+            await _storage.DeleteAsync(new[] { skillConversationId }, cancellationToken).ConfigureAwait(false);
         }
     }
 }

runtime/dotnet/tests/SkillConversationIdFactoryTests.cs

@@ -0,0 +1,156 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+using Microsoft.Bot.Builder;
+using Microsoft.Bot.Builder.Adapters;
+using Microsoft.Bot.Builder.Dialogs;
+using Microsoft.Bot.Builder.Dialogs.Adaptive;
+using Microsoft.Bot.Builder.Dialogs.Declarative;
+using Microsoft.Bot.Builder.Dialogs.Declarative.Resources;
+using Microsoft.Bot.Builder.Skills;
+using Microsoft.Bot.Connector.Authentication;
+using Microsoft.Bot.Schema;
+using Microsoft.BotFramework.Composer.Core;
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+using System;
+using System.Collections.Generic;
+using System.IO;
+using System.Security.Claims;
+using System.Threading;
+using System.Threading.Tasks;
+
+namespace Tests
+{
+    [TestClass]
+    public class SkillConversationIdFactoryTests
+    {
+        private readonly SkillConversationIdFactory _idFactory = new SkillConversationIdFactory(new MemoryStorage());
+        private string _botId = Guid.NewGuid().ToString("N");
+        private string _skillId = Guid.NewGuid().ToString("N");
+
+        [ClassInitialize]
+        public static void ClassInitialize(TestContext context)
+        {
+        }
+
+        [TestMethod]
+        public async Task ShouldCreateCorrectConversationId()
+        {
+            var claimsIdentity = new ClaimsIdentity();
+            claimsIdentity.AddClaim(new Claim(AuthenticationConstants.AudienceClaim, _botId));
+            claimsIdentity.AddClaim(new Claim(AuthenticationConstants.AppIdClaim, _skillId));
+            claimsIdentity.AddClaim(new Claim(AuthenticationConstants.ServiceUrlClaim, "http://testbot.com/api/messages"));
+            var conversationReference = new ConversationReference
+            {
+                Conversation = new ConversationAccount(id: Guid.NewGuid().ToString("N")),
+                ServiceUrl = "http://testbot.com/api/messages"
+            };
+
+            var activity = (Activity)Activity.CreateMessageActivity();
+            activity.ApplyConversationReference(conversationReference);
+            var skill = new BotFrameworkSkill()
+            {
+                AppId = _skillId,
+                Id = "skill",
+                SkillEndpoint = new Uri("http://testbot.com/api/messages")
+            };
+
+            var options = new SkillConversationIdFactoryOptions
+            {
+                FromBotOAuthScope = _botId,
+                FromBotId = _botId,
+                Activity = activity,
+                BotFrameworkSkill = skill
+            };
+
+            var conversationId = _idFactory.CreateSkillConversationIdAsync(options, CancellationToken.None).Result;
+            Assert.IsNotNull(conversationId);
+        }
+
+        [TestMethod]
+        public async Task ShouldGetConversationReferenceFromConversationId()
+        {
+            var claimsIdentity = new ClaimsIdentity();
+            claimsIdentity.AddClaim(new Claim(AuthenticationConstants.AudienceClaim, _botId));
+            claimsIdentity.AddClaim(new Claim(AuthenticationConstants.AppIdClaim, _skillId));
+            claimsIdentity.AddClaim(new Claim(AuthenticationConstants.ServiceUrlClaim, "http://testbot.com/api/messages"));
+            var conversationReference = new ConversationReference
+            {
+                Conversation = new ConversationAccount(id: Guid.NewGuid().ToString("N")),
+                ServiceUrl = "http://testbot.com/api/messages"
+            };
+
+            var activity = (Activity)Activity.CreateMessageActivity();
+            activity.ApplyConversationReference(conversationReference);
+            var skill = new BotFrameworkSkill()
+            {
+                AppId = _skillId,
+                Id = "skill",
+                SkillEndpoint = new Uri("http://testbot.com/api/messages")
+            };
+
+            var options = new SkillConversationIdFactoryOptions
+            {
+                FromBotOAuthScope = _botId,
+                FromBotId = _botId,
+                Activity = activity,
+                BotFrameworkSkill = skill
+            };
+
+            var conversationId = await _idFactory.CreateSkillConversationIdAsync(options, CancellationToken.None);
+            Assert.IsNotNull(conversationId);
+
+            var skillConversationRef = await _idFactory.GetSkillConversationReferenceAsync(conversationId, CancellationToken.None);
+            Assert.IsTrue(RefEquals(skillConversationRef.ConversationReference, conversationReference));
+        }
+
+        [TestMethod]
+        public async Task ShouldNotGetReferenceAfterDeleted()
+        {
+            var claimsIdentity = new ClaimsIdentity();
+            claimsIdentity.AddClaim(new Claim(AuthenticationConstants.AudienceClaim, _botId));
+            claimsIdentity.AddClaim(new Claim(AuthenticationConstants.AppIdClaim, _skillId));
+            claimsIdentity.AddClaim(new Claim(AuthenticationConstants.ServiceUrlClaim, "http://testbot.com/api/messages"));
+            var conversationReference = new ConversationReference
+            {
+                Conversation = new ConversationAccount(id: Guid.NewGuid().ToString("N")),
+                ServiceUrl = "http://testbot.com/api/messages"
+            };
+
+            var activity = (Activity)Activity.CreateMessageActivity();
+            activity.ApplyConversationReference(conversationReference);
+            var skill = new BotFrameworkSkill()
+            {
+                AppId = _skillId,
+                Id = "skill",
+                SkillEndpoint = new Uri("http://testbot.com/api/messages")
+            };
+
+            var options = new SkillConversationIdFactoryOptions
+            {
+                FromBotOAuthScope = _botId,
+                FromBotId = _botId,
+                Activity = activity,
+                BotFrameworkSkill = skill
+            };
+
+            var conversationId = await _idFactory.CreateSkillConversationIdAsync(options, CancellationToken.None);
+            Assert.IsNotNull(conversationId);
+
+            var skillConversationRef = await _idFactory.GetSkillConversationReferenceAsync(conversationId, CancellationToken.None);
+            Assert.IsTrue(RefEquals(skillConversationRef.ConversationReference, conversationReference));
+
+            await _idFactory.DeleteConversationReferenceAsync(conversationId, CancellationToken.None);
+
+            var skillConversationRefAfterDeleted = await _idFactory.GetSkillConversationReferenceAsync(conversationId, CancellationToken.None);
+            Assert.IsNull(skillConversationRefAfterDeleted);
+        }
+
+        private bool RefEquals(ConversationReference ref1, ConversationReference ref2)
+        {
+            return ref1.Conversation.Id == ref2.Conversation.Id && ref1.ServiceUrl == ref2.ServiceUrl;
+
+        }
+    }
+}
+