From 2e6c39bfb480dda8f19a62e6a8d5bdade458fddb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?erg=C3=BC?= Date: Wed, 10 May 2017 03:55:50 +0200 Subject: [PATCH 01/10] Added URL mapping for Release attachments like on github.com --- models/attachment.go | 31 ++++++++++++++++++++++++++++ routers/repo/repo.go | 35 ++++++++++++++++++++++++++++++++ routers/routes/routes.go | 4 ++++ templates/repo/release/list.tmpl | 22 ++++++++++++-------- 4 files changed, 83 insertions(+), 9 deletions(-) diff --git a/models/attachment.go b/models/attachment.go index d800a47109da7..497f7dbc92d99 100644 --- a/models/attachment.go +++ b/models/attachment.go @@ -58,6 +58,16 @@ func (a *Attachment) IncreaseDownloadCount() error { return nil } +// FileSize is returning the file datasize +func (a *Attachment) FileSize() (string, error) { + stats, err := os.Stat(AttachmentLocalPath(a.UUID)) + if err != nil { + return "error", fmt.Errorf("AttachmentFileSize: %v", err) + } + result := float64(stats.Size()) / float64(1048576) + return fmt.Sprintf("%.1f", result) + " MB", nil +} + // AttachmentLocalPath returns where attachment is stored in local file // system based on given UUID. func AttachmentLocalPath(uuid string) string { @@ -126,6 +136,11 @@ func GetAttachmentByUUID(uuid string) (*Attachment, error) { return getAttachmentByUUID(x, uuid) } +// GetAttachmentByReleaseIDFileName returns attachment by given releaseId and fileName. +func GetAttachmentByReleaseIDFileName(releaseID int64, fileName string) (*Attachment, error) { + return getAttachmentByReleaseIDFileName(x, releaseID, fileName) +} + func getAttachmentsByIssueID(e Engine, issueID int64) ([]*Attachment, error) { attachments := make([]*Attachment, 0, 10) return attachments, e.Where("issue_id = ? AND comment_id = 0", issueID).Find(&attachments) @@ -142,6 +157,22 @@ func GetAttachmentsByCommentID(commentID int64) ([]*Attachment, error) { return attachments, x.Where("comment_id=?", commentID).Find(&attachments) } +/* Author : github.com/gdeverlant + getAttachmentByReleaseIDFileName return a file based on the the following infos: + - releaseID + - fileName +*/ +func getAttachmentByReleaseIDFileName(e Engine, releaseID int64, fileName string) (*Attachment, error) { + attach := &Attachment{ReleaseID: releaseID, Name: fileName} + has, err := e.Get(attach) + if err != nil { + return nil, err + } else if !has { + return nil, ErrAttachmentNotExist{0, attach.UUID} + } + return attach, nil +} + // DeleteAttachment deletes the given attachment and optionally the associated file. func DeleteAttachment(a *Attachment, remove bool) error { _, err := DeleteAttachments([]*Attachment{a}, remove) diff --git a/routers/repo/repo.go b/routers/repo/repo.go index 9b411648c61cd..79c3b60cfcb0f 100644 --- a/routers/repo/repo.go +++ b/routers/repo/repo.go @@ -268,6 +268,41 @@ func Action(ctx *context.Context) { ctx.Redirect(redirectTo) } +// RedirectDownload return a file based on the the following infos: +func RedirectDownload(ctx *context.Context) { + var ( + vTag = ctx.Params("vTag") + fileName = ctx.Params("fileName") + ) + if len(vTag) > 0 { + fmt.Println("Value of vTag %v", vTag) + } + if len(fileName) > 0 { + fmt.Println("Value of fileName %v", fileName) + } + + var tagNames []string + tagNames = append(tagNames, vTag) + + curRepo := ctx.Repo.Repository + + releases, err := models.GetReleasesByRepoIDAndNames(curRepo.ID, tagNames) + if err != nil { + ctx.Handle(500, "RedirectDownload -> Release not found", err) + return + } + + if len(releases) == 1 { + release := releases[0] + att, err := models.GetAttachmentByReleaseIDFileName(release.ID, fileName) + if err != nil { + ctx.Handle(500, "RedirectDownload -> Attachment not found", err) + return + } + ctx.Redirect(setting.AppSubURL + "/attachments/" + att.UUID) + } +} + // Download download an archive of a repository func Download(ctx *context.Context) { var ( diff --git a/routers/routes/routes.go b/routers/routes/routes.go index 5cb561b640f59..00059b0dac2a1 100644 --- a/routers/routes/routes.go +++ b/routers/routes/routes.go @@ -486,6 +486,10 @@ func RegisterRoutes(m *macaron.Macaron) { m.Get("/:id/:action", repo.ChangeMilestonStatus) m.Post("/delete", repo.DeleteMilestone) }, reqRepoWriter, context.RepoRef()) + // New redirection from fake url + m.Group("/releases", func() { + m.Get("/download/:vTag/:fileName", repo.RedirectDownload) + }, repo.MustBeNotBare, reqRepoWriter, context.RepoRef()) m.Group("/releases", func() { m.Get("/new", repo.NewRelease) m.Post("/new", bindIgnErr(auth.NewReleaseForm{}), repo.NewReleasePost) diff --git a/templates/repo/release/list.tmpl b/templates/repo/release/list.tmpl index c18bc4884d29f..8f9ae2d3cc90e 100644 --- a/templates/repo/release/list.tmpl +++ b/templates/repo/release/list.tmpl @@ -53,21 +53,25 @@

{{$.i18n.Tr "repo.release.downloads"}}

{{else}} From 1a539a6abda69427116340949d08f4f092d417d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?erg=C3=BC?= Date: Wed, 10 May 2017 04:18:36 +0200 Subject: [PATCH 02/10] Removed formatting directive in Println call --- routers/repo/repo.go | 7 ------- 1 file changed, 7 deletions(-) diff --git a/routers/repo/repo.go b/routers/repo/repo.go index 79c3b60cfcb0f..0ad4aba3313c0 100644 --- a/routers/repo/repo.go +++ b/routers/repo/repo.go @@ -274,13 +274,6 @@ func RedirectDownload(ctx *context.Context) { vTag = ctx.Params("vTag") fileName = ctx.Params("fileName") ) - if len(vTag) > 0 { - fmt.Println("Value of vTag %v", vTag) - } - if len(fileName) > 0 { - fmt.Println("Value of fileName %v", fileName) - } - var tagNames []string tagNames = append(tagNames, vTag) From aefa570990bc1f063fad14b6f401130e000d1d0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?erg=C3=BC?= Date: Wed, 10 May 2017 11:38:20 +0200 Subject: [PATCH 03/10] Made changes from Staff code review --- routers/repo/repo.go | 7 ++----- templates/repo/release/list.tmpl | 5 ++--- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/routers/repo/repo.go b/routers/repo/repo.go index 0ad4aba3313c0..1b28dc698526d 100644 --- a/routers/repo/repo.go +++ b/routers/repo/repo.go @@ -268,17 +268,14 @@ func Action(ctx *context.Context) { ctx.Redirect(redirectTo) } -// RedirectDownload return a file based on the the following infos: +// RedirectDownload return a file based on the the following infos: func RedirectDownload(ctx *context.Context) { var ( vTag = ctx.Params("vTag") fileName = ctx.Params("fileName") ) - var tagNames []string - tagNames = append(tagNames, vTag) - + tagNames := []string{vTag} curRepo := ctx.Repo.Repository - releases, err := models.GetReleasesByRepoIDAndNames(curRepo.ID, tagNames) if err != nil { ctx.Handle(500, "RedirectDownload -> Release not found", err) diff --git a/templates/repo/release/list.tmpl b/templates/repo/release/list.tmpl index 8f9ae2d3cc90e..b36e93479e31e 100644 --- a/templates/repo/release/list.tmpl +++ b/templates/repo/release/list.tmpl @@ -53,14 +53,13 @@

{{$.i18n.Tr "repo.release.downloads"}}

    - {{$tagname := .TagName}} {{if .Attachments}} {{range $attachment := .Attachments}}
  • - + {{$attachment.Name}} - {{$attachment.FileSize}} + {{$attachment.FileSize}}
    • {{end}} From 0a9a812838b6f967c1f0e3acda3980f6d18f2799 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?erg=C3=BC?= Date: Wed, 10 May 2017 11:56:10 +0200 Subject: [PATCH 04/10] Reverted changes from Staff code review mistake --- routers/routes/routes.go | 1 - templates/repo/release/list.tmpl | 5 +++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/routers/routes/routes.go b/routers/routes/routes.go index 00059b0dac2a1..8407d6c87cb58 100644 --- a/routers/routes/routes.go +++ b/routers/routes/routes.go @@ -486,7 +486,6 @@ func RegisterRoutes(m *macaron.Macaron) { m.Get("/:id/:action", repo.ChangeMilestonStatus) m.Post("/delete", repo.DeleteMilestone) }, reqRepoWriter, context.RepoRef()) - // New redirection from fake url m.Group("/releases", func() { m.Get("/download/:vTag/:fileName", repo.RedirectDownload) }, repo.MustBeNotBare, reqRepoWriter, context.RepoRef()) diff --git a/templates/repo/release/list.tmpl b/templates/repo/release/list.tmpl index b36e93479e31e..8f9ae2d3cc90e 100644 --- a/templates/repo/release/list.tmpl +++ b/templates/repo/release/list.tmpl @@ -53,13 +53,14 @@

    {{$.i18n.Tr "repo.release.downloads"}}

      + {{$tagname := .TagName}} {{if .Attachments}} {{range $attachment := .Attachments}}
    • - + {{$attachment.Name}} - {{$attachment.FileSize}} + {{$attachment.FileSize}}
      • {{end}} From 5ca8651f0254a9b6318a38cd4dea07a1334be500 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?erg=C3=BC?= Date: Wed, 10 May 2017 12:01:41 +0200 Subject: [PATCH 05/10] Added 404 errors instead of 500 --- routers/repo/repo.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/routers/repo/repo.go b/routers/repo/repo.go index 1b28dc698526d..af29bc158efff 100644 --- a/routers/repo/repo.go +++ b/routers/repo/repo.go @@ -278,7 +278,7 @@ func RedirectDownload(ctx *context.Context) { curRepo := ctx.Repo.Repository releases, err := models.GetReleasesByRepoIDAndNames(curRepo.ID, tagNames) if err != nil { - ctx.Handle(500, "RedirectDownload -> Release not found", err) + ctx.Handle(404, "RedirectDownload -> Release not found", err) return } @@ -286,7 +286,7 @@ func RedirectDownload(ctx *context.Context) { release := releases[0] att, err := models.GetAttachmentByReleaseIDFileName(release.ID, fileName) if err != nil { - ctx.Handle(500, "RedirectDownload -> Attachment not found", err) + ctx.Handle(404, "RedirectDownload -> Attachment not found", err) return } ctx.Redirect(setting.AppSubURL + "/attachments/" + att.UUID) From 84d22d89c085444beef6ca5fdc34bcaef6fc16cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?erg=C3=BC?= Date: Wed, 10 May 2017 12:05:03 +0200 Subject: [PATCH 06/10] Removed author infos from code --- models/attachment.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/models/attachment.go b/models/attachment.go index 497f7dbc92d99..41418e02f70a2 100644 --- a/models/attachment.go +++ b/models/attachment.go @@ -157,11 +157,7 @@ func GetAttachmentsByCommentID(commentID int64) ([]*Attachment, error) { return attachments, x.Where("comment_id=?", commentID).Find(&attachments) } -/* Author : github.com/gdeverlant - getAttachmentByReleaseIDFileName return a file based on the the following infos: - - releaseID - - fileName -*/ +// getAttachmentByReleaseIDFileName return a file based on the the following infos: func getAttachmentByReleaseIDFileName(e Engine, releaseID int64, fileName string) (*Attachment, error) { attach := &Attachment{ReleaseID: releaseID, Name: fileName} has, err := e.Get(attach) From 52b4dea3eab26079edc329d2eed98fb97e046d47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?erg=C3=BC?= Date: Wed, 10 May 2017 13:27:21 +0200 Subject: [PATCH 07/10] Function end with 404 error and cleanup. --- routers/repo/repo.go | 4 +++- templates/repo/release/list.tmpl | 1 - 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/routers/repo/repo.go b/routers/repo/repo.go index af29bc158efff..3bff00beeceeb 100644 --- a/routers/repo/repo.go +++ b/routers/repo/repo.go @@ -278,7 +278,7 @@ func RedirectDownload(ctx *context.Context) { curRepo := ctx.Repo.Repository releases, err := models.GetReleasesByRepoIDAndNames(curRepo.ID, tagNames) if err != nil { - ctx.Handle(404, "RedirectDownload -> Release not found", err) + ctx.Handle(500, "RedirectDownload -> Release not found", err) return } @@ -291,6 +291,8 @@ func RedirectDownload(ctx *context.Context) { } ctx.Redirect(setting.AppSubURL + "/attachments/" + att.UUID) } + + ctx.Handle(404, "RedirectDownload -> Attachment not found", err) } // Download download an archive of a repository diff --git a/templates/repo/release/list.tmpl b/templates/repo/release/list.tmpl index 8f9ae2d3cc90e..b703d25499940 100644 --- a/templates/repo/release/list.tmpl +++ b/templates/repo/release/list.tmpl @@ -57,7 +57,6 @@ {{if .Attachments}} {{range $attachment := .Attachments}}
    • - {{$attachment.Name}} {{$attachment.FileSize}} From 98cb0d2b6154360f1057af00cd59a5c9e9e2c4f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?erg=C3=BC?= Date: Wed, 10 May 2017 14:17:28 +0200 Subject: [PATCH 08/10] Minor correction --- routers/repo/repo.go | 2 -- routers/routes/routes.go | 2 -- 2 files changed, 4 deletions(-) diff --git a/routers/repo/repo.go b/routers/repo/repo.go index 3bff00beeceeb..2b41543c84f86 100644 --- a/routers/repo/repo.go +++ b/routers/repo/repo.go @@ -281,7 +281,6 @@ func RedirectDownload(ctx *context.Context) { ctx.Handle(500, "RedirectDownload -> Release not found", err) return } - if len(releases) == 1 { release := releases[0] att, err := models.GetAttachmentByReleaseIDFileName(release.ID, fileName) @@ -291,7 +290,6 @@ func RedirectDownload(ctx *context.Context) { } ctx.Redirect(setting.AppSubURL + "/attachments/" + att.UUID) } - ctx.Handle(404, "RedirectDownload -> Attachment not found", err) } diff --git a/routers/routes/routes.go b/routers/routes/routes.go index 8407d6c87cb58..46dd71dd93f6f 100644 --- a/routers/routes/routes.go +++ b/routers/routes/routes.go @@ -488,8 +488,6 @@ func RegisterRoutes(m *macaron.Macaron) { }, reqRepoWriter, context.RepoRef()) m.Group("/releases", func() { m.Get("/download/:vTag/:fileName", repo.RedirectDownload) - }, repo.MustBeNotBare, reqRepoWriter, context.RepoRef()) - m.Group("/releases", func() { m.Get("/new", repo.NewRelease) m.Post("/new", bindIgnErr(auth.NewReleaseForm{}), repo.NewReleasePost) m.Post("/delete", repo.DeleteRelease) From 7aa17f7227124809631d6bca76ac27f7ac889d10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?erg=C3=BC?= Date: Wed, 10 May 2017 14:25:27 +0200 Subject: [PATCH 09/10] Wishlist added --- routers/repo/repo.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/routers/repo/repo.go b/routers/repo/repo.go index 2b41543c84f86..5e17d85cb6e25 100644 --- a/routers/repo/repo.go +++ b/routers/repo/repo.go @@ -288,7 +288,9 @@ func RedirectDownload(ctx *context.Context) { ctx.Handle(404, "RedirectDownload -> Attachment not found", err) return } - ctx.Redirect(setting.AppSubURL + "/attachments/" + att.UUID) + if att != nil { + ctx.Redirect(setting.AppSubURL + "/attachments/" + att.UUID) + } } ctx.Handle(404, "RedirectDownload -> Attachment not found", err) } From 41198166389dc5286bcc841749affac04cfe41ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?erg=C3=BC?= Date: Thu, 11 May 2017 00:53:27 +0200 Subject: [PATCH 10/10] Loads of wishlist for code review --- models/attachment.go | 2 +- routers/repo/repo.go | 8 ++++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/models/attachment.go b/models/attachment.go index 41418e02f70a2..639c37f5b9551 100644 --- a/models/attachment.go +++ b/models/attachment.go @@ -114,7 +114,7 @@ func getAttachmentByUUID(e Engine, uuid string) (*Attachment, error) { attach := &Attachment{UUID: uuid} has, err := e.Get(attach) if err != nil { - return nil, err + return nil, nil } else if !has { return nil, ErrAttachmentNotExist{0, uuid} } diff --git a/routers/repo/repo.go b/routers/repo/repo.go index 5e17d85cb6e25..9f9ebfb70c7bd 100644 --- a/routers/repo/repo.go +++ b/routers/repo/repo.go @@ -268,7 +268,7 @@ func Action(ctx *context.Context) { ctx.Redirect(redirectTo) } -// RedirectDownload return a file based on the the following infos: +// RedirectDownload return a file based on the following infos: func RedirectDownload(ctx *context.Context) { var ( vTag = ctx.Params("vTag") @@ -278,7 +278,11 @@ func RedirectDownload(ctx *context.Context) { curRepo := ctx.Repo.Repository releases, err := models.GetReleasesByRepoIDAndNames(curRepo.ID, tagNames) if err != nil { - ctx.Handle(500, "RedirectDownload -> Release not found", err) + if models.IsErrAttachmentNotExist(err) { + ctx.Error(404) + return + } + ctx.Handle(500, "RedirectDownload: Failed to get attachment", err) return } if len(releases) == 1 {