Skip to content
This repository has been archived by the owner on Apr 17, 2019. It is now read-only.

GetAccountTransactions Pagination #1903

Merged
Merged
Show file tree
Hide file tree
Changes from 17 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
11 changes: 6 additions & 5 deletions irohad/ametsuchi/impl/postgres_block_index.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,12 @@ namespace {
// make index tx hash -> block where hash is stored
std::string makeHashIndex(
const shared_model::interface::types::HashType &hash,
shared_model::interface::types::HeightType height) {
shared_model::interface::types::HeightType height,
size_t index) {
boost::format base(
"INSERT INTO height_by_hash(hash, height) VALUES ('%s', "
"'%s');");
return (base % hash.hex() % height).str();
"INSERT INTO position_by_hash(hash, height, index) VALUES ('%s', "
"'%s', '%s');");
return (base % hash.hex() % height % index).str();
}

// make index account_id:height -> list of tx indexes
Expand Down Expand Up @@ -121,7 +122,7 @@ namespace iroha {
query += makeAccountHeightIndex(creator_id, height);
query += makeAccountAssetIndex(
creator_id, height, index, tx.value().commands());
query += makeHashIndex(tx.value().hash(), height);
query += makeHashIndex(tx.value().hash(), height, index);
query += makeCreatorHeightIndex(creator_id, height, index);
return query;
});
Expand Down
2 changes: 1 addition & 1 deletion irohad/ametsuchi/impl/postgres_block_query.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ namespace iroha {
boost::optional<std::string> block_str;
auto hash_str = hash.hex();

sql_ << "SELECT height FROM height_by_hash WHERE hash = :hash",
sql_ << "SELECT height FROM position_by_hash WHERE hash = :hash",
soci::into(block_str), soci::use(hash_str);
if (block_str) {
blockId = std::stoull(block_str.get());
Expand Down
126 changes: 102 additions & 24 deletions irohad/ametsuchi/impl/postgres_query_executor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
#include "interfaces/queries/get_signatories.hpp"
#include "interfaces/queries/get_transactions.hpp"
#include "interfaces/queries/query.hpp"
#include "interfaces/queries/tx_pagination_meta.hpp"

using namespace shared_model::interface::permissions;

Expand Down Expand Up @@ -449,41 +450,100 @@ namespace iroha {

QueryExecutorResult PostgresQueryExecutorVisitor::operator()(
const shared_model::interface::GetAccountTransactions &q) {
using QueryTuple =
QueryType<shared_model::interface::types::HeightType, uint64_t>;
using QueryTuple = QueryType<shared_model::interface::types::HeightType,
uint64_t,
uint64_t>;
using PermissionTuple = boost::tuple<int>;

auto cmd = (boost::format(R"(WITH has_perms AS (%s),
auto &pagination_info = q.paginationMeta();
Copy link
Contributor

Choose a reason for hiding this comment

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

If paginationMeta() returns constref, it should be const auto &pagination_info

auto first_hash = pagination_info.firstTxHash();
// retrieve one extra transaction to populate next_hash
auto query_size = pagination_info.pageSize() + 1u;

auto base = boost::format(R"(WITH has_perms AS (%s),
first_hash AS (%s),
previous_txes AS (
SELECT position_by_hash.height, position_by_hash.index
FROM position_by_hash JOIN first_hash
ON position_by_hash.height > first_hash.height
OR (position_by_hash.height = first_hash.height AND
position_by_hash.index >= first_hash.index)
),
my_txs AS (
SELECT DISTINCT height, index
FROM index_by_creator_height
WHERE creator_id = :account_id
ORDER BY height, index ASC
),
total_size AS (
SELECT COUNT(*) FROM my_txs
),
t AS (
SELECT DISTINCT has.height, index
FROM height_by_account_set AS has
JOIN index_by_creator_height AS ich ON has.height = ich.height
AND has.account_id = ich.creator_id
WHERE account_id = :account_id
ORDER BY has.height, index ASC
SELECT my_txs.height, my_txs.index
FROM my_txs
JOIN previous_txes ON my_txs.height = previous_txes.height
AND my_txs.index = previous_txes.index
LIMIT :page_size
)
SELECT height, index, perm FROM t
SELECT height, index, count, perm FROM t
RIGHT OUTER JOIN has_perms ON TRUE
)")
% hasQueryPermission(creator_id_,
q.accountId(),
Role::kGetMyAccTxs,
Role::kGetAllAccTxs,
Role::kGetDomainAccTxs))
.str();
JOIN total_size ON TRUE
)");

// select tx with specified hash
auto first_by_hash = R"(SELECT height, index FROM position_by_hash
WHERE hash = :hash LIMIT 1)";

// select first ever tx
auto first_tx = R"(SELECT height, index FROM position_by_hash
ORDER BY height, index ASC LIMIT 1)";

auto cmd = boost::format(base
% hasQueryPermission(creator_id_,
q.accountId(),
Role::kGetMyAccTxs,
Role::kGetAllAccTxs,
Role::kGetDomainAccTxs));
if (first_hash) {
cmd = base % first_by_hash;
} else {
cmd = base % first_tx;
}

auto query = cmd.str();

return executeQuery<QueryTuple, PermissionTuple>(
[&] { return (sql_.prepare << cmd, soci::use(q.accountId())); },
[&] {
if (first_hash) {
return (sql_.prepare << query,
soci::use(first_hash->hex()),
soci::use(q.accountId()),
soci::use(query_size));
} else {
return (sql_.prepare << query,
soci::use(q.accountId()),
soci::use(query_size));
}
},
[&](auto range, auto &) {
uint64_t total_size = 0;
if (not boost::empty(range)) {
total_size = boost::get<2>(*range.begin());
}
std::map<uint64_t, std::vector<uint64_t>> index;
// unpack results to get map from block height to index of tx in
// a block
boost::for_each(range, [&index](auto t) {
apply(t, [&index](auto &height, auto &idx) {
index[height].push_back(idx);
});
apply(
t,
[&index](auto &height, auto &idx, auto &count) {
index[height].push_back(idx);
});
});

std::vector<std::unique_ptr<shared_model::interface::Transaction>>
response_txs;
// get transactions corresponding to indexes
for (auto &block : index) {
auto txs = this->getTransactionsFromBlock(
block.first,
Expand All @@ -492,9 +552,27 @@ namespace iroha {
std::move(
txs.begin(), txs.end(), std::back_inserter(response_txs));
}
// If 0 transactions are returned, we assume that hash is invalid.
// Since query with valid hash is guaranteed to return at least one
// transaction
if (first_hash and response_txs.empty()) {
auto error = (boost::format("invalid pagination hash: %s")
% first_hash->hex())
.str();
return this->logAndReturnErrorResponse(
QueryErrorType::kStatefulFailed, error);
}

return query_response_factory_->createTransactionsResponse(
std::move(response_txs), query_hash_);
// next transaction exists
Copy link
Contributor

Choose a reason for hiding this comment

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

Not very clear comment, please expand it a little

if (response_txs.size() == query_size) {
auto next_hash = response_txs.back()->hash();
response_txs.pop_back();
return query_response_factory_->createTransactionsPageResponse(
std::move(response_txs), next_hash, total_size, query_hash_);
}

return query_response_factory_->createTransactionsPageResponse(
std::move(response_txs), total_size, query_hash_);
},
notEnoughPermissionsResponse(perm_converter_,
Role::kGetMyAccTxs,
Expand All @@ -518,7 +596,7 @@ namespace iroha {
auto cmd = (boost::format(R"(WITH has_my_perm AS (%s),
has_all_perm AS (%s),
t AS (
SELECT height, hash FROM height_by_hash WHERE hash IN (%s)
SELECT height, hash FROM position_by_hash WHERE hash IN (%s)
)
SELECT height, hash, has_my_perm.perm, has_all_perm.perm FROM t
RIGHT OUTER JOIN has_my_perm ON TRUE
Expand Down
7 changes: 4 additions & 3 deletions irohad/ametsuchi/impl/storage_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -534,7 +534,7 @@ DELETE FROM domain;
DELETE FROM signatory;
DELETE FROM peer;
DELETE FROM role;
DELETE FROM height_by_hash;
DELETE FROM position_by_hash;
DELETE FROM height_by_account_set;
DELETE FROM index_by_creator_height;
DELETE FROM index_by_id_height_asset;
Expand Down Expand Up @@ -606,9 +606,10 @@ CREATE TABLE IF NOT EXISTS account_has_grantable_permissions (
+ R"() NOT NULL,
PRIMARY KEY (permittee_account_id, account_id)
);
CREATE TABLE IF NOT EXISTS height_by_hash (
Copy link
Contributor

Choose a reason for hiding this comment

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

We still have that table in ametsuchi fixture. Replace it there as well

CREATE TABLE IF NOT EXISTS position_by_hash (
hash varchar,
height text
height text,
index text
);
CREATE TABLE IF NOT EXISTS height_by_account_set (
account_id text,
Expand Down
7 changes: 7 additions & 0 deletions shared_model/utils/query_error_response_visitor.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,13 @@ namespace shared_model {
return false;
}
};

template <typename Error, typename QueryVariant>
bool checkForQueryError(QueryVariant &&query) {
return boost::apply_visitor(
shared_model::interface::QueryErrorResponseChecker<Error>(),
std::forward<QueryVariant>(query));
}
} // namespace interface
} // namespace shared_model

Expand Down
5 changes: 3 additions & 2 deletions test/integration/acceptance/query_permission_test_txs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

#include "integration/acceptance/query_permission_test_txs.hpp"

#include "interfaces/query_responses/transactions_response.hpp"
#include "interfaces/query_responses/transactions_page_response.hpp"

using namespace common_constants;

Expand Down Expand Up @@ -59,7 +59,8 @@ QueryPermissionTxs::getGeneralResponseChecker() {
return [this](const proto::QueryResponse &response) {
ASSERT_NO_THROW({
const auto &resp =
boost::get<const interface::TransactionsResponse &>(response.get());
boost::get<const interface::TransactionsPageResponse &>(
response.get());

const auto &transactions = resp.transactions();
ASSERT_EQ(boost::size(transactions), tx_hashes_.size());
Expand Down
Loading