Document Dbh.test_reactive, return saner values

In the FreeSWITCH core, the return value of switch_case_db_test_reactive
is ignored, but it is usable in LUA modules (and other bindings via
SWIG). The LUA API example[1] shows how to check the return value, but
that example miserably fails if the database did not exist before.

Changes:

 - Document the expected behavior of the test_reactive function.
 - Assert that test_sql and sql_reactive are both given. If either
   query is not given, the caller is using the wrong API.
 - When SCF_AUTO_SCHEMAS is cleared, use the return value of the
   test_sql execution. Does anybody use this? Why not remove it?
 - Do not unconditionally return SWITCH_FALSE when test_sql fails,
   instead allow it to become SWITCH_TRUE when reactive_sql passes.
 - Remove the unnecessary test_sql check for SCDB_TYPE_CORE_DB
   (this is now enforced through an assert check). (+reindent)
 - Clarify the error message of drop_sql, prepending "Ignoring" to
   the "SQL ERR" message.
 - LUA: Do not print "DBH NOT Connected" if the query fails. This was
   the initial source of confusion.

 [1]: https://confluence.freeswitch.org/display/FREESWITCH/Lua+API+Reference

diff --git a/src/mod/languages/mod_lua/freeswitch_lua.cpp b/src/mod/languages/mod_lua/freeswitch_lua.cpp
index 12a663c3a747ef36d1b98db939aec3ecd4a75176..593ff8a6c40201a252f01206c465508a2b6878e1 100644 (file)
--- a/src/mod/languages/mod_lua/freeswitch_lua.cpp
+++ b/src/mod/languages/mod_lua/freeswitch_lua.cpp
@@ -394,9 +394,9 @@ bool Dbh::test_reactive(char *test_sql, char *drop_sql, char *reactive_sql)
     if (switch_cache_db_test_reactive(dbh, test_sql, drop_sql, reactive_sql) == SWITCH_TRUE) {
       return true;
     }
+  } else {
+    switch_log_printf(SWITCH_CHANNEL_LOG, SWITCH_LOG_ERROR, "DBH NOT Connected.\n");
   }
-
-  switch_log_printf(SWITCH_CHANNEL_LOG, SWITCH_LOG_ERROR, "DBH NOT Connected.\n");
   return false;
 }
 

diff --git a/src/mod/legacy/languages/mod_lua/freeswitch_lua.cpp b/src/mod/legacy/languages/mod_lua/freeswitch_lua.cpp
index efef9679e71a00bee9d1dd7d529bc704319441d1..39076e62256bbe1f779c573e1b1fc1851cdef0be 100644 (file)
--- a/src/mod/legacy/languages/mod_lua/freeswitch_lua.cpp
+++ b/src/mod/legacy/languages/mod_lua/freeswitch_lua.cpp
@@ -394,9 +394,9 @@ bool Dbh::test_reactive(char *test_sql, char *drop_sql, char *reactive_sql)
     if (switch_cache_db_test_reactive(dbh, test_sql, drop_sql, reactive_sql) == SWITCH_TRUE) {
       return true;
     }
+  } else {
+    switch_log_printf(SWITCH_CHANNEL_LOG, SWITCH_LOG_ERROR, "DBH NOT Connected.\n");
   }
-
-  switch_log_printf(SWITCH_CHANNEL_LOG, SWITCH_LOG_ERROR, "DBH NOT Connected.\n");
   return false;
 }
 

diff --git a/src/switch_core_sqldb.c b/src/switch_core_sqldb.c
index 31e835e760717c6662219552114331c28af8116c..adcfbcf300a14f37815272c0e9ab13d0064e9447 100644 (file)
--- a/src/switch_core_sqldb.c
+++ b/src/switch_core_sqldb.c
@@ -1267,20 +1267,36 @@ SWITCH_DECLARE(switch_status_t) switch_cache_db_execute_sql_callback_err(switch_
        return status;
 }
 
+/*!
+ * \brief Performs test_sql and if it fails performs drop_sql and reactive_sql.
+ *
+ * If auto-clear-sql is disabled, then this function will do nothing and it is
+ * assumed that the queries are not needed. If auto-create-schemas is disabled,
+ * then just test_sql is executed, but drop_sql and reactive_sql are not.
+ *
+ * Otherwise, test_sql gets executed. If that succeeds, then there is nothing to
+ * do. Otherwise drop_sql is executed (its result is ignored) and then finally
+ * reactive_sql is executed.
+ *
+ * \return If auto-create-schemas is enabled, SWITCH_TRUE is returned if
+ * test_sql succeeds, SWITCH_FALSE otherwise. If reactive_sql is executed
+ * successfully SWITCH_TRUE is returned, otherwise SWITCH_FALSE is returned.
+ */
 SWITCH_DECLARE(switch_bool_t) switch_cache_db_test_reactive(switch_cache_db_handle_t *dbh,
                                                                                                                        const char *test_sql, const char *drop_sql, const char *reactive_sql)
 {
-       char *errmsg;
        switch_bool_t r = SWITCH_TRUE;
        switch_mutex_t *io_mutex = dbh->io_mutex;
 
+       switch_assert(test_sql != NULL);
+       switch_assert(reactive_sql != NULL);
+
        if (!switch_test_flag((&runtime), SCF_CLEAR_SQL)) {
                return SWITCH_TRUE;
        }
 
        if (!switch_test_flag((&runtime), SCF_AUTO_SCHEMAS)) {
-               switch_cache_db_execute_sql(dbh, (char *)test_sql, NULL);
-               return SWITCH_TRUE;
+               return switch_cache_db_execute_sql(dbh, (char *)test_sql, NULL);
        }
 
        if (io_mutex) switch_mutex_lock(io_mutex);
@@ -1289,49 +1305,48 @@ SWITCH_DECLARE(switch_bool_t) switch_cache_db_test_reactive(switch_cache_db_hand
        case SCDB_TYPE_PGSQL:
                {
                        if (switch_pgsql_handle_exec(dbh->native_handle.pgsql_dbh, test_sql, NULL) != SWITCH_PGSQL_SUCCESS) {
-                               r = SWITCH_FALSE;
                                if (drop_sql) {
                                        switch_pgsql_handle_exec(dbh->native_handle.pgsql_dbh, drop_sql, NULL);
                                }
-                               switch_pgsql_handle_exec(dbh->native_handle.pgsql_dbh, reactive_sql, NULL);
+                               r = switch_pgsql_handle_exec(dbh->native_handle.pgsql_dbh, reactive_sql, NULL) == SWITCH_PGSQL_SUCCESS;
                        }
                }
                break;
        case SCDB_TYPE_ODBC:
                {
                        if (switch_odbc_handle_exec(dbh->native_handle.odbc_dbh, test_sql, NULL, NULL) != SWITCH_ODBC_SUCCESS) {
-                               r = SWITCH_FALSE;
                                if (drop_sql) {
                                        switch_odbc_handle_exec(dbh->native_handle.odbc_dbh, drop_sql, NULL, NULL);
                                }
-                               switch_odbc_handle_exec(dbh->native_handle.odbc_dbh, reactive_sql, NULL, NULL);
+                               r = switch_odbc_handle_exec(dbh->native_handle.odbc_dbh, reactive_sql, NULL, NULL) == SWITCH_ODBC_SUCCESS;
                        }
                }
                break;
        case SCDB_TYPE_CORE_DB:
                {
-                       if (test_sql) {
-                               switch_core_db_exec(dbh->native_handle.core_db_dbh, test_sql, NULL, NULL, &errmsg);
+                       char *errmsg = NULL;
+                       switch_core_db_exec(dbh->native_handle.core_db_dbh, test_sql, NULL, NULL, &errmsg);
 
+                       if (errmsg) {
+                               switch_log_printf(SWITCH_CHANNEL_LOG, SWITCH_LOG_DEBUG, "SQL ERR [%s]\n[%s]\nAuto Generating Table!\n", errmsg, test_sql);
+                               switch_core_db_free(errmsg);
+                               errmsg = NULL;
+                               if (drop_sql) {
+                                       switch_core_db_exec(dbh->native_handle.core_db_dbh, drop_sql, NULL, NULL, &errmsg);
+                               }
+                               if (errmsg) {
+                                       switch_log_printf(SWITCH_CHANNEL_LOG, SWITCH_LOG_DEBUG, "Ignoring SQL ERR [%s]\n[%s]\n", errmsg, drop_sql);
+                                       switch_core_db_free(errmsg);
+                                       errmsg = NULL;
+                               }
+                               switch_core_db_exec(dbh->native_handle.core_db_dbh, reactive_sql, NULL, NULL, &errmsg);
                                if (errmsg) {
                                        r = SWITCH_FALSE;
-                                       switch_log_printf(SWITCH_CHANNEL_LOG, SWITCH_LOG_DEBUG, "SQL ERR [%s]\n[%s]\nAuto Generating Table!\n", errmsg, test_sql);
+                                       switch_log_printf(SWITCH_CHANNEL_LOG, SWITCH_LOG_DEBUG, "SQL ERR [%s]\n[%s]\n", errmsg, reactive_sql);
                                        switch_core_db_free(errmsg);
                                        errmsg = NULL;
-                                       if (drop_sql) {
-                                               switch_core_db_exec(dbh->native_handle.core_db_dbh, drop_sql, NULL, NULL, &errmsg);
-                                       }
-                                       if (errmsg) {
-                                               switch_log_printf(SWITCH_CHANNEL_LOG, SWITCH_LOG_DEBUG, "SQL ERR [%s]\n[%s]\n", errmsg, drop_sql);
-                                               switch_core_db_free(errmsg);
-                                               errmsg = NULL;
-                                       }
-                                       switch_core_db_exec(dbh->native_handle.core_db_dbh, reactive_sql, NULL, NULL, &errmsg);
-                                       if (errmsg) {
-                                               switch_log_printf(SWITCH_CHANNEL_LOG, SWITCH_LOG_DEBUG, "SQL ERR [%s]\n[%s]\n", errmsg, reactive_sql);
-                                               switch_core_db_free(errmsg);
-                                               errmsg = NULL;
-                                       }
+                               } else {
+                                       r = SWITCH_TRUE;
                                }
                        }
                }