mirror of
				https://gitea.osmocom.org/cellular-infrastructure/osmo-mgw.git
				synced 2025-11-03 21:43:32 +00:00 
			
		
		
		
	sms: Do not store received id in the SMS database.
That was a bad idea from the very beginning. A visible result of this is a wrong SMS routing when you change subscriber extensions, while having queued SMS. It's also a very wrong thing from the code layering perspective. I think the next logical step should be to remove "receiver" pointer from the gsm_sms structure into a structure, special for the internal SMS queue.
This commit is contained in:
		
				
					committed by
					
						
						Holger Hans Peter Freyther
					
				
			
			
				
	
			
			
			
						parent
						
							73bc51deea
						
					
				
				
					commit
					ca7ed2d2df
				
			@@ -95,7 +95,6 @@ static char *create_stmts[] = {
 | 
				
			|||||||
		"id INTEGER PRIMARY KEY AUTOINCREMENT, "
 | 
							"id INTEGER PRIMARY KEY AUTOINCREMENT, "
 | 
				
			||||||
		"created TIMESTAMP NOT NULL, "
 | 
							"created TIMESTAMP NOT NULL, "
 | 
				
			||||||
		"sent TIMESTAMP, "
 | 
							"sent TIMESTAMP, "
 | 
				
			||||||
		"receiver_id INTEGER NOT NULL, "
 | 
					 | 
				
			||||||
		"deliver_attempts INTEGER NOT NULL DEFAULT 0, "
 | 
							"deliver_attempts INTEGER NOT NULL DEFAULT 0, "
 | 
				
			||||||
		/* data directly copied/derived from SMS */
 | 
							/* data directly copied/derived from SMS */
 | 
				
			||||||
		"valid_until TIMESTAMP, "
 | 
							"valid_until TIMESTAMP, "
 | 
				
			||||||
@@ -1233,19 +1232,19 @@ int db_sms_store(struct gsm_sms *sms)
 | 
				
			|||||||
	/* FIXME: correct validity period */
 | 
						/* FIXME: correct validity period */
 | 
				
			||||||
	result = dbi_conn_queryf(conn,
 | 
						result = dbi_conn_queryf(conn,
 | 
				
			||||||
		"INSERT INTO SMS "
 | 
							"INSERT INTO SMS "
 | 
				
			||||||
		"(created, receiver_id, valid_until, "
 | 
							"(created, valid_until, "
 | 
				
			||||||
		 "reply_path_req, status_rep_req, protocol_id, "
 | 
							 "reply_path_req, status_rep_req, protocol_id, "
 | 
				
			||||||
		 "data_coding_scheme, ud_hdr_ind, "
 | 
							 "data_coding_scheme, ud_hdr_ind, "
 | 
				
			||||||
		 "user_data, text, "
 | 
							 "user_data, text, "
 | 
				
			||||||
		 "dest_addr, dest_ton, dest_npi, "
 | 
							 "dest_addr, dest_ton, dest_npi, "
 | 
				
			||||||
		 "src_addr, src_ton, src_npi) VALUES "
 | 
							 "src_addr, src_ton, src_npi) VALUES "
 | 
				
			||||||
		"(datetime('now'), %llu, %u, "
 | 
							"(datetime('now'), %u, "
 | 
				
			||||||
		"%u, %u, %u, "
 | 
							"%u, %u, %u, "
 | 
				
			||||||
		"%u, %u, "
 | 
							"%u, %u, "
 | 
				
			||||||
		"%s, %s, "
 | 
							"%s, %s, "
 | 
				
			||||||
		"%s, %u, %u, "
 | 
							"%s, %u, %u, "
 | 
				
			||||||
		"%s, %u, %u)",
 | 
							"%s, %u, %u)",
 | 
				
			||||||
		sms->receiver ? sms->receiver->id : 0, validity_timestamp,
 | 
							validity_timestamp,
 | 
				
			||||||
		sms->reply_path_req, sms->status_rep_req, sms->protocol_id,
 | 
							sms->reply_path_req, sms->status_rep_req, sms->protocol_id,
 | 
				
			||||||
		sms->data_coding_scheme, sms->ud_hdr_ind,
 | 
							sms->data_coding_scheme, sms->ud_hdr_ind,
 | 
				
			||||||
		q_udata, q_text,
 | 
							q_udata, q_text,
 | 
				
			||||||
@@ -1266,7 +1265,6 @@ int db_sms_store(struct gsm_sms *sms)
 | 
				
			|||||||
static struct gsm_sms *sms_from_result(struct gsm_network *net, dbi_result result)
 | 
					static struct gsm_sms *sms_from_result(struct gsm_network *net, dbi_result result)
 | 
				
			||||||
{
 | 
					{
 | 
				
			||||||
	struct gsm_sms *sms = sms_alloc();
 | 
						struct gsm_sms *sms = sms_alloc();
 | 
				
			||||||
	long long unsigned int receiver_id;
 | 
					 | 
				
			||||||
	const char *text, *daddr, *saddr;
 | 
						const char *text, *daddr, *saddr;
 | 
				
			||||||
	const unsigned char *user_data;
 | 
						const unsigned char *user_data;
 | 
				
			||||||
 | 
					
 | 
				
			||||||
@@ -1275,16 +1273,6 @@ static struct gsm_sms *sms_from_result(struct gsm_network *net, dbi_result resul
 | 
				
			|||||||
 | 
					
 | 
				
			||||||
	sms->id = dbi_result_get_ulonglong(result, "id");
 | 
						sms->id = dbi_result_get_ulonglong(result, "id");
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	receiver_id = dbi_result_get_ulonglong(result, "receiver_id");
 | 
					 | 
				
			||||||
	sms->receiver = subscr_get_by_id(net, receiver_id);
 | 
					 | 
				
			||||||
	if (!sms->receiver) {
 | 
					 | 
				
			||||||
		LOGP(DLSMS, LOGL_ERROR,
 | 
					 | 
				
			||||||
			"Failed to find receiver(%llu) for id(%llu)\n",
 | 
					 | 
				
			||||||
			receiver_id, sms->id);
 | 
					 | 
				
			||||||
		sms_free(sms);
 | 
					 | 
				
			||||||
		return NULL;
 | 
					 | 
				
			||||||
	}
 | 
					 | 
				
			||||||
 | 
					 | 
				
			||||||
	/* FIXME: validity */
 | 
						/* FIXME: validity */
 | 
				
			||||||
	/* FIXME: those should all be get_uchar, but sqlite3 is braindead */
 | 
						/* FIXME: those should all be get_uchar, but sqlite3 is braindead */
 | 
				
			||||||
	sms->reply_path_req = dbi_result_get_uint(result, "reply_path_req");
 | 
						sms->reply_path_req = dbi_result_get_uint(result, "reply_path_req");
 | 
				
			||||||
@@ -1302,6 +1290,7 @@ static struct gsm_sms *sms_from_result(struct gsm_network *net, dbi_result resul
 | 
				
			|||||||
		strncpy(sms->dst.addr, daddr, sizeof(sms->dst.addr));
 | 
							strncpy(sms->dst.addr, daddr, sizeof(sms->dst.addr));
 | 
				
			||||||
		sms->dst.addr[sizeof(sms->dst.addr)-1] = '\0';
 | 
							sms->dst.addr[sizeof(sms->dst.addr)-1] = '\0';
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
 | 
						sms->receiver = subscr_get_by_extension(net, sms->dst.addr);
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	sms->src.npi = dbi_result_get_uint(result, "src_npi");
 | 
						sms->src.npi = dbi_result_get_uint(result, "src_npi");
 | 
				
			||||||
	sms->src.ton = dbi_result_get_uint(result, "src_ton");
 | 
						sms->src.ton = dbi_result_get_uint(result, "src_ton");
 | 
				
			||||||
@@ -1356,7 +1345,7 @@ struct gsm_sms *db_sms_get_unsent(struct gsm_network *net, unsigned long long mi
 | 
				
			|||||||
	result = dbi_conn_queryf(conn,
 | 
						result = dbi_conn_queryf(conn,
 | 
				
			||||||
		"SELECT SMS.* "
 | 
							"SELECT SMS.* "
 | 
				
			||||||
			"FROM SMS JOIN Subscriber ON "
 | 
								"FROM SMS JOIN Subscriber ON "
 | 
				
			||||||
				"SMS.receiver_id = Subscriber.id "
 | 
									"SMS.dest_addr = Subscriber.extension "
 | 
				
			||||||
			"WHERE SMS.id >= %llu AND SMS.sent IS NULL "
 | 
								"WHERE SMS.id >= %llu AND SMS.sent IS NULL "
 | 
				
			||||||
				"AND Subscriber.lac > 0 "
 | 
									"AND Subscriber.lac > 0 "
 | 
				
			||||||
			"ORDER BY SMS.id LIMIT 1",
 | 
								"ORDER BY SMS.id LIMIT 1",
 | 
				
			||||||
@@ -1386,10 +1375,10 @@ struct gsm_sms *db_sms_get_unsent_by_subscr(struct gsm_network *net,
 | 
				
			|||||||
	result = dbi_conn_queryf(conn,
 | 
						result = dbi_conn_queryf(conn,
 | 
				
			||||||
		"SELECT SMS.* "
 | 
							"SELECT SMS.* "
 | 
				
			||||||
			"FROM SMS JOIN Subscriber ON "
 | 
								"FROM SMS JOIN Subscriber ON "
 | 
				
			||||||
				"SMS.receiver_id = Subscriber.id "
 | 
									"SMS.dest_addr = Subscriber.extension "
 | 
				
			||||||
			"WHERE SMS.receiver_id >= %llu AND SMS.sent IS NULL "
 | 
								"WHERE Subscriber.id >= %llu AND SMS.sent IS NULL "
 | 
				
			||||||
				"AND Subscriber.lac > 0 AND SMS.deliver_attempts < %u "
 | 
									"AND Subscriber.lac > 0 AND SMS.deliver_attempts < %u "
 | 
				
			||||||
			"ORDER BY SMS.receiver_id, SMS.id LIMIT 1",
 | 
								"ORDER BY Subscriber.id, SMS.id LIMIT 1",
 | 
				
			||||||
		min_subscr_id, failed);
 | 
							min_subscr_id, failed);
 | 
				
			||||||
	if (!result)
 | 
						if (!result)
 | 
				
			||||||
		return NULL;
 | 
							return NULL;
 | 
				
			||||||
@@ -1415,8 +1404,8 @@ struct gsm_sms *db_sms_get_unsent_for_subscr(struct gsm_subscriber *subscr)
 | 
				
			|||||||
	result = dbi_conn_queryf(conn,
 | 
						result = dbi_conn_queryf(conn,
 | 
				
			||||||
		"SELECT SMS.* "
 | 
							"SELECT SMS.* "
 | 
				
			||||||
			"FROM SMS JOIN Subscriber ON "
 | 
								"FROM SMS JOIN Subscriber ON "
 | 
				
			||||||
				"SMS.receiver_id = Subscriber.id "
 | 
									"SMS.dest_addr = Subscriber.extension "
 | 
				
			||||||
			"WHERE SMS.receiver_id = %llu AND SMS.sent IS NULL "
 | 
								"WHERE Subscriber.id = %llu AND SMS.sent IS NULL "
 | 
				
			||||||
				"AND Subscriber.lac > 0 "
 | 
									"AND Subscriber.lac > 0 "
 | 
				
			||||||
			"ORDER BY SMS.id LIMIT 1",
 | 
								"ORDER BY SMS.id LIMIT 1",
 | 
				
			||||||
		subscr->id);
 | 
							subscr->id);
 | 
				
			||||||
 
 | 
				
			|||||||
		Reference in New Issue
	
	Block a user