Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

make sure Domain.fromDict(domain) doesn't change its input domain #5794

Merged
merged 4 commits into from
May 14, 2020
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions changelog/5794.improvement.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Creating a ``Domain`` using ``Domain.fromDict`` can no longer alter the input dictionary.
chkoss marked this conversation as resolved.
Show resolved Hide resolved
Previously, there could be problems when the input dictionary was re-used for other
things after creating the ``Domain`` from it.
5 changes: 5 additions & 0 deletions rasa/core/domain.py
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ def from_yaml(cls, yaml: Text) -> "Domain":

@classmethod
def from_dict(cls, data: Dict) -> "Domain":
data = copy.deepcopy(data)
utter_templates = cls.collect_templates(data.get("responses", {}))
if "templates" in data:
raise_warning(
Expand Down Expand Up @@ -259,6 +260,8 @@ def collect_slots(slot_dict: Dict[Text, Any]) -> List[Slot]:
# it is super important to sort the slots here!!!
# otherwise state ordering is not consistent
slots = []
# make a copy to not alter the input dictionary
chkoss marked this conversation as resolved.
Show resolved Hide resolved
slot_dict = copy.deepcopy(slot_dict)
for slot_name in sorted(slot_dict):
slot_class = Slot.resolve_by_type(slot_dict[slot_name].get("type"))
if "type" in slot_dict[slot_name]:
Expand Down Expand Up @@ -332,6 +335,8 @@ def collect_intent_properties(
Returns:
The intent properties to be stored in the domain.
"""
# make a copy to not alter the input argument
intents = copy.deepcopy(intents)
intent_properties = {}
duplicates = set()
for intent in intents:
Expand Down
10 changes: 10 additions & 0 deletions tests/core/test_domain.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import copy
import json
from pathlib import Path

Expand Down Expand Up @@ -802,3 +803,12 @@ def test_domain_utterance_actions_deprecated_templates():
old_domain = Domain.from_yaml(old_yaml)
new_domain = Domain.from_yaml(new_yaml)
assert hash(old_domain) == hash(new_domain)


def test_domain_from_dict_does_not_change_input():
wochinge marked this conversation as resolved.
Show resolved Hide resolved
path = DEFAULT_DOMAIN_PATH_WITH_SLOTS
input_before = io_utils.read_yaml(io_utils.read_file(path))
chkoss marked this conversation as resolved.
Show resolved Hide resolved
input_after = copy.deepcopy(input_before)

Domain.from_dict(input_after)
assert input_after == input_before