http://www.mediawiki.org/wiki/Special:Code/MediaWiki/95786
Revision: 95786
Author: catrope
Date: 2011-08-30 15:50:30 +0000 (Tue, 30 Aug 2011)
Log Message:
-----------
RL2: Kill GadgetRepo::createGadget() and make modifyGadget() flexible enough to
handle both creations and modifications
Modified Paths:
--------------
branches/RL2/extensions/Gadgets/backend/GadgetRepo.php
branches/RL2/extensions/Gadgets/backend/LocalGadgetRepo.php
Modified: branches/RL2/extensions/Gadgets/backend/GadgetRepo.php
===================================================================
--- branches/RL2/extensions/Gadgets/backend/GadgetRepo.php 2011-08-30
15:43:19 UTC (rev 95785)
+++ branches/RL2/extensions/Gadgets/backend/GadgetRepo.php 2011-08-30
15:50:30 UTC (rev 95786)
@@ -56,22 +56,15 @@
abstract public function getDB();
/**
- * Add a new gadget to the repository. Will fail if there is already
- * a gadget with the same name.
- * @param $gadget Gadget object
- * @return Status
- */
- abstract public function addGadget( Gadget $gadget );
-
- /**
- * Modify an existing gadget, replacing its metadata with the
+ * Modify a gadget, replacing its metadata with the
* metadata in the provided Gadget object. The name is taken
- * from the Gadget object as well. Will fail if there is no
- * gadget by the same name.
+ * from the Gadget object as well. If no Gadget exists by that name,
+ * it will be created.
* @param $gadget Gadget object
+ * @param $timestamp Timestamp to record for this action, or current
timestamp if null
* @return Status
*/
- abstract public function modifyGadget( Gadget $gadget );
+ abstract public function modifyGadget( Gadget $gadget, $timestamp =
null );
/**
* Irrevocably delete a gadget from the repository. Will fail
Modified: branches/RL2/extensions/Gadgets/backend/LocalGadgetRepo.php
===================================================================
--- branches/RL2/extensions/Gadgets/backend/LocalGadgetRepo.php 2011-08-30
15:43:19 UTC (rev 95785)
+++ branches/RL2/extensions/Gadgets/backend/LocalGadgetRepo.php 2011-08-30
15:50:30 UTC (rev 95786)
@@ -42,83 +42,50 @@
return true;
}
- public function addGadget( Gadget $gadget ) {
+ public function modifyGadget( Gadget $gadget, $timestamp = null ) {
if ( !$this->isWriteable() ) {
return Status::newFatal(
'gadget-manager-readonly-repository' );
}
- // Try to detect a naming conflict beforehand
+ $dbw = $this->getMasterDB();
$this->loadData();
$name = $gadget->getName();
- if ( isset( $this->data[$name] ) ) {
- return Status::newFatal(
'gadget-manager-create-exists', $name );
- }
-
$json = $gadget->getJSON();
- $dbw = $this->getMasterDB();
- $ts = $dbw->timestamp();
- // Use INSERT IGNORE so we don't die when there's a race
condition causing a naming conflict
- $dbw->insert( 'gadgets', array( array(
- 'gd_name' => $name,
- 'gd_blob' => $json,
- 'gd_shared' => $gadget->isShared(),
- 'gd_timestamp' => $ts
- ) ), __METHOD__, array( 'IGNORE' )
+ $ts = $dbw->timestamp( $gadget->getTimestamp() );
+ $newTs = $dbw->timestamp( $timestamp );
+ $row = array(
+ 'gd_name' => $name,
+ 'gd_blob' => $json,
+ 'gd_shared' => $gadget->isShared(),
+ 'gd_timestamp' => $newTs
);
- // Detect naming conflict after the fact
- if ( $dbw->affectedRows() === 0 ) {
- return Status::newFatal(
'gadget-manager-create-exists', $name );
+ // First INSERT IGNORE the row, in case the gadget doesn't
already exist
+ $dbw->begin();
+ $created = false;
+ $dbw->insert( 'gadgets', $row, __METHOD__, array( 'IGNORE' ) );
+ $created = $dbw->affectedRows() > 0;
+ // Then UPDATE it if it did already exist
+ if ( !$created ) {
+ $dbw->update( 'gadgets', $row, array(
+ 'gd_name' => $name,
+ 'gd_timestamp <= ' . $dbw->addQuotes(
$ts ) // for conflict detection
+ ), __METHOD__
+ );
}
+ $dbw->commit();
- // Update our in-object cache
- // This looks stupid: we have an object that we could be
caching. But I prefer
- // to keep $this->data in a consistent format and have
getGadget() always return
- // a clone. If it returned a reference to a cached object, the
caller could change
- // that object and cause weird things to happen.
- $this->data[$name] = array( 'json' => $json, 'timestamp' => $ts
);
-
- return Status::newGood();
- }
-
- public function modifyGadget( Gadget $gadget ) {
- if ( !$this->isWriteable() ) {
- return Status::newFatal(
'gadget-manager-readonly-repository' );
- }
-
- $this->loadData();
- $name = $gadget->getName();
- if ( !isset( $this->data[$name] ) ) {
- return Status::newFatal( 'gadget-manager-nosuchgadget',
$name );
- }
-
- $json = $gadget->getJSON();
- $ts = $gadget->getTimestamp();
- $dbw = $this->getMasterDB();
- $newTs = $dbw->timestamp();
- $dbw->update( 'gadgets', array(
- 'gd_blob' => $json,
- 'gd_shared' => $gadget->isShared(),
- 'gd_timestamp' => $dbw->timestamp()
- ), array(
- 'gd_name' => $name,
- 'gd_timestamp' => $ts // for conflict detection
- ), __METHOD__, array( 'IGNORE' )
- );
-
// Detect conflicts
if ( $dbw->affectedRows() === 0 ) {
- // Some conflict occurred. Either the UPDATE failed
because the
- // timestamp condition didn't match, in which case
someone else
- // modified the gadget concurrently, or it failed to
find a row
- // for this gadget name at all, in which case someone
else deleted
- // the gadget entirely. We don't really care what
happened, we'll
- // just return an error and let the caller figure it
out.
+ // Some conflict occurred
return Status::newFatal(
'gadgets-manager-modify-conflict', $name, $ts );
}
// Update our in-object cache
- // See comment in addGadget() for an explanation of why it's
done this way
+ // This looks stupid: we have an object that we could be
caching. But I prefer
+ // to keep $this->data in a consistent format and have
getGadget() always return
+ // a clone. If it returned a reference to a cached object, the
caller could change
+ // that object and cause weird things to happen.
$this->data[$name] = array( 'json' => $json, 'timestamp' =>
$newTs );
return Status::newGood();
@@ -168,6 +135,8 @@
* $this->data must call this before accessing $this->data .
*/
protected function loadData() {
+ // FIXME: Make the cache shared somehow, it's getting
repopulated for every instance now
+ // FIXME: Reconsider the query-everything behavior; maybe use
memc?
if ( is_array( $this->data ) ) {
// Already loaded
return;
_______________________________________________
MediaWiki-CVS mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-cvs
