Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: return class in injectable decorator #59

Merged
merged 2 commits into from
May 20, 2024

Conversation

Ignacio-Freire
Copy link
Contributor

@Ignacio-Freire Ignacio-Freire commented May 12, 2024

Proposal

Linked to issue #58

The issue with the Injectable decorator returning a string instead of the class itself likely stems from the decorator definition.

In the definition of Injectable, cls is treated as if it could potentially be a string (cls: str = None), which suggests that the decorator might be designed to accept optional parameters or configurations. However, if not handled properly, this design could lead to unexpected behavior, such as treating a class definition as a string.

Correcting the Injectable Decorator

The decorator needs to correctly handle both cases: when it is used with and without parameters. Here’s a more standard way of creating a decorator that can optionally accept arguments

Explanation

  1. Decorator Factory: The Injectable function can now correctly handle being called either as @Injectable or @Injectable(). It uses a nested decorator function to apply the actual class modifications.

  2. Class Modifications: It checks if the class has an init method defined and, if not, assigns a default one. It then parses dependencies, sets necessary attributes, and applies injection.

  3. Handling Arguments: The outer function (Injectable) checks if it is given a class directly (cls is not None). If so, it directly returns the decorator applied to the class. Otherwise, it returns the decorator function itself, allowing for further customization or arguments.

Tests results

❯ pytest
============================================ test session starts ============================================
platform linux -- Python 3.11.7, pytest-8.2.0, pluggy-1.5.0
rootdir: /projects/personal/PyNest
configfile: pyproject.toml
plugins: anyio-4.3.0
collected 20 items

tests/test_core/test_database/test_odm.py ...                                                         [ 15%]
tests/test_core/test_database/test_orm.py ....                                                        [ 35%]
tests/test_core/test_decorators/test_controller.py .....                                              [ 60%]
tests/test_core/test_pynest_application.py ...                                                        [ 75%]
tests/test_core/test_pynest_container.py ..                                                           [ 85%]
tests/test_core/test_pynest_factory.py ...                                                            [100%]

============================================= warnings summary ==============================================
tests/test_core/test_decorators/test_controller.py:6
  cannot collect test class 'TestController' because it has a __init__ constructor (from: tests/test_core/test_decorators/test_controller.py)
    @Controller(prefix="api/v1/user", tag="test")

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
======================================= 20 passed, 1 warning in 0.26s =======================================

# Proposal
Linked to PythonNest#58

The issue with the Injectable decorator returning a string instead of the class itself likely stems from the decorator definition.

In the definition of Injectable, cls is treated as if it could potentially be a string (cls: str = None), which suggests that the decorator might be designed to accept optional parameters or configurations. However, if not handled properly, this design could lead to unexpected behavior, such as treating a class definition as a string.

# Correcting the Injectable Decorator
The decorator needs to correctly handle both cases: when it is used with and without parameters. Here’s a more standard way of creating a decorator that can optionally accept arguments

# Explanation
1. Decorator Factory: The Injectable function can now correctly handle being called either as @Injectable or @Injectable(). It uses a nested decorator function to apply the actual class modifications.

2. Class Modifications: It checks if the class has an __init__ method defined and, if not, assigns a default one. It then parses dependencies, sets necessary attributes, and applies injection.

3. Handling Arguments: The outer function (Injectable) checks if it is given a class directly (cls is not None). If so, it directly returns the decorator applied to the class. Otherwise, it returns the decorator function itself, allowing for further customization or arguments.
def decorator(inner_cls):
if "__init__" not in inner_cls.__dict__:
def __init__(self, *args, **kwargs):
pass
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is not much difference, but I find the "..." more aesthetic then "pass"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Completely agree! 6dabee6

from nest.core.decorators.utils import parse_dependencies


def Injectable(cls=None, *args, **kwargs):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's the different between cls and inner_cls?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technical summary

  • cls This is the class that the Injectable decorator is being applied to if the decorator is used without parentheses. In other words, if you use the decorator like this:
@Injectable
class MyClass:
    pass

In this case, cls will be MyClass.

  • inner_cls This is always the class being decorated, whether the decorator is called with parentheses or not. When the decorator is called with parentheses, inner_cls will be the class passed to the decorator function.

Why is this useful?

  • Code Reusability: This pattern makes the Injectable decorator reusable and extensible without needing to rewrite or overload it for different use cases.
  • Ease of Use: It provides a convenient way to apply dependency injection to classes with minimal syntax, making the code cleaner and more readable.
  • Future-Proofing: Even if no arguments are needed now, the pattern allows for easy addition of functionality in the future without changing the way existing classes are decorated.

Summary

This approach allows the Injectable decorator to be used in a flexible and future-proof manner, supporting both simple and potentially more complex usage scenarios while keeping the implementation clean and maintainable.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ohh Now i get it, this is brilliant, and in the future, it will enable to add some variables to this decorator (like scope for example, if I want that provider/service will be singleton or other type of injection scope. this is very nice change.

@ItayTheDar ItayTheDar self-requested a review May 17, 2024 05:34
@ItayTheDar ItayTheDar merged commit 21e6b04 into PythonNest:main May 20, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants