Support EDD #45

drzraf · 2022-08-17T18:58:12Z

Fix #22 by allowing a package URL indirection to take place.

PR supports:

Any URL (not restricted to EDD)
Custom HTTP method
Custom HTTP request content-type (form-urlencoded, ...)
Response content-type=JSON
- Custom key
- Custom nested keys

May support out of the box (or with further tweaks):

Arbitrary headers (including User-Agent, not tested, see $event->HttpDownloader)
authentication (Basic auth) Depends upon $event->HttpDownloader. Standard mechanism apply. May be possible out of the box

Not supported (but could be implemented)

Other response content-type (XML, plain/text, text/html / regex...)
Content encodings (gzip, base64, ...)

Not supported:

Multiple redirects curl_setopt($curlHandle, CURLOPT_FOLLOWLOCATION, false)

Configuration:

This is the extra property (at the package-level):

		"extra": {
		    "private-composer-installer": {
			"indirection": {
			    "http": { "method": "POST"},
			    "parse": {"format": "json", "key": "download_link"}
			}
		    }

For nested keys, it could be "key": {"foo": {"bar": "download_link"}}

A workflow is like:

Call activate_license for a given website (only once)
/?edd_action=activate_license&license=0123456789abcdf&name=package-slug&url=https%3A//my.domain.tld
Set dist.url to https://package-provider.tld/edd_url?edd_action=get_version&license=0123456789abcdf&name=foobar&url=https%3A//my.domain.tld

(@junaidbhura? In case you think this PR, if generic enough, could make composer-wp-pro-plugins somehow redundant?)

szepeviktor · 2022-08-17T19:58:32Z

Hallo @drzraf! 👋🏻

The Gmail address you used in the commit is not registered here on GitHub.

szepeviktor · 2022-08-17T20:31:20Z

src/Plugin.php

+ /**
+ * Handle indirection process, like used by WP EDD
+ * The "indirection" property can contain:
+ * "http" and "ssl" object, as defined by https://github.com/composer/composer/blob/main/src/Composer/Util/Http/CurlDownloader.php
+ * "parse": {
+ * "format": "json" # only supported value
+ * "key": # A string (or an object for specifying nested keys to fetch the package download URL from
+ * }
+ */
+ public function fetchIndirection(PreFileDownloadEvent $event, string $url, array $extra): string
+ {
+ $options = [
+ 'http' => array_replace_recursive(['method' => 'GET'], $extra['indirection']['http'] ?? []),
+ 'ssl' => $extra['indirection']['ssl'] ?? []
+ ];
+
+ $data = $event->getHttpDownloader()->get($url, $options);
+ if ($extra['indirection']['parse']['format'] ?? false === 'json') {
+ $response = $data->decodeJson();
+ $key = $extra['indirection']['parse']['key'] ?? false;
+ if ($key) {
+ // Look for a succession of (nested) keys within a recursive array (from the JSON)
+ $jsonObj = $response;
+ do {
+ $index = is_string($key) ? $key : key($key);
+ if (! isset($jsonObj[$index])) {
+ break;
+ }
+ $jsonObj = $jsonObj[$index];
+ if (!is_array($key)) {
+ break;
+ }
+ $key = $key[$index] ?? false;
+ } while ($key);
+
+ return is_array($jsonObj) ? json_encode($jsonObj) : $jsonObj;
+ }
+
+ // format=json but no key specified (!)
+ return json_encode($response);
+ } else {
+ // raw HTML (future usage of regexp?)
+ return $data->getBody();
+ }
+ }


Suggested change

/**

* Handle indirection process, like used by WP EDD

* The "indirection" property can contain:

* "http" and "ssl" object, as defined by https://github.com/composer/composer/blob/main/src/Composer/Util/Http/CurlDownloader.php

* "parse": {

* "format": "json" # only supported value

* "key": # A string (or an object for specifying nested keys to fetch the package download URL from

* }

*/

public function fetchIndirection(PreFileDownloadEvent $event, string $url, array $extra): string

{

$options = [

'http' => array_replace_recursive(['method' => 'GET'], $extra['indirection']['http'] ?? []),

'ssl' => $extra['indirection']['ssl'] ?? []

];

$data = $event->getHttpDownloader()->get($url, $options);

if ($extra['indirection']['parse']['format'] ?? false === 'json') {

$response = $data->decodeJson();

$key = $extra['indirection']['parse']['key'] ?? false;

if ($key) {

// Look for a succession of (nested) keys within a recursive array (from the JSON)

$jsonObj = $response;

do {

$index = is_string($key) ? $key : key($key);

if (! isset($jsonObj[$index])) {

break;

}

$jsonObj = $jsonObj[$index];

if (!is_array($key)) {

break;

}

$key = $key[$index] ?? false;

} while ($key);

return is_array($jsonObj) ? json_encode($jsonObj) : $jsonObj;

}

// format=json but no key specified (!)

return json_encode($response);

} else {

// raw HTML (future usage of regexp?)

return $data->getBody();

}

}

/**

* Handle indirection process, like used by WP EDD

*

* The "indirection" property can contain:

* "http" and "ssl" object, as defined by https://github.com/composer/composer/blob/main/src/Composer/Util/Http/Curl

* "parse": {

* "format": "json" # only supported value

* "key": # A string (or an object for specifying nested keys to fetch the package download URL from

* }

*/

public function fetchIndirection(PreFileDownloadEvent $event, string $url, array $extra): string

{

$options = [

'http' => array_replace_recursive(['method' => 'GET'], $extra['indirection']['http'] ?? []),

'ssl' => $extra['indirection']['ssl'] ?? [],

];

$response = $event->getHttpDownloader()->get($url, $options);

if ($extra['indirection']['parse']['format'] ?? false !== 'json') {

// Raw HTML

// @TODO Future usage of regexp

return $response->getBody();

}

$responseObject = $response->decodeJson();

$key = $extra['indirection']['parse']['key'] ?? false;

if ($key === false) {

// format=json but no key specified

return json_encode($responseObject);

}

// Look for a succession of possibly nested keys within a recursive array from the JSON object

do {

$index = is_array($key) ? key($key) : $key;

if (! isset($responseObject[$index])) {

break;

}

// Go one level deeper

$responseObject = $responseObject[$index];

if (! is_array($key)) {

break;

}

$key = $key[$index] ?? false;

} while (is_array($key) || is_string($key));

return is_array($responseObject) ? json_encode($responseObject) : $responseObject;

}

What change is that?

It does bail out early thus reducing indentations and making it easier to read.

I leave it 100% up to you.

drzraf · 2022-08-17T21:00:10Z

The Gmail address you used in the commit is not registered here on GitHub.

Sounds like GH decided to drop my +foobar delimiter from my email thus "breaking" the reference of all my commit. Does it cause any actual trouble?

szepeviktor · 2022-08-17T21:09:02Z

Does it cause any actual trouble?

No. Not at all. This is just one of my policies.

ffraenz · 2022-08-20T09:43:24Z

Hi @drzraf! Thank you for your pull request. I like the direction in which this is going. Here are some general considerations:

People may use this Composer installer for multiple (unrelated) packages. So we need to find a way to target specific packages/dist URLs in extra.private-composer-installer.

What I have in mind is adding a hash suffix #config-1 to the dist URL that we may use to target a specific configuration in extra.private-composer-installer.configs (for lack of a better name). This way multiple packages (from the same vendor) could reuse the same config. I would keep a config generic to allow for future additions that may be unrelated to an indirection feature.

Example:
```
{
  "extra": {
    "private-composer-installer": {
      "configs": {
        "config-1": {
          "indirection": {
            "http": { "method": "POST"},
            "parse": { "format": "json", "key": "download_link" }
          }
        }
      }
    }
  }
}
```
I don't like the way you provide the key location in the JSON payload using an object structure. I would go with a simple dot-notation string e.g. foo.bar.0.download_url to specify it.
Keep in mind that dist URLs without any placeholders should be ignored by this plugin to not alter unrelated packages. An indirection call must only be made if an indirection is actually configured (I think that's not the case right now in your code).

drzraf · 2022-09-01T22:22:18Z

People may use this Composer installer for multiple (unrelated) packages. So we need to find a way to target specific packages/dist URLs in extra.private-composer-in

But if is that already the case if the extra property lives at the package-level ?

* I don't like the way you provide the `key` location in the JSON payload using an object structure. I would go with a simple dot-notation string e.g. `foo.bar.0.download_url` to specify it.

Neither do I, but would you tolerate a dependency on https://symfony.com/doc/current/components/property_access.html ?
Or prefer a custom accessor based on a dot notation ? Or something more powerful based on xpath or json-path?

Keep in mind that dist URLs without any placeholders should be ignored by this plugin to not alter unrelated packages. An indirection call must only be made if an indirection is actually configured (I think that's not the case right now in your code).

But an URL without placeholder may still legitimately make use of this feature, isn't?

Support EDD

0b19167

drzraf force-pushed the feature/edd branch from 8c0ce9a to 0b19167 Compare August 17, 2022 19:55

szepeviktor reviewed Aug 17, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support EDD #45

Support EDD #45

drzraf commented Aug 17, 2022 •

edited

szepeviktor commented Aug 17, 2022 •

edited

szepeviktor Aug 17, 2022

drzraf Aug 17, 2022

szepeviktor Aug 17, 2022

szepeviktor Aug 17, 2022

drzraf commented Aug 17, 2022

szepeviktor commented Aug 17, 2022

ffraenz commented Aug 20, 2022

drzraf commented Sep 1, 2022

Support EDD #45

Are you sure you want to change the base?

Support EDD #45

Conversation

drzraf commented Aug 17, 2022 • edited

szepeviktor commented Aug 17, 2022 • edited

szepeviktor Aug 17, 2022

Choose a reason for hiding this comment

drzraf Aug 17, 2022

Choose a reason for hiding this comment

szepeviktor Aug 17, 2022

Choose a reason for hiding this comment

szepeviktor Aug 17, 2022

Choose a reason for hiding this comment

drzraf commented Aug 17, 2022

szepeviktor commented Aug 17, 2022

ffraenz commented Aug 20, 2022

drzraf commented Sep 1, 2022

drzraf commented Aug 17, 2022 •

edited

szepeviktor commented Aug 17, 2022 •

edited