diff --git a/roboflow/adapters/rfapi.py b/roboflow/adapters/rfapi.py index e56c0308..c0b00f39 100644 --- a/roboflow/adapters/rfapi.py +++ b/roboflow/adapters/rfapi.py @@ -654,6 +654,53 @@ def delete_folder(api_key, workspace_url, group_id): # --------------------------------------------------------------------------- +_WORKFLOW_SPEC_KEYS = frozenset({"version", "inputs", "steps", "outputs"}) + + +def _normalize_workflow_config(config): + """Return a JSON string suitable for the backend's ``config`` field. + + The backend stores the ``config`` value verbatim, and the Roboflow inference + server expects to parse it to ``{"specification": {...}}`` (see + ``inference.core.roboflow_api.get_workflow_specification``). User-facing + Workflows JSON — as published in docs.roboflow.com/workflows, + ``inference/development/workflows_examples/*``, and the web UI's "View JSON" + export — is the flat shape ``{"version", "inputs", "steps", "outputs"}``. + The web app silently wraps it in ``{"specification": ...}`` before POSTing; + this helper does the same for SDK/CLI callers so that users don't need to + know the backend's storage convention. + + Behavior: + - ``None`` -> ``"{}"`` (preserves legacy "empty workflow" default). + - Anything already wrapped (``{"specification": ...}``) is passed through. + - Dicts or JSON strings that look like a bare workflow spec — i.e., contain + any of ``version``/``inputs``/``steps``/``outputs`` at the top level — + get wrapped. + - A leading UTF-8 BOM on string input is stripped before parsing AND on + the returned value, so files saved from Windows editors don't ship a + BOM to the backend (the inference server's ``json.loads`` rejects it). + - When a wrap happens, the result is serialized with compact separators + (``","``, ``":"``) to match the shape the web app writes, so audit / + diff tools don't see SDK-written and UI-written rows as different. + - Any other input is preserved as-is (stringified if needed) so callers who + intentionally send custom payloads aren't second-guessed. + """ + if config is None: + return "{}" + if isinstance(config, str): + stripped = config.lstrip("\ufeff") + try: + parsed = json.loads(stripped) + except (ValueError, TypeError): + return stripped + if isinstance(parsed, dict) and "specification" not in parsed and _WORKFLOW_SPEC_KEYS & parsed.keys(): + return json.dumps({"specification": parsed}, separators=(",", ":")) + return stripped # preserve user-supplied string when no wrap needed (BOM stripped) + if isinstance(config, dict) and "specification" not in config and _WORKFLOW_SPEC_KEYS & config.keys(): + return json.dumps({"specification": config}, separators=(",", ":")) + return json.dumps(config) + + def list_workflows(api_key, workspace_url): """GET /{ws}/workflows — list workflows.""" response = requests.get(f"{API_URL}/{workspace_url}/workflows", params={"api_key": api_key}) @@ -689,13 +736,11 @@ def create_workflow(api_key, workspace_url, *, name, url=None, config=None, temp import re url = re.sub(r"[^a-z0-9]+", "-", name.lower()).strip("-") - if config is None: - config = "{}" if template is None: template = "{}" - # config/template must be strings (the API validates with Joi.string) - if not isinstance(config, str): - config = json.dumps(config) + # config must be the backend's stored shape (`{"specification": ...}`); + # auto-wrap bare workflow definitions so docs-shaped JSON works unchanged. + config = _normalize_workflow_config(config) if not isinstance(template, str): template = json.dumps(template) params: Dict[str, str] = { @@ -724,10 +769,12 @@ def update_workflow(api_key, workspace_url, *, workflow_id, workflow_name, workf workflow_id: The workflow's internal ID. workflow_name: The workflow's display name. workflow_url: The workflow's URL slug. - config: JSON string (or dict) of the workflow config. + config: JSON string (or dict) of the workflow config. Bare workflow + definitions (``{"version", "inputs", "steps", "outputs"}``) are + auto-wrapped in ``{"specification": ...}`` to match the backend's + stored shape; see ``_normalize_workflow_config``. """ - if not isinstance(config, str): - config = json.dumps(config) + config = _normalize_workflow_config(config) payload: Dict[str, str] = { "id": workflow_id, "name": workflow_name, diff --git a/tests/adapters/test_rfapi_phase2.py b/tests/adapters/test_rfapi_phase2.py index 3537e7fe..cc205df5 100644 --- a/tests/adapters/test_rfapi_phase2.py +++ b/tests/adapters/test_rfapi_phase2.py @@ -1,8 +1,11 @@ """Unit tests for Phase 2 rfapi functions.""" +import json import unittest from unittest.mock import MagicMock, patch +from roboflow.adapters.rfapi import _normalize_workflow_config + class TestListBatches(unittest.TestCase): @patch("roboflow.adapters.rfapi.requests.get") @@ -332,6 +335,140 @@ def test_error(self, mock_post): with self.assertRaises(RoboflowError): create_workflow("key", "ws", name="Bad") + @patch("roboflow.adapters.rfapi.requests.post") + def test_bare_spec_dict_is_auto_wrapped(self, mock_post): + """Docs-shaped workflow definitions get wrapped in {"specification": ...} + so they match the backend's stored format and the inference server's + expectation. See `_normalize_workflow_config`.""" + import json as _json + + from roboflow.adapters.rfapi import create_workflow + + mock_post.return_value = MagicMock(status_code=201, json=lambda: {"workflow": {"url": "wf"}}) + bare = {"version": "1.0", "inputs": [], "steps": [], "outputs": []} + create_workflow("key", "ws", name="WF", config=bare) + sent_config = _json.loads(mock_post.call_args[1]["params"]["config"]) + self.assertEqual(sent_config, {"specification": bare}) + + @patch("roboflow.adapters.rfapi.requests.post") + def test_already_wrapped_config_is_not_double_wrapped(self, mock_post): + import json as _json + + from roboflow.adapters.rfapi import create_workflow + + mock_post.return_value = MagicMock(status_code=201, json=lambda: {"workflow": {"url": "wf"}}) + wrapped = {"specification": {"version": "1.0", "inputs": [], "steps": [], "outputs": []}} + create_workflow("key", "ws", name="WF", config=wrapped) + sent_config = _json.loads(mock_post.call_args[1]["params"]["config"]) + self.assertEqual(sent_config, wrapped) + + @patch("roboflow.adapters.rfapi.requests.post") + def test_bare_spec_json_string_is_auto_wrapped(self, mock_post): + """JSON strings are parsed, wrapped if bare, and re-serialized.""" + import json as _json + + from roboflow.adapters.rfapi import create_workflow + + mock_post.return_value = MagicMock(status_code=201, json=lambda: {"workflow": {"url": "wf"}}) + bare_str = '{"version": "1.0", "steps": []}' + create_workflow("key", "ws", name="WF", config=bare_str) + sent_config = _json.loads(mock_post.call_args[1]["params"]["config"]) + self.assertEqual(sent_config, {"specification": {"version": "1.0", "steps": []}}) + + @patch("roboflow.adapters.rfapi.requests.post") + def test_non_workflow_dict_is_not_wrapped(self, mock_post): + """Dicts that don't look like a workflow spec (no version/inputs/steps/outputs) + are passed through unchanged to avoid second-guessing custom payloads.""" + import json as _json + + from roboflow.adapters.rfapi import create_workflow + + mock_post.return_value = MagicMock(status_code=201, json=lambda: {"workflow": {"url": "wf"}}) + create_workflow("key", "ws", name="WF", config={"a": 1}) + sent_config = _json.loads(mock_post.call_args[1]["params"]["config"]) + self.assertEqual(sent_config, {"a": 1}) + + +class TestNormalizeWorkflowConfig(unittest.TestCase): + """Direct unit tests for the private ``_normalize_workflow_config`` helper. + + Imported from the private API intentionally — whitebox tests lock the + behavior contract that ``create_workflow``/``update_workflow`` rely on. + """ + + def test_none_returns_empty_object(self): + self.assertEqual(_normalize_workflow_config(None), "{}") + + def test_empty_dict_serialized_to_empty_json(self): + # Empty dict has no workflow keys, so it falls through the wrap check + # and serializes to ``"{}"`` — coincidentally matching the legacy + # ``None -> "{}"`` default. + self.assertEqual(_normalize_workflow_config({}), "{}") + + def test_string_without_workflow_keys_preserved_byte_for_byte(self): + self.assertEqual(_normalize_workflow_config('{"a":1}'), '{"a":1}') + + def test_non_json_string_passthrough(self): + self.assertEqual(_normalize_workflow_config("not json"), "not json") + + def test_already_wrapped_json_string_preserved_byte_for_byte(self): + wrapped = '{"specification": {"version": "1.0"}}' + self.assertEqual(_normalize_workflow_config(wrapped), wrapped) + + def test_partial_workflow_dict_is_wrapped(self): + # Single workflow-shaped key at top level is enough to classify as a + # bare spec; users often build definitions incrementally. + result = _normalize_workflow_config({"steps": [{"id": "s1"}]}) + self.assertEqual(json.loads(result), {"specification": {"steps": [{"id": "s1"}]}}) + + def test_json_array_input_preserved(self): + # ``isinstance(parsed, dict)`` guards against calling ``.keys()`` on + # non-dict JSON; pinning the no-wrap behavior here protects that. + self.assertEqual(_normalize_workflow_config("[1,2,3]"), "[1,2,3]") + + def test_json_scalar_inputs_preserved(self): + self.assertEqual(_normalize_workflow_config("42"), "42") + self.assertEqual(_normalize_workflow_config("true"), "true") + self.assertEqual(_normalize_workflow_config("null"), "null") + + def test_utf8_bom_stripped_before_parse(self): + # Windows editors frequently prepend a UTF-8 BOM. Without the strip, + # ``json.loads`` raises and the raw (unwrapped) string would ship — + # reproducing the exact 502 this PR is meant to fix. + bom_str = '\ufeff{"version":"1.0","steps":[]}' + result = _normalize_workflow_config(bom_str) + self.assertEqual(json.loads(result), {"specification": {"version": "1.0", "steps": []}}) + + def test_utf8_bom_stripped_when_already_wrapped(self): + # Already-wrapped JSON saved from a Windows editor would otherwise + # ship the BOM through to the backend, where the inference server's + # ``json.loads`` rejects it ("Unexpected UTF-8 BOM") \u2014 same 502 in + # a different shape. + bom_wrapped = '\ufeff{"specification": {"version": "1.0"}}' + self.assertEqual( + _normalize_workflow_config(bom_wrapped), + '{"specification": {"version": "1.0"}}', + ) + + def test_utf8_bom_stripped_for_non_workflow_dict_string(self): + # A custom JSON payload (not a workflow spec) with a leading BOM + # also gets the BOM removed so the backend stores parseable JSON. + bom_custom = '\ufeff{"a":1}' + self.assertEqual(_normalize_workflow_config(bom_custom), '{"a":1}') + + def test_utf8_bom_stripped_for_non_json_string(self): + # Non-JSON string with a BOM: still strip the BOM, since shipping + # it verbatim has no upside and would only produce a downstream + # decode error if anything ever tries to parse it. + self.assertEqual(_normalize_workflow_config("\ufeffnot json"), "not json") + + def test_wrapped_output_uses_compact_separators(self): + # Matches the shape the web UI writes via ``JSON.stringify``, so + # Firestore audit/diff tooling sees SDK- and UI-written rows as + # byte-identical when the logical content matches. + result = _normalize_workflow_config({"version": "1.0", "steps": []}) + self.assertEqual(result, '{"specification":{"version":"1.0","steps":[]}}') + class TestUpdateWorkflow(unittest.TestCase): @patch("roboflow.adapters.rfapi.requests.post") @@ -360,6 +497,18 @@ def test_config_string_passthrough(self, mock_post): payload = mock_post.call_args[1]["json"] self.assertEqual(payload["config"], '{"a":1}') + @patch("roboflow.adapters.rfapi.requests.post") + def test_bare_spec_dict_is_auto_wrapped_on_update(self, mock_post): + import json as _json + + from roboflow.adapters.rfapi import update_workflow + + mock_post.return_value = MagicMock(status_code=200, json=lambda: {"status": "ok"}) + bare = {"version": "1.0", "inputs": [], "steps": [], "outputs": []} + update_workflow("key", "ws", workflow_id="id-1", workflow_name="WF1", workflow_url="wf1", config=bare) + sent_config = _json.loads(mock_post.call_args[1]["json"]["config"]) + self.assertEqual(sent_config, {"specification": bare}) + @patch("roboflow.adapters.rfapi.requests.post") def test_error(self, mock_post): from roboflow.adapters.rfapi import RoboflowError, update_workflow