Skip to content

Commit

Permalink
connection: Fix Phan issues and improve phpdocs in Client class (#37056)
Browse files Browse the repository at this point in the history
* Declare the shape of returned response arrays for better static
  analysis.
* Fix a bunch of other phpdocs to be more correct too.
* Remove some unnecessary boolean checks, that were confusing Phan into
  marking all the keys in arrays as being possibly-missing (cf. phan/phan#4846)
  * At least they're unnecessary based on the declared return values of
    stuff and a quick look through the code. If we really need them, we
    should be more specific about what we're checking for and we should
    always return the documented WP_Error instead of passing the
    weird-falsey-value along.

The most important fix here is getting the type of the `$body`
parameter to `wpcom_json_api_request_as_blog` right, which fixes
PhanTypeMismatchArgumentProbablyReal issues in a lot of callers.

Committed via a GitHub action: https://github.com/Automattic/jetpack/actions/runs/8821834019

Upstream-Ref: Automattic/jetpack@01d6d6d
  • Loading branch information
anomiex authored and matticbot committed Apr 24, 2024
1 parent b7c13b6 commit ab76a2d
Show file tree
Hide file tree
Showing 12 changed files with 210 additions and 191 deletions.
2 changes: 1 addition & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
"automattic/jetpack-composer-plugin": "^2.0.1",
"automattic/jetpack-config": "^2.0.1",
"automattic/jetpack-identity-crisis": "^0.18.3",
"automattic/jetpack-publicize": "^0.42.11",
"automattic/jetpack-publicize": "^0.42.12-alpha",
"automattic/jetpack-connection": "^2.7.3-alpha",
"automattic/jetpack-my-jetpack": "^4.22.2-alpha",
"automattic/jetpack-sync": "^2.13.0",
Expand Down
1 change: 1 addition & 0 deletions jetpack_vendor/automattic/jetpack-connection/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ This is an alpha version! The changes listed here are not final.

### Fixed
- Disconnect connection owner on removal.
- Improve phpdoc comments in Client class, and remove some unnecessary boolean checks.

## [2.7.2] - 2024-04-22
### Added
Expand Down
50 changes: 29 additions & 21 deletions jetpack_vendor/automattic/jetpack-connection/src/class-client.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@
use Automattic\Jetpack\Constants;
use WP_Error;

// `wp_remote_request` returns an array with a particular format.
'@phan-type _WP_Remote_Response_Array = array{headers:\WpOrg\Requests\Utility\CaseInsensitiveDictionary,body:string,response:array{code:int,message:string},cookies:\WP_HTTP_Cookie[],filename:?string,http_response:WP_HTTP_Requests_Response}';

/**
* The Client class that is used to connect to WordPress.com Jetpack API.
*/
Expand All @@ -19,9 +22,10 @@ class Client {
/**
* Makes an authorized remote request using Jetpack_Signature
*
* @param array $args the arguments for the remote request.
* @param array|String $body the request body.
* @param array $args the arguments for the remote request.
* @param array|string|null $body the request body.
* @return array|WP_Error WP HTTP response on success
* @phan-return _WP_Remote_Response_Array|WP_Error
*/
public static function remote_request( $args, $body = null ) {
if ( isset( $args['url'] ) ) {
Expand All @@ -36,7 +40,7 @@ public static function remote_request( $args, $body = null ) {
}

$result = self::build_signed_request( $args, $body );
if ( ! $result || is_wp_error( $result ) ) {
if ( is_wp_error( $result ) ) {
return $result;
}

Expand Down Expand Up @@ -65,12 +69,12 @@ public static function remote_request( $args, $body = null ) {
/**
* Adds authorization signature to a remote request using Jetpack_Signature
*
* @param array $args the arguments for the remote request.
* @param array|String $body the request body.
* @return WP_Error|array {
* @param array $args the arguments for the remote request.
* @param array|string|null $body the request body.
* @return WP_Error|array{url:string,request:array,auth:array} {
* An array containing URL and request items.
*
* @type String $url The request URL.
* @type string $url The request URL.
* @type array $request Request arguments.
* @type array $auth Authorization data.
* }
Expand Down Expand Up @@ -123,7 +127,7 @@ public static function build_signed_request( $args, $body = null ) {
$request = compact( 'method', 'body', 'timeout', 'redirection', 'stream', 'filename', 'sslverify' );

@list( $token_key, $secret ) = explode( '.', $token->secret ); // phpcs:ignore WordPress.PHP.NoSilencedErrors.Discouraged
if ( empty( $token ) || empty( $secret ) ) {
if ( ! $secret ) {
return new WP_Error( 'malformed_token' );
}

Expand All @@ -142,7 +146,7 @@ public static function build_signed_request( $args, $body = null ) {
if ( function_exists( 'wp_generate_password' ) ) {
$nonce = wp_generate_password( 10, false );
} else {
$nonce = substr( sha1( wp_rand( 0, 1000000 ) ), 0, 10 );
$nonce = substr( sha1( (string) wp_rand( 0, 1000000 ) ), 0, 10 );
}

// Kind of annoying. Maybe refactor Jetpack_Signature to handle body-hashing.
Expand Down Expand Up @@ -196,7 +200,7 @@ public static function build_signed_request( $args, $body = null ) {

$signature = $jetpack_signature->sign_request( $token_key, $timestamp, $nonce, $body_hash, $method, $url, $body, false );

if ( ! $signature || is_wp_error( $signature ) ) {
if ( is_wp_error( $signature ) ) {
return $signature;
}

Expand Down Expand Up @@ -233,10 +237,11 @@ public static function build_signed_request( $args, $body = null ) {
*
* @internal
*
* @param String $url the request URL.
* @param string $url the request URL.
* @param array $args request arguments.
* @param Boolean $set_fallback whether to allow flagging this request to use a fallback certficate override.
* @param boolean $set_fallback whether to allow flagging this request to use a fallback certficate override.
* @return array|WP_Error WP HTTP response on success
* @phan-return _WP_Remote_Response_Array|WP_Error
*/
public static function _wp_remote_request( $url, $args, $set_fallback = false ) { // phpcs:ignore PSR2.Methods.MethodDeclaration.Underscore
$fallback = \Jetpack_Options::get_option( 'fallback_no_verify_ssl_certs' );
Expand Down Expand Up @@ -316,8 +321,9 @@ public static function _wp_remote_request( $url, $args, $set_fallback = false )
/**
* Sets the time difference for correct signature computation.
*
* @param HTTP_Response $response the response object.
* @param Boolean $force_set whether to force setting the time difference.
* @param array $response Response array from `wp_remote_request`.
* @param bool $force_set whether to force setting the time difference.
* @phan-param _WP_Remote_Response_Array $response
*/
public static function set_time_diff( &$response, $force_set = false ) {
$code = wp_remote_retrieve_response_code( $response );
Expand Down Expand Up @@ -357,7 +363,7 @@ public static function set_time_diff( &$response, $force_set = false ) {
* @param array $args Arguments to {@see WP_Http}. Default is `array()`.
* @param string $base_api_path REST API root. Default is `wpcom`.
*
* @return array|WP_Error $response Response data, else {@see WP_Error} on failure.
* @return array Validated arguments.
*/
public static function validate_args_for_wpcom_json_api_request(
$path,
Expand Down Expand Up @@ -415,6 +421,7 @@ public static function validate_args_for_wpcom_json_api_request(
* @param string $base_api_path REST API root. Default is `wpcom`.
*
* @return array|WP_Error $response Response data, else {@see WP_Error} on failure.
* @phan-return _WP_Remote_Response_Array|WP_Error
*/
public static function wpcom_json_api_request_as_user(
$path,
Expand All @@ -440,12 +447,13 @@ public static function wpcom_json_api_request_as_user(
/**
* Query the WordPress.com REST API using the blog token
*
* @param String $path The API endpoint relative path.
* @param String $version The API version.
* @param array $args Request arguments.
* @param String $body Request body.
* @param String $base_api_path (optional) the API base path override, defaults to 'rest'.
* @param string $path The API endpoint relative path.
* @param string $version The API version.
* @param array $args Request arguments.
* @param array|string|null $body Request body.
* @param string $base_api_path (optional) the API base path override, defaults to 'rest'.
* @return array|WP_Error $response Data.
* @phan-return _WP_Remote_Response_Array|WP_Error
*/
public static function wpcom_json_api_request_as_blog(
$path,
Expand All @@ -472,7 +480,7 @@ public static function wpcom_json_api_request_as_blog(
* make sure that body hashes are made ith the string version, which is what will be seen after a
* server pulls up the data in the $_POST array.
*
* @param array|Mixed $data the data that needs to be stringified.
* @param mixed $data the data that needs to be stringified.
*
* @return array|string
*/
Expand Down
5 changes: 5 additions & 0 deletions jetpack_vendor/automattic/jetpack-jitm/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [3.1.8-alpha] - unreleased

This is an alpha version! The changes listed here are not final.

## [3.1.7] - 2024-04-22
### Changed
- Internal updates.
Expand Down Expand Up @@ -707,6 +711,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

- Update Jetpack to use new JITM package

[3.1.8-alpha]: https://github.com/Automattic/jetpack-jitm/compare/v3.1.7...v3.1.8-alpha
[3.1.7]: https://github.com/Automattic/jetpack-jitm/compare/v3.1.6...v3.1.7
[3.1.6]: https://github.com/Automattic/jetpack-jitm/compare/v3.1.5...v3.1.6
[3.1.5]: https://github.com/Automattic/jetpack-jitm/compare/v3.1.4...v3.1.5
Expand Down
2 changes: 1 addition & 1 deletion jetpack_vendor/automattic/jetpack-jitm/src/class-jitm.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
*/
class JITM {

const PACKAGE_VERSION = '3.1.7';
const PACKAGE_VERSION = '3.1.8-alpha';

/**
* The configuration method that is called from the jetpack-config package.
Expand Down
4 changes: 2 additions & 2 deletions jetpack_vendor/automattic/jetpack-my-jetpack/composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
"automattic/jetpack-assets": "^2.1.8",
"automattic/jetpack-boost-speed-score": "^0.3.11",
"automattic/jetpack-connection": "^2.7.3-alpha",
"automattic/jetpack-jitm": "^3.1.7",
"automattic/jetpack-jitm": "^3.1.8-alpha",
"automattic/jetpack-licensing": "^2.0.5",
"automattic/jetpack-plugins-installer": "^0.3.4",
"automattic/jetpack-redirect": "^2.0.1",
Expand All @@ -21,7 +21,7 @@
"yoast/phpunit-polyfills": "1.1.0",
"automattic/jetpack-changelogger": "^4.2.2",
"automattic/wordbless": "@dev",
"automattic/jetpack-videopress": "^0.23.16"
"automattic/jetpack-videopress": "^0.23.17-alpha"
},
"suggest": {
"automattic/jetpack-autoloader": "Allow for better interoperability with other plugins that use this package."
Expand Down
5 changes: 5 additions & 0 deletions jetpack_vendor/automattic/jetpack-publicize/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [0.42.12-alpha] - unreleased

This is an alpha version! The changes listed here are not final.

## [0.42.11] - 2024-04-22
### Changed
- Internal updates.
Expand Down Expand Up @@ -525,6 +529,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Updated package dependencies.
- Update package.json metadata.

[0.42.12-alpha]: https://github.com/Automattic/jetpack-publicize/compare/v0.42.11...v0.42.12-alpha
[0.42.11]: https://github.com/Automattic/jetpack-publicize/compare/v0.42.10...v0.42.11
[0.42.10]: https://github.com/Automattic/jetpack-publicize/compare/v0.42.9...v0.42.10
[0.42.9]: https://github.com/Automattic/jetpack-publicize/compare/v0.42.8...v0.42.9
Expand Down
8 changes: 4 additions & 4 deletions jetpack_vendor/i18n-map.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
),
'jetpack-connection' => array(
'path' => 'jetpack_vendor/automattic/jetpack-connection',
'ver' => '2.7.3-alpha1713964570',
'ver' => '2.7.3-alpha1713985059',
),
'jetpack-idc' => array(
'path' => 'jetpack_vendor/automattic/jetpack-identity-crisis',
Expand All @@ -38,15 +38,15 @@
),
'jetpack-jitm' => array(
'path' => 'jetpack_vendor/automattic/jetpack-jitm',
'ver' => '3.1.7',
'ver' => '3.1.8-alpha1713985059',
),
'jetpack-licensing' => array(
'path' => 'jetpack_vendor/automattic/jetpack-licensing',
'ver' => '2.0.5',
),
'jetpack-my-jetpack' => array(
'path' => 'jetpack_vendor/automattic/jetpack-my-jetpack',
'ver' => '4.22.2-alpha1713900720',
'ver' => '4.22.2-alpha1713985059',
),
'jetpack-password-checker' => array(
'path' => 'jetpack_vendor/automattic/jetpack-password-checker',
Expand All @@ -62,7 +62,7 @@
),
'jetpack-publicize-pkg' => array(
'path' => 'jetpack_vendor/automattic/jetpack-publicize',
'ver' => '0.42.11',
'ver' => '0.42.12-alpha1713985059',
),
'jetpack-sync' => array(
'path' => 'jetpack_vendor/automattic/jetpack-sync',
Expand Down

0 comments on commit ab76a2d

Please sign in to comment.