From: Omar Polo Subject: Re: improve the json structure To: Omar Polo Cc: gameoftrees@openbsd.org Date: Thu, 28 Mar 2024 10:44:37 +0100 On 2024/03/28 10:28:27 +0100, Omar Polo wrote: > This changes the produced json so that the "author" field becomes a > object itself with the following fields > > "author": { > "full": "Omar Polo ", > "name": "Omar Polo", > "mail": "op@openbsd.org", > "user": "op" > } > > and similarly for the committer. The only field that is guaranteed to > be always present is author.user since the short formats shows only the > (abbreviated) username. > > In the short format we should report "committer" instead of "author" > since it's actually the committer. > > To make things consistent, in the "long" format remember the author and > use that for the committer too if not provided. This way, we can > guaranteed that committer.user is always present in all notifications. > > Then, similarly, instead of a single field "message" that is the full > commit message in some cases and the first line in some others, add an > extra field "short_message" that is just the first line. Again, this is > to guarantee that "short_message" is always present. > > There is some redundancy in the JSON now, but I think it's better to > make the consuming easier. > > For example, knowing this, to produce notifications in the IRC channel > one only has to extract the id, committer.user and short_message field > (that are now guaranteed to always exist). > > ok? updated diff after the unicode one went in diff refs/heads/main refs/heads/json commit - ea5e974da9b1047689411a00ecc0a9c1fb101d73 commit + 8e0fc5336f9ab65e8836698df34c4855cdaa2152 blob - e1dd226bd532d7adaf1e4712e8c13bf3cc29ffba blob + 65830ea27b3c8094af7418498500f922ed75ecee --- gotd/libexec/got-notify-http/got-notify-http.c +++ gotd/libexec/got-notify-http/got-notify-http.c @@ -149,6 +149,46 @@ json_field(FILE *fp, const char *key, const char *val, fprintf(fp, "\"%s", comma ? "," : ""); } +static void +json_author(FILE *fp, const char *type, char *address, int comma) +{ + char *gt, *lt, *at, *email, *endname; + + fprintf(fp, "\"%s\":{", type); + + gt = strchr(address, '<'); + if (gt != NULL) { + /* long format, e.g. "Omar Polo " */ + + json_field(fp, "full", address, 1); + + endname = gt; + while (endname > address && endname[-1] == ' ') + endname--; + + *endname = '\0'; + json_field(fp, "name", address, 1); + + email = gt + 1; + lt = strchr(email, '>'); + if (lt) + *lt = '\0'; + + json_field(fp, "mail", email, 1); + + at = strchr(email, '@'); + if (at) + *at = '\0'; + + json_field(fp, "user", email, 0); + } else { + /* short format only shows the username */ + json_field(fp, "user", address, 0); + } + + fprintf(fp, "}%s", comma ? "," : ""); +} + static int jsonify_short(FILE *fp) { @@ -187,9 +227,9 @@ jsonify_short(FILE *fp) fprintf(fp, "{\"short\":true,"); json_field(fp, "id", id, 1); - json_field(fp, "author", author, 1); + json_author(fp, "committer", author, 1); json_field(fp, "date", date, 1); - json_field(fp, "message", message, 0); + json_field(fp, "short_message", message, 0); fprintf(fp, "}"); } @@ -206,6 +246,7 @@ static int jsonify(FILE *fp) { const char *errstr; + char *author = NULL; char *l; char *line = NULL; size_t linesize = 0; @@ -254,7 +295,13 @@ jsonify(FILE *fp) if (strncmp(l, "from: ", 6) != 0) errx(1, "unexpected from line"); l += 6; - json_field(fp, "author", l, 1); + + author = strdup(l); + if (author == NULL) + err(1, "strdup"); + + json_author(fp, "author", l, 1); + phase = P_VIA; break; @@ -262,10 +309,17 @@ jsonify(FILE *fp) /* optional */ if (!strncmp(l, "via: ", 5)) { l += 5; - json_field(fp, "via", l, 1); + json_author(fp, "committer", l, 1); phase = P_DATE; break; } + + if (author == NULL) /* impossible */ + err(1, "from not specified"); + json_author(fp, "committer", author, 1); + free(author); + author = NULL; + phase = P_DATE; /* fallthrough */ @@ -313,7 +367,6 @@ jsonify(FILE *fp) if (errstr) errx(1, "message len is %s: %s", errstr, l); - fprintf(fp, "\"message\":\""); phase = P_MSG; break; @@ -329,14 +382,21 @@ jsonify(FILE *fp) * tolerate one byte less than advertised. */ if (*l == ' ') { - escape(fp, l + 1); /* skip leading space */ + l++; /* skip leading space */ + linelen--; - /* avoid pre-pending \n to the commit msg */ - msgwrote += linelen - 1; - if (msgwrote != 0) + if (msgwrote == 0 && linelen != 0) { + json_field(fp, "short_message", l, 1); + fprintf(fp, "\"message\":\""); + escape(fp, l); escape(fp, "\n"); + msgwrote += linelen; + } else if (msgwrote != 0) { + escape(fp, l); + escape(fp, "\n"); + } } - msglen -= linelen; + msglen -= linelen + 1; if (msglen <= 1) { fprintf(fp, "\","); msgwrote = 0; blob - 3255fa55f2841d50b5170e35303b4e7622843e04 blob + 339f4a7c90ab5de6679eb61927eb9ff684c5acea --- regress/gotd/http_notification.sh +++ regress/gotd/http_notification.sh @@ -62,8 +62,20 @@ test_file_changed() { {"notifications":[{ "short":false, "id":"$commit_id", - "author":"$GOT_AUTHOR", + "author":{ + "full":"$GOT_AUTHOR", + "name":"$GIT_AUTHOR_NAME", + "mail":"$GIT_AUTHOR_EMAIL", + "user":"$GOT_AUTHOR_11" + }, + "committer":{ + "full":"$GOT_AUTHOR", + "name":"$GIT_AUTHOR_NAME", + "mail":"$GIT_AUTHOR_EMAIL", + "user":"$GOT_AUTHOR_11" + }, "date":"$d", + "short_message":"make changes", "message":"make changes\n", "diffstat":{}, "changes":{} @@ -131,8 +143,20 @@ test_bad_utf8() { {"notifications":[{ "short":false, "id":"$commit_id", - "author":"$GOT_AUTHOR", + "author":{ + "full":"$GOT_AUTHOR", + "name":"$GIT_AUTHOR_NAME", + "mail":"$GIT_AUTHOR_EMAIL", + "user":"$GOT_AUTHOR_11" + }, + "committer":{ + "full":"$GOT_AUTHOR", + "name":"$GIT_AUTHOR_NAME", + "mail":"$GIT_AUTHOR_EMAIL", + "user":"$GOT_AUTHOR_11" + }, "date":"$d", + "short_message":"make\uFFFD\uFFFDchanges", "message":"make\uFFFD\uFFFDchanges\n", "diffstat":{}, "changes":{} @@ -208,8 +232,20 @@ test_many_commits_not_summarized() { { "short":false, "id":"$commit_id", - "author":"$GOT_AUTHOR", + "author":{ + "full":"$GOT_AUTHOR", + "name":"$GIT_AUTHOR_NAME", + "mail":"$GIT_AUTHOR_EMAIL", + "user":"$GOT_AUTHOR_11" + }, + "committer":{ + "full":"$GOT_AUTHOR", + "name":"$GIT_AUTHOR_NAME", + "mail":"$GIT_AUTHOR_EMAIL", + "user":"$GOT_AUTHOR_11" + }, "date":"$commit_time", + "short_message":"make changes", "message":"make changes\n", "diffstat":{}, "changes":{} @@ -289,9 +325,11 @@ test_many_commits_summarized() { { "short":true, "id":"$commit_id", - "author":"$GOT_AUTHOR_8", + "committer":{ + "user":"$GOT_AUTHOR_8" + }, "date":"$commit_time", - "message":"make changes" + "short_message":"make changes" } EOF done >> $testroot/stdout.expected