Better design pattern for creating a polymorphic API

The other day I wrote a small lib to interact with AWS IAM (on top of ExAws). I wanted the internal API to have one Parser.parse/2 function that would handle any response. ExAws executes the request and calls this function with the response and the action name as the two arguments. For example:

parse.call(response, action)

Where response is {:ok, %{body: xml, status_code: status}} and action is the AWS IAM operation (“CreateUser”, “DeleteUser”, etc…).

I created a single module with a parse/2 action that dispatched to a function, which in turn called the right parser in the right module (pattern matching on the action name).

defmodule ExAws.Iam.Parser do
  alias ExAws.Iam.Parsers.{AccessKey, User}

  def parse({:ok, %{body: xml, status_code: status} = resp}, action) when status in 200..299 do
    parsed_body = dispatch(xml, action)
    {:ok, %{resp | body: parsed_body}}
  end
  def parse(resp, _), do: resp

  @user_actions ~w[ListUsers CreateUser]
  defp dispatch(xml, action) when action in @user_actions do
    User.parse(xml, action)
  end

  @access_key_actions ~w[ListAccessKeys GetAccessKeyLastUsed]
  defp dispatch(xml, action) when action in @access_key_actions do
    AccessKey.parse(xml, action)
  end
end

I then created individual parsing modules for each entity:

defmodule ExAws.Iam.Parsers.User do
  def parse(xml, "ListUsers") do
    # parse the xml
  end

  def parse(xml, "GetUser") do    
    # parse the xml
  end

  # etc...
end

defmodule ExAws.Iam.Parsers.AccessKey do
  # ...
end

I’m not a fan, though. The Parsers.AccessKey and Parsers.User should be private (which we don’t have in Elixir). I can’t import the functions because the names would clash. I also feel like I’m abusing pattern matching.

Protocols won’t work because the data types don’t change. What other patterns can I use to achieve the same API?

1 Like

Thinking out loud, I suppose behaviours could be a solution? If I’m willing to accept that passing the right parser as an argument is better than having a polymorphic parse/2 function.

I could declare parse/2 as a behaviour and implement it in each module (Parsers.User, and Parsers.AccessKey). I would then pass the right parser as an argument.

  def parse({:ok, %{body: xml, status_code: status} = resp}, action) when status in 200..299 do
    parsed_body = dispatch(xml, action)
    {:ok, %{resp | body: parsed_body}}
  end

Still, it would be nice to just call parse/2 and have the API figure out which module to call.

Have you considered:

  # in some module ..

  def list_users, do: {User,:list_users}
  def create_user, do: {User,:create_user}
  def list_access_keys, do: {AccessKey, :list_access_keys}
  def get_access_key_last_used, do: {AccessKey, :get_access_key_last_used}

  # somewhere else ...

  def parse({:ok, %{body: xml, status_code: status} = resp}, {mod,fun}) when status in 200..299 do
    parsed_body = apply(mod, fun, [xml])
    {:ok, %{resp | body: parsed_body}}
  end

I don’t control the calling code. ExAws does that part. It just calls the parser I provide as an argument.

Here’s how I currently send the parser:

  def list_users(opts \\ []) do
    :list_users
    |> to_params(opts)
    |> to_op(parser: &Parser.parse/2)
  end

So it’s easy enough to replace |> to_op(parser: &Parser.parse/2) with |> to_op(parser: &Parsers.User.parse/2). But I was looking for a better way.

I do this kind of things with adapter pattern where I set the adapter in config, have a behavior that is an interface for adapters and few adapter modules (some call API, some use ETS some just serve hardcoced responses). A “public” wrapper module can then call configured adapter and depending on the env different implementations can be used, the context (that is the only really public module) can delegate to that wrapper module then.

The best way in Elixir I could see is a mix of your original approach, and behaviours.

  1. Behaviour:
defmodule ExAws.Iam.Consumer do
  @callback consume_response(String.t, String.t) :: any # (xml, action)
end
  1. Implementers:
defmodule User do
  @behaviour ExAws.Iam.Consumer
  def consume_response(xml, action), do: #...
end
  1. Refactor dispatch to use router functions like so:
defp consumer(action) when action in @user_actions, do: User
defp consumer(action) when action in @access_key_actions, do: AccessKey

#...

defp dispatch(xml, action), do: consumer(action).consume_response(xml, action)

Code is not tested. But IMO this brings you close enough to some practical polymorphism while remaining terse enough.

(Another alternative would be to have a big module where you match on all possible combos but I don’t think it would be an improvement over your current approach – except maybe for having less source files.)

2 Likes
def SomeModule do

  @action_parsers %{
    "ListUsers" => User,
    "CreateUser" => User,
    "ListAccessKeys" => AccessKey,
    "GetAccessKeyLastUsed" => AccessKey
  }
 
  def parse({:ok, %{body: xml, status_code: status} = resp}, action) when status in 200..299 do
    with {:ok, mod} <- Map.fetch(@action_parsers, action) do
       parsed_body = apply(mod, :parse, [xml, action])
       {:ok, %{resp | body: parsed_body}}
    else
      _ ->
        # BOOM
    end
  end

end

But as it is you’ll probably be best off by pursuing the behaviour angle further if just to capture the implementation commonalities between your various parsers.

2 Likes

That’s essentially what I had before I went into a refactoring rabbit hole. It might just be simpler to pass the parser in the public function. For example:

  def list_users(opts \\ []) do
    :list_users
    |> to_params(opts)
    |> to_op(parser: &Parsers.User.parse/2)
  end

  def list_access_keys(opts \\ []) do
    :list_access_keys
    |> to_params(opts)
    |> to_op(parser: &Parsers.AccessKey.parse/2)
  end

That’s pretty cool, but it sacrifices explicitness especially as the codebase gets bigger (my current approach also sacrifices explicitness).

Valid reservation. I just feel we can’t go all the way explicit if we need polymorphism though.

As an extra step I would recommend you just make a separate configuration file and just use that in order not to have too much routing logic hardcoded in a module (where other team members might have trouble finding it or it will become hard to maintain as you pointed out).

If you opt for that you should definitely put that string_action->module map that @peerreynders suggested into that config.

I understand the nagging feeling of “it just doesn’t feel right” but don’t get stuck into paralysis analysis. Truth is, functional languages are not ideal when polymorphism is involved and that’s okay because it’s not their main focus. But behaviours and map routers get you close enough, and it’s not like the OOP languages don’t do the same anyway!

IMO just go for the minimal maintenance approach and move on. Even if the code is a bit more. Important part is to be able to add/remove consumers in a minute.

1 Like

In some contexts “passing a function” can be seen as the functional equivalent of OO’s strategy pattern.

In the end, I stuck with my approach of having a Parser module pattern-match on the action name and delegate to the correct module for parsing. This way I only have to specify a parser once.

  def parse({:ok, %{body: xml, status_code: status} = resp}, action) when status in 200..299 do
    parsed_body = dispatch(xml, action)
    {:ok, %{resp | body: parsed_body}}
  end
  def parse(resp, _), do: resp

  @user_actions ~w[ListUsers GetUser]  # etc ...

  defp dispatch(xml, action) when action in @user_actions do
    User.parse(xml, action)
  end

I also realise that the API I wrote was contrived. A single generic operation/2 function would have allowed me to interact with the entire IAM API without having to write a function for each IAM action/endpoint, as I was doing. It can be used by the internal API for convenience functions too.

  def operation(action, params, opts \\ []) do
    {parser, params} = Keyword.pop(params, :parser, &Parser.parse/2)
    opts = Keyword.put_new(opts, :parser, parser)

    @shared_opts
    |> Keyword.merge(params)
    |> Keyword.put(:action, camelize(action))
    |> list_to_camelized_map()
    |> to_operation(opts)
  end

operation(:create_user, user_name: "foo")

# or for internal use

def create_user(username, opts \\ []) do
  operation([user_name: username] ++ opts)
end

Below is what I had until now. Those functions (create_user/2, list_users, etc…) are now relegated being to convenience functions only.

  def create_user(username, opts \\ []) do
    operation(:create_user, [user_name: username] ++ opts)
  end

  defp to_operation(params, opts) do
    %ExAws.Operation.Query{
      action: params["Action"],
      params: params,
      parser: Keyword.get(opts, :parser),
      path: params["Path"] || "/",
      service: :iam
    }
  end

So, what good are they? Well, maybe I can convert them to execute the operation on AWS instead of returning an ExAws op. For example:

  def create_user(username, opts \\ []) do
    :create_user
    |> operation([user_name: username] ++ opts)
    |> ExAws.request()
    |> to_user_struct()
  end
%User{
 arn: ...,
 create_date: ...,
 path: ...,
 user_name: ...,
 user_id: ...
}

Finally, it would be nice to have a parser for all those actions. But it’s hard work to write one for 60 or so actions. I wrote my first macro ever (be very afraid) to define DSL for parsers:

  defparser(:get_user,
    fields: [
      get_user_result: [
        ~x"//GetUserResult",
        user: [
          ~x"./User",
          :path,
          :user_name,
          :arn,
          :user_id,
          :create_date
        ]
      ],
      response_metadata: [
        ~x"//ResponseMetadata",
        :request_id
      ]
    ]
  )

The macro itself, below, is still not optimal. I would rather do away with passing the XML paths (~x"//GetUserResult") and handle that internally in the macro, but I have no way passing the type.

defmodule ExAws.Iam.TestMacro do
  import SweetXml, only: [sigil_x: 2]

  defmacro defparser(action, opts) do
    action_name = to_camel(action)

    fields =
      opts
      |> Keyword.get(:fields)
      |> Enum.map(fn field ->
        compile(field)
      end)

    quote do
      def parse(xml, unquote(action_name)) do
        SweetXml.xpath(xml, ~x"//#{unquote(xml_path(action_name))}", [
          {
            unquote(xml_node(action_name)),
            [~x"//#{unquote(xml_path(action_name))}" | unquote(fields)]
          }
        ])
      end
    end
  end

  defp xml_path(action), do: action <> "Response"
  defp xml_node(action), do: xml_path(action) |> to_snake()

  defp compile(field) when is_atom(field) do
    quote do
      {unquote(field), ~x"./#{unquote(to_camel(field))}/text()"s}
    end
  end

  defp compile({:sigil_x, _, _} = field), do: field

  defp compile({key, value}) do
    quote do
      {unquote(key), unquote(compile(value))}
    end
  end

  defp compile(list) when is_list(list) do
    Enum.map(list, fn field ->
      compile(field)
    end)
  end

  defp to_camel(atom), do: atom |> Atom.to_string() |> Macro.camelize()
  defp to_snake(string), do: string |> Macro.underscore() |> String.to_atom()
end

Here’s the code on a separate branch.

Thoughts?

2 Likes