RestComm / Restcomm-Connect

The Open Source Cloud Communications Platform
http://www.restcomm.com/
GNU Affero General Public License v3.0
243 stars 215 forks source link

JSON responses inconsistent in terms of uri prefix #1723

Closed otsakir closed 7 years ago

otsakir commented 7 years ago

When using the API to retrieve an Account the uri fields is inconsistent with the _subresourceuris fields as the path they contain starts from a different level.

uri:

"uri": "/2012-04-24/Accounts/ACae6e420f425248d6a26948c17a9e2acf"

subresource_uris all start with:

"/restcomm/2012-04-24/Accounts/...

The same applies to Calls API and potentially to all remaining rest apis.

abdulazizali77 commented 7 years ago

@otsakir Looks like the rooturi is also present in Gateways endpoint imo, the Account subresource_uris are all incorrectly implemented and documented, i assume it should all be relative to root uri eg http://host/restcomm/
additionally Twilio's subresourceuri is consistent https://www.twilio.com/docs/api/rest/account#output The fix seems to be removing all the append(rootUri) calls where appropriate (though i might be wrong, again!)

otsakir commented 7 years ago

Hmmm, conceptually, /restcomm prefix of the url is more a part of the web application. In order to change it one should rename the application and not reconfigure the way the responses are generated. I think your thesis is right @abdulazizali77. Twilio confirms it too.

All uris should be relative to .../restcomm

@deruelle , @gvagenas any objections here ? @gvagenas do you have in mind functionality that will break if we strip the '/restcomm' part form the subresource_uris? I think inside restcomm we don't really use them so far. We should be ok to refactor.

@abdulazizali77 apart from Accounts, Calls, Gateways have you checked other APIs too?

deruelle commented 7 years ago

@otsakir @abdulazizali77 agreed. no objections on my side

abdulazizali77 commented 7 years ago

apologies for the late update @otsakir It seems like a lot more APIs incorrectly use the root-uri, most of them are simple fixes Accounts Accounts OutgoingCallerIds Applications Calls Clients Conferences ConferenceParticipants IncomingPhoneNumbers SMSMessages GatewaysManagement

Theres few places in the *Convertor classes which call getRequestURI().getPath() which incorrectly return the root uri, we have 3 options to fix this 1) Relativize the UriInfo to root-uri 2) Catenate strings 2a) "/" + getAPiVersion() + "/" +info.getPath() - this seems to be how most EPs do it, with StringBuffer 2b) request.getSevletPath() + "/" + info.getPath()

Ill try put a PR tonight highly likely with option 2a

otsakir commented 7 years ago

np @abdulazizali77 .

Thanks for the thorough investigation on the matter.

gvagenas commented 7 years ago

Closed by PR https://github.com/RestComm/Restcomm-Connect/pull/1735

Thanks for the contribution @abdulazizali77