Skip to content

Fix Elixir 1.3 warnings #21

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jun 14, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Figured that this was already set in the clause above.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto

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
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was wondering whether it was possible to refactor this to be set in a single if. Thoughts?

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