From 7a7417ac1c5c48a13d2d5e4e1830bef95f31b6fb Mon Sep 17 00:00:00 2001 From: Thastertyn Date: Thu, 6 Feb 2025 13:19:41 +0100 Subject: [PATCH] Added an error catching decorator and finished account deposit --- README.md | 10 ++- src/bank_node/bank_node.py | 32 +++++---- src/bank_protocol/command_handler.py | 5 +- .../commands/account_deposit_command.py | 25 +++++-- .../commands/bank_code_command.py | 8 +-- .../bank_number_of_clients_command.py | 11 +++- .../commands/bank_total_amount_command.py | 12 +++- src/core/config.py | 3 +- src/database/database_manager.py | 8 +-- src/database/exception_catcher_decorator.py | 13 ++++ src/database/exceptions.py | 66 ++++++------------- src/services/account_service.py | 50 ++++++++++---- src/utils/constants.py | 7 +- 13 files changed, 151 insertions(+), 99 deletions(-) create mode 100644 src/database/exception_catcher_decorator.py diff --git a/README.md b/README.md index 57f3887..78a1aec 100644 --- a/README.md +++ b/README.md @@ -1,14 +1,20 @@ -# ksi +# nu (ν) ## Sources ### Signal catching + - [Catch SIGTERM](https://dnmtechs.com/graceful-sigterm-signal-handling-in-python-3-best-practices-and-implementation/) -- [Get ENUM name from value](https://stackoverflow.com/a/38716384) - [Windows termination signals](https://stackoverflow.com/a/35792192) +- ~~[Get ENUM name from value](https://stackoverflow.com/a/38716384)~~ + Unused because of required compatibility with lower version python (3.9) ### Networking + - [Dynamically finding host IP address](https://stackoverflow.com/a/28950776) +- [IP Regex](https://ihateregex.io/expr/ip/) ### Database + - [SqlAlchemy session generator](https://stackoverflow.com/a/71053353) +- [Error handling decorator](https://chatgpt.com/share/67a46109-d38c-8005-ac36-677e6511ddcd) diff --git a/src/bank_node/bank_node.py b/src/bank_node/bank_node.py index 7e5af57..8fa88bc 100644 --- a/src/bank_node/bank_node.py +++ b/src/bank_node/bank_node.py @@ -51,18 +51,28 @@ class BankNode(): signal.signal(signal.SIGBREAK, self.gracefully_exit) def start(self): - for port in range(self.config.scan_port_start, self.config.scan_port_end + 1): - self.logger.debug("Trying port %d", port) - try: - self.config.used_port = port - self.__start_server(port) - return - except socket.error as e: - if e.errno == 98: # Address is in use - self.logger.info("Port %d in use, trying next port", port) + try: + for port in range(self.config.scan_port_start, self.config.scan_port_end + 1): + self.logger.debug("Trying port %d", port) + try: + self.config.used_port = port + self.__start_server(port) + return + except socket.error as e: + if e.errno == 98: # Address is in use + self.logger.info("Port %d in use, trying next port", port) + else: + raise - self.logger.error("All ports are in use") - self.exit_with_error() + + self.logger.error("All ports are in use") + self.exit_with_error() + except socket.error as e: + if e.errno == 99: # Cannot assign to requested address + self.logger.critical("Cannot use the IP address %s", self.config.ip) + else: + self.logger.critical("Unknown error: %s", e) + self.exit_with_error() def __start_server(self, port: int): with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as socket_server: diff --git a/src/bank_protocol/command_handler.py b/src/bank_protocol/command_handler.py index 0af99ea..61728f3 100644 --- a/src/bank_protocol/command_handler.py +++ b/src/bank_protocol/command_handler.py @@ -39,6 +39,9 @@ class CommandHandler: command = self.registered_commands[request.command_code] try: response = command(request, self.config) - return f"{request.command_code} {response}" + if response is not None: + return f"{request.command_code} {response}" + else: + return request.command_code except DatabaseError as e: return f"ER {e.message}" diff --git a/src/bank_protocol/commands/account_deposit_command.py b/src/bank_protocol/commands/account_deposit_command.py index a6d017e..78a2de8 100644 --- a/src/bank_protocol/commands/account_deposit_command.py +++ b/src/bank_protocol/commands/account_deposit_command.py @@ -1,14 +1,19 @@ from core import Request, BankNodeConfig from bank_protocol.exceptions import InvalidRequest +from services.account_service import deposit_into_account +from utils.constants import MONEY_AMOUNT_MAXIMUM def account_deposit(request: Request, config: BankNodeConfig): - split_body = request.body.split("/") - split_ip = split_body[1].split(" ") + try: + split_body = request.body.split("/") + split_ip = split_body[1].split(" ") - account = split_body[0] - ip = split_ip[0] - amount = split_ip[1] + account = split_body[0] + ip = split_ip[0] + amount = split_ip[1] + except IndexError as e: + raise InvalidRequest("Invalid request format") from e if not account.isdigit(): raise InvalidRequest("Account must be a number") @@ -18,5 +23,15 @@ def account_deposit(request: Request, config: BankNodeConfig): if account_parsed < 10_000 or account_parsed > 99_999: raise InvalidRequest("Account number out of range") + if not amount.isdigit(): + raise InvalidRequest("Deposit amount must be a number") + + amount_parsed = int(amount) + + if amount_parsed > MONEY_AMOUNT_MAXIMUM: + raise InvalidRequest("Cannot deposit this much") + + deposit_into_account(account_parsed, amount_parsed) + __all__ = ["account_deposit"] diff --git a/src/bank_protocol/commands/bank_code_command.py b/src/bank_protocol/commands/bank_code_command.py index 97e85a4..faa26db 100644 --- a/src/bank_protocol/commands/bank_code_command.py +++ b/src/bank_protocol/commands/bank_code_command.py @@ -1,12 +1,10 @@ -from typing import Dict - -from core import Request, Response +from core import Request, Response, BankNodeConfig from bank_protocol.exceptions import InvalidRequest -def bank_code(request: Request, config: Dict) -> Response: +def bank_code(request: Request, config: BankNodeConfig) -> Response: if request.body is not None: raise InvalidRequest("Incorrect usage") - return config["ip"] + return config.ip __all__ = ["bank_code"] diff --git a/src/bank_protocol/commands/bank_number_of_clients_command.py b/src/bank_protocol/commands/bank_number_of_clients_command.py index 0f34fa9..472c92b 100644 --- a/src/bank_protocol/commands/bank_number_of_clients_command.py +++ b/src/bank_protocol/commands/bank_number_of_clients_command.py @@ -1,7 +1,12 @@ -from core.request import Request +from core import Request, Response +from bank_protocol.exceptions import InvalidRequest +from services.account_service import get_account_count -def bank_number_of_clients(request: Request): - pass +def bank_number_of_clients(request: Request, _) -> Response: + if request.body is not None: + raise InvalidRequest("Incorrect usage") + number_of_clients = get_account_count() + return number_of_clients __all__ = ["bank_number_of_clients"] diff --git a/src/bank_protocol/commands/bank_total_amount_command.py b/src/bank_protocol/commands/bank_total_amount_command.py index 7ae4bed..e5b8aa3 100644 --- a/src/bank_protocol/commands/bank_total_amount_command.py +++ b/src/bank_protocol/commands/bank_total_amount_command.py @@ -1,7 +1,13 @@ -from core.request import Request +from core import Request, Response +from bank_protocol.exceptions import InvalidRequest +from services.account_service import get_total_balance -def bank_total_amount(request: Request): - pass +def bank_total_amount(request: Request, _): + if request.body is not None: + raise InvalidRequest("Incorrect usage") + + total_balace = get_total_balance() + return total_balace __all__ = ["bank_total_amount"] diff --git a/src/core/config.py b/src/core/config.py index 0d2c622..c270009 100644 --- a/src/core/config.py +++ b/src/core/config.py @@ -48,8 +48,7 @@ class BankNodeConfig: # IP validation if not re.match(IP_REGEX, ip): - self.logger.error("Invalid IP in configuration") - raise ConfigError("Invalid IP in configuration") + raise ConfigError(f"Invalid IP {ip} in configuration") self.used_port: int self.ip = ip diff --git a/src/database/database_manager.py b/src/database/database_manager.py index 72160d6..1990197 100644 --- a/src/database/database_manager.py +++ b/src/database/database_manager.py @@ -4,9 +4,9 @@ from contextlib import contextmanager from sqlalchemy.orm import sessionmaker, Session from sqlalchemy import create_engine, text -from sqlalchemy.exc import DatabaseError +from sqlalchemy.exc import DatabaseError as SqlAlchemyDatabaseError -from database.exceptions import DatabaseConnectionError +from database.exceptions import DatabaseError from models.base_model import Base @@ -45,9 +45,9 @@ class DatabaseManager(): connection.execute(text("select 1")) self.logger.debug("Database connection successful") return True - except DatabaseError as e: + except SqlAlchemyDatabaseError as e: self.logger.critical("Database connection failed: %s", e) - raise DatabaseConnectionError("Database connection failed") from e + raise DatabaseError("Database connection failed", DatabaseError.CONNECTION_ERROR) from e return False diff --git a/src/database/exception_catcher_decorator.py b/src/database/exception_catcher_decorator.py new file mode 100644 index 0000000..a905b0b --- /dev/null +++ b/src/database/exception_catcher_decorator.py @@ -0,0 +1,13 @@ +from functools import wraps +from sqlalchemy.exc import DatabaseError as SqlAlchemyDatabaseError +from database.exceptions import DatabaseError + + +def handle_database_errors(func): + @wraps(func) + def wrapper(*args, **kwargs): + try: + return func(*args, **kwargs) + except SqlAlchemyDatabaseError as e: + raise DatabaseError(str(e), -1) from e + return wrapper diff --git a/src/database/exceptions.py b/src/database/exceptions.py index 9376d59..c12ac43 100644 --- a/src/database/exceptions.py +++ b/src/database/exceptions.py @@ -1,52 +1,24 @@ class DatabaseError(Exception): - def __init__(self, message: str): + # Inspired by OSError which also uses errno's + # It's a better approach than using a class for each error + UNKNOWN_ERROR = -1 + + CONNECTION_ERROR = 1 + EMPTY_CONFIG = 2 + DUPLICATE_ENTRY = 3 + NONEXISTENT_ACCOUNT = 4 + OUT_OF_ACCOUNT_SPACE = 5 + INSUFFICIENT_BALANCE = 6 + INVALID_OPERATION = 7 + + def __init__(self, message: str, errno: int, **kwargs): + super().__init__(message) self.message = message + self.errno = errno + + for key, value in kwargs.items(): + setattr(self, key, value) -class DatabaseConnectionError(DatabaseError): - def __init__(self, message: str): - super().__init__(message) - self.message = message - - -class EmptyDatabaseConfigError(Exception): - def __init__(self, message: str, config_name: str): - super().__init__(message) - - self.message = message - self.config_name = config_name - - -class DuplicateEntryError(DatabaseError): - def __init__(self, duplicate_entry_name: str, message: str): - super().__init__(message) - self.duplicate_entry_name = duplicate_entry_name - self.message = message - - -class NonexistentAccountError(DatabaseError): - def __init__(self, message: str): - super().__init__(message) - self.message = message - - -class OutOfAccountSpaceError(DatabaseError): - def __init__(self, message: str): - super().__init__(message) - self.message = message - - -class InsufficientBalance(DatabaseError): - def __init__(self, message: str): - super().__init__(message) - self.message = message - - -class InvalidOperation(DatabaseError): - def __init__(self, message: str): - super().__init__(message) - self.message = message - - -__all__ = ["DatabaseError", "DatabaseConnectionError", "DuplicateEntryError", "InsufficientBalance", "OutOfAccountSpaceError", "NonexistentAccountError", "EmptyDatabaseConfigError", "InvalidOperation"] +__all__ = ["DatabaseError"] diff --git a/src/services/account_service.py b/src/services/account_service.py index 10eccc7..abd18a8 100644 --- a/src/services/account_service.py +++ b/src/services/account_service.py @@ -2,21 +2,24 @@ from sqlalchemy import func from models import Account from database import DatabaseManager -from database.exceptions import OutOfAccountSpaceError, NonexistentAccountError, InsufficientBalance, InvalidOperation +from database.exceptions import DatabaseError +from database.exception_catcher_decorator import handle_database_errors from utils.constants import MIN_ACCOUNT_NUMBER, MAX_ACCOUNT_NUMBER +@handle_database_errors def get_next_id() -> int: with DatabaseManager.get_session() as session: current_max_id = session.query(func.max(Account.account_number)).scalar() current_max_id = current_max_id + 1 if current_max_id is not None else MIN_ACCOUNT_NUMBER if current_max_id > MAX_ACCOUNT_NUMBER: - raise OutOfAccountSpaceError("Too many users already exist, cannot open new account") + raise DatabaseError("Too many users already exist, cannot open new account", DatabaseError.OUT_OF_ACCOUNT_SPACE) return current_max_id +@handle_database_errors def create_account() -> int: new_id = get_next_id() @@ -27,47 +30,68 @@ def create_account() -> int: return new_id +@handle_database_errors def get_account_balance(account_number: int) -> int: with DatabaseManager.get_session() as session: account: Account = session.query(Account).where(Account.account_number == account_number).one_or_none() if account is None: - raise NonexistentAccountError(f"Account with number {account_number} doesn't exist") + raise DatabaseError(f"Account with number {account_number} doesn't exist", DatabaseError.NONEXISTENT_ACCOUNT) return account.balance +@handle_database_errors def withdraw_from_account(account_number: int, amount: int): - modify_balance(account_number, amount, True) - - -def deposit_into_account(account_number: int, amount: int): modify_balance(account_number, amount, False) -def modify_balance(account_number: int, amount: int, subtract: bool): +@handle_database_errors +def deposit_into_account(account_number: int, amount: int): + modify_balance(account_number, amount, True) + + +@handle_database_errors +def modify_balance(account_number: int, amount: int, add: bool): with DatabaseManager.get_session() as session: account: Account = session.query(Account).where(Account.account_number == account_number).one_or_none() if account is None: - raise NonexistentAccountError(f"Account with number {account_number} doesn't exist") + raise DatabaseError(f"Account with number {account_number} doesn't exist", DatabaseError.NONEXISTENT_ACCOUNT) - if subtract: + if add: account.balance += amount else: if account.balance - amount < 0: - raise InsufficientBalance("Not enough funds on account to withdraw this much") + raise DatabaseError("Not enough funds on account to withdraw this much", DatabaseError.INSUFFICIENT_BALANCE) account.balance -= amount session.commit() +@handle_database_errors def delete_account(account_number: int): with DatabaseManager.get_session() as session: account: Account = session.query(Account).where(Account.account_number == account_number).one_or_none() if account is None: - raise NonexistentAccountError(f"Account with number {account_number} doesn't exist") + raise DatabaseError(f"Account with number {account_number} doesn't exist", DatabaseError.NONEXISTENT_ACCOUNT) if account.balance > 0: - raise InvalidOperation("Cannot delete an account with leftover funds") + raise DatabaseError("Cannot delete an account with leftover funds", DatabaseError.INVALID_OPERATION) session.delete(account) session.commit() + + +@handle_database_errors +def get_total_balance() -> int: + with DatabaseManager.get_session() as session: + total_sum = session.query(func.sum(Account.balance)).scalar() + + return total_sum if total_sum is not None else 0 + + +@handle_database_errors +def get_account_count() -> int: + with DatabaseManager.get_session() as session: + total_sum = session.query(func.count(Account.account_number)).scalar() + + return total_sum if total_sum is not None else 0 diff --git a/src/utils/constants.py b/src/utils/constants.py index 44ed129..534e3de 100644 --- a/src/utils/constants.py +++ b/src/utils/constants.py @@ -1,8 +1,9 @@ import re +import sys -IP_REGEX = r"^[0-9]{1,3}.[0-9]{1,3}.[0-9]{1,3}.[0-9]{1,3}$" -ACCOUNT_NUMBER_REGEX = r"[0-9]{9}" -MONEY_AMOUNT_MAXIMUM = (2 ^ 63) - 1 +IP_REGEX = r"^(\b25[0-5]|\b2[0-4][0-9]|\b[01]?[0-9][0-9]?)(\.(25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)){3}$" +ACCOUNT_NUMBER_REGEX = r"^(1[0-9]{8})|([2-9][0-9]{8})$" +MONEY_AMOUNT_MAXIMUM = 2**63 - 1 MIN_ACCOUNT_NUMBER = 10_000 MAX_ACCOUNT_NUMBER = 99_999