refactor(gateway): use a single path struct internally

[aschmahmann]

Builds off of #176 for separate review and merge.

Things that feel good about the PR:

• We are mostly passing a single type around instead of many
• Less reliance on variable naming conventions since we can rely on field names instead

Some things that don't feel great about this PR:

• Which fields in the struct are filled out at any given point in the code is implied rather than type checked
  • Could potentially be fixed by having more types as we progress from maybeMutable -> immutable -> resolved to last CID + path
• We use path.Resolved which isn't type checked as immutable (if we wanted we could switch to a finalCID and finalPathRemainder which would probably do the job)
• There's a bit of a disconnect around _redirects 200s where the finalResolvedPath includes following the redirect but the rest of the contentPathRequest struct is pre-redirect (not sure if this matters much, or if there's an obvious clean way to handle this other than introducing another type or passing more variables around)

Note: aside from this straight refactor there are some issues likely uncovered here (e.g. changing handleNonUnixFSRequestErrors to take a contentPath and choosing a consistent path type to report with).

[codecov]

Codecov Report

Merging #204 (629b022) into feat/gateway-refactor (5f7a833) will increase coverage by 0.07%.
The diff coverage is 42.03%.

@@                    Coverage Diff                    @@
##           feat/gateway-refactor     #204      +/-   ##
=========================================================
+ Coverage                  30.01%   30.09%   +0.07%     
=========================================================
  Files                        104      105       +1     
  Lines                      11756    11780      +24     
=========================================================
+ Hits                        3529     3545      +16     
- Misses                      7844     7851       +7     
- Partials                     383      384       +1     

Impacted Files	Coverage Δ
gateway/handler_block.go	0.00% <0.00%> (ø)
gateway/handler_car.go	0.00% <0.00%> (ø)
gateway/handler_codec.go	0.00% <0.00%> (ø)
gateway/handler_ipns_record.go	0.00% <0.00%> (ø)
gateway/handler_tar.go	0.00% <0.00%> (ø)
gateway/handler_unixfs__redirects.go	37.69% <23.07%> (-0.40%)	⬇️
gateway/handler_defaults.go	29.44% <57.14%> (ø)
gateway/handler_unixfs_dir.go	62.35% <60.71%> (-0.55%)	⬇️
gateway/handler.go	61.92% <61.11%> (+0.08%)	⬆️
gateway/handler_unixfs_file.go	65.75% <100.00%> (ø)
... and 1 more

[hacdias]

The changes look fine overall, but I share some of your sentiments. I would first like to ask why we do have 4 paths now. Before we had only 2, then on #176 we have 3. Why 4 now? Is it possible to remove any?

For example, do we need immutablePartiallyResolvedPath, or could we potentially just jump straight to finalResolvedPath?

[aschmahmann]

Before we had only 2

We had only two types before ipath.Path and ipath.Resolved.

then on #176 we have 3

In that PR there are 3 Go types, but 4 different kinds of paths being moved around:

• the requested (maybe mutable) path (ipath.Path)
• the immutable version of the requested path (ImmutablePath), which was added so we could use some type safety to make sure the data was actually immutable and not have to do type checks or wasted resolutions in IPFSBackend implementations
• maybeResolvedImPath, not a great name but takes into account that we have done some resolution of the path
• the final resolved path

Why 4 now? Is it possible to remove any?
For example, do we need immutablePartiallyResolvedPath, or could we potentially just jump straight to finalResolvedPath?

Nothing new here, but yes I think we could probably remove immutablePartiallyResolvedPath. You might have to do a bit of type massaging since ImmutablePath is not a ipath.Resolved and some functions need to take an ImmutablePath.

If you have a way you'd like to change ImmutablePath or the function signatures to make things better that's fine. Alternatively, you can probably just immutablePartiallyResolvedPath and use type conversions. I'd probably change finalResolvedPath to be two fields or a struct with CID + Remainder components though so you don't accidentally "unresolve" part of the path by passing resolvedPath.String().

[hacdias]

@lidel @aschmahmann is this something we still want to do, or can this be done in #198?

[lidel]

The amount of force pushes we did here makes it impossible to review 🙈
@aschmahmann do you remember which commits are relevant here, we can do interactiver rebase and drop everything else?

I think we probably want to park this until we have time for #198.

[aschmahmann]

closing this until we take a stab at #198