From 37489354ef6125b34e805a4f8b5339cd5fe150b6 Mon Sep 17 00:00:00 2001 From: louisevelayo Date: Fri, 10 Nov 2023 14:37:52 +0100 Subject: [PATCH] refactor: extract SQL statements to sql_constants.py --- node_monitor/node_monitor.py | 28 ++++----------- .../node_monitor_helpers/sql_constants.py | 22 ++++++++++++ node_monitor/node_provider_db.py | 2 +- tests/test_node_monitor.py | 4 +-- tests/test_node_provider_db.py | 35 ++++++------------- 5 files changed, 43 insertions(+), 48 deletions(-) create mode 100644 node_monitor/node_monitor_helpers/sql_constants.py diff --git a/node_monitor/node_monitor.py b/node_monitor/node_monitor.py index c738865d..ea24950b 100644 --- a/node_monitor/node_monitor.py +++ b/node_monitor/node_monitor.py @@ -5,17 +5,17 @@ import schedule import logging -import node_monitor.ic_api as ic_api -from node_monitor.ic_api import NodeProvider from node_monitor.bot_email import EmailBot from node_monitor.bot_slack import SlackBot from node_monitor.bot_telegram import TelegramBot from node_monitor.node_provider_db import NodeProviderDB +from node_monitor.node_monitor_helpers.sql_constants import INSERT_SUBSCRIBER from node_monitor.node_monitor_helpers.get_compromised_nodes import \ get_compromised_nodes from node_monitor.node_monitor_helpers.get_new_node_providers import \ get_new_node_providers import node_monitor.node_monitor_helpers.messages as messages +import node_monitor.ic_api as ic_api Seconds = int Principal = str @@ -194,7 +194,8 @@ def update_node_provider_lookup_if_new( override_data: If provided, this arg will be used instead of live fetching Node Providers from the ic-api. Useful for testing. """ - node_providers_api = override_api_data if override_api_data else ic_api.get_node_providers() + node_providers_api = \ + override_api_data if override_api_data else ic_api.get_node_providers() node_providers_api_dict = { node_provider.principal_id: node_provider.display_name for node_provider in node_providers_api.node_providers @@ -203,26 +204,11 @@ def update_node_provider_lookup_if_new( node_providers_new = get_new_node_providers( node_providers_api_dict, node_providers_db) - query = """ - INSERT INTO subscribers ( - node_provider_id, - notify_on_status_change, - notify_email, - notify_slack, - node_provider_name, - notify_telegram - ) VALUES (%s, %s, %s, %s, %s, %s) - ON CONFLICT (node_provider_id) DO UPDATE SET - notify_on_status_change = EXCLUDED.notify_on_status_change, - notify_email = EXCLUDED.notify_email, - notify_slack = EXCLUDED.notify_slack, - node_provider_name = EXCLUDED.node_provider_name, - notify_telegram = EXCLUDED.notify_telegram - """ if node_providers_new: for node_provider in node_providers_new: - params = (node_provider.principal_id, False, False, False, node_provider.display_name, False) - self.node_provider_db.execute_insert(query, params) + params = (node_provider.principal_id, False, False, + False, node_provider.display_name, False) + self.node_provider_db.execute_write(INSERT_SUBSCRIBER, params) diff --git a/node_monitor/node_monitor_helpers/sql_constants.py b/node_monitor/node_monitor_helpers/sql_constants.py new file mode 100644 index 00000000..43f7dbc8 --- /dev/null +++ b/node_monitor/node_monitor_helpers/sql_constants.py @@ -0,0 +1,22 @@ +### SQL queries for table `subscribers` +INSERT_SUBSCRIBER = """ + INSERT INTO subscribers ( + node_provider_id, + notify_on_status_change, + notify_email, + notify_slack, + node_provider_name, + notify_telegram + ) VALUES (%s, %s, %s, %s, %s, %s) + ON CONFLICT (node_provider_id) DO UPDATE SET + notify_on_status_change = EXCLUDED.notify_on_status_change, + notify_email = EXCLUDED.notify_email, + notify_slack = EXCLUDED.notify_slack, + node_provider_name = EXCLUDED.node_provider_name, + notify_telegram = EXCLUDED.notify_telegram +""" + +DELETE_SUBSCRIBER = """ + DELETE FROM subscribers + WHERE node_provider_id = %s +""" \ No newline at end of file diff --git a/node_monitor/node_provider_db.py b/node_monitor/node_provider_db.py index 097662bc..f2c3f6d6 100644 --- a/node_monitor/node_provider_db.py +++ b/node_monitor/node_provider_db.py @@ -151,7 +151,7 @@ def _execute1(self, sql: str, params: Tuple[Any, ...]) -> List[Tuple[Any, ...]]: return result - def execute_insert(self, sql: str, params: Tuple[Any, ...]) -> None: + def execute_write(self, sql: str, params: Tuple[Any, ...]) -> None: conn = self.pool.getconn() with conn.cursor() as cur: cur.execute(sql, params) diff --git a/tests/test_node_monitor.py b/tests/test_node_monitor.py index 71a253d1..c20b1119 100644 --- a/tests/test_node_monitor.py +++ b/tests/test_node_monitor.py @@ -232,7 +232,7 @@ def test_no_new_node_provider(): mock_slack_bot, mock_telegram_bot) nm.update_node_provider_lookup_if_new(cached['node_provider_control']) - assert mock_node_provider_db.execute_insert.call_count == 0 + assert mock_node_provider_db.execute_write.call_count == 0 @@ -245,7 +245,7 @@ def test_one_new_node_provider(): mock_slack_bot, mock_telegram_bot) nm.update_node_provider_lookup_if_new(cached['new_node_providers']) - assert mock_node_provider_db.execute_insert.call_count == 2 + assert mock_node_provider_db.execute_write.call_count == 2 diff --git a/tests/test_node_provider_db.py b/tests/test_node_provider_db.py index 6eb74b5f..97a1bc6c 100644 --- a/tests/test_node_provider_db.py +++ b/tests/test_node_provider_db.py @@ -2,6 +2,8 @@ from devtools import debug from node_monitor.node_provider_db import NodeProviderDB +from node_monitor.node_monitor_helpers.sql_constants import \ + DELETE_SUBSCRIBER, INSERT_SUBSCRIBER from tests.conftest import cached import node_monitor.load_config as c @@ -62,31 +64,19 @@ def test_validate_schema_node_label_lookup(): expected_schema = NodeProviderDB.schema_table_node_label_lookup actual_schema = node_provider_db._get_schema('node_label_lookup') assert set(expected_schema.items()) <= set(actual_schema.items()) + @pytest.mark.db def test_insert_subscriber_crud(): + + # Insert new node providers new_node_providers = cached["new_node_providers"] - query = """ - INSERT INTO subscribers ( - node_provider_id, - notify_on_status_change, - notify_email, - notify_slack, - node_provider_name, - notify_telegram - ) VALUES (%s, %s, %s, %s, %s, %s) - ON CONFLICT (node_provider_id) DO UPDATE SET - notify_on_status_change = EXCLUDED.notify_on_status_change, - notify_email = EXCLUDED.notify_email, - notify_slack = EXCLUDED.notify_slack, - node_provider_name = EXCLUDED.node_provider_name, - notify_telegram = EXCLUDED.notify_telegram - """ for node_provider in new_node_providers.node_providers: params = (node_provider.principal_id, False, False, False, node_provider.display_name, False) - node_provider_db.execute_insert(query, params) + node_provider_db.execute_write(INSERT_SUBSCRIBER, params) + # Check new node providers were added properly subs = node_provider_db.get_subscribers_as_dict() assert subs['test-dummy-principal-1'] == \ {'node_provider_id': 'test-dummy-principal-1', 'notify_on_status_change': False, 'notify_email': False, @@ -95,21 +85,18 @@ def test_insert_subscriber_crud(): {'node_provider_id': 'test-dummy-principal-2', 'notify_on_status_change': False, 'notify_email': False, 'notify_slack': False, 'node_provider_name': 'Node Provider B', 'notify_telegram': False,} + # Check overwriting existing records occurs properly params = ('test-dummy-principal-1', False, False, False, 'Node Provider C', False) - node_provider_db.execute_insert(query, params) + node_provider_db.execute_write(INSERT_SUBSCRIBER, params) subs = node_provider_db.get_subscribers_as_dict() assert subs['test-dummy-principal-1'] == \ {'node_provider_id': 'test-dummy-principal-1', 'notify_on_status_change': False, 'notify_email': False, 'notify_slack': False, 'node_provider_name': 'Node Provider C', 'notify_telegram': False,} - delete_query = """ - DELETE FROM subscribers - WHERE node_provider_id = %s - """ # Delete subscribers - node_provider_db.execute_insert(delete_query, ('test-dummy-principal-1',)) - node_provider_db.execute_insert(delete_query, ('test-dummy-principal-2',)) + node_provider_db.execute_write(DELETE_SUBSCRIBER, ('test-dummy-principal-1',)) + node_provider_db.execute_write(DELETE_SUBSCRIBER, ('test-dummy-principal-2',)) subs = node_provider_db.get_subscribers_as_dict() assert 'test-dummy-principal-1' not in subs assert 'test-dummy-principal-2' not in subs