config: support optional references; fix auto saving

data/test/config_compiler_test.yaml

@@ -52,6 +52,14 @@ dependency_priorities:
       player: bisu
     __include: /starcraft/protoss
 
+optional_reference:
+  __include: nonexistent.yaml:/?
+  __patch:
+    - local/nonexistent_patch?
+    - config_test:/nonexistent_patch?
+    - nonexistent:/patch?
+  untouched: true
+
 local:
   patch:
     battlefields/@next: match point

src/rime/config/config_compiler.cc

@@ -46,7 +46,7 @@ struct PendingChild : Dependency {
 };
 
 string Reference::repr() const {
-  return resource_id + ":" + local_path;
+  return resource_id + ":" + local_path + (optional ? " <optional>" : "");
 }
 
 template <class StreamT>
@@ -131,7 +131,7 @@ bool IncludeReference::Resolve(ConfigCompiler* compiler) {
   DLOG(INFO) << "IncludeReference::Resolve(reference = " << reference << ")";
   auto item = ResolveReference(compiler, reference);
   if (!item) {
-    return false;
+    return reference.optional;
   }
   *target = item;
   return true;
@@ -141,7 +141,7 @@ bool PatchReference::Resolve(ConfigCompiler* compiler) {
   DLOG(INFO) << "PatchReference::Resolve(reference = " << reference << ")";
   auto item = ResolveReference(compiler, reference);
   if (!item) {
-    return false;
+    return reference.optional;
   }
   if (!Is<ConfigMap>(item)) {
     LOG(ERROR) << "invalid patch at " << reference;
@@ -227,14 +227,18 @@ ConfigCompiler::~ConfigCompiler() {
 }
 
 Reference ConfigCompiler::CreateReference(const string& qualified_path) {
+  auto end = qualified_path.find_last_of("?");
+  bool optional = end != string::npos;
   auto separator = qualified_path.find_first_of(":");
   string resource_id = resource_resolver_->ToResourceId(
       (separator == string::npos || separator == 0) ?
-      graph_->current_resource_id() : qualified_path.substr(0, separator));
+      graph_->current_resource_id() :
+      qualified_path.substr(0, separator));
   string local_path = (separator == string::npos) ?
-      qualified_path :
-      qualified_path.substr(separator + 1);
-  return Reference{resource_id, local_path};
+      qualified_path.substr(0, end) :
+      qualified_path.substr(separator + 1,
+                            optional ? end - separator - 1 : end);
+  return Reference{resource_id, local_path, optional};
 }
 
 void ConfigCompiler::AddDependency(an<Dependency> dependency) {
@@ -267,10 +271,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;
 }
@@ -356,8 +358,16 @@ static an<ConfigItem> ResolveReference(ConfigCompiler* compiler,
                                        const Reference& reference) {
   auto resource = compiler->GetCompiledResource(reference.resource_id);
   if (!resource) {
-    LOG(INFO) << "resource not loaded, compiling: " << reference.resource_id;
+    DLOG(INFO) << "resource not loaded, compiling: " << reference.resource_id;
     resource = compiler->Compile(reference.resource_id);
+    if (!resource->loaded) {
+      if (reference.optional) {
+        LOG(INFO) << "optional resource not loaded: " << reference.resource_id;
+      } else {
+        LOG(ERROR) << "resource could not be loaded: " << reference.resource_id;
+      }
+      return nullptr;
+    }
   }
   return GetResolvedItem(compiler, resource, reference.local_path);
 }
@@ -439,6 +449,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 +461,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;
 }
 

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) {
@@ -29,6 +30,7 @@ struct ConfigResource : ConfigItemRef {
 struct Reference {
   string resource_id;
   string local_path;
+  bool optional;
 
   string repr() const;
 };

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;
   }

test/config_compiler_test.cc

@@ -129,4 +129,11 @@ TEST_F(RimeConfigCompilerTest, DependencyPriorities) {
   EXPECT_EQ("bisu", player);
 }
 
-// TODO: test failure cases
+TEST_F(RimeConfigCompilerTest, OptionalReference) {
+  const string& prefix = "optional_reference/";
+  EXPECT_TRUE(config_->IsNull(prefix + "__include"));
+  EXPECT_TRUE(config_->IsNull(prefix + "__patch"));
+  bool untouched;
+  EXPECT_TRUE(config_->GetBool(prefix + "untouched", &untouched));
+  EXPECT_TRUE(untouched);
+}

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");
 }