From 753868a15807ea7255b3fd34e198e02576d01c78 Mon Sep 17 00:00:00 2001 From: jimmay5469 Date: Tue, 12 Jun 2018 16:18:41 -0400 Subject: [PATCH] Standardize queryparam processing Co-authored-by: Stamates Co-authored-by: Tim Mecklem --- ...address_internal_transaction_controller.ex | 59 ++++++++----------- .../address_transaction_controller.ex | 45 +++++++------- .../controllers/transaction_controller.ex | 40 ++++++------- .../transaction_controller_test.exs | 10 ---- 4 files changed, 64 insertions(+), 90 deletions(-) diff --git a/apps/explorer_web/lib/explorer_web/controllers/address_internal_transaction_controller.ex b/apps/explorer_web/lib/explorer_web/controllers/address_internal_transaction_controller.ex index 592bf71a87..57baa6f92d 100644 --- a/apps/explorer_web/lib/explorer_web/controllers/address_internal_transaction_controller.ex +++ b/apps/explorer_web/lib/explorer_web/controllers/address_internal_transaction_controller.ex @@ -13,43 +13,18 @@ defmodule ExplorerWeb.AddressInternalTransactionController do @page_size 50 @default_paging_options %PagingOptions{page_size: @page_size + 1} - def index( - conn, - %{ - "block_number" => block_number_string, - "transaction_index" => transaction_index_string, - "index" => index_string - } = params - ) do - with {block_number, ""} <- Integer.parse(block_number_string), - {transaction_index, ""} <- Integer.parse(transaction_index_string), - {index, ""} <- Integer.parse(index_string) do - do_index( - conn, - Map.put(params, :paging_options, %{@default_paging_options | key: {block_number, transaction_index, index}}) - ) - else - _ -> - unprocessable_entity(conn) - end - end - - def index(conn, params), do: do_index(conn, params) - - def do_index(conn, %{"address_id" => address_hash_string} = params) do + def index(conn, %{"address_id" => address_hash_string} = params) do with {:ok, address_hash} <- Chain.string_to_address_hash(address_hash_string), {:ok, address} <- Chain.hash_to_address(address_hash) do full_options = - Keyword.merge( - [ - necessity_by_association: %{ - from_address: :optional, - to_address: :optional - }, - paging_options: @default_paging_options - ], - current_filter(params) - ) + [ + necessity_by_association: %{ + from_address: :optional, + to_address: :optional + } + ] + |> Keyword.merge(paging_options(params)) + |> Keyword.merge(current_filter(params)) internal_transactions_plus_one = Chain.address_to_internal_transactions(address, full_options) @@ -101,4 +76,20 @@ defmodule ExplorerWeb.AddressInternalTransactionController do {:ok, last_transaction} = Chain.hash_to_transaction(last.transaction_hash) %{block_number: last_transaction.block_number, transaction_index: last_transaction.index, index: last.index} end + + defp paging_options(params) do + with %{ + "block_number" => block_number_string, + "transaction_index" => transaction_index_string, + "index" => index_string + } <- params, + {block_number, ""} <- Integer.parse(block_number_string), + {transaction_index, ""} <- Integer.parse(transaction_index_string), + {index, ""} <- Integer.parse(index_string) do + [paging_options: %{@default_paging_options | key: {block_number, transaction_index, index}}] + else + _ -> + [paging_options: @default_paging_options] + end + end end diff --git a/apps/explorer_web/lib/explorer_web/controllers/address_transaction_controller.ex b/apps/explorer_web/lib/explorer_web/controllers/address_transaction_controller.ex index de36f3abc6..8fa2dc3e17 100644 --- a/apps/explorer_web/lib/explorer_web/controllers/address_transaction_controller.ex +++ b/apps/explorer_web/lib/explorer_web/controllers/address_transaction_controller.ex @@ -13,33 +13,19 @@ defmodule ExplorerWeb.AddressTransactionController do @page_size 50 @default_paging_options %PagingOptions{page_size: @page_size + 1} - def index(conn, %{"block_number" => block_number_string, "index" => index_string} = params) do - with {block_number, ""} <- Integer.parse(block_number_string), - {index, ""} <- Integer.parse(index_string) do - do_index(conn, Map.put(params, :paging_options, %{@default_paging_options | key: {block_number, index}})) - else - _ -> - unprocessable_entity(conn) - end - end - - def index(conn, params), do: do_index(conn, params) - - def do_index(conn, %{"address_id" => address_hash_string} = params) do + def index(conn, %{"address_id" => address_hash_string} = params) do with {:ok, address_hash} <- Chain.string_to_address_hash(address_hash_string), {:ok, address} <- Chain.hash_to_address(address_hash) do full_options = - Keyword.merge( - [ - necessity_by_association: %{ - block: :required, - from_address: :optional, - to_address: :optional - }, - paging_options: @default_paging_options - ], - current_filter(params) - ) + [ + necessity_by_association: %{ + block: :required, + from_address: :optional, + to_address: :optional + } + ] + |> Keyword.merge(paging_options(params)) + |> Keyword.merge(current_filter(params)) transactions_plus_one = Chain.address_to_transactions(address, full_options) @@ -90,4 +76,15 @@ defmodule ExplorerWeb.AddressTransactionController do last = List.last(transactions) %{block_number: last.block_number, index: last.index} end + + defp paging_options(params) do + with %{"block_number" => block_number_string, "index" => index_string} <- params, + {block_number, ""} <- Integer.parse(block_number_string), + {index, ""} <- Integer.parse(index_string) do + [paging_options: %{@default_paging_options | key: {block_number, index}}] + else + _ -> + [paging_options: @default_paging_options] + end + end end diff --git a/apps/explorer_web/lib/explorer_web/controllers/transaction_controller.ex b/apps/explorer_web/lib/explorer_web/controllers/transaction_controller.ex index 4ea6e8636e..10fccfcf5b 100644 --- a/apps/explorer_web/lib/explorer_web/controllers/transaction_controller.ex +++ b/apps/explorer_web/lib/explorer_web/controllers/transaction_controller.ex @@ -6,25 +6,7 @@ defmodule ExplorerWeb.TransactionController do @page_size 50 @default_paging_options %PagingOptions{page_size: @page_size + 1} - def index(conn, %{"block_number" => block_number_string, "index" => index_string}) do - with {block_number, ""} <- Integer.parse(block_number_string), - {index, ""} <- Integer.parse(index_string) do - do_index(conn, paging_options: %{@default_paging_options | key: {block_number, index}}) - else - _ -> - unprocessable_entity(conn) - end - end - - def index(conn, _params) do - do_index(conn) - end - - def show(conn, %{"id" => id, "locale" => locale}) do - redirect(conn, to: transaction_internal_transaction_path(conn, :index, locale, id)) - end - - defp do_index(conn, options \\ []) when is_list(options) do + def index(conn, params) do full_options = Keyword.merge( [ @@ -32,10 +14,9 @@ defmodule ExplorerWeb.TransactionController do block: :required, from_address: :optional, to_address: :optional - }, - paging_options: @default_paging_options + } ], - options + paging_options(params) ) transactions_plus_one = Chain.recent_collated_transactions(full_options) @@ -53,10 +34,25 @@ defmodule ExplorerWeb.TransactionController do ) end + def show(conn, %{"id" => id, "locale" => locale}) do + redirect(conn, to: transaction_internal_transaction_path(conn, :index, locale, id)) + end + defp next_page_params(nil, _transactions), do: nil defp next_page_params(_, transactions) do last = List.last(transactions) %{block_number: last.block_number, index: last.index} end + + defp paging_options(params) do + with %{"block_number" => block_number_string, "index" => index_string} <- params, + {block_number, ""} <- Integer.parse(block_number_string), + {index, ""} <- Integer.parse(index_string) do + [paging_options: %{@default_paging_options | key: {block_number, index}}] + else + _ -> + [paging_options: @default_paging_options] + end + end end diff --git a/apps/explorer_web/test/explorer_web/controllers/transaction_controller_test.exs b/apps/explorer_web/test/explorer_web/controllers/transaction_controller_test.exs index 3a873d190b..2fb6bcaec8 100644 --- a/apps/explorer_web/test/explorer_web/controllers/transaction_controller_test.exs +++ b/apps/explorer_web/test/explorer_web/controllers/transaction_controller_test.exs @@ -89,16 +89,6 @@ defmodule ExplorerWeb.TransactionControllerTest do refute conn.assigns.next_page_params end - test "guards against bad block_number input", %{conn: conn} do - conn = get(conn, "/en/transactions", %{"block_number" => "foo", "index" => "2"}) - assert html_response(conn, 422) - end - - test "guards against bad index input", %{conn: conn} do - conn = get(conn, "/en/transactions", %{"block_number" => "2", "index" => "bar"}) - assert html_response(conn, 422) - end - test "sends back the number of transactions", %{conn: conn} do insert(:transaction) |> with_block()