From 078f1e6955ff5f6d55541ef728ac8c6d9232a751 Mon Sep 17 00:00:00 2001 From: Stamates Date: Thu, 26 Jul 2018 11:28:45 -0400 Subject: [PATCH] Move transaction_hashes to transactions fetching to single query Co-authored-by: jimmay5469 --- apps/explorer/lib/explorer/chain.ex | 38 +++++++++++++++++++ apps/explorer/test/explorer/chain_test.exs | 37 +++++++++++++++++- .../explorer_web/lib/explorer_web/notifier.ex | 35 ++++++++--------- .../features/viewing_addresses_test.exs | 12 +++--- 4 files changed, 96 insertions(+), 26 deletions(-) diff --git a/apps/explorer/lib/explorer/chain.ex b/apps/explorer/lib/explorer/chain.ex index de945b5446..05bf71c17e 100644 --- a/apps/explorer/lib/explorer/chain.ex +++ b/apps/explorer/lib/explorer/chain.ex @@ -603,6 +603,44 @@ defmodule Explorer.Chain do end end + @doc """ + Converts list of `t:Explorer.Chain.Transaction.t/0` `hashes` to the list of `t:Explorer.Chain.Transaction.t/0`s for + those `hashes`. + + Returns list of `%Explorer.Chain.Transaction{}`s if found + + iex> [%Transaction{hash: hash1}, %Transaction{hash: hash2}] = insert_list(2, :transaction) + iex> [%Explorer.Chain.Transaction{hash: found_hash1}, %Explorer.Chain.Transaction{hash: found_hash2}] = + ...> Explorer.Chain.hashes_to_transactions([hash1, hash2]) + iex> found_hash1 == hash1 + true + iex> found_hash2 == hash2 + true + + Returns `[]` if not found + + iex> {:ok, hash} = Explorer.Chain.string_to_transaction_hash( + ...> "0x9fc76417374aa880d4449a1f7f31ec597f00b1f6f3dd2d66f4c9c6c445836d8b" + ...> ) + iex> Explorer.Chain.hashes_to_transactions([hash]) + [] + + ## Options + + * `:necessity_by_association` - use to load `t:association/0` as `:required` or `:optional`. If an association is + `:required`, and the `t:Explorer.Chain.Transaction.t/0` has no associated record for that association, then the + `t:Explorer.Chain.Transaction.t/0` will not be included in the page `entries`. + """ + @spec hashes_to_transactions([Hash.Full.t()], [necessity_by_association_option]) :: [Transaction.t()] | [] + def hashes_to_transactions(hashes, options \\ []) when is_list(hashes) and is_list(options) do + necessity_by_association = Keyword.get(options, :necessity_by_association, %{}) + + fetch_transactions() + |> where([transaction], transaction.hash in ^hashes) + |> join_associations(necessity_by_association) + |> Repo.all() + end + @doc """ Bulk insert blocks from a list of blocks. diff --git a/apps/explorer/test/explorer/chain_test.exs b/apps/explorer/test/explorer/chain_test.exs index 546034970b..3967ca2db4 100644 --- a/apps/explorer/test/explorer/chain_test.exs +++ b/apps/explorer/test/explorer/chain_test.exs @@ -4,7 +4,7 @@ defmodule Explorer.ChainTest do import Explorer.Factory alias Explorer.{Chain, Factory, PagingOptions, Repo} - alias Explorer.Chain.{Address, Block, InternalTransaction, Log, SmartContract, Transaction, Wei} + alias Explorer.Chain.{Address, Block, Hash, InternalTransaction, Log, SmartContract, Transaction, Wei} alias Explorer.Chain.Supply.ProofOfAuthority doctest Explorer.Chain @@ -404,6 +404,35 @@ defmodule Explorer.ChainTest do end end + describe "hashes_to_transactions/2" do + test "with transaction with block required without block returns nil" do + [%Transaction{hash: hash_with_block1}, %Transaction{hash: hash_with_block2}] = + 2 + |> insert_list(:transaction) + |> with_block() + + [%Transaction{hash: hash_without_index1}, %Transaction{hash: hash_without_index2}] = insert_list(2, :transaction) + + assert [%Transaction{hash: ^hash_with_block2}, %Transaction{hash: ^hash_with_block1}] = + Chain.hashes_to_transactions( + [hash_with_block1, hash_with_block2], + necessity_by_association: %{block: :required} + ) + + assert [] = + Chain.hashes_to_transactions( + [hash_without_index1, hash_without_index2], + necessity_by_association: %{block: :required} + ) + + assert [%Transaction{hash: ^hash_without_index1}, %Transaction{hash: ^hash_without_index2}] = + Chain.hashes_to_transactions( + [hash_without_index1, hash_without_index2], + necessity_by_association: %{block: :optional} + ) + end + end + describe "list_blocks/2" do test "without blocks" do assert [] = Chain.list_blocks() @@ -1545,6 +1574,12 @@ defmodule Explorer.ChainTest do assert_received {:chain_event, :logs, [%Log{}]} end + test "publishes transaction hashes data to subscribers on insert" do + Chain.subscribe_to_events(:transactions) + Chain.import_blocks(@import_data) + assert_received {:chain_event, :transactions, [%Hash{}]} + end + test "does not broadcast if broadcast option is false" do non_broadcast_data = Keyword.merge(@import_data, broadcast: false) diff --git a/apps/explorer_web/lib/explorer_web/notifier.ex b/apps/explorer_web/lib/explorer_web/notifier.ex index c5fd1e4aab..124b71c863 100644 --- a/apps/explorer_web/lib/explorer_web/notifier.ex +++ b/apps/explorer_web/lib/explorer_web/notifier.ex @@ -20,7 +20,11 @@ defmodule ExplorerWeb.Notifier do end def handle_event({:chain_event, :transactions, transaction_hashes}) do - Enum.each(transaction_hashes, &broadcast_transaction/1) + transaction_hashes + |> Chain.hashes_to_transactions( + necessity_by_association: %{block: :required, from_address: :optional, to_address: :optional} + ) + |> Enum.each(&broadcast_transaction/1) end defp broadcast_balance(%Address{hash: address_hash} = address) do @@ -30,26 +34,17 @@ defmodule ExplorerWeb.Notifier do }) end - defp broadcast_transaction(transaction_hash) do - case Chain.hash_to_transaction( - transaction_hash, - necessity_by_association: %{block: :required, from_address: :optional, to_address: :optional} - ) do - {:ok, transaction} -> - Endpoint.broadcast("addresses:#{transaction.from_address_hash}", "transaction", %{ - address: transaction.from_address, - transaction: transaction - }) - - if transaction.to_address_hash && transaction.to_address_hash != transaction.from_address_hash do - Endpoint.broadcast("addresses:#{transaction.to_address_hash}", "transaction", %{ - address: transaction.to_address, - transaction: transaction - }) - end + defp broadcast_transaction(transaction) do + Endpoint.broadcast("addresses:#{transaction.from_address_hash}", "transaction", %{ + address: transaction.from_address, + transaction: transaction + }) - {:error, _} -> - nil + if transaction.to_address_hash && transaction.to_address_hash != transaction.from_address_hash do + Endpoint.broadcast("addresses:#{transaction.to_address_hash}", "transaction", %{ + address: transaction.to_address, + transaction: transaction + }) end end end diff --git a/apps/explorer_web/test/explorer_web/features/viewing_addresses_test.exs b/apps/explorer_web/test/explorer_web/features/viewing_addresses_test.exs index 22f018dfc3..2dd91e6c16 100644 --- a/apps/explorer_web/test/explorer_web/features/viewing_addresses_test.exs +++ b/apps/explorer_web/test/explorer_web/features/viewing_addresses_test.exs @@ -233,18 +233,20 @@ defmodule ExplorerWeb.ViewingAddressesTest do end test "viewing new transactions via live update", %{addresses: addresses, session: session} do - transaction = - :transaction - |> insert(from_address: addresses.lincoln) + [transaction1, transaction2] = + 2 + |> insert_list(:transaction, from_address: addresses.lincoln) |> with_block() session |> AddressPage.visit_page(addresses.lincoln) |> assert_has(AddressPage.balance()) - Notifier.handle_event({:chain_event, :transactions, [transaction.hash]}) + Notifier.handle_event({:chain_event, :transactions, [transaction1.hash, transaction2.hash]}) - assert_has(session, AddressPage.transaction(transaction)) + session + |> assert_has(AddressPage.transaction(transaction1)) + |> assert_has(AddressPage.transaction(transaction2)) end test "transaction count live updates", %{addresses: addresses, session: session} do