From: Stefan Sperling Subject: Re: improve the json structure To: Omar Polo Cc: gameoftrees@openbsd.org Date: Thu, 28 Mar 2024 12:01:19 +0100 On Thu, Mar 28, 2024 at 10:44:37AM +0100, Omar Polo wrote: > 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 ok by me > 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 > >