Regression: Convert session.inc to the new database API (and _sess_write should use a merge)

We can't remove the COOKIE-test for performance reasons. That little check buys us a ton of performance on big sites.

Damien: ah, yes, you're right. Committed to CVS HEAD. Thanks!

This broke Blog API, Path, XML-RPC and Actions configuration tests.

Here's a revert.

http://ca.tchpole.net/node/3 :( :( :(

Without debug code. Trying to figure this out with cwgordon yesterday we got as far as simpletest finding drupal_set_messages which had already appeared being displayed again after a drupalGet.

patch was missing here it is.

Running all tests, some exceptions:

- Query altering tests, part 2: 20 passes, 0 fails, 1 exception
- DBLog functionality: 309 passes, 0 fails, 2 exceptions

Fails on upload - maybe my setup?

Upload functionality: 124 passes, 3 fails, 0 exceptions

The upload tests fail for me with or without the patch (but pass for webchick). I think a rollback (i.e. commit the patch above) of this until it can be fixed to not break tests is the right way forward for the moment.

Ok. Reverted the reversion to the reverted stuff. Hopefully all tests should pass now.

really back to CNW - the patch is part of the follow-on DB conversion, but needs to be re-worked to find why tests broke.

Also, I now get upload passing - I needed to delete files/simpletest when re-installing HEAD since some test files were missing and not created due to the logic in simpletest_install().

retitling.

@Damien - please clarify. The commit of #2 was reverted by the patch above, so #2 needs to be revised such that it doesn't cause random test breakage (or combined with another issue).

@Damien - ok, then the patch in #2 should be combined with #259623: Broken autoloader: convert includes/requires to use absolute paths in one patch.

Marking postponed until the autoloader issue is fixed.

subscribe

If we're going to update all the queries, let's just do a full DBTNG conversion on session.inc and be done with it. :-)

Delete queries should use the new query builder. db_fetch_*() is also now deprecated in favor of foreach(). db_result() is also deprecated in favor of fetchField().

I think we seem to be establishing a convention of using fetchObject() explicitly rather than just fetch() when directly fetching a record, as that is more self-documenting. Chained query builder calls should also be broken across multiple lines if there are 2 or more chained methods. It looks like a REQUEST_TIME is also being reverted to time(). :-(

Line 86, use fetchObject() rather than fetch() as it is more self-documenting.

Line 95, that's the sort of slick stuff I was hoping to enable with DBTNG. :-)

Line 141, use REQUEST_TIME, not time(), as it's a tiny bit faster. Same on line 149.

Lines 144, 149, 161, those built queries should be broken out across multiple lines.

Line 254, I don't understand what the () are for. They can be removed.

Should be easy to fix up. Let's get this in!

Not quite OK yet: time() should be REQUEST_TIME.

#32 applies, session tests all pass, and I can't find anything else to complain about with the coding style. Thanks, Damien!

Excellent. Committed to CVS HEAD. Thanks!

Sorry, folks. CVS annotate revealed this old issue.

+++ includes/xmlrpc.inc	31 Aug 2008 09:30:07 -0000
@@ -342,7 +342,7 @@ EOD;
-function xmlrpc_error($code = NULL, $message = NULL, $reset = FALSE) {
+function xmlrpc_error($code = NULL, $message = NULL) {

@@ -351,9 +351,6 @@ function xmlrpc_error($code = NULL, $mes
-  elseif ($reset) {
-    $xmlrpc_error = NULL;
-  }

@@ -430,7 +427,6 @@ function xmlrpc_base64_get_xml($xmlrpc_b
 function _xmlrpc() {
...
-  xmlrpc_clear_error();

@@ -479,10 +475,3 @@ function xmlrpc_error_msg() {
-/**
- * Clears any previous error.
- */
-function xmlrpc_clear_error() {
-  xmlrpc_error(NULL, NULL, TRUE);
-}

These changes should not have been in this patch. They revert the fix from #208270: XML-RPC error handling broken. This is a regression to D6.

i don't see that xmlrpc code in the patches here.

@see #11, the first patch that was committed

The change in #36 was incorrectly rolled back, it seems. That change was required to make the XML-RPC error handling work. It looks like that patch was accidentally reverted. Because it is accidentally reverted, XML-RPC errors are never reset, which makes it pretty unreasonable to do more than one XML-RPC request per HTTP request. Clearly, we need to fix that. See also #578470: XML-RPC error handling sometimes fails silently.

#36 was committed, it seems.