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

Make Link implement Extractable #2930

Closed
wants to merge 1 commit into from

Conversation

@damiantq
Copy link

@damiantq damiantq commented Oct 25, 2019

Make Link implement Extractable interface so that it can be extracted with org.http4s.Headers#get(key: HeaderKey.Extractable).

As discussed on gitter:

Based on these references I could find:

The Link header field provides a means for serialising one or more
links into HTTP headers.

That's why it implements HeaderKey.Singleton, not HeaderKey.Recurring.

This algorithm parses zero or more comma-separated link-values from a
Link header field. Given a string field_value, assuming ASCII
encoding, it returns a list of link objects.

@RafalSumislawski
Copy link
Member

@RafalSumislawski RafalSumislawski commented Nov 8, 2019

Hi @rossabaker
I think the CI failure is just a flaky test. Could you trigger a rerun?

@hamnis
Copy link
Contributor

@hamnis hamnis commented Nov 13, 2019

Isn't this a duplicate of #2967 ?

Copy link
Member

@rossabaker rossabaker left a comment

Thanks for submitting this, and for your patience in the long overdue review.

I think Recurring is correct, because it supports comma-separated values, and such headers should be splittable into multiple headers. I think #2967 is correct. It doesn't look like the extraction is tested, so if the unit test could be adjusted to compile against the new model, and even exercise multiple links, I think that would still have value.

@hamnis
Copy link
Contributor

@hamnis hamnis commented Apr 28, 2020

@damiantq would you be interested in picking this up again with the changes suggested by @rossabaker ?

@rossabaker rossabaker changed the base branch from master to main Oct 22, 2020
@m-sp
Copy link
Member

@m-sp m-sp commented Mar 3, 2021

thanks again for your contribution @damiantq we recently revamped the header model were this was cleaned up as well

@m-sp m-sp closed this Mar 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants