3
\$\begingroup\$

I've been writing a node.js web crawler, to get it running, I've found myself having to string together really rather a lot of different NPM modules.

I've done my best to keep the code DRY and well designed, but unfortunately, it's turned into a bit of a tangled mess, and in places I feel like I'm forced to use global variables to communicate between different functions, and that makes me very uncomfortable.

I'd really appreciate advice on how to structure this program more sensibly. Since I've spent hours trying to refactor it, and I've hit a plateau that my limited skills can't get me over. I can tell this is bad, but not how to improve it.

/* Modules */
//Load all imported modules
var Crawler = require("crawler").Crawler;
var Redis = require("redis");
var _ = require("underscore");
var url = require("url");
var express =require("express");
var app = express();
var httpServer = require('http').createServer(app);
var io = require('socket.io').listen(httpServer);
var robots = require('robots');

//Define inline - i.e. custom - modules

function CreateDataStore(storePort, storeUrl, passwd) {
    var client;
    //create the redis client
    client = Redis.createClient(storePort, storeUrl);

    //Set client password and create logging function for on connect event.
    client.auth(passwd, function(err, msg) {
        if (err) {
            console.log("redis-error: " + err);
        }
        console.log("redis: " + msg);
        console.log("redis: Connected");
    });



    function createSiteUpdater(site) {

        var testResults = {};

        function CreateUpdaterFunction(redisKey, redisCommand) {
            /*This function creates a property on the dataStore object
            that regularly updates itself with state of that key on the redis
            server. 

            These keys can then later be queried by the view layer to see the state
            of the crawl. It requires two arguments, redisKey (string) which is
            the key that we're queriying in the database, and redisCommand (string)
            which is the name of the function on the client object which runs the
            query. This should just be the name of the redis command you want to use */

            var updaterFunction = function() {
                if (typeof redisKey === "string" && typeof redisCommand === "string") {
                    client[redisCommand](redisKey, function(err, res) {
                        testResults[redisKey] = res;
                    });
                }
            };

            setInterval(updaterFunction, 500); // set regular pulse updating the variable
        }


        if (site) site = url.parse(site).hostname+":";
        console.log("Site post-parse is", site);

        for (var i = 0; i < analyticsPackages.length; i++) {
            CreateUpdaterFunction(site + "has" + analyticsPackages[i].name, "smembers");
            CreateUpdaterFunction(site + "no" + analyticsPackages[i].name, "smembers");
            CreateUpdaterFunction(site + "crawledPages", "smembers");
            console.log("Test Results Object Created for ", site, ":", analyticsPackages[i].name, testResults);
        }

        return testResults;
    }

    //exports
    return {
        "redisClient": client,
        "dataObject": createSiteUpdater("")
    };
}

function testForAnalytics(urlToTest, $, siteBeingTested) {
    var analyticsTestCases = [];

    //Constructor function for the functions which test for different analytics products
    function GenerateAnalyticsTest(testSelector, testName) {
        return function() {
            var foundAnalyticsPlatform = false;
            if ($(testSelector).length > 0) foundAnalyticsPlatform = true;
            return {
                "FoundCode": foundAnalyticsPlatform,
                "PlatformName": testName
            };
        };
    }

    //create the different analytics test cases and store them in an array
    for (var i = 0; i < analyticsPackages.length; i++) {
        analyticsTestCases.push(GenerateAnalyticsTest(analyticsPackages[i].test, analyticsPackages[i].name));
    }


    //create the function which runs the different analytics tests
    function runTest(testCase) {
        var testObject = testCase();
        if (testObject.FoundCode) {
            client.sadd(siteBeingTested + ":has" + testObject.PlatformName, urlToTest)
        }
        if (!testObject.FoundCode) {
            client.sadd(siteBeingTested + ":no" + testObject.PlatformName, urlToTest)
        }
    }

    //run the testing function for every test in the test cases array.
    for (var j = 0; j < analyticsTestCases.length; j++) {
        runTest(analyticsTestCases[j]);
    }
}



function runCrawler(urlToTest) {

    //Find the local robots.txt and parse it.    
    var changingUrl = url.parse(urlToTest);
    client.sadd("sitesCrawled",changingUrl.hostname);
    changingUrl.pathname = "/robots.txt";
    var robotsTxtUrl = url.format(changingUrl);
    console.log(robotsTxtUrl);
    var robotsParser = new robots.RobotsParser(
    robotsTxtUrl, 'Mozilla/5.0 (compatible; RobotTxtBot/1.0)',
    after_parse);




    //Once the robots text has been parsed, set up the crawler.
    function after_parse(parser, success) {
        console.log("Robots.txt loaded");
        if (success) {
            console.log("Robots.txt Parsed Successfully");
        }
        //Set up Crawler's item fetched callback. This callback is where all the actual business logic goes.
        function runOnPageCrawled(error, responseBuffer, $) {

            //Log any crawl errors
            if (error) {
                console.log("Page Crawl Error: " + error);
            }

            // Do something with the data in responseBuffer
            var thisPage = url.parse(responseBuffer.uri);
            console.log("Crawled Page: ",thisPage.href);
            testResults.currentlyCrawling = thisPage.href;

            //Payload
            testForAnalytics(thisPage.href, $, thisPage.hostname);

            //Admin to be done before we quit
            client.sadd("crawledPages", thisPage.href); //add this url to the list of pages we've crawled

            //add next pages to crawl queue
            $("body a").each(function(index, a) { // For each anchor tag in the body
                var thisLink = url.parse(a.href); //parse the link into a url
                if (success) { //if we have successfully parsed a robots.txt, 
                    if (thisLink.hostname === thisPage.hostname && parser.canFetchSync("*", thisLink.href)) { //check if we can crawl this link
                        tagCrawler.queue(thisLink.href); //add the link to the crawl queue
                    }
                }
                else { //if there is no robots.txt, then assume we can crawl this link
                    if (thisLink.hostname === thisPage.hostname) {
                        tagCrawler.queue(thisLink.href); //add the link to the crawl queue
                    }
                }
            });
        }


        //Create a crawler instance to crawl this site
        var tagCrawler = new Crawler({
            "forceUTF8": true,
            "callback": runOnPageCrawled,
            "skipDuplicates": true
        });

        tagCrawler.queue(urlToTest);

    }


}


/* Configuration */
var analyticsPackages = []; // this object is read by the analytics test module and the datastore creation module.
var Omniture = {
    "name": "Omniture",
    "test": "script:contains('s_code=s.t')"
};
analyticsPackages.push(Omniture);
var GoogleAnalytics = {
    "name": "GoogleAnalytics",
    "test": "script:contains('.google-analytics.com/ga.js')"
};
analyticsPackages.push(GoogleAnalytics);

/* Initialisation */

//Initialise Crawler object and Redis connection
var dataStore = CreateDataStore(6379, "redis.myDomain.com", "myRedisPassword");
console.log("Data Store initialised = ", dataStore);
var client = dataStore.redisClient; //this object is the redis client, and will be called to query redis;
console.log("Redis client initialised");
var testResults = dataStore.dataObject; //this object holds the current results of the scan, and is passed to the client.
console.log("Test Results Object initialised = ", testResults);

//Set up Express.js http server
//This is only used to serve the static site content
app.enable('trust proxy');
app.use(express.static(__dirname + '/static'));
app.use(app.router);



//Configure and initialise Socket.io
//Almost all user modifiable functions of the app are controlled via the socket.io handler.
io.enable('browser client minification');  // send minified client
io.enable('browser client etag');          // apply etag caching logic based on version number
io.enable('browser client gzip');          // gzip the file
io.set('log level', 1);                    // reduce logging
io.sockets.on('connection', function(socket) {
        socket.emit('message',testResults);
        socket.on('datarequest', function(res) {
            console.log("Received Update Request From Client");
            socket.emit('message', testResults);
        });
        socket.once("startCrawl",function(res){
            console.log("Recieved Crawl request for",res);
            if(_.isString(res) && res === "http:/myDomain.com"){ //temporary validation to prevent client side misuse;
            runCrawler(res);
            }
        });
});


/* Activation */
//Actually run the crawler webserver and sockets
httpServer.listen(process.env.PORT || 5000);
\$\endgroup\$
  • 2
    \$\begingroup\$ It seems to me there could be at least 3 different files: a data store, an analytics tester, and a crawler. I'm pretty sure the socket handling could be in another file too. \$\endgroup\$ – Florian Margaine Apr 18 '13 at 10:20
2
\$\begingroup\$

Isaacs, node.js current maintainer, recently wrote node.js' philosophy in this blog post: http://blog.izs.me/post/48281998870/unix-philosophy-and-node-js

In Node, the basic building block that people share and interact with is not a binary on the command line, but rather a module loaded in by require().

Use files. Use modules. Don't let your files grow. 100 lines is already too much.

Clearly, in your code, there is at least 3 entities that could be separated in their own module:

  • A data store
  • An analytics tester
  • A crawler
  • Eventually, a socket handler

I'd understand that you want to keep it all in a module, but you should at least split it up into different files, and use require() to your heart's content.

\$\endgroup\$
  • \$\begingroup\$ I've literally just read that blog post an hour ago, but failed to mentally apply it to my own code base. +1 for insight. \$\endgroup\$ – Racheet Apr 19 '13 at 15:47
  • \$\begingroup\$ @Racheet Just from a quick look, your code can be even more split up. Your redis client in the data store could be in another file too. The robots.txt parser could be in another file. Etc, etc. You just refactor till you feel satisfied :) \$\endgroup\$ – Florian Margaine Apr 19 '13 at 20:35

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.