diff --git a/data/test/config_compiler_test.yaml b/data/test/config_compiler_test.yaml index 408f672247..d28fe029c5 100644 --- a/data/test/config_compiler_test.yaml +++ b/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 diff --git a/src/rime/config/config_compiler.cc b/src/rime/config/config_compiler.cc index e33a7c39df..7dce33783a 100644 --- a/src/rime/config/config_compiler.cc +++ b/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 ? " " : ""); } template @@ -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(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) { @@ -267,10 +271,8 @@ an ConfigCompiler::Compile(const string& file_name) { auto resource = New(resource_id, New()); 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 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 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; } diff --git a/src/rime/config/config_compiler.h b/src/rime/config/config_compiler.h index d243fe17f7..efeb0fe9ec 100644 --- 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 data; + bool loaded = false; ConfigResource(const string& _id, an _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; }; diff --git a/src/rime/config/config_component.cc b/src/rime/config/config_component.cc index fb0822be04..e787aa04b6 100644 --- a/src/rime/config/config_component.cc +++ b/src/rime/config/config_component.cc @@ -167,12 +167,9 @@ an 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(); - } wp = resource->data; return resource->data; } diff --git a/test/config_compiler_test.cc b/test/config_compiler_test.cc index 69990ddbde..533d53b1ba 100644 --- a/test/config_compiler_test.cc +++ b/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); +} diff --git a/test/config_test.cc b/test/config_test.cc index 30c5ecaf7c..2bd681cb0a 100644 --- a/test/config_test.cc +++ b/test/config_test.cc @@ -27,16 +27,27 @@ class RimeConfigTest : public ::testing::Test { the 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(cc->Create("config_test")); - EXPECT_TRUE(bool(config)); + // create config and write modifications to file + { + the 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(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"); }