From 7bcbed3ac482224b226cba6c3129bef71e0c03b4 Mon Sep 17 00:00:00 2001 From: Shane Logsdon Date: Tue, 23 Dec 2014 01:19:42 -0500 Subject: [PATCH 1/3] updated plug. refactor router/controller. fixes #54. first part of a major refactor in order to allow sugar to track plug's latest versions easier. leveraging the placid way of handling routers and controllers (handlers) so that they are more plug-oriented and can use plugs in a more idiomatic way. --- lib/sugar/controller.ex | 370 ++++++------------- lib/sugar/controller/helpers.ex | 292 +++++++++++++++ lib/sugar/router.ex | 472 ++++++++++++++----------- lib/sugar/router/util.ex | 91 +++++ mix.exs | 2 +- mix.lock | 4 +- test/sugar/controller/helpers_test.exs | 221 ++++++++++++ test/sugar/controller/hooks_test.exs | 132 +++---- test/sugar/controller_test.exs | 229 +++--------- test/sugar/router/filters_test.exs | 38 +- test/sugar/router/util_test.exs | 47 +++ test/sugar/router_test.exs | 166 ++++++++- 12 files changed, 1327 insertions(+), 737 deletions(-) create mode 100644 lib/sugar/controller/helpers.ex create mode 100644 lib/sugar/router/util.ex create mode 100644 test/sugar/controller/helpers_test.exs create mode 100644 test/sugar/router/util_test.exs diff --git a/lib/sugar/controller.ex b/lib/sugar/controller.ex index 3aa6780..7737d8c 100644 --- a/lib/sugar/controller.ex +++ b/lib/sugar/controller.ex @@ -1,302 +1,160 @@ defmodule Sugar.Controller do - import Plug.Conn @moduledoc """ - All controller actions should have an arrity of 2, with the - first argument being a `Plug.Conn` representing the current - connection and the second argument being a `Keyword` list - of any parameters captured in the route path. + Controllers facilitate some separation of concerns for your application's logic. - Sugar bundles these response helpers to assist in sending a - response: + All handler actions should have an arrity of 2, with the first argument being + a `Plug.Conn` representing the current connection and the second argument + being a `Keyword` list of any parameters captured in the route path. - * `render/4` - `conn`, `template_key`, `assigns`, `opts` - sends a normal - response. - * `halt!/2` - `conn`, `opts` - ends the response. - * `not_found/1` - `conn`, `message` - sends a 404 (Not found) response. - * `json/2` - `conn`, `data` - sends a normal response with - `data` encoded as JSON. - * `raw/1` - `conn` - sends response as-is. It is expected - that status codes, headers, body, etc have been set by - the controller action. - * `static/2` - `conn`, `file` - reads and renders a single static file. + `Sugar.Controller` imports `Plug.Conn`, the `plug/1` and `plug/2` macros from + `Plug.Builder`, `Sugar.Controller`, and `Sugar.Controller.Helpers` for + convenience when creating handlers for your applications - #### Example + ## Example - defmodule Hello do + defmodule Controllers.Pages do use Sugar.Controller + @doc false def index(conn, []) do - render conn, "showing index controller" + # Somehow get our content + pages = Queries.Page.all + render conn, pages end + @doc false def show(conn, args) do - render conn, "showing page \#{args[:id]}" + result = case Integer.parse args["page_id"] do + :error -> + %Error{ id: "no_page_id", + message: "A valid page_id is required." } + {i, _} -> + Queries.Page.get i + end + + render conn, result end - def create(conn, []) do - render conn, "page created" + @doc false + def create(conn, args) do + render conn, Queries.Page.create args, status: :created end - def get_json(conn, []) do - json conn, [message: "foobar"] + @doc false + def update(conn, args) do + result = case Integer.parse args["page_id"] do + :error -> + %Error{ id: "no_page_id", + message: "A valid page_id is requried." } + {i, _} -> + Queries.Page.update i, args + end + + render conn, result end end """ - @doc """ - Macro used to add necessary items to a controller. - """ + @doc false defmacro __using__(_) do quote do - import unquote(__MODULE__) - import unquote(Plug.Conn) - use unquote(Sugar.Controller.Hooks) + import Plug.Conn + import Plug.Builder, only: [plug: 1, plug: 2] + import Sugar.Controller + import Sugar.Controller.Helpers + @before_compile Sugar.Controller + @behaviour Plug + Module.register_attribute(__MODULE__, :plugs, accumulate: true) end end - @doc """ - sets connection status - - ## Arguments - - * `conn` - `Plug.Conn` - * `status_code` - `Integer` + @doc false + defmacro __before_compile__(env) do + plugs = Module.get_attribute(env.module, :plugs) + |> Enum.map(fn { plug, opts, guard } -> + { plug, Keyword.put_new(opts, :run, :before), guard } + end) - ## Returns - - `Plug.Conn` - """ - def status(conn, status_code) do - %Plug.Conn{conn | status: status_code, state: :set} - end + plug_stacks = build_plug_stacks plugs - @doc """ - sets response headers - - ## Arguments - - * `conn` - `Plug.Conn` - * `status_code` - `Integer` - - ## Returns - - `Plug.Conn` - """ - def headers(conn, headers) do - %Plug.Conn{conn | resp_headers: headers, state: :set} - end - - @doc """ - reads and renders a single static file. - - ## Arguments + quote do + def init(opts) do + opts + end - * `conn` - `Plug.Conn` - * `file` - `String` + def call(conn, opts) do + conn = do_call conn, :before, opts[:action] + conn = apply __MODULE__, opts[:action], [ conn, opts[:args] ] + do_call conn, :after, opts[:action] + end - ## Returns + defoverridable [init: 1, call: 2] - `Plug.Conn` - """ - def static(conn, file) do - filename = Path.join(["priv/static", file]) - if File.exists? filename do - body = filename |> File.read! - conn - |> put_resp_content_type_if_not_sent("text/html") - |> send_resp_if_not_sent(200, body) - else - conn - |> not_found + unquote(plug_stacks) end end - @doc """ - Sends a normal response with `data` encoded as JSON. - - ## Arguments - - * `conn` - `Plug.Conn` - * `data` - `Keyword|List` - - ## Returns + defp build_plug_stacks(plugs) do + only_actions = get_only_actions plugs - `Plug.Conn` - """ - def json(conn, data, opts) do - opts = [status: 200] |> Keyword.merge opts - conn - |> put_resp_content_type_if_not_sent("application/json") - |> send_resp_if_not_sent(opts[:status], JSEX.encode! data) - end - def json(conn, data) do - status = conn.status || 200 - if get_resp_header(conn, "content-type") == [] do - conn = put_resp_content_type_if_not_sent(conn, "application/json") + Enum.map only_actions ++ [nil], fn action -> + build_plug_stacks_for action, plugs end - conn |> send_resp_if_not_sent(status, JSEX.encode! data) - end - - @doc """ - Sends response as-is. It is expected that status codes, - headers, body, etc have been set by the controller - action. - - ## Arguments - - * `conn` - `Plug.Conn` - - ## Returns - - `Plug.Conn` - """ - def raw(conn) do - conn |> send_resp - end - - @doc """ - Sends a normal response. - - Automatically renders a template based on the - current controller and action names when no - template is passed. - - ## Arguments - - * `conn` - `Plug.Conn` - * `template_key` - `String` - * `assigns` - `Keyword` - * `opts` - `Keyword` - - ## Returns - - `Plug.Conn` - """ - def render(conn, template \\ nil, assigns \\ [], opts \\ []) - def render(conn, template, assigns, opts) when is_atom(template) - or is_binary(template) do - template = build_template_key(conn, template) - render_view(conn, template, assigns, opts) - end - def render(conn, assigns, opts, _) when is_list(assigns) do - template = build_template_key(conn) - render_view(conn, template, assigns, opts) - end - - defp render_view(conn, template_key, assigns, opts) do - opts = [status: 200] |> Keyword.merge opts - - html = Sugar.Config.get(:sugar, :views_dir, "lib/#{Mix.Project.config[:app]}/views") - |> Sugar.Views.Finder.one(template_key) - |> Sugar.Templates.render(assigns) - - conn - |> put_resp_content_type_if_not_sent(opts[:content_type] || "text/html") - |> send_resp_if_not_sent(opts[:status], html) end - @doc """ - Ends the response. - - ## Arguments - - * `conn` - `Plug.Conn` - * `opts` - `Keyword` - - ## Returns - - `Plug.Conn` - """ - def halt!(conn, opts \\ []) do - opts = [status: 401, message: ""] |> Keyword.merge opts - conn - |> send_resp_if_not_sent(opts[:status], opts[:message]) - end - - @doc """ - Sends a 404 (Not found) response. - - ## Arguments - - * `conn` - `Plug.Conn` + defp build_plug_stacks_for(action, plugs) do + before_body = build_calls_for(:before, action, plugs) + after_body = build_calls_for(:after, action, plugs) - ## Returns - - `Plug.Conn` - """ - def not_found(conn, message \\ "Not Found") do - conn - |> send_resp_if_not_sent(404, message) - end - - @doc """ - Forwards the response to another controller action. - - ## Arguments - - * `conn` - `Plug.Conn` - * `controller` - `Atom` - * `action` - `Atom` - * `args` - `Keyword` - - ## Returns - - `Plug.Conn` - """ - def forward(conn, controller, action, args \\ []) do - apply controller, action, [conn, args] - end - - @doc """ - Redirects the response. - - ## Arguments - - * `conn` - `Plug.Conn` - * `location` - `String` - * `opts` - `Keyword` - - ## Returns - - `Plug.Conn` - """ - def redirect(conn, location, opts \\ []) do - opts = [status: 302] |> Keyword.merge opts - conn - |> put_resp_header_if_not_sent("Location", location) - |> send_resp_if_not_sent(opts[:status], "") + quote do + unquote(before_body) + unquote(after_body) + end end - defp build_template_key(conn, template \\ nil) do - default = Map.get(conn.private, :action) || :index - template = template || default - - controller = "#{Map.get(conn.private, :controller, "")}" - |> String.split(".") - |> List.last - |> String.downcase - - "#{controller}/#{template}" - end + defp build_calls_for(before_or_after, nil, plugs) do + { conn, body } = plugs + |> Enum.filter(fn { _, opts, _ } -> + opts[:only] === nil + end) + |> Enum.filter(fn { _, opts, _ } -> + opts[:run] === before_or_after + end) + |> Plug.Builder.compile - defp put_resp_header_if_not_sent(%Plug.Conn{state: :sent} = conn, _, _) do - conn - end - defp put_resp_header_if_not_sent(conn, key, value) do - conn |> put_resp_header(key, value) + quote do + defp do_call(unquote(conn), unquote(before_or_after), _) do + unquote(body) + end + end end + defp build_calls_for(before_or_after, action, plugs) do + { conn, body } = plugs + |> Enum.filter(fn { _, opts, _ } -> + opts[:only] === nil || + action === opts[:only] || + action in opts[:only] + end) + |> Enum.filter(fn { _, opts, _ } -> + opts[:run] === before_or_after + end) + |> Plug.Builder.compile - defp put_resp_content_type_if_not_sent(%Plug.Conn{state: :sent} = conn, _) do - conn - end - defp put_resp_content_type_if_not_sent(conn, resp_content_type) do - conn |> put_resp_content_type(resp_content_type) + quote do + defp do_call(unquote(conn), unquote(before_or_after), unquote(action)) do + unquote(body) + end + end end - defp send_resp_if_not_sent(%Plug.Conn{state: :sent} = conn, _, _) do - conn - end - defp send_resp_if_not_sent(conn, status, body) do - conn |> send_resp(status, body) + defp get_only_actions(plugs) do + plugs + |> Enum.filter(fn { _, opts, _ } -> + opts[:only] != nil + end) + |> Enum.flat_map(fn { _, opts, _ } -> + opts[:only] + end) + |> Enum.uniq end -end +end \ No newline at end of file diff --git a/lib/sugar/controller/helpers.ex b/lib/sugar/controller/helpers.ex new file mode 100644 index 0000000..c0b6dd0 --- /dev/null +++ b/lib/sugar/controller/helpers.ex @@ -0,0 +1,292 @@ +defmodule Sugar.Controller.Helpers do + @moduledoc """ + All controller actions should have an arrity of 2, with the + first argument being a `Plug.Conn` representing the current + connection and the second argument being a `Keyword` list + of any parameters captured in the route path. + + Sugar bundles these response helpers to assist in sending a + response: + + * `render/4` - `conn`, `template_key`, `assigns`, `opts` - sends a normal + response. + * `halt!/2` - `conn`, `opts` - ends the response. + * `not_found/1` - `conn`, `message` - sends a 404 (Not found) response. + * `json/2` - `conn`, `data` - sends a normal response with + `data` encoded as JSON. + * `raw/1` - `conn` - sends response as-is. It is expected + that status codes, headers, body, etc have been set by + the controller action. + * `static/2` - `conn`, `file` - reads and renders a single static file. + + #### Example + + defmodule Hello do + use Sugar.Controller + + def index(conn, []) do + render conn, "showing index controller" + end + + def show(conn, args) do + render conn, "showing page \#{args[:id]}" + end + + def create(conn, []) do + render conn, "page created" + end + + def get_json(conn, []) do + json conn, [message: "foobar"] + end + end + """ + + import Plug.Conn + + @doc """ + sets connection status + + ## Arguments + + * `conn` - `Plug.Conn` + * `status_code` - `Integer` + + ## Returns + + `Plug.Conn` + """ + def status(conn, status_code) do + %Plug.Conn{conn | status: status_code, state: :set} + end + + @doc """ + sets response headers + + ## Arguments + + * `conn` - `Plug.Conn` + * `status_code` - `Integer` + + ## Returns + + `Plug.Conn` + """ + def headers(conn, headers) do + %Plug.Conn{conn | resp_headers: headers, state: :set} + end + + @doc """ + reads and renders a single static file. + + ## Arguments + + * `conn` - `Plug.Conn` + * `file` - `String` + + ## Returns + + `Plug.Conn` + """ + def static(conn, file) do + filename = Path.join(["priv/static", file]) + if File.exists? filename do + body = filename |> File.read! + conn + |> put_resp_content_type_if_not_sent("text/html") + |> send_resp_if_not_sent(200, body) + else + conn + |> not_found + end + end + + @doc """ + Sends a normal response with `data` encoded as JSON. + + ## Arguments + + * `conn` - `Plug.Conn` + * `data` - `Keyword|List` + + ## Returns + + `Plug.Conn` + """ + def json(conn, data, opts) do + opts = [status: 200] |> Keyword.merge opts + conn + |> put_resp_content_type_if_not_sent("application/json") + |> send_resp_if_not_sent(opts[:status], JSEX.encode! data) + end + def json(conn, data) do + status = conn.status || 200 + if get_resp_header(conn, "content-type") == [] do + conn = put_resp_content_type_if_not_sent(conn, "application/json") + end + conn |> send_resp_if_not_sent(status, JSEX.encode! data) + end + + @doc """ + Sends response as-is. It is expected that status codes, + headers, body, etc have been set by the controller + action. + + ## Arguments + + * `conn` - `Plug.Conn` + + ## Returns + + `Plug.Conn` + """ + def raw(conn) do + conn |> send_resp + end + + @doc """ + Sends a normal response. + + Automatically renders a template based on the + current controller and action names when no + template is passed. + + ## Arguments + + * `conn` - `Plug.Conn` + * `template_key` - `String` + * `assigns` - `Keyword` + * `opts` - `Keyword` + + ## Returns + + `Plug.Conn` + """ + def render(conn, template \\ nil, assigns \\ [], opts \\ []) + def render(conn, template, assigns, opts) when is_atom(template) + or is_binary(template) do + template = build_template_key(conn, template) + render_view(conn, template, assigns, opts) + end + def render(conn, assigns, opts, _) when is_list(assigns) do + template = build_template_key(conn) + render_view(conn, template, assigns, opts) + end + + defp render_view(conn, template_key, assigns, opts) do + opts = [status: 200] |> Keyword.merge opts + + html = Sugar.Config.get(:sugar, :views_dir, "lib/#{Mix.Project.config[:app]}/views") + |> Sugar.Views.Finder.one(template_key) + |> Sugar.Templates.render(assigns) + + conn + |> put_resp_content_type_if_not_sent(opts[:content_type] || "text/html") + |> send_resp_if_not_sent(opts[:status], html) + end + + @doc """ + Ends the response. + + ## Arguments + + * `conn` - `Plug.Conn` + * `opts` - `Keyword` + + ## Returns + + `Plug.Conn` + """ + def halt!(conn, opts \\ []) do + opts = [status: 401, message: ""] |> Keyword.merge opts + conn + |> send_resp_if_not_sent(opts[:status], opts[:message]) + end + + @doc """ + Sends a 404 (Not found) response. + + ## Arguments + + * `conn` - `Plug.Conn` + + ## Returns + + `Plug.Conn` + """ + def not_found(conn, message \\ "Not Found") do + conn + |> send_resp_if_not_sent(404, message) + end + + @doc """ + Forwards the response to another controller action. + + ## Arguments + + * `conn` - `Plug.Conn` + * `controller` - `Atom` + * `action` - `Atom` + * `args` - `Keyword` + + ## Returns + + `Plug.Conn` + """ + def forward(conn, controller, action, args \\ []) do + apply controller, action, [conn, args] + end + + @doc """ + Redirects the response. + + ## Arguments + + * `conn` - `Plug.Conn` + * `location` - `String` + * `opts` - `Keyword` + + ## Returns + + `Plug.Conn` + """ + def redirect(conn, location, opts \\ []) do + opts = [status: 302] |> Keyword.merge opts + conn + |> put_resp_header_if_not_sent("Location", location) + |> send_resp_if_not_sent(opts[:status], "") + end + + defp build_template_key(conn, template \\ nil) do + default = Map.get(conn.private, :action) || :index + template = template || default + + controller = "#{Map.get(conn.private, :controller, "")}" + |> String.split(".") + |> List.last + |> String.downcase + + "#{controller}/#{template}" + end + + defp put_resp_header_if_not_sent(%Plug.Conn{state: :sent} = conn, _, _) do + conn + end + defp put_resp_header_if_not_sent(conn, key, value) do + conn |> put_resp_header(key, value) + end + + defp put_resp_content_type_if_not_sent(%Plug.Conn{state: :sent} = conn, _) do + conn + end + defp put_resp_content_type_if_not_sent(conn, resp_content_type) do + conn |> put_resp_content_type(resp_content_type) + end + + defp send_resp_if_not_sent(%Plug.Conn{state: :sent} = conn, _, _) do + conn + end + defp send_resp_if_not_sent(conn, status, body) do + conn |> send_resp(status, body) + end +end \ No newline at end of file diff --git a/lib/sugar/router.ex b/lib/sugar/router.ex index 00b3444..d36c6a1 100644 --- a/lib/sugar/router.ex +++ b/lib/sugar/router.ex @@ -5,287 +5,347 @@ defmodule Sugar.Router do Routes are defined with the form: - method route [guard], controller, action + method route [guard], handler, action - `method` is `get`, `post`, `put`, `patch`, `delete`, or `options`, - each responsible for a single HTTP method. `method` can also be - `any`, which will match on all HTTP methods. `controller` is any - valid Elixir module name, and `action` is any valid function - defined in the `controller` module. + `method` is `get`, `post`, `put`, `patch`, or `delete`, each + responsible for a single HTTP method. `method` can also be `any`, which will + match on all HTTP methods. `options` is yet another option for `method`, but + when using `options`, only a route path and the methods that route path + supports are needed. `handler` is any valid Elixir module name, and + `action` is any valid public function defined in the `handler` module. - ## Example - - defmodule Router do - use Sugar.Router, plugs: [ - { Plugs.HotCodeReload, [] }, - { Plug.Static, at: "/static", from: :my_app }, + `get/3`, `post/3`, `put/3`, `patch/3`, `delete/3`, `options/2`, and `any/3` + are already built-in as described. `resource/2` exists but will need + modifications to create everything as noted. - # Uncomment the following line for session store - # { Plug.Session, store: :ets, key: "sid", secure: true, table: :session }, + `raw/4` allows for using custom HTTP methods, allowing your application to be + HTTP spec compliant. - # Uncomment the following line for request logging, - # and add 'applications: [:exlager],' to the application - # Keyword list in your mix.exs - # { Plugs.Logger, [] } - ] + ## Example - before_filter Filters, :set_headers + defmodule Router do + use Sugar.Router + alias Controllers, as: C # Define your routes here - get "/", Hello, :index - get "/pages/:id", Hello, :show - post "/pages", Hello, :create - put "/pages/:id" when id == 1, Hello, :show + get "/", C.Pages, :index + get "/pages", C.Pages, :create + post "/pages", C.Pages, :create + put "/pages/:page_id" when id == 1, + C.Pages, :update_only_one + get "/pages/:page_id", C.Pages, :show + + # Auto-create a full set of routes for resources + # + resource :users, C.User, arg: :user_id + # + # Generates: + # + # get "/users", C.User, :index + # post "/users", C.User, :create + # get "/users/:user_id", C.User, :show + # put "/users/:user_id", C.User, :update + # patch "/users/:user_id", C.User, :patch + # delete "/users/:user_id", C.User, :delete + # + # options "/users", "HEAD,GET,POST" + # options "/users/:_user_id", "HEAD,GET,PUT,PATCH,DELETE" + + raw :trace, "/trace", C.Tracer, :trace end """ + import Sugar.Router.Util + + @typep ast :: tuple + @http_methods [ :get, :post, :put, :patch, :delete, :any ] + ## Macros - @doc """ - Macro used to add necessary items to a router. - """ - defmacro __using__(opts) do + @doc false + defmacro __using__(_) do quote do - import unquote(__MODULE__) - import Plug.Conn - use Plug.Router - use Sugar.Router.Filters - @before_compile unquote(__MODULE__) - - #plug Sugar.Plugs.Exceptions, dev_template: Sugar.Exceptions.dev_template - plug Plug.Parsers, parsers: [Sugar.Plugs.Parsers.JSON, :urlencoded, :multipart] - - opts = unquote(opts) - if opts[:plugs] do - Enum.map opts[:plugs], fn({plug_module, plug_opts}) -> - plug plug_module, plug_opts - end - end - - plug :match - plug :dispatch + import Sugar.Controller.Helpers, only: [not_found: 1] + import Sugar.Router + import Plug.Builder, only: [plug: 1, plug: 2] + @before_compile Sugar.Router + @behaviour Plug + Module.register_attribute(__MODULE__, :plugs, accumulate: true) + Module.register_attribute(__MODULE__, :version, accumulate: false) + + # Plugs we want early in the stack + plug Plug.Parsers, parsers: [ :json, + Sugar.Request.Parsers.XML, + :urlencoded, + :multipart ], + json_decoder: JSEX end end - @doc """ - Defines a default route to catch all unmatched routes. - """ + @doc false defmacro __before_compile__(env) do - module = env.module - # From Sugar.Router.Filters - filters = Module.get_attribute(module, :filters) + # Plugs we want predefined but aren't necessary to be before + # user-defined plugs + defaults = [ { Plug.Head, [], true }, + { Plug.MethodOverride, [], true }, + { :copy_req_content_type, [], true }, + { :match, [], true }, + { :dispatch, [], true } ] + { conn, body } = Enum.reverse(defaults) ++ + Module.get_attribute(env.module, :plugs) + |> Plug.Builder.compile quote do - # Our default match so Plug doesn't fall on its face - # when accessing an undefined route - Plug.Router.match _ do - conn = var!(conn) - Sugar.Controller.not_found conn + def init(opts) do + opts end - defp call_controller_action(%Plug.Conn{state: :unset} = conn, controller, action, binding) do - conn = call_before_filters(unquote(filters), action, conn) - conn = apply controller, :call_action, [action, conn, Keyword.delete(binding, :conn)] - call_after_filters(unquote(filters), action, conn) + def call(conn, opts) do + do_call(conn, opts) end - defp call_controller_action(conn, _, _, _) do - conn + + defoverridable [init: 1, call: 2] + + def copy_req_content_type(conn, _opts) do + default = Application.get_env(:sugar, :default_content_type, "application/json; charset=utf-8") + content_type = case Plug.Conn.get_req_header conn, "content-type" do + [content_type] -> content_type + _ -> default + end + conn |> Plug.Conn.put_resp_header("content-type", content_type) end - end - end - @doc """ - Macro for defining `GET` routes. + def run(opts \\ nil) do + adapter = Sugar.Config.get(:sugar, :plug_adapter, Plug.Adapters.Cowboy) + opts = opts || Sugar.Config.get(__MODULE__) - ## Arguments + adapter.https __MODULE__, [], opts[:https] + if opts[:https_only] do + # Sends `403 Forbidden` to all HTTP requests + adapter.http Sugar.Request.HttpsOnly, [], opts[:http] + else + adapter.http __MODULE__, [], opts[:http] + end + end - * `route` - `String|List` - * `controller` - `Atom` - * `action` - `Atom` - """ - defmacro get(route, controller, action) do - quote do - get unquote(route), do: unquote( - build_match controller, action - ) - end - end + def match(conn, _opts) do + plug_route = __MODULE__.do_match(conn.method, conn.path_info) + Plug.Conn.put_private(conn, :plug_route, plug_route) + end - @doc """ - Macro for defining `POST` routes. + def dispatch(%Plug.Conn{ assigns: assigns } = conn, _opts) do + Map.get(conn.private, :plug_route).(conn) + end - ## Arguments + # Our default match so `Plug` doesn't fall on + # its face when accessing an undefined route. + def do_match(_,_) do + fn conn -> + not_found conn + end + end - * `route` - `String|List` - * `controller` - `Atom` - * `action` - `Atom` - """ - defmacro post(route, controller, action) do - quote do - post unquote(route), do: unquote( - build_match controller, action - ) + defp do_call(unquote(conn), _), do: unquote(body) end end - @doc """ - Macro for defining `PUT` routes. + for verb <- @http_methods do + @doc """ + Macro for defining `#{verb |> to_string |> String.upcase}` routes. - ## Arguments + ## Arguments - * `route` - `String|List` - * `controller` - `Atom` - * `action` - `Atom` - """ - defmacro put(route, controller, action) do - quote do - put unquote(route), do: unquote( - build_match controller, action - ) + * `route` - `String|List` + * `handler` - `Atom` + * `action` - `Atom` + """ + @spec unquote(verb)(binary | list, atom, atom) :: ast + defmacro unquote(verb)(route, handler, action) do + build_match unquote(verb), route, handler, action, __CALLER__ end end @doc """ - Macro for defining `PATCH` routes. + Macro for defining `OPTIONS` routes. ## Arguments * `route` - `String|List` - * `controller` - `Atom` - * `action` - `Atom` + * `allows` - `String` """ - defmacro patch(route, controller, action) do - quote do - patch unquote(route), do: unquote( - build_match controller, action - ) - end + @spec options(binary | list, binary) :: ast + defmacro options(route, allows) do + build_match :options, route, allows, __CALLER__ end @doc """ - Macro for defining `DELETE` routes. + Macro for defining routes for custom HTTP methods. ## Arguments + * `method` - `Atom` * `route` - `String|List` - * `controller` - `Atom` + * `handler` - `Atom` * `action` - `Atom` """ - defmacro delete(route, controller, action) do - quote do - delete unquote(route), do: unquote( - build_match controller, action - ) - end + @spec raw(atom, binary | list, atom, atom) :: ast + defmacro raw(method, route, handler, action) do + build_match method, route, handler, action, __CALLER__ end @doc """ - Macro for defining `OPTIONS` routes. + Creates RESTful resource endpoints for a route/handler + combination. - ## Arguments + ## Example - * `route` - `String|List` - * `controller` - `Atom` - * `action` - `Atom` - """ - defmacro options(route, controller, action) do - quote do - options unquote(route), do: unquote( - build_match controller, action - ) - end - end + resource :users, Handlers.User - @doc """ - Macro for defining routes that match on all HTTP methods. + expands to - ## Arguments + get, "/users", Handlers.User, :index + post, "/users", Handlers.User, :create + get, "/users/:id", Handlers.User, :show + put, "/users/:id", Handlers.User, :update + patch, "/users/:id", Handlers.User, :patch + delete, "/users/:id", Handlers.User, :delete - * `route` - `String|List` - * `controller` - `Atom` - * `action` - `Atom` + options, "/users", "HEAD,GET,POST" + options, "/users/:_id", "HEAD,GET,PUT,PATCH,DELETE" """ - defmacro any(route, controller, action) do - quote do - match unquote(route), do: unquote( - build_match controller, action - ) + @spec resource(atom, atom, Keyword.t) :: [ast] + defmacro resource(resource, handler, opts \\ []) do + arg = Keyword.get opts, :arg, :id + allowed = Keyword.get opts, :only, [ :index, :create, :show, + :update, :patch, :delete ] + + prepend_path = Keyword.get opts, :prepend_path, nil + if prepend_path, do: prepend_path = "/" <> prepend_path <> "/" + + routes = + [ { :get, "#{prepend_path}#{resource}", :index }, + { :post, "#{prepend_path}#{resource}", :create }, + { :get, "#{prepend_path}#{resource}/:#{arg}", :show }, + { :put, "#{prepend_path}#{resource}/:#{arg}", :update }, + { :patch, "#{prepend_path}#{resource}/:#{arg}", :patch }, + { :delete, "#{prepend_path}#{resource}/:#{arg}", :delete } ] + + options_routes = + [ { "/#{ignore_args prepend_path}#{resource}", [ index: :get, create: :post ] }, + { "/#{ignore_args prepend_path}#{resource}/:_#{arg}", [ show: :get, update: :put, + patch: :patch, delete: :delete ] } ] + + for { method, path, action } <- routes |> filter(allowed) do + build_match method, path, handler, action, __CALLER__ + end ++ for { path, methods } <- options_routes do + allows = methods + |> filter(allowed) + |> Enum.map(fn { _, m } -> + normalize_method(m) + end) + |> Enum.join(",") + build_match :options, path, "HEAD,#{allows}", __CALLER__ end end - @doc """ - Creates RESTful resource endpoints for a route/controller - combination. + defp ignore_args(nil), do: "" + defp ignore_args(str) do + str + |> String.to_char_list + |> do_ignore_args + |> to_string + end - ## Example + defp do_ignore_args([]), do: [] + defp do_ignore_args([?:|t]), do: [?:,?_] ++ do_ignore_args(t) + defp do_ignore_args([h|t]), do: [h] ++ do_ignore_args(t) + + # Builds a `do_match/2` function body for a given route. + defp build_match(:options, route, allows, caller) do + body = quote do + conn + |> Plug.Conn.resp(200, "") + |> Plug.Conn.put_resp_header("Allow", unquote(allows)) + |> Plug.Conn.send_resp + end - resource "/path", Controller + do_build_match :options, route, body, caller + end + defp build_match(method, route, handler, action, caller) do + body = build_body handler, action + # body_json = build_body handler, action, :json + # body_xml = build_body handler, action, :xml + + [ #do_build_match(method, route <> ".json", body_json, caller), + #do_build_match(method, route <> ".xml", body_xml, caller), + do_build_match(method, route, body, caller) ] + end - expands to + defp do_build_match(verb, route, body, caller) do + { method, guards, _vars, match } = prep_match verb, route, caller + method = if verb == :any, do: quote(do: _), else: method - get "/path", Controller, :index - get "/path" <> "/new", Controller, :new - post "/path", Controller, :create - get "/path" <> "/:id", Controller, :show - get "/path" <> "/:id/edit", Controller, :edit - put "/path" <> "/:id", Controller, :update - patch "/path" <> "/:id", Controller, :patch - delete "/path" <> "/:id", Controller, :delete - - options "/path" do - {:ok, var!(conn) |> resp(200, "") |> put_resp_header("Allow", "HEAD,GET,POST") |> send_resp} - end - options "/path" <> "/new" do - {:ok, var!(conn) |> resp(200, "") |> put_resp_header("Allow", "HEAD,GET") |> send_resp} - end - options "/path" <> "/:_id" do - {:ok, var!(conn) |> resp(200, "") |> put_resp_header("Allow", "HEAD,GET,PUT,PATCH,DELETE") |> send_resp} - end - options "/path" <> "/:_id/edit" do - {:ok, var!(conn) |> resp(200, "") |> put_resp_header("Allow", "HEAD,GET") |> send_resp} - end - """ - defmacro resource(route, controller) do quote do - get unquote(route), unquote(controller), :index - get unquote(route) <> "/new", unquote(controller), :new - post unquote(route), unquote(controller), :create - get unquote(route) <> "/:id", unquote(controller), :show - get unquote(route) <> "/:id/edit", unquote(controller), :edit - put unquote(route) <> "/:id", unquote(controller), :update - patch unquote(route) <> "/:id", unquote(controller), :patch - delete unquote(route) <> "/:id", unquote(controller), :delete - - options unquote(route) do - conn = var!(conn) - conn |> resp(200, "") |> put_resp_header("Allow", "HEAD,GET,POST") |> send_resp - end - options unquote(route <> "/new") do - conn = var!(conn) - conn |> resp(200, "") |> put_resp_header("Allow", "HEAD,GET") |> send_resp - end - options unquote(route <> "/:_id") do - conn = var!(conn) - conn |> resp(200, "") |> put_resp_header("Allow", "HEAD,GET,PUT,PATCH,DELETE") |> send_resp - end - options unquote(route <> "/:_id/edit") do - conn = var!(conn) - conn |> resp(200, "") |> put_resp_header("Allow", "HEAD,GET") |> send_resp + def do_match(unquote(method), unquote(match)) when unquote(guards) do + fn conn -> + unquote(body) + end end end end - defp build_match(controller, action) do + defp build_body(handler, action), do: build_body(handler, action, :skip) + defp build_body(handler, action, add_header) do + header = case add_header do + :json -> [{"accept", "application/json"}] + :xml -> [{"accept", "application/xml"}] + _ -> [] + end + quote do - binding = binding() - conn = var!(conn) + opts = [ action: unquote(action), args: binding() ] + unquote(handler).call %{ conn | req_headers: unquote(header) ++ + conn.req_headers }, unquote(handler).init(opts) + end + end - conn = %{ conn | private: conn.private - |> Map.put(:controller, unquote(controller)) - |> Map.put(:action, unquote(action)) } + defp filter(list, allowed) do + Enum.filter list, &do_filter(&1, allowed) + end - # pass off to controller action - call_controller_action conn, unquote(controller), unquote(action), binding - end + defp do_filter({ _, _, action }, allowed) do + action in allowed + end + defp do_filter({ action, _ }, allowed) do + action in allowed + end + + ## Grabbed from `Plug.Router` + + defp prep_match(method, route, caller) do + { method, guard } = convert_methods(List.wrap(method)) + { path, guards } = extract_path_and_guards(route, guard) + { vars, match } = build_spec(Macro.expand(path, caller)) + { method, guards, vars, match } + end + + # Convert the verbs given with :via into a variable + # and guard set that can be added to the dispatch clause. + defp convert_methods([]) do + { quote(do: _), true } + end + defp convert_methods([method]) do + { normalize_method(method), true } + end + + # Extract the path and guards from the path. + defp extract_path_and_guards({ :when, _, [ path, guards ] }, true) do + { path, guards } + end + defp extract_path_and_guards({ :when, _, [ path, guards ] }, extra_guard) do + { path, { :and, [], [ guards, extra_guard ] } } + end + defp extract_path_and_guards(path, extra_guard) do + { path, extra_guard } end end diff --git a/lib/sugar/router/util.ex b/lib/sugar/router/util.ex new file mode 100644 index 0000000..a513def --- /dev/null +++ b/lib/sugar/router/util.ex @@ -0,0 +1,91 @@ +defmodule Sugar.Router.Util do + @moduledoc false + + defmodule InvalidSpecError do + defexception message: "invalid route specification" + end + + def normalize_method(method) do + method |> to_string |> String.upcase + end + + def split(bin) do + for segment <- String.split(bin, "/"), segment != "", do: segment + end + + def build_spec(spec, context \\ nil) + def build_spec(spec, context) when is_binary(spec) do + build_spec split(spec), context, [], [] + end + def build_spec(spec, _context) do + {[], spec} + end + defp build_spec([h|t], context, vars, acc) do + handle_segment_match segment_match(h, "", context), t, context, vars, acc + end + defp build_spec([], _context, vars, acc) do + {vars |> Enum.uniq |> Enum.reverse, Enum.reverse(acc)} + end + + defp handle_segment_match({:literal, literal}, t, context, vars, acc) do + build_spec t, context, vars, [literal|acc] + end + defp handle_segment_match({:identifier, identifier, expr}, t, context, vars, acc) do + build_spec t, context, [identifier|vars], [expr|acc] + end + defp handle_segment_match({:glob, identifier, expr}, t, context, vars, acc) do + if t != [] do + raise InvalidSpecError, message: "cannot have a *glob followed by other segments" + end + + case acc do + [hs|ts] -> + acc = [{:|, [], [hs, expr]} | ts] + build_spec([], context, [identifier|vars], acc) + _ -> + {vars, expr} = build_spec([], context, [identifier|vars], [expr]) + {vars, hd(expr)} + end + end + + defp segment_match(":" <> argument, buffer, context) do + identifier = binary_to_identifier(":", argument) + expr = quote_if_buffer identifier, buffer, context, fn var -> + quote do: unquote(buffer) <> unquote(var) + end + {:identifier, identifier, expr} + end + defp segment_match("*" <> argument, buffer, context) do + underscore = {:_, [], context} + identifier = binary_to_identifier("*", argument) + expr = quote_if_buffer identifier, buffer, context, fn var -> + quote do: [unquote(buffer) <> unquote(underscore)|unquote(underscore)] = unquote(var) + end + {:glob, identifier, expr} + end + defp segment_match(<>, buffer, context) do + segment_match t, buffer <> <>, context + end + defp segment_match(<<>>, buffer, _context) do + {:literal, buffer} + end + + defp quote_if_buffer(identifier, "", context, _fun) do + {identifier, [], context} + end + defp quote_if_buffer(identifier, _buffer, context, fun) do + fun.({identifier, [], context}) + end + + defp binary_to_identifier(prefix, <> = binary) when letter in ?a..?z + or letter == ?_ do + if binary =~ ~r/^\w+$/ do + String.to_atom(binary) + else + raise InvalidSpecError, message: "#{prefix}identifier in routes must be made of letters, numbers and underscore" + end + end + defp binary_to_identifier(prefix, _) do + raise InvalidSpecError, message: "#{prefix} in routes must be followed by lowercase letters" + end +end \ No newline at end of file diff --git a/mix.exs b/mix.exs index 4b40429..45bf51c 100644 --- a/mix.exs +++ b/mix.exs @@ -20,7 +20,7 @@ defmodule Sugar.Mixfile do defp deps(:prod) do [ { :cowboy, "~> 1.0.0" }, - { :plug, "~> 0.7.0", override: true }, + { :plug, "~> 0.9.0" }, { :jsex, "~> 2.0.0" }, { :ecto, "~> 0.2.5" }, { :postgrex, "~> 0.6.0" }, diff --git a/mix.lock b/mix.lock index 3d7afa1..c7a0a5b 100644 --- a/mix.lock +++ b/mix.lock @@ -11,8 +11,8 @@ "jsex": {:hex, :jsex, "2.0.0"}, "jsx": {:hex, :jsx, "2.0.4"}, "merl": {:git, "git://github.com/erlydtl/merl.git", "28e5b3829168199e8475fa91b997e0c03b90d280", [ref: "28e5b3829168199e8475fa91b997e0c03b90d280"]}, - "plug": {:hex, :plug, "0.7.0"}, - "plugs": {:git, "git://github.com/sugar-framework/plugs.git", "45a522109a881cd942a361e1e78aed7da699a64b", []}, + "plug": {:hex, :plug, "0.9.0"}, + "plugs": {:git, "git://github.com/sugar-framework/plugs.git", "4586b401de63fe97fb6ef201a6a9b357e46b41ca", []}, "poolboy": {:hex, :poolboy, "1.2.1"}, "postgrex": {:hex, :postgrex, "0.6.0"}, "ranch": {:hex, :ranch, "1.0.0"}, diff --git a/test/sugar/controller/helpers_test.exs b/test/sugar/controller/helpers_test.exs new file mode 100644 index 0000000..c8dd73e --- /dev/null +++ b/test/sugar/controller/helpers_test.exs @@ -0,0 +1,221 @@ +defmodule Sugar.Controller.HelpersTest do + use ExUnit.Case, async: false + use Plug.Test + + import Sugar.Controller.Helpers + import Plug.Conn + + test "static/2 file exists" do + conn = conn(:get, "/") + |> Map.put(:state, :set) + |> static("index.html") + + assert conn.state === :sent + assert get_resp_header(conn, "content-type") === ["text/html; charset=utf-8"] + assert conn.status === 200 + refute conn.resp_body === "" + end + + test "static/2 file no exists" do + conn = conn(:get, "/") + |> Map.put(:state, :set) + |> static("nofile.html") + + assert conn.state === :sent + assert conn.status === 404 + assert conn.resp_body === "Not Found" + end + + test "json/2" do + conn = conn(:get, "/") + |> Map.put(:state, :set) + |> json([]) + + assert conn.status === 200 + assert conn.state === :sent + assert get_resp_header(conn, "content-type") === ["application/json; charset=utf-8"] + assert conn.resp_body === "[]" + end + + test "json/3" do + conn = conn(:get, "/") + |> Map.put(:state, :set) + |> json([], []) + + assert conn.status === 200 + assert conn.state === :sent + assert get_resp_header(conn, "content-type") === ["application/json; charset=utf-8"] + assert conn.resp_body === "[]" + end + + test "status/2 and json/2" do + conn = conn(:get, "/") + |> Map.put(:state, :set) + |> status(201) + |> json([]) + + assert conn.status === 201 + assert conn.state === :sent + assert get_resp_header(conn, "content-type") === ["application/json; charset=utf-8"] + assert conn.resp_body === "[]" + end + + test "headers/2 and json/2" do + conn = conn(:get, "/") + |> Map.put(:state, :set) + |> headers([{"content-type", "application/vnd.company.myapp.customer-v1+json"}]) + |> json([]) + + assert conn.state === :sent + assert get_resp_header(conn, "content-type") === ["application/vnd.company.myapp.customer-v1+json"] + assert conn.resp_body === "[]" + end + + test "raw/1" do + conn = conn(:get, "/") + |> Map.put(:state, :set) + |> Map.put(:resp_body, "") + |> raw + + assert conn.state === :sent + end + + test "render/4 without template, assigns, or opts" do + conn = conn(:get, "/") + |> Map.put(:state, :set) + |> render + + assert conn.state === :sent + assert get_resp_header(conn, "content-type") === ["text/html; charset=utf-8"] + end + + test "render/4 without template but with assigns" do + conn = conn(:get, "/") + |> Map.put(:state, :set) + |> render([]) + + assert conn.state === :sent + assert get_resp_header(conn, "content-type") === ["text/html; charset=utf-8"] + end + + test "render/4 without assigns and opts" do + conn = conn(:get, "/") + |> Map.put(:state, :set) + |> render("foo/index.html.eex") + + assert conn.state === :sent + assert get_resp_header(conn, "content-type") === ["text/html; charset=utf-8"] + end + + test "render/4 with assigns and without opts" do + conn = conn(:get, "/") + |> Map.put(:state, :set) + |> render("foo/index.html.eex", []) + + assert conn.state === :sent + assert get_resp_header(conn, "content-type") === ["text/html; charset=utf-8"] + end + + test "render/4 with assigns and opts" do + conn = conn(:get, "/") + |> Map.put(:state, :set) + |> render("foo/index.html.eex", [], [content_type: "text/html"]) + + assert conn.state === :sent + assert get_resp_header(conn, "content-type") === ["text/html; charset=utf-8"] + end + + test "render/4 with a symbol" do + conn = conn(:get, "/") + |> Map.put(:state, :set) + |> render("foo/index.html.eex", [], [content_type: "text/html"]) + + assert conn.state === :sent + assert get_resp_header(conn, "content-type") === ["text/html; charset=utf-8"] + end + + test "halt!/2 without opts" do + conn = conn(:get, "/") + |> Map.put(:state, :set) + |> halt! + + assert conn.state === :sent + assert conn.status === 401 + end + + test "halt!/2 with opts" do + conn = conn(:get, "/") + |> Map.put(:state, :set) + |> halt!([status: 401, message: "Halted"]) + + assert conn.state === :sent + assert conn.status === 401 + assert conn.resp_body === "Halted" + end + + test "not_found/2" do + conn = conn(:get, "/") + |> Map.put(:state, :set) + |> not_found + + assert conn.state === :sent + assert conn.status === 404 + assert conn.resp_body === "Not Found" + end + + test "forward/4 without args" do + conn = conn(:get, "/") + |> Map.put(:state, :set) + |> forward(Sugar.Controller.HelpersTest.Controller, :create) + + assert conn.state === :sent + end + + test "forward/4 with args" do + conn = conn(:get, "/") + |> Map.put(:state, :set) + |> forward(Sugar.Controller.HelpersTest.Controller, :create, []) + + assert conn.state === :sent + end + + test "redirect/2 without opts" do + conn = conn(:get, "/") + |> Map.put(:state, :set) + |> redirect("/login") + + assert conn.state === :sent + assert conn.status === 302 + end + + test "redirect/2 with opts" do + conn = conn(:get, "/") + |> Map.put(:state, :set) + |> redirect("/login", [status: 301]) + + assert conn.state === :sent + assert conn.status === 301 + end + + test "resp already sent on set content-type" do + conn = conn(:get, "/") + |> Map.put(:state, :sent) + |> json([]) + + assert conn.state === :sent + end + + test "resp already sent on set header" do + conn = conn(:get, "/") + |> Map.put(:state, :sent) + |> redirect("/login") + + assert conn.state === :sent + end + + defmodule Controller do + def create(conn, _args) do + halt! conn, [status: 204, message: "Created"] + end + end +end diff --git a/test/sugar/controller/hooks_test.exs b/test/sugar/controller/hooks_test.exs index 12ec90c..326f53b 100644 --- a/test/sugar/controller/hooks_test.exs +++ b/test/sugar/controller/hooks_test.exs @@ -1,90 +1,90 @@ -defmodule Sugar.Controller.HooksTest do - use ExUnit.Case, async: true - use Plug.Test +# defmodule Sugar.Controller.HooksTest do +# use ExUnit.Case, async: true +# use Plug.Test - test "before_hook/1 all" do - conn = conn(:get, "/") - |> Sugar.Controller.HooksTest.Router.call([]) +# test "before_hook/1 all" do +# conn = conn(:get, "/") +# |> Sugar.Controller.HooksTest.Router.call([]) - assert get_resp_header(conn, "content-type") === ["application/json; charset=utf-8"] - end +# assert get_resp_header(conn, "content-type") === ["application/json; charset=utf-8"] +# end - test "before_hook/2 all + only show" do - conn = conn(:get, "/show") - |> Sugar.Controller.HooksTest.Router.call([]) +# test "before_hook/2 all + only show" do +# conn = conn(:get, "/show") +# |> Sugar.Controller.HooksTest.Router.call([]) - assert get_resp_header(conn, "content-type") === ["application/json; charset=utf-8"] - assert conn.assigns[:id] === 1 +# assert get_resp_header(conn, "content-type") === ["application/json; charset=utf-8"] +# assert conn.assigns[:id] === 1 - conn = conn(:get, "/") - |> Sugar.Controller.HooksTest.Router.call([]) +# conn = conn(:get, "/") +# |> Sugar.Controller.HooksTest.Router.call([]) - assert get_resp_header(conn, "content-type") === ["application/json; charset=utf-8"] - refute conn.assigns[:id] === 1 - end +# assert get_resp_header(conn, "content-type") === ["application/json; charset=utf-8"] +# refute conn.assigns[:id] === 1 +# end - test "after_hook/1 all" do - conn = conn(:get, "/") - |> Sugar.Controller.HooksTest.Router.call([]) +# test "after_hook/1 all" do +# conn = conn(:get, "/") +# |> Sugar.Controller.HooksTest.Router.call([]) - assert conn.private[:id] === 2 - end +# assert conn.private[:id] === 2 +# end - test "after_hook/2 all + only show" do - conn = conn(:get, "/show") - |> Sugar.Controller.HooksTest.Router.call([]) +# test "after_hook/2 all + only show" do +# conn = conn(:get, "/show") +# |> Sugar.Controller.HooksTest.Router.call([]) - assert conn.state === :sent - assert conn.private[:id] === 2 +# assert conn.state === :sent +# assert conn.private[:id] === 2 - conn = conn(:get, "/") - |> Sugar.Controller.HooksTest.Router.call([]) +# conn = conn(:get, "/") +# |> Sugar.Controller.HooksTest.Router.call([]) - refute conn.state === :sent - assert conn.private[:id] === 2 - end +# refute conn.state === :sent +# assert conn.private[:id] === 2 +# end - defmodule Router do - use Sugar.Router - alias Sugar.Controller.HooksTest.Controller +# defmodule Router do +# use Sugar.Router +# alias Sugar.Controller.HooksTest.Controller - get "/", Controller, :index - get "/show", Controller, :show - end +# get "/", Controller, :index +# get "/show", Controller, :show +# end - defmodule Controller do - use Sugar.Controller +# defmodule Controller do +# use Sugar.Controller - before_hook :set_json - before_hook :set_assign, only: [:show] +# before_hook :set_json +# before_hook :set_assign, only: [:show] - after_hook :send, only: [:show] - after_hook :set_private +# after_hook :send, only: [:show] +# after_hook :set_private - def index(conn, _args) do - conn |> resp(200, "[]") - end +# def index(conn, _args) do +# conn |> resp(200, "[]") +# end - def show(conn, _args) do - conn |> resp(200, "[]") - end +# def show(conn, _args) do +# conn |> resp(200, "[]") +# end - ## Hooks +# ## Hooks - def set_json(conn) do - conn |> put_resp_header("content-type", "application/json; charset=utf-8") - end +# def set_json(conn) do +# conn |> put_resp_header("content-type", "application/json; charset=utf-8") +# end - def set_assign(conn) do - conn |> assign(:id, 1) - end +# def set_assign(conn) do +# conn |> assign(:id, 1) +# end - def send(conn) do - conn |> send_resp - end +# def send(conn) do +# conn |> send_resp +# end - def set_private(conn) do - conn |> assign_private(:id, 2) - end - end -end +# def set_private(conn) do +# conn |> assign_private(:id, 2) +# end +# end +# end diff --git a/test/sugar/controller_test.exs b/test/sugar/controller_test.exs index 9e25dd4..fbb4320 100644 --- a/test/sugar/controller_test.exs +++ b/test/sugar/controller_test.exs @@ -1,221 +1,90 @@ defmodule Sugar.ControllerTest do - use ExUnit.Case, async: false + use ExUnit.Case, async: true use Plug.Test - import Sugar.Controller - import Plug.Conn - - test "static/2 file exists" do - conn = conn(:get, "/") - |> Map.put(:state, :set) - |> static("index.html") - - assert conn.state === :sent - assert get_resp_header(conn, "content-type") === ["text/html; charset=utf-8"] - assert conn.status === 200 - refute conn.resp_body === "" - end - - test "static/2 file no exists" do - conn = conn(:get, "/") - |> Map.put(:state, :set) - |> static("nofile.html") - - assert conn.state === :sent - assert conn.status === 404 - assert conn.resp_body === "Not Found" - end - - test "json/2" do + test "before_hook/1 all" do conn = conn(:get, "/") - |> Map.put(:state, :set) - |> json([]) + |> Sugar.ControllerTest.Router.call([]) - assert conn.status === 200 - assert conn.state === :sent assert get_resp_header(conn, "content-type") === ["application/json; charset=utf-8"] - assert conn.resp_body === "[]" end - test "json/3" do - conn = conn(:get, "/") - |> Map.put(:state, :set) - |> json([], []) + test "before_hook/2 only show" do + conn = conn(:get, "/show") + |> Sugar.ControllerTest.Router.call([]) - assert conn.status === 200 - assert conn.state === :sent assert get_resp_header(conn, "content-type") === ["application/json; charset=utf-8"] - assert conn.resp_body === "[]" - end + assert conn.assigns[:id] === 1 - test "status/2 and json/2" do conn = conn(:get, "/") - |> Map.put(:state, :set) - |> status(201) - |> json([]) + |> Sugar.ControllerTest.Router.call([]) - assert conn.status === 201 - assert conn.state === :sent assert get_resp_header(conn, "content-type") === ["application/json; charset=utf-8"] - assert conn.resp_body === "[]" + refute conn.assigns[:id] === 1 end - test "headers/2 and json/2" do + test "after_hook/1 all" do conn = conn(:get, "/") - |> Map.put(:state, :set) - |> headers([{"content-type", "application/vnd.company.myapp.customer-v1+json"}]) - |> json([]) + |> Sugar.ControllerTest.Router.call([]) - assert conn.state === :sent - assert get_resp_header(conn, "content-type") === ["application/vnd.company.myapp.customer-v1+json"] - assert conn.resp_body === "[]" + assert conn.private[:id] === 2 end - test "raw/1" do - conn = conn(:get, "/") - |> Map.put(:state, :set) - |> Map.put(:resp_body, "") - |> raw + test "after_hook/2 only show" do + conn = conn(:get, "/show") + |> Sugar.ControllerTest.Router.call([]) assert conn.state === :sent - end + assert conn.private[:id] === 2 - test "render/4 without template, assigns, or opts" do conn = conn(:get, "/") - |> Map.put(:state, :set) - |> render + |> Sugar.ControllerTest.Router.call([]) - assert conn.state === :sent - assert get_resp_header(conn, "content-type") === ["text/html; charset=utf-8"] + refute conn.state === :sent + assert conn.private[:id] === 2 end - test "render/4 without template but with assigns" do - conn = conn(:get, "/") - |> Map.put(:state, :set) - |> render([]) + defmodule Router do + use Sugar.Router + alias Sugar.ControllerTest.Controller - assert conn.state === :sent - assert get_resp_header(conn, "content-type") === ["text/html; charset=utf-8"] + get "/", Controller, :index + get "/show", Controller, :show end - test "render/4 without assigns and opts" do - conn = conn(:get, "/") - |> Map.put(:state, :set) - |> render("foo/index.html.eex") - - assert conn.state === :sent - assert get_resp_header(conn, "content-type") === ["text/html; charset=utf-8"] - end - - test "render/4 with assigns and without opts" do - conn = conn(:get, "/") - |> Map.put(:state, :set) - |> render("foo/index.html.eex", []) - - assert conn.state === :sent - assert get_resp_header(conn, "content-type") === ["text/html; charset=utf-8"] - end - - test "render/4 with assigns and opts" do - conn = conn(:get, "/") - |> Map.put(:state, :set) - |> render("foo/index.html.eex", [], [content_type: "text/html"]) - - assert conn.state === :sent - assert get_resp_header(conn, "content-type") === ["text/html; charset=utf-8"] - end - - test "render/4 with a symbol" do - conn = conn(:get, "/") - |> Map.put(:state, :set) - |> render("foo/index.html.eex", [], [content_type: "text/html"]) - - assert conn.state === :sent - assert get_resp_header(conn, "content-type") === ["text/html; charset=utf-8"] - end - - test "halt!/2 without opts" do - conn = conn(:get, "/") - |> Map.put(:state, :set) - |> halt! - - assert conn.state === :sent - assert conn.status === 401 - end - - test "halt!/2 with opts" do - conn = conn(:get, "/") - |> Map.put(:state, :set) - |> halt!([status: 401, message: "Halted"]) - - assert conn.state === :sent - assert conn.status === 401 - assert conn.resp_body === "Halted" - end - - test "not_found/2" do - conn = conn(:get, "/") - |> Map.put(:state, :set) - |> not_found - - assert conn.state === :sent - assert conn.status === 404 - assert conn.resp_body === "Not Found" - end - - test "forward/4 without args" do - conn = conn(:get, "/") - |> Map.put(:state, :set) - |> forward(Sugar.ControllerTest.Controller, :create) - - assert conn.state === :sent - end - - test "forward/4 with args" do - conn = conn(:get, "/") - |> Map.put(:state, :set) - |> forward(Sugar.ControllerTest.Controller, :create, []) - - assert conn.state === :sent - end + defmodule Controller do + use Sugar.Controller - test "redirect/2 without opts" do - conn = conn(:get, "/") - |> Map.put(:state, :set) - |> redirect("/login") + plug :set_json, run: :before + plug :set_assign, run: :before, only: [:show] - assert conn.state === :sent - assert conn.status === 302 - end + plug :send_response, run: :after, only: [:show] + plug :set_private, run: :after - test "redirect/2 with opts" do - conn = conn(:get, "/") - |> Map.put(:state, :set) - |> redirect("/login", [status: 301]) + def index(conn, _args) do + conn |> resp(200, "[]") + end - assert conn.state === :sent - assert conn.status === 301 - end + def show(conn, _args) do + conn |> resp(200, "[]") + end - test "resp already sent on set content-type" do - conn = conn(:get, "/") - |> Map.put(:state, :sent) - |> json([]) + ## Hooks - assert conn.state === :sent - end + def set_json(conn, _) do + conn |> put_resp_header("content-type", "application/json; charset=utf-8") + end - test "resp already sent on set header" do - conn = conn(:get, "/") - |> Map.put(:state, :sent) - |> redirect("/login") + def set_assign(conn, _) do + conn |> assign(:id, 1) + end - assert conn.state === :sent - end + def send_response(conn, _) do + conn |> send_resp + end - defmodule Controller do - def create(conn, _args) do - halt! conn, [status: 204, message: "Created"] + def set_private(conn, _) do + conn |> put_private(:id, 2) end end -end +end \ No newline at end of file diff --git a/test/sugar/router/filters_test.exs b/test/sugar/router/filters_test.exs index 25f54ed..5678e06 100644 --- a/test/sugar/router/filters_test.exs +++ b/test/sugar/router/filters_test.exs @@ -2,31 +2,31 @@ defmodule Sugar.Router.FiltersTest do use ExUnit.Case, async: true use Plug.Test - test "before_filter/1 all" do - conn = conn(:get, "/") - |> Sugar.Router.FiltersTest.Router.call([]) + # test "before_filter/1 all" do + # conn = conn(:get, "/") + # |> Sugar.Router.FiltersTest.Router.call([]) - assert get_resp_header(conn, "content-type") === ["application/json; charset=utf-8"] - end + # assert get_resp_header(conn, "content-type") === ["application/json; charset=utf-8"] + # end - test "after_filter/1 all" do - conn = conn(:get, "/") - |> Sugar.Router.FiltersTest.Router.call([]) + # test "after_filter/1 all" do + # conn = conn(:get, "/") + # |> Sugar.Router.FiltersTest.Router.call([]) - refute conn.assigns[:id] === 1 - end + # refute conn.assigns[:id] === 1 + # end - defmodule Router do - use Sugar.Router - alias Sugar.Router.FiltersTest.Controller - alias Sugar.Router.FiltersTest.Filters + # defmodule Router do + # use Sugar.Router + # alias Sugar.Router.FiltersTest.Controller + # alias Sugar.Router.FiltersTest.Filters - before_filter Filters, :set_json - after_filter Filters, :clear_assigns + # before_filter Filters, :set_json + # after_filter Filters, :clear_assigns - get "/", Controller, :index - get "/show", Controller, :show - end + # get "/", Controller, :index + # get "/show", Controller, :show + # end defmodule Controller do use Sugar.Controller diff --git a/test/sugar/router/util_test.exs b/test/sugar/router/util_test.exs new file mode 100644 index 0000000..3df4dbd --- /dev/null +++ b/test/sugar/router/util_test.exs @@ -0,0 +1,47 @@ +defmodule Sugar.Router.UtilTest do + use ExUnit.Case, async: true + import Sugar.Router.Util + + test "split/1" do + assert split("bin/sh") === ["bin", "sh"] + assert split("bin/:sh") === ["bin", ":sh"] + assert split("bin/*sh") === ["bin", "*sh"] + assert split("/bin/sh") === ["bin", "sh"] + assert split("//bin/sh/") === ["bin", "sh"] + end + + test "build_spec/2 proper" do + assert build_spec("bin/:sh", nil) === {[:sh], ["bin", {:sh, [], nil}]} + assert build_spec("bin/*sh", nil) === {[:sh], [{:|, [], ["bin", {:sh, [], nil}]}]} + assert build_spec("*sh", nil) === {[:sh], {:sh, [], nil}} + end + + test "build_spec/2 illegal chars" do + message = "identifier in routes must be made of letters, numbers and underscore" + assert_raise Sugar.Router.Util.InvalidSpecError, ":" <> message, fn -> + build_spec("bin/:sh-", nil) + end + + assert_raise Sugar.Router.Util.InvalidSpecError, "*" <> message, fn -> + build_spec("bin/*sh-", nil) + end + end + + test "build_spec/2 bad name" do + message = "in routes must be followed by lowercase letters" + assert_raise Sugar.Router.Util.InvalidSpecError, ": " <> message, fn -> + build_spec("bin/:-sh", nil) + end + + assert_raise Sugar.Router.Util.InvalidSpecError, "* " <> message, fn -> + build_spec("bin/*-sh", nil) + end + end + + test "build_spec/2 glob with segments after capture" do + message = "cannot have a *glob followed by other segments" + assert_raise Sugar.Router.Util.InvalidSpecError, message, fn -> + build_spec("bin/*sh/json", nil) + end + end +end \ No newline at end of file diff --git a/test/sugar/router_test.exs b/test/sugar/router_test.exs index 60a61d6..c520993 100644 --- a/test/sugar/router_test.exs +++ b/test/sugar/router_test.exs @@ -67,10 +67,83 @@ defmodule Sugar.RouterTest do assert conn.status === 200 end + test "raw/4 trace" do + conn = conn(:trace, "/trace") + |> Sugar.RouterTest.Router.call([]) + + assert conn.state === :sent + assert conn.status === 200 + end + test "resource/2" do assert true === true end + test "resource/2 index" do + conn = conn(:get, "/users") + |> Sugar.RouterTest.Router.call([]) + + assert conn.state === :sent + assert conn.status === 200 + end + + test "resource/2 create" do + conn = conn(:post, "/users") + |> Sugar.RouterTest.Router.call([]) + + assert conn.state === :sent + assert conn.status === 200 + end + + test "resource/2 show" do + conn = conn(:get, "/users/1") + |> Sugar.RouterTest.Router.call([]) + + assert conn.state === :sent + assert conn.status === 200 + end + + test "resource/2 update" do + conn = conn(:put, "/users/1") + |> Sugar.RouterTest.Router.call([]) + + assert conn.state === :sent + assert conn.status === 200 + end + + test "resource/2 patch" do + conn = conn(:patch, "/users/1") + |> Sugar.RouterTest.Router.call([]) + + assert conn.state === :sent + assert conn.status === 200 + end + + test "resource/2 delete" do + conn = conn(:delete, "/users/1") + |> Sugar.RouterTest.Router.call([]) + + assert conn.state === :sent + assert conn.status === 200 + end + + test "filter plug is run" do + conn = conn(:get, "/get") + |> Sugar.RouterTest.Router.call([]) + + assert conn.state === :sent + assert conn.status === 200 + assert get_resp_header(conn, "content-type") === ["application/json; charset=utf-8"] + end + + test "resource/2 index with prepended path" do + conn = conn(:get, "/users/2/comments") + |> Sugar.RouterTest.Router.call([]) + + assert conn.state === :sent + assert conn.status === 200 + end + test "parses json encoded bodies" do headers = [{"content-type", "application/json"}] conn = conn(:post, "/post", "{\"foo\": \"baz\"}", headers: headers) @@ -132,17 +205,96 @@ defmodule Sugar.RouterTest do |> Map.put(:state, :set) |> raw end + def trace(conn, _args) do + conn + |> Map.put(:resp_body, "") + |> Map.put(:status, 200) + |> Map.put(:state, :set) + |> raw + end + end + + defmodule Bar do + use Sugar.Controller + + def index(conn, _args) do + conn + |> Map.put(:resp_body, "") + |> Map.put(:status, 200) + |> Map.put(:state, :set) + |> raw + end + def create(conn, _args) do + conn + |> Map.put(:resp_body, "") + |> Map.put(:status, 200) + |> Map.put(:state, :set) + |> raw + end + def show(conn, _args) do + conn + |> Map.put(:resp_body, "") + |> Map.put(:status, 200) + |> Map.put(:state, :set) + |> raw + end + def update(conn, _args) do + conn + |> Map.put(:resp_body, "") + |> Map.put(:status, 200) + |> Map.put(:state, :set) + |> raw + end + def patch(conn, _args) do + conn + |> Map.put(:resp_body, "") + |> Map.put(:status, 200) + |> Map.put(:state, :set) + |> raw + end + def delete(conn, _args) do + conn + |> Map.put(:resp_body, "") + |> Map.put(:status, 200) + |> Map.put(:state, :set) + |> raw + end + end + + defmodule Baz do + use Sugar.Controller + + def index(conn, _args) do + conn + |> Map.put(:resp_body, "") + |> Map.put(:status, 200) + |> Map.put(:state, :set) + |> raw + end end defmodule Router do use Sugar.Router - get "/get", Foo, :get - post "/post", Foo, :post - put "/put", Foo, :put - patch "/patch", Foo, :patch - delete "/delete", Foo, :delete - options "/options", Foo, :options - any "/any", Foo, :any + plug :set_utf8_json + + get "/get", Foo, :get + get "/get/:id", Foo, :get + post "/post", Foo, :post + put "/put", Foo, :put + patch "/patch", Foo, :patch + delete "/delete", Foo, :delete + options "/options", "HEAD,GET" + any "/any", Foo, :any + raw :trace, "/trace", Foo, :trace + + resource :users, Bar + resource :comments, Baz, prepend_path: "/users/:user_id", + only: [:index] + + def set_utf8_json(%Plug.Conn{state: state} = conn, _) when state in [:unset, :set] do + conn |> put_resp_header("content-type", "application/json; charset=utf-8") + end + def set_utf8_json(conn, _), do: conn end end From 29d76da1df5efe3972c07adeb8ed91915b430894 Mon Sep 17 00:00:00 2001 From: Shane Logsdon Date: Tue, 23 Dec 2014 23:45:17 -0500 Subject: [PATCH 2/3] round two. strip out hooks and filters in favor of plugs. plug stack in controllers allows for before/after functionality, but router stack doesn't. not sure that it needs it. add functionality/tests for matched controller/action in conn.private. improved test coverage. --- config/config.exs | 4 +- lib/sugar/controller/hooks.ex | 133 --------------------------- lib/sugar/router.ex | 49 +++++----- lib/sugar/router/filters.ex | 90 ------------------ lib/sugar/views/finder.ex | 3 +- test/sugar/config_test.exs | 5 + test/sugar/controller/hooks_test.exs | 90 ------------------ test/sugar/router/filters_test.exs | 54 ----------- test/sugar/router/util_test.exs | 1 + test/sugar/router_test.exs | 8 ++ 10 files changed, 45 insertions(+), 392 deletions(-) delete mode 100644 lib/sugar/controller/hooks.ex delete mode 100644 lib/sugar/router/filters.ex delete mode 100644 test/sugar/controller/hooks_test.exs delete mode 100644 test/sugar/router/filters_test.exs diff --git a/config/config.exs b/config/config.exs index b1c5377..86175e4 100644 --- a/config/config.exs +++ b/config/config.exs @@ -1,3 +1,5 @@ use Mix.Config -config :sugar, views_dir: "test/fixtures/templates" \ No newline at end of file +config :sugar, + config_test: [true], + views_dir: "test/fixtures/templates" diff --git a/lib/sugar/controller/hooks.ex b/lib/sugar/controller/hooks.ex deleted file mode 100644 index b1e23d4..0000000 --- a/lib/sugar/controller/hooks.ex +++ /dev/null @@ -1,133 +0,0 @@ -defmodule Sugar.Controller.Hooks do - @moduledoc """ - Allows for before and after hooks to a controller. Each hook has the - opportunity to modify and/or read the request's `conn` in each stage. - - #### Example - - defmodule MyController do - use Sugar.Controller - - before_hook :set_headers - after_hook :send - - def index(conn, _args) do - conn |> resp(200, "[]") - end - - ## Hooks - - def set_headers(conn) do - conn |> put_resp_header("content-type", "application/json; charset=utf-8") - end - - def send(conn) do - conn |> send_resp - end - end - """ - - @doc """ - Macro used to add necessary items to a controller. - """ - defmacro __using__(_opts) do - quote do - import unquote(__MODULE__) - @hooks [] - @all_hooks_key :__all_hooks__ - @before_compile unquote(__MODULE__) - end - end - - defmacro __before_compile__(env) do - module = env.module - hooks = Module.get_attribute(module, :hooks) - - quote do - def call_action(action, conn, args) do - conn = call_before_hooks(unquote(hooks), unquote(module), action, conn) - conn = apply(unquote(module), action, [conn, args]) - call_after_hooks(unquote(hooks), unquote(module), action, conn) - end - - defp call_before_hooks(hooks, module, action, conn) do - call_hooks(:before, hooks, module, action, conn) - end - - defp call_after_hooks(hooks, module, action, conn) do - call_hooks(:after, hooks, module, action, conn) - end - - defp call_hooks(type, hooks, module, action, conn) do - hooks - |> Stream.filter(fn ({t, _}) -> t === type end) - |> Stream.filter(fn ({_, {act, _}}) -> act === action || act === :__all_hooks__ end) - |> Enum.reduce(conn, fn({_, {_, fun}}, conn) -> - apply module, fun, [conn] - end) - end - end - end - - @doc """ - Adds a before hook to a controller. - - ## Arguments - - * `function` - `atom` - name of the function to be called within the hook - * `opts` - `Keyword` - optional - used to target specific actions. Possible - options include: - * `only` - `List` - a list of atoms representing the actions on which the - hook should be applied - """ - defmacro before_hook(function) when is_atom(function) do - quote do - @hooks @hooks++[{:before, {@all_hooks_key, unquote(function)}}] - end - end - defmacro before_hook(function, opts) when is_atom(function) do - quote do - opts = unquote(opts) - function = unquote(function) - cond do - opts[:only] -> - @hooks Enum.reduce(opts[:only], @hooks, fn (method, acc) -> - acc++[{:before, {method, function}}] - end) - true -> - @hooks - end - end - end - - @doc """ - Adds an after hook to a controller. - - ## Arguments - - * `function` - `atom` - name of the function to be called within the hook - * `opts` - `Keyword` - optional - used to target specific actions. Possible - options include: - * `only` - `List` - a list of atoms representing the actions on which the - hook should be applied - """ - defmacro after_hook(function) when is_atom(function) do - quote do - @hooks @hooks++[{:after, {@all_hooks_key, unquote(function)}}] - end - end - defmacro after_hook(function, opts) when is_atom(function) do - quote do - opts = unquote(opts) - function = unquote(function) - cond do - opts[:only] -> - @hooks Enum.reduce(opts[:only], @hooks, fn (method, acc) -> - acc++[{:after, {method, function}}] - end) - true -> - @hooks - end - end - end -end diff --git a/lib/sugar/router.ex b/lib/sugar/router.ex index d36c6a1..6609c4c 100644 --- a/lib/sugar/router.ex +++ b/lib/sugar/router.ex @@ -5,14 +5,14 @@ defmodule Sugar.Router do Routes are defined with the form: - method route [guard], handler, action + method route [guard], controller, action `method` is `get`, `post`, `put`, `patch`, or `delete`, each responsible for a single HTTP method. `method` can also be `any`, which will match on all HTTP methods. `options` is yet another option for `method`, but when using `options`, only a route path and the methods that route path - supports are needed. `handler` is any valid Elixir module name, and - `action` is any valid public function defined in the `handler` module. + supports are needed. `controller` is any valid Elixir module name, and + `action` is any valid public function defined in the `controller` module. `get/3`, `post/3`, `put/3`, `patch/3`, `delete/3`, `options/2`, and `any/3` are already built-in as described. `resource/2` exists but will need @@ -156,12 +156,12 @@ defmodule Sugar.Router do ## Arguments * `route` - `String|List` - * `handler` - `Atom` + * `controller` - `Atom` * `action` - `Atom` """ @spec unquote(verb)(binary | list, atom, atom) :: ast - defmacro unquote(verb)(route, handler, action) do - build_match unquote(verb), route, handler, action, __CALLER__ + defmacro unquote(verb)(route, controller, action) do + build_match unquote(verb), route, controller, action, __CALLER__ end end @@ -185,16 +185,16 @@ defmodule Sugar.Router do * `method` - `Atom` * `route` - `String|List` - * `handler` - `Atom` + * `controller` - `Atom` * `action` - `Atom` """ @spec raw(atom, binary | list, atom, atom) :: ast - defmacro raw(method, route, handler, action) do - build_match method, route, handler, action, __CALLER__ + defmacro raw(method, route, controller, action) do + build_match method, route, controller, action, __CALLER__ end @doc """ - Creates RESTful resource endpoints for a route/handler + Creates RESTful resource endpoints for a route/controller combination. ## Example @@ -214,7 +214,7 @@ defmodule Sugar.Router do options, "/users/:_id", "HEAD,GET,PUT,PATCH,DELETE" """ @spec resource(atom, atom, Keyword.t) :: [ast] - defmacro resource(resource, handler, opts \\ []) do + defmacro resource(resource, controller, opts \\ []) do arg = Keyword.get opts, :arg, :id allowed = Keyword.get opts, :only, [ :index, :create, :show, :update, :patch, :delete ] @@ -231,12 +231,12 @@ defmodule Sugar.Router do { :delete, "#{prepend_path}#{resource}/:#{arg}", :delete } ] options_routes = - [ { "/#{ignore_args prepend_path}#{resource}", [ index: :get, create: :post ] }, - { "/#{ignore_args prepend_path}#{resource}/:_#{arg}", [ show: :get, update: :put, - patch: :patch, delete: :delete ] } ] + [ { "/" <> ignore_args(prepend_path) <> "#{resource}", [ index: :get, create: :post ] }, + { "/" <> ignore_args(prepend_path) <> "#{resource}/:_#{arg}", [ show: :get, update: :put, + patch: :patch, delete: :delete ] } ] for { method, path, action } <- routes |> filter(allowed) do - build_match method, path, handler, action, __CALLER__ + build_match method, path, controller, action, __CALLER__ end ++ for { path, methods } <- options_routes do allows = methods |> filter(allowed) @@ -271,10 +271,10 @@ defmodule Sugar.Router do do_build_match :options, route, body, caller end - defp build_match(method, route, handler, action, caller) do - body = build_body handler, action - # body_json = build_body handler, action, :json - # body_xml = build_body handler, action, :xml + defp build_match(method, route, controller, action, caller) do + body = build_body controller, action + # body_json = build_body controller, action, :json + # body_xml = build_body controller, action, :xml [ #do_build_match(method, route <> ".json", body_json, caller), #do_build_match(method, route <> ".xml", body_xml, caller), @@ -294,8 +294,8 @@ defmodule Sugar.Router do end end - defp build_body(handler, action), do: build_body(handler, action, :skip) - defp build_body(handler, action, add_header) do + defp build_body(controller, action), do: build_body(controller, action, :skip) + defp build_body(controller, action, add_header) do header = case add_header do :json -> [{"accept", "application/json"}] :xml -> [{"accept", "application/xml"}] @@ -304,8 +304,11 @@ defmodule Sugar.Router do quote do opts = [ action: unquote(action), args: binding() ] - unquote(handler).call %{ conn | req_headers: unquote(header) ++ - conn.req_headers }, unquote(handler).init(opts) + %{ conn | req_headers: unquote(header) ++ conn.req_headers, + private: conn.private + |> Map.put(:controller, unquote(controller)) + |> Map.put(:action, unquote(action)) } + |> unquote(controller).call(unquote(controller).init(opts)) end end diff --git a/lib/sugar/router/filters.ex b/lib/sugar/router/filters.ex deleted file mode 100644 index ef10486..0000000 --- a/lib/sugar/router/filters.ex +++ /dev/null @@ -1,90 +0,0 @@ -defmodule Sugar.Router.Filters do - @moduledoc """ - Allows for before and after hooks to a controller. Each hook has the - opportunity to modify and/or read the request's `conn` in each stage. - - #### Example - - defmodule Router do - use Sugar.Router - alias Filters - - before_filter Filters, :set_json - after_filter Filters, :clear_assigns - - # routes - # ... - end - - defmodule Filters do - import Plug.Conn - - def set_json(conn) do - conn |> put_resp_header("content-type", "application/json; charset=utf-8") - end - - def clear_assigns(conn) do - %{ conn | assigns: %{} } - end - end - """ - - @doc """ - Macro used to add necessary items to a router. - """ - defmacro __using__(_opts) do - quote do - import unquote(__MODULE__) - @filters [] - @all_filters_key :__all_filters__ - @before_compile unquote(Sugar.Router) - end - end - - @doc false - def call_before_filters(filters, action, conn) do - call_filters(:before, filters, action, conn) - end - - @doc false - def call_after_filters(filters, action, conn) do - call_filters(:after, filters, action, conn) - end - - defp call_filters(type, filters, action, conn) do - filters - |> Enum.filter(fn ({t, _}) -> t === type end) - |> Enum.filter(fn ({_, {act, _}}) -> act === action || act === :__all_filters__ end) - |> Enum.reduce(conn, fn({_, {_, {module, fun}}}, conn) -> - apply module, fun, [conn] - end) - end - - @doc """ - Adds a before hook to a controller. - - ## Arguments - - * `module` - `atom` - name of the module that contains the filter function - * `function` - `atom` - name of the function to be called within the hook - """ - defmacro before_filter(module, function) do - quote do - @filters @filters++[{:before, {@all_filters_key, {unquote(module), unquote(function)}}}] - end - end - - @doc """ - Adds a after hook to a controller. - - ## Arguments - - * `module` - `atom` - name of the module that contains the filter function - * `function` - `atom` - name of the function to be called within the hook - """ - defmacro after_filter(module, function) do - quote do - @filters @filters++[{:after, {@all_filters_key, {unquote(module), unquote(function)}}}] - end - end -end diff --git a/lib/sugar/views/finder.ex b/lib/sugar/views/finder.ex index 1593eb0..8564e94 100644 --- a/lib/sugar/views/finder.ex +++ b/lib/sugar/views/finder.ex @@ -16,7 +16,8 @@ defmodule Sugar.Views.Finder do List of `Sugar.Templates.Template` """ def all(root) do - Path.wildcard("#{root}/**/*.*") + root <> "/**/*.*" + |> Path.wildcard |> Enum.map(fn (path) -> build(path) end) end diff --git a/test/sugar/config_test.exs b/test/sugar/config_test.exs index 9f49328..10c181b 100644 --- a/test/sugar/config_test.exs +++ b/test/sugar/config_test.exs @@ -3,7 +3,12 @@ defmodule Sugar.ConfigTest do import Sugar.Config test "get/1" do + expected_list = [true, {:http, [port: 4000]}, + {:https, [certfile: "", + keyfile: "", + port: 4443]}] assert get(:router) === nil + assert get(:config_test) === expected_list end test "get/2" do diff --git a/test/sugar/controller/hooks_test.exs b/test/sugar/controller/hooks_test.exs deleted file mode 100644 index 326f53b..0000000 --- a/test/sugar/controller/hooks_test.exs +++ /dev/null @@ -1,90 +0,0 @@ -# defmodule Sugar.Controller.HooksTest do -# use ExUnit.Case, async: true -# use Plug.Test - -# test "before_hook/1 all" do -# conn = conn(:get, "/") -# |> Sugar.Controller.HooksTest.Router.call([]) - -# assert get_resp_header(conn, "content-type") === ["application/json; charset=utf-8"] -# end - -# test "before_hook/2 all + only show" do -# conn = conn(:get, "/show") -# |> Sugar.Controller.HooksTest.Router.call([]) - -# assert get_resp_header(conn, "content-type") === ["application/json; charset=utf-8"] -# assert conn.assigns[:id] === 1 - -# conn = conn(:get, "/") -# |> Sugar.Controller.HooksTest.Router.call([]) - -# assert get_resp_header(conn, "content-type") === ["application/json; charset=utf-8"] -# refute conn.assigns[:id] === 1 -# end - -# test "after_hook/1 all" do -# conn = conn(:get, "/") -# |> Sugar.Controller.HooksTest.Router.call([]) - -# assert conn.private[:id] === 2 -# end - -# test "after_hook/2 all + only show" do -# conn = conn(:get, "/show") -# |> Sugar.Controller.HooksTest.Router.call([]) - -# assert conn.state === :sent -# assert conn.private[:id] === 2 - -# conn = conn(:get, "/") -# |> Sugar.Controller.HooksTest.Router.call([]) - -# refute conn.state === :sent -# assert conn.private[:id] === 2 -# end - -# defmodule Router do -# use Sugar.Router -# alias Sugar.Controller.HooksTest.Controller - -# get "/", Controller, :index -# get "/show", Controller, :show -# end - -# defmodule Controller do -# use Sugar.Controller - -# before_hook :set_json -# before_hook :set_assign, only: [:show] - -# after_hook :send, only: [:show] -# after_hook :set_private - -# def index(conn, _args) do -# conn |> resp(200, "[]") -# end - -# def show(conn, _args) do -# conn |> resp(200, "[]") -# end - -# ## Hooks - -# def set_json(conn) do -# conn |> put_resp_header("content-type", "application/json; charset=utf-8") -# end - -# def set_assign(conn) do -# conn |> assign(:id, 1) -# end - -# def send(conn) do -# conn |> send_resp -# end - -# def set_private(conn) do -# conn |> assign_private(:id, 2) -# end -# end -# end diff --git a/test/sugar/router/filters_test.exs b/test/sugar/router/filters_test.exs deleted file mode 100644 index 5678e06..0000000 --- a/test/sugar/router/filters_test.exs +++ /dev/null @@ -1,54 +0,0 @@ -defmodule Sugar.Router.FiltersTest do - use ExUnit.Case, async: true - use Plug.Test - - # test "before_filter/1 all" do - # conn = conn(:get, "/") - # |> Sugar.Router.FiltersTest.Router.call([]) - - # assert get_resp_header(conn, "content-type") === ["application/json; charset=utf-8"] - # end - - # test "after_filter/1 all" do - # conn = conn(:get, "/") - # |> Sugar.Router.FiltersTest.Router.call([]) - - # refute conn.assigns[:id] === 1 - # end - - # defmodule Router do - # use Sugar.Router - # alias Sugar.Router.FiltersTest.Controller - # alias Sugar.Router.FiltersTest.Filters - - # before_filter Filters, :set_json - # after_filter Filters, :clear_assigns - - # get "/", Controller, :index - # get "/show", Controller, :show - # end - - defmodule Controller do - use Sugar.Controller - - def index(conn, _args) do - conn |> assign(:id, 1) |> resp(200, "[]") - end - - def show(conn, _args) do - conn |> resp(200, "[]") - end - end - - defmodule Filters do - import Plug.Conn - - def set_json(conn) do - conn |> put_resp_header("content-type", "application/json; charset=utf-8") - end - - def clear_assigns(conn) do - %{ conn | assigns: %{} } - end - end -end diff --git a/test/sugar/router/util_test.exs b/test/sugar/router/util_test.exs index 3df4dbd..0e7a431 100644 --- a/test/sugar/router/util_test.exs +++ b/test/sugar/router/util_test.exs @@ -11,6 +11,7 @@ defmodule Sugar.Router.UtilTest do end test "build_spec/2 proper" do + assert build_spec("bin/sh", nil) === {[], ["bin", "sh"]} assert build_spec("bin/:sh", nil) === {[:sh], ["bin", {:sh, [], nil}]} assert build_spec("bin/*sh", nil) === {[:sh], [{:|, [], ["bin", {:sh, [], nil}]}]} assert build_spec("*sh", nil) === {[:sh], {:sh, [], nil}} diff --git a/test/sugar/router_test.exs b/test/sugar/router_test.exs index c520993..5aa038a 100644 --- a/test/sugar/router_test.exs +++ b/test/sugar/router_test.exs @@ -153,6 +153,14 @@ defmodule Sugar.RouterTest do assert conn.params["foo"] == "baz" end + test "adds matched controller and action to conn" do + conn = conn(:get, "/get") + |> Sugar.RouterTest.Router.call([]) + + assert conn.private[:controller] === Sugar.RouterTest.Foo + assert conn.private[:action] === :get + end + defmodule Foo do use Sugar.Controller From 4d57f29f79803247ad5f16f97b4d275a88265989 Mon Sep 17 00:00:00 2001 From: Shane Logsdon Date: Wed, 24 Dec 2014 00:24:21 -0500 Subject: [PATCH 3/3] round three fix copy_req_content_type router plug. add HttpsOnly and XML parser --- lib/sugar/request/https_only.ex | 10 ++++ lib/sugar/request/parsers/xml.ex | 65 ++++++++++++++++++++++++++ lib/sugar/router.ex | 8 ++-- test/sugar/request/https_only_test.exs | 13 ++++++ test/sugar/request/parsers_test.exs | 56 ++++++++++++++++++++++ 5 files changed, 148 insertions(+), 4 deletions(-) create mode 100644 lib/sugar/request/https_only.ex create mode 100644 lib/sugar/request/parsers/xml.ex create mode 100644 test/sugar/request/https_only_test.exs create mode 100644 test/sugar/request/parsers_test.exs diff --git a/lib/sugar/request/https_only.ex b/lib/sugar/request/https_only.ex new file mode 100644 index 0000000..52d473c --- /dev/null +++ b/lib/sugar/request/https_only.ex @@ -0,0 +1,10 @@ +defmodule Sugar.Request.HttpsOnly do + @moduledoc false + import Plug.Conn + + def init(opts), do: opts + + def call(conn, _opts) do + conn |> send_resp(403, "Forbidden") + end +end \ No newline at end of file diff --git a/lib/sugar/request/parsers/xml.ex b/lib/sugar/request/parsers/xml.ex new file mode 100644 index 0000000..462d796 --- /dev/null +++ b/lib/sugar/request/parsers/xml.ex @@ -0,0 +1,65 @@ +defmodule Sugar.Request.Parsers.XML do + @moduledoc false + alias Plug.Conn + + @types [ "application", "text" ] + + @type conn :: map + @type headers :: map + @type opts :: Keyword.t + + @spec parse(conn, binary, binary, headers, opts) :: {:ok | :error, map | atom, conn} + def parse(%Conn{} = conn, type, "xml", _headers, opts) when type in @types do + case Conn.read_body(conn, opts) do + { :ok, body, conn } -> + { :ok, %{ xml: body |> do_parse }, conn } + { :more, _data, conn } -> + { :error, :too_large, conn } + end + end + def parse(conn, _type, _subtype, _headers, _opts) do + { :next, conn } + end + + defp do_parse(xml) do + :erlang.bitstring_to_list(xml) + |> :xmerl_scan.string + |> elem(0) + |> do_parse_nodes + end + + defp do_parse_nodes([]), do: [] + defp do_parse_nodes([ h | t ]) do + do_parse_nodes(h) ++ do_parse_nodes(t) + end + defp do_parse_nodes({ :xmlAttribute, name, _, _, _, _, _, _, value, _ }) do + [ { name, value |> to_string } ] + end + defp do_parse_nodes({ :xmlElement, name, _, _, _, _, _, attrs, els, _, _, _ }) do + value = els + |> do_parse_nodes + |> flatten + [ %{ name: name, + attr: attrs |> do_parse_nodes, + value: value } ] + end + defp do_parse_nodes({ :xmlText, _, _, _, value, _ }) do + string_value = value + |> to_string + |> String.strip + if string_value |> String.length > 0 do + [ string_value ] + else + [] + end + end + + defp flatten([]), do: [] + defp flatten(values) do + if Enum.all?(values, &(is_binary(&1))) do + [ values |> List.to_string ] + else + values + end + end +end \ No newline at end of file diff --git a/lib/sugar/router.ex b/lib/sugar/router.ex index 6609c4c..645103e 100644 --- a/lib/sugar/router.ex +++ b/lib/sugar/router.ex @@ -79,6 +79,7 @@ defmodule Sugar.Router do :urlencoded, :multipart ], json_decoder: JSEX + plug :copy_req_content_type end end @@ -87,11 +88,10 @@ defmodule Sugar.Router do # Plugs we want predefined but aren't necessary to be before # user-defined plugs defaults = [ { Plug.Head, [], true }, - { Plug.MethodOverride, [], true }, - { :copy_req_content_type, [], true }, + { Plug.MethodOverride, [], true }, { :match, [], true }, { :dispatch, [], true } ] - { conn, body } = Enum.reverse(defaults) ++ + { conn, body } = Enum.reverse(defaults) ++ Module.get_attribute(env.module, :plugs) |> Plug.Builder.compile @@ -107,7 +107,7 @@ defmodule Sugar.Router do defoverridable [init: 1, call: 2] def copy_req_content_type(conn, _opts) do - default = Application.get_env(:sugar, :default_content_type, "application/json; charset=utf-8") + default = Application.get_env(:sugar, :default_content_type, "text/html; charset=utf-8") content_type = case Plug.Conn.get_req_header conn, "content-type" do [content_type] -> content_type _ -> default diff --git a/test/sugar/request/https_only_test.exs b/test/sugar/request/https_only_test.exs new file mode 100644 index 0000000..e4ddfff --- /dev/null +++ b/test/sugar/request/https_only_test.exs @@ -0,0 +1,13 @@ +defmodule Sugar.Request.HttpsOnlyTest do + use ExUnit.Case, async: true + import Plug.Test + + test "translates json extension" do + opts = Sugar.Request.HttpsOnly.init([]) + conn = conn(:get, "/get.json") + |> Sugar.Request.HttpsOnly.call(opts) + + assert conn.status === 403 + assert conn.resp_body === "Forbidden" + end +end \ No newline at end of file diff --git a/test/sugar/request/parsers_test.exs b/test/sugar/request/parsers_test.exs new file mode 100644 index 0000000..6a6ea44 --- /dev/null +++ b/test/sugar/request/parsers_test.exs @@ -0,0 +1,56 @@ +defmodule Sugar.Request.ParsersTest do + use ExUnit.Case, async: true + import Plug.Test + + @parsers [ Sugar.Request.Parsers.XML ] + + def parse(conn, opts \\ []) do + opts = Keyword.put_new(opts, :parsers, @parsers) + Plug.Parsers.call(conn, Plug.Parsers.init(opts)) + end + + test "parses xml encoded bodies" do + headers = [{"content-type", "application/xml"}] + conn = parse(conn(:post, "/post", "baz", headers: headers)) + foo = conn.params.xml + |> Enum.find(fn node -> + node.name === :foo + end) + + assert foo.value |> hd === "baz" + end + + test "parses xml encoded bodies with xml nodes" do + headers = [{"content-type", "application/xml"}] + conn = parse(conn(:post, "/post", "", headers: headers)) + foo = conn.params.xml + |> Enum.find(fn node -> + node.name === :foo + end) + bar = foo.value |> hd + + assert foo.value |> Enum.count === 2 + assert bar.name === :bar + end + + test "parses xml encoded bodies with attributes" do + headers = [{"content-type", "application/xml"}] + conn = parse(conn(:post, "/post", "", headers: headers)) + foo = conn.params.xml + |> Enum.find(fn node -> + node.name === :foo + end) + + assert foo.attr[:bar] === "baz" + assert foo.attr[:id] === "1" + end + + test "xml parser errors when body too large" do + exception = assert_raise Plug.Parsers.RequestTooLargeError, fn -> + headers = [{"content-type", "application/xml"}] + parse(conn(:post, "/post", "baz", headers: headers), length: 5) + end + + assert Plug.Exception.status(exception) === 413 + end +end \ No newline at end of file