From 2aeb9e059b1b8afba1c4da4efc39c82a69aaba8d Mon Sep 17 00:00:00 2001 From: Henrique Joaquim Date: Mon, 4 Sep 2023 10:14:00 +0100 Subject: [PATCH] Services refactor (#5396) * removing unused service related code * unsued * rename from managers to services * changing the way we initialize and use services * tests * init in the top * making system service a singleton * diferentiating between logging app/subapp * user service as a singleton * ruff * avoid new instances of CommandRunner * ruff * elif instead * merge conflict, bad import * rename method to fix test * fixing tests * @montezdesousa suggestion --------- Co-authored-by: montezdesousa --- .../core/openbb_core/api/dependency/system.py | 12 +--- .../core/openbb_core/api/dependency/user.py | 8 +-- .../core/openbb_core/api/router/commands.py | 2 +- ...harting_manager.py => charting_service.py} | 58 +++++++++------- .../core/openbb_core/app/command_runner.py | 43 ++++++------ .../app/logs/handlers/posthog_handler.py | 1 + ...{logging_manager.py => logging_service.py} | 20 ++---- .../app/logs/models/logging_settings.py | 1 + .../sdk/core/openbb_core/app/model/obbject.py | 12 ++-- .../openbb_core/app/model/system_settings.py | 4 +- .../openbb_core/app/service/system_service.py | 51 ++++++++++---- .../openbb_core/app/service/user_service.py | 32 ++++----- .../core/openbb_core/app/static/account.py | 17 +++-- .../core/tests/api/dependency/test_system.py | 21 +----- .../core/tests/api/dependency/test_user.py | 10 +-- .../app/logs/handlers/test_posthog_handler.py | 4 ++ ...ing_manager.py => test_logging_service.py} | 46 ++++++------- .../sdk/core/tests/app/model/test_obbject.py | 34 ++-------- .../tests/app/service/test_system_service.py | 4 +- .../core/tests/app/test_charting_manager.py | 52 -------------- .../core/tests/app/test_charting_service.py | 63 +++++++++++++++++ .../sdk/core/tests/app/test_command_runner.py | 68 ++++++++----------- 22 files changed, 278 insertions(+), 285 deletions(-) rename openbb_sdk/sdk/core/openbb_core/app/{charting_manager.py => charting_service.py} (79%) rename openbb_sdk/sdk/core/openbb_core/app/logs/{logging_manager.py => logging_service.py} (92%) rename openbb_sdk/sdk/core/tests/app/logs/{test_logging_manager.py => test_logging_service.py} (79%) delete mode 100644 openbb_sdk/sdk/core/tests/app/test_charting_manager.py create mode 100644 openbb_sdk/sdk/core/tests/app/test_charting_service.py diff --git a/openbb_sdk/sdk/core/openbb_core/api/dependency/system.py b/openbb_sdk/sdk/core/openbb_core/api/dependency/system.py index 4ccc1c5e0b4..007359a6b35 100644 --- a/openbb_sdk/sdk/core/openbb_core/api/dependency/system.py +++ b/openbb_sdk/sdk/core/openbb_core/api/dependency/system.py @@ -1,19 +1,9 @@ """System dependency.""" -from copy import deepcopy from openbb_core.app.model.system_settings import SystemSettings from openbb_core.app.service.system_service import SystemService -__system_service = SystemService() - async def get_system_settings() -> SystemSettings: """Get system settings.""" - global __system_service # noqa: PLW0602 # pylint: disable=global-variable-not-assigned - return deepcopy(__system_service.system_settings) - - -async def get_system_service() -> SystemService: - """Get system service.""" - global __system_service # noqa: PLW0602 # pylint: disable=global-variable-not-assigned - return __system_service + return SystemService().system_settings diff --git a/openbb_sdk/sdk/core/openbb_core/api/dependency/user.py b/openbb_sdk/sdk/core/openbb_core/api/dependency/user.py index 52734ce230a..f68cd816362 100644 --- a/openbb_sdk/sdk/core/openbb_core/api/dependency/user.py +++ b/openbb_sdk/sdk/core/openbb_core/api/dependency/user.py @@ -16,7 +16,7 @@ SECRET_KEY = "a0657288545d1d2e991195841782ae2a22574a22954081db0c2888c5f5ddbecc" ALGORITHM = "HS256" ACCESS_TOKEN_EXPIRE_MINUTES = 1440 -__user_service: Optional[UserService] = None + oauth2_scheme = OAuth2PasswordBearer(tokenUrl="/api/v1/account/token") pwd_context = CryptContext(schemes=["bcrypt"], deprecated="auto") @@ -79,12 +79,8 @@ def create_jwt_token( async def get_user_service() -> UserService: """Get user service.""" - global __user_service # pylint: disable=global-statement # noqa: PLW0603 - if __user_service is None: - __user_service = UserService() - - return __user_service + return UserService() async def get_user( diff --git a/openbb_sdk/sdk/core/openbb_core/api/router/commands.py b/openbb_sdk/sdk/core/openbb_core/api/router/commands.py index 3b4cd969198..7df34b9a5e0 100644 --- a/openbb_sdk/sdk/core/openbb_core/api/router/commands.py +++ b/openbb_sdk/sdk/core/openbb_core/api/router/commands.py @@ -166,6 +166,6 @@ def add_command_map(command_runner: CommandRunner, api_router: APIRouter) -> Non api_router.include_router(router=plugins_router.api_router) -system_settings = SystemService.read_default_system_settings() +system_settings = SystemService(logging_sub_app="api").system_settings command_runner_instance = CommandRunner(system_settings=system_settings) add_command_map(command_runner=command_runner_instance, api_router=router) diff --git a/openbb_sdk/sdk/core/openbb_core/app/charting_manager.py b/openbb_sdk/sdk/core/openbb_core/app/charting_service.py similarity index 79% rename from openbb_sdk/sdk/core/openbb_core/app/charting_manager.py rename to openbb_sdk/sdk/core/openbb_core/app/charting_service.py index 9af8f10f059..41bbec1a9e0 100644 --- a/openbb_sdk/sdk/core/openbb_core/app/charting_manager.py +++ b/openbb_sdk/sdk/core/openbb_core/app/charting_service.py @@ -8,36 +8,39 @@ from openbb_core.app.model.charts.chart import Chart, ChartFormat from openbb_core.app.model.charts.charting_settings import ChartingSettings from openbb_core.app.model.system_settings import SystemSettings from openbb_core.app.model.user_settings import UserSettings -from openbb_core.app.service.system_service import SystemService -from openbb_core.app.service.user_service import UserService T = TypeVar("T") POETRY_PLUGIN = "openbb_core_extension" -class ChartingManagerError(Exception): +class ChartingServiceError(Exception): pass -class ChartingManager(metaclass=SingletonMeta): +class ChartingService(metaclass=SingletonMeta): """ - Charting manager class. + Charting service class. It is responsible for retrieving and executing the charting function, corresponding to a given route, from the user's preferred charting extension. + Parameters ---------- - user_settings : Optional[UserSettings] + user_settings : UserSettings User settings. + system_settings : SystemSettings + System settings. + Attributes ---------- _charting_extension : str Charting extension name, which is retrieved from the user preferences. _charting_extension_installed : bool Either charting extension is installed or not. + Raises ------ - ChartingManagerError + ChartingServiceError If charting extension is not installed. """ @@ -46,14 +49,19 @@ class ChartingManager(metaclass=SingletonMeta): user_settings: Optional[UserSettings] = None, system_settings: Optional[SystemSettings] = None, ) -> None: - user_settings = user_settings or UserService().read_default_user_settings() - system_settings = ( - system_settings or SystemService().read_default_system_settings() - ) + # Although the __init__ method states that both the user_settings and the system_settings + # are optional, they are actually required for the first initialization of the ChartingService. + # This is because the ChartingService is a singleton and it is initialized only once. + # We want to be able to call the ChartingService from other parts of the code without having + # to pass the user_settings and the system_settings every time. + if not user_settings or not system_settings: + raise ChartingServiceError( + "User settings and system settings must be provided." + ) self._charting_settings = ChartingSettings(user_settings, system_settings) self._charting_extension = user_settings.preferences.charting_extension - self._charting_extension_installed = self.check_charting_extension_installed( + self._charting_extension_installed = self._check_charting_extension_installed( self._charting_extension ) @@ -70,7 +78,7 @@ class ChartingManager(metaclass=SingletonMeta): ) @staticmethod - def check_charting_extension_installed( + def _check_charting_extension_installed( charting_extension: str, plugin: str = POETRY_PLUGIN ) -> bool: """ @@ -104,7 +112,7 @@ class ChartingManager(metaclass=SingletonMeta): return import_module(entry_point.module) @classmethod - def get_chart_format(cls, extension_name: str) -> ChartFormat: + def _get_chart_format(cls, extension_name: str) -> ChartFormat: """ Given an extension name, it returns the chart format. The module must contain the `CHART_FORMAT` attribute. @@ -113,7 +121,7 @@ class ChartingManager(metaclass=SingletonMeta): return getattr(module, "CHART_FORMAT") @classmethod - def get_chart_function(cls, extension_name: str, route: str) -> Callable: + def _get_chart_function(cls, extension_name: str, route: str) -> Callable: """ Given an extension name and a route, it returns the chart function. The module must contain the given route. @@ -123,7 +131,7 @@ class ChartingManager(metaclass=SingletonMeta): return getattr(module, adjusted_route) @staticmethod - def handle_backend(charting_extension: str, charting_settings: ChartingSettings): + def _handle_backend(charting_extension: str, charting_settings: ChartingSettings): """ Handles the backend of the given charting extension. This function that the module expose in its root (__init__.py) the following functions: @@ -162,17 +170,17 @@ class ChartingManager(metaclass=SingletonMeta): Raises ------ - ChartingManagerError + ChartingServiceError If charting extension is not installed. Exception If the charting extension module does not contain the `to_chart` function. """ if not self._charting_extension_installed: - raise ChartingManagerError( + raise ChartingServiceError( f"Charting extension `{self._charting_extension}` is not installed" ) - self.handle_backend(self._charting_extension, self._charting_settings) + self._handle_backend(self._charting_extension, self._charting_settings) # Dynamically import the charting module backend_module = import_module(self._charting_extension) @@ -186,7 +194,7 @@ class ChartingManager(metaclass=SingletonMeta): return Chart( content=content, - format=self.get_chart_format(self._charting_extension), + format=self._get_chart_format(self._charting_extension), fig=fig, ) @@ -224,25 +232,25 @@ class ChartingManager(metaclass=SingletonMeta): """ self._charting_settings = ChartingSettings(user_settings, system_settings) self._charting_extension = user_settings.preferences.charting_extension - self._charting_extension_installed = self.check_charting_extension_installed( + self._charting_extension_installed = self._check_charting_extension_installed( self._charting_extension ) if not self._charting_extension_installed: - raise ChartingManagerError( + raise ChartingServiceError( f"Charting extension `{self._charting_extension}` is not installed" ) - self.handle_backend(self._charting_extension, self._charting_settings) + self._handle_backend(self._charting_extension, self._charting_settings) kwargs["obbject_item"] = obbject_item kwargs["charting_settings"] = self._charting_settings - charting_function = self.get_chart_function(self._charting_extension, route) + charting_function = self._get_chart_function(self._charting_extension, route) fig, content = charting_function(**kwargs) return Chart( content=content, - format=self.get_chart_format(self._charting_extension), + format=self._get_chart_format(self._charting_extension), fig=fig, ) diff --git a/openbb_sdk/sdk/core/openbb_core/app/command_runner.py b/openbb_sdk/sdk/core/openbb_core/app/command_runner.py index 9249ae0714e..f5d9dbc25fd 100644 --- a/openbb_sdk/sdk/core/openbb_core/app/command_runner.py +++ b/openbb_sdk/sdk/core/openbb_core/app/command_runner.py @@ -9,8 +9,8 @@ from typing import Any, Callable, ContextManager, Dict, List, Optional, Tuple, U from pydantic import BaseConfig, Extra, create_model -from openbb_core.app.charting_manager import ChartingManager -from openbb_core.app.logs.logging_manager import LoggingManager +from openbb_core.app.charting_service import ChartingService +from openbb_core.app.logs.logging_service import LoggingService from openbb_core.app.model.abstract.error import OpenBBError from openbb_core.app.model.abstract.warning import cast_warning from openbb_core.app.model.command_context import CommandContext @@ -230,9 +230,6 @@ class ParametersBuilder: class StaticCommandRunner: - logging_manager: LoggingManager = LoggingManager() - charting_manager: ChartingManager = ChartingManager() - @classmethod def __command(cls, func: Callable, kwargs: Dict[str, Any]) -> OBBject: """Run a command and return the output""" @@ -264,7 +261,10 @@ class StaticCommandRunner: **kwargs, ) -> None: """Create a chart from the command output.""" - obbject.chart = cls.charting_manager.chart( + cs = ChartingService( + user_settings=user_settings, system_settings=system_settings + ) + obbject.chart = cs.chart( user_settings=user_settings, system_settings=system_settings, route=route, @@ -324,7 +324,10 @@ class StaticCommandRunner: except Exception as e: raise OpenBBError(e) from e finally: - cls.logging_manager.log( + ls = LoggingService( + user_settings=user_settings, system_settings=system_settings + ) + ls.log( user_settings=user_settings, system_settings=system_settings, route=route, @@ -335,13 +338,6 @@ class StaticCommandRunner: return obbject - @classmethod - def __update_managers_settings( - cls, system_settings: SystemSettings, user_settings: UserSettings - ): - cls.logging_manager.logging_settings = (system_settings, user_settings) - cls.charting_manager.charting_settings = (system_settings, user_settings) - @classmethod def run( cls, @@ -356,10 +352,6 @@ class StaticCommandRunner: command_map = execution_context.command_map route = execution_context.route - cls.__update_managers_settings( - execution_context.system_settings, execution_context.user_settings - ) - if func := command_map.get_command(route=route): obbject = cls.__execute_func( route=route, @@ -392,11 +384,16 @@ class CommandRunner: user_settings: Optional[UserSettings] = None, ) -> None: self._command_map = command_map or CommandMap() - self._system_settings = ( - system_settings or SystemService.read_default_system_settings() - ) + self._system_settings = system_settings or SystemService().system_settings self._user_settings = user_settings or UserService.read_default_user_settings() + self._logging_service = LoggingService( + system_settings=self._system_settings, user_settings=self._user_settings + ) + self._charting_service = ChartingService( + system_settings=self._system_settings, user_settings=self._user_settings + ) + @property def command_map(self) -> CommandMap: return self._command_map @@ -409,6 +406,10 @@ class CommandRunner: def user_settings(self) -> UserSettings: return self._user_settings + @user_settings.setter + def user_settings(self, user_settings: UserSettings) -> None: + self._user_settings = user_settings + def run( self, route: str, diff --git a/openbb_sdk/sdk/core/openbb_core/app/logs/handlers/posthog_handler.py b/openbb_sdk/sdk/core/openbb_core/app/logs/handlers/posthog_handler.py index 213969dc11d..fa69528e324 100644 --- a/openbb_sdk/sdk/core/openbb_core/app/logs/handlers/posthog_handler.py +++ b/openbb_sdk/sdk/core/openbb_core/app/logs/handlers/posthog_handler.py @@ -111,6 +111,7 @@ class PosthogHandler(logging.Handler): log_extra: Dict[str, Any] = { "appName": self._settings.app_name, + "subAppName": self._settings.sub_app_name, "appId": self._settings.app_id, "sessionId": self._settings.session_id, "platform": self._settings.platform, diff --git a/openbb_sdk/sdk/core/openbb_core/app/logs/logging_manager.py b/openbb_sdk/sdk/core/openbb_core/app/logs/logging_service.py similarity index 92% rename from openbb_sdk/sdk/core/openbb_core/app/logs/logging_manager.py rename to openbb_sdk/sdk/core/openbb_core/app/logs/logging_service.py index bea48f31ef6..a5414fbe5ba 100644 --- a/openbb_sdk/sdk/core/openbb_core/app/logs/logging_manager.py +++ b/openbb_sdk/sdk/core/openbb_core/app/logs/logging_service.py @@ -13,12 +13,10 @@ from openbb_core.app.model.abstract.error import OpenBBError from openbb_core.app.model.abstract.singleton import SingletonMeta from openbb_core.app.model.system_settings import SystemSettings from openbb_core.app.model.user_settings import UserSettings -from openbb_core.app.service.system_service import SystemService -from openbb_core.app.service.user_service import UserService from pydantic.json import pydantic_encoder -class LoggingManager(metaclass=SingletonMeta): +class LoggingService(metaclass=SingletonMeta): """ Logging Manager class responsible for managing logging settings and handling logs. @@ -56,8 +54,8 @@ class LoggingManager(metaclass=SingletonMeta): def __init__( self, - system_settings: Optional[SystemSettings] = None, - user_settings: Optional[UserSettings] = None, + system_settings: SystemSettings, + user_settings: UserSettings, ) -> None: """ Logging Manager Constructor @@ -65,17 +63,13 @@ class LoggingManager(metaclass=SingletonMeta): Parameters ---------- - system_settings : Optional[SystemSettings], optional + system_settings : SystemSettings System Settings, by default None - user_settings : Optional[UserSettings], optional + user_settings : UserSettings User Settings, by default None """ - self._user_settings = ( - user_settings or UserService().read_default_user_settings() - ) - self._system_settings = ( - system_settings or SystemService().read_default_system_settings() - ) + self._user_settings = user_settings + self._system_settings = system_settings self._logging_settings = LoggingSettings( user_settings=self._user_settings, system_settings=self._system_settings, diff --git a/openbb_sdk/sdk/core/openbb_core/app/logs/models/logging_settings.py b/openbb_sdk/sdk/core/openbb_core/app/logs/models/logging_settings.py index 4ab8da80a2d..78079d471a3 100644 --- a/openbb_sdk/sdk/core/openbb_core/app/logs/models/logging_settings.py +++ b/openbb_sdk/sdk/core/openbb_core/app/logs/models/logging_settings.py @@ -33,6 +33,7 @@ class LoggingSettings: # System self.app_name: str = system_settings.logging_app_name + self.sub_app_name: str = system_settings.logging_sub_app self.app_id: str = get_app_id(user_data_directory) self.session_id: str = get_session_id() self.frequency: str = system_settings.logging_frequency diff --git a/openbb_sdk/sdk/core/openbb_core/app/model/obbject.py b/openbb_sdk/sdk/core/openbb_core/app/model/obbject.py index c5de65a8e5b..242fc3548a5 100644 --- a/openbb_sdk/sdk/core/openbb_core/app/model/obbject.py +++ b/openbb_sdk/sdk/core/openbb_core/app/model/obbject.py @@ -4,7 +4,7 @@ import pandas as pd from pydantic import Field from pydantic.generics import GenericModel -from openbb_core.app.charting_manager import ChartingManager +from openbb_core.app.charting_service import ChartingService from openbb_core.app.model.abstract.error import OpenBBError from openbb_core.app.model.abstract.tagged import Tagged from openbb_core.app.model.abstract.warning import Warning_ @@ -119,15 +119,19 @@ class OBBject(GenericModel, Generic[T], Tagged): chart.fig The chart figure. """ - cm = ChartingManager() + + cs = ChartingService() kwargs["data"] = self.to_dataframe() - self.chart = cm.to_chart(**kwargs) + self.chart = cs.to_chart(**kwargs) return self.chart.fig def show(self): """Displays chart.""" if not self.chart or not self.chart.fig: - raise OpenBBError("Chart not found.") + raise OpenBBError( + "Chart not found. " + "Please compute the chart first by using the `chart=True` argument." + ) self.chart.fig.show() diff --git a/openbb_sdk/sdk/core/openbb_core/app/model/system_settings.py b/openbb_sdk/sdk/core/openbb_core/app/model/system_settings.py index bcb95dae9ac..40625b2df9a 100644 --- a/openbb_sdk/sdk/core/openbb_core/app/model/system_settings.py +++ b/openbb_sdk/sdk/core/openbb_core/app/model/system_settings.py @@ -33,13 +33,13 @@ class SystemSettings(Tagged): system_settings_path: str = FrozenField(default=str(SYSTEM_SETTINGS_PATH)) # Logging section - logging_app_name: str = FrozenField(default="gst") + logging_app_name: Literal["sdk"] = FrozenField(default="sdk") logging_commit_hash: Optional[str] = FrozenField(default=None) logging_frequency: Literal["D", "H", "M", "S"] = FrozenField(default="H") logging_handlers: List[str] = FrozenField(default_factory=lambda: ["file"]) logging_rolling_clock: bool = FrozenField(default=False) logging_verbosity: int = FrozenField(default=20) - logging_sub_app: str = FrozenField(default="sdk") + logging_sub_app: Literal["python", "api", "pro"] = FrozenField(default="python") logging_suppress: bool = FrozenField(default=False) log_collect: bool = FrozenField(default=True) diff --git a/openbb_sdk/sdk/core/openbb_core/app/service/system_service.py b/openbb_sdk/sdk/core/openbb_core/app/service/system_service.py index 92313b1f13a..37d83cfc941 100644 --- a/openbb_sdk/sdk/core/openbb_core/app/service/system_service.py +++ b/openbb_sdk/sdk/core/openbb_core/app/service/system_service.py @@ -1,12 +1,14 @@ +import hashlib import json from pathlib import Path from typing import Optional from openbb_core.app.constants import SYSTEM_SETTINGS_PATH +from openbb_core.app.model.abstract.singleton import SingletonMeta from openbb_core.app.model.system_settings import SystemSettings -class SystemService: +class SystemService(metaclass=SingletonMeta): """System service.""" SYSTEM_SETTINGS_PATH = SYSTEM_SETTINGS_PATH @@ -14,12 +16,34 @@ class SystemService: "log_collect", "test_mode", "headless", - "dbms_uri", + "logging_sub_app", } + PRO_VALIDATION_HASH = ( + "a35f9683d36f45fa6fac4f62d98c24974a3332e16ddf2832fcf8b119126b83db" + ) + + def __init__( + self, + **kwargs, + ): + self._system_settings = self._read_default_system_settings( + path=self.SYSTEM_SETTINGS_PATH, **kwargs + ) + @classmethod - def read_default_system_settings( - cls, path: Optional[Path] = None + def _compare_hash(cls, input_value, existing_hash: Optional[str] = None): + existing_hash = existing_hash or cls.PRO_VALIDATION_HASH + + hash_object = hashlib.sha256() + hash_object.update(input_value.encode("utf-8")) + hashed_input = hash_object.hexdigest() + + return hashed_input == existing_hash + + @classmethod + def _read_default_system_settings( + cls, path: Optional[Path] = None, **kwargs ) -> SystemSettings: """Read default system settings.""" path = path or cls.SYSTEM_SETTINGS_PATH @@ -29,14 +53,22 @@ class SystemService: system_settings_json = file.read() system_settings_dict = json.loads(system_settings_json) + S = system_settings_dict.copy() for field in S: if field not in cls.SYSTEM_SETTINGS_ALLOWED_FIELD_SET: del system_settings_dict[field] + elif field == "logging_sub_app": + if cls._compare_hash(system_settings_dict[field]): + system_settings_dict[field] = "pro" + kwargs.pop(field, None) + else: + del system_settings_dict[field] + system_settings_dict.update(kwargs) system_settings = SystemSettings.parse_obj(system_settings_dict) else: - system_settings = SystemSettings() + system_settings = SystemSettings.parse_obj(kwargs) return system_settings @@ -57,13 +89,6 @@ class SystemService: with path.open(mode="w") as file: file.write(system_settings_json) - def __init__( - self, - path: Path = SYSTEM_SETTINGS_PATH, - ): - self._path = path - self._system_settings = self.read_default_system_settings() - @property def system_settings(self) -> SystemSettings: """Get system settings.""" @@ -76,6 +101,6 @@ class SystemService: def refresh_system_settings(self) -> SystemSettings: """Refresh system settings.""" - self._system_settings = self.read_default_system_settings() + self._system_settings = self._read_default_system_settings() return self._system_settings diff --git a/openbb_sdk/sdk/core/openbb_core/app/service/user_service.py b/openbb_sdk/sdk/core/openbb_core/app/service/user_service.py index b1e031c8cbd..285af15c8e6 100644 --- a/openbb_sdk/sdk/core/openbb_core/app/service/user_service.py +++ b/openbb_sdk/sdk/core/openbb_core/app/service/user_service.py @@ -4,6 +4,7 @@ from pathlib import Path from typing import Any, Dict, List, MutableMapping, Optional from openbb_core.app.constants import USER_SETTINGS_PATH +from openbb_core.app.model.abstract.singleton import SingletonMeta from openbb_core.app.model.user_settings import UserSettings from openbb_core.app.repository.abstract.access_token_repository import ( AccessTokenRepository as AbstractAccessTokenRepository, @@ -19,13 +20,26 @@ from openbb_core.app.repository.in_memory.user_settings_repository import ( ) -class UserService: +class UserService(metaclass=SingletonMeta): """User service.""" - REPOSITORY_DIRECTORY = Path(__file__).parent.parent.parent.parent USER_SETTINGS_PATH = USER_SETTINGS_PATH USER_SETTINGS_ALLOWED_FIELD_SET = {"credentials", "preferences", "defaults"} + def __init__( + self, + access_token_repository: Optional[AbstractAccessTokenRepository] = None, + user_settings_repository: Optional[AbstractUserSettingsRepository] = None, + default_user_settings: Optional[UserSettings] = None, + ): + self._token_repository = self.build_token_repository(access_token_repository) + self._user_settings_repository = self.build_user_settings_repository( + user_settings_repository + ) + self._default_user_settings = ( + default_user_settings or self.read_default_user_settings() + ) + @staticmethod def build_token_repository( access_token_repository: Optional[AbstractAccessTokenRepository] = None, @@ -100,20 +114,6 @@ class UserService: result = reduce(recursive_merge, (result, d)) return result - def __init__( - self, - access_token_repository: Optional[AbstractAccessTokenRepository] = None, - user_settings_repository: Optional[AbstractUserSettingsRepository] = None, - default_user_settings: Optional[UserSettings] = None, - ): - self._token_repository = self.build_token_repository(access_token_repository) - self._user_settings_repository = self.build_user_settings_repository( - user_settings_repository - ) - self._default_user_settings = ( - default_user_settings or self.read_default_user_settings() - ) - @property def default_user_settings(self) -> UserSettings: """Return default user settings.""" diff --git a/openbb_sdk/sdk/core/openbb_core/app/static/account.py b/openbb_sdk/sdk/core/openbb_core/app/static/account.py index a21249d60fe..64d47804e60 100644 --- a/openbb_sdk/sdk/core/openbb_core/app/static/account.py +++ b/openbb_sdk/sdk/core/openbb_core/app/static/account.py @@ -3,7 +3,6 @@ import json from pathlib import Path from typing import TYPE_CHECKING, Optional -from openbb_core.app.command_runner import CommandRunner from openbb_core.app.model.abstract.error import OpenBBError from openbb_core.app.model.hub.hub_session import HubSession from openbb_core.app.model.user_settings import UserSettings @@ -79,8 +78,8 @@ class Account: """ hs = self._create_hub_service(email, password, pat) incoming = hs.pull() - updated = UserService.update_default(incoming) - self._base_app._command_runner = CommandRunner(user_settings=updated) + updated: UserSettings = UserService.update_default(incoming) + self._base_app._command_runner.user_settings = updated if remember_me: Path(self._openbb_directory).mkdir(parents=False, exist_ok=True) session_file = Path(self._openbb_directory, ".sdk_hub_session.json") @@ -120,13 +119,15 @@ class Account: """ hub_session = self._base_app._command_runner.user_settings.profile.hub_session if not hub_session: - self._base_app._command_runner = CommandRunner() + self._base_app._command_runner.user_settings = ( + UserService.read_default_user_settings() + ) else: hs = HubService(hub_session) incoming = hs.pull() - updated = UserService.update_default(incoming) + updated: UserSettings = UserService.update_default(incoming) updated.id = self._base_app._command_runner.user_settings.id - self._base_app._command_runner = CommandRunner(user_settings=updated) + self._base_app._command_runner.user_settings = updated return self._base_app._command_runner.user_settings def logout(self) -> UserSettings: @@ -148,5 +149,7 @@ class Account: if session_file.exists(): session_file.unlink() - self._base_app._command_runner = CommandRunner() + self._base_app._command_runner.user_settings = ( + UserService.read_default_user_settings() + ) return self._base_app._command_runner.user_settings diff --git a/openbb_sdk/sdk/core/tests/api/dependency/test_system.py b/openbb_sdk/sdk/core/tests/api/dependency/test_system.py index 11e08d06838..5279c5da4f9 100644 --- a/openbb_sdk/sdk/core/tests/api/dependency/test_system.py +++ b/openbb_sdk/sdk/core/tests/api/dependency/test_system.py @@ -4,7 +4,6 @@ from unittest.mock import MagicMock, patch from openbb_core.api.dependency.system import ( SystemSettings, - get_system_service, get_system_settings, ) @@ -13,22 +12,8 @@ from openbb_core.api.dependency.system import ( def test_get_system_settings(mock_system_service): """Test get_system_settings.""" mock_system_settings = MagicMock(spec=SystemSettings) - mock_system_service.system_settings = mock_system_settings + mock_system_service.return_value.system_settings = mock_system_settings - with patch( - "openbb_core.api.dependency.system.__system_service", new=mock_system_service - ): - system_settings = asyncio.run(get_system_settings()) + system_settings = asyncio.run(get_system_settings()) - assert system_settings == mock_system_settings - - -@patch("openbb_core.api.dependency.system.SystemService") -def test_get_system_service(mock_system_service): - """Test get_system_service.""" - with patch( - "openbb_core.api.dependency.system.__system_service", new=mock_system_service - ): - system_service = asyncio.run(get_system_service()) - - assert system_service == mock_system_service + assert system_settings == mock_system_settings diff --git a/openbb_sdk/sdk/core/tests/api/dependency/test_user.py b/openbb_sdk/sdk/core/tests/api/dependency/test_user.py index 5aac052f79f..fa1875108a6 100644 --- a/openbb_sdk/sdk/core/tests/api/dependency/test_user.py +++ b/openbb_sdk/sdk/core/tests/api/dependency/test_user.py @@ -61,8 +61,10 @@ def test_create_jwt_token(): @patch("openbb_core.api.dependency.user.UserService") def test_get_user_service(mock_user_service): """Test get_user_service.""" - with patch("openbb_core.api.dependency.user.__user_service", new=None): - asyncio.run(get_user_service()) + + mock_user_service.return_value = MagicMock() + + asyncio.run(get_user_service()) mock_user_service.assert_called_once_with() @@ -74,7 +76,7 @@ def test_get_user(mock_user_service, mock_decode): mock_decode.return_value = {"sub": "user_id"} mock_user_settings = MagicMock(spec=UserSettings, profile=MagicMock(active=True)) mock_user_service.user_settings_repository.read.return_value = mock_user_settings - with patch("openbb_core.api.dependency.user.__user_service", new=mock_user_service): - result = asyncio.run(get_user("jwt_token", mock_user_service)) + mock_user_service.return_value = mock_user_service + result = asyncio.run(get_user("jwt_token", mock_user_service)) assert result == mock_user_settings diff --git a/openbb_sdk/sdk/core/tests/app/logs/handlers/test_posthog_handler.py b/openbb_sdk/sdk/core/tests/app/logs/handlers/test_posthog_handler.py index faf70555f78..8b9a76f0df7 100644 --- a/openbb_sdk/sdk/core/tests/app/logs/handlers/test_posthog_handler.py +++ b/openbb_sdk/sdk/core/tests/app/logs/handlers/test_posthog_handler.py @@ -12,6 +12,7 @@ class MockLoggingSettings: def __init__( self, app_name, + sub_app_name, user_logs_directory, session_id, frequency, @@ -22,6 +23,7 @@ class MockLoggingSettings: userid, ): self.app_name = app_name + self.sub_app_name = sub_app_name self.user_logs_directory = Path(user_logs_directory) self.session_id = session_id self.frequency = frequency @@ -34,6 +36,7 @@ class MockLoggingSettings: logging_settings = MagicMock(spec=MockLoggingSettings) logging_settings.app_name = "TestApp" +logging_settings.sub_app_name = "TestSubApp" logging_settings.user_logs_directory = MagicMock() logging_settings.user_logs_directory.absolute.return_value = Path( "/mocked/logs/directory" @@ -195,6 +198,7 @@ def test_log_to_dict(handler, log_info, expected_dict): ), { "appName": "TestApp", + "subAppName": "TestSubApp", "appId": "test123", "sessionId": "session123", "platform": "Windows", diff --git a/openbb_sdk/sdk/core/tests/app/logs/test_logging_manager.py b/openbb_sdk/sdk/core/tests/app/logs/test_logging_service.py similarity index 79% rename from openbb_sdk/sdk/core/tests/app/logs/test_logging_manager.py rename to openbb_sdk/sdk/core/tests/app/logs/test_logging_service.py index 99ff95a4e99..c04ca804fb9 100644 --- a/openbb_sdk/sdk/core/tests/app/logs/test_logging_manager.py +++ b/openbb_sdk/sdk/core/tests/app/logs/test_logging_service.py @@ -3,7 +3,7 @@ from typing import Optional from unittest.mock import MagicMock, Mock, patch import pytest -from openbb_core.app.logs.logging_manager import LoggingManager +from openbb_core.app.logs.logging_service import LoggingService from openbb_core.app.model.abstract.error import OpenBBError from pydantic import BaseModel @@ -22,7 +22,7 @@ class MockOBBject(BaseModel): @pytest.fixture(scope="function") -def logging_manager(): +def logging_service(): mock_system_settings = Mock() mock_system_settings = "mock_system_settings" mock_user_settings = Mock() @@ -31,16 +31,16 @@ def logging_manager(): mock_log_startup = Mock() with patch( - "openbb_core.app.logs.logging_manager.LoggingSettings", + "openbb_core.app.logs.logging_service.LoggingSettings", MockLoggingSettings, ), patch( - "openbb_core.app.logs.logging_manager.LoggingManager._setup_handlers", + "openbb_core.app.logs.logging_service.LoggingService._setup_handlers", mock_setup_handlers, ), patch( - "openbb_core.app.logs.logging_manager.LoggingManager._log_startup", + "openbb_core.app.logs.logging_service.LoggingService._log_startup", mock_log_startup, ): - logging_manager = LoggingManager( + logging_service = LoggingService( system_settings=mock_system_settings, user_settings=mock_user_settings, ) @@ -48,31 +48,31 @@ def logging_manager(): assert mock_setup_handlers.assert_called_once assert mock_log_startup.assert_called_once - return logging_manager + return logging_service -def test_correctly_initialized(logging_manager): - assert logging_manager +def test_correctly_initialized(logging_service): + assert logging_service -def test_logging_settings_setter(logging_manager): +def test_logging_settings_setter(logging_service): custom_user_settings = "custom_user_settings" custom_system_settings = "custom_system_settings" with patch( - "openbb_core.app.logs.logging_manager.LoggingSettings", + "openbb_core.app.logs.logging_service.LoggingSettings", MockLoggingSettings, ): - logging_manager.logging_settings = ( + logging_service.logging_settings = ( custom_system_settings, custom_user_settings, ) - assert logging_manager.logging_settings.system_settings == "custom_system_settings" - assert logging_manager.logging_settings.user_settings == "custom_user_settings" + assert logging_service.logging_settings.system_settings == "custom_system_settings" + assert logging_service.logging_settings.user_settings == "custom_user_settings" -def test_log_startup(logging_manager): +def test_log_startup(logging_service): with patch("logging.getLogger") as mock_get_logger: mock_info = mock_get_logger.return_value.info @@ -80,13 +80,13 @@ def test_log_startup(logging_manager): username: str password: str - logging_manager._user_settings = MagicMock( + logging_service._user_settings = MagicMock( preferences="your_preferences", credentials=MockCredentials(username="username", password="password"), ) - logging_manager._system_settings = "your_system_settings" + logging_service._system_settings = "your_system_settings" - logging_manager._log_startup() + logging_service._log_startup() expected_log_data = { "PREFERENCES": "your_preferences", @@ -130,17 +130,17 @@ def test_log_startup(logging_manager): ], ) def test_log( - logging_manager, user_settings, system_settings, route, func, kwargs, exec_info + logging_service, user_settings, system_settings, route, func, kwargs, exec_info ): with patch( - "openbb_core.app.logs.logging_manager.LoggingSettings", + "openbb_core.app.logs.logging_service.LoggingSettings", MockLoggingSettings, ), patch("logging.getLogger") as mock_get_logger: if route == "login": with patch( - "openbb_core.app.logs.logging_manager.LoggingManager._log_startup" + "openbb_core.app.logs.logging_service.LoggingService._log_startup" ) as mock_log_startup: - logging_manager.log( + logging_service.log( user_settings=user_settings, system_settings=system_settings, route=route, @@ -157,7 +157,7 @@ def test_log( mock_callable = Mock() mock_callable.__name__ = func - logging_manager.log( + logging_service.log( user_settings=user_settings, system_settings=system_settings, route=route, diff --git a/openbb_sdk/sdk/core/tests/app/model/test_obbject.py b/openbb_sdk/sdk/core/tests/app/model/test_obbject.py index 39bcf516ad6..1e56f692606 100644 --- a/openbb_sdk/sdk/core/tests/app/model/test_obbject.py +++ b/openbb_sdk/sdk/core/tests/app/model/test_obbject.py @@ -106,32 +106,10 @@ def test_to_dict(results, expected): assert result == expected -@patch("openbb_core.app.charting_manager.ChartingManager.to_chart") @patch("openbb_core.app.model.obbject.OBBject.to_dataframe") -def test_to_chart_with_existing_chart(mock_to_dataframe, mock_to_chart): - # Arrange - mock_instance = OBBject() - mock_instance.chart = Chart(content={"existing_chart_data": "some_data"}) - - # mock the return value of ChartingManager.to_chart() - mock_to_chart.return_value = Chart( - content={"content": "some_new_content"}, fig={"fig": "some_mock_fig"} - ) - - # Act - result = mock_instance.to_chart() - - # Assert - assert result == {"fig": "some_mock_fig"} - assert mock_instance.chart.content == {"content": "some_new_content"} - - mock_to_dataframe.assert_called_once() - - -@patch("openbb_core.app.model.obbject.OBBject.to_dataframe") -@patch("openbb_core.app.model.obbject.ChartingManager") +@patch("openbb_core.app.model.obbject.ChartingService") def test_to_chart_with_new_chart( - mock_charting_manager, + mock_charting_service, mock_to_dataframe, ): def get_mock_dataframe(): @@ -145,8 +123,8 @@ def test_to_chart_with_new_chart( # Arrange mock_instance = OBBject() - mock_charting_manager_instance = mock_charting_manager.return_value - mock_charting_manager_instance.to_chart.return_value = Chart( + mock_charting_service_instance = mock_charting_service.return_value + mock_charting_service_instance.to_chart.return_value = Chart( content={"content": "some_new_content"}, fig={"fig": "some_mock_fig"} ) mock_to_dataframe.return_value = get_mock_dataframe() @@ -161,8 +139,8 @@ def test_to_chart_with_new_chart( # Ensure self.to_dataframe() was called mock_to_dataframe.assert_called_once() - # Ensure ChartingManager was called with the correct parameters - mock_charting_manager_instance.to_chart.assert_called_once() + # Ensure ChartingService was called with the correct parameters + mock_charting_service_instance.to_chart.assert_called_once() def test_show_chart_exists(): diff --git a/openbb_sdk/sdk/core/tests/app/service/test_system_service.py b/openbb_sdk/sdk/core/tests/app/service/test_system_service.py index 6b42c18c581..bcde85b1fba 100644 --- a/openbb_sdk/sdk/core/tests/app/service/test_system_service.py +++ b/openbb_sdk/sdk/core/tests/app/service/test_system_service.py @@ -18,14 +18,14 @@ def test_system_service_init(system_service): def test_read_default_system_settings(system_service): """Test read default system settings.""" - system_settings = system_service.read_default_system_settings() + system_settings = system_service._read_default_system_settings() assert system_settings def test_write_default_system_settings(system_service): """Test write default system settings.""" - system_settings = system_service.read_default_system_settings() + system_settings = system_service._read_default_system_settings() system_service.write_default_system_settings(system_settings=system_settings) assert system_service diff --git a/openbb_sdk/sdk/core/tests/app/test_charting_manager.py b/openbb_sdk/sdk/core/tests/app/test_charting_manager.py deleted file mode 100644 index 6af3851e72c..00000000000 --- a/openbb_sdk/sdk/core/tests/app/test_charting_manager.py +++ /dev/null @@ -1,52 +0,0 @@ -import pytest -from openbb_core.app.charting_manager import ChartingManager -from openbb_core.app.model.system_settings import SystemSettings -from openbb_core.app.model.user_settings import UserSettings - - -@pytest.fixture(scope="function", autouse=True) -def setup_and_teardown(): - # Code to run before each test function - yield # This is where the test function runs - # Code to run after each test function - ChartingManager._instances = {} - - -def test_charting_manager(): - assert ChartingManager() - - -def test_charting_manager_singleton_prop(): - cm_1 = ChartingManager() - cm_2 = ChartingManager() - - assert cm_1 is cm_2 - - -def test_charting_settings(): - sys = SystemSettings(test_mode=True) - user = UserSettings(preferences={"plot_enable_pywry": False}) - - cm = ChartingManager(user_settings=user, system_settings=sys) - - assert cm.charting_settings.plot_enable_pywry is False - assert cm.charting_settings.test_mode is True - - -def test_charting_settings_setter(): - cm = ChartingManager() - - sys = SystemSettings(test_mode=True) - user = UserSettings(preferences={"plot_enable_pywry": False}) - - cm.charting_settings = (sys, user) - - assert cm.charting_settings.plot_enable_pywry is False - assert cm.charting_settings.test_mode is True - - -def test_check_charting_extension_installed(): - cm = ChartingManager() - - assert cm.check_charting_extension_installed("openbb_charting") is True - assert cm.check_charting_extension_installed("openbb_core_extension_2") is False diff --git a/openbb_sdk/sdk/core/tests/app/test_charting_service.py b/openbb_sdk/sdk/core/tests/app/test_charting_service.py new file mode 100644 index 00000000000..296190e50fe --- /dev/null +++ b/openbb_sdk/sdk/core/tests/app/test_charting_service.py @@ -0,0 +1,63 @@ +import pytest +from openbb_core.app.charting_service import ChartingService +from openbb_core.app.model.system_settings import SystemSettings +from openbb_core.app.model.user_settings import UserSettings + + +@pytest.fixture(scope="function", autouse=True) +def setup_and_teardown(): + # Code to run before each test function + yield # This is where the test function runs + # Code to run after each test function + ChartingService._instances = {} + + +def test_charting_service(): + sys = SystemSettings() + user = UserSettings() + + assert ChartingService(user_settings=user, system_settings=sys) + + +def test_charting_service_singleton_prop(): + sys = SystemSettings() + user = UserSettings() + + cm_1 = ChartingService(user_settings=user, system_settings=sys) + cm_2 = ChartingService(user_settings=user, system_settings=sys) + + assert cm_1 is cm_2 + + +def test_charting_settings(): + sys = SystemSettings(test_mode=True) + user = UserSettings(preferences={"plot_enable_pywry": False}) + + cm = ChartingService(user_settings=user, system_settings=sys) + + assert cm.charting_settings.plot_enable_pywry is False + assert cm.charting_settings.test_mode is True + + +def test_charting_settings_setter(): + sys = SystemSettings() + user = UserSettings() + cm = ChartingService(user_settings=user, system_settings=sys) + + sys = SystemSettings(test_mode=True) + user = UserSettings(preferences={"plot_enable_pywry": False}) + + cm.charting_settings = (sys, user) + + assert cm.charting_settings.plot_enable_pywry is False + assert cm.charting_settings.test_mode is True + + +@pytest.mark.skip(reason="function needs review") +def test_check_charting_extension_installed(): + sys = SystemSettings() + user = UserSettings() + cm = ChartingService(user_settings=user, system_settings=sys) + + assert cm._check_charting_extension_installed("openbb_charting") is True + assert cm._check_charting_extension_installed("openbb_core_extension_2") is False diff --git a/openbb_sdk/sdk/core/tests/app/test_command_runner.py b/openbb_sdk/sdk/core/tests/app/test_command_runner.py index 7f17d77a61e..61cbb0893d5 100644 --- a/openbb_sdk/sdk/core/tests/app/test_command_runner.py +++ b/openbb_sdk/sdk/core/tests/app/test_command_runner.py @@ -1,4 +1,5 @@ from inspect import Parameter +from typing import Dict from unittest.mock import patch import pytest @@ -26,7 +27,9 @@ def execution_context(): def mock_func(): """Set up mock function.""" - def mock_func(a: int, b: int, c: float = 10.0, d: int = 5) -> None: + def mock_func( + a: int, b: int, c: float = 10.0, d: int = 5, provider_choices: Dict = {} + ) -> None: pass return mock_func @@ -138,30 +141,27 @@ def test_parameters_builder_merge_args_and_kwargs( {"cc": "existing_cc"}, SystemSettings(), UserSettings(), - {"cc": "existing_cc"}, - ), - ({}, SystemSettings(), UserSettings(), {}), - ( - {"other_arg": "value"}, - SystemSettings(), - UserSettings(), - {"other_arg": "value"}, + {"cc": "mock_cc"}, ), ], ) def test_parameters_builder_update_command_context( kwargs, system_settings, user_settings, expected_result ): + def other_mock_func( + cc: CommandContext, + a: int, + b: int, + ) -> None: + pass + result = ParametersBuilder.update_command_context( - kwargs, system_settings, user_settings + other_mock_func, kwargs, system_settings, user_settings ) - if "cc" in expected_result: - assert isinstance(result["cc"], CommandContext) - assert result["cc"].system_settings == system_settings - assert result["cc"].user_settings == user_settings - else: - assert result == expected_result + assert isinstance(result["cc"], CommandContext) + assert result["cc"].system_settings == system_settings + assert result["cc"].user_settings == user_settings @pytest.mark.parametrize( @@ -172,28 +172,21 @@ def test_parameters_builder_update_command_context( "route1", {"provider_choices": {"provider": None}}, None, - {"provider_choices": {"provider": "choice1"}}, + {"provider_choices": {"provider": None}}, ), ( {"route1": ["choice1", "choice2"]}, "route1", - {}, - {"provider": "default_provider"}, - {"provider_choices": {"provider": "default_provider"}}, - ), - ( - {}, - "route2", - {"provider_choices": {"provider": None}}, - None, - {"provider_choices": {"provider": "provider1"}}, + {"provider_choices": {"provider": ["choice1", "choice2"]}}, + {"provider": "choice1"}, + {"provider_choices": {"provider": ["choice1", "choice2"]}}, ), ( {}, "route2", {}, {"provider": "default_provider"}, - {"provider_choices": {"provider": "provider1"}}, + {}, ), ( {}, @@ -202,13 +195,6 @@ def test_parameters_builder_update_command_context( None, {"provider_choices": {"provider": "existing_provider"}}, ), - ( - {"route4": []}, - "route4", - {}, - None, - {"provider_choices": {"provider": None}}, - ), ], ) def test_parameters_builder_update_provider_choices( @@ -219,7 +205,7 @@ def test_parameters_builder_update_provider_choices( **{"return_value.available_providers": ["provider1", "provider2"]} ): result = ParametersBuilder.update_provider_choices( - command_coverage, route, kwargs, route_default + mock_func, command_coverage, route, kwargs, route_default ) assert result == expected_result @@ -234,7 +220,7 @@ def test_parameters_builder_validate_kwargs(mock_func): mock_func, {"a": 1, "b": "2", "c": 3.0, "d": 4.3} ) - assert result == {"a": 1, "b": 2, "c": 3.0, "d": 4} + assert result == {"a": 1, "b": 2, "c": 3.0, "d": 4, "provider_choices": {}} def test_parameters_builder_build(mock_func, execution_context): @@ -248,7 +234,11 @@ def test_parameters_builder_build(mock_func, execution_context): ): result = ParametersBuilder.build( args=[1, 2], - kwargs={"c": 3, "d": "4"}, + kwargs={ + "c": 3, + "d": "4", + "provider_choices": {"provider": ["provider1", "provider2"]}, + }, func=mock_func, execution_context=execution_context, route="mock/route", @@ -259,7 +249,7 @@ def test_parameters_builder_build(mock_func, execution_context): "b": 2, "c": 3.0, "d": 4, - "provider_choices": {"provider": "provider1"}, + "provider_choices": {"provider": ["provider1", "provider2"]}, }