Completed
Push — add/cli ( 74cf7f...2842da )
by
unknown
10:13
created

Full_Sync_Immediately   B

Complexity

Total Complexity 49

Size/Duplication

Total Lines 428
Duplicated Lines 10.05 %

Coupling/Cohesion

Components 1
Dependencies 6

Importance

Changes 0
Metric Value
dl 43
loc 428
rs 8.48
c 0
b 0
f 0
wmc 49
lcom 1
cbo 6

20 Methods

Rating   Name   Duplication   Size   Complexity  
A name() 0 3 1
A init_full_sync_listeners() 0 2 1
B start() 0 52 7
A is_started() 0 3 1
A get_status() 0 10 1
B get_sync_progress_percentage() 0 24 6
A is_finished() 0 3 1
A reset_data() 0 4 1
A clear_status() 0 3 1
A update_status() 0 3 1
A set_status() 0 3 1
A get_initial_progress() 0 14 2
A get_content_range() 14 14 5
A get_range() 29 29 5
A continue_enqueuing() 0 3 1
A send() 0 20 3
A get_remaining_modules_to_send() 0 29 5
A send_full_sync_end() 0 19 1
A update_sent_progress_action() 0 1 1
A continue_sending() 0 21 4

How to fix   Duplicated Code    Complexity   

Duplicated Code

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 Class

 Tip:   Before tackling complexity, make sure that you eliminate any duplication first. This often can reduce the size of classes significantly.

Complex classes like Full_Sync_Immediately 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 Full_Sync_Immediately, and based on these observations, apply Extract Interface, too.

1
<?php
2
/**
3
 * Full sync module.
4
 *
5
 * @package automattic/jetpack-sync
6
 */
7
8
namespace Automattic\Jetpack\Sync\Modules;
9
10
use Automattic\Jetpack\Sync\Defaults;
11
use Automattic\Jetpack\Sync\Lock;
12
use Automattic\Jetpack\Sync\Modules;
13
use Automattic\Jetpack\Sync\Settings;
14
15
/**
16
 * This class does a full resync of the database by
17
 * sending an outbound action for every single object
18
 * that we care about.
19
 */
20
class Full_Sync_Immediately extends Module {
21
	/**
22
	 * Prefix of the full sync status option name.
23
	 *
24
	 * @var string
25
	 */
26
	const STATUS_OPTION = 'jetpack_sync_full_status';
27
28
	/**
29
	 * Sync Lock name.
30
	 *
31
	 * @var string
32
	 */
33
	const LOCK_NAME = 'full_sync';
34
35
	/**
36
	 * Sync module name.
37
	 *
38
	 * @access public
39
	 *
40
	 * @return string
41
	 */
42
	public function name() {
43
		return 'full-sync';
44
	}
45
46
	/**
47
	 * Initialize action listeners for full sync.
48
	 *
49
	 * @access public
50
	 *
51
	 * @param callable $callable Action handler callable.
52
	 */
53
	public function init_full_sync_listeners( $callable ) { // phpcs:ignore VariableAnalysis.CodeAnalysis.VariableAnalysis.UnusedVariable
54
	}
55
56
	/**
57
	 * Start a full sync.
58
	 *
59
	 * @access public
60
	 *
61
	 * @param array $full_sync_config Full sync configuration.
0 ignored issues
show
Documentation introduced by
Should the type for parameter $full_sync_config not be array|null?

This check looks for @param annotations where the type inferred by our type inference engine differs from the declared type.

It makes a suggestion as to what type it considers more descriptive.

Most often this is a case of a parameter that can be null in addition to its declared types.

Loading history...
62
	 *
63
	 * @return bool Always returns true at success.
64
	 */
65
	public function start( $full_sync_config = null ) {
66
		// There was a full sync in progress.
67
		if ( $this->is_started() && ! $this->is_finished() ) {
68
			/**
69
			 * Fires when a full sync is cancelled.
70
			 *
71
			 * @since 4.2.0
72
			 */
73
			do_action( 'jetpack_full_sync_cancelled' );
74
			$this->send_action( 'jetpack_full_sync_cancelled' );
75
		}
76
77
		// Remove all evidence of previous full sync items and status.
78
		$this->reset_data();
79
80
		if ( ! is_array( $full_sync_config ) ) {
81
			$full_sync_config = Defaults::$default_full_sync_config;
82
			if ( is_multisite() ) {
83
				$full_sync_config['network_options'] = 1;
84
			}
85
		}
86
87
		if ( isset( $full_sync_config['users'] ) && 'initial' === $full_sync_config['users'] ) {
88
			$full_sync_config['users'] = Modules::get_module( 'users' )->get_initial_sync_user_config();
0 ignored issues
show
Bug introduced by
It seems like you code against a specific sub-type and not the parent class Automattic\Jetpack\Sync\Modules\Module as the method get_initial_sync_user_config() does only exist in the following sub-classes of Automattic\Jetpack\Sync\Modules\Module: Automattic\Jetpack\Sync\Modules\Users. Maybe you want to instanceof check for one of these explicitly?

Let’s take a look at an example:

abstract class User
{
    /** @return string */
    abstract public function getPassword();
}

class MyUser extends User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different sub-classes of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the parent class:

    abstract class User
    {
        /** @return string */
        abstract public function getPassword();
    
        /** @return string */
        abstract public function getDisplayName();
    }
    
Loading history...
89
		}
90
91
		$this->update_status(
92
			array(
93
				'started'  => time(),
94
				'config'   => $full_sync_config,
95
				'progress' => $this->get_initial_progress( $full_sync_config ),
96
			)
97
		);
98
99
		$range = $this->get_content_range( $full_sync_config );
0 ignored issues
show
Unused Code introduced by
The call to Full_Sync_Immediately::get_content_range() has too many arguments starting with $full_sync_config.

This check compares calls to functions or methods with their respective definitions. If the call has more arguments than are defined, it raises an issue.

If a function is defined several times with a different number of parameters, the check may pick up the wrong definition and report false positives. One codebase where this has been known to happen is Wordpress.

In this case you can add the @ignore PhpDoc annotation to the duplicate definition and it will be ignored.

Loading history...
100
		/**
101
		 * Fires when a full sync begins. This action is serialized
102
		 * and sent to the server so that it knows a full sync is coming.
103
		 *
104
		 * @param array $full_sync_config Sync configuration for all sync modules.
105
		 * @param array $range Range of the sync items, containing min and max IDs for some item types.
106
		 * @param array $empty The modules with no items to sync during a full sync.
107
		 *
108
		 * @since 4.2.0
109
		 * @since 7.3.0 Added $range arg.
110
		 * @since 7.4.0 Added $empty arg.
111
		 */
112
		do_action( 'jetpack_full_sync_start', $full_sync_config, $range );
113
		$this->send_action( 'jetpack_full_sync_start', array( $full_sync_config, $range ) );
114
115
		return true;
116
	}
117
118
	/**
119
	 * Whether full sync has started.
120
	 *
121
	 * @access public
122
	 *
123
	 * @return boolean
124
	 */
125
	public function is_started() {
126
		return (bool) $this->get_status()['started'];
127
	}
128
129
	/**
130
	 * Retrieve the status of the current full sync.
131
	 *
132
	 * @access public
133
	 *
134
	 * @return array Full sync status.
135
	 */
136
	public function get_status() {
137
		$default = array(
138
			'started'  => false,
139
			'finished' => false,
140
			'progress' => array(),
141
			'config'   => array(),
142
		);
143
144
		return wp_parse_args( \Jetpack_Options::get_raw_option( self::STATUS_OPTION ), $default );
0 ignored issues
show
Documentation introduced by
$default is of type array<string,false|array...ray","config":"array"}>, but the function expects a string.

It seems like the type of the argument is not accepted by the function/method which you are calling.

In some cases, in particular if PHP’s automatic type-juggling kicks in this might be fine. In other cases, however this might be a bug.

We suggest to add an explicit type cast like in the following example:

function acceptsInteger($int) { }

$x = '123'; // string "123"

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
Loading history...
145
	}
146
147
	/**
148
	 * Returns the progress percentage of a full sync.
149
	 *
150
	 * @access public
151
	 *
152
	 * @return int|null
153
	 */
154
	public function get_sync_progress_percentage() {
155
		if ( ! $this->is_started() || $this->is_finished() ) {
156
			return null;
157
		}
158
		$status = $this->get_status();
159
		if ( empty( $status['progress'] ) ) {
160
			return null;
161
		}
162
		$total_items = array_reduce(
163
			array_values( $status['progress'] ),
164
			function ( $sum, $sync_item ) {
165
				return isset( $sync_item['total'] ) ? ( $sum + (int) $sync_item['total'] ) : $sum;
166
			},
167
			0
168
		);
169
		$total_sent  = array_reduce(
170
			array_values( $status['progress'] ),
171
			function ( $sum, $sync_item ) {
172
				return isset( $sync_item['sent'] ) ? ( $sum + (int) $sync_item['sent'] ) : $sum;
173
			},
174
			0
175
		);
176
		return floor( ( $total_sent / $total_items ) * 100 );
177
	}
178
179
	/**
180
	 * Whether full sync has finished.
181
	 *
182
	 * @access public
183
	 *
184
	 * @return boolean
185
	 */
186
	public function is_finished() {
187
		return (bool) $this->get_status()['finished'];
188
	}
189
190
	/**
191
	 * Clear all the full sync data.
192
	 *
193
	 * @access public
194
	 */
195
	public function reset_data() {
196
		$this->clear_status();
197
		( new Lock() )->remove( self::LOCK_NAME, true );
198
	}
199
200
	/**
201
	 * Clear all the full sync status options.
202
	 *
203
	 * @access public
204
	 */
205
	public function clear_status() {
206
		\Jetpack_Options::delete_raw_option( self::STATUS_OPTION );
207
	}
208
209
	/**
210
	 * Updates the status of the current full sync.
211
	 *
212
	 * @access public
213
	 *
214
	 * @param array $values New values to set.
215
	 *
216
	 * @return bool True if success.
217
	 */
218
	public function update_status( $values ) {
219
		return $this->set_status( wp_parse_args( $values, $this->get_status() ) );
0 ignored issues
show
Documentation introduced by
$this->get_status() is of type array|null, but the function expects a string.

It seems like the type of the argument is not accepted by the function/method which you are calling.

In some cases, in particular if PHP’s automatic type-juggling kicks in this might be fine. In other cases, however this might be a bug.

We suggest to add an explicit type cast like in the following example:

function acceptsInteger($int) { }

$x = '123'; // string "123"

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
Loading history...
Bug introduced by
It seems like wp_parse_args($values, $this->get_status()) targeting wp_parse_args() can also be of type null; however, Automattic\Jetpack\Sync\...mediately::set_status() does only seem to accept array, maybe add an additional type check?

This check looks at variables that are passed out again to other methods.

If the outgoing method call has stricter type requirements than the method itself, an issue is raised.

An additional type check may prevent trouble.

Loading history...
220
	}
221
222
	/**
223
	 * Retrieve the status of the current full sync.
224
	 *
225
	 * @param array $values New values to set.
226
	 *
227
	 * @access public
228
	 *
229
	 * @return boolean Full sync status.
230
	 */
231
	public function set_status( $values ) {
232
		return \Jetpack_Options::update_raw_option( self::STATUS_OPTION, $values );
233
	}
234
235
	/**
236
	 * Given an initial Full Sync configuration get the initial status.
237
	 *
238
	 * @param array $full_sync_config Full sync configuration.
239
	 *
240
	 * @return array Initial Sent status.
241
	 */
242
	public function get_initial_progress( $full_sync_config ) {
243
		// Set default configuration, calculate totals, and save configuration if totals > 0.
244
		$status = array();
245
		foreach ( $full_sync_config as $name => $config ) {
246
			$module          = Modules::get_module( $name );
247
			$status[ $name ] = array(
248
				'total'    => $module->total( $config ),
249
				'sent'     => 0,
250
				'finished' => false,
251
			);
252
		}
253
254
		return $status;
255
	}
256
257
	/**
258
	 * Get the range for content (posts and comments) to sync.
259
	 *
260
	 * @access private
261
	 *
262
	 * @return array Array of range (min ID, max ID, total items) for all content types.
263
	 */
264 View Code Duplication
	private function get_content_range() {
265
		$range  = array();
266
		$config = $this->get_status()['config'];
267
		// Add range only when syncing all objects.
268
		if ( true === isset( $config['posts'] ) && $config['posts'] ) {
269
			$range['posts'] = $this->get_range( 'posts' );
270
		}
271
272
		if ( true === isset( $config['comments'] ) && $config['comments'] ) {
273
			$range['comments'] = $this->get_range( 'comments' );
274
		}
275
276
		return $range;
277
	}
278
279
	/**
280
	 * Get the range (min ID, max ID and total items) of items to sync.
281
	 *
282
	 * @access public
283
	 *
284
	 * @param string $type Type of sync item to get the range for.
285
	 *
286
	 * @return array Array of min ID, max ID and total items in the range.
287
	 */
288 View Code Duplication
	public function get_range( $type ) {
289
		global $wpdb;
290
		if ( ! in_array( $type, array( 'comments', 'posts' ), true ) ) {
291
			return array();
292
		}
293
294
		switch ( $type ) {
295
			case 'posts':
296
				$table     = $wpdb->posts;
297
				$id        = 'ID';
298
				$where_sql = Settings::get_blacklisted_post_types_sql();
299
300
				break;
301
			case 'comments':
302
				$table     = $wpdb->comments;
303
				$id        = 'comment_ID';
304
				$where_sql = Settings::get_comments_filter_sql();
305
				break;
306
		}
307
308
		// TODO: Call $wpdb->prepare on the following query.
309
		// phpcs:ignore WordPress.DB.PreparedSQL.InterpolatedNotPrepared
310
		$results = $wpdb->get_results( "SELECT MAX({$id}) as max, MIN({$id}) as min, COUNT({$id}) as count FROM {$table} WHERE {$where_sql}" );
0 ignored issues
show
Bug introduced by
The variable $id does not seem to be defined for all execution paths leading up to this point.

If you define a variable conditionally, it can happen that it is not defined for all execution paths.

Let’s take a look at an example:

function myFunction($a) {
    switch ($a) {
        case 'foo':
            $x = 1;
            break;

        case 'bar':
            $x = 2;
            break;
    }

    // $x is potentially undefined here.
    echo $x;
}

In the above example, the variable $x is defined if you pass “foo” or “bar” as argument for $a. However, since the switch statement has no default case statement, if you pass any other value, the variable $x would be undefined.

Available Fixes

  1. Check for existence of the variable explicitly:

    function myFunction($a) {
        switch ($a) {
            case 'foo':
                $x = 1;
                break;
    
            case 'bar':
                $x = 2;
                break;
        }
    
        if (isset($x)) { // Make sure it's always set.
            echo $x;
        }
    }
    
  2. Define a default value for the variable:

    function myFunction($a) {
        $x = ''; // Set a default which gets overridden for certain paths.
        switch ($a) {
            case 'foo':
                $x = 1;
                break;
    
            case 'bar':
                $x = 2;
                break;
        }
    
        echo $x;
    }
    
  3. Add a value for the missing path:

    function myFunction($a) {
        switch ($a) {
            case 'foo':
                $x = 1;
                break;
    
            case 'bar':
                $x = 2;
                break;
    
            // We add support for the missing case.
            default:
                $x = '';
                break;
        }
    
        echo $x;
    }
    
Loading history...
Bug introduced by
The variable $table does not seem to be defined for all execution paths leading up to this point.

If you define a variable conditionally, it can happen that it is not defined for all execution paths.

Let’s take a look at an example:

function myFunction($a) {
    switch ($a) {
        case 'foo':
            $x = 1;
            break;

        case 'bar':
            $x = 2;
            break;
    }

    // $x is potentially undefined here.
    echo $x;
}

In the above example, the variable $x is defined if you pass “foo” or “bar” as argument for $a. However, since the switch statement has no default case statement, if you pass any other value, the variable $x would be undefined.

Available Fixes

  1. Check for existence of the variable explicitly:

    function myFunction($a) {
        switch ($a) {
            case 'foo':
                $x = 1;
                break;
    
            case 'bar':
                $x = 2;
                break;
        }
    
        if (isset($x)) { // Make sure it's always set.
            echo $x;
        }
    }
    
  2. Define a default value for the variable:

    function myFunction($a) {
        $x = ''; // Set a default which gets overridden for certain paths.
        switch ($a) {
            case 'foo':
                $x = 1;
                break;
    
            case 'bar':
                $x = 2;
                break;
        }
    
        echo $x;
    }
    
  3. Add a value for the missing path:

    function myFunction($a) {
        switch ($a) {
            case 'foo':
                $x = 1;
                break;
    
            case 'bar':
                $x = 2;
                break;
    
            // We add support for the missing case.
            default:
                $x = '';
                break;
        }
    
        echo $x;
    }
    
Loading history...
Bug introduced by
The variable $where_sql does not seem to be defined for all execution paths leading up to this point.

If you define a variable conditionally, it can happen that it is not defined for all execution paths.

Let’s take a look at an example:

function myFunction($a) {
    switch ($a) {
        case 'foo':
            $x = 1;
            break;

        case 'bar':
            $x = 2;
            break;
    }

    // $x is potentially undefined here.
    echo $x;
}

In the above example, the variable $x is defined if you pass “foo” or “bar” as argument for $a. However, since the switch statement has no default case statement, if you pass any other value, the variable $x would be undefined.

Available Fixes

  1. Check for existence of the variable explicitly:

    function myFunction($a) {
        switch ($a) {
            case 'foo':
                $x = 1;
                break;
    
            case 'bar':
                $x = 2;
                break;
        }
    
        if (isset($x)) { // Make sure it's always set.
            echo $x;
        }
    }
    
  2. Define a default value for the variable:

    function myFunction($a) {
        $x = ''; // Set a default which gets overridden for certain paths.
        switch ($a) {
            case 'foo':
                $x = 1;
                break;
    
            case 'bar':
                $x = 2;
                break;
        }
    
        echo $x;
    }
    
  3. Add a value for the missing path:

    function myFunction($a) {
        switch ($a) {
            case 'foo':
                $x = 1;
                break;
    
            case 'bar':
                $x = 2;
                break;
    
            // We add support for the missing case.
            default:
                $x = '';
                break;
        }
    
        echo $x;
    }
    
Loading history...
311
		if ( isset( $results[0] ) ) {
312
			return $results[0];
313
		}
314
315
		return array();
316
	}
317
318
	/**
319
	 * Continue sending instead of enqueueing.
320
	 *
321
	 * @access public
322
	 */
323
	public function continue_enqueuing() {
324
		$this->continue_sending();
325
	}
326
327
	/**
328
	 * Continue sending.
329
	 *
330
	 * @access public
331
	 */
332
	public function continue_sending() {
333
		// Return early if Full Sync is not running.
334
		if ( ! $this->is_started() || $this->get_status()['finished'] ) {
335
			return;
336
		}
337
338
		// Obtain send Lock.
339
		$lock            = new Lock();
340
		$lock_expiration = $lock->attempt( self::LOCK_NAME );
341
342
		// Return if unable to obtain lock.
343
		if ( false === $lock_expiration ) {
344
			return;
345
		}
346
347
		// Send Full Sync actions.
348
		$this->send();
349
350
		// Remove lock.
351
		$lock->remove( self::LOCK_NAME, $lock_expiration );
352
	}
353
354
	/**
355
	 * Immediately send the next items to full sync.
356
	 *
357
	 * @access public
358
	 */
359
	public function send() {
360
		$config = $this->get_status()['config'];
361
362
		$max_duration = Settings::get_setting( 'full_sync_send_duration' );
363
		$send_until   = microtime( true ) + $max_duration;
364
365
		$progress = $this->get_status()['progress'];
366
367
		foreach ( $this->get_remaining_modules_to_send() as $module ) {
368
			$progress[ $module->name() ] = $module->send_full_sync_actions( $config[ $module->name() ], $progress[ $module->name() ], $send_until );
369
			if ( ! $progress[ $module->name() ]['finished'] ) {
370
				$this->update_status( array( 'progress' => $progress ) );
371
372
				return;
373
			}
374
		}
375
376
		$this->send_full_sync_end();
377
		$this->update_status( array( 'progress' => $progress ) );
378
	}
379
380
	/**
381
	 * Get Modules that are configured to Full Sync and haven't finished sending
382
	 *
383
	 * @return array
384
	 */
385
	public function get_remaining_modules_to_send() {
386
		$status = $this->get_status();
387
388
		return array_filter(
389
			Modules::get_modules(),
390
			/**
391
			 * Select configured and not finished modules.
392
			 *
393
			 * @return bool
394
			 * @var $module Module
395
			 */
396
			function ( $module ) use ( $status ) {
397
				// Skip module if not configured for this sync or module is done.
398
				if ( ! isset( $status['config'][ $module->name() ] ) ) {
399
					return false;
400
				}
401
				if ( ! $status['config'][ $module->name() ] ) {
402
					return false;
403
				}
404
				if ( isset( $status['progress'][ $module->name() ]['finished'] ) ) {
405
					if ( true === $status['progress'][ $module->name() ]['finished'] ) {
406
						return false;
407
					}
408
				}
409
410
				return true;
411
			}
412
		);
413
	}
414
415
	/**
416
	 * Send 'jetpack_full_sync_end' and update 'finished' status.
417
	 *
418
	 * @access public
419
	 */
420
	public function send_full_sync_end() {
421
		$range = $this->get_content_range();
422
423
		/**
424
		 * Fires when a full sync ends. This action is serialized
425
		 * and sent to the server.
426
		 *
427
		 * @param string $checksum Deprecated since 7.3.0 - @see https://github.com/Automattic/jetpack/pull/11945/
428
		 * @param array $range Range of the sync items, containing min and max IDs for some item types.
429
		 *
430
		 * @since 4.2.0
431
		 * @since 7.3.0 Added $range arg.
432
		 */
433
		do_action( 'jetpack_full_sync_end', '', $range );
434
		$this->send_action( 'jetpack_full_sync_end', array( '', $range ) );
435
436
		// Setting autoload to true means that it's faster to check whether we should continue enqueuing.
437
		$this->update_status( array( 'finished' => time() ) );
438
	}
439
440
	/**
441
	 * Empty Function as we don't close buffers on Immediate Full Sync.
442
	 *
443
	 * @param array $actions an array of actions, ignored for queueless sync.
444
	 */
445
	public function update_sent_progress_action( $actions ) { } // phpcs:ignore VariableAnalysis.CodeAnalysis.VariableAnalysis.UnusedVariable
446
447
}
448