Convert DrupalQueue system to PSR-0

You probably going to hate me but it would be nice if we could fix all the documentation in this issue. This issue isn't blocking any other critical issue (I think/hope) so we shouldn't rush things (like with the cmi patch).

+++ b/core/lib/Drupal/Core/Queue/QueueInterface.phpundefined
@@ -0,0 +1,108 @@
+   * Claim an item in the queue for processing.

Claims and newline after @param in that docblock

+++ b/core/lib/Drupal/Core/Queue/QueueInterface.phpundefined
@@ -0,0 +1,108 @@
+   * Delete a finished item from the queue.

Deletes

+++ b/core/lib/Drupal/Core/Queue/QueueInterface.phpundefined
@@ -0,0 +1,108 @@
+  /**
+   * Release an item that the worker could not process, so another
+   * worker can come in and process it before the timeout expires.
+   *
+   * @param $item

One line summary, newline between @param and @return and a description

+++ b/core/lib/Drupal/Core/Queue/System.phpundefined
@@ -0,0 +1,102 @@
+class System implements ReliableQueueInterface {

This class has no docblocks for functions.

Other similar issues are found, but it should be trivial to fix them.

It looks like a pretty straight forward to me. I kind-of prefer the previous factory but I'm lead to believe this is the rough standard atm so that's ok and suddenly some of those class names make no sense but that's also not this patches fault :)

I don't see a problem with these trivial doc changes if that's acceptable these days. They don't confuse any history.

I'm not sure there's anything to rush though, this is very straight forward.

By rush I mean, push this patch with partially fixed doc blocks. That way we have to reopen an issue specially for the docs.

+++ b/core/includes/common.inc
@@ -7843,3 +7843,98 @@ function drupal_get_filetransfer_info() {
+function queue($name, $reliable = FALSE) {
+  static $queues;
+  if (!isset($queues[$name])) {
+    $class = variable_get('queue_class_' . $name, NULL);
+    if (!$class) {
+      $class = variable_get('queue_default_class', 'Drupal\Core\Queue\System');
+    }
+    $object = new $class($name);
+    if ($reliable && !$object instanceof ReliableQueueInterface) {
+      $class = variable_get('queue_default_reliable_class', 'Drupal\Core\Queue\System');
+      $object = new $class($name);
+    }
+    $queues[$name] = $object;
+  }
+  return $queues[$name];
+}

This may be carried over from the previous code, but this is going to double-instantiate a queue object if you need a reliable interface. Rather, we can use http://us.php.net/class_implements to determine if the class implements an interface and then not even instantiate it if not.

If that's not appropriate to fix here because it's an existing bug, then we should open up a follow-up issue to fix it. I'm fine either way.

+++ b/core/modules/simpletest/tests/queue.test
@@ -0,0 +1,92 @@
+  function testQueue() {
+    // Create two queues.
+    $queue1 = queue($this->randomName());
+    $queue1->createQueue();
+    $queue2 = queue($this->randomName());
+    $queue2->createQueue();

So strictly speaking, we should not be using the queue() static factor in unit tests. We should be instantiating the actual class directly and configuring it directly, so that we have it in isolation. That does mean separate tests for System and Memory queues, which we should have anyway because those are separate pieces of code and should be tested independently (even if with similar tests, even if with tests that are the same and have a prepared object passed to them).

Very very few core tests do that properly, sadly, but it's a practice we should start following. May as well start now. :-)

Otherwise, this looks good to me. Thanks, amateescu!

23 days to next Drupal core point release.

+++ b/core/includes/common.incundefined
@@ -7843,3 +7843,96 @@ function drupal_get_filetransfer_info() {
+ * @param $name
+ *   Arbitrary string. The name of the queue to work with.
+ * @param $reliable
+ *   TRUE if the ordering of items and guaranteeing every item executes at
+ *   least once is important, FALSE if scalability is the main concern.
+ *
+ * @return Drupal\Core\Queue\QueueInterface
+ *   The queue object for a given name.

When this is re-rolled, could we include variable type hinting here?

Also, since method getAllItems is not part of the interface, I think that our coding standards require that it get a complete docblock which includes @return directive.

+++ b/core/lib/Drupal/Core/Queue/System.phpundefined
@@ -0,0 +1,104 @@
+  /* Implementations of Drupal\Core\Queue\ReliableQueueInterface. */

We can't do this, we have to document each function.

I did this for you. Needs work for Crell his comments.

Small improvement

+++ b/core/includes/common.inc
@@ -7843,3 +7843,97 @@ function drupal_get_filetransfer_info() {
+    if ($class && $reliable && in_array('ReliableQueueInterface', class_implements($class))) {

When specifying an interface name in a string, like a class it needs to be fully qualified since that's what class_implements() should be returning.

+++ b/core/modules/simpletest/tests/queue.test
@@ -0,0 +1,120 @@
+  function queueTest($queue1, $queue2) {

Yeah, I'm being a pedant. :-) This method should be protected, since it's internal to this class. The test* methods (yay!) should be explicitly declared public. All methods should have their visibility specified.

I think I'm out of things to complain about otherwise. Nicely done. :-)

22 days to next Drupal core point release.

+++ b/core/lib/Drupal/Core/Queue/Memory.php
@@ -0,0 +1,107 @@
+  /**
+   * Counter for item ids.
+   *
+   * @var int
+   */
+  protected $id_sequence;

This should be lowerCamel case. Sorry I missed it before. :-(

That's it, then we should be done.

I can't imagine the bot having an issue with that small a change, and it will complain anyway if it does, so...

This looks great. We could do with a better name for the default reliably queue than 'System' but that problem pre-exists this patch, so committed/pushed to 8.x.

This will need a change notification - new factory added etc.

Courtesy of core office hours! http://drupal.org/node/1479680

Updated the change notice to reflect that a different factory was added not it was converted to use a factory. The documentation on the notice already showed the change.