From bbcdac0189b847718026c77023a9f2c24523c4ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sigbj=C3=B8rn=20Skj=C3=A6ret?= Date: Sun, 18 Jan 2026 03:40:06 +0100 Subject: [PATCH] jinja : fix object item order (and properly implement dictsort) (#18904) * fix object item order * as_ordered_object * copy whole object --- common/jinja/runtime.cpp | 12 ++++-------- common/jinja/runtime.h | 2 +- common/jinja/value.cpp | 38 +++++++++++++++----------------------- common/jinja/value.h | 12 ++++++++++-- 4 files changed, 30 insertions(+), 34 deletions(-) diff --git a/common/jinja/runtime.cpp b/common/jinja/runtime.cpp index 03ac3d16ee..d8ef279080 100644 --- a/common/jinja/runtime.cpp +++ b/common/jinja/runtime.cpp @@ -268,8 +268,7 @@ value binary_expression::execute_impl(context & ctx) { // String in object if (is_val(left_val) && is_val(right_val)) { auto key = left_val->as_string().str(); - auto & obj = right_val->as_object(); - bool has_key = obj.find(key) != obj.end(); + bool has_key = right_val->has_key(key); if (op.value == "in") { return mk_val(has_key); } else if (op.value == "not in") { @@ -464,7 +463,7 @@ value for_statement::execute_impl(context & ctx) { std::vector items; if (is_val(iterable_val)) { JJ_DEBUG("%s", "For loop over object keys"); - auto & obj = iterable_val->as_object(); + auto & obj = iterable_val->as_ordered_object(); for (auto & p : obj) { auto tuple = mk_val(); if (iterable_val->val_obj.is_key_numeric) { @@ -779,11 +778,8 @@ value member_expression::execute_impl(context & ctx) { throw std::runtime_error("Cannot access object with non-string: got " + property->type()); } auto key = property->as_string().str(); - auto & obj = object->as_object(); - auto it = obj.find(key); - if (it != obj.end()) { - val = it->second; - } else { + val = object->at(key, val); + if (is_val(val)) { val = try_builtin_func(ctx, key, object, true); } JJ_DEBUG("Accessed property '%s' value, got type: %s", key.c_str(), val->type().c_str()); diff --git a/common/jinja/runtime.h b/common/jinja/runtime.h index de42b66c5f..dc7f4e471c 100644 --- a/common/jinja/runtime.h +++ b/common/jinja/runtime.h @@ -69,7 +69,7 @@ struct context { context(const context & parent) : context() { // inherit variables (for example, when entering a new scope) - auto & pvar = parent.env->as_object(); + auto & pvar = parent.env->as_ordered_object(); for (const auto & pair : pvar) { set_val(pair.first, pair.second); } diff --git a/common/jinja/value.cpp b/common/jinja/value.cpp index 4e309c1ad0..e414aad444 100644 --- a/common/jinja/value.cpp +++ b/common/jinja/value.cpp @@ -929,18 +929,13 @@ const func_builtins & value_object_t::get_builtins() const { if (args.count() == 3) { default_val = args.get_pos(2); } - const auto & obj = args.get_pos(0)->as_object(); + const value obj = args.get_pos(0); std::string key = args.get_pos(1)->as_string().str(); - auto it = obj.find(key); - if (it != obj.end()) { - return it->second; - } else { - return default_val; - } + return obj->at(key, default_val); }}, {"keys", [](const func_args & args) -> value { args.ensure_vals(); - const auto & obj = args.get_pos(0)->as_object(); + const auto & obj = args.get_pos(0)->as_ordered_object(); auto result = mk_val(); for (const auto & pair : obj) { result->push_back(mk_val(pair.first)); @@ -949,7 +944,7 @@ const func_builtins & value_object_t::get_builtins() const { }}, {"values", [](const func_args & args) -> value { args.ensure_vals(); - const auto & obj = args.get_pos(0)->as_object(); + const auto & obj = args.get_pos(0)->as_ordered_object(); auto result = mk_val(); for (const auto & pair : obj) { result->push_back(pair.second); @@ -958,7 +953,7 @@ const func_builtins & value_object_t::get_builtins() const { }}, {"items", [](const func_args & args) -> value { args.ensure_vals(); - const auto & obj = args.get_pos(0)->as_object(); + const auto & obj = args.get_pos(0)->as_ordered_object(); auto result = mk_val(); for (const auto & pair : obj) { auto item = mk_val(); @@ -972,7 +967,7 @@ const func_builtins & value_object_t::get_builtins() const { {"string", tojson}, {"length", [](const func_args & args) -> value { args.ensure_vals(); - const auto & obj = args.get_pos(0)->as_object(); + const auto & obj = args.get_pos(0)->as_ordered_object(); return mk_val(static_cast(obj.size())); }}, {"tojson", [](const func_args & args) -> value { @@ -988,18 +983,15 @@ const func_builtins & value_object_t::get_builtins() const { // FIXME: sorting is currently always case sensitive //const bool case_sensitive = val_case->as_bool(); // undefined == false const bool reverse = val_reverse->as_bool(); // undefined == false - if (!val_by->is_undefined()) { - throw not_implemented_exception("dictsort by key not implemented"); - } - if (reverse) { - throw not_implemented_exception("dictsort reverse not implemented"); - } - value_t::map obj = val_input->val_obj; // copy - std::sort(obj.ordered.begin(), obj.ordered.end(), [&](const auto & a, const auto & b) { - return a.first < b.first; + const bool by_value = is_val(val_by) && val_by->as_string().str() == "value" ? true : false; + auto result = mk_val(val_input); // copy + std::sort(result->val_obj.ordered.begin(), result->val_obj.ordered.end(), [&](const auto & a, const auto & b) { + if (by_value) { + return value_compare(a.second, b.second, reverse ? value_compare_op::gt : value_compare_op::lt); + } else { + return reverse ? a.first > b.first : a.first < b.first; + } }); - auto result = mk_val(); - result->val_obj = std::move(obj); return result; }}, {"join", [](const func_args &) -> value { @@ -1196,7 +1188,7 @@ static void value_to_json_internal(std::ostringstream & oss, const value & val, } oss << "]"; } else if (is_val(val)) { - const auto & obj = val->val_obj.ordered; // IMPORTANT: need to keep exact order + const auto & obj = val->as_ordered_object(); // IMPORTANT: need to keep exact order oss << "{"; if (!obj.empty()) { oss << newline(); diff --git a/common/jinja/value.h b/common/jinja/value.h index 6ccb9e5611..4e916919b3 100644 --- a/common/jinja/value.h +++ b/common/jinja/value.h @@ -146,7 +146,7 @@ struct value_t { virtual string as_string() const { throw std::runtime_error(type() + " is not a string value"); } virtual bool as_bool() const { throw std::runtime_error(type() + " is not a bool value"); } virtual const std::vector & as_array() const { throw std::runtime_error(type() + " is not an array value"); } - virtual const std::map & as_object() const { throw std::runtime_error(type() + " is not an object value"); } + virtual const std::vector> & as_ordered_object() const { throw std::runtime_error(type() + " is not an object value"); } virtual value invoke(const func_args &) const { throw std::runtime_error(type() + " is not a function value"); } virtual bool is_none() const { return false; } virtual bool is_undefined() const { return false; } @@ -154,6 +154,9 @@ struct value_t { throw std::runtime_error("No builtins available for type " + type()); } + virtual bool has_key(const std::string & key) { + return val_obj.unordered.find(key) != val_obj.unordered.end(); + } virtual value & at(const std::string & key, value & default_val) { auto it = val_obj.unordered.find(key); if (it == val_obj.unordered.end()) { @@ -308,11 +311,16 @@ struct value_object_t : public value_t { val_obj.insert(pair.first, pair.second); } } + value_object_t(const std::vector> & obj) { + for (const auto & pair : obj) { + val_obj.insert(pair.first, pair.second); + } + } void insert(const std::string & key, const value & val) { val_obj.insert(key, val); } virtual std::string type() const override { return "Object"; } - virtual const std::map & as_object() const override { return val_obj.unordered; } + virtual const std::vector> & as_ordered_object() const override { return val_obj.ordered; } virtual bool as_bool() const override { return !val_obj.unordered.empty(); }