Skip to content

Commit

Permalink
Fix Elixir 1.3 warnings (#21)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
Josh Price authored Jun 14, 2016
1 parent 6d1b724 commit 44e6912
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 78 deletions.
72 changes: 31 additions & 41 deletions lib/graphql/relay/connection/ecto.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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
_ ->
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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

Expand All @@ -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
53 changes: 23 additions & 30 deletions lib/graphql/relay/connection/list.ex
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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)
Expand All @@ -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
Expand Down
9 changes: 2 additions & 7 deletions lib/graphql_relay.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down

0 comments on commit 44e6912

Please sign in to comment.