[Convergence] Fixes for start() stop() and send() methods.

[Features] Removed dynamic variables which could cause
memory leaks, added variable which shows if service
is connected, fixed validation.

[Verification] Code compiles.

Change-Id: I522797ef4d39114fe88fdc7b1bcaee96ea42ab7c
Signed-off-by: Tomasz Marciniak <t.marciniak@samsung.com>

diff --git a/src/convergence/convergence_api.js b/src/convergence/convergence_api.js
index 7cad85002f28dc849af2bd09ca4f62d336182024..8764443795bdf81d81b2a9db12fb944db20559cc 100644 (file)
--- a/src/convergence/convergence_api.js
+++ b/src/convergence/convergence_api.js
@@ -589,6 +589,11 @@ function AppCommunicationService() {
       value: null,
       writable: true,
       enumerable: false
+    },
+    _isStarted : {
+      value: false,
+      writable: true,
+      enumerable: false
     }
   });
 
@@ -633,6 +638,7 @@ native_.addListener('APP_COMMUNICATION_SERVICE_LISTENER', function(result) {
       native_.callIfPossible(s._connectCallback, s);
       break;
     case 'onStart':
+      s._isStarted = true;
       native_.callIfPossible(s._startCallback,
         new ChannelInfo(result.channel.uri, result.channel.id), null);
       break;
@@ -641,6 +647,7 @@ native_.addListener('APP_COMMUNICATION_SERVICE_LISTENER', function(result) {
         new ChannelInfo(result.channel.uri, result.channel.id), null);
       break;
     case 'onStop':
+      s._isStarted = false;
       native_.callIfPossible(s._stopCallback,
         new ChannelInfo(result.channel.uri, result.channel.id), null);
       break;
@@ -696,21 +703,20 @@ AppCommunicationService.prototype.start = function(channel, successCallback, err
     {
       name: 'successCallback',
       type: types_.FUNCTION,
-      //values: ConnectSuccessCallback,
       optional: false,
       nullable: false
     },
     {
       name: 'errorCallback',
       type: types_.FUNCTION,
-      //values: ErrorCallback,
       optional: true,
       nullable: true
     }
   ]);
 
-  // TODO check if the service is connected and started
-  // Raise the exception instead
+  if (this._isStarted) {
+    throw new WebAPIException(WebAPIException.INVALID_VALUES_ERR, 'Service already started.');
+  }
 
   var lid = this._serviceId;
   this._startCallback = successCallback;
@@ -725,6 +731,7 @@ AppCommunicationService.prototype.start = function(channel, successCallback, err
                     native_.callIfPossible(errorCallback, native_.getErrorObject(result));
                   }
                });
+
   if (native_.isFailure(result)) {
     throw native_.getErrorObject(result);
   }
@@ -744,21 +751,20 @@ AppCommunicationService.prototype.stop = function(channel, successCallback, erro
     {
       name: 'successCallback',
       type: types_.FUNCTION,
-      //values: ConnectSuccessCallback,
       optional: true,
       nullable: true
     },
     {
       name: 'errorCallback',
       type: types_.FUNCTION,
-      //values: ErrorCallback,
       optional: true,
       nullable: true
     }
   ]);
 
-  // TODO check if the service is connected and started
-  // Raise the exception instead
+  if (!this._isStarted) {
+    throw new WebAPIException(WebAPIException.INVALID_VALUES_ERR, 'Service is not started.');
+  }
 
   var lid = -1;
   if (successCallback) {
@@ -776,6 +782,7 @@ AppCommunicationService.prototype.stop = function(channel, successCallback, erro
                     native_.callIfPossible(errorCallback, native_.getErrorObject(result));
                   }
                });
+
   if (native_.isFailure(result)) {
     throw native_.getErrorObject(result);
   }
@@ -794,29 +801,27 @@ AppCommunicationService.prototype.send = function(channel, payload, successCallb
     },
     {
       name: 'payload',
-      type: types_.PLATFORM_OBJECT,
-      values: [tizen.PayloadString, tizen.PayloadRawBytes],
+      type: types_.ARRAY,
       optional: false,
       nullable: false
     },
     {
       name: 'successCallback',
       type: types_.FUNCTION,
-      //values: ConnectSuccessCallback,
       optional: false,
       nullable: false
     },
     {
       name: 'errorCallback',
       type: types_.FUNCTION,
-      //values: ErrorCallback,
       optional: true,
       nullable: true
     }
   ]);
 
-  // TODO check if the service is connected and started
-  // Raise the exception instead
+  if (!this._isStarted) {
+    throw new WebAPIException(WebAPIException.INVALID_VALUES_ERR, 'Service is not started.');
+  }
 
   var lid = this._serviceId;
   this._sendCallback = successCallback;
@@ -832,6 +837,7 @@ AppCommunicationService.prototype.send = function(channel, payload, successCallb
                     native_.callIfPossible(errorCallback, native_.getErrorObject(result));
                   }
                });
+
   if (native_.isFailure(result)) {
     throw native_.getErrorObject(result);
   }

diff --git a/src/convergence/convergence_app_communication_service.cc b/src/convergence/convergence_app_communication_service.cc
index afd5a1e04fa8085d8f97030966f53d678b3832a2..c2c33e5a177d85322aad4b5467c1eb09e8cabeb4 100644 (file)
--- a/src/convergence/convergence_app_communication_service.cc
+++ b/src/convergence/convergence_app_communication_service.cc
@@ -59,7 +59,7 @@ ConvergenceAppCommunicationService::~ConvergenceAppCommunicationService() {
 }
 
 common::TizenResult ConvergenceAppCommunicationService::Start(
-  ConvergenceChannel *channel,
+  ConvergenceChannel& channel,
   const int cur_listener_id) {
   ScopeLogger();
 
@@ -67,119 +67,62 @@ common::TizenResult ConvergenceAppCommunicationService::Start(
 
   conv_service_h service_handle = FindServiceHandle();
   if (!service_handle) {
-    LoggerE("AAAAAA!!! Service not found");
-    return LogAndCreateTizenError(NotFoundError,
-      "Service with specified type does not exist");
+    return LogAndCreateTizenError(AbortError, "Service with specified type does not exist");
   }
 
-LoggerI("1");
-
   UpdateListener(cur_listener_id);
 
-LoggerI("2");
-
-
-  { // DBG
-    conv_channel_h ch = channel->GetHandle();
-    char *id = NULL;
-    conv_channel_get_string(ch, "channel_id", &id);
-    char *uri = NULL;
-    conv_channel_get_string(ch, "uri", &uri);
-    LoggerI("===== CHANNEL ID [%s] URI [%s]", id, uri);
-  }
-
-  const int error = conv_service_start(service_handle,
-    channel->GetHandle(), nullptr);
+  const int error = conv_service_start(service_handle, channel.GetHandle(), nullptr);
   if (CONV_ERROR_NONE != error) {
-    // TODO: Handle error
-    trace_conv_error(error, __LINE__, "conv_service_start");
+    return LogAndCreateTizenError(AbortError, "Failed to start service");
   }
 
-LoggerI("3");
-  delete channel;
-LoggerI("4");
   return TizenSuccess();
 }
 
 common::TizenResult ConvergenceAppCommunicationService::Stop(
-  ConvergenceChannel *channel,
+  ConvergenceChannel& channel,
   const int cur_listener_id) {
   ScopeLogger();
 
   conv_service_h service_handle = FindServiceHandle();
   if (!service_handle) {
-    return LogAndCreateTizenError(NotFoundError,
-      "Service with specified type does not exist");
+    return LogAndCreateTizenError(AbortError, "Service with specified type does not exist");
   }
 
   UpdateListener(cur_listener_id);
 
-  const int error = conv_service_stop(service_handle,
-    channel->GetHandle(), nullptr);
+  const int error = conv_service_stop(service_handle, channel.GetHandle(), nullptr);
   if (CONV_ERROR_NONE != error) {
-    // TODO: Handle error
-    trace_conv_error(error, __LINE__, "conv_service_stop");
+    return LogAndCreateTizenError(AbortError, "Failed to stop service");
   }
 
-  delete channel;
   return TizenSuccess();
 }
 
 common::TizenResult ConvergenceAppCommunicationService::Send(
-  ConvergenceChannel *channel,
-  ConvergencePayloadArray *payload,
+  ConvergenceChannel& channel,
+  ConvergencePayloadArray& payload,
   const int cur_listener_id) {
   ScopeLogger();
 
   conv_service_h service_handle = FindServiceHandle();
   if (!service_handle) {
-    return LogAndCreateTizenError(NotFoundError,
-      "Service with specified type does not exist");
+    return LogAndCreateTizenError(AbortError, "Service with specified type does not exist");
   }
 
   UpdateListener(cur_listener_id);
 
-  { // DBG
-    LoggerI("...PUBLISHING for service handle [0x0%x]", service_handle);
-
-    conv_service_e t = CONV_SERVICE_NONE;
-    int e = conv_service_get_type(service_handle, &t);
-    if (CONV_ERROR_NONE != e) {
-      trace_conv_error(e, __LINE__, "get service type");
-    }
-    LoggerI("....type [%d]", t);
-
-    char *sid = nullptr;
-    e = conv_service_get_property_string(service_handle, CONV_SERVICE_ID, &sid);
-    if (CONV_ERROR_NONE != e) {
-      trace_conv_error(e, __LINE__, "get service id");
-    }
-    LoggerI("....id [%s]", sid);
-    free(sid);
-
-    conv_channel_h ch = channel->GetHandle();
-    char *id = NULL;
-    conv_channel_get_string(ch, "channel_id", &id);
-    char *uri = NULL;
-    conv_channel_get_string(ch, "uri", &uri);
-    LoggerI("===== CHANNEL ID [%s] URI [%s]", id, uri);
-  }
-
-  const int error = conv_service_publish(service_handle,
-    channel->GetHandle(), payload->GetHandle());
+  const int error = conv_service_publish(service_handle, channel.GetHandle(), payload.GetHandle());
   if (CONV_ERROR_NONE != error) {
-    // TODO: Handle error
-    trace_conv_error(error, __LINE__, "conv_service_publish");
+    return LogAndCreateTizenError(AbortError, "Failed to publish message");
   } else {
     LoggerI("PUBLISHED SUCCESSFULY listener is [%d]", cur_listener_id);
   }
 
-  delete channel;
-  delete payload;
   return TizenSuccess();
 }
 
-
 // TODO move to Payload class
 
 
@@ -381,7 +324,7 @@ ConvergenceAppCommunicationServerService::~ConvergenceAppCommunicationServerServ
 }
 
 common::TizenResult ConvergenceAppCommunicationServerService::Start(
-  ConvergenceChannel *channel,
+  ConvergenceChannel& channel,
   const int cur_listener_id) {
   ScopeLogger();
 
@@ -393,24 +336,20 @@ common::TizenResult ConvergenceAppCommunicationServerService::Start(
   picojson::object param;
   param[kServiceListenerStatus] = picojson::value(kServiceListenerStatusOk);
 
-  // The object 'channel' will be deleted in the super::Start() so we should
-  // add its json before
-  param[kChannel] = ConvergenceChannel::ToJson(channel->GetHandle()); // Define string as constant
+  param[kChannel] = ConvergenceChannel::ToJson(channel.GetHandle()); // Define string as constant
   param[kServiceResultType] = picojson::value(kServiceResultTypeOnStart);
 
-  common::TizenResult result = ConvergenceAppCommunicationService::Start(
-    channel, cur_listener_id);
+  common::TizenResult result = ConvergenceAppCommunicationService::Start(channel, cur_listener_id);
   if (!result) {
     return result;
   }
 
-  convergence_plugin_->ReplyAsync(kAppCommunicationListenerCallback,
-    cur_listener_id, true, param);
+  convergence_plugin_->ReplyAsync(kAppCommunicationListenerCallback, cur_listener_id, true, param);
   return result;
 }
 
 common::TizenResult ConvergenceAppCommunicationServerService::Stop(
-  ConvergenceChannel *channel,
+  ConvergenceChannel& channel,
   const int cur_listener_id) {
   ScopeLogger();
 
@@ -421,19 +360,15 @@ common::TizenResult ConvergenceAppCommunicationServerService::Stop(
   picojson::object param;
   param[kServiceListenerStatus] = picojson::value(kServiceListenerStatusOk);
 
-  // The object 'channel' will be deleted in the super::Start() so we should
-  // add its json before
-  param[kChannel] = ConvergenceChannel::ToJson(channel->GetHandle());
+  param[kChannel] = ConvergenceChannel::ToJson(channel.GetHandle());
   param[kServiceResultType] = picojson::value(kServiceResultTypeOnStop);
 
-  common::TizenResult result = ConvergenceAppCommunicationService::Stop(
-    channel, cur_listener_id);
+  common::TizenResult result = ConvergenceAppCommunicationService::Stop(channel, cur_listener_id);
   if (!result) {
     return result;
   }
 
-  convergence_plugin_->ReplyAsync(kAppCommunicationListenerCallback,
-    cur_listener_id, true, param);
+  convergence_plugin_->ReplyAsync(kAppCommunicationListenerCallback, cur_listener_id, true, param);
   return result;
 }
 

diff --git a/src/convergence/convergence_app_communication_service.h b/src/convergence/convergence_app_communication_service.h
index df35762c7d3861fcaa769030bf988653f7e30c4f..ca91bb38741044a2462302ab51bdcae6eb3ef4ec 100644 (file)
--- a/src/convergence/convergence_app_communication_service.h
+++ b/src/convergence/convergence_app_communication_service.h
@@ -43,13 +43,11 @@ class ConvergenceAppCommunicationService : public ConvergenceService {
   ConvergenceAppCommunicationService& operator=(const ConvergenceAppCommunicationService&) = delete;
   ConvergenceAppCommunicationService& operator=(ConvergenceAppCommunicationService&&) = delete;
  public:
-  virtual common::TizenResult Start(ConvergenceChannel *channel,
-   const int cur_listener_id);
-  virtual common::TizenResult Stop(ConvergenceChannel *channel,
-   const int cur_listener_id);
-  virtual common::TizenResult Send(ConvergenceChannel *channel,
-   ConvergencePayloadArray *payload,
-   const int cur_listener_id);
+  virtual common::TizenResult Start(ConvergenceChannel& channel, const int cur_listener_id);
+  virtual common::TizenResult Stop(ConvergenceChannel& channel, const int cur_listener_id);
+  virtual common::TizenResult Send(ConvergenceChannel& channel,
+                                   ConvergencePayloadArray& payload,
+                                   const int cur_listener_id);
   common::TizenResult SetListener(const int cur_listener_id);
   common::TizenResult RemoveListener();
  private:
@@ -73,9 +71,9 @@ class ConvergenceAppCommunicationServerService : public ConvergenceAppCommunicat
   ConvergenceAppCommunicationServerService& operator=(ConvergenceAppCommunicationServerService&&) = delete;
 
  public:
-  virtual common::TizenResult Start(ConvergenceChannel *channel,
+  virtual common::TizenResult Start(ConvergenceChannel& channel,
    const int cur_listener_id);
-  virtual common::TizenResult Stop(ConvergenceChannel *channel,
+  virtual common::TizenResult Stop(ConvergenceChannel& channel,
    const int cur_listener_id);
 };
 

diff --git a/src/convergence/convergence_instance.cc b/src/convergence/convergence_instance.cc
index 51316fcffad2b1355b25ecedf8e43dfefbabd1ee..a9d69022090e6385566523e8cc9d9bb808919e36 100644 (file)
--- a/src/convergence/convergence_instance.cc
+++ b/src/convergence/convergence_instance.cc
@@ -425,11 +425,16 @@ common::TizenResult ConvergenceInstance::AppCommunicationServiceStart(const pico
       result = LogAndCreateTizenError(NotFoundError, "Can not find the service type = 1");
     } else {
       // Running the service start procedure
-      result = service->Start(new ConvergenceChannel(ConvergenceUtils::GetArg(args, kJSArgumentChannel)),
-                              static_cast<int>(ConvergenceUtils::GetArg(args, kJSCurrentListenerId).get<double>()));
+      ConvergenceChannel channel(ConvergenceUtils::GetArg(args, kJSArgumentChannel));
+      const int id = static_cast<int>(ConvergenceUtils::GetArg(args, kJSCurrentListenerId).get<double>());
+
+      result = service->Start(channel, id);
     }
 
-    this->Post(token, result);
+    //in case of failure call error callback, success callback will be triggered by listener
+    if (!result) {
+      this->Post(token, result);
+    }
   };
 
   std::thread(start, token).detach();
@@ -463,13 +468,17 @@ common::TizenResult ConvergenceInstance::AppCommunicationServiceSend(const picoj
       result = LogAndCreateTizenError(NotFoundError, "Can not find the service type = 1");
     } else {
       // Running the service send procedure
-      result = service->Send(new ConvergenceChannel(ConvergenceUtils::GetArg(args, kJSArgumentChannel)),
-                          new ConvergencePayloadArray(ConvergenceUtils::GetArg(args, kJSArgumentPayload)),
-                          static_cast<int>(ConvergenceUtils::GetArg(args, kJSCurrentListenerId).get<double>()));
+      ConvergenceChannel channel(ConvergenceUtils::GetArg(args, kJSArgumentChannel));
+      ConvergencePayloadArray payload(ConvergenceUtils::GetArg(args, kJSArgumentPayload));
+      const int id = static_cast<int>(ConvergenceUtils::GetArg(args, kJSCurrentListenerId).get<double>());
 
+      result = service->Send(channel, payload, id);
     }
 
-    this->Post(token, result);
+    //in case of failure call error callback, success callback will be triggered by listener
+    if (!result) {
+      this->Post(token, result);
+    }
   };
 
   std::thread(send, token).detach();
@@ -502,11 +511,16 @@ common::TizenResult ConvergenceInstance::AppCommunicationServiceStop(const picoj
       result = LogAndCreateTizenError(NotFoundError, "Can not find the service type = 1");
     } else {
       // Running the service stop procedure
-      result = service->Stop(new ConvergenceChannel(ConvergenceUtils::GetArg(args, kJSArgumentChannel)),
-                             static_cast<int>(ConvergenceUtils::GetArg(args, kJSCurrentListenerId).get<double>()));
+      ConvergenceChannel channel(ConvergenceUtils::GetArg(args, kJSArgumentChannel));
+      const int id = static_cast<int>(ConvergenceUtils::GetArg(args, kJSCurrentListenerId).get<double>());
+
+      result = service->Stop(channel, id);
     }
 
-    this->Post(token, result);
+    //in case of failure call error callback, success callback will be triggered by listener
+    if (!result) {
+      this->Post(token, result);
+    }
   };
 
   std::thread(stop, token).detach();