linnovate / mean-socket

6 stars 5 forks source link

Proposed alternate solution #5

Open gulsharan opened 10 years ago

gulsharan commented 10 years ago

@rivkatzur Thanks for this awesome plugin, it certainly helped me get started integrating chat into my application. However, I faced a few issues when I tried to deploy it in production environment, as my own app was running on one port, and this particular plugin needed to listen on another port of its own.

So, I had to rethink the implementation and finally came up with a solution listed below. The solution is totally non-invasive and would be a good starting point for anyone else who's still looking to add chat to their mean.io app.

app.js

In this file, I just invoke the code that creates and sets up a socket.io object for me, which is then passed to the routes module.

'use strict';

/*
 * Defining the Package
 */
var Module = require('meanio').Module;

var MeanSocket = new Module('chat');

/*
 * All MEAN packages require registration
 * Dependency injection is used to define required modules
 */
MeanSocket.register(function(app, http) {

    var io = require('./server/config/socketio')(http);

    //We enable routing. By default the Package Object is passed to the routes
    MeanSocket.routes(io);

    return MeanSocket;
});

server/config/socketio.js

This file simply configures the socket.io object. Please note that I had to upgrade meanio module to version 0.5.26 for this work, as http object (express server) is not available in older meanio versions. Moreover, in case you want to use ssl, you can inject https instead of http.

'use strict';

var config = require('meanio').loadConfig(),
    cookie = require('cookie'),
    cookieParser = require('cookie-parser'),
    socketio = require('socket.io');

module.exports = function(http) {

    var io = socketio.listen(http);

    io.use(function(socket, next) {
        var data = socket.request;

        if (!data.headers.cookie) {
            return next(new Error('No cookie transmitted.'));
        }

        var parsedCookie = cookie.parse(data.headers.cookie);
        var sessionID = parsedCookie[config.sessionName];
        var parsedSessionID = cookieParser.signedCookie(parsedCookie[config.sessionName], config.sessionSecret);

        if (sessionID === parsedSessionID) {
            return next(new Error('Cookie is invalid.'));
        }

        next();
    });

    return io;
};

routes/chat.js

Finally, use the routes file to define the socket events, etc.

'use strict';

// The Package is passed automatically as first parameter
module.exports = function(MeanSocket, io) {

    io.on('connection', function(socket) {

        console.log('Client Connected');

        socket.on('authenticate', function(data, callback) {

        });
    });
};

Thoughts?

gulsharan commented 10 years ago

I can create a PR if you think this solution looks good.

liorkesos commented 10 years ago

Currently the package is broken with the latest mean so if your PR works it will be a great step forward. Also hosting behind the same port was a major drawback for adopting the package. So I will be happy to test Lior

On Mon, Nov 3, 2014 at 10:51 PM, gulsharan notifications@github.com wrote:

I can create a PR if you think this solution looks good.

— Reply to this email directly or view it on GitHub https://github.com/linnovate/mean-socket/issues/5#issuecomment-61546250.

Lior Kesos - http://www.linnovate.net Linnovate - Community Infrastructure Care mail: lior@linnovate.net office: +972 722500881 cell: +972 524305252 skype: liorkesos

gulsharan commented 10 years ago

Okay, sounds good. I'll try it with the latest mean, and send a PR if it all works fine.

liorkesos commented 10 years ago

oh one second. We use a different package now called socket and hosted on https://git.mean.io/linnovate/socket All of the packages installed come from git.mean.io and the code is maaged there This lets us hav all of the packages in one repository (like apt-get) Please look at the situation of socket, and try to apply it there. Lior

On Mon, Nov 3, 2014 at 11:50 PM, gulsharan notifications@github.com wrote:

Okay, sounds good. I'll try it with the latest mean, and send a PR if it all works fine.

— Reply to this email directly or view it on GitHub https://github.com/linnovate/mean-socket/issues/5#issuecomment-61555483.

Lior Kesos - http://www.linnovate.net Linnovate - Community Infrastructure Care mail: lior@linnovate.net office: +972 722500881 cell: +972 524305252 skype: liorkesos

gulsharan commented 10 years ago

okay, but how do I fork that package? The link you gave me looks like a custom site. Is it hosted on github's infrastructure?

gulsharan commented 10 years ago

ah! never mind .. it's gitlab .. signing up now..

gulsharan commented 10 years ago

Hey @liorkesos

I've submitted a merge request on gitlab. Here's the link: https://git.mean.io/linnovate/socket/merge_requests/1

Hope you like it.

liorkesos commented 10 years ago

Will definitely check it out. Lior

On Tue, Nov 4, 2014 at 4:45 AM, gulsharan notifications@github.com wrote:

Hey @liorkesos https://github.com/liorkesos

I've submitted a merge request on gitlab. Here's the link: https://git.mean.io/linnovate/socket/merge_requests/1

Hope you like it.

— Reply to this email directly or view it on GitHub https://github.com/linnovate/mean-socket/issues/5#issuecomment-61585884.

Lior Kesos - http://www.linnovate.net Linnovate - Community Infrastructure Care mail: lior@linnovate.net office: +972 722500881 cell: +972 524305252 skype: liorkesos