From 4897c721796c7e15094410c7dd6de2677ad14cef Mon Sep 17 00:00:00 2001 From: Axel Suarez Date: Tue, 20 Oct 2020 15:28:23 -0700 Subject: [PATCH] TranscriptLogger should not log continue conversation --- .../botbuilder/core/__init__.py | 3 ++ .../botbuilder/core/adapters/test_adapter.py | 1 + .../botbuilder/core/bot_framework_adapter.py | 3 +- .../core/conversation_reference_extension.py | 9 +++- .../core/memory_transcript_store.py | 11 +++-- .../botbuilder/core/transcript_logger.py | 18 +++++++- .../test_transcript_logger_middleware.py | 44 +++++++++++++++++++ .../botbuilder/schema/__init__.py | 2 + .../botbuilder/schema/_models_py3.py | 6 +++ 9 files changed, 89 insertions(+), 8 deletions(-) create mode 100644 libraries/botbuilder-core/tests/test_transcript_logger_middleware.py diff --git a/libraries/botbuilder-core/botbuilder/core/__init__.py b/libraries/botbuilder-core/botbuilder/core/__init__.py index ec7b45807..0a9a218fa 100644 --- a/libraries/botbuilder-core/botbuilder/core/__init__.py +++ b/libraries/botbuilder-core/botbuilder/core/__init__.py @@ -39,6 +39,7 @@ from .telemetry_logger_constants import TelemetryLoggerConstants from .telemetry_logger_middleware import TelemetryLoggerMiddleware from .turn_context import TurnContext +from .transcript_logger import TranscriptLogger, TranscriptLoggerMiddleware from .user_state import UserState from .register_class_middleware import RegisterClassMiddleware from .adapter_extensions import AdapterExtensions @@ -87,6 +88,8 @@ "TelemetryLoggerConstants", "TelemetryLoggerMiddleware", "TopIntent", + "TranscriptLogger", + "TranscriptLoggerMiddleware", "TurnContext", "UserState", "UserTokenProvider", diff --git a/libraries/botbuilder-core/botbuilder/core/adapters/test_adapter.py b/libraries/botbuilder-core/botbuilder/core/adapters/test_adapter.py index 09ffb3e76..6488a2726 100644 --- a/libraries/botbuilder-core/botbuilder/core/adapters/test_adapter.py +++ b/libraries/botbuilder-core/botbuilder/core/adapters/test_adapter.py @@ -225,6 +225,7 @@ async def create_conversation( type=ActivityTypes.conversation_update, members_added=[], members_removed=[], + channel_id=channel_id, conversation=ConversationAccount(id=str(uuid.uuid4())), ) context = self.create_turn_context(update) diff --git a/libraries/botbuilder-core/botbuilder/core/bot_framework_adapter.py b/libraries/botbuilder-core/botbuilder/core/bot_framework_adapter.py index 31540ccec..930f29d44 100644 --- a/libraries/botbuilder-core/botbuilder/core/bot_framework_adapter.py +++ b/libraries/botbuilder-core/botbuilder/core/bot_framework_adapter.py @@ -39,6 +39,7 @@ ) from botbuilder.schema import ( Activity, + ActivityEventNames, ActivityTypes, ChannelAccount, ConversationAccount, @@ -390,7 +391,7 @@ async def create_conversation( event_activity = Activity( type=ActivityTypes.event, - name="CreateConversation", + name=ActivityEventNames.create_conversation, channel_id=channel_id, service_url=service_url, id=resource_response.activity_id diff --git a/libraries/botbuilder-core/botbuilder/core/conversation_reference_extension.py b/libraries/botbuilder-core/botbuilder/core/conversation_reference_extension.py index 6dd4172e9..a04ce237d 100644 --- a/libraries/botbuilder-core/botbuilder/core/conversation_reference_extension.py +++ b/libraries/botbuilder-core/botbuilder/core/conversation_reference_extension.py @@ -1,13 +1,18 @@ # Copyright (c) Microsoft Corporation. All rights reserved. # Licensed under the MIT License. import uuid -from botbuilder.schema import Activity, ActivityTypes, ConversationReference +from botbuilder.schema import ( + Activity, + ActivityEventNames, + ActivityTypes, + ConversationReference, +) def get_continuation_activity(reference: ConversationReference) -> Activity: return Activity( type=ActivityTypes.event, - name="ContinueConversation", + name=ActivityEventNames.continue_conversation, id=str(uuid.uuid1()), channel_id=reference.channel_id, service_url=reference.service_url, diff --git a/libraries/botbuilder-core/botbuilder/core/memory_transcript_store.py b/libraries/botbuilder-core/botbuilder/core/memory_transcript_store.py index e8953e0ae..325cf32f6 100644 --- a/libraries/botbuilder-core/botbuilder/core/memory_transcript_store.py +++ b/libraries/botbuilder-core/botbuilder/core/memory_transcript_store.py @@ -6,6 +6,7 @@ from botbuilder.schema import Activity from .transcript_logger import PagedResult, TranscriptInfo, TranscriptStore + # pylint: disable=line-too-long class MemoryTranscriptStore(TranscriptStore): """This provider is most useful for simulating production storage when running locally against the @@ -59,7 +60,9 @@ async def get_transcript_activities( [ x for x in sorted( - transcript, key=lambda x: x.timestamp, reverse=False + transcript, + key=lambda x: x.timestamp or str(datetime.datetime.min), + reverse=False, ) if x.timestamp >= start_date ] @@ -72,9 +75,11 @@ async def get_transcript_activities( paged_result.items = [ x for x in sorted( - transcript, key=lambda x: x.timestamp, reverse=False + transcript, + key=lambda x: x.timestamp or datetime.datetime.min, + reverse=False, ) - if x.timestamp >= start_date + if (x.timestamp or datetime.datetime.min) >= start_date ][:20] if paged_result.items.count == 20: paged_result.continuation_token = paged_result.items[-1].id diff --git a/libraries/botbuilder-core/botbuilder/core/transcript_logger.py b/libraries/botbuilder-core/botbuilder/core/transcript_logger.py index 91aba2ac1..bfd838f24 100644 --- a/libraries/botbuilder-core/botbuilder/core/transcript_logger.py +++ b/libraries/botbuilder-core/botbuilder/core/transcript_logger.py @@ -9,7 +9,13 @@ from queue import Queue from abc import ABC, abstractmethod from typing import Awaitable, Callable, List -from botbuilder.schema import Activity, ActivityTypes, ConversationReference +from botbuilder.schema import ( + Activity, + ActivityEventNames, + ActivityTypes, + ChannelAccount, + ConversationReference, +) from .middleware_set import Middleware from .turn_context import TurnContext @@ -46,9 +52,17 @@ async def on_turn( activity = context.activity # Log incoming activity at beginning of turn if activity: + if not activity.from_property: + activity.from_property = ChannelAccount() if not activity.from_property.role: activity.from_property.role = "user" - await self.log_activity(transcript, copy.copy(activity)) + + # We should not log ContinueConversation events used by skills to initialize the middleware. + if not ( + context.activity.type == ActivityTypes.event + and context.activity.name == ActivityEventNames.continue_conversation + ): + await self.log_activity(transcript, copy.copy(activity)) # hook up onSend pipeline # pylint: disable=unused-argument diff --git a/libraries/botbuilder-core/tests/test_transcript_logger_middleware.py b/libraries/botbuilder-core/tests/test_transcript_logger_middleware.py new file mode 100644 index 000000000..2752043e5 --- /dev/null +++ b/libraries/botbuilder-core/tests/test_transcript_logger_middleware.py @@ -0,0 +1,44 @@ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. + +import aiounittest + +from botbuilder.core import ( + MemoryTranscriptStore, + TranscriptLoggerMiddleware, + TurnContext, +) +from botbuilder.core.adapters import TestAdapter, TestFlow +from botbuilder.schema import Activity, ActivityEventNames, ActivityTypes + + +class TestTranscriptLoggerMiddleware(aiounittest.AsyncTestCase): + async def test_should_not_log_continue_conversation(self): + transcript_store = MemoryTranscriptStore() + conversation_id = "" + sut = TranscriptLoggerMiddleware(transcript_store) + + async def aux_logic(context: TurnContext): + nonlocal conversation_id + conversation_id = context.activity.conversation.id + + adapter = TestAdapter(aux_logic) + adapter.use(sut) + + continue_conversation_activity = Activity( + type=ActivityTypes.event, name=ActivityEventNames.continue_conversation + ) + + test_flow = TestFlow(None, adapter) + step1 = await test_flow.send("foo") + step2 = await step1.send("bar") + await step2.send(continue_conversation_activity) + + paged_result = await transcript_store.get_transcript_activities( + "test", conversation_id + ) + self.assertEqual( + len(paged_result.items), + 2, + "only the two message activities should be logged", + ) diff --git a/libraries/botbuilder-schema/botbuilder/schema/__init__.py b/libraries/botbuilder-schema/botbuilder/schema/__init__.py index d2183f6eb..734d6d91c 100644 --- a/libraries/botbuilder-schema/botbuilder/schema/__init__.py +++ b/libraries/botbuilder-schema/botbuilder/schema/__init__.py @@ -2,6 +2,7 @@ # Licensed under the MIT License. from ._models_py3 import Activity +from ._models_py3 import ActivityEventNames from ._models_py3 import AnimationCard from ._models_py3 import Attachment from ._models_py3 import AttachmentData @@ -74,6 +75,7 @@ __all__ = [ "Activity", + "ActivityEventNames", "AnimationCard", "Attachment", "AttachmentData", diff --git a/libraries/botbuilder-schema/botbuilder/schema/_models_py3.py b/libraries/botbuilder-schema/botbuilder/schema/_models_py3.py index 151b6feb2..472ff51ce 100644 --- a/libraries/botbuilder-schema/botbuilder/schema/_models_py3.py +++ b/libraries/botbuilder-schema/botbuilder/schema/_models_py3.py @@ -3,10 +3,16 @@ from botbuilder.schema._connector_client_enums import ActivityTypes from datetime import datetime +from enum import Enum from msrest.serialization import Model from msrest.exceptions import HttpOperationError +class ActivityEventNames(str, Enum): + continue_conversation = "ContinueConversation" + create_conversation = "CreateConversation" + + class ConversationReference(Model): """An object relating to a particular point in a conversation.