ICEpush
  1. ICEpush
  2. PUSH-233

Add API for server-side registration of notify back URLs

    Details

    • Type: New Feature New Feature
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: 3.3
    • Fix Version/s: None
    • Component/s: Push Library
    • Labels:
      None
    • Environment:
      ICEpush
    • Affects:
      Documentation (User Guide, Ref. Guide, etc.), Sample App./Tutorial

      Description

      As part of PUSH-221, we wanted to be able to programmatically register a user to receive Cloud push notifications using SMS. Currently we don't have an "official" mechanism for doing this. Right now, it appears that the application developer has to manually obtain pushIds from cookies and then submit them to the PushGroupManager but this is unwieldy and looks like it might be somewhat fragile in that it exposes too many internals.

        Activity

        Deryk Sinotte created issue -
        Hide
        Deryk Sinotte added a comment -

        Given a sample use case:

        • user composes an SMS containing ice.push.browser and sends it
          to an ICEsoft phone number. This is convenient because we can
          create a link for them that pre-fills the message with
          the browser ID
          sms:5551212?body=Register%20for%20cloud%20push%20ve6oa
        • we receive an HTTP POST at our server from twilio containing
          the SMS

        Ted's initial suggestion about how it might work using just the browser id:

        At that point we want to call something like this, because we
        have the notifyBackURI, which is sms:phonenumber

        public void park(final String pushId, final NotifyBackURI notifyBackURI)

        { parkedPushIDs.put(pushId, notifyBackURI); }

        So, we should look at how difficult it would be to add more support
        for browserID. Maybe the ability to registerCloudPushId would be
        a good place to start (even if we just kept a table of browserID/pushID
        we could iterate through them and park them all).

        Show
        Deryk Sinotte added a comment - Given a sample use case: user composes an SMS containing ice.push.browser and sends it to an ICEsoft phone number. This is convenient because we can create a link for them that pre-fills the message with the browser ID sms:5551212?body=Register%20for%20cloud%20push%20ve6oa we receive an HTTP POST at our server from twilio containing the SMS Ted's initial suggestion about how it might work using just the browser id: At that point we want to call something like this, because we have the notifyBackURI, which is sms:phonenumber public void park(final String pushId, final NotifyBackURI notifyBackURI) { parkedPushIDs.put(pushId, notifyBackURI); } So, we should look at how difficult it would be to add more support for browserID. Maybe the ability to registerCloudPushId would be a good place to start (even if we just kept a table of browserID/pushID we could iterate through them and park them all).
        Deryk Sinotte made changes -
        Field Original Value New Value
        Assignee Jack Van Ooststroom [ jack.van.ooststroom ]
        Fix Version/s 3.4 [ 10971 ]
        Affects Documentation (User Guide, Ref. Guide, etc.),Sample App./Tutorial [ 10003, 10001 ]
        Hide
        Deryk Sinotte added a comment -

        The current mechanism I've been using in the chat-sms sample application looks something like:

        public static boolean registerForNotification(ServletContext sc, String protocol, String message, List<String> pushIDList) {
            MainServlet ms = MainServlet.getInstance(sc);
            PushGroupManager pusher = ms.getPushGroupManager();
            NotifyBackURI notifyBackURI = new NotifyBackURI(protocol + ":" + message);
            boolean notificationSet = pusher.setNotifyBackURI(pushIDList, notifyBackURI, true);
            log.info("\n  notification set: " + notificationSet +
                    "\n  notify back URI : " + notifyBackURI +
                    "\n  push ID list    : " + pushIDList
            );
            return notificationSet;
        }

        However this also relies some changes that I needed to make to icepush-ee:

        r34339 PUSH-221: some tweaks to allow server side registration of notification providers to work
        r34550 PUSH-221: some tweaks to allow server side registration of notification providers to work

        The main changes involved retrieving the existing notifyBackURI for a PushID (if one exists) and then setting that on the BlockingConnectionServer as it currently only examines requests from the client that contain the specific header.

        I also added a LoggingNotificationProvider that allows desktop browsers to simulate Cloud push behaviour by logging those notifications to the app server's console.

        r34340 PUSH-221: add a LoggingNotificationProvider to help with debugging

        Show
        Deryk Sinotte added a comment - The current mechanism I've been using in the chat-sms sample application looks something like: public static boolean registerForNotification(ServletContext sc, String protocol, String message, List< String > pushIDList) { MainServlet ms = MainServlet.getInstance(sc); PushGroupManager pusher = ms.getPushGroupManager(); NotifyBackURI notifyBackURI = new NotifyBackURI(protocol + ":" + message); boolean notificationSet = pusher.setNotifyBackURI(pushIDList, notifyBackURI, true ); log.info( "\n notification set: " + notificationSet + "\n notify back URI : " + notifyBackURI + "\n push ID list : " + pushIDList ); return notificationSet; } However this also relies some changes that I needed to make to icepush-ee: r34339 PUSH-221 : some tweaks to allow server side registration of notification providers to work r34550 PUSH-221 : some tweaks to allow server side registration of notification providers to work The main changes involved retrieving the existing notifyBackURI for a PushID (if one exists) and then setting that on the BlockingConnectionServer as it currently only examines requests from the client that contain the specific header. I also added a LoggingNotificationProvider that allows desktop browsers to simulate Cloud push behaviour by logging those notifications to the app server's console. r34340 PUSH-221 : add a LoggingNotificationProvider to help with debugging
        Repository Revision Date User Message
        ICEsoft Public SVN Repository #35151 Thu May 16 17:47:54 MDT 2013 deryk.sinotte PUSH-233: fix NPE to avoid causing a Push storm
        Files Changed
        Commit graph MODIFY /icefaces3/trunk/icefaces/core/src/main/java/org/icefaces/impl/push/servlet/ProxyHttpServletRequest.java
        Commit graph MODIFY /icepush/trunk/icepush/core/src/main/java/org/icepush/LocalPushGroupManager.java
        Commit graph MODIFY /icepush/trunk/icepush/core/src/main/java/org/icepush/BlockingConnectionServer.java
        Hide
        Deryk Sinotte added a comment -

        I fixed an NPE that was occurring due the changes noted in the previous comment. Those changes should be re-evaluated during the course of developing the required API.

        I also adjusted the logging to include the exception because it appears that any RuntimeException that occurs here will potentially cause a Push storm.

        Show
        Deryk Sinotte added a comment - I fixed an NPE that was occurring due the changes noted in the previous comment. Those changes should be re-evaluated during the course of developing the required API. I also adjusted the logging to include the exception because it appears that any RuntimeException that occurs here will potentially cause a Push storm.
        Jack Van Ooststroom made changes -
        Fix Version/s 4.0 [ 11383 ]
        Fix Version/s 4.0.BETA [ 10971 ]
        Jack Van Ooststroom made changes -
        Fix Version/s EE-4.0.0.GA [ 11170 ]
        Fix Version/s 4.0 [ 11383 ]
        Jack Van Ooststroom made changes -
        Fix Version/s EE-4.1.0.GA [ 12172 ]
        Fix Version/s EE-4.0.0.GA [ 11170 ]
        Hide
        Jack Van Ooststroom added a comment -

        Changed target to EE-4.1.0.GA for now.

        After discussion this is deemed to be an edge case. We should keep an eye on this one though as eventually we might want/need it for both ICEfaces-EE and BridgeIt. Therefore I'll keep this one on my list.

        Show
        Jack Van Ooststroom added a comment - Changed target to EE-4.1.0.GA for now. After discussion this is deemed to be an edge case. We should keep an eye on this one though as eventually we might want/need it for both ICEfaces-EE and BridgeIt. Therefore I'll keep this one on my list.
        Ken Fyten made changes -
        Fix Version/s EE-4.1.0.GA [ 12172 ]

          People

          • Assignee:
            Jack Van Ooststroom
            Reporter:
            Deryk Sinotte
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated: