From 108ae845697198aacca69a2a859363cb59e9c35d Mon Sep 17 00:00:00 2001
From: Francois Kritzinger <francois@codesynthesis.com>
Date: Tue, 23 Apr 2024 09:35:58 +0200
Subject: Re-implement build_queued()

---
 mod/mod-ci-github.cxx | 68 +++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 63 insertions(+), 5 deletions(-)

(limited to 'mod')

diff --git a/mod/mod-ci-github.cxx b/mod/mod-ci-github.cxx
index f2aae97..388deb4 100644
--- a/mod/mod-ci-github.cxx
+++ b/mod/mod-ci-github.cxx
@@ -443,6 +443,11 @@ namespace brep
   //    if we have node_id, then we update, otherwise, we create (potentially
   //    overriding the check run created previously).
   //
+  //      @@ TMP If we can't update to built in queued due to missing result,
+  //             then we can't do it in building either, right? And in built
+  //             we're updating to built regardless of whether previous
+  //             updates failed? So what is the state_synced used for then?
+  //
   function<optional<string> (const tenant_service&)> ci_github::
   build_queued (const tenant_service& ts,
                 const vector<build>& builds,
@@ -463,16 +468,58 @@ namespace brep
       return nullptr;
     }
 
+    // The builds for which we will be creating check runs.
+    //
     vector<reference_wrapper<const build>> bs;
     vector<check_run> crs; // Parallel to bs.
 
+    // Exclude the builds for which we won't be creating check runs.
+    //
     for (const build& b: builds)
     {
       string bid (gh_check_run_name (b)); // Full Build ID.
 
-      const check_run* scr (sd.find_check_run (bid));
+      if (const check_run* scr = sd.find_check_run (bid))
+      {
+        // Another notification has already stored this check run.
+        //
+        if (!istate)
+        {
+          // Out of order queued notification.
+          //
+          warn << "check run " << scr->build_id << ": out of order "
+               << to_string (build_state::queued)
+               << " notification; previously saved state: "
+               << scr->state_string ();
+        }
+        else if (*istate == build_state::built)
+        {
+          // Invalid built->queued transition.
+          //
+          warn << "check run " << scr->build_id
+               << ": invalid transition from "
+               << to_string (build_state::built) << " to "
+               << to_string (build_state::queued);
+        }
+        else
+          ; // Ignore interrupted.
+      }
+      else
+      {
+        // No stored check run for this build so prepare to create one for it.
+        //
+        bs.push_back (b);
+
+        crs.emplace_back (move (bid),
+                          nullopt, /* node_id */
+                          build_state::queued,
+                          false /* state_synced */);
+      }
     }
 
+    if (bs.empty ()) // Nothing to do.
+      return nullptr;
+
     // Get a new installation access token if the current one has expired.
     //
     const gh_installation_access_token* iat (nullptr);
@@ -505,7 +552,7 @@ namespace brep
                                 error))
       {
         for (check_run& cr: crs)
-          l3 ([&] { trace << "created check_run { " << cr << " }"; });
+          l3 ([&]{trace << "created check_run { " << cr << " }";});
       }
     }
 
@@ -532,12 +579,23 @@ namespace brep
       if (iat)
         sd.installation_access = *iat;
 
-      // Note that we've already ignored all the builds for which this
-      // notification was out of order.
-      //
       for (size_t i (0); i != bs.size (); ++i)
       {
         const check_run& cr (crs[i]);
+
+        // Note that this service data may not be the same as what we observed
+        // in the build_queued() function above. For example, some check runs
+        // that we have queued may have already transitioned to building. So
+        // we skip any check runs that are already present.
+        //
+        if (const check_run* scr = sd.find_check_run (cr.build_id))
+        {
+          warn << cr << " state " << scr->state_string ()
+               << " was stored before notified state " << cr.state_string ()
+               << " could be stored";
+        }
+        else
+          sd.check_runs.push_back (cr);
       }
 
       return sd.json ();
-- 
cgit v1.1