8000 clean state classes, add tests · BKHMSI/botbuilder-python@c563149 · GitHub
[go: up one dir, main page]

Skip to content

Commit c563149

Browse files
committed
clean state classes, add tests
1 parent 684c62f commit c563149

File tree

5 files changed

+190
-18
lines changed

5 files changed

+190
-18
lines changed

libraries/botbuilder-core/botbuilder/core/bot_state.py

Lines changed: 5 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -29,14 +29,10 @@ async def on_process_request(self, context, next_middleware):
2929
"""
3030
await self.read(context, True)
3131
# For libraries like aiohttp, the web.Response need to be bubbled up from the process_activity logic, which is
32-
# why we store the value from next_middleware()
32+
# the results are stored from next_middleware()
3333
logic_results = await next_middleware()
3434

35-
# print('Printing after log ran')
36-
# print(context.services['state']) # This is appearing as a dict which doesn't seem right
37-
# print(context.services['state']['state'])
38-
39-
await self.write(context, True) # Both of these probably shouldn't be True
35+
await self.write(context)
4036
return logic_results
4137

4238
async def read(self, context: BotContext, force: bool=False):
@@ -51,14 +47,9 @@ async def read(self, context: BotContext, force: bool=False):
5147
if force or cached is None or ('state' in cached and cached['state'] is None):
5248
key = self.storage_key(context)
5349
items = await self.storage.read([key])
54-
55-
# state = items.get(key, {}) # This is a problem, we need to decide how the data is going to be handled:
56-
# Is it going to be serialized the entire way down or only partially serialized?
57-
# The current code is a bad implementation...
58-
5950
state = items.get(key, StoreItem())
6051
hash_state = calculate_change_hash(state)
61-
# context.services.set(self.state_key, {'state': state, 'hash': hash_state}) # <-- dict.set() doesn't exist
52+
6253
context.services[self.state_key] = {'state': state, 'hash': hash_state}
6354
return state
6455

@@ -78,13 +69,11 @@ async def write(self, context: BotContext, force: bool=False):
7869

7970
if cached is None:
8071
cached = {'state': StoreItem(e_tag='*'), 'hash': ''}
81-
changes = {key: cached['state']} # this doesn't seem right
82-
# changes.key = cached['state'] # Wtf is this going to be used for?
72+
changes = {key: cached['state']}
8373
await self.storage.write(changes)
8474

8575
cached['hash'] = calculate_change_hash(cached['state'])
86-
context.services[self.state_key] = cached # instead of `cached` shouldn't this be
87-
# context.services[self.state_key][key] = cached
76+
context.services[self.state_key] = cached
8877

8978
async def clear(self, context: BotContext):
9079
"""

libraries/botbuilder-core/botbuilder/core/conversation_state.py

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,24 @@ class ConversationState(BotState):
1010
"""
1111
Reads and writes conversation state for your bot to storage.
1212
"""
13+
14+
no_key_error_message = 'ConversationState: channelId and/or conversation missing from context.activity.'
15+
1316
def __init__(self, storage: Storage, namespace: str=''):
1417
"""
1518
Creates a new ConversationState instance.
1619
:param storage:
1720
:param namespace:
1821
"""
19-
super(ConversationState, self).__init__(storage, self.get_storage_key)
22+
23+
def call_get_storage_key(context):
24+
key = self.get_storage_key(context)
25+
if key is None:
26+
raise AttributeError(self.no_key_error_message)
27+
else:
28+
return key
29+
30+
super(ConversationState, self).__init__(storage, call_get_storage_key)
2031
self.namespace = namespace
2132

2233
def get_storage_key(self, context: BotContext):

libraries/botbuilder-core/botbuilder/core/user_state.py

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,25 @@ class UserState(BotState):
1010
"""
1111
Reads and writes user state for your bot to storage.
1212
"""
13+
14+
no_key_error_message = 'UserState: channel_id and/or conversation missing from context.activity.'
15+
1316
def __init__(self, storage: Storage, namespace=''):
1417
"""
1518
Creates a new UserState instance.
1619
:param storage:
1720
:param namespace:
1821
"""
1922
self.namespace = namespace
20-
super(UserState, self).__init__(storage, self.get_storage_key)
23+
24+
def call_get_storage_key(context):
25+
key = self.get_storage_key(context)
26+
if key is None:
27+
raise AttributeError(self.no_key_error_message)
28+
else:
29+
return key
30+
31+
super(UserState, self).__init__(storage, call_get_storage_key)
2132

2233
def get_storage_key(self, context: BotContext) -> str:
2334
"""
Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
# Copyright (c) Microsoft Corporation. All rights reserved.
2+
# Licensed under the MIT License.
3+
4+
import pytest
5+
6+
from botbuilder.core import BotContext, MemoryStorage, TestAdapter, ConversationState
7+
from botbuilder.schema import Activity, ConversationAccount
8+
9+
RECEIVED_MESSAGE = Activity(type='message',
10+
text='received',
11+
channel_id='test',
12+
conversation=ConversationAccount(id='convo'))
13+
MISSING_CHANNEL_ID = Activity(type='message',
14+
text='received',
15+
conversation=ConversationAccount(id='convo'))
16+
MISSING_CONVERSATION = Activity(type='message',
17+
text='received',
18+
channel_id='test')
19+
END_OF_CONVERSATION = Activity(type='endOfConversation',
20+
channel_id='test',
21+
conversation=ConversationAccount(id='convo'))
22+
23+
24+
class TestConversationState:
25+
storage = MemoryStorage()
26+
adapter = TestAdapter(None)
27+
context = BotContext(adapter, RECEIVED_MESSAGE)
28+
middleware = ConversationState(storage)
29+
30+
@pytest.mark.asyncio
31+
async def test_should_load_and_save_state_from_storage(self):
32+
key = None
33+
34+
async def next_middleware():
35+
nonlocal key
36+
key = self.middleware.get_storage_key(self.context)
37+
state = await self.middleware.get(self.context)
38+
assert state is not None, 'State not loaded'
39+
assert key is not None, 'Key not found'
40+
state.test = 'foo'
41+
42+
await self.middleware.on_process_request(self.context, next_middleware)
43+
44+
items = await self.storage.read([key])
45+
assert key in items, 'Saved state not found in storage.'
46+
assert items[key].test == 'foo', 'Missing test value in stored state.'
47+
48+
@pytest.mark.asyncio
49+
async def test_should_ignore_any_activities_that_are_not_endOfConversation(self):
50+
key = None
51+
52+
async def next_middleware():
53+
nonlocal key
54+
key = self.middleware.get_storage_key(self.context)
55+
state = await self.middleware.get(self.context)
56+
assert state.test == 'foo', 'invalid initial state'
57+
await self.context.send_activity(Activity(type='message', text='foo'))
58+
59+
await self.middleware.on_process_request(self.context, next_middleware)
60+
items = await self.storage.read([key])
61+
assert hasattr(items[key], 'test'), 'state cleared and should not have been'
62+
63+
@pytest.mark.asyncio
64+
async def test_should_reject_with_error_if_channel_id_is_missing(self):
65+
context = BotContext(self.adapter, MISSING_CHANNEL_ID)
66+
67+
async def next_middleware():
68+
assert False, 'should not have called next_middleware'
69+
70+
try:
71+
await self.middleware.on_process_request(context, next_middleware)
72+
except AttributeError:
73+
pass
74+
except Exception as e:
75+
raise e
76+
else:
77+
raise AssertionError('Should not have completed and not raised AttributeError.')
78+
79+
@pytest.mark.asyncio
80+
async def test_should_reject_with_error_if_conversation_is_missing(self):
81+
context = BotContext(self.adapter, MISSING_CONVERSATION)
82+
83+
async def next_middleware():
84+
assert False, 'should not have called next_middleware'
85+
86+
try:
87+
await self.middleware.on_process_request(context, next_middleware)
88+
except AttributeError:
89+
pass
90+
except Exception as e:
91+
raise e
92+
else:
93+
raise AssertionError('Should not have completed and not raised AttributeError.')
Lines changed: 68 additions & 0 deletions
< 8447 /tr>
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
# Copyright (c) Microsoft Corporation. All rights reserved.
2+
# Licensed under the MIT License.
3+
4+
import pytest
5+
6+
from botbuilder.core import BotContext, MemoryStorage, StoreItem, TestAdapter, UserState
7+
from botbuilder.schema import Activity, ChannelAccount
8+
9+
RECEIVED_MESSAGE = Activity(type='message',
10+
text='received',
11+
channel_id='test',
12+
from_property=ChannelAccount(id='user'))
13+
MISSING_CHANNEL_ID = Activity(type='message',
14+
text='received',
15+
from_property=ChannelAccount(id='user'))
16+
MISSING_FROM_PROPERTY = Activity(type='message',
17+
text='received',
18+
channel_id='test')
19+
20+
21+
class TestUserState:
22+
storage = MemoryStorage()
23+
adapter = TestAdapter(None)
24+
context = BotContext(adapter, RECEIVED_MESSAGE)
25+
middleware = UserState(storage)
26+
27+
@pytest.mark.asyncio
28+
async def test_should_load_and_save_state_from_storage(self):
29+
30+
async def next_middleware():
31+
state = await self.middleware.get(self.context)
32+
assert isinstance(state, StoreItem), 'State not loaded'
33+
state.test = 'foo'
34+
35+
await self.middleware.on_process_request(self.context, next_middleware)
36+
key = self.middleware.get_storage_key(self.context)
37+
assert type(key) == str, 'Key not found'
38+
items = await self.storage.read([key])
39+
assert key in items, 'Saved state not found in storage'
40+
assert items[key].test == 'foo', 'Missing test value in stored state.'
41+
42+
@pytest.mark.asyncio
43+
async def test_should_reject_with_error_if_channel_id_is_missing(self):
44+
context = BotContext(self.adapter, MISSING_CHANNEL_ID)
45+
46+
async def next_middleware():
47+
assert False, 'Should not have called next_middleware'
48+
49+
try:
50+
await self.middleware.on_process_request(context, next_middleware)
51+
except AttributeError:
52+
pass
53+
else:
54+
raise AssertionError('Should not have completed and not raised AttributeError.')
55+
56+
@pytest.mark.asyncio
57+
async def test_should_reject_with_error_if_from_property_is_missing(self):
58+
context = BotContext(self.adapter, MISSING_FROM_PROPERTY)
59+
60+
async def next_middleware():
61+
assert False, 'Should not have called next_middleware'
62+
63+
try:
64+
await self.middleware.on_process_request(context, next_middleware)
65+
except AttributeError:
66+
pass
67+
else:
68+
raise AssertionError('Should not have completed and not raised AttributeError.')

0 commit comments

Comments
 (0)
0