Need better sid generation in UserBlocksUnitTests->testWhosOnlineBlock()

andypost - June 26, 2009 - 21:48
Project:Drupal
Version:7.x-dev
Component:user system
Category:bug report
Priority:normal
Assigned:boombatower
Status:closed
Issue tags:PIFR 2.x blocker
Description

sid is primary key so mostly md5(microtime()) is the same and second insert failed with

'UserBlocksUnitTests', 'exception',
'SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry ''3886465a51d1dfce61c06aa04f332ecd'' for key 1',
'Uncaught exception', 'PDOStatement->execute()', 1691,
'/home/drupaltesting/web/public/sites/drupaltesting.deekayen.net/files/checkout/includes/database/database.inc'

This happens in modules\user\user.test

<?php
    $this
->insertSession();
   
$this->insertSession();
...
  private function
insertSession(array $fields = array()) {
   
$fields += array(
     
'uid' => 0,
     
'sid' => md5(microtime()),
     
'timestamp' => REQUEST_TIME,
    );
?>

#1

andypost - June 26, 2009 - 21:57

I cant understand a logic of test.

<?php
  $schema
['sessions'] = array(
...
     
'sid' => array(
       
'description' => "Primary key: A session ID. The value is generated by PHP's Session API.",
       
'type' => 'varchar',
       
'length' => 64,
       
'not null' => TRUE,
       
'default' => '',
      ),
...
   
'primary key' => array('sid'),
?>

#2

andypost - June 26, 2009 - 22:00
Title:Test seems broken» Logic inside UserBlocksUnitTests->testWhosOnlineBlock() out of DB-constraints

More descriptive title

modules\user\user.test testWhosOnlineBlock()

#3

Damien Tournoud - June 26, 2009 - 22:43

On which OS / hardware are you running this test?

I agree that inserting rows into the database is really not a good method for testing the who's online block... it will fail for non DB session.inc, and session.inc is supposed to remain pluggable.

#4

andypost - June 27, 2009 - 00:47

Stable error when using php-apc - on debian 5 (apache2 php 5.2.6)

#5

deekayen - June 27, 2009 - 02:03

I got the error on Ubuntu 8.04 Hardy with eaccelerator 0.9.5.3 and PHP 5.2.4 Suhosin.

#6

andypost - June 29, 2009 - 00:13

Suppose using md5(microtime()) is not a good random generator mainly.

#7

boombatower - July 9, 2009 - 19:45
Title:Logic inside UserBlocksUnitTests->testWhosOnlineBlock() out of DB-constraints» Need better sid generation in UserBlocksUnitTests->testWhosOnlineBlock()
Component:mysql database» user system
Assigned to:Anonymous» boombatower
Status:active» needs review

Test still passes locally and would seem to solve problem.

AttachmentSizeStatusTest resultOperations
503496-sid.patch697 bytesIdleUnable to apply patch 503496-sid.patchView details | Re-test

#8

Dave Reid - July 9, 2009 - 19:45
Title:Need better sid generation in UserBlocksUnitTests->testWhosOnlineBlock()» Logic inside UserBlocksUnitTests->testWhosOnlineBlock() out of DB-constraints
Component:user system» mysql database
Assigned to:boombatower» Anonymous
Status:needs review» active

I think this was my test. :) Let's change it to md5(mt_rand()) and see if that fixes it.

#9

boombatower - July 9, 2009 - 19:46
Title:Logic inside UserBlocksUnitTests->testWhosOnlineBlock() out of DB-constraints» Need better sid generation in UserBlocksUnitTests->testWhosOnlineBlock()
Component:mysql database» user system
Assigned to:Anonymous» boombatower
Status:active» needs review

Fix cross-post.

#10

andypost - July 24, 2009 - 00:53

Looks good for me. Need some reviews except mine...

#11

deekayen - July 25, 2009 - 21:25

How about using the PHP function actually intended for unique IDs: md5(uniqid(mt_rand(), true))

AttachmentSizeStatusTest resultOperations
503496_uniqid.patch591 bytesIdleUnable to apply patch 503496_uniqid.patchView details | Re-test

#12

boombatower - July 25, 2009 - 22:29
Status:needs review» reviewed & tested by the community

Looks great. Lets not hold up PIFR any longer, eh?

#13

Dries - July 28, 2009 - 19:14
Status:reviewed & tested by the community» fixed

Committed to CVS HEAD. Thanks!

#14

System Message - August 11, 2009 - 19:20
Status:fixed» closed

Automatically closed -- issue fixed for 2 weeks with no activity.

 
 

Drupal is a registered trademark of Dries Buytaert.