{"action":"created","issue":{"url":"https://api.github.com/repos/onsip/SIP.js/issues/231","labels_url":"https://api.github.com/repos/onsip/SIP.js/issues/231/labels{/name}","comments_url":"https://api.github.com/repos/onsip/SIP.js/issues/231/comments","events_url":"https://api.github.com/repos/onsip/SIP.js/issues/231/events","html_url":"https://github.com/onsip/SIP.js/pull/231","id":107826008,"number":231,"title":"Make Session SDP-agnostic","user":{"login":"markandrus","id":359154,"avatar_url":"https://avatars.githubusercontent.com/u/359154?v=3","gravatar_id":"","url":"https://api.github.com/users/markandrus","html_url":"https://github.com/markandrus","followers_url":"https://api.github.com/users/markandrus/followers","following_url":"https://api.github.com/users/markandrus/following{/other_user}","gists_url":"https://api.github.com/users/markandrus/gists{/gist_id}","starred_url":"https://api.github.com/users/markandrus/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/markandrus/subscriptions","organizations_url":"https://api.github.com/users/markandrus/orgs","repos_url":"https://api.github.com/users/markandrus/repos","events_url":"https://api.github.com/users/markandrus/events{/privacy}","received_events_url":"https://api.github.com/users/markandrus/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":10,"created_at":"2015-09-22T23:52:55Z","updated_at":"2016-01-27T18:42:58Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/onsip/SIP.js/pulls/231","html_url":"https://github.com/onsip/SIP.js/pull/231","diff_url":"https://github.com/onsip/SIP.js/pull/231.diff","patch_url":"https://github.com/onsip/SIP.js/pull/231.patch"},"body":"I have a use case where I need to be prepared to send and receive offers and answers in \"multipart/mixed\" messages. This use case may be too specific for SIP.js, so I looked at enabling it in a custom `MediaHandler`. Following up on #166, this PR includes changes that\r\n\r\n- Move SDP hacks into `WebRTC/MediaHandler`.\r\n- Expose an `onhold` property on `WebRTC/MediaHandler`—this is used in `Session` instead of matching a `RegExp` on the SDP.\r\n- Remove SDP mangling from `sendReinvite`—now `WebRTC/MediaHandler` will check an `onhold` property in order to determine if it should rewrite the SDP in `getDescription`.\r\n- Remove Content-Type checking from `Session`—now `Session` will pass the entire `SIPMessage` to `MediaHandler.hasDescription` (which can check its Content-Type, check if it has an empty `body`, or anything else).\r\n- Pass the entire `SIPMessage` to `MediaHandler.setDescription`—for example, a `MediaHandler` expecting \"multipart/mixed\" messages needs to query the Content-Type in order to parse the `body`. There may be other `MediaHandler`s that observe other headers.\r\n- Allow `MediaHandler.getDescription` to specify `extraHeaders` and `body`—for example, a `MediaHandler` could specify a Content-Type other than \"application/sdp\".\r\n\r\nThese are significant changes, and I only have the Jasmine tests passing (I have not yet tested with my application), but would you consider such a refactor?"},"comment":{"url":"https://api.github.com/repos/onsip/SIP.js/issues/comments/175787704","html_url":"https://github.com/onsip/SIP.js/pull/231#issuecomment-175787704","issue_url":"https://api.github.com/repos/onsip/SIP.js/issues/231","id":175787704,"user":{"login":"james-onsip","id":7011816,"avatar_url":"https://avatars.githubusercontent.com/u/7011816?v=3","gravatar_id":"","url":"https://api.github.com/users/james-onsip","html_url":"https://github.com/james-onsip","followers_url":"https://api.github.com/users/james-onsip/followers","following_url":"https://api.github.com/users/james-onsip/following{/other_user}","gists_url":"https://api.github.com/users/james-onsip/gists{/gist_id}","starred_url":"https://api.github.com/users/james-onsip/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/james-onsip/subscriptions","organizations_url":"https://api.github.com/users/james-onsip/orgs","repos_url":"https://api.github.com/users/james-onsip/repos","events_url":"https://api.github.com/users/james-onsip/events{/privacy}","received_events_url":"https://api.github.com/users/james-onsip/received_events","type":"User","site_admin":false},"created_at":"2016-01-27T18:42:58Z","updated_at":"2016-01-27T18:42:58Z","body":"I agree, that the smaller, \"make the string the argument\" was not as good as the larger refactor, but it may have been enough to get this by. I do prefer the larger refactor as well.\r\n\r\n> Please let me know if I am off track here, but one thing I notice is that the object returned by getDescription now shares a couple properties (body and extraHeaders) with the options accepted by sendRequest. If we added support for the contentType property to sendRequest, then Session.js could simply merge the options it constructs with the object returned by getDescription before calling sendRequest. reply could be updated similarly.\r\n\r\nThis would definitely be the cleanest, preferred way to do it, and hopefully the conclusion I would've reached had i thought about it more. Giving contentType it's own property in the object passed to `sendRequest` and `reply` certainly feels more in line with how the RFC describes it. I think passing it through and merging with options is good, certainly better than what is there now, but in the future maybe more could be done.\r\n\r\n(This paragraph may be overkill and unnecessarily complicated, I just wanted to put it down as potential for the future).\r\nI'm thinking it may make sense for it to work like [route_set](https://github.com/onsip/SIP.js/blob/master/src/SIPMessage.js#L87-L91). If you are using a custom MediaHandler that can only handle a specific type of message, making a config option that defines what type is acceptable may make sense. Maybe just making it a property of the MediaHandler that could be read when necessary? And, just like route set, this could be overridden by the options merging idea you describe above. This way, the contentType argument becomes optional, and contentType is coded to be  more like an RFC MUST. If that doesn't make any sense, doing it as you described would still work just great.\r\n\r\nThanks,\r\nJames"}}