(ok, obtuse title, but see the example code)

function path_redirect_save($edit) {
  $edit += array(
    'type' => variable_get('path_redirect_default_status', 301),
    'query' => '',
    'fragment' => '',
  );

  if (!empty($edit['rid'])) {
    $return = db_query("UPDATE {path_redirect} SET path = '%s', redirect = '%s', query = '%s', fragment = '%s', type = %d WHERE rid = %d", $edit['path'], $edit['redirect'], $edit['query'], $edit['fragment'], $edit['type'], $edit['rid']);
  }
  else {
    $return = db_query("INSERT INTO {path_redirect} (path, redirect, query, fragment, type) VALUES ('%s', '%s', '%s', '%s', '%s')", $edit['path'], $edit['redirect'], $edit['query'], $edit['fragment'], $edit['type']);
  }
  return $return;
}

To use that to do an update I need to do a select on the database before I call it.

vs.

function path_redirect_save($edit) {
  $edit += array(
    'type' => variable_get('path_redirect_default_status', 301),
    'query' => '',
    'fragment' => '',
  );

  $return = db_query("UPDATE {path_redirect} SET path = '%s', redirect = '%s', query = '%s', fragment = '%s', type = %d WHERE rid = %d", $edit['path'], $edit['redirect'], $edit['query'], $edit['fragment'], $edit['type'], $edit['rid']);
  if (!db_affected_rows()) {
    $return = db_query("INSERT INTO {path_redirect} (path, redirect, query, fragment, type) VALUES ('%s', '%s', '%s', '%s', '%s')", $edit['path'], $edit['redirect'], $edit['query'], $edit['fragment'], $edit['type']);
  }
  return $return;
}

The second version reduces the chances of a race condition and improves the DX. It's a pattern that's very common in core.

Comments

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
Status: Active » Closed (won't fix)

The latest 6.x-1.x code actually makes things even better by using drupal_write_record:

function path_redirect_save($edit) {
  $edit += array(
    'type' => variable_get('path_redirect_default_status', 301),
    'last_used' => $_SERVER['REQUEST_TIME'],
  );

  return drupal_write_record('path_redirect', $edit, (!empty($edit['rid']) ? 'rid' : array()));
}

So I'm gunna go ahead and mark this as won't fix since there's no "already implemented" status. :) Thanks for the code review though, if you have more suggestions let me know!