Allow markdown files to read from the LFS

[zeripath]

This PR makes it possible for the markdown renderer to render images and media straight from the LFS.

Fix #5746

Signed-off-by: Andrew Thornton [email protected]

[codecov-io]

Codecov Report

Merging #5787 into master will increase coverage by 0.03%.
The diff coverage is 42.4%.

@@            Coverage Diff             @@
##           master    #5787      +/-   ##
==========================================
+ Coverage   38.83%   38.87%   +0.03%     
==========================================
  Files         344      345       +1     
  Lines       49393    49491      +98     
==========================================
+ Hits        19184    19239      +55     
- Misses      27440    27471      +31     
- Partials     2769     2781      +12

Impacted Files	Coverage Δ
modules/markup/markdown/markdown.go	73.14% <100%> (ø)	⬆️
routers/routes/routes.go	83.28% <100%> (+0.19%)	⬆️
routers/repo/view.go	46.22% <22.22%> (-1.09%)	⬇️
routers/repo/download.go	45.83% <30.95%> (-6.02%)	⬇️
modules/lfs/pointers.go	60.52% <60.52%> (ø)
modules/base/tool.go	75.2% <0%> (+1.07%)	⬆️
models/repo_list.go	64.55% <0%> (+1.26%)	⬆️
modules/lfs/content_store.go	53.12% <0%> (+1.56%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 296814e...94bfc5d. Read the comment docs.

[zeripath]

At present this is not a perfect implementation as it just redirects to the LFS Get which sets the content type as application/octet-stream and sets a filename to force a download. So /media URLs behave slightly different from /raw URLs. However it appears to solve the users issue Now /media actually serves the file and lets http set the content type.

[zeripath]

As there are no tests of this functionality at present I've marked this WIP until I've put some up one basic test implemented Have wired in tests in to TestGit. I think that should be enough.

[zeripath]

OK it appears that creating tests for this might be a bit more involved so I'm going to remove the WIP.

If proper integration tests are required please ask and I'll add some more.

More tests wired into TestGit .

[lunny]

@zeripath please add integration test

[zeripath]

Ok, basically the issue is that we don't have proper integration tests for LFS so I'll have to add those. This may broaden the scope the of this PR considerably as I suspect that a number of other issues will be found, for example, if we do any integration tests with gzip enabled then the issues that led to #5722 will manifest.

[techknowlogick]

Perhaps a different PR is needed for the tests (at least all the ones that will be successful) which will at least reduce the potential increase in size of this PR

[zeripath]

@techknowlogick I think that's the only sensible way to do it - I'm just loathe to make PR dependencies.

Our integration tests of the editor view are also somewhat lacking, as are those of the /raw endpoints and so on. This basically means that in order to get this 200 line change in, I need to write tests for at least 2000 lines of code.

I guess that's technical debt for you.

[zeripath]

Of course if #5722 was merged I'd actually have the beginnings of an LFS test framework to work with...

[zeripath]

OK the /raw and /media pathways are now checked in TestGit.

[zeripath]

I've removed the /wiki/media pathways as at present wiki files can't be placed in to the LFS and #5814 shows that a simple /wiki/media url route would need to reserve media from the wiki.

[zeripath]

@lunny is the change of name to lfs_pointers.go ok?

[zeripath]

Ping @lunny

[lunny]

@zeripath I think the file names could be more simpler as pointer.go and the methods could remove LFS since all these are on package lfs. When you use them, you will reference them as lfs.ReadLFSPointerFile so the duplicated lfs on method name is unnecessary.

[zeripath]

OK, I've done that.

[zeripath]

@lunny any more work needed on this?