[asterisk-bugs] [JIRA] (ASTERISK-22130) Bridge API Enhancements - refactor Bridging API to hide protected functions and break up large file structure

[Matt Jordan (JIRA)]

     [ https://issues.asterisk.org/jira/browse/ASTERISK-22130?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Matt Jordan updated ASTERISK-22130:
-----------------------------------

    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


  was:
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.

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


    
> Bridge API Enhancements - refactor Bridging API to hide protected functions and break up large file structure
> -------------------------------------------------------------------------------------------------------------
>
>                 Key: ASTERISK-22130
>                 URL: https://issues.asterisk.org/jira/browse/ASTERISK-22130
>             Project: Asterisk
>          Issue Type: Improvement
>          Components: Core/Bridging
>    Affects Versions: 12
>            Reporter: Matt Jordan
>
> 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

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.asterisk.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira