WP_Async_Task - Code Metrics - Inspection of "bump to 3.15.0" - insideout10/wordlift-plugin - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — master ( 5db201...585add )

by David

created 2017-10-07 06:23 UTC

WP_Async_Task A

↳ Parent: Project

Complexity

Total Complexity

Size/Duplication

Total Lines	290
Duplicated Lines	2.07 %

Coupling/Cohesion

Components	1
Dependencies	1

Importance

Changes

Metric	Value
dl	6
loc	290
rs	10
c	0
b	0
f	0
wmc	22
lcom	1
cbo	1

9 Methods

Rating	Name	Duplication	Size	Complexity
B	__construct()	0	24	4
A	launch()	0	21	3
B	launch_on_shutdown()	0	31	5
A	handle_postback()	0	13	4
A	create_async_nonce()	0	6	1
A	verify_async_nonce()	6	17	3
A	get_nonce_action()	0	9	2
	prepare_data()	0	1	?
	run_action()	0	1	?

How to fix Duplicated Code

<?php
/**
 * Plugin Name: WP Asynchronous Tasks
 * Version: 1.0
 * Description: Creates an abstract class to execute asynchronous tasks
 * Author: 10up, Eric Mann, Luke Gedeon, John P. Bloch
 * License: MIT
 */

if ( ! class_exists( 'WP_Async_Task' ) ) {
	abstract class WP_Async_Task {

		/**
		 * Constant identifier for a task that should be available to logged-in users
		 *
		 * See constructor documentation for more details.
		 */
		const LOGGED_IN = 1;

		/**
		 * Constant identifier for a task that should be available to logged-out users
		 *
		 * See constructor documentation for more details.
		 */
		const LOGGED_OUT = 2;

		/**
		 * Constant identifier for a task that should be available to all users regardless of auth status
		 *
		 * See constructor documentation for more details.
		 */
		const BOTH = 3;

		/**
		 * This is the argument count for the main action set in the constructor. It
		 * is set to an arbitrarily high value of twenty, but can be overridden if
		 * necessary
		 *
		 * @var int
		 */
		protected $argument_count = 20;

		/**
		 * Priority to fire intermediate action.
		 *
		 * @var int
		 */
		protected $priority = 10;

		/**
		 * @var string
		 */
		protected $action;

		/**
		 * @var array
		 */
		protected $_body_data;

		/**
		 * A {@link Wordlift_Log_Service} instance.
		 *
		 * @since  3.15.0
		 * @access private
		 * @var \Wordlift_Log_Service $log A {@link Wordlift_Log_Service} instance.
		 */
		private $log;

		/**
		 * Constructor to wire up the necessary actions
		 *
		 * Which hooks the asynchronous postback happens on can be set by the
		 * $auth_level parameter. There are essentially three options: logged in users
		 * only, logged out users only, or both. Set this when you instantiate an
		 * object by using one of the three class constants to do so:
		 *  - LOGGED_IN
		 *  - LOGGED_OUT
		 *  - BOTH
		 * $auth_level defaults to BOTH
		 *
		 * @throws Exception If the class' $action value hasn't been set
		 *
		 * @param int $auth_level The authentication level to use (see above)
		 */
		public function __construct( $auth_level = self::BOTH ) {

			$this->log = Wordlift_Log_Service::get_logger( 'WP_Async_Task' );

			if ( empty( $this->action ) ) {
				throw new Exception( 'Action not defined for class ' . __CLASS__ );
			}
			add_action( $this->action, array(
				$this,
				'launch',
			), (int) $this->priority, (int) $this->argument_count );
			if ( $auth_level & self::LOGGED_IN ) {
				add_action( "admin_post_wp_async_$this->action", array(
					$this,
					'handle_postback',
				) );
			}
			if ( $auth_level & self::LOGGED_OUT ) {
				add_action( "admin_post_nopriv_wp_async_$this->action", array(
					$this,
					'handle_postback',
				) );
			}
		}

		/**
		 * Add the shutdown action for launching the real postback if we don't
		 * get an exception thrown by prepare_data().
		 *
		 * @uses func_get_args() To grab any arguments passed by the action
		 */
		public function launch() {
			$data = func_get_args();
			try {
				$data = $this->prepare_data( $data );
			} catch ( Exception $e ) {
				return;
			}

			$data['action'] = "wp_async_$this->action";
			$data['_nonce'] = $this->create_async_nonce();

			$this->_body_data = $data;

			if ( ! has_action( 'shutdown', array(
				$this,
				'launch_on_shutdown',
			) )
			) {
				add_action( 'shutdown', array( $this, 'launch_on_shutdown' ) );
			}
		}

		/**
		 * Launch the request on the WordPress shutdown hook
		 *
		 * On VIP we got into data races due to the postback sometimes completing
		 * faster than the data could propogate to the database server cluster.
		 * This made WordPress get empty data sets from the database without
		 * failing. On their advice, we're moving the actual firing of the async
		 * postback to the shutdown hook. Supposedly that will ensure that the
		 * data at least has time to get into the object cache.
		 *
		 * @uses $_COOKIE        To send a cookie header for async postback
		 * @uses apply_filters()
		 * @uses admin_url()
		 * @uses wp_remote_post()
		 */
		public function launch_on_shutdown() {
// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

    public function __construct($host)
    {
        $this->host = $host;
    }

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}

			$this->log->debug( 'Launching Async Task...' );

			if ( ! empty( $this->_body_data ) ) {
				$cookies = array();
				foreach ( $_COOKIE as $name => $value ) {
					$cookies[] = "$name=" . urlencode( is_array( $value ) ? serialize( $value ) : $value );
				}

				$request_args = array(
					'timeout'   => 0.01,
					'blocking'  => false,
					'sslverify' => apply_filters( 'https_local_ssl_verify', false ),
					'body'      => $this->_body_data,
					'headers'   => array(
						'cookie' => implode( '; ', $cookies ),
					),
				);

				$url = admin_url( 'admin-post.php' );

				$this->log->debug( "Posting URL $url..." );

				$result = wp_remote_post( $url, $request_args );

				if ( is_wp_error( $result ) ) {
					$this->log->error( 'Posting URL returned an error: ' . $result->get_error_message() );
				}
			}
		}

		/**
		 * Verify the postback is valid, then fire any scheduled events.
		 *
		 * @uses $_POST['_nonce']
		 * @uses is_user_logged_in()
		 * @uses add_filter()
		 * @uses wp_die()
		 */
		public function handle_postback() {
// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

    public function __construct($host)
    {
        $this->host = $host;
    }

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
			if ( isset( $_POST['_nonce'] ) && $this->verify_async_nonce( $_POST['_nonce'] ) ) {
0   == false // true
0   == null  // true
123 == false // false
123 == null  // false

// It is often better to use strict comparison
0 === false // false
0 === null  // false
				if ( ! is_user_logged_in() ) {
					$this->action = "nopriv_$this->action";
				}
				$this->run_action();
			}

			add_filter( 'wp_die_handler', function () {
				die();

			} );
			wp_die();
		}

		/**
		 * Create a random, one time use token.
		 *
		 * Based entirely on wp_create_nonce() but does not tie the nonce to the
		 * current logged-in user.
		 *
		 * @uses wp_nonce_tick()
		 * @uses wp_hash()
		 *
		 * @return string The one-time use token
		 */
		protected function create_async_nonce() {
			$action = $this->get_nonce_action();
			$i      = wp_nonce_tick();

			return substr( wp_hash( $i . $action . get_class( $this ), 'nonce' ), - 12, 10 );
		}

		/**
		 * Verify that the correct nonce was used within the time limit.
		 *
		 * @uses wp_nonce_tick()
		 * @uses wp_hash()
		 *
		 * @param string $nonce Nonce to be verified
		 *
		 * @return bool Whether the nonce check passed or failed
		 */
		protected function verify_async_nonce( $nonce ) {
			$action = $this->get_nonce_action();
			$i      = wp_nonce_tick();

			// Nonce generated 0-12 hours ago
			if ( substr( wp_hash( $i . $action . get_class( $this ), 'nonce' ), - 12, 10 ) == $nonce ) {

				return 1;
			}

			// Nonce generated 12-24 hours ago
			if ( substr( wp_hash( ( $i - 1 ) . $action . get_class( $this ), 'nonce' ), - 12, 10 ) == $nonce ) {

				return 2;
			}

			// Invalid nonce
			return false;
		}

		/**
		 * Get a nonce action based on the $action property of the class
		 *
		 * @return string The nonce action for the current instance
		 */
		protected function get_nonce_action() {
			$action = $this->action;
			if ( substr( $action, 0, 7 ) === 'nopriv_' ) {
				$action = substr( $action, 7 );
			}
			$action = "wp_async_$action";

			return $action;
		}

		/**
		 * Prepare any data to be passed to the asynchronous postback
		 *
		 * The array this function receives will be a numerically keyed array from
		 * func_get_args(). It is expected that you will return an associative array
		 * so that the $_POST values used in the asynchronous call will make sense.
		 *
		 * The array you send back may or may not have anything to do with the data
		 * passed into this method. It all depends on the implementation details and
		 * what data is needed in the asynchronous postback.
		 *
		 * Do not set values for 'action' or '_nonce', as those will get overwritten
		 * later in launch().
		 *
		 * @throws Exception If the postback should not occur for any reason
		 *
		 * @param array $data The raw data received by the launch method
		 *
		 * @return array The prepared data
		 */
		abstract protected function prepare_data( $data );

		/**
		 * Run the do_action function for the asynchronous postback.
		 *
		 * This method needs to fetch and sanitize any and all data from the $_POST
		 * superglobal and provide them to the do_action call.
		 *
		 * The action should be constructed as "wp_async_task_$this->action"
		 */
		abstract protected function run_action();

	}

}



1		<?php
2		/**
3		* Plugin Name: WP Asynchronous Tasks
4		* Version: 1.0
5		* Description: Creates an abstract class to execute asynchronous tasks
6		* Author: 10up, Eric Mann, Luke Gedeon, John P. Bloch
7		* License: MIT
8		*/
9
10		if ( ! class_exists( 'WP_Async_Task' ) ) {
11		abstract class WP_Async_Task {
12
13		/**
14		* Constant identifier for a task that should be available to logged-in users
15		*
16		* See constructor documentation for more details.
17		*/
18		const LOGGED_IN = 1;
19
20		/**
21		* Constant identifier for a task that should be available to logged-out users
22		*
23		* See constructor documentation for more details.
24		*/
25		const LOGGED_OUT = 2;
26
27		/**
28		* Constant identifier for a task that should be available to all users regardless of auth status
29		*
30		* See constructor documentation for more details.
31		*/
32		const BOTH = 3;
33
34		/**
35		* This is the argument count for the main action set in the constructor. It
36		* is set to an arbitrarily high value of twenty, but can be overridden if
37		* necessary
38		*
39		* @var int
40		*/
41		protected $argument_count = 20;
42
43		/**
44		* Priority to fire intermediate action.
45		*
46		* @var int
47		*/
48		protected $priority = 10;
49
50		/**
51		* @var string
52		*/
53		protected $action;
54
55		/**
56		* @var array
57		*/
58		protected $_body_data;
59
60		/**
61		* A {@link Wordlift_Log_Service} instance.
62		*
63		* @since 3.15.0
64		* @access private
65		* @var \Wordlift_Log_Service $log A {@link Wordlift_Log_Service} instance.
66		*/
67		private $log;
68
69		/**
70		* Constructor to wire up the necessary actions
71		*
72		* Which hooks the asynchronous postback happens on can be set by the
73		* $auth_level parameter. There are essentially three options: logged in users
74		* only, logged out users only, or both. Set this when you instantiate an
75		* object by using one of the three class constants to do so:
76		* - LOGGED_IN
77		* - LOGGED_OUT
78		* - BOTH
79		* $auth_level defaults to BOTH
80		*
81		* @throws Exception If the class' $action value hasn't been set
82		*
83		* @param int $auth_level The authentication level to use (see above)
84		*/
85		public function __construct( $auth_level = self::BOTH ) {
86
87		$this->log = Wordlift_Log_Service::get_logger( 'WP_Async_Task' );
88
89		if ( empty( $this->action ) ) {
90		throw new Exception( 'Action not defined for class ' . __CLASS__ );
91		}
92		add_action( $this->action, array(
93		$this,
94		'launch',
95		), (int) $this->priority, (int) $this->argument_count );
96		if ( $auth_level & self::LOGGED_IN ) {
97		add_action( "admin_post_wp_async_$this->action", array(
98		$this,
99		'handle_postback',
100		) );
101		}
102		if ( $auth_level & self::LOGGED_OUT ) {
103		add_action( "admin_post_nopriv_wp_async_$this->action", array(
104		$this,
105		'handle_postback',
106		) );
107		}
108		}
109
110		/**
111		* Add the shutdown action for launching the real postback if we don't
112		* get an exception thrown by prepare_data().
113		*
114		* @uses func_get_args() To grab any arguments passed by the action
115		*/
116		public function launch() {
117		$data = func_get_args();
118		try {
119		$data = $this->prepare_data( $data );
120		} catch ( Exception $e ) {
121		return;
122		}
123
124		$data['action'] = "wp_async_$this->action";
125		$data['_nonce'] = $this->create_async_nonce();
126
127		$this->_body_data = $data;
128
129		if ( ! has_action( 'shutdown', array(
130		$this,
131		'launch_on_shutdown',
132		) )
133		) {
134		add_action( 'shutdown', array( $this, 'launch_on_shutdown' ) );
135		}
136		}
137
138		/**
139		* Launch the request on the WordPress shutdown hook
140		*
141		* On VIP we got into data races due to the postback sometimes completing
142		* faster than the data could propogate to the database server cluster.
143		* This made WordPress get empty data sets from the database without
144		* failing. On their advice, we're moving the actual firing of the async
145		* postback to the shutdown hook. Supposedly that will ensure that the
146		* data at least has time to get into the object cache.
147		*
148		* @uses $_COOKIE To send a cookie header for async postback
149		* @uses apply_filters()
150		* @uses admin_url()
151		* @uses wp_remote_post()
152		*/
153		public function launch_on_shutdown() {
		0 ignored issues – show Coding Style introduced 2017-08-28 11:18 UTC by Report Bug Copy Issue Report `launch_on_shutdown` uses the super-global variable `$_COOKIE` which is generally not recommended. Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable: // Bad class Router { public function generate($path) { return $_SERVER['HOST'].$path; } } // Better class Router { private $host; public function __construct($host) { $this->host = $host; } public function generate($path) { return $this->host.$path; } } class Controller { public function myAction(Request $request) { // Instead of $page = isset($_GET['page']) ? intval($_GET['page']) : 1; // Better (assuming you use the Symfony2 request) $page = $request->query->get('page', 1); } } Loading history...
154
155		$this->log->debug( 'Launching Async Task...' );
156
157		if ( ! empty( $this->_body_data ) ) {
158		$cookies = array();
159		foreach ( $_COOKIE as $name => $value ) {
160		$cookies[] = "$name=" . urlencode( is_array( $value ) ? serialize( $value ) : $value );
161		}
162
163		$request_args = array(
164		'timeout' => 0.01,
165		'blocking' => false,
166		'sslverify' => apply_filters( 'https_local_ssl_verify', false ),
167		'body' => $this->_body_data,
168		'headers' => array(
169		'cookie' => implode( '; ', $cookies ),
170		),
171		);
172
173		$url = admin_url( 'admin-post.php' );
174
175		$this->log->debug( "Posting URL $url..." );
176
177		$result = wp_remote_post( $url, $request_args );
178
179		if ( is_wp_error( $result ) ) {
180		$this->log->error( 'Posting URL returned an error: ' . $result->get_error_message() );
181		}
182		}
183		}
184
185		/**
186		* Verify the postback is valid, then fire any scheduled events.
187		*
188		* @uses $_POST['_nonce']
189		* @uses is_user_logged_in()
190		* @uses add_filter()
191		* @uses wp_die()
192		*/
193		public function handle_postback() {
		0 ignored issues – show Coding Style introduced 2017-07-31 09:35 UTC by Report Bug Copy Issue Report `handle_postback` uses the super-global variable `$_POST` which is generally not recommended. Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable: // Bad class Router { public function generate($path) { return $_SERVER['HOST'].$path; } } // Better class Router { private $host; public function __construct($host) { $this->host = $host; } public function generate($path) { return $this->host.$path; } } class Controller { public function myAction(Request $request) { // Instead of $page = isset($_GET['page']) ? intval($_GET['page']) : 1; // Better (assuming you use the Symfony2 request) $page = $request->query->get('page', 1); } } Loading history...
194		if ( isset( $_POST['_nonce'] ) && $this->verify_async_nonce( $_POST['_nonce'] ) ) {
		0 ignored issues – show Bug Best Practice introduced 2017-07-12 13:33 UTC by Report Bug Copy Issue Report The expression `$this->verify_async_nonce($_POST['_nonce'])` of type `integer\|false` is loosely compared to `true`; this is ambiguous if the integer can be zero. You might want to explicitly use `!== null` instead. In PHP, under loose comparison (like `==`, or `!=`, or `switch` conditions), values of different types might be equal. For `integer` values, zero is a special case, in particular the following results might be unexpected: 0 == false // true 0 == null // true 123 == false // false 123 == null // false // It is often better to use strict comparison 0 === false // false 0 === null // false Loading history...
195		if ( ! is_user_logged_in() ) {
196		$this->action = "nopriv_$this->action";
197		}
198		$this->run_action();
199		}
200
201		add_filter( 'wp_die_handler', function () {
202		die();
		0 ignored issues – show Coding Style Compatibility introduced 2017-07-12 13:33 UTC by Report Bug Copy Issue Report The method `handle_postback()` contains an exit expression. An exit expression should only be used in rare cases. For example, if you write a short command line script. In most cases however, using an `exit` expression makes the code untestable and often causes incompatibilities with other libraries. Thus, unless you are absolutely sure it is required here, we recommend to refactor your code to avoid its usage. Loading history...
203		} );
204		wp_die();
205		}
206
207		/**
208		* Create a random, one time use token.
209		*
210		* Based entirely on wp_create_nonce() but does not tie the nonce to the
211		* current logged-in user.
212		*
213		* @uses wp_nonce_tick()
214		* @uses wp_hash()
215		*
216		* @return string The one-time use token
217		*/
218		protected function create_async_nonce() {
219		$action = $this->get_nonce_action();
220		$i = wp_nonce_tick();
221
222		return substr( wp_hash( $i . $action . get_class( $this ), 'nonce' ), - 12, 10 );
223		}
224
225		/**
226		* Verify that the correct nonce was used within the time limit.
227		*
228		* @uses wp_nonce_tick()
229		* @uses wp_hash()
230		*
231		* @param string $nonce Nonce to be verified
232		*
233		* @return bool Whether the nonce check passed or failed
234		*/
235		protected function verify_async_nonce( $nonce ) {
236		$action = $this->get_nonce_action();
237		$i = wp_nonce_tick();
238
239		// Nonce generated 0-12 hours ago
240	View Code Duplication	if ( substr( wp_hash( $i . $action . get_class( $this ), 'nonce' ), - 12, 10 ) == $nonce ) {
		0 ignored issues – show Duplication introduced 2017-07-12 13:33 UTC by Report Bug Copy Issue Report This code seems to be duplicated across your project. Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation. You can also find more detailed suggestions in the “Code” section of your repository. Loading history...
241		return 1;
242		}
243
244		// Nonce generated 12-24 hours ago
245	View Code Duplication	if ( substr( wp_hash( ( $i - 1 ) . $action . get_class( $this ), 'nonce' ), - 12, 10 ) == $nonce ) {
		0 ignored issues – show Duplication introduced 2017-07-12 13:33 UTC by Report Bug Copy Issue Report This code seems to be duplicated across your project. Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation. You can also find more detailed suggestions in the “Code” section of your repository. Loading history...
246		return 2;
247		}
248
249		// Invalid nonce
250		return false;
251		}
252
253		/**
254		* Get a nonce action based on the $action property of the class
255		*
256		* @return string The nonce action for the current instance
257		*/
258		protected function get_nonce_action() {
259		$action = $this->action;
260		if ( substr( $action, 0, 7 ) === 'nopriv_' ) {
261		$action = substr( $action, 7 );
262		}
263		$action = "wp_async_$action";
264
265		return $action;
266		}
267
268		/**
269		* Prepare any data to be passed to the asynchronous postback
270		*
271		* The array this function receives will be a numerically keyed array from
272		* func_get_args(). It is expected that you will return an associative array
273		* so that the $_POST values used in the asynchronous call will make sense.
274		*
275		* The array you send back may or may not have anything to do with the data
276		* passed into this method. It all depends on the implementation details and
277		* what data is needed in the asynchronous postback.
278		*
279		* Do not set values for 'action' or '_nonce', as those will get overwritten
280		* later in launch().
281		*
282		* @throws Exception If the postback should not occur for any reason
283		*
284		* @param array $data The raw data received by the launch method
285		*
286		* @return array The prepared data
287		*/
288		abstract protected function prepare_data( $data );
289
290		/**
291		* Run the do_action function for the asynchronous postback.
292		*
293		* This method needs to fetch and sanitize any and all data from the $_POST
294		* superglobal and provide them to the do_action call.
295		*
296		* The action should be constructed as "wp_async_task_$this->action"
297		*/
298		abstract protected function run_action();
299
300		}
301
302		}
303
304

insideout10 / wordlift-plugin

Push — master ( 5db201...585add )

WP_Async_Task A

Complexity

Size/Duplication

Coupling/Cohesion

Importance

9 Methods

How to fix Duplicated Code

Duplicated Code

Duplication Side-by-Side

Filter issues like