Duplicate code is one of the most pungent code smells. A rule that is often used is to re-structure code once it is duplicated in three or more places.
Common duplication problems, and corresponding solutions are:
Complex classes like LBFactory often do a lot of different things. To break such a class down, we need to identify a cohesive component within that class. A common approach to find such a component is to look for fields/methods that share the same prefixes, or suffixes. You can also have a look at the cohesion graph to spot any un-connected, or weakly-connected components.
Once you have determined the fields that belong together, you can apply the Extract Class refactoring. If the component makes sense as a sub-class, Extract Subclass is also a candidate, and is often faster.
While breaking up the class, it is a good idea to analyze how other classes use LBFactory, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
31 | abstract class LBFactory { |
||
32 | /** @var ChronologyProtector */ |
||
33 | protected $chronProt; |
||
34 | |||
35 | /** @var TransactionProfiler */ |
||
36 | protected $trxProfiler; |
||
37 | |||
38 | /** @var LoggerInterface */ |
||
39 | protected $logger; |
||
40 | |||
41 | /** @var LBFactory */ |
||
42 | private static $instance; |
||
43 | |||
44 | /** @var string|bool Reason all LBs are read-only or false if not */ |
||
45 | protected $readOnlyReason = false; |
||
46 | |||
47 | const SHUTDOWN_NO_CHRONPROT = 1; // don't save ChronologyProtector positions (for async code) |
||
48 | |||
49 | /** |
||
50 | * Construct a factory based on a configuration array (typically from $wgLBFactoryConf) |
||
51 | * @param array $conf |
||
52 | */ |
||
53 | public function __construct( array $conf ) { |
||
62 | |||
63 | /** |
||
64 | * Disables all access to the load balancer, will cause all database access |
||
65 | * to throw a DBAccessError |
||
66 | */ |
||
67 | public static function disableBackend() { |
||
68 | global $wgLBFactoryConf; |
||
69 | self::$instance = new LBFactoryFake( $wgLBFactoryConf ); |
||
70 | } |
||
71 | |||
72 | /** |
||
73 | * Get an LBFactory instance |
||
74 | * |
||
75 | * @return LBFactory |
||
76 | */ |
||
77 | public static function singleton() { |
||
78 | global $wgLBFactoryConf; |
||
79 | |||
80 | if ( is_null( self::$instance ) ) { |
||
81 | $class = self::getLBFactoryClass( $wgLBFactoryConf ); |
||
82 | $config = $wgLBFactoryConf; |
||
83 | if ( !isset( $config['readOnlyReason'] ) ) { |
||
84 | $config['readOnlyReason'] = wfConfiguredReadOnlyReason(); |
||
85 | } |
||
86 | self::$instance = new $class( $config ); |
||
87 | } |
||
88 | |||
89 | return self::$instance; |
||
90 | } |
||
91 | |||
92 | /** |
||
93 | * Returns the LBFactory class to use and the load balancer configuration. |
||
94 | * |
||
95 | * @param array $config (e.g. $wgLBFactoryConf) |
||
96 | * @return string Class name |
||
97 | */ |
||
98 | public static function getLBFactoryClass( array $config ) { |
||
120 | |||
121 | /** |
||
122 | * Shut down, close connections and destroy the cached instance. |
||
123 | */ |
||
124 | public static function destroyInstance() { |
||
125 | if ( self::$instance ) { |
||
126 | self::$instance->shutdown(); |
||
127 | self::$instance->forEachLBCallMethod( 'closeAll' ); |
||
128 | self::$instance = null; |
||
129 | } |
||
130 | } |
||
131 | |||
132 | /** |
||
133 | * Set the instance to be the given object |
||
134 | * |
||
135 | * @param LBFactory $instance |
||
136 | */ |
||
137 | public static function setInstance( $instance ) { |
||
138 | self::destroyInstance(); |
||
139 | self::$instance = $instance; |
||
140 | } |
||
141 | |||
142 | /** |
||
143 | * Create a new load balancer object. The resulting object will be untracked, |
||
144 | * not chronology-protected, and the caller is responsible for cleaning it up. |
||
145 | * |
||
146 | * @param bool|string $wiki Wiki ID, or false for the current wiki |
||
147 | * @return LoadBalancer |
||
148 | */ |
||
149 | abstract public function newMainLB( $wiki = false ); |
||
150 | |||
151 | /** |
||
152 | * Get a cached (tracked) load balancer object. |
||
153 | * |
||
154 | * @param bool|string $wiki Wiki ID, or false for the current wiki |
||
155 | * @return LoadBalancer |
||
156 | */ |
||
157 | abstract public function getMainLB( $wiki = false ); |
||
158 | |||
159 | /** |
||
160 | * Create a new load balancer for external storage. The resulting object will be |
||
161 | * untracked, not chronology-protected, and the caller is responsible for |
||
162 | * cleaning it up. |
||
163 | * |
||
164 | * @param string $cluster External storage cluster, or false for core |
||
165 | * @param bool|string $wiki Wiki ID, or false for the current wiki |
||
166 | * @return LoadBalancer |
||
167 | */ |
||
168 | abstract protected function newExternalLB( $cluster, $wiki = false ); |
||
169 | |||
170 | /** |
||
171 | * Get a cached (tracked) load balancer for external storage |
||
172 | * |
||
173 | * @param string $cluster External storage cluster, or false for core |
||
174 | * @param bool|string $wiki Wiki ID, or false for the current wiki |
||
175 | * @return LoadBalancer |
||
176 | */ |
||
177 | abstract public function &getExternalLB( $cluster, $wiki = false ); |
||
178 | |||
179 | /** |
||
180 | * Execute a function for each tracked load balancer |
||
181 | * The callback is called with the load balancer as the first parameter, |
||
182 | * and $params passed as the subsequent parameters. |
||
183 | * |
||
184 | * @param callable $callback |
||
185 | * @param array $params |
||
186 | */ |
||
187 | abstract public function forEachLB( $callback, array $params = [] ); |
||
188 | |||
189 | /** |
||
190 | * Prepare all tracked load balancers for shutdown |
||
191 | * @param integer $flags Supports SHUTDOWN_* flags |
||
192 | * STUB |
||
193 | */ |
||
194 | public function shutdown( $flags = 0 ) { |
||
196 | |||
197 | /** |
||
198 | * Call a method of each tracked load balancer |
||
199 | * |
||
200 | * @param string $methodName |
||
201 | * @param array $args |
||
202 | */ |
||
203 | private function forEachLBCallMethod( $methodName, array $args = [] ) { |
||
211 | |||
212 | /** |
||
213 | * Commit on all connections. Done for two reasons: |
||
214 | * 1. To commit changes to the masters. |
||
215 | * 2. To release the snapshot on all connections, master and slave. |
||
216 | * @param string $fname Caller name |
||
217 | */ |
||
218 | public function commitAll( $fname = __METHOD__ ) { |
||
227 | |||
228 | /** |
||
229 | * Commit changes on all master connections |
||
230 | * @param string $fname Caller name |
||
231 | * @param array $options Options map: |
||
232 | * - maxWriteDuration: abort if more than this much time was spent in write queries |
||
233 | */ |
||
234 | public function commitMasterChanges( $fname = __METHOD__, array $options = [] ) { |
||
252 | |||
253 | /** |
||
254 | * Rollback changes on all master connections |
||
255 | * @param string $fname Caller name |
||
256 | * @since 1.23 |
||
257 | */ |
||
258 | public function rollbackMasterChanges( $fname = __METHOD__ ) { |
||
261 | |||
262 | /** |
||
263 | * Log query info if multi DB transactions are going to be committed now |
||
264 | */ |
||
265 | private function logMultiDbTransaction() { |
||
284 | |||
285 | /** |
||
286 | * Determine if any master connection has pending changes |
||
287 | * @return bool |
||
288 | * @since 1.23 |
||
289 | */ |
||
290 | public function hasMasterChanges() { |
||
298 | |||
299 | /** |
||
300 | * Detemine if any lagged slave connection was used |
||
301 | * @since 1.27 |
||
302 | * @return bool |
||
303 | */ |
||
304 | public function laggedSlaveUsed() { |
||
312 | |||
313 | /** |
||
314 | * Determine if any master connection has pending/written changes from this request |
||
315 | * @return bool |
||
316 | * @since 1.27 |
||
317 | */ |
||
318 | public function hasOrMadeRecentMasterChanges() { |
||
325 | |||
326 | /** |
||
327 | * Waits for the slave DBs to catch up to the current master position |
||
328 | * |
||
329 | * Use this when updating very large numbers of rows, as in maintenance scripts, |
||
330 | * to avoid causing too much lag. Of course, this is a no-op if there are no slaves. |
||
331 | * |
||
332 | * By default this waits on all DB clusters actually used in this request. |
||
333 | * This makes sense when lag being waiting on is caused by the code that does this check. |
||
334 | * In that case, setting "ifWritesSince" can avoid the overhead of waiting for clusters |
||
335 | * that were not changed since the last wait check. To forcefully wait on a specific cluster |
||
336 | * for a given wiki, use the 'wiki' parameter. To forcefully wait on an "external" cluster, |
||
337 | * use the "cluster" parameter. |
||
338 | * |
||
339 | * Never call this function after a large DB write that is *still* in a transaction. |
||
340 | * It only makes sense to call this after the possible lag inducing changes were committed. |
||
341 | * |
||
342 | * @param array $opts Optional fields that include: |
||
343 | * - wiki : wait on the load balancer DBs that handles the given wiki |
||
344 | * - cluster : wait on the given external load balancer DBs |
||
345 | * - timeout : Max wait time. Default: ~60 seconds |
||
346 | * - ifWritesSince: Only wait if writes were done since this UNIX timestamp |
||
347 | * @throws DBReplicationWaitError If a timeout or error occured waiting on a DB cluster |
||
348 | * @since 1.27 |
||
349 | */ |
||
350 | public function waitForReplication( array $opts = [] ) { |
||
409 | |||
410 | /** |
||
411 | * Disable the ChronologyProtector for all load balancers |
||
412 | * |
||
413 | * This can be called at the start of special API entry points |
||
414 | * |
||
415 | * @since 1.27 |
||
416 | */ |
||
417 | public function disableChronologyProtection() { |
||
420 | |||
421 | /** |
||
422 | * @return ChronologyProtector |
||
423 | */ |
||
424 | protected function newChronologyProtector() { |
||
443 | |||
444 | /** |
||
445 | * @param ChronologyProtector $cp |
||
446 | */ |
||
447 | protected function shutdownChronologyProtector( ChronologyProtector $cp ) { |
||
465 | } |
||
466 | |||
482 |
This check marks implicit conversions of arrays to boolean values in a comparison. While in PHP an empty array is considered to be equal (but not identical) to false, this is not always apparent.
Consider making the comparison explicit by using
empty(..)
or! empty(...)
instead.