Alternative nodejs implementation

Thanks for your reply. I agree with all of your suggestions - I've been concerned with the number of queries and user_loads all this live activity requires when not optimised. It was hard imagining all the potential scenarios where adding +1 could fail and I haven't played around enough with node.js/sockets to know how it behaves under various scenarios. Thanks for fixes and additional hooks - if I get the time I'll try to extend the audio module.

The hard-coded 'mnc_count_callback' (rather than the call_user_func) I entered was a bad stuff up during testing - I wasn't even using the notifications callback (and I haven't tried notifications yet) - but I did have the 'messages unread' option overriding the $view->build (i.e. 'the display used for unread notification messages' in the admin page) so it worked during testing (but with my stuff up I would have missed the performance advantage of the mnc cache table).

That 'nodejs_presence' table is great and I can confirm it functions normally on my setup (I haven't double checked yet to see if it relies on sub-modules but probably not).

Regarding the js file I wasn't too sure how it would have worked but I haven't tested it yet or maybe not thought it through enough. After getCount() is called, if the client's cache says the count is expired (or doesn't exist etc) then it would perform an AJAX call to fetch the unread count (if notifications are activated or cache expiry set to 0 it would fetch the same slow view that I was sending, defeating the performance benefit) and then add 1 to it. So would the count end up being be 1 too high (and was this getCount() a mistake)? Even if this worked, if I set the client-side cache life time to say, 30 seconds (instead of 0 as I have it now), and I receive 3 messages within 30 seconds, would that mean that the count won't ever be updated despite the node.js alerts (since I've turned off AJAX polling/refresh since that was the intent of node.js)?

Drupal.Nodejs.callbacks.mnc_nodejs_counter_add = {
  callback: function (message) {
    if (message.callback == 'mnc_nodejs_counter_add') {
      var count = Drupal.message_notification_center.getCount();
      count = count + 1;
      if (count >= 0) {
        Drupal.message_notification_center.setCount(count);
      }
    }
  }
};

To take advantage of the cache table and avoid redundancy, originally I was going to simply add a hook into the existing mnc_message_insert function and send the cached count and uid directly to the client's channel (if messages_unread is set), but this requires the cache to already be set. It appears the view query will be run eventually regardless if the cache is empty - the only thing we can potentially control or defer is when it happens (which could be if present in nodejs_presence table (which should already be cached on initial page load), queuing cron jobs if benefiting the server/database setup etc?).

[Note that I may be confused since I'm not considering or experienced with all the scenarios mnc intends to support, or when the hooks are made].

One other thing in the .module is that $nodejs_message->channel = 'nodejs_user_' . $account->uid; doesn't exist on my setup. Unless I can't find it, variable "nodejs_enable_userchannel" that is looked for in nodejs_nodejs_user_channels($account) in nodejs.module is never even created by my installed version of the nodejs module (and at least doesn't exist in my 'variable' table).

/**
 * Implements hook_nodejs_user_channels().
 */
function nodejs_nodejs_user_channels($account) {
  if (variable_get('nodejs_enable_userchannel', TRUE) && $account->uid) {
    return array('nodejs_user_' . $account->uid);
  }
  return array();
}

Thanks again!

Hmm...regarding the nodejs_presence table I take that back. It doesn't appear to update consistently on my setup. I'll have to look into it.

Hi,

Sorry I couldn't get back sooner. I believe I solved the bug regarding the nodejs_presence table not being updated but I don't know if it's specific to only some setups (https://drupal.org/node/1823254#comment-8770211). I can also confirm my user channels were loading normally after all, although I couldn't find the configurable setting. In fact, re-reading variable_get('nodejs_enable_userchannel', TRUE) my absent variable defaults to TRUE anyway (but I missed it since unlike content token channels these channels (and broadcasts) don't log to the node.js console unless debug mode is on).

Thanks for the elaboration. I understand your intention with the count + 1 and maintaining AJAX refresh as a disconnection backup but I thought I could consider some scenarios with the current code and maybe an alternative.

To make sure I have things right, this is how I understand the progression of events flow:

(1) Client connects. The mnc attach function will run Drupal.mnc.setCount(settings.mnc.unread_count) if the typeof settings.mnc.unread_count != "undefined" and the anonymous function upon page load will load Drupal.mnc.updateCountInterval at the designated refresh interval (if > 0, via one of two methods - exploiting localstorage or fixed polling), which with Modernizr will set the expiry timestamp for the unread_count at every interval.

Just a long side note: It may be better to load the initial unread_count through the theme to avoid depending on javascript to update the number on screen. We can separate the js settings unread count function fetching in the backend into 2 functions, where the new reuseable function exclusively fetches and caches in memory the initial unread count. For example,

function mnc_add_js_setting_unread_count($ajax = FALSE) {
  $setting['mnc']['unread_count'] = mnc_intial_page_unread_count();
  
  if ($ajax) {
    $commands[] = ajax_command_settings($setting, TRUE);
    return $commands;
  }
  else {
    drupal_add_js($setting, 'setting');
  }
}

/**
 * Obtains the unread count upon initial page load functions
 
 * @return string
 *   Unread count
 */
function mnc_intial_page_unread_count() {
  global $user;
  $callback = variable_get_value('mnc_count_callback');
  $initial_unread_count = &drupal_static(__FUNCTION__);
  if (!isset($initial_unread_count)) {
		$initial_unread_count = call_user_func($callback, $user);
	}
	return $initial_unread_count;
}

This new function can be called by the mnc_theme variables array to print an $initial_unread_count variable to the relevant in the mnc.tpl.php template.

(2) Let's say we want a 20 minute expiry and we're using Modernizr.localstorage (with a 1 minute refresh or whatever less than 20). The initial unread count was 0.

At 5 minutes into the interval, node.js sends a counter update via callback mnc_nodejs_counter_add.

Firstly, it checks the count via getCount:

Among the expectedly true requirements in getCount mnc_valid_timestamp is not < new Date().getTime(), since 20 minutes hasn't been reached, so the cached value is returned. Since the requestUnreadCounter is never fired, the expiry date has not been updated (localStorage.setItem("mnc_notifications_cache_valid_timestamp", new Date().getTime() + expire_time); has never been run).

Then our callback will add 1 to the cached count and setCount() to count + 1. The DOM and Drupal.settings.mnc.unread_count are updated to (0 +1 = 1) , but not the timestamp.

Everything is working nicely.

(3) At 20 minutes the interval is up and the expiry is reached. The server will be contacted for a fresh value which also equals 1, and the +20 minutes expiry is started again. At the same time the last 20 minutes expired co-incidentally a new message was received so the count is updated prematurely to 2, but the asynchronous delayed AJAX callback would override this with the same value of 2. Everything still should work fine.

(4) During the next 20 minutes the client is disconnected on the train for 5 minutes where they receive another message count that isn't added. So instead of 3 they still have 2 showing. Within a minute of reconnection they receive another message so rather than showing 4 messages it will say 3 (though not catastrophic for our purposes and the value will be refreshed in ~10 minutes anyway => if they really cared about the messages they would press the MNC and get the new messages via AJAX anyway).

I realise the use of localstorage is a great solution where a user has 6 tabs open of the one website such that if the timestamp expiry were 5 minutes we would have ~12 client requests instead of 72. So the current implementation would work fine in most circumstances for the module's intended purpose.

However, I was wondering if taking advantage of socket.io's own functions could relegate AJAX refreshing as a backup function only (when MNC node.js is enabled of course by the site developer).

socket.on('disconnect', function(){}); // wait for reconnect, initiate AJAX refreshing on a setInterval (which can be set shorter, but can drain battery and/or processing speed of user) for when node.js server only fails (not client). Right now the count will reset to 0 should JSON requests fail

socket.on('reconnect', function(){}); // connection restored (client restored or node.js restored), terminate interval then requestCount once more, ready to take on plus counts again from node.js

This could remove 100s of Apache requests from idle browsers I suppose.

But I haven't tested this yet nor have I done enough research to confirm this is best practice.

Cheers

Hi,

I can see the potential benefits in AJAXifying the count display. In the long-term I might have to consider a reduced serving from my full bootstrap. My personal scenario involves a tonne of instantaneously relevant dynamically-generated content on most pages (another thing I might have to reconsider, but that's the nature of the application) so I haven't fully thought out the feasibility of full page-caching yet and pushing dynamic content to AJAX. I'd probably need to hack some modules I've installed.

Since I'm serving dynamic content via full page reloads I probably can't take advantage of a localstorage count between pages since node.js "+1" updates will fail between page loads.

My only requirement is that on slow mobile connections the first thing to show (i.e. as part of my page skeleton) is the number of private messages and notifications - like Facebook does now. This is so when mobile push notifications aren't exploited, the client doesn't need to worry about the whole page loading to know if they've been replied to.

Regarding the socket functions, I was under the impression that this module depended specifically on node.js integration module, which if I'm not mistaken relies on socket.io (https://drupal.org/node/1849552)?. I blindly installed the module as suggested in the docs so I never really considered alternate configurations.

From preliminary testing if a node.js server is turned off and reconnects to a client that kept their window open the token channel message will fail to send but a broadcast will work. I still have to try the standard node_send_message so hopefully that works.

Another issue with an alternate module is that currently if a client connects when node.js is disconnected the client will never connect via socket.io even once node.js is turned back on, so a backup is a good idea for anything essential (though to a much reduced scale since it probably would not be scalable without node.js).

Cheers

Hi,

I have a few observations I've made that I'll present in no particular order.

(1) Right now the function mnc_preprocess_views_view will flag messages as read by the person requesting the view of messages (and not the person to whom these messages were intended). If the admin decides to view the messages for a user in the backend a flag will be set in the flagging table for the admin (user 1). Since the unread messages view assumes the flagging.uid IS NULL, this will cause corruption in the counts for the viewed messages (that can only be fixed by manually clearing the flagging database. An alternative is proposed below where the logged in user id but be the same as the recipient's id to invoke flagging.

function mnc_preprocess_views_view(&$variables) {
  if ($variables['view']->name == 'mnc' && isset($variables['view']->args[0]) && preg_match('/^\d+$/', $variables['view']->args[0]) && $variables['view']->args[0] > 0) {
		$user = user_load($variables['view']->args[0]);
		if($variables['user']->uid == $variables['view']->args[0]) {
			foreach ($variables['view']->result as $message) {
			  //mnc_mark_read($variables['user'], $message->mid);
			  mnc_mark_read($user, $message->mid);
		}
	}
  }
}

(2) This might need further investigation but I believe any time a message is read or unread it is necessary to generate a new count cache on the server to avoid subsequent miscounts (which could involve simple subtraction/addition to the cache, or forcing a new view query count). If the server db cache is set anywhere from a few seconds (to a few days) there will always be a chance for subsequent miscalculations.

On this note, I believe the function mnc_message_insert has an error. It currently will only update the cache if time() > $cache->expire, but this appears incorrect - an expired cache will never be called by the mnc_count_unread function. So it should be time() < $cache->expire.

Using this same mechanism we can update mnc_mark_all_read, mnc_mark_all_unread, mnc_mark_read and mnc_mark_unread to adjust the counts in the cache directly (rather than await a view refresh).

function mnc_mark_read($user, $message_id) {
  $flag = flag_get_flag(variable_get_value('mnc_flag'))
    or die('no "' . variable_get_value('mnc_flag') . '" flag defined');
//!$flag->is_flagged was added since $flag->flag will return true even if already flagged
  if (!$flag->is_flagged($message_id, $user->uid) && $flag->flag('flag', $message_id, $user)) {
  
  	$cache_key = 'mnc_count_unread:' . $user->uid;
	$cache = cache_get($cache_key, 'cache_entity_mnc');
	//Only update count if not expired (otherwise not necessary since value will be replaced)
	if (
	  variable_get_value('mnc_cache_lifetime_count_unread') > 0 &&
	  $cache &&
	  (time() < $cache->expire)
	) {
	  $count = $cache->data;
	  $count = $count - 1;
	  if($count < 1) {
			$count = 0; //shouldn't be needed if every change accounted for but added here in case something went wrong to make it negative
	  }
	  cache_set($cache_key, $count, 'cache_entity_mnc', $cache->expire);
	}
  
	rules_invoke_event('mnc_mark_read', $user, $message_id);
	}
}

function mnc_mark_unread($user, $message_id) {
  $flag = flag_get_flag(variable_get_value('mnc_flag'))
    or die('no "' . variable_get_value('mnc_flag') . '" flag defined');
   if($flag->is_flagged($message_id, $user->uid) && $flag->flag('unflag', $message_id, $user)) {
  
    $cache_key = 'mnc_count_unread:' . $user->uid;
	$cache = cache_get($cache_key, 'cache_entity_mnc');
	if (
	  variable_get_value('mnc_cache_lifetime_count_unread') > 0 &&
	  $cache &&
	  (time() < $cache->expire)
	) {
	  $count = $cache->data;
	  $count = $count - 1;
	  cache_set($cache_key, $count, 'cache_entity_mnc', $cache->expire);
	}
  }
  
  rules_invoke_event('mnc_mark_read', $user, $message_id);
}

function mnc_mark_all_read($user, $first_page_only = FALSE) {
  $view = views_get_view(variable_get_value('mnc_view'));
  if (!$first_page_only) {
    // Mark all notifications for this user, not just the first page of results.
    $view->items_per_page = 0;
  }
  $view->set_display(variable_get_value('mnc_view_display_unread'));
  $view->set_arguments(array($user->uid));
  $view->pre_execute();
  $view->execute();
  $flag = flag_get_flag(variable_get_value('mnc_flag'))
    or die('no "' . variable_get_value('mnc_flag') . '" flag defined');
    //Update count changes for updating the database cache
    $count_delta = 0;
  foreach ($view->result as $message) {
    if (!$flag->is_flagged($message->id, $user->uid) && $flag->flag('flag', $message->mid, $user)) {
		$count_delta += -1;
	}
  }
  
	$cache_key = 'mnc_count_unread:' . $user->uid;
	$cache = cache_get($cache_key, 'cache_entity_mnc');
	//Only update count if not expired (otherwise not necessary since value will be replaced)
	if (
	  variable_get_value('mnc_cache_lifetime_count_unread') > 0 &&
	  $cache &&
	  (time() < $cache->expire)
	) {
	  $count = $cache->data;
	  //$count = $count + $count_delta;
	  //$count should be 0 anyway if no inconcsistencies - but risky to assume
	  $count = 0;
	  cache_set($cache_key, $count, 'cache_entity_mnc', $cache->expire);
	}
  
  
  rules_invoke_event('mnc_mark_all_read', $user);
  return $view;
}

function mnc_mark_all_unread($user, $first_page_only = FALSE) {
  $view = views_get_view(variable_get_value('mnc_view'));
  if (!$first_page_only) {
    // Mark all notifications for this user, not just the first page of results.
    $view->items_per_page = 0;
  }
  $view->set_display(variable_get_value('mnc_view_display'));
  $view->set_arguments(array($user->uid));
  $view->pre_execute();
  $view->execute();
  $flag = flag_get_flag(variable_get_value('mnc_flag'))
    or die('no "' . variable_get_value('mnc_flag') . '" flag defined');
     $count_delta = 0;
  foreach ($view->result as $message) {
    //We'll flag all message ids (flagged and unflagged) in the count
    if ($flag->flag('unflag', $message->mid, $user)) {
		$count_delta += 1;
		}
  }
  
  	$cache_key = 'mnc_count_unread:' . $user->uid;
	$cache = cache_get($cache_key, 'cache_entity_mnc');
	//Only update count if not expired (otherwise not necessary since value will be replaced)
	if (
	  variable_get_value('mnc_cache_lifetime_count_unread') > 0 &&
	  $cache &&
	  (time() < $cache->expire)
	) {
	  $count = $count_delta;
	  cache_set($cache_key, $count, 'cache_entity_mnc', $cache->expire);
	}

  rules_invoke_event('mnc_mark_all_unread', $user);
  return $view;
}

(3) Although not important since the module is loading the unread count via javascript to permit page caching, I'll add that I had problems with pre-caching of the mnc block (caches first user's count) when it was added to the user menu using the Menu Attach Block module (I haven't checked to see if the block was cached due to this module but I can confirm that the menu does change counts for the Private message module).

(4) Reminder to self to investigate the extended viewport plugins for the qTip module.

(5) Although only receiving potential errors in the console and not on screen from sending multiple requests, the re-attachment of Drupal.behaviors.mnc in mnc.js causes the mark all read/unread links to be tied to the same click events 3 or 4 times. This could not be overridden with the $.once() or Drupal context methods since the click events would fail when the block was re-fetched. I haven't had a chance to look heavily into it but for now I've simply added an unbinding of all click events on those elements.

			$('.mnc-inner-container .markAllRead').unbind('click').click(function(event) {
			  event.preventDefault();
			  Drupal.mnc.markAllRead();
        });

			$('.mnc-inner-container .markAllUnread').unbind('click').click(function(event) {
			  event.preventDefault();
			  Drupal.mnc.markAllUnread();
			});	

(6) Views Load More has not updated the dev version since November last year so none of the new changes are present.

(7) I can confirm that nodejs_send_message reconnects fine with the client following disconnection and node.js messages continue to work.

(8) Here's a working example of code that will fetch a fresh count from the server should the client be reconnected to node.js. This can be extended by shortening the count's expiry upon Drupal.Nodejs.socket.on('disconnect', ...), perhaps using another stored customisable variable, and restoring the original expiry upon 'reconnect'. Using this method and the above mentioned changes can allow much longer expiry times for the client and server side as defaults.

Drupal.behaviors.mncSocketReconnect = {
		attach: function(context, settings) {
			if(typeof(Drupal.Nodejs.socket.on) === "function") {
				mnc_same_page = true;
				    $(window).on('beforeunload', function(){
						mnc_same_page = false; //don't try to update count when page leaving and socket.io reconnects - I can force disconnection beforeunload too?
				    });
				Drupal.Nodejs.socket.on('reconnect', function(){
					if(typeof(Drupal.mnc.getCount) != undefined && typeof(Drupal.mnc.setCount) != undefined && mnc_same_page) {
						var count = Drupal.mnc.getCount();
						Drupal.mnc.setCount(count);
						console.log('Socket reconnected - updating Notification count');
	  			    		} 
					});	
				

				} else {
					console.log('Socket.io could not be setup for notifications');
				}
		}
};

(9) Drupal.Nodejs.callbacks.mnc_nodejs_counter_add can cause 1 extra to be added to the client side count. It adds 1 regardless of whether the count was obtained from the client side or not. This is even possible if the Modernizr refresh is set to <5 seconds as the window of opportunity is still there and the situation won't be rectified until the next requestUnreadCounter() (that isn't initiated by a node.js call). One solution is to not call getCount if the unread_count variable has already been set and simply add 1 to the existing unread_count:

Drupal.Nodejs.callbacks.mnc_nodejs_counter_add = {
  callback: function (message) {
    if (message.callback == 'mnc_nodejs_counter_add' && typeof Drupal.mnc.getCount === "function" && typeof Drupal.mnc.setCount === "function") {
		if (typeof Drupal.settings.mnc.unread_count != undefined) {
			var count = Drupal.settings.mnc.unread_count;
			count = count + 1;
		} else {
			var count = Drupal.mnc.getCount();
		}
      if (count >= 0) {
        Drupal.mnc.setCount(count);
      }
    }
  }
};

By the way, eventually the callback function above will have to be camelCased to conform with Drupal JS coding standards. I've fixed a few of my mistakes like typeof parentheses that don't conform but I'm not generally familiar with all the standards in Drupal JS (https://drupal.org/node/172169) and PHP (https://drupal.org/coding-standards).

Hopefully I didn't forget anything or make any coding mistakes (other than performance mistakes :)).

Cheers