search

lanthaler / JsonLD

JSON-LD processor for PHP
MIT License
335 stars 62 forks source link

Added support for the 'Link' header for content negotiation. #100

Closed tomgillett closed 4 years ago

tomgillett commented 4 years ago

schema.org recently dropped support for content negotiation. They instead adopted a Link header, with rel="alternate" and type="application/ld+json". This attempts to utilise that header.

See: https://github.com/schemaorg/schemaorg/issues/2578#issuecomment-632227864

tomgillett commented 4 years ago

@Sarke thanks for letting me know about https://github.com/schemaorg/schemaorg/issues/2595. I've tested and schema.org is now returning the correct content type (it wasn't yesterday). I've updated the PR accordingly, dropping application/octet-stream makes me much happier!

rvanlaak commented 4 years ago

This repo's latest commit is of November 2018, so I took the liberty to Tweet @lanthaler to try getting some traction here to get related fixes merged and released: https://twitter.com/Rvanlaak/status/1268460128963084288

tomgillett commented 4 years ago

If you have a little bit of extra time, it would be fantastic if you could also add a unit test for the Link header parsing.

@lanthaler I'm happy to do this.

Is there a reason why parseContextLinkHeaders() is private-scope? I'll need to change this to public in order to unit test.

A cleaner approach would be to leave it private (or protected?) and refactor the class to use an HTTP client (rather than file_get_contents) so we can use a mock client with response... but I think this would be a much bigger refactor.

I can see this was originally considered in https://github.com/lanthaler/JsonLD/issues/4. PSR-17 and PSR-18 might also be helpful?

tomgillett commented 4 years ago

I can see this was originally considered in #4. PSR-17 and PSR-18 might also be helpful?

Slightly off-topic, but I've put together a proof-of-concept for this https://github.com/tomgillett/JsonLD/commit/df1cff3b610a243cbba25988b25787e287a6b295

tomgillett commented 4 years ago

@lanthaler I think I've resolved that latest round of feedback. Sticking with your original parsing logic is the safer approach and so I'm happier where we've landed!

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'll pick this thread up over on https://github.com/lanthaler/JsonLD/issues/4 😊