Re: Patch: EMI UUCP DLRs (final)
"Nikos Balkanas" <[email protected]>
| Newsgroups | gmane.comp.mobile.kannel.devel |
|---|---|
| Message-ID | <22BC742A4646456E82E10BA40A8254B1@drwho> |
OK. I believe this is it. BR, Nikos ----- Original Message ----- From: "Alexander Malysh" <[email protected]> To: "Nikos Balkanas" <[email protected]> Cc: "Byron Kiourtzoglou" <[email protected]>; <[email protected]> Sent: Tuesday, July 20, 2010 12:14 PM Subject: Re: Patch: EMI UUCP DLRs (final) Hi Nikos, see below ... Am 20.07.2010 um 03:03 schrieb Nikos Balkanas: > Hi Alex, > > Please see inlined comments: I am still waiting feedback from some users > testing it. > >> ----- Original Message ----- From: "Alexander Malysh" >> <[email protected]> >> To: "Nikos Balkanas" <[email protected]> >> Cc: "Byron Kiourtzoglou" <[email protected]>; <[email protected]> >> Sent: Tuesday, July 20, 2010 1:14 AM >> Subject: Re: Patch: EMI UUCP DLRs (final) > > >> Hi Nikos, > >> I'm back from vacation and here are comments to your patch. Patch looks >> OK but still some things to fix: > >> --- gw/dlr_mem.c (revision 4833) >> +++ gw/dlr_mem.c (working copy) >> @@ -125,8 +125,28 @@ >> /* XXX: check destination addr too, because e.g. for UCP is not enough >> to check only >> * smsc and timestamp (timestamp is even without >> milliseconds) >> */ >> - if(octstr_compare(dlr->smsc,smsc) == 0 && >> octstr_compare(dlr->timestamp,ts) == 0) >> + if (dst){ >> + Octstr *dst_min; >> + int len1 = octstr_len(dlr->destination), len2 = octstr_len(dst); >> + >> + if (len1 < len2) >> + return(1); >> + >> + dst_min = octstr_duplicate(dlr->destination); >> + if (len1 > len2) >> + octstr_delete(dst_min, 0, len1 - len2); >> + >> + if (octstr_compare(dlr->smsc, smsc) == 0 && octstr_compare(dlr-> >> + timestamp, ts) == 0 && octstr_compare(dst_min, dst) == 0) { >> + octstr_destroy(dst_min); >> return 0; >> + } >> + octstr_destroy(dst_min); >> + } >> + else >> + if (octstr_compare(dlr->smsc, smsc) == 0 && octstr_compare(dlr-> >> + timestamp, ts) == 0) >> + return 0; > >> why so complicated? >> if (dst) { >> pos = octstr_len(dlr->destination) - octstr_len(dst); >> if(pos < 0) >> pos = 0; >> if (octstr_compare(dlr->smsc, smsc) == 0 && >> octstr_compare(dlr->timestamp, ts) == 0 && >> octstr_search(dlr->destination, dst, pos) !> = -1) >> return 0; >> } else ... > > Same degree of complexity. You use octstr_search, I use octstr_compare. > Personally I wouldn't use either, but octstr_search is slightly faster. > The only reason I used octstr_delete & compare is because you told me to > use truncate on a copy, and I even asked you twice about it. Will change > it. It's not the same. In my version you don't need duplicate, destroy etc. and it just simpler to read... > >> --- gw/dlr_sdb.c (revision 4833) >> +++ gw/dlr_sdb.c (working copy) >> + if (dst) >> + like = octstr_format("AND \"%S\" LIKE '%%%S' %s", >> fields->field_dst, >> + dst, sdb_get_limit_str()); >> + else >> + like = octstr_imm(sdb_get_limit_str()); > >> why not keep sdb_get_limit() in place and do it as for other DBs? >> and \"%S... seems to be wrong... > > I don't understand what you are asking for here. The code was like that > before and I didn't change it. \"%S\" is a typo. Will fix. why did you put sdb_get_limit_str() here? it should be keep in sql = octstr_format("select ... %s", sdb_get_limit_str, ...)... > >> --- gw/dlr.c (revision 4833) >> +++ gw/dlr.c (working copy) >> Msg *msg = NULL; > > struct dlr_entry *dlr = NULL; >> + Octstr *dst_min = NULL; > >> indents.... > >> + if (use_dst && dst) { >> + dst_min = octstr_duplicate(dst); >> + int len = octstr_len(dst); >> + >> + if (len > MIN_DST_LEN) >> + octstr_delete(dst_min, 0, len - MIN_DST_LEN); >> + } > > I see. Intends are 3 spaces, instead of 4. That's why I don't use spaces. > I tried my best, but please feel free to adjust them exactly the way you > like them. :) kannel uses 4 spaces instead of tabs... > >> indents... > >> /* destroy struct dlr_entry */ >> dlr_entry_destroy(dlr); >> + octstr_destroy(dst_min); > >> ditto > > > BR, > Nikos > --- gw/dlr.h (revision 4833) > +++ gw/dlr.h (working copy) > +Msg* dlr_find(const Octstr *smsc, const Octstr *ts, const Octstr *dst, > int type, > + int use_dst); > > change to > +Msg* dlr_find(const Octstr *smsc, const Octstr *ts, const Octstr *dst, > int type, int use_dst); > > > Thanks, > Alexander Malysh > > Am 14.07.2010 um 14:38 schrieb Nikos Balkanas: > >> Here it is. Added support for dest also for cimd2 as reported by Byron. >> >> I have tested thoroughly dlr_mem.c, but only compilation for DBs, since i >> have no access to them. >> >> @Byron: Could you please test patch for CIMD2 against some of the DBs you >> use? >> >> Thanks, >> Nikos >> ----- Original Message ----- From: "Alexander Malysh" >> <[email protected]> >> To: "Nikos Balkanas" <[email protected]> >> Cc: <[email protected]> >> Sent: Wednesday, July 07, 2010 6:11 PM >> Subject: Re: Patch: EMI UUCP DLRs (final) >> >> >> Hi, >> >> sorry for delay... I have limited inet access now... see answers bellow. >> >> Am 30.06.2010 um 19:30 schrieb Nikos Balkanas: >> >>> Hi, >>> >>> Please see inlined answers. >>> Thanks for the comments and corrections. Please confirm a few remaining >>> choices. >>> >>> BR, >>> Nikos >>> ----- Original Message ----- From: "Alexander Malysh" >>> <[email protected]> >>> To: "Nikos Balkanas" <[email protected]> >>> Cc: <[email protected]> >>> Sent: Wednesday, June 30, 2010 12:02 PM >>> Subject: Re: Patch: EMI UUCP DLRs (final) >>> >>> >>>> Hi, >>> >>>> IMO we don't need to handle no destination case in DLR lookup but maybe >>>> it's not a wrong idea to be able to ignore destination for some >>>> reasons when SMSC send some junk to us. >>> >>> Not doing it, could miss some queries altogether. It would still work >>> for the large majority, but could miss a few matches that would have >>> gotten otherwise. >> >> this is ok for me, we make it optional... >> >>> >>>> ok here is the review for your patch: >>>> + if (dst){ >>>> + int len = octstr_len(dst); >>>> + char *p = octstr_get_cstr(dst); >>> >>>> + if (len > MIN_DST_LEN) >>>> + p += len - MIN_DST_LEN; /* get last MIN_DST_LEN >>>> digits */ >>>> + like = octstr_create(strcat("%", p)); >>>> + gwlist_append(binds, like); >>>> + } >>> >>>> why is this in every dlr implementation? we have abstraction for this, >>>> see dlr.c >>> >>> I don't know where in dlr.c you are referring, but I see your reasoning. >>> This is leftover from a previous implementation, where I passed use_dest >>> in the dlr_<DB>. I considered it at the time important to pass the >>> whole dest to dlr_<db> so that debug messages get the whole dest. Since >>> I started passing NULL for dest, this serves no more purpose. I will >>> abstract it in dlr_find and use octstr_delete instead. Will also correct >>> debug messages accordingly in dlr_db.c. >> >> you have to abstract it in dlr_find and not repeat the same code in each >> dlr_db driver. >> >>> >>>> - sql = octstr_format("SELECT `%S`, `%S`, `%S`, `%S`, `%S`, `%S` >>>> FROM `%S` WHERE `%S`=? AND `%S`=? LIMIT 1", >>>> + if (dst) >>>> + sql = octstr_format("SELECT `%S`, `%S`, `%S`, `%S`, `%S`, `%S` >>>> FROM `%S` WHERE `%S`=? AND `%S`=? AND `%S` >LIKE ? LIMIT 1", >>>> fields->field_mask, fields->field_serv, >>>> fields->field_url, fields->field_src, >>>> fields->field_dst, fields->field_boxc, >>>> fields->table, fields->field_smsc, >>>> + fields->field_ts, fields->field_dst); >>>> ... >>>> First of all: like ? doesn't work as you expect... it should be >>>> something like: LIKE CONCAT('%%', ?) and >>> >>> Thanks. I will look into it. >>> >>>> this is too much maintenance for SQL that defined two times, how about >>>> like this: >>>> if (dst) >>>> like = octstr_format('LIKE CONCAT('%%', ?)' ...); >>>> else >>>> like = octstr_create(""); >>> >>> You probably mean: >>> >>> like = octstr_create("=?"); >> >> no, I mean octstr_create("") or better use octstr_imm("") >> >>> >>>> sql = ...(".... %S", like) >>> >>> This is a classical maintenance vs overhead. Malloc is expensive, much >>> more so in Linux than in Solaris. Furthermore, sql="%s%S" is more >>> difficult to read and understand, since SQL mechanism is not explicit, >>> but hidden in variables. Do you really want that? >> >> yes, because maintenance is then easier and malloc overhead is in linux >> not so much expensive as you think because glibc has preallocated memory >> pools and not always is system call >> needed. >> >>> >>>> The same is for DELETE, UPDATE... >>> >>>> + if (like) octstr_destroy(like); >>> >>>> octstr_destroy will check for NULL for you... >>> >>> I am aware of that, but it costs a function call and a few more >>> statements...Either way is fine. I can change it. >> >> function call is not really issue against code readability... and if this >> is really your argument then convert all calls to if (..!=NULL) bla... >> (just a joke) >> >>> >>>> + if (octstr_compare(dlr->smsc, smsc) == 0 && >>>> octstr_compare(dlr-> >>>> + timestamp, ts) == 0 && memcmp(p1 + len1 - size, p2 + len2 - >>>> size, >>>> + size) == 0) >>>> + return 0; >>> >>>> memcmp??? why not just use truncated destination and do: >>>> octstr_search??? >>> >>> Actually octstr_truncate won't work since it truncates from the end. >>> octstr_delete, would work, however, destroying the original Octstr in >>> the process, unless I duplicated them. It would need to be done on both >>> destinations to work. Code would be more, and the malloc, free and copy, >>> have an overhead. Memcmp doesn't change the original Octstr and is >>> natural for such operations. However, it is out of kannel style, so you >>> have every right to ask me to change it. Do you? >> >> yes, please change it. >> >>> >>>> +Msg *dlr_find(const Octstr *smsc, const Octstr *ts, const Octstr *dst, >>>> int typ, int use_dst) >>> >>>> you don't need to change function. Just use dst = NULL and check it. >>> >>> No. dst is needed for debug msgs inside dlr_find. Furthermore, use_dst, >>> currently is set only for EMI. Decision is made at driver level so it >>> can easily change to an smsc configuration variable if needed. >> >> ok, maybe you are right. What other people think about this ? >> >>> >>> Am 26.06.2010 um 10:23 schrieb Nikos Balkanas: >>> >>>> Hi, >>>> >>>> I believe I have accounted for almost all your comments to produce the >>>> final working patch. I have no way of testing, other than compilation. >>>> >>>> This patch will povide: >>>> >>>> 1) Align dlr_oracle.c with the rest. Currently, Oracle does a full dst >>>> find/remove/update on each DLR. >>>> 2) Dst use on find/remove/update is controlled in dlr_find by a single >>>> variable, use_dst. This is currently set only at driver level and only >>>> in the emi driver, while everyone else has it false. However, very >>>> easily, if need arises, it can be set in smsc configuration. >>>> 3) All DLR handling for all smscs will remain as it used to be till >>>> now. Only emi handling changes, hopefully for the better (that is the >>>> purpose of the patch :-)). >>>> 4) It will try to match the last 7 digits of the destination or the >>>> length of the destination if it smaller. This is defined in gw/dlr_p.h >>>> as MIN_DST_LEN. Didn't want to make it larger, wanted to avoid prefix >>>> territory at all costs, since I have seen a lot of mangling there by >>>> the SMScs. Besides, I believe that 7 digits give enough resolution >>>> 5) People using emi, should rebuild their indeces, especially if they >>>> are running large batch jobs through EMI. The LIKE % construction is >>>> not very efficient. >>>> >>>> Enjoy, >>>> Nikos >>>> <kannel.diff> >>> >> <kannel.diff> >
kannel.diff
(application/octet-stream, 32.4 KB)
Index: gw/dlr_mysql.c
===================================================================
--- gw/dlr_mysql.c (revision 4833)
+++ gw/dlr_mysql.c (working copy)
@@ -138,7 +138,7 @@
static struct dlr_entry* dlr_mysql_get(const Octstr *smsc, const Octstr *ts, const Octstr *dst)
{
- Octstr *sql;
+ Octstr *sql, *like;
DBPoolConn *pconn;
List *result = NULL, *row;
struct dlr_entry *res = NULL;
@@ -148,15 +148,22 @@
if (pconn == NULL) /* should not happens, but sure is sure */
return NULL;
- sql = octstr_format("SELECT `%S`, `%S`, `%S`, `%S`, `%S`, `%S` FROM `%S` WHERE `%S`=? AND `%S`=? LIMIT 1",
+ if (dst)
+ like = octstr_format("AND `%S` LIKE CONCAT('%%', ?)", fields->field_dst);
+ else
+ like = octstr_imm("");
+
+ sql = octstr_format("SELECT `%S`, `%S`, `%S`, `%S`, `%S`, `%S` FROM `%S` WHERE `%S`=? AND `%S`=? %S LIMIT 1",
fields->field_mask, fields->field_serv,
fields->field_url, fields->field_src,
fields->field_dst, fields->field_boxc,
fields->table, fields->field_smsc,
- fields->field_ts);
+ fields->field_ts, like);
gwlist_append(binds, (Octstr *)smsc);
gwlist_append(binds, (Octstr *)ts);
+ if (dst)
+ gwlist_append(binds, (Octstr *)dst);
#if defined(DLR_TRACE)
debug("dlr.mysql", 0, "sql: %s", octstr_get_cstr(sql));
@@ -164,10 +171,12 @@
if (dbpool_conn_select(pconn, sql, binds, &result) != 0) {
octstr_destroy(sql);
+ octstr_destroy(like);
dbpool_conn_produce(pconn);
return NULL;
}
octstr_destroy(sql);
+ octstr_destroy(like);
gwlist_destroy(binds, NULL);
dbpool_conn_produce(pconn);
@@ -195,7 +204,7 @@
static void dlr_mysql_remove(const Octstr *smsc, const Octstr *ts, const Octstr *dst)
{
- Octstr *sql;
+ Octstr *sql, *like;
DBPoolConn *pconn;
List *binds = gwlist_create();
int res;
@@ -207,12 +216,19 @@
if (pconn == NULL)
return;
- sql = octstr_format("DELETE FROM `%S` WHERE `%S`=? AND `%S`=? LIMIT 1",
+ if (dst)
+ like = octstr_format("AND `%S` LIKE CONCAT('%%', ?)", fields->field_dst);
+ else
+ like = octstr_imm("");
+
+ sql = octstr_format("DELETE FROM `%S` WHERE `%S`=? AND `%S`=? %S LIMIT 1",
fields->table, fields->field_smsc,
- fields->field_ts);
+ fields->field_ts, like);
gwlist_append(binds, (Octstr *)smsc);
gwlist_append(binds, (Octstr *)ts);
+ if (dst)
+ gwlist_append(binds, (Octstr *)dst);
#if defined(DLR_TRACE)
debug("dlr.mysql", 0, "sql: %s", octstr_get_cstr(sql));
@@ -226,11 +242,12 @@
dbpool_conn_produce(pconn);
gwlist_destroy(binds, NULL);
octstr_destroy(sql);
+ octstr_destroy(like);
}
static void dlr_mysql_update(const Octstr *smsc, const Octstr *ts, const Octstr *dst, int status)
{
- Octstr *sql, *os_status;
+ Octstr *sql, *os_status, *like;
DBPoolConn *pconn;
List *binds = gwlist_create();
int res;
@@ -242,14 +259,22 @@
if (pconn == NULL)
return;
- sql = octstr_format("UPDATE `%S` SET `%S`=? WHERE `%S`=? AND `%S`=? LIMIT 1",
+ if (dst)
+ like = octstr_format("AND `%S` LIKE CONCAT('%%', ?)", fields->field_dst);
+ else
+ like = octstr_imm("");
+
+ sql = octstr_format("UPDATE `%S` SET `%S`=? WHERE `%S`=? AND `%S`=? %S LIMIT 1",
fields->table, fields->field_status,
- fields->field_smsc, fields->field_ts);
+ fields->field_smsc, fields->field_ts,
+ like);
os_status = octstr_format("%d", status);
gwlist_append(binds, (Octstr *)os_status);
gwlist_append(binds, (Octstr *)smsc);
gwlist_append(binds, (Octstr *)ts);
+ if (dst)
+ gwlist_append(binds, (Octstr *)like);
#if defined(DLR_TRACE)
debug("dlr.mysql", 0, "sql: %s", octstr_get_cstr(sql));
@@ -263,9 +288,9 @@
gwlist_destroy(binds, NULL);
octstr_destroy(os_status);
octstr_destroy(sql);
+ octstr_destroy(like);
}
-
static long dlr_mysql_messages(void)
{
List *result, *row;
@@ -446,4 +471,3 @@
return NULL;
}
#endif /* HAVE_MYSQL */
-
Index: gw/dlr_oracle.c
===================================================================
--- gw/dlr_oracle.c (revision 4833)
+++ gw/dlr_oracle.c (working copy)
@@ -171,7 +171,7 @@
static void dlr_remove_oracle(const Octstr *smsc, const Octstr *ts, const Octstr *dst)
{
- Octstr *sql;
+ Octstr *sql, *like;
DBPoolConn *pconn;
List *binds = gwlist_create();
int res;
@@ -183,13 +183,19 @@
if (pconn == NULL)
return;
- sql = octstr_format("DELETE FROM %S WHERE %S=:1 AND %S=:2 AND %S=:3 AND ROWNUM < 2",
+ if (dst)
+ like = octstr_format("AND %S LIKE CONCAT('%%', :3)", fields->field_dst);
+ else
+ like = octstr_imm("");
+
+ sql = octstr_format("DELETE FROM %S WHERE %S=:1 AND %S=:2 %S AND ROWNUM < 2",
fields->table, fields->field_smsc,
- fields->field_ts, fields->field_dst);
+ fields->field_ts, like);
gwlist_append(binds, (Octstr *)smsc); /* :1 */
gwlist_append(binds, (Octstr *)ts); /* :2 */
- gwlist_append(binds, (Octstr *)dst); /* :3 */
+ if (dst) /* :3 */
+ gwlist_append(binds, (Octstr *)dst);
#if defined(DLR_TRACE)
debug("dlr.oracle", 0, "sql: %s", octstr_get_cstr(sql));
@@ -203,11 +209,12 @@
dbpool_conn_produce(pconn);
gwlist_destroy(binds, NULL);
octstr_destroy(sql);
+ octstr_destroy(like);
}
static struct dlr_entry* dlr_get_oracle(const Octstr *smsc, const Octstr *ts, const Octstr *dst)
{
- Octstr *sql;
+ Octstr *sql, *like;
DBPoolConn *pconn;
List *result = NULL, *row;
struct dlr_entry *res = NULL;
@@ -217,26 +224,35 @@
if (pconn == NULL) /* should not happens, but sure is sure */
return NULL;
- sql = octstr_format("SELECT %S, %S, %S, %S, %S, %S FROM %S WHERE %S=:1 AND %S=:2 AND %S=:3 AND ROWNUM < 2",
+ if (dst)
+ like = octstr_format("AND %S LIKE CONCAT('%%', :3)",
+ fields->field_dst);
+ else
+ like = octstr_imm("");
+
+ sql = octstr_format("SELECT %S, %S, %S, %S, %S, %S FROM %S WHERE %S=:1 AND %S=:2 %S ROWNUM < 2",
fields->field_mask, fields->field_serv,
fields->field_url, fields->field_src,
fields->field_dst, fields->field_boxc,
fields->table, fields->field_smsc,
- fields->field_ts, fields->field_dst);
+ fields->field_ts, like);
gwlist_append(binds, (Octstr *)smsc); /* :1 */
gwlist_append(binds, (Octstr *)ts); /* :2 */
- gwlist_append(binds, (Octstr *)dst); /* :3 */
+ if (dst) /* :3 */
+ gwlist_append(binds, (Octstr *)dst);
#if defined(DLR_TRACE)
debug("dlr.oracle", 0, "sql: %s", octstr_get_cstr(sql));
#endif
if (dbpool_conn_select(pconn, sql, binds, &result) != 0) {
octstr_destroy(sql);
+ octstr_destroy(like);
dbpool_conn_produce(pconn);
return NULL;
}
octstr_destroy(sql);
+ octstr_destroy(like);
gwlist_destroy(binds, NULL);
dbpool_conn_produce(pconn);
@@ -264,7 +280,7 @@
static void dlr_update_oracle(const Octstr *smsc, const Octstr *ts, const Octstr *dst, int status)
{
- Octstr *sql, *os_status;
+ Octstr *sql, *os_status, *like;
DBPoolConn *pconn;
List *binds = gwlist_create();
int res;
@@ -276,15 +292,22 @@
if (pconn == NULL)
return;
- sql = octstr_format("UPDATE %S SET %S=:1 WHERE %S=:2 AND %S=:3 AND %S=:4 AND ROWNUM < 2",
+ if (dst)
+ like = octstr_format("AND %S LIKE CONCAT('%%', :4)", fields->field_dst);
+ else
+ like = octstr_imm("");
+
+ sql = octstr_format("UPDATE %S SET %S=:1 WHERE %S=:2 AND %S=:3 %S AND ROWNUM < 2",
fields->table, fields->field_status,
- fields->field_smsc, fields->field_ts, fields->field_dst);
+ fields->field_smsc, fields->field_ts, like);
os_status = octstr_format("%d", status);
gwlist_append(binds, (Octstr *)os_status); /* :1 */
gwlist_append(binds, (Octstr *)smsc); /* :2 */
gwlist_append(binds, (Octstr *)ts); /* :3 */
- gwlist_append(binds, (Octstr *)dst); /* :4 */
+ if (dst) /* :4 */
+ gwlist_append(binds, (Octstr *)dst);
+
#if defined(DLR_TRACE)
debug("dlr.oracle", 0, "sql: %s", octstr_get_cstr(sql));
#endif
@@ -297,6 +320,7 @@
gwlist_destroy(binds, NULL);
octstr_destroy(os_status);
octstr_destroy(sql);
+ octstr_destroy(like);
}
static void dlr_flush_oracle (void)
Index: gw/dlr_pgsql.c
===================================================================
--- gw/dlr_pgsql.c (revision 4833)
+++ gw/dlr_pgsql.c (working copy)
@@ -165,19 +165,23 @@
static struct dlr_entry *dlr_pgsql_get(const Octstr *smsc, const Octstr *ts, const Octstr *dst)
{
struct dlr_entry *res = NULL;
- Octstr *sql;
+ Octstr *sql, *like;
List *result, *row;
- sql = octstr_format("SELECT \"%s\", \"%s\", \"%s\", \"%s\", \"%s\", \"%s\" FROM \"%s\" WHERE \"%s\"='%s' AND \"%s\"='%s' LIMIT 1;",
- octstr_get_cstr(fields->field_mask), octstr_get_cstr(fields->field_serv),
- octstr_get_cstr(fields->field_url), octstr_get_cstr(fields->field_src),
- octstr_get_cstr(fields->field_dst), octstr_get_cstr(fields->field_boxc),
- octstr_get_cstr(fields->table), octstr_get_cstr(fields->field_smsc),
- octstr_get_cstr(smsc), octstr_get_cstr(fields->field_ts), octstr_get_cstr(ts));
+ if (dst)
+ like = octstr_format("AND \"%S\" LIKE '%%%S'", fields->field_dst, dst);
+ else
+ like = octstr_imm("");
+ sql = octstr_format("SELECT \"%S\", \"%S\", \"%S\", \"%S\", \"%S\", "
+ "\"%S\" FROM \"%S\" WHERE \"%S\"='%S' AND \"%S\"='%S' %S LIMIT 1;",
+ fields->field_mask, fields->field_serv, fields->field_url,
+ fields->field_src, fields->field_dst, fields->field_boxc,
+ fields->table, fields->field_smsc, smsc, fields->field_ts, ts, like);
result = pgsql_select(sql);
octstr_destroy(sql);
+ octstr_destroy(like);
if (result == NULL || gwlist_len(result) < 1) {
debug("dlr.pgsql", 0, "no rows found");
@@ -186,7 +190,7 @@
gwlist_destroy(result, NULL);
return NULL;
}
-
+
row = gwlist_get(result, 0);
debug("dlr.pgsql", 0, "Found entry, col1=%s, col2=%s, col3=%s, col4=%s, col5=%s col6=%s",
@@ -211,42 +215,52 @@
while((row = gwlist_extract_first(result)))
gwlist_destroy(row, octstr_destroy_item);
gwlist_destroy(result, NULL);
-
+
return res;
}
static void dlr_pgsql_remove(const Octstr *smsc, const Octstr *ts, const Octstr *dst)
{
- Octstr *sql;
+ Octstr *sql, *like;
debug("dlr.pgsql", 0, "removing DLR from database");
- sql = octstr_format("DELETE FROM \"%s\" WHERE oid = (SELECT oid FROM \"%s\" WHERE \"%s\"='%s' AND \"%s\"='%s' LIMIT 1);",
- octstr_get_cstr(fields->table), octstr_get_cstr(fields->table),
- octstr_get_cstr(fields->field_smsc),
- octstr_get_cstr(smsc), octstr_get_cstr(fields->field_ts), octstr_get_cstr(ts));
+ if (dst)
+ like = octstr_format("AND \"%S\" LIKE '%%%S'", fields->field_dst, dst);
+ else
+ like = octstr_imm("");
+ sql = octstr_format("DELETE FROM \"%S\" WHERE oid = (SELECT oid FROM "
+ "\"%S\" WHERE \"%S\"='%S' AND \"%S\"='%S' %S LIMIT 1);",
+ fields->table, fields->table, fields->field_smsc, smsc,
+ fields->field_ts, ts, like);
if (!pgsql_update(sql))
warning(0, "DLR: PGSQL: No dlr deleted for DST<%s>", octstr_get_cstr(dst));
octstr_destroy(sql);
+ octstr_destroy(like);
}
static void dlr_pgsql_update(const Octstr *smsc, const Octstr *ts, const Octstr *dst, int status)
{
- Octstr *sql;
+ Octstr *sql, *like;
debug("dlr.pgsql", 0, "updating DLR status in database");
- sql = octstr_format("UPDATE \"%s\" SET \"%s\"=%d WHERE oid = (SELECT oid FROM \"%s\" WHERE \"%s\"='%s' AND \"%s\"='%s' LIMIT 1);",
- octstr_get_cstr(fields->table),
- octstr_get_cstr(fields->field_status), status,
- octstr_get_cstr(fields->table),
- octstr_get_cstr(fields->field_smsc), octstr_get_cstr(smsc),
- octstr_get_cstr(fields->field_ts), octstr_get_cstr(ts));
+ if (dst)
+ like = octstr_format("AND \"%S\" LIKE '%%%S'", fields->field_dst, dst);
+ else
+ like = octstr_imm("");
+
+ sql = octstr_format("UPDATE \"%S\" SET \"%S\"=%d WHERE oid = (SELECT "
+ "oid FROM \"%S\" WHERE \"%S\"='%S' AND \"%S\"='%S' %S LIMIT 1);",
+ fields->table, fields->field_status, status, fields->table,
+ fields->field_smsc, smsc, fields->field_ts, ts, like);
+
if (!pgsql_update(sql))
warning(0, "DLR: PGSQL: No dlr updated for DST<%s> (status: %d)", octstr_get_cstr(dst), status);
octstr_destroy(sql);
+ octstr_destroy(like);
}
Index: gw/dlr_mem.c
===================================================================
--- gw/dlr_mem.c (revision 4833)
+++ gw/dlr_mem.c (working copy)
@@ -125,8 +125,21 @@
/* XXX: check destination addr too, because e.g. for UCP is not enough to check only
* smsc and timestamp (timestamp is even without milliseconds)
*/
- if(octstr_compare(dlr->smsc,smsc) == 0 && octstr_compare(dlr->timestamp,ts) == 0)
+ if (dst){
+ int pos = octstr_len(dlr->destination) - octstr_len(dst);
+
+ if (pos < 0)
+ return(1);
+
+ if (octstr_compare(dlr->smsc, smsc) == 0 && octstr_compare(dlr->
+ timestamp, ts) == 0 && octstr_search(dlr->destination, dst, pos) !=
+ -1)
return 0;
+ }
+ else
+ if (octstr_compare(dlr->smsc, smsc) == 0 && octstr_compare(dlr->
+ timestamp, ts) == 0)
+ return 0;
return 1;
}
Index: gw/dlr_mssql.c
===================================================================
--- gw/dlr_mssql.c (revision 4833)
+++ gw/dlr_mssql.c (working copy)
@@ -161,7 +161,7 @@
static void dlr_remove_mssql(const Octstr *smsc, const Octstr *ts, const Octstr *dst)
{
- Octstr *sql;
+ Octstr *sql, *like;
DBPoolConn *pconn;
int res;
@@ -172,12 +172,15 @@
if (pconn == NULL)
return;
- sql = octstr_format("SET ROWCOUNT 1\n"
- "DELETE FROM %S WHERE %S='%S' AND %S='%S'\n"
- "SET ROWCOUNT 0",
- fields->table, fields->field_smsc, smsc,
- fields->field_ts, ts);
+ if (dst)
+ like = octstr_format("AND %S LIKE '%%%S'", fields->field_dst, dst);
+ else
+ like = octstr_imm("");
+ sql = octstr_format("SET ROWCOUNT 1\nDELETE FROM %S WHERE %S='%S' AND "
+ "%S='%S' %S \nSET ROWCOUNT 0", fields->table, fields->field_smsc,
+ smsc, fields->field_ts, ts, like);
+
#if defined(DLR_TRACE)
debug("dlr.mssql", 0, "sql: %s", octstr_get_cstr(sql));
#endif
@@ -189,11 +192,12 @@
dbpool_conn_produce(pconn);
octstr_destroy(sql);
+ octstr_destroy(like);
}
static struct dlr_entry* dlr_get_mssql(const Octstr *smsc, const Octstr *ts, const Octstr *dst)
{
- Octstr *sql;
+ Octstr *sql, *like;
DBPoolConn *pconn;
List *result = NULL, *row;
struct dlr_entry *res = NULL;
@@ -202,11 +206,17 @@
if (pconn == NULL) /* should not happens, but sure is sure */
return NULL;
- sql = octstr_format("SELECT %S, %S, %S, %S, %S, %S FROM %S WHERE %S='%S' AND %S='%S'",
- fields->field_mask, fields->field_serv, fields->field_url,
- fields->field_src, fields->field_dst, fields->field_boxc,
- fields->table, fields->field_smsc, smsc, fields->field_ts, ts);
+ if (dst)
+ like = octstr_format("AND %S LIKE '%%%S'", fields->field_dst, dst);
+ else
+ like = octstr_imm("");
+ sql = octstr_format("SELECT %S, %S, %S, %S, %S, %S FROM %S WHERE %S='%S'"
+ " AND %S='%S' %S", fields->field_mask, fields->field_serv,
+ fields->field_url, fields->field_src, fields->field_dst,
+ fields->field_boxc, fields->table, fields->field_smsc, smsc,
+ fields->field_ts, ts, like);
+
#if defined(DLR_TRACE)
debug("dlr.mssql", 0, "sql: %s", octstr_get_cstr(sql));
#endif
@@ -216,6 +226,7 @@
return NULL;
}
octstr_destroy(sql);
+ octstr_destroy(like);
dbpool_conn_produce(pconn);
#define LO2CSTR(r, i) octstr_get_cstr(gwlist_get(r, i))
@@ -242,7 +253,7 @@
static void dlr_update_mssql(const Octstr *smsc, const Octstr *ts, const Octstr *dst, int status)
{
- Octstr *sql, *os_status;
+ Octstr *sql, *like;
DBPoolConn *pconn;
int res;
@@ -253,12 +264,16 @@
if (pconn == NULL)
return;
- sql = octstr_format("SET ROWCOUNT 1\n"
- "UPDATE %S SET %S=%d WHERE %S='%S' AND %S='%S'\n"
- "SET ROWCOUNT 0",
- fields->table, fields->field_status, status,
- fields->field_smsc, smsc, fields->field_ts, ts);
+ if (dst)
+ like = octstr_format("AND %S LIKE '%%%S'", fields->field_dst, dst);
+ else
+ like = octstr_imm("");
+ sql = octstr_format("SET ROWCOUNT 1\nUPDATE %S SET %S=%d WHERE %S='%S' "
+ "AND %S='%S' %S\nSET ROWCOUNT 0",
+ fields->table, fields->field_status, status, fields->field_smsc, smsc,
+ fields->field_ts, ts, like);
+
#if defined(DLR_TRACE)
debug("dlr.mssql", 0, "sql: %s", octstr_get_cstr(sql));
#endif
@@ -268,8 +283,8 @@
warning(0, "DLR: MSSQL: No dlr found to update for DST<%s> (status: %d)", octstr_get_cstr(dst), status);
dbpool_conn_produce(pconn);
- octstr_destroy(os_status);
octstr_destroy(sql);
+ octstr_destroy(like);
}
static void dlr_flush_mssql (void)
Index: gw/dlr_sdb.c
===================================================================
--- gw/dlr_sdb.c (revision 4833)
+++ gw/dlr_sdb.c (working copy)
@@ -212,26 +212,30 @@
static struct dlr_entry* dlr_sdb_get(const Octstr *smsc, const Octstr *ts, const Octstr *dst)
{
- Octstr *sql;
+ Octstr *sql, *like;
int state;
struct dlr_entry *res = dlr_entry_create();
gw_assert(res != NULL);
- sql = octstr_format("SELECT %s, %s, %s, %s, %s, %s FROM %s WHERE %s='%s' AND %s='%s' %s",
- octstr_get_cstr(fields->field_mask), octstr_get_cstr(fields->field_serv),
- octstr_get_cstr(fields->field_url), octstr_get_cstr(fields->field_src),
- octstr_get_cstr(fields->field_dst), octstr_get_cstr(fields->field_boxc),
- octstr_get_cstr(fields->table),
- octstr_get_cstr(fields->field_smsc), octstr_get_cstr(smsc),
- octstr_get_cstr(fields->field_ts), octstr_get_cstr(ts), sdb_get_limit_str());
+ if (dst)
+ like = octstr_format("AND %S LIKE '%%%S'", fields->field_dst, dst);
+ else
+ like = octstr_imm("");
+ sql = octstr_format("SELECT %S, %S, %S, %S, %S, %S FROM %S WHERE %S='%S' "
+ "AND %S='%S' %S %s", fields->field_mask, fields->field_serv,
+ fields->field_url, fields->field_src, fields->field_dst,
+ fields->field_boxc, fields->table, fields->field_smsc, smsc,
+ fields->field_ts, ts, like, sdb_get_limit_str());
+
#if defined(DLR_TRACE)
debug("dlr.sdb", 0, "SDB: sql: %s", octstr_get_cstr(sql));
#endif
state = gw_sdb_query(octstr_get_cstr(sql), sdb_callback_add, res);
octstr_destroy(sql);
+ octstr_destroy(like);
if (state == -1) {
error(0, "SDB: error in finding DLR");
goto notfound;
@@ -252,22 +256,27 @@
static void dlr_sdb_update(const Octstr *smsc, const Octstr *ts, const Octstr *dst, int status)
{
- Octstr *sql;
+ Octstr *sql, *like;
int state;
debug("dlr.sdb", 0, "SDB: updating DLR status in database");
- sql = octstr_format("UPDATE %s SET %s=%d WHERE %s='%s' AND %s='%s' %s",
- octstr_get_cstr(fields->table),
- octstr_get_cstr(fields->field_status), status,
- octstr_get_cstr(fields->field_smsc), octstr_get_cstr(smsc),
- octstr_get_cstr(fields->field_ts), octstr_get_cstr(ts), sdb_get_limit_str());
+ if (dst)
+ like = octstr_format("AND %S LIKE '%%%S'", fields->field_dst, dst);
+ else
+ like = octstr_imm("");
+
+ sql = octstr_format("UPDATE %S SET %S=%d WHERE %S='%S' AND %S='%S' %S %s",
+ fields->table, fields->field_status, status, fields->field_smsc,
+ smsc, fields->field_ts, ts, like, sdb_get_limit_str());
+
#if defined(DLR_TRACE)
debug("dlr.sdb", 0, "SDB: sql: %s", octstr_get_cstr(sql));
#endif
state = gw_sdb_query(octstr_get_cstr(sql), NULL, NULL);
octstr_destroy(sql);
+ octstr_destroy(like);
if (state == -1)
error(0, "SDB: error in updating DLR");
else if (!state)
@@ -276,7 +285,7 @@
static void dlr_sdb_remove(const Octstr *smsc, const Octstr *ts, const Octstr *dst)
{
- Octstr *sql;
+ Octstr *sql, *like;
int state;
debug("dlr.sdb", 0, "removing DLR from database");
@@ -288,17 +297,24 @@
* to do vacuum regularly, even if it's virtually impossible
* to hit duplicates since oid's are given in a row
*/
- sql = octstr_format("DELETE FROM %s WHERE oid = \
- (SELECT oid FROM %s WHERE %s='%s' AND %s='%s' LIMIT 1)",
- octstr_get_cstr(fields->table),
- octstr_get_cstr(fields->table),
- octstr_get_cstr(fields->field_smsc), octstr_get_cstr(smsc),
- octstr_get_cstr(fields->field_ts), octstr_get_cstr(ts));
+ if (dst)
+ like = octstr_format("AND %S LIKE '%%%S')",
+ fields->field_dst, dst);
+ else
+ like = octstr_imm("LIMIT 1)");
+ sql = octstr_format("DELETE FROM %S WHERE oid = (SELECT oid FROM %S "
+ "WHERE %S='%S' AND %S='%S' %S LIMIT 1", fields->table,
+ fields->table, fields->field_smsc, smsc, fields->field_ts, ts,
+ like);
} else {
- sql = octstr_format("DELETE FROM %s WHERE %s='%s' AND %s='%s' %s",
- octstr_get_cstr(fields->table),
- octstr_get_cstr(fields->field_smsc), octstr_get_cstr(smsc),
- octstr_get_cstr(fields->field_ts), octstr_get_cstr(ts), sdb_get_limit_str());
+ if (dst)
+ like = octstr_format("AND %S LIKE '%%%S'", fields->field_dst, dst);
+ else
+ like = octstr_imm("");
+
+ sql = octstr_format("DELETE FROM %S WHERE %S='%S' AND %S='%S' %S %s",
+ fields->table, fields->field_smsc, smsc, fields->field_ts, ts,
+ like, sdb_get_limit_str());
}
#if defined(DLR_TRACE)
@@ -307,6 +323,7 @@
state = gw_sdb_query(octstr_get_cstr(sql), NULL, NULL);
octstr_destroy(sql);
+ octstr_destroy(like);
if (state == -1)
error(0, "SDB: error in deleting DLR");
else if (!state)
Index: gw/dlr.c
===================================================================
--- gw/dlr.c (revision 4833)
+++ gw/dlr.c (working copy)
@@ -371,10 +371,11 @@
* NOTE: If typ is end status (e.g. DELIVERED) then dlr entry
* will be removed from DB.
*/
-Msg *dlr_find(const Octstr *smsc, const Octstr *ts, const Octstr *dst, int typ)
+Msg *dlr_find(const Octstr *smsc, const Octstr *ts, const Octstr *dst, int typ, int use_dst)
{
Msg *msg = NULL;
struct dlr_entry *dlr = NULL;
+ Octstr *dst_min = NULL;
if(octstr_len(smsc) == 0) {
warning(0, "DLR[%s]: Can't find a dlr without smsc-id", dlr_type());
@@ -385,10 +386,17 @@
if (handles == NULL || handles->dlr_get == NULL)
return NULL;
+ if (use_dst && dst) {
+ dst_min = octstr_duplicate(dst);
+ int len = octstr_len(dst);
+
+ if (len > MIN_DST_LEN)
+ octstr_delete(dst_min, 0, len - MIN_DST_LEN);
+ }
debug("dlr.dlr", 0, "DLR[%s]: Looking for DLR smsc=%s, ts=%s, dst=%s, type=%d",
dlr_type(), octstr_get_cstr(smsc), octstr_get_cstr(ts), octstr_get_cstr(dst), typ);
- dlr = handles->dlr_get(smsc, ts, dst);
+ dlr = handles->dlr_get(smsc, ts, dst_min);
if (dlr == NULL) {
warning(0, "DLR[%s]: DLR from SMSC<%s> for DST<%s> not found.",
dlr_type(), octstr_get_cstr(smsc), octstr_get_cstr(dst));
@@ -436,12 +444,13 @@
if ((typ & DLR_BUFFERED) && ((dlr->mask & DLR_SUCCESS) || (dlr->mask & DLR_FAIL))) {
debug("dlr.dlr", 0, "DLR[%s]: DLR not destroyed, still waiting for other delivery report", dlr_type());
/* update dlr entry status if function defined */
- if (handles != NULL && handles->dlr_update != NULL)
- handles->dlr_update(smsc, ts, dst, typ);
+ if (handles != NULL && handles->dlr_update != NULL){
+ handles->dlr_update(smsc, ts, dst_min, typ);
+ }
} else {
- if (handles != NULL && handles->dlr_remove != NULL) {
+ if (handles != NULL && handles->dlr_remove != NULL){
/* it's not good for internal storage, but better for all others */
- handles->dlr_remove(smsc, ts, dst);
+ handles->dlr_remove(smsc, ts, dst_min);
} else {
warning(0, "DLR[%s]: Storage don't have remove operation defined", dlr_type());
}
@@ -449,6 +458,7 @@
/* destroy struct dlr_entry */
dlr_entry_destroy(dlr);
+ octstr_destroy(dst_min);
return msg;
}
Index: gw/dlr.h
===================================================================
--- gw/dlr.h (revision 4833)
+++ gw/dlr.h (working copy)
@@ -103,7 +103,8 @@
* Find an entry in the list. If there is one a message is returned and
* the entry is removed from the list otherwhise the message returned is NULL
*/
-Msg* dlr_find(const Octstr *smsc, const Octstr *ts, const Octstr *dst, int type);
+Msg* dlr_find(const Octstr *smsc, const Octstr *ts, const Octstr *dst, int type,
+ int use_dst);
/* return the number of DLR messages in the current waiting queue */
long dlr_messages(void);
Index: gw/dlr_p.h
===================================================================
--- gw/dlr_p.h (revision 4833)
+++ gw/dlr_p.h (working copy)
@@ -70,6 +70,8 @@
#define DLR_TRACE 1
+/* Used in destination based queries for EMI/UUCP DLRs */
+#define MIN_DST_LEN 7
/*
* The structure of a delivery report entry.
*/
Index: gw/smsc/smsc_cimd2.c
===================================================================
--- gw/smsc/smsc_cimd2.c (revision 4833)
+++ gw/smsc/smsc_cimd2.c (working copy)
@@ -2114,7 +2114,7 @@
code = 0;
}
if(code)
- msg = dlr_find(conn->name, timestamp, destination, code);
+ msg = dlr_find(conn->name, timestamp, destination, code, 1);
else
msg = NULL;
Index: gw/smsc/smsc_soap.c
===================================================================
--- gw/smsc/smsc_soap.c (revision 4833)
+++ gw/smsc/smsc_soap.c (working copy)
@@ -1616,7 +1616,7 @@
/* fetch the DLR */
dlrmsg = dlr_find(conn->id, octstr_imm(msgid), octstr_imm("receiver"), /* destination */
- dlrtype);
+ dlrtype, 0);
if (!dlrmsg) {
error(0,"SOAP[%s]: parse_dlr invoked (%ld), but no DLR found for MsgID %s", octstr_get_cstr(privdata->name),dlrtype,msgid);
Index: gw/smsc/smsc_oisd.c
===================================================================
--- gw/smsc/smsc_oisd.c (revision 4833)
+++ gw/smsc/smsc_oisd.c (working copy)
@@ -1306,7 +1306,7 @@
}
if (code)
- msg = dlr_find(conn->name, timestamp, destination, code);
+ msg = dlr_find(conn->name, timestamp, destination, code, 0);
octstr_destroy(destination);
octstr_destroy(timestamp);
Index: gw/smsc/smsc_at.c
===================================================================
--- gw/smsc/smsc_at.c (revision 4833)
+++ gw/smsc/smsc_at.c (working copy)
@@ -2124,7 +2124,7 @@
* categories. It will catch "reserved" values where the first 3 MSBits
* are not set as "Success" which may not be correct. */
- if ((dlrmsg = dlr_find(privdata->conn->id, msg_id, receiver, type)) == NULL) {
+ if ((dlrmsg = dlr_find(privdata->conn->id, msg_id, receiver, type, 0)) == NULL) {
debug("bb.smsc.at2", 1, "AT2[%s]: Received delivery notification but can't find that ID in the DLR storage",
octstr_get_cstr(privdata->name));
goto error;
Index: gw/smsc/smsc_fake.c
===================================================================
--- gw/smsc/smsc_fake.c (revision 4833)
+++ gw/smsc/smsc_fake.c (working copy)
@@ -291,7 +291,7 @@
dlrmsg = dlr_find(conn->id,
tmp, /* smsc message id */
copy->sms.receiver, /* destination */
- dlrstat);
+ dlrstat, 0);
if (dlrmsg != NULL) {
/* XXX TODO: Provide a SMPP DLR text in msgdata */
bb_smscconn_receive(conn, dlrmsg);
Index: gw/smsc/smsc_http.c
===================================================================
--- gw/smsc/smsc_http.c (revision 4833)
+++ gw/smsc/smsc_http.c (working copy)
@@ -643,7 +643,7 @@
dlrmsg = dlr_find(conn->id,
dlrmid, /* message id */
to, /* destination */
- dlrmask);
+ dlrmask, 0);
if (dlrmsg != NULL) {
dlrmsg->sms.sms_type = report_mo;
@@ -931,8 +931,8 @@
}
dlrmsg = dlr_find(conn->id,
apimsgid, /* smsc message id */
- dest , /* destination */
- dlrstat);
+ dest, /* destination */
+ dlrstat, 0);
if (dlrmsg != NULL) {
/* dlrmsg->sms.msgdata = octstr_duplicate(apimsgid); */
@@ -1410,8 +1410,8 @@
dlrmsg = dlr_find(conn->id,
mid, /* smsc message id */
- dest , /* destination */
- dlrstat);
+ dest, /* destination */
+ dlrstat, 0);
if (dlrmsg != NULL) {
dlrmsg->sms.msgdata = octstr_duplicate(mid);
@@ -1681,7 +1681,7 @@
dlrmsg = dlr_find(conn->id,
dlrmid, /* message id */
to, /* destination */
- dlrmask);
+ dlrmask, 0);
if (dlrmsg != NULL) {
dlrmsg->sms.sms_type = report_mo;
Index: gw/smsc/smsc_smpp.c
===================================================================
--- gw/smsc/smsc_smpp.c (revision 4833)
+++ gw/smsc/smsc_smpp.c (working copy)
@@ -1392,7 +1392,7 @@
dlrmsg = dlr_find(smpp->conn->id,
tmp, /* smsc message id */
destination_addr, /* destination */
- dlrstat);
+ dlrstat, 0);
octstr_destroy(msgid);
} else
Index: gw/smsc/smsc_cgw.c
===================================================================
--- gw/smsc/smsc_cgw.c (revision 4833)
+++ gw/smsc/smsc_cgw.c (working copy)
@@ -1137,19 +1137,19 @@
dlrmsg = dlr_find(conn->id,
ts, /* timestamp */
msid, /* destination */
- DLR_SUCCESS);
+ DLR_SUCCESS, 0);
break;
case 1: /* buffered */
dlrmsg = dlr_find(conn->id,
ts, /* timestamp */
msid, /* destination */
- DLR_BUFFERED);
+ DLR_BUFFERED, 0);
break;
case 2: /* not delivered */
dlrmsg = dlr_find(conn->id,
ts, /* timestamp */
msid, /* destination */
- DLR_FAIL);
+ DLR_FAIL, 0);
break;
}