Merge pull request #170 from unnonouno/feature/catch-bad-cast

Feature/catch bad cast

src/framework/server_util.cpp

@@ -27,11 +27,6 @@
 #include "../common/exception.hpp"
 #include "../common/membership.hpp"
 
-#include "../fv_converter/datum_to_fv_converter.hpp"
-#include "../fv_converter/converter_config.hpp"
-#include "../fv_converter/exception.hpp"
-
-
 namespace jubatus { namespace framework {
 
   static const std::string VERSION(JUBATUS_VERSION);
@@ -244,19 +239,5 @@ namespace jubatus { namespace framework {
 #endif
   }
 
-pfi::lang::shared_ptr<fv_converter::datum_to_fv_converter>
-make_fv_converter(const std::string& config) {
-  if (config == "")
-    throw JUBATUS_EXCEPTION(fv_converter::converter_exception("Config of feature vector converter is empty"));
-  pfi::lang::shared_ptr<fv_converter::datum_to_fv_converter>
-      converter(new fv_converter::datum_to_fv_converter);
-  fv_converter::converter_config c;
-  std::stringstream ss(config);
-  // FIXME: check invalid json format
-  ss >> pfi::text::json::via_json(c);
-  fv_converter::initialize_converter(c, *converter);
-  return converter;
-}
-
 }
 }

src/framework/server_util.hpp

@@ -126,7 +126,4 @@ int run_server(int args, char** argv, const std::string& type)
   }
 }
 
-pfi::lang::shared_ptr<fv_converter::datum_to_fv_converter>
-make_fv_converter(const std::string& config);
-
 }}

src/framework/server_util_test.cpp

@@ -6,9 +6,5 @@
 namespace jubatus {
 namespace framework {
 
-TEST(make_fv_converter, empty) {
-  EXPECT_THROW(jubatus::framework::make_fv_converter(""), fv_converter::converter_exception);
-}
-
 }
 }

src/fv_converter/converter_config.cpp

@@ -15,6 +15,9 @@
 // Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA
 
 #include "converter_config.hpp"
+
+#include <pficommon/text/json.h>
+
 #include "datum_to_fv_converter.hpp"
 #include "splitter_factory.hpp"
 #include "key_matcher.hpp"
@@ -236,5 +239,29 @@ void initialize_converter(const converter_config& config,
   }
 }
 
+pfi::lang::shared_ptr<datum_to_fv_converter>
+make_fv_converter(const std::string& config) {
+  if (config == "")
+    throw JUBATUS_EXCEPTION(fv_converter::converter_exception("Config of feature vector converter is empty"));
+  pfi::lang::shared_ptr<fv_converter::datum_to_fv_converter>
+      converter(new fv_converter::datum_to_fv_converter);
+  converter_config c;
+  std::stringstream ss(config);
+  try {
+    ss >> pfi::text::json::via_json(c);
+  } catch (pfi::lang::end_of_data& e) {
+    std::string msg = std::string("Unexpected end of string is detected: ") + e.what();
+    throw JUBATUS_EXCEPTION(fv_converter::converter_exception(msg.c_str()));
+  } catch (pfi::lang::parse_error& e) {
+    std::string msg = std::string("Cannot parse config JSON: ") + e.what();
+    throw JUBATUS_EXCEPTION(fv_converter::converter_exception(msg.c_str()));
+  } catch (std::bad_cast& e) {
+    std::string msg = std::string("Invalid config format: ") + e.what();
+    throw JUBATUS_EXCEPTION(fv_converter::converter_exception(msg.c_str()));
+  }
+  fv_converter::initialize_converter(c, *converter);
+  return converter;
+}
+
 }
 }

src/fv_converter/converter_config.hpp

@@ -21,7 +21,7 @@
 #include <map>
 #include <pficommon/data/serialization.h>
 #include <pficommon/data/optional.h>
-
+#include <pficommon/lang/shared_ptr.h>
 #include <pficommon/network/mprpc.h>
 #include <msgpack.hpp>
 
@@ -119,5 +119,8 @@ struct converter_config {
 void initialize_converter(const converter_config& config,
                           datum_to_fv_converter& converter);
 
+pfi::lang::shared_ptr<datum_to_fv_converter>
+make_fv_converter(const std::string& config);
+
 }
 }

src/fv_converter/converter_config_test.cpp

@@ -105,3 +105,20 @@ TEST(converter_config, hash_negative) {
 
   EXPECT_THROW(initialize_converter(config, conv), converter_exception);
 }
+
+TEST(make_fv_converter, empty) {
+  EXPECT_THROW(make_fv_converter(""), fv_converter::converter_exception);
+}
+
+TEST(make_fv_converter, invalid_config_json) {
+  EXPECT_THROW(make_fv_converter("{"), fv_converter::converter_exception);
+}
+
+TEST(make_fv_converter, config_json_parse_error) {
+  EXPECT_THROW(make_fv_converter("AA"), fv_converter::converter_exception);
+}
+
+TEST(make_fv_converter, config_cast_error) {
+  EXPECT_THROW(make_fv_converter("{}"), fv_converter::converter_exception);
+}
+

src/server/classifier_serv.cpp

@@ -22,6 +22,7 @@
 #include "../framework/mixer/mixer_factory.hpp"
 #include "../fv_converter/datum.hpp"
 #include "../fv_converter/datum_to_fv_converter.hpp"
+#include "../fv_converter/converter_config.hpp"
 #include "../storage/storage_factory.hpp"
 
 using namespace std;
@@ -70,7 +71,7 @@ int classifier_serv::set_config(const config_data& config) {
   LOG(INFO) << __func__;
 
   shared_ptr<datum_to_fv_converter> converter =
-      framework::make_fv_converter(config.config);
+      fv_converter::make_fv_converter(config.config);
 
   config_ = config;
   converter_ = converter;

src/server/recommender_serv.cpp

@@ -63,7 +63,7 @@ void recommender_serv::get_status(status_t& status) const {
 int recommender_serv::set_config(config_data config) {
   LOG(INFO) << __func__;
   shared_ptr<fv_converter::datum_to_fv_converter> converter
-      = framework::make_fv_converter(config.converter);
+      = fv_converter::make_fv_converter(config.converter);
   config_ = config;
   converter_ = converter;
   rcmdr_.set_model(make_model());

src/server/regression_serv.cpp

@@ -22,6 +22,7 @@
 #include "../framework/mixer/mixer_factory.hpp"
 #include "../fv_converter/datum.hpp"
 #include "../fv_converter/datum_to_fv_converter.hpp"
+#include "../fv_converter/converter_config.hpp"
 #include "../storage/storage_factory.hpp"
 
 using namespace std;
@@ -70,7 +71,7 @@ int regression_serv::set_config(const config_data& config) {
   LOG(INFO) << __func__;
 
   shared_ptr<datum_to_fv_converter> converter
-      = framework::make_fv_converter(config.config);
+      = fv_converter::make_fv_converter(config.config);
 
   config_ = config;
   converter_ = converter;