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

Replace repo.namedBlob by git.TreeEntry. #22898

Merged
merged 9 commits into from
Mar 15, 2023
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion models/fixtures/repository.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
fork_id: 0
is_template: false
template_id: 0
size: 7028
size: 7320
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This had to change due to the test additions I did. I used the same test repo (user2/repo1.git) that #23152 did, but I don't know why that PR didn't force changing this number.

is_fsck_enabled: true
close_issues_via_commit_in_any_branch: false

Expand Down
82 changes: 38 additions & 44 deletions routers/web/repo/view.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,6 @@ const (
tplMigrating base.TplName = "repo/migrate/migrating"
)

type namedBlob struct {
name string
isSymlink bool
blob *git.Blob
}

// locate a README for a tree in one of the supported paths.
//
// entries is passed to reduce calls to ListEntries(), so
Expand All @@ -64,14 +58,14 @@ type namedBlob struct {
// entries == ctx.Repo.Commit.SubTree(ctx.Repo.TreePath).ListEntries()
//
// FIXME: There has to be a more efficient way of doing this
func findReadmeFileInEntries(ctx *context.Context, entries []*git.TreeEntry) (*namedBlob, error) {
func findReadmeFileInEntries(ctx *context.Context, entries []*git.TreeEntry) (string, *git.TreeEntry, error) {
// Create a list of extensions in priority order
// 1. Markdown files - with and without localisation - e.g. README.en-us.md or README.md
// 2. Txt files - e.g. README.txt
// 3. No extension - e.g. README
exts := append(localizedExtensions(".md", ctx.Language()), ".txt", "") // sorted by priority
extCount := len(exts)
readmeFiles := make([]*namedBlob, extCount+1)
readmeFiles := make([]*git.TreeEntry, extCount+1)

docsEntries := make([]*git.TreeEntry, 3) // (one of docs/, .gitea/ or .github/)
for _, entry := range entries {
Expand All @@ -98,28 +92,21 @@ func findReadmeFileInEntries(ctx *context.Context, entries []*git.TreeEntry) (*n
}
if i, ok := util.IsReadmeFileExtension(entry.Name(), exts...); ok {
log.Debug("Potential readme file: %s", entry.Name())
if readmeFiles[i] == nil || base.NaturalSortLess(readmeFiles[i].name, entry.Blob().Name()) {
name := entry.Name()
isSymlink := entry.IsLink()
target := entry
if isSymlink {
var err error
target, err = entry.FollowLinks()
if readmeFiles[i] == nil || base.NaturalSortLess(readmeFiles[i].Name(), entry.Blob().Name()) {
if entry.IsLink() {
target, err := entry.FollowLinks()
if err != nil && !git.IsErrBadLink(err) {
return nil, err
}
}
if target != nil && (target.IsExecutable() || target.IsRegular()) {
readmeFiles[i] = &namedBlob{
name,
isSymlink,
target.Blob(),
return "", nil, err
} else if target != nil && (target.IsExecutable() || target.IsRegular()) {
readmeFiles[i] = entry
}
} else {
readmeFiles[i] = entry
}
}
}
}
var readmeFile *namedBlob
var readmeFile *git.TreeEntry
for _, f := range readmeFiles {
if f != nil {
readmeFile = f
Expand All @@ -140,20 +127,20 @@ func findReadmeFileInEntries(ctx *context.Context, entries []*git.TreeEntry) (*n
var err error
childEntries, err := subTree.ListEntries()
if err != nil {
return nil, err
return "", nil, err
}
readmeFile, err = findReadmeFileInEntries(ctx, childEntries)

subfolder, readmeFile, err := findReadmeFileInEntries(ctx, childEntries)
if err != nil && !git.IsErrNotExist(err) {
return nil, err
return "", nil, err
}
if readmeFile != nil {
readmeFile.name = subTreeEntry.Name() + "/" + readmeFile.name
break
return path.Join(subTreeEntry.Name(), subfolder), readmeFile, nil
}
}
}

return readmeFile, nil
return "", readmeFile, nil
}

func renderDirectory(ctx *context.Context, treeLink string) {
Expand All @@ -177,16 +164,13 @@ func renderDirectory(ctx *context.Context, treeLink string) {
return
}

readmeFile, err := findReadmeFileInEntries(ctx, entries)
subfolder, readmeFile, err := findReadmeFileInEntries(ctx, entries)
if err != nil {
ctx.ServerError("findReadmeFileInEntries", err)
return
}
if readmeFile == nil {
return
}

renderReadmeFile(ctx, readmeFile, fmt.Sprintf("%s/%s", treeLink, readmeFile.name))
renderReadmeFile(ctx, subfolder, readmeFile, treeLink)
}

// localizedExtensions prepends the provided language code with and without a
Expand Down Expand Up @@ -270,25 +254,35 @@ func getFileReader(repoID int64, blob *git.Blob) ([]byte, io.ReadCloser, *fileIn
return buf, dataRc, &fileInfo{st.IsText(), true, meta.Size, &meta.Pointer, st}, nil
}

func renderReadmeFile(ctx *context.Context, readmeFile *namedBlob, readmeTreelink string) {
func renderReadmeFile(ctx *context.Context, subfolder string, readmeFile *git.TreeEntry, readmeTreelink string) {
target := readmeFile
if readmeFile != nil && readmeFile.IsLink() {
target, _ = readmeFile.FollowLinks()
}
if target == nil {
// if findReadmeFile() failed and/or gave us a broken symlink (which it shouldn't)
// simply skip rendering the README
return
}

ctx.Data["RawFileLink"] = ""
ctx.Data["ReadmeInList"] = true
ctx.Data["ReadmeExist"] = true
ctx.Data["FileIsSymlink"] = readmeFile.isSymlink
ctx.Data["FileIsSymlink"] = readmeFile.IsLink()

buf, dataRc, fInfo, err := getFileReader(ctx.Repo.Repository.ID, readmeFile.blob)
buf, dataRc, fInfo, err := getFileReader(ctx.Repo.Repository.ID, target.Blob())
if err != nil {
ctx.ServerError("getFileReader", err)
return
}
defer dataRc.Close()

ctx.Data["FileIsText"] = fInfo.isTextFile
ctx.Data["FileName"] = readmeFile.name
ctx.Data["FileName"] = path.Join(subfolder, readmeFile.Name())
lunny marked this conversation as resolved.
Show resolved Hide resolved
ctx.Data["IsLFSFile"] = fInfo.isLFSFile

if fInfo.isLFSFile {
filenameBase64 := base64.RawURLEncoding.EncodeToString([]byte(readmeFile.name))
filenameBase64 := base64.RawURLEncoding.EncodeToString([]byte(readmeFile.Name()))
ctx.Data["RawFileLink"] = fmt.Sprintf("%s.git/info/lfs/objects/%s/%s", ctx.Repo.Repository.Link(), url.PathEscape(fInfo.lfsMeta.Oid), url.PathEscape(filenameBase64))
}

Expand All @@ -306,19 +300,19 @@ func renderReadmeFile(ctx *context.Context, readmeFile *namedBlob, readmeTreelin

rd := charset.ToUTF8WithFallbackReader(io.MultiReader(bytes.NewReader(buf), dataRc))

if markupType := markup.Type(readmeFile.name); markupType != "" {
if markupType := markup.Type(readmeFile.Name()); markupType != "" {
ctx.Data["IsMarkup"] = true
ctx.Data["MarkupType"] = markupType

ctx.Data["EscapeStatus"], ctx.Data["FileContent"], err = markupRender(ctx, &markup.RenderContext{
Ctx: ctx,
RelativePath: path.Join(ctx.Repo.TreePath, readmeFile.name), // ctx.Repo.TreePath is the directory not the Readme so we must append the Readme filename (and path).
URLPrefix: path.Dir(readmeTreelink),
RelativePath: path.Join(ctx.Repo.TreePath, readmeFile.Name()), // ctx.Repo.TreePath is the directory not the Readme so we must append the Readme filename (and path).
URLPrefix: path.Join(readmeTreelink, subfolder),
Metas: ctx.Repo.Repository.ComposeDocumentMetas(),
GitRepo: ctx.Repo.GitRepo,
}, rd)
if err != nil {
log.Error("Render failed for %s in %-v: %v Falling back to rendering source", readmeFile.name, ctx.Repo.Repository, err)
log.Error("Render failed for %s in %-v: %v Falling back to rendering source", readmeFile.Name(), ctx.Repo.Repository, err)
Copy link
Contributor Author

@kousu kousu Mar 10, 2023

Choose a reason for hiding this comment

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

Perhaps

Suggested change
log.Error("Render failed for %s in %-v: %v Falling back to rendering source", readmeFile.Name(), ctx.Repo.Repository, err)
log.Error("Render failed for %s in %-v: %v Falling back to rendering source", path.Join(subfolder, readmeFile.Name()), ctx.Repo.Repository, err)

?

buf := &bytes.Buffer{}
ctx.Data["EscapeStatus"], _ = charset.EscapeControlStringReader(rd, buf, ctx.Locale)
ctx.Data["FileContent"] = buf.String()
Expand Down
Binary file not shown.
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
x��QJ�0E��*f>���I@D��_�!n`�L^�m�hS�� ����^�e]�
�3wu�n�zr�,��]�.6ԋ���C��$u�Mr����
1za�I\����� 㘺�(>�T6x����:�����Oײ|�u9~l"�i$c�� ��kZ[���S�
S��������C;���Ev�M�!�#G�30�ǘ���y�]
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
4649299398e4d39a5c09eb4f534df6f1e1eb87cc
35 changes: 30 additions & 5 deletions tests/integration/repo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,7 @@ func TestViewRepoDirectoryReadme(t *testing.T) {
missing("symlink-loop", "/user2/readme-test/src/branch/symlink-loop/")
}

func TestMarkDownImage(t *testing.T) {
func TestMarkDownReadmeImage(t *testing.T) {
defer tests.PrepareTestEnv(t)()

session := loginUser(t, "user2")
Expand All @@ -371,13 +371,38 @@ func TestMarkDownImage(t *testing.T) {
resp := session.MakeRequest(t, req, http.StatusOK)

htmlDoc := NewHTMLParser(t, resp.Body)
_, exists := htmlDoc.doc.Find(`img[src="/user2/repo1/media/branch/home-md-img-check/test-fake-img.jpg"]`).Attr("src")
assert.True(t, exists, "Repo home page markdown image link check failed")
src, exists := htmlDoc.doc.Find(`.markdown img`).Attr("src")
assert.True(t, exists, "Image not found in README")
assert.Equal(t, src, "/user2/repo1/media/branch/home-md-img-check/test-fake-img.jpg")

req = NewRequest(t, "GET", "/user2/repo1/src/branch/home-md-img-check/README.md")
resp = session.MakeRequest(t, req, http.StatusOK)

htmlDoc = NewHTMLParser(t, resp.Body)
_, exists = htmlDoc.doc.Find(`img[src="/user2/repo1/media/branch/home-md-img-check/test-fake-img.jpg"]`).Attr("src")
assert.True(t, exists, "Repo src page markdown image link check failed")
src, exists = htmlDoc.doc.Find(`.markdown img`).Attr("src")
assert.True(t, exists, "Image not found in markdown file")
assert.Equal(t, src, "/user2/repo1/media/branch/home-md-img-check/test-fake-img.jpg")
}

func TestMarkDownReadmeImageSubfolder(t *testing.T) {
defer tests.PrepareTestEnv(t)()

session := loginUser(t, "user2")

// this branch has the README in the special docs/README.md location
req := NewRequest(t, "GET", "/user2/repo1/src/branch/sub-home-md-img-check")
resp := session.MakeRequest(t, req, http.StatusOK)

htmlDoc := NewHTMLParser(t, resp.Body)
src, exists := htmlDoc.doc.Find(`.markdown img`).Attr("src")
assert.True(t, exists, "Image not found in README")
assert.Equal(t, src, "/user2/repo1/media/branch/sub-home-md-img-check/docs/test-fake-img.jpg")

req = NewRequest(t, "GET", "/user2/repo1/src/branch/sub-home-md-img-check/docs/README.md")
resp = session.MakeRequest(t, req, http.StatusOK)

htmlDoc = NewHTMLParser(t, resp.Body)
src, exists = htmlDoc.doc.Find(`.markdown img`).Attr("src")
assert.True(t, exists, "Image not found in markdown file")
assert.Equal(t, src, "/user2/repo1/media/branch/sub-home-md-img-check/docs/test-fake-img.jpg")
}