Add service data flag state_synced

4 files changed, 32 insertions, 48 deletions

diff --git a/mod/mod-ci-github-gq.cxx b/mod/mod-ci-github-gq.cxx
index e2d68a9..1e6e6c9 100644
--- a/mod/mod-ci-github-gq.cxx
+++ b/mod/mod-ci-github-gq.cxx
@@ -254,6 +254,7 @@ namespace brep
                 cr.node_id = move (rcr.node_id);
 
               cr.state = gh_from_status (rcr.status);
+              cr.state_synced = true;
             }
           }
 
diff --git a/mod/mod-ci-github-service-data.cxx b/mod/mod-ci-github-service-data.cxx
index e519936..f79550c 100644
--- a/mod/mod-ci-github-service-data.cxx
+++ b/mod/mod-ci-github-service-data.cxx
@@ -51,14 +51,10 @@ namespace brep
           nid = *v;
       }
 
-      optional<build_state> s;
-      {
-        string* v (p.next_expect_member_string_null ("state"));
-        if (v != nullptr)
-          s = to_build_state (*v);
-      }
+      build_state s (to_build_state (p.next_expect_member_string ("state")));
+      bool ss (p.next_expect_member_boolean<bool> ("state_synced"));
 
-      check_runs.emplace_back (move (bid), move (nid), s);
+      check_runs.emplace_back (move (bid), move (nid), s, ss);
 
       p.next_expect (event::end_object);
     }
@@ -112,11 +108,8 @@ namespace brep
       else
         s.value (nullptr);
 
-      s.member_name ("state");
-      if (cr.state)
-        s.value (to_string (*cr.state));
-      else
-        s.value (nullptr);
+      s.member ("state", to_string (cr.state));
+      s.member ("state_synced", cr.state_synced);
 
       s.end_object ();
     }
@@ -143,7 +136,7 @@ namespace brep
   {
     os << "node_id: " << cr.node_id.value_or ("null")
        << ", build_id: " << cr.build_id
-       << ", state: " << (cr.state ? to_string (*cr.state) : "null");
+       << ", state: " << cr.state_string ();
 
     return os;
   }
diff --git a/mod/mod-ci-github-service-data.hxx b/mod/mod-ci-github-service-data.hxx
index 7ea01ff..d4a47d5 100644
--- a/mod/mod-ci-github-service-data.hxx
+++ b/mod/mod-ci-github-service-data.hxx
@@ -27,26 +27,16 @@ namespace brep
     string                build_id; // Full build id.
     optional<string>      node_id;  // GitHub id.
 
-    // @@ TODO
-    //
-    // build_state           state;
-    // bool                  state_synced;
-
-    // string
-    // state_string () const
-    // {
-    //   string r (to_string (*state));
-    //   if (!state_synced)
-    //     r += "(unsynchronized)";
-    //   return r;
-    // }
-
-    optional<build_state> state;
+    build_state           state;
+    bool                  state_synced;
 
     string
     state_string () const
     {
-      return state ? to_string (*state) : "null";
+      string r (to_string (state));
+      if (!state_synced)
+        r += "(unsynchronized)";
+      return r;
     }
   };
 
diff --git a/mod/mod-ci-github.cxx b/mod/mod-ci-github.cxx
index 9d31210..34dd8cc 100644
--- a/mod/mod-ci-github.cxx
+++ b/mod/mod-ci-github.cxx
@@ -491,10 +491,13 @@ namespace brep
 
       if (scr == nullptr)
       {
-        crs.emplace_back (move (bid), nullopt, nullopt);
+        crs.emplace_back (move (bid),
+                          nullopt,
+                          build_state::queued,
+                          false /* state_synced */);
         bs.push_back (b);
       }
-      else if (!scr->state)
+      else if (!scr->state_synced)
         ; // Ignore network issue.
       else if (istate && *istate == build_state::building)
         ; // Ignore interrupted.
@@ -540,9 +543,6 @@ namespace brep
 
     if (iat != nullptr)
     {
-      // @@ TODO Check whether any of these check runs exist on GH before
-      //         creating them.
-      //
       // Queue a check_run for each build.
       //
       if (gq_create_check_runs (crs,
@@ -669,7 +669,7 @@ namespace brep
       {
         // The check run exists on GitHub and in the persisted service data.
         //
-        if (!scr->state || *scr->state == build_state::queued)
+        if (!scr->state_synced || scr->state == build_state::queued)
         {
           // If the stored state is queued (most likely), at least
           // build_queued() and its lambda has run. If this is all then GitHub
@@ -699,15 +699,15 @@ namespace brep
           // API has the same semantics) so we can just try to update to
           // building and see what the actual status it returns is.
           //
-          // If scr->state is nullopt then GitHub has either queued or built
-          // but we know build_built() has run so we need to update to built
-          // instead of building.
+          // If scr->state_synced is false then GitHub has either queued or
+          // built but we know build_built() has run so we need to update to
+          // built instead of building.
           //
           cr = move (*scr);
-          cr.state = nullopt;
+          cr.state_synced = false;
 
-          build_state st (scr->state ? build_state::building
-                                     : build_state::built);
+          build_state st (scr->state_synced ? build_state::building
+                                            : build_state::built);
 
           if (gq_update_check_run (cr,
                                    iat->token,
@@ -716,10 +716,10 @@ namespace brep
                                    st,
                                    error))
           {
-            // @@ TODO If !scr->state and GH had built then we probably don't
-            //         want to run the lambda either but currently it will run
-            //         and this update message is not accurate. Is stored
-            //         failed state the only way?
+            // @@ TODO If !scr->state_synced and GH had built then we probably
+            //         don't want to run the lambda either but currently it
+            //         will run and this update message is not accurate. Is
+            //         stored failed state the only way?
             //
             if (cr.state == st)
               l3 ([&]{trace << "updated check_run { " << cr << " }";});
@@ -734,11 +734,11 @@ namespace brep
             }
           }
         }
-        else if (*scr->state == build_state::built)
+        else if (scr->state == build_state::built)
         {
           // Ignore out of order built notification.
           //
-          assert (*scr->state == build_state::built);
+          assert (scr->state == build_state::built);
 
           warn << *scr << ": "
                << "out of order transition from "
@@ -794,7 +794,7 @@ namespace brep
         // @@ TODO What if the failed GH update was for built? May end up with
         //         permanently building check run.
         //
-        if (!scr->state || scr->state == build_state::queued)
+        if (!scr->state_synced || scr->state == build_state::queued)
         {
           scr->state = cr.state;
           if (!scr->node_id)