Skip to content

Commit

Permalink
Merge pull request #3918 from microsoft/ccastro/unique-dialogset-ref
Browse files Browse the repository at this point in the history
DialogSet.Add: Unique dialog ids for name collisions but not for reference collisions
  • Loading branch information
carlosscastro authored May 13, 2020
2 parents e503349 + 5d7205d commit 300cf7a
Show file tree
Hide file tree
Showing 10 changed files with 396 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,17 @@ public bool OnBeforeLoadToken<T>(JToken token, out T result)
var hashCode = Hash<T>(token);

// Pass 1: We track the already visited objects' hash in the 'visited' collection
// If we've already visited this hash code and we are still in pass 1,
// we found a loop! If we found a loop, we want to return a value and stop deserializing
// to avoid infinite loops.
// If we've already visited this hash code and we are still in pass 1, no need to revisit.
if (visitedPassOne.Contains(hashCode) && CycleDetectionPass == CycleDetectionPasses.PassOne)
{
// If we already have a hydrated object for this hash code, return it
// If we already have a hydrated object for this hash code,
// there is no cycle, just repetition of the object in different parts of the object tree.
// We can get it from the cache but we don't necessarily want it to be the same reference.
// In pass 2 when we connect cycles, we won't actually clone because at that point we do want
// to conserve references.
if (cache.ContainsKey(hashCode))
{
result = cache[hashCode] as T;
result = ObjectPath.Clone<T>(cache[hashCode] as T);
}

// If we don't have a cached value for this hash code, we send null as the value.
Expand Down
9 changes: 9 additions & 0 deletions libraries/Microsoft.Bot.Builder.Dialogs/DialogSet.cs
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,15 @@ public DialogSet Add(Dialog dialog)

if (_dialogs.ContainsKey(dialog.Id))
{
// If we are trying to add the same exact instance, it's not a name collision.
// No operation required since the instance is already in the dialog set.
if (_dialogs[dialog.Id] == dialog)
{
return this;
}

// If we are adding a new dialog with a conflicting name, add a suffix to avoid
// dialog name collisions.
var nextSuffix = 2;

while (true)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,12 @@ public async Task Action_CancelAllDialogs()
await TestUtils.RunTestScript(ResourceExplorer);
}

[TestMethod]
public async Task Action_CancelAllDialogs_DoubleCancel()
{
await TestUtils.RunTestScript(ResourceExplorer);
}

[TestMethod]
public async Task Action_ChoiceInput()
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
{
"$schema": "../../../tests.schema",
"$kind": "Microsoft.Test.Script",
"dialog": {
"$kind": "Microsoft.AdaptiveDialog",
"recognizer": {
"$kind": "Microsoft.RegexRecognizer",
"intents": [
{
"intent": "CancelIntent",
"pattern": "(?i)cancel|never mind"
},
{
"intent": "RestartIntent",
"pattern": "(?i)restart"
}
]
},
"triggers": [
{
"$kind": "Microsoft.OnBeginDialog",
"actions": [
{
"$kind": "Microsoft.AdaptiveDialog",
"recognizer": {
"$kind": "Microsoft.RegexRecognizer",
"intents": [
{
"intent": "CancelIntent",
"pattern": "(?i)cancel|never mind"
},
{
"intent": "RestartIntent",
"pattern": "(?i)restart"
}
]
},
"triggers": [
{
"$kind": "Microsoft.OnBeginDialog",
"actions": [
{
"$kind": "Microsoft.SendActivity",
"activity": "Why did the chicken cross the road?"
},
{
"$kind": "Microsoft.EndTurn"
},
{
"$kind": "Microsoft.SendActivity",
"activity": "To get to the other side"
}
]
},
{
"$kind": "Microsoft.OnIntent",
"intent": "CancelIntent",
"actions": [
{
"$kind": "Microsoft.SendActivity",
"activity": "Canceled"
},
{
"$kind": "Microsoft.SetProperty",
"property": "$disabled",
"value": "=false"
},
{
"$kind": "Microsoft.CancelAllDialogs",
"disabled": "=$disabled"
},
{
"$kind": "Microsoft.SendActivity",
"activity": "Not cancelled"
}
]
},
{
"$kind": "Microsoft.OnIntent",
"intent": "RestartIntent",
"actions": [
{
"$kind": "Microsoft.SendActivity",
"activity": "Restarted"
},
{
"$kind": "Microsoft.SetProperty",
"property": "$disabled",
"value": "=true"
},
{
"$kind": "Microsoft.CancelAllDialogs",
"disabled": "=$disabled"
},
{
"$kind": "Microsoft.SendActivity",
"activity": "Not cancelled"
}
]
}
]
},
{
"$kind": "Microsoft.SendActivity",
"activity": "What am I doing here?"
}
]
}
]
},
"script": [
{
"$kind": "Microsoft.Test.UserSays",
"text": "hi"
},
{
"$kind": "Microsoft.Test.AssertReply",
"text": "Why did the chicken cross the road?"
},
{
"$kind": "Microsoft.Test.UserSays",
"text": "cancel"
},
{
"$kind": "Microsoft.Test.AssertReply",
"text": "Canceled"
},
{
"$kind": "Microsoft.Test.UserSays",
"text": "hi"
},
{
"$kind": "Microsoft.Test.AssertReply",
"text": "Why did the chicken cross the road?"
},
{
"$kind": "Microsoft.Test.UserSays",
"text": "restart"
},
{
"$kind": "Microsoft.Test.AssertReply",
"text": "Restarted"
},
{
"$kind": "Microsoft.Test.AssertReply",
"text": "Not cancelled"
},
]
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,17 @@ public static void ClassCleanup()
resourceExplorer.Dispose();
}

[TestMethod]
public async Task JsonDialogLoad_DoubleReference()
{
await BuildTestFlow(@"DoubleReference.dialog")
.SendConversationUpdate()
.AssertReply("what is your name?")
.Send("c")
.AssertReply("sub0")
.StartTestAsync();
}

[TestMethod]
public async Task JsonDialogLoad_CycleDetection()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@

<ItemGroup>
<Folder Include="CustomDialogs\" />
<Folder Include="Samples\DoubleReference\" />
<Folder Include="wwwroot\" />
</ItemGroup>

Expand All @@ -56,6 +57,21 @@
<ProjectReference Include="..\..\libraries\Microsoft.Bot.Connector\Microsoft.Bot.Connector.csproj" />
</ItemGroup>

<ItemGroup>
<Content Update="Samples\DoubleReference\DoubleReference.dialog">
<CopyToOutputDirectory>Always</CopyToOutputDirectory>
</Content>
<Content Update="Samples\DoubleReference\sub0.dialog">
<CopyToOutputDirectory>Always</CopyToOutputDirectory>
</Content>
<Content Update="Samples\DoubleReference\sub2.dialog">
<CopyToOutputDirectory>Always</CopyToOutputDirectory>
</Content>
<Content Update="Samples\DoubleReference\sub3.dialog">
<CopyToOutputDirectory>Always</CopyToOutputDirectory>
</Content>
</ItemGroup>

<ItemGroup>
<None Update="Samples\CycleDetection\Child1.dialog">
<CopyToOutputDirectory>Always</CopyToOutputDirectory>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
{
"$schema": "../../testbot.schema",
"$kind": "Microsoft.AdaptiveDialog",
"recognizer": {
"$kind": "Microsoft.RegexRecognizer",
"id": "x",
"intents": [
{
"intent": "aIntent",
"pattern": "a"
},
{
"intent": "bIntent",
"pattern": "b"
},
{
"intent": "cIntent",
"pattern": "c"
},
{
"intent": "dIntent",
"pattern": "d"
},
{
"intent": "eIntent",
"pattern": "e"
},
{
"intent": "fIntent",
"pattern": "f"
}
]
},
"triggers": [
{
"$kind": "Microsoft.OnBeginDialog",
"actions": [
{
"$kind": "Microsoft.BeginDialog",
"dialog": "AskForName",
"resultProperty": "$name"
},
{
"$kind": "Microsoft.SendActivity",
"activity": "Hello ${dialog.name}, nice to meet you!"
}
]
},
{
"$kind": "Microsoft.OnIntent",
"intent": "aIntent",
"actions": [
{
"$kind": "Microsoft.BeginDialog",
"dialog": "sub3"
}
]
},
{
"$kind": "Microsoft.OnIntent",
"intent": "bIntent",
"actions": [
{
"$kind": "Microsoft.BeginDialog",
"dialog": "sub2"
}
]
},
{
"$kind": "Microsoft.OnIntent",
"intent": "cIntent",
"actions": [
{
"$kind": "Microsoft.BeginDialog",
"dialog": "sub0"
}
]
},
{
"$kind": "Microsoft.OnIntent",
"intent": "dIntent",
"actions": [
{
"$kind": "Microsoft.BeginDialog",
"dialog": "sub0"
}
]
},
{
"$kind": "Microsoft.OnIntent",
"intent": "eIntent",
"actions": [
{
"$kind": "Microsoft.BeginDialog",
"dialog": "sub3"
}
]
},
{
"$kind": "Microsoft.OnIntent",
"intent": "fIntent",
"actions": [
{
"$kind": "Microsoft.BeginDialog",
"dialog": "sub2"
}
]
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
{
"$schema": "../../testbot.schema",
"$kind": "Microsoft.AdaptiveDialog",
"id": "sub0",
"triggers": [
{
"$kind": "Microsoft.OnBeginDialog",
"actions": [
{
"$kind": "Microsoft.SendActivity",
"activity": "sub0"
}
]
}
]
}
Loading

0 comments on commit 300cf7a

Please sign in to comment.