Skip to content

Commit

Permalink
CXXCBC-287 Public txn api should match the rest of cxx api (#371)
Browse files Browse the repository at this point in the history 
* CXXCBC-287 Public txn api should match the rest of cxx api

Specifically, returning an error, result pair from the transaction
operations themselves, and the transactions.run as well.   Or the async
equivalent.

* linter...

* gcc warning

* windows compiler warning

* another windows warning

* fix constness of statement in query(), remove unused variable

---------

Co-authored-by: Sergey Avseyev <[email protected]>
  • Loading branch information
@davidkelly @avsej
davidkelly and avsej authored Feb 19, 2023
1 parent 673974c commit ec53d75
Show file tree
Hide file tree
Showing 20 changed files with 549 additions and 414 deletions.
26 changes: 14 additions & 12 deletions core/impl/query.cxx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ build_query_request(query_options::built options)
return request;
}

couchbase::transactions::transaction_query_result
std::pair<couchbase::transaction_op_error_context, couchbase::transactions::transaction_query_result>
build_transaction_query_result(operations::query_response resp, std::error_code txn_ec /*defaults to 0*/)
{
if (resp.ctx.ec) {
Expand All @@ -204,17 +204,19 @@ build_transaction_query_result(operations::query_response resp, std::error_code
txn_ec = errc::transaction_op::not_set;
}
}
return { query_meta_data{
std::move(resp.meta.request_id),
std::move(resp.meta.client_context_id),
map_status(resp.meta.status),
map_warnings(resp),
map_metrics(resp),
map_signature(resp),
map_profile(resp),
},
map_rows(resp),
{ txn_ec, build_context(resp) } };
return {
{ txn_ec, build_context(resp) },
{ query_meta_data{
std::move(resp.meta.request_id),
std::move(resp.meta.client_context_id),
map_status(resp.meta.status),
map_warnings(resp),
map_metrics(resp),
map_signature(resp),
map_profile(resp),
},
map_rows(resp) },
};
}

core::operations::query_request
Expand Down
2 changes: 1 addition & 1 deletion core/transactions.hxx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ class transactions : public couchbase::transactions::transactions

couchbase::transactions::transaction_result run(const couchbase::transactions::transaction_options& config, logic&& code);

couchbase::transactions::transaction_result run(
std::pair<couchbase::transaction_error_context, couchbase::transactions::transaction_result> run(
::couchbase::transactions::txn_logic&& code,
const couchbase::transactions::transaction_options& cfg = couchbase::transactions::transaction_options()) override;

Expand Down
26 changes: 13 additions & 13 deletions core/transactions/attempt_context_impl.cxx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -847,15 +847,15 @@ attempt_context_impl::handle_query_error(const core::operations::query_response&
if (!resp.ctx.ec && !resp.meta.errors) {
return {};
}
auto tx_resp = couchbase::core::impl::build_transaction_query_result(resp);
auto [tx_err, query_result] = couchbase::core::impl::build_transaction_query_result(resp);
// TODO: look at ambiguous and unambiguous timeout errors vs the codes, etc...
CB_ATTEMPT_CTX_LOG_TRACE(
this, "handling query error {}, {} errors in meta_data", resp.ctx.ec.message(), resp.meta.errors ? "has" : "no");
if (resp.ctx.ec == couchbase::errc::common::ambiguous_timeout || resp.ctx.ec == couchbase::errc::common::unambiguous_timeout) {
return std::make_exception_ptr(query_attempt_expired(tx_resp.ctx()));
return std::make_exception_ptr(query_attempt_expired(tx_err));
}
if (resp.ctx.ec == couchbase::errc::common::parsing_failure) {
return std::make_exception_ptr(query_parsing_failure(tx_resp.ctx()));
return std::make_exception_ptr(query_parsing_failure(tx_err));
}
if (!resp.meta.errors) {
// can't choose an error, map using the ec...
Expand Down Expand Up @@ -883,16 +883,16 @@ attempt_context_impl::handle_query_error(const core::operations::query_response&
transaction_operation_failed(FAIL_OTHER, "This couchbase server requires all queries use a scope.")
.cause(FEATURE_NOT_AVAILABLE_EXCEPTION));
case 17004:
return std::make_exception_ptr(query_attempt_not_found(tx_resp.ctx()));
return std::make_exception_ptr(query_attempt_not_found(tx_err));
case 1080:
case 17010:
return std::make_exception_ptr(transaction_operation_failed(FAIL_EXPIRY, "transaction expired").expired());
case 17012:
return std::make_exception_ptr(document_exists(tx_resp.ctx()));
return std::make_exception_ptr(document_exists(tx_err));
case 17014:
return std::make_exception_ptr(document_not_found(tx_resp.ctx()));
return std::make_exception_ptr(document_not_found(tx_err));
case 17015:
return std::make_exception_ptr(query_cas_mismatch(tx_resp.ctx()));
return std::make_exception_ptr(query_cas_mismatch(tx_err));
}

// For these errors, we will create a transaction_operation_failed from the info in it.
Expand Down Expand Up @@ -922,7 +922,7 @@ attempt_context_impl::handle_query_error(const core::operations::query_response&
}
}

return { std::make_exception_ptr(op_exception(tx_resp.ctx())) };
return { std::make_exception_ptr(op_exception(tx_err)) };
}

void
Expand Down Expand Up @@ -1071,22 +1071,22 @@ attempt_context_impl::do_core_query(const std::string& statement,
return f.get();
}

couchbase::transactions::transaction_query_result_ptr
std::pair<couchbase::transaction_op_error_context, couchbase::transactions::transaction_query_result>
attempt_context_impl::do_public_query(const std::string& statement,
const couchbase::transactions::transaction_query_options& opts,
std::optional<std::string> query_context)
{
try {
auto result = do_core_query(statement, opts, query_context);
return std::make_shared<couchbase::transactions::transaction_query_result>(core::impl::build_transaction_query_result(result));
return core::impl::build_transaction_query_result(result);
} catch (const transaction_operation_failed& e) {
return std::make_shared<couchbase::transactions::transaction_query_result>(e.get_error_ctx());
return { e.get_error_ctx(), {} };
} catch (const op_exception& qe) {
return std::make_shared<couchbase::transactions::transaction_query_result>(qe.ctx());
return { qe.ctx(), {} };
} catch (...) {
// should not be necessary, but just in case...
transaction_op_error_context ctx(couchbase::errc::transaction_op::unknown);
return std::make_shared<couchbase::transactions::transaction_query_result>(ctx);
return { ctx, {} };
}
}

Expand Down
92 changes: 51 additions & 41 deletions core/transactions/attempt_context_impl.hxx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
namespace couchbase::core::impl
{

couchbase::transactions::transaction_query_result
std::pair<couchbase::transaction_op_error_context, couchbase::transactions::transaction_query_result>
build_transaction_query_result(operations::query_response resp, std::error_code ec = {});

core::operations::query_request
Expand Down Expand Up @@ -87,14 +87,14 @@ class attempt_context_impl
// transaction_context needs access to the two functions below
friend class transaction_context;

couchbase::transactions::transaction_get_result_ptr insert_raw(const couchbase::collection& coll,
const std::string& id,
std::vector<std::byte> content) override
std::pair<couchbase::transaction_op_error_context, couchbase::transactions::transaction_get_result>
insert_raw(const couchbase::collection& coll, const std::string& id, std::vector<std::byte> content) override
{
return wrap_call_for_public_api([this, coll, &id, &content]() -> transaction_get_result {
return insert_raw({ coll.bucket_name(), coll.scope_name(), coll.name(), id }, content);
});
}

transaction_get_result insert_raw(const core::document_id& id, const std::vector<std::byte>& content) override;
void insert_raw(const collection& coll,
std::string id,
Expand All @@ -110,17 +110,20 @@ class attempt_context_impl
void insert_raw(const core::document_id& id, const std::vector<std::byte>& content, Callback&& cb) override;

transaction_get_result replace_raw(const transaction_get_result& document, const std::vector<std::byte>& content) override;
couchbase::transactions::transaction_get_result_ptr replace_raw(const couchbase::transactions::transaction_get_result_ptr doc,
std::vector<std::byte> content) override

std::pair<couchbase::transaction_op_error_context, couchbase::transactions::transaction_get_result> replace_raw(
const couchbase::transactions::transaction_get_result& doc,
std::vector<std::byte> content) override
{
return wrap_call_for_public_api(
[this, doc, &content]() -> transaction_get_result { return replace_raw(dynamic_cast<transaction_get_result&>(*doc), content); });
[this, doc, &content]() -> transaction_get_result { return replace_raw(transaction_get_result(doc), content); });
}
void replace_raw(couchbase::transactions::transaction_get_result_ptr doc,

void replace_raw(couchbase::transactions::transaction_get_result doc,
std::vector<std::byte> content,
couchbase::transactions::async_result_handler&& handler) override
{
replace_raw(dynamic_cast<transaction_get_result&>(*doc),
replace_raw(core::transactions::transaction_get_result(doc),
content,
[this, handler = std::move(handler)](std::exception_ptr err, std::optional<transaction_get_result> res) mutable {
wrap_callback_for_async_public_api(err, res, std::move(handler));
Expand Down Expand Up @@ -323,23 +326,28 @@ class attempt_context_impl
~attempt_context_impl() override;

transaction_get_result get(const core::document_id& id) override;
couchbase::transactions::transaction_get_result_ptr get(const couchbase::collection& coll, const std::string& id) override
std::pair<couchbase::transaction_op_error_context, couchbase::transactions::transaction_get_result> get(
const couchbase::collection& coll,
const std::string& id) override
{
return wrap_call_for_public_api([this, coll, id]() mutable -> transaction_get_result {
auto [ctx, res] = wrap_call_for_public_api([this, coll, id]() mutable -> transaction_get_result {
auto ret = get_optional({ coll.bucket_name(), coll.scope_name(), coll.name(), id });
if (ret) {
return *ret;
return ret.value();
}
return { transaction_op_error_context{ errc::transaction_op::document_not_found_exception } };
return {};
});
if (!ctx.ec() && res.cas().empty()) {
return { transaction_op_error_context{ errc::transaction_op::document_not_found_exception }, res };
}
return { ctx, res };
}
void get(const couchbase::collection& coll, std::string id, couchbase::transactions::async_result_handler&& handler) override
{
get_optional({ coll.bucket_name(), coll.scope_name(), coll.name(), std::move(id) },
[this, handler = std::move(handler)](std::exception_ptr err, std::optional<transaction_get_result> res) mutable {
if (!res) {
return handler(std::make_shared<transaction_get_result>(
transaction_op_error_context{ errc::transaction_op::document_not_found_exception }));
return handler(transaction_op_error_context{ errc::transaction_op::document_not_found_exception }, {});
}
return wrap_callback_for_async_public_api(err, res, std::move(handler));
});
Expand All @@ -350,14 +358,14 @@ class attempt_context_impl
void get_optional(const core::document_id& id, Callback&& cb) override;

void remove(const transaction_get_result& document) override;
couchbase::transaction_op_error_context remove(couchbase::transactions::transaction_get_result_ptr doc) override
couchbase::transaction_op_error_context remove(const couchbase::transactions::transaction_get_result& doc) override
{
return wrap_void_call_for_public_api([this, doc]() { remove(dynamic_cast<transaction_get_result&>(*doc)); });
return wrap_void_call_for_public_api([this, doc]() { remove(transaction_get_result(doc)); });
}
void remove(const transaction_get_result& document, VoidCallback&& cb) override;
void remove(couchbase::transactions::transaction_get_result_ptr doc, couchbase::transactions::async_err_handler&& handler) override
void remove(couchbase::transactions::transaction_get_result doc, couchbase::transactions::async_err_handler&& handler) override
{
remove(dynamic_cast<transaction_get_result&>(*doc), [this, handler = std::move(handler)](std::exception_ptr e) mutable {
remove(transaction_get_result(doc), [this, handler = std::move(handler)](std::exception_ptr e) mutable {
wrap_err_callback_for_async_api(e, std::move(handler));
});
};
Expand All @@ -366,9 +374,10 @@ class attempt_context_impl
const couchbase::transactions::transaction_query_options& options,
std::optional<std::string> query_context) override;

couchbase::transactions::transaction_query_result_ptr do_public_query(const std::string& statement,
const couchbase::transactions::transaction_query_options& opts,
std::optional<std::string> query_context) override;
std::pair<couchbase::transaction_op_error_context, couchbase::transactions::transaction_query_result> do_public_query(
const std::string& statement,
const couchbase::transactions::transaction_query_options& opts,
std::optional<std::string> query_context) override;

void query(const std::string& statement,
const couchbase::transactions::transaction_query_options& options,
Expand All @@ -389,17 +398,16 @@ class attempt_context_impl
try {
std::rethrow_exception(err);
} catch (const transaction_operation_failed& e) {
return handler(std::make_shared<couchbase::transactions::transaction_query_result>(e.get_error_ctx()));
return handler(e.get_error_ctx(), {});
} catch (const op_exception& ex) {
return handler(std::make_shared<couchbase::transactions::transaction_query_result>(ex.ctx()));
return handler(ex.ctx(), {});
} catch (...) {
// just in case...
return handler(std::make_shared<couchbase::transactions::transaction_query_result>(
transaction_op_error_context(couchbase::errc::transaction_op::unknown)));
return handler(transaction_op_error_context(couchbase::errc::transaction_op::unknown), {});
}
}
handler(
std::make_shared<couchbase::transactions::transaction_query_result>(core::impl::build_transaction_query_result(*resp)));
auto [ctx, res] = core::impl::build_transaction_query_result(*resp);
handler(ctx, res);
});
}

Expand Down Expand Up @@ -527,17 +535,18 @@ class attempt_context_impl
error_class ec,
const std::string& message);

couchbase::transactions::transaction_get_result_ptr wrap_call_for_public_api(std::function<transaction_get_result()>&& handler)
std::pair<couchbase::transaction_op_error_context, couchbase::transactions::transaction_get_result> wrap_call_for_public_api(
std::function<transaction_get_result()>&& handler)
{
try {
return std::make_shared<transaction_get_result>(handler());
return { {}, handler().to_public_result() };
} catch (const transaction_operation_failed& e) {
return std::make_shared<transaction_get_result>(e.get_error_ctx());
return { e.get_error_ctx(), {} };
} catch (const op_exception& ex) {
return std::make_shared<transaction_get_result>(ex.ctx());
return { ex.ctx(), {} };
} catch (...) {
// the handler should catch everything else, but just in case...
return std::make_shared<transaction_get_result>(transaction_op_error_context(errc::transaction_op::unknown));
return { transaction_op_error_context(errc::transaction_op::unknown), {} };
}
}

Expand All @@ -554,25 +563,26 @@ class attempt_context_impl
}
}

void wrap_callback_for_async_public_api(std::exception_ptr err,
std::optional<transaction_get_result> res,
std::function<void(couchbase::transactions::transaction_get_result_ptr)>&& cb)
void wrap_callback_for_async_public_api(
std::exception_ptr err,
std::optional<transaction_get_result> res,
std::function<void(couchbase::transaction_op_error_context, couchbase::transactions::transaction_get_result)>&& cb)
{
if (res) {
return cb(std::make_shared<transaction_get_result>(*res));
return cb({}, res->to_public_result());
}
if (err) {
try {
std::rethrow_exception(err);
} catch (const op_exception& e) {
return cb(std::make_shared<transaction_get_result>(e.ctx()));
return cb(e.ctx(), {});
} catch (const transaction_operation_failed& e) {
return cb(std::make_shared<transaction_get_result>(e.get_error_ctx()));
return cb(e.get_error_ctx(), {});
} catch (...) {
return cb(std::make_shared<transaction_get_result>(transaction_op_error_context(errc::transaction_op::unknown)));
return cb(transaction_op_error_context(errc::transaction_op::unknown), {});
}
}
return cb(std::make_shared<transaction_get_result>(transaction_op_error_context(errc::transaction_op::unknown)));
return cb(transaction_op_error_context(errc::transaction_op::unknown), {});
}

void wrap_err_callback_for_async_api(std::exception_ptr err, std::function<void(couchbase::transaction_op_error_context)>&& cb)
Expand Down
1 change: 0 additions & 1 deletion core/transactions/exceptions.cxx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,6 @@ transaction_exception::transaction_exception(const std::runtime_error& cause, co
if (nullptr != txn_op) {
cause_ = txn_op->cause();
}
result_.ctx = error_context();
}

errc::transaction_op
Expand Down
4 changes: 2 additions & 2 deletions core/transactions/exceptions.hxx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,9 @@ class transaction_exception : public std::runtime_error
*
* @returns Internal state of transaction.
*/
::couchbase::transactions::transaction_result get_transaction_result() const
std::pair<couchbase::transaction_error_context, couchbase::transactions::transaction_result> get_transaction_result() const
{
return { result_.transaction_id, result_.unstaging_complete, error_context() };
return { error_context(), { result_.transaction_id, result_.unstaging_complete } };
}

/**
Expand Down
2 changes: 1 addition & 1 deletion core/transactions/internal/transaction_context.hxx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ class transaction_context

[[nodiscard]] ::couchbase::transactions::transaction_result get_transaction_result() const
{
return couchbase::transactions::transaction_result{ transaction_id(), current_attempt().state == attempt_state::COMPLETED, {} };
return couchbase::transactions::transaction_result{ transaction_id(), current_attempt().state == attempt_state::COMPLETED };
}
void new_attempt_context()
{
Expand Down
Loading

0 comments on commit ec53d75

Please sign in to comment.