Handle errors on signal initialization

This reports errors at signals getSignals action call
and removed incorrect signals from the source. This allows
the binding to correctly respond to the user if there was an
error in its signal definitions.

Change-Id: I7779ab9339a979fcb2b7b41f8ebf4aa27b8af3a1
Signed-off-by: Romain Forlot <romain.forlot@iot.bzh>

5 files changed, 45 insertions, 13 deletions

diff --git a/signal-composer-binding/signal-composer-binding.cpp b/signal-composer-binding/signal-composer-binding.cpp
index 5932640..310cb40 100644
--- a/signal-composer-binding/signal-composer-binding.cpp
+++ b/signal-composer-binding/signal-composer-binding.cpp
@@ -211,13 +211,19 @@ void addObjects(afb_req request)
 
 		if(sourcesJ && composer.loadSources(sourcesJ))
 		{
-			afb_req_fail_f(request, "Loading 'sources' configuration or subscription error", "Error code: -2");
+			afb_req_fail_f(request, "Loading 'sources' configuration or subscription error", "Error code: -4");
 			return;
 		}
 		if(signalsJ)
 		{
 			if(!composer.loadSignals(signalsJ))
-				{composer.initSignals();}
+			{
+				if(composer.initSignals())
+				{
+					afb_req_fail_f(request, "Loading 'signals' configuration or subscription error", "Error code: -3");
+					return;
+				}
+			}
 			else
 			{
 				afb_req_fail_f(request, "Loading 'signals' configuration or subscription error", "Error code: -2");
diff --git a/signal-composer-binding/signal-composer.cpp b/signal-composer-binding/signal-composer.cpp
index 79b6022..13bfd64 100644
--- a/signal-composer-binding/signal-composer.cpp
+++ b/signal-composer-binding/signal-composer.cpp
@@ -480,14 +480,14 @@ void Composer::initSourcesAPI()
 	}
 }
 
-void Composer::initSignals()
+int Composer::initSignals()
 {
 	for(int i=0; i < sourcesListV_.size(); i++)
 	{
 		std::shared_ptr<SourceAPI> src = sourcesListV_[i];
 		src->initSignals();
 	}
-	execSignalsSubscription();
+	return execSignalsSubscription();
 }
 
 std::shared_ptr<SourceAPI> Composer::getSourceAPI(const std::string& api)
@@ -579,13 +579,16 @@ json_object* Composer::getsignalValue(const std::string& sig, json_object* optio
 	return finalResponse;
 }
 
-void Composer::execSignalsSubscription()
+int Composer::execSignalsSubscription()
 {
+	int err = 0;;
 	for(std::shared_ptr<SourceAPI> srcAPI: sourcesListV_)
 	{
 		if (srcAPI->api() != std::string(ctlConfig_->api))
 		{
-			srcAPI->makeSubscription();
+			err += srcAPI->makeSubscription();
 		}
 	}
+
+	return err;
 }
diff --git a/signal-composer-binding/signal-composer.hpp b/signal-composer-binding/signal-composer.hpp
index 7139a72..72cd3ee 100644
--- a/signal-composer-binding/signal-composer.hpp
+++ b/signal-composer-binding/signal-composer.hpp
@@ -42,7 +42,7 @@ private:
 	int loadOneSignal(json_object* signalsJ);
 	static int loadSignals(AFB_ApiT apihandle, CtlSectionT* section, json_object *signalsJ);
 
-	void execSignalsSubscription();
+	int execSignalsSubscription();
 	std::shared_ptr<SourceAPI> getSourceAPI(const std::string& api);
 	void processOptions(const std::map<std::string, int>& opts, std::shared_ptr<Signal> sig, json_object* response) const;
 public:
@@ -52,7 +52,7 @@ public:
 	int loadConfig(std::string& filepath);
 	int loadSources(json_object* sourcesJ);
 	int loadSignals(json_object* signalsJ);
-	void initSignals();
+	int initSignals();
 	void initSourcesAPI();
 
 	CtlConfigT* ctlConfig();
diff --git a/signal-composer-binding/source.cpp b/signal-composer-binding/source.cpp
index bd988c9..939e3f6 100644
--- a/signal-composer-binding/source.cpp
+++ b/signal-composer-binding/source.cpp
@@ -141,8 +141,25 @@ std::vector<std::shared_ptr<Signal>> SourceAPI::searchSignals(const std::string&
 	return signals;
 }
 
-void SourceAPI::makeSubscription()
+int SourceAPI::cleanNotSubscribedSignals() {
+	int erased = 0;
+	for(auto sig = signalsM_.begin(); sig != signalsM_.end();)
+	{
+		if(!sig->second->subscribed_)
+		{
+			sig = signalsM_.erase(sig);
+			erased++;
+		}
+		else
+			{sig++;}
+	}
+
+	return erased;
+}
+
+int SourceAPI::makeSubscription()
 {
+	int err = 0;
 	if(getSignals_)
 	{
 		CtlSourceT source;
@@ -168,13 +185,18 @@ void SourceAPI::makeSubscription()
 				getSignals_->argsJ = sig.second->getSignalsArgs() ?
 					sig.second->getSignalsArgs() :
 					argsSaveJ;
-				ActionExecOne(&source, getSignals_, signalJ);
-				// Considerate signal subscribed no matter what
-				sig.second->subscribed_ = true;
+
+				sig.second->subscribed_ = ActionExecOne(&source, getSignals_, signalJ) ? false : true;
 			}
 		}
+		err += cleanNotSubscribedSignals();
 		getSignals_->argsJ = argsSaveJ;
 		source.uid = uid_.c_str();
 		ActionExecOne(&source, getSignals_, nullptr);
 	}
+
+	if(err)
+		{AFB_ERROR("%d signal(s) removed because invalid. Please review your signal definition.", err);}
+
+	return err;
 }
diff --git a/signal-composer-binding/source.hpp b/signal-composer-binding/source.hpp
index 6585d0c..4fef274 100644
--- a/signal-composer-binding/source.hpp
+++ b/signal-composer-binding/source.hpp
@@ -53,5 +53,6 @@ public:
 	std::vector<std::shared_ptr<Signal>> getSignals() const;
 	std::vector<std::shared_ptr<Signal>> searchSignals(const std::string& name);
 
-	void makeSubscription();
+	int cleanNotSubscribedSignals();
+	int makeSubscription();
 };