From 3c5931bb13b5f88edcebb375fa6964dde8b85563 Mon Sep 17 00:00:00 2001 From: David Robillard Date: Thu, 26 Jul 2012 15:27:03 +0000 Subject: Report subject with errors to client for more meaningful error messages. git-svn-id: http://svn.drobilla.net/lad/trunk/ingen@4556 a436a847-0d15-0410-975c-d299462d15a1 --- src/server/Broadcaster.hpp | 2 +- src/server/Event.hpp | 22 +++++++++++++++------- src/server/EventWriter.hpp | 2 +- src/server/events/Connect.cpp | 19 ++++++++++--------- src/server/events/CreateNode.cpp | 11 +++++------ src/server/events/CreatePatch.cpp | 9 ++++----- src/server/events/CreatePort.cpp | 13 ++++++------- src/server/events/Delete.cpp | 17 +++++------------ src/server/events/Delta.cpp | 11 ++++++----- src/server/events/Disconnect.cpp | 15 ++++++++------- src/server/events/DisconnectAll.cpp | 9 ++++----- src/server/events/Get.cpp | 29 ++++++++++------------------- src/server/events/Move.cpp | 9 ++++----- src/server/events/SetPortValue.cpp | 5 ++--- 14 files changed, 81 insertions(+), 92 deletions(-) (limited to 'src/server') diff --git a/src/server/Broadcaster.hpp b/src/server/Broadcaster.hpp index 2c88c5f8..32a1ab92 100644 --- a/src/server/Broadcaster.hpp +++ b/src/server/Broadcaster.hpp @@ -106,7 +106,7 @@ public: void set_response_id(int32_t id) {} ///< N/A void get(const Raul::URI& uri) {} ///< N/A - void response(int32_t id, Status status) {} ///< N/A + void response(int32_t id, Status status, const std::string& subject) {} ///< N/A void error(const std::string& msg) { BROADCAST(error, msg); } diff --git a/src/server/Event.hpp b/src/server/Event.hpp index 963671ab..5f2e99bb 100644 --- a/src/server/Event.hpp +++ b/src/server/Event.hpp @@ -78,13 +78,6 @@ public: /** Return the status (success or error code) of this event. */ Status status() const { return _status; } - /** Respond to the originating client. */ - void respond(Status status) { - if (_request_client) { - _request_client->response(_request_id, status); - } - } - protected: Event(Engine& engine, SharedPtr client, int32_t id, FrameTime time) : _engine(engine) @@ -107,12 +100,27 @@ protected: return !st; } + inline bool pre_process_done(Status st, const Raul::URI& subject) { + _status = st; + _err_subject = subject.str(); + return !st; + } + + /** Respond to the originating client. */ + inline Status respond() { + if (_request_client) { + _request_client->response(_request_id, _status, _err_subject); + } + return _status; + } + Engine& _engine; Raul::AtomicPtr _next; SharedPtr _request_client; int32_t _request_id; FrameTime _time; Status _status; + std::string _err_subject; }; } // namespace Server diff --git a/src/server/EventWriter.hpp b/src/server/EventWriter.hpp index 2a080be4..d25dc113 100644 --- a/src/server/EventWriter.hpp +++ b/src/server/EventWriter.hpp @@ -84,7 +84,7 @@ public: virtual void get(const Raul::URI& uri); - virtual void response(int32_t id, Status status) {} ///< N/A + virtual void response(int32_t id, Status status, const std::string& subject) {} ///< N/A virtual void error(const std::string& msg) {} ///< N/A diff --git a/src/server/events/Connect.cpp b/src/server/events/Connect.cpp index b9ea148a..dc6f7b99 100644 --- a/src/server/events/Connect.cpp +++ b/src/server/events/Connect.cpp @@ -57,30 +57,32 @@ Connect::pre_process() PortImpl* tail = _engine.engine_store()->find_port(_tail_path); PortImpl* head = _engine.engine_store()->find_port(_head_path); - if (!tail || !head) { - return Event::pre_process_done(PORT_NOT_FOUND); + if (!tail) { + return Event::pre_process_done(PORT_NOT_FOUND, _tail_path.str()); + } else if (!head) { + return Event::pre_process_done(PORT_NOT_FOUND, _head_path.str()); } _dst_input_port = dynamic_cast(head); _src_output_port = dynamic_cast(tail); if (!_dst_input_port || !_src_output_port) { - return Event::pre_process_done(DIRECTION_MISMATCH); + return Event::pre_process_done(DIRECTION_MISMATCH, _head_path.str()); } NodeImpl* const src_node = tail->parent_node(); NodeImpl* const dst_node = head->parent_node(); if (!src_node || !dst_node) { - return Event::pre_process_done(PARENT_NOT_FOUND); + return Event::pre_process_done(PARENT_NOT_FOUND, _head_path.str()); } if (src_node->parent() != dst_node->parent() && src_node != dst_node->parent() && src_node->parent() != dst_node) { - return Event::pre_process_done(PARENT_DIFFERS); + return Event::pre_process_done(PARENT_DIFFERS, _head_path.str()); } if (!EdgeImpl::can_connect(_src_output_port, _dst_input_port)) { - return Event::pre_process_done(TYPE_MISMATCH); + return Event::pre_process_done(TYPE_MISMATCH, _head_path); } if (src_node->parent_patch() != dst_node->parent_patch()) { @@ -100,7 +102,7 @@ Connect::pre_process() } if (_patch->has_edge(_src_output_port, _dst_input_port)) { - return Event::pre_process_done(EXISTS); + return Event::pre_process_done(EXISTS, _head_path); } _edge = SharedPtr( @@ -151,8 +153,7 @@ Connect::execute(ProcessContext& context) void Connect::post_process() { - respond(_status); - if (!_status) { + if (!respond()) { _engine.broadcaster()->connect(_tail_path, _head_path); } } diff --git a/src/server/events/CreateNode.cpp b/src/server/events/CreateNode.cpp index 76f29613..b8e795e8 100644 --- a/src/server/events/CreateNode.cpp +++ b/src/server/events/CreateNode.cpp @@ -61,16 +61,16 @@ CreateNode::pre_process() } if (_engine.engine_store()->find_object(_path)) { - return Event::pre_process_done(EXISTS); + return Event::pre_process_done(EXISTS, _path.str()); } if (!(_patch = _engine.engine_store()->find_patch(_path.parent()))) { - return Event::pre_process_done(PARENT_NOT_FOUND); + return Event::pre_process_done(PARENT_NOT_FOUND, _path.parent().str()); } PluginImpl* plugin = _engine.node_factory()->plugin(_plugin_uri); if (!plugin) { - return Event::pre_process_done(PLUGIN_NOT_FOUND); + return Event::pre_process_done(PLUGIN_NOT_FOUND, _plugin_uri); } const iterator p = _properties.find(uris.ingen_polyphonic); @@ -84,7 +84,7 @@ CreateNode::pre_process() polyphonic, _patch, _engine))) { - return Event::pre_process_done(CREATION_FAILED); + return Event::pre_process_done(CREATION_FAILED, _path.str()); } _node->properties().insert(_properties.begin(), _properties.end()); @@ -125,8 +125,7 @@ CreateNode::execute(ProcessContext& context) void CreateNode::post_process() { - respond(_status); - if (!_status) { + if (!respond()) { for (Update::const_iterator i = _update.begin(); i != _update.end(); ++i) { _engine.broadcaster()->put(i->first, i->second); } diff --git a/src/server/events/CreatePatch.cpp b/src/server/events/CreatePatch.cpp index b6b62f71..602fe2a4 100644 --- a/src/server/events/CreatePatch.cpp +++ b/src/server/events/CreatePatch.cpp @@ -48,14 +48,14 @@ bool CreatePatch::pre_process() { if (_path.is_root() || _engine.engine_store()->find_object(_path) != NULL) { - return Event::pre_process_done(EXISTS); + return Event::pre_process_done(EXISTS, _path); } const Raul::Path& path = (const Raul::Path&)_path; _parent = _engine.engine_store()->find_patch(path.parent()); if (!_parent) { - return Event::pre_process_done(PARENT_NOT_FOUND); + return Event::pre_process_done(PARENT_NOT_FOUND, path.parent()); } const Ingen::Shared::URIs& uris = _engine.world()->uris(); @@ -70,7 +70,7 @@ CreatePatch::pre_process() } if (int_poly < 1) { - return Event::pre_process_done(INVALID_POLY); + return Event::pre_process_done(INVALID_POLY, path); } if (int_poly == _parent->internal_poly()) { @@ -114,8 +114,7 @@ CreatePatch::execute(ProcessContext& context) void CreatePatch::post_process() { - respond(_status); - if (!_status) { + if (!respond()) { _engine.broadcaster()->put(_path, _update); } } diff --git a/src/server/events/CreatePort.cpp b/src/server/events/CreatePort.cpp index 9ddb8894..b89a90cc 100644 --- a/src/server/events/CreatePort.cpp +++ b/src/server/events/CreatePort.cpp @@ -85,15 +85,15 @@ bool CreatePort::pre_process() { if (_port_type == PortType::UNKNOWN) { - return Event::pre_process_done(UNKNOWN_TYPE); + return Event::pre_process_done(UNKNOWN_TYPE, _path); } if (_engine.engine_store()->find_object(_path)) { - return Event::pre_process_done(_status); + return Event::pre_process_done(_status, _path); } if (!(_patch = _engine.engine_store()->find_patch(_path.parent()))) { - return Event::pre_process_done(PARENT_NOT_FOUND); + return Event::pre_process_done(PARENT_NOT_FOUND, _path.parent()); } const Shared::URIs& uris = _engine.world()->uris(); @@ -112,7 +112,7 @@ CreatePort::pre_process() _engine.world()->forge().make(old_n_ports))); } else if (index_i->second.type() != uris.forge.Int || index_i->second.get_int32() != old_n_ports) { - return Event::pre_process_done(BAD_INDEX); + return Event::pre_process_done(BAD_INDEX, _path); } const PropIter poly_i = _properties.find(uris.ingen_polyphonic); @@ -123,7 +123,7 @@ CreatePort::pre_process() if (!(_patch_port = _patch->create_port( *_engine.buffer_factory(), _path.symbol(), _port_type, _buf_type, buf_size, _is_output, polyphonic))) { - return Event::pre_process_done(CREATION_FAILED); + return Event::pre_process_done(CREATION_FAILED, _path); } _patch_port->properties().insert(_properties.begin(), _properties.end()); @@ -174,8 +174,7 @@ CreatePort::execute(ProcessContext& context) void CreatePort::post_process() { - respond(_status); - if (!_status) { + if (!respond()) { _engine.broadcaster()->put(_path, _update); } diff --git a/src/server/events/Delete.cpp b/src/server/events/Delete.cpp index 7ef4d2a5..9c68f4fe 100644 --- a/src/server/events/Delete.cpp +++ b/src/server/events/Delete.cpp @@ -63,8 +63,10 @@ Delete::~Delete() bool Delete::pre_process() { - if (_path.is_root() || _path == "path:/control_in" || _path == "path:/control_out") { - return Event::pre_process_done(NOT_DELETABLE); + if (_path.is_root() || + _path == "path:/control_in" || + _path == "path:/control_out") { + return Event::pre_process_done(NOT_DELETABLE, _path); } _lock.acquire(); @@ -157,13 +159,7 @@ Delete::post_process() { _lock.release(); _removed_bindings.reset(); - - if (!Raul::Path::is_path(_uri) - || _path.is_root() || _path == "path:/control_in" || _path == "path:/control_out") { - // XXX: Report error? Silently ignore? - } else if (!_node && !_port) { - respond(NOT_FOUND); - } else if (_patch_node_listnode || _patch_port_listnode) { + if (!respond() && (_patch_node_listnode || _patch_port_listnode)) { if (_patch_node_listnode) { _node->deactivate(); delete _patch_node_listnode; @@ -171,15 +167,12 @@ Delete::post_process() delete _patch_port_listnode; } - respond(SUCCESS); _engine.broadcaster()->bundle_begin(); if (_disconnect_event) { _disconnect_event->post_process(); } _engine.broadcaster()->del(_path); _engine.broadcaster()->bundle_end(); - } else { - respond(FAILURE); } if (_engine_port) { diff --git a/src/server/events/Delta.cpp b/src/server/events/Delta.cpp index 8da4b230..0f067a7e 100644 --- a/src/server/events/Delta.cpp +++ b/src/server/events/Delta.cpp @@ -113,7 +113,7 @@ Delta::pre_process() : static_cast(_engine.node_factory()->plugin(_subject)); if (!_object && (!is_graph_object || !_create)) { - return Event::pre_process_done(NOT_FOUND); + return Event::pre_process_done(NOT_FOUND, _subject); } const Ingen::Shared::URIs& uris = _engine.world()->uris(); @@ -139,7 +139,7 @@ Delta::pre_process() // Grab the object for applying properties, if the create-event succeeded _object = _engine.engine_store()->find_object(Raul::Path(_subject.str())); } else { - return Event::pre_process_done(BAD_OBJECT_TYPE); + return Event::pre_process_done(BAD_OBJECT_TYPE, _subject); } } @@ -250,7 +250,8 @@ Delta::pre_process() _types.push_back(op); } - return Event::pre_process_done(_status == NOT_PREPARED ? SUCCESS : _status); + return Event::pre_process_done(_status == NOT_PREPARED ? SUCCESS : _status, + _subject); } void @@ -346,7 +347,7 @@ Delta::post_process() if (_create_event) { _create_event->post_process(); } else { - respond(SUCCESS); + respond(); if (_create) { _engine.broadcaster()->put(_subject, _properties, _context); } else { @@ -354,7 +355,7 @@ Delta::post_process() } } } else { - respond(_status); + respond(); } } diff --git a/src/server/events/Disconnect.cpp b/src/server/events/Disconnect.cpp index 3ad9f650..00e4b374 100644 --- a/src/server/events/Disconnect.cpp +++ b/src/server/events/Disconnect.cpp @@ -119,14 +119,16 @@ Disconnect::pre_process() if (_tail_path.parent().parent() != _head_path.parent().parent() && _tail_path.parent() != _head_path.parent().parent() && _tail_path.parent().parent() != _head_path.parent()) { - return Event::pre_process_done(PARENT_DIFFERS); + return Event::pre_process_done(PARENT_DIFFERS, _head_path); } _tail = _engine.engine_store()->find_port(_tail_path); _head = _engine.engine_store()->find_port(_head_path); - if (_tail == NULL || _head == NULL) { - return Event::pre_process_done(PORT_NOT_FOUND); + if (!_tail) { + return Event::pre_process_done(PORT_NOT_FOUND, _tail_path); + } else if (!_head) { + return Event::pre_process_done(PORT_NOT_FOUND, _head_path); } NodeImpl* const src_node = _tail->parent_node(); @@ -151,11 +153,11 @@ Disconnect::pre_process() assert(_patch); if (!_patch->has_edge(_tail, _head)) { - return Event::pre_process_done(NOT_FOUND); + return Event::pre_process_done(NOT_FOUND, _head_path); } if (src_node == NULL || dst_node == NULL) { - return Event::pre_process_done(PARENT_NOT_FOUND); + return Event::pre_process_done(PARENT_NOT_FOUND, _head_path); } _impl = new Impl(_engine, @@ -214,8 +216,7 @@ Disconnect::execute(ProcessContext& context) void Disconnect::post_process() { - respond(_status); - if (!_status) { + if (!respond()) { _engine.broadcaster()->disconnect(_tail->path(), _head->path()); } diff --git a/src/server/events/DisconnectAll.cpp b/src/server/events/DisconnectAll.cpp index d360073c..e1310251 100644 --- a/src/server/events/DisconnectAll.cpp +++ b/src/server/events/DisconnectAll.cpp @@ -89,17 +89,17 @@ DisconnectAll::pre_process() _parent = _engine.engine_store()->find_patch(_parent_path); if (!_parent) { - return Event::pre_process_done(PARENT_NOT_FOUND); + return Event::pre_process_done(PARENT_NOT_FOUND, _parent_path); } GraphObjectImpl* object = _engine.engine_store()->find_object(_path); if (!object) { - return Event::pre_process_done(NOT_FOUND); + return Event::pre_process_done(NOT_FOUND, _path); } if (object->parent_patch() != _parent && object->parent()->parent_patch() != _parent) { - return Event::pre_process_done(INVALID_PARENT_PATH); + return Event::pre_process_done(INVALID_PARENT_PATH, _parent_path); } // Only one of these will succeed @@ -159,8 +159,7 @@ DisconnectAll::execute(ProcessContext& context) void DisconnectAll::post_process() { - respond(_status); - if (!_status) { + if (!respond()) { _engine.broadcaster()->disconnect_all(_parent_path, _path); } } diff --git a/src/server/events/Get.cpp b/src/server/events/Get.cpp index 24f3d1d9..cc64aaef 100644 --- a/src/server/events/Get.cpp +++ b/src/server/events/Get.cpp @@ -56,12 +56,14 @@ Get::pre_process() if (_uri == "ingen:plugins") { _plugins = _engine.node_factory()->plugins(); return Event::pre_process_done(SUCCESS); + } else if (_uri == "ingen:engine") { + return Event::pre_process_done(SUCCESS); } else if (Raul::Path::is_valid(_uri.str())) { _object = _engine.engine_store()->find_object(Raul::Path(_uri.str())); - return Event::pre_process_done(_object ? SUCCESS : NOT_FOUND); + return Event::pre_process_done(_object ? SUCCESS : NOT_FOUND, _uri); } else { _plugin = _engine.node_factory()->plugin(_uri); - return Event::pre_process_done(_plugin ? SUCCESS : NOT_FOUND); + return Event::pre_process_done(_plugin ? SUCCESS : NOT_FOUND, _uri); } } @@ -127,28 +129,17 @@ send_patch(Interface* client, const PatchImpl* patch) void Get::post_process() { - if (_uri == "ingen:plugins") { - respond(SUCCESS); - if (_request_client) { + if (!respond() && _request_client) { + if (_uri == "ingen:plugins") { _engine.broadcaster()->send_plugins_to(_request_client.get(), _plugins); - } - } else if (_uri == "ingen:engine") { - respond(SUCCESS); - // TODO: Keep a proper RDF model of the engine - if (_request_client) { + } else if (_uri == "ingen:engine") { + // TODO: Keep a proper RDF model of the engine Shared::URIs& uris = _engine.world()->uris(); _request_client->set_property( uris.ingen_engine, uris.ingen_sampleRate, uris.forge.make(int32_t(_engine.driver()->sample_rate()))); - } - } else if (!_object && !_plugin) { - respond(NOT_FOUND); - } else if (!_request_client) { - respond(CLIENT_NOT_FOUND); - } else { - respond(SUCCESS); - if (_object) { + } else if (_object) { _request_client->bundle_begin(); const NodeImpl* node = NULL; const PatchImpl* patch = NULL; @@ -164,8 +155,8 @@ Get::post_process() } else if (_plugin) { _request_client->put(_uri, _plugin->properties()); } - _lock.release(); } + _lock.release(); } } // namespace Events diff --git a/src/server/events/Move.cpp b/src/server/events/Move.cpp index 9a894e43..483c77e7 100644 --- a/src/server/events/Move.cpp +++ b/src/server/events/Move.cpp @@ -55,16 +55,16 @@ Move::pre_process() Glib::RWLock::WriterLock lock(_engine.engine_store()->lock()); if (!_old_path.parent().is_parent_of(_new_path)) { - return Event::pre_process_done(PARENT_DIFFERS); + return Event::pre_process_done(PARENT_DIFFERS, _new_path); } _store_iterator = _engine.engine_store()->find(_old_path); if (_store_iterator == _engine.engine_store()->end()) { - return Event::pre_process_done(NOT_FOUND); + return Event::pre_process_done(NOT_FOUND, _old_path); } if (_engine.engine_store()->find_object(_new_path)) { - return Event::pre_process_done(EXISTS); + return Event::pre_process_done(EXISTS, _new_path); } SharedPtr< Raul::Table< Raul::Path, SharedPtr > > removed @@ -106,8 +106,7 @@ Move::execute(ProcessContext& context) void Move::post_process() { - respond(_status); - if (!_status) { + if (!respond()) { _engine.broadcaster()->move(_old_path, _new_path); } } diff --git a/src/server/events/SetPortValue.cpp b/src/server/events/SetPortValue.cpp index 673c9984..005974a2 100644 --- a/src/server/events/SetPortValue.cpp +++ b/src/server/events/SetPortValue.cpp @@ -57,7 +57,7 @@ bool SetPortValue::pre_process() { if (_port->is_output()) { - return Event::pre_process_done(DIRECTION_MISMATCH); + return Event::pre_process_done(DIRECTION_MISMATCH, _port_path); } // Port is on a message context node, set value and run @@ -129,8 +129,7 @@ SetPortValue::apply(Context& context) void SetPortValue::post_process() { - respond(_status); - if (!_status) { + if (!respond()) { _engine.broadcaster()->set_property( _port_path, _engine.world()->uris().ingen_value, -- cgit v1.2.1