diff --git a/openbb_platform/core/openbb_core/api/router/commands.py b/openbb_platform/core/openbb_core/api/router/commands.py index c6a2f06b2d8..2ed68123fe7 100644 --- a/openbb_platform/core/openbb_core/api/router/commands.py +++ b/openbb_platform/core/openbb_core/api/router/commands.py @@ -7,8 +7,11 @@ from inspect import Parameter, Signature, signature from typing import Annotated, Any, TypeVar from fastapi import APIRouter, Depends, Header +from fastapi.encoders import jsonable_encoder +from fastapi.responses import JSONResponse from fastapi.routing import APIRoute from openbb_core.app.command_runner import CommandRunner +from openbb_core.app.model.abstract.error import OpenBBError from openbb_core.app.model.command_context import CommandContext from openbb_core.app.model.obbject import OBBject from openbb_core.app.model.user_settings import UserSettings @@ -140,7 +143,7 @@ def validate_output(c_out: OBBject) -> OBBject: def exclude_fields_from_api(key: str, value: Any): type_ = type(value) - field = c_out.model_fields.get(key, None) + field = getattr(type(c_out), "model_fields", {}).get(key, None) json_schema_extra = field.json_schema_extra if field else None # case where 1st layer field needs to be excluded @@ -198,7 +201,9 @@ def build_api_wrapper( route.response_model = None @wraps(wrapped=func) - async def wrapper(*args: tuple[Any], **kwargs: dict[str, Any]) -> OBBject: + async def wrapper( + *args: tuple[Any], **kwargs: dict[str, Any] + ) -> OBBject | JSONResponse: user_settings: UserSettings = UserSettings.model_validate( kwargs.pop( "__authenticated_user_settings", @@ -213,7 +218,7 @@ def build_api_wrapper( ) if defaults: - _provider = defaults.pop("provider", None) + _ = defaults.pop("provider", None) standard_params = getattr( kwargs.pop("standard_params", None), "__dict__", {} ) @@ -243,6 +248,23 @@ def build_api_wrapper( execute = partial(command_runner.run, path, user_settings) output = await execute(*args, **kwargs) + # This is where we check for `on_command_output` extensions + mutated_output = getattr(output, "_extension_modified", False) + results_only = getattr(output, "_results_only", False) + try: + if results_only is True: + content = output.model_dump(exclude_unset=True).get("results", []) + return JSONResponse(content=jsonable_encoder(content), status_code=200) + + if (mutated_output and isinstance(output, OBBject)) or ( + isinstance(output, OBBject) and no_validate + ): + return JSONResponse(content=jsonable_encoder(output), status_code=200) + except Exception as exc: # pylint: disable=W0703 + raise OpenBBError( + f"Error serializing output for an extension-modified endpoint {path}: {exc}", + ) from exc + if isinstance(output, OBBject) and not no_validate: return validate_output(output) diff --git a/openbb_platform/core/openbb_core/app/command_runner.py b/openbb_platform/core/openbb_core/app/command_runner.py index 6a96dc800b4..2c3c2ad487b 100644 --- a/openbb_platform/core/openbb_core/app/command_runner.py +++ b/openbb_platform/core/openbb_core/app/command_runner.py @@ -6,12 +6,13 @@ from collections.abc import Callable from copy import deepcopy from dataclasses import asdict, is_dataclass from datetime import datetime -from inspect import Parameter, signature +from inspect import Parameter, iscoroutinefunction, signature from sys import exc_info from time import perf_counter_ns from typing import TYPE_CHECKING, Any, Optional from warnings import catch_warnings, showwarning, warn +from openbb_core.app.extension_loader import ExtensionLoader from openbb_core.app.model.abstract.error import OpenBBError from openbb_core.app.model.abstract.warning import OpenBBWarning, cast_warning from openbb_core.app.model.metadata import Metadata @@ -272,6 +273,7 @@ class StaticCommandRunner: if chart_params: kwargs.update(chart_params) + obbject.charting.show(render=False, **kwargs) # type: ignore[attr-defined] except Exception as e: # pylint: disable=broad-exception-caught if Env().DEBUG_MODE: @@ -311,6 +313,9 @@ class StaticCommandRunner: # in the charting extension then we add it there. This way we can remove # the chart parameter from the commands.py and package_builder, it will be # added to the function signature in the router decorator + # If the ProviderInterface is not in use, we need to pass a copy of the + # kwargs dictionary before it is validated, otherwise we lose those items. + kwargs_copy = deepcopy(kwargs) chart = kwargs.pop("chart", False) kwargs = ParametersBuilder.build( @@ -348,7 +353,21 @@ class StaticCommandRunner: extra_params ) if chart and obbject.results: - cls._chart(obbject, **kwargs) + if "extra_params" not in kwargs_copy: + kwargs_copy["extra_params"] = {} + # Restore any kwargs passed that were removed by the ParametersBuilder + for k in kwargs_copy.copy(): + if k == "chart": + kwargs_copy.pop("chart", None) + continue + if ( + not extra_params or k not in extra_params + ) and k != "extra_params": + kwargs_copy["extra_params"][k] = kwargs_copy.pop( + k, None + ) + + cls._chart(obbject, **kwargs_copy) raised_warnings = warning_list if warning_list else [] finally: @@ -429,8 +448,79 @@ class StaticCommandRunner: raise OpenBBError(e) from e warn(str(e), OpenBBWarning) + try: + cls._trigger_command_output_callbacks(route, obbject) + + except Exception as e: + if Env().DEBUG_MODE: + raise OpenBBError(e) from e + warn(str(e), OpenBBWarning) + return obbject + @classmethod + def _trigger_command_output_callbacks(cls, route: str, obbject: OBBject) -> None: + """Trigger command output callbacks for extensions.""" + loader = ExtensionLoader() + callbacks = loader.on_command_output_callbacks + results_only = False + + # For each extension registered for all routes or the specific route, + # we call its accessor on the OBBject. + # We check if the accessor is immutable or not to decide whether to pass + # a copy of the OBBject or the original one. + # We set the _extension_modified attribute to True if any extension + # mutates the OBBject so we can pass this information to the interface. + # We also set the _results_only attribute to True if any extension + # indicates that only results should be returned. + if "*" in callbacks: + for ext in callbacks["*"]: + if ext.results_only is True: + results_only = True + if ext.immutable is True: + if hasattr(obbject, ext.name): + obbject_copy = deepcopy(obbject) + accessor = getattr(obbject_copy, ext.name) + if iscoroutinefunction(accessor): + run_async(accessor) + elif callable(accessor): + accessor() + elif ext.immutable is False: + if ext.results_only is True: + results_only = True + if hasattr(obbject, ext.name): + accessor = getattr(obbject, ext.name) + if iscoroutinefunction(accessor): + run_async(accessor) + elif callable(accessor): + accessor() + setattr(obbject, "_extension_modified", True) + + if route in callbacks: + for ext in callbacks[route]: + if ext.results_only is True: + results_only = True + + if ext.immutable is True: + if hasattr(obbject, ext.name): + obbject_copy = deepcopy(obbject) + accessor = getattr(obbject_copy, ext.name) + if iscoroutinefunction(accessor): + run_async(accessor) + elif callable(accessor): + accessor() + elif ext.immutable is False and hasattr(obbject, ext.name): + accessor = getattr(obbject, ext.name) + if iscoroutinefunction(accessor): + run_async(accessor) + elif callable(accessor): + accessor() + setattr(obbject, "_extension_modified", True) + + if results_only is True: + setattr(obbject, "_results_only", True) + setattr(obbject, "_extension_modified", True) + class CommandRunner: """Command runner.""" diff --git a/openbb_platform/core/openbb_core/app/extension_loader.py b/openbb_platform/core/openbb_core/app/extension_loader.py index 35938c59db6..4db8abea77a 100644 --- a/openbb_platform/core/openbb_core/app/extension_loader.py +++ b/openbb_platform/core/openbb_core/app/extension_loader.py @@ -49,6 +49,23 @@ class ExtensionLoader(metaclass=SingletonMeta): self._obbject_objects: dict[str, Extension] = {} self._core_objects: dict[str, Router] = {} self._provider_objects: dict[str, Provider] = {} + self._on_command_output_callbacks: dict[str, list[Extension]] = {} + self._register_command_output_callbacks() + + @property + def on_command_output_callbacks(self) -> dict[str, list[Extension]]: + """Return the on command output callbacks.""" + return self._on_command_output_callbacks + + def _register_command_output_callbacks(self) -> None: + """Register extensions that act on command output.""" + for ext in self.obbject_objects.values(): + if ext.on_command_output: + paths = ext.command_output_paths or ["*"] + for path in paths: + if path not in self._on_command_output_callbacks: + self._on_command_output_callbacks[path] = [] + self._on_command_output_callbacks[path].append(ext) @property def obbject_entry_points(self) -> EntryPoints: diff --git a/openbb_platform/core/openbb_core/app/model/abstract/tagged.py b/openbb_platform/core/openbb_core/app/model/abstract/tagged.py index 80cfe1678e0..df250a8d9bd 100644 --- a/openbb_platform/core/openbb_core/app/model/abstract/tagged.py +++ b/openbb_platform/core/openbb_core/app/model/abstract/tagged.py @@ -1,10 +1,10 @@ """OpenBB Core App Abstract Model Tagged.""" from pydantic import BaseModel, Field -from uuid_extensions import uuid7str # type: ignore +from uuid_extensions import uuid7str class Tagged(BaseModel): """Model for Tagged.""" - id: str = Field(default_factory=uuid7str, alias="_id") + id: str = Field(default_factory=uuid7str) diff --git a/openbb_platform/core/openbb_core/app/model/extension.py b/openbb_platform/core/openbb_core/app/model/extension.py index 1f4f3357f7c..a6edb731c8e 100644 --- a/openbb_platform/core/openbb_core/app/model/extension.py +++ b/openbb_platform/core/openbb_core/app/model/extension.py @@ -8,14 +8,19 @@ class Extension: """ Serves as OBBject extension entry point and must be created by each extension package. - See https://docs.openbb.co/platform/development/developer-guidelines/obbject_extensions. + See https://docs.openbb.co/developer/extension_types/obbject for more information. """ + # pylint: disable=R0917 def __init__( self, name: str, credentials: list[str] | None = None, description: str | None = None, + on_command_output: bool = False, + command_output_paths: list[str] | None = None, + immutable: bool = True, + results_only: bool = False, ) -> None: """Initialize the extension. @@ -23,20 +28,76 @@ class Extension: ---------- name : str Name of the extension. - credentials : Optional[List[str]], optional + credentials : list[str], optional List of required credentials, by default None description: Optional[str] Extension description. + on_command_output : bool, optional + Whether the extension acts on command output, by default False + command_output_paths : list[str], optional + List of endpoint paths the extension acts on, where None means all, by default None. + immutable : bool, optional + Whether the function output is immutable, by default True. + results_only : bool, optional + Whether the extension returns only the results instead of the OBBject, by default False. """ + # pylint: disable=import-outside-toplevel + from openbb_core.app.service.system_service import SystemService + self.name = name self.credentials = credentials or [] self.description = description + self.on_command_output = on_command_output + self.command_output_paths = command_output_paths or [] + self.immutable = immutable + self.results_only = results_only + + # This must be explicitly enabled. + if self.on_command_output is False and ( + self.command_output_paths + or self.results_only is True + or self.immutable is False + ): + raise ValueError( + "OBBject Extension Error -> 'on_command_output' must be set as True when" + + " 'command_output_paths', 'results_only' or 'immutable' is set.", + ) + + # The user must explicitly enable OBBject extensions that act on command output. + if ( + self.on_command_output + and not SystemService().system_settings.allow_on_command_output + ): + raise RuntimeError( + "OBBject Extension Error -> \n\n" + + "An OBBject extension that acts on command output is installed " + + "but has not been enabled in `system_settings.json`.\n\n" + + "Set `allow_on_command_output` to True to enable it.\n" + + "Or, set the environment variable `OPENBB_ALLOW_ON_COMMAND_OUTPUT` to True." + + "\n\nProceed with caution as this may have security implications.\n\n" + + "Ensure the extension is installed from a trusted source.\n\n", + ) + + # The user must explicitly enable OBBject extensions that modify output. + if ( + self.on_command_output + and self.immutable is False + and not SystemService().system_settings.allow_mutable_extensions + ): + raise RuntimeError( + "OBBject Extension Error -> \n\n" + + "An OBBject extension that modifies the output is installed " + + "but has not been enabled in `system_settings.json`.\n\n" + + "Set `allow_mutable_extensions` to True to enable it.\n" + + "Or, set the environment variable `OPENBB_ALLOW_MUTABLE_EXTENSIONS` to True." + + "\n\nProceed with caution as this may have security implications.\n\n" + + "Ensure the extension is installed from a trusted source.\n\n", + ) @property def obbject_accessor(self) -> Callable: """Extend an OBBject, inspired by pandas.""" # pylint: disable=import-outside-toplevel - # Avoid circular imports from openbb_core.app.model.obbject import OBBject @@ -55,8 +116,8 @@ class Extension: UserWarning, ) setattr(cls, name, CachedAccessor(name, accessor)) - # pylint: disable=protected-access cls.accessors.add(name) + return accessor return decorator diff --git a/openbb_platform/core/openbb_core/app/model/obbject.py b/openbb_platform/core/openbb_core/app/model/obbject.py index 1c7cab26ec2..ac7026884ad 100644 --- a/openbb_platform/core/openbb_core/app/model/obbject.py +++ b/openbb_platform/core/openbb_core/app/model/obbject.py @@ -60,7 +60,7 @@ class OBBject(Tagged, Generic[T]): default_factory=dict, description="Extra info.", ) - _route: str = PrivateAttr( + _route: str | None = PrivateAttr( default=None, ) _standard_params: dict[str, Any] | None = PrivateAttr( @@ -178,9 +178,9 @@ class OBBject(Tagged, Generic[T]): # BaseModel if isinstance(res, BaseModel): - res_dict = res.model_dump( + res_dict = res.model_dump( # pylint: disable=no-member exclude_unset=True, exclude_none=True - ) # pylint: disable=no-member + ) # Model is serialized as a dict[str, list] or list[dict] if ( ( @@ -322,13 +322,16 @@ class OBBject(Tagged, Generic[T]): orient == "list" and isinstance(self.results, dict) and all( - isinstance(value, dict) for value in self.results.values() - ) # pylint: disable=no-member + isinstance(value, dict) + for value in self.results.values() # pylint: disable=no-member + ) ): df = df.T - results = df.to_dict(orient=orient) + results: dict | list = df.to_dict(orient=orient) + if isinstance(results, dict) and orient == "list" and "index" in results: del results["index"] + return results def to_llm(self) -> dict[Hashable, Any] | list[dict[Hashable, Any]]: diff --git a/openbb_platform/core/openbb_core/app/model/system_settings.py b/openbb_platform/core/openbb_core/app/model/system_settings.py index 4d1bbe1e8dc..027c4aa3679 100644 --- a/openbb_platform/core/openbb_core/app/model/system_settings.py +++ b/openbb_platform/core/openbb_core/app/model/system_settings.py @@ -15,6 +15,7 @@ from openbb_core.app.model.abstract.tagged import Tagged from openbb_core.app.model.api_settings import APISettings from openbb_core.app.model.python_settings import PythonSettings from openbb_core.app.version import CORE_VERSION, VERSION +from openbb_core.env import Env from pydantic import ConfigDict, Field, field_validator, model_validator @@ -54,6 +55,8 @@ class SystemSettings(Tagged): debug_mode: bool = False test_mode: bool = False headless: bool = False + allow_mutable_extensions: bool = getattr(Env(), "ALLOW_MUTABLE_EXTENSIONS", False) + allow_on_command_output: bool = getattr(Env(), "ALLOW_ON_COMMAND_OUTPUT", False) model_config = ConfigDict(validate_assignment=True, frozen=True) diff --git a/openbb_platform/core/openbb_core/app/service/system_service.py b/openbb_platform/core/openbb_core/app/service/system_service.py index 6c3a86eff26..28dab9d256d 100644 --- a/openbb_platform/core/openbb_core/app/service/system_service.py +++ b/openbb_platform/core/openbb_core/app/service/system_service.py @@ -21,6 +21,8 @@ class SystemService(metaclass=SingletonMeta): "python_settings", "debug_mode", "logging_suppress", + "allow_mutable_extensions", + "allow_on_command_output", } PRO_VALIDATION_HASH = "300ac59fdcc8f899e0bc5c18cda8652220735da1a00e2af365efe9d8e5fe8306" # pragma: allowlist secret diff --git a/openbb_platform/core/openbb_core/app/static/container.py b/openbb_platform/core/openbb_core/app/static/container.py index 5e53d3ec1e5..c4dabfa9517 100644 --- a/openbb_platform/core/openbb_core/app/static/container.py +++ b/openbb_platform/core/openbb_core/app/static/container.py @@ -43,9 +43,18 @@ class Container: kwargs["extra_params"][k] = v obbject = self._command_runner.sync_run(*args, **kwargs) + + results_only = getattr(obbject, "_results_only", False) + + if results_only is True: + content = obbject.model_dump(exclude_unset=True).get("results", []) + return content + output_type = self._command_runner.user_settings.preferences.output_type + if output_type == "OBBject": return obbject + return getattr(obbject, "to_" + output_type)() def _check_credentials(self, provider: str) -> bool | None: diff --git a/openbb_platform/core/openbb_core/env.py b/openbb_platform/core/openbb_core/env.py index 4b8c551a6e4..1dd2e85dbaa 100644 --- a/openbb_platform/core/openbb_core/env.py +++ b/openbb_platform/core/openbb_core/env.py @@ -54,9 +54,16 @@ class Env(metaclass=SingletonMeta): return self.str2bool(self._environ.get("OPENBB_DEV_MODE", False)) @property - def HUB_BACKEND(self) -> str: - """Hub backend: sets the backend for the OpenBB Hub.""" - return self._environ.get("OPENBB_HUB_BACKEND", "https://payments.openbb.co") + def ALLOW_MUTABLE_EXTENSIONS(self) -> bool: + """Allow mutable extensions: enables extensions that modify OBBject output.""" + return self.str2bool( + self._environ.get("OPENBB_ALLOW_MUTABLE_EXTENSIONS", False) + ) + + @property + def ALLOW_ON_COMMAND_OUTPUT(self) -> bool: + """Allow on command output: enables extensions that act on command output.""" + return self.str2bool(self._environ.get("OPENBB_ALLOW_ON_COMMAND_OUTPUT", False)) @staticmethod def str2bool(value) -> bool: diff --git a/openbb_platform/core/tests/app/model/test_extension.py b/openbb_platform/core/tests/app/model/test_extension.py new file mode 100644 index 00000000000..f817aafda65 --- /dev/null +++ b/openbb_platform/core/tests/app/model/test_extension.py @@ -0,0 +1,121 @@ +"""Test Extension model and its functionalities.""" + +import sys +import types +from types import SimpleNamespace + +import pytest +from openbb_core.app.model.extension import CachedAccessor, Extension + + +def test_init_raises_if_invalid_configuration(): + # on_command_output False but other flags set -> should raise + with pytest.raises(ValueError): + Extension( + name="bad", + on_command_output=False, + command_output_paths=["/some/path"], + ) + + with pytest.raises(ValueError): + Extension(name="bad2", on_command_output=False, results_only=True) + + with pytest.raises(ValueError): + Extension(name="bad3", on_command_output=False, immutable=False) + + +def test_register_accessor_registers_and_warns_on_override(): + class Dummy: + accessors = set() + + def accessor_factory(obj): + return SimpleNamespace(called_with=obj) + + # Register new accessor + decorator = Extension.register_accessor("foo", Dummy) + returned = decorator(accessor_factory) + assert returned is accessor_factory # decorator returns the original accessor + assert hasattr(Dummy, "foo") + assert "foo" in Dummy.accessors + # descriptor instance is stored in the class __dict__; accessing via the + # class returns the result of descriptor.__get__, so inspect __dict__ + assert isinstance(Dummy.__dict__["foo"], CachedAccessor) + + # If attribute already exists, registration should warn + Dummy.existing = "I exist" # type: ignore + with pytest.warns(UserWarning): + Extension.register_accessor("existing", Dummy)(accessor_factory) + + # Clean up + if "foo" in Dummy.accessors: + Dummy.accessors.remove("foo") + if hasattr(Dummy, "foo"): + delattr(Dummy, "foo") + if hasattr(Dummy, "existing"): + delattr(Dummy, "existing") + + +def test_cached_accessor_descriptor_behavior_and_caching(): + class Dummy: + accessors = set() + + def __init__(self, name="n"): + self.name = name + + def make_accessor(obj): + # returns an object instance to be cached on the target instance + return SimpleNamespace(result=f"accessed-{obj.name}") + + Extension.register_accessor("myext", Dummy)(make_accessor) + + inst = Dummy("alice") + # Accessing on instance should call accessor and cache the result on instance + returned = inst.myext # type: ignore # pylint: disable=E1101 + assert isinstance(returned, SimpleNamespace) + assert returned.result == "accessed-alice" + # The attribute should now exist on the instance (cached) + assert getattr(inst, "myext") is returned + + # The descriptor object itself lives in the class __dict__ + cls_attr = Dummy.__dict__["myext"] + assert isinstance(cls_attr, CachedAccessor) + + # Clean up + Dummy.accessors.discard("myext") + if hasattr(Dummy, "myext"): + delattr(Dummy, "myext") + + +def test_obbject_accessor_registers_on_mocked_obbject_module(monkeypatch): + # Create a fake module to be imported in the property + mod_name = "openbb_core.app.model.obbject" + fake_mod = types.ModuleType(mod_name) + + class FakeOBBject: + accessors = set() + + fake_mod.OBBject = FakeOBBject # type: ignore + + # Install fake module into sys.modules so import inside property picks it up + monkeypatch.setitem(sys.modules, mod_name, fake_mod) + + ext = Extension(name="ext_name") + decorator = ( + ext.obbject_accessor + ) # should return the register decorator for FakeOBBject + + def sample_accessor(obj): + return SimpleNamespace(tag=f"ext-{getattr(obj,'_marker',None)}") + + decorator(sample_accessor) + + assert "ext_name" in FakeOBBject.accessors + assert hasattr(FakeOBBject, "ext_name") + # descriptor instance must be in the class __dict__ + assert isinstance(FakeOBBject.__dict__["ext_name"], CachedAccessor) + + # cleanup + FakeOBBject.accessors.discard("ext_name") + if hasattr(FakeOBBject, "ext_name"): + delattr(FakeOBBject, "ext_name") + monkeypatch.delitem(sys.modules, mod_name) diff --git a/openbb_platform/core/tests/app/test_command_runner.py b/openbb_platform/core/tests/app/test_command_runner.py index cff9a9ea335..fb7940a8f4d 100644 --- a/openbb_platform/core/tests/app/test_command_runner.py +++ b/openbb_platform/core/tests/app/test_command_runner.py @@ -2,6 +2,7 @@ from dataclasses import dataclass from inspect import Parameter +from types import SimpleNamespace from unittest.mock import Mock, patch import pytest @@ -15,6 +16,7 @@ from openbb_core.app.command_runner import ( ) from openbb_core.app.model.abstract.warning import OpenBBWarning from openbb_core.app.model.command_context import CommandContext +from openbb_core.app.model.extension import Extension from openbb_core.app.model.obbject import OBBject from openbb_core.app.model.system_settings import SystemSettings from openbb_core.app.model.user_settings import UserSettings @@ -49,7 +51,7 @@ class MockExecutionContext: @property def api_route(self) -> str: """Mock API route.""" - return MockAPIRoute(self.route) + return MockAPIRoute(self.route) # type: ignore @pytest.fixture() @@ -303,9 +305,9 @@ def test_command_runner_run(_): """Test run.""" runner = CommandRunner() - with patch( + with patch( # type: ignore "openbb_core.app.command_runner.StaticCommandRunner", - **{"return_value.run": True}, + **{"return_value.run": True}, # type: ignore ): assert runner.run("mock/route") @@ -371,7 +373,7 @@ async def test_static_command_runner_execute_func( mock_command.return_value = OBBject( results=[1, 2, 3, 4], provider="mock_provider", - accessors={"charting": Mock()}, + accessors={"charting": Mock()}, # type: ignore ) mock_chart.return_value = None @@ -396,13 +398,13 @@ def test_static_command_runner_chart(): {"date": "1992", "value": 300}, ], provider="mock_provider", - accessors={"charting": Mock()}, + accessors={"charting": Mock()}, # type: ignore ) - mock_obbject.charting.show = Mock() + mock_obbject.charting.show = Mock() # type: ignore StaticCommandRunner._chart(mock_obbject) # pylint: disable=protected-access - mock_obbject.charting.show.assert_called_once() + mock_obbject.charting.show.assert_called_once() # type: ignore @pytest.mark.asyncio @@ -415,7 +417,7 @@ async def test_static_command_runner_command(): def __init__(self, results, **kwargs): self.results = results self.extra = {} - self.provider = kwargs.get("provider_choices").provider + self.provider = kwargs.get("provider_choices").provider # type: ignore class MockProviderChoices: """Mock ProviderChoices""" @@ -435,3 +437,116 @@ async def test_static_command_runner_command(): assert result.results == [1, 2, 3, 4] assert result.provider == "mock_provider" + + +def test_extension_immutable_preserves_original_and_does_not_set_extension_modified( + monkeypatch, +): + """Immutable extensions must run against a copy and must not mutate the original OBBject.""" + monkeypatch.setattr( + "openbb_core.app.service.system_service.SystemService", + lambda: SimpleNamespace( + system_settings=SimpleNamespace( + allow_on_command_output=True, allow_mutable_extensions=False + ) + ), + ) + ext = Extension(name="imm_ext_test", on_command_output=True, immutable=True) + + def imm_accessor(self): + # Mutate the (copied) obbject if called + if isinstance(getattr(self, "results", None), list): + self.results.append("modified_by_imm") + + # Attach accessor to OBBject class for binding on instances (monkeypatch will revert) + monkeypatch.setattr(OBBject, ext.name, imm_accessor, raising=False) + + fake_loader = SimpleNamespace(on_command_output_callbacks={"*": [ext]}) + monkeypatch.setattr( + "openbb_core.app.command_runner.ExtensionLoader", lambda: fake_loader + ) + + obb = OBBject(results=[1], provider="mock_provider") + + StaticCommandRunner._trigger_command_output_callbacks("any/route", obb) + + # original must remain unchanged + assert obb.results == [1] + # immutable extension should not mark the original as modified + assert getattr(obb, "_extension_modified", False) is False + + +def test_extension_mutable_modifies_original_and_sets_extension_modified_and_route_scoping( + monkeypatch, +): + """Mutable extensions must modify the original OBBject and set the modification flag; + registration must be route-scoped.""" + monkeypatch.setattr( + "openbb_core.app.service.system_service.SystemService", + lambda: SimpleNamespace( + system_settings=SimpleNamespace( + allow_mutable_extensions=True, allow_on_command_output=True + ) + ), + ) + ext = Extension(name="mut_ext_test", on_command_output=True, immutable=False) + + def mut_accessor(self): + if isinstance(getattr(self, "results", None), list): + self.results.append("modified_by_mut") + + monkeypatch.setattr(OBBject, ext.name, mut_accessor, raising=False) + + # register the extension only for "mock/route" + fake_loader = SimpleNamespace(on_command_output_callbacks={"mock/route": [ext]}) + monkeypatch.setattr( + "openbb_core.app.command_runner.ExtensionLoader", lambda: fake_loader + ) + + obb = OBBject(results=[], provider="mock_provider") + + # not executed for other routes + StaticCommandRunner._trigger_command_output_callbacks("other/route", obb) + assert obb.results == [] + + # executed for the registered route and should mutate original + StaticCommandRunner._trigger_command_output_callbacks("mock/route", obb) + assert obb.results == ["modified_by_mut"] + assert getattr(obb, "_extension_modified", False) is True + + +def test_results_only_flag_sets_attribute_and_accessor_runs(monkeypatch): + """Extensions that declare results_only should toggle the _results_only attribute + and still run their accessor.""" + monkeypatch.setattr( + "openbb_core.app.service.system_service.SystemService", + lambda: SimpleNamespace( + system_settings=SimpleNamespace( + allow_on_command_output=True, allow_mutable_extensions=False + ) + ), + ) + ext = Extension( + name="ro_ext_test", on_command_output=True, results_only=True, immutable=True + ) + + called = {"hit": False} + + def ro_accessor(self): + called["hit"] = True + + monkeypatch.setattr(OBBject, ext.name, ro_accessor, raising=False) + + fake_loader = SimpleNamespace(on_command_output_callbacks={"*": [ext]}) + monkeypatch.setattr( + "openbb_core.app.command_runner.ExtensionLoader", lambda: fake_loader + ) + + obb = OBBject(results=[1, 2, 3], provider="mock_provider") + + StaticCommandRunner._trigger_command_output_callbacks("any/route", obb) + + # results_only attribute must be set on the output OBBject + assert getattr(obb, "_results_only", False) is True + # accessor should have been called (even if on a copy for immutable extensions) + assert called["hit"] is True diff --git a/openbb_platform/obbject_extensions/charting/openbb_charting/charting.py b/openbb_platform/obbject_extensions/charting/openbb_charting/charting.py index 6ee3ed5bd95..6b31261c786 100644 --- a/openbb_platform/obbject_extensions/charting/openbb_charting/charting.py +++ b/openbb_platform/obbject_extensions/charting/openbb_charting/charting.py @@ -472,24 +472,63 @@ class Charting: def show(self, render: bool = True, **kwargs): """Display chart and save it to the OBBject.""" + # pylint: disable=import-outside-toplevel + from openbb_charting.core.openbb_figure import OpenBBFigure + try: charting_function = self._get_chart_function( - self._obbject._route # pylint: disable=protected-access + self._obbject._route # pylint: disable=protected-access # type: ignore ) kwargs["obbject_item"] = self._obbject.results kwargs["charting_settings"] = self._charting_settings kwargs["standard_params"] = ( self._obbject._standard_params # pylint: disable=protected-access ) - kwargs["extra_params"] = ( - self._obbject._extra_params # pylint: disable=protected-access + # If the provider interface isn't used, endpoint kwargs are already here. + # Don't overwrite them. + obb_kwargs = ( + self._obbject._extra_params or {} # pylint: disable=protected-access ) + if obb_kwargs: + for k, v in obb_kwargs.items(): + kwargs["extra_params"].update({k: v}) + kwargs["provider"] = self._obbject.provider kwargs["extra"] = self._obbject.extra - fig, content = charting_function(**kwargs) - content = fig.show(external=True, **kwargs).to_plotly_json() # type: ignore - self._obbject.chart = Chart(fig=fig, content=content, format=self._format) - if render: + + # Handle different types of output from the charting endpoint. + chart_response: Any = charting_function(**kwargs) + + # If returned a Chart object, set as-is. + if isinstance(chart_response, Chart): + self._obbject.chart = chart_response + # If just an OpenBBFigure gets returned, create the serialized version for the API. + elif isinstance(chart_response, OpenBBFigure): + fig = chart_response + content = fig.show(external=True, **kwargs).to_plotly_json() + self._obbject.chart = Chart( + fig=fig, content=content, format=self._format + ) + # Current functions return this. + elif isinstance(chart_response, tuple) and len(chart_response) == 2: + fig, content = chart_response + + if isinstance(fig, OpenBBFigure): + content = fig.show(external=True, **kwargs).to_plotly_json() # type: ignore + self._obbject.chart = Chart( + fig=fig, content=content, format=self._format + ) + else: + self._obbject.chart = Chart( + fig=fig, content=content, format=type(fig).__name__ + ) + + else: + self._obbject.chart = Chart( + fig=chart_response, content=None, format="unknown" + ) + + if render and hasattr(fig, "show"): fig.show(**kwargs) except (RuntimeError, OpenBBError) as e: @@ -712,8 +751,9 @@ class Charting: self._backend.send_table( df_table=data_as_df, title=title - or self._obbject._route, # pylint: disable=protected-access - theme=self._charting_settings.table_style, + or "" + or self._obbject._route, # pylint: disable=protected-access # type: ignore + theme=self._charting_settings.table_style, # pylint: disable=protected-access ) except Exception as e: # pylint: disable=W0718 warn(f"Failed to show figure with backend. {e}")