[PATCH] osmo-hlr[master]: add db_subscr_get_by_msisdn() and db_subscr_get_by_id()

Neels Hofmeyr gerrit-no-reply at lists.osmocom.org
Wed Oct 11 20:32:29 UTC 2017


Hello Harald Welte, Jenkins Builder,

I'd like you to reexamine a change.  Please visit

    https://gerrit.osmocom.org/4181

to look at the new patch set (#3).

add db_subscr_get_by_msisdn() and db_subscr_get_by_id()

Factor out the selected SQL columns as SEL_COLUMNS macro, so that each of the
new DB_STMTs will select identical columns: the old DB_STMT_SEL_BY_IMSI as well
as the new DB_STMT_SEL_BY_MSISDN and DB_STMT_SEL_BY_ID.

Add the new functions db_subscr_get_by_msisdn() and db_subscr_get_by_id() and
factor out common parts with db_subscr_get_by_imsi() to static db_sel().

Change-Id: I6d0ddd1b7e3f6b180b4b1b2663c5725d2a4a9428
---
M src/db.c
M src/db.h
M src/db_hlr.c
M tests/db/db_test.c
M tests/db/db_test.err
5 files changed, 185 insertions(+), 39 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-hlr refs/changes/81/4181/3

diff --git a/src/db.c b/src/db.c
index 179eba8..5a38d55 100644
--- a/src/db.c
+++ b/src/db.c
@@ -26,8 +26,25 @@
 #include "logging.h"
 #include "db.h"
 
+#define SEL_COLUMNS \
+	"id," \
+	"imsi," \
+	"msisdn," \
+	"vlr_number," \
+	"sgsn_number," \
+	"sgsn_address," \
+	"periodic_lu_tmr," \
+	"periodic_rau_tau_tmr," \
+	"nam_cs," \
+	"nam_ps," \
+	"lmsi," \
+	"ms_purged_cs," \
+	"ms_purged_ps"
+
 static const char *stmt_sql[] = {
-	[DB_STMT_SEL_BY_IMSI] = "SELECT id,imsi,msisdn,vlr_number,sgsn_number,sgsn_address,periodic_lu_tmr,periodic_rau_tau_tmr,nam_cs,nam_ps,lmsi,ms_purged_cs,ms_purged_ps FROM subscriber WHERE imsi = ?",
+	[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_AUC_BY_IMSI] =
diff --git a/src/db.h b/src/db.h
index 761d88e..5ec7b72 100644
--- a/src/db.h
+++ b/src/db.h
@@ -5,6 +5,8 @@
 
 enum stmt_idx {
 	DB_STMT_SEL_BY_IMSI,
+	DB_STMT_SEL_BY_MSISDN,
+	DB_STMT_SEL_BY_ID,
 	DB_STMT_UPD_VLR_BY_ID,
 	DB_STMT_UPD_SGSN_BY_ID,
 	DB_STMT_AUC_BY_IMSI,
@@ -84,6 +86,10 @@
 
 int db_subscr_get_by_imsi(struct db_context *dbc, const char *imsi,
 			  struct hlr_subscriber *subscr);
+int db_subscr_get_by_msisdn(struct db_context *dbc, const char *msisdn,
+			    struct hlr_subscriber *subscr);
+int db_subscr_get_by_id(struct db_context *dbc, int64_t id,
+			struct hlr_subscriber *subscr);
 int db_subscr_ps(struct db_context *dbc, const char *imsi, bool enable);
 int db_subscr_lu(struct db_context *dbc,
 		 const struct hlr_subscriber *subscr,
diff --git a/src/db_hlr.c b/src/db_hlr.c
index b232dfd..8df4bcb 100644
--- a/src/db_hlr.c
+++ b/src/db_hlr.c
@@ -154,27 +154,26 @@
 
 }
 
-int db_subscr_get_by_imsi(struct db_context *dbc, const char *imsi,
-			  struct hlr_subscriber *subscr)
+/* Common code for db_subscr_get_by_*() functions. */
+static int db_sel(struct db_context *dbc, sqlite3_stmt *stmt, struct hlr_subscriber *subscr,
+		  const char **err)
 {
-	sqlite3_stmt *stmt = dbc->stmt[DB_STMT_SEL_BY_IMSI];
 	int rc;
-
-	if (!db_bind_text(stmt, NULL, imsi))
-		return -EINVAL;
+	int ret = 0;
 
 	/* execute the statement */
 	rc = sqlite3_step(stmt);
+	if (rc == SQLITE_DONE) {
+		ret = -ENOENT;
+		goto out;
+	}
 	if (rc != SQLITE_ROW) {
-		LOGHLR(imsi, LOGL_ERROR, "Error executing SQL: %d\n", rc);
-		db_remove_reset(stmt);
-		return -ENOEXEC;
+		ret = -EIO;
+		goto out;
 	}
 
-	if (!subscr) {
-		db_remove_reset(stmt);
-		return 0;
-	}
+	if (!subscr)
+		goto out;
 
 	/* obtain the various columns */
 	subscr->id = sqlite3_column_int64(stmt, 0);
@@ -192,9 +191,72 @@
 	subscr->ms_purged_cs = sqlite3_column_int(stmt, 11);
 	subscr->ms_purged_ps = sqlite3_column_int(stmt, 12);
 
+out:
 	db_remove_reset(stmt);
 
-	return 0;
+	switch (ret) {
+	case 0:
+		*err = NULL;
+		break;
+	case -ENOENT:
+		*err = "No such subscriber";
+		break;
+	default:
+		*err = sqlite3_errmsg(dbc->db);
+		break;
+	}
+	return ret;
+}
+
+int db_subscr_get_by_imsi(struct db_context *dbc, const char *imsi,
+			  struct hlr_subscriber *subscr)
+{
+	sqlite3_stmt *stmt = dbc->stmt[DB_STMT_SEL_BY_IMSI];
+	const char *err;
+	int rc;
+
+	if (!db_bind_text(stmt, NULL, imsi))
+		return -EIO;
+
+	rc = db_sel(dbc, stmt, subscr, &err);
+	if (rc)
+		LOGP(DAUC, LOGL_ERROR, "Cannot read subscriber from db: IMSI='%s': %s\n",
+		     imsi, err);
+	return rc;
+}
+
+int db_subscr_get_by_msisdn(struct db_context *dbc, const char *msisdn,
+			    struct hlr_subscriber *subscr)
+{
+	sqlite3_stmt *stmt = dbc->stmt[DB_STMT_SEL_BY_MSISDN];
+	const char *err;
+	int rc;
+
+	if (!db_bind_text(stmt, NULL, msisdn))
+		return -EIO;
+
+	rc = db_sel(dbc, stmt, subscr, &err);
+	if (rc)
+		LOGP(DAUC, LOGL_ERROR, "Cannot read subscriber from db: MSISDN='%s': %s\n",
+		     msisdn, err);
+	return rc;
+}
+
+int db_subscr_get_by_id(struct db_context *dbc, int64_t id,
+			struct hlr_subscriber *subscr)
+{
+	sqlite3_stmt *stmt = dbc->stmt[DB_STMT_SEL_BY_ID];
+	const char *err;
+	int rc;
+
+	if (!db_bind_int64(stmt, NULL, id))
+		return -EIO;
+
+	rc = db_sel(dbc, stmt, subscr, &err);
+	if (rc)
+		LOGP(DAUC, LOGL_ERROR, "Cannot read subscriber from db: ID=%"PRId64": %s\n",
+		     id, err);
+	return rc;
 }
 
 int db_subscr_ps(struct db_context *dbc, const char *imsi, bool enable)
diff --git a/tests/db/db_test.c b/tests/db/db_test.c
index 9bd082e..250b363 100644
--- a/tests/db/db_test.c
+++ b/tests/db/db_test.c
@@ -70,6 +70,7 @@
 static void *ctx = NULL;
 static struct hlr_subscriber g_subscr;
 static int g_rc;
+static int64_t g_id;
 
 #define Pfv(name, fmt, val) \
 	fprintf(stderr, "  ." #name " = " fmt ",\n", val)
@@ -186,16 +187,16 @@
 	ASSERT_SEL(imsi, imsi2, 0);
 
 	ASSERT_RC(db_subscr_create(dbc, "123456789 000003"), -EINVAL);
-	ASSERT_SEL(imsi, "123456789000003", -ENOEXEC);
+	ASSERT_SEL(imsi, "123456789000003", -ENOENT);
 
 	ASSERT_RC(db_subscr_create(dbc, "123456789000002123456"), -EINVAL);
-	ASSERT_SEL(imsi, "123456789000002123456", -ENOEXEC);
+	ASSERT_SEL(imsi, "123456789000002123456", -ENOENT);
 
 	ASSERT_RC(db_subscr_create(dbc, "foobar123"), -EINVAL);
-	ASSERT_SEL(imsi, "foobar123", -ENOEXEC);
+	ASSERT_SEL(imsi, "foobar123", -ENOENT);
 
 	ASSERT_RC(db_subscr_create(dbc, "123"), -EINVAL);
-	ASSERT_SEL(imsi, "123", -ENOEXEC);
+	ASSERT_SEL(imsi, "123", -ENOENT);
 
 	ASSERT_RC(db_subscr_create(dbc, short_imsi), 0);
 	ASSERT_SEL(imsi, short_imsi, 0);
@@ -207,30 +208,41 @@
 	ASSERT_SEL(imsi, imsi0, 0);
 	ASSERT_RC(db_subscr_update_msisdn_by_imsi(dbc, imsi0, "54321"), 0);
 	ASSERT_SEL(imsi, imsi0, 0);
+	ASSERT_SEL(msisdn, "54321", 0);
 	ASSERT_RC(db_subscr_update_msisdn_by_imsi(dbc, imsi0,
 					  "54321012345678912345678"), -EINVAL);
 	ASSERT_SEL(imsi, imsi0, 0);
+	ASSERT_SEL(msisdn, "54321", 0);
+	ASSERT_SEL(msisdn, "54321012345678912345678", -ENOENT);
 	ASSERT_RC(db_subscr_update_msisdn_by_imsi(dbc, imsi0,
 					  "543 21"), -EINVAL);
 	ASSERT_SEL(imsi, imsi0, 0);
+	ASSERT_SEL(msisdn, "543 21", -ENOENT);
 	ASSERT_RC(db_subscr_update_msisdn_by_imsi(dbc, imsi0,
 					  "foobar123"), -EINVAL);
 	ASSERT_SEL(imsi, imsi0, 0);
+	ASSERT_SEL(msisdn, "foobar123", -ENOENT);
 	ASSERT_RC(db_subscr_update_msisdn_by_imsi(dbc, imsi0,
 					  "5"), 0);
 	ASSERT_SEL(imsi, imsi0, 0);
+	ASSERT_SEL(msisdn, "5", 0);
+	ASSERT_SEL(msisdn, "54321", -ENOENT);
 	ASSERT_RC(db_subscr_update_msisdn_by_imsi(dbc, imsi0,
 					  "543210123456789"), 0);
 	ASSERT_SEL(imsi, imsi0, 0);
+	ASSERT_SEL(msisdn, "543210123456789", 0);
 	ASSERT_RC(db_subscr_update_msisdn_by_imsi(dbc, imsi0,
 					  "5432101234567891"), -EINVAL);
 	ASSERT_SEL(imsi, imsi0, 0);
+	ASSERT_SEL(msisdn, "5432101234567891", -ENOENT);
 
 	comment("Set MSISDN on non-existent / invalid IMSI");
 
 	ASSERT_RC(db_subscr_update_msisdn_by_imsi(dbc, unknown_imsi, "99"), -ENOENT);
+	ASSERT_SEL(msisdn, "99", -ENOENT);
 
 	ASSERT_RC(db_subscr_update_msisdn_by_imsi(dbc, "foobar", "99"), -ENOENT);
+	ASSERT_SEL(msisdn, "99", -ENOENT);
 
 	comment("Delete non-existent / invalid IDs");
 
@@ -241,20 +253,20 @@
 
 	ASSERT_SEL(imsi, imsi0, 0);
 	ASSERT_RC(db_subscr_delete_by_id(dbc, id0), 0);
-	ASSERT_SEL(imsi, imsi0, -ENOEXEC);
+	ASSERT_SEL(imsi, imsi0, -ENOENT);
 	ASSERT_RC(db_subscr_delete_by_id(dbc, id0), -ENOENT);
 
 	ASSERT_SEL(imsi, imsi1, 0);
 	ASSERT_RC(db_subscr_delete_by_id(dbc, id1), 0);
-	ASSERT_SEL(imsi, imsi1, -ENOEXEC);
+	ASSERT_SEL(imsi, imsi1, -ENOENT);
 
 	ASSERT_SEL(imsi, imsi2, 0);
 	ASSERT_RC(db_subscr_delete_by_id(dbc, id2), 0);
-	ASSERT_SEL(imsi, imsi2, -ENOEXEC);
+	ASSERT_SEL(imsi, imsi2, -ENOENT);
 
 	ASSERT_SEL(imsi, short_imsi, 0);
 	ASSERT_RC(db_subscr_delete_by_id(dbc, id_short), 0);
-	ASSERT_SEL(imsi, short_imsi, -ENOEXEC);
+	ASSERT_SEL(imsi, short_imsi, -ENOENT);
 
 	comment_end();
 }
diff --git a/tests/db/db_test.err b/tests/db/db_test.err
index 1b08cec..17323b5 100644
--- a/tests/db/db_test.err
+++ b/tests/db/db_test.err
@@ -68,26 +68,26 @@
 db_subscr_create(dbc, "123456789 000003") --> -EINVAL
 DAUC Cannot create subscriber: invalid IMSI: '123456789 000003'
 
-db_subscr_get_by_imsi(dbc, "123456789000003", &g_subscr) --> -ENOEXEC
-DAUC IMSI='123456789000003': Error executing SQL: 101
+db_subscr_get_by_imsi(dbc, "123456789000003", &g_subscr) --> -ENOENT
+DAUC Cannot read subscriber from db: IMSI='123456789000003': No such subscriber
 
 db_subscr_create(dbc, "123456789000002123456") --> -EINVAL
 DAUC Cannot create subscriber: invalid IMSI: '123456789000002123456'
 
-db_subscr_get_by_imsi(dbc, "123456789000002123456", &g_subscr) --> -ENOEXEC
-DAUC IMSI='123456789000002123456': Error executing SQL: 101
+db_subscr_get_by_imsi(dbc, "123456789000002123456", &g_subscr) --> -ENOENT
+DAUC Cannot read subscriber from db: IMSI='123456789000002123456': No such subscriber
 
 db_subscr_create(dbc, "foobar123") --> -EINVAL
 DAUC Cannot create subscriber: invalid IMSI: 'foobar123'
 
-db_subscr_get_by_imsi(dbc, "foobar123", &g_subscr) --> -ENOEXEC
-DAUC IMSI='foobar123': Error executing SQL: 101
+db_subscr_get_by_imsi(dbc, "foobar123", &g_subscr) --> -ENOENT
+DAUC Cannot read subscriber from db: IMSI='foobar123': No such subscriber
 
 db_subscr_create(dbc, "123") --> -EINVAL
 DAUC Cannot create subscriber: invalid IMSI: '123'
 
-db_subscr_get_by_imsi(dbc, "123", &g_subscr) --> -ENOEXEC
-DAUC IMSI='123': Error executing SQL: 101
+db_subscr_get_by_imsi(dbc, "123", &g_subscr) --> -ENOENT
+DAUC Cannot read subscriber from db: IMSI='123': No such subscriber
 
 db_subscr_create(dbc, short_imsi) --> 0
 
@@ -115,6 +115,13 @@
   .msisdn = '54321',
 }
 
+db_subscr_get_by_msisdn(dbc, "54321", &g_subscr) --> 0
+struct hlr_subscriber {
+  .id = 1,
+  .imsi = '123456789000000',
+  .msisdn = '54321',
+}
+
 db_subscr_update_msisdn_by_imsi(dbc, imsi0, "54321012345678912345678") --> -EINVAL
 DAUC IMSI='123456789000000': Cannot update subscriber: invalid MSISDN: '54321012345678912345678'
 
@@ -124,6 +131,16 @@
   .imsi = '123456789000000',
   .msisdn = '54321',
 }
+
+db_subscr_get_by_msisdn(dbc, "54321", &g_subscr) --> 0
+struct hlr_subscriber {
+  .id = 1,
+  .imsi = '123456789000000',
+  .msisdn = '54321',
+}
+
+db_subscr_get_by_msisdn(dbc, "54321012345678912345678", &g_subscr) --> -ENOENT
+DAUC Cannot read subscriber from db: MSISDN='54321012345678912345678': No such subscriber
 
 db_subscr_update_msisdn_by_imsi(dbc, imsi0, "543 21") --> -EINVAL
 DAUC IMSI='123456789000000': Cannot update subscriber: invalid MSISDN: '543 21'
@@ -135,6 +152,9 @@
   .msisdn = '54321',
 }
 
+db_subscr_get_by_msisdn(dbc, "543 21", &g_subscr) --> -ENOENT
+DAUC Cannot read subscriber from db: MSISDN='543 21': No such subscriber
+
 db_subscr_update_msisdn_by_imsi(dbc, imsi0, "foobar123") --> -EINVAL
 DAUC IMSI='123456789000000': Cannot update subscriber: invalid MSISDN: 'foobar123'
 
@@ -145,6 +165,9 @@
   .msisdn = '54321',
 }
 
+db_subscr_get_by_msisdn(dbc, "foobar123", &g_subscr) --> -ENOENT
+DAUC Cannot read subscriber from db: MSISDN='foobar123': No such subscriber
+
 db_subscr_update_msisdn_by_imsi(dbc, imsi0, "5") --> 0
 
 db_subscr_get_by_imsi(dbc, imsi0, &g_subscr) --> 0
@@ -154,9 +177,26 @@
   .msisdn = '5',
 }
 
+db_subscr_get_by_msisdn(dbc, "5", &g_subscr) --> 0
+struct hlr_subscriber {
+  .id = 1,
+  .imsi = '123456789000000',
+  .msisdn = '5',
+}
+
+db_subscr_get_by_msisdn(dbc, "54321", &g_subscr) --> -ENOENT
+DAUC Cannot read subscriber from db: MSISDN='54321': No such subscriber
+
 db_subscr_update_msisdn_by_imsi(dbc, imsi0, "543210123456789") --> 0
 
 db_subscr_get_by_imsi(dbc, imsi0, &g_subscr) --> 0
+struct hlr_subscriber {
+  .id = 1,
+  .imsi = '123456789000000',
+  .msisdn = '543210123456789',
+}
+
+db_subscr_get_by_msisdn(dbc, "543210123456789", &g_subscr) --> 0
 struct hlr_subscriber {
   .id = 1,
   .imsi = '123456789000000',
@@ -173,14 +213,23 @@
   .msisdn = '543210123456789',
 }
 
+db_subscr_get_by_msisdn(dbc, "5432101234567891", &g_subscr) --> -ENOENT
+DAUC Cannot read subscriber from db: MSISDN='5432101234567891': No such subscriber
+
 
 --- Set MSISDN on non-existent / invalid IMSI
 
 db_subscr_update_msisdn_by_imsi(dbc, unknown_imsi, "99") --> -ENOENT
 DAUC Cannot update MSISDN: no such subscriber: IMSI='999999999'
 
+db_subscr_get_by_msisdn(dbc, "99", &g_subscr) --> -ENOENT
+DAUC Cannot read subscriber from db: MSISDN='99': No such subscriber
+
 db_subscr_update_msisdn_by_imsi(dbc, "foobar", "99") --> -ENOENT
 DAUC Cannot update MSISDN: no such subscriber: IMSI='foobar'
+
+db_subscr_get_by_msisdn(dbc, "99", &g_subscr) --> -ENOENT
+DAUC Cannot read subscriber from db: MSISDN='99': No such subscriber
 
 
 --- Delete non-existent / invalid IDs
@@ -203,8 +252,8 @@
 
 db_subscr_delete_by_id(dbc, id0) --> 0
 
-db_subscr_get_by_imsi(dbc, imsi0, &g_subscr) --> -ENOEXEC
-DAUC IMSI='123456789000000': Error executing SQL: 101
+db_subscr_get_by_imsi(dbc, imsi0, &g_subscr) --> -ENOENT
+DAUC Cannot read subscriber from db: IMSI='123456789000000': No such subscriber
 
 db_subscr_delete_by_id(dbc, id0) --> -ENOENT
 DAUC Cannot delete: no such subscriber: ID=1
@@ -217,8 +266,8 @@
 
 db_subscr_delete_by_id(dbc, id1) --> 0
 
-db_subscr_get_by_imsi(dbc, imsi1, &g_subscr) --> -ENOEXEC
-DAUC IMSI='123456789000001': Error executing SQL: 101
+db_subscr_get_by_imsi(dbc, imsi1, &g_subscr) --> -ENOENT
+DAUC Cannot read subscriber from db: IMSI='123456789000001': No such subscriber
 
 db_subscr_get_by_imsi(dbc, imsi2, &g_subscr) --> 0
 struct hlr_subscriber {
@@ -228,8 +277,8 @@
 
 db_subscr_delete_by_id(dbc, id2) --> 0
 
-db_subscr_get_by_imsi(dbc, imsi2, &g_subscr) --> -ENOEXEC
-DAUC IMSI='123456789000002': Error executing SQL: 101
+db_subscr_get_by_imsi(dbc, imsi2, &g_subscr) --> -ENOENT
+DAUC Cannot read subscriber from db: IMSI='123456789000002': No such subscriber
 
 db_subscr_get_by_imsi(dbc, short_imsi, &g_subscr) --> 0
 struct hlr_subscriber {
@@ -239,8 +288,8 @@
 
 db_subscr_delete_by_id(dbc, id_short) --> 0
 
-db_subscr_get_by_imsi(dbc, short_imsi, &g_subscr) --> -ENOEXEC
-DAUC IMSI='123456': Error executing SQL: 101
+db_subscr_get_by_imsi(dbc, short_imsi, &g_subscr) --> -ENOENT
+DAUC Cannot read subscriber from db: IMSI='123456': No such subscriber
 
 ===== test_subscr_create_update_sel_delete: SUCCESS
 

-- 
To view, visit https://gerrit.osmocom.org/4181
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6d0ddd1b7e3f6b180b4b1b2663c5725d2a4a9428
Gerrit-PatchSet: 3
Gerrit-Project: osmo-hlr
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder


More information about the gerrit-log mailing list