From 76649fa8c3912bba84b272b9a12ca245390eabec Mon Sep 17 00:00:00 2001
From: Vickie Cheng <vickiecheng@google.com>
Date: Thu, 11 Apr 2019 03:10:20 +0000
Subject: [PATCH] [modular][config] Manually parse strings to enums
XDR expects an integer for enums so this updates modular_config_xdr.cc to
manually parse the config file for any fields that need to be converted to
enums.
This also moves story_shell_url to basemgr because basemgr is currently passing
it through to sessionmgr.
Change-Id: I19bf734132b1b8f6fada93f3b754a010554ca29b
---
.../modular_config/modular_config_schema.json | 12 +--
peridot/docs/modular/guide/config.md | 15 ++-
peridot/lib/modular_config/modular_config.cc | 11 +-
peridot/lib/modular_config/modular_config.h | 4 +-
.../lib/modular_config/modular_config_xdr.cc | 100 ++++++++++++++++--
.../modular_config_xdr_unittest.cc | 4 +-
peridot/tests/modular_config/test_config.json | 6 +-
7 files changed, 122 insertions(+), 30 deletions(-)
diff --git a/peridot/build/modular_config/modular_config_schema.json b/peridot/build/modular_config/modular_config_schema.json
index 175d78ff65f..5157ae10ebc 100644
--- a/peridot/build/modular_config/modular_config_schema.json
+++ b/peridot/build/modular_config/modular_config_schema.json
@@ -11,9 +11,14 @@
"use_minfs": { "type": "boolean", "default": true },
"use_session_shell_for_story_shell_factory": { "type": "boolean", "default": false },
"base_shell": { "$ref": "#definitions/base_shell" },
- "session_shells": {
+ "session_shells": {
"type": "array",
"items": { "$ref": "#/definitions/session_shell" }
+ },
+ "story_shell_url": {
+ "type": "string",
+ "pattern": "^fuchsia-pkg://([^/]+)/([^/#]+)(/([^/#]+))?(#(.+))?$",
+ "default": "fuchsia-pkg://fuchsia.com/mondrian#meta/mondrian.cmx"
}
},
"additionalProperties": false,
@@ -73,11 +78,6 @@
"pattern": "^fuchsia-pkg://([^/]+)/([^/#]+)(/([^/#]+))?(#(.+))?$"
},
"uniqueItems": true
- },
- "story_shell_url": {
- "type": "string",
- "pattern": "^fuchsia-pkg://([^/]+)/([^/#]+)(/([^/#]+))?(#(.+))?$",
- "default": "fuchsia-pkg://fuchsia.com/mondrian#meta/mondrian.cmx"
}
},
"additionalProperties": false
diff --git a/peridot/docs/modular/guide/config.md b/peridot/docs/modular/guide/config.md
index 3dc7ccd68ce..dbb04b79c13 100644
--- a/peridot/docs/modular/guide/config.md
+++ b/peridot/docs/modular/guide/config.md
@@ -56,11 +56,19 @@ modular_config() target in the product's monolith packages.
* The fuchsia component url for which session shell to use.
- `display_usage`: **string** *(optional)*
* The display usage policy for this session shell.
- * **Examples**: handheld, close, near, midrange, far
+ * Options:
+ - `handheld`: the display is used well within arm's reach.
+ - `close`: the display is used at arm's reach.
+ - `near`: the display is used beyond arm's reach.
+ - `midrange`: the display is used beyond arm's reach.
+ - `far`: the display is used well beyond arm's reach.
- `screen_height`: **float** *(optional)*
* The screen height in millimeters for the session shell's display.
- `screen_width`: **float** *(optional)*
* The screen width in millimeters for the session shell's display.
+* `story_shell_url`: **string** *(optional)*
+ - The fuchsia component url for which story shell to use.
+ - **default**: `fuchsia-pkg://fuchsia.com/mondrian#meta/mondrian.cmx`
* `enable_cobalt`: **boolean** *(optional)*
- When set to false, Cobalt statistics are disabled.
- **default**: `true`
@@ -106,7 +114,4 @@ modular_config() target in the product's monolith packages.
startup.
* `session_agents`: **string[]** *(optional)*
- A list of fuchsia component urls that specify which agents to launch at
- startup with PuppetMaster and FocusProvider services.
-* `story_shell_url`: **string** *(optional)*
- - The fuchsia component url for which story shell to use.
- - **default**: `fuchsia-pkg://fuchsia.com/mondrian#meta/mondrian.cmx`
\ No newline at end of file
+ startup with PuppetMaster and FocusProvider services.
\ No newline at end of file
diff --git a/peridot/lib/modular_config/modular_config.cc b/peridot/lib/modular_config/modular_config.cc
index 826f413640c..2a913ac8843 100644
--- a/peridot/lib/modular_config/modular_config.cc
+++ b/peridot/lib/modular_config/modular_config.cc
@@ -42,8 +42,7 @@ ModularConfigReader::GetSessionmgrConfig() {
// Parse with xdr
fuchsia::modular::internal::SessionmgrConfig sessionmgr_config;
- if (!sessionmgr_config_str.empty() &&
- !XdrRead(sessionmgr_config_str, &sessionmgr_config,
+ if (!XdrRead(sessionmgr_config_str, &sessionmgr_config,
XdrSessionmgrConfig)) {
FXL_LOG(ERROR) << "Unable to parse startup.json";
}
@@ -56,13 +55,13 @@ std::string ModularConfigReader::GetConfigAsString(
// Check that config file exists
if (!files::IsFile(kStartupConfigPath)) {
FXL_LOG(ERROR) << kStartupConfigPath << " does not exist.";
- return "";
+ return "\"\"";
}
std::string json;
if (!files::ReadFileToString(kStartupConfigPath, &json)) {
FXL_LOG(ERROR) << "Unable to read " << kStartupConfigPath;
- return "";
+ return "\"\"";
}
json::JSONParser json_parser;
@@ -70,7 +69,7 @@ std::string ModularConfigReader::GetConfigAsString(
if (json_parser.HasError()) {
FXL_LOG(ERROR) << "Error while parsing " << kStartupConfigPath
<< " to string. Error: " << json_parser.error_str();
- return "";
+ return "\"\"";
}
// Get |config_name| section from file
@@ -79,7 +78,7 @@ std::string ModularConfigReader::GetConfigAsString(
// |config_name| was not found
FXL_LOG(ERROR) << config_name << " configurations were not found in "
<< kStartupConfigPath;
- return "";
+ return "\"\"";
}
return modular::JsonValueToString(config_json->value);
diff --git a/peridot/lib/modular_config/modular_config.h b/peridot/lib/modular_config/modular_config.h
index 4d8b265c783..7a98fed43c6 100644
--- a/peridot/lib/modular_config/modular_config.h
+++ b/peridot/lib/modular_config/modular_config.h
@@ -23,8 +23,8 @@ class ModularConfigReader {
fuchsia::modular::internal::SessionmgrConfig GetSessionmgrConfig();
private:
- // Reads startup.config into a string if the file exists. Returns an empty
- // string otherwise.
+ // Reads startup.config into a string if the file exists. Otherwise, returns a
+ // string containing two quotes, representing an empty JSON structure.
std::string GetConfigAsString(const std::string& config_name);
};
diff --git a/peridot/lib/modular_config/modular_config_xdr.cc b/peridot/lib/modular_config/modular_config_xdr.cc
index 5c6b03c4581..376ad6235ca 100644
--- a/peridot/lib/modular_config/modular_config_xdr.cc
+++ b/peridot/lib/modular_config/modular_config_xdr.cc
@@ -33,6 +33,40 @@ void XdrBaseShellConfig(
has_keep_alive_after_login, false);
}
+fuchsia::ui::policy::DisplayUsage GetDisplayUsageFromString(std::string usage) {
+ if (usage == "handheld") {
+ return fuchsia::ui::policy::DisplayUsage::kHandheld;
+ } else if (usage == "close") {
+ return fuchsia::ui::policy::DisplayUsage::kClose;
+ } else if (usage == "near") {
+ return fuchsia::ui::policy::DisplayUsage::kNear;
+ } else if (usage == "midrange") {
+ return fuchsia::ui::policy::DisplayUsage::kMidrange;
+ } else if (usage == "far") {
+ return fuchsia::ui::policy::DisplayUsage::kFar;
+ }
+
+ FXL_LOG(WARNING) << "Unknown display usage string: " << usage;
+ return fuchsia::ui::policy::DisplayUsage::kUnknown;
+}
+
+std::string GetDisplayUsageAsString(fuchsia::ui::policy::DisplayUsage usage) {
+ switch (usage) {
+ case fuchsia::ui::policy::DisplayUsage::kUnknown:
+ return "unknown";
+ case fuchsia::ui::policy::DisplayUsage::kHandheld:
+ return "handheld";
+ case fuchsia::ui::policy::DisplayUsage::kClose:
+ return "close";
+ case fuchsia::ui::policy::DisplayUsage::kNear:
+ return "near";
+ case fuchsia::ui::policy::DisplayUsage::kMidrange:
+ return "midrange";
+ case fuchsia::ui::policy::DisplayUsage::kFar:
+ return "far";
+ }
+}
+
void XdrSessionShellMapEntry(
XdrContext* const xdr,
fuchsia::modular::internal::SessionShellMapEntry* const data) {
@@ -52,9 +86,24 @@ void XdrSessionShellMapEntry(
has_screen_width = data->config().has_screen_width();
}
- xdr->FieldWithDefault(
- "display_usage", data->mutable_config()->mutable_display_usage(),
- has_display_usage, fuchsia::ui::policy::DisplayUsage::kUnknown);
+ std::string display_usage_str = "unknown";
+ if (has_display_usage) {
+ display_usage_str = GetDisplayUsageAsString(data->config().display_usage());
+ }
+
+ // We need to manually parse any field in JSON that is a represented as a fidl
+ // enum because XDR expects a number, rather than a string, for enums.
+ // If writing, this will set the value of "display_usage" in JSON as the
+ // value of |display_usage|. If reading, this will read the value of
+ // "display_usage" into |display_usage|.
+ std::string display_usage;
+ xdr->FieldWithDefault("display_usage", &display_usage, false,
+ display_usage_str);
+
+ // This is only used when reading. We want to set the value read into
+ // |display_usage| into |data|.
+ auto display_usage_fidl = GetDisplayUsageFromString(display_usage);
+ data->mutable_config()->set_display_usage(display_usage_fidl);
xdr->FieldWithDefault("screen_height",
data->mutable_config()->mutable_screen_height(),
@@ -94,6 +143,29 @@ fuchsia::modular::internal::BaseShellConfig GetDefaultBaseShellConfig() {
return base_shell_config;
}
+fuchsia::modular::internal::CloudProvider GetCloudProviderFromString(
+ std::string provider) {
+ if (provider == "FROM_ENVIRONMENT") {
+ return fuchsia::modular::internal::CloudProvider::FROM_ENVIRONMENT;
+ } else if (provider == "NONE") {
+ return fuchsia::modular::internal::CloudProvider::NONE;
+ }
+
+ return fuchsia::modular::internal::CloudProvider::LET_LEDGER_DECIDE;
+}
+
+std::string GetCloudProviderAsString(
+ fuchsia::modular::internal::CloudProvider provider) {
+ switch (provider) {
+ case fuchsia::modular::internal::CloudProvider::LET_LEDGER_DECIDE:
+ return "LET_LEDGER_DECIDE";
+ case fuchsia::modular::internal::CloudProvider::FROM_ENVIRONMENT:
+ return "FROM_ENVIRONMENT";
+ case fuchsia::modular::internal::CloudProvider::NONE:
+ return "NONE";
+ }
+}
+
} // namespace
void XdrBasemgrConfig_v1(
@@ -158,9 +230,25 @@ void XdrSessionmgrConfig_v1(
XdrContext* const xdr,
fuchsia::modular::internal::SessionmgrConfig* const data) {
bool has_cloud_provider = data->has_cloud_provider();
- xdr->FieldWithDefault(
- "cloud_provider", data->mutable_cloud_provider(), has_cloud_provider,
- fuchsia::modular::internal::CloudProvider::LET_LEDGER_DECIDE);
+
+ std::string cloud_provider_str = "LET_LEDGER_DECIDE";
+ if (has_cloud_provider) {
+ cloud_provider_str = GetCloudProviderAsString(data->cloud_provider());
+ }
+
+ // We need to manually parse any field in JSON that is a represented as a
+ // fidl enum because XDR expects a number, rather than a string, for enums.
+ // If writing, this will set the value of "cloud_provider" in JSON as the
+ // value of |cloud_provider|. If reading, this will read the value of
+ // "cloud_provider" into |cloud_provider|.
+ std::string cloud_provider;
+ xdr->FieldWithDefault("cloud_provider", &cloud_provider, has_cloud_provider,
+ cloud_provider_str);
+
+ // This is only used when reading. We want to set the value read into
+ // |cloud_provider| into |data|.
+ auto cloud_provider_fidl = GetCloudProviderFromString(cloud_provider);
+ data->set_cloud_provider(cloud_provider_fidl);
bool has_enable_cobalt = data->has_enable_cobalt();
xdr->FieldWithDefault("enable_cobalt", data->mutable_enable_cobalt(),
diff --git a/peridot/lib/modular_config/modular_config_xdr_unittest.cc b/peridot/lib/modular_config/modular_config_xdr_unittest.cc
index 072bb08bbb8..fd1da3072e1 100644
--- a/peridot/lib/modular_config/modular_config_xdr_unittest.cc
+++ b/peridot/lib/modular_config/modular_config_xdr_unittest.cc
@@ -36,7 +36,7 @@ TEST(ModularConfigXdr, BasemgrDefaultValues) {
"session_shells":[
{
"url":"fuchsia-pkg://fuchsia.com/ermine_session_shell#meta/ermine_session_shell.cmx",
- "display_usage":0,
+ "display_usage":"unknown",
"screen_height":0.0,
"screen_width":0.0
}
@@ -89,7 +89,7 @@ TEST(ModularConfigXdr, SessionmgrDefaultValues) {
XdrWrite(&write_json, &write_config, XdrSessionmgrConfig);
std::string expected_json = R"({
- "cloud_provider":1,
+ "cloud_provider":"LET_LEDGER_DECIDE",
"enable_cobalt":true,
"enable_story_shell_preload":true,
"use_memfs_for_ledger":false,
diff --git a/peridot/tests/modular_config/test_config.json b/peridot/tests/modular_config/test_config.json
index fd4207400b2..e74c30ed7a3 100644
--- a/peridot/tests/modular_config/test_config.json
+++ b/peridot/tests/modular_config/test_config.json
@@ -13,7 +13,8 @@
"screen_height": 50,
"screen_width": 100
}
- ]
+ ],
+ "story_shell_url": "fuchsia-pkg://fuchsia.com/dev_story_shell#meta/dev_story_shell.cmx"
},
"sessionmgr": {
"use_memfs_for_ledger": true,
@@ -25,7 +26,6 @@
],
"session_agents": [
"fuchsia-pkg://fuchsia.com/session_agent#meta/session_agent.cmx"
- ],
- "story_shell_url": "fuchsia-pkg://fuchsia.com/dev_story_shell#meta/dev_story_shell.cmx"
+ ]
}
}
\ No newline at end of file
--
GitLab