refactor db_subscr_purge

Use named parameters in the SQL statements.

Use db_bind_* functions to drop some code dup.

Adopt error handling (rc and logging) to match the other db functions: return
-ENOENT for unknown subscriber, -EIO for SQL failures.

Change-Id: Iad49d29b90a708c6cf55bfb3bcc02d9e29001a15
This commit is contained in:
Neels Hofmeyr
2017-10-09 17:48:51 +02:00
parent dd783056f7
commit e50121ec96
6 changed files with 214 additions and 17 deletions

View File

@@ -54,8 +54,8 @@ static const char *stmt_sql[] = {
" LEFT JOIN auc_3g ON auc_3g.subscriber_id = subscriber.id"
" WHERE imsi = ?",
[DB_STMT_AUC_UPD_SQN] = "UPDATE auc_3g SET sqn = ? WHERE subscriber_id = ?",
[DB_STMT_UPD_PURGE_CS_BY_IMSI] = "UPDATE subscriber SET ms_purged_cs=1 WHERE imsi = ?",
[DB_STMT_UPD_PURGE_PS_BY_IMSI] = "UPDATE subscriber SET ms_purged_ps=1 WHERE imsi = ?",
[DB_STMT_UPD_PURGE_CS_BY_IMSI] = "UPDATE subscriber SET ms_purged_cs = $val WHERE imsi = $imsi",
[DB_STMT_UPD_PURGE_PS_BY_IMSI] = "UPDATE subscriber SET ms_purged_ps = $val WHERE imsi = $imsi",
[DB_STMT_UPD_NAM_CS_BY_IMSI] = "UPDATE subscriber SET nam_cs = $val WHERE imsi = $imsi",
[DB_STMT_UPD_NAM_PS_BY_IMSI] = "UPDATE subscriber SET nam_ps = $val WHERE imsi = $imsi",
[DB_STMT_SUBSCR_CREATE] = "INSERT INTO subscriber (imsi) VALUES ($imsi)",

View File

@@ -94,5 +94,5 @@ int db_subscr_nam(struct db_context *dbc, const char *imsi, bool nam_val, bool i
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);
int db_subscr_purge(struct db_context *dbc, const char *by_imsi,
bool purge_val, bool is_ps);

View File

@@ -356,27 +356,49 @@ out:
return ret;
}
int db_subscr_purge(struct db_context *dbc, const char *imsi, bool is_ps)
int db_subscr_purge(struct db_context *dbc, const char *by_imsi,
bool purge_val, bool is_ps)
{
sqlite3_stmt *stmt = dbc->stmt[DB_STMT_UPD_VLR_BY_ID];
int rc, ret = 1;
sqlite3_stmt *stmt;
int rc, ret = 0;
if (is_ps)
stmt = dbc->stmt[DB_STMT_UPD_PURGE_PS_BY_IMSI];
else
stmt = dbc->stmt[DB_STMT_UPD_PURGE_CS_BY_IMSI];
stmt = dbc->stmt[is_ps ? DB_STMT_UPD_PURGE_PS_BY_IMSI
: DB_STMT_UPD_PURGE_CS_BY_IMSI];
if (!db_bind_text(stmt, NULL, imsi))
return -EINVAL;
if (!db_bind_text(stmt, "$imsi", by_imsi))
return -EIO;
if (!db_bind_int(stmt, "$val", purge_val ? 1 : 0))
return -EIO;
/* execute the statement */
rc = sqlite3_step(stmt);
if (rc != SQLITE_DONE) {
LOGP(DAUC, LOGL_ERROR, "Error setting Purged: %d\n", rc);
ret = -ENOEXEC;
LOGP(DAUC, LOGL_ERROR, "%s %s: SQL error: %s\n",
purge_val ? "purge" : "un-purge",
is_ps ? "PS" : "CS",
sqlite3_errmsg(dbc->db));
ret = -EIO;
goto out;
}
/* FIXME: return 0 in case IMSI not known */
/* verify execution result */
rc = sqlite3_changes(dbc->db);
if (!rc) {
LOGP(DAUC, LOGL_ERROR, "Cannot %s %s: no such subscriber: IMSI='%s'\n",
purge_val ? "purge" : "un-purge",
is_ps ? "PS" : "CS",
by_imsi);
ret = -ENOENT;
goto out;
} else if (rc != 1) {
LOGHLR(by_imsi, LOGL_ERROR, "%s %s: SQL modified %d rows (expected 1)\n",
purge_val ? "purge" : "un-purge",
is_ps ? "PS" : "CS",
rc);
ret = -EIO;
}
out:
db_remove_reset(stmt);
return ret;

View File

@@ -221,7 +221,7 @@ static int rx_purge_ms_req(struct osmo_gsup_conn *conn,
* we have on record. Only update if yes */
/* Perform the actual update of the DB */
rc = db_subscr_purge(g_hlr->dbc, gsup->imsi, is_ps);
rc = db_subscr_purge(g_hlr->dbc, gsup->imsi, true, is_ps);
if (rc == 1)
gsup_reply.message_type = OSMO_GSUP_MSGT_PURGE_MS_RESULT;

View File

@@ -320,6 +320,44 @@ static void test_subscr_create_update_sel_delete()
ASSERT_RC(db_subscr_lu(dbc, 99999, "712", false), -ENOENT);
ASSERT_SEL(id, 99999, -ENOENT);
comment("Purge and un-purge PS and CS");
/* purge_val, is_ps */
ASSERT_RC(db_subscr_purge(dbc, imsi0, true, true), 0);
ASSERT_SEL(imsi, imsi0, 0);
ASSERT_RC(db_subscr_purge(dbc, imsi0, true, false), 0);
ASSERT_SEL(imsi, imsi0, 0);
ASSERT_RC(db_subscr_purge(dbc, imsi0, false, false), 0);
ASSERT_SEL(imsi, imsi0, 0);
ASSERT_RC(db_subscr_purge(dbc, imsi0, false, true), 0);
ASSERT_SEL(imsi, imsi0, 0);
comment("Purge PS and CS *again*");
ASSERT_RC(db_subscr_purge(dbc, imsi0, true, true), 0);
ASSERT_SEL(imsi, imsi0, 0);
ASSERT_RC(db_subscr_purge(dbc, imsi0, true, true), 0);
ASSERT_SEL(imsi, imsi0, 0);
ASSERT_RC(db_subscr_purge(dbc, imsi0, false, true), 0);
ASSERT_SEL(imsi, imsi0, 0);
ASSERT_RC(db_subscr_purge(dbc, imsi0, false, true), 0);
ASSERT_SEL(imsi, imsi0, 0);
ASSERT_RC(db_subscr_purge(dbc, imsi0, true, false), 0);
ASSERT_SEL(imsi, imsi0, 0);
ASSERT_RC(db_subscr_purge(dbc, imsi0, true, false), 0);
ASSERT_SEL(imsi, imsi0, 0);
ASSERT_RC(db_subscr_purge(dbc, imsi0, false, false), 0);
ASSERT_SEL(imsi, imsi0, 0);
ASSERT_RC(db_subscr_purge(dbc, imsi0, false, false), 0);
ASSERT_SEL(imsi, imsi0, 0);
comment("Purge on non-existent / invalid IMSI");
ASSERT_RC(db_subscr_purge(dbc, unknown_imsi, true, true), -ENOENT);
ASSERT_SEL(imsi, unknown_imsi, -ENOENT);
ASSERT_RC(db_subscr_purge(dbc, unknown_imsi, true, false), -ENOENT);
ASSERT_SEL(imsi, unknown_imsi, -ENOENT);
comment("Delete non-existent / invalid IDs");
ASSERT_RC(db_subscr_delete_by_id(dbc, 999), -ENOENT);

View File

@@ -513,6 +513,143 @@ db_subscr_get_by_id(dbc, 99999, &g_subscr) --> -ENOENT
DAUC Cannot read subscriber from db: ID=99999: No such subscriber
--- Purge and un-purge PS and CS
db_subscr_purge(dbc, imsi0, true, true) --> 0
db_subscr_get_by_imsi(dbc, imsi0, &g_subscr) --> 0
struct hlr_subscriber {
.id = 1,
.imsi = '123456789000000',
.msisdn = '543210123456789',
.ms_purged_ps = true,
}
db_subscr_purge(dbc, imsi0, true, false) --> 0
db_subscr_get_by_imsi(dbc, imsi0, &g_subscr) --> 0
struct hlr_subscriber {
.id = 1,
.imsi = '123456789000000',
.msisdn = '543210123456789',
.ms_purged_cs = true,
.ms_purged_ps = true,
}
db_subscr_purge(dbc, imsi0, false, false) --> 0
db_subscr_get_by_imsi(dbc, imsi0, &g_subscr) --> 0
struct hlr_subscriber {
.id = 1,
.imsi = '123456789000000',
.msisdn = '543210123456789',
.ms_purged_ps = true,
}
db_subscr_purge(dbc, imsi0, false, true) --> 0
db_subscr_get_by_imsi(dbc, imsi0, &g_subscr) --> 0
struct hlr_subscriber {
.id = 1,
.imsi = '123456789000000',
.msisdn = '543210123456789',
}
--- Purge PS and CS *again*
db_subscr_purge(dbc, imsi0, true, true) --> 0
db_subscr_get_by_imsi(dbc, imsi0, &g_subscr) --> 0
struct hlr_subscriber {
.id = 1,
.imsi = '123456789000000',
.msisdn = '543210123456789',
.ms_purged_ps = true,
}
db_subscr_purge(dbc, imsi0, true, true) --> 0
db_subscr_get_by_imsi(dbc, imsi0, &g_subscr) --> 0
struct hlr_subscriber {
.id = 1,
.imsi = '123456789000000',
.msisdn = '543210123456789',
.ms_purged_ps = true,
}
db_subscr_purge(dbc, imsi0, false, true) --> 0
db_subscr_get_by_imsi(dbc, imsi0, &g_subscr) --> 0
struct hlr_subscriber {
.id = 1,
.imsi = '123456789000000',
.msisdn = '543210123456789',
}
db_subscr_purge(dbc, imsi0, false, true) --> 0
db_subscr_get_by_imsi(dbc, imsi0, &g_subscr) --> 0
struct hlr_subscriber {
.id = 1,
.imsi = '123456789000000',
.msisdn = '543210123456789',
}
db_subscr_purge(dbc, imsi0, true, false) --> 0
db_subscr_get_by_imsi(dbc, imsi0, &g_subscr) --> 0
struct hlr_subscriber {
.id = 1,
.imsi = '123456789000000',
.msisdn = '543210123456789',
.ms_purged_cs = true,
}
db_subscr_purge(dbc, imsi0, true, false) --> 0
db_subscr_get_by_imsi(dbc, imsi0, &g_subscr) --> 0
struct hlr_subscriber {
.id = 1,
.imsi = '123456789000000',
.msisdn = '543210123456789',
.ms_purged_cs = true,
}
db_subscr_purge(dbc, imsi0, false, false) --> 0
db_subscr_get_by_imsi(dbc, imsi0, &g_subscr) --> 0
struct hlr_subscriber {
.id = 1,
.imsi = '123456789000000',
.msisdn = '543210123456789',
}
db_subscr_purge(dbc, imsi0, false, false) --> 0
db_subscr_get_by_imsi(dbc, imsi0, &g_subscr) --> 0
struct hlr_subscriber {
.id = 1,
.imsi = '123456789000000',
.msisdn = '543210123456789',
}
--- Purge on non-existent / invalid IMSI
db_subscr_purge(dbc, unknown_imsi, true, true) --> -ENOENT
DAUC Cannot purge PS: no such subscriber: IMSI='999999999'
db_subscr_get_by_imsi(dbc, unknown_imsi, &g_subscr) --> -ENOENT
DAUC Cannot read subscriber from db: IMSI='999999999': No such subscriber
db_subscr_purge(dbc, unknown_imsi, true, false) --> -ENOENT
DAUC Cannot purge CS: no such subscriber: IMSI='999999999'
db_subscr_get_by_imsi(dbc, unknown_imsi, &g_subscr) --> -ENOENT
DAUC Cannot read subscriber from db: IMSI='999999999': No such subscriber
--- Delete non-existent / invalid IDs
db_subscr_delete_by_id(dbc, 999) --> -ENOENT