Skip to content

Commit

Permalink
fix: Importing inputs from other inputs (#27041)
Browse files Browse the repository at this point in the history
Co-authored-by: Marius Andra <[email protected]>
  • Loading branch information
benjackwhite and mariusandra authored Dec 20, 2024
1 parent 67dfe8d commit 198678d
Show file tree
Hide file tree
Showing 9 changed files with 305 additions and 41 deletions.
4 changes: 3 additions & 1 deletion ee/api/test/test_hooks.py
Original file line number Diff line number Diff line change
Expand Up @@ -139,8 +139,9 @@ def test_create_hog_function_via_hook(self):
"target": "https://hooks.zapier.com/{inputs.hook}",
},
},
"order": 2,
},
"debug": {},
"debug": {"order": 1},
"hook": {
"bytecode": [
"_H",
Expand All @@ -149,6 +150,7 @@ def test_create_hog_function_via_hook(self):
"hooks/standard/1234/abcd",
],
"value": "hooks/standard/1234/abcd",
"order": 0,
},
}

Expand Down
40 changes: 22 additions & 18 deletions plugin-server/src/cdp/hog-executor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { status } from '../utils/status'
import { HogFunctionManager } from './hog-function-manager'
import {
CyclotronFetchFailureInfo,
HogFunctionInputType,
HogFunctionInvocation,
HogFunctionInvocationGlobals,
HogFunctionInvocationGlobalsWithInputs,
Expand Down Expand Up @@ -103,6 +104,16 @@ const sanitizeLogMessage = (args: any[], sensitiveValues?: string[]): string =>
return message
}

const orderInputsByDependency = (hogFunction: HogFunctionType): [string, HogFunctionInputType][] => {
const allInputs: HogFunctionType['inputs'] = {
...hogFunction.inputs,
...hogFunction.encrypted_inputs,
}
return Object.entries(allInputs).sort(([_, input1], [__, input2]) => {
return (input1.order ?? -1) - (input2.order ?? -1)
})
}

export class HogExecutor {
private telemetryMatcher: ValueMatcher<number>

Expand Down Expand Up @@ -529,30 +540,23 @@ export class HogExecutor {
}

buildHogFunctionGlobals(invocation: HogFunctionInvocation): HogFunctionInvocationGlobalsWithInputs {
const builtInputs: Record<string, any> = {}
const newGlobals: HogFunctionInvocationGlobalsWithInputs = {
...invocation.globals,
inputs: {},
}

Object.entries(invocation.hogFunction.inputs ?? {}).forEach(([key, item]) => {
builtInputs[key] = item.value
const orderedInputs = orderInputsByDependency(invocation.hogFunction)

if (item.bytecode) {
// Use the bytecode to compile the field
builtInputs[key] = formatInput(item.bytecode, invocation.globals, key)
}
})

Object.entries(invocation.hogFunction.encrypted_inputs ?? {}).forEach(([key, item]) => {
builtInputs[key] = item.value
for (const [key, input] of orderedInputs) {
newGlobals.inputs[key] = input.value

if (item.bytecode) {
if (input.bytecode) {
// Use the bytecode to compile the field
builtInputs[key] = formatInput(item.bytecode, invocation.globals, key)
newGlobals.inputs[key] = formatInput(input.bytecode, newGlobals, key)
}
})

return {
...invocation.globals,
inputs: builtInputs,
}

return newGlobals
}

getSensitiveValues(hogFunction: HogFunctionType, inputs: Record<string, any>): string[] {
Expand Down
1 change: 1 addition & 0 deletions plugin-server/src/cdp/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,7 @@ export type HogFunctionInputType = {
value: any
secret?: boolean
bytecode?: HogBytecode | object
order?: number
}

export type IntegrationType = {
Expand Down
2 changes: 1 addition & 1 deletion posthog/api/hog_function.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ class HogFunctionMaskingSerializer(serializers.Serializer):
bytecode = serializers.JSONField(required=False, allow_null=True)

def validate(self, attrs):
attrs["bytecode"] = generate_template_bytecode(attrs["hash"])
attrs["bytecode"] = generate_template_bytecode(attrs["hash"], input_collector=set())

return super().validate(attrs)

Expand Down
9 changes: 7 additions & 2 deletions posthog/api/test/test_hog_function.py
Original file line number Diff line number Diff line change
Expand Up @@ -479,14 +479,15 @@ def test_secret_inputs_not_returned(self, *args):
"I AM SECRET",
],
"value": "I AM SECRET",
"order": 0,
},
}

raw_encrypted_inputs = get_db_field_value("encrypted_inputs", obj.id)

assert (
raw_encrypted_inputs
== "gAAAAABlkgC8AAAAAAAAAAAAAAAAAAAAAKvzDjuLG689YjjVhmmbXAtZSRoucXuT8VtokVrCotIx3ttPcVufoVt76dyr2phbuotMldKMVv_Y6uzMDZFjX1WLE6eeZEhBJqFv8fQacoHXhDbDh5fvL7DTr1sc2R_DmTwvPQDiSss790vZ6d_vm1Q="
== "gAAAAABlkgC8AAAAAAAAAAAAAAAAAAAAAKvzDjuLG689YjjVhmmbXAtZSRoucXuT8VtokVrCotIx3ttPcVufoVt76dyr2phbuotMldKMVv_Y6uzMDZFjX1Uvej4GHsYRbsTN_txcQHNnU7zvLee83DhHIrThEjceoq8i7hbfKrvqjEi7GCGc_k_Gi3V5KFxDOfLKnke4KM4s"
)

def test_secret_inputs_not_updated_if_not_changed(self, *args):
Expand Down Expand Up @@ -642,6 +643,7 @@ def test_generates_inputs_bytecode(self, *args):
32,
"http://localhost:2080/0e02d917-563f-4050-9725-aad881b69937",
],
"order": 0,
},
"payload": {
"value": {
Expand All @@ -651,6 +653,7 @@ def test_generates_inputs_bytecode(self, *args):
"person": "{person}",
"event_url": "{f'{event.url}-test'}",
},
"order": 1,
"bytecode": {
"event": ["_H", HOGQL_BYTECODE_VERSION, 32, "event", 1, 1],
"groups": ["_H", HOGQL_BYTECODE_VERSION, 32, "groups", 1, 1],
Expand All @@ -673,7 +676,7 @@ def test_generates_inputs_bytecode(self, *args):
],
},
},
"method": {"value": "POST"},
"method": {"value": "POST", "order": 2},
"headers": {
"value": {"version": "v={event.properties.$lib_version}"},
"bytecode": {
Expand All @@ -695,6 +698,7 @@ def test_generates_inputs_bytecode(self, *args):
2,
]
},
"order": 3,
},
}

Expand Down Expand Up @@ -1164,6 +1168,7 @@ def test_create_typescript_destination_with_inputs(self):
inputs["message"]["transpiled"]["stl"].sort()
assert result["inputs"] == {
"message": {
"order": 0,
"transpiled": {
"code": 'concat("Hello, TypeScript ", arrayMap(__lambda((a) => a), [1, 2, 3]), "!")',
"lang": "ts",
Expand Down
5 changes: 3 additions & 2 deletions posthog/cdp/site_functions.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,9 @@ def get_transpiled_function(hog_function: HogFunction) -> str:

compiler = JavaScriptCompiler()

# TODO: reorder inputs to make dependencies work
for key, input in (hog_function.inputs or {}).items():
all_inputs = hog_function.inputs or {}
all_inputs = sorted(all_inputs.items(), key=lambda x: x[1].get("order", -1))
for key, input in all_inputs:
value = input.get("value")
key_string = json.dumps(str(key) or "<empty>")
if (isinstance(value, str) and "{" in value) or isinstance(value, dict) or isinstance(value, list):
Expand Down
59 changes: 54 additions & 5 deletions posthog/cdp/test/test_site_functions.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,12 @@ def compile_and_run(self):

return result

def _execute_javascript(self, js) -> str:
with tempfile.NamedTemporaryFile(delete=False) as f:
f.write(js.encode("utf-8"))
f.flush()
return subprocess.check_output(["node", f.name]).decode("utf-8")

def test_get_transpiled_function_basic(self):
result = self.compile_and_run()
assert isinstance(result, str)
Expand Down Expand Up @@ -343,8 +349,51 @@ def test_run_function_onevent(self):
)
assert "Loaded" == response.strip()

def _execute_javascript(self, js) -> str:
with tempfile.NamedTemporaryFile(delete=False) as f:
f.write(js.encode("utf-8"))
f.flush()
return subprocess.check_output(["node", f.name]).decode("utf-8")
def test_get_transpiled_function_with_ordered_inputs(self):
self.hog_function.hog = "export function onLoad() { console.log(inputs); }"
self.hog_function.inputs = {
"first": {"value": "I am first", "order": 0},
"second": {"value": "{person.properties.name}", "order": 1},
"third": {"value": "{event.properties.url}", "order": 2},
}

result = self.compile_and_run()

assert '"first": "I am first"' in result
idx_first = result.index('"first": "I am first"')
idx_second = result.index('inputs["second"] = getInputsKey("second");')
idx_third = result.index('inputs["third"] = getInputsKey("third");')

assert idx_first < idx_second < idx_third

def test_get_transpiled_function_without_order(self):
self.hog_function.hog = "export function onLoad() { console.log(inputs); }"
self.hog_function.inputs = {
"noOrder": {"value": "I have no order"},
"alsoNoOrder": {"value": "{person.properties.name}"},
"withOrder": {"value": "{event.properties.url}", "order": 10},
}

result = self.compile_and_run()

idx_noOrder = result.index('"noOrder": "I have no order"')
idx_alsoNoOrder = result.index('inputs["alsoNoOrder"] = getInputsKey("alsoNoOrder");')
idx_withOrder = result.index('inputs["withOrder"] = getInputsKey("withOrder");')

assert idx_noOrder < idx_alsoNoOrder < idx_withOrder

def test_get_transpiled_function_with_duplicate_orders(self):
self.hog_function.hog = "export function onLoad() { console.log(inputs); }"
self.hog_function.inputs = {
"alpha": {"value": "{person.properties.alpha}", "order": 1},
"beta": {"value": "{person.properties.beta}", "order": 1},
"gamma": {"value": "Just gamma", "order": 1},
}

result = self.compile_and_run()

idx_alpha = result.index('inputs["alpha"] = getInputsKey("alpha");')
idx_beta = result.index('inputs["beta"] = getInputsKey("beta");')
idx_gamma = result.index('"gamma": "Just gamma"')

assert idx_alpha is not None and idx_beta is not None and idx_gamma is not None
111 changes: 110 additions & 1 deletion posthog/cdp/test/test_validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ def test_validate_inputs(self):
32,
"http://localhost:2080/0e02d917-563f-4050-9725-aad881b69937",
],
"order": 0, # Now that we have ordering, url should have some order assigned
},
"payload": {
"value": {
Expand Down Expand Up @@ -115,8 +116,12 @@ def test_validate_inputs(self):
2,
],
},
"order": 1,
},
"method": {
"value": "POST",
"order": 2,
},
"method": {"value": "POST"},
"headers": {
"value": {"version": "v={event.properties.$lib_version}"},
"bytecode": {
Expand All @@ -138,6 +143,7 @@ def test_validate_inputs(self):
2,
]
},
"order": 3,
},
}
)
Expand Down Expand Up @@ -180,6 +186,109 @@ def test_validate_inputs_creates_bytecode_for_html(self):
3,
],
"value": '<html>\n<head>\n<style type="text/css">\n .css \\{\n width: 500px !important;\n }</style>\n</head>\n\n<body>\n <p>Hi {person.properties.email}</p>\n</body>\n</html>',
"order": 0,
},
}
)

# New tests for ordering
def test_validate_inputs_with_dependencies_simple_chain(self):
# Schema: A->B->C
# A has no deps, B uses A, C uses B
inputs_schema = [
{"key": "A", "type": "string", "required": True},
{"key": "C", "type": "string", "required": True},
{"key": "B", "type": "string", "required": True},
]
# Values: B depends on A, C depends on B
# We'll use templates referencing inputs.A, inputs.B
inputs = {
"A": {"value": "A value"},
"C": {"value": "{inputs.B} + C value"},
"B": {"value": "{inputs.A} + B value"},
}

validated = validate_inputs(inputs_schema, inputs)
# Order should be A=0, B=1, C=2
assert validated["A"]["order"] == 0
assert validated["B"]["order"] == 1
assert validated["C"]["order"] == 2

def test_validate_inputs_with_multiple_dependencies(self):
# Schema: W, X, Y, Z
# Z depends on W and Y
# Y depends on X
# X depends on W
# So order: W=0, X=1, Y=2, Z=3
inputs_schema = [
{"key": "X", "type": "string", "required": True},
{"key": "W", "type": "string", "required": True},
{"key": "Z", "type": "string", "required": True},
{"key": "Y", "type": "string", "required": True},
]
inputs = {
"X": {"value": "{inputs.W}_x"},
"W": {"value": "w"},
"Z": {"value": "{inputs.W}{inputs.Y}_z"}, # depends on W and Y
"Y": {"value": "{inputs.X}_y"},
}

validated = validate_inputs(inputs_schema, inputs)
assert validated["W"]["order"] == 0
assert validated["X"]["order"] == 1
assert validated["Y"]["order"] == 2
assert validated["Z"]["order"] == 3

def test_validate_inputs_with_no_dependencies(self):
# All inputs have no references. Any order is fine but all should start from 0 and increment.
inputs_schema = [
{"key": "one", "type": "string", "required": True},
{"key": "two", "type": "string", "required": True},
{"key": "three", "type": "string", "required": True},
]
inputs = {
"one": {"value": "1"},
"two": {"value": "2"},
"three": {"value": "3"},
}

validated = validate_inputs(inputs_schema, inputs)
# Should just assign order in any stable manner (likely alphabetical since no deps):
# Typically: one=0, two=1, three=2
# The actual order might depend on dictionary ordering, but given code, it should be alphabetical keys since we topologically sort by dependencies.
assert validated["one"]["order"] == 0
assert validated["two"]["order"] == 1
assert validated["three"]["order"] == 2

def test_validate_inputs_with_circular_dependencies(self):
# A depends on B, B depends on A -> should fail
inputs_schema = [
{"key": "A", "type": "string", "required": True},
{"key": "B", "type": "string", "required": True},
]

inputs = {
"A": {"value": "{inputs.B} + A"},
"B": {"value": "{inputs.A} + B"},
}

try:
validate_inputs(inputs_schema, inputs)
raise AssertionError("Expected circular dependency error")
except Exception as e:
assert "Circular dependency" in str(e)

def test_validate_inputs_with_extraneous_dependencies(self):
# A depends on a non-existing input X
# This should ignore X since it's not defined.
# So no error, but A has no real dependencies that matter.
inputs_schema = [
{"key": "A", "type": "string", "required": True},
]
inputs = {
"A": {"value": "{inputs.X} + A"},
}

validated = validate_inputs(inputs_schema, inputs)
# Only A is present, so A=0
assert validated["A"]["order"] == 0
Loading

0 comments on commit 198678d

Please sign in to comment.