diff --git a/libraries/bot-builder/src/main/java/com/microsoft/bot/builder/TranscriptLoggerMiddleware.java b/libraries/bot-builder/src/main/java/com/microsoft/bot/builder/TranscriptLoggerMiddleware.java index 634e34c03..81f841f71 100644 --- a/libraries/bot-builder/src/main/java/com/microsoft/bot/builder/TranscriptLoggerMiddleware.java +++ b/libraries/bot-builder/src/main/java/com/microsoft/bot/builder/TranscriptLoggerMiddleware.java @@ -3,13 +3,10 @@ package com.microsoft.bot.builder; -import com.fasterxml.jackson.databind.JsonNode; -import com.fasterxml.jackson.databind.ObjectMapper; -import com.fasterxml.jackson.databind.SerializationFeature; import com.microsoft.bot.schema.Activity; import com.microsoft.bot.schema.ActivityTypes; import com.microsoft.bot.schema.ChannelAccount; -import org.apache.commons.lang3.StringUtils; +import com.microsoft.bot.schema.RoleTypes; import java.time.OffsetDateTime; import java.time.ZoneId; @@ -22,16 +19,6 @@ * When added, this middleware will log incoming and outgoing activities to a TranscriptStore. */ public class TranscriptLoggerMiddleware implements Middleware { - /** - * To/From JSON. - */ - private static ObjectMapper mapper; - - static { - mapper = new ObjectMapper() - .enable(SerializationFeature.INDENT_OUTPUT) - .findAndRegisterModules(); - } /** * The TranscriptLogger to log to. @@ -68,20 +55,7 @@ public TranscriptLoggerMiddleware(TranscriptLogger withTranscriptLogger) { public CompletableFuture onTurn(TurnContext context, NextDelegate next) { // log incoming activity at beginning of turn if (context.getActivity() != null) { - if (context.getActivity().getFrom() == null) { - context.getActivity().setFrom(new ChannelAccount()); - } - - JsonNode role = null; - if (context.getActivity().getFrom().getProperties().containsKey("role")) { - role = context.getActivity().getFrom().getProperties().get("role"); - } - - if (role == null || StringUtils.isBlank(role.asText())) { - context.getActivity().getFrom().getProperties().put("role", mapper.createObjectNode().with("user")); - } - - logActivity(Activity.clone(context.getActivity())); + logActivity(Activity.clone(context.getActivity()), true); } // hook up onSend pipeline @@ -90,7 +64,7 @@ public CompletableFuture onTurn(TurnContext context, NextDelegate next) { return nextSend.get() .thenApply(responses -> { for (Activity activity : activities) { - logActivity(Activity.clone(activity)); + logActivity(Activity.clone(activity), false); } return responses; @@ -105,7 +79,7 @@ public CompletableFuture onTurn(TurnContext context, NextDelegate next) { // add Message Update activity Activity updateActivity = Activity.clone(activity); updateActivity.setType(ActivityTypes.MESSAGE_UPDATE); - logActivity(updateActivity); + logActivity(updateActivity, false); return resourceResponse; }); @@ -123,7 +97,7 @@ public CompletableFuture onTurn(TurnContext context, NextDelegate next) { applyConversationReference(reference, false); }}; - logActivity(deleteActivity); + logActivity(deleteActivity, false); return null; }); @@ -141,10 +115,19 @@ public CompletableFuture onTurn(TurnContext context, NextDelegate next) { }); } - private void logActivity(Activity activity) { + private void logActivity(Activity activity, boolean incoming) { if (activity.getTimestamp() == null) { activity.setTimestamp(OffsetDateTime.now(ZoneId.of("UTC"))); } + + if (activity.getFrom() == null) { + activity.setFrom(new ChannelAccount()); + } + + if (activity.getFrom().getRole() == null) { + activity.getFrom().setRole(incoming ? RoleTypes.USER : RoleTypes.BOT); + } + transcript.offer(activity); } } diff --git a/libraries/bot-builder/src/test/java/com/microsoft/bot/builder/TranscriptMiddlewareTest.java b/libraries/bot-builder/src/test/java/com/microsoft/bot/builder/TranscriptMiddlewareTest.java index 00fb40a20..7435dc62d 100644 --- a/libraries/bot-builder/src/test/java/com/microsoft/bot/builder/TranscriptMiddlewareTest.java +++ b/libraries/bot-builder/src/test/java/com/microsoft/bot/builder/TranscriptMiddlewareTest.java @@ -223,5 +223,34 @@ public void Transcript_TestDateLogUpdateActivities() { pagedResult = transcriptStore.getTranscriptActivities("test", conversationId[0], null, OffsetDateTime.MAX).join(); Assert.assertEquals(0, pagedResult.getItems().size()); } + + @Test + public final void Transcript_RolesAreFilled() { + MemoryTranscriptStore transcriptStore = new MemoryTranscriptStore(); + TestAdapter adapter = (new TestAdapter()).use(new TranscriptLoggerMiddleware(transcriptStore)); + final String[] conversationId = {null}; + + new TestFlow(adapter, (context) -> { + // The next assert implicitly tests the immutability of the incoming + // message. As demonstrated by the asserts after this TestFlow block + // the role attribute is present on the activity as it is passed to + // the transcript, but still missing inside the flow + Assert.assertNull(context.getActivity().getFrom().getRole()); + conversationId[0] = context.getActivity().getConversation().getId(); + context.sendActivity("echo:" + context.getActivity().getText()).join(); + return CompletableFuture.completedFuture(null); + }) + .send("test") + .startTest().join(); + + PagedResult pagedResult = transcriptStore.getTranscriptActivities("test", conversationId[0]).join(); + Assert.assertEquals(2, pagedResult.getItems().size()); + Assert.assertNotNull(pagedResult.getItems().get(0).getFrom()); + Assert.assertEquals(RoleTypes.USER, pagedResult.getItems().get(0).getFrom().getRole()); + Assert.assertNotNull(pagedResult.getItems().get(1).getFrom()); + Assert.assertEquals(RoleTypes.BOT, pagedResult.getItems().get(1).getFrom().getRole()); + + System.out.printf("Complete"); + } }