Fix API-Endpoint release (#3005) #3012

SnowMB · 2017-11-28T14:43:16Z

I'm just getting started with go. But here is my try to fix #3005 😁

codecov-io · 2017-11-28T14:52:01Z

Codecov Report

Merging #3012 into master will increase coverage by 0.26%.
The diff coverage is 66.66%.

@@            Coverage Diff             @@
##           master    #3012      +/-   ##
==========================================
+ Coverage   35.01%   35.28%   +0.26%     
==========================================
  Files         281      281              
  Lines       40560    40563       +3     
==========================================
+ Hits        14204    14314     +110     
+ Misses      24255    24138     -117     
- Partials     2101     2111      +10

Impacted Files	Coverage Δ
routers/api/v1/api.go	`75.24% <100%> (ø)`	⬆️
routers/api/v1/repo/release.go	`25.36% <50%> (+25.36%)`	⬆️
models/repo.go	`43.37% <0%> (ø)`	⬆️
modules/process/manager.go	`81.15% <0%> (+4.34%)`	⬆️
models/release.go	`42.75% <0%> (+13.07%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cc0c4a3...fc48827. Read the comment docs.

lunny · 2017-11-28T15:21:02Z

Could you explain why we need that change?

SnowMB · 2017-11-28T16:11:56Z

When I tried to create a release via the api, I always got an internal server error (#3005).

Digging around I found out that in \vendor\code.gitea.io\git\repo_tag.go the following lines crashed, because repo was nil(l. 21-24):

// IsTagExist returns true if given tag exists in the repository.
func (repo *Repository) IsTagExist(name string) bool {
	return IsTagExist(repo.Path, name)
}

That call comes from \routers\api\v1\repo\release.go (l. 128-130), where ctx.Repo is fine but ctx.Repo.GitRepo is not initialized.

if !ctx.Repo.GitRepo.IsTagExist(form.TagName) {
		ctx.Status(404)
		return
	}

lafriks · 2017-11-28T17:14:30Z

@SnowMB Try using context.ReferencesGitRepo() instead

SnowMB · 2017-11-29T09:13:45Z

@lafriks done. Also fixed the same error in the PATCH route.

lafriks · 2017-11-29T09:33:32Z

Integration test for this would be great but otherwise LGTM

lunny · 2017-11-29T09:46:20Z

Yes, it's better to have tests.

SnowMB · 2017-12-01T16:52:52Z

Would love to add some tests. But I've got some problems to get the tests running on my machine (windows 10 / vs code). My error is that the environment variable GITEA_ROOT is not set. Any hints how to configure this properly?

vtemian · 2017-12-02T13:44:45Z

@SnowMB I'm also new here, but I've managed to run the tests, on a Linux machine.
You can take a look at the Makefile.

The target to start is integration-test-coverage (since it is the one executed by the CI https://github.com/go-gitea/gitea/blob/master/.drone.yml#L122). That target depends on integrations.cover.test.

integrations.cover.test: will compile code.gitea.io/gitea/integrations into integrations.cover.test, which is the binary used to run the integration tests.

Besides that, you will need the gitea binary, that can be easily obtained via make build or by following the instructions from the Makefile (https://github.com/go-gitea/gitea/blob/master/Makefile#L221).
GITEA_ROOT is actually the directory in which the newly compiled gitea binary can be found, which, in the Makefile, is set to ${CURDIR}, meaning the current working directory.

I suggest to work with the Makefile, is much easier and on Windows, I think that you can do that via MinGW (https://stackoverflow.com/questions/12881854/how-to-use-gnu-make-on-windows)

lunny · 2017-12-08T06:06:40Z

@SnowMB it's still panic on my test:


<html>
<head><title>PANIC: runtime error: invalid memory address or nil pointer dereference</title>
<meta charset="utf-8" />
<style type="text/css">
html, body {
	font-family: "Roboto", sans-serif;
	color: #333333;
	background-color: #ea5343;
	margin: 0px;
}
h1 {
	color: #d04526;
	background-color: #ffffff;
	padding: 20px;
	border-bottom: 1px dashed #2b3848;
}
pre {
	margin: 20px;
	padding: 20px;
	border: 2px solid #2b3848;
	background-color: #ffffff;
	white-space: pre-wrap;       /* css-3 */
	white-space: -moz-pre-wrap;  /* Mozilla, since 1999 */
	white-space: -pre-wrap;      /* Opera 4-6 */
	white-space: -o-pre-wrap;    /* Opera 7 */
	word-wrap: break-word;       /* Internet Explorer 5.5+ */
}
</style>
</head><body>
<h1>PANIC</h1>
<pre style="font-weight: bold;">runtime error: invalid memory address or nil pointer dereference</pre>
<pre>/Users/lunny/goroot/src/runtime/panic.go:491 (0x402d692)
	gopanic: reflectcall(nil, unsafe.Pointer(d.fn), deferArgs(d), uint32(d.siz), uint32(d.siz))
/Users/lunny/goroot/src/runtime/panic.go:63 (0x402c59d)
	panicmem: panic(memoryError)
/Users/lunny/goroot/src/runtime/signal_unix.go:367 (0x4044d3b)
	sigpanic: panicmem()
/Users/lunny/gopath/src/code.gitea.io/gitea/models/repo.go:467 (0x4895436)
	(*Repository).getOwner: if repo.Owner != nil {
/Users/lunny/gopath/src/code.gitea.io/gitea/models/repo.go:481 (0x48955b2)
	(*Repository).mustOwner: if err := repo.getOwner(e); err != nil {
/Users/lunny/gopath/src/code.gitea.io/gitea/models/repo.go:239 (0x4893a62)
	(*Repository).MustOwner: return repo.mustOwner(x)
/Users/lunny/gopath/src/code.gitea.io/gitea/models/repo.go:244 (0x4893aae)
	(*Repository).FullName: return repo.MustOwner().Name + "/" + repo.Name
/Users/lunny/gopath/src/code.gitea.io/gitea/models/release.go:82 (0x488de88)
	(*Release).APIURL: setting.AppURL, r.Repo.FullName(), r.ID)
/Users/lunny/gopath/src/code.gitea.io/gitea/models/release.go:102 (0x488e24e)
	(*Release).APIFormat: URL:          r.APIURL(),
/Users/lunny/gopath/src/code.gitea.io/gitea/routers/api/v1/repo/release.go:176 (0x4a403a0)
	CreateRelease: ctx.JSON(201, rel.APIFormat())
/Users/lunny/goroot/src/runtime/asm_amd64.s:511 (0x405c121)
	call128: CALLFN(·call128, 128)
/Users/lunny/goroot/src/reflect/value.go:434 (0x40c24d4)
	Value.call: call(frametype, fn, args, uint32(frametype.size), uint32(retOffset))
/Users/lunny/goroot/src/reflect/value.go:302 (0x40c1ab3)
	Value.Call: return v.call("Call", in)
/Users/lunny/gopath/src/code.gitea.io/gitea/vendor/github.com/go-macaron/inject/inject.go:177 (0x43ca498)
	(*injector).callInvoke: return reflect.ValueOf(f).Call(in), nil
/Users/lunny/gopath/src/code.gitea.io/gitea/vendor/github.com/go-macaron/inject/inject.go:137 (0x43c9d99)
	(*injector).Invoke: return inj.callInvoke(f, t, t.NumIn())
/Users/lunny/gopath/src/code.gitea.io/gitea/vendor/gopkg.in/macaron.v1/context.go:121 (0x43f91ed)
	(*Context).run: vals, err := c.Invoke(c.handler())
/Users/lunny/gopath/src/code.gitea.io/gitea/vendor/gopkg.in/macaron.v1/context.go:112 (0x43f9125)
	(*Context).Next: c.run()
/Users/lunny/gopath/src/code.gitea.io/gitea/vendor/github.com/go-macaron/session/session.go:186 (0x4450ea6)
	Sessioner.func1: ctx.Next()
/Users/lunny/gopath/src/code.gitea.io/gitea/vendor/gopkg.in/macaron.v1/context.go:79 (0x43f8fd0)
	ContextInvoker.Invoke: invoke(params[0].(*Context))
/Users/lunny/gopath/src/code.gitea.io/gitea/vendor/github.com/go-macaron/inject/inject.go:157 (0x43ca153)
	(*injector).fastInvoke: return f.Invoke(in)
/Users/lunny/gopath/src/code.gitea.io/gitea/vendor/github.com/go-macaron/inject/inject.go:135 (0x43c9e8a)
	(*injector).Invoke: return inj.fastInvoke(v, t, t.NumIn())
/Users/lunny/gopath/src/code.gitea.io/gitea/vendor/gopkg.in/macaron.v1/context.go:121 (0x43f91ed)
	(*Context).run: vals, err := c.Invoke(c.handler())
/Users/lunny/gopath/src/code.gitea.io/gitea/vendor/gopkg.in/macaron.v1/context.go:112 (0x43f9125)
	(*Context).Next: c.run()
/Users/lunny/gopath/src/code.gitea.io/gitea/vendor/gopkg.in/macaron.v1/recovery.go:161 (0x440c60a)
	Recovery.func1: c.Next()
/Users/lunny/gopath/src/code.gitea.io/gitea/vendor/gopkg.in/macaron.v1/logger.go:40 (0x43fc4bb)
	LoggerInvoker.Invoke: invoke(params[0].(*Context), params[1].(*log.Logger))
/Users/lunny/gopath/src/code.gitea.io/gitea/vendor/github.com/go-macaron/inject/inject.go:157 (0x43ca153)
	(*injector).fastInvoke: return f.Invoke(in)
/Users/lunny/gopath/src/code.gitea.io/gitea/vendor/github.com/go-macaron/inject/inject.go:135 (0x43c9e8a)
	(*injector).Invoke: return inj.fastInvoke(v, t, t.NumIn())
/Users/lunny/gopath/src/code.gitea.io/gitea/vendor/gopkg.in/macaron.v1/context.go:121 (0x43f91ed)
	(*Context).run: vals, err := c.Invoke(c.handler())
/Users/lunny/gopath/src/code.gitea.io/gitea/vendor/gopkg.in/macaron.v1/context.go:112 (0x43f9125)
	(*Context).Next: c.run()
/Users/lunny/gopath/src/code.gitea.io/gitea/vendor/gopkg.in/macaron.v1/logger.go:52 (0x440b89b)
	Logger.func1: ctx.Next()
/Users/lunny/gopath/src/code.gitea.io/gitea/vendor/gopkg.in/macaron.v1/logger.go:40 (0x43fc4bb)
	LoggerInvoker.Invoke: invoke(params[0].(*Context), params[1].(*log.Logger))
/Users/lunny/gopath/src/code.gitea.io/gitea/vendor/github.com/go-macaron/inject/inject.go:157 (0x43ca153)
	(*injector).fastInvoke: return f.Invoke(in)
/Users/lunny/gopath/src/code.gitea.io/gitea/vendor/github.com/go-macaron/inject/inject.go:135 (0x43c9e8a)
	(*injector).Invoke: return inj.fastInvoke(v, t, t.NumIn())
/Users/lunny/gopath/src/code.gitea.io/gitea/vendor/gopkg.in/macaron.v1/context.go:121 (0x43f91ed)
	(*Context).run: vals, err := c.Invoke(c.handler())
/Users/lunny/gopath/src/code.gitea.io/gitea/vendor/gopkg.in/macaron.v1/router.go:187 (0x440d8f2)
	(*Router).Handle.func1: c.run()
/Users/lunny/gopath/src/code.gitea.io/gitea/vendor/gopkg.in/macaron.v1/router.go:303 (0x4405a5b)
	(*Router).ServeHTTP: h(rw, req, p)
/Users/lunny/gopath/src/code.gitea.io/gitea/vendor/gopkg.in/macaron.v1/macaron.go:220 (0x43fda27)
	(*Macaron).ServeHTTP: m.Router.ServeHTTP(rw, req)
/Users/lunny/gopath/src/code.gitea.io/gitea/vendor/github.com/gorilla/context/context.go:141 (0x46e7fda)
	ClearHandler.func1: h.ServeHTTP(w, r)
/Users/lunny/goroot/src/net/http/server.go:1918 (0x42d5b03)
	HandlerFunc.ServeHTTP: f(w, r)
/Users/lunny/goroot/src/net/http/server.go:2619 (0x42d87e3)
	serverHandler.ServeHTTP: handler.ServeHTTP(rw, req)
/Users/lunny/goroot/src/net/http/server.go:1801 (0x42d49dc)
	(*conn).serve: serverHandler{c.server}.ServeHTTP(w, w.req)
/Users/lunny/goroot/src/runtime/asm_amd64.s:2337 (0x405e870)
	goexit: BYTE	$0x90	// NOP
</pre>
</body>
</html>%

lunny · 2017-12-23T08:03:25Z

@SnowMB please fix this or could you give the push permission to maintainers on this PR.

diff --git a/routers/api/v1/repo/release.go b/routers/api/v1/repo/release.go
index 17cddc565..d26d337cb 100644
--- a/routers/api/v1/repo/release.go
+++ b/routers/api/v1/repo/release.go
@@ -146,6 +146,7 @@ func CreateRelease(ctx *context.APIContext, form api.CreateReleaseOption) {
                        IsDraft:      form.IsDraft,
                        IsPrerelease: form.IsPrerelease,
                        IsTag:        false,
+                       Repo:         ctx.Repo.Repository,
                }
                if err := models.CreateRelease(ctx.Repo.GitRepo, rel, nil); err != nil {
                        if models.IsErrReleaseAlreadyExist(err) {
@@ -167,6 +168,8 @@ func CreateRelease(ctx *context.APIContext, form api.CreateReleaseOption) {
                rel.IsPrerelease = form.IsPrerelease
                rel.PublisherID = ctx.User.ID
                rel.IsTag = false
+               rel.Repo = ctx.Repo.Repository
+               rel.Publisher = ctx.User

                if err = models.UpdateRelease(ctx.Repo.GitRepo, rel, nil); err != nil {
                        ctx.Handle(500, "UpdateRelease", err)

SnowMB · 2017-12-23T15:04:15Z

Hey @lunny,

Unfortunately I don't have time to fix this myself. But you should have permission to add commits to the pr. (allow edits from maintainers is checked). Is there something else I have to do, so that you can work on the branch in my fork?

lunny · 2017-12-24T05:58:09Z

@SnowMB thanks. I have sent a commit. I think it's ready to review now.

strk · 2017-12-24T08:17:26Z

I'd also like to see a test added for this

lafriks · 2018-01-16T07:22:43Z

@SnowMB @strk @lunny I added tests for this

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

lafriks added type/bug modifies/api This PR adds API routes or modifies them labels Nov 28, 2017

tboerger added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Nov 29, 2017

lunny added this to the 1.4.0 milestone Nov 29, 2017

lunny added the backport/v1.3 label Dec 5, 2017

SnowMB and others added 3 commits December 24, 2017 13:56

Fix API-Endpoint release (#3005)

85d0c28

Using context.ReferencesGitRepo(); doing the same in the PATCH route.

d21da94

fix returns

6b99eee

Merge branch 'master' into fix_api_release

ef92108

lafriks removed the backport/v1.3 label Jan 15, 2018

lafriks added 2 commits January 16, 2018 09:18

Add release create and get test

3187809

Merge branch 'master' into fix_api_release

42d77d4

Add also test edit

fc48827

sapk approved these changes Jan 16, 2018

View reviewed changes

tboerger added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Jan 16, 2018

lafriks merged commit 695b10b into go-gitea:master Jan 16, 2018

SnowMB deleted the fix_api_release branch April 12, 2018 16:11

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix API-Endpoint release (#3005) #3012

Fix API-Endpoint release (#3005) #3012

SnowMB commented Nov 28, 2017 •

edited

Loading

codecov-io commented Nov 28, 2017 •

edited

Loading

lunny commented Nov 28, 2017

SnowMB commented Nov 28, 2017

lafriks commented Nov 28, 2017 •

edited

Loading

SnowMB commented Nov 29, 2017

lafriks commented Nov 29, 2017

lunny commented Nov 29, 2017

SnowMB commented Dec 1, 2017

vtemian commented Dec 2, 2017

lunny commented Dec 8, 2017

lunny commented Dec 23, 2017

SnowMB commented Dec 23, 2017

lunny commented Dec 24, 2017

strk commented Dec 24, 2017

lafriks commented Jan 16, 2018

Fix API-Endpoint release (#3005) #3012

Fix API-Endpoint release (#3005) #3012

Conversation

SnowMB commented Nov 28, 2017 • edited Loading

codecov-io commented Nov 28, 2017 • edited Loading

Codecov Report

lunny commented Nov 28, 2017

SnowMB commented Nov 28, 2017

lafriks commented Nov 28, 2017 • edited Loading

SnowMB commented Nov 29, 2017

lafriks commented Nov 29, 2017

lunny commented Nov 29, 2017

SnowMB commented Dec 1, 2017

vtemian commented Dec 2, 2017

lunny commented Dec 8, 2017

lunny commented Dec 23, 2017

SnowMB commented Dec 23, 2017

lunny commented Dec 24, 2017

strk commented Dec 24, 2017

lafriks commented Jan 16, 2018

SnowMB commented Nov 28, 2017 •

edited

Loading

codecov-io commented Nov 28, 2017 •

edited

Loading

lafriks commented Nov 28, 2017 •

edited

Loading