Fix bot state so that the context service key is used consistently to access the cached state

[v-kydela]

Fixes #3643

A bot state class's "context service key" is used to access the cached bot state in the turn state collection. Because the context service key is usually the name of the type (e.g. "UserState") there were some cases where the SDK was just using the type name instead of the actual context service key. This fails when a bot state class has a context service key that's different from the name of the type, as my new tests show.

In the case of a bot state memory scope, I also fixed the code to not rely on reflection to access the cached bot state. BotStateMemoryScope<T>.GetMemory now accesses the bot state instance in the same way that BotStateMemoryScope<T>.LoadAsync and BotStateMemoryScope<T>.SaveChangesAsync do, and once it has the bot state instance it can simply call BotState.Get. Because the classes that call GetMemory are expecting a dictionary and not a JObject, I modified BotState.Get to return an IDictionary<string, object>. I believe this is an improvement because that's the form that the cached bot state is already in and there's no need to convert it to a JObject. While it may seem like a dangerous change, the only place that BotState.Get is used is in one InspectionMiddleware method, and if any customers are using it and expecting a JObject then the compiler will just tell them to convert it to a JObject on their end, so there won't be any surprising behavior changes. I'm confident that the code is better this way and I can't imagine this breaking anything because the tests are all passing.

I have discovered that the problems I was seeing in BotStateMemoryScope<T> were not about GetMemory returning a JObject instead of a dictionary, they were about BotState.Get returning a copy of the of the state, which means any modifications wouldn't persist. I have also discovered that my "best option" to fix BotStateMemoryScope<T> by having Get return the original dictionary instead of a JObject will not work because it violates the contract. I see four other options:

1. Create a new public Get method (with a different name) to return the actual dictionary instead of a copy
2. Create an internal Get method and have the Bot Builder assembly share its members with the dialogs library
3. Create a private Get method and have BotStateMemoryScope<T>.GetMemory access it using reflection
4. Don't create any new methods and have BotStateMemoryScope<T>.GetMemory use reflection twice

I went with option 4 because it's the most similar to the way things are currently set up. The reason we need to use reflection twice is because the CachedBotState class is private and the context service key we need to access it is also private.

EDIT: I've thought of a fifth option. Since bot state is meant to be accessed using bot state property accessors, we could try to rework the adaptive dialogs library to access bot state using bot state property accessors. This would probably represent significant work/changes so I would not attempt it without some kind of go-ahead from the SDK team.

[tomlm]

🕐

[tomlm]

OK, here's what I think we should do:

1. Make existing class CachedBotState public
2. Make existing method BotState.GetCachedState() method
3. Change BotStateMemoryScope to use it

           var botState = GetBotState(dialogContext);
           var cachedState = botState.GetCachedState();
           return cachedState.State;

This gets rid of all reflection, and should be correct.

[v-kydela]

@tomlm - I've implemented your solution, and all checks are passing

[tomlm]

[tomlm]

looks good, thanks for the changess