Use Zend\Http\Client in Zend\Version

[gws]

With this update, we can perform version checks without relying on the
environment having allow_url_fopen = 1.

We can now optionally pass in a Zend\Http\Client instance.

Tests were enhanced to run in a separate process to avoid problems during online
tests where tests would not actually be run because the class would cache the
first response, and subsequent responses would use that cached response.

This addresses #4610

[gws]

Travis is failing on 5.3.3, but I think this is only because master is currently failing.

[tca3]

Like @weierophinney said, it'd be better to detect the ini settings before forcing an http client to be used. I suppose it would cover both people having allow_fopen_url = "1" but no cURL support and those with allow_fopen_url = "0" with cURL support.

Then I guess it would return false (and a warning/exception ?) if neither file_get_contents or cURL is available.

[gws]

He said

Alternately, or additionally, we should allow passing an HTTP client as a second argument to the method, which would allow you to continue to contact the service when the stream wrapper is unavailable.

This is "alternately," and it should cover everybody regardless of environment support.

[tca3]

Well the file_get_contents method "covered everybody" but yet didn't always work depending your ini settings. Here, I can inject a client if I have no wrapper but the other way around doesn't work.

So I'd still be screwed if I had no cURL support but a working wrapper, which IMHO is the case of more people.

I'd be more in favor of

if (ini_get('allow_url_fopen') === '1' && $client === null) { 
    file_get_contents()
} 
else { 
    set default HTTP client or use the one passed in the arguments
}

[gws]

Well the file_get_contents method "covered everybody" but yet didn't always work depending your ini settings.

So not everyone is covered - that's the reason for making this change in the first place. #4610

So I'd still be screwed if I had no cURL support but a working wrapper

How? Zend\Http\Client does not use cURL by default, it uses the Socket adapter, which should work for people without cURL (it should work for everyone).

[prolic]

Please use static instead of self.

[Baachi]

self make more sense, because this class is marked as final, which means that this class cannot be extended.

[tca3]

What I am saying is that issue #4610 is about handling the warning, not changing the implementation already in place. So the workflow would be: detect ini settings, trigger E_USER_WARNING if problem, then return false or proceed with Http\Client if it is set.

But if the client is relying on Socket, which I am not familiar with, then why not.

[gws]

So the workflow would be: detect ini settings, trigger E_USER_WARNING if problem, then return false or proceed with Http\Client if it is set.

I see what you're saying. My thinking is that if we're already introducing the dependency on Zend\Http\Client as a fallback, we might as well go all in, which has two advantages: simplifies internal logic (over checking environment settings) and doesn't introduce an easily-avoidable warning or a BC break (a new return value, say false, would be).

[tca3]

Makes sense, besides I just checked Socket and it seems to be handling everything internally already, so my bad :p . For the BC: it should have returned "not available" like it's done already.

[weierophinney]

I'm going to be up-front and say that I'm a bit hesitant to add a dependency on zend-http to zend-version. At best, it should be optional: if no HTTP client is passed to the getLatest(), it should try to use streams, and emit a warning if it cannot do so. It should not create an instance, but only use one that is passed to it. (On the flip side, if an HTTP client is passed, it should always be used.)

[gws]

@weierophinney Understood. Updated and tests added.

[mwillbanks]

Why did you change static:: to self:: ?

static:: provides more flexibility for extending classes.

[Baachi]

This class cannot be extended, its a final class.

[weierophinney]

Merged to develop for release with 2.3.0 (as it introduces new features).