Implement CI common API changes

3 files changed, 99 insertions, 46 deletions

diff --git a/mod/ci-common.cxx b/mod/ci-common.cxx
index b3fc432..e9b1b07 100644
--- a/mod/ci-common.cxx
+++ b/mod/ci-common.cxx
@@ -534,7 +534,7 @@ namespace brep
     s.next ("", "");                         // End of manifest.
   }
 
-  pair<optional<string>, ci_start::duplicate_tenant_result> ci_start::
+  optional<pair<string, ci_start::duplicate_tenant_result>> ci_start::
   create (const basic_mark& error,
           const basic_mark&,
           const basic_mark* trace,
@@ -542,10 +542,61 @@ namespace brep
           tenant_service&& service,
           duration notify_interval,
           duration notify_delay,
-          duplicate_tenant_mode) const
+          duplicate_tenant_mode mode) const
   {
     using namespace odb::core;
 
+    assert (!transaction::has_current ());
+
+    duplicate_tenant_result r (duplicate_tenant_result::created);
+
+    transaction tr (db.begin ());
+
+    // Unless we are in the 'fail on duplicate' mode, check if this service
+    // type/id pair is already in use and, if that's the case, either ignore
+    // it or reassign this service to a new tenant, canceling the old one.
+    //
+    if (mode != duplicate_tenant_mode::fail)
+    {
+      using query = query<build_tenant>;
+
+      shared_ptr<build_tenant> t (
+        db.query_one<build_tenant> (query::service.id == service.id &&
+                                    query::service.type == service.type));
+      if (t != nullptr)
+      {
+        // Reduce the replace_archived mode to the replace or ignore mode.
+        //
+        if (mode == duplicate_tenant_mode::replace_archived)
+        {
+          mode = (t->archived
+                  ? duplicate_tenant_mode::replace
+                  : duplicate_tenant_mode::ignore);
+        }
+
+        // Bail out in the ignore mode and cancel the tenant in the replace
+        // mode.
+        //
+        if (mode == duplicate_tenant_mode::ignore)
+          return make_pair (move (t->id), duplicate_tenant_result::ignored);
+
+        assert (mode == duplicate_tenant_mode::replace);
+
+        if (t->unloaded_timestamp)
+        {
+          db.erase (t);
+        }
+        else
+        {
+          t->service = nullopt;
+          t->archived = true;
+          db.update (t);
+        }
+
+        r = duplicate_tenant_result::replaced;
+      }
+    }
+
     // Generate the request id.
     //
     string request_id;
@@ -557,7 +608,7 @@ namespace brep
     catch (const system_error& e)
     {
       error << "unable to generate request id: " << e;
-      return {nullopt, duplicate_tenant_result::ignored}; // @@ TODO HACKED AROUND
+      return nullopt;
     }
 
     // Use the generated request id if the tenant service id is not specified.
@@ -569,43 +620,37 @@ namespace brep
                     move (service),
                     system_clock::now () - notify_interval + notify_delay,
                     notify_interval);
-    {
-      assert (!transaction::has_current ());
-
-      transaction tr (db.begin ());
-
-      // Note that in contrast to brep-load, we know that the tenant id is
-      // unique and thus we don't try to remove a tenant with such an id.
-      // There is also not much reason to assume that we may have switched
-      // from the single-tenant mode here and remove the respective tenant,
-      // unless we are in the tenant-service functionality development mode.
-      //
+    // Note that in contrast to brep-load, we know that the tenant id is
+    // unique and thus we don't try to remove a tenant with such an id.
+    // There is also not much reason to assume that we may have switched
+    // from the single-tenant mode here and remove the respective tenant,
+    // unless we are in the tenant-service functionality development mode.
+    //
 #ifdef BREP_CI_TENANT_SERVICE_UNLOADED
-      cstrings ts ({""});
+    cstrings ts ({""});
 
-      db.erase_query<build_package> (
-        query<build_package>::id.tenant.in_range (ts.begin (), ts.end ()));
+    db.erase_query<build_package> (
+      query<build_package>::id.tenant.in_range (ts.begin (), ts.end ()));
 
-      db.erase_query<build_repository> (
-        query<build_repository>::id.tenant.in_range (ts.begin (), ts.end ()));
+    db.erase_query<build_repository> (
+      query<build_repository>::id.tenant.in_range (ts.begin (), ts.end ()));
 
-      db.erase_query<build_public_key> (
-        query<build_public_key>::id.tenant.in_range (ts.begin (), ts.end ()));
+    db.erase_query<build_public_key> (
+      query<build_public_key>::id.tenant.in_range (ts.begin (), ts.end ()));
 
-      db.erase_query<build_tenant> (
-        query<build_tenant>::id.in_range (ts.begin (), ts.end ()));
+    db.erase_query<build_tenant> (
+      query<build_tenant>::id.in_range (ts.begin (), ts.end ()));
 #endif
 
-      db.persist (t);
+    db.persist (t);
 
-      tr.commit ();
-    }
+    tr.commit ();
 
     if (trace != nullptr)
       *trace << "unloaded CI request " << t.id << " for service "
              << t.service->id << ' ' << t.service->type << " is created";
 
-    return {move (t.id), duplicate_tenant_result::created}; // @@ TODO HACKED AROUND
+    return make_pair (move (t.id), r);
   }
 
   optional<ci_start::start_result> ci_start::
@@ -708,12 +753,18 @@ namespace brep
     if (t == nullptr)
       return nullopt;
 
-    // @@ Why not remove it if unloaded (and below)?
-
     optional<tenant_service> r (move (t->service));
-    t->service = nullopt;
-    t->archived = true;
-    db.update (t);
+
+    if (t->unloaded_timestamp)
+    {
+      db.erase (t);
+    }
+    else
+    {
+      t->service = nullopt;
+      t->archived = true;
+      db.update (t);
+    }
 
     tr.commit ();
 
@@ -743,7 +794,11 @@ namespace brep
     if (t == nullptr)
       return false;
 
-    if (!t->archived)
+    if (t->unloaded_timestamp)
+    {
+      db.erase (t);
+    }
+    else if (!t->archived)
     {
       t->archived = true;
       db.update (t);
diff --git a/mod/ci-common.hxx b/mod/ci-common.hxx
index 23e6360..4bd63c2 100644
--- a/mod/ci-common.hxx
+++ b/mod/ci-common.hxx
@@ -96,16 +96,15 @@ namespace brep
     // having the same semantics as in the replace and ignore modes).
     //
     // Note also that the duplicate_tenant_mode::replace modes are not the
-    // same as separate calls to create() and then to cancel() since the
-    // latter would happen in two separate transactions and will thus be
-    // racy. @@@ TODO
+    // same as separate calls to cancel() and then to create() since the
+    // latter would happen in two separate transactions and will thus be racy.
     //
     // Note: should be called out of the database transaction.
     //
     enum class duplicate_tenant_mode {fail, ignore, replace, replace_archived};
     enum class duplicate_tenant_result {created, ignored, replaced};
 
-    pair<optional<string>, duplicate_tenant_result>
+    optional<pair<string, duplicate_tenant_result>>
     create (const basic_mark& error,
             const basic_mark& warn,
             const basic_mark* trace,
@@ -136,8 +135,8 @@ namespace brep
     // Specifically, this function clears the tenant service state (thus
     // allowing reusing the same service type/id pair in another tenant) and
     // archives the tenant, unless the tenant is unloaded, in which case it is
-    // dropped (@@@ TODO). Note that the latter allow using unloaded tenants
-    // as a relatively cheap asynchronous execution mechanism.
+    // dropped. Note that the latter allow using unloaded tenants as a
+    // relatively cheap asynchronous execution mechanism.
     //
     // Note: should be called out of the database transaction.
     //
@@ -154,9 +153,9 @@ namespace brep
     // is only used for tracing.
     //
     // Similarly to above, this function archives the tenant, unless the
-    // tenant is unloaded, in which case it is dropped (@@@ TODO). Note,
-    // however, that this version does not touch the service state (use the
-    // above version if you want to clear it).
+    // tenant is unloaded, in which case it is dropped. Note, however, that
+    // this version does not touch the service state (use the above version if
+    // you want to clear it).
     //
     // Note: should be called out of the database transaction.
     //
diff --git a/mod/mod-ci-github.cxx b/mod/mod-ci-github.cxx
index 20c9dc3..84633fd 100644
--- a/mod/mod-ci-github.cxx
+++ b/mod/mod-ci-github.cxx
@@ -505,14 +505,14 @@ namespace brep
                      chrono::seconds (0) /* delay */,
                      dtm));
 
-    if (!pr.first)
+    if (!pr)
     {
       fail << "check suite " << cs.check_suite.node_id
            << ": unable to create unloaded CI request";
     }
 
     if (dtm == duplicate_tenant_mode::replace &&
-        pr.second == duplicate_tenant_result::created)
+        pr->second == duplicate_tenant_result::created)
     {
       error << "check suite " << cs.check_suite.node_id
             << ": re-requested but tenant_service with id " << sid
@@ -1908,8 +1908,7 @@ namespace brep
     return create (error, warn, &trace,
                    *build_db_, move (ts),
                    chrono::seconds (30) /* interval */,
-                   chrono::seconds (0)  /* delay */)
-      .first.has_value (); // @@ TODO HACKED AROUND
+                   chrono::seconds (0)  /* delay */).has_value ();
   }
 
   string ci_github::