Skip to content

Commit

Permalink
Allow message and presencemessage to work without data
Browse files Browse the repository at this point in the history
  • Loading branch information
Simon Woolf committed May 11, 2015
1 parent c1c7606 commit 440f4ec
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 8 deletions.
2 changes: 1 addition & 1 deletion common/lib/client/presence.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ var Presence = (function() {
Presence.prototype.enterClient = function(clientId, data, callback) {
if (!callback && (typeof(data)==='function')) {
callback = data;
data = '';
data = null;
}
Logger.logAction(Logger.LOG_MICRO, 'Presence.enterClient()', 'entering; channel = ' + this.channel.name + ', client = ' + clientId);
var presence = PresenceMessage.fromValues({
Expand Down
14 changes: 8 additions & 6 deletions common/lib/types/message.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,11 @@ var Message = (function() {
encoding: this.encoding
};

/* encode to base64 if we're returning real JSON;
/* encode data to base64 if present and we're returning real JSON;
* although msgpack calls toJSON(), we know it is a stringify()
* call if it has a non-empty arguments list */
var data = this.data;
if(arguments.length > 0 && BufferUtils.isBuffer(data)) {
if(data && arguments.length > 0 && BufferUtils.isBuffer(data)) {
var encoding = this.encoding;
result.encoding = encoding ? (encoding + '/base64') : 'base64';
data = BufferUtils.base64Encode(data);
Expand Down Expand Up @@ -78,10 +78,12 @@ var Message = (function() {
};

Message.encode = function(msg, options) {
var data = msg.data, encoding;
if(typeof(data) != 'string' && !BufferUtils.isBuffer(data)) {
msg.data = JSON.stringify(data);
msg.encoding = (encoding = msg.encoding) ? (encoding + '/json') : 'json';
if(msg.data){

This comment has been minimized.

Copy link
@paddybyers

paddybyers May 12, 2015

Member

Why read msg.data here and then read it again on the next line?

Don't we just need to add a data && to the following if ?

Edit: actually that won't work either, because '' is falsy but we want to send ''.

This comment has been minimized.

Copy link
@SimonWoolf

SimonWoolf May 12, 2015

Member

👍 448b73f

('' isn't a problem here as strings aren't altered by this clause anyway. I've added a test that empty strings are preserved in 00e46b5#diff-b65edc1b42f0febc3e85b28340d743c8R85)

var data = msg.data, encoding;
if(typeof(data) != 'string' && !BufferUtils.isBuffer(data)) {
msg.data = JSON.stringify(data);
msg.encoding = (encoding = msg.encoding) ? (encoding + '/json') : 'json';
}
}
if(options != null && options.encrypted)
Message.encrypt(msg, options);
Expand Down
2 changes: 1 addition & 1 deletion common/lib/types/presencemessage.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ var PresenceMessage = (function() {
* although msgpack calls toJSON(), we know it is a stringify()
* call if it passes on the stringify arguments */
var data = this.data;
if(arguments.length > 0 && BufferUtils.isBuffer(data)) {
if(data && arguments.length > 0 && BufferUtils.isBuffer(data)) {
var encoding = this.encoding;
result.encoding = encoding ? (encoding + '/base64') : 'base64';
data = data.toString('base64');
Expand Down

0 comments on commit 440f4ec

Please sign in to comment.