diff options
author | Neels Hofmeyr <neels@hofmeyr.de> | 2017-10-09 17:36:08 +0200 |
---|---|---|
committer | Neels Hofmeyr <neels@hofmeyr.de> | 2017-10-11 22:32:19 +0200 |
commit | dd783056f7ede461dd27347fa86743a24c4c4eed (patch) | |
tree | 52062a5972a2b0255c9c1975ae3116618268f45a | |
parent | e8ccd5013abe0e04b2400661405fd9e18e88cd89 (diff) |
refactor db_subscr_lu()
Use named parameters in the SQL statement.
Use db_bind_* functions to drop some code dup.
Use explicit subscriber id arg instead of subscriber struct.
Match return values and error logging to other db functions.
Change-Id: I35665e84ddbe54a6f218b24033df969ad2e669a0
-rw-r--r-- | src/db.c | 4 | ||||
-rw-r--r-- | src/db.h | 6 | ||||
-rw-r--r-- | src/db_hlr.c | 54 | ||||
-rw-r--r-- | tests/db/db_test.c | 37 | ||||
-rw-r--r-- | tests/db/db_test.err | 137 |
5 files changed, 206 insertions, 32 deletions
@@ -45,8 +45,8 @@ static const char *stmt_sql[] = { [DB_STMT_SEL_BY_IMSI] = "SELECT " SEL_COLUMNS " FROM subscriber WHERE imsi = ?", [DB_STMT_SEL_BY_MSISDN] = "SELECT " SEL_COLUMNS " FROM subscriber WHERE msisdn = ?", [DB_STMT_SEL_BY_ID] = "SELECT " SEL_COLUMNS " FROM subscriber WHERE id = ?", - [DB_STMT_UPD_VLR_BY_ID] = "UPDATE subscriber SET vlr_number = ? WHERE id = ?", - [DB_STMT_UPD_SGSN_BY_ID] = "UPDATE subscriber SET sgsn_number = ? WHERE id = ?", + [DB_STMT_UPD_VLR_BY_ID] = "UPDATE subscriber SET vlr_number = $number WHERE id = $subscriber_id", + [DB_STMT_UPD_SGSN_BY_ID] = "UPDATE subscriber SET sgsn_number = $number WHERE id = $subscriber_id", [DB_STMT_AUC_BY_IMSI] = "SELECT id, algo_id_2g, ki, algo_id_3g, k, op, opc, sqn, ind_bitlen" " FROM subscriber" @@ -91,10 +91,8 @@ int db_subscr_get_by_msisdn(struct db_context *dbc, const char *msisdn, int db_subscr_get_by_id(struct db_context *dbc, int64_t id, struct hlr_subscriber *subscr); int db_subscr_nam(struct db_context *dbc, const char *imsi, bool nam_val, bool is_ps); -int db_subscr_lu(struct db_context *dbc, - const struct hlr_subscriber *subscr, - const char *vlr_or_sgsn_number, - bool lu_is_ps); +int db_subscr_lu(struct db_context *dbc, int64_t subscr_id, + const char *vlr_or_sgsn_number, bool is_ps); int db_subscr_purge(struct db_context *dbc, const char *imsi, bool is_ps); diff --git a/src/db_hlr.c b/src/db_hlr.c index 9bdb372..ac3c730 100644 --- a/src/db_hlr.c +++ b/src/db_hlr.c @@ -313,44 +313,46 @@ out: return ret; } -int db_subscr_lu(struct db_context *dbc, - const struct hlr_subscriber *subscr, - const char *vlr_or_sgsn_number, bool lu_is_ps) +int db_subscr_lu(struct db_context *dbc, int64_t subscr_id, + const char *vlr_or_sgsn_number, bool is_ps) { - sqlite3_stmt *stmt = dbc->stmt[DB_STMT_UPD_VLR_BY_ID]; - const char *txt; + sqlite3_stmt *stmt; int rc, ret = 0; - if (lu_is_ps) { - stmt = dbc->stmt[DB_STMT_UPD_SGSN_BY_ID]; - txt = subscr->sgsn_number; - } else { - stmt = dbc->stmt[DB_STMT_UPD_VLR_BY_ID]; - txt = subscr->vlr_number; - } + stmt = dbc->stmt[is_ps ? DB_STMT_UPD_SGSN_BY_ID + : DB_STMT_UPD_VLR_BY_ID]; - rc = sqlite3_bind_int64(stmt, 1, subscr->id); - if (rc != SQLITE_OK) { - LOGP(DAUC, LOGL_ERROR, "Error binding ID: %d\n", rc); - return -EINVAL; - } + if (!db_bind_int64(stmt, "$subscriber_id", subscr_id)) + return -EIO; - rc = sqlite3_bind_text(stmt, 2, txt, -1, SQLITE_STATIC); - if (rc != SQLITE_OK) { - LOGP(DAUC, LOGL_ERROR, "Error binding VLR/SGSN Number: %d\n", rc); - ret = -EBADMSG; - goto out; - } + if (!db_bind_text(stmt, "$number", vlr_or_sgsn_number)) + return -EIO; /* execute the statement */ rc = sqlite3_step(stmt); if (rc != SQLITE_DONE) { - LOGP(DAUC, LOGL_ERROR, "Error updating SQN: %d\n", rc); - ret = -ENOEXEC; + LOGP(DAUC, LOGL_ERROR, "Update %s number for subscriber ID=%"PRId64": SQL Error: %s\n", + is_ps? "SGSN" : "VLR", subscr_id, sqlite3_errmsg(dbc->db)); + ret = -EIO; + goto out; } + + /* verify execution result */ + rc = sqlite3_changes(dbc->db); + if (!rc) { + LOGP(DAUC, LOGL_ERROR, "Cannot update %s number for subscriber ID=%"PRId64 + ": no such subscriber\n", + is_ps? "SGSN" : "VLR", subscr_id); + ret = -ENOENT; + } else if (rc != 1) { + LOGP(DAUC, LOGL_ERROR, "Update %s number for subscriber ID=%"PRId64 + ": SQL modified %d rows (expected 1)\n", + is_ps? "SGSN" : "VLR", subscr_id, rc); + ret = -EIO; + } + out: db_remove_reset(stmt); - return ret; } diff --git a/tests/db/db_test.c b/tests/db/db_test.c index 30de51d..5acd1e8 100644 --- a/tests/db/db_test.c +++ b/tests/db/db_test.c @@ -283,6 +283,43 @@ static void test_subscr_create_update_sel_delete() ASSERT_RC(db_subscr_nam(dbc, "foobar", false, true), -ENOENT); ASSERT_RC(db_subscr_nam(dbc, "foobar", false, false), -ENOENT); + comment("Record LU for PS and CS (SGSN and VLR names)"); + + ASSERT_RC(db_subscr_lu(dbc, id0, "5952", true), 0); + ASSERT_SEL(id, id0, 0); + ASSERT_RC(db_subscr_lu(dbc, id0, "712", false), 0); + ASSERT_SEL(id, id0, 0); + + comment("Record LU for PS and CS (SGSN and VLR names) *again*"); + + ASSERT_RC(db_subscr_lu(dbc, id0, "111", true), 0); + ASSERT_SEL(id, id0, 0); + ASSERT_RC(db_subscr_lu(dbc, id0, "111", true), 0); + ASSERT_SEL(id, id0, 0); + ASSERT_RC(db_subscr_lu(dbc, id0, "222", false), 0); + ASSERT_SEL(id, id0, 0); + ASSERT_RC(db_subscr_lu(dbc, id0, "222", false), 0); + ASSERT_SEL(id, id0, 0); + + comment("Unset LU info for PS and CS (SGSN and VLR names)"); + ASSERT_RC(db_subscr_lu(dbc, id0, "", true), 0); + ASSERT_SEL(id, id0, 0); + ASSERT_RC(db_subscr_lu(dbc, id0, "", false), 0); + ASSERT_SEL(id, id0, 0); + + ASSERT_RC(db_subscr_lu(dbc, id0, "111", true), 0); + ASSERT_RC(db_subscr_lu(dbc, id0, "222", false), 0); + ASSERT_SEL(id, id0, 0); + ASSERT_RC(db_subscr_lu(dbc, id0, NULL, true), 0); + ASSERT_SEL(id, id0, 0); + ASSERT_RC(db_subscr_lu(dbc, id0, NULL, false), 0); + ASSERT_SEL(id, id0, 0); + + comment("Record LU for non-existent ID"); + ASSERT_RC(db_subscr_lu(dbc, 99999, "5952", true), -ENOENT); + ASSERT_RC(db_subscr_lu(dbc, 99999, "712", false), -ENOENT); + ASSERT_SEL(id, 99999, -ENOENT); + comment("Delete non-existent / invalid IDs"); ASSERT_RC(db_subscr_delete_by_id(dbc, 999), -ENOENT); diff --git a/tests/db/db_test.err b/tests/db/db_test.err index f7c03d9..6e6a0ac 100644 --- a/tests/db/db_test.err +++ b/tests/db/db_test.err @@ -376,6 +376,143 @@ db_subscr_nam(dbc, "foobar", false, false) --> -ENOENT DAUC Cannot disable CS: no such subscriber: IMSI='foobar' +--- Record LU for PS and CS (SGSN and VLR names) + +db_subscr_lu(dbc, id0, "5952", true) --> 0 + +db_subscr_get_by_id(dbc, id0, &g_subscr) --> 0 +struct hlr_subscriber { + .id = 1, + .imsi = '123456789000000', + .msisdn = '543210123456789', + .sgsn_number = '5952', +} + +db_subscr_lu(dbc, id0, "712", false) --> 0 + +db_subscr_get_by_id(dbc, id0, &g_subscr) --> 0 +struct hlr_subscriber { + .id = 1, + .imsi = '123456789000000', + .msisdn = '543210123456789', + .vlr_number = '712', + .sgsn_number = '5952', +} + + +--- Record LU for PS and CS (SGSN and VLR names) *again* + +db_subscr_lu(dbc, id0, "111", true) --> 0 + +db_subscr_get_by_id(dbc, id0, &g_subscr) --> 0 +struct hlr_subscriber { + .id = 1, + .imsi = '123456789000000', + .msisdn = '543210123456789', + .vlr_number = '712', + .sgsn_number = '111', +} + +db_subscr_lu(dbc, id0, "111", true) --> 0 + +db_subscr_get_by_id(dbc, id0, &g_subscr) --> 0 +struct hlr_subscriber { + .id = 1, + .imsi = '123456789000000', + .msisdn = '543210123456789', + .vlr_number = '712', + .sgsn_number = '111', +} + +db_subscr_lu(dbc, id0, "222", false) --> 0 + +db_subscr_get_by_id(dbc, id0, &g_subscr) --> 0 +struct hlr_subscriber { + .id = 1, + .imsi = '123456789000000', + .msisdn = '543210123456789', + .vlr_number = '222', + .sgsn_number = '111', +} + +db_subscr_lu(dbc, id0, "222", false) --> 0 + +db_subscr_get_by_id(dbc, id0, &g_subscr) --> 0 +struct hlr_subscriber { + .id = 1, + .imsi = '123456789000000', + .msisdn = '543210123456789', + .vlr_number = '222', + .sgsn_number = '111', +} + + +--- Unset LU info for PS and CS (SGSN and VLR names) + +db_subscr_lu(dbc, id0, "", true) --> 0 + +db_subscr_get_by_id(dbc, id0, &g_subscr) --> 0 +struct hlr_subscriber { + .id = 1, + .imsi = '123456789000000', + .msisdn = '543210123456789', + .vlr_number = '222', +} + +db_subscr_lu(dbc, id0, "", false) --> 0 + +db_subscr_get_by_id(dbc, id0, &g_subscr) --> 0 +struct hlr_subscriber { + .id = 1, + .imsi = '123456789000000', + .msisdn = '543210123456789', +} + +db_subscr_lu(dbc, id0, "111", true) --> 0 + +db_subscr_lu(dbc, id0, "222", false) --> 0 + +db_subscr_get_by_id(dbc, id0, &g_subscr) --> 0 +struct hlr_subscriber { + .id = 1, + .imsi = '123456789000000', + .msisdn = '543210123456789', + .vlr_number = '222', + .sgsn_number = '111', +} + +db_subscr_lu(dbc, id0, NULL, true) --> 0 + +db_subscr_get_by_id(dbc, id0, &g_subscr) --> 0 +struct hlr_subscriber { + .id = 1, + .imsi = '123456789000000', + .msisdn = '543210123456789', + .vlr_number = '222', +} + +db_subscr_lu(dbc, id0, NULL, false) --> 0 + +db_subscr_get_by_id(dbc, id0, &g_subscr) --> 0 +struct hlr_subscriber { + .id = 1, + .imsi = '123456789000000', + .msisdn = '543210123456789', +} + + +--- Record LU for non-existent ID + +db_subscr_lu(dbc, 99999, "5952", true) --> -ENOENT +DAUC Cannot update SGSN number for subscriber ID=99999: no such subscriber + +db_subscr_lu(dbc, 99999, "712", false) --> -ENOENT +DAUC Cannot update VLR number for subscriber ID=99999: no such subscriber + +db_subscr_get_by_id(dbc, 99999, &g_subscr) --> -ENOENT +DAUC Cannot read subscriber from db: ID=99999: No such subscriber + + --- Delete non-existent / invalid IDs db_subscr_delete_by_id(dbc, 999) --> -ENOENT |