Add Requests::has_capability() which can be used to determine if SSL is available.

[jrfnl]

Note: this is a recreation of PR #251, which was closed due to the branch rename and the remote no longer existing.
Please see PR #251 for the original discussion on the PR.

Add Requests::has_capability() which can be used to determine if SSL is available.

An example of it's usage: Requests::has_capability( array( Capability::SSL => true ) );

See #250 and #251

/cc @dd32

---

After review of the original PR, the following additional changes have been made and are included in this commit:

• Add a WpOrg\Requests\Capability interface which holds constants for the queryable capabilities.
• Split the Requests::get_transport() method in two:
  • Requests::get_transport_class() which will retrieve the fully qualified class name for a working transport and will cache the result.
  • Requests::get_transport() to get an instantiated instance of a working transport, same as before.
• Adjusting the Transport interface and classes implementing it, to take a $capabilities parameter for the test() method and use the constants from the Capability interface internally.
• Add tests for the new Requests::has_capabilities() method.
• Update the Transport related tests and mocks.
• Add a usage example to the prose documentation.

[schlessera]

Having a way to test for a capability makes sense, but the $capabilities array is not the best way to define what to actually test.

We'll rework this in the following way:

• Change the signature to has_capability( string $capability ).
• Create an interface Capability which contains constants for all the known capabilities (which seems to be only ssl right now). This will mean the usage looks like this: Requests::has_capability( Capability::SSL ).
• Document usage for has_capability() on where to find the list of supported capabilities.
• Add tests.

[schlessera]

While going through the code to make the changes I suggested above, I noticed an issue with the current implementation.

The thing is that the way this is implemented, it always triggers the transport constructor if it has the capabilities, only to then discard that transport (the check returns a boolean only).

While the Requests_Transport_fsockopen doesn't have a constructor at all, the Requests_Transport_cURL (which should be used in most cases) has a constructor that configure cURL and then create a new resource handle, only to then discard that resource handle again in the destructor.

I think this is extremely wasteful for such a simple check. Avoiding this will need a change to the Requests::get_transport() method as well.

[schlessera]

@jrfnl I can't set you as reviewer, as you've created the PR. Please review this latest change.

[jrfnl]

Gone through it in detail and left some comments and questions. I look forward to the next iteration.

[jrfnl]

Oh and one more question: should this method and this new class be mentioned anywhere in the docs/are there docs which need updating for the changes which will now be made ?

[schlessera]

@jrfnl This can be reviewed again.

[jrfnl]

The thing is that the way this is implemented, it always triggers the transport constructor if it has the capabilities, only to then discard that transport (the check returns a boolean only).

While the Requests_Transport_fsockopen doesn't have a constructor at all, the Requests_Transport_cURL (which should be used in most cases) has a constructor that configure cURL and then create a new resource handle, only to then discard that resource handle again in the destructor.

I think this is extremely wasteful for such a simple check. Avoiding this will need a change to the Requests::get_transport() method as well.

There may actually be good reason for this - I think this may be loosely related to #429.

[schlessera]

@jrfnl This should be ready for a new review.

[jrfnl]

@schlessera Gone through it with a fine toothcomb. Hope I caught everything. Nearly there! Thanks for all your work on this.

[jrfnl]

Nearly done, will fix up those last few things now.

[jrfnl]

Ready for merge IMO