Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for HTTP libraries (support PSR-7) #4

Open
lanthaler opened this issue Jun 8, 2012 · 18 comments
Open

Add support for HTTP libraries (support PSR-7) #4

lanthaler opened this issue Jun 8, 2012 · 18 comments

Comments

@lanthaler
Copy link
Owner

Currently file_get_contents is used to retrieve remote contexts and documents. This isn't really flexible and should sooner or later be replaced with a more sophisticated library.

Keep an eye on the PHP FIG Proposal for a HTTP Client interface.

@lanthaler
Copy link
Owner Author

The HTTP message interfaces (PSR-7) have now been released. Update the processor to make use of it.

@lanthaler lanthaler changed the title Add support for HTTP libraries Add support for HTTP libraries (support PSR-7) Jun 21, 2015
@cKlee
Copy link

cKlee commented Dec 29, 2015

To achieve this maybe http://httplug.io/ would be a good choice. As I understand the the docs an own client implementation is no longer necessary. Meaning a separate class CurlLoad like you mentioned in #68 is also unnecessary but a HTTPlug client-implementation will do this through composers require. Is this the way you want to go?

@lanthaler
Copy link
Owner Author

Indeed, HTTPlug looks very promising. There seem to exist adapters for Guzzle and cURL but not file_get_contents (which often is the only thing that works on cheap, shared hosting). Too bad the PHP-FIG couldn’t agree on a client interface yet… but given how simple HTTPlug’s HttpClient interface is, I think this is the way to go for now. Do you have some time to work on this?

@cKlee
Copy link

cKlee commented Dec 30, 2015

I'm on holiday for two weeks now. After this I will look into it, if you have no other schedule. I'm just wonder why they say file_get_content is not important php-http/httplug#32

@sagikazarmark
Copy link

Hey there,

HTTPlug developer here.

The fact that we marked it as not important doesn't mean you can't have an adapter for file_get_contents. We believe that it is not something which we can easily support and didn't really want to spend time with it. That said, if you would like to work on one, we will happily provide support if necessary.

Thanks for considering using HTTPlug.

@lanthaler
Copy link
Owner Author

Carsten, I doubt I’ll get to it before you. Would really appreciate if you could spend some cycles on it.

Thanks Márk for weighting in here. If we end up creating a file_get_contents adapter, where should it live? In one of our repos or directly under php-http?

@sagikazarmark
Copy link

@lanthaler it's up to you: if you want to contribute it to php-http, I can't see a reason why we couldn't host it, but it is not necessary to make it work. I guess we can add it to a third-party list if you decide to host it yourself.

@lanthaler
Copy link
Owner Author

OK, as it was raining, I had a couple of hours.. the result can be found at https://github.com/lanthaler/fgc-client :-)

The JsonLD library still needs to be refactored to use this. I have already started updating ml/iri to be PSR-7 conformant (https://github.com/lanthaler/IRI/tree/psr7)

@sagikazarmark
Copy link

You might also be interested in providing an UriFactory along with your IRI implementation.

@lanthaler
Copy link
Owner Author

Hmm… what depends on it? If it doesn’t provide a clear advantage, I would like to avoid adding a dependency on php-http/message-factory to ml/iri

@sagikazarmark
Copy link

The advantage: your package will work with HTTPlug out of the box. By adding additional Puli configuration, even the discovery layer will find your implementation and return the UriFactory to the user when an UriFactory is requested.

(You can imagine it as a universal dependency injection module which will just work anywhere)

@lanthaler
Copy link
Owner Author

Understood. My question was what in HTTPlug depends on UriFactory. At the moment nothing seems to depend on it and thus I find it hard to justify pulling in an additional dependency.

@sagikazarmark
Copy link

I understand the reasons, I think this is the same reason why Guzzle rejected to do so.

However, this way your library (and guzzle psr7) are not really interoperable, or at least not switchable. There is no way to make a library using this package implementation independent, since the construction logic is unique for each implementation (unless you provide something which standardizes construction logic: factory).

I think this is enough justification (and given the fact that you already rely on another contract (psr7), I don't think adding another interface package is a problem).

This is my opinion of course. No hard feelings if you disagree. 😄

@zobzn
Copy link
Contributor

zobzn commented Jul 29, 2016

The first step should be to support injection of the loader interface (#72)
And then it is already possible to substitute any convenient interface implementation (FileGetContentsLoader, CurlLoader, HttpPugLoader, Guzzle6Loader, etc.)

@sagikazarmark
Copy link

Not sure if I understand: you want another abstraction layer above httplug?

@zobzn
Copy link
Contributor

zobzn commented Jul 29, 2016

I want to be able to inject right now any other loader implementation instead of FileGetContentsLoader.
Maybe later it will be HTTPlug, maybe.
But the most important thing is that now i can not use anything other than the built-in FileGetContentsLoader.

lanthaler added a commit that referenced this issue Oct 10, 2016
This is a first step to add support for PSR-7 HTTP libraries (see #4).

Squashed commit of the following:

commit b682a81cf94e21a8a579ea9187a27a97a19476da
Author: Markus Lanthaler <mark_lanthaler@gmx.net>
Date:   Mon Oct 10 17:35:51 2016 +1300

    Various fixes

commit ab499e0011df1528ed7e009fbdb8c3fffff75e58
Author: zema <zlobzn@gmail.com>
Date:   Fri Jul 29 12:01:34 2016 +0300

    injection of RemoteDocumentLoader

commit 967c1a2593396cba470356e100af42366c48d1b1
Author: zema <zlobzn@gmail.com>
Date:   Fri Jul 29 11:57:19 2016 +0300

    injection of RemoteDocumentLoader

commit 252f65deaf9e258b8e573e18659b1d0403a45e74
Author: zema <zlobzn@gmail.com>
Date:   Fri Jul 29 11:52:17 2016 +0300

    injection of RemoteDocumentLoader

commit 86d04d2a1671e7e09e86689239a5f4c53735d6f8
Author: zema <zlobzn@gmail.com>
Date:   Fri Jul 29 10:23:13 2016 +0300

    injection of RemoteDocumentLoader

This closes #72.
@tomgillett
Copy link
Contributor

I've hacked together a proof-of-concept for a PSR-7 / PSR-17 / PSR-18 loader: tomgillett@df1cff3

@tomgillett
Copy link
Contributor

tomgillett commented Jun 15, 2020

Following on from conversation in #100

Absolutely. I started moving this into https://github.com/lanthaler/fgc-client a few years ago before those PSRs existed and used HTTPlug instead. If you are interested, we should update that package and then have JsonLD have depend on it and allow people to use other clients. The reason I like file_get_contents is that it supports both local and remote files and is available basically everywhere incl. the cheapest shared hosts.

I'd be happy to contribute some time to this. To explain what I'm looking to achieve...

I'm using this library within an application which integrates with several services via HTTP. If libraries support PSR-18 I can have my application provide a consistent client for all outbound HTTP requests. To me, this is better for maintenance and security and means I can add global configuration to that client if required (i.e. for an outbound proxy, for caching or for logging, etc.)

The alternative is an inconsisent mess - some libraries use file_get_contents, others cURL, others Guzzle - and I have little control over outbound traffic from my application.

I can achieve this now by writing my own implementation of ML\JsonLD\DocumentLoaderInterface (see proof of concept here: tomgillett@df1cff3) but native PSR-18 support feels cleaner to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants