Commit 455ec60b authored by Javier Velázquez's avatar Javier Velázquez
Browse files

Code refactoring 3: improve API design

parent 7477ee62

src/api/base_handler.py

+7 −6
Original line number Diff line number Diff line
@@ -103,11 +103,11 @@ class BaseSliceHandler: 
            logger.info("Slice created successfully")

            return send_response(True, code=201, data=result)

        except RuntimeError as exc:

            return send_response(False, code=200, message=str(exc))

            return send_response(False, code=422, message=str(exc))

        except Exception as exc:

            return send_response(False, code=500, message=str(exc))



    def get_flows(self, slice_id: str | None = None) -> tuple[dict[str, Any] | list[dict[str, Any]], int]:

    def get_flows(self, slice_id: str | None = None) -> tuple[dict[str, Any], int]:

        """Retrieve transport network slice information."""

        try:

            get_all_data_fn = _dep("get_all_data", _db_get_all_data)
@@ -115,20 +115,21 @@ class BaseSliceHandler: 
            if slice_id:

                for slice_item in content:

                    if slice_item.get("slice_id") == slice_id:

                        return slice_item, 200

                raise ValueError("Transport network slices not found")

                        return send_response(True, code=200, data=slice_item)

                raise ValueError("Transport network slice not found")



            if not content:

                raise ValueError("Transport network slices not found")

                return send_response(True, code=200, data=[])



            filtered = [s for s in content if s.get("controller") == self.slice_service.controller_type]

            return filtered, 200

            return send_response(True, code=200, data=filtered)



        except ValueError as exc:

            return send_response(False, code=404, message=str(exc))

        except Exception as exc:

            return send_response(False, code=500, message=str(exc))





    def modify_flow(self, slice_id: str, intent: dict[str, Any]) -> tuple[dict[str, Any], int]:

        """Modify an existing transport network slice."""

        try:

src/api/restconf_handler.py

+12 −11
Original line number Diff line number Diff line
@@ -183,7 +183,7 @@ class RestconfHandler: 
                data=result,

            )

        except RuntimeError as exc:

            return send_response(False, code=200, message=str(exc))

            return send_response(False, code=422, message=str(exc))

        except Exception as exc:

            return send_response(False, code=500, message=str(exc))
@@ -432,7 +432,7 @@ class RestconfHandler: 
                data=result,

            )

        except RuntimeError as exc:

            return send_response(False, code=200, message=str(exc))

            return send_response(False, code=422, message=str(exc))

        except Exception as exc:

            return send_response(False, code=500, message=str(exc))
@@ -471,7 +471,7 @@ class RestconfHandler: 
        except ValueError as exc:

            return send_response(False, code=404, message=str(exc))

        except RuntimeError as exc:

            return send_response(False, code=200, message=str(exc))

            return send_response(False, code=422, message=str(exc))

        except Exception as exc:

            return send_response(False, code=500, message=str(exc))
@@ -696,17 +696,18 @@ class RestconfHandler: 
    # RESTCONF Telemetry Operations: Clients

    # -------------------------------------------------------------------------



    def get_clients(self, client_id: str | None = None) -> tuple[dict[str, Any] | list[dict[str, Any]], int]:

    def get_clients(self, client_id: str | None = None) -> tuple[dict[str, Any], int]:

        """Retrieve one or all registered telemetry clients."""

        try:

            if client_id:

                get_cli_fn = _dep("get_client", _real_get_client)

                return get_cli_fn(client_id), 200

                cli = get_cli_fn(client_id)

                if not cli:

                    raise ValueError(f"Client '{client_id}' not found")

                return send_response(True, code=200, data=cli)

            get_all_cli_fn = _dep("get_all_clients", _real_get_all_clients)

            clients = get_all_cli_fn()

            if not clients:

                raise ValueError("No clients found")

            return clients, 200

            return send_response(True, code=200, data=clients or [])

        except ValueError as exc:

            return send_response(False, code=404, message=str(exc))

        except Exception as exc:
@@ -808,7 +809,7 @@ class RestconfHandler: 
                True,

                code=201,

                message="Subscription successfully created",

                data={"sliceId": slice_id, "frequency": frequency},

                data={"slice_id": slice_id, "frequency": frequency},

            )

        except KeyError as exc:

            return send_response(False, code=400, message=str(exc))
@@ -836,9 +837,9 @@ class RestconfHandler: 
            logger.info("Subscription for slice '%s' and client '%s' modified successfully", slice_id, client_id)

            return send_response(

                True,

                code=201,

                code=200,

                message="Subscription successfully modified",

                data={"sliceId": slice_id, "frequency": frequency},

                data={"slice_id": slice_id, "frequency": frequency},

            )

        except KeyError as exc:

            return send_response(False, code=400, message=str(exc))

src/tests/test_api.py

+39 −19
Original line number Diff line number Diff line
@@ -170,11 +170,11 @@ class TestBasicApiOperations: 
    """Tests for basic API operations."""



    def test_get_flows_empty(self, controller_with_mocked_db):

        """Should return an error when there are no slices."""

        """Should return 200 with an empty list when there are no slices."""

        result, code = Api(controller_with_mocked_db).get_flows()

        assert code == 404

        assert result["success"] is False

        assert result["data"] is None

        assert code == 200

        assert result["success"] is True

        assert result["data"] == []



    def test_add_flow_success(self, controller_with_mocked_db, ietf_intent):

        """Should successfully add a flow."""
@@ -193,7 +193,8 @@ class TestBasicApiOperations: 


            flows, code = Api(controller_with_mocked_db).get_flows()

            assert code == 200

            assert any(s["slice_id"] == "slice-test-1" for s in flows)

            assert flows["success"] is True

            assert any(s["slice_id"] == "slice-test-1" for s in flows["data"])



    def test_modify_flow_success(self, controller_with_mocked_db, ietf_intent):

        """Should successfully modify an existing flow."""
@@ -232,7 +233,8 @@ class TestBasicApiOperations: 


            result, code = Api(controller_with_mocked_db).get_flows("slice-test-1")

            assert code == 200

            assert result["slice_id"] == "slice-test-1"

            assert result["success"] is True

            assert result["data"]["slice_id"] == "slice-test-1"





class TestErrorHandling:
@@ -317,7 +319,8 @@ class TestClientAndSubscriptionOperations: 


        # Update subscription

        res_upd, code_upd = api.update_subscription("client-sub-1", "slice-1", frequency=20)

        assert code_upd == 201

        assert code_upd == 200

        assert res_upd["data"]["slice_id"] == "slice-1"



        # Delete all subscriptions for client

        res_del_all, code_del_all = api.delete_subscriptions("client-sub-1")
@@ -761,7 +764,8 @@ class TestApiFullCoverage: 


        with patch("src.api.main.get_all_data", return_value=[]):

            res_none, code_none = api.get_flows()

            assert code_none == 404

            assert code_none == 200

            assert res_none["data"] == []



        with (

            flask_app.app_context(),
@@ -859,10 +863,10 @@ class TestApiExtendedCoverage: 
    def test_add_network_slice_service_branches(self, controller_with_mocked_db, sample_ietf_intent):

        api = Api(controller_with_mocked_db)



        # RuntimeError -> 200

        # RuntimeError -> 422

        with patch.object(api.slice_service, "nsc", side_effect=RuntimeError("No content")):

            res_rt, code_rt = api.add_network_slice_service(sample_ietf_intent)

            assert code_rt == 200

            assert code_rt == 422



        # Exception -> 500

        with patch.object(api.slice_service, "nsc", side_effect=Exception("Uncaught")):
@@ -1021,10 +1025,10 @@ class TestApiRequestedMethodsCoverage: 
            res, code = api.add_network_slice_service(sample_ietf_intent)

            assert code == 201



        # RuntimeError -> 200

        # RuntimeError -> 422

        with patch.object(api.slice_service, "nsc", side_effect=RuntimeError("No content")):

            res_rt, code_rt = api.add_network_slice_service(sample_ietf_intent)

            assert code_rt == 200

            assert code_rt == 422



        # Exception -> 500

        with patch.object(api.slice_service, "nsc", side_effect=Exception("Uncaught")):
@@ -1048,14 +1052,14 @@ class TestApiRequestedMethodsCoverage: 
            res, code = api.add_slice_service(intent_tmpl)

            assert code == 201



        # RuntimeError -> 200

        # RuntimeError -> 422

        with (

            patch("src.api.main.get_data_store", side_effect=[None, tmpl_store_data]),

            patch("src.api.main.normalize_libyang_data", side_effect=lambda x: x),

            patch.object(api.slice_service, "nsc", side_effect=RuntimeError("No content")),

        ):

            res_rt, code_rt = api.add_slice_service(intent_tmpl)

            assert code_rt == 200

            assert code_rt == 422



        # Exception -> 500

        with (
@@ -1094,14 +1098,14 @@ class TestApiRequestedMethodsCoverage: 
            res_500, code_500 = api.update_slice_service("slice-1", intent.copy())

            assert code_500 == 500



        # RuntimeError -> 200

        # RuntimeError -> 422

        with (

            patch("src.api.main.get_data_store", side_effect=[existing_slice, tmpl_data]),

            patch("src.api.main.normalize_libyang_data", side_effect=lambda x: x),

            patch.object(api.slice_service, "nsc", side_effect=RuntimeError("No service")),

        ):

            res_rt, code_rt = api.update_slice_service("slice-1", intent.copy())

            assert code_rt == 200

            assert code_rt == 422



        # ValueError -> 404

        with (
@@ -1203,10 +1207,11 @@ class TestApiRequestedMethodsCoverage: 
            res_all, code_all = api.get_clients()

            assert code_all == 200



        # get_clients all - empty -> 404

        # get_clients all - empty -> 200 with []

        with patch("src.api.main.get_all_clients", return_value=[]):

            res_404, code_404 = api.get_clients()

            assert code_404 == 404

            res_empty, code_empty = api.get_clients()

            assert code_empty == 200

            assert res_empty["data"] == []



        # get_clients Exception -> 500

        with patch("src.api.main.get_all_clients", side_effect=Exception("Client DB Error")):
@@ -1364,3 +1369,18 @@ class TestApiRequestedMethodsCoverage: 
            items = list(gen)

            assert len(items) == 1

            assert "event: error" in items[0]





class TestSendResponseSecurity:

    """Tests to ensure send_response does not leak server internals."""



    def test_send_response_no_file_path_leak(self):

        from src.utils.send_response import send_response



        res, code = send_response(False, message="Invalid payload", code=400)

        assert code == 400

        assert res["success"] is False

        assert res["error"] == "Invalid payload"

        assert "File:" not in res["error"]

        assert "Line:" not in res["error"]

src/tests/test_e2e.py

+23 −17
Original line number Diff line number Diff line
@@ -255,20 +255,13 @@ def test_add_and_delete_flow(app, rel_path, json_data, namespace, flags, set_fla 
        else:

            pytest.fail(f"Unsupported namespace: {namespace}")



        if namespace in ["tfs", "ixia", "e2e"]:

            assert code in [200, 201], (

                f"Creation failed for request '{rel_path}' in namespace '{namespace}' with flags: {flags}. Response: {data}"

            )

        elif namespace == "restconf":

            assert code in [200, 201, 400], (

                f"Creation failed for request '{rel_path}' in namespace '{namespace}' with flags: {flags}. Response: {data}"

            )



        # Delete flow if created

        # Strict contract validation:

        if code in [200, 201]:

            # Success path: must have success=True and proper payload structure

            assert isinstance(data, dict) and data.get("success") is True, (

                f"Successful creation with code {code} must have success=True: {data}"

            )

            slice_id = None

            if isinstance(data, dict):

                # Check data payload for slice_id

            payload_data = data.get("data")

            if isinstance(payload_data, dict):

                slices = payload_data.get("slices", [])
@@ -284,3 +277,16 @@ def test_add_and_delete_flow(app, rel_path, json_data, namespace, flags, set_fla 
                _, delete_code = api.delete_slice_services()



            assert delete_code in [200, 204, 404], f"Deletion failed for slice '{slice_id}' in namespace '{namespace}'"



        elif code in [400, 422]:

            # Controlled business rejection path: must have success=False and explicit error reason

            assert isinstance(data, dict) and data.get("success") is False, (

                f"Controlled rejection with code {code} must have success=False: {data}"

            )

            assert data.get("error"), f"Rejection with code {code} must return an error reason: {data}"



        else:

            pytest.fail(

                f"Unexpected status code {code} for request '{rel_path}' in namespace '{namespace}' "

                f"with flags: {flags}. Response: {data}"

            )

src/tests/test_namespaces.py

+31 −2
Original line number Diff line number Diff line
@@ -169,7 +169,7 @@ def test_restconf_services_crud(client, auth_headers, sample_ietf_intent, temp_s 
        headers=auth_headers,

        data=json.dumps(sample_ietf_intent),

    )

    assert post_resp.status_code in [200, 201, 500]

    assert post_resp.status_code in [200, 201, 422, 500]



    # PUT (update)

    put_resp = client.put(
@@ -177,7 +177,36 @@ def test_restconf_services_crud(client, auth_headers, sample_ietf_intent, temp_s 
        headers=auth_headers,

        data=json.dumps(sample_ietf_intent),

    )

    assert put_resp.status_code in [200, 404, 500]

    assert put_resp.status_code in [200, 404, 422, 500]





def test_plural_and_singular_routes(client, auth_headers, temp_sqlite_db):

    """Test that plural and singular routes work consistently across namespaces."""

    # TFS

    resp_slices = client.get("/tfs/slices", headers=auth_headers)

    resp_slice = client.get("/tfs/slice", headers=auth_headers)

    assert resp_slices.status_code == 200

    assert resp_slice.status_code == 200

    assert resp_slices.get_json()["data"] == resp_slice.get_json()["data"]



    # IXIA

    resp_ixia_slices = client.get("/ixia/slices", headers=auth_headers)

    resp_ixia_slice = client.get("/ixia/slice", headers=auth_headers)

    assert resp_ixia_slices.status_code == 200

    assert resp_ixia_slice.status_code == 200



    # E2E

    resp_e2e_slices = client.get("/e2e/slices", headers=auth_headers)

    resp_e2e_slice = client.get("/e2e/slice", headers=auth_headers)

    assert resp_e2e_slices.status_code == 200

    assert resp_e2e_slice.status_code == 200



    # E2E alerts

    resp_alerts = client.get("/e2e/alerts", headers=auth_headers)

    resp_alert = client.get("/e2e/alert", headers=auth_headers)

    assert resp_alerts.status_code in [200, 404]

    assert resp_alert.status_code in [200, 404]







def test_delete_slice_endpoints(client, auth_headers):