From 93bd6e275dc00c4dc7f397e474995b63c9e0b377 Mon Sep 17 00:00:00 2001 From: elipaz Date: Fri, 1 Nov 2024 20:50:49 +0200 Subject: [PATCH 01/13] Add Client code files --- client/Communicator.py | 36 ++++++++++++++++++++++++++++++++++++ client/Controller.py | 10 ++++++++++ client/View.py | 10 ++++++++++ client/main.py | 7 +++++++ 4 files changed, 63 insertions(+) create mode 100644 client/Communicator.py create mode 100644 client/Controller.py create mode 100644 client/View.py create mode 100644 client/main.py diff --git a/client/Communicator.py b/client/Communicator.py new file mode 100644 index 0000000..04eda8e --- /dev/null +++ b/client/Communicator.py @@ -0,0 +1,36 @@ +import socket + +PORT = 5000 +ADDRESS = 'localhost' +HOST = ADDRESS + +class Communicator: + def __init__(self): + self._host = ADDRESS + self._port = PORT + self._socket = None + + # self.setup() + + def setup(self): + self._socket = socket.socket(socket.AF_INET, socket.SOCK_STREAM) + self._socket.connect((self._host, self._port)) + + def send_message(self, message): + if not self._socket: + raise RuntimeError("Socket not set up. Call setup method first.") + + message_bytes = message.encode('utf-8') + self._socket.send(message_bytes) + + def receive_message(self): + if not self._socket: + raise RuntimeError("Socket not set up. Call setup method first.") + + message_bytes = self._socket.recv(1024) + return message_bytes.decode('utf-8') + + def close(self): + if self._socket: + self._socket.close() + self._socket = None diff --git a/client/Controller.py b/client/Controller.py new file mode 100644 index 0000000..b63f4f2 --- /dev/null +++ b/client/Controller.py @@ -0,0 +1,10 @@ +from View import Viewer +from Communicator import Communicator + +class Controller: + def __init__(self): + self._view = Viewer() + self._communicator = Communicator() + + def run(self): + self._view.run() diff --git a/client/View.py b/client/View.py new file mode 100644 index 0000000..04b2b0a --- /dev/null +++ b/client/View.py @@ -0,0 +1,10 @@ +import tkinter as tk + +class Viewer: + def __init__(self): + self.root = tk.Tk() + self.root.title("My Application") + self.root.geometry("800x600") + + def run(self): + self.root.mainloop() diff --git a/client/main.py b/client/main.py new file mode 100644 index 0000000..f39593b --- /dev/null +++ b/client/main.py @@ -0,0 +1,7 @@ +from Controller import Controller + +def main(): + Controller().run() + +if __name__ == "__main__": + main() From 94c2d2bea3c2ad912444fd3cba2ea4a4e9bf6daf Mon Sep 17 00:00:00 2001 From: elipaz Date: Sun, 3 Nov 2024 15:21:29 +0200 Subject: [PATCH 02/13] Add logger --- client/Controller.py | 34 +++++++++++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/client/Controller.py b/client/Controller.py index b63f4f2..98a9ea7 100644 --- a/client/Controller.py +++ b/client/Controller.py @@ -1,10 +1,42 @@ from View import Viewer from Communicator import Communicator +import os +import logging +from datetime import datetime + +LOG_DIR = "client_logs" class Controller: def __init__(self): self._view = Viewer() self._communicator = Communicator() + self._logger = None + self._logger_setup() def run(self): - self._view.run() + self._logger.info("Starting application") + + try: + self._view.run() + + except Exception as e: + self._logger.error(f"Error during execution: {str(e)}", exc_info=True) + raise + + def _logger_setup(self): + if not os.path.exists(LOG_DIR): + os.makedirs(LOG_DIR) + + log_file = os.path.join(LOG_DIR, f"client_{datetime.now().strftime('%Y%m%d_%H%M%S')}.log") + + logging.basicConfig( + level=logging.INFO, + format='%(asctime)s - %(name)s - %(levelname)s - %(message)s', + handlers=[ + logging.FileHandler(log_file), + logging.StreamHandler() + ] + ) + + self._logger = logging.getLogger(__name__) + self._logger.info("Logger setup complete") From e57f88cf6f0d1aa0285d2b23d86501cf5705504f Mon Sep 17 00:00:00 2001 From: elipaz Date: Sun, 3 Nov 2024 23:26:20 +0200 Subject: [PATCH 03/13] Add tests --- client/__init__.py | 0 client/main.py | 2 +- client/requirments.txt | 8 +++ client/setup.py | 7 ++ client/{ => src}/Communicator.py | 34 +++++----- client/{ => src}/Controller.py | 33 +++++---- client/{ => src}/View.py | 8 ++- client/src/__init__.py | 0 client/tests/__init__.py | 0 client/tests/test_communicator.py | 76 +++++++++++++++++++++ client/tests/test_controller.py | 107 ++++++++++++++++++++++++++++++ client/tests/test_view.py | 24 +++++++ 12 files changed, 265 insertions(+), 34 deletions(-) create mode 100644 client/__init__.py create mode 100644 client/requirments.txt create mode 100644 client/setup.py rename client/{ => src}/Communicator.py (65%) rename client/{ => src}/Controller.py (51%) rename client/{ => src}/View.py (53%) create mode 100644 client/src/__init__.py create mode 100644 client/tests/__init__.py create mode 100644 client/tests/test_communicator.py create mode 100644 client/tests/test_controller.py create mode 100644 client/tests/test_view.py diff --git a/client/__init__.py b/client/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/client/main.py b/client/main.py index f39593b..ad5b89a 100644 --- a/client/main.py +++ b/client/main.py @@ -1,4 +1,4 @@ -from Controller import Controller +from src.Controller import Controller def main(): Controller().run() diff --git a/client/requirments.txt b/client/requirments.txt new file mode 100644 index 0000000..ea7c08e --- /dev/null +++ b/client/requirments.txt @@ -0,0 +1,8 @@ +-e git+https://github.com/pazMenachem/My_Internet.git@94c2d2bea3c2ad912444fd3cba2ea4a4e9bf6daf#egg=client&subdirectory=client +colorama==0.4.6 +exceptiongroup==1.2.2 +iniconfig==2.0.0 +packaging==24.1 +pluggy==1.5.0 +pytest==8.3.3 +tomli==2.0.2 diff --git a/client/setup.py b/client/setup.py new file mode 100644 index 0000000..37ab367 --- /dev/null +++ b/client/setup.py @@ -0,0 +1,7 @@ +from setuptools import setup, find_packages + +setup( + name="client", + packages=find_packages(), + version="0.1", +) \ No newline at end of file diff --git a/client/Communicator.py b/client/src/Communicator.py similarity index 65% rename from client/Communicator.py rename to client/src/Communicator.py index 04eda8e..4d0e2e0 100644 --- a/client/Communicator.py +++ b/client/src/Communicator.py @@ -1,36 +1,36 @@ import socket +from typing import Optional -PORT = 5000 -ADDRESS = 'localhost' -HOST = ADDRESS +PORT = 65432 +HOST = '127.0.0.1' class Communicator: - def __init__(self): - self._host = ADDRESS - self._port = PORT - self._socket = None + def __init__(self) -> None: + self._host: str = HOST + self._port: int = PORT + self._socket: Optional[socket.socket] = None # self.setup() - - def setup(self): + + def setup(self) -> None: self._socket = socket.socket(socket.AF_INET, socket.SOCK_STREAM) self._socket.connect((self._host, self._port)) - - def send_message(self, message): + + def send_message(self, message: str) -> None: if not self._socket: raise RuntimeError("Socket not set up. Call setup method first.") - + message_bytes = message.encode('utf-8') self._socket.send(message_bytes) - - def receive_message(self): + + def receive_message(self) -> str: if not self._socket: raise RuntimeError("Socket not set up. Call setup method first.") - + message_bytes = self._socket.recv(1024) return message_bytes.decode('utf-8') - - def close(self): + + def close(self) -> None: if self._socket: self._socket.close() self._socket = None diff --git a/client/Controller.py b/client/src/Controller.py similarity index 51% rename from client/Controller.py rename to client/src/Controller.py index 98a9ea7..574ae2f 100644 --- a/client/Controller.py +++ b/client/src/Controller.py @@ -1,19 +1,23 @@ -from View import Viewer -from Communicator import Communicator -import os import logging +import os from datetime import datetime +from typing import Optional + +from .Communicator import Communicator +from .View import Viewer LOG_DIR = "client_logs" class Controller: - def __init__(self): - self._view = Viewer() - self._communicator = Communicator() - self._logger = None + def __init__(self) -> None: + self._view: Viewer = Viewer() + self._communicator: Communicator = Communicator() + self._logger: Optional[logging.Logger] = None + self._logger_setup() - def run(self): + def run(self) -> None: + """Run the controller.""" self._logger.info("Starting application") try: @@ -23,19 +27,22 @@ def run(self): self._logger.error(f"Error during execution: {str(e)}", exc_info=True) raise - def _logger_setup(self): + def _logger_setup(self) -> None: + """Set up the logger.""" if not os.path.exists(LOG_DIR): os.makedirs(LOG_DIR) - log_file = os.path.join(LOG_DIR, f"client_{datetime.now().strftime('%Y%m%d_%H%M%S')}.log") + log_file: str = os.path.join( + LOG_DIR, f"Client_{datetime.now().strftime('%Y%m%d_%H%M%S')}.log" + ) logging.basicConfig( level=logging.INFO, - format='%(asctime)s - %(name)s - %(levelname)s - %(message)s', + format="%(asctime)s - %(name)s - %(levelname)s - %(message)s", handlers=[ logging.FileHandler(log_file), - logging.StreamHandler() - ] + logging.StreamHandler(), + ], ) self._logger = logging.getLogger(__name__) diff --git a/client/View.py b/client/src/View.py similarity index 53% rename from client/View.py rename to client/src/View.py index 04b2b0a..2b4c293 100644 --- a/client/View.py +++ b/client/src/View.py @@ -1,10 +1,12 @@ import tkinter as tk + class Viewer: - def __init__(self): - self.root = tk.Tk() + def __init__(self) -> None: + self.root: tk.Tk = tk.Tk() self.root.title("My Application") self.root.geometry("800x600") - def run(self): + def run(self) -> None: + """Run the viewer.""" self.root.mainloop() diff --git a/client/src/__init__.py b/client/src/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/client/tests/__init__.py b/client/tests/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/client/tests/test_communicator.py b/client/tests/test_communicator.py new file mode 100644 index 0000000..bc561a2 --- /dev/null +++ b/client/tests/test_communicator.py @@ -0,0 +1,76 @@ +import socket +from unittest import mock +from typing import Optional + +import pytest + +from src.Communicator import Communicator, HOST, PORT + + +@pytest.fixture +def communicator() -> Communicator: + """Fixture to create a Communicator instance.""" + return Communicator() + + +@mock.patch('socket.socket') +def test_setup(mock_socket_class: mock.Mock, communicator: Communicator) -> None: + """Test the setup method initializes and connects the socket.""" + mock_socket_instance = mock_socket_class.return_value + communicator.setup() + mock_socket_class.assert_called_once_with(socket.AF_INET, socket.SOCK_STREAM) + mock_socket_instance.connect.assert_called_once_with((HOST, PORT)) + assert communicator._socket is mock_socket_instance + + +@mock.patch('socket.socket') +def test_send_message_without_setup(mock_socket_class: mock.Mock, communicator: Communicator) -> None: + """Test sending a message without setting up the socket raises RuntimeError.""" + with pytest.raises(RuntimeError) as exc_info: + communicator.send_message("Hello") + assert str(exc_info.value) == "Socket not set up. Call setup method first." + + +@mock.patch('socket.socket') +def test_send_message(mock_socket_class: mock.Mock, communicator: Communicator) -> None: + """Test sending a message successfully.""" + mock_socket_instance = mock_socket_class.return_value + communicator._socket = mock_socket_instance + + message: str = "Hello, World!" + communicator.send_message(message) + + mock_socket_instance.send.assert_called_once_with(message.encode('utf-8')) + + +@mock.patch('socket.socket') +def test_receive_message_without_setup(mock_socket_class: mock.Mock, communicator: Communicator) -> None: + """Test receiving a message without setting up the socket raises RuntimeError.""" + with pytest.raises(RuntimeError) as exc_info: + communicator.receive_message() + assert str(exc_info.value) == "Socket not set up. Call setup method first." + + +@mock.patch('socket.socket') +def test_receive_message(mock_socket_class: mock.Mock, communicator: Communicator) -> None: + """Test receiving a message successfully.""" + mock_socket_instance = mock_socket_class.return_value + communicator._socket = mock_socket_instance + + mock_socket_instance.recv.return_value = b'Hello, Client!' + message: str = communicator.receive_message() + + mock_socket_instance.recv.assert_called_once_with(1024) + assert message == 'Hello, Client!' + + +@mock.patch('socket.socket') +def test_close_socket(mock_socket_class: mock.Mock, communicator: Communicator) -> None: + """Test closing the socket.""" + mock_socket_instance = mock_socket_class.return_value + communicator._socket = mock_socket_instance + + communicator.close() + + mock_socket_instance.close.assert_called_once() + assert communicator._socket is None \ No newline at end of file diff --git a/client/tests/test_controller.py b/client/tests/test_controller.py new file mode 100644 index 0000000..655cc9b --- /dev/null +++ b/client/tests/test_controller.py @@ -0,0 +1,107 @@ +import os +import logging +from unittest import mock +from datetime import datetime +from typing import Optional + +import pytest + +from src.Controller import Controller +from src.View import Viewer +from src.Communicator import Communicator, HOST, PORT + + +@pytest.fixture +def controller() -> Controller: + """Fixture to create a Controller instance.""" + with mock.patch('src.Controller.Viewer'), \ + mock.patch('src.Controller.Communicator'), \ + mock.patch('src.Controller.logging'): + yield Controller() + + +@mock.patch('src.Controller.os.path.exists') +@mock.patch('src.Controller.os.makedirs') +@mock.patch('src.Controller.logging.getLogger') +@mock.patch('src.Controller.logging.basicConfig') +@mock.patch('src.Controller.datetime') +def test_logger_setup( + mock_datetime: mock.Mock, + mock_basicConfig: mock.Mock, + mock_getLogger: mock.Mock, + mock_makedirs: mock.Mock, + mock_exists: mock.Mock, + controller: Controller +) -> None: + """Test the logger setup in Controller.""" + mock_exists.return_value = False + mock_datetime.now.return_value = datetime(2023, 10, 1, 12, 0, 0) + mock_logger = mock.Mock() + mock_logger.info = mock.Mock() + mock_getLogger.return_value = mock_logger + + controller._logger_setup() + + mock_exists.assert_called_once_with("client_logs") + mock_makedirs.assert_called_once_with("client_logs") + mock_datetime.now.assert_called_once() + expected_log_file = os.path.join("client_logs", "Client_20231001_120000.log") + mock_basicConfig.assert_called_once_with( + level=mock.ANY, + format="%(asctime)s - %(name)s - %(levelname)s - %(message)s", + handlers=[ + mock.ANY, + mock.ANY, + ], + ) + mock_getLogger.assert_called_once_with('src.Controller') + mock_logger.info.assert_any_call("Logger setup complete") + + +@mock.patch('src.Controller.Viewer') +@mock.patch('src.Controller.logging.Logger.info') +@mock.patch('src.Controller.logging.Logger.error') +def test_run_success( + mock_error: mock.Mock, + mock_info: mock.Mock, + mock_viewer: mock.Mock, + controller: Controller +) -> None: + """Test the run method executes successfully.""" + controller._view = mock_viewer.return_value + controller._logger = mock.Mock(spec=logging.Logger) + + controller.run() + + controller._logger.info.assert_called_with("Starting application") + controller._view.run.assert_called_once() + mock_error.assert_not_called() + + +@mock.patch('src.Controller.logging.Logger.info') +@mock.patch('src.Controller.logging.Logger.error') +def test_run_exception( + mock_error: mock.Mock, + mock_info: mock.Mock, + controller: Controller +) -> None: + """Test the run method handles exceptions properly.""" + # Setup the mock viewer instance + mock_viewer = mock.Mock() + mock_viewer.run.side_effect = Exception("Test Exception") + + controller._view = mock_viewer + + mock_logger = mock.Mock(spec=logging.Logger) + controller._logger = mock_logger + + with pytest.raises(Exception) as exc_info: + controller.run() + + assert str(exc_info.value) == "Test Exception" + mock_logger.info.assert_called_with("Starting application") + mock_logger.error.assert_called_with( + "Error during execution: Test Exception", + exc_info=True + ) + mock_viewer.run.assert_called_once() diff --git a/client/tests/test_view.py b/client/tests/test_view.py new file mode 100644 index 0000000..813399c --- /dev/null +++ b/client/tests/test_view.py @@ -0,0 +1,24 @@ +import tkinter as tk +from unittest import mock + +import pytest + +from src.View import Viewer + +@pytest.fixture +def viewer() -> Viewer: + """Fixture to create a Viewer instance.""" + with mock.patch('src.View.tk.Tk'): + yield Viewer() + + +def test_init(viewer: Viewer) -> None: + """Test the initialization of Viewer.""" + viewer.root.title.assert_called_once_with("My Application") + viewer.root.geometry.assert_called_once_with("800x600") + + +def test_run(viewer: Viewer) -> None: + """Test running the viewer's main loop.""" + viewer.run() + viewer.root.mainloop.assert_called_once() From b67a51a248501bf900336a989919e902f6f54188 Mon Sep 17 00:00:00 2001 From: elipaz Date: Mon, 4 Nov 2024 12:17:05 +0200 Subject: [PATCH 04/13] Modify Controller name to Application --- client/src/Application.py | 51 +++++++++++++++ client/tests/test_application.py | 106 +++++++++++++++++++++++++++++++ 2 files changed, 157 insertions(+) create mode 100644 client/src/Application.py create mode 100644 client/tests/test_application.py diff --git a/client/src/Application.py b/client/src/Application.py new file mode 100644 index 0000000..c6986b0 --- /dev/null +++ b/client/src/Application.py @@ -0,0 +1,51 @@ +import logging +import os +from datetime import datetime +from typing import Optional + +from .Communicator import Communicator +from .View import Viewer + +LOG_DIR = "client_logs" + +class Application: + def __init__(self) -> None: + self._view: Viewer = Viewer() + self._communicator: Communicator = Communicator() + self._logger: Optional[logging.Logger] = None + + self._logger_setup() + + def run(self) -> None: + self._logger.info("Starting application") + + try: + self._view.run() + self._communicator.run() + + except Exception as e: + self._logger.error(f"Error during execution: {str(e)}", exc_info=True) + raise + + def send_message(self, message: str) -> None: + self._communicator.send_message(message) + + def _logger_setup(self) -> None: + if not os.path.exists(LOG_DIR): + os.makedirs(LOG_DIR) + + log_file: str = os.path.join( + LOG_DIR, f"Client_{datetime.now().strftime('%Y%m%d_%H%M%S')}.log" + ) + + logging.basicConfig( + level=logging.INFO, + format="%(asctime)s - %(name)s - %(levelname)s - %(message)s", + handlers=[ + logging.FileHandler(log_file), + logging.StreamHandler(), + ], + ) + + self._logger = logging.getLogger(__name__) + self._logger.info("Logger setup complete") \ No newline at end of file diff --git a/client/tests/test_application.py b/client/tests/test_application.py new file mode 100644 index 0000000..f32802b --- /dev/null +++ b/client/tests/test_application.py @@ -0,0 +1,106 @@ +import os +import logging +from unittest import mock +from datetime import datetime +from typing import Optional + +import pytest + +from src.Application import Application +from src.View import Viewer +from src.Communicator import Communicator, HOST, PORT + + +@pytest.fixture +def application() -> Application: + """Fixture to create an Application instance.""" + with mock.patch('src.Application.Viewer'), \ + mock.patch('src.Application.Communicator'), \ + mock.patch('src.Application.logging'): + yield Application() + + +@mock.patch('src.Application.os.path.exists') +@mock.patch('src.Application.os.makedirs') +@mock.patch('src.Application.logging.getLogger') +@mock.patch('src.Application.logging.basicConfig') +@mock.patch('src.Application.datetime') +def test_logger_setup( + mock_datetime: mock.Mock, + mock_basicConfig: mock.Mock, + mock_getLogger: mock.Mock, + mock_makedirs: mock.Mock, + mock_exists: mock.Mock, + application: Application +) -> None: + """Test the logger setup in Application.""" + mock_exists.return_value = False + mock_datetime.now.return_value = datetime(2023, 10, 1, 12, 0, 0) + mock_logger = mock.Mock() + mock_logger.info = mock.Mock() + mock_getLogger.return_value = mock_logger + + application._logger_setup() + + mock_exists.assert_called_once_with("client_logs") + mock_makedirs.assert_called_once_with("client_logs") + mock_datetime.now.assert_called_once() + expected_log_file = os.path.join("client_logs", "Client_20231001_120000.log") + mock_basicConfig.assert_called_once_with( + level=mock.ANY, + format="%(asctime)s - %(name)s - %(levelname)s - %(message)s", + handlers=[ + mock.ANY, + mock.ANY, + ], + ) + mock_getLogger.assert_called_once_with('src.Application') + mock_logger.info.assert_any_call("Logger setup complete") + + +@mock.patch('src.Application.Viewer') +@mock.patch('src.Application.logging.Logger.info') +@mock.patch('src.Application.logging.Logger.error') +def test_run_success( + mock_error: mock.Mock, + mock_info: mock.Mock, + mock_viewer: mock.Mock, + application: Application +) -> None: + """Test the run method executes successfully.""" + application._view = mock_viewer.return_value + application._logger = mock.Mock(spec=logging.Logger) + + application.run() + + application._logger.info.assert_called_with("Starting application") + application._view.run.assert_called_once() + mock_error.assert_not_called() + + +@mock.patch('src.Application.logging.Logger.info') +@mock.patch('src.Application.logging.Logger.error') +def test_run_exception( + mock_error: mock.Mock, + mock_info: mock.Mock, + application: Application +) -> None: + """Test the run method handles exceptions properly.""" + mock_viewer = mock.Mock() + mock_viewer.run.side_effect = Exception("Test Exception") + + application._view = mock_viewer + + mock_logger = mock.Mock(spec=logging.Logger) + application._logger = mock_logger + + with pytest.raises(Exception) as exc_info: + application.run() + + assert str(exc_info.value) == "Test Exception" + mock_logger.info.assert_called_with("Starting application") + mock_logger.error.assert_called_with( + "Error during execution: Test Exception", + exc_info=True + ) + mock_viewer.run.assert_called_once() \ No newline at end of file From b6f1ef155c88421e06f631010e4d62aa0c2b0f73 Mon Sep 17 00:00:00 2001 From: elipaz Date: Mon, 4 Nov 2024 15:39:44 +0200 Subject: [PATCH 05/13] Modify Code Receiving requests from View and Communicator to Application now works. --- client/main.py | 7 ++- client/src/Application.py | 110 +++++++++++++++++++++++++------------ client/src/Communicator.py | 62 ++++++++++++++++----- client/src/Controller.py | 49 ----------------- client/src/Logger.py | 45 +++++++++++++++ client/src/View.py | 90 ++++++++++++++++++++++++++++-- 6 files changed, 259 insertions(+), 104 deletions(-) delete mode 100644 client/src/Controller.py create mode 100644 client/src/Logger.py diff --git a/client/main.py b/client/main.py index ad5b89a..7ef3d01 100644 --- a/client/main.py +++ b/client/main.py @@ -1,7 +1,8 @@ -from src.Controller import Controller +from src.Application import Application -def main(): - Controller().run() +def main() -> None: + application: Application = Application() + application.run() if __name__ == "__main__": main() diff --git a/client/src/Application.py b/client/src/Application.py index c6986b0..3fa2803 100644 --- a/client/src/Application.py +++ b/client/src/Application.py @@ -1,51 +1,93 @@ -import logging -import os -from datetime import datetime -from typing import Optional - +import json +import threading from .Communicator import Communicator from .View import Viewer - -LOG_DIR = "client_logs" +from .Logger import setup_logger class Application: + """ + Main application class that coordinates communication between UI and server. + + Uses threading to handle simultaneous GUI and network operations. + + Attributes: + _logger: Logger instance for application logging + _view: Viewer instance for GUI operations + _communicator: Communicator instance for network operations + """ + def __init__(self) -> None: - self._view: Viewer = Viewer() - self._communicator: Communicator = Communicator() - self._logger: Optional[logging.Logger] = None - - self._logger_setup() + """Initialize application components.""" + self._logger = setup_logger(__name__) + self._view = Viewer(message_callback=self._handle_request) + self._communicator = Communicator(message_callback=self._handle_request) def run(self) -> None: + """ + Start the application with threaded communication handling. + + Raises: + Exception: If there's an error during startup of either component. + """ self._logger.info("Starting application") try: - self._view.run() - self._communicator.run() - + self._start_communication() + self._start_gui() + except Exception as e: self._logger.error(f"Error during execution: {str(e)}", exc_info=True) raise - - def send_message(self, message: str) -> None: - self._communicator.send_message(message) + finally: + self._cleanup() + + def _start_communication(self) -> None: + """Initialize and start the communication thread.""" + try: + self._communicator.connect() + threading.Thread( + target=self._communicator.receive_message, + daemon=True + ).start() + + self._logger.info("Communication server started successfully") + except Exception as e: + self._logger.error(f"Failed to start communication: {str(e)}") + raise - def _logger_setup(self) -> None: - if not os.path.exists(LOG_DIR): - os.makedirs(LOG_DIR) + def _start_gui(self) -> None: + """Start the GUI main loop.""" + try: + self._logger.info("Starting GUI") + self._view.run() + + except Exception as e: + self._logger.error(f"Failed to start GUI: {str(e)}") + raise - log_file: str = os.path.join( - LOG_DIR, f"Client_{datetime.now().strftime('%Y%m%d_%H%M%S')}.log" - ) + def _handle_request(self, request: str) -> None: + """ + Handle outgoing messages from the UI and Server. - logging.basicConfig( - level=logging.INFO, - format="%(asctime)s - %(name)s - %(levelname)s - %(message)s", - handlers=[ - logging.FileHandler(log_file), - logging.StreamHandler(), - ], - ) + Args: + request: received request from server or user input from UI. + """ + try: + self._logger.debug(f"Processing request: {request}") + + pass ## TODO: Implement request handling from server or UI. + + except json.JSONDecodeError as e: + self._logger.error(f"Invalid JSON format: {str(e)}") + raise + except Exception as e: + self._logger.error(f"Error handling request: {str(e)}") + raise - self._logger = logging.getLogger(__name__) - self._logger.info("Logger setup complete") \ No newline at end of file + def _cleanup(self) -> None: + """Clean up resources and stop threads.""" + self._logger.info("Cleaning up application resources") + if self._communicator: + self._communicator.close() + if self._view: + self._view.root.destroy() diff --git a/client/src/Communicator.py b/client/src/Communicator.py index 4d0e2e0..037373e 100644 --- a/client/src/Communicator.py +++ b/client/src/Communicator.py @@ -1,36 +1,70 @@ import socket -from typing import Optional +from typing import Optional, Callable +import json PORT = 65432 HOST = '127.0.0.1' +RECEIVE_BUFFER_SIZE = 1024 class Communicator: - def __init__(self) -> None: - self._host: str = HOST - self._port: int = PORT + def __init__(self, message_callback: Callable[[str], None]) -> None: + """ + Initialize the communicator. + + Args: + message_callback: Callback function to handle received messages. + """ + self._host = HOST + self._port = PORT self._socket: Optional[socket.socket] = None + self._message_callback = message_callback - # self.setup() - - def setup(self) -> None: + def connect(self) -> None: + """ + Establish connection to the server. + + Raises: + socket.error: If connection cannot be established. + """ self._socket = socket.socket(socket.AF_INET, socket.SOCK_STREAM) self._socket.connect((self._host, self._port)) def send_message(self, message: str) -> None: + """ + Send a json message to the server. + + Args: + message_json: The message to send to the server. + + Raises: + RuntimeError: If socket connection is not established. + """ if not self._socket: - raise RuntimeError("Socket not set up. Call setup method first.") + raise RuntimeError("Socket not set up. Call connect method first.") + + self._socket.send(message.encode('utf-8')) + + def receive_message(self) -> None: + """Continuously receive and process messages from the socket connection. - message_bytes = message.encode('utf-8') - self._socket.send(message_bytes) + This method runs in a loop to receive messages from the socket. Each received + message is decoded from UTF-8 and passed to the message callback function. - def receive_message(self) -> str: + Raises: + RuntimeError: If socket connection is not established. + socket.error: If there's an error receiving data from the socket. + UnicodeDecodeError: If received data cannot be decoded as UTF-8. + """ if not self._socket: - raise RuntimeError("Socket not set up. Call setup method first.") + raise RuntimeError("Socket not set up. Call connect method first.") - message_bytes = self._socket.recv(1024) - return message_bytes.decode('utf-8') + while message_bytes := self._socket.recv(RECEIVE_BUFFER_SIZE): + if not message_bytes: + break + self._message_callback(message_bytes.decode('utf-8')) def close(self) -> None: + """Close the socket connection and clean up resources.""" if self._socket: self._socket.close() self._socket = None diff --git a/client/src/Controller.py b/client/src/Controller.py deleted file mode 100644 index 574ae2f..0000000 --- a/client/src/Controller.py +++ /dev/null @@ -1,49 +0,0 @@ -import logging -import os -from datetime import datetime -from typing import Optional - -from .Communicator import Communicator -from .View import Viewer - -LOG_DIR = "client_logs" - -class Controller: - def __init__(self) -> None: - self._view: Viewer = Viewer() - self._communicator: Communicator = Communicator() - self._logger: Optional[logging.Logger] = None - - self._logger_setup() - - def run(self) -> None: - """Run the controller.""" - self._logger.info("Starting application") - - try: - self._view.run() - - except Exception as e: - self._logger.error(f"Error during execution: {str(e)}", exc_info=True) - raise - - def _logger_setup(self) -> None: - """Set up the logger.""" - if not os.path.exists(LOG_DIR): - os.makedirs(LOG_DIR) - - log_file: str = os.path.join( - LOG_DIR, f"Client_{datetime.now().strftime('%Y%m%d_%H%M%S')}.log" - ) - - logging.basicConfig( - level=logging.INFO, - format="%(asctime)s - %(name)s - %(levelname)s - %(message)s", - handlers=[ - logging.FileHandler(log_file), - logging.StreamHandler(), - ], - ) - - self._logger = logging.getLogger(__name__) - self._logger.info("Logger setup complete") diff --git a/client/src/Logger.py b/client/src/Logger.py new file mode 100644 index 0000000..034d9b7 --- /dev/null +++ b/client/src/Logger.py @@ -0,0 +1,45 @@ +"""Logger module for handling application-wide logging configuration.""" + +import logging +import os +from datetime import datetime +from typing import Optional + +LOG_DIR = "client_logs" +_logger: Optional[logging.Logger] = None + +def setup_logger(name: str) -> logging.Logger: + """ + Configure and return a logger instance. + + Args: + name: The name of the module requesting the logger. + + Returns: + logging.Logger: Configured logger instance. + """ + global _logger + + if _logger is not None: + return logging.getLogger(name) + + if not os.path.exists(LOG_DIR): + os.makedirs(LOG_DIR) + + log_file: str = os.path.join( + LOG_DIR, f"Client_{datetime.now().strftime('%Y%m%d_%H%M%S')}.log" + ) + + logging.basicConfig( + level=logging.INFO, + format="%(asctime)s - %(name)s - %(levelname)s - %(message)s", + handlers=[ + logging.FileHandler(log_file), + logging.StreamHandler(), + ], + ) + + _logger = logging.getLogger(name) + _logger.info("Logger setup complete") + + return _logger \ No newline at end of file diff --git a/client/src/View.py b/client/src/View.py index 2b4c293..72b660f 100644 --- a/client/src/View.py +++ b/client/src/View.py @@ -1,12 +1,94 @@ import tkinter as tk - +from tkinter import scrolledtext, ttk +from typing import Callable +import json class Viewer: - def __init__(self) -> None: + """ + Graphical user interface for the application. + """ + + def __init__(self, message_callback: Callable[[str], None]) -> None: + """ + Initialize the viewer window and its components. + + Args: + message_callback: Callback function to handle message sending. + """ self.root: tk.Tk = tk.Tk() - self.root.title("My Application") + self.root.title("Chat Application") self.root.geometry("800x600") + self._message_callback = message_callback + self._setup_ui() + + def _send_message(self) -> None: + """Handle the sending of messages from the input field.""" + message = self.input_field.get().strip() + if message: + message_json = json.dumps({"CODE": "100", "content": message}) + self._message_callback(message_json) + self.input_field.delete(0, tk.END) + self.display_message("You", message) def run(self) -> None: - """Run the viewer.""" + """Start the main event loop of the viewer.""" self.root.mainloop() + + def _setup_ui(self) -> None: + """Set up the UI components including text areas and buttons.""" + main_container = ttk.Frame(self.root, padding="5") + main_container.grid(row=0, column=0, sticky=(tk.W, tk.E, tk.N, tk.S)) + self.root.columnconfigure(0, weight=1) + self.root.rowconfigure(0, weight=1) + + self.message_area = scrolledtext.ScrolledText( + main_container, + wrap=tk.WORD, + width=70, + height=30 + ) + + self.message_area.grid(row=0, column=0, columnspan=2, sticky=(tk.W, tk.E, tk.N, tk.S)) + self.message_area.config(state=tk.DISABLED) + + self.input_field = ttk.Entry(main_container) + self.input_field.grid(row=1, column=0, sticky=(tk.W, tk.E)) + self.input_field.bind("", lambda e: self._send_message()) + + self.send_button = ttk.Button( + main_container, + text="Send", + command=self._send_message + ) + self.send_button.grid(row=1, column=1) + + main_container.columnconfigure(0, weight=3) + main_container.columnconfigure(1, weight=1) + main_container.rowconfigure(0, weight=1) + + ## TODO: This method won't be relevant for the final version + def display_message(self, sender: str, message: str) -> None: + """ + Display a message in the message area. + + Args: + sender: The name of the message sender. + message: The message content to display. + """ + self.message_area.config(state=tk.NORMAL) + self.message_area.insert(tk.END, f"{sender}: {message}\n") + self.message_area.see(tk.END) + self.message_area.config(state=tk.DISABLED) + + ## TODO: This method won't be relevant for the final version + def display_error(self, error_message: str) -> None: + """ + Display an error message in the message area. + + Args: + error_message: The error message to display. + """ + self.message_area.config(state=tk.NORMAL) + self.message_area.insert(tk.END, f"Error: {error_message}\n") + self.message_area.see(tk.END) + self.message_area.config(state=tk.DISABLED) From bac6fd9d89bfcc855b1c7fd42628f4eead51aa14 Mon Sep 17 00:00:00 2001 From: elipaz Date: Mon, 4 Nov 2024 15:50:33 +0200 Subject: [PATCH 06/13] Modify tests --- client/pytest.ini | 2 + client/tests/test_application.py | 173 +++++++++++++++--------------- client/tests/test_communicator.py | 80 +++++++++++--- client/tests/test_controller.py | 107 ------------------ client/tests/test_view.py | 61 +++++++++-- 5 files changed, 203 insertions(+), 220 deletions(-) create mode 100644 client/pytest.ini delete mode 100644 client/tests/test_controller.py diff --git a/client/pytest.ini b/client/pytest.ini new file mode 100644 index 0000000..a635c5c --- /dev/null +++ b/client/pytest.ini @@ -0,0 +1,2 @@ +[pytest] +pythonpath = . diff --git a/client/tests/test_application.py b/client/tests/test_application.py index f32802b..ca9df06 100644 --- a/client/tests/test_application.py +++ b/client/tests/test_application.py @@ -2,105 +2,104 @@ import logging from unittest import mock from datetime import datetime -from typing import Optional +from typing import Optional, Callable import pytest from src.Application import Application from src.View import Viewer -from src.Communicator import Communicator, HOST, PORT +from src.Communicator import Communicator @pytest.fixture -def application() -> Application: +def mock_callback() -> Callable[[str], None]: + """Fixture to provide a mock callback function.""" + return mock.Mock() + + +@pytest.fixture +def application(mock_callback: Callable[[str], None]) -> Application: """Fixture to create an Application instance.""" - with mock.patch('src.Application.Viewer'), \ - mock.patch('src.Application.Communicator'), \ - mock.patch('src.Application.logging'): - yield Application() - - -@mock.patch('src.Application.os.path.exists') -@mock.patch('src.Application.os.makedirs') -@mock.patch('src.Application.logging.getLogger') -@mock.patch('src.Application.logging.basicConfig') -@mock.patch('src.Application.datetime') -def test_logger_setup( - mock_datetime: mock.Mock, - mock_basicConfig: mock.Mock, - mock_getLogger: mock.Mock, - mock_makedirs: mock.Mock, - mock_exists: mock.Mock, - application: Application -) -> None: - """Test the logger setup in Application.""" - mock_exists.return_value = False - mock_datetime.now.return_value = datetime(2023, 10, 1, 12, 0, 0) - mock_logger = mock.Mock() - mock_logger.info = mock.Mock() - mock_getLogger.return_value = mock_logger - - application._logger_setup() - - mock_exists.assert_called_once_with("client_logs") - mock_makedirs.assert_called_once_with("client_logs") - mock_datetime.now.assert_called_once() - expected_log_file = os.path.join("client_logs", "Client_20231001_120000.log") - mock_basicConfig.assert_called_once_with( - level=mock.ANY, - format="%(asctime)s - %(name)s - %(levelname)s - %(message)s", - handlers=[ - mock.ANY, - mock.ANY, - ], - ) - mock_getLogger.assert_called_once_with('src.Application') - mock_logger.info.assert_any_call("Logger setup complete") - - -@mock.patch('src.Application.Viewer') -@mock.patch('src.Application.logging.Logger.info') -@mock.patch('src.Application.logging.Logger.error') -def test_run_success( - mock_error: mock.Mock, - mock_info: mock.Mock, - mock_viewer: mock.Mock, - application: Application -) -> None: - """Test the run method executes successfully.""" - application._view = mock_viewer.return_value - application._logger = mock.Mock(spec=logging.Logger) - - application.run() + with mock.patch('src.Application.Viewer') as mock_viewer, \ + mock.patch('src.Application.Communicator') as mock_comm, \ + mock.patch('src.Application.setup_logger') as mock_logger: + app = Application() + app._logger = mock.Mock() + return app + + +def test_init(application: Application) -> None: + """Test the initialization of Application.""" + assert hasattr(application, '_logger') + assert hasattr(application, '_view') + assert hasattr(application, '_communicator') + + +@mock.patch('src.Application.threading.Thread') +def test_start_communication(mock_thread: mock.Mock, application: Application) -> None: + """Test the communication startup.""" + application._start_communication() - application._logger.info.assert_called_with("Starting application") + application._communicator.connect.assert_called_once() + mock_thread.assert_called_once_with( + target=application._communicator.receive_message, + daemon=True + ) + mock_thread.return_value.start.assert_called_once() + + +def test_start_gui(application: Application) -> None: + """Test the GUI startup.""" + application._start_gui() application._view.run.assert_called_once() - mock_error.assert_not_called() - - -@mock.patch('src.Application.logging.Logger.info') -@mock.patch('src.Application.logging.Logger.error') -def test_run_exception( - mock_error: mock.Mock, - mock_info: mock.Mock, - application: Application -) -> None: - """Test the run method handles exceptions properly.""" - mock_viewer = mock.Mock() - mock_viewer.run.side_effect = Exception("Test Exception") - - application._view = mock_viewer + + +def test_handle_request(application: Application) -> None: + """Test request handling.""" + test_request = '{"type": "test", "content": "message"}' - mock_logger = mock.Mock(spec=logging.Logger) - application._logger = mock_logger + # Currently just testing logging as implementation is pending + application._handle_request(test_request) + application._logger.debug.assert_called_once_with(f"Processing request: {test_request}") + + +def test_cleanup(application: Application) -> None: + """Test cleanup process.""" + application._cleanup() - with pytest.raises(Exception) as exc_info: + application._communicator.close.assert_called_once() + application._view.root.destroy.assert_called_once() + + +def test_run_success(application: Application) -> None: + """Test successful application run.""" + with mock.patch.object(application, '_start_communication'), \ + mock.patch.object(application, '_start_gui'), \ + mock.patch.object(application, '_cleanup'): + application.run() + + application._start_communication.assert_called_once() + application._start_gui.assert_called_once() + application._cleanup.assert_called_once() + + +def test_run_exception(application: Application) -> None: + """Test application run with exception.""" + error_msg = "Test error" - assert str(exc_info.value) == "Test Exception" - mock_logger.info.assert_called_with("Starting application") - mock_logger.error.assert_called_with( - "Error during execution: Test Exception", - exc_info=True - ) - mock_viewer.run.assert_called_once() \ No newline at end of file + with mock.patch.object(application, '_start_communication') as mock_start_comm, \ + mock.patch.object(application, '_cleanup') as mock_cleanup: + + mock_start_comm.side_effect = Exception(error_msg) + + with pytest.raises(Exception) as exc_info: + application.run() + + assert str(exc_info.value) == error_msg + application._logger.error.assert_called_with( + f"Error during execution: {error_msg}", + exc_info=True + ) + mock_cleanup.assert_called_once() + \ No newline at end of file diff --git a/client/tests/test_communicator.py b/client/tests/test_communicator.py index bc561a2..1ed8e81 100644 --- a/client/tests/test_communicator.py +++ b/client/tests/test_communicator.py @@ -1,34 +1,52 @@ import socket from unittest import mock -from typing import Optional +from typing import Optional, Callable import pytest -from src.Communicator import Communicator, HOST, PORT +from src.Communicator import Communicator, HOST, PORT, RECEIVE_BUFFER_SIZE @pytest.fixture -def communicator() -> Communicator: +def mock_callback() -> Callable[[str], None]: + """Fixture to provide a mock callback function.""" + return mock.Mock() + + +@pytest.fixture +def communicator(mock_callback: Callable[[str], None]) -> Communicator: """Fixture to create a Communicator instance.""" - return Communicator() + return Communicator(message_callback=mock_callback) + + +def test_init(communicator: Communicator, mock_callback: Callable[[str], None]) -> None: + """Test the initialization of Communicator.""" + assert communicator._host == HOST + assert communicator._port == PORT + assert communicator._socket is None + assert communicator._message_callback == mock_callback @mock.patch('socket.socket') -def test_setup(mock_socket_class: mock.Mock, communicator: Communicator) -> None: - """Test the setup method initializes and connects the socket.""" +def test_connect(mock_socket_class: mock.Mock, communicator: Communicator) -> None: + """Test the connect method initializes and connects the socket.""" mock_socket_instance = mock_socket_class.return_value - communicator.setup() + communicator.connect() + mock_socket_class.assert_called_once_with(socket.AF_INET, socket.SOCK_STREAM) mock_socket_instance.connect.assert_called_once_with((HOST, PORT)) assert communicator._socket is mock_socket_instance @mock.patch('socket.socket') -def test_send_message_without_setup(mock_socket_class: mock.Mock, communicator: Communicator) -> None: +def test_send_message_without_setup( + mock_socket_class: mock.Mock, + communicator: Communicator +) -> None: """Test sending a message without setting up the socket raises RuntimeError.""" with pytest.raises(RuntimeError) as exc_info: communicator.send_message("Hello") - assert str(exc_info.value) == "Socket not set up. Call setup method first." + assert str(exc_info.value) == "Socket not set up. Call connect method first." @mock.patch('socket.socket') @@ -44,24 +62,33 @@ def test_send_message(mock_socket_class: mock.Mock, communicator: Communicator) @mock.patch('socket.socket') -def test_receive_message_without_setup(mock_socket_class: mock.Mock, communicator: Communicator) -> None: +def test_receive_message_without_setup( + mock_socket_class: mock.Mock, + communicator: Communicator +) -> None: """Test receiving a message without setting up the socket raises RuntimeError.""" with pytest.raises(RuntimeError) as exc_info: communicator.receive_message() - assert str(exc_info.value) == "Socket not set up. Call setup method first." + assert str(exc_info.value) == "Socket not set up. Call connect method first." @mock.patch('socket.socket') -def test_receive_message(mock_socket_class: mock.Mock, communicator: Communicator) -> None: +def test_receive_message( + mock_socket_class: mock.Mock, + communicator: Communicator, + mock_callback: Callable[[str], None] +) -> None: """Test receiving a message successfully.""" mock_socket_instance = mock_socket_class.return_value communicator._socket = mock_socket_instance - mock_socket_instance.recv.return_value = b'Hello, Client!' - message: str = communicator.receive_message() + # Setup mock to return a message once and then empty string to break the loop + mock_socket_instance.recv.side_effect = [b'Hello, Client!', b''] + + communicator.receive_message() - mock_socket_instance.recv.assert_called_once_with(1024) - assert message == 'Hello, Client!' + mock_socket_instance.recv.assert_called_with(RECEIVE_BUFFER_SIZE) + mock_callback.assert_called_once_with('Hello, Client!') @mock.patch('socket.socket') @@ -73,4 +100,23 @@ def test_close_socket(mock_socket_class: mock.Mock, communicator: Communicator) communicator.close() mock_socket_instance.close.assert_called_once() - assert communicator._socket is None \ No newline at end of file + assert communicator._socket is None + + +@mock.patch('socket.socket') +def test_receive_message_decode_error( + mock_socket_class: mock.Mock, + communicator: Communicator, + mock_callback: Callable[[str], None] +) -> None: + """Test handling of decode errors in receive_message.""" + mock_socket_instance = mock_socket_class.return_value + communicator._socket = mock_socket_instance + + # Setup mock to return invalid UTF-8 bytes + mock_socket_instance.recv.side_effect = [bytes([0xFF, 0xFE, 0xFD]), b''] + + with pytest.raises(UnicodeDecodeError): + communicator.receive_message() + + mock_callback.assert_not_called() diff --git a/client/tests/test_controller.py b/client/tests/test_controller.py deleted file mode 100644 index 655cc9b..0000000 --- a/client/tests/test_controller.py +++ /dev/null @@ -1,107 +0,0 @@ -import os -import logging -from unittest import mock -from datetime import datetime -from typing import Optional - -import pytest - -from src.Controller import Controller -from src.View import Viewer -from src.Communicator import Communicator, HOST, PORT - - -@pytest.fixture -def controller() -> Controller: - """Fixture to create a Controller instance.""" - with mock.patch('src.Controller.Viewer'), \ - mock.patch('src.Controller.Communicator'), \ - mock.patch('src.Controller.logging'): - yield Controller() - - -@mock.patch('src.Controller.os.path.exists') -@mock.patch('src.Controller.os.makedirs') -@mock.patch('src.Controller.logging.getLogger') -@mock.patch('src.Controller.logging.basicConfig') -@mock.patch('src.Controller.datetime') -def test_logger_setup( - mock_datetime: mock.Mock, - mock_basicConfig: mock.Mock, - mock_getLogger: mock.Mock, - mock_makedirs: mock.Mock, - mock_exists: mock.Mock, - controller: Controller -) -> None: - """Test the logger setup in Controller.""" - mock_exists.return_value = False - mock_datetime.now.return_value = datetime(2023, 10, 1, 12, 0, 0) - mock_logger = mock.Mock() - mock_logger.info = mock.Mock() - mock_getLogger.return_value = mock_logger - - controller._logger_setup() - - mock_exists.assert_called_once_with("client_logs") - mock_makedirs.assert_called_once_with("client_logs") - mock_datetime.now.assert_called_once() - expected_log_file = os.path.join("client_logs", "Client_20231001_120000.log") - mock_basicConfig.assert_called_once_with( - level=mock.ANY, - format="%(asctime)s - %(name)s - %(levelname)s - %(message)s", - handlers=[ - mock.ANY, - mock.ANY, - ], - ) - mock_getLogger.assert_called_once_with('src.Controller') - mock_logger.info.assert_any_call("Logger setup complete") - - -@mock.patch('src.Controller.Viewer') -@mock.patch('src.Controller.logging.Logger.info') -@mock.patch('src.Controller.logging.Logger.error') -def test_run_success( - mock_error: mock.Mock, - mock_info: mock.Mock, - mock_viewer: mock.Mock, - controller: Controller -) -> None: - """Test the run method executes successfully.""" - controller._view = mock_viewer.return_value - controller._logger = mock.Mock(spec=logging.Logger) - - controller.run() - - controller._logger.info.assert_called_with("Starting application") - controller._view.run.assert_called_once() - mock_error.assert_not_called() - - -@mock.patch('src.Controller.logging.Logger.info') -@mock.patch('src.Controller.logging.Logger.error') -def test_run_exception( - mock_error: mock.Mock, - mock_info: mock.Mock, - controller: Controller -) -> None: - """Test the run method handles exceptions properly.""" - # Setup the mock viewer instance - mock_viewer = mock.Mock() - mock_viewer.run.side_effect = Exception("Test Exception") - - controller._view = mock_viewer - - mock_logger = mock.Mock(spec=logging.Logger) - controller._logger = mock_logger - - with pytest.raises(Exception) as exc_info: - controller.run() - - assert str(exc_info.value) == "Test Exception" - mock_logger.info.assert_called_with("Starting application") - mock_logger.error.assert_called_with( - "Error during execution: Test Exception", - exc_info=True - ) - mock_viewer.run.assert_called_once() diff --git a/client/tests/test_view.py b/client/tests/test_view.py index 813399c..888e78b 100644 --- a/client/tests/test_view.py +++ b/client/tests/test_view.py @@ -1,24 +1,67 @@ import tkinter as tk from unittest import mock +import json +from typing import Callable import pytest from src.View import Viewer + +@pytest.fixture +def mock_callback() -> Callable[[str], None]: + """Fixture to provide a mock callback function.""" + return mock.Mock() + + @pytest.fixture -def viewer() -> Viewer: +def viewer(mock_callback: Callable[[str], None]) -> Viewer: """Fixture to create a Viewer instance.""" - with mock.patch('src.View.tk.Tk'): - yield Viewer() + with mock.patch('src.View.tk.Tk') as mock_tk: + mock_tk.return_value.title = mock.Mock() + mock_tk.return_value.geometry = mock.Mock() + return Viewer(message_callback=mock_callback) -def test_init(viewer: Viewer) -> None: +def test_init(viewer: Viewer, mock_callback: Callable[[str], None]) -> None: """Test the initialization of Viewer.""" - viewer.root.title.assert_called_once_with("My Application") + viewer.root.title.assert_called_once_with("Chat Application") viewer.root.geometry.assert_called_once_with("800x600") + assert viewer._message_callback == mock_callback + + +def test_send_message(viewer: Viewer, mock_callback: Callable[[str], None]) -> None: + """Test sending a message.""" + test_message = "Hello, World!" + viewer.input_field = mock.Mock() + viewer.input_field.get.return_value = test_message + + viewer._send_message() + + expected_json = json.dumps({"CODE": "100", "content": test_message}) + mock_callback.assert_called_once_with(expected_json) + viewer.input_field.delete.assert_called_once_with(0, tk.END) + + +def test_display_message(viewer: Viewer) -> None: + """Test displaying a message.""" + viewer.message_area = mock.Mock() + + viewer.display_message("User", "Test message") + + viewer.message_area.config.assert_any_call(state=tk.NORMAL) + viewer.message_area.insert.assert_called_once_with(tk.END, "User: Test message\n") + viewer.message_area.see.assert_called_once_with(tk.END) + viewer.message_area.config.assert_any_call(state=tk.DISABLED) -def test_run(viewer: Viewer) -> None: - """Test running the viewer's main loop.""" - viewer.run() - viewer.root.mainloop.assert_called_once() +def test_display_error(viewer: Viewer) -> None: + """Test displaying an error message.""" + viewer.message_area = mock.Mock() + + viewer.display_error("Test error") + + viewer.message_area.config.assert_any_call(state=tk.NORMAL) + viewer.message_area.insert.assert_called_once_with(tk.END, "Error: Test error\n") + viewer.message_area.see.assert_called_once_with(tk.END) + viewer.message_area.config.assert_any_call(state=tk.DISABLED) From f1ebd70041b62495f7948f2415287e71bf4e83e9 Mon Sep 17 00:00:00 2001 From: elipaz Date: Tue, 5 Nov 2024 16:44:38 +0200 Subject: [PATCH 07/13] Add config file and ConfigManager --- client/config.json | 19 +++++++ client/src/ConfigManager.py | 102 ++++++++++++++++++++++++++++++++++++ 2 files changed, 121 insertions(+) create mode 100644 client/config.json create mode 100644 client/src/ConfigManager.py diff --git a/client/config.json b/client/config.json new file mode 100644 index 0000000..b662c6b --- /dev/null +++ b/client/config.json @@ -0,0 +1,19 @@ +{ + "network": { + "host": "127.0.0.1", + "port": 65432, + "receive_buffer_size": 1024 + }, + "blocked_domains": { + "example.com": true, + "ads.example.com": true + }, + "settings": { + "ad_block": "on", + "adult_block": "on" + }, + "logging": { + "level": "INFO", + "log_dir": "client_logs" + } +} \ No newline at end of file diff --git a/client/src/ConfigManager.py b/client/src/ConfigManager.py new file mode 100644 index 0000000..f7fa92a --- /dev/null +++ b/client/src/ConfigManager.py @@ -0,0 +1,102 @@ +"""Configuration management module for the application.""" + +import json +import os +from typing import Dict, Any +from .Logger import setup_logger + +DEFAULT_CONFIG = { + "network": { + "host": "127.0.0.1", + "port": 65432, + "receive_buffer_size": 1024 + }, + "blocked_domains": {}, + "settings": { + "ad_block": "off", + "adult_block": "off" + }, + "logging": { + "level": "INFO", + "log_dir": "client_logs" + } +} + +class ConfigManager: + """Manages application configuration loading and saving.""" + + def __init__(self, config_file: str = "config.json") -> None: + """ + Initialize the configuration manager. + + Args: + config_file: Path to the configuration file. + """ + self.logger = setup_logger(__name__) + self.config_file = config_file + self.config = self._load_config() + + def _load_config(self) -> Dict[str, Any]: + """ + Load configuration from JSON file. + + Returns: + Dict containing configuration settings. + """ + try: + if os.path.exists(self.config_file): + self.logger.info(f"Loading configuration from {self.config_file}") + with open(self.config_file, 'r') as f: + user_config = json.load(f) + return self._merge_configs(DEFAULT_CONFIG, user_config) + + self.logger.warning(f"Configuration file not found, using default configuration") + + except json.JSONDecodeError: + self.logger.error(f"Error decoding {self.config_file}, using default configuration") + + return DEFAULT_CONFIG.copy() + + def _merge_configs(self, default: Dict[str, Any], user: Dict[str, Any]) -> Dict[str, Any]: + """ + Recursively merge user configuration with default configuration. + + Args: + default: Default configuration dictionary + user: User configuration dictionary + + Returns: + Merged configuration dictionary + """ + result = default.copy() + + for key, value in user.items(): + if key in result and isinstance(result[key], dict) and isinstance(value, dict): + result[key] = self._merge_configs(result[key], value) + else: + result[key] = value + + return result + + def save_config(self, config: Dict[str, Any]) -> None: + """ + Save configuration to JSON file. + + Args: + config: Configuration dictionary to save + """ + try: + with open(self.config_file, 'w') as f: + json.dump(config, f, indent=4) + self.logger.info("Configuration saved successfully") + except Exception as e: + self.logger.error(f"Error saving configuration: {str(e)}") + + def get_config(self) -> Dict[str, Any]: + """ + Get the current configuration. + + Returns: + Current configuration dictionary + """ + return self.config From c4a2a4a1ccbbcf88051174857859e58abd8dd015 Mon Sep 17 00:00:00 2001 From: elipaz Date: Tue, 5 Nov 2024 16:45:57 +0200 Subject: [PATCH 08/13] Add config manager to files --- client/src/Application.py | 23 +++-- client/src/Communicator.py | 62 +++++++++---- client/src/View.py | 180 +++++++++++++++++++++++++++---------- 3 files changed, 194 insertions(+), 71 deletions(-) diff --git a/client/src/Application.py b/client/src/Application.py index 3fa2803..8a06a97 100644 --- a/client/src/Application.py +++ b/client/src/Application.py @@ -3,6 +3,7 @@ from .Communicator import Communicator from .View import Viewer from .Logger import setup_logger +from .ConfigManager import ConfigManager class Application: """ @@ -19,8 +20,9 @@ class Application: def __init__(self) -> None: """Initialize application components.""" self._logger = setup_logger(__name__) - self._view = Viewer(message_callback=self._handle_request) - self._communicator = Communicator(message_callback=self._handle_request) + self._config_manager = ConfigManager() + self._view = Viewer(config_manager=self._config_manager, message_callback=self._handle_request) + self._communicator = Communicator(config_manager=self._config_manager, message_callback=self._handle_request) def run(self) -> None: """ @@ -32,7 +34,7 @@ def run(self) -> None: self._logger.info("Starting application") try: - self._start_communication() + # self._start_communication() self._start_gui() except Exception as e: @@ -73,7 +75,7 @@ def _handle_request(self, request: str) -> None: request: received request from server or user input from UI. """ try: - self._logger.debug(f"Processing request: {request}") + self._logger.info(f"Processing request: {request}") pass ## TODO: Implement request handling from server or UI. @@ -87,7 +89,12 @@ def _handle_request(self, request: str) -> None: def _cleanup(self) -> None: """Clean up resources and stop threads.""" self._logger.info("Cleaning up application resources") - if self._communicator: - self._communicator.close() - if self._view: - self._view.root.destroy() + try: + if self._communicator: + self._communicator.close() + + if self._view and self._view.root.winfo_exists(): + self._view.root.destroy() + + except Exception as e: + self._logger.warning(f"Cleanup encountered an error: {str(e)}") diff --git a/client/src/Communicator.py b/client/src/Communicator.py index 037373e..0590ace 100644 --- a/client/src/Communicator.py +++ b/client/src/Communicator.py @@ -1,23 +1,26 @@ import socket from typing import Optional, Callable import json - -PORT = 65432 -HOST = '127.0.0.1' -RECEIVE_BUFFER_SIZE = 1024 +from .Logger import setup_logger class Communicator: - def __init__(self, message_callback: Callable[[str], None]) -> None: + def __init__(self, config_manager, message_callback: Callable[[str], None]) -> None: """ Initialize the communicator. Args: + config_manager: Configuration manager instance message_callback: Callback function to handle received messages. """ - self._host = HOST - self._port = PORT - self._socket: Optional[socket.socket] = None + self.logger = setup_logger(__name__) + self.logger.info("Initializing Communicator") + self.config = config_manager.get_config() self._message_callback = message_callback + + self._host = self.config["network"]["host"] + self._port = self.config["network"]["port"] + self._receive_buffer_size = self.config["network"]["receive_buffer_size"] + self._socket: Optional[socket.socket] = None def connect(self) -> None: """ @@ -26,8 +29,13 @@ def connect(self) -> None: Raises: socket.error: If connection cannot be established. """ - self._socket = socket.socket(socket.AF_INET, socket.SOCK_STREAM) - self._socket.connect((self._host, self._port)) + try: + self._socket = socket.socket(socket.AF_INET, socket.SOCK_STREAM) + self._socket.connect((self._host, self._port)) + self.logger.info(f"Connected to server at {self._host}:{self._port}") + except socket.error as e: + self.logger.error(f"Failed to connect to server: {str(e)}") + raise def send_message(self, message: str) -> None: """ @@ -40,9 +48,15 @@ def send_message(self, message: str) -> None: RuntimeError: If socket connection is not established. """ if not self._socket: + self.logger.error("Attempted to send message without connection") raise RuntimeError("Socket not set up. Call connect method first.") - self._socket.send(message.encode('utf-8')) + try: + self._socket.send(message.encode('utf-8')) + self.logger.info(f"Message sent: {message}") + except Exception as e: + self.logger.error(f"Failed to send message: {str(e)}") + raise def receive_message(self) -> None: """Continuously receive and process messages from the socket connection. @@ -56,15 +70,29 @@ def receive_message(self) -> None: UnicodeDecodeError: If received data cannot be decoded as UTF-8. """ if not self._socket: + self.logger.error("Attempted to receive message without connection") raise RuntimeError("Socket not set up. Call connect method first.") - while message_bytes := self._socket.recv(RECEIVE_BUFFER_SIZE): - if not message_bytes: - break - self._message_callback(message_bytes.decode('utf-8')) + self.logger.info("Starting message receive loop") + try: + while message_bytes := self._socket.recv(self._receive_buffer_size): + if not message_bytes: + self.logger.warning("Received empty message, breaking receive loop") + break + message = message_bytes.decode('utf-8') + self.logger.info(f"Received message: {message}") + self._message_callback(message) + except Exception as e: + self.logger.error(f"Error receiving message: {str(e)}") + raise def close(self) -> None: """Close the socket connection and clean up resources.""" if self._socket: - self._socket.close() - self._socket = None + try: + self._socket.close() + self.logger.info("Socket connection closed") + except Exception as e: + self.logger.error(f"Error closing socket: {str(e)}") + finally: + self._socket = None diff --git a/client/src/View.py b/client/src/View.py index 72b660f..d9a1530 100644 --- a/client/src/View.py +++ b/client/src/View.py @@ -1,25 +1,37 @@ import tkinter as tk -from tkinter import scrolledtext, ttk -from typing import Callable +from tkinter import scrolledtext, ttk, messagebox +from typing import Callable, Dict, List, Any import json +import os +from .Logger import setup_logger +from .ConfigManager import ConfigManager + class Viewer: """ Graphical user interface for the application. """ - def __init__(self, message_callback: Callable[[str], None]) -> None: + def __init__(self, config_manager: ConfigManager, message_callback: Callable[[str], None]) -> None: """ Initialize the viewer window and its components. Args: + config_manager: Configuration manager instance message_callback: Callback function to handle message sending. """ + self.logger = setup_logger(__name__) + self.logger.info("Initializing Viewer") + self.config_manager = config_manager + self.config = config_manager.get_config() + self._message_callback = message_callback + self.root: tk.Tk = tk.Tk() - self.root.title("Chat Application") + self.root.title("Site Blocker") self.root.geometry("800x600") - self._message_callback = message_callback + self._setup_ui() + self.logger.info("Viewer initialization complete") def _send_message(self) -> None: """Handle the sending of messages from the input field.""" @@ -32,63 +44,139 @@ def _send_message(self) -> None: def run(self) -> None: """Start the main event loop of the viewer.""" + self.logger.info("Starting main event loop") self.root.mainloop() def _setup_ui(self) -> None: - """Set up the UI components including text areas and buttons.""" - main_container = ttk.Frame(self.root, padding="5") + """Set up the UI components including block controls and domain list.""" + main_container = ttk.Frame(self.root, padding="10") main_container.grid(row=0, column=0, sticky=(tk.W, tk.E, tk.N, tk.S)) self.root.columnconfigure(0, weight=1) self.root.rowconfigure(0, weight=1) - self.message_area = scrolledtext.ScrolledText( - main_container, - wrap=tk.WORD, - width=70, - height=30 - ) - - self.message_area.grid(row=0, column=0, columnspan=2, sticky=(tk.W, tk.E, tk.N, tk.S)) - self.message_area.config(state=tk.DISABLED) + # Left side - Specific sites block + sites_frame = ttk.LabelFrame(main_container, text="Specific sites block", padding="5") + sites_frame.grid(row=0, column=0, rowspan=3, padx=5, sticky=(tk.W, tk.E, tk.N, tk.S)) + + # Domains listbox + self.domains_listbox = tk.Listbox(sites_frame, width=40, height=15) + self.domains_listbox.grid(row=0, column=0, pady=5, sticky=(tk.W, tk.E, tk.N, tk.S)) + + # Add domain entry + domain_entry_frame = ttk.Frame(sites_frame) + domain_entry_frame.grid(row=1, column=0, sticky=(tk.W, tk.E)) + + ttk.Label(domain_entry_frame, text="Add Domain:").grid(row=0, column=0, padx=5) + self.domain_entry = ttk.Entry(domain_entry_frame) + self.domain_entry.grid(row=0, column=1, padx=5, sticky=(tk.W, tk.E)) + + # Add buttons for domain management + button_frame = ttk.Frame(sites_frame) + button_frame.grid(row=2, column=0, pady=5, sticky=(tk.W, tk.E)) + + ttk.Button(button_frame, text="Add", command=self._add_domain).grid(row=0, column=0, padx=5) + ttk.Button(button_frame, text="Remove", command=self._remove_domain).grid(row=0, column=1, padx=5) + + # Bind double-click event for removing domains + self.domains_listbox.bind('', lambda e: self._remove_domain()) + + # Load saved domains into listbox + for domain in self.config["blocked_domains"].keys(): + self.domains_listbox.insert(tk.END, domain) + + # Ad Block controls + ad_frame = ttk.LabelFrame(main_container, text="Ad Block", padding="5") + ad_frame.grid(row=0, column=1, pady=10, sticky=(tk.W, tk.E)) + + self.ad_var = tk.StringVar(value=self.config["settings"]["ad_block"]) + ttk.Radiobutton(ad_frame, text="on", value="on", variable=self.ad_var).grid(row=0, column=0, padx=10) + ttk.Radiobutton(ad_frame, text="off", value="off", variable=self.ad_var).grid(row=0, column=1, padx=10) + + # Adult sites Block controls + adult_frame = ttk.LabelFrame(main_container, text="Adult sites Block", padding="5") + adult_frame.grid(row=1, column=1, pady=10, sticky=(tk.W, tk.E)) + + self.adult_var = tk.StringVar(value=self.config["settings"]["adult_block"]) + ttk.Radiobutton(adult_frame, text="on", value="on", variable=self.adult_var).grid(row=0, column=0, padx=10) + ttk.Radiobutton(adult_frame, text="off", value="off", variable=self.adult_var).grid(row=0, column=1, padx=10) + + # Bind radio button commands + self.ad_var.trace_add('write', lambda *args: self._handle_ad_block()) + self.adult_var.trace_add('write', lambda *args: self._handle_adult_block()) + + # Configure grid weights + main_container.columnconfigure(0, weight=1) + sites_frame.columnconfigure(0, weight=1) + domain_entry_frame.columnconfigure(1, weight=1) + + def _add_domain(self) -> None: + """Add a domain to the blocked sites list.""" + domain = self.domain_entry.get().strip() + if domain: + if domain not in self.config["blocked_domains"]: + self.domains_listbox.insert(tk.END, domain) + self.config["blocked_domains"][domain] = True + self.domain_entry.delete(0, tk.END) + self.config_manager.save_config(self.config) + self.logger.info(f"Domain added: {domain}") + else: + self.logger.warning(f"Attempted to add duplicate domain: {domain}") + self._show_error("Domain already exists in the list") - self.input_field = ttk.Entry(main_container) - self.input_field.grid(row=1, column=0, sticky=(tk.W, tk.E)) - self.input_field.bind("", lambda e: self._send_message()) + def _remove_domain(self) -> None: + """Remove the selected domain from the blocked sites list.""" + selection = self.domains_listbox.curselection() + if selection: + domain = self.domains_listbox.get(selection) + self.domains_listbox.delete(selection) + del self.config["blocked_domains"][domain] + self.config_manager.save_config(self.config) + self.logger.info(f"Domain removed: {domain}") + else: + self.logger.warning("Attempted to remove domain without selection") + self._show_error("Please select a domain to remove") - self.send_button = ttk.Button( - main_container, - text="Send", - command=self._send_message - ) - self.send_button.grid(row=1, column=1) + def _handle_ad_block(self) -> None: + """Handle changes to the ad block setting.""" + state = self.ad_var.get() + self.config["settings"]["ad_block"] = state + self.config_manager.save_config(self.config) + self.logger.info(f"Ad blocking state changed to: {state}") - main_container.columnconfigure(0, weight=3) - main_container.columnconfigure(1, weight=1) - main_container.rowconfigure(0, weight=1) + def _handle_adult_block(self) -> None: + """Handle changes to the adult sites block setting.""" + state = self.adult_var.get() + self.config["settings"]["adult_block"] = state + self.config_manager.save_config(self.config) + self.logger.info(f"Adult site blocking state changed to: {state}") - ## TODO: This method won't be relevant for the final version - def display_message(self, sender: str, message: str) -> None: + def _show_error(self, message: str) -> None: """ - Display a message in the message area. + Display an error message in a popup window. Args: - sender: The name of the message sender. - message: The message content to display. + message: The error message to display. """ - self.message_area.config(state=tk.NORMAL) - self.message_area.insert(tk.END, f"{sender}: {message}\n") - self.message_area.see(tk.END) - self.message_area.config(state=tk.DISABLED) + self.logger.error(f"Error message displayed: {message}") + tk.messagebox.showerror("Error", message) - ## TODO: This method won't be relevant for the final version - def display_error(self, error_message: str) -> None: + def get_blocked_domains(self) -> tuple[str, ...]: """ - Display an error message in the message area. + Get the list of currently blocked domains. - Args: - error_message: The error message to display. + Returns: + A tuple containing all blocked domains. + """ + return self.domains_listbox.get(0, tk.END) + + def get_block_settings(self) -> dict[str, str]: + """ + Get the current state of blocking settings. + + Returns: + A dictionary containing the current state of ad and adult content blocking. """ - self.message_area.config(state=tk.NORMAL) - self.message_area.insert(tk.END, f"Error: {error_message}\n") - self.message_area.see(tk.END) - self.message_area.config(state=tk.DISABLED) + return { + "ad_block": self.ad_var.get(), + "adult_block": self.adult_var.get() + } From 1dce5099a7e96fa0f1c964ea8552a6d25a9fbc5d Mon Sep 17 00:00:00 2001 From: elipaz Date: Tue, 5 Nov 2024 17:49:14 +0200 Subject: [PATCH 09/13] Add Update domain list func and Thread safety messures --- client/src/Application.py | 14 ++- client/src/View.py | 179 ++++++++++++++++++++++---------------- 2 files changed, 115 insertions(+), 78 deletions(-) diff --git a/client/src/Application.py b/client/src/Application.py index 8a06a97..054ce79 100644 --- a/client/src/Application.py +++ b/client/src/Application.py @@ -21,6 +21,8 @@ def __init__(self) -> None: """Initialize application components.""" self._logger = setup_logger(__name__) self._config_manager = ConfigManager() + self._request_lock = threading.Lock() + self._view = Viewer(config_manager=self._config_manager, message_callback=self._handle_request) self._communicator = Communicator(config_manager=self._config_manager, message_callback=self._handle_request) @@ -77,8 +79,16 @@ def _handle_request(self, request: str) -> None: try: self._logger.info(f"Processing request: {request}") - pass ## TODO: Implement request handling from server or UI. - + with self._request_lock: + match request["CODE"]: + case "50" | "51" | "52": + self._communicator.send_message(request) + case "53": + if not isinstance(request["content"], list): + self._logger.error("Invalid content format for domain list update") + return + self._view.update_domain_list(request["content"]) + except json.JSONDecodeError as e: self._logger.error(f"Invalid JSON format: {str(e)}") raise diff --git a/client/src/View.py b/client/src/View.py index d9a1530..bc2926b 100644 --- a/client/src/View.py +++ b/client/src/View.py @@ -33,6 +33,51 @@ def __init__(self, config_manager: ConfigManager, message_callback: Callable[[st self._setup_ui() self.logger.info("Viewer initialization complete") + def run(self) -> None: + """Start the main event loop of the viewer.""" + self.logger.info("Starting main event loop") + self.root.mainloop() + + def get_blocked_domains(self) -> tuple[str, ...]: + """ + Get the list of currently blocked domains. + + Returns: + A tuple containing all blocked domains. + """ + return self.domains_listbox.get(0, tk.END) + + def get_block_settings(self) -> dict[str, str]: + """ + Get the current state of blocking settings. + + Returns: + A dictionary containing the current state of ad and adult content blocking. + """ + return { + "ad_block": self.ad_var.get(), + "adult_block": self.adult_var.get() + } + + def update_domain_list(self, domains: List[str]) -> None: + """ + Update the domains listbox with a new list of domains from the server. + + Args: + domains: List of domain strings to be displayed in the listbox. + """ + self.logger.info("Updating domain list from server") + try: + self.domains_listbox.delete(0, tk.END) + + for domain in domains: + self.domains_listbox.insert(tk.END, domain) + + self.logger.info(f"Updated domain list with {len(domains)} domains") + except Exception as e: + self.logger.error(f"Error updating domain list: {str(e)}") + self._show_error("Failed to update domain list") + def _send_message(self) -> None: """Handle the sending of messages from the input field.""" message = self.input_field.get().strip() @@ -42,10 +87,64 @@ def _send_message(self) -> None: self.input_field.delete(0, tk.END) self.display_message("You", message) - def run(self) -> None: - """Start the main event loop of the viewer.""" - self.logger.info("Starting main event loop") - self.root.mainloop() + def _add_domain(self) -> None: + """Add a domain to the blocked sites list.""" + domain = self.domain_entry.get().strip() + if domain: + if domain not in self.config["blocked_domains"]: + self.domains_listbox.insert(tk.END, domain) + self.config["blocked_domains"][domain] = True + self.domain_entry.delete(0, tk.END) + self.config_manager.save_config(self.config) + self.logger.info(f"Domain added: {domain}") + + self._message_callback(json.dumps({"CODE": "53", "content": domain})) + else: + self.logger.warning(f"Attempted to add duplicate domain: {domain}") + self._show_error("Domain already exists in the list") + + def _remove_domain(self) -> None: + """Remove the selected domain from the blocked sites list.""" + selection = self.domains_listbox.curselection() + if selection: + domain = self.domains_listbox.get(selection) + self.domains_listbox.delete(selection) + del self.config["blocked_domains"][domain] + self.config_manager.save_config(self.config) + self.logger.info(f"Domain removed: {domain}") + + self._message_callback(json.dumps({"CODE": "54", "content": domain})) + else: + self.logger.warning("Attempted to remove domain without selection") + self._show_error("Please select a domain to remove") + + def _handle_ad_block(self) -> None: + """Handle changes to the ad block setting.""" + state = self.ad_var.get() + self.config["settings"]["ad_block"] = state + self.config_manager.save_config(self.config) + self.logger.info(f"Ad blocking state changed to: {state}") + + self._message_callback(json.dumps({"CODE": "50", "content": state})) + + def _handle_adult_block(self) -> None: + """Handle changes to the adult sites block setting.""" + state = self.adult_var.get() + self.config["settings"]["adult_block"] = state + self.config_manager.save_config(self.config) + self.logger.info(f"Adult site blocking state changed to: {state}") + + self._message_callback(json.dumps({"CODE": "51", "content": state})) + + def _show_error(self, message: str) -> None: + """ + Display an error message in a popup window. + + Args: + message: The error message to display. + """ + self.logger.error(f"Error message displayed: {message}") + tk.messagebox.showerror("Error", message) def _setup_ui(self) -> None: """Set up the UI components including block controls and domain list.""" @@ -108,75 +207,3 @@ def _setup_ui(self) -> None: main_container.columnconfigure(0, weight=1) sites_frame.columnconfigure(0, weight=1) domain_entry_frame.columnconfigure(1, weight=1) - - def _add_domain(self) -> None: - """Add a domain to the blocked sites list.""" - domain = self.domain_entry.get().strip() - if domain: - if domain not in self.config["blocked_domains"]: - self.domains_listbox.insert(tk.END, domain) - self.config["blocked_domains"][domain] = True - self.domain_entry.delete(0, tk.END) - self.config_manager.save_config(self.config) - self.logger.info(f"Domain added: {domain}") - else: - self.logger.warning(f"Attempted to add duplicate domain: {domain}") - self._show_error("Domain already exists in the list") - - def _remove_domain(self) -> None: - """Remove the selected domain from the blocked sites list.""" - selection = self.domains_listbox.curselection() - if selection: - domain = self.domains_listbox.get(selection) - self.domains_listbox.delete(selection) - del self.config["blocked_domains"][domain] - self.config_manager.save_config(self.config) - self.logger.info(f"Domain removed: {domain}") - else: - self.logger.warning("Attempted to remove domain without selection") - self._show_error("Please select a domain to remove") - - def _handle_ad_block(self) -> None: - """Handle changes to the ad block setting.""" - state = self.ad_var.get() - self.config["settings"]["ad_block"] = state - self.config_manager.save_config(self.config) - self.logger.info(f"Ad blocking state changed to: {state}") - - def _handle_adult_block(self) -> None: - """Handle changes to the adult sites block setting.""" - state = self.adult_var.get() - self.config["settings"]["adult_block"] = state - self.config_manager.save_config(self.config) - self.logger.info(f"Adult site blocking state changed to: {state}") - - def _show_error(self, message: str) -> None: - """ - Display an error message in a popup window. - - Args: - message: The error message to display. - """ - self.logger.error(f"Error message displayed: {message}") - tk.messagebox.showerror("Error", message) - - def get_blocked_domains(self) -> tuple[str, ...]: - """ - Get the list of currently blocked domains. - - Returns: - A tuple containing all blocked domains. - """ - return self.domains_listbox.get(0, tk.END) - - def get_block_settings(self) -> dict[str, str]: - """ - Get the current state of blocking settings. - - Returns: - A dictionary containing the current state of ad and adult content blocking. - """ - return { - "ad_block": self.ad_var.get(), - "adult_block": self.adult_var.get() - } From f82015fb8340eaa9d79ecf6b92252df38b793cbc Mon Sep 17 00:00:00 2001 From: elipaz Date: Wed, 6 Nov 2024 13:44:51 +0200 Subject: [PATCH 10/13] Add utlis and consts --- client/src/Application.py | 22 ++++--- client/src/Communicator.py | 25 +++++--- client/src/ConfigManager.py | 17 +----- client/src/Logger.py | 6 +- client/src/View.py | 116 ++++++++++++++++++++++-------------- client/src/utils.py | 64 ++++++++++++++++++++ 6 files changed, 168 insertions(+), 82 deletions(-) create mode 100644 client/src/utils.py diff --git a/client/src/Application.py b/client/src/Application.py index 054ce79..9108050 100644 --- a/client/src/Application.py +++ b/client/src/Application.py @@ -5,6 +5,11 @@ from .Logger import setup_logger from .ConfigManager import ConfigManager +from .utils import ( + STR_CODE, STR_CONTENT, + Codes +) + class Application: """ Main application class that coordinates communication between UI and server. @@ -78,16 +83,17 @@ def _handle_request(self, request: str) -> None: """ try: self._logger.info(f"Processing request: {request}") + request_dict = json.loads(request) with self._request_lock: - match request["CODE"]: - case "50" | "51" | "52": - self._communicator.send_message(request) - case "53": - if not isinstance(request["content"], list): - self._logger.error("Invalid content format for domain list update") - return - self._view.update_domain_list(request["content"]) + match request_dict[STR_CODE]: + case Codes.CODE_AD_BLOCK | \ + Codes.CODE_ADULT_BLOCK | \ + Codes.CODE_ADD_DOMAIN | \ + Codes.CODE_REMOVE_DOMAIN: + self._communicator.send_message(json.dumps(request)) + case Codes.CODE_DOMAIN_LIST_UPDATE: + self._view.update_domain_list(request_dict[STR_CONTENT]) except json.JSONDecodeError as e: self._logger.error(f"Invalid JSON format: {str(e)}") diff --git a/client/src/Communicator.py b/client/src/Communicator.py index 0590ace..e6275b7 100644 --- a/client/src/Communicator.py +++ b/client/src/Communicator.py @@ -2,6 +2,11 @@ from typing import Optional, Callable import json from .Logger import setup_logger +from .utils import ( + DEFAULT_HOST, DEFAULT_PORT, DEFAULT_BUFFER_SIZE, + ERR_SOCKET_NOT_SETUP, + STR_NETWORK +) class Communicator: def __init__(self, config_manager, message_callback: Callable[[str], None]) -> None: @@ -17,9 +22,9 @@ def __init__(self, config_manager, message_callback: Callable[[str], None]) -> N self.config = config_manager.get_config() self._message_callback = message_callback - self._host = self.config["network"]["host"] - self._port = self.config["network"]["port"] - self._receive_buffer_size = self.config["network"]["receive_buffer_size"] + self._host = self.config[STR_NETWORK][DEFAULT_HOST] + self._port = self.config[STR_NETWORK][DEFAULT_PORT] + self._receive_buffer_size = self.config[STR_NETWORK][DEFAULT_BUFFER_SIZE] self._socket: Optional[socket.socket] = None def connect(self) -> None: @@ -47,9 +52,7 @@ def send_message(self, message: str) -> None: Raises: RuntimeError: If socket connection is not established. """ - if not self._socket: - self.logger.error("Attempted to send message without connection") - raise RuntimeError("Socket not set up. Call connect method first.") + self._validate_connection() try: self._socket.send(message.encode('utf-8')) @@ -69,9 +72,7 @@ def receive_message(self) -> None: socket.error: If there's an error receiving data from the socket. UnicodeDecodeError: If received data cannot be decoded as UTF-8. """ - if not self._socket: - self.logger.error("Attempted to receive message without connection") - raise RuntimeError("Socket not set up. Call connect method first.") + self._validate_connection() self.logger.info("Starting message receive loop") try: @@ -96,3 +97,9 @@ def close(self) -> None: self.logger.error(f"Error closing socket: {str(e)}") finally: self._socket = None + + def _validate_connection(self) -> None: + """Validate the socket connection.""" + if not self._socket: + self.logger.error(ERR_SOCKET_NOT_SETUP) + raise RuntimeError(ERR_SOCKET_NOT_SETUP) diff --git a/client/src/ConfigManager.py b/client/src/ConfigManager.py index f7fa92a..62e67df 100644 --- a/client/src/ConfigManager.py +++ b/client/src/ConfigManager.py @@ -4,23 +4,8 @@ import os from typing import Dict, Any from .Logger import setup_logger +from .utils import DEFAULT_CONFIG -DEFAULT_CONFIG = { - "network": { - "host": "127.0.0.1", - "port": 65432, - "receive_buffer_size": 1024 - }, - "blocked_domains": {}, - "settings": { - "ad_block": "off", - "adult_block": "off" - }, - "logging": { - "level": "INFO", - "log_dir": "client_logs" - } -} class ConfigManager: """Manages application configuration loading and saving.""" diff --git a/client/src/Logger.py b/client/src/Logger.py index 034d9b7..a6c5709 100644 --- a/client/src/Logger.py +++ b/client/src/Logger.py @@ -4,8 +4,8 @@ import os from datetime import datetime from typing import Optional +from .utils import LOG_DIR, LOG_FORMAT, LOG_DATE_FORMAT -LOG_DIR = "client_logs" _logger: Optional[logging.Logger] = None def setup_logger(name: str) -> logging.Logger: @@ -27,12 +27,12 @@ def setup_logger(name: str) -> logging.Logger: os.makedirs(LOG_DIR) log_file: str = os.path.join( - LOG_DIR, f"Client_{datetime.now().strftime('%Y%m%d_%H%M%S')}.log" + LOG_DIR, f"Client_{datetime.now().strftime(LOG_DATE_FORMAT)}.log" ) logging.basicConfig( level=logging.INFO, - format="%(asctime)s - %(name)s - %(levelname)s - %(message)s", + format=LOG_FORMAT, handlers=[ logging.FileHandler(log_file), logging.StreamHandler(), diff --git a/client/src/View.py b/client/src/View.py index bc2926b..6037bb6 100644 --- a/client/src/View.py +++ b/client/src/View.py @@ -1,11 +1,19 @@ import tkinter as tk -from tkinter import scrolledtext, ttk, messagebox -from typing import Callable, Dict, List, Any +from tkinter import ttk, messagebox +from typing import Callable, List import json -import os +import threading from .Logger import setup_logger from .ConfigManager import ConfigManager +from .utils import ( + Codes, + WINDOW_SIZE, WINDOW_TITLE, + ERR_DUPLICATE_DOMAIN, ERR_NO_DOMAIN_SELECTED, ERR_DOMAIN_LIST_UPDATE_FAILED, + STR_AD_BLOCK, STR_ADULT_BLOCK, STR_CODE, + STR_BLOCKED_DOMAINS, STR_CONTENT, STR_SETTINGS, STR_ERROR +) + class Viewer: """ @@ -25,10 +33,12 @@ def __init__(self, config_manager: ConfigManager, message_callback: Callable[[st self.config_manager = config_manager self.config = config_manager.get_config() self._message_callback = message_callback + self._update_list_lock = threading.Lock() + self.root: tk.Tk = tk.Tk() - self.root.title("Site Blocker") - self.root.geometry("800x600") + self.root.title(WINDOW_TITLE) + self.root.geometry(WINDOW_SIZE) self._setup_ui() self.logger.info("Viewer initialization complete") @@ -55,8 +65,8 @@ def get_block_settings(self) -> dict[str, str]: A dictionary containing the current state of ad and adult content blocking. """ return { - "ad_block": self.ad_var.get(), - "adult_block": self.adult_var.get() + STR_AD_BLOCK: self.ad_var.get(), + STR_ADULT_BLOCK: self.adult_var.get() } def update_domain_list(self, domains: List[str]) -> None: @@ -66,75 +76,89 @@ def update_domain_list(self, domains: List[str]) -> None: Args: domains: List of domain strings to be displayed in the listbox. """ - self.logger.info("Updating domain list from server") - try: - self.domains_listbox.delete(0, tk.END) - - for domain in domains: - self.domains_listbox.insert(tk.END, domain) + with self._update_list_lock: + self.logger.info("Updating domain list from server") + + try: + self.domains_listbox.delete(0, tk.END) - self.logger.info(f"Updated domain list with {len(domains)} domains") - except Exception as e: - self.logger.error(f"Error updating domain list: {str(e)}") - self._show_error("Failed to update domain list") - - def _send_message(self) -> None: - """Handle the sending of messages from the input field.""" - message = self.input_field.get().strip() - if message: - message_json = json.dumps({"CODE": "100", "content": message}) - self._message_callback(message_json) - self.input_field.delete(0, tk.END) - self.display_message("You", message) + for domain in domains: + self.domains_listbox.insert(tk.END, domain) + + self.logger.info(f"Updated domain list with {len(domains)} domains") + + except Exception as e: + self.logger.error(f"Error updating domain list: {str(e)}") + self._show_error(ERR_DOMAIN_LIST_UPDATE_FAILED) def _add_domain(self) -> None: """Add a domain to the blocked sites list.""" domain = self.domain_entry.get().strip() + if domain: - if domain not in self.config["blocked_domains"]: + if domain not in self.config[STR_BLOCKED_DOMAINS]: self.domains_listbox.insert(tk.END, domain) - self.config["blocked_domains"][domain] = True self.domain_entry.delete(0, tk.END) + + self.config[STR_BLOCKED_DOMAINS][domain] = True self.config_manager.save_config(self.config) + + self._message_callback(json.dumps({ + STR_CODE: Codes.CODE_ADD_DOMAIN, + STR_CONTENT: domain + })) + self.logger.info(f"Domain added: {domain}") - - self._message_callback(json.dumps({"CODE": "53", "content": domain})) else: self.logger.warning(f"Attempted to add duplicate domain: {domain}") - self._show_error("Domain already exists in the list") + self._show_error(ERR_DUPLICATE_DOMAIN) def _remove_domain(self) -> None: """Remove the selected domain from the blocked sites list.""" selection = self.domains_listbox.curselection() + if selection: domain = self.domains_listbox.get(selection) self.domains_listbox.delete(selection) - del self.config["blocked_domains"][domain] + + del self.config[STR_BLOCKED_DOMAINS][domain] self.config_manager.save_config(self.config) - self.logger.info(f"Domain removed: {domain}") - self._message_callback(json.dumps({"CODE": "54", "content": domain})) + self._message_callback(json.dumps({ + STR_CODE: Codes.CODE_REMOVE_DOMAIN, + STR_CONTENT: domain + })) + + self.logger.info(f"Domain removed: {domain}") else: self.logger.warning("Attempted to remove domain without selection") - self._show_error("Please select a domain to remove") + self._show_error(ERR_NO_DOMAIN_SELECTED) def _handle_ad_block(self) -> None: """Handle changes to the ad block setting.""" state = self.ad_var.get() - self.config["settings"]["ad_block"] = state + self.config[STR_SETTINGS][STR_AD_BLOCK] = state self.config_manager.save_config(self.config) - self.logger.info(f"Ad blocking state changed to: {state}") - self._message_callback(json.dumps({"CODE": "50", "content": state})) + self._message_callback(json.dumps({ + STR_CODE: Codes.CODE_AD_BLOCK, + STR_CONTENT: state + })) + + self.logger.info(f"Ad blocking state changed to: {state}") def _handle_adult_block(self) -> None: """Handle changes to the adult sites block setting.""" state = self.adult_var.get() - self.config["settings"]["adult_block"] = state + self.config[STR_SETTINGS][STR_ADULT_BLOCK] = state self.config_manager.save_config(self.config) - self.logger.info(f"Adult site blocking state changed to: {state}") - self._message_callback(json.dumps({"CODE": "51", "content": state})) + self._message_callback(json.dumps({ + STR_CODE: Codes.CODE_ADULT_BLOCK, + STR_CONTENT: state + })) + + self.logger.info(f"Adult site blocking state changed to: {state}") def _show_error(self, message: str) -> None: """ @@ -144,7 +168,7 @@ def _show_error(self, message: str) -> None: message: The error message to display. """ self.logger.error(f"Error message displayed: {message}") - tk.messagebox.showerror("Error", message) + tk.messagebox.showerror(STR_ERROR, message) def _setup_ui(self) -> None: """Set up the UI components including block controls and domain list.""" @@ -180,14 +204,14 @@ def _setup_ui(self) -> None: self.domains_listbox.bind('', lambda e: self._remove_domain()) # Load saved domains into listbox - for domain in self.config["blocked_domains"].keys(): + for domain in self.config[STR_BLOCKED_DOMAINS].keys(): self.domains_listbox.insert(tk.END, domain) - + # Ad Block controls ad_frame = ttk.LabelFrame(main_container, text="Ad Block", padding="5") ad_frame.grid(row=0, column=1, pady=10, sticky=(tk.W, tk.E)) - self.ad_var = tk.StringVar(value=self.config["settings"]["ad_block"]) + self.ad_var = tk.StringVar(value=self.config[STR_SETTINGS][STR_AD_BLOCK]) ttk.Radiobutton(ad_frame, text="on", value="on", variable=self.ad_var).grid(row=0, column=0, padx=10) ttk.Radiobutton(ad_frame, text="off", value="off", variable=self.ad_var).grid(row=0, column=1, padx=10) @@ -195,7 +219,7 @@ def _setup_ui(self) -> None: adult_frame = ttk.LabelFrame(main_container, text="Adult sites Block", padding="5") adult_frame.grid(row=1, column=1, pady=10, sticky=(tk.W, tk.E)) - self.adult_var = tk.StringVar(value=self.config["settings"]["adult_block"]) + self.adult_var = tk.StringVar(value=self.config[STR_SETTINGS][STR_ADULT_BLOCK]) ttk.Radiobutton(adult_frame, text="on", value="on", variable=self.adult_var).grid(row=0, column=0, padx=10) ttk.Radiobutton(adult_frame, text="off", value="off", variable=self.adult_var).grid(row=0, column=1, padx=10) diff --git a/client/src/utils.py b/client/src/utils.py new file mode 100644 index 0000000..31b59f2 --- /dev/null +++ b/client/src/utils.py @@ -0,0 +1,64 @@ +"""Utility module containing constants and common functions for the application.""" + +# Network related constants +DEFAULT_HOST = "host" +DEFAULT_PORT = "port" +DEFAULT_BUFFER_SIZE = "receive_buffer_size" + +# GUI constants +WINDOW_TITLE = "Site Blocker" +WINDOW_SIZE = "800x600" +PADDING_SMALL = "5" +PADDING_MEDIUM = "10" + +# Message codes +class Codes: + """Constants for message codes used in communication.""" + CODE_AD_BLOCK = "50" + CODE_ADULT_BLOCK = "51" + CODE_ADD_DOMAIN = "52" + CODE_REMOVE_DOMAIN = "53" + CODE_DOMAIN_LIST_UPDATE = "54" + +# Default settings +DEFAULT_CONFIG = { + "network": { + "host": DEFAULT_HOST, + "port": DEFAULT_PORT, + "receive_buffer_size": DEFAULT_BUFFER_SIZE + }, + "blocked_domains": {}, + "settings": { + "ad_block": "off", + "adult_block": "off" + }, + "logging": { + "level": "INFO", + "log_dir": "client_logs" + } +} + +# Logging constants +LOG_DIR = "client_logs" +LOG_FORMAT = "%(asctime)s - %(name)s - %(levelname)s - %(message)s" +LOG_DATE_FORMAT = "%Y%m%d_%H%M%S" + +# Error messages +ERR_SOCKET_NOT_SETUP = "Socket not set up. Call connect method first." +ERR_NO_CONNECTION = "Attempted to send message without connection" +ERR_DUPLICATE_DOMAIN = "Domain already exists in the list" +ERR_NO_DOMAIN_SELECTED = "Please select a domain to remove" +ERR_DOMAIN_LIST_UPDATE_FAILED = "Failed to update domain list" + +# String Constants +STR_AD_BLOCK = "ad_block" +STR_ADULT_BLOCK = "adult_block" +STR_CODE = "code" +STR_CONTENT = "content" +STR_ERROR = "Error" + +# Config Constants +STR_BLOCKED_DOMAINS = "blocked_domains" +STR_NETWORK = "network" +STR_SETTINGS = "settings" +STR_LOGGING = "logging" From 957850bd222760984ad6b5af77e2d3a86955a60d Mon Sep 17 00:00:00 2001 From: elipaz Date: Wed, 6 Nov 2024 15:21:51 +0200 Subject: [PATCH 11/13] Finish without tests --- client/config.json | 7 +- client/src/Application.py | 2 +- client/src/View.py | 221 +++++++++++++++++++++++++++++++------- 3 files changed, 188 insertions(+), 42 deletions(-) diff --git a/client/config.json b/client/config.json index b662c6b..a0dfe4e 100644 --- a/client/config.json +++ b/client/config.json @@ -6,11 +6,12 @@ }, "blocked_domains": { "example.com": true, - "ads.example.com": true + "ads.example.com": true, + "fxp.co.il": true }, "settings": { - "ad_block": "on", - "adult_block": "on" + "ad_block": "off", + "adult_block": "off" }, "logging": { "level": "INFO", diff --git a/client/src/Application.py b/client/src/Application.py index 9108050..d4ec6bf 100644 --- a/client/src/Application.py +++ b/client/src/Application.py @@ -41,7 +41,7 @@ def run(self) -> None: self._logger.info("Starting application") try: - # self._start_communication() + self._start_communication() self._start_gui() except Exception as e: diff --git a/client/src/View.py b/client/src/View.py index 6037bb6..2943421 100644 --- a/client/src/View.py +++ b/client/src/View.py @@ -35,12 +35,25 @@ def __init__(self, config_manager: ConfigManager, message_callback: Callable[[st self._message_callback = message_callback self._update_list_lock = threading.Lock() - + # Initialize root window first self.root: tk.Tk = tk.Tk() self.root.title(WINDOW_TITLE) self.root.geometry(WINDOW_SIZE) + self.root.withdraw() # Hide the window temporarily + + # Configure styles + style = ttk.Style() + style.configure('TLabelframe', padding=10) + style.configure('TLabelframe.Label', font=('Arial', 10, 'bold')) + style.configure('TButton', padding=5) + style.configure('TRadiobutton', font=('Arial', 10)) + style.configure('TLabel', font=('Arial', 10)) + self._setup_ui() + + # Show the window after setup is complete + self.root.deiconify() self.logger.info("Viewer initialization complete") def run(self) -> None: @@ -172,62 +185,194 @@ def _show_error(self, message: str) -> None: def _setup_ui(self) -> None: """Set up the UI components including block controls and domain list.""" - main_container = ttk.Frame(self.root, padding="10") + # Main container with increased padding + main_container = ttk.Frame(self.root, padding="20") main_container.grid(row=0, column=0, sticky=(tk.W, tk.E, tk.N, tk.S)) self.root.columnconfigure(0, weight=1) self.root.rowconfigure(0, weight=1) - # Left side - Specific sites block - sites_frame = ttk.LabelFrame(main_container, text="Specific sites block", padding="5") - sites_frame.grid(row=0, column=0, rowspan=3, padx=5, sticky=(tk.W, tk.E, tk.N, tk.S)) + # Left side - Specific sites block (now with better proportions) + sites_frame = ttk.LabelFrame( + main_container, + text="Specific Sites Block", + padding="15" + ) + sites_frame.grid( + row=0, + column=0, + rowspan=3, + padx=10, + sticky=(tk.W, tk.E, tk.N, tk.S) + ) + + # Create a frame for listbox and scrollbar + listbox_frame = ttk.Frame(sites_frame) + listbox_frame.grid(row=0, column=0, pady=5, sticky=(tk.W, tk.E, tk.N, tk.S)) - # Domains listbox - self.domains_listbox = tk.Listbox(sites_frame, width=40, height=15) - self.domains_listbox.grid(row=0, column=0, pady=5, sticky=(tk.W, tk.E, tk.N, tk.S)) + # Domains listbox with scrollbars + self.domains_listbox = tk.Listbox( + listbox_frame, + width=40, + height=15, + selectmode=tk.SINGLE, + activestyle='dotbox', + font=('Arial', 10) + ) + scrollbar_y = ttk.Scrollbar( + listbox_frame, + orient=tk.VERTICAL, + command=self.domains_listbox.yview + ) + scrollbar_x = ttk.Scrollbar( + listbox_frame, + orient=tk.HORIZONTAL, + command=self.domains_listbox.xview + ) - # Add domain entry + self.domains_listbox.configure( + yscrollcommand=scrollbar_y.set, + xscrollcommand=scrollbar_x.set + ) + + # Grid layout for listbox and scrollbars + self.domains_listbox.grid(row=0, column=0, sticky=(tk.W, tk.E, tk.N, tk.S)) + scrollbar_y.grid(row=0, column=1, sticky=(tk.N, tk.S)) + scrollbar_x.grid(row=1, column=0, sticky=(tk.W, tk.E)) + + # Add domain entry with improved layout domain_entry_frame = ttk.Frame(sites_frame) - domain_entry_frame.grid(row=1, column=0, sticky=(tk.W, tk.E)) + domain_entry_frame.grid( + row=1, + column=0, + pady=15, + sticky=(tk.W, tk.E) + ) + + ttk.Label( + domain_entry_frame, + text="Add Domain:", + font=('Arial', 10) + ).grid(row=0, column=0, padx=5) - ttk.Label(domain_entry_frame, text="Add Domain:").grid(row=0, column=0, padx=5) - self.domain_entry = ttk.Entry(domain_entry_frame) - self.domain_entry.grid(row=0, column=1, padx=5, sticky=(tk.W, tk.E)) + self.domain_entry = ttk.Entry( + domain_entry_frame, + font=('Arial', 10) + ) + self.domain_entry.grid( + row=0, + column=1, + padx=5, + sticky=(tk.W, tk.E) + ) - # Add buttons for domain management + # Buttons with improved styling button_frame = ttk.Frame(sites_frame) - button_frame.grid(row=2, column=0, pady=5, sticky=(tk.W, tk.E)) + button_frame.grid( + row=2, + column=0, + pady=10, + sticky=(tk.W, tk.E) + ) - ttk.Button(button_frame, text="Add", command=self._add_domain).grid(row=0, column=0, padx=5) - ttk.Button(button_frame, text="Remove", command=self._remove_domain).grid(row=0, column=1, padx=5) + style = ttk.Style() + style.configure('Action.TButton', padding=5) - # Bind double-click event for removing domains - self.domains_listbox.bind('', lambda e: self._remove_domain()) + ttk.Button( + button_frame, + text="Add Domain", + style='Action.TButton', + command=self._add_domain + ).grid(row=0, column=0, padx=5) - # Load saved domains into listbox - for domain in self.config[STR_BLOCKED_DOMAINS].keys(): - self.domains_listbox.insert(tk.END, domain) + ttk.Button( + button_frame, + text="Remove Domain", + style='Action.TButton', + command=self._remove_domain + ).grid(row=0, column=1, padx=5) + + # Right side controls with improved spacing + controls_frame = ttk.Frame(main_container) + controls_frame.grid( + row=0, + column=1, + padx=20, + sticky=(tk.N, tk.S) + ) - # Ad Block controls - ad_frame = ttk.LabelFrame(main_container, text="Ad Block", padding="5") - ad_frame.grid(row=0, column=1, pady=10, sticky=(tk.W, tk.E)) + # Ad Block controls with better styling + ad_frame = ttk.LabelFrame( + controls_frame, + text="Ad Blocking", + padding="15" + ) + ad_frame.grid( + row=0, + column=0, + pady=10, + sticky=(tk.W, tk.E) + ) + # Initialize with config value self.ad_var = tk.StringVar(value=self.config[STR_SETTINGS][STR_AD_BLOCK]) - ttk.Radiobutton(ad_frame, text="on", value="on", variable=self.ad_var).grid(row=0, column=0, padx=10) - ttk.Radiobutton(ad_frame, text="off", value="off", variable=self.ad_var).grid(row=0, column=1, padx=10) + ttk.Radiobutton( + ad_frame, + text="Enable", + value="on", + variable=self.ad_var, + command=self._handle_ad_block + ).grid(row=0, column=0, padx=10) + ttk.Radiobutton( + ad_frame, + text="Disable", + value="off", + variable=self.ad_var, + command=self._handle_ad_block + ).grid(row=0, column=1, padx=10) # Adult sites Block controls - adult_frame = ttk.LabelFrame(main_container, text="Adult sites Block", padding="5") - adult_frame.grid(row=1, column=1, pady=10, sticky=(tk.W, tk.E)) + adult_frame = ttk.LabelFrame( + controls_frame, + text="Adult Content Blocking", + padding="15" + ) + adult_frame.grid( + row=1, + column=0, + pady=10, + sticky=(tk.W, tk.E) + ) + # Initialize with config value self.adult_var = tk.StringVar(value=self.config[STR_SETTINGS][STR_ADULT_BLOCK]) - ttk.Radiobutton(adult_frame, text="on", value="on", variable=self.adult_var).grid(row=0, column=0, padx=10) - ttk.Radiobutton(adult_frame, text="off", value="off", variable=self.adult_var).grid(row=0, column=1, padx=10) - - # Bind radio button commands - self.ad_var.trace_add('write', lambda *args: self._handle_ad_block()) - self.adult_var.trace_add('write', lambda *args: self._handle_adult_block()) - - # Configure grid weights - main_container.columnconfigure(0, weight=1) + ttk.Radiobutton( + adult_frame, + text="Enable", + value="on", + variable=self.adult_var, + command=self._handle_adult_block + ).grid(row=0, column=0, padx=10) + ttk.Radiobutton( + adult_frame, + text="Disable", + value="off", + variable=self.adult_var, + command=self._handle_adult_block + ).grid(row=0, column=1, padx=10) + + # Configure grid weights for better resizing + main_container.columnconfigure(0, weight=3) + main_container.columnconfigure(1, weight=1) sites_frame.columnconfigure(0, weight=1) + listbox_frame.columnconfigure(0, weight=1) + listbox_frame.rowconfigure(0, weight=1) domain_entry_frame.columnconfigure(1, weight=1) + button_frame.columnconfigure(0, weight=1) + button_frame.columnconfigure(1, weight=1) + + # Bind events + self.domains_listbox.bind('', lambda e: self._remove_domain()) + + # Load saved domains + for domain in self.config[STR_BLOCKED_DOMAINS].keys(): + self.domains_listbox.insert(tk.END, domain) From b0c04b626f09baa0dace19fb70902cc8189f7ce0 Mon Sep 17 00:00:00 2001 From: elipaz Date: Wed, 6 Nov 2024 15:56:48 +0200 Subject: [PATCH 12/13] Finish tests --- client/tests/test_application.py | 64 +++++++++-- client/tests/test_communicator.py | 73 ++++++++++--- client/tests/test_view.py | 175 +++++++++++++++++++++++------- 3 files changed, 241 insertions(+), 71 deletions(-) diff --git a/client/tests/test_application.py b/client/tests/test_application.py index ca9df06..f55c206 100644 --- a/client/tests/test_application.py +++ b/client/tests/test_application.py @@ -1,30 +1,36 @@ -import os import logging from unittest import mock -from datetime import datetime from typing import Optional, Callable +import json import pytest from src.Application import Application from src.View import Viewer from src.Communicator import Communicator +from src.utils import ( + STR_CODE, STR_CONTENT, + Codes, DEFAULT_CONFIG +) @pytest.fixture -def mock_callback() -> Callable[[str], None]: - """Fixture to provide a mock callback function.""" - return mock.Mock() +def mock_config_manager() -> mock.Mock: + """Fixture to provide a mock configuration manager.""" + config_manager = mock.Mock() + config_manager.get_config.return_value = DEFAULT_CONFIG + return config_manager @pytest.fixture -def application(mock_callback: Callable[[str], None]) -> Application: +def application(mock_config_manager: mock.Mock) -> Application: """Fixture to create an Application instance.""" with mock.patch('src.Application.Viewer') as mock_viewer, \ mock.patch('src.Application.Communicator') as mock_comm, \ mock.patch('src.Application.setup_logger') as mock_logger: app = Application() app._logger = mock.Mock() + app._config_manager = mock_config_manager return app @@ -33,10 +39,15 @@ def test_init(application: Application) -> None: assert hasattr(application, '_logger') assert hasattr(application, '_view') assert hasattr(application, '_communicator') + assert hasattr(application, '_request_lock') + assert hasattr(application, '_config_manager') @mock.patch('src.Application.threading.Thread') -def test_start_communication(mock_thread: mock.Mock, application: Application) -> None: +def test_start_communication( + mock_thread: mock.Mock, + application: Application +) -> None: """Test the communication startup.""" application._start_communication() @@ -54,13 +65,32 @@ def test_start_gui(application: Application) -> None: application._view.run.assert_called_once() -def test_handle_request(application: Application) -> None: - """Test request handling.""" - test_request = '{"type": "test", "content": "message"}' +def test_handle_request_ad_block(application: Application) -> None: + """Test handling ad block request.""" + test_request = { + STR_CODE: Codes.CODE_AD_BLOCK, + STR_CONTENT: "test" + } + + application._communicator.send_message = mock.Mock() + + application._handle_request(json.dumps(test_request)) + + actual_arg = application._communicator.send_message.call_args[0][0] + + assert json.loads(json.loads(actual_arg)) == test_request + + +def test_handle_request_domain_list_update(application: Application) -> None: + """Test handling domain list update request.""" + test_content = ["domain1.com", "domain2.com"] + test_request = json.dumps({ + STR_CODE: Codes.CODE_DOMAIN_LIST_UPDATE, + STR_CONTENT: test_content + }) - # Currently just testing logging as implementation is pending application._handle_request(test_request) - application._logger.debug.assert_called_once_with(f"Processing request: {test_request}") + application._view.update_domain_list.assert_called_once_with(test_content) def test_cleanup(application: Application) -> None: @@ -102,4 +132,14 @@ def test_run_exception(application: Application) -> None: exc_info=True ) mock_cleanup.assert_called_once() + + +def test_handle_request_json_error(application: Application) -> None: + """Test handling of invalid JSON in request.""" + invalid_json = "{" + + with pytest.raises(json.JSONDecodeError): + application._handle_request(invalid_json) + + application._logger.error.assert_called() \ No newline at end of file diff --git a/client/tests/test_communicator.py b/client/tests/test_communicator.py index 1ed8e81..2a761e4 100644 --- a/client/tests/test_communicator.py +++ b/client/tests/test_communicator.py @@ -4,7 +4,20 @@ import pytest -from src.Communicator import Communicator, HOST, PORT, RECEIVE_BUFFER_SIZE +from src.Communicator import Communicator +from src.utils import ( + DEFAULT_HOST, DEFAULT_PORT, DEFAULT_BUFFER_SIZE, + ERR_SOCKET_NOT_SETUP, STR_NETWORK, + DEFAULT_CONFIG +) + + +@pytest.fixture +def mock_config_manager() -> mock.Mock: + """Fixture to provide a mock configuration manager.""" + config_manager = mock.Mock() + config_manager.get_config.return_value = DEFAULT_CONFIG + return config_manager @pytest.fixture @@ -14,27 +27,45 @@ def mock_callback() -> Callable[[str], None]: @pytest.fixture -def communicator(mock_callback: Callable[[str], None]) -> Communicator: +def communicator( + mock_config_manager: mock.Mock, + mock_callback: Callable[[str], None] +) -> Communicator: """Fixture to create a Communicator instance.""" - return Communicator(message_callback=mock_callback) + return Communicator( + config_manager=mock_config_manager, + message_callback=mock_callback + ) -def test_init(communicator: Communicator, mock_callback: Callable[[str], None]) -> None: +def test_init( + communicator: Communicator, + mock_callback: Callable[[str], None] +) -> None: """Test the initialization of Communicator.""" - assert communicator._host == HOST - assert communicator._port == PORT + assert communicator._host == DEFAULT_CONFIG[STR_NETWORK][DEFAULT_HOST] + assert communicator._port == DEFAULT_CONFIG[STR_NETWORK][DEFAULT_PORT] + assert communicator._receive_buffer_size == DEFAULT_CONFIG[STR_NETWORK][DEFAULT_BUFFER_SIZE] assert communicator._socket is None assert communicator._message_callback == mock_callback @mock.patch('socket.socket') -def test_connect(mock_socket_class: mock.Mock, communicator: Communicator) -> None: +def test_connect( + mock_socket_class: mock.Mock, + communicator: Communicator +) -> None: """Test the connect method initializes and connects the socket.""" mock_socket_instance = mock_socket_class.return_value communicator.connect() - mock_socket_class.assert_called_once_with(socket.AF_INET, socket.SOCK_STREAM) - mock_socket_instance.connect.assert_called_once_with((HOST, PORT)) + mock_socket_class.assert_called_once_with( + socket.AF_INET, + socket.SOCK_STREAM + ) + mock_socket_instance.connect.assert_called_once_with( + (communicator._host, communicator._port) + ) assert communicator._socket is mock_socket_instance @@ -46,11 +77,14 @@ def test_send_message_without_setup( """Test sending a message without setting up the socket raises RuntimeError.""" with pytest.raises(RuntimeError) as exc_info: communicator.send_message("Hello") - assert str(exc_info.value) == "Socket not set up. Call connect method first." + assert str(exc_info.value) == ERR_SOCKET_NOT_SETUP @mock.patch('socket.socket') -def test_send_message(mock_socket_class: mock.Mock, communicator: Communicator) -> None: +def test_send_message( + mock_socket_class: mock.Mock, + communicator: Communicator +) -> None: """Test sending a message successfully.""" mock_socket_instance = mock_socket_class.return_value communicator._socket = mock_socket_instance @@ -58,7 +92,9 @@ def test_send_message(mock_socket_class: mock.Mock, communicator: Communicator) message: str = "Hello, World!" communicator.send_message(message) - mock_socket_instance.send.assert_called_once_with(message.encode('utf-8')) + mock_socket_instance.send.assert_called_once_with( + message.encode('utf-8') + ) @mock.patch('socket.socket') @@ -69,7 +105,7 @@ def test_receive_message_without_setup( """Test receiving a message without setting up the socket raises RuntimeError.""" with pytest.raises(RuntimeError) as exc_info: communicator.receive_message() - assert str(exc_info.value) == "Socket not set up. Call connect method first." + assert str(exc_info.value) == ERR_SOCKET_NOT_SETUP @mock.patch('socket.socket') @@ -82,17 +118,21 @@ def test_receive_message( mock_socket_instance = mock_socket_class.return_value communicator._socket = mock_socket_instance - # Setup mock to return a message once and then empty string to break the loop mock_socket_instance.recv.side_effect = [b'Hello, Client!', b''] communicator.receive_message() - mock_socket_instance.recv.assert_called_with(RECEIVE_BUFFER_SIZE) + mock_socket_instance.recv.assert_called_with( + DEFAULT_CONFIG[STR_NETWORK][DEFAULT_BUFFER_SIZE] + ) mock_callback.assert_called_once_with('Hello, Client!') @mock.patch('socket.socket') -def test_close_socket(mock_socket_class: mock.Mock, communicator: Communicator) -> None: +def test_close_socket( + mock_socket_class: mock.Mock, + communicator: Communicator +) -> None: """Test closing the socket.""" mock_socket_instance = mock_socket_class.return_value communicator._socket = mock_socket_instance @@ -113,7 +153,6 @@ def test_receive_message_decode_error( mock_socket_instance = mock_socket_class.return_value communicator._socket = mock_socket_instance - # Setup mock to return invalid UTF-8 bytes mock_socket_instance.recv.side_effect = [bytes([0xFF, 0xFE, 0xFD]), b''] with pytest.raises(UnicodeDecodeError): diff --git a/client/tests/test_view.py b/client/tests/test_view.py index 888e78b..f83f0b9 100644 --- a/client/tests/test_view.py +++ b/client/tests/test_view.py @@ -1,67 +1,158 @@ -import tkinter as tk +import pytest from unittest import mock import json from typing import Callable -import pytest - from src.View import Viewer +from src.utils import ( + Codes, STR_CODE, STR_CONTENT, + STR_SETTINGS, STR_AD_BLOCK, STR_ADULT_BLOCK, + STR_BLOCKED_DOMAINS, DEFAULT_CONFIG, + ERR_DUPLICATE_DOMAIN +) +@pytest.fixture +def mock_config_manager() -> mock.Mock: + """Fixture to provide a mock configuration manager.""" + config_manager = mock.Mock() + config_manager.get_config.return_value = DEFAULT_CONFIG.copy() + return config_manager @pytest.fixture def mock_callback() -> Callable[[str], None]: """Fixture to provide a mock callback function.""" return mock.Mock() - @pytest.fixture -def viewer(mock_callback: Callable[[str], None]) -> Viewer: - """Fixture to create a Viewer instance.""" - with mock.patch('src.View.tk.Tk') as mock_tk: - mock_tk.return_value.title = mock.Mock() - mock_tk.return_value.geometry = mock.Mock() - return Viewer(message_callback=mock_callback) - - -def test_init(viewer: Viewer, mock_callback: Callable[[str], None]) -> None: - """Test the initialization of Viewer.""" - viewer.root.title.assert_called_once_with("Chat Application") - viewer.root.geometry.assert_called_once_with("800x600") - assert viewer._message_callback == mock_callback +def viewer(mock_config_manager: mock.Mock, mock_callback: mock.Mock) -> Viewer: + """Fixture to create a Viewer instance with mocked components.""" + with mock.patch('tkinter.Tk') as mock_tk, \ + mock.patch('tkinter.ttk.Style'): + # Create a mock Tk instance + root = mock_tk.return_value + + # Set up the mock root properly + mock_tk._default_root = root + root._default_root = root + + # Create StringVar mock that returns string values + with mock.patch('tkinter.StringVar') as mock_string_var: + string_var_instance = mock.Mock() + string_var_instance.get.return_value = "off" + mock_string_var.return_value = string_var_instance + + # Create Entry and Listbox mocks + with mock.patch('tkinter.Entry') as mock_entry, \ + mock.patch('tkinter.Listbox') as mock_listbox: + + # Setup Entry mock + entry_instance = mock.Mock() + entry_instance.get.return_value = "" + mock_entry.return_value = entry_instance + + # Setup Listbox mock + listbox_instance = mock.Mock() + listbox_instance.curselection.return_value = () + listbox_instance.get.return_value = "" + mock_listbox.return_value = listbox_instance + + viewer = Viewer( + config_manager=mock_config_manager, + message_callback=mock_callback + ) + + # Store mock instances for easy access in tests + viewer.domain_entry = entry_instance + viewer.domains_listbox = listbox_instance + + # Mock the _show_error method + viewer._show_error = mock.Mock() + + return viewer +def test_get_block_settings(viewer: Viewer) -> None: + """Test getting block settings.""" + # Configure the mock StringVar to return specific values + viewer.ad_var.get.return_value = "off" + viewer.adult_var.get.return_value = "off" + + settings = viewer.get_block_settings() + assert STR_AD_BLOCK in settings + assert STR_ADULT_BLOCK in settings + assert isinstance(settings[STR_AD_BLOCK], str) + assert isinstance(settings[STR_ADULT_BLOCK], str) -def test_send_message(viewer: Viewer, mock_callback: Callable[[str], None]) -> None: - """Test sending a message.""" - test_message = "Hello, World!" - viewer.input_field = mock.Mock() - viewer.input_field.get.return_value = test_message +def test_handle_ad_block(viewer: Viewer) -> None: + """Test handling ad block setting changes.""" + # Configure the mock StringVar to return "on" + viewer.ad_var.get.return_value = "on" + viewer._handle_ad_block() - viewer._send_message() + expected_json = json.dumps({ + STR_CODE: Codes.CODE_AD_BLOCK, + STR_CONTENT: "on" + }) - expected_json = json.dumps({"CODE": "100", "content": test_message}) - mock_callback.assert_called_once_with(expected_json) - viewer.input_field.delete.assert_called_once_with(0, tk.END) + viewer._message_callback.assert_called_once_with(expected_json) + viewer.config_manager.save_config.assert_called_once_with(viewer.config) + assert viewer.config[STR_SETTINGS][STR_AD_BLOCK] == "on" +def test_handle_adult_block(viewer: Viewer) -> None: + """Test handling adult block setting changes.""" + # Configure the mock StringVar to return "on" + viewer.adult_var.get.return_value = "on" + viewer._handle_adult_block() + + expected_json = json.dumps({ + STR_CODE: Codes.CODE_ADULT_BLOCK, + STR_CONTENT: "on" + }) + + viewer._message_callback.assert_called_once_with(expected_json) + viewer.config_manager.save_config.assert_called_once_with(viewer.config) + assert viewer.config[STR_SETTINGS][STR_ADULT_BLOCK] == "on" -def test_display_message(viewer: Viewer) -> None: - """Test displaying a message.""" - viewer.message_area = mock.Mock() +def test_add_domain(viewer: Viewer) -> None: + """Test adding a domain.""" + domain = "test.com" + viewer.domain_entry.get.return_value = domain + viewer._add_domain() - viewer.display_message("User", "Test message") + expected_json = json.dumps({ + STR_CODE: Codes.CODE_ADD_DOMAIN, + STR_CONTENT: domain + }) - viewer.message_area.config.assert_any_call(state=tk.NORMAL) - viewer.message_area.insert.assert_called_once_with(tk.END, "User: Test message\n") - viewer.message_area.see.assert_called_once_with(tk.END) - viewer.message_area.config.assert_any_call(state=tk.DISABLED) + viewer._message_callback.assert_called_once_with(expected_json) + viewer.config_manager.save_config.assert_called_once_with(viewer.config) + assert viewer.config[STR_BLOCKED_DOMAINS][domain] is True +def test_add_duplicate_domain(viewer: Viewer) -> None: + """Test adding a duplicate domain.""" + domain = "test.com" + viewer.config[STR_BLOCKED_DOMAINS][domain] = True + viewer.domain_entry.get.return_value = domain + + viewer._add_domain() + + viewer._message_callback.assert_not_called() + viewer._show_error.assert_called_once_with(ERR_DUPLICATE_DOMAIN) + assert len(viewer.config[STR_BLOCKED_DOMAINS]) == 1 -def test_display_error(viewer: Viewer) -> None: - """Test displaying an error message.""" - viewer.message_area = mock.Mock() +def test_remove_domain(viewer: Viewer) -> None: + """Test removing a domain.""" + domain = "test.com" + viewer.config[STR_BLOCKED_DOMAINS][domain] = True + viewer.domains_listbox.curselection.return_value = (0,) + viewer.domains_listbox.get.return_value = domain + + viewer._remove_domain() - viewer.display_error("Test error") + expected_json = json.dumps({ + STR_CODE: Codes.CODE_REMOVE_DOMAIN, + STR_CONTENT: domain + }) - viewer.message_area.config.assert_any_call(state=tk.NORMAL) - viewer.message_area.insert.assert_called_once_with(tk.END, "Error: Test error\n") - viewer.message_area.see.assert_called_once_with(tk.END) - viewer.message_area.config.assert_any_call(state=tk.DISABLED) + viewer._message_callback.assert_called_once_with(expected_json) + viewer.config_manager.save_config.assert_called_once_with(viewer.config) + assert domain not in viewer.config[STR_BLOCKED_DOMAINS] From 22bdea473534787bceaccd0fb63554de9c745036 Mon Sep 17 00:00:00 2001 From: elipaz Date: Wed, 6 Nov 2024 15:59:31 +0200 Subject: [PATCH 13/13] Update requirments --- client/requirments.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/requirments.txt b/client/requirments.txt index ea7c08e..0a08159 100644 --- a/client/requirments.txt +++ b/client/requirments.txt @@ -1,4 +1,4 @@ --e git+https://github.com/pazMenachem/My_Internet.git@94c2d2bea3c2ad912444fd3cba2ea4a4e9bf6daf#egg=client&subdirectory=client +-e git+https://github.com/pazMenachem/My_Internet.git@b0c04b626f09baa0dace19fb70902cc8189f7ce0#egg=client&subdirectory=client colorama==0.4.6 exceptiongroup==1.2.2 iniconfig==2.0.0