Added attachments to the releases API #2084

stefan-lacatus · 2017-06-29T21:54:24Z

Very simple initial fix for #711.
It's also my first interaction with GO so i'm sorry for stupid mistakes.

Includes changes to the endpoints:

Get releases GET /repos/:owner/:repo/releases[/:id] - now includes assets
List assets: GET /repos/:owner/:repo/releases/:id/assets
Get single asset: GET /repos/:owner/:repo/releases/assets/:id

If the API for github must be followed, where attachments are "assets", with a lot of additional metadata then this PR can be skipped, but for my limited usecase this is all I need.

Depends on go-gitea/go-sdk#63

ethantkoenig · 2017-06-29T22:21:35Z

models/release.go

@@ -71,6 +71,10 @@ func (r *Release) loadAttributes(e Engine) error {
 			return err
 		}
 	}
+	// load the attachments of this release
+	if r.Attachments == nil {
+		GetReleaseAttachments(r)


The error returned by GetReleaseAttachments(r) is unchecked.

lafriks

I think we should follow GitHub api as close as possible so it would be better to name features same as GitHub so in this case as Assets.

Also do not change anything under vendor directory directly. If changes need to be done in sdk first you need to submit PR in https://github.com/go-gitea/go-sdk

Aslo this would need test

Don't ignore gravatar error

Somes layer are created and aren't usefull, so I compress this :)

stefan-lacatus · 2017-06-30T04:43:07Z

Understood. I'll separate this into 2 PRs for the appropriate repos.
I'll also rename the json keys to match the github ones as much as possible.

Thanks for the tips

Signed-off-by: Petrisor Lacatus <[email protected]>

This reverts commit 789188f.

Signed-off-by: Petrisor Lacatus <[email protected]>

…nd to get all the attachments associated with a given release Signed-off-by: Petrisor Lacatus <[email protected]>

as well getting a given attachment of a release Signed-off-by: Petrisor Lacatus <[email protected]>

Signed-off-by: Petrisor Lacatus <[email protected]>

stefan-lacatus · 2017-06-30T07:37:26Z

Fixed the issue mentioned by @ethantkoenig

Get releases GET /repos/:owner/:repo/releases[/:id] - now includes assets
List assets: GET /repos/:owner/:repo/releases/:id/assets
Get single asset: GET /repos/:owner/:repo/releases/assets/:id

@tboerger : can you give me some quick tips about adding tests? I'm a bit in the dark there.

Signed-off-by: Petrisor Lacatus <[email protected]>

lafriks · 2017-06-30T07:53:11Z

models/attachment.go

@@ -58,6 +59,20 @@ func (a *Attachment) IncreaseDownloadCount() error {
 	return nil
 }

+// GetSize gets the size of the attachment in bytes
+func (a *Attachment) GetSize() (int64, error) {
+	f, err := os.Open(a.LocalPath())


I think it would be better to use

info, err := os.Stat(a.LocalPath()); if err != nil { return 0, err } return info.Size(), nil

lafriks · 2017-06-30T07:56:34Z

models/attachment.go

+		DownloadURL:   setting.AppURL + "attachments/" + a.UUID,
+		DownloadCount: a.DownloadCount,
+	}
+	fileSize, err := a.GetSize()


If there is error please log it

Good point, thanks

lafriks · 2017-06-30T07:57:27Z

models/attachment.go

@@ -142,6 +186,12 @@ func GetAttachmentsByCommentID(commentID int64) ([]*Attachment, error) {
 	return attachments, x.Where("comment_id=?", commentID).Find(&attachments)
 }

+// GetAttachmentsByReleaseID returns all attachments of a release
+func GetAttachmentsByReleaseID(releaseId int64) ([]*Attachment, error) {


naming releaseId -> releaseID

Added error logging. Signed-off-by: Petrisor Lacatus <[email protected]>

Fix exit status 1 not handled @ getMergeCommit (#2087)

Signed-off-by: Petrisor Lacatus <[email protected]>

lafriks · 2017-06-30T15:18:34Z

Please rebase on master

Signed-off-by: Petrisor Lacatus <[email protected]>

…nd to get all the attachments associated with a given release Signed-off-by: Petrisor Lacatus <[email protected]>

as well getting a given attachment of a release Signed-off-by: Petrisor Lacatus <[email protected]>

Signed-off-by: Petrisor Lacatus <[email protected]>

Added error logging. Signed-off-by: Petrisor Lacatus <[email protected]>

Signed-off-by: Petrisor Lacatus <[email protected]>

# Conflicts: # models/release.go # routers/api/v1/repo/release.go # routers/repo/release.go

lafriks · 2017-07-05T07:07:34Z

models/release.go

@@ -246,13 +260,16 @@ func (opts *FindReleasesOptions) toConds(repoID int64) builder.Cond {
 	if !opts.IncludeDrafts {
 		cond = cond.And(builder.Eq{"is_draft": false})
 	}
+	if !opts.IncludeDrafts {


There should be check for IncludePrereleases

Signed-off-by: Petrisor Lacatus <[email protected]>

bkcsoft

Error Checking and a question

bkcsoft · 2017-08-23T21:32:17Z

models/attachment.go

+		DownloadCount: a.DownloadCount,
+	}
+	fileSize, err := a.GetSize()
+	log.Warn("Error getting the file size for attachment %s. ", a.UUID, err)


This should if in a if err != nil check.

I'd do this:

fileSize, err := a.GetSize() if err != nil { log.Warn(...) } // Effectively doing `Size = 0` on errors. apiAttachment.Size = fileSize return apiAttachment

bkcsoft · 2017-08-23T21:36:03Z

routers/api/v1/repo/release.go

+		ctx.Error(500, "LoadAttributes", err)
+		return
+	}
+	ctx.JSON(200, releases[0].APIFormat())


e we sure that [0] is the latest one? (I don't see anything about sorting in the PR 🙂 )

stefan-lacatus · 2017-08-25T20:21:00Z

I'll revisit this next week, as i've also identified some bugs internally.

lunny · 2017-11-21T00:59:27Z

any update?

stefan-lacatus · 2017-12-03T06:08:51Z

I'll redo on top of the master

ethantkoenig and others added 4 commits June 29, 2017 10:37

Don't ignore gravatar error

23f0728

Use default avatar on error

05e3726

lint

2b410e4

Added attachments to the releases API

49f5ddc

ethantkoenig suggested changes Jun 29, 2017

View reviewed changes

lafriks requested changes Jun 29, 2017

View reviewed changes

lafriks added this to the 1.x.x milestone Jun 29, 2017

lafriks added modifies/api This PR adds API routes or modifies them type/enhancement An improvement of existing functionality type/testing and removed type/testing labels Jun 29, 2017

andreynering and others added 2 commits June 29, 2017 21:30

Merge pull request #2083 from ethantkoenig/check_err/org_avatar

935b40f

Don't ignore gravatar error

Reduce number of layer (#2078)

789188f

Somes layer are created and aren't usefull, so I compress this :)

tboerger added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jun 30, 2017

stefan-lacatus and others added 6 commits June 30, 2017 09:08

Revert changes done to vendor package gitea-sdk

b187b79

Signed-off-by: Petrisor Lacatus <[email protected]>

Revert "Reduce number of layer" (#2086)

8fd43f2

This reverts commit 789188f.

Added size of the attachment to the api format

bfd3b7d

Signed-off-by: Petrisor Lacatus <[email protected]>

Added two utility methods on attachment, to get an attachment by id a…

270098c

…nd to get all the attachments associated with a given release Signed-off-by: Petrisor Lacatus <[email protected]>

Implemented the api for getting only the attachments of a release,

7e14752

as well getting a given attachment of a release Signed-off-by: Petrisor Lacatus <[email protected]>

When searching for a release attachments, use the simpler method.

ba596d1

Signed-off-by: Petrisor Lacatus <[email protected]>

stefan-lacatus mentioned this pull request Jun 30, 2017

Added read-only attachments to the Releases API go-gitea/go-sdk#63

Merged

stefan-lacatus added 2 commits June 30, 2017 10:57

Fix linting errors

b23c0fa

Signed-off-by: Petrisor Lacatus <[email protected]>

Fix more linting errors

36e1893

Signed-off-by: Petrisor Lacatus <[email protected]>

lafriks requested changes Jun 30, 2017

View reviewed changes

stefan-lacatus and others added 4 commits June 30, 2017 12:20

Changed GetSize for attachment implementation.

a5ee8e1

Added error logging. Signed-off-by: Petrisor Lacatus <[email protected]>

Fix exit status 1 not handled @ getMergeCommit

b36849d

Merge pull request #2087 from Bwko/fix_error_exit

12cb6cd

Fix exit status 1 not handled @ getMergeCommit (#2087)

Implementation of GET /repos/:owner/:repo/releases/latest

78f201a

Signed-off-by: Petrisor Lacatus <[email protected]>

stefan-lacatus added 10 commits June 30, 2017 20:16

Added size of the attachment to the api format

31ab468

Signed-off-by: Petrisor Lacatus <[email protected]>

Added two utility methods on attachment, to get an attachment by id a…

9a74130

…nd to get all the attachments associated with a given release Signed-off-by: Petrisor Lacatus <[email protected]>

Implemented the api for getting only the attachments of a release,

377cf38

as well getting a given attachment of a release Signed-off-by: Petrisor Lacatus <[email protected]>

When searching for a release attachments, use the simpler method.

c67b34d

Signed-off-by: Petrisor Lacatus <[email protected]>

Fix linting errors

3e72f55

Signed-off-by: Petrisor Lacatus <[email protected]>

Fix more linting errors

ad3ba65

Signed-off-by: Petrisor Lacatus <[email protected]>

Changed GetSize for attachment implementation.

6f6e21b

Added error logging. Signed-off-by: Petrisor Lacatus <[email protected]>

Implementation of GET /repos/:owner/:repo/releases/latest

49b4cd7

Signed-off-by: Petrisor Lacatus <[email protected]>

Rebase on top of master

159c7f5

Signed-off-by: Petrisor Lacatus <[email protected]>

Merge remote-tracking branch 'origin/master'

6822f6f

# Conflicts: # models/release.go # routers/api/v1/repo/release.go # routers/repo/release.go

lafriks requested changes Jul 5, 2017

View reviewed changes

Fix incorrect check

856d8ec

Signed-off-by: Petrisor Lacatus <[email protected]>

bkcsoft suggested changes Aug 23, 2017

View reviewed changes

stefan-lacatus closed this Dec 3, 2017

stefan-lacatus mentioned this pull request Dec 3, 2017

Added attachments to releases API #3075

Closed

lunny removed this from the 1.x.x milestone Dec 3, 2017

lunny mentioned this pull request Dec 3, 2017

Added attachment releases data go-gitea/go-sdk#83

Closed

go-gitea locked and limited conversation to collaborators Nov 23, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added attachments to the releases API #2084

Added attachments to the releases API #2084

stefan-lacatus commented Jun 29, 2017 •

edited

Loading

ethantkoenig Jun 29, 2017

lafriks left a comment •

edited

Loading

stefan-lacatus commented Jun 30, 2017

stefan-lacatus commented Jun 30, 2017

lafriks Jun 30, 2017

lafriks Jun 30, 2017

stefan-lacatus Jun 30, 2017

lafriks Jun 30, 2017

lafriks commented Jun 30, 2017

lafriks Jul 5, 2017

bkcsoft left a comment

bkcsoft Aug 23, 2017

bkcsoft Aug 23, 2017

stefan-lacatus commented Aug 25, 2017

lunny commented Nov 21, 2017

stefan-lacatus commented Dec 3, 2017

Added attachments to the releases API #2084

Added attachments to the releases API #2084

Conversation

stefan-lacatus commented Jun 29, 2017 • edited Loading

Choose a reason for hiding this comment

lafriks left a comment • edited Loading

Choose a reason for hiding this comment

stefan-lacatus commented Jun 30, 2017

stefan-lacatus commented Jun 30, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lafriks commented Jun 30, 2017

Choose a reason for hiding this comment

bkcsoft left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stefan-lacatus commented Aug 25, 2017

lunny commented Nov 21, 2017

stefan-lacatus commented Dec 3, 2017

stefan-lacatus commented Jun 29, 2017 •

edited

Loading

lafriks left a comment •

edited

Loading