-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3150/
-----------------------------------------------------------

Review request for Asterisk Developers.


Repository: Asterisk


Description
-------

When the PJSIP pubsub framework was created, subscription handlers were 
required to state what event they handled along with what body types they knew 
how to generate. While this serves well when implementing a base RFC, it has 
problems when trying to extend the body to support non-standard or proprietary 
body elements. The code also was NOTIFY-specific, meaning that when the time 
comes that we start writing code to send out PUBLISH requests with MWI or 
presence bodies, we would likely find ourselves duplicating code that had 
previously been written.

This changeset introduces the concept of body generators and body supplements. 
A body generator is responsible for allocating a native structure for a given 
body type, providing the primary body content, converting the native structure 
to a string, and deallocating resources. A body supplement takes the primary 
body content (the native structure, not a string) generated by the body 
generator and adds nonstandard elements to the body. With these elements living 
in their own module, it becomes easy to extend our support for body types and 
to re-use resources when sending a PUBLISH request.

Body generators and body supplements register themselves with the pubsub core, 
similar to how subscription and publish handlers had done. Now, subscription 
handlers do not need to know what type of body content they generate, but they 
still need to inform the pubsub core about what the default body type for a 
given event package is. The pubsub core keeps track of what body generators and 
body supplements have been registered. When a SUBSCRIBE arrives, the pubsub 
core will check that there is a subscription handler for the event in the 
SUBSCRIBE, then it will check that there is a body generator that can provide 
the content specified in the Accept header(s).

Because of the nature of body generators and supplements, it means 
res_pjsip_exten_state and res_pjsip_mwi have been completely gutted. They no 
longer worry about body types, instead calling 
ast_sip_pubsub_generate_body_content() when they need to generate a NOTIFY body.

Some potentially dodgy elements in play here:
* The use of void pointers for body allocation and generation means that 
reading the source will be necessary in order to figure out what type of data 
is being passed to a body supplement that you add. This information could 
probably stand to be documented someplace.
* There is no module reference counting happening here. If there is a live 
subscription and the body generator for that subscription is unloaded, some 
terrible things are likely to happen.

NOTE:
There are red blobs on the initial diff that is being posted here. I have 
already fixed these in the branch that I have been working in, so there is no 
need to point them out during review.


Diffs
-----

  /trunk/res/res_pjsip_xpidf_body_generator.c PRE-CREATION 
  /trunk/res/res_pjsip_pubsub.exports.in 406303 
  /trunk/res/res_pjsip_pubsub.c 406303 
  /trunk/res/res_pjsip_pidf_nonstandard_body_supplement.c PRE-CREATION 
  /trunk/res/res_pjsip_pidf_body_generator.c PRE-CREATION 
  /trunk/res/res_pjsip_pidf.c 406303 
  /trunk/res/res_pjsip_mwi_body_generator.c PRE-CREATION 
  /trunk/res/res_pjsip_mwi.c 406303 
  /trunk/res/res_pjsip_exten_state.c 406303 
  /trunk/res/res_pjsip/presence_xml.c PRE-CREATION 
  /trunk/include/asterisk/res_pjsip_pubsub.h 406303 
  /trunk/include/asterisk/res_pjsip_presence_xml.h PRE-CREATION 
  /trunk/include/asterisk/res_pjsip_exten_state.h 406303 
  /trunk/include/asterisk/res_pjsip_body_generator_types.h PRE-CREATION 

Diff: https://reviewboard.asterisk.org/r/3150/diff/


Testing
-------

In addition to manual testing, I have also posted 
https://reviewboard.asterisk.org/r/3151 that contains new tests for the 
testsuite.


Thanks,

Mark Michelson

-- 
_____________________________________________________________________
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Reply via email to