[
https://issues.apache.org/jira/browse/PROTON-1442?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17903153#comment-17903153
]
ASF GitHub Bot commented on PROTON-1442:
----------------------------------------
astitcher commented on code in PR #437:
URL: https://github.com/apache/qpid-proton/pull/437#discussion_r1870227065
##########
cpp/include/proton/container.hpp:
##########
@@ -312,6 +312,7 @@ class PN_CPP_CLASS_EXTERN container {
/// Cancel task for the given work_handle.
PN_CPP_EXTERN void cancel(work_handle);
+ PN_CPP_EXTERN transaction declare_transaction(proton::connection conn,
proton::transaction_handler &handler, bool settle_before_discharge = false);
Review Comment:
Since you take a connection as the first parameter here it probably makes
more sense to move this entire method into the proton::connection class.
Actually maybe this should be a method on session to match the cms api, in
either case you can find the connection easily - as a result it would make
sense to hold the transaction as state in the connection or session and only
allow a single transaction at that level.
In this case it will be easy enough to find the transaction as you can
always navigate upwards from the delivery to find its session and on to the
connection.
##########
cpp/include/proton/transfer.hpp:
##########
@@ -77,20 +94,28 @@ class transfer : public internal::object<pn_delivery_t> {
/// Return true if the transfer has been settled.
PN_CPP_EXTERN bool settled() const;
+ // Set transaction
+ PN_CPP_EXTERN void transaction(transaction t);
+
+ PN_CPP_EXTERN class transaction transaction() const;
Review Comment:
I'm pretty sure that transaction should not be a value object - it has a
lifecycle (starting with declare and finishing with commit/abort). Any use of a
transaction can only be within the lifecycle of the transaction and should be
via a reference to the transaction.
So these signatures should be:
```
void transaction(transaction& t);
transaction& transaction() const;
```
And the transfer should not be holding the transaction at all it should just
find the transaction from either its session or connection depending on where
we end up storing the transaction in the end.
##########
cpp/examples/tx_recv.cpp:
##########
@@ -0,0 +1,130 @@
+/*
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+
+#include "options.hpp"
+
+#include <proton/connection.hpp>
+#include <proton/container.hpp>
+#include <proton/message.hpp>
+#include <proton/message_id.hpp>
+#include <proton/messaging_handler.hpp>
+#include <proton/types.hpp>
+#include <proton/transaction.hpp>
+
+#include <iostream>
+#include <map>
+#include <string>
+
+#include <chrono>
+#include <thread>
+
+class tx_recv : public proton::messaging_handler, proton::transaction_handler {
+ private:
+ proton::receiver receiver;
+ std::string url;
+ int expected;
+ int batch_size;
+ int current_batch = 0;
+ int committed = 0;
+
+ proton::container *container;
+ proton::transaction transaction;
+ proton::connection connection;
+ public:
+ tx_recv(const std::string &s, int c, int b):
+ url(s), expected(c), batch_size(b) {}
+
+ void on_container_start(proton::container &c) override {
+ container = &c;
+ receiver = c.open_receiver(url);
+ connection = receiver.connection();
+ std::cout << " [on_container_start] declare_txn started..." <<
std::endl;
+ c.declare_transaction(connection, *this);
+ std::cout << " [on_container_start] completed!!" << &transaction
+ << std::endl;
+ }
+
+ void on_transaction_declare_failed(proton::transaction) {}
+ void on_transaction_commit_failed(proton::transaction) {
+ std::cout << "Transaction Commit Failed" << std::endl;
+ connection.close();
Review Comment:
we need to bwe able to get from the transaction to the session/connection
it's associated with so we don't need to store the connection away we should
just get it from the transaction in the handler.
##########
cpp/include/proton/transfer.hpp:
##########
@@ -33,9 +33,26 @@
/// @copybrief proton::transfer
struct pn_delivery_t;
+struct pn_disposition_t;
namespace proton {
+class disposition : public internal::object<pn_disposition_t> {
+ /// @cond INTERNAL
+ disposition(pn_disposition_t *d) : internal::object<pn_disposition_t>(d) {}
+ /// @endcond
+
+ public:
+ /// Create an empty disposition.
+ disposition() : internal::object<pn_disposition_t>(0) {}
+
+ proton::value data() const;
+
+ /// @cond INTERNAL
+ friend class internal::factory<disposition>;
+ /// @endcond
+};
+
Review Comment:
Disposition is not a user level concept in the C++ API. This class and
everything related to it should be purely internal code only used/accessible by
the transaction implementation code.
##########
cpp/include/proton/transaction.hpp:
##########
@@ -0,0 +1,124 @@
+#ifndef PROTON_TRANSACTION_HPP
+#define PROTON_TRANSACTION_HPP
+
+
+/*
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+
+
+#include "./fwd.hpp"
+#include "./internal/export.hpp"
+#include "./sender.hpp"
+#include "./tracker.hpp"
+#include "./container.hpp"
+
+/// @file
+/// @copybrief proton::transaction
+
+namespace proton {
+
+class transaction_handler;
+
+// TODO: This should not be accessible to users.
+class transaction_impl {
+ public:
+ proton::sender txn_ctrl;
+ proton::transaction_handler *handler = nullptr;
+ proton::binary id;
+ proton::tracker _declare;
+ proton::tracker _discharge;
+ bool failed = false;
+ std::vector<proton::tracker> pending;
+
+ void commit();
+ void abort();
+ void declare();
+ proton::tracker send(proton::sender s, proton::message msg);
+
+ void discharge(bool failed);
+ void release_pending();
+ void accept(delivery &d);
+ void update(tracker &d, uint64_t state);
+ void set_id(binary _id);
+
+ proton::tracker send_ctrl(proton::symbol descriptor, proton::value _value);
+ void handle_outcome(proton::tracker t);
+ transaction_impl(proton::sender &_txn_ctrl,
+ proton::transaction_handler &_handler,
+ bool _settle_before_discharge);
+
+ // delete copy and assignment operator to ensure no copy of this object is
+ // every made transaction_impl(const transaction_impl&) = delete;
+ // transaction_impl& operator=(const transaction_impl&) = delete;
+};
+
+class
+PN_CPP_CLASS_EXTERN transaction {
+ private:
+ // PN_CPP_EXTERN transaction(proton::sender& _txn_ctrl,
+ // proton::transaction_handler& _handler, bool _settle_before_discharge);
+
+ static transaction mk_transaction_impl(sender &s, transaction_handler &h,
+ bool f);
+ PN_CPP_EXTERN transaction(transaction_impl *impl);
+ transaction_impl *_impl;
+
+ public:
+ // TODO:
+ // PN_CPP_EXTERN transaction(transaction &o);
+ PN_CPP_EXTERN transaction();
+ PN_CPP_EXTERN ~transaction();
+ PN_CPP_EXTERN bool is_empty();
+ PN_CPP_EXTERN void commit();
+ PN_CPP_EXTERN void abort();
+ PN_CPP_EXTERN void declare();
+ PN_CPP_EXTERN void handle_outcome(proton::tracker);
+ PN_CPP_EXTERN proton::tracker send(proton::sender s, proton::message msg);
+ PN_CPP_EXTERN void accept(delivery &t);
+
+ friend class transaction_impl;
+ friend class container::impl;
+};
+
+class
+PN_CPP_CLASS_EXTERN transaction_handler {
+ public:
+ PN_CPP_EXTERN virtual ~transaction_handler();
+
+ /// Called when a local transaction is declared.
+ PN_CPP_EXTERN virtual void on_transaction_declared(transaction);
+
+ /// Called when a local transaction is discharged successfully.
+ PN_CPP_EXTERN virtual void on_transaction_committed(transaction);
+
+ /// Called when a local transaction is discharged unsuccessfully (aborted).
+ PN_CPP_EXTERN virtual void on_transaction_aborted(transaction);
+
+ /// Called when a local transaction declare fails.
+ PN_CPP_EXTERN virtual void on_transaction_declare_failed(transaction);
+
+ /// Called when the commit of a local transaction fails.
+ PN_CPP_EXTERN virtual void on_transaction_commit_failed(transaction);
+};
+
+} // namespace proton
+
Review Comment:
I think this should be in a different header file like message and
messaging_handler
##########
cpp/src/tracker.cpp:
##########
@@ -26,12 +26,13 @@
#include "proton_bits.hpp"
#include "types_internal.hpp"
#include "proton/binary.hpp"
+#include "proton/transaction.hpp"
Review Comment:
Not needed: no actual change to the file
##########
cpp/include/proton/tracker.hpp:
##########
@@ -25,6 +25,7 @@
#include "./binary.hpp"
#include "./internal/export.hpp"
#include "./transfer.hpp"
+#include "./messaging_handler.hpp"
Review Comment:
This doesn't seem needed, presumably tracker now needs to be able to
delegate to transaction_handler but I don't think it should need to delegate to
a message_handler.
##########
cpp/include/proton/transfer.hpp:
##########
@@ -77,20 +94,28 @@ class transfer : public internal::object<pn_delivery_t> {
/// Return true if the transfer has been settled.
PN_CPP_EXTERN bool settled() const;
+ // Set transaction
+ PN_CPP_EXTERN void transaction(transaction t);
+
+ PN_CPP_EXTERN class transaction transaction() const;
+
/// Set user data on this transfer.
PN_CPP_EXTERN void user_data(void* user_data) const;
/// Get user data from this transfer.
PN_CPP_EXTERN void* user_data() const;
+ PN_CPP_EXTERN disposition remote();
+ PN_CPP_EXTERN disposition local();
+
Review Comment:
These are not part of the user API (and shouldn't be either at the moment)
so can't be in the user header file.
##########
cpp/include/proton/target_options.hpp:
##########
@@ -88,6 +88,8 @@ class target_options {
/// **Unsettled API** Set the dynamic node properties.
PN_CPP_EXTERN target_options& dynamic_properties(const
target::dynamic_property_map&);
+ PN_CPP_EXTERN target_options& type(const int);
+
Review Comment:
This should not be needed. Introduce a new coordinator class that is peer to
sender and receiver
##########
cpp/src/container.cpp:
##########
@@ -45,6 +46,10 @@ returned<connection> container::connect(const std::string
&url) {
return connect(url, connection_options());
}
+transaction container::declare_transaction(proton::connection conn,
proton::transaction_handler &handler, bool settle_before_discharge) {
+ return impl_->declare_transaction(conn, handler, settle_before_discharge);
+}
+
Review Comment:
As above this code should be a part of the connection API
##########
cpp/src/container.cpp:
##########
@@ -28,6 +28,7 @@
#include "proton/uuid.hpp"
#include "proactor_container_impl.hpp"
+#include <vector>
Review Comment:
Why? The code added below doesn't use vector.
##########
cpp/src/tracker.cpp:
##########
@@ -26,12 +26,13 @@
#include "proton_bits.hpp"
#include "types_internal.hpp"
#include "proton/binary.hpp"
+#include "proton/transaction.hpp"
#include <proton/delivery.h>
namespace proton {
-tracker::tracker(pn_delivery_t *d): transfer(make_wrapper(d)) {}
+tracker::tracker(pn_delivery_t *d) : transfer(make_wrapper(d)) {}
Review Comment:
Please don't arbitrarily change the formatting!
##########
cpp/include/proton/transaction.hpp:
##########
@@ -0,0 +1,124 @@
+#ifndef PROTON_TRANSACTION_HPP
+#define PROTON_TRANSACTION_HPP
+
+
+/*
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+
+
+#include "./fwd.hpp"
+#include "./internal/export.hpp"
+#include "./sender.hpp"
+#include "./tracker.hpp"
+#include "./container.hpp"
+
+/// @file
+/// @copybrief proton::transaction
+
+namespace proton {
+
+class transaction_handler;
+
+// TODO: This should not be accessible to users.
+class transaction_impl {
+ public:
+ proton::sender txn_ctrl;
+ proton::transaction_handler *handler = nullptr;
+ proton::binary id;
+ proton::tracker _declare;
+ proton::tracker _discharge;
+ bool failed = false;
+ std::vector<proton::tracker> pending;
+
+ void commit();
+ void abort();
+ void declare();
+ proton::tracker send(proton::sender s, proton::message msg);
+
+ void discharge(bool failed);
+ void release_pending();
+ void accept(delivery &d);
+ void update(tracker &d, uint64_t state);
+ void set_id(binary _id);
+
+ proton::tracker send_ctrl(proton::symbol descriptor, proton::value _value);
+ void handle_outcome(proton::tracker t);
+ transaction_impl(proton::sender &_txn_ctrl,
+ proton::transaction_handler &_handler,
+ bool _settle_before_discharge);
+
+ // delete copy and assignment operator to ensure no copy of this object is
+ // every made transaction_impl(const transaction_impl&) = delete;
+ // transaction_impl& operator=(const transaction_impl&) = delete;
+};
Review Comment:
Create an internal header file for this class
##########
cpp/src/messaging_adapter.cpp:
##########
@@ -109,14 +112,55 @@ void message_decode(message& msg, proton::delivery
delivery) {
msg.decode(buf);
pn_link_advance(unwrap(link));
}
-
Review Comment:
Please don't remove spacing between methods
##########
cpp/src/messaging_adapter.cpp:
##########
@@ -69,7 +71,8 @@ void on_link_flow(messaging_handler& handler, pn_event_t*
event) {
// TODO: process session flow data, if no link-specific data, just return.
if (!lnk) return;
int state = pn_link_state(lnk);
- if ((state&PN_LOCAL_ACTIVE) && (state&PN_REMOTE_ACTIVE)) {
+ if (pn_terminus_get_type(pn_link_remote_target(lnk)) == PN_COORDINATOR ||
Review Comment:
Hmm, I'm a little skeptical this should be needed - if it is needed then
maybe there is a bug elsewhere - probably worth looking at.
##########
cpp/src/handler.cpp:
##########
@@ -34,6 +34,7 @@
#include "proton/connection.h"
#include "proton/session.h"
+#include "proton/tracker.hpp"
Review Comment:
Can't see why this shold be neded as nothing else changed in the file!
##########
cpp/src/contexts.hpp:
##########
@@ -161,6 +163,7 @@ class transfer_context : public context {
transfer_context() : user_data_(nullptr) {}
static transfer_context& get(pn_delivery_t* s);
+ std::unique_ptr<transaction> transaction_;
Review Comment:
`transaction* transaction_;`
##########
cpp/include/proton/transaction.hpp:
##########
@@ -0,0 +1,124 @@
+#ifndef PROTON_TRANSACTION_HPP
+#define PROTON_TRANSACTION_HPP
+
+
+/*
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+
+
+#include "./fwd.hpp"
+#include "./internal/export.hpp"
+#include "./sender.hpp"
+#include "./tracker.hpp"
+#include "./container.hpp"
+
+/// @file
+/// @copybrief proton::transaction
+
+namespace proton {
+
+class transaction_handler;
+
+// TODO: This should not be accessible to users.
+class transaction_impl {
+ public:
+ proton::sender txn_ctrl;
+ proton::transaction_handler *handler = nullptr;
+ proton::binary id;
+ proton::tracker _declare;
+ proton::tracker _discharge;
+ bool failed = false;
+ std::vector<proton::tracker> pending;
+
+ void commit();
+ void abort();
+ void declare();
+ proton::tracker send(proton::sender s, proton::message msg);
+
+ void discharge(bool failed);
+ void release_pending();
+ void accept(delivery &d);
+ void update(tracker &d, uint64_t state);
+ void set_id(binary _id);
+
+ proton::tracker send_ctrl(proton::symbol descriptor, proton::value _value);
+ void handle_outcome(proton::tracker t);
+ transaction_impl(proton::sender &_txn_ctrl,
+ proton::transaction_handler &_handler,
+ bool _settle_before_discharge);
+
+ // delete copy and assignment operator to ensure no copy of this object is
+ // every made transaction_impl(const transaction_impl&) = delete;
+ // transaction_impl& operator=(const transaction_impl&) = delete;
+};
+
+class
+PN_CPP_CLASS_EXTERN transaction {
+ private:
+ // PN_CPP_EXTERN transaction(proton::sender& _txn_ctrl,
+ // proton::transaction_handler& _handler, bool _settle_before_discharge);
+
+ static transaction mk_transaction_impl(sender &s, transaction_handler &h,
+ bool f);
+ PN_CPP_EXTERN transaction(transaction_impl *impl);
+ transaction_impl *_impl;
+
+ public:
+ // TODO:
+ // PN_CPP_EXTERN transaction(transaction &o);
+ PN_CPP_EXTERN transaction();
+ PN_CPP_EXTERN ~transaction();
+ PN_CPP_EXTERN bool is_empty();
+ PN_CPP_EXTERN void commit();
+ PN_CPP_EXTERN void abort();
+ PN_CPP_EXTERN void declare();
+ PN_CPP_EXTERN void handle_outcome(proton::tracker);
+ PN_CPP_EXTERN proton::tracker send(proton::sender s, proton::message msg);
+ PN_CPP_EXTERN void accept(delivery &t);
Review Comment:
I think that you should be able to reject, release & modify in transaction
too
##########
cpp/examples/tx_recv.cpp:
##########
@@ -0,0 +1,130 @@
+/*
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+
+#include "options.hpp"
+
+#include <proton/connection.hpp>
+#include <proton/container.hpp>
+#include <proton/message.hpp>
+#include <proton/message_id.hpp>
+#include <proton/messaging_handler.hpp>
+#include <proton/types.hpp>
+#include <proton/transaction.hpp>
+
+#include <iostream>
+#include <map>
+#include <string>
+
+#include <chrono>
+#include <thread>
+
+class tx_recv : public proton::messaging_handler, proton::transaction_handler {
+ private:
+ proton::receiver receiver;
+ std::string url;
+ int expected;
+ int batch_size;
+ int current_batch = 0;
+ int committed = 0;
+
+ proton::container *container;
+ proton::transaction transaction;
+ proton::connection connection;
Review Comment:
We shouldn't need to hold the connection as we should always be able to find
the connection in a handler. This may be true for the transaction too - either
we restrict the API to be one transaction per session or one transaction per
connection. I think per session is porbably better as it matches the cms/jms
api.
##########
cpp/include/proton/transaction.hpp:
##########
@@ -0,0 +1,124 @@
+#ifndef PROTON_TRANSACTION_HPP
+#define PROTON_TRANSACTION_HPP
+
+
+/*
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+
+
+#include "./fwd.hpp"
+#include "./internal/export.hpp"
+#include "./sender.hpp"
+#include "./tracker.hpp"
+#include "./container.hpp"
+
+/// @file
+/// @copybrief proton::transaction
+
+namespace proton {
+
+class transaction_handler;
+
+// TODO: This should not be accessible to users.
+class transaction_impl {
+ public:
+ proton::sender txn_ctrl;
+ proton::transaction_handler *handler = nullptr;
+ proton::binary id;
+ proton::tracker _declare;
+ proton::tracker _discharge;
Review Comment:
These must be exclusive as you cannot be delivering a declare and a
discharge at the same time so I think you only need to keep track of a single
outgoing delivery.
##########
cpp/include/proton/transaction.hpp:
##########
@@ -0,0 +1,124 @@
+#ifndef PROTON_TRANSACTION_HPP
+#define PROTON_TRANSACTION_HPP
+
+
+/*
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+
+
+#include "./fwd.hpp"
+#include "./internal/export.hpp"
+#include "./sender.hpp"
+#include "./tracker.hpp"
+#include "./container.hpp"
+
+/// @file
+/// @copybrief proton::transaction
+
+namespace proton {
+
+class transaction_handler;
+
+// TODO: This should not be accessible to users.
+class transaction_impl {
+ public:
+ proton::sender txn_ctrl;
Review Comment:
I think this is one per connection not per transaction. The transaction
needs a way to find its conection (or session if it's per session) anyway so
you should always be able to find the transaction controller
##########
cpp/src/transfer.cpp:
##########
@@ -50,6 +52,16 @@ enum transfer::state transfer::state() const { return
static_cast<enum state>(pn
std::string to_string(enum transfer::state s) { return
pn_disposition_type_name(s); }
std::ostream& operator<<(std::ostream& o, const enum transfer::state s) {
return o << to_string(s); }
+void transfer::transaction(proton::transaction t) {
+ transfer_context &cc = transfer_context::get(pn_object());
+ cc.transaction_ = std::make_unique<proton::transaction>(t);
+}
+
+transaction transfer::transaction() const {
+ transfer_context& cc = transfer_context::get(pn_object());
+ return *cc.transaction_;
+}
+
Review Comment:
As above these should take and return a transaction reference or const ref.
##########
cpp/include/proton/transaction.hpp:
##########
@@ -0,0 +1,124 @@
+#ifndef PROTON_TRANSACTION_HPP
+#define PROTON_TRANSACTION_HPP
+
+
+/*
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+
+
+#include "./fwd.hpp"
+#include "./internal/export.hpp"
+#include "./sender.hpp"
+#include "./tracker.hpp"
+#include "./container.hpp"
+
+/// @file
+/// @copybrief proton::transaction
+
+namespace proton {
+
+class transaction_handler;
+
+// TODO: This should not be accessible to users.
+class transaction_impl {
+ public:
+ proton::sender txn_ctrl;
+ proton::transaction_handler *handler = nullptr;
+ proton::binary id;
+ proton::tracker _declare;
+ proton::tracker _discharge;
+ bool failed = false;
+ std::vector<proton::tracker> pending;
+
+ void commit();
+ void abort();
+ void declare();
+ proton::tracker send(proton::sender s, proton::message msg);
+
+ void discharge(bool failed);
+ void release_pending();
+ void accept(delivery &d);
+ void update(tracker &d, uint64_t state);
+ void set_id(binary _id);
+
+ proton::tracker send_ctrl(proton::symbol descriptor, proton::value _value);
+ void handle_outcome(proton::tracker t);
+ transaction_impl(proton::sender &_txn_ctrl,
+ proton::transaction_handler &_handler,
+ bool _settle_before_discharge);
+
+ // delete copy and assignment operator to ensure no copy of this object is
+ // every made transaction_impl(const transaction_impl&) = delete;
+ // transaction_impl& operator=(const transaction_impl&) = delete;
+};
+
+class
+PN_CPP_CLASS_EXTERN transaction {
+ private:
+ // PN_CPP_EXTERN transaction(proton::sender& _txn_ctrl,
+ // proton::transaction_handler& _handler, bool _settle_before_discharge);
+
+ static transaction mk_transaction_impl(sender &s, transaction_handler &h,
+ bool f);
+ PN_CPP_EXTERN transaction(transaction_impl *impl);
+ transaction_impl *_impl;
+
+ public:
+ // TODO:
+ // PN_CPP_EXTERN transaction(transaction &o);
+ PN_CPP_EXTERN transaction();
+ PN_CPP_EXTERN ~transaction();
+ PN_CPP_EXTERN bool is_empty();
+ PN_CPP_EXTERN void commit();
+ PN_CPP_EXTERN void abort();
+ PN_CPP_EXTERN void declare();
+ PN_CPP_EXTERN void handle_outcome(proton::tracker);
+ PN_CPP_EXTERN proton::tracker send(proton::sender s, proton::message msg);
+ PN_CPP_EXTERN void accept(delivery &t);
+
+ friend class transaction_impl;
+ friend class container::impl;
Review Comment:
Pretty sure this can't be needed as only the implementation could need to
use other implementatino classes.
> [c++] Support for transactions
> ------------------------------
>
> Key: PROTON-1442
> URL: https://issues.apache.org/jira/browse/PROTON-1442
> Project: Qpid Proton
> Issue Type: Improvement
> Components: cpp-binding
> Reporter: Radim Kubis
> Assignee: Rakhi Kumari
> Priority: Major
>
> Support for transactions in Qpid Proton C++.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org
