Replace `repo.namedBlob` by `git.TreeEntry`. (#22898)

`namedBlob` turned out to be a poor imitation of a `TreeEntry`. Using
the latter directly shortens this code.

This partially undoes https://github.com/go-gitea/gitea/pull/23152/,
which I found a merge conflict with, and also expands the test it added
to cover the subtle README-in-a-subfolder case.

diff --git a/models/fixtures/repository.yml b/models/fixtures/repository.yml
index 32ba8744d4522f490b873df30fd1a6ca7f4bdf9b..4c888e47ed2c4745ba709753be872cfdfde8303a 100644 (file)
--- a/models/fixtures/repository.yml
+++ b/models/fixtures/repository.yml
@@ -25,7 +25,7 @@
   fork_id: 0
   is_template: false
   template_id: 0
-  size: 7028
+  size: 7320
   is_fsck_enabled: true
   close_issues_via_commit_in_any_branch: false
 

diff --git a/routers/web/repo/view.go b/routers/web/repo/view.go
index 8663e113820854d82bfee620d3215a948d9753af..4d49ab635952251d5a24bee0b3f2e7e74873b1df 100644 (file)
--- a/routers/web/repo/view.go
+++ b/routers/web/repo/view.go
@@ -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
@@ -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 {
@@ -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
@@ -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) {
@@ -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
@@ -270,13 +254,23 @@ 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
@@ -284,11 +278,11 @@ func renderReadmeFile(ctx *context.Context, readmeFile *namedBlob, readmeTreelin
        defer dataRc.Close()
 
        ctx.Data["FileIsText"] = fInfo.isTextFile
-       ctx.Data["FileName"] = readmeFile.name
+       ctx.Data["FileName"] = path.Join(subfolder, readmeFile.Name())
        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))
        }
 
@@ -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)
                        buf := &bytes.Buffer{}
                        ctx.Data["EscapeStatus"], _ = charset.EscapeControlStringReader(rd, buf, ctx.Locale)
                        ctx.Data["FileContent"] = buf.String()

diff --git a/tests/gitea-repositories-meta/user2/repo1.git/objects/16/633238d370a441f98dca532e4296a619c4c85f b/tests/gitea-repositories-meta/user2/repo1.git/objects/16/633238d370a441f98dca532e4296a619c4c85f
new file mode 100644 (file)
index 0000000..310f0ca
Binary files /dev/null and b/tests/gitea-repositories-meta/user2/repo1.git/objects/16/633238d370a441f98dca532e4296a619c4c85f differ

diff --git a/tests/gitea-repositories-meta/user2/repo1.git/objects/46/49299398e4d39a5c09eb4f534df6f1e1eb87cc b/tests/gitea-repositories-meta/user2/repo1.git/objects/46/49299398e4d39a5c09eb4f534df6f1e1eb87cc
new file mode 100644 (file)
index 0000000..b32e1d2
--- /dev/null
+++ b/tests/gitea-repositories-meta/user2/repo1.git/objects/46/49299398e4d39a5c09eb4f534df6f1e1eb87cc
@@ -0,0 +1,4 @@
+x\ 1\9d\8fQJÅ0\10EýÎ*f\ 3>¦\9d¤I@DÁ\87_ú!n`\9aL^\8bmòhS\1eîÞ ®ÀÏ\ 3÷^Î\re]ç
+½3wu\13\81n\18\88zr\91,²Ö]ò.\ 66Ô\8bîýÀCç\83\ eÎ$uåMr\ 5ëÒèÑ
+1\11zaÑI\\92\84\84Îê ã\98º\10\8d(>êT6x\9fÃ\17¼\1e­:É\ 6\ f¹áéò\87Oײ|¯u9\ 5~l"Öi$c\10îÑ ªðkZ[ëÿ\eêSö
+S¹ÁÇùùåí¼C;\11Ûä¼òEv¸M\92!\95#G\9830ìÇ\98ÊÒ\ 2ê\ay³] 
\ No newline at end of file

diff --git a/tests/gitea-repositories-meta/user2/repo1.git/refs/heads/sub-home-md-img-check b/tests/gitea-repositories-meta/user2/repo1.git/refs/heads/sub-home-md-img-check
new file mode 100644 (file)
index 0000000..dfe1105
--- /dev/null
+++ b/tests/gitea-repositories-meta/user2/repo1.git/refs/heads/sub-home-md-img-check
@@ -0,0 +1 @@
+4649299398e4d39a5c09eb4f534df6f1e1eb87cc

diff --git a/tests/integration/repo_test.go b/tests/integration/repo_test.go
index cef9dccf24f603e1303b79ea6cf8c6fd885889ba..40f5f56a35e2640fab76f10d91ca677dad931187 100644 (file)
--- a/tests/integration/repo_test.go
+++ b/tests/integration/repo_test.go
@@ -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")
@@ -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")
 }