fix(review): address CodeRabbit feedback on PR #266

- Add Field(max_length=) constraints to FlowExistsBody.name (1024) and
  .external_version (128) to mirror the bounds already on the Flow model
- Add smoke test for the deprecated GET shim (flow_exists_get) to prevent
  silent regressions on the backward-compatibility surface
- Add regression test for URI-unsafe flow names (containing '/') via
  POST /flows/exists, guarding the core motivation for the GET->POST migration
- Extract duplicated marker expression into a job-level TEST_MARKER env var
  in tests.yml to eliminate drift risk

Author: Jayant Kernel

.github/workflows/tests.yml

@@ -14,6 +14,8 @@ permissions:
 jobs:
   tests:
     name: "${{ matrix.php_api == true && 'Migration' || 'Python-only' }} ${{ matrix.mutations == true && 'with mutations' || 'read-only' }}"
+    env:
+      TEST_MARKER: "${{ matrix.php_api == true && 'php_api' || 'not php_api' }} and ${{ matrix.mutations == true && 'mut' || 'not mut' }}"
     strategy:
       matrix:
         php_api: [true, false]
@@ -36,12 +38,10 @@ jobs:
 
       - name: Run internal tests
         run: |
-          marker="${{ matrix.php_api == true && 'php_api' || 'not php_api' }} and ${{ matrix.mutations == true && 'mut' || 'not mut' }}"
-          docker exec openml-python-rest-api coverage run -m pytest tests/database tests/config_test.py -n auto -v -m "$marker"
+          docker exec openml-python-rest-api coverage run -m pytest tests/database tests/config_test.py -n auto -v -m "$TEST_MARKER"
       - name: Run API tests
         run: |
-          marker="${{ matrix.php_api == true && 'php_api' || 'not php_api' }} and ${{ matrix.mutations == true && 'mut' || 'not mut' }}"
-          docker exec openml-python-rest-api coverage run -a -m pytest tests/routers -n auto -v -m "$marker"
+          docker exec openml-python-rest-api coverage run -a -m pytest tests/routers -n auto -v -m "$TEST_MARKER"
       - name: Produce coverage report
         run: docker exec openml-python-rest-api coverage xml
       - name: Upload results to Codecov

src/schemas/flows.py

@@ -7,8 +7,8 @@
 
 
 class FlowExistsBody(BaseModel):
-    name: str
-    external_version: str
+    name: str = Field(max_length=1024)
+    external_version: str = Field(max_length=128)
 
 
 class Parameter(BaseModel):

tests/routers/openml/flows_test.py

@@ -4,7 +4,7 @@
 import pytest
 from fastapi import HTTPException
 from pytest_mock import MockerFixture
-from sqlalchemy import Connection
+from sqlalchemy import Connection, text
 from starlette.testclient import TestClient
 
 from routers.openml.flows import flow_exists
@@ -76,6 +76,29 @@ def test_flow_exists_not_exists(py_api: TestClient) -> None:
     assert response.json()["detail"] == "Flow not found."
 
 
+def test_flow_exists_get_deprecated(flow: Flow, py_api: TestClient) -> None:
+    response = py_api.get(f"/flows/exists/{flow.name}/{flow.external_version}")
+    assert response.status_code == HTTPStatus.OK
+    assert response.json() == {"flow_id": flow.id}
+
+
+def test_flow_exists_uri_unsafe(expdb_test: Connection, py_api: TestClient) -> None:
+    expdb_test.execute(
+        text(
+            """
+            INSERT INTO implementation(fullname,name,version,external_version,uploadDate)
+            VALUES ('weka/ZeroR','weka/ZeroR',2,'1.0/beta','2024-02-02 02:23:23');
+            """,
+        ),
+    )
+    (flow_id,) = expdb_test.execute(text("""SELECT LAST_INSERT_ID();""")).one()
+    response = py_api.post(
+        "/flows/exists", json={"name": "weka/ZeroR", "external_version": "1.0/beta"}
+    )
+    assert response.status_code == HTTPStatus.OK
+    assert response.json() == {"flow_id": flow_id}
+
+
 def test_get_flow_no_subflow(py_api: TestClient) -> None:
     response = py_api.get("/flows/1")
     assert response.status_code == HTTPStatus.OK