Merge pull request #26 from aweiker/code-cleanup

Code cleanup

Author: Josh Price

config/.credo.exs

@@ -0,0 +1,91 @@
+# This file contains the configuration for Credo.
+#
+# If you find anything wrong or unclear in this file, please report an
+# issue on GitHub: https://github.com/rrrene/credo/issues
+%{
+  #
+  # You can have as many configs as you like in the `configs:` field.
+  configs: [
+    %{
+      #
+      # Run any config using `mix credo -C <name>`. If no config name is given
+      # "default" is used.
+      name: "default",
+      #
+      # these are the files included in the analysis
+      files: %{
+        #
+        # you can give explicit globs or simply directories
+        # in the latter case `**/*.{ex,exs}` will be used
+        included: ["lib/", "src/", "web/", "apps/"],
+        excluded: []
+      },
+      #
+      # The `checks:` field contains all the checks that are run. You can
+      # customize the parameters of any given check by adding a second element
+      # to the tuple.
+      #
+      # There are two ways of deactivating a check:
+      # 1. deleting the check from this list
+      # 2. putting `false` as second element (to quickly "comment it out"):
+      #
+      #      {Credo.Check.Consistency.ExceptionNames, false}
+      #
+      checks: [
+        {Credo.Check.Consistency.ExceptionNames},
+        {Credo.Check.Consistency.LineEndings},
+        {Credo.Check.Consistency.SpaceAroundOperators},
+        {Credo.Check.Consistency.SpaceInParentheses},
+        {Credo.Check.Consistency.TabsOrSpaces},
+
+        # For some checks, like AliasUsage, you can only customize the priority
+        # Priority values are: `low, normal, high, higher`
+        {Credo.Check.Design.AliasUsage, priority: :low},
+        # For others you can set parameters
+        {Credo.Check.Design.DuplicatedCode, mass_threshold: 16, nodes_threshold: 2},
+
+        # You can also customize the exit_status of each check.
+        # If you don't want TODO comments to cause `mix credo` to fail, just
+        # set this value to 0 (zero).
+        {Credo.Check.Design.TagTODO, false},
+        {Credo.Check.Design.TagFIXME},
+
+        {Credo.Check.Readability.FunctionNames},
+        {Credo.Check.Readability.MaxLineLength, priority: :low, max_length: 120},
+        {Credo.Check.Readability.ModuleAttributeNames},
+        {Credo.Check.Readability.ModuleDoc},
+        {Credo.Check.Readability.ModuleNames},
+        {Credo.Check.Readability.PredicateFunctionNames},
+        {Credo.Check.Readability.TrailingBlankLine},
+        {Credo.Check.Readability.TrailingWhiteSpace},
+        {Credo.Check.Readability.VariableNames},
+
+        {Credo.Check.Refactor.ABCSize},
+        {Credo.Check.Refactor.CaseTrivialMatches},
+        {Credo.Check.Refactor.CondStatements},
+        {Credo.Check.Refactor.FunctionArity},
+        {Credo.Check.Refactor.MatchInCondition},
+        {Credo.Check.Refactor.PipeChainStart},
+        {Credo.Check.Refactor.CyclomaticComplexity},
+        {Credo.Check.Refactor.NegatedConditionsInUnless},
+        {Credo.Check.Refactor.NegatedConditionsWithElse},
+        {Credo.Check.Refactor.Nesting},
+        {Credo.Check.Refactor.UnlessWithElse},
+
+        {Credo.Check.Warning.IExPry},
+        {Credo.Check.Warning.IoInspect},
+        {Credo.Check.Warning.NameRedeclarationByAssignment},
+        {Credo.Check.Warning.NameRedeclarationByCase},
+        {Credo.Check.Warning.NameRedeclarationByDef},
+        {Credo.Check.Warning.NameRedeclarationByFn},
+        {Credo.Check.Warning.OperationOnSameValues},
+        {Credo.Check.Warning.UnusedEnumOperation},
+        {Credo.Check.Warning.UnusedKeywordOperation},
+        {Credo.Check.Warning.UnusedListOperation},
+        {Credo.Check.Warning.UnusedStringOperation},
+        {Credo.Check.Warning.UnusedTupleOperation},
+        {Credo.Check.Warning.OperationWithConstantResult},
+      ]
+    }
+  ]
+}

config/dogma.exs

@@ -14,4 +14,5 @@ config :dogma,
   # Override an existing rule configuration
   override: [
     %Rule.LineLength{max_length: 120},
+    %Rule.FunctionArity{max: 5}
   ]

lib/graphql/plug.ex

@@ -18,37 +18,40 @@ defmodule GraphQL.Plug do
   """
 
   use Plug.Builder
+  alias GraphQL.Schema
+  alias GraphQL.Plug.{GraphiQL, Endpoint}
+  alias Plug.Parsers
 
   require Logger
 
-  plug Plug.Parsers,
+  plug Parsers,
     parsers: [:graphql, :urlencoded, :multipart, :json],
     pass: ["*/*"],
     json_decoder: Poison
 
   @type init :: %{
-    schema: GraphQL.Schema.t,
+    schema: Schema.t,
     root_value: ConfigurableValue.t,
     query:  ConfigurableValue.t,
     allow_graphiql?: true | false
   }
 
   @spec init(Map) :: init
   def init(opts) do
-    graphiql = GraphQL.Plug.GraphiQL.init(opts)
-    endpoint = GraphQL.Plug.Endpoint.init(opts)
+    graphiql = GraphiQL.init(opts)
+    endpoint = Endpoint.init(opts)
 
-    Keyword.merge(graphiql, endpoint)
-    |> Enum.dedup
+    opts = Keyword.merge(graphiql, endpoint)
+    Enum.dedup(opts)
   end
 
   def call(conn, opts) do
     conn = super(conn, opts)
 
-    conn = if GraphQL.Plug.GraphiQL.use_graphiql?(conn, opts) do
-      GraphQL.Plug.GraphiQL.call(conn, opts)
+    conn = if GraphiQL.use_graphiql?(conn, opts) do
+      GraphiQL.call(conn, opts)
     else
-      GraphQL.Plug.Endpoint.call(conn, opts)
+      Endpoint.call(conn, opts)
     end
 
     # TODO consider not logging instrospection queries

lib/graphql/plug/configurable_value.ex

@@ -9,8 +9,10 @@ defmodule GraphQL.Plug.ConfigurableValue do
   an arity of 1 accepting a `Plug.Conn`.
   """
 
-  @type t :: {module, atom} | (Plug.Conn.t -> Map) | Map | nil
-  @spec evaluate(Plug.Conn.t, t, any) :: Map
+  alias Plug.Conn
+
+  @type t :: {module, atom} | (Conn.t -> Map) | Map | nil
+  @spec evaluate(Conn.t, t, any) :: Map
 
   @error_msg "Configured function must only be arity of 1 that accepts a value of Plug.Conn"
 

lib/graphql/plug/endpoint.ex

@@ -41,22 +41,28 @@ defmodule GraphQL.Plug.Endpoint do
   end
 
   def call(%Conn{method: m} = conn, opts) when m in ["GET", "POST"] do
+    args = extract_arguments(conn, opts)
+    handle_call(conn, args)
+  end
+
+  def call(%Conn{method: _} = conn, _) do
+    handle_error(conn, "GraphQL only supports GET and POST requests.")
+  end
+
+  def extract_arguments(conn, opts) do
     query           = Parameter.query(conn) ||
                       ConfigurableValue.evaluate(conn, opts[:query], nil)
     variables       = Parameter.variables(conn)
     operation_name  = Parameter.operation_name(conn)
     root_value      = ConfigurableValue.evaluate(conn, opts[:root_value], %{})
 
-    cond do
-      query ->
-        handle_call(conn, opts[:schema], root_value, query, variables, operation_name)
-      true ->
-        handle_error(conn, "Must provide query string.")
-    end
-  end
-
-  def call(%Conn{method: _} = conn, _) do
-    handle_error(conn, "GraphQL only supports GET and POST requests.")
+    %{
+      query:          query,
+      variables:      variables,
+      operation_name: operation_name,
+      root_value:     root_value,
+      schema:         opts[:schema]
+    }
   end
 
   def handle_error(conn, message) do
@@ -66,14 +72,17 @@ defmodule GraphQL.Plug.Endpoint do
     |> send_resp(400, errors)
   end
 
-  def handle_call(conn, schema, root_value, query, variables, operation_name) do
+  def handle_call(conn, %{query: nil}) do
+    handle_error(conn, "Must provide query string.")
+  end
+  def handle_call(conn, args) do
     conn
     |> put_resp_content_type("application/json")
-    |> execute(schema, root_value, query, variables, operation_name)
+    |> execute(args)
   end
 
-  defp execute(conn, schema, root_value, query, variables, operation_name) do
-    case GraphQL.execute(schema, query, root_value, variables, operation_name) do
+  defp execute(conn, args) do
+    case GraphQL.execute(args.schema, args.query, args.root_value, args.variables, args.operation_name) do
       {:ok, data} ->
         case Poison.encode(data) do
           {:ok, json}      -> send_resp(conn, 200, json)

lib/graphql/plug/graphiql.ex

@@ -16,8 +16,6 @@ defmodule GraphQL.Plug.GraphiQL do
   import Plug.Conn
   alias Plug.Conn
   alias GraphQL.Plug.Endpoint
-  alias GraphQL.Plug.ConfigurableValue
-  alias GraphQL.Plug.Parameter
 
   @behaviour Plug
 
@@ -47,23 +45,16 @@ defmodule GraphQL.Plug.GraphiQL do
   def init(opts) do
     allow_graphiql? = Keyword.get(opts, :allow_graphiql?, true)
 
-    GraphQL.Plug.Endpoint.init(opts) ++ [allow_graphiql?: allow_graphiql?]
+    Endpoint.init(opts) ++ [allow_graphiql?: allow_graphiql?]
   end
 
   def call(%Conn{method: m} = conn, opts) when m in ["GET", "POST"] do
-    query           = Parameter.query(conn) ||
-                      ConfigurableValue.evaluate(conn, opts[:query], nil)
-    variables       = Parameter.variables(conn)
-    operation_name  = Parameter.operation_name(conn)
-    root_value      = ConfigurableValue.evaluate(conn, opts[:root_value], %{})
-
-    cond do
-      use_graphiql?(conn, opts) ->
-        handle_graphiql_call(conn, opts[:schema], root_value, query, variables, operation_name)
-      query ->
-        Endpoint.handle_call(conn, opts[:schema], root_value, query, variables, operation_name)
-      true ->
-        Endpoint.handle_error(conn, "Must provide query string.")
+    args = Endpoint.extract_arguments(conn, opts)
+
+    if use_graphiql?(conn, opts) do
+        handle_call(conn, args)
+    else
+        Endpoint.handle_call(conn, args)
     end
   end
 
@@ -77,12 +68,12 @@ defmodule GraphQL.Plug.GraphiQL do
     |> String.replace(~r/'/, "\\'")
   end
 
-  defp handle_graphiql_call(conn, schema, root_value, query, variables, operation_name) do
+  defp handle_call(conn, args) do
     # TODO construct a simple query from the schema (ie `schema.query.fields[0].fields[0..5]`)
-    query = query || @graphiql_instructions <> "\n{\n\tfield\n}\n"
+    query = args.query || @graphiql_instructions <> "\n{\n\tfield\n}\n"
 
-    {_, data} = GraphQL.execute(schema, query, root_value, variables, operation_name)
-    {:ok, variables} = Poison.encode(variables, pretty: true)
+    {_, data} = GraphQL.execute(args.schema, query, args.root_value, args.variables, args.operation_name)
+    {:ok, variables} = Poison.encode(args.variables, pretty: true)
     {:ok, result}    = Poison.encode(data, pretty: true)
 
     graphiql = graphiql_html(@graphiql_version, escape_string(query), escape_string(variables), escape_string(result))

lib/graphql/plug/parameter.ex

@@ -4,21 +4,23 @@ defmodule GraphQL.Plug.Parameter do
   value from `Plug.Conn`
   """
 
-  @spec operation_name(Plug.Conn.t) :: String.t
+  alias Plug.Conn
+
+  @spec operation_name(Conn.t) :: String.t
   def operation_name(conn) do
     conn
     |> operation_name_params
     |> cleanup_string
   end
 
-  @spec query(Plug.Conn.t) :: String.t | nil
+  @spec query(Conn.t) :: String.t | nil
   def query(conn) do
     conn.params
     |> Map.get("query")
     |> cleanup_string
   end
 
-  @spec variables(Plug.Conn.t) :: Map
+  @spec variables(Conn.t) :: Map
   def variables(conn) do
     decode_variables(Map.get(conn.params, "variables", %{}))
   end
@@ -32,9 +34,9 @@ defmodule GraphQL.Plug.Parameter do
     Map.get(conn.params, "operation_name")
   end
 
-  defp decode_variables(variables) when is_binary(variables) do
-    case Poison.decode(variables) do
-      {:ok, variables} -> variables
+  defp decode_variables(values) when is_binary(values) do
+    case Poison.decode(values) do
+      {:ok, values} -> values
       {:error, _} -> %{} # express-graphql ignores these errors currently
     end
   end

mix.exs

@@ -28,7 +28,7 @@ defmodule GraphQL.Plug.Mixfile do
      {:dogma, "~> 0.1", only: :dev},
      {:earmark, "~> 0.1", only: :dev},
      {:ex_doc, "~> 0.11", only: :dev},
-     {:mix_test_watch, only: :dev},
+     {:mix_test_watch, "~> 0.2", only: :dev},
      {:cowboy, "~> 1.0"},
      {:plug, "~> 0.14 or ~> 1.0"},
      {:poison, "~> 1.5 or ~> 2.0", override: true},