r92340 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r92339‎ | r92340 | r92341 >
Date:23:18, 15 July 2011
Author:salvatoreingala
Status:deferred
Tags:
Comment:
- Added timestamp to saved preferences, so that gadget preferences module may compute getModifiedTime() meaningfully
- Splitted gadget options from gadget module: Gadget's module is now 'ext.Gadget.foo' => GadgetResourceLoaderModule while gadget preferences are in 'ext.Gadget.foo.prefs' => GadgetOptionsResourceLoaderModule; the latter is in the 'private' group
- Now it loads default preferences for anonymous users.
- Moved GadgetsMainModule.php to the /backend folder
Modified paths:
  • /branches/salvatoreingala/Gadgets/Gadgets.php (modified) (history)
  • /branches/salvatoreingala/Gadgets/Gadgets_tests.php (modified) (history)
  • /branches/salvatoreingala/Gadgets/backend/Gadget.php (modified) (history)
  • /branches/salvatoreingala/Gadgets/backend/GadgetHooks.php (modified) (history)
  • /branches/salvatoreingala/Gadgets/backend/GadgetOptionsResourceLoaderModule.php (added) (history)
  • /branches/salvatoreingala/Gadgets/backend/GadgetPrefs.php (modified) (history)
  • /branches/salvatoreingala/Gadgets/backend/GadgetResourceLoaderModule.php (modified) (history)
  • /branches/salvatoreingala/Gadgets/backend/GadgetsMainModule.php (added) (history)
  • /branches/salvatoreingala/Gadgets/ui/GadgetsMainModule.php (deleted) (history)

Diff [purge]

Index: branches/salvatoreingala/Gadgets/Gadgets.php
——@@ -48,8 +48,9 @@
4949 $wgAutoloadClasses['Gadget'] = $dir . 'backend/Gadget.php';
5050 $wgAutoloadClasses['GadgetHooks'] = $dir . 'backend/GadgetHooks.php';
5151 $wgAutoloadClasses['GadgetResourceLoaderModule'] = $dir . 'backend/GadgetResourceLoaderModule.php';
 52+$wgAutoloadClasses['GadgetOptionsResourceLoaderModule'] = $dir . 'backend/GadgetOptionsResourceLoaderModule.php';
5253 $wgAutoloadClasses['GadgetPrefs'] = $dir . 'backend/GadgetPrefs.php';
53 -$wgAutoloadClasses['GadgetsMainModule'] = $dir . 'ui/GadgetsMainModule.php';
 54+$wgAutoloadClasses['GadgetsMainModule'] = $dir . 'backend/GadgetsMainModule.php';
5455 $wgAutoloadClasses['SpecialGadgets'] = $dir . 'ui/SpecialGadgets.php';
5556
5657 $wgSpecialPages['Gadgets'] = 'SpecialGadgets';
——@@ -62,7 +63,7 @@
6364 $wgAjaxExportList[] = 'GadgetsAjax::setPreferences';
6465
6566 $wgResourceModules['ext.gadgets'] = array(
66 - 'class' => 'GadgetsMainModule'
 67+ 'class' => 'GadgetsMainModule',
6768 );
6869
6970 $wgResourceModules['jquery.validate'] = array(
Index: branches/salvatoreingala/Gadgets/Gadgets_tests.php
——@@ -589,8 +589,24 @@
590590 )
591591 ) );
592592 }
593 -
 593+
594594 /**
 595+ * Tests Gadget::getDefaults
 596+ *
 597+ * @dataProvider prefsDescProvider
 598+ */
 599+ function testGetDefaults( $prefsDescription ) {
 600+ $this->assertEquals( GadgetPrefs::getDefaults( $prefsDescription ), array(
 601+ 'testBoolean' => true,
 602+ 'testBoolean2' => true,
 603+ 'testNumber' => 7,
 604+ 'testNumber2' => 7,
 605+ 'testSelect' => 3,
 606+ 'testSelect2' => 3
 607+ ) );
 608+ }
 609+
 610+ /**
595611 * Tests Gadget::setPrefsDescription, GadgetPrefs::checkPrefsAgainstDescription,
596612 * GadgetPrefs::matchPrefsWithDescription and Gadget::setPrefs.
597613 *
Index: branches/salvatoreingala/Gadgets/backend/GadgetsMainModule.php
——@@ -0,0 +1,50 @@
 2+<?php
 3+
 4+/**
 5+ * Gadgets extension - lets users select custom javascript gadgets
 6+ *
 7+ *
 8+ * For more info see http://mediawiki.org/wiki/Extension:Gadgets
 9+ *
 10+ * @file
 11+ * @ingroup Extensions
 12+ * @author Daniel Kinzler, brightbyte.de
 13+ * @copyright © 2007 Daniel Kinzler
 14+ * @license GNU General Public Licence 2.0 or later
 15+ */
 16+
 17+/**
 18+ * Class implementing the ext.gadgets module. Required by ext.gadgets.preferences.
 19+ */
 20+class GadgetsMainModule extends ResourceLoaderModule {
 21+
 22+ public function getModifiedTime( ResourceLoaderContext $context ) {
 23+ $gadgets = Gadget::loadList();
 24+
 25+ $m = 1;
 26+ $resourceLoader = $context->getResourceLoader();
 27+ foreach ( $gadgets as $gadget ) {
 28+ if ( $gadget->hasModule() ) {
 29+ $module = $resourceLoader->getModule( $gadget->getModuleName() );
 30+ $m = max( $m, $module->getModifiedTime( $context ) );
 31+ }
 32+ }
 33+ return $m;
 34+ }
 35+
 36+ public function getScript( ResourceLoaderContext $context ) {
 37+ $configurableGadgets = array();
 38+ $gadgets = Gadget::loadList();
 39+
 40+ foreach ( $gadgets as $gadget ) {
 41+ if ( $gadget->getPrefsDescription() !== null ) {
 42+ $configurableGadgets[] = $gadget->getName();
 43+ }
 44+ }
 45+
 46+ $script = "mw.gadgets = {};\n";
 47+ $script .= "mw.gadgets.info = new mw.Map();\n";
 48+ $script .= "mw.gadgets.configurableGadgets = " . Xml::encodeJsVar( $configurableGadgets ) . ";\n";
 49+ return $script;
 50+ }
 51+}
Property changes on: branches/salvatoreingala/Gadgets/backend/GadgetsMainModule.php
___________________________________________________________________
Added: svn:eol-style
152 + native
Index: branches/salvatoreingala/Gadgets/backend/Gadget.php
——@@ -22,6 +22,7 @@
2323 */
2424 const GADGET_CLASS_VERSION = 5;
2525
 26+ //Fields stored in cache
2627 private $version = self::GADGET_CLASS_VERSION,
2728 $scripts = array(),
2829 $styles = array(),
——@@ -32,9 +33,10 @@
3334 $requiredRights = array(),
3435 $onByDefault = false,
3536 $category,
36 - $mTime = null, //upper bound on last modification time; UNIX timestamp
3737 $prefsDescription = null,
38 - $preferences = null;
 38+ //The following fields are initialized after saving the gadget to cache for registered users
 39+ $preferences = null,
 40+ $prefsTimestamp = 1; //last time user preferences for this gadget have been changed; UNIX timestamp
3941
4042 /**
4143 * Creates an instance of this class from definition in MediaWiki:Gadgets-definition
——@@ -55,9 +57,6 @@
5658 $gadget = new Gadget();
5759 $gadget->name = trim( str_replace(' ', '_', $m[1] ) );
5860 $gadget->definition = $definition;
59 -
60 - //TODO: make this more precise with gadget-specific info. Untile then, 'now' is an upper bound.
61 - $gadget->mTime = wfTimestamp( TS_UNIX );
6261
6362 //Parse gadget options
6463 $options = trim( $m[2], ' []' );
——@@ -114,6 +113,9 @@
115114 if ( isset( $prefsDescriptionJson ) ) {
116115 $prefsDescription = FormatJson::decode( $prefsDescriptionJson, true );
117116 $gadget->setPrefsDescription( $prefsDescription );
 117+
 118+ //Load default gadget preferences. Only useful for anonymous users
 119+ $gadget->setPrefs( GadgetPrefs::getDefaults( $prefsDescription ) );
118120 }
119121 }
120122
——@@ -156,6 +158,13 @@
157159 }
158160
159161 /**
 162+ * @return String: Name of ResourceLoader module for this gadget's preferences
 163+ */
 164+ public function getPrefsModuleName() {
 165+ return "{$this->getModuleName()}.prefs";
 166+ }
 167+
 168+ /**
160169 * Checks whether this is an instance of an older version of this class deserialized from cache
161170 * @return Boolean
162171 */
——@@ -255,14 +264,15 @@
256265 return new GadgetResourceLoaderModule( $pages, $this->dependencies, $this );
257266 }
258267
 268+
259269 /**
260 - * Returns an upper bound on the modification time of the gadget.
261 - * Used by GadgetResourceLoaderModule to compute its own mTime.
 270+ * Returns ResourceLoader module for this gadget's preferences, see
 271+ * getPrefsModuleName().
262272 *
263 - * @return String the UNIX timestamp of this gadget's modificaton time.
 273+ * @return Mixed: GadgetOptionsResourceLoaderModule or false
264274 */
265 - public function getModifiedTime() {
266 - return $this->mTime;
 275+ public function getPrefsModule() {
 276+ return new GadgetOptionsResourceLoaderModule( $this );
267277 }
268278
269279 /**
——@@ -492,9 +502,30 @@
493503 $this->preferences = $prefs;
494504
495505 if ( $savePrefs ) {
 506+ $this->setPrefsTimestamp( wfTimestamp( TS_UNIX ) ); //update timestamp before saving
496507 $user = RequestContext::getMain()->getUser();
497508 $user->saveSettings();
498509 }
499510 return true;
500511 }
 512+
 513+ /**
 514+ * Returns the modification time of gadget preferences.
 515+ * Used by GadgetResourceLoaderModule to compute its own mTime.
 516+ * Returns 1 if setPrefsTimestamp() has never been called, or the previously stored value otherwise.
 517+ *
 518+ * @return String the UNIX timestamp of this gadget's modificaton time.
 519+ */
 520+ public function getPrefsTimestamp() {
 521+ return $this->prefsTimestamp;
 522+ }
 523+
 524+ /**
 525+ * Sets the modification time of gadget preferences.
 526+ *
 527+ * @param $timestamp String: the UNIX timestamp of the last time preferences has been saved.
 528+ */
 529+ public function setPrefsTimestamp( $timestamp ) {
 530+ $this->prefsTimestamp = $timestamp;
 531+ }
501532 }
Index: branches/salvatoreingala/Gadgets/backend/GadgetPrefs.php
——@@ -534,6 +534,20 @@
535535 }
536536
537537 /**
 538+ * Return default preferences according to the given description.
 539+ *
 540+ * @param $prefsDescription Array: reference of the array of preferences to match.
 541+ * It is assumed that $prefsDescription is a valid description of preferences.
 542+ *
 543+ * @return Array: the set of default preferences, keyed by preference name.
 544+ */
 545+ public static function getDefaults( $prefsDescription ) {
 546+ $prefs = array();
 547+ self::matchPrefsWithDescription( $prefsDescription, $prefs );
 548+ return $prefs;
 549+ }
 550+
 551+ /**
538552 * Returns true if $str should be interpreted as a message, false otherwise.
539553 *
540554 * @param $str String
Index: branches/salvatoreingala/Gadgets/backend/GadgetOptionsResourceLoaderModule.php
——@@ -0,0 +1,75 @@
 2+<?php
 3+
 4+/**
 5+ * Gadgets extension - lets users select custom javascript gadgets
 6+ *
 7+ *
 8+ * For more info see http://mediawiki.org/wiki/Extension:Gadgets
 9+ *
 10+ * @file
 11+ * @ingroup Extensions
 12+ * @author Daniel Kinzler, brightbyte.de
 13+ * @copyright © 2007 Daniel Kinzler
 14+ * @license GNU General Public Licence 2.0 or later
 15+ */
 16+
 17+/**
 18+ * Class representing the user-specific options for a gadget
 19+ */
 20+class GadgetOptionsResourceLoaderModule extends ResourceLoaderModule {
 21+ private $gadget;
 22+
 23+ /**
 24+ * Creates an instance of this class
 25+ * @param $gadget Gadget: the gadget this module is built upon.
 26+ */
 27+ public function __construct( $gadget ) {
 28+ $this->gadget = $gadget;
 29+ }
 30+
 31+ /**
 32+ * Overrides ResourceLoaderModule::getDependencies()
 33+ * @return Array: Names of resources this module depends on
 34+ */
 35+ public function getDependencies() {
 36+ return array( 'ext.gadgets' );
 37+ }
 38+
 39+ /**
 40+ * Overrides ResourceLoaderModule::getGroup()
 41+ * @return String
 42+ */
 43+ public function getGroup() {
 44+ return 'private';
 45+ }
 46+
 47+ /**
 48+ * Overrides ResourceLoaderModule::getScript()
 49+ * @param $context ResourceLoaderContext
 50+ * @return String
 51+ */
 52+ public function getScript( ResourceLoaderContext $context ) {
 53+ $gadgetInfo = array(
 54+ 'name' => $this->gadget->getName(),
 55+ 'config' => $this->gadget->getPrefs()
 56+ );
 57+ return Xml::encodeJsCall( 'mw.gadgets.info.set',
 58+ array( $this->gadget->getName(), $gadgetInfo ) );
 59+ }
 60+
 61+ /**
 62+ * Overrides ResourceLoaderModule::getModifiedTime()
 63+ * @param $context ResourceLoaderContext
 64+ * @return Integer
 65+ */
 66+ public function getModifiedTime( ResourceLoaderContext $context ) {
 67+ $prefsMTime = $this->gadget->getPrefsTimestamp();
 68+
 69+ $resourceLoader = $context->getResourceLoader();
 70+ $parentModule = $resourceLoader->getModule( $this->gadget->getModuleName() );
 71+ $gadgetMTime = $parentModule->getModifiedTime( $context );
 72+
 73+ return max( $gadgetMTime, $prefsMTime );
 74+ }
 75+}
 76+
Property changes on: branches/salvatoreingala/Gadgets/backend/GadgetOptionsResourceLoaderModule.php
___________________________________________________________________
Added: svn:eol-style
177 + native
Index: branches/salvatoreingala/Gadgets/backend/GadgetHooks.php
——@@ -148,6 +148,9 @@
149149 foreach ( $gadgets as $g ) {
150150 $module = $g->getModule();
151151 if ( $module ) {
 152+ if ( $g->getPrefsDescription() !== null ) {
 153+ $resourceLoader->register( $g->getPrefsModuleName(), $g->getPrefsModule() );
 154+ }
152155 $resourceLoader->register( $g->getModuleName(), $module );
153156 }
154157 }
——@@ -230,13 +233,15 @@
231234
232235 //Find out all existing gadget preferences and save them in a map
233236 $preferencesCache = array();
 237+ $timestampCache = array();
234238 foreach ( $options as $option => $value ) {
235239 $m = array();
236240 if ( preg_match( '/gadget-([a-zA-Z](?:[-_:.\w\d ]*[a-zA-Z0-9])?)-config/', $option, $m ) ) {
237241 $gadgetName = trim( str_replace(' ', '_', $m[1] ) );
238 - $gadgetPrefs = unserialize( $value );
239 - if ( $gadgetPrefs !== false ) {
240 - $preferencesCache[$gadgetName] = $gadgetPrefs;
 242+ $bundle = unserialize( $value );
 243+ if ( is_array( $bundle ) && isset( $bundle['timestamp'] ) && isset( $bundle['prefs'] ) ) {
 244+ $preferencesCache[$gadgetName] = $bundle['prefs'];
 245+ $timestampCache[$gadgetName] = $bundle['timestamp'];
241246 } else {
242247 //should not happen; just in case
243248 wfDebug( __METHOD__ . ": couldn't unserialize settings for gadget " .
——@@ -252,15 +257,18 @@
253258 if ( $prefsDescription !== null ) {
254259 if ( isset( $preferencesCache[$gadget->getName()] ) ) {
255260 $userPrefs = $preferencesCache[$gadget->getName()];
 261+ $timestamp = $timestampCache[$gadget->getName()];
256262 }
257263
258264 if ( !isset( $userPrefs ) ) {
259265 $userPrefs = array(); //no saved prefs (or invalid entry in DB), use defaults
 266+ $timestamp = 1;
260267 }
261 -
 268+
262269 GadgetPrefs::matchPrefsWithDescription( $prefsDescription, $userPrefs );
263270
264271 $gadget->setPrefs( $userPrefs );
 272+ $gadget->setPrefsTimestamp( $timestamp );
265273 }
266274 }
267275
——@@ -296,16 +304,19 @@
297305 $prefsDescription = $gadget->getPrefsDescription();
298306
299307 //Remove preferences that equal their default
300 - foreach ( $prefs as $prefName => $value ) {
301 - if ( $prefsDescription['fields'][$prefName]['default'] === $value ) {
 308+ foreach ( $prefsDescription['fields'] as $prefDescription ) {
 309+ $prefName = $prefDescription['name'];
 310+ $prefDefault = $prefDescription['default'];
 311+ if ( $prefs[$prefName] === $prefDefault ) {
302312 unset( $prefs[$prefName] );
303313 }
304314 }
305315
306 - //Only save it if at least one preference differs from default
307 - if ( !empty( $prefs ) ) {
308 - $options["gadget-{$gadget->getName()}-config"] = serialize( $prefs );
309 - }
 316+ //Save back preferences
 317+ $options["gadget-{$gadget->getName()}-config"] = serialize( array(
 318+ 'timestamp' => $gadget->getPrefsTimestamp(),
 319+ 'prefs' => $prefs
 320+ ) );
310321 }
311322 }
312323
Index: branches/salvatoreingala/Gadgets/backend/GadgetResourceLoaderModule.php
——@@ -28,6 +28,7 @@
2929 * 'MediaWiki:Gadget-foo.css' => array( 'type' => 'style' ),
3030 * )
3131 * @param $dependencies Array: Names of resources this module depends on
 32+ * @param $gadget Gadget: the gadget this module is built upon.
3233 */
3334 public function __construct( $pages, $dependencies, $gadget ) {
3435 $this->pages = $pages;
——@@ -48,47 +49,48 @@
4950 * @return Array: Names of resources this module depends on
5051 */
5152 public function getDependencies() {
52 - return $this->dependencies;
53 - }
54 -
55 - public function getGroup() {
56 - //Modules for gadgets with preferences must be kept private, if the user can set preferences
57 - if ( $this->gadget->getPrefsDescription() !== null
58 - && RequestContext::getMain()->getUser()->isLoggedIn() )
59 - {
60 - return 'private';
61 - } else {
62 - return parent::getGroup();
 53+ $deps = array( 'ext.gadgets' );
 54+ if ( $this->gadget->getPrefsDescription() !== null ){
 55+ $deps[] = $this->gadget->getPrefsModuleName();
6356 }
 57+
 58+ return array_merge(
 59+ $this->dependencies,
 60+ $deps
 61+ );
6462 }
6563
 64+ /**
 65+ * Overrides ResourceLoaderModule::getScript()
 66+ * @param $context ResourceLoaderContext
 67+ * @return String
 68+ */
6669 public function getScript( ResourceLoaderContext $context ) {
6770 $prefs = $this->gadget->getPrefs();
6871
6972 //Enclose gadget's code in a closure, with "this" bound to the
7073 //configuration object (or to "window" for non-configurable gadgets)
71 - $header = '(function(){';
 74+ $header = "(function(){";
7275
73 - //TODO: it may be nice add other metadata for the gadget
74 - $boundObject = array( 'config' => $prefs );
75 -
76 - if ( $prefs !== NULL ) {
77 - //Bind configuration object to "this".
78 - $footer = '}).' . Xml::encodeJsCall( 'apply',
79 - array( $boundObject, array() )
80 - ) . ';';
 76+ if ( $prefs !== null ) {
 77+ //Bind gadget info to "this".
 78+ $footer = "}).apply( mw.gadgets.info.get('{$this->gadget->getName()}') );";
8179 } else {
8280 //Bind window to "this"
83 - $footer = '}).apply( window, [] );';
 81+ $footer = "}).apply( window );";
8482 }
8583
8684 return $header . parent::getScript( $context ) . $footer;
8785 }
8886
 87+ /**
 88+ * Overrides ResourceLoaderModule::getModifiedTime()
 89+ * @param $context ResourceLoaderContext
 90+ * @return Integer
 91+ */
8992 public function getModifiedTime( ResourceLoaderContext $context ) {
90 - $touched = wfTimestamp( TS_UNIX, RequestContext::getMain()->getUser()->getTouched() );
91 - $gadgetMTime = $this->gadget->getModifiedTime();
92 - return max( parent::getModifiedTime( $context ), $touched, $gadgetMTime );
 93+ //TODO: should also depend on the mTime of preferences description page
 94+ return parent::getModifiedTime( $context );
9395 }
9496 }
9597
Index: branches/salvatoreingala/Gadgets/ui/GadgetsMainModule.php
——@@ -1,45 +0,0 @@
2 -<?php
3 -
4 -/**
5 - * Gadgets extension - lets users select custom javascript gadgets
6 - *
7 - *
8 - * For more info see http://mediawiki.org/wiki/Extension:Gadgets
9 - *
10 - * @file
11 - * @ingroup Extensions
12 - * @author Daniel Kinzler, brightbyte.de
13 - * @copyright © 2007 Daniel Kinzler
14 - * @license GNU General Public Licence 2.0 or later
15 - */
16 -
17 -/**
18 - * Class implementing the ext.gadgets module. Required by ext.gadgets.preferences.
19 - */
20 -class GadgetsMainModule extends ResourceLoaderModule {
21 -
22 - public function getModifiedTime( ResourceLoaderContext $context ) {
23 - $gadgets = Gadget::loadList();
24 -
25 - $m = 0;
26 - foreach ( $gadgets as $gadget ) {
27 - $m = max( $m, $gadget->getModifiedTime() );
28 - }
29 - return $m;
30 - }
31 -
32 - public function getScript( ResourceLoaderContext $context ) {
33 - $configurableGadgets = array();
34 - $gadgets = Gadget::loadList();
35 -
36 - foreach ( $gadgets as $gadget ) {
37 - if ( $gadget->getPrefsDescription() !== null ) {
38 - $configurableGadgets[] = $gadget->getName();
39 - }
40 - }
41 -
42 - $script = "mw.gadgets = {}\n";
43 - $script .= "mw.gadgets.configurableGadgets = " . Xml::encodeJsVar( $configurableGadgets ) . ";\n";
44 - return $script;
45 - }
46 -}

Status & tagging log