Skip to content
This repository has been archived by the owner on May 22, 2020. It is now read-only.

Run plugs on the socket before the actual action #10

Merged
merged 21 commits into from
Jun 21, 2018

Conversation

crbelaus
Copy link
Contributor

@crbelaus crbelaus commented Jun 4, 2018

This pull request closes #4.

The main objective of this PR is to allow Frankt users to act on the socket before the action handler is executed.

Acting on the socket

In some projects we have found the need of performing certain actions on the socket even before running the action handler. For example: assigning certain values in the socket or setting up monitoring instrumentation.

Before this PR there was no way to perform those actions on the socket, so the only possibility was to throw all those responsibilities into the action handler. It was soon obvious that we can handle those cases in a better way.

We took the inspiration from the Plug package. Frankt channels can now implement a plugs function which returns a list of modules implementing the Frankt.Plug behaviour. Those modules are run in order and can do whathever they want with the socket: assigning or modifying values, pushing messages, setting up instrumentation, etc.

As way to test the implementation and provide live documentation we reimplemented Frankt internals as a series of plugs, the last one being the action handler provided by the user. This has led to clearer and more modular code, while maintaing Frankt public interface unmodified.

Tasks

This is a detailed list of the tasks that have been done in this pull request:

  • Allow plugs to be run before the action handler.
    • Rewrite Frankt action steps into plugs.
    • Maintain previous interface compatibility.
    • Allow configuring options to the plugs.
  • Add tests for a Frankt channel with a custom plug.
  • Add tests for a Frankt channel with a Gettext configuration.
  • Configure the test application in the test helper and remove the configuration files.
  • Improve documentation

I've also added Credo as a direct dependency so we can control the linting configuration and use the new checks that are not included in Ebert.

  • Add Credo as a development dependency.
  • Configure Credo to run on strict mode.
  • Lint the project with Credo in Travis builds.

crbelaus added 2 commits June 4, 2018 20:38
Frankt now allows the configuration of plugs that will be run before the
actual action handler. The plugs may modify the socket and add assigns
that can be used later either in another socket or in the action
handler.
@crbelaus crbelaus self-assigned this Jun 4, 2018
@crbelaus crbelaus force-pushed the features/4-allow-plugs branch from 7634002 to 7929ca5 Compare June 8, 2018 18:24
@acutario acutario deleted a comment from sourcelevel-bot bot Jun 8, 2018
crbelaus added 6 commits June 8, 2018 20:30
When a channel configures its plugs, it can add some extra options that
will be later received in the plug when called.
Travis builds will now lint the project with Credo in strict mode. If
any errors are found, the test won't be run.
@crbelaus crbelaus force-pushed the features/4-allow-plugs branch from 095b821 to de8863b Compare June 14, 2018 17:17
Copy link
Member

@odarriba odarriba left a comment

Choose a reason for hiding this comment

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

Besides the comment, it looks amazing!
💙 ❤️ 💛 💜 💚

end

@impl true
def call(%{private: %{frankt_module: frankt_module}}, _opts) do
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this line pattern match to this?

def call(%{private: %{frankt_gettext: frankt_gettext, frankt_module: frankt_module}}, _opts) when not is_nil(frankt_gettext) do``` 

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am taking a look at the code again and I think that the when not is_nil(frankt_gettext) guard can be omitted since it would previously match with the function declaration in line 10.

def call(socket = %{private: %{frankt_gettext: nil}}, _opts) do
  invoke_action(socket)
end

The function clause order would be the following one:

  1. No Gettext configured: go ahead and invoke the action.
  2. Gettext configured and locale set: go ahead and invoke the action in an internationalized environment.
  3. Other cases (Gettext configured but no locale set), raise an error.

Copy link
Member

Choose a reason for hiding this comment

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

Totally agree on that!

crbelaus added 6 commits June 18, 2018 19:09
When using the `Frankt.Plug` module, it automatically declares the
`Frankt.Plug` behaviour and imports the `Phoenix.Socket.assign/3`
function into modules.
@crbelaus crbelaus force-pushed the features/4-allow-plugs branch from 2ec41f4 to a10dfbc Compare June 19, 2018 15:54
@crbelaus crbelaus force-pushed the features/4-allow-plugs branch from a10dfbc to 87a4fb2 Compare June 19, 2018 15:56
@crbelaus
Copy link
Contributor Author

I've been adding some more improvements to the docs and updating the README.

The docs still require more improvements and updates, but I think that we can merge this branch and prepare the 1.0 release. The docs can be improved bit by bit over time.

@vortizhe vortizhe self-requested a review June 21, 2018 14:45
Copy link
Member

@vortizhe vortizhe left a comment

Choose a reason for hiding this comment

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

LGTM!

@crbelaus crbelaus merged commit 18b036f into master Jun 21, 2018
@crbelaus crbelaus deleted the features/4-allow-plugs branch June 21, 2018 14:46
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow plugging functions to act on the socket before handling the action
3 participants