Add history_read_multiple()

While on that, here's simple patch

PS: Also I'm trying to implement manager #2081585: [META] Modernize the History module

Fixed doc-block comment

+++ b/core/modules/history/history.module
@@ -41,6 +41,48 @@ function history_read($nid) {
+  $nodes_to_read = array();
+  foreach ($nids as $nid) {
+    if (isset($history[$nid])) {
+      $return[$nid] = $history[$nid];
+    }
+    else {
+      $nodes_to_read[$nid] = 0;
+    }
+  }
+
+  if (empty($nodes_to_read)) {
+    return $return;
+  }

I think using array_diff_key() here would make this much more readable. Also, from reading the code it seems because of the " = 0" the empty() check will never return TRUE. I might be mistaken, though.

The reason to use loop is to initialize array with 0 for nodes that are not been viewed by user so added comment.
So the static cache is filled with a proper return value.

Check for array emptiness could be done without empty() but this more readable.
php -r '$a = array(1 => 0); if (empty($a)) print "empty"; else print "not empty";'

Ahh, I totally missed that you're filling two different arrays, there. $return != $nodes_to_read. I get it now.

Forgot to add: Code looks quite good. I don't feel comfortable RTBCing this myself, however.

I doubt having a static cache is valuable in this case. It's extremely unlikely this is being called many times. We only use static caches for frequently called functions.

+++ b/core/modules/history/history.module
@@ -41,6 +41,49 @@ function history_read($nid) {
+      // Initialize value if current user has not been viewed the node.

s/has not been viewed/has not viewed/

Re #7: This code is only re-using the static from history_read(). That way history_read_multiple() doesn't re-fetch items that history_read() already fetched. Thinking about this I think that part could be streamlined by history_read_multiple() maintaining its own static, and history_read() simply being a one-line call to history_read_multiple(array($nid));. That would be sort of in-line with entity_load() and so forth. Also, I think we should definitely use a drupal_static() here to avoid double DB queries. Even if the chances are small. In this particular case a drupal_static() doesn't hurt, and there are efforts to move this to a HistoryManager anyway, which would store this properly in the object state.

Don't know if you guys agree with my proposal but leaving needs work for the second item in #7 at lesat.

I prefer to preserve the name of static cache as 'history_read' because there's could be a code that uses it.
And this static cache should be shared by both functions, same as #8 said

New patch incorporated suggested changes

This allows to test new function! #9 shows that we have test coverage

Looks great, thanks! But for the test fails, this looks ready to go.

Fix test failures (json need int value)
Used new function in controller

Just some minor nitpicks

1. +++ b/core/modules/history/history.module
   @@ -30,14 +30,51 @@
   + * Retrieves the timestamp for the current user's last view of a nodes.
   

   'Retrieves the last viewed timestamp for each of the passed node IDs.' perhaps?

2. +++ b/core/modules/history/history.module
   @@ -30,14 +30,51 @@
   + *   A node IDs.
   

   An array of node IDs

3. +++ b/core/modules/history/history.module
   @@ -30,14 +30,51 @@
   +    $nodes_to_read[$row->nid] = (int)$row->timestamp;
   

   (int) $row->timestamp = needs a space after the cast

1. +++ b/core/modules/history/history.module
   @@ -100,7 +101,10 @@ function history_write($nid, $account = NULL) {
   +    // Update static cache.
   +    $history = &drupal_static('history_read', array());
   +    $history[$nid] = REQUEST_TIME;
   

   Ohh, that's very savvy. Nice!

I'd re-roll for #15 but then I can't RTBC :-P

Looks good. I don't have anything to complain about.

Suppose there's no modules dealing with this static but I preserved history_read name for BC.
__FUNCTION__ should not used because logic for static cache moved to new function

So here's suggested __FUNCTION__

Nicely done — HistoryController now looks much better :)

Added https://drupal.org/node/2094029

fix tags