Re: [PATCH] Pass meta-data from message to dlrs
Alexander Malysh <[email protected]>
| Newsgroups | gmane.comp.mobile.kannel.devel |
|---|---|
| Message-ID | <[email protected]> |
Am 04.09.2009 um 13:05 schrieb Alejandro Guerrieri: > 2009-09-04 Alexander Malysh <amalysh at kannel.org> > * aclocal.m4, configure, configure.in, gw/dlr_mysql.c, gwlib/ > dbpool_mysql.c, > test/test_dbpool.c: implemented select/update function calls > for dbpool_mysql. This > require mysql prepaid statement support and therefore mysql > version >= 4.1. > Changed dlr_mysql to use these new function calls. This fixes > #258. > > Would make sense if I update sqlbox to use those calls as well right? yep > > Regards, > -- > Alejandro Guerrieri > [email protected] > > > > On 04/09/2009, at 9:45, Alexander Malysh wrote: > >> ahh alex, >> before you start to add meta_data to db storages please give me >> time to merge mysql-prepaid branch. Just update your cvs in 30-60min. >> >> Thanks, >> Alexander Malysh >> >> Am 04.09.2009 um 09:40 schrieb Alexander Malysh: >> >>> Hi Alex, >>> >>> if you add: >>> + Octstr *meta_data; >>> to dlr struct then you have add meta_data column to all storage >>> types :) otherwise don't add it. If you ask me, I would say, if >>> you add it for faked dlrs then you have >>> to add it to db storages as well. >>> >>> as to the points: >>> 1) +0 >>> 2) -1 because I don't see reason to add internal smpp sequence. >>> this is just one counter and will not add any useful information. >>> 3) ++1 :) >>> >>> Thanks, >>> Alexander Malysh >>> >>> Am 04.09.2009 um 01:08 schrieb Alejandro Guerrieri: >>> >>>> Ok, silly diff errors aside (I wasn't including dlr_p.h :P), >>>> here's the new patches also changing the error code format as >>>> Alex suggested. >>>> >>>> <kannel-dlr-meta-data.diff> >>>> <kannel-dlr-command-status.diff> >>>> >>>> Userguide patch coming later. >>>> >>>> Just a few more ideas to consider: >>>> >>>> 1. Adding one more called "dlr_status_text" with the text >>>> description from smpp_error_to_string(pdu- >>>> >u.submit_sm_resp.command_status). Not sure if it makes sense, >>>> this could be done at the application level with a simple table... >>>> >>>> 2. Adding one more called "dlr_sequence" with the sequence_number >>>> parameter. >>>> >>>> 3. Extending patch #1 adding the meta-data column on all DB >>>> implementations, so meta-data could be passed and stored on dlr's >>>> other than the first one generated by kannel. >>>> >>>> Regards, >>>> -- >>>> Alejandro Guerrieri >>>> [email protected] >>>> >>>> >>>> >>>> On 03/09/2009, at 23:33, Alexander Malysh wrote: >>>> >>>>> Hi Alex, >>>>> >>>>> first patch seems still broken: >>>>> + dlr->meta_data = (msg->sms.meta_data ? octstr_duplicate(msg- >>>>> >sms.meta_data) : octstr_create("")); >>>>> >>>>> we don't have meta_data in dlr struct. otherwise +1 >>>>> >>>>> second patch: >>>>> + if (msg->sms.meta_data == NULL) >>>>> + msg->sms.meta_data = octstr_create(""); >>>>> + meta_data_set_value(msg->sms.meta_data, "smpp", >>>>> octstr_imm("dlr_status"), >>>>> + octstr_format("%d", pdu- >>>>> >u.submit_sm_resp.command_status), 1); >>>>> + >>>>> >>>>> Would it it not better to have error code formated as smpp-tlv >>>>> group and as error message? >>>>> + octstr_format("0x%08lx", pdu- >>>>> >u.submit_sm_resp.command_status), 1); >>>>> >>>>> Thanks, >>>>> Alexander Malysh >>>>> >>>>> >>>>> Am 03.09.2009 um 21:13 schrieb Alejandro Guerrieri: >>>>> >>>>>> Oh, I see, I've attached the ~ backup file from vim. This are >>>>>> the right files, just in case: >>>>>> >>>>>> <kannel-dlr-command-status.diff> >>>>>> <kannel-dlr-meta-data.diff> >>>>>> >>>>>> -- >>>>>> Alejandro Guerrieri >>>>>> [email protected] >>>>>> >>>>>> >>>>>> >>>>>> On 03/09/2009, at 21:06, Alejandro Guerrieri wrote: >>>>>> >>>>>>> Oh, that must have filtered into the patch. Weird :P >>>>>>> >>>>>>> Sorry, it's safe to remove it. >>>>>>> >>>>>>> Regards, >>>>>>> -- >>>>>>> Alejandro Guerrieri >>>>>>> [email protected] >>>>>>> >>>>>>> >>>>>>> >>>>>>> On 03/09/2009, at 20:56, Gustavo Mohme C. wrote: >>>>>>> >>>>>>>> Hi Alejandro, >>>>>>>> I get this error after applying your patch. >>>>>>>> >>>>>>>> gw/dlr.c: In function ‘dlr_add’: >>>>>>>> gw/dlr.c:352: error: ‘struct dlr_entry’ has no member named >>>>>>>> ‘meta_data’ >>>>>>>> make: *** [gw/dlr.o] Error 1 >>>>>>>> >>>>>>>> After editing /gw/dlr_p.h and adding meta_data to the struct >>>>>>>> dlr_entry, it compiled with no complaints. Is this the >>>>>>>> correct thing to do? >>>>>>>> I was patching the latest kannel snapshot.... >>>>>>>> Thanks, >>>>>>>> Gustavo >>>>>>>> >>>>>>>> On Thu, Sep 3, 2009 at 3:39 AM, Alejandro Guerrieri <[email protected] >>>>>>>> > wrote: >>>>>>>> Hi, >>>>>>>> >>>>>>>> This is a set of two patches, though they're both very simple >>>>>>>> and could have been mixed, I've preferred to split to honor >>>>>>>> the rule of not mixing features in a single patch. >>>>>>>> >>>>>>>> Patch #1, kannel-drl-meta-data.diff allows meta-data to be >>>>>>>> passed when sending a message to come back into the "fake" >>>>>>>> dlr that kannel generates when the smsc accepts or rejects a >>>>>>>> message. It could be extended to work with the different dlr >>>>>>>> engines so the meta-data would be also passed on the >>>>>>>> intermediate and final dlr's delivered by the carriers >>>>>>>> (though it would require changes on the database structure to >>>>>>>> add a "meta_data" column of course. I'm not sure this would >>>>>>>> be needed, the dlr-url could be easily abused to pass >>>>>>>> application-specific parameters without much hassle. >>>>>>>> >>>>>>>> Patch #2, kannel-dlr-command-status.diff builds on top of >>>>>>>> patch #1 and creates a meta data value called "dlr_status" >>>>>>>> that comes back on the "fake" dlr generated by kannel (This >>>>>>>> was the real reason why patch #1 was created). The value >>>>>>>> there is the "command_status" value some people on the list >>>>>>>> needs to get from the submit_sm_resp PDU's. >>>>>>>> >>>>>>>> Example usage: >>>>>>>> >>>>>>>> On sendsms: >>>>>>>> >>>>>>>> http://localhost:13013/cgi-bin/sendsms?username=kannel&password=kannel&from=12345&to=12345678&smsc=mysmsc&text=Hello&dlr-mask=31&meta-data=%3Fsmpp%3Fmy_own_field%3D1234&dlr-url=http%3A%2F%2Flocalhost%2Fx%3Fdata%3D%25D >>>>>>>> >>>>>>>> Notes: >>>>>>>> >>>>>>>> meta-data is urlencoded version of: ?smpp?my_own_field=1234 >>>>>>>> dlr-url is urlencoded version of: http://localhost/x?data=%D >>>>>>>> >>>>>>>> So, after applying Patch #1, kannel would call the following >>>>>>>> url: >>>>>>>> >>>>>>>> http://localhost/x?md=%3Fsmpp%3Fmy_own_field%3D1234 >>>>>>>> >>>>>>>> The "md" parameter, once urldecoded would look like: >>>>>>>> >>>>>>>> ?smpp?my_own_field=1234 >>>>>>>> >>>>>>>> You can pass as many parameters as you want of course and >>>>>>>> they would be added to meta-data along with any other fields >>>>>>>> you've defined on your smpp-tlv groups, etc. >>>>>>>> >>>>>>>> So far so good, in fact you could pass this kind of stuff on >>>>>>>> regular url variables, but the goal of this patch was to >>>>>>>> allow to add stuff back from the smsc's response. >>>>>>>> >>>>>>>> Now, with patch #2 also applied, the url returned would be >>>>>>>> something like this: >>>>>>>> >>>>>>>> http://localhost/x?md=%3Fsmpp%3Fmy_own_field%3D1234%26dlr_status%3D69%26 >>>>>>>> >>>>>>>> The "md" parameter, once urldecoded would look like: >>>>>>>> >>>>>>>> ?smpp?my_own_field=1234&dlr_status=69& >>>>>>>> >>>>>>>> In this case, dlr_status gets loaded with submit_sm_resp's >>>>>>>> command_status which was: 69 = 0x00000045 (Submit Failed). >>>>>>>> >>>>>>>> The "sequence_number" could be added just as easily (not a >>>>>>>> bad idea imho). What do you think? "dlr_sequence" or >>>>>>>> "dlr_seq" would be ok? >>>>>>>> >>>>>>>> I'm writing the userguide patch if this goes forward of course. >>>>>>>> >>>>>>>> Comments? >>>>>>>> >>>>>>>> Regards, >>>>>>>> -- >>>>>>>> Alejandro Guerrieri >>>>>>>> [email protected] >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>> >>>> >>> >> >