search

RestComm / Restcomm-Connect

The Open Source Cloud Communications Platform
http://www.restcomm.com/
GNU Affero General Public License v3.0
242 stars 215 forks source link

RESTCOMM-2064: Refactor ExtensionController and implement getEffectiveConfiguration #2923

Open abdulazizali77 opened 6 years ago

abdulazizali77 commented 6 years ago

What this PR does / why we need it: Centralize configuration hierarchy lookup in getEffectiveConfiguration. Add an ExtensionContext interface for implementing extensions to access public extension framework functionality.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged): Fixes # RESTCOMM-2064, RESTCOMM-1617

Special notes for your reviewer: Decided to go with ExtensionContext instead of ExtensionControllerFacade. One other possible option is to not have ExtensionController implement ExtensionContext but create new ExtensionContextImpl class instead

abdulazizali77 commented 6 years ago

@jaimecasero @maria-farooq PTAL

gvagenas commented 6 years ago

Team, I did a review in this branch and here is my comment related to the Configuration sets we make available to the Extensions.

So we have two sets of configuration sets

  1. First set is the extension base configuration which is expressed as a sub-class of DefaultExtensionConfiguration. This configuration set is somewhat static (file based or similar)

  2. And the second set, the configuration expressed with the ExtensionConfiguration object that contains the extension’s rules private Object configurationData and have some common properties with the first set, the default conf, such as the private boolean enabled;. We use the common properties in order to dynamically control an extension by updating the DB record, for example, disable an extension at runtime. This configuration set is stored in DB and we provide DAO and now with this PR, we provide ExtensionController.getEffectiveConfiguration to retrieve this configuration set.

The separation of concerns between those two configuration sets is clear but the naming used so far is ambiguous and doesn't make clear what is the role of each configuration sets.

For this reason, I suggest a name change for the ExtensionConfiguration to ExtensionRules in order to be more clear what this object represents.

In future work on the Extension API, where we plan for handling configuration the same way we handle Profiles, we should merge the two sets of configuration (DefaultConfiguration and ExtensionRules) and let the ExtensionController provide the methods to retrieve each subset of the conf, for example

As part of this story and PR, I will provide the ExtensionConfiguration renaming to ExtensionRules

Important Database table restcomm_extensions_configuration and rest api endpoint path @Path("/ExtensionsConfiguration") will NOT be refactored in order to avoid DB migration scripts and uneccessary changes in the REST API.