derrickaw commented on code in PR #38130:
URL: https://github.com/apache/beam/pull/38130#discussion_r3093228559


##########
sdks/python/apache_beam/yaml/yaml_mapping.py:
##########
@@ -210,78 +225,106 @@ def py_value_to_js_dict(py_value):
 def _expand_javascript_mapping_func(
     original_fields, expression=None, callable=None, path=None, name=None):
 
-  # Check for installed js2py package
-  if js2py is None:
+  # Check for installed quickjs package
+  if quickjs is None:
     raise ValueError(
-        "Javascript mapping functions are not supported on"
-        " Python 3.12 or later.")
-
-  # import remaining js2py objects
-  from js2py import base
-  from js2py.constructors import jsdate
-  from js2py.internals import simplex
-
-  js_array_type = (
-      base.PyJsArray,
-      base.PyJsArrayBuffer,
-      base.PyJsInt8Array,
-      base.PyJsUint8Array,
-      base.PyJsUint8ClampedArray,
-      base.PyJsInt16Array,
-      base.PyJsUint16Array,
-      base.PyJsInt32Array,
-      base.PyJsUint32Array,
-      base.PyJsFloat32Array,
-      base.PyJsFloat64Array)
-
-  def _js_object_to_py_object(obj):
-    if isinstance(obj, (base.PyJsNumber, base.PyJsString, base.PyJsBoolean)):
-      return base.to_python(obj)
-    elif isinstance(obj, js_array_type):
-      return [_js_object_to_py_object(value) for value in obj.to_list()]
-    elif isinstance(obj, jsdate.PyJsDate):
-      return obj.to_utc_dt()
-    elif isinstance(obj, (base.PyJsNull, base.PyJsUndefined)):
-      return None
-    elif isinstance(obj, base.PyJsError):
-      raise RuntimeError(obj['message'])
-    elif isinstance(obj, base.PyJsObject):
-      return {
-          key: _js_object_to_py_object(value['value'])
-          for (key, value) in obj.own.items()
-      }
-    elif isinstance(obj, base.JsObjectWrapper):
-      return _js_object_to_py_object(obj._obj)
+        "Javascript mapping functions require the 'quickjs' package.")
 
-    return obj
+  def make_bridge_source(func_name, call_expr):
+    keys_json = json.dumps(list(original_fields))
+    return (
+        f"function {func_name}(serialized_flags, ...values) {{ "
+        f"  const keys = JSON.parse('{keys_json}'); "
+        f"  const flags = serialized_flags.split(','); "
+        f"  const row = {{}}; "
+        f"  for (let i = 0; i < keys.length; i++) {{ "
+        f"    let val = values[i]; "
+        f"    if (flags[i] === '1') val = JSON.parse(val); "
+        f"    row[keys[i]] = val; "
+        f"  }} "
+        f"  const result = {call_expr}; "
+        f"  return (typeof result === 'object' && result !== null) "
+        f"? '__json__:' + JSON.stringify(result) : result; "
+        f"}}")
 
   if expression:
-    source = '\n'.join(['function(__row__) {'] + [
-        f'  {name} = __row__.{name}'
-        for name in original_fields if name in expression
-    ] + ['  return (' + expression + ')'] + ['}'])
-    js_func = _CustomJsObjectWrapper(js2py.eval_js(source))
+    args = [
+        name for name in original_fields
+        if name.isidentifier() and name in expression
+    ]
+    parses = []
+    for i, arg in enumerate(args):
+      parses.append(f"  if (flags[{i}] === '1') {arg} = JSON.parse({arg});")
+
+    source = f"""
+function fn(serialized_flags, {", ".join(args)}) {{
+  const flags = serialized_flags.split(',');
+{chr(10).join(parses)}
+  const result = ({expression});
+  return (typeof result === 'object' && result !== null) ?
+    "__json__:" + JSON.stringify(result) : result;
+}}
+"""
+    js_func = _QuickJsCallable(source, "fn")
+    used_fields = args
 
   elif callable:
-    js_func = _CustomJsObjectWrapper(js2py.eval_js(callable))
+    source = make_bridge_source("fn", f"({callable})(row)")
+    js_func = _QuickJsCallable(source, "fn")
+    used_fields = None
 
   else:
     if not path.endswith('.js'):
       raise ValueError(f'File "{path}" is not a valid .js file.')
     udf_code = FileSystems.open(path).read().decode()
-    js = js2py.EvalJs()
-    js.eval(udf_code)
-    js_func = _CustomJsObjectWrapper(getattr(js, name))
+    bridge_source = udf_code + "\n" + make_bridge_source(
+        "bridge_fn", f"{name}(row)")
+    js_func = _QuickJsCallable(bridge_source, "bridge_fn")
+    used_fields = None
 
   def js_wrapper(row):
-    row_as_dict = py_value_to_js_dict(row)
-    try:
-      js_result = js_func(row_as_dict)
-    except simplex.JsException as exn:
-      raise RuntimeError(
-          f"Error evaluating javascript expression: "
-          f"{exn.mes['message']}") from exn
-    return dicts_to_rows(_js_object_to_py_object(js_result))
+    if expression:
+      vals = [py_value_to_js_dict(getattr(row, name)) for name in used_fields]
+      js_vals = []
+      flags = []
+      for val in vals:
+        if isinstance(val, (list, dict)):
+          js_vals.append(json.dumps(val))
+          flags.append('1')
+        else:
+          js_vals.append(val)
+          flags.append('0')
+
+      flags_str = ",".join(flags)
+      try:
+        js_result = js_func(flags_str, *js_vals)
+      except Exception as exn:
+        raise RuntimeError(
+            f"Error evaluating javascript expression: {exn}") from exn
+    else:
+      row_as_dict = py_value_to_js_dict(row)
+      js_vals = []
+      flags = []
+      for name in original_fields:
+        val = row_as_dict.get(name)
+        if isinstance(val, (list, dict)):
+          js_vals.append(json.dumps(val))
+          flags.append('1')
+        else:
+          js_vals.append(val)
+          flags.append('0')
+
+      flags_str = ",".join(flags)
+      try:
+        js_result = js_func(flags_str, *js_vals)
+      except Exception as exn:
+        raise RuntimeError(
+            f"Error evaluating javascript expression: {exn}") from exn

Review Comment:
   good point, done



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to