pcah / python-clean-architecture

A Python toolkit for applications driven by The Clean Architecture
MIT License
455 stars 43 forks source link

Application: Alternative design for Interactors #67

Open lhaze opened 4 years ago

lhaze commented 4 years ago

Current implementations of Interactor pattern (#4, #51) has several flaws to consider:

  1. It uses hook methods (like Django's CBV) which may end up in a hook hell when your code flow gets more complex
  2. It promotes writing more complex code flow when you have to do something non-trivial, i.e. different handling of errors got from different parts of the business logic flow.
  3. Closely couples calling the interactor with the process of gathering input which might not always be the case, i.e. heartbeat-driven interactions with gathering data being part of the interactions and not the calling process.
  4. It presumes that there is a single source of input, represented by the InputPort and RequestModel. Doesn't generalize enough about sources of data, i.e. a model got from a database repository is treated differently than the data got from request.
  5. It doesn't promote actual modeling of the InputPort: a generic interface of the RequestModel doesn't have such nice dataclass features as domain objects got from any Repository.
  6. Separation of validation and actual processing might be somewhat artificial. There's no fundamental difference between validation errors (i.e. this id is invalid) and business process errors (i.e. this item is in an invalid state).
lhaze commented 4 years ago

Example of problems mentioned in (2):

Exceptions are not exceptional

Let’s look at another example, a typical code to access remote HTTP API:

def fetch_user_profile(user_id: int) -> 'UserProfile':
   """Fetches UserProfile dict from foreign API."""
   response = requests.get('/api/users/{0}'.format(user_id))
   response.raise_for_status()
   return response.json()

Literally, everything in this example can go wrong. Here’s an incomplete list of all possible errors that might occur:

  1. Your network might be down, so request won’t happen at all
  2. The server might be down
  3. The server might be too busy and you will face a timeout
  4. The server might require an authentication
  5. API endpoint might not exist
  6. The user might not exist
  7. You might not have enough permissions to view
  8. The server might fail with an internal error while processing your request
  9. The server might return an invalid or corrupted response
  10. The server might return invalid json, so the parsing will fail

And the list goes on and on! There are so maybe potential problems with these three lines of code, that it is easier to say that it only accidentally works. And normally it fails with the exception.

-- Nikita Sobolev, Python exceptions considered an anti-pattern

The most common solutions (try-catches, if-Nones, else ifing inside _handle_foo methods) too often lead either to very complex code or to the problem (1), e.g. hell of a hoard of hook methods for handling different cases.

lhaze commented 4 years ago

What can be done:

lhaze commented 4 years ago

Ok, let's take Sobolev's example to formulate expectations about Interactor (aka Use Case):

class AddMember(Interactor):

    @in
    class Request:
        team_id: str
        member_id: str

    @out
    class Reply:
        success: bool

    team_repo: Repository = Inject(qualifier=Team)
    user_repo: UserRepository = Inject()
    service: AuthorizationService = Inject()

    process = combine(
        (validate_input, bail_out),
        (get_team, bail_out),
        (get_user, take_current_user),
        (add_user_to_team, bail_out),
        (send_notification, ignore_errors),
    )
lhaze commented 4 years ago

What can be done (cont'd):

class AddMember(Interactor):

    @in
    class Request:
        team_id: str
        member_id: str

    @out
    class Reply:
        success: bool

    request: Request = Inject()
    team_repo: Repository = Inject(qualifier=Team)
    user_repo: UserRepository = Inject()
    service: AuthorizationService = Inject()

    process = combine(
        (validate_input, bail_out),
        (get_team, bail_out),
        (get_user, take_current_user),
        (add_user_to_team, bail_out),
        (send_notification, ignore_errors),
    )

    __call__ = process
lhaze commented 4 years ago

Interactor as process orchestrator vs. immutability

Process Orchestrator

You might think that Interactor, as an orchestrator of all the fragments of process' logic, might be a container for all potential side products of each step. E.g. get_team step has to pass an instance of a Team to any next step, it may assign it to an explicitly defined field on the Interactor instance.

class AddMember(Interactor):

    ...

    request: Request = Inject()
    team_repo: Repository = Inject(qualifier=Team)
    user_repo: UserRepository = Inject()
    service: AuthorizationService = Inject()

    team: Team
    user: User

    process = combine(
        (validate_input, bail_out),
        (get_team, bail_out),
        (get_user, take_current_user),
        (add_user_to_team, bail_out),
        (send_notification, ignore_errors),
    )

So when add_user_to_team comes into play, it could take it out of the AddMember:

def add_member_to_team(interactor: AddMember):
    interactor.team.append(interactor.user)

It has the advantage that interactor: AddMember is probably the only argument every step of process logic has to have. Another feature is the possibility to explicitly state all the by-products of the process. The drawback is: steps aren't pure functions (vide: The Principles, #⁠2). Every step mutates the argument a bit. None of them would be pure.

Immutability

It's great to have key fragments of logic made pure and with immutable arguments. All you have to test is the mapping between its input and output. But this means that there is a state, specifying values being transferred from a step to any other (dict? Bunch?, TypedDict from Py38? but always a new copy). And this means that steps have other argument that represents the current state of the process and possible problems with typing.

def add_member_to_team(interactor: AddMember, state: AddMemberState):
    state['team'].append(state['team'])

The interactor instance can't serve as an immutable copy of the state: creating another instance of the interactor would reset DI instances on Inject markers, which makes DI and the whole idea of an orchestrator pointless.

What a choice...

lhaze commented 4 years ago

Ok, let's keep it simple. Imagine we have a function defining interaction logic. We want to make it:

... then you can do it just by

Example 1 (no composition within process)

class AddMember(Interactor):

    ...

    request: Request = Inject()
    presenter: Presenter = Inject(qualifier='AddMember')

    team_repo: Repository = Inject(qualifier=Team)
    user_repo: UserRepository = Inject()
    ...

    @success
    def process(self):
        request_model: AddMemberRequest = AddMemberModel(self.request.params)
        team: Team = team_repo.find(request.team_id)
        user: User = team_repo.find(request.team_id)
        team.users.append(user)
        team_repo.update(team)
        self.presenter(team_name=team.name, user_id=user.id)

    @process.failure(raises=TeamManagementErrors.TEAM_NOT_FOUND)
    def _(self, error: LogicError, **kwargs):
        self.presenter(error=error.short_description)

# and then:

interactor = AddMember()
result = interactor()
assert result is None, \
    "The content to present is served by Presenter, and not returned"

NB:

lhaze commented 4 years ago

An interaction logic function defined outside the interactor.

Example 2 (no composition, external function)

# pure_domain_logic.py
@success
@inject
def add_member(
    interactor: Interactor,
    team_repo: Repository = Inject(qualifier=Team),
    **kwargs) -> Union[Success, Failure]:
    ...

@add_member.on_failure
def _(interactor: Interactor, **kwargs)
    return ...

# my_application_logic.py
class AddMember(Interactor):
    ...
    request: Request = Inject()
    presenter: Presenter = Inject(qualifier='AddMember')
    user_repo: UserRepository = Inject()

    process = add_member

NB:

lhaze commented 4 years ago

The reason behind all this stuff: Railway-Oriented Programming

pedro2555 commented 4 years ago

An interaction logic function defined outside the interactor.

Example 2 (no composition, external function)

# pure_domain_logic.py
@success
@inject
def add_member(
    interactor: Interactor,
    team_repo: Repository = Inject(qualifier=Team),
    **kwargs) -> Union[Success, Failure]:
    ...

@add_member.on_failure
def _(interactor: Interactor, **kwargs)
    return ...

# my_application_logic.py
class AddMember(Interactor):
    ...
    request: Request = Inject()
    presenter: Presenter = Inject(qualifier='AddMember')
    user_repo: UserRepository = Inject()

    process = add_member

NB:

  • From one side, The Clean Architecture imposes an explicit call to the presenter, without passing the result with return. On the other side, functional programming (and our Principles too!) wants you to make side-effect-free, and Railway-Oriented Programming expects you to return Result = Success | Failure. Is a way of return_message (see the presentation) the only way?
  • The domain logic execution functions may not have the way to get interactor's DI attributes type-safely. There may be no reasonable way to pass the interactor's interface to them. Still, the functional injection way is valid. But why to have Interactor class at all? To specify InputPort explicit?
  • And where's the place for RequestModel in this picture?

This creates a dependency from the domain layer to the application. It should be the other way around.