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

feat(gateway): support for order=, dups= parameters from IPIP-412 #370

Merged
merged 3 commits into from
Jul 24, 2023

Conversation

hacdias
Copy link
Member

@hacdias hacdias commented Jun 22, 2023

@hacdias hacdias requested a review from lidel as a code owner June 22, 2023 12:22
@hacdias hacdias marked this pull request as draft June 22, 2023 12:22
@hacdias hacdias changed the base branch from main to issue/221 June 22, 2023 12:23
@codecov
Copy link

codecov bot commented Jun 22, 2023

Codecov Report

Merging #370 (c033fe5) into main (4b29eb0) will increase coverage by 0.18%.
The diff coverage is 91.50%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #370      +/-   ##
==========================================
+ Coverage   49.17%   49.36%   +0.18%     
==========================================
  Files         245      245              
  Lines       29812    29886      +74     
==========================================
+ Hits        14660    14753      +93     
+ Misses      13713    13695      -18     
+ Partials     1439     1438       -1     
Impacted Files Coverage Δ
gateway/handler_car.go 75.56% <85.00%> (+5.33%) ⬆️
gateway/blocks_backend.go 45.45% <100.00%> (+0.51%) ⬆️
gateway/gateway.go 93.75% <100.00%> (+1.19%) ⬆️
gateway/handler.go 77.93% <100.00%> (+0.23%) ⬆️

... and 7 files with indirect coverage changes

@hacdias hacdias force-pushed the ipip-412 branch 3 times, most recently from b28776f to 2f2e7f2 Compare June 22, 2023 13:51
@hacdias hacdias force-pushed the ipip-412 branch 2 times, most recently from 819c808 to cf9117b Compare June 23, 2023 09:28
@hacdias hacdias changed the title feat(gateway): ipip-412 feat(gateway): support for duplicates and order parameters from IPIP-412 Jun 23, 2023
@hacdias hacdias changed the title feat(gateway): support for duplicates and order parameters from IPIP-412 feat(gateway): support for order=, dups= parameters from IPIP-412 Jun 23, 2023
@hacdias hacdias self-assigned this Jun 23, 2023
@hacdias hacdias marked this pull request as ready for review June 23, 2023 09:58
@hacdias hacdias requested a review from a team as a code owner June 23, 2023 09:58
Base automatically changed from issue/221 to main June 27, 2023 10:00
@hacdias hacdias force-pushed the ipip-412 branch 2 times, most recently from 9b5f313 to 65cc6a8 Compare June 29, 2023 11:35
@lidel lidel mentioned this pull request Jul 5, 2023
@lidel lidel assigned lidel and unassigned hacdias Jul 6, 2023
@lidel
Copy link
Member

lidel commented Jul 13, 2023

Pushed a minor refactor in c6e10eb:

  • adds more type safety: DuplicateBlocksPolicy is no longer a pointer to a bool, but a real type wit h String() and Bool(), which simplified code a bit, and together with DagOrder both have an explicit *Unspecified state that also makes it easier to clearly define implicit default in a single place + find it if needed.
  • removes passing pointer and reliance on mutation. now mutation happens only once, in buildCarParams func, where implicit defaults are set when order or dups are unspecified in the request
  • moved car version validation and other parameter related logic into the same build funcs
  • fixed a bug where ?format=car eclipsed params from Accept header (would be a problem in many apps, as they often send both)

Will bubble up to Kubo and then add some tests in ipfs/gateway-conformance#87 tomorrow.

Copy link
Member

@lidel lidel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In case I don't get to this before you are back @hacdias, feel free to bubble this up to ipfs/kubo#9989 and ipfs/gateway-conformance#87

@lidel lidel mentioned this pull request Jul 19, 2023
hacdias and others added 3 commits July 24, 2023 10:34
- adds more type safety: DuplicateBlocksPolicy is no longer a pointer to
  bool, and together with DagOrder has an explicit *Unspecified state.
- removes passing pointer and reliance on mutation. now mutation
  happens only once, in buildCarParams func, where implicit defaults
  are set when order or dups are unspecified in the request
- moved car version validation and other parameter related logic
  into the same build funcs
- fixed a bug where ?format=car eclipsed params from Accept header
@hacdias hacdias enabled auto-merge (squash) July 24, 2023 08:34
@hacdias hacdias merged commit f6b448b into main Jul 24, 2023
@hacdias hacdias deleted the ipip-412 branch July 24, 2023 08:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Support IPIP-412
2 participants