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 Jetpack_XMLRPC_Server 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 Jetpack_XMLRPC_Server, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 6 | class Jetpack_XMLRPC_Server { |
||
| 7 | /** |
||
| 8 | * The current error object |
||
| 9 | */ |
||
| 10 | public $error = null; |
||
| 11 | |||
| 12 | /** |
||
| 13 | * The current user |
||
| 14 | */ |
||
| 15 | public $user = null; |
||
| 16 | |||
| 17 | /** |
||
| 18 | * Whitelist of the XML-RPC methods available to the Jetpack Server. If the |
||
| 19 | * user is not authenticated (->login()) then the methods are never added, |
||
| 20 | * so they will get a "does not exist" error. |
||
| 21 | */ |
||
| 22 | function xmlrpc_methods( $core_methods ) { |
||
| 69 | |||
| 70 | /** |
||
| 71 | * Whitelist of the bootstrap XML-RPC methods |
||
| 72 | */ |
||
| 73 | function bootstrap_xmlrpc_methods() { |
||
| 79 | |||
| 80 | function authorize_xmlrpc_methods() { |
||
| 85 | |||
| 86 | function remote_authorize( $request ) { |
||
| 121 | |||
| 122 | /** |
||
| 123 | * Verifies that Jetpack.WordPress.com received a registration request from this site |
||
| 124 | */ |
||
| 125 | function verify_registration( $data ) { |
||
| 128 | |||
| 129 | /** |
||
| 130 | * @return WP_Error|string secret_2 on success, WP_Error( error_code => error_code, error_message => error description, error_data => status code ) on failure |
||
| 131 | * |
||
| 132 | * Possible error_codes: |
||
| 133 | * |
||
| 134 | * verify_secret_1_missing |
||
| 135 | * verify_secret_1_malformed |
||
| 136 | * verify_secrets_missing: verification secrets are not found in database |
||
| 137 | * verify_secrets_incomplete: verification secrets are only partially found in database |
||
| 138 | * verify_secrets_expired: verification secrets have expired |
||
| 139 | * verify_secrets_mismatch: stored secret_1 does not match secret_1 sent by Jetpack.WordPress.com |
||
| 140 | * state_missing: required parameter of state not found |
||
| 141 | * state_malformed: state is not a digit |
||
| 142 | * invalid_state: state in request does not match the stored state |
||
| 143 | * |
||
| 144 | * The 'authorize' and 'register' actions have additional error codes |
||
| 145 | * |
||
| 146 | * state_missing: a state ( user id ) was not supplied |
||
| 147 | * state_malformed: state is not the correct data type |
||
| 148 | * invalid_state: supplied state does not match the stored state |
||
| 149 | */ |
||
| 150 | function verify_action( $params ) { |
||
| 151 | $action = $params[0]; |
||
| 152 | $verify_secret = $params[1]; |
||
| 153 | $state = isset( $params[2] ) ? $params[2] : ''; |
||
| 154 | |||
| 155 | if ( empty( $verify_secret ) ) { |
||
| 156 | return $this->error( new Jetpack_Error( 'verify_secret_1_missing', sprintf( 'The required "%s" parameter is missing.', 'secret_1' ), 400 ) ); |
||
| 157 | } else if ( ! is_string( $verify_secret ) ) { |
||
| 158 | return $this->error( new Jetpack_Error( 'verify_secret_1_malformed', sprintf( 'The required "%s" parameter is malformed.', 'secret_1' ), 400 ) ); |
||
| 159 | } else if ( empty( $state ) ) { |
||
| 160 | return $this->error( new Jetpack_Error( 'state_missing', sprintf( 'The required "%s" parameter is missing.', 'state' ), 400 ) ); |
||
| 161 | } else if ( ! ctype_digit( $state ) ) { |
||
| 162 | return $this->error( new Jetpack_Error( 'state_malformed', sprintf( 'The required "%s" parameter is malformed.', 'state' ), 400 ) ); |
||
| 163 | } |
||
| 164 | |||
| 165 | $secrets = Jetpack::get_secrets( $action, $state ); |
||
| 166 | |||
| 167 | if ( ! $secrets ) { |
||
| 168 | Jetpack::delete_secrets( $action, $state ); |
||
| 169 | return $this->error( new Jetpack_Error( 'verify_secrets_missing', 'Verification secrets not found', 400 ) ); |
||
| 170 | } |
||
| 171 | |||
| 172 | if ( is_wp_error( $secrets ) ) { |
||
| 173 | Jetpack::delete_secrets( $action, $state ); |
||
| 174 | return $this->error( new Jetpack_Error( $secrets->get_error_code(), $secrets->get_error_message(), 400 ) ); |
||
| 175 | } |
||
| 176 | |||
| 177 | if ( empty( $secrets['secret_1'] ) || empty( $secrets['secret_2'] ) || empty( $secrets['exp'] ) ) { |
||
| 178 | Jetpack::delete_secrets( $action, $state ); |
||
| 179 | return $this->error( new Jetpack_Error( 'verify_secrets_incomplete', 'Verification secrets are incomplete', 400 ) ); |
||
| 180 | } |
||
| 181 | |||
| 182 | if ( ! hash_equals( $verify_secret, $secrets['secret_1'] ) ) { |
||
| 183 | Jetpack::delete_secrets( $action, $state ); |
||
| 184 | return $this->error( new Jetpack_Error( 'verify_secrets_mismatch', 'Secret mismatch', 400 ) ); |
||
| 185 | } |
||
| 186 | |||
| 187 | Jetpack::delete_secrets( $action, $state ); |
||
| 188 | |||
| 189 | return $secrets['secret_2']; |
||
| 190 | } |
||
| 191 | |||
| 192 | /** |
||
| 193 | * Wrapper for wp_authenticate( $username, $password ); |
||
| 194 | * |
||
| 195 | * @return WP_User|bool |
||
| 196 | */ |
||
| 197 | function login() { |
||
| 214 | |||
| 215 | /** |
||
| 216 | * Returns the current error as an IXR_Error |
||
| 217 | * |
||
| 218 | * @return bool|IXR_Error |
||
| 219 | */ |
||
| 220 | function error( $error = null ) { |
||
| 238 | |||
| 239 | /* API Methods */ |
||
| 240 | |||
| 241 | /** |
||
| 242 | * Just authenticates with the given Jetpack credentials. |
||
| 243 | * |
||
| 244 | * @return string The current Jetpack version number |
||
| 245 | */ |
||
| 246 | function test_connection() { |
||
| 249 | |||
| 250 | function test_api_user_code( $args ) { |
||
| 292 | |||
| 293 | /** |
||
| 294 | * Disconnect this blog from the connected wordpress.com account |
||
| 295 | * @return boolean |
||
| 296 | */ |
||
| 297 | function disconnect_blog() { |
||
| 309 | |||
| 310 | /** |
||
| 311 | * Unlink a user from WordPress.com |
||
| 312 | * |
||
| 313 | * This will fail if called by the Master User. |
||
| 314 | */ |
||
| 315 | function unlink_user() { |
||
| 319 | |||
| 320 | /** |
||
| 321 | * Returns any object that is able to be synced |
||
| 322 | */ |
||
| 323 | function sync_object( $args ) { |
||
| 334 | |||
| 335 | /** |
||
| 336 | * Returns the home URL and site URL for the current site which can be used on the WPCOM side for |
||
| 337 | * IDC mitigation to decide whether sync should be allowed if the home and siteurl values differ between WPCOM |
||
| 338 | * and the remote Jetpack site. |
||
| 339 | * |
||
| 340 | * @return array |
||
| 341 | */ |
||
| 342 | function validate_urls_for_idc_mitigation() { |
||
| 348 | |||
| 349 | /** |
||
| 350 | * Returns what features are available. Uses the slug of the module files. |
||
| 351 | * |
||
| 352 | * @return array |
||
| 353 | */ |
||
| 354 | View Code Duplication | function features_available() { |
|
| 363 | |||
| 364 | /** |
||
| 365 | * Returns what features are enabled. Uses the slug of the modules files. |
||
| 366 | * |
||
| 367 | * @return array |
||
| 368 | */ |
||
| 369 | View Code Duplication | function features_enabled() { |
|
| 378 | |||
| 379 | function update_attachment_parent( $args ) { |
||
| 388 | |||
| 389 | function json_api( $args = array() ) { |
||
| 483 | } |
||
| 484 |
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: