From bf01601f11454cdf7f26c674893ccbca7c09a1ce Mon Sep 17 00:00:00 2001 From: Adrian Vovk Date: Wed, 3 Jul 2024 17:51:35 -0400 Subject: [PATCH 1/3] table: Add TABLE_SET_JSON_FIELD_NAME Lets you conveniently set JSON field names in table_add_many. Especially useful for vertical tables. For example: table_add_many(t, TABLE_FIELD, "Display Name", TABLE_STRING, obj->display_name, TABLE_SET_JSON_FIELD_NAME, "displayName", TABLE_FIELD, "Timestamp", TABLE_TIMESTAMP, obj->timestamp, TABLE_SET_JSON_FIELD_NAME, "timestampUSec"); --- src/shared/format-table.c | 14 ++++++++++++++ src/shared/format-table.h | 1 + src/test/test-format-table.c | 24 +++++++++++++++++++----- 3 files changed, 34 insertions(+), 5 deletions(-) diff --git a/src/shared/format-table.c b/src/shared/format-table.c index 14eeb4647f..e538d96414 100644 --- a/src/shared/format-table.c +++ b/src/shared/format-table.c @@ -1184,6 +1184,20 @@ int table_add_many_internal(Table *t, TableDataType first_type, ...) { goto check; } + case TABLE_SET_JSON_FIELD_NAME: { + const char *n = va_arg(ap, const char*); + size_t idx; + if (t->vertical) { + assert(TABLE_CELL_TO_INDEX(last_cell) >= t->n_columns); + idx = TABLE_CELL_TO_INDEX(last_cell) / t->n_columns - 1; + } else { + idx = TABLE_CELL_TO_INDEX(last_cell); + assert(idx < t->n_columns); + } + r = table_set_json_field_name(t, idx, n); + goto check; + } + case _TABLE_DATA_TYPE_MAX: /* Used as end marker */ va_end(ap); diff --git a/src/shared/format-table.h b/src/shared/format-table.h index dcd339727f..15c0547b94 100644 --- a/src/shared/format-table.h +++ b/src/shared/format-table.h @@ -74,6 +74,7 @@ typedef enum TableDataType { TABLE_SET_BOTH_UNDERLINES, TABLE_SET_URL, TABLE_SET_UPPERCASE, + TABLE_SET_JSON_FIELD_NAME, _TABLE_DATA_TYPE_INVALID = -EINVAL, } TableDataType; diff --git a/src/test/test-format-table.c b/src/test/test-format-table.c index 85cd31ecee..fa34c29b02 100644 --- a/src/test/test-format-table.c +++ b/src/test/test-format-table.c @@ -364,18 +364,27 @@ TEST(json) { _cleanup_(sd_json_variant_unrefp) sd_json_variant *v = NULL, *w = NULL; _cleanup_(table_unrefp) Table *t = NULL; - assert_se(t = table_new("foo bar", "quux", "piep miau")); + assert_se(t = table_new_raw(4)); + + assert_se(table_add_many(t, + TABLE_HEADER, "foo bar", + TABLE_HEADER, "quux", + TABLE_HEADER, "piep miau", + TABLE_HEADER, "asdf", + TABLE_SET_JSON_FIELD_NAME, "asdf-custom") >= 0); assert_se(table_set_json_field_name(t, 2, "zzz") >= 0); assert_se(table_add_many(t, TABLE_STRING, "v1", TABLE_UINT64, UINT64_C(4711), - TABLE_BOOLEAN, true) >= 0); + TABLE_BOOLEAN, true, + TABLE_EMPTY) >= 0); assert_se(table_add_many(t, TABLE_STRV, STRV_MAKE("a", "b", "c"), TABLE_EMPTY, - TABLE_MODE, 0755) >= 0); + TABLE_MODE, 0755, + TABLE_EMPTY) >= 0); assert_se(table_to_json(t, &v) >= 0); @@ -384,11 +393,13 @@ TEST(json) { SD_JSON_BUILD_OBJECT( SD_JSON_BUILD_PAIR("foo_bar", JSON_BUILD_CONST_STRING("v1")), SD_JSON_BUILD_PAIR("quux", SD_JSON_BUILD_UNSIGNED(4711)), - SD_JSON_BUILD_PAIR("zzz", SD_JSON_BUILD_BOOLEAN(true))), + SD_JSON_BUILD_PAIR("zzz", SD_JSON_BUILD_BOOLEAN(true)), + SD_JSON_BUILD_PAIR("asdf-custom", SD_JSON_BUILD_NULL)), SD_JSON_BUILD_OBJECT( SD_JSON_BUILD_PAIR("foo_bar", SD_JSON_BUILD_STRV(STRV_MAKE("a", "b", "c"))), SD_JSON_BUILD_PAIR("quux", SD_JSON_BUILD_NULL), - SD_JSON_BUILD_PAIR("zzz", SD_JSON_BUILD_UNSIGNED(0755))))) >= 0); + SD_JSON_BUILD_PAIR("zzz", SD_JSON_BUILD_UNSIGNED(0755)), + SD_JSON_BUILD_PAIR("asdf-custom", SD_JSON_BUILD_NULL)))) >= 0); assert_se(sd_json_variant_equal(v, w)); } @@ -544,6 +555,7 @@ TEST(vertical) { assert_se(table_add_many(t, TABLE_FIELD, "pfft aa", TABLE_STRING, "foo", TABLE_FIELD, "uuu o", TABLE_SIZE, UINT64_C(1024), + TABLE_FIELD, "quux", TABLE_STRING, "asdf", TABLE_SET_JSON_FIELD_NAME, "custom-quux", TABLE_FIELD, "lllllllllllo", TABLE_STRING, "jjjjjjjjjjjjjjjjj") >= 0); assert_se(table_set_json_field_name(t, 1, "dimpfelmoser") >= 0); @@ -553,6 +565,7 @@ TEST(vertical) { assert_se(streq(formatted, " pfft aa: foo\n" " uuu o: 1K\n" + " quux: asdf\n" "lllllllllllo: jjjjjjjjjjjjjjjjj\n")); _cleanup_(sd_json_variant_unrefp) sd_json_variant *a = NULL, *b = NULL; @@ -561,6 +574,7 @@ TEST(vertical) { assert_se(sd_json_build(&b, SD_JSON_BUILD_OBJECT( SD_JSON_BUILD_PAIR("pfft_aa", SD_JSON_BUILD_STRING("foo")), SD_JSON_BUILD_PAIR("dimpfelmoser", SD_JSON_BUILD_UNSIGNED(1024)), + SD_JSON_BUILD_PAIR("custom-quux", SD_JSON_BUILD_STRING("asdf")), SD_JSON_BUILD_PAIR("lllllllllllo", SD_JSON_BUILD_STRING("jjjjjjjjjjjjjjjjj")))) >= 0); assert_se(sd_json_variant_equal(a, b)); From c4cb49eb342bb347fb974eb55236d2eb3e565086 Mon Sep 17 00:00:00 2001 From: Adrian Vovk Date: Wed, 3 Jul 2024 17:57:42 -0400 Subject: [PATCH 2/3] table: Improve mangling of JSON field names First, when displaying JSON we convert dashes into underscores. We want to avoid using dashes in JSON field names in new code, because some JSON parsers don't support dashes very well. Second, we make the first character of every word lower-case. This better matches our JSON field name style, and makes the automatic JSON name mangling a lot more useful for vertical tables, where fields are given a display name. For example, "Foo Bar" would be converted into "foo_bar" instead of "Foo_Bar", which much better matches our style. We don't make the whole string lowercase to support cases like: "fooBar" should stay as "fooBar". Some situations don't behave quite perfectly, such as "Foo BarBaz" gets converted into "foo_barBaz", or all-caps headings get mangled incorrectly. In these situations, the JSON field should be overridden manually. In most cases, or at least more cases than before, this heuristic does good enough. --- src/shared/format-table.c | 35 ++++++++++++++++++++++++++++------- src/shared/format-table.h | 1 + src/test/test-format-table.c | 32 ++++++++++++++++++++++++++++++++ 3 files changed, 61 insertions(+), 7 deletions(-) diff --git a/src/shared/format-table.c b/src/shared/format-table.c index e538d96414..ee641d1144 100644 --- a/src/shared/format-table.c +++ b/src/shared/format-table.c @@ -2895,18 +2895,39 @@ static int table_data_to_json(TableData *d, sd_json_variant **ret) { } } -static char* string_to_json_field_name(const char *f) { +char* table_mangle_to_json_field_name(const char *str) { /* Tries to make a string more suitable as JSON field name. There are no strict rules defined what a - * field name can be hence this is a bit vague and black magic. Right now we only convert spaces to - * underscores and leave everything as is. */ + * field name can be hence this is a bit vague and black magic. Here's what we do: + * - Convert spaces to underscores + * - Convert dashes to underscores (some JSON parsers don't like dealing with dashes) + * - Convert most other symbols to underscores (for similar reasons) + * - Make the first letter of each word lowercase (unless it looks like the whole word is uppercase) + */ - char *c = strdup(f); + bool new_word = true; + char *c; + + assert(str); + + c = strdup(str); if (!c) return NULL; - for (char *x = c; *x; x++) - if (isspace(*x)) + for (char *x = c; *x; x++) { + if (!strchr(ALPHANUMERICAL, *x)) { *x = '_'; + new_word = true; + continue; + } + + if (new_word) { + if (ascii_tolower(*(x + 1)) == *(x + 1)) /* Heuristic: if next char is upper-case + * then we assume the whole word is all-caps + * and avoid lowercasing it. */ + *x = ascii_tolower(*x); + new_word = false; + } + } return c; } @@ -2927,7 +2948,7 @@ static int table_make_json_field_name(Table *t, TableData *d, char **ret) { return -ENOMEM; } - mangled = string_to_json_field_name(n); + mangled = table_mangle_to_json_field_name(n); if (!mangled) return -ENOMEM; diff --git a/src/shared/format-table.h b/src/shared/format-table.h index 15c0547b94..bb2eb70759 100644 --- a/src/shared/format-table.h +++ b/src/shared/format-table.h @@ -166,6 +166,7 @@ int table_print_json(Table *t, FILE *f, sd_json_format_flags_t json_flags); int table_print_with_pager(Table *t, sd_json_format_flags_t json_format_flags, PagerFlags pager_flags, bool show_header); +char* table_mangle_to_json_field_name(const char *str); int table_set_json_field_name(Table *t, size_t idx, const char *name); #define table_log_add_error(r) \ diff --git a/src/test/test-format-table.c b/src/test/test-format-table.c index fa34c29b02..41621dbb5b 100644 --- a/src/test/test-format-table.c +++ b/src/test/test-format-table.c @@ -404,6 +404,38 @@ TEST(json) { assert_se(sd_json_variant_equal(v, w)); } +TEST(json_mangling) { + static const struct { + const char *arg; + const char *exp; + } cases[] = { + /* Not Mangled */ + { "foo", "foo" }, + { "foo_bar", "foo_bar" }, + { "fooBar", "fooBar" }, + { "fooBar123", "fooBar123" }, + { "foo_bar123", "foo_bar123" }, + { ALPHANUMERICAL, ALPHANUMERICAL }, + { "_123", "_123" }, + + /* Mangled */ + { "Foo Bar", "foo_bar" }, + { "Foo-Bar", "foo_bar" }, + { "Foo@Bar", "foo_bar" }, + { "Foo (Bar)", "foo__bar_"}, + { "MixedCase ALLCAPS", "mixedCase_ALLCAPS" }, + { "_X", "_x" }, + { "_Foo", "_foo" }, + }; + + FOREACH_ELEMENT(i, cases) { + _cleanup_free_ char *ret = NULL; + assert_se(ret = table_mangle_to_json_field_name(i->arg)); + printf("\"%s\" -> \"%s\"\n", i->arg, ret); + assert_se(streq(ret, i->exp)); + } +} + TEST(table) { _cleanup_(table_unrefp) Table *t = NULL; _cleanup_free_ char *formatted = NULL; From c82dfaf7e2f6171fe599fdd325c7f7c57198f096 Mon Sep 17 00:00:00 2001 From: Adrian Vovk Date: Thu, 11 Jul 2024 19:35:10 -0400 Subject: [PATCH 3/3] table: Fix JSON name mangling breaking changes In previous commits, we've changed the JSON name mangling logic. This, of course, will cause breaking changes to occur on anything that relied on the JSON mangling logic. This commit fixes those breaking changes by manually forcing the JSON name back to what it was before. --- src/import/importctl.c | 5 +++++ src/partition/repart.c | 4 ++++ src/sysupdate/sysupdate.c | 5 +++++ 3 files changed, 14 insertions(+) diff --git a/src/import/importctl.c b/src/import/importctl.c index 3334f37b94..30566a1917 100644 --- a/src/import/importctl.c +++ b/src/import/importctl.c @@ -912,6 +912,11 @@ static int list_images(int argc, char *argv[], void *userdata) { (void) table_hide_column_from_display(t, 8); (void) table_hide_column_from_display(t, 10); + /* Starting in v257, these fields would be automatically formatted with underscores. However, this + * command was introduced in v256, so changing the field name would be a breaking change. */ + (void) table_set_json_field_name(t, 8, "usage-exclusive"); + (void) table_set_json_field_name(t, 10, "limit-exclusive"); + for (;;) { uint64_t crtime, mtime, usage, usage_exclusive, limit, limit_exclusive; const char *class, *name, *type, *path; diff --git a/src/partition/repart.c b/src/partition/repart.c index 0509364063..0ad8fad150 100644 --- a/src/partition/repart.c +++ b/src/partition/repart.c @@ -3043,6 +3043,10 @@ static int context_dump_partitions(Context *context) { if (!t) return log_oom(); + /* Starting in v257, these fields would be automatically formatted with underscores. This would have + * been a breaking change, so to avoid that let's hard-code their original names. */ + table_set_json_field_name(t, 15, "drop-in_files"); + if (!DEBUG_LOGGING) { if (arg_json_format_flags & SD_JSON_FORMAT_OFF) (void) table_set_display(t, (size_t) 0, (size_t) 1, (size_t) 2, (size_t) 3, (size_t) 4, diff --git a/src/sysupdate/sysupdate.c b/src/sysupdate/sysupdate.c index 3fcb2839d0..aded7dde0b 100644 --- a/src/sysupdate/sysupdate.c +++ b/src/sysupdate/sysupdate.c @@ -470,6 +470,11 @@ static int context_show_version(Context *c, const char *version) { (void) table_set_align_percent(t, table_get_cell(t, 0, 8), 100); table_set_ersatz_string(t, TABLE_ERSATZ_DASH); + /* Starting in v257, these fields would be automatically formatted with underscores. This would have + * been a breaking change, so to avoid that let's hard-code their original names. */ + (void) table_set_json_field_name(t, 7, "tries-done"); + (void) table_set_json_field_name(t, 8, "tries-left"); + /* Determine if the target will make use of partition/fs attributes for any of the transfers */ FOREACH_ARRAY(transfer, c->transfers, c->n_transfers) { Transfer *tr = *transfer;