Improves tests for the CLMC service Graph API

src/service/clmcservice/graphapi/tests.py

@@ -51,66 +51,65 @@ class TestGraphAPI(object):
 
         testing.tearDown()
 
-    def test_build(self, db_testing_data):
+    @pytest.mark.parametrize("body, from_timestamp, to_timestamp, error_msg", [
+        (None, None, None, "A bad request error must have been raised in case of missing request body."),
+        ('{}', 12341412, 1234897, "A bad request error must have been raised in case of invalid request body."),
+        ('{"database": "CLMCMetrics", "retention_policy": "autogen", "service_function_chain_instance": "sfc_i"}', 12341412, 1234897, "A bad request error must have been raised in case of invalid request body."),
+        ('{"database": "TestInfluxDB", "retention_policy": "autogen", "service_function_chain_instance": "sfc_1", "service_functions": "{invalid_json}"}', 1528386860, 1528389860, "A bad request error must have been raised in case of invalid request body."),
+        ('{"database": "TestInfluxDB", "retention_policy": "autogen", "service_function_chain_instance": "sfc_1", "service_functions": ["nginx", "minio"]}', 1528386860, 1528389860, "A bad request error must have been raised in case of invalid request body."),
+        ('{"retention_policy": "autogen", "service_function_chain_instance": "sfc_1", "service_functions": {"nginx": {"measurement_name": "nginx", "response_time_field": "mean(avg_processing_time)"}, "minio": {"measurement_name": "minio_http", "response_time_field": "mean(total_processing_time)/mean(total_requests_count)"}, "apache": {"measurement_name": "apache", "response_time_field": "mean(avg_processing_time)"}}}',
+         1528386860, 1528389860, "A bad request error must have been raised in case of missing database value in the request body"),
+        ('{"database": "TestInfluxDB", "retention_policy": "autogen", "service_function_chain_instance": "sfc_id", "service_functions": {"nginx": {"measurement_name": "nginx", "response_time_field": "mean(avg_processing_time)"}, "minio": {"measurement_name": "minio_http", "response_time_field": "mean(total_processing_time)/mean(total_requests_count)"}, "apache": {"measurement_name": "apache", "response_time_field": "mean(avg_processing_time)"}}}',
+         1528386860, 1528389860, "A bad request error must have been raised in case of invalid sfc_i ID in the request body"),
+        ('{"database": "TestInfluxDB", "retention_policy": "autogen", "service_function_chain_instance": "testsfc1", "service_functions": {"nginx": {"measurement_name": "nginx", "response_time_field": "mean(avg_processing_time)"}, "minio": {"measurement_name": "minio_http", "response_time_field": "mean(total_processing_time)/mean(total_requests_count)"}}}',
+         1528386860, 1528389860, "A bad request error must have been raised in case of invalid sfc_i ID in the request body"),
+        ('{"database": "TestInfluxDB", "retention_policy": "autogen", "service_function_chain_instance": "sfc_1", "service_functions": {"nginx": {"measurement_name": "nginx", "response_time_field": "mean(avg_processing_time)"}, "minio": {"measurement_name": "minio_http", "response_time_field": "mean(total_processing_time)/mean(total_requests_count)"}, "apache": {"measurement_name": "apache", "response_time_field": "mean(avg_processing_time)"}}}',
+         "not a timestamp", "not a timestamp", "A bad request error must have been raised in case of invalid URL parameters."),
+        ('{"database": "TestInfluxDB", "retention_policy": "autogen", "service_function_chain_instance": "sfc_1", "service_functions": {"nginx": {"measurement_name": "nginx", "response_time_field": "mean(avg_processing_time)"}, "minio": {"measurement_name": "minio_http", "response_time_field": "mean(total_processing_time)/mean(total_requests_count)"}, "apache": {"measurement_name": "apache", "response_time_field": "mean(avg_processing_time)"}}}',
+         None, "not a timestamp", "A bad request error must have been raised in case of invalid URL parameters."),
+        ('{"database": "TestInfluxDB", "retention_policy": "autogen", "service_function_chain_instance": "sfc_1", "service_functions": {"nginx": {"measurement_name": "nginx", "response_time_field": "mean(avg_processing_time)"}, "minio": {"measurement_name": "minio_http", "response_time_field": "mean(total_processing_time)/mean(total_requests_count)"}, "apache": {"measurement_name": "apache", "response_time_field": "mean(avg_processing_time)"}}}',
+         2131212, None, "A bad request error must have been raised in case of invalid URL parameters."),
+
+    ])
+    def test_build_error_handling(self, body, from_timestamp, to_timestamp, error_msg):
         """
-        Tests the graph build API endpoint
+        Tests the error handling of the graph build API endpoint.
 
-        :param db_testing_data: pair of time stamps - the from-to range of the generated influx test data, test database name and the graph db client
+        :param body: body of the request to test
+        :param from_timestamp: the 'from' URL param
+        :param to_timestamp: the 'to' URL param
+        :param error_msg: the error message to pass in case of an error
         """
-
-        global graph_1_id, graph_2_id
-
-        from_timestamp, to_timestamp, test_db_name, graph_db = db_testing_data
-
-        dc_nodes = set([node["name"] for node in graph_db.nodes.match("ComputeNode")])
-        assert dc_nodes == set("DC" + str(i) for i in range(1, 7)), "Compute nodes must have been created by the db_testing_data fixture"
-
         request = testing.DummyRequest()
+        if body is not None:
+            request.body = body
         request.body = request.body.encode(request.charset)
+        if from_timestamp is not None:
+            request.params["from"] = from_timestamp
+        if to_timestamp is not None:
+            request.params["to"] = to_timestamp
         error_raised = False
         try:
             GraphAPI(request).build_temporal_graph()
         except HTTPBadRequest:
             error_raised = True
-        assert error_raised, "A bad request error must have been raised in case of missing request body."
+        assert error_raised, error_msg
 
-        body = dumps(dict(database=test_db_name, retention_policy="autogen", service_function_chain_instance="sfc_i"))
-        request = testing.DummyRequest()
-        request.params["from"] = 12341412
-        request.params["to"] = 12341412
-        request.body = body.encode(request.charset)
-        error_raised = False
-        try:
-            GraphAPI(request).build_temporal_graph()
-        except HTTPBadRequest:
-            error_raised = True
-        assert error_raised, "A bad request error must have been raised in case of invalid request body."
+    def test_build(self, db_testing_data):
+        """
+        Tests the graph build API endpoint
 
-        body = dumps(dict(database=test_db_name, retention_policy="autogen", service_function_chain_instance="sfc_1", service_functions="{invalid_json}"))
-        request = testing.DummyRequest()
-        request.params["from"] = 12341412
-        request.params["to"] = 12341412
-        request.body = body.encode(request.charset)
-        error_raised = False
-        try:
-            GraphAPI(request).build_temporal_graph()
-        except HTTPBadRequest:
-            error_raised = True
-        assert error_raised, "A bad request error must have been raised in case of invalid request body."
+        :param db_testing_data: pair of time stamps - the from-to range of the generated influx test data, test database name and the graph db client
+        """
 
-        service_functions = ["nginx", "minio"]
-        body = dumps(dict(database=test_db_name, retention_policy="autogen", service_function_chain_instance="sfc_1", service_functions=service_functions))
-        request = testing.DummyRequest()
-        request.params["from"] = 12341412
-        request.params["to"] = 12341412
-        request.body = body.encode(request.charset)
-        error_raised = False
-        try:
-            GraphAPI(request).build_temporal_graph()
-        except HTTPBadRequest:
-            error_raised = True
-        assert error_raised, "A bad request error must have been raised in case of invalid request body."
+        global graph_1_id, graph_2_id
 
+        from_timestamp, to_timestamp, test_db_name, graph_db = db_testing_data
+
+        dc_nodes = set([node["name"] for node in graph_db.nodes.match("ComputeNode")])
+        assert dc_nodes == set("DC" + str(i) for i in range(1, 7)), "Compute nodes must have been created by the db_testing_data fixture"
+
+        # test with invalid URL parameters naming
         service_functions = dict(nginx={"measurement_name": "nginx", "response_time_field": "mean(avg_processing_time)"},
                                  minio={"measurement_name": "minio_http", "response_time_field": "mean(total_processing_time)/mean(total_requests_count)"},
                                  apache={"measurement_name": "apache", "response_time_field": "mean(avg_processing_time)"})
@@ -126,36 +125,6 @@ class TestGraphAPI(object):
             error_raised = True
         assert error_raised, "A bad request error must have been raised in case of invalid URL parameters."
 
-        service_functions = dict(nginx={"measurement_name": "nginx", "response_time_field": "mean(avg_processing_time)"},
-                                 minio={"measurement_name": "minio_http", "response_time_field": "mean(total_processing_time)/mean(total_requests_count)"},
-                                 apache={"measurement_name": "apache", "response_time_field": "mean(avg_processing_time)"})
-        body = dumps(dict(database=test_db_name, retention_policy="autogen", service_function_chain_instance="sfc_1", service_functions=service_functions))
-        request = testing.DummyRequest()
-        request.params["from"] = "not a timestamp"
-        request.params["to"] = "not a timestamp"
-        request.body = body.encode(request.charset)
-        error_raised = False
-        try:
-            GraphAPI(request).build_temporal_graph()
-        except HTTPBadRequest:
-            error_raised = True
-        assert error_raised, "A bad request error must have been raised in case of invalid URL parameters."
-
-        service_functions = dict(nginx={"measurement_name": "nginx", "response_time_field": "mean(avg_processing_time)"},
-                                 minio={"measurement_name": "minio_http", "response_time_field": "mean(total_processing_time)/mean(total_requests_count)"})
-        build_json_body = dict(database=test_db_name, retention_policy="autogen", service_function_chain_instance="testsfc1", service_functions=service_functions)
-        body = dumps(build_json_body)
-        request = testing.DummyRequest()
-        request.params["from"] = from_timestamp
-        request.params["to"] = to_timestamp
-        request.body = body.encode(request.charset)
-        error_raised = False
-        try:
-            GraphAPI(request).build_temporal_graph()
-        except HTTPBadRequest:
-            error_raised = True
-        assert error_raised, "A bad request error must have been raised in case of invalid SFC ID."
-
         service_functions = dict(nginx={"measurement_name": "nginx", "response_time_field": "mean(avg_processing_time)"},
                                  minio={"measurement_name": "minio_http", "response_time_field": "mean(total_processing_time)/mean(total_requests_count)"})
         build_json_body = dict(database=test_db_name, retention_policy="autogen", service_function_chain_instance="test_sfc1_1", service_functions=service_functions)
@@ -290,6 +259,37 @@ class TestGraphAPI(object):
         assert set([node["name"] for node in graph_db.nodes.match("ServiceFunctionInstance")]) == {"nginx_1", "apache_1", "minio_1", "minio_2"}, "Service function instances must not be deleted."
         assert set([node["name"] for node in graph_db.nodes.match("ServiceFunction")]) == {"nginx", "minio", "apache"}, "Service functions must not be deleted"
 
+    @pytest.mark.parametrize("graph_id, endpoint, compute_node, error_type, error_msg", [
+        ('e8cd4768-47dd-48cd-9c74-7f8926ddbad8', None, None, HTTPBadRequest, "HTTP Bad Request must be thrown in case of missing or invalid url parameters"),
+        ('e8cd4768-47dd-48cd-9c74-7f8926ddbad8', None, "nginx", HTTPBadRequest, "HTTP Bad Request must be thrown in case of missing or invalid url parameters"),
+        ('e8cd4768-47dd-48cd-9c74-7f8926ddbad8', "nginx_1_ep1", None, HTTPBadRequest, "HTTP Bad Request must be thrown in case of missing or invalid url parameters"),
+        ('random-uuid', "nginx_1_ep1", "nginx", HTTPNotFound, "HTTP Not Found error must be thrown for an endpoint node with incorrect request ID"),
+        ('random-uuid', "minio_1_ep1", "minio", HTTPNotFound, "HTTP Not Found error must be thrown for an endpoint node with incorrect request ID"),
+    ])
+    def test_rtt_error_handling(self, graph_id, endpoint, compute_node, error_type, error_msg):
+        """
+        Tests the error handling of the graph round trip time API endpoint.
+
+        :param graph_id: the UUID of the subgraph
+        :param endpoint: endpoint ID
+        :param compute_node: compute node ID
+        :param error_type: error type to expect
+        :param error_msg: error message in case of a test failure
+        """
+
+        request = testing.DummyRequest()
+        request.matchdict["graph_id"] = graph_id
+        if endpoint is not None:
+            request.params["endpoint"] = endpoint
+        if compute_node is not None:
+            request.params["compute_node"] = compute_node
+        error_raised = False
+        try:
+            GraphAPI(request).run_rtt_query()
+        except error_type:
+            error_raised = True
+        assert error_raised, error_msg
+
     def test_rtt(self, db_testing_data):
         """
         Tests the rtt API endpoint of the Graph API
@@ -313,15 +313,6 @@ class TestGraphAPI(object):
         assert graph_subresponse.get("uuid") is not None, "Request UUID must be attached to the response."
         request_id = graph_subresponse["uuid"]
 
-        request = testing.DummyRequest()
-        request.matchdict["graph_id"] = request_id
-        error_raised = False
-        try:
-            GraphAPI(request).run_rtt_query()
-        except HTTPBadRequest:
-            error_raised = True
-        assert error_raised, "HTTP Bad Request must be thrown in case of missing or invalid url parameters"
-
         request = testing.DummyRequest()
         request.matchdict["graph_id"] = request_id
         request.params["endpoint"] = "nginx_1_ep1"
@@ -344,17 +335,6 @@ class TestGraphAPI(object):
             error_raised = True
         assert error_raised, "HTTP Not Found error must be thrown for non existing compute node"
 
-        request = testing.DummyRequest()
-        request.matchdict["graph_id"] = "random-uuid"
-        request.params["endpoint"] = "nginx_1_ep1"
-        request.params["compute_node"] = "DC0"
-        error_raised = False
-        try:
-            GraphAPI(request).run_rtt_query()
-        except HTTPNotFound:
-            error_raised = True
-        assert error_raised, "HTTP Not Found error must be thrown for an endpoint node with incorrect request ID"
-
         request = testing.DummyRequest()
         request.matchdict["graph_id"] = request_id
         request.params["endpoint"] = "apache_1_ep1"
@@ -408,6 +388,14 @@ class TestGraphAPI(object):
 
     @staticmethod
     def check_exist_relationship(relationships_tuple, graph, uuid):
+        """
+        Iterates through a tuple of relation ships and checks that each of those exists.
+
+        :param relationships_tuple: the tuple to iterate
+        :param graph: the graph object
+        :param uuid: the uuid of the request
+        """
+
         for relationship in relationships_tuple:
             from_node_name, from_node_type, to_node_name, to_node_type, relationship_type = relationship
             if from_node_type == "Endpoint":

src/service/clmcservice/graphapi/utilities.py

@@ -71,7 +71,14 @@ def validate_json_queries_body(body):
     assert GRAPH_BUILD_QUERY_PARAMS == set(body.keys()), "Invalid JSON query document."
 
     sfc_i = body["service_function_chain_instance"]
-    assert len(sfc_i.split('_')) > 1, "Incorrect format of service function chain instance ID - use format <sfcID>_<instanceNum>"
+    sfc_i_subparts = sfc_i.split('_')
+    assert len(sfc_i_subparts) > 1, "Incorrect format of service function chain instance ID - use format <sfcID>_<instanceNum>"
+
+    # check the last part of the sfc_i ID is a number
+    try:
+        int(sfc_i_subparts[-1])
+    except ValueError:
+        assert False, "Incorrect format of service function chain instance ID - use format <sfcID>_<instanceNum>"
 
     assert type(body["service_functions"]) == dict, "The service function description should be represented with a dictionary."