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

Customizable Client struct module #123

Closed
christhekeele opened this issue Oct 15, 2017 · 4 comments
Closed

Customizable Client struct module #123

christhekeele opened this issue Oct 15, 2017 · 4 comments

Comments

@christhekeele
Copy link

Hello again! You may remember me from such issues as #122. 😄

The project I'm working on wraps several APIs. This means that I'm __using__ Tesla in a few places to construct several API client modules with different behaviours.

Each API client is associated with several modules that actually perform the API calls––the surface of these APIs is very wide and benefits from being split up into conceptual buckets.

It would be nice to have my API-client-calling modules type-check that you are using the correct client for the given module. The cleanest and most idiomatic way to do this would be to pattern match on the client struct type, but all Tesla-built clients are Tesla.Client structs. It would be helpful if I could customize the struct for any given call to use Tesla.

I envision providing an extra option to the using call, i.e: use Tesla, client: My.API.Client. If this parameter is provided it would define a new struct and thread it throughout all pattern matching on its type in Tesla macros. If it is not provided it would thread the existing Tesla.Client module reference through instead, keeping everything backwards compatible.

This seems possible based on the current implementation of Tesla––all pattern matching on the module name of the client struct lives within macro calls that could be meta-programmed or parameterized. The few other places it is explicitly referenced we could adjust for, I have some ideas on how to manage. With your blessing I would be happy to pursue this further and submit a PR.

The places we are currently matching on it are within the builder:

@teamon
Copy link
Member

teamon commented Oct 16, 2017

Hi!

Can you show an example of your code? I don't think we can skip the %Tesla.Client{} match since this is the way we can achieve current api - get/2 (path, opts) vs get/2 (client, path)

@christhekeele
Copy link
Author

christhekeele commented Oct 31, 2017

To clarify, the point of this wouldn't be to skip the %Tesla.Client{} match, it would be to allow you to modify that match to use a different struct name, so that different get/2 functions in different modules that both use Tesla will refuse to use each-other's clients.

Currently, this works fine, because APITwo.get/2 matches on %Tesla.Client{} returned by APIOne.client:

defmodule APIOne do
  use Tesla
  def client do
    Tesla.build_client [
      {Tesla.Middleware.Headers, %{"Authorization" => "token: " <> token }}
    ]
  end
end

defmodule APITwo, do: use Tesla

APITwo.get(APIOne.client, "foo")

My proposal is to allow Tesla to generate client structs 'private' to a particular invocation of use Tesla, such that:

defmodule APIOne do
  use Tesla, client: APIOne.Client
  def client do
    APIOne.Client.build [
      {Tesla.Middleware.Headers, %{"Authorization" => "token: " <> token }}
    ]
  end
end

defmodule APITwo, do: use Tesla, client: APITwo.Client

APITwo.get(APIOne.client, "foo")

raises:

** (FunctionClauseError) no function clause matching in APITwo.get/2

    The following arguments were given to APITwo.get/2:

        # 1
        %APIOne.Client{...}

        # 2
        "foo"

    iex:1: Foo.bar/1

Because, since APIOne's use Tesla was given a client struct name, it generated a client struct with that name (%APIOne.Client{}) , and because APITwo was also given a client struct name, it generated a APITwo.get/2 that is matching on that specific struct (%APITwo.Client{}) instead of any old %Tesla.Client{}.

@christhekeele
Copy link
Author

Here, this is kind of what I was envisioning.

@teamon
Copy link
Member

teamon commented Mar 5, 2018

Closing in favour of #133

@teamon teamon closed this as completed Mar 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants