brbzull0 commented on code in PR #9942:
URL: https://github.com/apache/trafficserver/pull/9942#discussion_r1283296777
##########
plugins/conf_remap/conf_remap.cc:
##########
@@ -122,16 +124,99 @@ RemapConfigs::parse_inline(const char *arg)
return true;
}
-// Config file parser, somewhat borrowed from P_RecCore.i
-bool
-RemapConfigs::parse_file(const char *filename)
+namespace
+{
+TSRecordDataType
+try_deduce_type(YAML::Node node)
+{
+ std::string_view tag = node.Tag();
+ if (tag == ts::Yaml::YAML_FLOAT_TAG_URI) {
+ return TS_RECORDDATATYPE_FLOAT;
+ } else if (tag == ts::Yaml::YAML_INT_TAG_URI) {
+ return TS_RECORDDATATYPE_INT;
+ } else if (tag == ts::Yaml::YAML_STR_TAG_URI) {
+ return TS_RECORDDATATYPE_STRING;
+ }
+ // we only care about string, int and float.
+ return TS_RECORDDATATYPE_NULL;
+}
+/// @brief Context class used to pass information to the TSYAMLRecNodeHandler
as the data obj.
+struct Context {
+ RemapConfigs::Item *items;
+ int *current;
+};
+
+TSReturnCode
+scalar_node_handler(const TSYAMLRecCfgFieldData *cfg, void *data)
{
- int line_num = 0;
- TSFile file;
- char buf[8192];
TSOverridableConfigKey name;
- TSRecordDataType type, expected_type;
+ TSRecordDataType expected_type;
+ std::string text;
+ auto &ctx = *static_cast<Context *>(data);
+ YAML::Node value = *reinterpret_cast<YAML::Node *>(cfg->value_node);
+
+ if (TSHttpTxnConfigFind(cfg->record_name, strlen(cfg->record_name), &name,
&expected_type) != TS_SUCCESS) {
+ TSError("[%s] '%s' is not a configuration variable or cannot be
overridden", PLUGIN_NAME, cfg->record_name);
+ return TS_ERROR;
+ }
+
+ RemapConfigs::Item *item = &ctx.items[*ctx.current];
+
+ auto type = try_deduce_type(value);
+
+ // If we detected a type but it's different from the one registered with the
in ATS, then we ignore it.
+ if (type != TS_RECORDDATATYPE_NULL && expected_type != type) {
+ TSError("%s", swoc::bwprint(text, "[{}] '{}' variable type mismatch,
expected {}, got {}", PLUGIN_NAME, cfg->record_name,
+ static_cast<int>(expected_type),
static_cast<int>(type))
+ .c_str());
+ return TS_ERROR; // Ignore the field
+ }
+
+ // We use the expected type. If no type set or the time did match, then it's
+ // safe to use the expected type.
+ try {
+ switch (expected_type) {
+ case TS_RECORDDATATYPE_INT:
+ item->_data.rec_int = value.as<int64_t>();
+ break;
+ case TS_RECORDDATATYPE_STRING: {
+ std::string str = value.as<std::string>();
+ if (str == "NULL") {
Review Comment:
@cmcfarlen : I've enhanced the null check to add `value.isNull()` as part
of the condition. I think both means null in terms of ATS.
I've also added an unit test to cover this sort of records.
Also, while testing this out I found a potential issue which I've fixed
[here](https://github.com/apache/trafficserver/pull/10135) (already merged).
Thanks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
