Skip to content

Commit 8742e3d

Browse files
gvagenasgvagenas
committed
- Patch for Recording and Dial with record, race condition
- Patch for DialStatusCallback to don't execute the same state more than once This close #2332 This close #2335
1 parent d88bac6 commit 8742e3d

File tree

7 files changed

+159
-32
lines changed

7 files changed

+159
-32
lines changed

Diff for: ‎restcomm/restcomm.http/src/main/java/org/restcomm/connect/http/CallsEndpoint.java

+3-2
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@
8181
import java.text.ParseException;
8282
import java.util.ArrayList;
8383
import java.util.Arrays;
84+
import java.util.LinkedList;
8485
import java.util.List;
8586
import java.util.concurrent.CopyOnWriteArrayList;
8687
import java.util.concurrent.TimeUnit;
@@ -350,7 +351,7 @@ protected Response putCall(final String accountSid, final MultivaluedMap<String,
350351

351352
URI statusCallback = null;
352353
String statusCallbackMethod = "POST";
353-
List<String> statusCallbackEvent = new ArrayList<String>();
354+
List<String> statusCallbackEvent = new LinkedList<String>();
354355
statusCallbackEvent.add("initiated");
355356
statusCallbackEvent.add("ringing");
356357
statusCallbackEvent.add("answered");
@@ -377,7 +378,7 @@ protected Response putCall(final String accountSid, final MultivaluedMap<String,
377378
statusCallbackMethod = data.getFirst("StatusCallbackMethod").trim();
378379
}
379380
if (data.containsKey("StatusCallbackEvent")) {
380-
statusCallbackEvent = Arrays.asList(data.getFirst("StatusCallbackEvent").trim().split(","));
381+
statusCallbackEvent.addAll(Arrays.asList(data.getFirst("StatusCallbackEvent").trim().split(",")));
381382
}
382383
}
383384

Diff for: ‎restcomm/restcomm.interpreter/src/main/java/org/restcomm/connect/interpreter/BaseVoiceInterpreter.java

+12-11
Original file line numberDiff line numberDiff line change
@@ -1780,6 +1780,10 @@ public void execute(final Object message) throws Exception {
17801780
if (duration.equals(0.0)) {
17811781
final DateTime end = DateTime.now();
17821782
duration = new Double((end.getMillis() - callRecord.getStartTime().getMillis()) / 1000);
1783+
if (logger.isDebugEnabled()) {
1784+
String msg = String.format("Recording duration %s, startTime %s endTime %s", duration, callRecord.getStartTime().getMillis(), end.getMillis());
1785+
logger.debug(msg);
1786+
}
17831787
} else if(logger.isDebugEnabled()) {
17841788
logger.debug("File already exists, length: "+ (new File(recordingUri).length()));
17851789
}
@@ -1860,8 +1864,10 @@ public void execute(final Object message) throws Exception {
18601864
logger.error(exception.getMessage(), exception);
18611865
}
18621866
}
1863-
} else if(logger.isInfoEnabled()){
1864-
logger.info("AsrService is not enabled");
1867+
} else {
1868+
if(logger.isDebugEnabled()){
1869+
logger.debug("AsrService is not enabled");
1870+
}
18651871
}
18661872

18671873
// If action is present redirect to the action URI.
@@ -1944,15 +1950,10 @@ public void execute(final Object message) throws Exception {
19441950
// final StopInterpreter stop = new StopInterpreter();
19451951
// source.tell(stop, source);
19461952
}
1947-
}
1948-
if (CallStateChanged.class.equals(klass) ) {
1949-
if (action == null || action.isEmpty()) {
1950-
source.tell(new StopInterpreter(), source);
1951-
} else {
1952-
// Ask the parser for the next action to take.
1953-
final GetNextVerb next = new GetNextVerb();
1954-
parser.tell(next, source);
1955-
}
1953+
} else {
1954+
//Action is null here
1955+
final GetNextVerb next = new GetNextVerb();
1956+
parser.tell(next, source);
19561957
}
19571958
// A little clean up.
19581959
recordingSid = null;

Diff for: ‎restcomm/restcomm.interpreter/src/main/java/org/restcomm/connect/interpreter/VoiceInterpreter.java

+21-10
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,7 @@
127127
import java.util.Currency;
128128
import java.util.HashMap;
129129
import java.util.Iterator;
130+
import java.util.LinkedList;
130131
import java.util.List;
131132
import java.util.Map;
132133
import java.util.concurrent.TimeUnit;
@@ -2079,7 +2080,7 @@ else if (CallManagerResponse.class.equals(klass) && !((CallManagerResponse)messa
20792080

20802081
URI statusCallback = null;
20812082
String statusCallbackMethod = "POST";
2082-
List<String> statusCallbackEvent = null;
2083+
List<String> statusCallbackEvent = new LinkedList<String>();
20832084

20842085
if (child.hasAttribute("statusCallback")) {
20852086
statusCallback = new URI(child.attribute("statusCallback").value());
@@ -2089,9 +2090,9 @@ else if (CallManagerResponse.class.equals(klass) && !((CallManagerResponse)messa
20892090
statusCallbackMethod = child.attribute("statusCallbackMethod").value();
20902091
}
20912092
if (child.hasAttribute("statusCallbackEvent")) {
2092-
statusCallbackEvent = Arrays.asList(child.attribute("statusCallbackEvent").value().replaceAll("\\s+","").split(","));
2093+
statusCallbackEvent.addAll(Arrays.asList(child.attribute("statusCallbackEvent").value().replaceAll("\\s+","").split(",")));
20932094
} else {
2094-
statusCallbackEvent = new ArrayList<String>();
2095+
statusCallbackEvent = new LinkedList<String>();
20952096
statusCallbackEvent.add("initiated");
20962097
statusCallbackEvent.add("ringing");
20972098
statusCallbackEvent.add("answered");
@@ -2300,15 +2301,21 @@ private void recordConference() {
23002301

23012302
@SuppressWarnings("unchecked")
23022303
private void executeDialAction(final Object message, final ActorRef outboundCall) {
2303-
if (!dialActionExecuted && verb != null && Verbs.dial.equals(verb.name())) {
2304+
Attribute attribute = null;
2305+
if (verb != null && Verbs.dial.equals(verb.name())) {
2306+
attribute = verb.attribute("action");
2307+
} else {
2308+
if (logger.isInfoEnabled()) {
2309+
logger.info("Either Verb is null OR not Dial, Dial Action will not be executed");
2310+
}
2311+
}
2312+
if (attribute != null && !dialActionExecuted) {
23042313
if(logger.isInfoEnabled()){
23052314
logger.info("Proceeding to execute Dial Action attribute");
23062315
}
23072316
this.dialActionExecuted = true;
23082317
final List<NameValuePair> parameters = parameters();
23092318

2310-
Attribute attribute = verb.attribute("action");
2311-
23122319
if (call != null) {
23132320
try {
23142321
if(logger.isInfoEnabled()) {
@@ -2473,9 +2480,13 @@ else if (message instanceof ReceiveTimeout) {
24732480
return;
24742481
}
24752482
}
2476-
} else if (verb == null) {
2477-
if(logger.isInfoEnabled()) {
2478-
logger.info("Dial action didn't executed because verb is null");
2483+
} else {
2484+
if (logger.isInfoEnabled()) {
2485+
if (attribute == null) {
2486+
logger.info("DialAction URL is null, DialAction will not be executed");
2487+
} else {
2488+
logger.info("DialAction has already been executed");
2489+
}
24792490
}
24802491
}
24812492
}
@@ -3098,7 +3109,7 @@ public void execute(final Object message) throws Exception {
30983109
callManager.tell(new DestroyCall(call), super.source);
30993110
if (outboundCall != null) {
31003111
callManager.tell(new DestroyCall(outboundCall), super.source);
3101-
} if (sender != call) {
3112+
} if (sender != call && !sender.equals(self())) {
31023113
callManager.tell(new DestroyCall(sender), super.source);
31033114
}
31043115
} else {

Diff for: ‎restcomm/restcomm.telephony/src/main/java/org/restcomm/connect/telephony/Call.java

+22-4
Original file line numberDiff line numberDiff line change
@@ -561,7 +561,7 @@ private List<NameValuePair> dialStatusCallbackParameters(final CallbackState sta
561561

562562
private void executeStatusCallback(final CallbackState state) {
563563
if (statusCallback != null) {
564-
if (statusCallbackEvent.contains(state.toString())) {
564+
if (statusCallbackEvent.remove(state.toString())) {
565565
if (logger.isDebugEnabled()) {
566566
String msg = String.format("About to execute Call StatusCallback for state %s to StatusCallback %s. Call from %s to %s direction %s", state.text, statusCallback.toString(), from.toString(), to.toString(), direction);
567567
logger.debug(msg);
@@ -577,7 +577,7 @@ private void executeStatusCallback(final CallbackState state) {
577577
}
578578
} else {
579579
if (logger.isDebugEnabled()) {
580-
String msg = String.format("Call StatusCallback did not run because state %s no in the statusCallbackEvent list", state.text);
580+
String msg = String.format("Call StatusCallback will not be sent because its either not in the statusCallbackEvent list or already sent, state %s . Call from %s to %s direction %s", state.text, from.toString(), to.toString(), direction);
581581
logger.debug(msg);
582582
}
583583
}
@@ -1722,10 +1722,26 @@ public Stopping(final ActorRef source) {
17221722
@Override
17231723
public void execute(Object message) throws Exception {
17241724
// Stops media operations and closes media session
1725+
if (logger.isDebugEnabled()) {
1726+
if (message instanceof SipServletRequest) {
1727+
logger.debug("At Call Stopping state because of SipServletRequest: "+((SipServletRequest)message).getMethod());
1728+
} else if (message instanceof Hangup) {
1729+
logger.debug("At Call Stopping state because of Hangup: "+((Hangup)message));
1730+
} else {
1731+
logger.debug("At Call Stopping state because of Message: "+message);
1732+
}
1733+
}
1734+
17251735
msController.tell(new CloseMediaSession(), source);
17261736
if (fail) {
1737+
if (logger.isDebugEnabled()) {
1738+
logger.debug("At Call Stopping state, moving to Failed state");
1739+
}
17271740
fsm.transition(message, failed);
17281741
} else {
1742+
if (logger.isDebugEnabled()) {
1743+
logger.debug("At Call Stopping state, moving to Completed state");
1744+
}
17291745
fsm.transition(message, completed);
17301746
}
17311747
}
@@ -2198,7 +2214,7 @@ private void onSipServletResponse(SipServletResponse message, ActorRef self, Act
21982214

21992215
private void onHangup(Hangup message, ActorRef self, ActorRef sender) throws Exception {
22002216
if(logger.isDebugEnabled()) {
2201-
logger.debug("Got Hangup: "+message+" for Call, from: "+from+" to: "+to+" state: "+fsm.state()+" conferencing: "+conferencing +" conference: "+conference);
2217+
logger.debug("Got Hangup: "+message+" for Call, from: "+from+", to: "+to+", state: "+fsm.state()+", conferencing: "+conferencing +", conference: "+conference);
22022218
}
22032219

22042220
if (is(completed)) {
@@ -2217,7 +2233,7 @@ private void onHangup(Hangup message, ActorRef self, ActorRef sender) throws Exc
22172233
msController.tell(new Stop(true), self);
22182234
}
22192235

2220-
if (is(updatingMediaSession) || is(ringing) || is(queued) || is(dialing) || is(inProgress) || is(completed) || is(waitingForAnswer)) {
2236+
if (is(updatingMediaSession) || is(ringing) || is(queued) || is(dialing) || is(inProgress) || is(waitingForAnswer)) {
22212237
if (conferencing) {
22222238
// Tell conference to remove the call from participants list
22232239
// before moving to a stopping state
@@ -2233,6 +2249,8 @@ private void onHangup(Hangup message, ActorRef self, ActorRef sender) throws Exc
22332249
}
22342250
} else if (is(failingNoAnswer)) {
22352251
fsm.transition(message, canceling);
2252+
} else if (is(stopping)) {
2253+
fsm.transition(message, completed);
22362254
}
22372255
}
22382256

Diff for: ‎restcomm/restcomm.testsuite/src/test/java/org/restcomm/connect/testsuite/telephony/DialRecordingS3UploadTest_Secure.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -410,7 +410,7 @@ public synchronized void testRecordCall() throws InterruptedException, ParseExce
410410
assertNotNull(recording);
411411
assertEquals(1, recording.size());
412412
double duration = recording.get(0).getAsJsonObject().get("duration").getAsDouble();
413-
assertEquals(6.0, duration,1);
413+
assertEquals(3.0, duration,1);
414414
assertTrue(recording.get(0).getAsJsonObject().get("file_uri").getAsString().startsWith("http://localhost:8080/restcomm/2012-04-24/Accounts/ACae6e420f425248d6a26948c17a9e2acf/Recordings/"));
415415

416416
//Since we are in secure mode the s3_uri shouldn't be here
@@ -487,7 +487,7 @@ public synchronized void testRecordCallWithAction() throws InterruptedException,
487487
assertNotNull(recording);
488488
assertEquals(1, recording.size());
489489
double duration = recording.get(0).getAsJsonObject().get("duration").getAsDouble();
490-
assertEquals(6.0, duration,1);
490+
assertEquals(3.0, duration,1);
491491
assertTrue(recording.get(0).getAsJsonObject().get("file_uri").getAsString().startsWith("http://localhost:8080/restcomm/2012-04-24/Accounts/ACae6e420f425248d6a26948c17a9e2acf/Recordings/"));
492492

493493
//Since we are in secure mode the s3_uri shouldn't be here

Diff for: ‎restcomm/restcomm.testsuite/src/test/java/org/restcomm/connect/testsuite/telephony/DialRecordingTest.java

+71
Original file line numberDiff line numberDiff line change
@@ -535,6 +535,77 @@ public synchronized void testRecordCallWithAction() throws InterruptedException,
535535
assertEquals(0, liveCallsArraySize);
536536
}
537537

538+
@Test
539+
public synchronized void testRecordCallWithActionWithMaxRecordingReached() throws InterruptedException, ParseException {
540+
stubFor(get(urlPathEqualTo("/1111"))
541+
.willReturn(aResponse()
542+
.withStatus(200)
543+
.withHeader("Content-Type", "text/xml")
544+
.withBody(recordCallWithAction)));
545+
546+
stubFor(post(urlPathEqualTo("/record-action"))
547+
.willReturn(aResponse()
548+
.withStatus(200)
549+
.withHeader("Content-Type", "text/xml")
550+
.withBody(hangupRcml)));
551+
552+
// Create outgoing call with first phone
553+
final SipCall bobCall = bobPhone.createSipCall();
554+
bobCall.initiateOutgoingCall(bobContact, dialRestcomm, null, body, "application", "sdp", null, null);
555+
assertLastOperationSuccess(bobCall);
556+
assertTrue(bobCall.waitOutgoingCallResponse(5 * 1000));
557+
final int response = bobCall.getLastReceivedResponse().getStatusCode();
558+
assertTrue(response == Response.TRYING || response == Response.RINGING);
559+
560+
if (response == Response.TRYING) {
561+
assertTrue(bobCall.waitOutgoingCallResponse(5 * 1000));
562+
assertEquals(Response.RINGING, bobCall.getLastReceivedResponse().getStatusCode());
563+
}
564+
565+
assertTrue(bobCall.waitOutgoingCallResponse(5 * 1000));
566+
assertEquals(Response.OK, bobCall.getLastReceivedResponse().getStatusCode());
567+
568+
bobCall.sendInviteOkAck();
569+
DateTime start = DateTime.now();
570+
assertTrue(!(bobCall.getLastReceivedResponse().getStatusCode() >= 400));
571+
String callSid = bobCall.getLastReceivedResponse().getMessage().getHeader("X-RestComm-CallSid").toString().split(":")[1].trim().split("-")[1];
572+
573+
JsonObject metrics = MonitoringServiceTool.getInstance().getMetrics(deploymentUrl.toString(),adminAccountSid, adminAuthToken);
574+
assertNotNull(metrics);
575+
int liveCalls = metrics.getAsJsonObject("Metrics").get("LiveCalls").getAsInt();
576+
logger.info("LiveCalls: "+liveCalls);
577+
int liveCallsArraySize = metrics.getAsJsonArray("LiveCallDetails").size();
578+
logger.info("LiveCallsArraySize: "+liveCallsArraySize);
579+
assertEquals(1,liveCalls);
580+
assertEquals(1, liveCallsArraySize);
581+
582+
bobCall.listenForDisconnect();
583+
assertTrue(bobCall.waitForDisconnect(70000));
584+
assertTrue(bobCall.respondToDisconnect());
585+
DateTime end = DateTime.now();
586+
587+
Thread.sleep(500);
588+
589+
//Check recording
590+
JsonArray recording = RestcommCallsTool.getInstance().getCallRecordings(deploymentUrl.toString(),adminAccountSid,adminAuthToken,callSid);
591+
assertNotNull(recording);
592+
assertEquals(1, recording.size());
593+
double recordedDuration = (end.getMillis() - start.getMillis())/1000;
594+
double duration = recording.get(0).getAsJsonObject().get("duration").getAsDouble();
595+
assertEquals(recordedDuration, duration,0);
596+
597+
logger.info("\n\n &&&&&& About to check liveCalls &&&&&& \n");
598+
599+
metrics = MonitoringServiceTool.getInstance().getMetrics(deploymentUrl.toString(),adminAccountSid, adminAuthToken);
600+
assertNotNull(metrics);
601+
liveCalls = metrics.getAsJsonObject("Metrics").get("LiveCalls").getAsInt();
602+
logger.info("LiveCalls: "+liveCalls);
603+
liveCallsArraySize = metrics.getAsJsonArray("LiveCallDetails").size();
604+
logger.info("LiveCallsArraySize: "+liveCallsArraySize);
605+
assertEquals(0,liveCalls);
606+
assertEquals(0, liveCallsArraySize);
607+
}
608+
538609
@Test
539610
public void testGetRecordingWithOldS3Url() {
540611
String callSid = "CA2d3f6354e75e46b3ac76f534129ff511";

Diff for: ‎restcomm/restcomm.testsuite/src/test/java/org/restcomm/connect/testsuite/telephony/DialStatusCallbackTest.java

+28-3
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,9 @@
5252
import java.net.URL;
5353
import java.text.ParseException;
5454
import java.util.Arrays;
55+
import java.util.HashMap;
5556
import java.util.List;
57+
import java.util.Map;
5658

5759
import static com.github.tomakehurst.wiremock.client.WireMock.aResponse;
5860
import static com.github.tomakehurst.wiremock.client.WireMock.findAll;
@@ -640,7 +642,7 @@ public void testDialStatusCallbackOnlyRingingCompleted() throws ParseException,
640642
assertTrue(aliceCall.waitForDisconnect(5000));
641643
assertTrue(aliceCall.respondToDisconnect());
642644

643-
Thread.sleep(10000);
645+
Thread.sleep(12000);
644646

645647
logger.info("About to check the StatusCallback Requests");
646648
List<LoggedRequest> requests = findAll(postRequestedFor(urlPathMatching("/status.*")));
@@ -946,7 +948,7 @@ public synchronized void testDialForkNoAnswerButHenriqueStatusCallbackOnAll() th
946948

947949
assertTrue(alicePhone.unregister(aliceContact, 3600));
948950

949-
Thread.sleep(10000);
951+
Thread.sleep(12000);
950952

951953
logger.info("About to check the StatusCallback Requests");
952954
List<LoggedRequest> requests = findAll(getRequestedFor(urlPathMatching("/status.*")));
@@ -1094,7 +1096,7 @@ public synchronized void testDialForkNoAnswerButHenriqueStatusCallbackOnAllPost(
10941096

10951097
assertTrue(alicePhone.unregister(aliceContact, 3600));
10961098

1097-
Thread.sleep(10000);
1099+
Thread.sleep(12000);
10981100

10991101
logger.info("About to check the StatusCallback Requests");
11001102
List<LoggedRequest> requests = findAll(postRequestedFor(urlPathMatching("/status.*")));
@@ -1578,9 +1580,32 @@ public synchronized void testDialForkNoAnswerExecuteRCML_ReturnedFromActionURLWi
15781580

15791581
logger.info("About to check the StatusCallback Requests");
15801582
requests = findAll(getRequestedFor(urlPathMatching("/status.*")));
1583+
Map<String, String> requestMap = getRequestMap(requests);
15811584
assertEquals(13, requests.size());
15821585
}
15831586

1587+
private Map<String, String> getRequestMap(final List<LoggedRequest> requestList) {
1588+
Map<String, String> resultMap = new HashMap<String, String>();
1589+
for(LoggedRequest request: requestList) {
1590+
String[] tokens = request.getUrl().split("&");
1591+
String to = null;
1592+
String callStatus = null;
1593+
for (String token: tokens) {
1594+
if (token.contains("To")) {
1595+
to = token;
1596+
}
1597+
if (token.contains("SequenceNumber")) {
1598+
to = to+token;
1599+
}
1600+
if (token.contains("CallStatus")) {
1601+
callStatus = token;
1602+
}
1603+
}
1604+
resultMap.put(to,callStatus);
1605+
}
1606+
return resultMap;
1607+
}
1608+
15841609
@Deployment(name = "DialAction", managed = true, testable = false)
15851610
public static WebArchive createWebArchiveNoGw() {
15861611
logger.info("Packaging Test App");

0 commit comments

Comments
 (0)