summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorNeels Hofmeyr <neels@hofmeyr.de>2017-10-09 17:36:08 +0200
committerNeels Hofmeyr <neels@hofmeyr.de>2017-10-11 22:32:19 +0200
commitdd783056f7ede461dd27347fa86743a24c4c4eed (patch)
tree52062a5972a2b0255c9c1975ae3116618268f45a
parente8ccd5013abe0e04b2400661405fd9e18e88cd89 (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.c4
-rw-r--r--src/db.h6
-rw-r--r--src/db_hlr.c54
-rw-r--r--tests/db/db_test.c37
-rw-r--r--tests/db/db_test.err137
5 files changed, 206 insertions, 32 deletions
diff --git a/src/db.c b/src/db.c
index 8286ba8..eb29434 100644
--- a/src/db.c
+++ b/src/db.c
@@ -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"
diff --git a/src/db.h b/src/db.h
index 5e234ff..9d4274d 100644
--- a/src/db.h
+++ b/src/db.h
@@ -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