fix(config): auto save modified config data; fixes #144

rime · Sep 7, 2017 · 2736f4b · 2736f4b
1 parent bcc4d10
commit 2736f4b
Show file tree

Hide file tree

Showing 4 changed files with 27 additions and 13 deletions.
diff --git a/src/rime/config/config_compiler.cc b/src/rime/config/config_compiler.cc
@@ -267,10 +267,8 @@ an<ConfigResource> ConfigCompiler::Compile(const string& file_name) {
   auto resource = New<ConfigResource>(resource_id, New<ConfigData>());
   graph_->resources[resource_id] = resource;
   graph_->Push(resource, resource_id + ":");
-  if (!resource->data->LoadFromFile(
-          resource_resolver_->ResolvePath(resource_id).string(), this)) {
-    resource.reset();
-  }
+  resource->loaded = resource->data->LoadFromFile(
+      resource_resolver_->ResolvePath(resource_id).string(), this);
   graph_->Pop();
   return resource;
 }
@@ -358,6 +356,10 @@ static an<ConfigItem> ResolveReference(ConfigCompiler* compiler,
   if (!resource) {
     LOG(INFO) << "resource not loaded, compiling: " << reference.resource_id;
     resource = compiler->Compile(reference.resource_id);
+    if (!resource->loaded) {
+      LOG(ERROR) << "resource could not be loaded: " << reference.resource_id;
+      return nullptr;
+    }
   }
   return GetResolvedItem(compiler, resource, reference.local_path);
 }
@@ -439,6 +441,9 @@ bool ConfigCompiler::Link(an<ConfigResource> target) {
 
 bool ConfigCompiler::ResolveDependencies(const string& path) {
   DLOG(INFO) << "ResolveDependencies(" << path << ")";
+  if (!graph_->deps.count(path)) {
+    return true;
+  }
   auto& deps = graph_->deps[path];
   for (auto iter = deps.begin(); iter != deps.end(); ) {
     if (!(*iter)->Resolve(this)) {
@@ -448,7 +453,7 @@ bool ConfigCompiler::ResolveDependencies(const string& path) {
     LOG(INFO) << "resolved: " << **iter;
     iter = deps.erase(iter);
   }
-  LOG(INFO) << "all dependencies resolved.";
+  DLOG(INFO) << "all dependencies resolved.";
   return true;
 }
 

diff --git a/src/rime/config/config_compiler.h b/src/rime/config/config_compiler.h
@@ -14,6 +14,7 @@ namespace rime {
 struct ConfigResource : ConfigItemRef {
   string resource_id;
   an<ConfigData> data;
+  bool loaded = false;
 
   ConfigResource(const string& _id, an<ConfigData> _data)
       : ConfigItemRef(nullptr), resource_id(_id), data(_data) {

diff --git a/src/rime/config/config_component.cc b/src/rime/config/config_component.cc
@@ -167,12 +167,9 @@ an<ConfigData> ConfigComponent::GetConfigData(const string& file_name) {
   if (wp.expired()) {  // create a new copy and load it
     ConfigCompiler compiler(resource_resolver_.get());
     auto resource = compiler.Compile(file_name);
-    if (!resource || !compiler.Link(resource)) {
+    if (resource->loaded && !compiler.Link(resource)) {
       LOG(ERROR) << "error loading config from: " << file_name;
     }
-    if (!resource) {
-      return New<ConfigData>();
-    }
     wp = resource->data;
     return resource->data;
   }

diff --git a/test/config_test.cc b/test/config_test.cc
@@ -27,16 +27,27 @@ class RimeConfigTest : public ::testing::Test {
   the<Config> config_;
 };
 
-TEST(RimeConfigComponentTest, RealCreationWorkflow) {
+TEST(RimeConfigComponentTest, RoundTrip) {
   // registration
   Registry& r = Registry::instance();
   r.Register("test_config", new ConfigComponent);
   // find component
   Config::Component* cc = Config::Require("test_config");
   ASSERT_TRUE(cc != NULL);
-  // create Config with id
-  the<Config> config(cc->Create("config_test"));
-  EXPECT_TRUE(bool(config));
+  // create config and write modifications to file
+  {
+    the<Config> config(cc->Create("config_round_trip_test"));
+    EXPECT_TRUE(bool(config));
+    EXPECT_TRUE(config->SetString("key", "value"));
+  }
+  // read from file and verify contents
+  {
+    the<Config> config(cc->Create("config_round_trip_test"));
+    EXPECT_TRUE(bool(config));
+    string value;
+    EXPECT_TRUE(config->GetString("key", &value));
+    EXPECT_EQ("value", value);
+  }
   r.Unregister("test_config");
 }