Re: [PATCH] custom MO Parameters for smsc-http

Looks fine, +1. This definitely completes (no longer semi) generic  
http interface.

Regards,
--
Alejandro Guerrieri
[email protected]



On 22/06/2009, at 18:35, Franck LAMASUTA wrote:

> Here is the new version...
>
>
>
>
>> -----Original Message-----
>> From: [email protected]
>> [mailto:[email protected]] On Behalf Of Franck LAMASUTA
>> Sent: Monday, June 22, 2009 4:53 PM
>> To: [email protected]
>> Subject: RE: [PATCH] custom MO Parameters for smsc-http
>>
>> Thanks for your comments.
>>
>> First comment regarding the variable name:
>> The new variable is closely related to status-success-regex
>> which doesn't start with "generic-".
>> But ok, I will change it if you think it's better...
>>
>>
>> Second comment regarding pmatch[1].rm_eo:
>> The test seems to be useless according to regexec() documentation.
>> But it's cheap so it can't hurt.
>>
>> I will update userguide.xml also.
>>
>> Franck
>>
>>
>>
>>> -----Original Message-----
>>> From: Alejandro Guerrieri [mailto:[email protected]]
>>> Sent: Monday, June 22, 2009 12:37 PM
>>> To: Alexander Malysh
>>> Cc: [email protected];[email protected]
>>> Subject: Re: [PATCH] custom MO Parameters for smsc-http
>>>
>>> +1 with the changes Alex proposed. I'd rename
>>> ack_foreign_id_regex as
>>> well, for clarity's sake.
>>>
>>> Regards,
>>> --
>>> Alejandro Guerrieri
>>> [email protected]
>>>
>>> On 22/06/2009, at 12:26, Alexander Malysh wrote:
>>>
>>>> Hi,
>>>>
>>>> patch looks ok. Here some minor comments:
>>>>
>>>> +    OCTSTR(ack-foreign-id-regex)
>>>> would it be better to name it something like generic-foreign-id-
>>>> regex? so user know it belongs only to generic http...
>>>>
>>>> +                if (pmatch[1].rm_so != -1) {
>>>> better to check the end as well -> if (pmatch[1].rm_eo != -1 &&
>>>> pmatch[1].rm_so != -1)
>>>>
>>>> Otherwise patch looks good.
>>>> Please make patch to userguide and I will commit it to CVS.
>>>>
>>>> Thanks,
>>>> Alex
>>>>
>>>> Am 19.06.2009 um 14:40 schrieb Franck LAMASUTA:
>>>>
>>>>> Here it is, implemented with second idea...
>>>>>
>>>>> I did a small optimization of previous gw_regex_exec() calls in
>>>>> generic_parse_reply().
>>>>> I have also fixed 3 debug() calls in generic_receive_sms()
>>>>> ("smsc.http.kannel" => "smsc.http.generic").
>>>>>
>>>>> Waiting for your comments...
>>>>>
>>>>> Franck
>>>>>
>>>>>
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Alejandro Guerrieri [mailto:[email protected]]
>>>>>> Sent: Wednesday, June 10, 2009 3:08 PM
>>>>>> To: Alexander Malysh
>>>>>> Cc: [email protected];[email protected]
>>>>>> Subject: Re: [PATCH] custom MO Parameters for smsc-http
>>>>>>
>>>>>> Me too, second one is far more flexible and clean.
>>>>>> --
>>>>>> Alejandro Guerrieri
>>>>>> [email protected]
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 10/06/2009, at 14:31, Alexander Malysh wrote:
>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> I'm for the second idea, regex rocks ;)
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Alex
>>>>>>>
>>>>>>> Am 10.06.2009 um 12:37 schrieb Franck LAMASUTA:
>>>>>>>
>>>>>>>> It's not so urgent but I could try, it could be interesting...
>>>>>>>>
>>>>>>>> I have 2 ideas so far...
>>>>>>>>
>>>>>>>>
>>>>>>>> First idea
>>>>>>>> ==========
>>>>>>>>
>>>>>>>> I could add 2 new config parameters to define 2 regex:
>>>>>>>> - the first one, to find the start of the foreign id.
>>>>>>>> - the second one, to find the end of the foreign id.
>>>>>>>>
>>>>>>>> For Claro, they could be defined like this:
>>>>>>>> foreign-id-regex-begin = "<transaction-id>"
>>>>>>>> foreign-id-regex-end = "</transaction-id>"
>>>>>>>>
>>>>>>>> For Clickatell, they could be defined like this:
>>>>>>>> foreign-id-regex-begin = "ID: "
>>>>>>>> foreign-id-regex-end = " "
>>>>>>>>
>>>>>>>> For Brunet, they could be defined like this:
>>>>>>>> foreign-id-regex-begin = "MessageId="
>>>>>>>> foreign-id-regex-end = " "
>>>>>>>>
>>>>>>>> In generic_parse_reply(), only in case of success,
>>>>>> regex-begin will
>>>>>>>> be used to find the beginning of the id. Starting from
>>>>>> there, regex-
>>>>>>>> end will be used to find the end of the id. If regex-end
>>>>>> can't find
>>>>>>>> a match, the id will be read until the end of text
>> (body) and an
>>>>>>>> ending CRLF will be removed.
>>>>>>>> Then, the id will be used to call dlr_add() (like done in
>>>>>>>> clickatell_parse_reply()).
>>>>>>>>
>>>>>>>>
>>>>>>>> Second idea
>>>>>>>> ===========
>>>>>>>>
>>>>>>>> It's basically the same idea except that only one regex
>>>>>> will be used.
>>>>>>>>
>>>>>>>> For Claro, it could be defined like this:
>>>>>>>> foreign-id-regex = "<transaction-id>(.*)</transaction-id>"
>>>>>>>>
>>>>>>>> For Clickatell, it could be defined like this (if an
>> id contains
>>>>>>>> only digits):
>>>>>>>> foreign-id-regex = "ID: ([0-9]+)"
>>>>>>>>
>>>>>>>> The offsets given by gw_regex_exec() in pmatch[1] could be
>>>>>> used to
>>>>>>>> extract the id.
>>>>>>>> However, it could be more difficult to understand for
>> people who
>>>>>>>> are not regex experts...
>>>>>>>>
>>>>>>>>
>>>>>>>> Any comment, opinion, suggestion or better idea? :)
>>>>>>>>
>>>>>>>> Franck
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>> From: Alexander Malysh
>>>>>>>>> Sent: Tuesday, June 09, 2009 6:05 PM
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Am 09.06.2009 um 17:55 schrieb Alejandro Guerrieri:
>>>>>>>>>
>>>>>>>>>> Franck,
>>>>>>>>>>
>>>>>>>>>> The dlr part is inherited from the "kannel" smsc
>>> format. In the
>>>>>>>>>> actual implementation there's no way to grab the remote
>>>>>> message id
>>>>>>>>>> from a request, and it'd make a lot of sense to be
>>> able to do
>>>>>>>>>> it.
>>>>>>>>>>
>>>>>>>>>> I can try adding it, but don't refrain to do it so
>>>>>> yourself if you
>>>>>>>>>> feel the urge (you'll learn a lot about the code in the
>>>>>> process!).
>>>>>>>>>
>>>>>>>>> yep, Alex is right here :) and we need more contributors :)
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Opinions?
>>>>>>>>>> --
>>>>>>>>>> Alejandro Guerrieri
>>>>>>>>>> [email protected]
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On 09/06/2009, at 17:47, Franck LAMASUTA wrote:
>>>>>>>>>>
>>>>>>>>>>> Hi Alejandro ,
>>>>>>>>>>>
>>>>>>>>>>> I work with Hervé who posted to this list the 25th of May
>>>>>>>>>>> (Claro
>>>>>>>>>>> http communication).
>>>>>>>>>>> It seems the patch you've done on the generic part
>> could be
>>>>>>>>>>> very
>>>>>>>>>>> useful for us to manage our communications with Claro. It
>>>>>>>>> could be
>>>>>>>>>>> better than a specific implementation for Claro.
>>>>>>>>>>>
>>>>>>>>>>> However, I don't understand how your patch manages a DLR:
>>>>>>>>>>> In generic_receive_sms(), you call dlr_find() (line
>>> 1674 of the
>>>>>>>>>>> patched source) to retrieve a previous record stored
>>> in the DLR
>>>>>>>>>>> storage.
>>>>>>>>>>> It seems fine except that dlr_add() is never called in
>>>>>>>>>>> generic_parse_reply(), so I guess dlr_find() will always
>>>>>>>>> return NULL.
>>>>>>>>>>> Am I wrong???
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> In fact, when we submit a MT SMS to Claro with this request
>>>>>>>>>>>
>>>>>>>>>
>> http://retail.mds.claro.com.br/MSE/api?profile=xxxx&pwd=xxxx&m
>>>>>>>> ode=assync-delivery&ANUM=3333&BNUM=1234567&TEXT=test
>>>>>>>>>>>
>>>>>>>>>>> we get a HTTP code 200 with this body:
>>>>>>>>>>>
>>>>>>>>>>> <?xml version="1.0" encoding="UTF-8" ?>
>>>>>>>>>>> <mse-response>
>>>>>>>>>>> <status-code>0</status-code>
>>>>>>>>>>> <profile>profleID</profile>
>>>>>>>>>>> <transaction-id>1020606201622668099</transaction-id>
>>>>>>>>>>> </mse-response>
>>>>>>>>>>>
>>>>>>>>>>> So I guess that the transaction-id should be used in
>>>>>>>>>>> generic_parse_reply() to call dlr_add().
>>>>>>>>>>> Unfortunately, the current implementation does not allow
>>>>>>>>> to manage
>>>>>>>>>>> such an id, it only allows to search if the request was
>>>>>>>>> successful
>>>>>>>>>>> or not through the regex.
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> If all my hypothesis are right... :-)
>>>>>>>>>>> I could try to patch generic_parse_reply() to manage
>>> also the
>>>>>>>>>>> id
>>>>>>>>>>> through another regex. It will be my first Kannel
>> patch! ;-)
>>>>>>>>>>> If you prefer to do it, no problem, just let me know.
>>>>>>>>>>>
>>>>>>>>>>> If I'm wrong, please clarify how it works.
>>>>>>>>>>>
>>>>>>>>>>> Regards,
>>>>>>>>>>> Franck
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>> <kannel_http-generic.patch>
>>>>
>>>>
>>>
>>>
>>>
>>
>>
>>
> <kannel_http-generic_2.patch>