On Sat, 2009-04-18 at 22:10 +0200, Paolo Abeni wrote: > hello, > > Mapi notifications can occurs inside any emsmdb_transaction; currently > only the notification send by the server inside the > emsmdb_transaction_null() in MonitorNotification() are > parsed/processed/delivered to the appropriate callback. > > To be able to process all the received notifications, the notification > structure must comprise the private data to be passed to the > notification callback, since in the average mapi call this data is not > available. This patch set first [1/2] modify the Subscribe() call and > the mapi_notification structure, adding the private_data parameter > (also the related Subscribe() calls are update) and add a libmapi > call (DispatchNotifications) to force the notification dispatching in > a [quite] non blocking way, then [2/2] call ProcessNotification() > after each successful emsmdb_transaction() in all the libmapi calls. > > I split the patch in the hope of making it more managable for review, > since it touches a lot of files (with small changes). I think that the > main issue is breaking the current API (due to Subscribe() changes), > but I think that is necessary in order to reliably process all the > mapi notifications. > > The DispatchNotifications() call allow the caller to implement > notification handling both with multi-threading or with > polling/select/single thread and also allow easier creation of > notification related mapitests, according to the following schema: > > <initialize notification: Subscribe, ecc.> > > <do the action that trigger the event to be tested> > > DispatchNotifications() > > <test the callback has been invoked with the correct data> > > > Any comments are welcome
Hi Paolo, Your proposal and supplied patches looks pretty good to me and is worthwhile/needed improvement, so ready to enter trunk. I only have one comment/thought regarding the private_data pointer assignation within the Subscribe call. The current implementation assumes the developer's application is responsible for keeping reference to this private_data pointer all along its lifetime and until Unsubscribe gets called. This is IMO good when private_data references to mapi_object_t (We don't want the mapi_object memory context gets stolen by the notification API and got released upon Unsubscribe. That would be really wrong). However it is very likely some developers will be looking for a Subscribe function they can call from a sub-routine, assign the notification some complex structure, don't keep reference to it within upper part of the application and expect Unsubscribe from doing the whole necessary work or at least supply a function to retrieve it. I think adding a small assessor such as get_notification_private_data could be useful in this latter case. In the meantime, no show-stopper for me, just want to keep it in mind upon really needed. Last point, probably unrelated to your patch but worth asking. It looks like openchangeclient --notifications now processes the newmail several times while it used to be unique. We need to give a closer look at the ProcessNotification & operation. Cheers, Julien. -- Julien Kerihuel [email protected] OpenChange Project Manager GPG Fingerprint: 0B55 783D A781 6329 108A B609 7EF6 FE11 A35F 1F79
signature.asc
Description: This is a digitally signed message part
_______________________________________________ devel mailing list [email protected] http://mailman.openchange.org/listinfo/devel
