r15832 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r15831‎ | r15832 | r15833 >
Date:14:58, 26 July 2006
Author:evan
Status:old
Tags:
Comment:
Added a test script for the MemcStore.php class, and debugged the
class based on the results. Two tests still come up incorrectly,
but... I think the tests are wrong!

Also, added a few lines to the TODO file based on feedback from
Jonathan Daugherty of JanRain, Inc.
Modified paths:
  • /trunk/extensions/OpenID/MemcStore.php (modified) (history)
  • /trunk/extensions/OpenID/TODO (modified) (history)
  • /trunk/extensions/OpenID/testMemcStore.php (added) (history)

Diff [purge]

Index: trunk/extensions/OpenID/MemcStore.php
——@@ -82,19 +82,23 @@
8383 $handles = $this->_getHandles($server_url);
8484 if (array_key_exists($handle, $handles)) {
8585 unset($handles[$handle]);
 86+ $this->_setHandles($server_url, $handles);
8687 }
87 - $this->_setHandles($server_url, $handles);
8888
8989 # Now, delete the association record
9090 $k = $this->_associationKey($server_url, $handle);
9191 $v = $wgMemc->get($k);
92 - $res = $wgMemc->delete($k);
93 - return ($v === false || strlen($v) == 0);
 92+
 93+ if ($v === false || strlen($v) == 0) {
 94+ return false;
 95+ } else {
 96+ $res = $wgMemc->delete($k);
 97+ return true;
 98+ }
9499 }
95100
96101 function storeNonce($nonce)
97102 {
98 - global $wgMemc;
99103 $nonces = $this->_getNonces();
100104 $nonces[$nonce] = time() + MEMCSTORE_NONCE_EXPIRY;
101105 $this->_setNonces($nonces);
——@@ -172,17 +176,17 @@
173177 }
174178
175179 function _getBestAssociation($server_url) {
176 - global $wgMemc;
177180 $handles = $this->_getHandles($server_url);
178181 $maxexp = time();
179182 $besth = null;
180183 foreach ($handles as $handle => $expiry) {
181 - if ($expire > $maxexp) {
 184+ if ($expiry > $maxexp) {
182185 $besth = $handle;
 186+ $maxexp = $expiry;
183187 }
184188 }
185189 if (!is_null($besth)) {
186 - return $this->getKnownAssociation($server_url, $besth);
 190+ return $this->_getKnownAssociation($server_url, $besth);
187191 } else {
188192 return null;
189193 }
Index: trunk/extensions/OpenID/testMemcStore.php
——@@ -0,0 +1,314 @@
 2+<?php
 3+/**
 4+ * testMemcStore.php -- Command-line test tool for MemcStore
 5+ * Copyright 2006 Internet Brands (http://www.internetbrands.com/)
 6+ * By Evan Prodromou <evan@wikitravel.org>
 7+ *
 8+ * This program is free software; you can redistribute it and/or modify
 9+ * it under the terms of the GNU General Public License as published by
 10+ * the Free Software Foundation; either version 2 of the License, or
 11+ * (at your option) any later version.
 12+ *
 13+ * This program is distributed in the hope that it will be useful,
 14+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
 15+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
 16+ * GNU General Public License for more details.
 17+ *
 18+ * You should have received a copy of the GNU General Public License
 19+ * along with this program; if not, write to the Free Software
 20+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
 21+ *
 22+ * @author Evan Prodromou <evan@wikitravel.org>
 23+ * @package MediaWiki
 24+ * @subpackage Extensions
 25+ *
 26+ * Based in part on Tests/Auth/OpenID/StoreTest.php from PHP-openid package
 27+ * From JanRain, Inc.
 28+ *
 29+ * @package OpenID
 30+ * @author JanRain, Inc. <openid@janrain.com>
 31+ * @copyright 2005 Janrain, Inc.
 32+ * @license http://www.gnu.org/copyleft/lesser.html LGPL
 33+ */
 34+
 35+/**
 36+ * To use this test file, you must have PHPUnit (the old one for
 37+ * PHP4!) installed. Copy this file to the "maintenance" subdirectory
 38+ * of your MediaWiki source directory and run it from the command line.
 39+ *
 40+ */
 41+
 42+require_once('commandLine.inc');
 43+ini_set( "include_path", "/usr/share/php:" . ini_get("include_path"));
 44+
 45+require_once("$IP/extensions/OpenID/MemcStore.php");
 46+require_once('Auth/OpenID/Association.php');
 47+require_once('Auth/OpenID/CryptUtil.php');
 48+require_once('PHPUnit.php');
 49+
 50+class Tests_OpenID_MemcStore extends PHPUnit_TestCase {
 51+
 52+ function Tests_OpenID_MemcStore($name) {
 53+ $this->PHPUnit_TestCase($name);
 54+ }
 55+
 56+ /**
 57+ * Prepares for the SQL store tests.
 58+ */
 59+ function setUp()
 60+ {
 61+ $this->letters = Auth_OpenID_letters;
 62+ $this->digits = Auth_OpenID_digits;
 63+ $this->punct = Auth_OpenID_punct;
 64+ $this->allowed_nonce = $this->letters . $this->digits;
 65+ $this->allowed_handle = $this->letters . $this->digits . $this->punct;
 66+ }
 67+
 68+ /**
 69+ * Generates a nonce value.
 70+ */
 71+ function generateNonce()
 72+ {
 73+ return Auth_OpenID_CryptUtil::randomString(8, $this->allowed_nonce);
 74+ }
 75+
 76+ /**
 77+ * Generates an association with the specified parameters.
 78+ */
 79+ function genAssoc($now, $issued = 0, $lifetime = 600)
 80+ {
 81+ $sec = Auth_OpenID_CryptUtil::randomString(20);
 82+ $hdl = Auth_OpenID_CryptUtil::randomString(128, $this->allowed_handle);
 83+ return new Auth_OpenID_Association($hdl, $sec, $now + $issued,
 84+ $lifetime, 'HMAC-SHA1');
 85+ }
 86+
 87+ /**
 88+ * @access private
 89+ */
 90+ function _checkRetrieve(&$store, $url, $handle, $expected, $name = null)
 91+ {
 92+ $retrieved_assoc = $store->getAssociation($url, $handle);
 93+ if (($expected === null) || ($store->isDumb())) {
 94+ $this->assertNull($retrieved_assoc, "Retrieved association " .
 95+ "was non-null");
 96+ } else {
 97+ if ($retrieved_assoc === null) {
 98+ $this->fail("$name: Got null when expecting " .
 99+ $expected->serialize());
 100+ } else {
 101+ $this->assertEquals($expected->serialize(),
 102+ $retrieved_assoc->serialize(), $name);
 103+ }
 104+ }
 105+ }
 106+
 107+ function _checkRemove(&$store, $url, $handle, $expected, $name = null)
 108+ {
 109+ $present = $store->removeAssociation($url, $handle);
 110+ $expectedPresent = (!$store->isDumb() && $expected);
 111+ $this->assertTrue((!$expectedPresent && !$present) ||
 112+ ($expectedPresent && $present),
 113+ $name);
 114+ }
 115+
 116+ /**
 117+ * Make sure a given store has a minimum of API compliance. Call
 118+ * this function with an empty store.
 119+ *
 120+ * Raises AssertionError if the store does not work as expected.
 121+ *
 122+ * OpenIDStore -> NoneType
 123+ */
 124+ function _testStore($store)
 125+ {
 126+
 127+ // Association functions
 128+ $now = time();
 129+
 130+ $server_url = 'http://www.myopenid.com/openid';
 131+
 132+ $assoc = $this->genAssoc($now);
 133+
 134+ $this->_checkRetrieve($store, $server_url, null, null,
 135+ 'Make sure that a missing association returns no result');
 136+
 137+ $store->storeAssociation($server_url, $assoc);
 138+ $this->_checkRetrieve($store, $server_url, null, $assoc,
 139+ 'Check that after storage, getting returns the same result');
 140+
 141+ $this->_checkRetrieve($store, $server_url, null, $assoc,
 142+ 'more than once');
 143+
 144+ $store->storeAssociation($server_url, $assoc);
 145+ $this->_checkRetrieve($store, $server_url, null, $assoc,
 146+ 'Storing more than once has no ill effect');
 147+
 148+ // Removing an association that does not exist returns not present
 149+ $this->_checkRemove($store, $server_url, $assoc->handle . 'x', false,
 150+ "Remove nonexistent association (1)");
 151+
 152+ // Removing an association that does not exist returns not present
 153+ $this->_checkRemove($store, $server_url . 'x', $assoc->handle, false,
 154+ "Remove nonexistent association (2)");
 155+
 156+ // Removing an association that is present returns present
 157+ $this->_checkRemove($store, $server_url, $assoc->handle, true,
 158+ "Remove existent association");
 159+
 160+ // but not present on subsequent calls
 161+ $this->_checkRemove($store, $server_url, $assoc->handle, false,
 162+ "Remove nonexistent association after removal");
 163+
 164+ // Put assoc back in the store
 165+ $store->storeAssociation($server_url, $assoc);
 166+
 167+ // More recent and expires after assoc
 168+ $assoc2 = $this->genAssoc($now, $issued = 1);
 169+ $store->storeAssociation($server_url, $assoc2);
 170+
 171+ $this->_checkRetrieve($store, $server_url, null, $assoc2,
 172+ 'After storing an association with a different handle, but the same $server_url, the handle with the later expiration isreturned.');
 173+
 174+ $this->_checkRetrieve($store, $server_url, $assoc->handle, $assoc,
 175+ 'We can still retrieve the older association');
 176+
 177+ $this->_checkRetrieve($store, $server_url, $assoc2->handle, $assoc2,
 178+ 'Plus we can retrieve the association with the later expiration explicitly');
 179+
 180+ $assoc3 = $this->genAssoc($now, $issued = 2, $lifetime = 100);
 181+ $store->storeAssociation($server_url, $assoc3);
 182+
 183+ // More recent issued time, so assoc3 is expected.
 184+ $this->_checkRetrieve($store, $server_url, null, $assoc3, "(1)");
 185+
 186+ $this->_checkRetrieve($store, $server_url, $assoc->handle,
 187+ $assoc, "(2)");
 188+
 189+ $this->_checkRetrieve($store, $server_url, $assoc2->handle,
 190+ $assoc2, "(3)");
 191+
 192+ $this->_checkRetrieve($store, $server_url, $assoc3->handle,
 193+ $assoc3, "(4)");
 194+
 195+ $this->_checkRemove($store, $server_url, $assoc2->handle, true, "(5)");
 196+
 197+ $this->_checkRetrieve($store, $server_url, null, $assoc3, "(6)");
 198+
 199+ $this->_checkRetrieve($store, $server_url, $assoc->handle,
 200+ $assoc, "(7)");
 201+
 202+ $this->_checkRetrieve($store, $server_url, $assoc2->handle,
 203+ null, "(8)");
 204+
 205+ $this->_checkRetrieve($store, $server_url, $assoc3->handle,
 206+ $assoc3, "(9)");
 207+
 208+ $this->_checkRemove($store, $server_url, $assoc2->handle,
 209+ false, "(10)");
 210+
 211+ $this->_checkRemove($store, $server_url, $assoc3->handle,
 212+ true, "(11)");
 213+
 214+ $this->_checkRetrieve($store, $server_url, null, $assoc, "(12)");
 215+
 216+ $this->_checkRetrieve($store, $server_url, $assoc->handle,
 217+ $assoc, "(13)");
 218+
 219+ $this->_checkRetrieve($store, $server_url, $assoc2->handle,
 220+ null, "(14)");
 221+
 222+ $this->_checkRetrieve($store, $server_url, $assoc3->handle,
 223+ null, "(15)");
 224+
 225+ $this->_checkRemove($store, $server_url, $assoc2->handle,
 226+ false, "(16)");
 227+
 228+ $this->_checkRemove($store, $server_url, $assoc->handle,
 229+ true, "(17)");
 230+
 231+ $this->_checkRemove($store, $server_url, $assoc3->handle,
 232+ false, "(18)");
 233+
 234+ $this->_checkRetrieve($store, $server_url, null, null, "(19)");
 235+
 236+ $this->_checkRetrieve($store, $server_url, $assoc->handle,
 237+ null, "(20)");
 238+
 239+ $this->_checkRetrieve($store, $server_url, $assoc2->handle,
 240+ null, "(21)");
 241+
 242+ $this->_checkRetrieve($store, $server_url,$assoc3->handle,
 243+ null, "(22)");
 244+
 245+ $this->_checkRemove($store, $server_url, $assoc2->handle,
 246+ false, "(23)");
 247+
 248+ $this->_checkRemove($store, $server_url, $assoc->handle,
 249+ false, "(24)");
 250+
 251+ $this->_checkRemove($store, $server_url, $assoc3->handle,
 252+ false, "(25)");
 253+ }
 254+
 255+ function _checkUseNonce(&$store, $nonce, $expected, $msg=null)
 256+ {
 257+ $actual = $store->useNonce($nonce);
 258+ $expected = $store->isDumb() || $expected;
 259+ $this->assertTrue(($actual && $expected) || (!$actual && !$expected),
 260+ "_checkUseNonce failed: $msg");
 261+ }
 262+
 263+ function _testNonce(&$store)
 264+ {
 265+ // Nonce functions
 266+
 267+ // Random nonce (not in store)
 268+ $nonce1 = $this->generateNonce();
 269+
 270+ // A nonce is not present by default
 271+ $this->_checkUseNonce($store, $nonce1, false, 1);
 272+
 273+ // Storing once causes useNonce to return true the first, and
 274+ // only the first, time it is called after the $store->
 275+ $store->storeNonce($nonce1);
 276+ $this->_checkUseNonce($store, $nonce1, true, 2);
 277+ $this->_checkUseNonce($store, $nonce1, false, 3);
 278+ $this->_checkUseNonce($store, $nonce1, false, 4);
 279+
 280+ // Storing twice has the same effect as storing once.
 281+ $store->storeNonce($nonce1);
 282+ $store->storeNonce($nonce1);
 283+ $this->_checkUseNonce($store, $nonce1, true, 5);
 284+ $this->_checkUseNonce($store, $nonce1, false, 6);
 285+ $this->_checkUseNonce($store, $nonce1, false, 7);
 286+
 287+ // Auth key functions
 288+
 289+ // There is no key to start with, so generate a new key and
 290+ // return it.
 291+ $key = $store->getAuthKey();
 292+
 293+ // The second time around should return the same as last time.
 294+ $key2 = $store->getAuthKey();
 295+ $this->assertEquals($key, $key2, "Auth keys differ");
 296+ $this->assertEquals(strlen($key), $store->AUTH_KEY_LEN,
 297+ "Key length not equals AUTH_KEY_LEN");
 298+ }
 299+
 300+ function testMemcStore() {
 301+ # Unique prefix for this test
 302+ $prefix = sprintf("test-%x", time());
 303+ $store = new OpenID_MemcStore('test', $prefix);
 304+ $this->_testStore($store);
 305+ $this->_testNonce($store);
 306+ }
 307+}
 308+
 309+$suite = new PHPUnit_TestSuite();
 310+$suite->addTest(new Tests_OpenID_MemcStore('testMemcStore'));
 311+
 312+$result = PHPUnit::run($suite);
 313+print $result->toString();
 314+
 315+?>
\ No newline at end of file
Index: trunk/extensions/OpenID/TODO
——@@ -27,7 +27,7 @@
2828 + change trust data separators from , and | to FS and RS
2929 - User preferences tab for OpenID
3030 - Manage allow-to-trust settings in User preferences
31 -- Optionally use OpenID URL for home page
 31+- Optionally redirect User: page to OpenID URL
3232 - If user logs in with OpenID, add a cookie, and auto-login next time
3333 + Have some $trust_root values that we _always_ allow to trust
3434 + README
——@@ -35,4 +35,11 @@
3636 + try to guess user name from URL with /
3737 like http://getopenid.com/evanprodromou or
3838 http://profile.typekey.com/EvanProdromou/
 39+- Allow specifying a FileStore rather than a MemcStore
 40+- Allow specifying a MySQLStore rather than a MemcStore
 41+- Fall back to DumbStore if Memc is bogus
 42+- Unit test for MemcStore
 43+- for just-a-hostname OpenIDs, if the first element is not
 44+ www, use the first element as a proposed ID
 45+- configurable regexps for finding a user ID from an OpenID.
3946