final attribute on the CURL class breaks caching plugin functionality.

[gnif]

As a hosting provider that provides tailored WordPress hosting for numerous clients we are often tasked with working around poorly written plugins and/or code that does not cache fetched results from external sources. As such we have developed a plugin that extends the Curl transport with a caching mechanism based on a whitelist of URLs.

As of the update to the 2.0.0 requests library, it is now impossible to do this anymore due to the use of the final keyword on the Curl class.

Here is how we were doing this:

  // register our transport wrapper
  Requests::add_transport('\HF\Cache\cURL');

  // force our transport to load first
  Requests::$transport[serialize(['ssl' => true ])] = '\HF\Cache\cURL';
  Requests::$transport[serialize(['ssl' => false])] = '\HF\Cache\cURL';
  Requests::$transport[serialize([]              )] = '\HF\Cache\cURL';

Our class would then override the request method and either inherit the default functionality or respond with a cached value:

class cURL extends \Requests_Transport_cURL
{
  public function request($url, $headers = array(), $data = array(), $options = array())
  {
    // custom caching logic here

    // inherit default logic
    return parent::request($url, $headers, $data, $options);
  }
}

Instead we now get the fatal error:

Class HF\Cache\cURL may not inherit from final class (WpOrg\Requests\Transport\Curl)

Please remove the final attribute from the Curl class.

[jrfnl]

@gnif Thanks for reporting this. To be honest, I don't see anything in the above issue which justifies the need to extend the Requests class.

Is there any particular reason why you are not using a code pattern along the lines of the below ?

class cURL_With_Caching
{
  private $curl;
  public function __construct() {
    $this->curl = new WpOrg\Requests\Transport\Curl();
  }
  public function request($url, $headers = array(), $data = array(), $options = array())
  {
    // custom caching logic here

    // use default logic
    return $this->curl->request($url, $headers, $data, $options);
  }
}

[gnif]

I had not considered this however it does seem like a bit of a hack as to remain compatible I would need to implement Requests\Transport and all three methods it exposes. While these would be single lines and simple to do, the entire point of extending a class is to add extra functionality and/or inherit the original code, wrapping it as you suggest seems like a step backwards.

Edit: Note I am happy with the proposed solution, it just feels backwards to me.

[jrfnl]

@gnif An alternative might be to not add a new transport at all, but to hook into Requests via the hooking mechanism ? Would the requests.before_request hook suit your needs ?

https://requests.ryanmccue.info/docs/hooks.html

[gnif]

No because the hooking mechanism doesn't allow for overriding and/or preventing the default behaviour of the request. The entire point of this plugin is to use and return a locally cached request if one exists instead of performing an external request again.

See: https://github.com/WordPress/Requests/blob/5a44c925f68ba0bc5f3af867055b4ea936342d12/src/Requests.php#L456 https://github.com/WordPress/Requests/blob/5a44c925f68ba0bc5f3af867055b4ea936342d12/src/Hooks.php#L93

[Art4]

@gnif I'd like to recommend you this explanation, why you should implement the Transport interface instead of using extends.

https://ocramius.github.io/blog/when-to-declare-classes-final/

[gnif]

To be frank I feel that all proposed solutions here are not ideal (including mine). Even if I implement the Transport interface I still have to use the hack I demonstrated in the original post here to force my transport to load first.

Like I said, I am happy with this solution, but IMO it's still not great.

If there is no desire to make any changes as a result of this issue, feel free to close it. I appreciate the effort those have put in here to assist me.