Re: [PATCH] Pass meta-data from message to dlrs

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]
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>