junaidbhura / composer-wp-pro-plugins

Composer installer for Pro WordPress plugins.
https://packagist.org/packages/junaidbhura/composer-wp-pro-plugins
MIT License
92 stars 28 forks source link

Polylang: Fail if we download the incorrect version #38

Closed jarstelfox closed 1 year ago

jarstelfox commented 2 years ago

We are seeing the api give us the most recent version, but this version broke our website.

We spent a lot of time running down the fact that the version we requested was not being downloaded.

https://polylang.pro?edd_action=get_version&license=REDACTED&item_name=Polylang+Pro&url=REDACTED&version=3.1.4

returns

{ "new_version": "3.2.3", "stable_version": "3.2.3", "name": "Polylang Pro", "slug": "polylang-pro", ... }

It even says it is installing 3.1.4 during the composer install step.

I would have liked it to fail loudly when this occurs.

mcaskill commented 1 year ago

This is a good idea. Would prevent some headaches I've encountered in the past. Can be applied to other plugins using Easy Digital Downloads.

junaidbhura commented 1 year ago

@mcaskill Thanks, could you elaborate what problems this could fix?

mcaskill commented 1 year ago

[…] could you elaborate what problems this could fix?

For example: say your project requires Polylang 3.3.0. Since Polylang's EDD only returns the latest version, by aborting when there's a version conflict, you prevent Composer from caching this new version (3.4.0) under the key constructed from the required/locked version (3.3.0). This saves you from having to delete the cached download and alerts you to the existance of an update (which could be breaking).

junaidbhura commented 1 year ago

I see thanks @mcaskill . @jarstelfox happy to merge this in once you make that update!

mcaskill commented 1 year ago

It should be noted that I have not tested this pull request yet.

mcaskill commented 1 year ago

Should the equivalent be applied to WP All Import / Export Pro (WpAiPro) or should that be a separate pull request?

mcaskill commented 1 year ago

I've been doing some tests and I've stumbled upon an edge case with the current logic.

Say the plugin's latest version is 3.3 but your package constraint is 3.3.0, the identical operator will return false.

The solution to that is to use composer/semver:

use Composer\Semver\Semver;

if ( empty( $response['download_link'] ) ) {
    return '';
}

if ( empty( $response['new_version'] ) ) {
    return '';
}

if ( ! Semver::satisfies( $response['new_version'], $this->version ) ) {
    return '';
}

return $response['download_link'];

I'm using Semver class since it has the widest feature support compared to Composer's Comparator::equalTo() or PHP's version_compare() which does not consider 1 and 1.0 and 1.0.0 as equal.

The composer/semver package is required by composer/composer but since junaidbhura/composer-wp-pro-plugins would be directly using it, we would have to add that package to the plugin's requirements.

junaidbhura commented 1 year ago

@mcaskill @jarstelfox Closing this PR in favor of #47