From f21e3cdd98e7de8f092d64a62a29fe2df14528bd Mon Sep 17 00:00:00 2001 From: Michal Michalski Date: Wed, 20 Mar 2019 18:19:15 +0100 Subject: [PATCH] [mediacontroller] Use request_id for listener matching in SendCommand. SendCommand method was using watchId to identify CommandReply callback which should be called. It was passed in bundle object to mc_client_send_custom_cmd function. Now instead of passing watchId we use already available request_id to perform the same matching for command reply listeners. [Verification] Manual test. tct-mediacontroller-tizen-tests - pass 100% Change-Id: I57e90dbe36d7726f2e2f66866b11910d5d1d5d0d Signed-off-by: Michal Michalski --- src/mediacontroller/mediacontroller_api.js | 32 +++++++------- src/mediacontroller/mediacontroller_client.cc | 44 +++++++++---------- src/mediacontroller/mediacontroller_client.h | 4 +- .../mediacontroller_instance.cc | 24 ++++++---- src/mediacontroller/mediacontroller_server.cc | 17 ------- src/mediacontroller/mediacontroller_server.h | 2 +- 6 files changed, 56 insertions(+), 67 deletions(-) diff --git a/src/mediacontroller/mediacontroller_api.js b/src/mediacontroller/mediacontroller_api.js index ce05e2dc..1dd7f287 100755 --- a/src/mediacontroller/mediacontroller_api.js +++ b/src/mediacontroller/mediacontroller_api.js @@ -32,6 +32,7 @@ function ListenerManager(native, listenerName, handle) { this.native = native; this.listenerName = listenerName; this.handle = handle || function(msg, listener, watchId) {}; + this.requestIdToListenerId = {}; } ListenerManager.prototype.addListener = function(callback) { @@ -115,7 +116,6 @@ var ServerCommandListener = new ListenerManager(native_, '_ServerCommandListener var nativeData = { clientName: msg.clientName, - replyId: msg.replyId, requestId: msg.requestId, data: data }; @@ -127,12 +127,12 @@ var ServerCommandListener = new ListenerManager(native_, '_ServerCommandListener }); var ReplyCommandListener = new ListenerManager(native_, '_ReplyCommandListener', function(msg, listener, watchId) { - if (msg.replyId === watchId) { - listener(msg.data); + if (this.requestIdToListenerId[watchId] === msg.requestId) { + listener(msg); this.removeListener(watchId); + delete this.requestIdToListenerId[watchId]; return true; } - return false; }); @@ -712,25 +712,25 @@ MediaControllerServerInfo.prototype.sendCommand = function(command, data, succes {name: 'errorCallback', type: types_.FUNCTION, optional: true, nullable: true} ]); - var nativeData = { - command: args.command, - data: args.data, - name: this.name - }; - - var replyId = ReplyCommandListener.addListener(successCallback); - - nativeData.replyId = replyId; - nativeData.listenerId = ReplyCommandListener.listenerName; var callback = function(result) { if (native_.isFailure(result)) { native_.callIfPossible(args.errorCallback, native_.getErrorObject(result)); return; } - args.successCallback(native_.getResultObject(result)); + native_.callIfPossible(args.successCallback, native_.getResultObject(result).data); }; - native_.call('MediaControllerServerInfo_sendCommand', nativeData, callback); + var nativeData = { + command: args.command, + data: args.data, + name: this.name, + listenerId: ReplyCommandListener.listenerName + }; + + var replyListenerId = ReplyCommandListener.addListener(callback); + var result = native_.callSync('MediaControllerServerInfo_sendCommand', nativeData); + + ReplyCommandListener.requestIdToListenerId[replyListenerId] = result.requestId; }; MediaControllerServerInfo.prototype.addServerStatusChangeListener = function(listener) { diff --git a/src/mediacontroller/mediacontroller_client.cc b/src/mediacontroller/mediacontroller_client.cc index 99c2ebbb..05d0bda7 100644 --- a/src/mediacontroller/mediacontroller_client.cc +++ b/src/mediacontroller/mediacontroller_client.cc @@ -30,6 +30,8 @@ namespace mediacontroller { using common::PlatformResult; using common::ErrorCode; +using common::tools::ReportError; +using common::tools::ReportSuccess; MediaControllerClient::MediaControllerClient() : handle_(nullptr) { ScopeLogger(); @@ -470,24 +472,15 @@ void MediaControllerClient::OnMetadataUpdate(const char* server_name, mc_metadat PlatformResult MediaControllerClient::SendCommand(const std::string& server_name, const std::string& command, const picojson::value& data, - const std::string& reply_id, - const JsonCallback& reply_cb) { + const JsonCallback& reply_cb, + char** request_id) { ScopeLogger(); bundle* bundle = bundle_create(); - char* request_id = nullptr; SCOPE_EXIT { bundle_free(bundle); - free(request_id); }; int ret; - ret = bundle_add(bundle, "replyId", reply_id.c_str()); - if (MEDIA_CONTROLLER_ERROR_NONE != ret) { - return LogAndCreateResult( - ErrorCode::UNKNOWN_ERR, "Unable to add replyId to bundle", - ("bundle_add(replyId) error: %d, message: %s", ret, get_error_message(ret))); - } - ret = bundle_add(bundle, "data", data.serialize().c_str()); if (MEDIA_CONTROLLER_ERROR_NONE != ret) { return LogAndCreateResult( @@ -495,7 +488,7 @@ PlatformResult MediaControllerClient::SendCommand(const std::string& server_name ("bundle_add(data) error: %d, message: %s", ret, get_error_message(ret))); } - ret = mc_client_send_custom_cmd(handle_, server_name.c_str(), command.c_str(), bundle, &request_id); + ret = mc_client_send_custom_cmd(handle_, server_name.c_str(), command.c_str(), bundle, request_id); if (MEDIA_CONTROLLER_ERROR_NONE != ret) { return LogAndCreateResult( ErrorCode::UNKNOWN_ERR, "Error sending custom command", @@ -512,28 +505,22 @@ void MediaControllerClient::OnCommandReply(const char* server_name, const char* ScopeLogger(); MediaControllerClient* client = static_cast(user_data); + picojson::value out = picojson::value(picojson::object()); + picojson::object& out_o = out.get(); picojson::value reply = picojson::value(picojson::object()); picojson::object& reply_o = reply.get(); int ret; - char* reply_id_str = nullptr; char* data_str = nullptr; SCOPE_EXIT { - free(reply_id_str); free(data_str); }; - ret = bundle_get_str(bundle, "replyId", &reply_id_str); - if (MEDIA_CONTROLLER_ERROR_NONE != ret) { - LoggerE("bundle_get_str(replyId) failed, error: %d", ret); - return; - } - - reply_o["replyId"] = picojson::value(std::string(reply_id_str)); - ret = bundle_get_str(bundle, "data", &data_str); if (MEDIA_CONTROLLER_ERROR_NONE != ret) { LoggerE("bundle_get_str(data) failed, error: %d", ret); + ReportError(out_o); + client->command_reply_callback_(&out); return; } @@ -542,12 +529,23 @@ void MediaControllerClient::OnCommandReply(const char* server_name, const char* picojson::parse(data, data_str, data_str + strlen(data_str), &err); if (!err.empty()) { LoggerE("Failed to parse bundle data: %s", err.c_str()); + ReportError(out_o); + client->command_reply_callback_(&out); return; } reply_o["data"] = data; reply_o["name"] = picojson::value(server_name); - client->command_reply_callback_(&reply); + if (nullptr == request_id) { + LoggerE("Request id is null."); + ReportError(out_o); + client->command_reply_callback_(&out); + return; + } + out_o["requestId"] = picojson::value(std::string(request_id)); + + ReportSuccess(reply, out_o); + client->command_reply_callback_(&out); } PlatformResult MediaControllerClient::SendPlaybackState(const std::string& server_name, diff --git a/src/mediacontroller/mediacontroller_client.h b/src/mediacontroller/mediacontroller_client.h index 6108eb52..e0e669d0 100644 --- a/src/mediacontroller/mediacontroller_client.h +++ b/src/mediacontroller/mediacontroller_client.h @@ -46,8 +46,8 @@ class MediaControllerClient { common::PlatformResult SendRepeatMode(const std::string& server_name, bool mode); common::PlatformResult SendCommand(const std::string& server_name, const std::string& command, - const picojson::value& data, const std::string& reply_id, - const JsonCallback& reply_cb); + const picojson::value& data, const JsonCallback& reply_cb, + char** request_id); common::PlatformResult SetServerStatusChangeListener(const JsonCallback& callback); common::PlatformResult UnsetServerStatusChangeListener(); diff --git a/src/mediacontroller/mediacontroller_instance.cc b/src/mediacontroller/mediacontroller_instance.cc index aafc294d..c7547a18 100644 --- a/src/mediacontroller/mediacontroller_instance.cc +++ b/src/mediacontroller/mediacontroller_instance.cc @@ -21,6 +21,7 @@ #include "common/platform_result.h" #include "common/task-queue.h" #include "common/tools.h" +#include "common/scope_exit.h" #include "mediacontroller/mediacontroller_types.h" @@ -317,12 +318,13 @@ void MediaControllerInstance::MediaControllerServerReplyCommand(const picojson:: } CHECK_EXIST(args, "clientName", out) - CHECK_EXIST(args, "replyId", out) CHECK_EXIST(args, "requestId", out) CHECK_EXIST(args, "data", out) - server_->CommandReply(args.get("clientName").get(), args.get("requestId").to_str(), - args.get("replyId").to_str(), args.get("data")); + server_->CommandReply( + args.get("clientName").get(), + args.get("requestId").get(), + args.get("data")); ReportSuccess(out); } @@ -591,25 +593,31 @@ void MediaControllerInstance::MediaControllerServerInfoSendCommand(const picojso } CHECK_EXIST(args, "listenerId", out) - CHECK_EXIST(args, "replyId", out) CHECK_EXIST(args, "name", out) CHECK_EXIST(args, "command", out) CHECK_EXIST(args, "data", out) JsonCallback reply_cb = [this, args](picojson::value* reply) -> void { picojson::object& reply_obj = reply->get(); - reply_obj["listenerId"] = args.get("listenerId"); - Instance::PostMessage(this, reply->serialize().c_str()); }; + char* request_id = nullptr; + SCOPE_EXIT { + free(request_id); + }; + PlatformResult result = client_->SendCommand( - args.get("name").get(), args.get("command").get(), args.get("data"), - args.get("replyId").to_str(), reply_cb); + args.get("name").get(), + args.get("command").get(), + args.get("data"), + reply_cb, + &request_id); if (result) { ReportSuccess(out); + out["requestId"] = picojson::value(std::string(request_id)); } else { LogAndReportError(result, &out, ("Failed to send command.")); } diff --git a/src/mediacontroller/mediacontroller_server.cc b/src/mediacontroller/mediacontroller_server.cc index 214b8913..9c089cc1 100644 --- a/src/mediacontroller/mediacontroller_server.cc +++ b/src/mediacontroller/mediacontroller_server.cc @@ -215,10 +215,8 @@ void MediaControllerServer::OnCommandReceived(const char* client_name, const cha int ret; char* data_str = nullptr; - char* reply_id_str = nullptr; SCOPE_EXIT { free(data_str); - free(reply_id_str); }; ret = bundle_get_str(bundle, "data", &data_str); @@ -227,12 +225,6 @@ void MediaControllerServer::OnCommandReceived(const char* client_name, const cha return; } - ret = bundle_get_str(bundle, "replyId", &reply_id_str); - if (MEDIA_CONTROLLER_ERROR_NONE != ret) { - LoggerE("bundle_get_str(replyId) failed, error: %d", ret); - return; - } - picojson::value data; std::string err; picojson::parse(data, data_str, data_str + strlen(data_str), &err); @@ -246,7 +238,6 @@ void MediaControllerServer::OnCommandReceived(const char* client_name, const cha request_o["clientName"] = picojson::value(std::string(client_name)); request_o["command"] = picojson::value(std::string(command)); - request_o["replyId"] = picojson::value(std::string(reply_id_str)); request_o["requestId"] = picojson::value(std::string(request_id)); request_o["data"] = data; @@ -255,7 +246,6 @@ void MediaControllerServer::OnCommandReceived(const char* client_name, const cha PlatformResult MediaControllerServer::CommandReply(const std::string& client_name, const std::string& request_id, - const std::string& reply_id, const picojson::value& data) { ScopeLogger(); @@ -266,13 +256,6 @@ PlatformResult MediaControllerServer::CommandReply(const std::string& client_nam bundle_free(bundle); }; - ret = bundle_add(bundle, "replyId", reply_id.c_str()); - if (MEDIA_CONTROLLER_ERROR_NONE != ret) { - return LogAndCreateResult( - ErrorCode::UNKNOWN_ERR, "Unable to add replyId to bundle", - ("bundle_add(replyId) error: %d, message: %s", ret, get_error_message(ret))); - } - ret = bundle_add(bundle, "data", data.serialize().c_str()); if (MEDIA_CONTROLLER_ERROR_NONE != ret) { return LogAndCreateResult( diff --git a/src/mediacontroller/mediacontroller_server.h b/src/mediacontroller/mediacontroller_server.h index 19405e9f..010f59b0 100644 --- a/src/mediacontroller/mediacontroller_server.h +++ b/src/mediacontroller/mediacontroller_server.h @@ -42,7 +42,7 @@ class MediaControllerServer { common::PlatformResult UnsetChangeRequestPlaybackInfoListener(); common::PlatformResult CommandReply(const std::string& client_name, const std::string& request_id, - const std::string& reply_id, const picojson::value& data); + const picojson::value& data); common::PlatformResult SetCommandListener(const JsonCallback& callback); common::PlatformResult UnsetCommandListener(); -- 2.34.1