From da72f83b35b5f0752adb0e1484517881954331fc Mon Sep 17 00:00:00 2001 From: Felipe Renan Date: Thu, 22 Nov 2018 14:13:45 -0200 Subject: [PATCH] Refactor token transfers' pagination We changed the token transfer's pagination uses the block_number instead of the inserted_at. It will make the pagination consistent since the list is sorted by the block_number. --- .../lib/block_scout_web/chain.ex | 12 +----- ...saction_token_transfer_controller_test.exs | 18 +++----- .../lib/explorer/chain/token_transfer.ex | 19 +++----- .../explorer/chain/token_transfer_test.exs | 43 ++++++++++++++++--- apps/explorer/test/explorer/chain_test.exs | 10 ++--- 5 files changed, 57 insertions(+), 45 deletions(-) diff --git a/apps/block_scout_web/lib/block_scout_web/chain.ex b/apps/block_scout_web/lib/block_scout_web/chain.ex index 08e4e5c321..1d956cfa65 100644 --- a/apps/block_scout_web/lib/block_scout_web/chain.ex +++ b/apps/block_scout_web/lib/block_scout_web/chain.ex @@ -130,9 +130,6 @@ defmodule BlockScoutWeb.Chain do end end - def paging_options(%{"inserted_at" => inserted_at}), - do: [paging_options: %{@default_paging_options | key: inserted_at}] - def paging_options(%{"token_name" => name, "token_type" => type, "token_inserted_at" => inserted_at}), do: [paging_options: %{@default_paging_options | key: {name, type, inserted_at}}] @@ -180,13 +177,8 @@ defmodule BlockScoutWeb.Chain do %{"block_number" => block_number, "index" => index} end - defp paging_params(%TokenTransfer{inserted_at: inserted_at}) do - inserted_at_datetime = - inserted_at - |> DateTime.from_naive!("Etc/UTC") - |> DateTime.to_iso8601() - - %{"inserted_at" => inserted_at_datetime} + defp paging_params(%TokenTransfer{block_number: block_number, log_index: index}) do + %{"block_number" => block_number, "index" => index} end defp paging_params(%Address.Token{name: name, type: type, inserted_at: inserted_at}) do diff --git a/apps/block_scout_web/test/block_scout_web/controllers/transaction_token_transfer_controller_test.exs b/apps/block_scout_web/test/block_scout_web/controllers/transaction_token_transfer_controller_test.exs index a5b13b6bb0..4ff1801610 100644 --- a/apps/block_scout_web/test/block_scout_web/controllers/transaction_token_transfer_controller_test.exs +++ b/apps/block_scout_web/test/block_scout_web/controllers/transaction_token_transfer_controller_test.exs @@ -82,25 +82,21 @@ defmodule BlockScoutWeb.TransactionTokenTransferControllerTest do |> insert() |> with_block() - {:ok, first_transfer_time} = NaiveDateTime.new(2000, 1, 1, 0, 0, 5) - {:ok, remaining_transfers_time} = NaiveDateTime.new(1999, 1, 1, 0, 0, 0) - insert(:token_transfer, transaction: transaction, inserted_at: first_transfer_time) + token_transfer = insert(:token_transfer, transaction: transaction, block_number: 1000, log_index: 1) - 1..5 - |> Enum.each(fn log_index -> - insert(:token_transfer, transaction: transaction, inserted_at: remaining_transfers_time, log_index: log_index) + Enum.each(2..5, fn item -> + insert(:token_transfer, transaction: transaction, block_number: item + 1001, log_index: item + 1) end) conn = get(conn, transaction_token_transfer_path(BlockScoutWeb.Endpoint, :index, transaction.hash), %{ - "inserted_at" => first_transfer_time |> DateTime.from_naive!("Etc/UTC") |> DateTime.to_iso8601() + "block_number" => "1000", + "index" => "1" }) - actual_times = - conn.assigns.token_transfers - |> Enum.map(& &1.inserted_at) + actual_log_indexes = Enum.map(conn.assigns.token_transfers, & &1.log_index) - refute Enum.any?(actual_times, fn time -> first_transfer_time == time end) + refute Enum.any?(actual_log_indexes, fn log_index -> log_index == token_transfer.log_index end) end test "next_page_params exist if not on last page", %{conn: conn} do diff --git a/apps/explorer/lib/explorer/chain/token_transfer.ex b/apps/explorer/lib/explorer/chain/token_transfer.ex index a539dfa7e0..c50bc23c64 100644 --- a/apps/explorer/lib/explorer/chain/token_transfer.ex +++ b/apps/explorer/lib/explorer/chain/token_transfer.ex @@ -24,7 +24,8 @@ defmodule Explorer.Chain.TokenTransfer do use Ecto.Schema - import Ecto.{Changeset, Query} + import Ecto.Changeset + import Ecto.Query, only: [from: 2, dynamic: 2, limit: 2, where: 3] alias Explorer.Chain.{Address, Hash, Token, TokenTransfer, Transaction} alias Explorer.{PagingOptions, Repo} @@ -123,7 +124,7 @@ defmodule Explorer.Chain.TokenTransfer do tt in TokenTransfer, where: tt.token_contract_address_hash == ^token_address_hash, preload: [{:transaction, :block}, :token, :from_address, :to_address], - order_by: [desc: tt.block_number] + order_by: [desc: tt.block_number, desc: tt.log_index] ) query @@ -134,19 +135,11 @@ defmodule Explorer.Chain.TokenTransfer do def page_token_transfer(query, %PagingOptions{key: nil}), do: query - def page_token_transfer(query, %PagingOptions{key: {token_id}}) do + def page_token_transfer(query, %PagingOptions{key: {block_number, log_index}}) do where( query, - [token_transfer], - token_transfer.token_id > ^token_id - ) - end - - def page_token_transfer(query, %PagingOptions{key: inserted_at}) do - where( - query, - [token_transfer], - token_transfer.inserted_at < ^inserted_at + [tt], + tt.block_number < ^block_number or (tt.block_number == ^block_number and tt.log_index < ^log_index) ) end diff --git a/apps/explorer/test/explorer/chain/token_transfer_test.exs b/apps/explorer/test/explorer/chain/token_transfer_test.exs index a48326cc8c..0ba6b11b66 100644 --- a/apps/explorer/test/explorer/chain/token_transfer_test.exs +++ b/apps/explorer/test/explorer/chain/token_transfer_test.exs @@ -8,7 +8,7 @@ defmodule Explorer.Chain.TokenTransferTest do doctest Explorer.Chain.TokenTransfer - describe "fetch_token_transfers/2" do + describe "fetch_token_transfers_from_token_hash/2" do test "returns token transfers for the given address" do token_contract_address = insert(:contract_address) @@ -87,6 +87,7 @@ defmodule Explorer.Chain.TokenTransferTest do second_page = insert( :token_transfer, + block_number: 999, to_address: build(:address), transaction: transaction, token_contract_address: token_contract_address, @@ -96,23 +97,53 @@ defmodule Explorer.Chain.TokenTransferTest do first_page = insert( :token_transfer, + block_number: 1000, to_address: build(:address), transaction: transaction, token_contract_address: token_contract_address, token: token ) - paging_options = %PagingOptions{key: first_page.inserted_at, page_size: 1} + paging_options = %PagingOptions{key: {first_page.block_number, first_page.log_index}, page_size: 1} token_transfers_primary_keys_paginated = - TokenTransfer.fetch_token_transfers_from_token_hash( - token_contract_address.hash, - paging_options: paging_options - ) + token_contract_address.hash + |> TokenTransfer.fetch_token_transfers_from_token_hash(paging_options: paging_options) |> Enum.map(&{&1.transaction_hash, &1.log_index}) assert token_transfers_primary_keys_paginated == [{second_page.transaction_hash, second_page.log_index}] end + + test "paginates considering the log_index when there are repeated block numbers" do + token_contract_address = insert(:contract_address) + + transaction = + :transaction + |> insert() + |> with_block() + + token = insert(:token) + + token_transfer = + insert( + :token_transfer, + block_number: 1000, + log_index: 0, + to_address: build(:address), + transaction: transaction, + token_contract_address: token_contract_address, + token: token + ) + + paging_options = %PagingOptions{key: {token_transfer.block_number, token_transfer.log_index + 1}, page_size: 1} + + token_transfers_primary_keys_paginated = + token_contract_address.hash + |> TokenTransfer.fetch_token_transfers_from_token_hash(paging_options: paging_options) + |> Enum.map(&{&1.transaction_hash, &1.log_index}) + + assert token_transfers_primary_keys_paginated == [{token_transfer.transaction_hash, token_transfer.log_index}] + end end describe "count_token_transfers/0" do diff --git a/apps/explorer/test/explorer/chain_test.exs b/apps/explorer/test/explorer/chain_test.exs index 13c0f44906..7c4dbdf8e0 100644 --- a/apps/explorer/test/explorer/chain_test.exs +++ b/apps/explorer/test/explorer/chain_test.exs @@ -3112,6 +3112,7 @@ defmodule Explorer.ChainTest do first_page = insert( :token_transfer, + block_number: 1000, to_address: build(:address), transaction: transaction, token_contract_address: token_contract_address, @@ -3122,6 +3123,7 @@ defmodule Explorer.ChainTest do second_page = insert( :token_transfer, + block_number: 999, to_address: build(:address), transaction: transaction, token_contract_address: token_contract_address, @@ -3129,13 +3131,11 @@ defmodule Explorer.ChainTest do token_id: 29 ) - paging_options = %PagingOptions{key: {first_page.token_id}, page_size: 1} + paging_options = %PagingOptions{key: {first_page.block_number, first_page.log_index}, page_size: 1} unique_tokens_ids_paginated = - Chain.address_to_unique_tokens( - token_contract_address.hash, - paging_options: paging_options - ) + token_contract_address.hash + |> Chain.address_to_unique_tokens(paging_options: paging_options) |> Enum.map(& &1.token_id) assert unique_tokens_ids_paginated == [second_page.token_id]