From 85abb4d0b0581550efdc712ced54f2ae0065c51f Mon Sep 17 00:00:00 2001 From: "Federico M. Facca" Date: Sat, 19 Sep 2020 17:59:59 +0200 Subject: [PATCH 1/5] fix #208 * added code to group entities by servicepath when multiple notifications arrive at once (#208) * add tests and integration tests (so we have also test with orion triggering such case) * fix notification tests to rely only on api calls (remove call to translator) * changed error code to 400 when due to invalid request * upgraded python to 3.8.5 * fix integration test to use new docker image with entrypoint (#362) --- src/translators/sql_translator.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/translators/sql_translator.py b/src/translators/sql_translator.py index b4825364..d55be944 100644 --- a/src/translators/sql_translator.py +++ b/src/translators/sql_translator.py @@ -206,7 +206,7 @@ def insert(self, entities, fiware_service=None, fiware_servicepath=None): path) else: msg = 'Multiple servicePath are allowed only ' \ - 'if their size is match the size of entities' + 'if their size match the size of entities' raise InvalidHeaderValue('Fiware-ServicePath', fiware_servicepath, msg) From bd03af841913d59d3b3d3fa5b8f0da2fcb28d773 Mon Sep 17 00:00:00 2001 From: "Federico M. Facca" Date: Sat, 19 Sep 2020 18:36:21 +0200 Subject: [PATCH 2/5] minor fixes * remove build (we can assume a build is available) * remove volumes at the end of the test * increase timesleep for one test * improve code --- src/translators/sql_translator.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/translators/sql_translator.py b/src/translators/sql_translator.py index d55be944..5e905ad9 100644 --- a/src/translators/sql_translator.py +++ b/src/translators/sql_translator.py @@ -206,7 +206,7 @@ def insert(self, entities, fiware_service=None, fiware_servicepath=None): path) else: msg = 'Multiple servicePath are allowed only ' \ - 'if their size match the size of entities' + 'if their number match the number of entities' raise InvalidHeaderValue('Fiware-ServicePath', fiware_servicepath, msg) From ef771241f0aee304b4f2653a00bc8c22022809c1 Mon Sep 17 00:00:00 2001 From: "Federico M. Facca" Date: Sun, 20 Sep 2020 20:28:56 +0200 Subject: [PATCH 3/5] fix #347 * fix wrong delete logic not keeping into account correctly servicePath!=None * add test for #347 bug * add test for selective service path delete --- src/reporter/tests/test_delete.py | 154 +++++++++++++++++++++++++++++- src/translators/sql_translator.py | 34 ++++--- 2 files changed, 168 insertions(+), 20 deletions(-) diff --git a/src/reporter/tests/test_delete.py b/src/reporter/tests/test_delete.py index 7e2c6e2e..fc2b0095 100644 --- a/src/reporter/tests/test_delete.py +++ b/src/reporter/tests/test_delete.py @@ -10,7 +10,7 @@ notify_url = "{}/notify".format(QL_URL) -def insert_test_data(service, entity_id=None): +def insert_test_data(service, service_path=None, entity_id=None): # 3 entity types, 2 entities for each, 10 updates for each entity. for t in ("AirQualityObserved", "Room", "TrafficFlowObserved"): for e in range(2): @@ -22,12 +22,15 @@ def insert_test_data(service, entity_id=None): 'Content-Type': 'application/json', 'Fiware-Service': service } + if service_path: + h['Fiware-ServicePath'] = service_path r = requests.post(notify_url, data=data, headers=h) assert r.status_code == 200, r.text time.sleep(1) + @pytest.mark.parametrize("service", [ "t1", "t2" ]) @@ -44,7 +47,7 @@ def test_delete_entity(service): h = { 'Fiware-Service': service } - url = '{}/entities/{}'.format(QL_URL, entity_type+'0') + url = '{}/entities/{}'.format(QL_URL, entity_type + '0') # Values are there r = requests.get(url, params=params, headers=h) @@ -61,7 +64,7 @@ def test_delete_entity(service): assert r.status_code == 404, r.text # But not for other entities of same type - url = '{}/entities/{}'.format(QL_URL, entity_type+'1') + url = '{}/entities/{}'.format(QL_URL, entity_type + '1') r = requests.get(url, params=params, headers=h) assert r.status_code == 200, r.text assert r.text != '' @@ -124,7 +127,7 @@ def test_not_found(service): h = { 'Fiware-Service': service } - url = '{}/entities/{}'.format(QL_URL, entity_type+'0') + url = '{}/entities/{}'.format(QL_URL, entity_type + '0') r = requests.delete(url, params=params, headers=h) assert r.status_code == 404, r.text @@ -160,6 +163,7 @@ def test_no_type_not_unique(service): for t in ("AirQualityObserved", "Room", "TrafficFlowObserved"): delete_test_data(service, [t]) + @pytest.mark.parametrize("service", [ "t1", "t2" ]) @@ -216,4 +220,144 @@ def test_delete_no_type_with_multitenancy(service): assert r.status_code == 200, r.text delete_test_data(service, ["Car"]) delete_test_data('USA', ["Car"]) - delete_test_data('EU', ["Car"]) \ No newline at end of file + delete_test_data('EU', ["Car"]) + + +def test_delete_347(): + """ + Test to replicate issue #347. + """ + entity_type = "deletetestDuno" + service = 'bbbbb' + service_path = '/' + params = { + 'type': entity_type, + } + h = { + 'Fiware-Service': service, + 'Fiware-ServicePath': service_path + } + + data = { + 'subscriptionId': 'ID_FROM_SUB', + 'data': [{ + 'id': 'un3', + 'type': 'deletetestDuno', + 'batteryVoltage': { + 'type': 'Text', + 'value': 'ilariso' + } + }] + } + + hn = { + 'Content-Type': 'application/json', + 'Fiware-Service': service, + 'Fiware-ServicePath': service_path + } + r = requests.post(notify_url, + data=json.dumps(data), + headers=hn) + assert r.status_code == 200, r.text + time.sleep(1) + # check that value is in the database + url = '{}/entities/{}'.format(QL_URL, 'un3') + r = requests.get(url, params=params, headers=h) + assert r.status_code == 200, r.text + assert r.text != '' + + # Delete call + time.sleep(1) + url = '{}/types/{}'.format(QL_URL, entity_type) + r = requests.delete(url, params=params, headers=h) + assert r.status_code == 204, r.text + + # Values are gone + time.sleep(1) + url = '{}/entities/{}'.format(QL_URL, '{}{}'.format(entity_type, 'un3')) + r = requests.get(url, params=params, headers=h) + assert r.status_code == 404, r.text + +def test_delete_different_servicepaths(): + """ + Selective delete by service Path. + """ + entity_type = "deletetestDuno" + service = 'bbbbb' + service_path = '/a' + params = { + 'type': entity_type, + } + h = { + 'Fiware-Service': service, + 'Fiware-ServicePath': service_path + } + + data = { + 'subscriptionId': 'ID_FROM_SUB', + 'data': [{ + 'id': 'un3', + 'type': 'deletetestDuno', + 'batteryVoltage': { + 'type': 'Text', + 'value': 'ilariso' + } + }] + } + + hn = { + 'Content-Type': 'application/json', + 'Fiware-Service': service, + 'Fiware-ServicePath': service_path + } + r = requests.post(notify_url, + data=json.dumps(data), + headers=hn) + assert r.status_code == 200, r.text + + #insert the same entity in a different service path + service_path = '/b' + hn = { + 'Content-Type': 'application/json', + 'Fiware-Service': service, + 'Fiware-ServicePath': service_path + } + r = requests.post(notify_url, + data=json.dumps(data), + headers=hn) + assert r.status_code == 200, r.text + time.sleep(1) + # check that value is in the database + url = '{}/entities/{}'.format(QL_URL, 'un3') + r = requests.get(url, params=params, headers=h) + assert r.status_code == 200, r.text + assert r.text != '' + + # Delete /a + time.sleep(2) + url = '{}/types/{}'.format(QL_URL, entity_type) + r = requests.delete(url, params=params, headers=h) + assert r.status_code == 204, r.text + + h = { + 'Fiware-Service': service, + 'Fiware-ServicePath': service_path + } + + # 1 entity is still there + time.sleep(2) + url = '{}/entities/{}'.format(QL_URL, 'un3') + r = requests.get(url, params=params, headers=h) + assert r.status_code == 200, r.text + + # Delete /b + time.sleep(2) + url = '{}/types/{}'.format(QL_URL, entity_type) + r = requests.delete(url, params=params, headers=h) + assert r.status_code == 204, r.text + + # No entity + time.sleep(2) + url = '{}/entities/{}'.format(QL_URL, 'un3') + r = requests.get(url, params=params, headers=h) + assert r.status_code == 404, r.text \ No newline at end of file diff --git a/src/translators/sql_translator.py b/src/translators/sql_translator.py index 5e905ad9..f4e81bb1 100644 --- a/src/translators/sql_translator.py +++ b/src/translators/sql_translator.py @@ -1028,19 +1028,19 @@ def delete_entities(self, entity_type, from_date=None, to_date=None, table_name = self._et2tn(entity_type, fiware_service) # Delete only requested range - if from_date or to_date or fiware_servicepath: - entity_id = None - where_clause = self._get_where_clause(entity_id, - from_date, - to_date, - fiware_servicepath) - op = "delete from {} {}".format(table_name, where_clause) - try: - self.cursor.execute(op) - except Exception as e: - logging.error("{}".format(e)) - return 0 - return self.cursor.rowcount + entity_id = None + where_clause = self._get_where_clause(entity_id, + from_date, + to_date, + fiware_servicepath) + op = "delete from {} {}".format(table_name, where_clause) + try: + self.cursor.execute(op) + except Exception as e: + logging.error("{}".format(e)) + return 0 + + deleted_rows = self.cursor.rowcount # Drop whole table try: @@ -1048,7 +1048,11 @@ def delete_entities(self, entity_type, from_date=None, to_date=None, except Exception as e: logging.error("{}".format(e)) return 0 - count = self.cursor.fetchone()[0] + #TODO why the result still keeps into account the deleted rows??? + count = self.cursor.fetchall()[0][0] - deleted_rows + + if count != 0: + return deleted_rows op = "drop table {}".format(table_name) try: @@ -1072,7 +1076,7 @@ def delete_entities(self, entity_type, from_date=None, to_date=None, except Exception as e: logging.error("{}".format(e)) - return count + return deleted_rows def _get_entity_type(self, entity_id, fiware_service): """ From b80d3dacafafe4ad3f4e52b4a3c1119ef42426b0 Mon Sep 17 00:00:00 2001 From: "Federico M. Facca" Date: Sun, 20 Sep 2020 21:02:41 +0200 Subject: [PATCH 4/5] add drop all also to single entity delete if table is empty --- src/translators/sql_translator.py | 53 +++++++++++++++++++++++-------- 1 file changed, 39 insertions(+), 14 deletions(-) diff --git a/src/translators/sql_translator.py b/src/translators/sql_translator.py index f4e81bb1..ff6eab7c 100644 --- a/src/translators/sql_translator.py +++ b/src/translators/sql_translator.py @@ -1009,6 +1009,18 @@ def delete_entity(self, entity_id, entity_type=None, from_date=None, # First delete entries from table table_name = self._et2tn(entity_type, fiware_service) + + # how many rows in total? + try: + self.cursor.execute("select count(*) from {}".format(table_name)) + except Exception as e: + logging.error("{}".format(e)) + return 0 + # TODO why the result still keeps into account the deleted rows??? + # this query was moved up to make it "consistent" also with db that + # may execute delete instantly (this does not seem the case of crate) + count = self.cursor.fetchall()[0][0] + where_clause = self._get_where_clause([entity_id, ], from_date, to_date, @@ -1021,12 +1033,31 @@ def delete_entity(self, entity_id, entity_type=None, from_date=None, logging.error("{}".format(e)) return 0 - return self.cursor.rowcount + deleted_rows = self.cursor.rowcount + + count = count - deleted_rows + + if count == 0: + # Drop whole table + self._drop_table(table_name) + + return deleted_rows def delete_entities(self, entity_type, from_date=None, to_date=None, fiware_service=None, fiware_servicepath=None): table_name = self._et2tn(entity_type, fiware_service) + # how many rows in total? + try: + self.cursor.execute("select count(*) from {}".format(table_name)) + except Exception as e: + logging.error("{}".format(e)) + return 0 + # TODO why the result still keeps into account the deleted rows??? + # this query was moved up to make it "consistent" also with db that + # may execute delete instantly (this does not seem the case of crate) + count = self.cursor.fetchall()[0][0] + # Delete only requested range entity_id = None where_clause = self._get_where_clause(entity_id, @@ -1042,24 +1073,20 @@ def delete_entities(self, entity_type, from_date=None, to_date=None, deleted_rows = self.cursor.rowcount - # Drop whole table - try: - self.cursor.execute("select count(*) from {}".format(table_name)) - except Exception as e: - logging.error("{}".format(e)) - return 0 - #TODO why the result still keeps into account the deleted rows??? - count = self.cursor.fetchall()[0][0] - deleted_rows + count = count - deleted_rows - if count != 0: - return deleted_rows + if count == 0: + # Drop whole table + self._drop_table(table_name) + + return deleted_rows + def _drop_table(self, table_name): op = "drop table {}".format(table_name) try: self.cursor.execute(op) except Exception as e: logging.error("{}".format(e)) - return 0 # Delete entry from metadata table op = "delete from {} where table_name = ?".format(METADATA_TABLE_NAME) @@ -1076,8 +1103,6 @@ def delete_entities(self, entity_type, from_date=None, to_date=None, except Exception as e: logging.error("{}".format(e)) - return deleted_rows - def _get_entity_type(self, entity_id, fiware_service): """ Find the type of the given entity_id. From 7f18d539b8456744ea656eb13f68a8452104ad93 Mon Sep 17 00:00:00 2001 From: "Federico M. Facca" Date: Sun, 20 Sep 2020 22:15:22 +0200 Subject: [PATCH 5/5] fix tests to correctly delete entities with service paths --- src/reporter/tests/test_notify.py | 6 +++--- src/reporter/tests/utils.py | 7 +++++-- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/reporter/tests/test_notify.py b/src/reporter/tests/test_notify.py index 92e0ed11..049bcbbf 100644 --- a/src/reporter/tests/test_notify.py +++ b/src/reporter/tests/test_notify.py @@ -267,7 +267,7 @@ def test_integration_multiple_entities(diffEntityWithDifferentAttrs, orion_clien assert r.status_code == 200 entities = r.json() assert len(entities) == 3 - delete_entity_type("service", diffEntityWithDifferentAttrs[0]['type']) + delete_entity_type("service", diffEntityWithDifferentAttrs[0]['type'], "/Root") @pytest.mark.skip(reason="See issue #105") @pytest.mark.parametrize("service", services) @@ -327,7 +327,7 @@ def test_multiple_data_elements(service, notification, diffEntityWithDifferentAt r = requests.get(entities_url, params=None, headers=query_header(service)) entities = r.json() assert len(entities) == 3 - delete_entity_type(None, diffEntityWithDifferentAttrs[0]['type']) + delete_entity_type(service, diffEntityWithDifferentAttrs[0]['type']) @pytest.mark.parametrize("service", services) @@ -376,7 +376,7 @@ def test_multiple_data_elements_different_servicepath(service, notification, dif r = requests.get(entities_url, params=None, headers=query_headers) entities = r.json() assert len(entities) == 3 - delete_entity_type(service, diffEntityWithDifferentAttrs[0]['type']) + delete_entity_type(service, diffEntityWithDifferentAttrs[0]['type'], '/Test') @pytest.mark.parametrize("service", services) diff --git a/src/reporter/tests/utils.py b/src/reporter/tests/utils.py index b2df17e1..124ae97a 100644 --- a/src/reporter/tests/utils.py +++ b/src/reporter/tests/utils.py @@ -84,10 +84,13 @@ def insert_test_data(service, entity_types, n_entities=1, index_size=30, time.sleep(1) -def delete_entity_type(service, entity_type): +def delete_entity_type(service, entity_type, service_path=None): h = {} if service: - h = {'Fiware-Service': service} + h['Fiware-Service'] = service + if service_path: + h['Fiware-ServicePath'] = service_path + url = '{}/types/{}'.format(QL_URL, entity_type) r = requests.delete(url, headers=h)