From fd3bd7041f7975c4243b64fbc0a23eb7e3525aea Mon Sep 17 00:00:00 2001 From: JakobDev Date: Mon, 3 Apr 2023 10:41:27 +0200 Subject: [PATCH 1/5] Make Release Download URLs predictable --- models/repo/attachment.go | 29 +++++++++++++---------- models/repo/release.go | 43 ++++++++++++++++++++++++++++++++++ routers/web/repo/attachment.go | 11 ++++++--- routers/web/repo/release.go | 11 +++++++++ routers/web/repo/repo.go | 2 +- 5 files changed, 80 insertions(+), 16 deletions(-) diff --git a/models/repo/attachment.go b/models/repo/attachment.go index cb05386d93871..2a471c2d35e15 100644 --- a/models/repo/attachment.go +++ b/models/repo/attachment.go @@ -18,17 +18,18 @@ import ( // Attachment represent a attachment of issue/comment/release. type Attachment struct { - ID int64 `xorm:"pk autoincr"` - UUID string `xorm:"uuid UNIQUE"` - RepoID int64 `xorm:"INDEX"` // this should not be zero - IssueID int64 `xorm:"INDEX"` // maybe zero when creating - ReleaseID int64 `xorm:"INDEX"` // maybe zero when creating - UploaderID int64 `xorm:"INDEX DEFAULT 0"` // Notice: will be zero before this column added - CommentID int64 - Name string - DownloadCount int64 `xorm:"DEFAULT 0"` - Size int64 `xorm:"DEFAULT 0"` - CreatedUnix timeutil.TimeStamp `xorm:"created"` + ID int64 `xorm:"pk autoincr"` + UUID string `xorm:"uuid UNIQUE"` + RepoID int64 `xorm:"INDEX"` // this should not be zero + IssueID int64 `xorm:"INDEX"` // maybe zero when creating + ReleaseID int64 `xorm:"INDEX"` // maybe zero when creating + UploaderID int64 `xorm:"INDEX DEFAULT 0"` // Notice: will be zero before this column added + CommentID int64 + Name string + DownloadCount int64 `xorm:"DEFAULT 0"` + Size int64 `xorm:"DEFAULT 0"` + CreatedUnix timeutil.TimeStamp `xorm:"created"` + CustomDownloadURL string `xorm:"-"` } func init() { @@ -57,7 +58,11 @@ func (a *Attachment) RelativePath() string { // DownloadURL returns the download url of the attached file func (a *Attachment) DownloadURL() string { - return setting.AppURL + "attachments/" + url.PathEscape(a.UUID) + if a.CustomDownloadURL == "" { + return setting.AppURL + "attachments/" + url.PathEscape(a.UUID) + } else { + return a.CustomDownloadURL + } } // _____ __ __ .__ __ diff --git a/models/repo/release.go b/models/repo/release.go index f7b24044b9c2c..f0daf7d6d5d00 100644 --- a/models/repo/release.go +++ b/models/repo/release.go @@ -7,6 +7,7 @@ package repo import ( "context" "fmt" + "net/url" "sort" "strconv" "strings" @@ -334,6 +335,20 @@ func (s releaseMetaSearch) Less(i, j int) bool { return s.ID[i] < s.ID[j] } +// InitReleaseRepo makes sure the Repo field of releases is not nil +func InitReleaseRepo(ctx context.Context, rels ...*Release) error { + var err error + for _, release := range rels { + if release.Repo == nil { + release.Repo, err = GetRepositoryByID(ctx, release.RepoID) + if err != nil { + return err + } + } + } + return err +} + // GetReleaseAttachments retrieves the attachments for releases func GetReleaseAttachments(ctx context.Context, rels ...*Release) (err error) { if len(rels) == 0 { @@ -372,6 +387,34 @@ func GetReleaseAttachments(ctx context.Context, rels ...*Release) (err error) { sortedRels.Rel[currentIndex].Attachments = append(sortedRels.Rel[currentIndex].Attachments, attachment) } + // Makes URL's predictable + for _, release := range rels { + // If we have no Repo, we don't need to execute this loop + if release.Repo == nil { + continue + } + + // Check if there are two or more attachments with the same name + isDoubled := false + foundNames := make(map[string]bool) + for _, attachment := range release.Attachments { + _, found := foundNames[attachment.Name] + if found { + isDoubled = true + break + } else { + foundNames[attachment.Name] = true + } + } + + // If the names unique, use the URL with the Name instead of the UUID + if !isDoubled { + for _, attachment := range release.Attachments { + attachment.CustomDownloadURL = release.Repo.HTMLURL() + "/releases/download/" + url.PathEscape(release.TagName) + "/" + url.PathEscape(attachment.Name) + } + } + } + return err } diff --git a/routers/web/repo/attachment.go b/routers/web/repo/attachment.go index c6d8828fac603..9fb9cb00bf90c 100644 --- a/routers/web/repo/attachment.go +++ b/routers/web/repo/attachment.go @@ -86,9 +86,9 @@ func DeleteAttachment(ctx *context.Context) { }) } -// GetAttachment serve attachments -func GetAttachment(ctx *context.Context) { - attach, err := repo_model.GetAttachmentByUUID(ctx, ctx.Params(":uuid")) +// GetAttachment serve attachments with the given UUID +func ServeAttachment(ctx *context.Context, uuid string) { + attach, err := repo_model.GetAttachmentByUUID(ctx, uuid) if err != nil { if repo_model.IsErrAttachmentNotExist(err) { ctx.Error(http.StatusNotFound) @@ -153,3 +153,8 @@ func GetAttachment(ctx *context.Context) { return } } + +// GetAttachment serve attachments +func GetAttachment(ctx *context.Context) { + ServeAttachment(ctx, ctx.Params(":uuid")) +} diff --git a/routers/web/repo/release.go b/routers/web/repo/release.go index 3ffadd34ace7b..b2a25a8e0f1ea 100644 --- a/routers/web/repo/release.go +++ b/routers/web/repo/release.go @@ -142,6 +142,11 @@ func releasesOrTags(ctx *context.Context, isTagList bool) { return } + if err = repo_model.InitReleaseRepo(ctx, releases...); err != nil { + ctx.ServerError("InitReleaseRepo", err) + return + } + if err = repo_model.GetReleaseAttachments(ctx, releases...); err != nil { ctx.ServerError("GetReleaseAttachments", err) return @@ -248,6 +253,12 @@ func SingleRelease(ctx *context.Context) { ctx.Data["Title"] = release.Title } + err = repo_model.InitReleaseRepo(ctx, release) + if err != nil { + ctx.ServerError("InitReleaseRepo", err) + return + } + err = repo_model.GetReleaseAttachments(ctx, release) if err != nil { ctx.ServerError("GetReleaseAttachments", err) diff --git a/routers/web/repo/repo.go b/routers/web/repo/repo.go index b4e7b5a46e2a7..f61e5981d4673 100644 --- a/routers/web/repo/repo.go +++ b/routers/web/repo/repo.go @@ -373,7 +373,7 @@ func RedirectDownload(ctx *context.Context) { return } if att != nil { - ctx.Redirect(att.DownloadURL()) + ServeAttachment(ctx, att.UUID) return } } From 0f135a31eb4caa8748e3dded5b5e332b05caf758 Mon Sep 17 00:00:00 2001 From: JakobDev Date: Mon, 3 Apr 2023 11:11:33 +0200 Subject: [PATCH 2/5] Make linter happy --- models/repo/attachment.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/models/repo/attachment.go b/models/repo/attachment.go index 2a471c2d35e15..93b83aae8a8db 100644 --- a/models/repo/attachment.go +++ b/models/repo/attachment.go @@ -58,11 +58,11 @@ func (a *Attachment) RelativePath() string { // DownloadURL returns the download url of the attached file func (a *Attachment) DownloadURL() string { - if a.CustomDownloadURL == "" { - return setting.AppURL + "attachments/" + url.PathEscape(a.UUID) - } else { + if a.CustomDownloadURL != "" { return a.CustomDownloadURL } + + return setting.AppURL + "attachments/" + url.PathEscape(a.UUID) } // _____ __ __ .__ __ From c35bfac6088f6f75f1967268c2aa4297924c0a0c Mon Sep 17 00:00:00 2001 From: silverwind Date: Mon, 3 Apr 2023 23:19:15 +0200 Subject: [PATCH 3/5] Update models/repo/release.go --- models/repo/release.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/models/repo/release.go b/models/repo/release.go index f0daf7d6d5d00..390933527a03f 100644 --- a/models/repo/release.go +++ b/models/repo/release.go @@ -395,12 +395,12 @@ func GetReleaseAttachments(ctx context.Context, rels ...*Release) (err error) { } // Check if there are two or more attachments with the same name - isDoubled := false + hasDuplicates := false foundNames := make(map[string]bool) for _, attachment := range release.Attachments { _, found := foundNames[attachment.Name] if found { - isDoubled = true + hasDuplicates = true break } else { foundNames[attachment.Name] = true @@ -408,7 +408,7 @@ func GetReleaseAttachments(ctx context.Context, rels ...*Release) (err error) { } // If the names unique, use the URL with the Name instead of the UUID - if !isDoubled { + if !hasDuplicates { for _, attachment := range release.Attachments { attachment.CustomDownloadURL = release.Repo.HTMLURL() + "/releases/download/" + url.PathEscape(release.TagName) + "/" + url.PathEscape(attachment.Name) } From 86298a6fc8578718db5d50c10a0296b263d2bac1 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 7 Apr 2023 17:05:23 +0800 Subject: [PATCH 4/5] Apply suggestions from code review Co-authored-by: a1012112796 <1012112796@qq.com> --- routers/web/repo/release.go | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/routers/web/repo/release.go b/routers/web/repo/release.go index b2a25a8e0f1ea..2a08265356a8c 100644 --- a/routers/web/repo/release.go +++ b/routers/web/repo/release.go @@ -142,9 +142,8 @@ func releasesOrTags(ctx *context.Context, isTagList bool) { return } - if err = repo_model.InitReleaseRepo(ctx, releases...); err != nil { - ctx.ServerError("InitReleaseRepo", err) - return + for _, release := range releases { + release.Repo = ctx.Repo.Repository } if err = repo_model.GetReleaseAttachments(ctx, releases...); err != nil { @@ -253,11 +252,7 @@ func SingleRelease(ctx *context.Context) { ctx.Data["Title"] = release.Title } - err = repo_model.InitReleaseRepo(ctx, release) - if err != nil { - ctx.ServerError("InitReleaseRepo", err) - return - } + release.Repo = ctx.Repo.Repository err = repo_model.GetReleaseAttachments(ctx, release) if err != nil { From 23b361290d4061a9de94557709f0d7d0023ea264 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 7 Apr 2023 17:06:19 +0800 Subject: [PATCH 5/5] Update models/repo/release.go --- models/repo/release.go | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/models/repo/release.go b/models/repo/release.go index 390933527a03f..95d7c8086bb13 100644 --- a/models/repo/release.go +++ b/models/repo/release.go @@ -335,20 +335,6 @@ func (s releaseMetaSearch) Less(i, j int) bool { return s.ID[i] < s.ID[j] } -// InitReleaseRepo makes sure the Repo field of releases is not nil -func InitReleaseRepo(ctx context.Context, rels ...*Release) error { - var err error - for _, release := range rels { - if release.Repo == nil { - release.Repo, err = GetRepositoryByID(ctx, release.RepoID) - if err != nil { - return err - } - } - } - return err -} - // GetReleaseAttachments retrieves the attachments for releases func GetReleaseAttachments(ctx context.Context, rels ...*Release) (err error) { if len(rels) == 0 {