From 44e6912a08468743f7dfa1343723914bbf6809e1 Mon Sep 17 00:00:00 2001 From: Josh Price Date: Tue, 14 Jun 2016 12:11:00 +1000 Subject: [PATCH] Fix Elixir 1.3 warnings (#21) * Fix Elixir 1.3 warnings * Made some other small refactorings for clarity Tests are still passing but not sure if every case was covered. * Depipeline single function calls * Add a note about `after` keyword * Refactoring for clarity --- lib/graphql/relay/connection/ecto.ex | 72 ++++++++++++---------------- lib/graphql/relay/connection/list.ex | 53 +++++++++----------- lib/graphql_relay.ex | 9 +--- 3 files changed, 56 insertions(+), 78 deletions(-) diff --git a/lib/graphql/relay/connection/ecto.ex b/lib/graphql/relay/connection/ecto.ex index 5a502c5..922c618 100644 --- a/lib/graphql/relay/connection/ecto.ex +++ b/lib/graphql/relay/connection/ecto.ex @@ -11,21 +11,25 @@ if Code.ensure_loaded?(Ecto) do def resolve(query, %{repo: repo} = args) do before = cursor_to_offset(args[:before]) + # `after` is a keyword http://elixir-lang.org/docs/master/elixir/Kernel.SpecialForms.html#try/1 a_after = cursor_to_offset(args[:after]) first = args[:first] last = args[:last] limit = Enum.min([first, last, connection_count(repo, query)]) - if a_after do - query = from things in query, where: things.id > ^a_after + query = if a_after do + from things in query, where: things.id > ^a_after + else + query end - if before do - query = from things in query, where: things.id < ^before + query = if before do + from things in query, where: things.id < ^before + else + query end - # Calculate has_next_page/has_prev_page before order_by to avoid group_by - # requirement + # Calculate has_next_page/has_prev_page before order_by to avoid group_by requirement has_next_page = case first do nil -> false _ -> @@ -44,16 +48,16 @@ if Code.ensure_loaded?(Ecto) do repo.one(has_prev_records_query) > last end - if first do - query = from things in query, order_by: [asc: things.id], limit: ^limit + query = if first do + from things in query, order_by: [asc: things.id], limit: ^limit else - has_next_page = false + query end - if last do - query = from things in query, order_by: [desc: things.id], limit: ^limit + query = if last do + from things in query, order_by: [desc: things.id], limit: ^limit else - has_prev_page = false + query end records = repo.all(query) @@ -66,10 +70,8 @@ if Code.ensure_loaded?(Ecto) do end) edges = case last do - nil -> - edges - _ -> - Enum.reverse(edges) + nil -> edges + _ -> Enum.reverse(edges) end first_edge = List.first(edges) @@ -78,34 +80,22 @@ if Code.ensure_loaded?(Ecto) do %{ edges: edges, pageInfo: %{ - startCursor: first_edge && Map.get(first_edge, :cursor) || nil, - endCursor: last_edge && Map.get(last_edge, :cursor) || nil, + startCursor: first_edge && Map.get(first_edge, :cursor), + endCursor: last_edge && Map.get(last_edge, :cursor), hasPreviousPage: has_prev_page, hasNextPage: has_next_page } } end - def get_offset_with_default(cursor, default_offset) do - unless cursor do - default_offset - else - offset = cursor_to_offset(cursor) - offset || default_offset - end - end - + def cursor_to_offset(nil), do: nil def cursor_to_offset(cursor) do - case cursor do - nil -> nil - _ -> - case Base.decode64(cursor) do - {:ok, decoded_cursor} -> - {int, _} = Integer.parse(String.slice(decoded_cursor, String.length(@prefix)..String.length(decoded_cursor))) - int - :error -> - nil - end + case Base.decode64(cursor) do + {:ok, decoded_cursor} -> + {int, _} = Integer.parse(String.slice(decoded_cursor, String.length(@prefix)..String.length(decoded_cursor))) + int + :error -> + nil end end @@ -121,20 +111,20 @@ if Code.ensure_loaded?(Ecto) do defp make_query_countable(query) do query - |> remove_select - |> remove_order + |> remove_select + |> remove_order end # Remove select if it exists so that we avoid `only one select # expression is allowed in query` Ecto exception defp remove_select(query) do - query |> Ecto.Query.exclude(:select) + Ecto.Query.exclude(query, :select) end # Remove order by if it exists so that we avoid `field X in "order_by" # does not exist in the model source in query` defp remove_order(query) do - query |> Ecto.Query.exclude(:order_by) + Ecto.Query.exclude(query, :order_by) end end end diff --git a/lib/graphql/relay/connection/list.ex b/lib/graphql/relay/connection/list.ex index fb0ce11..c006e9c 100644 --- a/lib/graphql/relay/connection/list.ex +++ b/lib/graphql/relay/connection/list.ex @@ -1,9 +1,7 @@ defmodule GraphQL.Relay.Connection.List do @prefix "arrayconnection:" - def resolve(data) do - resolve(data, %{}) - end + def resolve(data), do: resolve(data, %{}) def resolve(data, args) do resolve_slice(data, args, %{ slice_start: 0, @@ -13,34 +11,40 @@ defmodule GraphQL.Relay.Connection.List do def resolve_slice(records, args, meta) do before = args[:before] + # `after` is a keyword http://elixir-lang.org/docs/master/elixir/Kernel.SpecialForms.html#try/1 a_after = args[:after] first = args[:first] last = args[:last] slice_start = meta[:slice_start] || 0 list_length = meta[:list_length] || length(records) slice_end = slice_start + length(records) - before_offset = get_offset_with_default(before, list_length) - after_offset = get_offset_with_default(a_after, -1) + before_offset = cursor_to_offset(before, list_length) + after_offset = cursor_to_offset(a_after, -1) start_offset = Enum.max([slice_start - 1, after_offset, -1]) + 1 end_offset = Enum.min([slice_end, before_offset, list_length]) - if first do - end_offset = Enum.min([end_offset, start_offset + first]) + end_offset = if first do + Enum.min([end_offset, start_offset + first]) + else + end_offset end - if last do - start_offset = Enum.max([start_offset, end_offset - last]) + start_offset = if last do + Enum.max([start_offset, end_offset - last]) + else + start_offset end from_slice = Enum.max([start_offset - slice_start, 0]) to_slice = length(records) - (slice_end - end_offset) - 1 slice = case first do 0 -> [] - _ -> - Enum.slice(records, from_slice..to_slice) + _ -> Enum.slice(records, from_slice..to_slice) end - {edges, _count} = Enum.map_reduce(slice, 0, fn(record, acc) -> {%{ cursor: offset_to_cursor(start_offset+acc), node: record }, acc + 1} end) + {edges, _count} = Enum.map_reduce(slice, 0, fn(record, acc) -> + {%{cursor: offset_to_cursor(start_offset+acc), node: record}, acc + 1} + end) first_edge = List.first(edges) last_edge = List.last(edges) @@ -50,42 +54,31 @@ defmodule GraphQL.Relay.Connection.List do %{ edges: edges, pageInfo: %{ - startCursor: first_edge && Map.get(first_edge, :cursor) || nil, - endCursor: last_edge && Map.get(last_edge, :cursor) || nil, + startCursor: first_edge && Map.get(first_edge, :cursor), + endCursor: last_edge && Map.get(last_edge, :cursor), hasPreviousPage: last && (start_offset > lower_bound) || false, hasNextPage: first && (end_offset < upper_bound) || false } } end - def get_offset_with_default(cursor, default_offset) do - unless cursor do - default_offset - else - offset = cursor_to_offset(cursor) - offset || default_offset - end - end - - def cursor_to_offset(cursor) do + def cursor_to_offset(nil, default), do: default + def cursor_to_offset(cursor, default) do case Base.decode64(cursor) do {:ok, decoded_cursor} -> {int, _} = Integer.parse(String.slice(decoded_cursor, String.length(@prefix)..String.length(decoded_cursor))) int :error -> - nil + default end end def cursor_for_object_in_connection(data, object) do offset = Enum.find_index(data, fn(obj) -> object == obj end) - unless offset do - nil - else - offset_to_cursor(offset) - end + offset_to_cursor(offset) end + def offset_to_cursor(nil), do: nil def offset_to_cursor(offset) do Base.encode64("#{@prefix}#{offset}") end diff --git a/lib/graphql_relay.ex b/lib/graphql_relay.ex index 3d383f2..6fc9970 100644 --- a/lib/graphql_relay.ex +++ b/lib/graphql_relay.ex @@ -6,13 +6,8 @@ defmodule GraphQL.Relay do """ @spec resolve_maybe_thunk(fun | map) :: %{} - def resolve_maybe_thunk(thing_or_thunk) do - if Kernel.is_function(thing_or_thunk) do - thing_or_thunk.() - else - thing_or_thunk - end - end + def resolve_maybe_thunk(t) when is_function(t), do: t.() + def resolve_maybe_thunk(t), do: t def generate_schema_json! do Logger.debug "Updating GraphQL schema.json"