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

Completed

Push — master ( ce5918...20c3c4 )

by David

created 2017-10-25 22:26 UTC

Wordlift_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
 */

abstract class Wordlift_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( get_class() );

		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_wl_async_$this->action", array(
				$this,
				'handle_postback',
			) );
		}
		if ( $auth_level & self::LOGGED_OUT ) {
			add_action( "admin_post_nopriv_wl_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'] = "wl_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 = get_site_url( null, 'wl-api' );

			$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 = "wl_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 "wl_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		abstract class Wordlift_Async_Task {
11
12		/**
13		* Constant identifier for a task that should be available to logged-in users
14		*
15		* See constructor documentation for more details.
16		*/
17		const LOGGED_IN = 1;
18
19		/**
20		* Constant identifier for a task that should be available to logged-out users
21		*
22		* See constructor documentation for more details.
23		*/
24		const LOGGED_OUT = 2;
25
26		/**
27		* Constant identifier for a task that should be available to all users regardless of auth status
28		*
29		* See constructor documentation for more details.
30		*/
31		const BOTH = 3;
32
33		/**
34		* This is the argument count for the main action set in the constructor. It
35		* is set to an arbitrarily high value of twenty, but can be overridden if
36		* necessary
37		*
38		* @var int
39		*/
40		protected $argument_count = 20;
41
42		/**
43		* Priority to fire intermediate action.
44		*
45		* @var int
46		*/
47		protected $priority = 10;
48
49		/**
50		* @var string
51		*/
52		protected $action;
53
54		/**
55		* @var array
56		*/
57		protected $_body_data;
58
59		/**
60		* A {@link Wordlift_Log_Service} instance.
61		*
62		* @since 3.15.0
63		* @access private
64		* @var \Wordlift_Log_Service $log A {@link Wordlift_Log_Service} instance.
65		*/
66		private $log;
67
68		/**
69		* Constructor to wire up the necessary actions
70		*
71		* Which hooks the asynchronous postback happens on can be set by the
72		* $auth_level parameter. There are essentially three options: logged in users
73		* only, logged out users only, or both. Set this when you instantiate an
74		* object by using one of the three class constants to do so:
75		* - LOGGED_IN
76		* - LOGGED_OUT
77		* - BOTH
78		* $auth_level defaults to BOTH
79		*
80		* @throws Exception If the class' $action value hasn't been set
81		*
82		* @param int $auth_level The authentication level to use (see above)
83		*/
84		public function __construct( $auth_level = self::BOTH ) {
85
86		$this->log = Wordlift_Log_Service::get_logger( get_class() );
87
88		if ( empty( $this->action ) ) {
89		throw new Exception( 'Action not defined for class ' . __CLASS__ );
90		}
91		add_action( $this->action, array(
92		$this,
93		'launch',
94		), (int) $this->priority, (int) $this->argument_count );
95		if ( $auth_level & self::LOGGED_IN ) {
96		add_action( "admin_post_wl_async_$this->action", array(
97		$this,
98		'handle_postback',
99		) );
100		}
101		if ( $auth_level & self::LOGGED_OUT ) {
102		add_action( "admin_post_nopriv_wl_async_$this->action", array(
103		$this,
104		'handle_postback',
105		) );
106		}
107		}
108
109		/**
110		* Add the shutdown action for launching the real postback if we don't
111		* get an exception thrown by prepare_data().
112		*
113		* @uses func_get_args() To grab any arguments passed by the action
114		*/
115		public function launch() {
116		$data = func_get_args();
117		try {
118		$data = $this->prepare_data( $data );
119		} catch ( Exception $e ) {
120		return;
121		}
122
123		$data['action'] = "wl_async_$this->action";
124		$data['_nonce'] = $this->create_async_nonce();
125
126		$this->_body_data = $data;
127
128		if ( ! has_action( 'shutdown', array(
129		$this,
130		'launch_on_shutdown',
131		) )
132		) {
133		add_action( 'shutdown', array( $this, 'launch_on_shutdown' ) );
134		}
135		}
136
137		/**
138		* Launch the request on the WordPress shutdown hook
139		*
140		* On VIP we got into data races due to the postback sometimes completing
141		* faster than the data could propogate to the database server cluster.
142		* This made WordPress get empty data sets from the database without
143		* failing. On their advice, we're moving the actual firing of the async
144		* postback to the shutdown hook. Supposedly that will ensure that the
145		* data at least has time to get into the object cache.
146		*
147		* @uses $_COOKIE To send a cookie header for async postback
148		* @uses apply_filters()
149		* @uses admin_url()
150		* @uses wp_remote_post()
151		*/
152		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...
153
154		$this->log->debug( 'Launching Async Task...' );
155
156		if ( ! empty( $this->_body_data ) ) {
157		$cookies = array();
158		foreach ( $_COOKIE as $name => $value ) {
159		$cookies[] = "$name=" . urlencode( is_array( $value ) ? serialize( $value ) : $value );
160		}
161
162		$request_args = array(
163		'timeout' => 0.01,
164		'blocking' => false,
165		'sslverify' => apply_filters( 'https_local_ssl_verify', false ),
166		'body' => $this->_body_data,
167		'headers' => array(
168		'cookie' => implode( '; ', $cookies ),
169		),
170		);
171
172		$url = get_site_url( null, 'wl-api' );
173
174		$this->log->debug( "Posting URL $url..." );
175
176		$result = wp_remote_post( $url, $request_args );
177
178		if ( is_wp_error( $result ) ) {
179		$this->log->error( 'Posting URL returned an error: ' . $result->get_error_message() );
180		}
181		}
182		}
183
184		/**
185		* Verify the postback is valid, then fire any scheduled events.
186		*
187		* @uses $_POST['_nonce']
188		* @uses is_user_logged_in()
189		* @uses add_filter()
190		* @uses wp_die()
191		*/
192		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...
193		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...
194		if ( ! is_user_logged_in() ) {
195		$this->action = "nopriv_$this->action";
196		}
197		$this->run_action();
198		}
199
200		add_filter( 'wp_die_handler', function () {
201		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...
202		} );
203		wp_die();
204		}
205
206		/**
207		* Create a random, one time use token.
208		*
209		* Based entirely on wp_create_nonce() but does not tie the nonce to the
210		* current logged-in user.
211		*
212		* @uses wp_nonce_tick()
213		* @uses wp_hash()
214		*
215		* @return string The one-time use token
216		*/
217		protected function create_async_nonce() {
218		$action = $this->get_nonce_action();
219		$i = wp_nonce_tick();
220
221		return substr( wp_hash( $i . $action . get_class( $this ), 'nonce' ), - 12, 10 );
222		}
223
224		/**
225		* Verify that the correct nonce was used within the time limit.
226		*
227		* @uses wp_nonce_tick()
228		* @uses wp_hash()
229		*
230		* @param string $nonce Nonce to be verified
231		*
232		* @return bool Whether the nonce check passed or failed
233		*/
234		protected function verify_async_nonce( $nonce ) {
235		$action = $this->get_nonce_action();
236		$i = wp_nonce_tick();
237
238		// Nonce generated 0-12 hours ago
239	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...
240		return 1;
241		}
242
243		// Nonce generated 12-24 hours ago
244	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...
245		return 2;
246		}
247
248		// Invalid nonce
249		return false;
250		}
251
252		/**
253		* Get a nonce action based on the $action property of the class
254		*
255		* @return string The nonce action for the current instance
256		*/
257		protected function get_nonce_action() {
258		$action = $this->action;
259		if ( substr( $action, 0, 7 ) === 'nopriv_' ) {
260		$action = substr( $action, 7 );
261		}
262		$action = "wl_async_$action";
263
264		return $action;
265		}
266
267		/**
268		* Prepare any data to be passed to the asynchronous postback
269		*
270		* The array this function receives will be a numerically keyed array from
271		* func_get_args(). It is expected that you will return an associative array
272		* so that the $_POST values used in the asynchronous call will make sense.
273		*
274		* The array you send back may or may not have anything to do with the data
275		* passed into this method. It all depends on the implementation details and
276		* what data is needed in the asynchronous postback.
277		*
278		* Do not set values for 'action' or '_nonce', as those will get overwritten
279		* later in launch().
280		*
281		* @throws Exception If the postback should not occur for any reason
282		*
283		* @param array $data The raw data received by the launch method
284		*
285		* @return array The prepared data
286		*/
287		abstract protected function prepare_data( $data );
288
289		/**
290		* Run the do_action function for the asynchronous postback.
291		*
292		* This method needs to fetch and sanitize any and all data from the $_POST
293		* superglobal and provide them to the do_action call.
294		*
295		* The action should be constructed as "wl_async_task_$this->action"
296		*/
297		abstract protected function run_action();
298
299		}
300
301

insideout10 / wordlift-plugin

Push — master ( ce5918...20c3c4 )

Wordlift_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