[Home]

Summary:ASTERISK-22130: Bridge API Enhancements - refactor Bridging API to hide protected functions and break up large file structure
Reporter:Matt Jordan (mjordan)Labels:Asterisk12
Date Opened:2013-07-20 15:59:52Date Closed:2013-07-24 23:10:01
Priority:MajorRegression?
Status:Closed/CompleteComponents:Core/Bridging
Versions:12 Frequency of
Occurrence
Related
Issues:
Environment:Attachments:
Description:We are dangerously close to once again creating a monolithic file that exposes implementation details to consumers that it shouldn't.

Fortunately, the Bridging API does have a lot of structure - it just needs to hide some the internals and be broken up into its various pieces. A significant amount of renaming should also be done.

This includes:

* Make (at a minimum) the following functions "protected" in a separate header file from {{bridging.h}}:
** ast_bridge_channel_state
** ast_bridge_channel_thread_state
** ast_bridge_action_type
** ast_bridge_methods and all function pointer type definitions
** ast_bridge_register
** ast_bridge_alloc
** ast_bridge_lock_both
** ast_bridge_base_init (protected)
* Create factory functions that a create a bridge of a requested type. Ideally, we will have many types of sub-classed bridges. Construction of these bridges can be complex and require many flags. If someone wants they can go through the mechanics of customizing bridge creation; however, this is not ideal:
{noformat}
ast_mutex_lock(&bridgewait_lock);
if (!holding_bridge) {
holding_bridge = ast_bridge_base_new(AST_BRIDGE_CAPABILITY_HOLDING,
AST_BRIDGE_FLAG_MERGE_INHIBIT_TO | AST_BRIDGE_FLAG_MERGE_INHIBIT_FROM
| AST_BRIDGE_FLAG_SWAP_INHIBIT_FROM | AST_BRIDGE_FLAG_TRANSFER_PROHIBITED);
}
ast_mutex_unlock(&bridgewait_lock);
{noformat}
Ideally you should be able to request a subclass by its type, and get an instance of it. Things that produce bridges of a particular type would register a factory function that produces that bridge and associate it with that type. That will cut down on the header file inclusions and make this more flexible for further sub-classing.
* Create a bridge_channel file and move all operations on bridge_channel to this file
** ast_bridge_channel_try_lock
** ast_bridge_channel_lock
** ast_bridge_channel_lock_bridge
** Rename ast_bridge_change_state_no_lock to indicate it is performed on a bridge_channel
** ast_bridge_change_state
** ast_bridge_update_linkedids - can probably just use bridge_channel
** ast_bridge_update_accountcode - can probably just use bridge_channel
** ast_bridge_channel_queue_frame
** ast_bridge_channel_queue_action_data
** ast_bridge_channel_write_action_data
** etc.
* Perform a complete refactoring of names. The following nomenclature should be used:
** Public functions should use {{ast_[object]_[verb]_etc.}}. Objects are:
*** {{bridge}}
*** {{bridge_features}}
*** {{bridge_channel}}
*** Others will exist as well - but in general, we should not mix nomenclature.
** Protected functions should be prefixed with their namespace, but not {{ast}}
** Example: Rename ast_after_bridge_set_goto - to ast_bridge_set_after_goto
** Example: Rename ast_after_bridge_set_h to ast_bridge_set_after_h
** Continue with all of the ast_after...
* Rename bridging_basic to bridge_feature or something similar. It is no longer a "basic" bridge
* Decide what should be "bridging" versus "bridge"
** Potentially rename bridging_roles to something else - bridge_roles
* Rename bridge_simple. It is not really "simple", it is a two party mixing bridge.
Comments: