Botbuilder-dotnet: Storing / retrieving conversation data within middleware

[garypretty]

What is the best way right now to store / retrieve conversation data within middleware that is specific to and only used within that middleware. e.g. storing a simple key/value like a Boolean flag so that I can alter the behavior of the middleware depending on the setting.

I might be missing something, but it looks like right now I need to define a specific StoreItem for the bot I am building and specify the storage medium, e.g. in memory, but if I want to create some middleware that is generic and can be used in any bot I am not sure how I would go about storing those settings. Is there still a generic key/value store for conversation data?

Can someone advise on this?

cc @cleemullins

[drub0y]

So it's not very obvious right now, but you should just be able to call context.GetConversationState<T>() and that T can be a POCO that's specific to your middleware. By default each T is keyed by T's type name (w/namespace and stuff) with a prefix of "ConversationState:" into the underlying state bag. You can see that here:

https://github.com/Microsoft/botbuilder-dotnet/blob/de2e744247138e72695507b5e598e737e481cbe7/libraries/Microsoft.Bot.Builder.Core/Middleware/BotState.cs#L107

It would probably be good to allow named instances as well, but I think there's still a lot of discussion that needs to be had around state/storage so please provide as much feedback as you can with this experience.

Let me know if this works for you!

[drub0y]

Oh and I should point out that there is no SetConversationState. The design is such that you call GetConversationState<T> and if your T doesn't exist yet, it will new one up. Also, any time you get the state, any changes to it are automatically going to be persisted back by the state management middleware later. Again, I think we'd love to hear any thoughts you might have about this as you start working with it.

[garypretty]

@drub0y Thanks. That's really helpful. One more quick question then on the back of this. How do I specify the storage method for the state, e.g. in memory? Let's say I am building a bot that doesn't need to manage state within the bot itself, but the user uses my middleware which does need state. If I try and use GetConversationState within middleware that's not going to work right? Is the only way to manage this to have some sort of dummy StoreItem defined on the bot itself, so something like new middleware.Add(ConversationState(new MemoryStorage()))?

Is there a way of simply specifying that state should be stored in memory without specifying a StoreItem if the bot doesn't need one?

Does that make sense? :)

[drub0y]

Yes, great observation and it's something that I have on a list of things that need some thought with respect to the design of state/storage right now. I intend to write most of these things up in a bigger DCR issue eventually.

Right now you would have to document that your middleware requires state and therefore the person who wants to run your bot would have to opt-into configuring the state middleware with a storage provider even though they're not using state themselves. I _believe_ you can just do this to work around it for now:

C# new ConversationState<object>(new MemoryStorage());
I could be wrong that this will end up working, I will try it out myself later today if you don't have a chance before then.

Either way, that obviously is not a good experience, so I believe ultimately that the ConversationState/BotState/UserState middleware should be decoupled from the actual TState of the bot itself.

[garypretty]

@drub0y Gave that a try and it won't work because it cannot convert from object to IStoreItem.. It's fine for now as I can work around by simply creating a dummy StoreItem, but I totally agree with you that the decoupling is probably necessary so that we can always rely on the fact that some sort of state storage has been defined, similar to the way you always specify your storage medium in v3.

[drub0y]

Oh really? That's actually a "bug" then. A PR just went in on Friday that was supposed to remove all need for implementing IStoreItem, I guess it didn't achieve that full goal yet. /cc @tomlm

[garypretty]

@drub0y I'll check that I have the latest - it's possible that I need to pull.

[garypretty]

@drub0y pretty sure I have latest so appears it is a bug but give it a go yourself later to make sure 😉

[drub0y]

Nope, I just peeked real quick and BotState<TState> still requires TState to be IStoreItem even though internally in its implementation it was updated to be something that is optional. This is definitely something @tomlm will want to fix.

[garypretty]

@drub0y on the subject of next having GetConversationState and no explicit set, I think this will cause confusion. Its nice, but I did spend about an hour this morning trying to find out how to set the state 🤔

[cleemullins]

@garypretty, @drub0y The "Get with no set" confused me. I think I even called out (incorrectly) in code-reviews "This code doesn't look like it will work". The behavior (to me) was totally non-intuitive.

We'll need to iterate on that pattern at some point in the very near future.

[garypretty]

@drub0y Let me run something past you if that's ok, because I just cannot get this working ;)

I have specified the InMemory storage on my bot as below, where DummyStateObject is just a StoreItem with no properties on it;

.Use(new ConversationState<DummyStateObject>(new MemoryStorage()))

Then in my middleware I am storing / retrieving the specific state for the middleware like this

MiddlewareState state = context.GetConversationState<MiddlewareState>() ?? new MiddlewareState();

However, when I call the line above the bot throws a 500 error and looking at the error it boils down to the below....

Unable to cast object of type Microsoft.Bot.Samples.Ai.QnA.Controllers.DummyStateObject to type Bot.Builder.ToyBox.Middleware.MiddlewareState

I am not sure why it is trying to cast one to the other, but it seems like because I have specified my InMemory storage explicitly with type DummyStateObject, I can't then store anything of a different type. If that's the case then right now there is no way of decoupling the middleware from the bot itself if the middleware needs to store state.

[drub0y]

@garypretty Hmmm, ok, that's interesting. I've been dealing with some other commitments today, so I apologize for not being able to jump in and test this out myself yet before replying.

I'm not super up to speed on the changes to state that have taken place recently, but I just had to take a minute to look at what you're saying and I can confirm this definitely appears to be a limitation right now. The state middleware itself is coupled specifically to a chosen TState which is logically tied to the bot. Other middleware does not appear to be able to maintain their own, ancillary state to the side after all. 😕 I'm pretty sure this is a regression over the original state management design which was more dictionary like. I don't suspect this design will stand as is, but it's definitely a limitation right now.

I'm sure you're already thinking of this, but one thing you can do to work around this is throw a property on your DummyStateObject that represents the MiddlewareState for your middleware and then, whenever the changes come to the state management APIs to address this, you would just patch up to getting the state directly from the state API yourself instead of via a property off the bot's state.

[garypretty]

@drub0y makes sense. One way to handle this would be to put some sort of generic dictionary on the StoreItem class which you could then take advantage of from within things like middleware. Best of both worlds potentially?

[drub0y]

@garypretty so technically StoreItem is little more than a glorified dictionary to begin with, but I personally don't think StoreItem should exist at all now that we're aiming to support POCO state objects.

To your point, at the very top level, state management should certainly be based on a some kind of simple key/value API... maybe not exposing raw Dictionary<string, object> as the surface API itself, but certainly providing the same semantics. All sorts of magical strong typing, auto-keying, state partitioning, etc can be layered on to that through extension methods and a stronger downstream state management API.

This is of course entirely my opinion and still needs to be laid out and discussed in a DCR issue to poke holes in it. I hope to get around to writing something up this week.

[garypretty]

@drub0y great. Look forward to reading it and contributing to the discussion. Will certainly be a good one to get figured out as I have some nice ideas for generic middleware, but most of them need to manage state of some sort. Thanks for the discussions and assistance today too!

[Stevenic]

So a couple of things... And I'll start by saying that I skimmed this so may be missing a few details:

• IStoreItem is mainly defining an eTag property which is needed to make concurrency work even if we changed the item to be a dictionary. Things like this are a little more loose on the JS side of things so I can understand how this interface causes grief for shared scenarios like this on the C# side of things.
• Currently your middleware can take a dependency on state in one of two ways:
  
  
  
  • Assuming its been installed before you in the middleware stack. If you try to get it and its not there you should just throw a detailed error saying that you're missing a dependency.
  
  
  • Have it passed into your middleware as a configuration parameter. This is more explicit but doesn't solve the TState problem raised above.
  
  

[drub0y]

• IStoreItem is mainly defining an eTag property which is needed to make concurrency work even if we changed the item to be a dictionary. Things like this are a little more loose on the JS side of things so I can understand how this interface causes grief for shared scenarios like this on the C# side of things.

Yeah, so IStoreItem (the interface) is still there and keeping that as a crutch for the people who look for/expect the fully strongly typed experience is probably "fine" (more on this in a sec), but with the recent work @tomlm has done is to support POCOs the StoreItem class (and FlexObject) can (should IMHO) be removed at this point. FWIW, if someone _wants_ to use a raw Dictionary<K, V> for their state, they can choose to do that, but we should support (as of now, we do) and encourage POCOs.

Ok, so what am I talking about with regards to the IStoreItem interface? Of course we need a strongly typed interface to have people expose an ETag property so that we can use it right? Well, no! There is a move towards supporting convention based programming in .NET these days so that frameworks do not have to permeate into the application domain. You see this in things like Entity Framework and ASP.NET (even more so in Core). What does this mean? Well it means that the storage providers don't have to actually only support the statically known IStoreItem::ETag property, they can just look for a string ETag property exposed by any POCO using a dynamic style of programming based on runtime code generation (e.g. _not_ reflection) that doesn't have a negative impact performance on runtime performance.

That said, all this talk about ETags is really interesting in and of itself because the whole point would be to deal with optimistic concurrency violations on state, right? Well... what happens if the ETag doesn't match? The storage layer is going to throw, right? Great... now what? How is the Bot Framework supposed to deal with this in a meaningful way? Is it purely to avoid stomping on state and therefore throwing exceptions is all we need to do? Will it automatically refresh the state, wipe any current responses and replay the request to the downstream middleware? What?

[Stevenic]

@drub0y let me respond to the concurrency issues... The way I have ConversationState and UserState middleware coded in the JS SDK is that they always set the eTag = '*' so that it's always last writer wins. This can't even be configured to work differently because as you said, there's nothing the bot can really do should a collision occur. Conversation and user state are essentially cookie storage and should be treated that way, If your bot needs to care about concurrency then you shouldn't put it in one of the state bags. You should instead do what you would do with cookies on a website. Store an immutable key in one of the state bags and then read & write your state against something that maintains concurrency. You can use your Storage provider for that as they do maintain concurrency by default but you're also free to just make database calls or anything else you want to do.

[garypretty]

Adding a note that this seems to be related to #262. I know that @drub0y has a plan to write something up around state all up. We had some conversations whilst I was out in Redmond and I think we are aligned in terms of our thinking and this is similar to what is being discussed in #262. Anyway, seems sensible to close this based on the DCR for state being tackled soon.