squash w/ [?] Add test for delay rule locking API.

lib/api/include/irods/delay_rule_unlock.h

@@ -12,12 +12,14 @@ struct RcComm;
 /// \since 5.0.0
 typedef struct DelayRuleUnlockInput
 {
-    /// The catalog ID of the delay rule to unlock.
+    /// The list of delay rule IDs to unlock.
     ///
-    /// Must be a non-empty string.
+    /// The following requirements must be satisfied:
+    /// - It must be non-empty
+    /// - It must be a JSON list
     ///
     /// \since 5.0.0
-    char rule_id[32]; // NOLINT(cppcoreguidelines-avoid-c-arrays, modernize-avoid-c-arrays)
+    char* rule_ids; // NOLINT(cppcoreguidelines-avoid-c-arrays, modernize-avoid-c-arrays)
 
     /// The set of key-value pair strings used to influence the behavior of the API
     /// operation.
@@ -27,27 +29,28 @@ typedef struct DelayRuleUnlockInput
 } delayRuleUnlockInp_t;
 
 // NOLINTNEXTLINE(cppcoreguidelines-macro-usage)
-#define DelayRuleUnlockInput_PI "str rule_id[32]; struct KeyValPair_PI;"
+#define DelayRuleUnlockInput_PI "str *rule_ids; struct KeyValPair_PI;"
 
 #ifdef __cplusplus
 extern "C" {
 #endif
 
-/// Removes the delay server lock information from a delay rule if it exists.
+/// Atomically removes the delay server lock information from one or more delay rules.
 ///
 /// Requires \p rodsadmin level privileges.
 ///
-/// On success, the following information will be removed from the target delay rule's catalog
-/// entry:
+/// On success, the following information will be removed from the delay rule catalog entries:
 /// - The host identifying the delay server
 /// - The PID of the delay server
 /// - The time the entry was locked
 ///
+/// On failure, all catalog updates are rolled back and an error code is returned to the client.
+///
 /// \param[in] _comm  A pointer to a RcComm.
 /// \param[in] _input A pointer to a DelayRuleUnlockInput.
 ///
 /// \return An integer.
-/// \retval 0  On success.
+/// \retval  0 On success.
 /// \retval <0 On failure.
 ///
 /// \b Example
@@ -58,7 +61,9 @@ extern "C" {
 /// struct DelayRuleUnlockInput input;
 /// memset(&input, 0, sizeof(struct DelayRuleUnlockInput));
 ///
-/// strncpy(input.rule_id, "12345", sizeof(DelayRuleUnlockInput::rule_id) - 1);
+/// // Don't forget to deallocate the memory pointed to by "input.rule_ids"
+/// // following the completion of the API operation.
+/// input.rule_ids = strdup("[\"12345\", \"67890\"]");
 ///
 /// const int ec = rc_delay_rule_unlock(comm, &input);
 ///

lib/core/src/rcMisc.cpp

@@ -1836,6 +1836,7 @@ void clearDelayRuleUnlockInput(void* _p)
 
     auto* q = static_cast<DelayRuleUnlockInput*>(_p);
 
+    free_pointer(q->rule_ids);
     clearKeyVal(&q->cond_input);
 
     std::memset(q, 0, sizeof(DelayRuleUnlockInput));

plugins/database/src/db_plugin.cpp

@@ -15936,32 +15936,74 @@ auto db_delay_rule_lock(irods::plugin_context& _ctx,
     }
 } // db_delay_rule_lock
 
-auto db_delay_rule_unlock(irods::plugin_context& _ctx, const char* _rule_id) -> irods::error
+auto db_delay_rule_unlock(irods::plugin_context& _ctx, const char* _rule_ids) -> irods::error
 {
     if (const auto ret = _ctx.valid(); !ret.ok()) {
         return PASS(ret);
     }
 
-    if (!_rule_id) {
-        log_db::error("{}: Rule ID cannot be a null pointer.", __func__);
-        return ERROR(SYS_INTERNAL_NULL_INPUT_ERR, "Rule ID cannot be a null pointer.");
+    if (!_rule_ids) {
+        log_db::error("{}: Rule ID list cannot be a null pointer.", __func__);
+        return ERROR(SYS_INTERNAL_NULL_INPUT_ERR, "Rule ID list cannot be a null pointer.");
     }
 
     try {
+        const auto rule_ids = nlohmann::json::parse(_rule_ids);
+#define IRODS_DB_IMPL_A
+#ifdef IRODS_DB_IMPL_A
+        const auto rule_ids_vec = rule_ids.get<std::vector<std::string>>();
+
         auto [db_instance, db_conn] = irods::experimental::catalog::new_database_connection();
 
         nanodbc::statement stmt{db_conn};
         nanodbc::prepare(stmt, "update R_RULE_EXEC set lock_host = '', lock_host_pid = '', lock_ts = '' where rule_exec_id = ?");
+        stmt.bind_strings(0, rule_ids_vec);
+        // This will throw an exception and rollback all updates on failure.
+        nanodbc::transact(stmt, rule_ids_vec.size());
 
-        stmt.bind(0, _rule_id);
+        // It would be amazing to return CODE(result.affected_rows()), but we cannot
+        // because the ODBC drivers can produce different results. For this reason, the only
+        // thing we can do is return success or an error.
+        return SUCCESS();
+#elif defined(IRODS_DB_IMPL_B)
+        auto [db_instance, db_conn] = irods::experimental::catalog::new_database_connection();
 
-        if (const auto result = nanodbc::execute(stmt); result.affected_rows() != 1) {
-            auto msg = fmt::format("{}: Failed to unlock delay rule [rule_id={}].", __func__, _rule_id);
-            log_db::error(msg);
-            return ERROR(CAT_NO_ROWS_UPDATED, std::move(msg));
+        nanodbc::statement stmt{db_conn};
+        nanodbc::prepare(stmt, "update R_RULE_EXEC set lock_host = '', lock_host_pid = '', lock_ts = '' where rule_exec_id = ?");
+
+        std::size_t unlock_count = 0;
+
+        for (auto&& rule_id : rule_ids) {
+            stmt.bind(0, rule_id.get_ref<const std::string&>().c_str());
+
+            if (const auto result = nanodbc::execute(stmt); result.affected_rows() == 1) {
+                ++unlock_count;
+            }
         }
 
-        return SUCCESS();
+        return CODE(unlock_count);
+#else
+        std::vector<std::string> placeholders;
+        placeholders.reserve(rule_ids.size());
+        std::for_each(std::begin(rule_ids), std::end(rule_ids), [&placeholders](auto) { placeholders.emplace_back("?"); });
+
+        const auto sql = fmt::format(
+            "update R_RULE_EXEC set lock_host = '', lock_host_pid = '', lock_ts = '' where rule_exec_id in ({})",
+            fmt::join(placeholders, ", "));
+
+        auto [db_instance, db_conn] = irods::experimental::catalog::new_database_connection();
+
+        nanodbc::statement stmt{db_conn};
+        nanodbc::prepare(stmt, sql);
+
+        for (decltype(rule_ids)::size_type i = 0; i < rule_ids.size(); ++i) {
+            stmt.bind(i, rule_ids.at(i).get_ref<const std::string&>().c_str());
+        }
+
+        const auto result = nanodbc::execute(stmt);
+
+        return CODE(result.affected_rows());
+#endif
     }
     catch (const irods::exception& e) {
         log_db::error("{}: {}", __func__, e.client_display_what());

server/api/include/irods/rs_delay_rule_unlock.hpp

@@ -7,21 +7,22 @@
 
 struct RsComm;
 
-/// Removes the delay server lock information from a delay rule if it exists.
+/// Atomically removes the delay server lock information from one or more delay rules.
 ///
 /// Requires \p rodsadmin level privileges.
 ///
-/// On success, the following information will be removed from the target delay rule's catalog
-/// entry:
+/// On success, the following information will be removed from the delay rule catalog entries:
 /// - The host identifying the delay server
 /// - The PID of the delay server
 /// - The time the entry was locked
 ///
+/// On failure, all catalog updates are rolled back and an error code is returned to the client.
+///
 /// \param[in] _comm  A pointer to a RsComm.
 /// \param[in] _input A pointer to a DelayRuleUnlockInput.
 ///
 /// \return An integer.
-/// \retval 0  On success.
+/// \retval  0 On success.
 /// \retval <0 On failure.
 ///
 /// \b Example
@@ -32,7 +33,9 @@ struct RsComm;
 /// struct DelayRuleUnlockInput input;
 /// memset(&input, 0, sizeof(struct DelayRuleUnlockInput));
 ///
-/// strncpy(input.rule_id, "12345", sizeof(DelayRuleUnlockInput::rule_id) - 1);
+/// // Don't forget to deallocate the memory pointed to by "input.rule_ids"
+/// // following the completion of the API operation.
+/// input.rule_ids = strdup("[\"12345\", \"67890\"]");
 ///
 /// const int ec = rs_delay_rule_unlock(comm, &input);
 ///

server/api/src/rs_delay_rule_unlock.cpp

@@ -16,8 +16,8 @@ auto rs_delay_rule_unlock(RsComm* _comm, DelayRuleUnlockInput* _input) -> int
         return SYS_INTERNAL_NULL_INPUT_ERR;
     }
 
-    if (!is_non_empty_string(_input->rule_id, sizeof(DelayRuleUnlockInput::rule_id))) {
-        log_api::error("{}: Rule ID must be a non-empty string.", __func__);
+    if (!_input->rule_ids) {
+        log_api::error("{}: Rule ID must be a non-empty JSON string.", __func__);
         return SYS_INVALID_INPUT_PARAM;
     }
 
@@ -41,7 +41,7 @@ auto rs_delay_rule_unlock(RsComm* _comm, DelayRuleUnlockInput* _input) -> int
     //
 
     try {
-        const auto ec = chl_delay_rule_unlock(*_comm, _input->rule_id);
+        const auto ec = chl_delay_rule_unlock(*_comm, _input->rule_ids);
 
         if (ec < 0) {
             log_api::error("{}: chl_delay_rule_unlock failed with error code [{}].", __func__, ec);

server/icat/src/icatHighLevelRoutines.cpp

@@ -4918,7 +4918,7 @@ auto chl_delay_rule_lock(RsComm& _comm, const char* _rule_id, const char* _lock_
     return ret.code();
 } // chl_delay_rule_lock
 
-auto chl_delay_rule_unlock(RsComm& _comm, const char* _rule_id) -> int
+auto chl_delay_rule_unlock(RsComm& _comm, const char* _rule_ids) -> int
 {
     irods::database_object_ptr db_obj_ptr;
     if (const auto ret = irods::database_factory(database_plugin_type, db_obj_ptr); !ret.ok()) {
@@ -4937,7 +4937,7 @@ auto chl_delay_rule_unlock(RsComm& _comm, const char* _rule_id) -> int
     irods::first_class_object_ptr ptr = boost::dynamic_pointer_cast<irods::first_class_object>(db_obj_ptr);
     irods::database_ptr db = boost::dynamic_pointer_cast<irods::database>(db_plug_ptr);
 
-    const auto ret = db->call(&_comm, irods::DATABASE_OP_DELAY_RULE_UNLOCK, ptr, _rule_id);
+    const auto ret = db->call(&_comm, irods::DATABASE_OP_DELAY_RULE_UNLOCK, ptr, _rule_ids);
 
     // NOLINTNEXTLINE(bugprone-narrowing-conversions,cppcoreguidelines-narrowing-conversions)
     return ret.code();

unit_tests/src/test_delay_rule_locking_api.cpp

@@ -105,7 +105,8 @@ TEST_CASE("lock and unlock delay rule")
 
     // Unlock the delay rule.
     DelayRuleUnlockInput unlock_input{};
-    rule_id.copy(unlock_input.rule_id, sizeof(DelayRuleUnlockInput::rule_id) - 1);
+    irods::at_scope_exit_unsafe clear_unlock_input{[&unlock_input] { clearDelayRuleUnlockInput(&unlock_input); }};
+    unlock_input.rule_ids = strdup(nlohmann::json::array({rule_id}).dump().c_str());
     REQUIRE(rc_delay_rule_unlock(static_cast<RcComm*>(conn), &unlock_input) == 0);
 
     {
@@ -136,6 +137,100 @@ TEST_CASE("lock and unlock delay rule")
     }
 }
 
+// NOLINTNEXTLINE(readability-function-cognitive-complexity)
+TEST_CASE("unlock multiple delay rules")
+{
+    load_client_api_plugins();
+
+    irods::experimental::client_connection conn;
+
+    // Delete all existing delay rules to avoid issues with the tests.
+    REQUIRE(delete_all_delay_rules() == 0);
+
+    // Delete the delay rule regardless of the test results.
+    irods::at_scope_exit_unsafe delete_delay_rules{[] { delete_all_delay_rules(); }};
+
+    // Schedule five delay rules for execution in the distant future. The delay rules
+    // MUST NOT be executed by the delay server. They are just placeholders to test the API.
+    constexpr const char* rule = R"___(writeLine("serverLog", "SHOULD NOT EXECUTE BEFORE TEST COMPLETES"))___";
+    constexpr auto delay_rule_count = 5;
+    for (int i = 0; i < delay_rule_count; ++i) {
+        REQUIRE(create_delay_rule(rule) == 0);
+    }
+
+    std::vector<std::string> rule_ids;
+    {
+        DelayRuleLockInput lock_input{};
+        boost::asio::ip::host_name().copy(lock_input.lock_host, sizeof(DelayRuleLockInput::lock_host) - 1);
+        lock_input.lock_host_pid = getpid();
+
+        // Capture the IDs of the delay rules and lock them.
+        for (auto&& row : irods::query{static_cast<RcComm*>(conn), "select RULE_EXEC_ID where RULE_EXEC_LOCK_HOST = ''"}) {
+            rule_ids.push_back(row[0]);
+
+            // Lock the delay rule.
+            row[0].copy(lock_input.rule_id, sizeof(DelayRuleLockInput::rule_id) - 1);
+            REQUIRE(rc_delay_rule_lock(static_cast<RcComm*>(conn), &lock_input) == 0);
+        }
+    }
+    CHECK(rule_ids.size() == delay_rule_count);
+
+    // Show the delay rules are now locked.
+    for (auto&& rule_id : rule_ids) {
+        const auto query_string = fmt::format("select RULE_EXEC_ID, RULE_EXEC_LOCK_HOST, RULE_EXEC_LOCK_HOST_PID, RULE_EXEC_LOCK_TIME where RULE_EXEC_ID = '{}'", rule_id);
+        irods::query query{static_cast<RcComm*>(conn), query_string};
+        REQUIRE_FALSE(query.empty());
+        const auto row = query.front();
+        CHECK(row[0] == rule_id);
+        CHECK(row[1] == boost::asio::ip::host_name());
+        CHECK(row[2] == std::to_string(getpid()));
+        CHECK_FALSE(row[3].empty());
+    }
+
+    // Unlock the delay rules.
+    DelayRuleUnlockInput unlock_input{};
+    irods::at_scope_exit_unsafe clear_unlock_input{[&unlock_input] { clearDelayRuleUnlockInput(&unlock_input); }};
+    unlock_input.rule_ids = strdup(nlohmann::json(rule_ids).dump().c_str());
+    REQUIRE(rc_delay_rule_unlock(static_cast<RcComm*>(conn), &unlock_input) == 0);
+
+    // Show the delay rules are now unlocked.
+    for (auto&& rule_id : rule_ids) {
+        const auto query_string = fmt::format("select RULE_EXEC_ID, RULE_EXEC_LOCK_HOST, RULE_EXEC_LOCK_HOST_PID, RULE_EXEC_LOCK_TIME where RULE_EXEC_ID = '{}'", rule_id);
+        irods::query query{static_cast<RcComm*>(conn), query_string};
+        REQUIRE_FALSE(query.empty());
+        const auto row = query.front();
+        CHECK(row[0] == rule_id);
+        CHECK(row[1].empty());
+        CHECK(row[2].empty());
+        CHECK(row[3].empty());
+    }
+
+    // Show that invoking the unlock API with the same IDs is safe and does not
+    // affect the delay rules.
+    CHECK(rc_delay_rule_unlock(static_cast<RcComm*>(conn), &unlock_input) == 0);
+}
+
+TEST_CASE("invalid delay rule ids")
+{
+    irods::experimental::client_connection conn;
+
+    // Unlock a nonexistent delay rule.
+    DelayRuleUnlockInput unlock_input{};
+    irods::at_scope_exit_unsafe clear_unlock_input{[&unlock_input] { clearDelayRuleUnlockInput(&unlock_input); }};
+
+    SECTION("an integer which does not identify a delay rule")
+    {
+        unlock_input.rule_ids = strdup(R"(["10"])");
+        REQUIRE(rc_delay_rule_unlock(static_cast<RcComm*>(conn), &unlock_input) == 0);
+    }
+
+    SECTION("not an integer")
+    {
+        unlock_input.rule_ids = strdup(R"(["xyz"])");
+        REQUIRE(rc_delay_rule_unlock(static_cast<RcComm*>(conn), &unlock_input) == SYS_LIBRARY_ERROR);
+    }
+}
+
 auto delete_all_delay_rules() -> int
 {
     // NOLINTNEXTLINE(cert-env33-c)