Bs 176 SMS statuscallback by jaimecasero · Pull Request #2970 · RestComm/Restcomm-Connect

jaimecasero · 2018-06-13T08:38:28Z

What this PR does / why we need it:
Implement statuscallback for SMS

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 #Bs-176

gvagenas · 2018-06-18T10:08:26Z

            buffer.append(initialSessionSid.toString());
            final URI uri = URI.create(buffer.toString());
            builder.setUri(uri);
+            SmsVerb.populateAttributes(verb, builder);


@jaimecasero to me it seems redundant to hide the preparation of action attributes in a method. Its only 5 lines of code and reduces the visibility of the full task here.

i think this is a good effort on hiding specific verb stuff in a particular class. Right now the interpreter is already to big, and this way we reuse code for other interpreters.

gvagenas · 2018-06-18T10:11:06Z

            } else {
                // Start observing events from the sms session.
                session.tell(new Observe(source), source);
-                // Store the status callback in the sms session.


@jaimecasero I agree removing the check for callback attribute at this point and especially to not stop the interpreter in case the callback url is wrong. As far as I can see in the DAO classes if the callback url is wrong it will result to null, which to me is the correct way to handle the url at this point.
But here we miss the relsolve of relative urls which might be used in an RVD application (and for other application servers)

ummm,this really need careful thinking. Let me find out more details on this.

gvagenas · 2018-06-18T10:12:16Z

+ */
+package org.restcomm.connect.interpreter;
+
+public class CallbackRequestConstants {


@jaimecasero seems redundant since it is never used at the project, unless if your patch is not finished yet.

this was finalyl moved to another class,removing

gvagenas · 2018-06-18T10:17:14Z

+import akka.actor.Extension;
+import org.apache.commons.configuration.Configuration;
+
+public class RCConfExtension implements Extension {


@jaimecasero can you elaborate on the use of this class and also org.restcomm.connect.interpreter.RCConfProvider.

gvagenas · 2018-06-18T10:18:44Z

+public class RCDaoManagerExtension implements Extension {
+    public static DaoManager instance = null;
+
+    public DaoManager getBean() {


@jaimecasero seems unfinished since the setBean() is never called and getBean() will always result to null

yep,let me remvoe this, i was planning to add a new actor,and use extensions to inject DAOs/Conf ,but i didnt added the actor in the end

hrosa · 2018-06-18T09:18:41Z

 uri MEDIUMTEXT NOT NULL,
 smpp_message_id MEDIUMTEXT,
+status_callback MEDIUMTEXT,
+status_callback_method MEDIUMTEXT,


Does the column type need to be so long?

hrosa · 2018-06-18T09:23:01Z

 CREATE MEMORY TABLE PUBLIC."restcomm_registrations"("sid" VARCHAR(34) NOT NULL PRIMARY KEY,"date_created" TIMESTAMP NOT NULL,"date_updated" TIMESTAMP NOT NULL,"date_expires" TIMESTAMP NOT NULL,"address_of_record" VARCHAR(16777216) NOT NULL,"display_name" VARCHAR(255),"user_name" VARCHAR(64) NOT NULL,"user_agent" VARCHAR(16777216),"ttl" INTEGER NOT NULL,"location" VARCHAR(16777216) NOT NULL,"webrtc" BOOLEAN DEFAULT FALSE,"instanceid" VARCHAR(255),"isLBPresent" BOOLEAN DEFAULT FALSE, "organization_sid" VARCHAR(34))
 CREATE MEMORY TABLE PUBLIC."restcomm_short_codes"("sid" VARCHAR(34) NOT NULL PRIMARY KEY,"date_created" TIMESTAMP NOT NULL,"date_updated" TIMESTAMP NOT NULL,"friendly_name" VARCHAR(64) NOT NULL,"account_sid" VARCHAR(34) NOT NULL,"short_code" INTEGER NOT NULL,"api_version" VARCHAR(10) NOT NULL,"sms_url" VARCHAR(16777216),"sms_method" VARCHAR(4),"sms_fallback_url" VARCHAR(16777216),"sms_fallback_method" VARCHAR(4),"uri" VARCHAR(16777216) NOT NULL)
-CREATE MEMORY TABLE PUBLIC."restcomm_sms_messages"("sid" VARCHAR(34) NOT NULL PRIMARY KEY,"date_created" DATETIME NOT NULL,"date_updated" DATETIME NOT NULL,"date_sent" DATETIME,"account_sid" VARCHAR(34) NOT NULL,"sender" VARCHAR(15) NOT NULL,"recipient" VARCHAR(64) NOT NULL,"body" VARCHAR(999) NOT NULL,"status" VARCHAR(20) NOT NULL,"direction" VARCHAR(14) NOT NULL,"price" VARCHAR(8) NOT NULL,"api_version" VARCHAR(10) NOT NULL,"uri" LONGVARCHAR NOT NULL, "smpp_message_id" VARCHAR(1000), "error_code" INT)
+CREATE MEMORY TABLE PUBLIC."restcomm_sms_messages"("sid" VARCHAR(34) NOT NULL PRIMARY KEY,"date_created" DATETIME NOT NULL,"date_updated" DATETIME NOT NULL,"date_sent" DATETIME,"account_sid" VARCHAR(34) NOT NULL,"sender" VARCHAR(15) NOT NULL,"recipient" VARCHAR(64) NOT NULL,"body" VARCHAR(999) NOT NULL,"status" VARCHAR(20) NOT NULL,"direction" VARCHAR(14) NOT NULL,"price" VARCHAR(8) NOT NULL,"api_version" VARCHAR(10) NOT NULL,"uri" LONGVARCHAR NOT NULL, "smpp_message_id" VARCHAR(1000), "error_code" INT, "status_callback" LONGVARCHAR, "status_callback_method" LONGVARCHAR)


Does status_callback_method column type need to be so large?

i dont care much, for me is simply varchar, i dont like the DB schema to be so strict on types.

hrosa · 2018-06-18T09:23:25Z

 CREATE MEMORY TABLE "restcomm_registrations"("sid" VARCHAR(34) NOT NULL PRIMARY KEY,"date_created" DATETIME NOT NULL,"date_updated" DATETIME NOT NULL,"date_expires" DATETIME NOT NULL,"address_of_record" LONGVARCHAR NOT NULL,"display_name" VARCHAR(255),"user_name" VARCHAR(64) NOT NULL,"user_agent" LONGVARCHAR,"ttl" INTEGER NOT NULL,"location" LONGVARCHAR NOT NULL, "webrtc" BOOLEAN DEFAULT FALSE, "instanceid" VARCHAR(255), "isLBPresent" BOOLEAN DEFAULT FALSE, "organization_sid" VARCHAR(34))
 CREATE MEMORY TABLE "restcomm_short_codes"("sid" VARCHAR(34) NOT NULL PRIMARY KEY,"date_created" DATETIME NOT NULL,"date_updated" DATETIME NOT NULL,"friendly_name" VARCHAR(64) NOT NULL,"account_sid" VARCHAR(34) NOT NULL,"short_code" INTEGER NOT NULL,"api_version" VARCHAR(10) NOT NULL,"sms_url" LONGVARCHAR,"sms_method" VARCHAR(4),"sms_fallback_url" LONGVARCHAR,"sms_fallback_method" VARCHAR(4),"uri" LONGVARCHAR NOT NULL)
-CREATE MEMORY TABLE "restcomm_sms_messages"("sid" VARCHAR(34) NOT NULL PRIMARY KEY,"date_created" DATETIME NOT NULL,"date_updated" DATETIME NOT NULL,"date_sent" DATETIME,"account_sid" VARCHAR(34) NOT NULL,"sender" VARCHAR(15) NOT NULL,"recipient" VARCHAR(64) NOT NULL,"body" VARCHAR(999) NOT NULL,"status" VARCHAR(20) NOT NULL,"direction" VARCHAR(14) NOT NULL,"price" VARCHAR(8) NOT NULL,"api_version" VARCHAR(10) NOT NULL,"uri" LONGVARCHAR NOT NULL, "smpp_message_id" VARCHAR(1000), "error_code" INT)
+CREATE MEMORY TABLE "restcomm_sms_messages"("sid" VARCHAR(34) NOT NULL PRIMARY KEY,"date_created" DATETIME NOT NULL,"date_updated" DATETIME NOT NULL,"date_sent" DATETIME,"account_sid" VARCHAR(34) NOT NULL,"sender" VARCHAR(15) NOT NULL,"recipient" VARCHAR(64) NOT NULL,"body" VARCHAR(999) NOT NULL,"status" VARCHAR(20) NOT NULL,"direction" VARCHAR(14) NOT NULL,"price" VARCHAR(8) NOT NULL,"api_version" VARCHAR(10) NOT NULL,"uri" LONGVARCHAR NOT NULL, "smpp_message_id" VARCHAR(1000), "error_code" INT, "status_callback" LONGVARCHAR, "status_callback_method" LONGVARCHAR)


Does status_callback_method column type need to be so large?

hrosa · 2018-06-18T09:31:25Z

                    // Create an SMS record for the text message.
                    final SmsMessage record = sms(new Sid(accountSid), getApiVersion(data), sender, recipient, body,
-                            SmsMessage.Status.SENDING, SmsMessage.Direction.OUTBOUND_API);
+                            SmsMessage.Status.SENDING, SmsMessage.Direction.OUTBOUND_API, statusCallback);


aren't we missing statusCallbackMethod ?

for REST API is only POST, only with RCML you can force GET. At least i couldn't find any param for this on Twilio API

hrosa · 2018-06-18T09:32:33Z


    private SmsMessage sms(final Sid accountSid, final String apiVersion, final String sender, final String recipient,
-            final String body, final SmsMessage.Status status, final SmsMessage.Direction direction) {
+            final String body, final SmsMessage.Status status, final SmsMessage.Direction direction, URI callback) {


aren't we missing statusCallbackMethod?

hrosa · 2018-06-18T10:39:54Z

 CREATE MEMORY TABLE "restcomm_registrations"("sid" VARCHAR(34) NOT NULL PRIMARY KEY,"date_created" DATETIME NOT NULL,"date_updated" DATETIME NOT NULL,"date_expires" DATETIME NOT NULL,"address_of_record" LONGVARCHAR NOT NULL,"display_name" VARCHAR(255),"user_name" VARCHAR(64) NOT NULL,"user_agent" LONGVARCHAR,"ttl" INTEGER NOT NULL,"location" LONGVARCHAR NOT NULL, "webrtc" BOOLEAN DEFAULT FALSE, "instanceid" VARCHAR(255), "isLBPresent" BOOLEAN DEFAULT FALSE, "organization_sid" VARCHAR(34))
 CREATE MEMORY TABLE "restcomm_short_codes"("sid" VARCHAR(34) NOT NULL PRIMARY KEY,"date_created" DATETIME NOT NULL,"date_updated" DATETIME NOT NULL,"friendly_name" VARCHAR(64) NOT NULL,"account_sid" VARCHAR(34) NOT NULL,"short_code" INTEGER NOT NULL,"api_version" VARCHAR(10) NOT NULL,"sms_url" LONGVARCHAR,"sms_method" VARCHAR(4),"sms_fallback_url" LONGVARCHAR,"sms_fallback_method" VARCHAR(4),"uri" LONGVARCHAR NOT NULL)
-CREATE MEMORY TABLE "restcomm_sms_messages"("sid" VARCHAR(34) NOT NULL PRIMARY KEY,"date_created" DATETIME NOT NULL,"date_updated" DATETIME NOT NULL,"date_sent" DATETIME,"account_sid" VARCHAR(34) NOT NULL,"sender" VARCHAR(15) NOT NULL,"recipient" VARCHAR(64) NOT NULL,"body" VARCHAR(999) NOT NULL,"status" VARCHAR(20) NOT NULL,"direction" VARCHAR(14) NOT NULL,"price" VARCHAR(8) NOT NULL,"api_version" VARCHAR(10) NOT NULL,"uri" LONGVARCHAR NOT NULL, "smpp_message_id" VARCHAR(1000), "error_code" INT)
+CREATE MEMORY TABLE "restcomm_sms_messages"("sid" VARCHAR(34) NOT NULL PRIMARY KEY,"date_created" DATETIME NOT NULL,"date_updated" DATETIME NOT NULL,"date_sent" DATETIME,"account_sid" VARCHAR(34) NOT NULL,"sender" VARCHAR(15) NOT NULL,"recipient" VARCHAR(64) NOT NULL,"body" VARCHAR(999) NOT NULL,"status" VARCHAR(20) NOT NULL,"direction" VARCHAR(14) NOT NULL,"price" VARCHAR(8) NOT NULL,"api_version" VARCHAR(10) NOT NULL,"uri" LONGVARCHAR NOT NULL, "smpp_message_id" VARCHAR(1000), "error_code" INT, "status_callback" LONGVARCHAR, "status_callback_method" LONGVARCHAR)
