Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Release version 2.0.0 #627

Merged
merged 411 commits into from
Nov 24, 2021
Merged

Release version 2.0.0 #627

merged 411 commits into from
Nov 24, 2021

Conversation

jrfnl
Copy link
Member

@jrfnl jrfnl commented Nov 15, 2021

PR for tracking changes for the 2.0.0 release. Target release date: Tue November 16 2021. Release date: Wed November 24 2021

Closes #460

schlessera and others added 30 commits September 17, 2021 11:37
This method does not throw.
... which was clearly unfinished.
The PHP Curl extension worked with a Curl resource in PHP 4/5/7. As of PHP 8.0, `curl_init()` returns an instance of `CurlHandle` instead of a resource.

This updates the documentation to indicate that either a `resource` or `CurlHandle` instance can be expected in certain places in the code base (and that both are supported).

Ref: https://www.php.net/manual/en/migration80.incompatible.php#migration80.incompatible.resource2object
... to allow for standardizing the messages thrown via this Exception.

Note: the `parent::__construct()` method is not overloaded with a `private` version, so variation is still possible, just not recommended.

Includes adding a perfunctory unit test for the new method.
The default supported transports do not change during the request, so should be a constant and as the minimum PHP version is now PHP 5.6, we can use constant arrays.

I've elected to set this array with the same value for key and value to allow using `isset()` against the array.
I've also adjusted the `Requests::add_transport()` method to set the key as well, which automatically allows us to get rid of the expensive and unnecessary `array_merge()`.

The list of supported constants was also used in multiple (duplicate) data providers in the tests.
This duplication has been removed by adding this data provider to the base `TestCase`, making it available to all tests.

The data provider in the base `TestCase`:
* Makes use of the new constant to always provide the list of all supported transport classes.
* Has been set up to provide named data sets to allow for more descriptive tests when running the tests using `--testdox` and for easier debugging.
... to encapsulate Port numbers and allow for retrieving a port number based on a supported request type.

Includes unit tests.
1. APIGen 4.x does not support "modern" PHP, and that includes such things as the PHP 5.5 `::class` syntax.
2. There is an APIGen 5.x RC available, but that version is in its current state is no longer installable.
3. APIGen has not seen a release in over four years, nor a commit in more than three years and is currently unmaintained.

With that in mind, I've taken the decision to switch to the actively maintained phpDocumentor library to generate the API documentation.

This commit actions that decision by:
* Adding a base `phpdoc.dist.xml` configuration file.
* Adding a separate `composer.json` file for use when generating the API docs.
    The `require-dev` will pull in the phpDocumentor Phar via the shim package rather than install the individual dependencies.
* Adding this file and the `build` directory to the `.gitattributes` file for ignoring in distribution zips
* Adding the structure cache directory which will be used and the config overload file to the `.gitignore` file.

In a sister-PR to the `gh-pages` branch, the old `apigen.neon` config file will be removed.

Refs:
* https://github.com/ApiGen/ApiGen/releases
* https://tomasvotruba.com/blog/2017/09/04/how-apigen-survived-its-own-death/
* https://www.phpdoc.org/
* https://github.com/phpDocumentor/phpDocumentor/releases
* https://github.com/phpDocumentor/shim
This commit adds a script which can be run both locally as well as in a GH Actions workflow.

The script:
* Will retrieve the version number of the latest Requests release from the GH API.
* If no `phpdoc.xml` file exists, it will create a `phpdoc.xml` file based on the existing `phpdoc.dist.xml` file and make sure the title contains the correct version number.
* If a `phpdoc.xml` already exists, it will just and only update the title for the docs with the correct version number.

Includes a minor tweak to the PHP `lint` workflow to exclude the scripts in the `build/ghpages` directory from linting on PHP < 7.2.

Open question: once Requests 2.0.0 has been released, should we update the website to only contain the API docs for Requests 2.0.0 or should we keep a copy of the last Requests 1.x docs available as well ?
This commit adds a script which can be run both locally as well as in a GH Actions workflow.

The script:
* Will take the `README.md` file from the main repo as well as the `md` files in the `/docs/` directory and prepare them for use in the GH Pages website.
    This preparation includes:
    - Adding frontmatter to the files for use by Jekyll.
    - Replacing `md` in links with `html`.
    - Various other tweaks depending on the file.
* The script will, by default, place the adjusted files in the `build/ghpages/artifacts` directory. For local runs where there is already a separate clone of this repo available which has the `gh-pages` branch checked out, a `--target=path/to/ghpages` CLI argument can be passed to place the files directory in the `gh-pages` clone.

Includes a minor tweaks to the `docs/README.md` file for easier processing of that file.

This script effectively replaces the `build.py` script which is in the `gh-pages` branch. That script will be removed in a sister-PR to that branch.
... to the new directory containing the scripts documenting how these scripts can be run locally to test what changes would be made when the website would be updated.
This new workflow will run when:
* A new release of Requests has been published.
* A pull request has been send in which updates the scripts which are involved in updating the website.

The workflow can also be manually triggered, but will not run in forks of this repo (as they can't update or publish the website).

:point_right: Open question: should the workflow always run for pushes to `stable` ?
Those will normally be the release pushes and the "next" version nr may not be known yet in that case, but if something is "off" with the website, an update to regen the website would now require a new tag (or manual trigger, but we'll have to see how well that works for this).

The script will:
* In the `prepare` job:
    - (Re-)Generate the API docs using phpDocumentor.
    - Prepare the `README` and the files in the `docs` directory for use in the website.
    - Upload the results as an artifact.
        The artifact will only be available for one day as it's only used in the next job.
* In the `createpr` job:
    - Download the artifact.
    - Remove outdated files and replace them with the newly generated ones.
        Note: files can be updated, as well as added/removed, so for the commits we need to make sure that both tracked as well as untracked changes are committed.
    - Create a PR to the `gh-pages` branch containing two commits:
        1. A commit for the API docs update.
            This commit _may_ be empty if no inline docs have been updated, though I suspect that will be exceedingly rare.
        2. A commit containing the remaining changes (docs + homepage).

For PRs created because the workflow or the associated scripts have changed, the PR title will be prefixed with `[TEST | DO NOT MERGE]` so the effect of workflow/script changes can still be verified, without updating the website with the docs for an unreleased `develop` version of Requests.
For those PRs, the website change PR will be based on the feature branch against `develop` as otherwise the action doesn't have access to the latest version of the workflow/scripts. For releases and manual triggering, the website change PR will always be based on the `stable` branch.

Note: these steps could be combined into one job, but for debugging, it felt cleaner to separate it into two jobs.

:point_right: Note: we'll need to monitor if a PR correctly gets created when there are no changes to the prose docs (committed via the `create-pr` action), but there are API doc changes (committed manually), I hope it will be, but we'll have to see.

Refs:
* https://github.com/marketplace/actions/create-pull-request
* https://github.com/peter-evans/create-pull-request/blob/main/docs/concepts-guidelines.md
* Add docblocks to the test methods.
* Add `@covers` tags.
* Use more specific assertions.
* Add the `$message` parameter to each assertion to allow for distinguishing which one failed more easily.
* Use named data sets and indexed array items in the data provider.
... grouping tests using the same data provider together, with the data provider they are using directly below it.
* Add docblock documentation to the tests and the data provider.
* Use same parameter names in the test method, as used in the functions under test.
* Rename the data provider method to match the test name(s) it applies to.
* Remove `static` keyword from data provider.
* Used named datasets and keyed data entries in the data provider
jrfnl and others added 7 commits November 15, 2021 16:32
While the documented parameter type for the `$dictionary` parameter was `array`, in reality, the only real requirement is that the parameter is iterable, so adding input validation to match the real requirement.

Includes adding perfunctory tests for the new exception + for the method itself which was otherwise untested.
The documented accepted parameter type for both these methods is `string`, but no input validation was done on the parameter, which could lead to various PHP errors, most notably a "passing null to non-nullable" deprecation notice on PHP 8.1.

This commit adds input validation to the `Requests::decompress()` and the `Requests::compatible_gzinflate()` methods, allowing only for strings.
Note: Stringable objects will not be considered as valid input for these methods.

Includes adding perfunctory tests for the new exceptions.
Docs/Advanced usage: update docs to provide context about ca fallback

Co-authored-by: jrfnl <[email protected]>
* Update the PHPCS ruleset to enforce short arrays.
* Update the complete codebase to comply.
To prevent "dead links" from search engines and otherwise, the _old_ documentation will remain in the `/api/` directory and the _new_ documentation will be placed in the _new_ `/api-2.x/` directory.

Includes updating links in the prose docs.
@jrfnl jrfnl added this to the 2.0.0 milestone Nov 15, 2021
jrfnl and others added 15 commits November 16, 2021 06:04
This commit adds input validation to the `Curl::request()` and the `Fsockopen::request()` methods along the same lines as added to `Requests::request()`.

* The `$url` parameter is validated to allow for a string or Stringable (Iri) object.
* The `$headers` parameter is validated to allow only an array for consistency between both transport classes.
* The `$data` parameter already had (looser) input validation. This has been tightened up to only allow for the declared supported types, array and string (and null).
* The `$options` parameter is validated to allow only an array - same as in `Requests::request()`.

Includes:
* Adding perfunctory tests for the new exceptions.
* Adjusting one existing test to ensure that passing the `$url` as a stringable object is safeguarded.
* Adjusting the existing tests for the `$data` parameter validation to be in line with the new validation.
This commit adds input validation to the `Curl::request_multiple()` and the `Fsockopen::request_multiple()` methods along the same lines as added to `Requests::request_multiple()`.

* For `$requests`, arrays and iterable objects with array access should be accepted based on how the parameter is used within this class.
* The `$options` parameter is validated to allow only an array - same as in `Requests::request()`.

Includes aligning the behaviour when receiving an empty `$requests` array between the `Curl` and the `Fsockopen` classes and returning an empty array in that case.

Includes adding perfunctory tests for the new exceptions.
Updated with the latest version as per Tue Oct 26 03:12:05 2021 GMT.
Source: https://curl.se/docs/caextract.html

This update is a plain include and does not follow the previous philosophy of not  always removing expired certificates straight away.
The builds against PHP 8.1 are all green at this time and the general release is only two weeks away, so let's just keep it that way and not allow failures against PHP 8.1 anymore.

Note: I'm not adding PHP 8.2 yet at this time as IMO it is too early to start testing against that version. Let's leave that till May 2022 or something.
Skip alternate port test while portquiz has issues
This prevents the `Upgrading to Requests 2.0` page from always being bold in the submenu on the website.
* Updates the version constant in `src/Requests.php` for the v2.0.0 release.
* Adds a changelog for the v2.0.0 release.

Co-authored-by: Alain Schlesser <[email protected]>
Co-authored-by: jrfnl <[email protected]>
@jrfnl jrfnl marked this pull request as ready for review November 24, 2021 14:03
@schlessera schlessera merged commit 7ef0774 into stable Nov 24, 2021
@jrfnl
Copy link
Member Author

jrfnl commented Nov 24, 2021

@jrfnl
Copy link
Member Author

jrfnl commented Nov 24, 2021

Related WP Core ticket: https://core.trac.wordpress.org/ticket/54504

@jrfnl
Copy link
Member Author

jrfnl commented Nov 26, 2021

Text used for the "Month in WordPress" submission:

Requests 2.0.0 has been released.

The Requests project - a dependency of WP Core - was adopted into the WordPress organisation earlier this year.
Since then a lot of work has been done to modernize the code base and improve stability.

One of the biggest changes is that all code within Requests is now namespaced.
Now before you get your knickers in a twist: Requests 2.0.0 contains a full backward-compatibility layer and all old class name references will still work for the lifetime of Request 2.x and 3.x. A deprecation warning will be thrown when using the old class names, but that notice can be silenced (in Requests 2.x).

Another big change is that all typical entry point methods for Requests will now validate the parameter input received. If Requests was being used as documented, this will be unnoticeable, however, for invalid input, catchable and informative Exceptions will now be thrown.

While this may not seem like something worthy of a mention in Month in WordPress, the significance of this is that the Requests 2.0.0 release showcases how a legacy codebase can be modernized and made more stable and secure without breaking backward-compatibility and with that, it could be regarded as a proof of concept for a way forward for WordPress itself. So let's see what the future will hold...

Requests 2.0.0 will be included in the upcoming WordPress 5.9 release and is fully compatible with PHP 8.0 and 8.1.

For plugins/themes which directly use Requests, bypassing the WP Core function wrappers, it is highly recommended to read the release notes and the published upgrade guide.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants