Conference timeout and termination API - Restcomm 1171 #2617

maria-farooq · 2017-11-07T10:22:47Z

related issues:
https://telestax.atlassian.net/browse/RESTCOMM-1171
https://telestax.atlassian.net/browse/RESTCOMM-758

* master: #2519 fixing testcases 1/ ProxyManager: user name contains host part fix 2/ Set max expire time to 3600 removed double quotes. SQL tested in stagin DB new method to count by filter. fixes #2610

jaimecasero · 2017-11-07T11:50:00Z

....commons/src/main/java/org/restcomm/connect/commons/common/http/CustomHttpClientBuilder.java

@@ -58,12 +68,21 @@ private CustomHttpClientBuilder() {
    }

    private static CloseableHttpClient defaultClient = null;
+    private static CloseableHttpAsyncClient closeableHttpAsyncClient = null;


not sure putting sync and async factory in same class is meaningful (maybe a separated async factory would have work better). Lets keep it like this by now, since we are reuding the lifecycle, and final closing during app shutdown...

jaimecasero · 2017-11-07T12:25:53Z

.../restcomm.http/src/main/java/org/restcomm/connect/http/asyncclient/HttpAsycClientHelper.java

+        if (sender != null && !sender.isTerminated()) {
+            sender.tell(response, self());
+        } else {
+            if (logger.isInfoEnabled()) {


what happens if we send msg to terminated actor? checking actor state from the outside is not guaranteed,as in the next instruction the state may have changed. So, there will be chances where we send the response,and the actor is terminated. Not sure is worth to make the distinction here, or maybe send response blindly to actor, and let Akka figure out what to do (deadletter box..)

agree with letting akka handle the delivery dynamics

jaimecasero · 2017-11-07T12:37:00Z

.../restcomm.http/src/main/java/org/restcomm/connect/http/asyncclient/HttpAsycClientHelper.java

+                if (contentType != null) {
+                    builder.setContentType(contentType.getValue());
+                }
+                builder.setContent(StringUtils.toString(stream));


i dont like how response is stringified here, i think it should be original "sender" responsability to do this if appropiate. I can see the existing HttpResponseDescriptor already expectsa String content, but i think its a bit dangerous in terms of encoding(this is assuming body is JVM charset encoded/UTF8...) and performance(we loss any streaming chance, and we create an String in memory with potentially big body with no control on actual length)

not sure worth the refactoring, but very worrying...

should we refactor this as part of this PR?

it will impact Downloader as well,so I guess we can do that later...

jaimecasero · 2017-11-07T12:41:49Z

restcomm/restcomm.telephony/src/main/java/org/restcomm/connect/telephony/Conference.java

+                if(logger.isDebugEnabled())
+                    logger.debug(String.format("Conference cdr is null for sid: %s", sid));
+            }else{
+                return DateTime.now().getMillis()-cdr.getDateCreated().getMillis();


can we assume all JVM/OS nodes in the cluster are clock synced? are the nodes configured to sync system clock from a central server regularly?

@jaimecasero Network Time Protocol (NTP) is configured by default on Amazon Linux instances and for cloud service is up and running to make it synchronised to NTP server.

jaimecasero · 2017-11-07T12:44:48Z

...te/src/test/java/org/restcomm/connect/testsuite/telephony/DialConferenceTerminationTest.java

+
+        String conferenceSid = getConferenceSid(confRoom1);
+        assertEquals(2, getParticipantsSize(conferenceSid));
+        int liveCalls = MonitoringServiceTool.getInstance().getStatistics(deploymentUrl.toString(), adminAccountSid, adminAuthToken);


is it worthit to depend test result on monitorignService? i expect monitoring service to collect on some collection frequency, will the test be stable if we include this assertion here?.

imo monitoring service is a nice way to check live calls in memory, not sure why test would be unstable.

I though monitoringService was getting stats from other components from time to time. Apparently components push data into monitoring service in real time instead. So i guess its safe to use the MonService during test... nevermind my comment

jaimecasero · 2017-11-07T13:50:04Z

...te/src/test/java/org/restcomm/connect/testsuite/telephony/DialConferenceTerminationTest.java

+    }
+
+    private int getActiveConferencesSize() {
+        JsonObject conferences = RestcommConferenceTool.getInstance().getConferences(deploymentUrl.toString(),adminAccountSid, adminAuthToken);


this may reuse my new countByFilter metthod :)

@jaimecasero can you explain how can i use a dao method at this level, is there an api available on top of that ?

jaimecasero · 2017-11-07T13:51:56Z

...te/src/test/java/org/restcomm/connect/testsuite/telephony/DialConferenceTerminationTest.java

+        bobCall.listenForDisconnect();
+        georgeCall.listenForDisconnect();
+
+        assertTrue(bobCall.waitForDisconnect(80 * 1000));


i guess this depends on conf timeout, where is the conf timeout configured here? i guess the default 4hours is not good for testing. is this configured at rcml level for this tetscase?

@jaimecasero its not 4 hours for testing, its 60 seconds for testing

where are those 60 seconds configured?

in restcomm-conference.xml aka restcomm.xml for this specific test class

jaimecasero · 2017-11-07T13:52:40Z

...te/src/test/java/org/restcomm/connect/testsuite/telephony/DialConferenceTerminationTest.java

+    // Bob is a simple SIP Client. Will not register with Restcomm
+    private SipStack bobSipStack;
+    private SipPhone bobPhone;
+    private String bobContact = "sip:bob@127.0.0.1:5090";


can we make this test follow paralle testing approach?

letme change

jaimecasero · 2017-11-07T14:15:34Z

restcomm/restcomm.http/src/main/java/org/restcomm/connect/http/SecuredEndpoint.java

@@ -331,6 +331,9 @@ private AuthOutcome secureLevelControl(Account operatedAccount, String resourceA
        String operatedAccountSid = null;
        if (operatedAccount != null)
            operatedAccountSid = operatedAccount.getSid().toString();
+        if (isSuperAdmin()) {


will this affect to other existing operations? Seems like a big change.... Of course this means is the superAdmin anyway, but just wanted to check if this will change anything for other existing RESTAPI operations...

this change would mean that superadmin account would have control over restcomm APIs (number, clients, call, conference etc) regardless of the fact that resource being owned by another account in the system

@maria-farooq but there is already a method for this org.restcomm.connect.http.SecuredEndpoint#isSuperAdmin and can be used from any endpoint its needed.

I think if we go with this patch then we mess the SecuredEndpoints and the rules to follow and it will be harder to debug later.

I think we should remove this and instead check with isSuperAdmin method

@gvagenas good point, i added this check for superadmin in conference and call endpoints intead

jaimecasero · 2017-11-07T14:17:20Z

restcomm/restcomm.http/src/main/java/org/restcomm/connect/http/ConferencesEndpoint.java

+        logger.info(String.format("updateConference accountsid: %s conferenceSid: %s", accountSid, sid));
+        Account account = daoManager.getAccountsDao().getAccount(accountSid);
+        try {
+            secure(account, "RestComm:Read:Conferences");


is this the correct literal "RestComm:Read:Conferences"? i dont know the convention, but this is not a ReadOnly operation, here we are modifying the conference, shouldnt be somethign like "Restcomm:Write:Conferences"?

nice catch, updated

jaimecasero

let me know your opinion on comments

…wer's feedback

gvagenas

@maria-farooq I provided my comments and one of the most important is that the CallApiClient actor is a top-level actor and as far as I can see this actors will never get destroyed unless the Akka system stops.

Also, using LCM from Restcomm to Restcomm inside the cluster is very bad and restrictive design to my point of view, but this is not related to the patch here but rather to the old design decisions that were taken.

gvagenas · 2017-11-09T14:00:26Z

restcomm/restcomm.http/src/main/java/org/restcomm/connect/http/ConferencesEndpoint.java

+            final String status = data.getFirst("Status");
+
+            if(status != null){
+                if(status.equalsIgnoreCase("completed")){


@maria-farooq I think we need to provide a default response in the case where the provided status DOESN NOT match the completed

gvagenas · 2017-11-09T14:02:56Z

restcomm/restcomm.http/src/main/java/org/restcomm/connect/http/ConferencesEndpoint.java

+    private void kickoutAllActiveParticipants(ConferenceDetailRecord conferenceDetailRecord){
+        List<CallDetailRecord> callDetailRecords = daoManager.getCallDetailRecordsDao().getRunningCallDetailRecordsByConferenceSid(conferenceDetailRecord.getSid());
+
+        if(callDetailRecords == null || callDetailRecords.isEmpty()){


@maria-farooq also here I think we need a response to tell user that there were no participants

@gvagenas I dont think it need other than a suceesful response, user will get conference resource in response.
this case here is just to make sure in a race case when participants leave themselves before system kick them out

main issue with any single response is that this would require to wait for result on async actions started from here, and which may take time (LVM remote invocation). The response should be succesful if the action of terminating the conf may be initiated.

I think any client can check the RESTAPI later to see how many participants are left at any moment...

gvagenas · 2017-11-09T14:21:55Z

restcomm/restcomm.http/src/main/java/org/restcomm/connect/http/ConferencesEndpoint.java

+                return new CallApiClient(sid, daoManager);
+            }
+        });
+        return system.actorOf(props);


@maria-farooq here you create a top-level actor with a parent the system itself.

Nowhere in the patch, you destroy this actor, thus this actor and its child actors HttpAsycClientHelper will stay in memory for as long as the akka system is up and running.

@maria-farooq I believe better design would be to use ASK pattern, delegate the task to CallManager which create CallApiClient , use to kill the calls and then destroy the actor.

On top, we can use ASK pattern without blocking, just fire the request and return, since we don't need the response.

@maria-farooq also delegating the task to CallManager, we can save some LCM (http) requests.

CallManager can check which of the CDRs are local calls or not and either use update call method or LCM.

@maria-farooq I see the local calls are removed by the Conferences endpoint (Evicting private class) but still I think the task to remove remote calls actors should be delegated to CallManager that will create CallApiClient actors and then destroy them

gvagenas · 2017-11-09T14:31:50Z

restcomm/restcomm.http/src/main/java/org/restcomm/connect/http/client/api/CallApiClient.java

+            logger.info("Call api response succeeded " + response.succeeded());
+            logger.info("Call api response: " + response);
+        }
+        requestee.tell(message, self);


@maria-farooq with the current implementation the requestee will always be null.
Shouldn't we protect the code for this?

please chek comments below

gvagenas · 2017-11-09T14:33:01Z

restcomm/restcomm.http/src/main/java/org/restcomm/connect/http/client/api/CallApiClient.java

+    }
+
+    protected void onHangup(Hangup message, ActorRef self, ActorRef sender) throws URISyntaxException, ParseException {
+        requestee = sender;


@maria-farooq What is the reason behind this? Any plans to use the CallApiClient in other places where we will need to return back the response, because currently there is no use of that

i have changed both CallApiClient and ConferenceApiClient to be disposable (used for one time request), they will clean them selves after sending back DownloaderResponse

gvagenas · 2017-11-09T14:33:46Z

restcomm/restcomm.http/src/main/java/org/restcomm/connect/http/client/api/CallApiClient.java

+    }
+
+    @Override
+    public void postStop () {


@maria-farooq as far as I can see, CallApiClient will never be destroyed and thus this method will never be called, see my comments on ConferencesEndpoint

i have changed both CallApiClient and ConferenceApiClient to be disposable (used for one time request), they will clean them selves after sending back DownloaderResponse

gvagenas · 2017-11-09T14:38:45Z

restcomm/restcomm.http/src/main/java/org/restcomm/connect/http/client/api/CallApiClient.java

+        }
+    }
+
+    private String getSuperAdminAuthenticationHeader(){


@maria-farooq since the ConferencesEndpoint.updateConference() can be used by any account with RestComm:Modify:Conferences permision, why we create LCM using super admin credentials?
I think we should use the credentials of the account that requested the update initially

gvagenas · 2017-11-09T14:39:05Z

...mm/restcomm.http/src/main/java/org/restcomm/connect/http/client/api/ConferenceApiClient.java

+        }
+    }
+
+    private String getSuperAdminAuthenticationHeader(){


@maria-farooq I think we should use the credentials of the account that requested the stop conference initially and not the super admin.

gvagenas · 2017-11-09T14:47:15Z

restcomm/restcomm.telephony/src/main/java/org/restcomm/connect/telephony/Conference.java

+                    + "At the postStop() method.");
+            }
+            if(conferenceApiClient != null && !conferenceApiClient.isTerminated())
+                getContext().stop(conferenceApiClient);


@maria-farooq instead of waiting for the Conference actor to get destroyed in order to destroy the conferenceApiClient, we can make use of the fact the ConferenceApiClient will send the DownloaderResponse to the Confernece actor and kill the actor at this point

@gvagenas i have changed both CallApiClient and ConferenceApiClient to be disposable (used for one time request), they will clean them selves after sending back DownloaderResponse

gvagenas · 2017-11-09T14:54:32Z

restcomm/restcomm.http/src/main/java/org/restcomm/connect/http/SecuredEndpoint.java

@@ -331,6 +331,9 @@ private AuthOutcome secureLevelControl(Account operatedAccount, String resourceA
        String operatedAccountSid = null;
        if (operatedAccount != null)
            operatedAccountSid = operatedAccount.getSid().toString();
+        if (isSuperAdmin()) {


@maria-farooq but there is already a method for this org.restcomm.connect.http.SecuredEndpoint#isSuperAdmin and can be used from any endpoint its needed.

I think if we go with this patch then we mess the SecuredEndpoints and the rules to follow and it will be harder to debug later.

I think we should remove this and instead check with isSuperAdmin method

* master: fixed counter increase created notice in doc to alert about SMPP regex disable added test for new RegexRemover restore previous number selection,and disable regexes for smpp allow usage for nonSIP scenarios use organizationutil to match incomingphonenumber. make test more stable by preventing conunt of data from previous testcase Fixed #2625: Update Quick Start documentation for Android BETA7 removed listPhones from dao attributes, DAO needs to be stateless. fixed toString method to show fields fixed sip and tomcat as test dependencies in parent.defined proper dependencies at child level. Patch to fix Record race condition. Also provides enhancements for MockMediaGateway. This close #RESTCOMM-1219 This close #RESTCOMM-1229 This close #RESTCOMM-1230 This close #RESTCOMM-1231 Work in progress This refer to #RESTCOMM-1219 Patch for MockMediaGateway to create recording file on RQNT ES (sg=pr) request This refer to #RESTCOMM-1219 Work in progress This refer to #RESTCOMM-1219 Patch for MockMediaGateway to create recording file on RQNT ES (sg=pr) request This refer to #RESTCOMM-1219 Patch for MockMediaGateway to create recording file on RQNT ES (sg=pr) request This refer to #RESTCOMM-1219 Work in progress This refer to #RESTCOMM-1219 Work in progress This refer to #RESTCOMM-1219 Work in progress This refer to #RESTCOMM-1219

…tead

…est) and get destroyed after that.

gvagenas

@maria-farooq please check my comments, I think there few things to improve.
There are some comments related to the local and remote call actors, that I am not sure are correct, lets discuss about them

gvagenas · 2017-11-13T15:30:46Z

restcomm/restcomm.telephony/src/main/java/org/restcomm/connect/telephony/Conference.java

@@ -309,8 +326,10 @@ public void execute(Object message) throws Exception {
                final Leave leave = new Leave();
                call.tell(leave, super.source);
            }
+            //tell conference api client to call conference api to terminate conference and kick all calls
+            conferenceApiClient = conferenceApiClient();


@maria-farooq Is there a way to tell if a conference has remote participants or not? This would be a great improvement in order to save resources by not creating and executing the ConferenceApiClient

we can check by DB but that is like alternative to me not an optimisation, in most cases it would be an overhead query

gvagenas · 2017-11-13T15:36:03Z

restcomm/restcomm.telephony/src/main/java/org/restcomm/connect/telephony/CallManager.java

+            if(message.getCallDetailRecord().getInstanceId().equals(RestcommConfiguration.getInstance().getMain().getInstanceId())){
+                ActorRef callTobeHangup = lookup(new GetCall(message.getCallDetailRecord().getCallPath()));
+                if (callTobeHangup != null){
+                    callTobeHangup.tell(message, self);


@maria-farooq at the Conference.onStopConference() method, the Conference actor will ask all local call actors to leave.
The call actor at this point will send BYE to the sip client.
So there is no need to send Hangup to the local call actors.

Important! I think there is a bug in the call actor where after Leave message is received, the call actor will never get destroyed.
As far as I can see, when the call actor receives Leavemessage:

Will send bye

Will tell MmsCallController to leave (destroy media resources)
Possible BUG -> 3. The Call.onMediaServerControllerStateChanged doesn't take care of the MmsCallController response while state is Leaving
We need to investigate this one

@gvagenas do you think this is part of the scop of this PR? this bug is there regardless if we merge this patch or not no? can we handle that seperatly

@maria-farooq do you think this is part of the scop of this PR? the bug is not in the scope of this PR but I believe CallManager shouldn't try to Hangup local calls.

gvagenas · 2017-11-13T15:40:29Z

restcomm/restcomm.telephony/src/main/java/org/restcomm/connect/telephony/Conference.java

+        if (logger.isInfoEnabled()) {
+            logger.info("Conference Received Timeout, will stop conference now.");
+        }
+        onStopConference(new StopConference(SUPER_ADMIN_ACCOUNT_SID), self, sender);


@maria-farooq Maybe we should differentiate the StopConference initiated by SuperAdmin and the one initiated by the timer. This is just an idea, not sure about the implementation but I think if it sounds feasible to you we should investigate it

we can provide a reason field in StopConference as well in API, in case of timeout its will be conference time limit reached this field will later be passed as reason header in corresponding BYE to sip clients

@maria-farooq sounds good

gvagenas · 2017-11-13T15:42:58Z

...mm/restcomm.http/src/main/java/org/restcomm/connect/http/client/api/ConferenceApiClient.java

+                logger.warning(String.format("could not retrive any conference record for this conference accountSid %s name: %s", accountSid, name));
+                return null;
+            }else{
+                return conferenceDetailRecords.get(0);


@maria-farooq isn't this dangerous since we pick the first available Conference in the DB? I believe we should stop and fail the request when conferenceSid is null.

conferenceSid is not always available at local conference we care about accountSid:name

@maria-farooq but still picking up the first on the list its dangerous since this one might not be the conference we are looking for

@gvagenas as per our conference design/state, there is only one conference in DB in running state.

@maria-farooq ??? Only one conference in running state? I am missing something here.

There will be only 1 RUNNING Conference for AccountSid and given Name
Conference ID is unique because AccountSid+ConferenceName is unique

gvagenas · 2017-11-13T15:46:28Z

...mm/restcomm.http/src/main/java/org/restcomm/connect/http/client/api/ConferenceApiClient.java

+        } else {
+            try {
+                URI uri = new URI("/restcomm"+conferenceDetailRecord.getUri());
+                uri = UriUtils.resolve(uri);


@maria-farooq here we rely on the fact that the cluster has a common public IP address (the LB public IP). Nothing bad in the current phase but maybe the conferenceDetailRecord.getUri() should contain the remote IP Address (when/if the conference is remote).
This would help later to have conferences between instances that don't belong to the same "cluster".
This is just an idea for discussion.

gvagenas · 2017-11-13T15:49:07Z

restcomm/restcomm.http/src/main/java/org/restcomm/connect/http/ConferencesEndpoint.java

+        Account account = daoManager.getAccountsDao().getAccount(accountSid);
+        Account effectiveAccount = userIdentityContext.getEffectiveAccount();
+        try {
+            secure(account, "RestComm:Modify:Conferences");


@maria-farooq Since later we use isSuperAdmin() I don't think we need to check permissions here.
The updateConference() method could be protected with just the isSuperAdmin()

conference termination request can be initiated by owner of the conference as well hence we need to make sure that resource is being accessed by proper account

gvagenas · 2017-11-13T15:50:14Z

restcomm/restcomm.http/src/main/java/org/restcomm/connect/http/ConferencesEndpoint.java

+            //allow super admin to perform update on any conference - https://telestax.atlassian.net/browse/RESTCOMM-1171
+            if (!isSuperAdmin()) {
+                try {
+                    secure(account, cdr.getAccountSid(), SecuredType.SECURED_STANDARD);


@maria-farooq can you elaborate on that? Why we use secure again here?

conference termination request can be initiated by owner of the conference as well hence we need to make sure that resource is being accessed by proper account

gvagenas · 2017-11-13T15:51:35Z

restcomm/restcomm.http/src/main/java/org/restcomm/connect/http/ConferencesEndpoint.java

+            final String status = data.getFirst("Status");
+
+            if(status != null){
+                if (status.equalsIgnoreCase("completed")) {


@maria-farooq you can combine status != null AND status.equalsIgnoreCase("completed") in the same if statement so we don't need duplicate else clause that does exactly the same thing

good point, changed

gvagenas · 2017-11-13T15:54:28Z

restcomm/restcomm.http/src/main/java/org/restcomm/connect/http/ConferencesEndpoint.java

+                while(iterator.hasNext()){
+                    final CallDetailRecord CallDR = iterator.next();
+                    if (callManager == null)
+                        callManager = (ActorRef) context.getAttribute("org.restcomm.connect.telephony.CallManager");


@maria-farooq why don't you retrieve the CallManager actor outside the while statement so you do it just once and not for every CDR in the list?

gvagenas · 2017-11-13T15:56:14Z

restcomm/restcomm.http/src/main/java/org/restcomm/connect/http/ConferencesEndpoint.java

+                    final CallDetailRecord CallDR = iterator.next();
+                    if (callManager == null)
+                        callManager = (ActorRef) context.getAttribute("org.restcomm.connect.telephony.CallManager");
+                    callManager.tell(new Hangup("Conference Terminated", effectiveAccount.getSid(), CallDR), null);


@maria-farooq Does it worth to create a new message instead of use Hangup that is used by VoiceInterpreter to ask the Call actor to terminate the call?
Not important, just wondering if it would be a better approach to have a new message here

gvagenas · 2017-11-14T10:13:55Z

@maria-farooq I am not comfortable with the current implementation because I can see redundant functionality and not a clear separation of concerns in the actors/endpoints involved. Let me elaborate:

The feature here is about how to terminate a Conference and remove all call actors.
The conference termination might be initiated because of a timer or because of a user action (REST API)
Call actors might be either local actors or remote actors.
Local actors should be removed using Akka messaging (send Leave message)
Remote actors should be removed using LCM

Currently, your patch implements the following:

On timer timeout Conference actor moves to Evicting state where a) asks Local actors to Leave b) creates a ConferenceApiClient actor and send to it the StopConference message
The ConferenceApiClient will then create a REST API request for the ConferencesEndpoint to update Conference.
The ConferencesEndpoint will ask CallManager to Hangup all calls found in the CDR table related to the given Conference
CallManager will first try to remove local calls and then will try to remove local calls using CallsApiClient
CallApiClient will create an LCM REST API request to CallsEndpoint to complete the remote call

My comments on the above implementation:

When Conference actor reach the Evicting state, should be self-sufficient and a) remove local call actor b) check DB and remove remote call actors using LCM
In both timeout and user-initiated StopConfernece, Conference actor should be responsible to move FSM state to Evicting and thus remove local and remote call actors
There is no reason for the CallManager to attempt to clean up local call actors since given the above steps, we know that Conference actor cleaned up already the local call actors, and will initiate LCM for the remote actors
The ConferenceApiClient is redundant in that case, it shouldn't be used
On user-initiated StopConference the ConferencesEndpoint should not try to kick out any participant, this endpoint role is not to deal with participants and calls. The ConferencesEndpoint should only try to update the state of the Conference to Evicting by sending StopConference to either the local or remote Conference actor (akka messaging or LCM)
This way, we don't have duplicate functionality where ConferencesEndpoint kicks out participants, its the responsibility of Conference actor to kick out call actors
Thus CallApiClient is also redundant here
LCM needs to be enhanced in order to:
- When we complete a call actor because Conference stopped, we need the proper Header in the Bye request
- Be able to reach remote Conference actors and send to them StopConference

* master: Updated quick start for beta.5. added unstable beased on latest test results from CI LiveCallModificationTest modified to test LiveCall and Mgcp metrics This refer to #RESTCOMM-1238 fixes subsequent regex removal includign category to exclude unstable tests

…incharge of removing local and remote participants

gvagenas

@maria-farooq see my comments for a minor issue that I think should be fixed.
Other than that looks good

gvagenas · 2017-11-15T14:09:29Z

restcomm/restcomm.telephony/src/main/java/org/restcomm/connect/telephony/Conference.java

@@ -95,6 +108,8 @@
    private Sid sid;
    private final List<ActorRef> calls;
    private final List<ActorRef> observers;
+    //list of callApiClients created by Conference
+    private List<ActorRef> callApiClients;


@maria-farooq There is no need to keep a list of CallApiClients, this is redundant and will consume memory for no reason. See my comment later for the onDownloaderResponse method

gvagenas · 2017-11-15T14:13:00Z

restcomm/restcomm.telephony/src/main/java/org/restcomm/connect/telephony/Conference.java

+    private void onDownloaderResponse(DownloaderResponse message, ActorRef self, ActorRef sender) {
+        //make sure we only remove sender that is callApiClient and was created by this conference actor
+        //so we don't kill a sender that has accidently broadcast a DownloaderResponse to this conference.
+        if(callApiClients.contains(sender)){


@maria-farooq as said earlier no need to keep a list of CallApiClient and then check if sender is on the list.
I believe we should stop sender straight away UNLESS if there is a chance to receive DownloaderResponse from other actors, which I cannot think of such a case.

Even if that's the case (other actors sends DownloaderResponse to Conference) maybe its better to create another wrapper message for the responses of CallApiClient

…er response

maria-farooq · 2017-11-15T15:27:30Z

@gvagenas created a new wrapper for callapiresponse instead of reusing downloader response.

Maria Farooq added 22 commits November 2, 2017 16:27

this refer to #RESTCOMM-1171

4f33141

commit local changes will check later

5ba838a

started using asynch client

fec439f

added LCM

40d77d4

formatting and added kick

2fe05d4

added test to terminate conference via api

c4fa7cc

trying to create a reusable asycn http client

c5af011

build reusable CloseableHttpAsyncClient

dc68ed7

added new actor to perform http client operation in async way

4b7c374

pushing pending changes

1de1a58

updated access level for methods and test case

fdda0a3

adde proper client classes for call and conference apis

b9e5734

implemented conference poststop

793f114

fixed issue

4fc12c0

corrected unit test

aeebd97

fixed test

1d346c2

fixed test cases

aacf91d

renamed

17151ba

refcatoring

f89c59c

did pending cleanup in logging

2b90186

Merge branch 'master' into RESTCOMM-1171

247ae53

* master: #2519 fixing testcases 1/ ProxyManager: user name contains host part fix 2/ Set max expire time to 3600 removed double quotes. SQL tested in stagin DB new method to count by filter. fixes #2610

allow super admin to perform any standard secure operation

71dc97d

jaimecasero reviewed Nov 7, 2017

View reviewed changes

made class to execute in parallel and some other changes as per revie…

a32529d

…wer's feedback

gvagenas changed the title ~~Restcomm 1171~~ Conference timeout and termination API - Restcomm 1171 Nov 9, 2017

gvagenas suggested changes Nov 9, 2017

View reviewed changes

Maria Farooq added 11 commits November 10, 2017 10:10

handled bad request

99c1591

i added this check for superadmin in conference and call endpoints in…

92a4033

…tead

made call/conference api client to be disposeable (used for one requ…

9f17919

…est) and get destroyed after that.

let requesting account be incharge of terminating conference/calls

bbe2e99

dedicate callmanager to hangup, seperating local and remote calls

27a247f

updated conference documentation

47b1268

corrected the actor creation

299f3c9

implemented timeout

b4e6389

check style fix

585b102

changed timeout unit

0303a1f

gvagenas reviewed Nov 13, 2017

View reviewed changes

added unit test for actor cleanup and some code optimisation

9590d8f

Maria Farooq added 4 commits November 15, 2017 12:21

revert conferencet termination api changed and made conference actor …

6b2fc12

…incharge of removing local and remote participants

remove space for better diff

3346140

reverse conference tool test class

2a10bd9

gvagenas reviewed Nov 15, 2017

View reviewed changes

created a new wrapper for callapiresponse instead od reusing download…

853d9c5

…er response

remove new param from StopConference

11ebe24

maria-farooq merged commit 4e8dfe2 into master Nov 30, 2017

jaimecasero deleted the RESTCOMM-1171 branch April 11, 2018 20:15

Conference timeout and termination API - Restcomm 1171 #2617

Conference timeout and termination API - Restcomm 1171 #2617

Conversation

maria-farooq commented Nov 7, 2017 • edited Loading

jaimecasero Nov 7, 2017 • edited Loading

Choose a reason for hiding this comment

jaimecasero Nov 7, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jaimecasero Nov 7, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maria-farooq Nov 10, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maria-farooq Nov 9, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jaimecasero left a comment

Choose a reason for hiding this comment

gvagenas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gvagenas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maria-farooq commented Nov 7, 2017 •

edited

Loading

jaimecasero Nov 7, 2017 •

edited

Loading

jaimecasero Nov 7, 2017 •

edited

Loading

jaimecasero Nov 7, 2017 •

edited

Loading

maria-farooq Nov 10, 2017 •

edited

Loading

maria-farooq Nov 9, 2017 •

edited

Loading