| Index: branches/RL2/extensions/Gadgets/backend/CachedGadgetRepo.php |
| — | — | @@ -1,5 +1,4 @@ |
| 2 | 2 | <?php |
| 3 | | -// TODO: Reconsider memc key naming |
| 4 | 3 | /** |
| 5 | 4 | * Abstract class that enhances GadgetRepo with caching. This is useful for |
| 6 | 5 | * repos that obtain gadget data from a database or a REST API. Currently, all |
| — | — | @@ -26,16 +25,6 @@ |
| 27 | 26 | */ |
| 28 | 27 | protected $idsLoaded = false; |
| 29 | 28 | |
| 30 | | - /** |
| 31 | | - * Memcached key of the gadget names list. Subclasses must set this in their constructor. |
| 32 | | - * This could've been a static member if we had PHP 5.3's late static binding. |
| 33 | | - * |
| 34 | | - * Set to false if memcached is not available. |
| 35 | | - * |
| 36 | | - * TODO: make this an abstract getter, and redesign the whole memc key naming scheme |
| 37 | | - */ |
| 38 | | - protected $namesKey; |
| 39 | | - |
| 40 | 29 | /*** Abstract methods ***/ |
| 41 | 30 | |
| 42 | 31 | /** |
| — | — | @@ -51,6 +40,14 @@ |
| 52 | 41 | */ |
| 53 | 42 | abstract protected function loadDataFor( $id ); |
| 54 | 43 | |
| | 44 | + /** |
| | 45 | + * Get the memc key for caching the data for a given gadget, or for |
| | 46 | + * caching the gadget IDs list. |
| | 47 | + * @param $id string|null Gadget ID to get the memc key for, or null to get the memc key for the IDs list |
| | 48 | + * @return string Memc key including wiki prefix, i.e. the return value of wfMemcKey() or wfForeignMemcKey() |
| | 49 | + */ |
| | 50 | + abstract protected function getCacheKey( $id ); |
| | 51 | + |
| 55 | 52 | /*** Protected methods ***/ |
| 56 | 53 | |
| 57 | 54 | /** |
| — | — | @@ -79,14 +76,9 @@ |
| 80 | 77 | } |
| 81 | 78 | |
| 82 | 79 | // Write to memc |
| 83 | | - $key = $this->getMemcKey( 'gadgets', 'localrepodata', $id ); |
| 84 | | - if ( $key !== false ) { |
| 85 | | - $wgMemc->set( $key, $toCache ); |
| 86 | | - } |
| | 80 | + $wgMemc->set( $this->getCacheKey( $id ), $toCache ); |
| 87 | 81 | // Clear the gadget names array in memc so it'll be regenerated later |
| 88 | | - if ( $this->namesKey !== false ) { |
| 89 | | - $wgMemc->delete( $this->namesKey ); |
| 90 | | - } |
| | 82 | + $wgMemc->delete( $this->getCacheKey( null ) ); |
| 91 | 83 | } |
| 92 | 84 | |
| 93 | 85 | /*** Public methods inherited from GadgetRepo ***/ |
| — | — | @@ -118,7 +110,8 @@ |
| 119 | 111 | } |
| 120 | 112 | |
| 121 | 113 | // Try memc |
| 122 | | - $cached = $this->namesKey !== false ? $wgMemc->get( $this->namesKey ) : false; |
| | 114 | + $key = $this->getCacheKey( null ); |
| | 115 | + $cached = $wgMemc->get( $key ); |
| 123 | 116 | if ( is_array( $cached ) ) { |
| 124 | 117 | // Yay, data is in cache |
| 125 | 118 | // Add to $this->data , but let things already in $this->data take precedence |
| — | — | @@ -131,7 +124,7 @@ |
| 132 | 125 | // For memc, prepare an array with the IDs as keys but with each value set to null |
| 133 | 126 | $toCache = array_combine( array_keys( $data ), array_fill( 0, count( $this->data ), null ) ); |
| 134 | 127 | |
| 135 | | - $wgMemc->set( $this->namesKey, $toCache ); |
| | 128 | + $wgMemc->set( $key, $toCache ); |
| 136 | 129 | $this->idsLoaded = true; |
| 137 | 130 | return array_keys( $this->data ); |
| 138 | 131 | } |
| — | — | @@ -160,9 +153,8 @@ |
| 161 | 154 | } |
| 162 | 155 | |
| 163 | 156 | // Try cache |
| 164 | | - // FIXME getMemcKey is defined in LocalGadgetRepo but not abstract. Cache key naming needs redesign |
| 165 | | - $key = $this->getMemcKey( 'gadgets', 'localrepodata', $id ); |
| 166 | | - $cached = $key !== false ? $wgMemc->get( $key ) : false; |
| | 157 | + $key = $this->getCacheKey( $id ); |
| | 158 | + $cached = $wgMemc->get( $key ); |
| 167 | 159 | if ( is_array( $cached ) ) { |
| 168 | 160 | // Yay, data is in cache |
| 169 | 161 | if ( count( $cached ) ) { |
| Index: branches/RL2/extensions/Gadgets/backend/LocalGadgetRepo.php |
| — | — | @@ -20,6 +20,14 @@ |
| 21 | 21 | |
| 22 | 22 | /*** Protected methods inherited from CachedGadgetRepo ***/ |
| 23 | 23 | |
| | 24 | + protected function getCacheKey( $id ) { |
| | 25 | + if ( $id === null ) { |
| | 26 | + return wfMemcKey( 'gadgets', 'localrepoids' ); |
| | 27 | + } else { |
| | 28 | + return wfMemcKey( 'gadgets', 'localrepodata', $id ); |
| | 29 | + } |
| | 30 | + } |
| | 31 | + |
| 24 | 32 | protected function loadAllData() { |
| 25 | 33 | $query = $this->getLoadAllDataQuery(); |
| 26 | 34 | $dbr = $this->getDB(); |
| — | — | @@ -49,15 +57,6 @@ |
| 50 | 58 | |
| 51 | 59 | /*** Public methods inherited from GadgetRepo ***/ |
| 52 | 60 | |
| 53 | | - /** |
| 54 | | - * Constructor. |
| 55 | | - * @param info array of options. There are no applicable options for this class |
| 56 | | - */ |
| 57 | | - public function __construct( array $options = array() ) { |
| 58 | | - parent::__construct( $options ); |
| 59 | | - $this->namesKey = $this->getMemcKey( 'gadgets', 'localreponames' ); |
| 60 | | - } |
| 61 | | - |
| 62 | 61 | public function getSource() { |
| 63 | 62 | return 'local'; |
| 64 | 63 | } |
| — | — | @@ -180,15 +179,6 @@ |
| 181 | 180 | } |
| 182 | 181 | |
| 183 | 182 | /** |
| 184 | | - * Get a memcached key. Subclasses can override this to use a foreign memc |
| 185 | | - * @return string|bool Cache key, or false if this repo has no shared memc |
| 186 | | - */ |
| 187 | | - protected function getMemcKey( /* ... */ ) { |
| 188 | | - $args = func_get_args(); |
| 189 | | - return call_user_func_array( 'wfMemcKey', $args ); |
| 190 | | - } |
| 191 | | - |
| 192 | | - /** |
| 193 | 183 | * Get the DB query to use in loadAllData(). Subclasses can override this to tweak the query. |
| 194 | 184 | * @return Array |
| 195 | 185 | */ |
| Index: branches/RL2/extensions/Gadgets/backend/ForeignDBGadgetRepo.php |
| — | — | @@ -30,8 +30,6 @@ |
| 31 | 31 | foreach ( $optionKeys as $optionKey ) { |
| 32 | 32 | $this->{$optionKey} = $options[$optionKey]; |
| 33 | 33 | } |
| 34 | | - |
| 35 | | - $this->namesKey = $this->getMemcKey( 'gadgets', 'foreigndbreponames' ); |
| 36 | 34 | } |
| 37 | 35 | |
| 38 | 36 | public function isWriteable() { |
| — | — | @@ -67,13 +65,27 @@ |
| 68 | 66 | return $this->db; |
| 69 | 67 | } |
| 70 | 68 | |
| 71 | | - protected function getMemcKey( /* ... */ ) { |
| | 69 | + protected function getCacheKey( $id ) { |
| 72 | 70 | if ( $this->hasSharedCache ) { |
| 73 | | - $args = func_get_args(); |
| 74 | | - array_unshift( $args, $this->dbName, $this->tablePrefix ); |
| 75 | | - return call_user_func_array( 'wfForeignMemcKey', $args ); |
| | 71 | + // Access the foreign wiki's memc |
| | 72 | + if ( $id === null ) { |
| | 73 | + // Use 'localrepoidsshared' instead of 'localrepoids', otherwise we |
| | 74 | + // will be accessing the same cache entry as the foreign wiki's LocalGadgetRepo |
| | 75 | + // We can't allow that to happen because the local repo's ID list includes |
| | 76 | + // shared gadgets but ours doesn't |
| | 77 | + return wfForeignMemcKey( $this->dbName, $this->tablePrefix, 'gadgets', 'localrepoidsshared' ); |
| | 78 | + } else { |
| | 79 | + // We don't have to prevent collisions here because sharing gadget data |
| | 80 | + // caches between us and the foreign wiki's local repo is OK |
| | 81 | + return wfForeignMemcKey( $this->dbName, $this->tablePrefix, 'gadgets', 'localrepodata', $id ); |
| | 82 | + } |
| 76 | 83 | } else { |
| 77 | | - return false; |
| | 84 | + // No access to the foreign wiki's memc, so cache locally |
| | 85 | + if ( $id === null ) { |
| | 86 | + return wfMemcKey( 'gadgets', 'foreignrepoids', $this->source ); |
| | 87 | + } else { |
| | 88 | + return wfMemcKey( 'gadgets', 'foreignrepodata', $this->source, $id ); |
| | 89 | + } |
| 78 | 90 | } |
| 79 | 91 | } |
| 80 | 92 | |