DBTNG: User module

I'm at the gate waiting to get on my flight home from Drupalcon, so I'll claim this for something to do on board.

No kittens were harmed in the making of this 20K patch. I promise.

I could definitely use a second opinion on user_load, or at least the part where it's setting up a dynamic query so you can pass in any old array and get a user back out. Here's the original:

  foreach ($array as $key => $value) {
    if ($key == 'uid' || $key == 'status') {
      $query[] = "$key = %d";
      $params[] = $value;
    }
    elseif ($key == 'pass') {
      $query[] = "pass = '%s'";
      $params[] = $value;
    }
    else {
      $query[]= "LOWER($key) = LOWER('%s')";
      $params[] = $value;
    }
  }

I'm not 100% sure why the LOWER($key) = LOWER('%s') bit is in there -- it seems like callers could be expected to have the correct case for their database columns and values -- but I imagine there's a good reason, so in the interest of not changing the API, I left it there. Or at least I tried to. Here's the new version:

  foreach ($array as $key => $value) {
    switch ($key) {
      case 'uid':
      case 'status':
      case 'pass':
        $query->condition($key, $value);
        break;
      default:
        $query->where("LOWER($key) = LOWER(:$key)", array(":$key" => $value));
        break;
    }
  }

It's the $query->where line I'm wondering about. It works, but it seems wrong somehow. Is it?

I just saw #347250: Multiple load users is in, so the user_load question is no longer valid. I'll reroll. I think I missed a pager_query or two as well.

This patch covers everything but user_admin_account(), which is the form builder for the user administration page. It depends on user_build_filter_query() to come up with bits of dynamic SQL, which are then added to the query string. Clearly both functions need a rewrite for the new db syntax. It seems like it'll be easier to review if we keep that issue separate from this straightforward (but long) patch, so I'm marking this CNR.

code looks good. if tests still pass, should be rtbc.

Committed to CVS HEAD. Thanks K!

Seems like we are doing a bit more looping that we need to. See fetchAll method which does the loop for you (e.g. selecting roles). Also, when inserting into permissions table we are doing an insert for each permission. We should do a multi-insert by passing in an array of values ... There may be more places where we can do same.

- Fixed some coding style stuff with chained methods and so on..
- DBTNG'd filter query builder stuff
- Used multi-insert where possible
- Used fetchAllKeyed() in one case, all other instances do some special handling and can't use fetchAll()...

I shouldn't change things after running the tests :)

I noticed some curious things when I tested the filtering, it probably needs to be improved, I'm not yet sure. especially with multiple permission.

Reviewed and committed. Thanks for the follow-up patch -- it is great to see us do that as it encourages me to commit smaller chunks of DBTNG work.