apache / openwhisk-package-pushnotifications

OpenWhisk Package for Bluemix Push Notifications Service
Apache License 2.0
10 stars 17 forks source link

Support Push Notifications service in other regions than ng #85

Open l2fprod opened 6 years ago

l2fprod commented 6 years ago

It looks like https://github.com/apache/incubator-openwhisk-package-pushnotifications/blob/e12092790eb194fe36980aaf841f7fdcfae2b7bb/packages/actions/sendMessage.js#L432 is hardcoding the pushnotification endpoint to the US South region of IBM Cloud.

When you provision Push Notifications service in other regions, you get local endpoints.

It should be possible when binding the Push Notifications package to specify the URL to use to send the messages.

csantanapr commented 6 years ago

There should be a parameter to override this. For IBM Cloud bindings the credential key url contains the url to use

{
  "appGuid": "11111111-2222-3333-4444-555555555555",
  "url": "http://imfpush.ng.bluemix.net/imfpush/v1/apps/"11111111-2222-3333-4444-555555555555"",
  "admin_url": "//mobile.ng.bluemix.net/imfpushdashboard/?appGuid="11111111-2222-3333-4444-555555555555"",
  "appSecret": "****",
  "clientSecret": "****"
}

It looks like what we want is to allow user to bind the service credentials to the action, and the value url should have the correct value. But there is kind of a problem today the parameter url is already taken

@param {string} url - An optional URL that can be sent along with the alert. Eg : -p url "https:\\www.mycompany.com".
*  

So I proposed that we try to keep backwards compatibility

Option 1

Create a new parameter messageUrl that will be the new parameter for the url to include inside the message. Now check url if it looks like is the APIURL for example checking domain and endpoint *.bluemix.net/imfpush/v1/apps/* then use it as the API baseURL

Sudo code:

if params.url {
  if params.url.RegExp('*.bluemix.net/imfpush/v1/apps/*'){
  //probably using auto bind credentials
  let apiURL = params.url
  let messageUrl = params.messageUrl
  } else {
  let apiURL = params.apiURL ? params.apiURL : 'https://mobile.ng.bluemix.net/imfpush/v1/apps/' + appId;
  // backwards compat url contains message url to include
   let messageUrl = params.url
  }
} 

//...
request({
      method: 'post',
      uri:  apiURL + '/messages',
      headers: {
        'appSecret': appSecret,
        'Accept': 'application/json',
        'Content-Type': 'application/json'
      },
      body: bodyData
    },)

Option 2

Use the bind parameter admin_url

"admin_url": "//mobile.ng.bluemix.net/imfpushdashboard/?appGuid="11111111-2222-3333-4444-555555555555"",

extract the mobile.ng.bluemix.net and use that in API Call sudo code:

if (params.admin_url) {
  let regionDomain  = params.domain || extractRegionDomain(params.admin_url) || `mobile.ng.bluemix.net`
}
//...
request({
      method: 'post',
      uri: 'https://` + regionDomain + `/imfpush/v1/apps/' + appId + '/messages',
      headers: {
        'appSecret': appSecret,
        'Accept': 'application/json',
        'Content-Type': 'application/json'
      },
      body: bodyData
    },

I'm inclining more with Option 2