feat: expose closer for GetResponse
This is more meant as a discussion then necessarily for merging as is.
High level problem
Given that we return interfaces that dynamically pull data for some of the gateway.IPFSBackend requests (e.g. Get) the implementation servicing these requests might want to know when a given interface is done being used so it can deal with any wrap up related to the request. This is doable for most requests by overriding the Close () error function, however with Get this can't be done because there is no closer associated with the directoryMetadata.
Bonus problem
Additionally, even if directoryMetadata had a close function it wouldn't be possible for middleware to add in an additional close function to the loop (e.g. an implementation that wraps another implementations Get function and wants to add a close function).
Specific issue
Listing so as to allow addressing of https://en.wikipedia.org/wiki/XY_problem concerns, and encourage alternative solutions if anyone has a better idea.
In implementing a CAR (and blocks) retrieval based implementation of gateway.IPFSBackend I ended up (at least for now) doing this by:
- Asynchronously starting a fetch for the relevant blocks + CARs
- Using a BlocksGateway internally which is back by a blockservice
- Feeding the blocks received over the network into the blockservice
In order for the passed in blockservice to be able to get blocks from lots of different inbound graph requests it needs to communicate with those other requests. However, we don't want that communication to continue once the HTTP request is done. While I was able to overload Close() error for most functions I couldn't do anything for Get so I used runtime.SetFinalizer for now which isn't great https://github.com/ipfs/bifrost-gateway/pull/61#discussion_r1153489302.
Solution
The draft solution here is one of a family of related ones, but basically the idea is being able to set a close function that will get called when the response is done being used.
Alternatives
Assume the channel will either be closed due to finishing, or will have a context cancellation. Wait on a goroutine for directory listing closures. This would work, but not using a goroutine would be nice.
Also, either way we need to solve the bonus problem of allowing users access to the internals (e.g. exposing the fields, adding getters, adding modifer functions, etc.)
@lidel @hacdias thoughts, suggestions?
Codecov Report
Merging #241 (952f135) into main (6f324be) will increase coverage by
0.26%. The diff coverage is50.00%.
@@ Coverage Diff @@
## main #241 +/- ##
==========================================
+ Coverage 47.70% 47.96% +0.26%
==========================================
Files 266 269 +3
Lines 32950 32990 +40
==========================================
+ Hits 15718 15824 +106
+ Misses 15549 15500 -49
+ Partials 1683 1666 -17
| Impacted Files | Coverage Δ | |
|---|---|---|
| gateway/handler_unixfs_dir.go | 62.50% <0.00%> (-0.40%) |
:arrow_down: |
| gateway/handler_unixfs__redirects.go | 36.64% <33.33%> (-0.45%) |
:arrow_down: |
| gateway/gateway.go | 85.29% <57.14%> (-3.24%) |
:arrow_down: |
| gateway/handler_defaults.go | 31.92% <100.00%> (+0.41%) |
:arrow_up: |