Skip to content

Commit

Permalink
fix: UI input with escaped references
Browse files Browse the repository at this point in the history
  • Loading branch information
danielgrittner committed Sep 22, 2024
1 parent 7dcf3b2 commit a2f713c
Show file tree
Hide file tree
Showing 5 changed files with 152 additions and 23 deletions.
36 changes: 26 additions & 10 deletions admyral/editor/json_with_references_serde.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,14 @@
"""


def _unescape_string(value: str) -> str:
return value.encode("utf-8").decode("unicode_escape")


def _escape_string(value: str) -> str:
return value.encode("unicode_escape").decode("utf-8")


def _is_float(value: str) -> bool:
try:
float(value)
Expand Down Expand Up @@ -71,8 +79,8 @@ def serialize_json_with_reference(value: JsonValue) -> str:
if isinstance(value, str):
# handle string escaping for ints, floats, bools, dicts, and lists
if _is_string_escaped_json_value(value):
return f'"{value}"'
return value
return f'"{_escape_string(value)}"'
return _escape_string(value)

if isinstance(value, list):
content = ", ".join(_handle_value_inside_container(item) for item in value)
Expand All @@ -90,6 +98,7 @@ def serialize_json_with_reference(value: JsonValue) -> str:

def deserialize_json_with_reference(value: str) -> JsonValue:
value = value.strip().strip("\n")
value = _unescape_string(value)

if value == "":
return None
Expand All @@ -98,7 +107,6 @@ def deserialize_json_with_reference(value: str) -> JsonValue:
if (
value.startswith('"')
and value.endswith('"')
and _is_string_escaped_json_value(value[1:-1])
):
return value[1:-1]

Expand Down Expand Up @@ -180,12 +188,16 @@ def deserialize_json_with_reference(value: str) -> JsonValue:
for match in REFERENCE_REGEX.finditer(value):
start, end = match.span()
if is_within_string[start]:
# Already within a quote
continue

replacements.append(
(start, end, f'"{match.group().replace('\\"', '\"').replace('\"', '\\"')}"')
)
# Already within a quote, however, we must still replace "
replacements.append(
(start, end, match.group().replace('\\"', '\"').replace('\"', '\\"'))
)
else:
# We need to wrap the reference within quotes because it is currently not
# within quotes
replacements.append(
(start, end, f'"{match.group().replace('\\"', '\"').replace('\"', '\\"')}"')
)

# Wrap references into qutoes which are not yet within quotes
# We can't simply just use regex replace because a reference might be used multiple times
Expand All @@ -201,4 +213,8 @@ def deserialize_json_with_reference(value: str) -> JsonValue:

out = "".join(reversed(out))

return json.loads(out)
try:
return json.loads(out)
except json.JSONDecodeError:
# normal string
return value
5 changes: 2 additions & 3 deletions admyral/workers/references.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ def _resolve_access_path(action_outputs: dict, input: str) -> JsonValue:

access_path = input.lstrip("{{").rstrip("}}").strip()
if len(access_path) == 0:
raise ValueError("Invalid reference: Access path is empty")
raise AdmyralFailureError(message="Invalid reference: Access path is empty.")
current_value = action_outputs

# access the base variable and check if it exists
Expand All @@ -40,7 +40,6 @@ def _resolve_access_path(action_outputs: dict, input: str) -> JsonValue:
if current_value is None:
return None

# TODO: this needs to change
for key in ACCESS_PATH_REGEX.finditer(access_path):
raw_value = key.group()

Expand Down Expand Up @@ -115,4 +114,4 @@ def evaluate_references(value: JsonValue, execution_state: dict) -> JsonValue:
if isinstance(value, list):
return [evaluate_references(val, execution_state) for val in value]

raise ValueError(f"Unsupported type: {type(value)}")
raise AdmyralFailureError(message=f"Unsupported type: {type(value).__name__}")
66 changes: 60 additions & 6 deletions tests/editor/test_json_with_references_serde.py
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ def test_serde_pure_reference2():
json_str = '"something {{ a[\'b\'][0][\\"c\\"] }} else"'
assert (
deserialize_json_with_reference(json_str)
== '"something {{ a[\'b\'][0][\\"c\\"] }} else"'
== "something {{ a['b'][0][\"c\"] }} else"
)


Expand All @@ -209,7 +209,7 @@ def test_serde_pure_reference3():
json_str = '"something {{ a[\'b\'][0]["c"] }} else"'
assert (
deserialize_json_with_reference(json_str)
== '"something {{ a[\'b\'][0]["c"] }} else"'
== "something {{ a['b'][0][\"c\"] }} else"
)


Expand All @@ -224,11 +224,11 @@ def test_escaped_string_serialization():
```
"""
input_json_obj = '"something before and after"'
json_str = serialize_json_with_reference(input_json_obj)
json_obj = deserialize_json_with_reference(json_str)
json_obj = deserialize_json_with_reference(input_json_obj)
json_str = serialize_json_with_reference(json_obj)

assert json_str == '"something before and after"'
assert input_json_obj == json_obj
assert json_obj == "something before and after"
assert json_str == "something before and after"


#########################################################################################################
Expand Down Expand Up @@ -279,3 +279,57 @@ def test_empty_string_serialization():

assert json_obj is None
assert json_str == "null"


#########################################################################################################


def test_invalid_list():
"""
Textfield Input in UI:
```
[\n"]
```
"""
input_value = '[\\n"]'
json_obj = deserialize_json_with_reference(input_value)
json_str = serialize_json_with_reference(json_obj)

assert json_obj == '[\n"]'
assert json_str == '[\\n"]'


#########################################################################################################


def test_invalid_list_in_string():
"""
Textfield Input in UI:
```
"[\n]"
```
"""
input_value = '"[\\n"]"'
json_obj = deserialize_json_with_reference(input_value)
json_str = serialize_json_with_reference(json_obj)

assert json_obj == '[\n"]'
assert json_str == '[\\n"]'


#########################################################################################################


def test_empty_reference():
"""
Textfield Input in UI:
```
{{}}
```
"""
input_value = "{{}}"
json_obj = deserialize_json_with_reference(input_value)
json_str = serialize_json_with_reference(json_obj)

assert json_obj == "{{}}"
assert json_str == "{{}}"
60 changes: 60 additions & 0 deletions tests/workers/test_references.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,24 +10,36 @@ def test_multiple_references_in_a_single_string():
assert evaluate_references(value, execution_state) == "abc def ghi"


#########################################################################################################


def test_deeply_nested_references():
execution_state = {"a": {"b": {"c": [{"d": ["", "", "abc"]}]}}}
value = "{{ a['b']['c'][0]['d'][2] }}"
assert evaluate_references(value, execution_state) == "abc"


#########################################################################################################


def test_non_string_reference_int():
execution_state = {"a": 123}
value = "{{ a }}"
assert evaluate_references(value, execution_state) == 123


#########################################################################################################


def test_non_string_reference_bool():
execution_state = {"a": True}
value = "{{ a }}"
assert evaluate_references(value, execution_state)


#########################################################################################################


def test_dict_with_references():
execution_state = {"a": "abc", "b": "def", "c": "ghi"}
value = {"a": "{{ a }}", "b": {"c": "{{ b }}"}}
Expand All @@ -37,6 +49,9 @@ def test_dict_with_references():
}


#########################################################################################################


def test_list_with_references():
execution_state = {"a": "abc", "b": "def", "c": "ghi"}
value = ["{{ a }}", {"b": "{{ b }}"}, "something before and after", 42, True]
Expand All @@ -49,6 +64,9 @@ def test_list_with_references():
]


#########################################################################################################


def test_key_with_list():
execution_state = {"a": ["abc", "def", "ghi"]}
value = '{{ a["abc"] }}'
Expand All @@ -60,6 +78,9 @@ def test_key_with_list():
)


#########################################################################################################


def test_key_not_in_dict():
execution_state = {"a": {"b": "def"}}
value = "{{ a['c'] }}"
Expand All @@ -68,6 +89,9 @@ def test_key_not_in_dict():
assert e.value.message == "Invalid access path: a['c']. Key 'c' not found."


#########################################################################################################


def test_invalid_int_conversion():
execution_state = {"a": ["b", "def"]}
value = "{{ a[True] }}"
Expand All @@ -79,6 +103,9 @@ def test_invalid_int_conversion():
)


#########################################################################################################


def test_invalid_list_access():
execution_state = {"a": {"b": "def"}}
value = "{{ a[1] }}"
Expand All @@ -87,6 +114,9 @@ def test_invalid_list_access():
assert e.value.message == "Invalid access path: a[1]. Expected a list, got dict."


#########################################################################################################


def test_index_out_of_bounds():
execution_state = {"a": ["abc", "def", "ghi"]}
value = "{{ a[3] }}"
Expand All @@ -95,6 +125,9 @@ def test_index_out_of_bounds():
assert e.value.message == "Invalid access path: a[3]. Index 3 out of bounds."


#########################################################################################################


def test_empty_access_path():
execution_state = {"a": "abc"}
value = "{{ a[] }}"
Expand All @@ -104,3 +137,30 @@ def test_empty_access_path():
e.value.message
== "Invalid access path segment: a[]. Must be either a string or integer."
)


#########################################################################################################


def test_empty_reference():
execution_state = {"a": "abc"}
value = "{{}}"
with pytest.raises(AdmyralFailureError) as e:
evaluate_references(value, execution_state)
assert e.value.message == "Invalid reference: Access path is empty."


#########################################################################################################


def test_evaluate_references_unsupported_type():
execution_state = {"a": {"b": "def"}}

class Dummy:
def __init__(self, value):
self.value = value

value = Dummy("abc")
with pytest.raises(AdmyralFailureError) as e:
evaluate_references(value, execution_state)
assert e.value.message == "Unsupported type: Dummy"
8 changes: 4 additions & 4 deletions tests/workers/test_wokflow_executor.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ async def test_missing_secret(store: AdmyralStore):
workflow_code=workflow_test_missing_secret,
)

assert exception is not None
assert exception is None
assert run.failed_at is not None
assert run_steps[1].error == "Secret '123' not found."

Expand Down Expand Up @@ -84,7 +84,7 @@ async def test_action_raises_error(store: AdmyralStore):
workflow_code=workflow_test_action_raises_error,
)

assert exception is not None
assert exception is None
assert run.failed_at is not None
assert run_steps[1].error == "This is an error."

Expand Down Expand Up @@ -130,7 +130,7 @@ async def test_action_missing_params(store: AdmyralStore):
workflow_code=workflow_test_action_missing_params,
)

assert exception is not None
assert exception is None
assert run.failed_at is not None
assert (
run_steps[1].error
Expand Down Expand Up @@ -167,7 +167,7 @@ async def test_missing_custom_action(store: AdmyralStore):
workflow_code=workflow_test_action_missing_custom_action,
)

assert exception is not None
assert exception is None
assert run.failed_at is not None
assert (
run_steps[1].error
Expand Down

0 comments on commit a2f713c

Please sign in to comment.