From 55f5424797037d6c98ed6f5312c837243f11642e Mon Sep 17 00:00:00 2001 From: Antoine GIRARD Date: Sat, 18 Nov 2017 17:17:38 +0100 Subject: [PATCH 01/29] Implement routes --- modules/lfs/locks.go | 114 +++++++++++++++++++++++++++++++++++++++ routers/routes/routes.go | 4 ++ 2 files changed, 118 insertions(+) create mode 100644 modules/lfs/locks.go diff --git a/modules/lfs/locks.go b/modules/lfs/locks.go new file mode 100644 index 0000000000000..137ec794aed30 --- /dev/null +++ b/modules/lfs/locks.go @@ -0,0 +1,114 @@ +package lfs + +import ( + "time" + + "code.gitea.io/gitea/modules/context" + "code.gitea.io/gitea/modules/setting" + + "gopkg.in/macaron.v1" +) + +// Lock represent a lock +// for use with the locks API. +type Lock struct { + ID string `json:"id"` + Path string `json:"path"` + LockedAt time.Time `json:"locked_at"` + Owner *LockOwner `json:"owner"` +} + +// LockOwner represent a lock owner +// for use with the locks API. +type LockOwner struct { + Name string `json:"name"` +} + +// LockRequest contains the path of the lock to create +// https://github.com/git-lfs/git-lfs/blob/master/docs/api/locking.md#create-lock +type LockRequest struct { + Path string `json:"path"` +} + +//LockResponse represent a lock created +// https://github.com/git-lfs/git-lfs/blob/master/docs/api/locking.md#create-lock +type LockResponse struct { + Lock *Lock `json:"lock"` +} + +//LockList represent a list of lock requested +// https://github.com/git-lfs/git-lfs/blob/master/docs/api/locking.md#list-locks +type LockList struct { + Locks []*Lock `json:"locks"` + Next string `json:"next_cursor,omitempty"` +} + +//LockListVerify represent a list of lock verification requested +// https://github.com/git-lfs/git-lfs/blob/master/docs/api/locking.md#list-locks-for-verification +type LockListVerify struct { + Ours []*Lock `json:"ours"` + Theirs []*Lock `json:"theirs"` + Next string `json:"next_cursor,omitempty"` +} + +// LockError contains information on the error that occurs +type LockError struct { + Message string `json:"message"` + Lock *Lock `json:"lock,omitempty"` + Documentation string `json:"documentation_url,omitempty"` + RequestID string `json:"request_id,omitempty"` +} + +func checkRequest(req macaron.Request) int { + if !setting.LFS.StartServer { + return 404 + } + if !ContentMatcher(req) || req.Header.Get("Content-Type") != contentMediaType { + return 400 + } + return 200 +} + +// GetLockHandler list locks +func GetLockHandler(ctx *context.Context) { + status := checkRequest(ctx.Req) + if status != 200 { + writeStatus(ctx, status) + return + } + ctx.Resp.Header().Set("Content-Type", metaMediaType) + ctx.JSON(404, LockError{Message: "Not found"}) +} + +// PostLockHandler create lock +func PostLockHandler(ctx *context.Context) { + status := checkRequest(ctx.Req) + if status != 200 { + writeStatus(ctx, status) + return + } + ctx.Resp.Header().Set("Content-Type", metaMediaType) + ctx.JSON(404, LockError{Message: "Not found"}) +} + +// VerifyLockHandler list locks for verification +func VerifyLockHandler(ctx *context.Context) { + status := checkRequest(ctx.Req) + if status != 200 { + writeStatus(ctx, status) + return + } + ctx.Resp.Header().Set("Content-Type", metaMediaType) + ctx.JSON(404, LockError{Message: "Not found"}) +} + +// UnLockHandler delete locks +func UnLockHandler(ctx *context.Context) { + status := checkRequest(ctx.Req) + if status != 200 { + writeStatus(ctx, status) + return + } + ctx.Resp.Header().Set("Content-Type", metaMediaType) + ctx.JSON(404, LockError{Message: "Not found"}) +} diff --git a/routers/routes/routes.go b/routers/routes/routes.go index ece2565683c7e..857e36b9e33b2 100644 --- a/routers/routes/routes.go +++ b/routers/routes/routes.go @@ -686,6 +686,10 @@ func RegisterRoutes(m *macaron.Macaron) { m.Any("/objects/:oid", lfs.ObjectOidHandler) m.Post("/objects", lfs.PostHandler) m.Post("/verify", lfs.VerifyHandler) + m.Get("/locks", lfs.GetLockHandler) + m.Post("/locks", lfs.PostLockHandler) + m.Post("/locks/verify", lfs.VerifyLockHandler) + m.Post("/locks/:id/unlock", lfs.UnLockHandler) m.Any("/*", func(ctx *context.Context) { ctx.Handle(404, "", nil) }) From 80b3ce0f09c33e189fac74daa1e8b732625158ec Mon Sep 17 00:00:00 2001 From: Antoine GIRARD Date: Sun, 19 Nov 2017 19:38:35 +0100 Subject: [PATCH 02/29] move to api/sdk and create model --- models/error.go | 37 ++++++++ models/lfs_lock.go | 104 +++++++++++++++++++++ modules/lfs/locks.go | 61 +----------- vendor/code.gitea.io/sdk/gitea/lfs_lock.go | 59 ++++++++++++ vendor/vendor.json | 9 +- 5 files changed, 211 insertions(+), 59 deletions(-) create mode 100644 models/lfs_lock.go create mode 100644 vendor/code.gitea.io/sdk/gitea/lfs_lock.go diff --git a/models/error.go b/models/error.go index 9df419aee8470..bf922940272cb 100644 --- a/models/error.go +++ b/models/error.go @@ -491,6 +491,43 @@ func (err ErrLastOrgOwner) Error() string { return fmt.Sprintf("user is the last member of owner team [uid: %d]", err.UID) } +// +// TODO LFS header +// + +// ErrRepoNotExist represents a "LFSLockNotExist" kind of error. +type ErrLFSLockNotExist struct { + ID int64 + RepoID int64 + Path string +} + +// IsErrLFSLockNotExist checks if an error is a ErrLFSLockNotExist. +func IsErrLFSLockNotExist(err error) bool { + _, ok := err.(ErrLFSLockNotExist) + return ok +} + +func (err ErrLFSLockNotExist) Error() string { + return fmt.Sprintf("lfs lock does not exist [id: %d, rid: %d, path: %s]", err.ID, err.RepoID, err.Path) +} + +// ErrLFSLockAlreadyExist represents a "LFSLockAlreadyExist" kind of error. +type ErrLFSLockAlreadyExist struct { + RepoID int64 + Path string +} + +// IsErrLFSLockAlreadyExist checks if an error is a ErrLFSLockAlreadyExist. +func IsErrLFSLockAlreadyExist(err error) bool { + _, ok := err.(ErrLFSLockAlreadyExist) + return ok +} + +func (err ErrLFSLockAlreadyExist) Error() string { + return fmt.Sprintf("lfs lock already exists [rid: %d, path: %s]", err.RepoID, err.Path) +} + // __________ .__ __ // \______ \ ____ ______ ____ _____|__|/ |_ ___________ ___.__. // | _// __ \\____ \ / _ \/ ___/ \ __\/ _ \_ __ < | | diff --git a/models/lfs_lock.go b/models/lfs_lock.go new file mode 100644 index 0000000000000..ca2df489a63c5 --- /dev/null +++ b/models/lfs_lock.go @@ -0,0 +1,104 @@ +// Copyright 2014 The Gogs Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package models + +import ( + "time" +"strconv" + "code.gitea.io/git" + api "code.gitea.io/sdk/gitea" +) + +// LFSLock represents a git lfs lfock of repository. +type LFSLock struct { + ID int64 `xorm:"pk autoincr"` + RepoID int64 `xorm:"INDEX UNIQUE(n)"` + Repo *Repository `xorm:"-"` + OwnerID int64 `xorm:"INDEX"` + Owner *User `xorm:"-"` + Path string + Created time.Time `xorm:"-"` + CreatedUnix int64 `xorm:"INDEX"` +} + +// BeforeInsert is invoked from XORM before inserting an object of this type. +func (l *LFSLock) BeforeInsert() { + if l.CreatedUnix == 0 { + l.CreatedUnix = time.Now().Unix() + } +} + +// AfterLoad is invoked from XORM after setting the values of all fields of this object. +func (l *LFSLock) AfterLoad() { + l.Created = time.Unix(l.CreatedUnix, 0).Local() +} + +func (l *LFSLock) loadAttributes(e Engine) error { + var err error + if l.Repo == nil { + l.Repo, err = GetRepositoryByID(l.RepoID) + if err != nil { + return err + } + } + if l.Owner == nil { + l.Owner, err = GetUserByID(l.OwnerID) + if err != nil { + return err + } + } + return nil +} + +// LoadAttributes load repo and publisher attributes for a lock +func (l *LFSLock) LoadAttributes() error { + return l.loadAttributes(x) +} + +// APIFormat convert a Release to lfs.LFSLock +func (l *LFSLock) APIFormat() *api.LFSLock { + //TODO move to api + return &api.LFSLock{ + ID: strconv.FormatInt(l.ID, 10), + Path: l.Path, + LockedAt: l.Created, + Owner: &api.LFSLockOwner{ + Name: l.Owner.DisplayName(), + }, + } +} + + +// IsLFSLockExist returns true if lock with given path already exists. +func IsLFSLockExist(repoID int64, path string) (bool, error) { + return x.Get(&LFSLock{RepoID: repoID, Path: path}) //TODO Define if path should needed to be lower for windows compat ? +} + +// CreateLFSLock creates a new lock. +func CreateLFSLock(gitRepo *git.Repository, lock *LFSLock) error { + isExist, err := IsLFSLockExist(lock.RepoID, lock.Path) + if err != nil { + return err + } else if isExist { + return ErrLFSLockAlreadyExist{lock.RepoID, lock.Path} + } + + _, err = x.InsertOne(lock) + return err +} + +// GetLFSLock returns release by given path. +func GetLFSLock(repoID int64, path string) (*LFSLock, error) { + isExist, err := IsLFSLockExist(repoID, path) + if err != nil { + return nil, err + } else if !isExist { + return nil, ErrLFSLockNotExist{0, repoID, path} + } + + rel := &LFSLock{RepoID: repoID, Path: path} //TODO Define if path should needed to be lower for windows compat ? + _, err = x.Get(rel) + return rel, err +} diff --git a/modules/lfs/locks.go b/modules/lfs/locks.go index 137ec794aed30..779892142f1c0 100644 --- a/modules/lfs/locks.go +++ b/modules/lfs/locks.go @@ -1,64 +1,13 @@ package lfs import ( - "time" - "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/setting" + api "code.gitea.io/sdk/gitea" "gopkg.in/macaron.v1" ) -// Lock represent a lock -// for use with the locks API. -type Lock struct { - ID string `json:"id"` - Path string `json:"path"` - LockedAt time.Time `json:"locked_at"` - Owner *LockOwner `json:"owner"` -} - -// LockOwner represent a lock owner -// for use with the locks API. -type LockOwner struct { - Name string `json:"name"` -} - -// LockRequest contains the path of the lock to create -// https://github.com/git-lfs/git-lfs/blob/master/docs/api/locking.md#create-lock -type LockRequest struct { - Path string `json:"path"` -} - -//LockResponse represent a lock created -// https://github.com/git-lfs/git-lfs/blob/master/docs/api/locking.md#create-lock -type LockResponse struct { - Lock *Lock `json:"lock"` -} - -//LockList represent a list of lock requested -// https://github.com/git-lfs/git-lfs/blob/master/docs/api/locking.md#list-locks -type LockList struct { - Locks []*Lock `json:"locks"` - Next string `json:"next_cursor,omitempty"` -} - -//LockListVerify represent a list of lock verification requested -// https://github.com/git-lfs/git-lfs/blob/master/docs/api/locking.md#list-locks-for-verification -type LockListVerify struct { - Ours []*Lock `json:"ours"` - Theirs []*Lock `json:"theirs"` - Next string `json:"next_cursor,omitempty"` -} - -// LockError contains information on the error that occurs -type LockError struct { - Message string `json:"message"` - Lock *Lock `json:"lock,omitempty"` - Documentation string `json:"documentation_url,omitempty"` - RequestID string `json:"request_id,omitempty"` -} - func checkRequest(req macaron.Request) int { if !setting.LFS.StartServer { return 404 @@ -77,7 +26,7 @@ func GetLockHandler(ctx *context.Context) { return } ctx.Resp.Header().Set("Content-Type", metaMediaType) - ctx.JSON(404, LockError{Message: "Not found"}) + ctx.JSON(404, api.LockError{Message: "Not found"}) } // PostLockHandler create lock @@ -88,7 +37,7 @@ func PostLockHandler(ctx *context.Context) { return } ctx.Resp.Header().Set("Content-Type", metaMediaType) - ctx.JSON(404, LockError{Message: "Not found"}) + ctx.JSON(404, api.LockError{Message: "Not found"}) } // VerifyLockHandler list locks for verification @@ -99,7 +48,7 @@ func VerifyLockHandler(ctx *context.Context) { return } ctx.Resp.Header().Set("Content-Type", metaMediaType) - ctx.JSON(404, LockError{Message: "Not found"}) + ctx.JSON(404, api.LockError{Message: "Not found"}) } // UnLockHandler delete locks @@ -110,5 +59,5 @@ func UnLockHandler(ctx *context.Context) { return } ctx.Resp.Header().Set("Content-Type", metaMediaType) - ctx.JSON(404, LockError{Message: "Not found"}) + ctx.JSON(404, api.LockError{Message: "Not found"}) } diff --git a/vendor/code.gitea.io/sdk/gitea/lfs_lock.go b/vendor/code.gitea.io/sdk/gitea/lfs_lock.go new file mode 100644 index 0000000000000..8e9f007e68a58 --- /dev/null +++ b/vendor/code.gitea.io/sdk/gitea/lfs_lock.go @@ -0,0 +1,59 @@ +// Copyright 2015 The Gogs Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package gitea + +import ( + "time" +) + +// LFSLock represent a lock +// for use with the locks API. +type LFSLock struct { + ID string `json:"id"` + Path string `json:"path"` + LockedAt time.Time `json:"locked_at"` + Owner *LFSLockOwner `json:"owner"` +} + +// LFSLockOwner represent a lock owner +// for use with the locks API. +type LFSLockOwner struct { + Name string `json:"name"` +} + +// LFSLockRequest contains the path of the lock to create +// https://github.com/git-lfs/git-lfs/blob/master/docs/api/locking.md#create-lock +type LFSLockRequest struct { + Path string `json:"path"` +} + +// LFSLockResponse represent a lock created +// https://github.com/git-lfs/git-lfs/blob/master/docs/api/locking.md#create-lock +type LFSLockResponse struct { + Lock *LFSLock `json:"lock"` +} + +// LFSLockList represent a list of lock requested +// https://github.com/git-lfs/git-lfs/blob/master/docs/api/locking.md#list-locks +type LFSLockList struct { + Locks []*LFSLock `json:"locks"` + Next string `json:"next_cursor,omitempty"` +} + +// LFSLockListVerify represent a list of lock verification requested +// https://github.com/git-lfs/git-lfs/blob/master/docs/api/locking.md#list-locks-for-verification +type LFSLockListVerify struct { + Ours []*LFSLock `json:"ours"` + Theirs []*LFSLock `json:"theirs"` + Next string `json:"next_cursor,omitempty"` +} + +// LFSLockError contains information on the error that occurs +type LFSLockError struct { + Message string `json:"message"` + Lock *LFSLock `json:"lock,omitempty"` + Documentation string `json:"documentation_url,omitempty"` + RequestID string `json:"request_id,omitempty"` +} diff --git a/vendor/vendor.json b/vendor/vendor.json index 8e91a66b881de..accd8f521bb66 100644 --- a/vendor/vendor.json +++ b/vendor/vendor.json @@ -9,10 +9,13 @@ "revisionTime": "2017-10-23T00:52:09Z" }, { - "checksumSHA1": "OICEgmUefW4L4l/FK/NVFnl/aOM=", + "checksumSHA1": "Y4vg8lfEOks0sFfFNK/nyhkSzkM=", + "origin": "github.com/sapk-fork/gitea-go-sdk/gitea", "path": "code.gitea.io/sdk/gitea", - "revision": "1da52cf95ff3e7953227cfa0469e1c05a7d02557", - "revisionTime": "2017-11-12T09:10:33Z" + "revision": "21fbc4f0ce5771f3ab489d768db7b27a92febb04", + "revisionTime": "2017-11-12T09:10:33Z", + "version": "=api-lfs-lock", + "versionExact": "api-lfs-lock" }, { "checksumSHA1": "bOODD4Gbw3GfcuQPU2dI40crxxk=", From 144ab60395df50d1dcaa78ba0431f00024bb73b6 Mon Sep 17 00:00:00 2001 From: Antoine GIRARD Date: Mon, 20 Nov 2017 00:45:15 +0100 Subject: [PATCH 03/29] Implement add + list --- models/lfs_lock.go | 31 +++++- models/models.go | 1 + modules/lfs/locks.go | 106 +++++++++++++++++++-- routers/routes/routes.go | 2 +- vendor/code.gitea.io/sdk/gitea/lfs_lock.go | 2 +- vendor/vendor.json | 10 +- 6 files changed, 134 insertions(+), 18 deletions(-) diff --git a/models/lfs_lock.go b/models/lfs_lock.go index ca2df489a63c5..97d5177fc78cb 100644 --- a/models/lfs_lock.go +++ b/models/lfs_lock.go @@ -7,7 +7,6 @@ package models import ( "time" "strconv" - "code.gitea.io/git" api "code.gitea.io/sdk/gitea" ) @@ -77,16 +76,20 @@ func IsLFSLockExist(repoID int64, path string) (bool, error) { } // CreateLFSLock creates a new lock. -func CreateLFSLock(gitRepo *git.Repository, lock *LFSLock) error { +func CreateLFSLock(lock *LFSLock) (*LFSLock, error) { isExist, err := IsLFSLockExist(lock.RepoID, lock.Path) if err != nil { - return err + return nil, err } else if isExist { - return ErrLFSLockAlreadyExist{lock.RepoID, lock.Path} + l, err := GetLFSLock(lock.RepoID, lock.Path) + if err != nil { + return nil, err + } + return l, ErrLFSLockAlreadyExist{lock.RepoID, lock.Path} } _, err = x.InsertOne(lock) - return err + return lock, err } // GetLFSLock returns release by given path. @@ -102,3 +105,21 @@ func GetLFSLock(repoID int64, path string) (*LFSLock, error) { _, err = x.Get(rel) return rel, err } + +// GetLFSLockByID returns release by given id. +func GetLFSLockByID(id int64) (*LFSLock, error) { + lock := new(LFSLock) + has, err := x.ID(id).Get(lock) + if err != nil { + return nil, err + } else if !has { + return nil, ErrLFSLockNotExist{id, 0, ""} + } + return lock, nil +} + +// GetLFSLockByRepoID returns a list of locks of repository. +func GetLFSLockByRepoID(repoID int64) (locks []*LFSLock, err error) { + err = x.Where("repo_id = ?", repoID).Find(&locks) + return locks, err +} diff --git a/models/models.go b/models/models.go index 836a14db5ae7c..8a3850b6ff5d8 100644 --- a/models/models.go +++ b/models/models.go @@ -117,6 +117,7 @@ func init() { new(TrackedTime), new(DeletedBranch), new(RepoIndexerStatus), + new(LFSLock), ) gonicNames := []string{"SSL", "UID"} diff --git a/modules/lfs/locks.go b/modules/lfs/locks.go index 779892142f1c0..23f2aeb3f5206 100644 --- a/modules/lfs/locks.go +++ b/modules/lfs/locks.go @@ -1,6 +1,10 @@ package lfs import ( + "encoding/json" + "strconv" + + "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/setting" api "code.gitea.io/sdk/gitea" @@ -18,15 +22,77 @@ func checkRequest(req macaron.Request) int { return 200 } -// GetLockHandler list locks -func GetLockHandler(ctx *context.Context) { +func handleLockListOut(ctx *context.Context, lock *models.LFSLock, err error) { + if err != nil { + if models.IsErrLFSLockNotExist(err) { + ctx.JSON(404, api.LFSLockError{ + Message: "not found : " + err.Error(), + }) + return + } + ctx.JSON(500, api.LFSLockError{ + Message: "internal server error : " + err.Error(), + }) + return + } + if ctx.Repo.Repository.ID != lock.RepoID { + ctx.JSON(404, api.LFSLockError{ + Message: "not found : " + err.Error(), + }) + return + } + ctx.JSON(200, api.LFSLockList{ + Locks: []*api.LFSLock{lock.APIFormat()}, + }) +} + +// GetListLockHandler list locks +func GetListLockHandler(ctx *context.Context) { status := checkRequest(ctx.Req) if status != 200 { writeStatus(ctx, status) return } ctx.Resp.Header().Set("Content-Type", metaMediaType) - ctx.JSON(404, api.LockError{Message: "Not found"}) + + //TODO handle query cursor and limit + + id := ctx.Query("id") + if id != "" { //Case where we request a specific id + v, err := strconv.ParseInt(id, 10, 64) + if err != nil { + ctx.JSON(400, api.LFSLockError{ + Message: "bad request : " + err.Error(), + }) + return + } + lock, err := models.GetLFSLockByID(int64(v)) + handleLockListOut(ctx, lock, err) + return + } + + path := ctx.Query("path") + if path != "" { //Case where we request a specific id + lock, err := models.GetLFSLock(ctx.Repo.Repository.ID, path) + handleLockListOut(ctx, lock, err) + return + } + + //If no query params path or id + lockList, err := models.GetLFSLockByRepoID(ctx.Repo.Repository.ID) + if err != nil { + ctx.JSON(500, api.LFSLockError{ + Message: "internal server error : " + err.Error(), + }) + return + } + lockListAPI := make([]*api.LFSLock, len(lockList)) + for i, l := range lockList { + lockListAPI[i] = l.APIFormat() + } + ctx.JSON(200, api.LFSLockList{ + Locks: lockListAPI, + }) } // PostLockHandler create lock @@ -37,7 +103,33 @@ func PostLockHandler(ctx *context.Context) { return } ctx.Resp.Header().Set("Content-Type", metaMediaType) - ctx.JSON(404, api.LockError{Message: "Not found"}) + + var req api.LFSLockRequest + dec := json.NewDecoder(ctx.Req.Body().ReadCloser()) + err := dec.Decode(&req) + if err != nil { + writeStatus(ctx, 400) + return + } + + lock, err := models.CreateLFSLock(&models.LFSLock{ + RepoID: ctx.Repo.Repository.ID, + Path: req.Path, + }) + if err != nil { + if models.IsErrLFSLockAlreadyExist(err) { + ctx.JSON(409, api.LFSLockError{ + Lock: lock.APIFormat(), + Message: "already created lock", + }) + return + } + ctx.JSON(500, api.LFSLockError{ + Message: "internal server error : " + err.Error(), + }) + return + } + ctx.JSON(201, api.LFSLockResponse{Lock: lock.APIFormat()}) } // VerifyLockHandler list locks for verification @@ -48,7 +140,8 @@ func VerifyLockHandler(ctx *context.Context) { return } ctx.Resp.Header().Set("Content-Type", metaMediaType) - ctx.JSON(404, api.LockError{Message: "Not found"}) + //TODO + ctx.JSON(404, api.LFSLockError{Message: "Not found"}) } // UnLockHandler delete locks @@ -59,5 +152,6 @@ func UnLockHandler(ctx *context.Context) { return } ctx.Resp.Header().Set("Content-Type", metaMediaType) - ctx.JSON(404, api.LockError{Message: "Not found"}) + //TODO + ctx.JSON(404, api.LFSLockError{Message: "Not found"}) } diff --git a/routers/routes/routes.go b/routers/routes/routes.go index 857e36b9e33b2..2ff1c6e136c5b 100644 --- a/routers/routes/routes.go +++ b/routers/routes/routes.go @@ -686,7 +686,7 @@ func RegisterRoutes(m *macaron.Macaron) { m.Any("/objects/:oid", lfs.ObjectOidHandler) m.Post("/objects", lfs.PostHandler) m.Post("/verify", lfs.VerifyHandler) - m.Get("/locks", lfs.GetLockHandler) + m.Get("/locks", lfs.GetListLockHandler) m.Post("/locks", lfs.PostLockHandler) m.Post("/locks/verify", lfs.VerifyLockHandler) m.Post("/locks/:id/unlock", lfs.UnLockHandler) diff --git a/vendor/code.gitea.io/sdk/gitea/lfs_lock.go b/vendor/code.gitea.io/sdk/gitea/lfs_lock.go index 8e9f007e68a58..ff660875aade1 100644 --- a/vendor/code.gitea.io/sdk/gitea/lfs_lock.go +++ b/vendor/code.gitea.io/sdk/gitea/lfs_lock.go @@ -1,4 +1,4 @@ -// Copyright 2015 The Gogs Authors. All rights reserved. +// Copyright 2017 The Gitea Authors. All rights reserved. // Use of this source code is governed by a MIT-style // license that can be found in the LICENSE file. diff --git a/vendor/vendor.json b/vendor/vendor.json index accd8f521bb66..9c070162f437e 100644 --- a/vendor/vendor.json +++ b/vendor/vendor.json @@ -9,13 +9,13 @@ "revisionTime": "2017-10-23T00:52:09Z" }, { - "checksumSHA1": "Y4vg8lfEOks0sFfFNK/nyhkSzkM=", + "checksumSHA1": "nnnuJ2nosJ3WVsAWvSkdR33PHKY=", "origin": "github.com/sapk-fork/gitea-go-sdk/gitea", "path": "code.gitea.io/sdk/gitea", - "revision": "21fbc4f0ce5771f3ab489d768db7b27a92febb04", - "revisionTime": "2017-11-12T09:10:33Z", - "version": "=api-lfs-lock", - "versionExact": "api-lfs-lock" + "revision": "838cc92ec431055a81a6e84ea91f3e36dc9a422b", + "revisionTime": "2017-11-19T22:19:51Z", + "version": "=add-api-lfs-lock", + "versionExact": "add-api-lfs-lock" }, { "checksumSHA1": "bOODD4Gbw3GfcuQPU2dI40crxxk=", From 3ef7315087b925e4b0d227f345d2af6bb9e06207 Mon Sep 17 00:00:00 2001 From: Antoine GIRARD Date: Mon, 20 Nov 2017 01:01:55 +0100 Subject: [PATCH 04/29] List return 200 empty list no 404 --- modules/lfs/locks.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/modules/lfs/locks.go b/modules/lfs/locks.go index 23f2aeb3f5206..40f941a8e98c0 100644 --- a/modules/lfs/locks.go +++ b/modules/lfs/locks.go @@ -12,6 +12,8 @@ import ( "gopkg.in/macaron.v1" ) +//TODO handle 403 forbidden + func checkRequest(req macaron.Request) int { if !setting.LFS.StartServer { return 404 @@ -25,8 +27,8 @@ func checkRequest(req macaron.Request) int { func handleLockListOut(ctx *context.Context, lock *models.LFSLock, err error) { if err != nil { if models.IsErrLFSLockNotExist(err) { - ctx.JSON(404, api.LFSLockError{ - Message: "not found : " + err.Error(), + ctx.JSON(200, api.LFSLockList{ + Locks: []*api.LFSLock{}, }) return } @@ -36,8 +38,8 @@ func handleLockListOut(ctx *context.Context, lock *models.LFSLock, err error) { return } if ctx.Repo.Repository.ID != lock.RepoID { - ctx.JSON(404, api.LFSLockError{ - Message: "not found : " + err.Error(), + ctx.JSON(200, api.LFSLockList{ + Locks: []*api.LFSLock{}, }) return } @@ -48,6 +50,7 @@ func handleLockListOut(ctx *context.Context, lock *models.LFSLock, err error) { // GetListLockHandler list locks func GetListLockHandler(ctx *context.Context) { + //TODO LFS Servers should ensure that users have at least pull access to the repository status := checkRequest(ctx.Req) if status != 200 { writeStatus(ctx, status) @@ -97,6 +100,7 @@ func GetListLockHandler(ctx *context.Context) { // PostLockHandler create lock func PostLockHandler(ctx *context.Context) { + //TODO Servers should ensure that users have push access to the repository, and that files are locked exclusively to one user. status := checkRequest(ctx.Req) if status != 200 { writeStatus(ctx, status) From 8ac22196a86244d078f5e0f35cb0f86b6682c3db Mon Sep 17 00:00:00 2001 From: Antoine GIRARD Date: Mon, 20 Nov 2017 01:10:30 +0100 Subject: [PATCH 05/29] Add verify lfs lock api --- modules/lfs/locks.go | 29 +++++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/modules/lfs/locks.go b/modules/lfs/locks.go index 40f941a8e98c0..5215fe5d378a4 100644 --- a/modules/lfs/locks.go +++ b/modules/lfs/locks.go @@ -33,7 +33,7 @@ func handleLockListOut(ctx *context.Context, lock *models.LFSLock, err error) { return } ctx.JSON(500, api.LFSLockError{ - Message: "internal server error : " + err.Error(), + Message: "unable to list locks : " + err.Error(), }) return } @@ -85,7 +85,7 @@ func GetListLockHandler(ctx *context.Context) { lockList, err := models.GetLFSLockByRepoID(ctx.Repo.Repository.ID) if err != nil { ctx.JSON(500, api.LFSLockError{ - Message: "internal server error : " + err.Error(), + Message: "unable to list locks : " + err.Error(), }) return } @@ -138,18 +138,39 @@ func PostLockHandler(ctx *context.Context) { // VerifyLockHandler list locks for verification func VerifyLockHandler(ctx *context.Context) { + //TODO LFS Servers should ensure that users have push access to the repository. status := checkRequest(ctx.Req) if status != 200 { writeStatus(ctx, status) return } ctx.Resp.Header().Set("Content-Type", metaMediaType) - //TODO - ctx.JSON(404, api.LFSLockError{Message: "Not found"}) + //TODO handle body json cursor and limit + lockList, err := models.GetLFSLockByRepoID(ctx.Repo.Repository.ID) + if err != nil { + ctx.JSON(500, api.LFSLockError{ + Message: "unable to list locks : " + err.Error(), + }) + return + } + lockOursListAPI := make([]*api.LFSLock, 0, len(lockList)) + lockTheirsListAPI := make([]*api.LFSLock, 0, len(lockList)) + for i, l := range lockList { + if l.Owner.ID == ctx.User.ID { + lockOursListAPI[i] = l.APIFormat() + } else { + lockTheirsListAPI[i] = l.APIFormat() + } + } + ctx.JSON(200, api.LFSLockListVerify{ + Ours: lockOursListAPI, + Theirs: lockTheirsListAPI, + }) } // UnLockHandler delete locks func UnLockHandler(ctx *context.Context) { + //TODO LFS servers should ensure that callers have push access to the repository. They should also prevent a user from deleting another user's lock, unless the force property is given. status := checkRequest(ctx.Req) if status != 200 { writeStatus(ctx, status) From b3ad36662726cdd1174da9c28630258ec1851aae Mon Sep 17 00:00:00 2001 From: Antoine GIRARD Date: Mon, 20 Nov 2017 01:46:55 +0100 Subject: [PATCH 06/29] Add delete and start implementing auth control --- models/error.go | 18 +++++++++ models/lfs_lock.go | 47 ++++++++++++++++++++-- modules/lfs/locks.go | 33 +++++++++++++-- vendor/code.gitea.io/sdk/gitea/lfs_lock.go | 6 +++ vendor/vendor.json | 6 +-- 5 files changed, 99 insertions(+), 11 deletions(-) diff --git a/models/error.go b/models/error.go index bf922940272cb..4a4ffef124f38 100644 --- a/models/error.go +++ b/models/error.go @@ -512,6 +512,24 @@ func (err ErrLFSLockNotExist) Error() string { return fmt.Sprintf("lfs lock does not exist [id: %d, rid: %d, path: %s]", err.ID, err.RepoID, err.Path) } +// ErrLFSLockUnauthorizedAction represents a "LFSLockUnauthorizedAction" kind of error. +type ErrLFSLockUnauthorizedAction struct { + ID int64 + RepoID int64 + User *User + Action string +} + +// IsErrLFSLockUnauthorizedAction checks if an error is a ErrLFSLockUnauthorizedAction. +func IsErrLFSLockUnauthorizedAction(err error) bool { + _, ok := err.(ErrLFSLockUnauthorizedAction) + return ok +} + +func (err ErrLFSLockUnauthorizedAction) Error() string { + return fmt.Sprintf("User %s doesn't rigth to %s for lfs lock [id: %d, rid: %d]", err.User.DisplayName(), err.Action, err.ID, err.RepoID) +} + // ErrLFSLockAlreadyExist represents a "LFSLockAlreadyExist" kind of error. type ErrLFSLockAlreadyExist struct { RepoID int64 diff --git a/models/lfs_lock.go b/models/lfs_lock.go index 97d5177fc78cb..e029e9083524b 100644 --- a/models/lfs_lock.go +++ b/models/lfs_lock.go @@ -5,9 +5,10 @@ package models import ( - "time" -"strconv" api "code.gitea.io/sdk/gitea" + "fmt" + "strconv" + "time" ) // LFSLock represents a git lfs lfock of repository. @@ -69,14 +70,13 @@ func (l *LFSLock) APIFormat() *api.LFSLock { } } - // IsLFSLockExist returns true if lock with given path already exists. func IsLFSLockExist(repoID int64, path string) (bool, error) { return x.Get(&LFSLock{RepoID: repoID, Path: path}) //TODO Define if path should needed to be lower for windows compat ? } // CreateLFSLock creates a new lock. -func CreateLFSLock(lock *LFSLock) (*LFSLock, error) { +func CreateLFSLock(lock *LFSLock, u *User) (*LFSLock, error) { isExist, err := IsLFSLockExist(lock.RepoID, lock.Path) if err != nil { return nil, err @@ -88,6 +88,18 @@ func CreateLFSLock(lock *LFSLock) (*LFSLock, error) { return l, ErrLFSLockAlreadyExist{lock.RepoID, lock.Path} } + repo, err := GetRepositoryByID(lock.RepoID) + if err != nil { + return nil, err + } + + has, err := HasAccess(u.ID, repo, AccessModeWrite) + if err != nil { + return nil, err + } else if !has { + return nil, ErrLFSLockUnauthorizedAction{0, lock.RepoID, u, "create"} + } + _, err = x.InsertOne(lock) return lock, err } @@ -123,3 +135,30 @@ func GetLFSLockByRepoID(repoID int64) (locks []*LFSLock, err error) { err = x.Where("repo_id = ?", repoID).Find(&locks) return locks, err } + +// DeleteLFSLockByID deletes a lock by given ID. +func DeleteLFSLockByID(id int64, u *User, force bool) error { + + lock, err := GetLFSLockByID(id) + if err != nil { + return err + } + repo, err := GetRepositoryByID(lock.RepoID) + if err != nil { + return err + } + + has, err := HasAccess(u.ID, repo, AccessModeWrite) + if err != nil { + return err + } else if !has { + return ErrLFSLockUnauthorizedAction{id, repo.ID, u, "delete"} + } + + if !force && u.ID != lock.OwnerID { + return fmt.Errorf("user doesn't own lock and force flag is not set") + } + + _, err = x.ID(id).Delete(new(LFSLock)) + return err +} diff --git a/modules/lfs/locks.go b/modules/lfs/locks.go index 5215fe5d378a4..62d353fb0dd5e 100644 --- a/modules/lfs/locks.go +++ b/modules/lfs/locks.go @@ -100,7 +100,6 @@ func GetListLockHandler(ctx *context.Context) { // PostLockHandler create lock func PostLockHandler(ctx *context.Context) { - //TODO Servers should ensure that users have push access to the repository, and that files are locked exclusively to one user. status := checkRequest(ctx.Req) if status != 200 { writeStatus(ctx, status) @@ -119,7 +118,7 @@ func PostLockHandler(ctx *context.Context) { lock, err := models.CreateLFSLock(&models.LFSLock{ RepoID: ctx.Repo.Repository.ID, Path: req.Path, - }) + }, ctx.User) if err != nil { if models.IsErrLFSLockAlreadyExist(err) { ctx.JSON(409, api.LFSLockError{ @@ -128,6 +127,12 @@ func PostLockHandler(ctx *context.Context) { }) return } + if models.IsErrLFSLockUnauthorizedAction(err) { + ctx.JSON(403, api.LFSLockError{ + Message: "You must have push access to create locks : " + err.Error(), + }) + return + } ctx.JSON(500, api.LFSLockError{ Message: "internal server error : " + err.Error(), }) @@ -170,13 +175,33 @@ func VerifyLockHandler(ctx *context.Context) { // UnLockHandler delete locks func UnLockHandler(ctx *context.Context) { - //TODO LFS servers should ensure that callers have push access to the repository. They should also prevent a user from deleting another user's lock, unless the force property is given. status := checkRequest(ctx.Req) if status != 200 { writeStatus(ctx, status) return } ctx.Resp.Header().Set("Content-Type", metaMediaType) - //TODO + + var req api.LFSLockDeleteRequest + dec := json.NewDecoder(ctx.Req.Body().ReadCloser()) + err := dec.Decode(&req) + if err != nil { + writeStatus(ctx, 400) + return + } + + err = models.DeleteLFSLockByID(ctx.ParamsInt64("id"), ctx.User, req.Force) + if err != nil { + if models.IsErrLFSLockUnauthorizedAction(err) { + ctx.JSON(403, api.LFSLockError{ + Message: "You must have push access to delete locks : " + err.Error(), + }) + return + } + ctx.JSON(500, api.LFSLockError{ + Message: "unable to delete lock : " + err.Error(), + }) + return + } ctx.JSON(404, api.LFSLockError{Message: "Not found"}) } diff --git a/vendor/code.gitea.io/sdk/gitea/lfs_lock.go b/vendor/code.gitea.io/sdk/gitea/lfs_lock.go index ff660875aade1..356636a3a2519 100644 --- a/vendor/code.gitea.io/sdk/gitea/lfs_lock.go +++ b/vendor/code.gitea.io/sdk/gitea/lfs_lock.go @@ -57,3 +57,9 @@ type LFSLockError struct { Documentation string `json:"documentation_url,omitempty"` RequestID string `json:"request_id,omitempty"` } + +// LFSLockDeleteRequest contains params of a delete request +// https://github.com/git-lfs/git-lfs/blob/master/docs/api/locking.md#delete-lock +type LFSLockDeleteRequest struct { + Force bool `json:"force"` +} diff --git a/vendor/vendor.json b/vendor/vendor.json index 9c070162f437e..8d568adfc1a82 100644 --- a/vendor/vendor.json +++ b/vendor/vendor.json @@ -9,11 +9,11 @@ "revisionTime": "2017-10-23T00:52:09Z" }, { - "checksumSHA1": "nnnuJ2nosJ3WVsAWvSkdR33PHKY=", + "checksumSHA1": "pRjEfdHYr4Esk+FMC+c9UA2BAIE=", "origin": "github.com/sapk-fork/gitea-go-sdk/gitea", "path": "code.gitea.io/sdk/gitea", - "revision": "838cc92ec431055a81a6e84ea91f3e36dc9a422b", - "revisionTime": "2017-11-19T22:19:51Z", + "revision": "7463c04702e61963168607e211a8216bbb035a69", + "revisionTime": "2017-11-20T00:39:56Z", "version": "=add-api-lfs-lock", "versionExact": "add-api-lfs-lock" }, From bcd3b391cb97d44b18974e5fc1ae763c384381cf Mon Sep 17 00:00:00 2001 From: Antoine GIRARD Date: Tue, 21 Nov 2017 22:36:10 +0100 Subject: [PATCH 07/29] Revert to code.gitea.io/sdk/gitea vendor --- vendor/vendor.json | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/vendor/vendor.json b/vendor/vendor.json index 8d568adfc1a82..d9aed5006f2e9 100644 --- a/vendor/vendor.json +++ b/vendor/vendor.json @@ -10,12 +10,9 @@ }, { "checksumSHA1": "pRjEfdHYr4Esk+FMC+c9UA2BAIE=", - "origin": "github.com/sapk-fork/gitea-go-sdk/gitea", "path": "code.gitea.io/sdk/gitea", - "revision": "7463c04702e61963168607e211a8216bbb035a69", - "revisionTime": "2017-11-20T00:39:56Z", - "version": "=add-api-lfs-lock", - "versionExact": "add-api-lfs-lock" + "revision": "4319a1eaff39116a6d5150b09acaca7f5144defe", + "revisionTime": "2017-11-20T06:55:09Z" }, { "checksumSHA1": "bOODD4Gbw3GfcuQPU2dI40crxxk=", From 74a04bc1af89ab0841100275e7108af6a45a0897 Mon Sep 17 00:00:00 2001 From: Antoine GIRARD Date: Tue, 21 Nov 2017 23:02:09 +0100 Subject: [PATCH 08/29] Apply needed check for all lfs locks route --- models/error.go | 3 +-- models/lfs_lock.go | 45 ++++++++++++++++++++++++-------------------- modules/lfs/locks.go | 34 ++++++++++++++++++++++++++++----- 3 files changed, 55 insertions(+), 27 deletions(-) diff --git a/models/error.go b/models/error.go index 4a4ffef124f38..a6f6f933037e6 100644 --- a/models/error.go +++ b/models/error.go @@ -514,7 +514,6 @@ func (err ErrLFSLockNotExist) Error() string { // ErrLFSLockUnauthorizedAction represents a "LFSLockUnauthorizedAction" kind of error. type ErrLFSLockUnauthorizedAction struct { - ID int64 RepoID int64 User *User Action string @@ -527,7 +526,7 @@ func IsErrLFSLockUnauthorizedAction(err error) bool { } func (err ErrLFSLockUnauthorizedAction) Error() string { - return fmt.Sprintf("User %s doesn't rigth to %s for lfs lock [id: %d, rid: %d]", err.User.DisplayName(), err.Action, err.ID, err.RepoID) + return fmt.Sprintf("User %s doesn't rigth to %s for lfs lock [rid: %d]", err.User.DisplayName(), err.Action, err.RepoID) } // ErrLFSLockAlreadyExist represents a "LFSLockAlreadyExist" kind of error. diff --git a/models/lfs_lock.go b/models/lfs_lock.go index e029e9083524b..0d0f947e67fdf 100644 --- a/models/lfs_lock.go +++ b/models/lfs_lock.go @@ -77,6 +77,10 @@ func IsLFSLockExist(repoID int64, path string) (bool, error) { // CreateLFSLock creates a new lock. func CreateLFSLock(lock *LFSLock, u *User) (*LFSLock, error) { + err := CheckLFSAccessForRepo(u, lock.RepoID, "create") + if err != nil { + return nil, err + } isExist, err := IsLFSLockExist(lock.RepoID, lock.Path) if err != nil { return nil, err @@ -87,19 +91,6 @@ func CreateLFSLock(lock *LFSLock, u *User) (*LFSLock, error) { } return l, ErrLFSLockAlreadyExist{lock.RepoID, lock.Path} } - - repo, err := GetRepositoryByID(lock.RepoID) - if err != nil { - return nil, err - } - - has, err := HasAccess(u.ID, repo, AccessModeWrite) - if err != nil { - return nil, err - } else if !has { - return nil, ErrLFSLockUnauthorizedAction{0, lock.RepoID, u, "create"} - } - _, err = x.InsertOne(lock) return lock, err } @@ -143,16 +134,10 @@ func DeleteLFSLockByID(id int64, u *User, force bool) error { if err != nil { return err } - repo, err := GetRepositoryByID(lock.RepoID) - if err != nil { - return err - } - has, err := HasAccess(u.ID, repo, AccessModeWrite) + err = CheckLFSAccessForRepo(u, lock.RepoID, "delete") if err != nil { return err - } else if !has { - return ErrLFSLockUnauthorizedAction{id, repo.ID, u, "delete"} } if !force && u.ID != lock.OwnerID { @@ -162,3 +147,23 @@ func DeleteLFSLockByID(id int64, u *User, force bool) error { _, err = x.ID(id).Delete(new(LFSLock)) return err } + +//CheckLFSAccessForRepo check needed access mode base on action +func CheckLFSAccessForRepo(u *User, repoID int64, action string) error { + mode := AccessModeRead + if action == "create" || action == "delete" || action == "verify" { + mode = AccessModeWrite + } + + repo, err := GetRepositoryByID(repoID) + if err != nil { + return err + } + has, err := HasAccess(u.ID, repo, mode) + if err != nil { + return err + } else if !has { + return ErrLFSLockUnauthorizedAction{repo.ID, u, action} + } + return nil +} diff --git a/modules/lfs/locks.go b/modules/lfs/locks.go index 62d353fb0dd5e..15d6d8bd99f38 100644 --- a/modules/lfs/locks.go +++ b/modules/lfs/locks.go @@ -12,8 +12,6 @@ import ( "gopkg.in/macaron.v1" ) -//TODO handle 403 forbidden - func checkRequest(req macaron.Request) int { if !setting.LFS.StartServer { return 404 @@ -50,7 +48,6 @@ func handleLockListOut(ctx *context.Context, lock *models.LFSLock, err error) { // GetListLockHandler list locks func GetListLockHandler(ctx *context.Context) { - //TODO LFS Servers should ensure that users have at least pull access to the repository status := checkRequest(ctx.Req) if status != 200 { writeStatus(ctx, status) @@ -58,8 +55,20 @@ func GetListLockHandler(ctx *context.Context) { } ctx.Resp.Header().Set("Content-Type", metaMediaType) + err := models.CheckLFSAccessForRepo(ctx.User, ctx.Repo.Repository.ID, "list") + if err != nil { + if models.IsErrLFSLockUnauthorizedAction(err) { + ctx.JSON(403, api.LFSLockError{ + Message: "You must have pull access to list locks : " + err.Error(), + }) + return + } + ctx.JSON(500, api.LFSLockError{ + Message: "unable to list lock : " + err.Error(), + }) + return + } //TODO handle query cursor and limit - id := ctx.Query("id") if id != "" { //Case where we request a specific id v, err := strconv.ParseInt(id, 10, 64) @@ -143,13 +152,28 @@ func PostLockHandler(ctx *context.Context) { // VerifyLockHandler list locks for verification func VerifyLockHandler(ctx *context.Context) { - //TODO LFS Servers should ensure that users have push access to the repository. status := checkRequest(ctx.Req) if status != 200 { writeStatus(ctx, status) return } + ctx.Resp.Header().Set("Content-Type", metaMediaType) + + err := models.CheckLFSAccessForRepo(ctx.User, ctx.Repo.Repository.ID, "verify") + if err != nil { + if models.IsErrLFSLockUnauthorizedAction(err) { + ctx.JSON(403, api.LFSLockError{ + Message: "You must have push access to verify locks : " + err.Error(), + }) + return + } + ctx.JSON(500, api.LFSLockError{ + Message: "unable to verify lock : " + err.Error(), + }) + return + } + //TODO handle body json cursor and limit lockList, err := models.GetLFSLockByRepoID(ctx.Repo.Repository.ID) if err != nil { From 82bab1bd634f073db348331e8d4025cd14683785 Mon Sep 17 00:00:00 2001 From: Antoine GIRARD Date: Wed, 22 Nov 2017 00:13:16 +0100 Subject: [PATCH 09/29] Add simple tests --- integrations/api_repo_lfs_locks_test.go | 73 +++++++++++++++++++++++++ integrations/mysql.ini.tmpl | 3 +- integrations/pgsql.ini.tmpl | 2 +- integrations/sqlite.ini | 27 ++++----- models/error.go | 8 +-- models/lfs_lock.go | 5 +- modules/lfs/locks.go | 8 ++- routers/routes/routes.go | 10 ++-- 8 files changed, 108 insertions(+), 28 deletions(-) create mode 100644 integrations/api_repo_lfs_locks_test.go diff --git a/integrations/api_repo_lfs_locks_test.go b/integrations/api_repo_lfs_locks_test.go new file mode 100644 index 0000000000000..9df72c7d76afc --- /dev/null +++ b/integrations/api_repo_lfs_locks_test.go @@ -0,0 +1,73 @@ +// Copyright 2017 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package integrations + +import ( + "fmt" + "net/http" + "testing" + + "code.gitea.io/gitea/models" + "code.gitea.io/gitea/modules/setting" + api "code.gitea.io/sdk/gitea" + + "github.com/stretchr/testify/assert" +) + +func TestAPILFSLocksNotStarted(t *testing.T) { + prepareTestEnv(t) + setting.LFS.StartServer = false + user := models.AssertExistsAndLoadBean(t, &models.User{ID: 2}).(*models.User) + repo := models.AssertExistsAndLoadBean(t, &models.Repository{ID: 1}).(*models.Repository) + + req := NewRequestf(t, "GET", "/%s/%s/info/lfs/locks", user.Name, repo.Name) + MakeRequest(t, req, http.StatusNotFound) + req = NewRequestf(t, "POST", "/%s/%s/info/lfs/locks", user.Name, repo.Name) + MakeRequest(t, req, http.StatusNotFound) + req = NewRequestf(t, "GET", "/%s/%s/info/lfs/locks/verify", user.Name, repo.Name) + MakeRequest(t, req, http.StatusNotFound) + req = NewRequestf(t, "GET", "/%s/%s/info/lfs/locks/10/unlock", user.Name, repo.Name) + MakeRequest(t, req, http.StatusNotFound) +} + +func TestAPILFSLocksNotLogin(t *testing.T) { + prepareTestEnv(t) + setting.LFS.StartServer = true + user := models.AssertExistsAndLoadBean(t, &models.User{ID: 2}).(*models.User) + repo := models.AssertExistsAndLoadBean(t, &models.Repository{ID: 1}).(*models.Repository) + + req := NewRequestf(t, "GET", "/%s/%s/info/lfs/locks", user.Name, repo.Name) + req.Header.Add("Accept", "application/vnd.git-lfs+json") + req.Header.Add("Content-Type", "application/vnd.git-lfs+json") + resp := MakeRequest(t, req, http.StatusForbidden) + fmt.Println(string(resp.Body)) + var lfsLockError api.LFSLockError + DecodeJSON(t, resp, &lfsLockError) + assert.Equal(t, "You must have pull access to list locks : User undefined doesn't have rigth to list for lfs lock [rid: 1]", lfsLockError.Message) +} + +func TestAPILFSLocksLogged(t *testing.T) { + prepareTestEnv(t) + setting.LFS.StartServer = true + session := loginUser(t, "user2") + user := models.AssertExistsAndLoadBean(t, &models.User{ID: 2}).(*models.User) + repo := models.AssertExistsAndLoadBean(t, &models.Repository{ID: 1}).(*models.Repository) + + req := NewRequestf(t, "GET", "/%s/%s/info/lfs/locks", user.Name, repo.Name) + req.Header.Add("Accept", "application/vnd.git-lfs+json") + req.Header.Add("Content-Type", "application/vnd.git-lfs+json") + resp := session.MakeRequest(t, req, http.StatusOK) + + fmt.Println(string(resp.Body)) + var lfsLocks api.LFSLockList + DecodeJSON(t, resp, &lfsLocks) + assert.Len(t, lfsLocks.Locks, 0) + /* + for _, repo := range apiRepos { + assert.EqualValues(t, user.ID, repo.Owner.ID) + assert.False(t, repo.Private) + } + */ +} diff --git a/integrations/mysql.ini.tmpl b/integrations/mysql.ini.tmpl index fb1a4f5810589..e01362607b396 100644 --- a/integrations/mysql.ini.tmpl +++ b/integrations/mysql.ini.tmpl @@ -27,7 +27,7 @@ HTTP_PORT = 3001 ROOT_URL = http://localhost:3001/ DISABLE_SSH = false SSH_PORT = 22 -LFS_START_SERVER = false +LFS_START_SERVER = true OFFLINE_MODE = false [mailer] @@ -65,4 +65,3 @@ LEVEL = Debug INSTALL_LOCK = true SECRET_KEY = 9pCviYTWSb INTERNAL_TOKEN = eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJuYmYiOjE0OTU1NTE2MTh9.hhSVGOANkaKk3vfCd2jDOIww4pUk0xtg9JRde5UogyQ - diff --git a/integrations/pgsql.ini.tmpl b/integrations/pgsql.ini.tmpl index 01916af9ac913..853d6fb3be35d 100644 --- a/integrations/pgsql.ini.tmpl +++ b/integrations/pgsql.ini.tmpl @@ -27,7 +27,7 @@ HTTP_PORT = 3002 ROOT_URL = http://localhost:3002/ DISABLE_SSH = false SSH_PORT = 22 -LFS_START_SERVER = false +LFS_START_SERVER = true OFFLINE_MODE = false [mailer] diff --git a/integrations/sqlite.ini b/integrations/sqlite.ini index 8a3a5356b4e1a..b3462a19b43d4 100644 --- a/integrations/sqlite.ini +++ b/integrations/sqlite.ini @@ -2,13 +2,13 @@ APP_NAME = Gitea: Git with a cup of tea RUN_MODE = prod [database] -DB_TYPE = sqlite3 -PATH = :memory: +DB_TYPE = sqlite3 +PATH = :memory: [indexer] -ISSUE_INDEXER_PATH = integrations/indexers-sqlite/issues.bleve +ISSUE_INDEXER_PATH = integrations/indexers-sqlite/issues.bleve REPO_INDEXER_ENABLED = true -REPO_INDEXER_PATH = integrations/indexers-sqlite/repos.bleve +REPO_INDEXER_PATH = integrations/indexers-sqlite/repos.bleve [repository] ROOT = integrations/gitea-integration-sqlite/gitea-repositories @@ -22,21 +22,22 @@ HTTP_PORT = 3003 ROOT_URL = http://localhost:3003/ DISABLE_SSH = false SSH_PORT = 22 -LFS_START_SERVER = false +LFS_START_SERVER = true OFFLINE_MODE = false +LFS_JWT_SECRET = Tv_MjmZuHqpIY6GFl12ebgkRAMt4RlWt0v4EHKSXO0w [mailer] ENABLED = false [service] -REGISTER_EMAIL_CONFIRM = false -ENABLE_NOTIFY_MAIL = false -DISABLE_REGISTRATION = false -ENABLE_CAPTCHA = false -REQUIRE_SIGNIN_VIEW = false -DEFAULT_KEEP_EMAIL_PRIVATE = false +REGISTER_EMAIL_CONFIRM = false +ENABLE_NOTIFY_MAIL = false +DISABLE_REGISTRATION = false +ENABLE_CAPTCHA = false +REQUIRE_SIGNIN_VIEW = false +DEFAULT_KEEP_EMAIL_PRIVATE = false DEFAULT_ALLOW_CREATE_ORGANIZATION = true -NO_REPLY_ADDRESS = noreply.example.org +NO_REPLY_ADDRESS = noreply.example.org [picture] DISABLE_GRAVATAR = false @@ -46,7 +47,7 @@ ENABLE_FEDERATED_AVATAR = false PROVIDER = file [log] -MODE = console,file +MODE = console,file ROOT_PATH = sqlite-log [log.console] diff --git a/models/error.go b/models/error.go index a6f6f933037e6..d6c3a22b7b9cd 100644 --- a/models/error.go +++ b/models/error.go @@ -514,9 +514,9 @@ func (err ErrLFSLockNotExist) Error() string { // ErrLFSLockUnauthorizedAction represents a "LFSLockUnauthorizedAction" kind of error. type ErrLFSLockUnauthorizedAction struct { - RepoID int64 - User *User - Action string + RepoID int64 + UserName string + Action string } // IsErrLFSLockUnauthorizedAction checks if an error is a ErrLFSLockUnauthorizedAction. @@ -526,7 +526,7 @@ func IsErrLFSLockUnauthorizedAction(err error) bool { } func (err ErrLFSLockUnauthorizedAction) Error() string { - return fmt.Sprintf("User %s doesn't rigth to %s for lfs lock [rid: %d]", err.User.DisplayName(), err.Action, err.RepoID) + return fmt.Sprintf("User %s doesn't have rigth to %s for lfs lock [rid: %d]", err.UserName, err.Action, err.RepoID) } // ErrLFSLockAlreadyExist represents a "LFSLockAlreadyExist" kind of error. diff --git a/models/lfs_lock.go b/models/lfs_lock.go index 0d0f947e67fdf..7430dc6c6dec7 100644 --- a/models/lfs_lock.go +++ b/models/lfs_lock.go @@ -150,6 +150,9 @@ func DeleteLFSLockByID(id int64, u *User, force bool) error { //CheckLFSAccessForRepo check needed access mode base on action func CheckLFSAccessForRepo(u *User, repoID int64, action string) error { + if u == nil { + return ErrLFSLockUnauthorizedAction{repoID, "undefined", action} + } mode := AccessModeRead if action == "create" || action == "delete" || action == "verify" { mode = AccessModeWrite @@ -163,7 +166,7 @@ func CheckLFSAccessForRepo(u *User, repoID int64, action string) error { if err != nil { return err } else if !has { - return ErrLFSLockUnauthorizedAction{repo.ID, u, action} + return ErrLFSLockUnauthorizedAction{repo.ID, u.DisplayName(), action} } return nil } diff --git a/modules/lfs/locks.go b/modules/lfs/locks.go index 15d6d8bd99f38..b26a847432142 100644 --- a/modules/lfs/locks.go +++ b/modules/lfs/locks.go @@ -16,9 +16,11 @@ func checkRequest(req macaron.Request) int { if !setting.LFS.StartServer { return 404 } - if !ContentMatcher(req) || req.Header.Get("Content-Type") != contentMediaType { - return 400 - } + /* + if !ContentMatcher(req) || req.Header.Get("Content-Type") != contentMediaType { + return 400 + } + */ return 200 } diff --git a/routers/routes/routes.go b/routers/routes/routes.go index 2ff1c6e136c5b..4882aab6dd271 100644 --- a/routers/routes/routes.go +++ b/routers/routes/routes.go @@ -686,10 +686,12 @@ func RegisterRoutes(m *macaron.Macaron) { m.Any("/objects/:oid", lfs.ObjectOidHandler) m.Post("/objects", lfs.PostHandler) m.Post("/verify", lfs.VerifyHandler) - m.Get("/locks", lfs.GetListLockHandler) - m.Post("/locks", lfs.PostLockHandler) - m.Post("/locks/verify", lfs.VerifyLockHandler) - m.Post("/locks/:id/unlock", lfs.UnLockHandler) + m.Group("/locks", func() { + m.Get("/", lfs.GetListLockHandler) + m.Post("/", lfs.PostLockHandler) + m.Post("/verify", lfs.VerifyLockHandler) + m.Post("/:id/unlock", lfs.UnLockHandler) + }, context.RepoAssignment()) m.Any("/*", func(ctx *context.Context) { ctx.Handle(404, "", nil) }) From cc66370e1fb3b4fc640a5bae7746563ecc5adc50 Mon Sep 17 00:00:00 2001 From: Antoine GIRARD Date: Wed, 22 Nov 2017 00:16:00 +0100 Subject: [PATCH 10/29] fix lint --- models/error.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/error.go b/models/error.go index d6c3a22b7b9cd..97c3a463c092f 100644 --- a/models/error.go +++ b/models/error.go @@ -495,7 +495,7 @@ func (err ErrLastOrgOwner) Error() string { // TODO LFS header // -// ErrRepoNotExist represents a "LFSLockNotExist" kind of error. +// ErrLFSLockNotExist represents a "LFSLockNotExist" kind of error. type ErrLFSLockNotExist struct { ID int64 RepoID int64 From 18eed10756b15ca43baa75b88652f676c5fa4dfd Mon Sep 17 00:00:00 2001 From: Antoine GIRARD Date: Wed, 22 Nov 2017 01:06:34 +0100 Subject: [PATCH 11/29] Improve tests --- integrations/api_repo_lfs_locks_test.go | 62 +++++++++++++++++++------ models/lfs_lock.go | 8 ++-- modules/lfs/locks.go | 17 ++++--- 3 files changed, 62 insertions(+), 25 deletions(-) diff --git a/integrations/api_repo_lfs_locks_test.go b/integrations/api_repo_lfs_locks_test.go index 9df72c7d76afc..b8b69431c72b9 100644 --- a/integrations/api_repo_lfs_locks_test.go +++ b/integrations/api_repo_lfs_locks_test.go @@ -6,7 +6,9 @@ package integrations import ( "fmt" + "io/ioutil" "net/http" + "strings" "testing" "code.gitea.io/gitea/models" @@ -39,10 +41,9 @@ func TestAPILFSLocksNotLogin(t *testing.T) { repo := models.AssertExistsAndLoadBean(t, &models.Repository{ID: 1}).(*models.Repository) req := NewRequestf(t, "GET", "/%s/%s/info/lfs/locks", user.Name, repo.Name) - req.Header.Add("Accept", "application/vnd.git-lfs+json") - req.Header.Add("Content-Type", "application/vnd.git-lfs+json") + req.Header.Set("Accept", "application/vnd.git-lfs+json") + req.Header.Set("Content-Type", "application/vnd.git-lfs+json") resp := MakeRequest(t, req, http.StatusForbidden) - fmt.Println(string(resp.Body)) var lfsLockError api.LFSLockError DecodeJSON(t, resp, &lfsLockError) assert.Equal(t, "You must have pull access to list locks : User undefined doesn't have rigth to list for lfs lock [rid: 1]", lfsLockError.Message) @@ -56,18 +57,53 @@ func TestAPILFSLocksLogged(t *testing.T) { repo := models.AssertExistsAndLoadBean(t, &models.Repository{ID: 1}).(*models.Repository) req := NewRequestf(t, "GET", "/%s/%s/info/lfs/locks", user.Name, repo.Name) - req.Header.Add("Accept", "application/vnd.git-lfs+json") - req.Header.Add("Content-Type", "application/vnd.git-lfs+json") + req.Header.Set("Accept", "application/vnd.git-lfs+json") + req.Header.Set("Content-Type", "application/vnd.git-lfs+json") resp := session.MakeRequest(t, req, http.StatusOK) - - fmt.Println(string(resp.Body)) var lfsLocks api.LFSLockList DecodeJSON(t, resp, &lfsLocks) assert.Len(t, lfsLocks.Locks, 0) - /* - for _, repo := range apiRepos { - assert.EqualValues(t, user.ID, repo.Owner.ID) - assert.False(t, repo.Private) - } - */ + + req = NewRequestf(t, "POST", "/%s/%s/info/lfs/locks", user.Name, repo.Name) + req.Header.Set("Accept", "application/vnd.git-lfs+json") + req.Header.Set("Content-Type", "application/vnd.git-lfs+json") + req.Body = ioutil.NopCloser(strings.NewReader("{\"path\": \"foo/bar.zip\"}")) + resp = session.MakeRequest(t, req, http.StatusCreated) + + req = NewRequestf(t, "POST", "/%s/%s/info/lfs/locks", user.Name, repo.Name) + req.Header.Set("Accept", "application/vnd.git-lfs+json") + req.Header.Set("Content-Type", "application/vnd.git-lfs+json") + req.Body = ioutil.NopCloser(strings.NewReader("{\"path\": \"path/test\"}")) + resp = session.MakeRequest(t, req, http.StatusCreated) + + req = NewRequestf(t, "POST", "/%s/%s/info/lfs/locks", user.Name, repo.Name) + req.Header.Set("Accept", "application/vnd.git-lfs+json") + req.Header.Set("Content-Type", "application/vnd.git-lfs+json") + req.Body = ioutil.NopCloser(strings.NewReader("{\"path\": \"path/test\"}")) + resp = session.MakeRequest(t, req, http.StatusConflict) + + req = NewRequestf(t, "GET", "/%s/%s/info/lfs/locks", user.Name, repo.Name) + req.Header.Set("Accept", "application/vnd.git-lfs+json") + req.Header.Set("Content-Type", "application/vnd.git-lfs+json") + resp = session.MakeRequest(t, req, http.StatusOK) + DecodeJSON(t, resp, &lfsLocks) + assert.Len(t, lfsLocks.Locks, 2) + for _, lock := range lfsLocks.Locks { + assert.EqualValues(t, user.DisplayName(), lock.Owner.Name) + } + + req = NewRequestf(t, "POST", "/%s/%s/info/lfs/locks/verify", user.Name, repo.Name) + req.Header.Set("Accept", "application/vnd.git-lfs+json") + req.Header.Set("Content-Type", "application/vnd.git-lfs+json") + req.Body = ioutil.NopCloser(strings.NewReader("{}")) + resp = session.MakeRequest(t, req, http.StatusOK) + fmt.Println(string(resp.Body)) + var lfsLocksVerify api.LFSLockListVerify + DecodeJSON(t, resp, &lfsLocksVerify) + assert.Len(t, lfsLocksVerify.Ours, 2) + assert.Len(t, lfsLocksVerify.Theirs, 0) + for _, lock := range lfsLocksVerify.Ours { + assert.EqualValues(t, user.DisplayName(), lock.Owner.Name) + } + } diff --git a/models/lfs_lock.go b/models/lfs_lock.go index 7430dc6c6dec7..1e3a1841be7a3 100644 --- a/models/lfs_lock.go +++ b/models/lfs_lock.go @@ -14,7 +14,7 @@ import ( // LFSLock represents a git lfs lfock of repository. type LFSLock struct { ID int64 `xorm:"pk autoincr"` - RepoID int64 `xorm:"INDEX UNIQUE(n)"` + RepoID int64 `xorm:"INDEX"` Repo *Repository `xorm:"-"` OwnerID int64 `xorm:"INDEX"` Owner *User `xorm:"-"` @@ -28,11 +28,13 @@ func (l *LFSLock) BeforeInsert() { if l.CreatedUnix == 0 { l.CreatedUnix = time.Now().Unix() } + l.OwnerID = l.Owner.ID } // AfterLoad is invoked from XORM after setting the values of all fields of this object. func (l *LFSLock) AfterLoad() { l.Created = time.Unix(l.CreatedUnix, 0).Local() + l.Owner, _ = GetUserByID(l.OwnerID) } func (l *LFSLock) loadAttributes(e Engine) error { @@ -76,8 +78,8 @@ func IsLFSLockExist(repoID int64, path string) (bool, error) { } // CreateLFSLock creates a new lock. -func CreateLFSLock(lock *LFSLock, u *User) (*LFSLock, error) { - err := CheckLFSAccessForRepo(u, lock.RepoID, "create") +func CreateLFSLock(lock *LFSLock) (*LFSLock, error) { + err := CheckLFSAccessForRepo(lock.Owner, lock.RepoID, "create") if err != nil { return nil, err } diff --git a/modules/lfs/locks.go b/modules/lfs/locks.go index b26a847432142..cafbb180ebaa5 100644 --- a/modules/lfs/locks.go +++ b/modules/lfs/locks.go @@ -16,11 +16,9 @@ func checkRequest(req macaron.Request) int { if !setting.LFS.StartServer { return 404 } - /* - if !ContentMatcher(req) || req.Header.Get("Content-Type") != contentMediaType { - return 400 - } - */ + if !MetaMatcher(req) || req.Header.Get("Content-Type") != metaMediaType { + return 400 + } return 200 } @@ -129,7 +127,8 @@ func PostLockHandler(ctx *context.Context) { lock, err := models.CreateLFSLock(&models.LFSLock{ RepoID: ctx.Repo.Repository.ID, Path: req.Path, - }, ctx.User) + Owner: ctx.User, + }) if err != nil { if models.IsErrLFSLockAlreadyExist(err) { ctx.JSON(409, api.LFSLockError{ @@ -186,11 +185,11 @@ func VerifyLockHandler(ctx *context.Context) { } lockOursListAPI := make([]*api.LFSLock, 0, len(lockList)) lockTheirsListAPI := make([]*api.LFSLock, 0, len(lockList)) - for i, l := range lockList { + for _, l := range lockList { if l.Owner.ID == ctx.User.ID { - lockOursListAPI[i] = l.APIFormat() + lockOursListAPI = append(lockOursListAPI, l.APIFormat()) } else { - lockTheirsListAPI[i] = l.APIFormat() + lockTheirsListAPI = append(lockTheirsListAPI, l.APIFormat()) } } ctx.JSON(200, api.LFSLockListVerify{ From 61606fc4a2569869bd538ccd5308d958726a343a Mon Sep 17 00:00:00 2001 From: Antoine GIRARD Date: Wed, 22 Nov 2017 01:22:06 +0100 Subject: [PATCH 12/29] Add delete test + fix --- integrations/api_repo_lfs_locks_test.go | 19 +++++++++++++++++-- models/lfs_lock.go | 11 +++++------ modules/lfs/locks.go | 4 ++-- routers/routes/routes.go | 2 +- 4 files changed, 25 insertions(+), 11 deletions(-) diff --git a/integrations/api_repo_lfs_locks_test.go b/integrations/api_repo_lfs_locks_test.go index b8b69431c72b9..1f64de54b9ab6 100644 --- a/integrations/api_repo_lfs_locks_test.go +++ b/integrations/api_repo_lfs_locks_test.go @@ -5,7 +5,6 @@ package integrations import ( - "fmt" "io/ioutil" "net/http" "strings" @@ -97,7 +96,6 @@ func TestAPILFSLocksLogged(t *testing.T) { req.Header.Set("Content-Type", "application/vnd.git-lfs+json") req.Body = ioutil.NopCloser(strings.NewReader("{}")) resp = session.MakeRequest(t, req, http.StatusOK) - fmt.Println(string(resp.Body)) var lfsLocksVerify api.LFSLockListVerify DecodeJSON(t, resp, &lfsLocksVerify) assert.Len(t, lfsLocksVerify.Ours, 2) @@ -106,4 +104,21 @@ func TestAPILFSLocksLogged(t *testing.T) { assert.EqualValues(t, user.DisplayName(), lock.Owner.Name) } + req = NewRequestf(t, "POST", "/%s/%s/info/lfs/locks/%s/unlock", user.Name, repo.Name, lfsLocksVerify.Ours[0].ID) + req.Header.Set("Accept", "application/vnd.git-lfs+json") + req.Header.Set("Content-Type", "application/vnd.git-lfs+json") + req.Body = ioutil.NopCloser(strings.NewReader("{}")) + resp = session.MakeRequest(t, req, http.StatusOK) + var lfsLockRep api.LFSLockResponse + DecodeJSON(t, resp, &lfsLockRep) + assert.Equal(t, lfsLocksVerify.Ours[0].ID, lfsLockRep.Lock.ID) + assert.Equal(t, user.DisplayName(), lfsLockRep.Lock.Owner.Name) + + req = NewRequestf(t, "GET", "/%s/%s/info/lfs/locks", user.Name, repo.Name) + req.Header.Set("Accept", "application/vnd.git-lfs+json") + req.Header.Set("Content-Type", "application/vnd.git-lfs+json") + resp = session.MakeRequest(t, req, http.StatusOK) + DecodeJSON(t, resp, &lfsLocks) + assert.Len(t, lfsLocks.Locks, 1) + } diff --git a/models/lfs_lock.go b/models/lfs_lock.go index 1e3a1841be7a3..42655ece801c4 100644 --- a/models/lfs_lock.go +++ b/models/lfs_lock.go @@ -130,24 +130,23 @@ func GetLFSLockByRepoID(repoID int64) (locks []*LFSLock, err error) { } // DeleteLFSLockByID deletes a lock by given ID. -func DeleteLFSLockByID(id int64, u *User, force bool) error { - +func DeleteLFSLockByID(id int64, u *User, force bool) (*LFSLock, error) { lock, err := GetLFSLockByID(id) if err != nil { - return err + return nil, err } err = CheckLFSAccessForRepo(u, lock.RepoID, "delete") if err != nil { - return err + return nil, err } if !force && u.ID != lock.OwnerID { - return fmt.Errorf("user doesn't own lock and force flag is not set") + return nil, fmt.Errorf("user doesn't own lock and force flag is not set") } _, err = x.ID(id).Delete(new(LFSLock)) - return err + return lock, err } //CheckLFSAccessForRepo check needed access mode base on action diff --git a/modules/lfs/locks.go b/modules/lfs/locks.go index cafbb180ebaa5..5c71445b70723 100644 --- a/modules/lfs/locks.go +++ b/modules/lfs/locks.go @@ -215,7 +215,7 @@ func UnLockHandler(ctx *context.Context) { return } - err = models.DeleteLFSLockByID(ctx.ParamsInt64("id"), ctx.User, req.Force) + lock, err := models.DeleteLFSLockByID(ctx.ParamsInt64("lid"), ctx.User, req.Force) if err != nil { if models.IsErrLFSLockUnauthorizedAction(err) { ctx.JSON(403, api.LFSLockError{ @@ -228,5 +228,5 @@ func UnLockHandler(ctx *context.Context) { }) return } - ctx.JSON(404, api.LFSLockError{Message: "Not found"}) + ctx.JSON(200, api.LFSLockResponse{Lock: lock.APIFormat()}) } diff --git a/routers/routes/routes.go b/routers/routes/routes.go index 4882aab6dd271..1a384961ea89d 100644 --- a/routers/routes/routes.go +++ b/routers/routes/routes.go @@ -690,7 +690,7 @@ func RegisterRoutes(m *macaron.Macaron) { m.Get("/", lfs.GetListLockHandler) m.Post("/", lfs.PostLockHandler) m.Post("/verify", lfs.VerifyLockHandler) - m.Post("/:id/unlock", lfs.UnLockHandler) + m.Post("/:lid/unlock", lfs.UnLockHandler) }, context.RepoAssignment()) m.Any("/*", func(ctx *context.Context) { ctx.Handle(404, "", nil) From 3015595ba88cdd1726bf2543e6056c579a2b40e1 Mon Sep 17 00:00:00 2001 From: Antoine GIRARD Date: Wed, 22 Nov 2017 01:29:25 +0100 Subject: [PATCH 13/29] Add lfs ascii header --- models/error.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/models/error.go b/models/error.go index 97c3a463c092f..72603a7cc3ec3 100644 --- a/models/error.go +++ b/models/error.go @@ -491,9 +491,12 @@ func (err ErrLastOrgOwner) Error() string { return fmt.Sprintf("user is the last member of owner team [uid: %d]", err.UID) } -// -// TODO LFS header -// +//.____ ____________________ +//| | \_ _____/ _____/ +//| | | __) \_____ \ +//| |___| \ / \ +//|_______ \___ / /_______ / +// \/ \/ \/ // ErrLFSLockNotExist represents a "LFSLockNotExist" kind of error. type ErrLFSLockNotExist struct { From 5b3b3015b65e41ee36d44d47b6a387d68a5d800a Mon Sep 17 00:00:00 2001 From: Antoine GIRARD Date: Sat, 25 Nov 2017 00:19:23 +0100 Subject: [PATCH 14/29] Various fixes from review + remove useless code + add more corner case testing --- integrations/api_repo_lfs_locks_test.go | 8 +++ models/lfs_lock.go | 86 ++++++++----------------- modules/lfs/locks.go | 4 ++ 3 files changed, 40 insertions(+), 58 deletions(-) diff --git a/integrations/api_repo_lfs_locks_test.go b/integrations/api_repo_lfs_locks_test.go index 1f64de54b9ab6..c0a3b1b21415a 100644 --- a/integrations/api_repo_lfs_locks_test.go +++ b/integrations/api_repo_lfs_locks_test.go @@ -9,6 +9,7 @@ import ( "net/http" "strings" "testing" + "time" "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/setting" @@ -81,6 +82,12 @@ func TestAPILFSLocksLogged(t *testing.T) { req.Body = ioutil.NopCloser(strings.NewReader("{\"path\": \"path/test\"}")) resp = session.MakeRequest(t, req, http.StatusConflict) + req = NewRequestf(t, "POST", "/%s/%s/info/lfs/locks", user.Name, repo.Name) + req.Header.Set("Accept", "application/vnd.git-lfs+json") + req.Header.Set("Content-Type", "application/vnd.git-lfs+json") + req.Body = ioutil.NopCloser(strings.NewReader("{\"path\": \"Foo/BaR.zip\"}")) + resp = session.MakeRequest(t, req, http.StatusConflict) + req = NewRequestf(t, "GET", "/%s/%s/info/lfs/locks", user.Name, repo.Name) req.Header.Set("Accept", "application/vnd.git-lfs+json") req.Header.Set("Content-Type", "application/vnd.git-lfs+json") @@ -102,6 +109,7 @@ func TestAPILFSLocksLogged(t *testing.T) { assert.Len(t, lfsLocksVerify.Theirs, 0) for _, lock := range lfsLocksVerify.Ours { assert.EqualValues(t, user.DisplayName(), lock.Owner.Name) + assert.WithinDuration(t, time.Now(), lock.LockedAt, 10*time.Second) } req = NewRequestf(t, "POST", "/%s/%s/info/lfs/locks/%s/unlock", user.Name, repo.Name, lfsLocksVerify.Ours[0].ID) diff --git a/models/lfs_lock.go b/models/lfs_lock.go index 42655ece801c4..691d0916cf897 100644 --- a/models/lfs_lock.go +++ b/models/lfs_lock.go @@ -1,67 +1,43 @@ -// Copyright 2014 The Gogs Authors. All rights reserved. +// Copyright 2017 The Gitea Authors. All rights reserved. // Use of this source code is governed by a MIT-style // license that can be found in the LICENSE file. package models import ( - api "code.gitea.io/sdk/gitea" "fmt" "strconv" + "strings" "time" + + api "code.gitea.io/sdk/gitea" ) -// LFSLock represents a git lfs lfock of repository. +// LFSLock represents a git lfs lock of repository. type LFSLock struct { - ID int64 `xorm:"pk autoincr"` - RepoID int64 `xorm:"INDEX"` - Repo *Repository `xorm:"-"` - OwnerID int64 `xorm:"INDEX"` - Owner *User `xorm:"-"` - Path string - Created time.Time `xorm:"-"` - CreatedUnix int64 `xorm:"INDEX"` + ID int64 `xorm:"pk autoincr"` + Repo *Repository `xorm:"-"` + RepoID int64 `xorm:"INDEX"` + Owner *User `xorm:"-"` + OwnerID int64 `xorm:"INDEX"` + Path string + Created time.Time `xorm:"created"` } // BeforeInsert is invoked from XORM before inserting an object of this type. func (l *LFSLock) BeforeInsert() { - if l.CreatedUnix == 0 { - l.CreatedUnix = time.Now().Unix() - } l.OwnerID = l.Owner.ID + l.Path = strings.ToLower(l.Path) } // AfterLoad is invoked from XORM after setting the values of all fields of this object. func (l *LFSLock) AfterLoad() { - l.Created = time.Unix(l.CreatedUnix, 0).Local() l.Owner, _ = GetUserByID(l.OwnerID) -} - -func (l *LFSLock) loadAttributes(e Engine) error { - var err error - if l.Repo == nil { - l.Repo, err = GetRepositoryByID(l.RepoID) - if err != nil { - return err - } - } - if l.Owner == nil { - l.Owner, err = GetUserByID(l.OwnerID) - if err != nil { - return err - } - } - return nil -} - -// LoadAttributes load repo and publisher attributes for a lock -func (l *LFSLock) LoadAttributes() error { - return l.loadAttributes(x) + l.Repo, _ = GetRepositoryByID(l.RepoID) } // APIFormat convert a Release to lfs.LFSLock func (l *LFSLock) APIFormat() *api.LFSLock { - //TODO move to api return &api.LFSLock{ ID: strconv.FormatInt(l.ID, 10), Path: l.Path, @@ -72,43 +48,37 @@ func (l *LFSLock) APIFormat() *api.LFSLock { } } -// IsLFSLockExist returns true if lock with given path already exists. -func IsLFSLockExist(repoID int64, path string) (bool, error) { - return x.Get(&LFSLock{RepoID: repoID, Path: path}) //TODO Define if path should needed to be lower for windows compat ? -} - // CreateLFSLock creates a new lock. func CreateLFSLock(lock *LFSLock) (*LFSLock, error) { err := CheckLFSAccessForRepo(lock.Owner, lock.RepoID, "create") if err != nil { return nil, err } - isExist, err := IsLFSLockExist(lock.RepoID, lock.Path) - if err != nil { - return nil, err - } else if isExist { - l, err := GetLFSLock(lock.RepoID, lock.Path) - if err != nil { - return nil, err - } + + l, err := GetLFSLock(lock.RepoID, lock.Path) + if err == nil { return l, ErrLFSLockAlreadyExist{lock.RepoID, lock.Path} } + if !IsErrLFSLockNotExist(err) { + return nil, err + } + _, err = x.InsertOne(lock) return lock, err } // GetLFSLock returns release by given path. func GetLFSLock(repoID int64, path string) (*LFSLock, error) { - isExist, err := IsLFSLockExist(repoID, path) + path = strings.ToLower(path) + rel := &LFSLock{RepoID: repoID, Path: path} //TODO Define if path should needed to be lower for windows compat ? + has, err := x.Get(rel) if err != nil { return nil, err - } else if !isExist { + } + if !has { return nil, ErrLFSLockNotExist{0, repoID, path} } - - rel := &LFSLock{RepoID: repoID, Path: path} //TODO Define if path should needed to be lower for windows compat ? - _, err = x.Get(rel) - return rel, err + return rel, nil } // GetLFSLockByID returns release by given id. @@ -126,7 +96,7 @@ func GetLFSLockByID(id int64) (*LFSLock, error) { // GetLFSLockByRepoID returns a list of locks of repository. func GetLFSLockByRepoID(repoID int64) (locks []*LFSLock, err error) { err = x.Where("repo_id = ?", repoID).Find(&locks) - return locks, err + return } // DeleteLFSLockByID deletes a lock by given ID. diff --git a/modules/lfs/locks.go b/modules/lfs/locks.go index 5c71445b70723..38d290e13babc 100644 --- a/modules/lfs/locks.go +++ b/modules/lfs/locks.go @@ -1,3 +1,7 @@ +// Copyright 2017 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + package lfs import ( From b37282e8f1e200ad120171b507a1bd11c9375d1b Mon Sep 17 00:00:00 2001 From: Antoine GIRARD Date: Sat, 25 Nov 2017 00:24:54 +0100 Subject: [PATCH 15/29] Remove repo link since only id is needed. Save a little of memory and cpu time. --- models/lfs_lock.go | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/models/lfs_lock.go b/models/lfs_lock.go index 691d0916cf897..dba920383d788 100644 --- a/models/lfs_lock.go +++ b/models/lfs_lock.go @@ -15,11 +15,10 @@ import ( // LFSLock represents a git lfs lock of repository. type LFSLock struct { - ID int64 `xorm:"pk autoincr"` - Repo *Repository `xorm:"-"` - RepoID int64 `xorm:"INDEX"` - Owner *User `xorm:"-"` - OwnerID int64 `xorm:"INDEX"` + ID int64 `xorm:"pk autoincr"` + RepoID int64 `xorm:"INDEX"` + Owner *User `xorm:"-"` + OwnerID int64 `xorm:"INDEX"` Path string Created time.Time `xorm:"created"` } @@ -33,7 +32,6 @@ func (l *LFSLock) BeforeInsert() { // AfterLoad is invoked from XORM after setting the values of all fields of this object. func (l *LFSLock) AfterLoad() { l.Owner, _ = GetUserByID(l.OwnerID) - l.Repo, _ = GetRepositoryByID(l.RepoID) } // APIFormat convert a Release to lfs.LFSLock From 3289ddb1d9f44c352ce907d818fb30cea0870cfb Mon Sep 17 00:00:00 2001 From: Antoine GIRARD Date: Sat, 25 Nov 2017 02:19:14 +0100 Subject: [PATCH 16/29] Improve tests --- integrations/api_repo_lfs_locks_test.go | 186 +++++++++++++++--------- 1 file changed, 116 insertions(+), 70 deletions(-) diff --git a/integrations/api_repo_lfs_locks_test.go b/integrations/api_repo_lfs_locks_test.go index c0a3b1b21415a..8ccb1c672317e 100644 --- a/integrations/api_repo_lfs_locks_test.go +++ b/integrations/api_repo_lfs_locks_test.go @@ -52,81 +52,127 @@ func TestAPILFSLocksNotLogin(t *testing.T) { func TestAPILFSLocksLogged(t *testing.T) { prepareTestEnv(t) setting.LFS.StartServer = true - session := loginUser(t, "user2") - user := models.AssertExistsAndLoadBean(t, &models.User{ID: 2}).(*models.User) - repo := models.AssertExistsAndLoadBean(t, &models.Repository{ID: 1}).(*models.Repository) - - req := NewRequestf(t, "GET", "/%s/%s/info/lfs/locks", user.Name, repo.Name) - req.Header.Set("Accept", "application/vnd.git-lfs+json") - req.Header.Set("Content-Type", "application/vnd.git-lfs+json") - resp := session.MakeRequest(t, req, http.StatusOK) - var lfsLocks api.LFSLockList - DecodeJSON(t, resp, &lfsLocks) - assert.Len(t, lfsLocks.Locks, 0) - - req = NewRequestf(t, "POST", "/%s/%s/info/lfs/locks", user.Name, repo.Name) - req.Header.Set("Accept", "application/vnd.git-lfs+json") - req.Header.Set("Content-Type", "application/vnd.git-lfs+json") - req.Body = ioutil.NopCloser(strings.NewReader("{\"path\": \"foo/bar.zip\"}")) - resp = session.MakeRequest(t, req, http.StatusCreated) - - req = NewRequestf(t, "POST", "/%s/%s/info/lfs/locks", user.Name, repo.Name) - req.Header.Set("Accept", "application/vnd.git-lfs+json") - req.Header.Set("Content-Type", "application/vnd.git-lfs+json") - req.Body = ioutil.NopCloser(strings.NewReader("{\"path\": \"path/test\"}")) - resp = session.MakeRequest(t, req, http.StatusCreated) - - req = NewRequestf(t, "POST", "/%s/%s/info/lfs/locks", user.Name, repo.Name) - req.Header.Set("Accept", "application/vnd.git-lfs+json") - req.Header.Set("Content-Type", "application/vnd.git-lfs+json") - req.Body = ioutil.NopCloser(strings.NewReader("{\"path\": \"path/test\"}")) - resp = session.MakeRequest(t, req, http.StatusConflict) + user2 := models.AssertExistsAndLoadBean(t, &models.User{ID: 2}).(*models.User) //in org 3 + user4 := models.AssertExistsAndLoadBean(t, &models.User{ID: 4}).(*models.User) //in org 3 + + repo1 := models.AssertExistsAndLoadBean(t, &models.Repository{ID: 1}).(*models.Repository) + repo3 := models.AssertExistsAndLoadBean(t, &models.Repository{ID: 3}).(*models.Repository) // own by org 3 + + tests := []struct { + user *models.User + repo *models.Repository + path string + httpResult int + addTime []int + }{ + {user: user2, repo: repo1, path: "foo/bar.zip", httpResult: http.StatusCreated, addTime: []int{0}}, + {user: user2, repo: repo1, path: "path/test", httpResult: http.StatusCreated, addTime: []int{0}}, + {user: user2, repo: repo1, path: "path/test", httpResult: http.StatusConflict}, + {user: user2, repo: repo1, path: "Foo/BaR.zip", httpResult: http.StatusConflict}, + {user: user4, repo: repo1, path: "FoO/BaR.zip", httpResult: http.StatusForbidden}, + {user: user4, repo: repo1, path: "path/test-user4", httpResult: http.StatusForbidden}, + {user: user2, repo: repo1, path: "patH/Test-user4", httpResult: http.StatusCreated, addTime: []int{0}}, + + {user: user2, repo: repo3, path: "test/foo/bar.zip", httpResult: http.StatusCreated, addTime: []int{1, 2}}, + {user: user4, repo: repo3, path: "test/foo/bar.zip", httpResult: http.StatusConflict}, + {user: user4, repo: repo3, path: "test/foo/bar.bin", httpResult: http.StatusCreated, addTime: []int{1, 2}}, + } - req = NewRequestf(t, "POST", "/%s/%s/info/lfs/locks", user.Name, repo.Name) - req.Header.Set("Accept", "application/vnd.git-lfs+json") - req.Header.Set("Content-Type", "application/vnd.git-lfs+json") - req.Body = ioutil.NopCloser(strings.NewReader("{\"path\": \"Foo/BaR.zip\"}")) - resp = session.MakeRequest(t, req, http.StatusConflict) + resultsTests := []struct { + user *models.User + repo *models.Repository + totalCount int + oursCount int + theirsCount int + locksOwners []*models.User + locksTimes []time.Time + }{ + {user: user2, repo: repo1, totalCount: 3, oursCount: 3, theirsCount: 0, locksOwners: []*models.User{user2, user2, user2}, locksTimes: []time.Time{}}, + {user: user2, repo: repo3, totalCount: 2, oursCount: 1, theirsCount: 1, locksOwners: []*models.User{user2, user4}, locksTimes: []time.Time{}}, + {user: user4, repo: repo3, totalCount: 2, oursCount: 1, theirsCount: 1, locksOwners: []*models.User{user2, user4}, locksTimes: []time.Time{}}, + } - req = NewRequestf(t, "GET", "/%s/%s/info/lfs/locks", user.Name, repo.Name) - req.Header.Set("Accept", "application/vnd.git-lfs+json") - req.Header.Set("Content-Type", "application/vnd.git-lfs+json") - resp = session.MakeRequest(t, req, http.StatusOK) - DecodeJSON(t, resp, &lfsLocks) - assert.Len(t, lfsLocks.Locks, 2) - for _, lock := range lfsLocks.Locks { - assert.EqualValues(t, user.DisplayName(), lock.Owner.Name) + deleteTests := []struct { + user *models.User + repo *models.Repository + lockID string + }{} + + //create locks + for _, test := range tests { + session := loginUser(t, test.user.Name) + req := NewRequestf(t, "POST", "/%s/info/lfs/locks", test.repo.FullName()) + req.Header.Set("Accept", "application/vnd.git-lfs+json") + req.Header.Set("Content-Type", "application/vnd.git-lfs+json") + req.Body = ioutil.NopCloser(strings.NewReader("{\"path\": \"" + test.path + "\"}")) + session.MakeRequest(t, req, test.httpResult) + if len(test.addTime) > 0 { + for _, id := range test.addTime { + resultsTests[id].locksTimes = append(resultsTests[id].locksTimes, time.Now()) + } + } } - req = NewRequestf(t, "POST", "/%s/%s/info/lfs/locks/verify", user.Name, repo.Name) - req.Header.Set("Accept", "application/vnd.git-lfs+json") - req.Header.Set("Content-Type", "application/vnd.git-lfs+json") - req.Body = ioutil.NopCloser(strings.NewReader("{}")) - resp = session.MakeRequest(t, req, http.StatusOK) - var lfsLocksVerify api.LFSLockListVerify - DecodeJSON(t, resp, &lfsLocksVerify) - assert.Len(t, lfsLocksVerify.Ours, 2) - assert.Len(t, lfsLocksVerify.Theirs, 0) - for _, lock := range lfsLocksVerify.Ours { - assert.EqualValues(t, user.DisplayName(), lock.Owner.Name) - assert.WithinDuration(t, time.Now(), lock.LockedAt, 10*time.Second) + //check creation + for _, test := range resultsTests { + session := loginUser(t, test.user.Name) + req := NewRequestf(t, "GET", "/%s/info/lfs/locks", test.repo.FullName()) + req.Header.Set("Accept", "application/vnd.git-lfs+json") + req.Header.Set("Content-Type", "application/vnd.git-lfs+json") + resp := session.MakeRequest(t, req, http.StatusOK) + var lfsLocks api.LFSLockList + DecodeJSON(t, resp, &lfsLocks) + assert.Len(t, lfsLocks.Locks, test.totalCount) + for i, lock := range lfsLocks.Locks { + assert.EqualValues(t, test.locksOwners[i].DisplayName(), lock.Owner.Name) + assert.WithinDuration(t, test.locksTimes[i], lock.LockedAt, 1*time.Second) + } + + req = NewRequestf(t, "POST", "/%s/info/lfs/locks/verify", test.repo.FullName()) + req.Header.Set("Accept", "application/vnd.git-lfs+json") + req.Header.Set("Content-Type", "application/vnd.git-lfs+json") + req.Body = ioutil.NopCloser(strings.NewReader("{}")) + resp = session.MakeRequest(t, req, http.StatusOK) + var lfsLocksVerify api.LFSLockListVerify + DecodeJSON(t, resp, &lfsLocksVerify) + assert.Len(t, lfsLocksVerify.Ours, test.oursCount) + assert.Len(t, lfsLocksVerify.Theirs, test.theirsCount) + for _, lock := range lfsLocksVerify.Ours { + assert.EqualValues(t, test.user.DisplayName(), lock.Owner.Name) + deleteTests = append(deleteTests, struct { + user *models.User + repo *models.Repository + lockID string + }{test.user, test.repo, lock.ID}) + } + for _, lock := range lfsLocksVerify.Theirs { + assert.NotEqual(t, test.user.DisplayName(), lock.Owner.Name) + } } - req = NewRequestf(t, "POST", "/%s/%s/info/lfs/locks/%s/unlock", user.Name, repo.Name, lfsLocksVerify.Ours[0].ID) - req.Header.Set("Accept", "application/vnd.git-lfs+json") - req.Header.Set("Content-Type", "application/vnd.git-lfs+json") - req.Body = ioutil.NopCloser(strings.NewReader("{}")) - resp = session.MakeRequest(t, req, http.StatusOK) - var lfsLockRep api.LFSLockResponse - DecodeJSON(t, resp, &lfsLockRep) - assert.Equal(t, lfsLocksVerify.Ours[0].ID, lfsLockRep.Lock.ID) - assert.Equal(t, user.DisplayName(), lfsLockRep.Lock.Owner.Name) - - req = NewRequestf(t, "GET", "/%s/%s/info/lfs/locks", user.Name, repo.Name) - req.Header.Set("Accept", "application/vnd.git-lfs+json") - req.Header.Set("Content-Type", "application/vnd.git-lfs+json") - resp = session.MakeRequest(t, req, http.StatusOK) - DecodeJSON(t, resp, &lfsLocks) - assert.Len(t, lfsLocks.Locks, 1) + //remove all locks + for _, test := range deleteTests { + session := loginUser(t, test.user.Name) + req := NewRequestf(t, "POST", "/%s/info/lfs/locks/%s/unlock", test.repo.FullName(), test.lockID) + req.Header.Set("Accept", "application/vnd.git-lfs+json") + req.Header.Set("Content-Type", "application/vnd.git-lfs+json") + req.Body = ioutil.NopCloser(strings.NewReader("{}")) + resp := session.MakeRequest(t, req, http.StatusOK) + var lfsLockRep api.LFSLockResponse + DecodeJSON(t, resp, &lfsLockRep) + assert.Equal(t, test.lockID, lfsLockRep.Lock.ID) + assert.Equal(t, test.user.DisplayName(), lfsLockRep.Lock.Owner.Name) + } + // check taht we don't have any lock + for _, test := range resultsTests { + session := loginUser(t, test.user.Name) + req := NewRequestf(t, "GET", "/%s/info/lfs/locks", test.repo.FullName()) + req.Header.Set("Accept", "application/vnd.git-lfs+json") + req.Header.Set("Content-Type", "application/vnd.git-lfs+json") + resp := session.MakeRequest(t, req, http.StatusOK) + var lfsLocks api.LFSLockList + DecodeJSON(t, resp, &lfsLocks) + assert.Len(t, lfsLocks.Locks, 0) + } } From d529d02d05a5d30ee63095889c5bd490d7c47935 Mon Sep 17 00:00:00 2001 From: Antoine GIRARD Date: Sat, 25 Nov 2017 15:08:01 +0100 Subject: [PATCH 17/29] Use TEXT column format for path + test --- integrations/api_repo_lfs_locks_test.go | 3 ++- models/lfs_lock.go | 10 +++++----- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/integrations/api_repo_lfs_locks_test.go b/integrations/api_repo_lfs_locks_test.go index 8ccb1c672317e..8df3b8460a9d8 100644 --- a/integrations/api_repo_lfs_locks_test.go +++ b/integrations/api_repo_lfs_locks_test.go @@ -72,6 +72,7 @@ func TestAPILFSLocksLogged(t *testing.T) { {user: user4, repo: repo1, path: "FoO/BaR.zip", httpResult: http.StatusForbidden}, {user: user4, repo: repo1, path: "path/test-user4", httpResult: http.StatusForbidden}, {user: user2, repo: repo1, path: "patH/Test-user4", httpResult: http.StatusCreated, addTime: []int{0}}, + {user: user2, repo: repo1, path: "some/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/path", httpResult: http.StatusCreated, addTime: []int{0}}, {user: user2, repo: repo3, path: "test/foo/bar.zip", httpResult: http.StatusCreated, addTime: []int{1, 2}}, {user: user4, repo: repo3, path: "test/foo/bar.zip", httpResult: http.StatusConflict}, @@ -87,7 +88,7 @@ func TestAPILFSLocksLogged(t *testing.T) { locksOwners []*models.User locksTimes []time.Time }{ - {user: user2, repo: repo1, totalCount: 3, oursCount: 3, theirsCount: 0, locksOwners: []*models.User{user2, user2, user2}, locksTimes: []time.Time{}}, + {user: user2, repo: repo1, totalCount: 4, oursCount: 4, theirsCount: 0, locksOwners: []*models.User{user2, user2, user2, user2}, locksTimes: []time.Time{}}, {user: user2, repo: repo3, totalCount: 2, oursCount: 1, theirsCount: 1, locksOwners: []*models.User{user2, user4}, locksTimes: []time.Time{}}, {user: user4, repo: repo3, totalCount: 2, oursCount: 1, theirsCount: 1, locksOwners: []*models.User{user2, user4}, locksTimes: []time.Time{}}, } diff --git a/models/lfs_lock.go b/models/lfs_lock.go index dba920383d788..8baee2d4300b0 100644 --- a/models/lfs_lock.go +++ b/models/lfs_lock.go @@ -15,11 +15,11 @@ import ( // LFSLock represents a git lfs lock of repository. type LFSLock struct { - ID int64 `xorm:"pk autoincr"` - RepoID int64 `xorm:"INDEX"` - Owner *User `xorm:"-"` - OwnerID int64 `xorm:"INDEX"` - Path string + ID int64 `xorm:"pk autoincr"` + RepoID int64 `xorm:"INDEX"` + Owner *User `xorm:"-"` + OwnerID int64 `xorm:"INDEX"` + Path string `xorm:"TEXT"` Created time.Time `xorm:"created"` } From 51befcfcac757d099b42f6def3b482d88c33bbe6 Mon Sep 17 00:00:00 2001 From: Antoine GIRARD Date: Sat, 25 Nov 2017 15:10:31 +0100 Subject: [PATCH 18/29] fix mispell --- integrations/api_repo_lfs_locks_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integrations/api_repo_lfs_locks_test.go b/integrations/api_repo_lfs_locks_test.go index 8df3b8460a9d8..bbcb1521fe008 100644 --- a/integrations/api_repo_lfs_locks_test.go +++ b/integrations/api_repo_lfs_locks_test.go @@ -165,7 +165,7 @@ func TestAPILFSLocksLogged(t *testing.T) { assert.Equal(t, test.user.DisplayName(), lfsLockRep.Lock.Owner.Name) } - // check taht we don't have any lock + // check that we don't have any lock for _, test := range resultsTests { session := loginUser(t, test.user.Name) req := NewRequestf(t, "GET", "/%s/info/lfs/locks", test.repo.FullName()) From ed57927c4ded1e3345991daf0741276e2c7b2338 Mon Sep 17 00:00:00 2001 From: Antoine GIRARD Date: Sun, 26 Nov 2017 00:37:54 +0100 Subject: [PATCH 19/29] Use NewRequestWithJSON for POST tests --- integrations/api_repo_lfs_locks_test.go | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/integrations/api_repo_lfs_locks_test.go b/integrations/api_repo_lfs_locks_test.go index bbcb1521fe008..4c3e0f258ce9a 100644 --- a/integrations/api_repo_lfs_locks_test.go +++ b/integrations/api_repo_lfs_locks_test.go @@ -5,9 +5,8 @@ package integrations import ( - "io/ioutil" + "fmt" "net/http" - "strings" "testing" "time" @@ -102,10 +101,9 @@ func TestAPILFSLocksLogged(t *testing.T) { //create locks for _, test := range tests { session := loginUser(t, test.user.Name) - req := NewRequestf(t, "POST", "/%s/info/lfs/locks", test.repo.FullName()) + req := NewRequestWithJSON(t, "POST", fmt.Sprintf("/%s/info/lfs/locks", test.repo.FullName()), map[string]string{"path": test.path}) req.Header.Set("Accept", "application/vnd.git-lfs+json") req.Header.Set("Content-Type", "application/vnd.git-lfs+json") - req.Body = ioutil.NopCloser(strings.NewReader("{\"path\": \"" + test.path + "\"}")) session.MakeRequest(t, req, test.httpResult) if len(test.addTime) > 0 { for _, id := range test.addTime { @@ -129,10 +127,9 @@ func TestAPILFSLocksLogged(t *testing.T) { assert.WithinDuration(t, test.locksTimes[i], lock.LockedAt, 1*time.Second) } - req = NewRequestf(t, "POST", "/%s/info/lfs/locks/verify", test.repo.FullName()) + req = NewRequestWithJSON(t, "POST", fmt.Sprintf("/%s/info/lfs/locks/verify", test.repo.FullName()), map[string]string{}) req.Header.Set("Accept", "application/vnd.git-lfs+json") req.Header.Set("Content-Type", "application/vnd.git-lfs+json") - req.Body = ioutil.NopCloser(strings.NewReader("{}")) resp = session.MakeRequest(t, req, http.StatusOK) var lfsLocksVerify api.LFSLockListVerify DecodeJSON(t, resp, &lfsLocksVerify) @@ -154,10 +151,9 @@ func TestAPILFSLocksLogged(t *testing.T) { //remove all locks for _, test := range deleteTests { session := loginUser(t, test.user.Name) - req := NewRequestf(t, "POST", "/%s/info/lfs/locks/%s/unlock", test.repo.FullName(), test.lockID) + req := NewRequestWithJSON(t, "POST", fmt.Sprintf("/%s/info/lfs/locks/%s/unlock", test.repo.FullName(), test.lockID), map[string]string{}) req.Header.Set("Accept", "application/vnd.git-lfs+json") req.Header.Set("Content-Type", "application/vnd.git-lfs+json") - req.Body = ioutil.NopCloser(strings.NewReader("{}")) resp := session.MakeRequest(t, req, http.StatusOK) var lfsLockRep api.LFSLockResponse DecodeJSON(t, resp, &lfsLockRep) From bd3e9e7a5e214b3790cf75af7792e90b17fdd349 Mon Sep 17 00:00:00 2001 From: Antoine GIRARD Date: Sun, 26 Nov 2017 18:45:00 +0100 Subject: [PATCH 20/29] Clean path --- integrations/api_repo_lfs_locks_test.go | 1 + models/lfs_lock.go | 11 ++++++++--- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/integrations/api_repo_lfs_locks_test.go b/integrations/api_repo_lfs_locks_test.go index 4c3e0f258ce9a..9fd796b6c5db4 100644 --- a/integrations/api_repo_lfs_locks_test.go +++ b/integrations/api_repo_lfs_locks_test.go @@ -68,6 +68,7 @@ func TestAPILFSLocksLogged(t *testing.T) { {user: user2, repo: repo1, path: "path/test", httpResult: http.StatusCreated, addTime: []int{0}}, {user: user2, repo: repo1, path: "path/test", httpResult: http.StatusConflict}, {user: user2, repo: repo1, path: "Foo/BaR.zip", httpResult: http.StatusConflict}, + {user: user2, repo: repo1, path: "Foo/Test/../subFOlder/../Relative/../BaR.zip", httpResult: http.StatusConflict}, {user: user4, repo: repo1, path: "FoO/BaR.zip", httpResult: http.StatusForbidden}, {user: user4, repo: repo1, path: "path/test-user4", httpResult: http.StatusForbidden}, {user: user2, repo: repo1, path: "patH/Test-user4", httpResult: http.StatusCreated, addTime: []int{0}}, diff --git a/models/lfs_lock.go b/models/lfs_lock.go index 8baee2d4300b0..eaa431ade1492 100644 --- a/models/lfs_lock.go +++ b/models/lfs_lock.go @@ -6,6 +6,7 @@ package models import ( "fmt" + "path" "strconv" "strings" "time" @@ -26,7 +27,7 @@ type LFSLock struct { // BeforeInsert is invoked from XORM before inserting an object of this type. func (l *LFSLock) BeforeInsert() { l.OwnerID = l.Owner.ID - l.Path = strings.ToLower(l.Path) + l.Path = cleanPath(l.Path) } // AfterLoad is invoked from XORM after setting the values of all fields of this object. @@ -34,6 +35,10 @@ func (l *LFSLock) AfterLoad() { l.Owner, _ = GetUserByID(l.OwnerID) } +func cleanPath(p string) string { + return strings.ToLower(path.Clean(p)) +} + // APIFormat convert a Release to lfs.LFSLock func (l *LFSLock) APIFormat() *api.LFSLock { return &api.LFSLock{ @@ -67,8 +72,8 @@ func CreateLFSLock(lock *LFSLock) (*LFSLock, error) { // GetLFSLock returns release by given path. func GetLFSLock(repoID int64, path string) (*LFSLock, error) { - path = strings.ToLower(path) - rel := &LFSLock{RepoID: repoID, Path: path} //TODO Define if path should needed to be lower for windows compat ? + path = cleanPath(path) + rel := &LFSLock{RepoID: repoID, Path: path} has, err := x.Get(rel) if err != nil { return nil, err From caf92f94eb0b2224760bc0804f5c56bf35152b8d Mon Sep 17 00:00:00 2001 From: Antoine GIRARD Date: Sun, 26 Nov 2017 18:49:08 +0100 Subject: [PATCH 21/29] Improve DB format --- models/lfs_lock.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/models/lfs_lock.go b/models/lfs_lock.go index eaa431ade1492..fc13c5f7d3821 100644 --- a/models/lfs_lock.go +++ b/models/lfs_lock.go @@ -17,10 +17,10 @@ import ( // LFSLock represents a git lfs lock of repository. type LFSLock struct { ID int64 `xorm:"pk autoincr"` - RepoID int64 `xorm:"INDEX"` + RepoID int64 `xorm:"UNIQUE(path_by_repo) INDEX NOT NULL"` Owner *User `xorm:"-"` - OwnerID int64 `xorm:"INDEX"` - Path string `xorm:"TEXT"` + OwnerID int64 `xorm:"INDEX NOT NULL"` + Path string `xorm:"TEXT UNIQUE(path_by_repo)"` Created time.Time `xorm:"created"` } From 2988c1037a940422d03f88bdf7e4c89c71784d55 Mon Sep 17 00:00:00 2001 From: Antoine GIRARD Date: Mon, 27 Nov 2017 00:30:56 +0100 Subject: [PATCH 22/29] Revert uniquess repoid+path --- models/lfs_lock.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/models/lfs_lock.go b/models/lfs_lock.go index fc13c5f7d3821..83811bc7bd647 100644 --- a/models/lfs_lock.go +++ b/models/lfs_lock.go @@ -17,10 +17,10 @@ import ( // LFSLock represents a git lfs lock of repository. type LFSLock struct { ID int64 `xorm:"pk autoincr"` - RepoID int64 `xorm:"UNIQUE(path_by_repo) INDEX NOT NULL"` + RepoID int64 `xorm:"INDEX NOT NULL"` Owner *User `xorm:"-"` OwnerID int64 `xorm:"INDEX NOT NULL"` - Path string `xorm:"TEXT UNIQUE(path_by_repo)"` + Path string `xorm:"TEXT"` Created time.Time `xorm:"created"` } From b6acea82479d1138c95022770a5be80c4b11eeec Mon Sep 17 00:00:00 2001 From: Antoine GIRARD Date: Mon, 27 Nov 2017 03:29:48 +0100 Subject: [PATCH 23/29] (Re)-setup uniqueness + max path length --- models/lfs_lock.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/models/lfs_lock.go b/models/lfs_lock.go index 83811bc7bd647..a9ffbd3485b37 100644 --- a/models/lfs_lock.go +++ b/models/lfs_lock.go @@ -17,10 +17,10 @@ import ( // LFSLock represents a git lfs lock of repository. type LFSLock struct { ID int64 `xorm:"pk autoincr"` - RepoID int64 `xorm:"INDEX NOT NULL"` + RepoID int64 `xorm:"UNIQUE(path_by_repo) INDEX NOT NULL"` Owner *User `xorm:"-"` OwnerID int64 `xorm:"INDEX NOT NULL"` - Path string `xorm:"TEXT"` + Path string `xorm:"VARCHAR(4096) UNIQUE(path_by_repo)"` Created time.Time `xorm:"created"` } From 40b3cbb93a73b214d653c3a42018202b0a6ad200 Mon Sep 17 00:00:00 2001 From: Antoine GIRARD Date: Mon, 27 Nov 2017 03:49:59 +0100 Subject: [PATCH 24/29] Fixed TEXT in place of VARCHAR --- models/lfs_lock.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/lfs_lock.go b/models/lfs_lock.go index a9ffbd3485b37..cec7d4859e486 100644 --- a/models/lfs_lock.go +++ b/models/lfs_lock.go @@ -20,7 +20,7 @@ type LFSLock struct { RepoID int64 `xorm:"UNIQUE(path_by_repo) INDEX NOT NULL"` Owner *User `xorm:"-"` OwnerID int64 `xorm:"INDEX NOT NULL"` - Path string `xorm:"VARCHAR(4096) UNIQUE(path_by_repo)"` + Path string `xorm:"TEXT(4096) UNIQUE(path_by_repo)"` Created time.Time `xorm:"created"` } From ca344b5e3f418702d3888cb14e5d415cedfef87f Mon Sep 17 00:00:00 2001 From: Antoine GIRARD Date: Tue, 28 Nov 2017 00:24:12 +0100 Subject: [PATCH 25/29] Settle back to maximum VARCHAR(3072) --- models/lfs_lock.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/lfs_lock.go b/models/lfs_lock.go index cec7d4859e486..c297ea8e8d502 100644 --- a/models/lfs_lock.go +++ b/models/lfs_lock.go @@ -20,7 +20,7 @@ type LFSLock struct { RepoID int64 `xorm:"UNIQUE(path_by_repo) INDEX NOT NULL"` Owner *User `xorm:"-"` OwnerID int64 `xorm:"INDEX NOT NULL"` - Path string `xorm:"TEXT(4096) UNIQUE(path_by_repo)"` + Path string `xorm:"VARCHAR(3072) UNIQUE(path_by_repo)"` Created time.Time `xorm:"created"` } From ab0a3eb739f67aacfe083690ab82ee6d1ec226c1 Mon Sep 17 00:00:00 2001 From: Antoine GIRARD Date: Tue, 28 Nov 2017 00:32:04 +0100 Subject: [PATCH 26/29] Let place for repoid in key --- models/lfs_lock.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/lfs_lock.go b/models/lfs_lock.go index c297ea8e8d502..1c9b4100602a2 100644 --- a/models/lfs_lock.go +++ b/models/lfs_lock.go @@ -20,7 +20,7 @@ type LFSLock struct { RepoID int64 `xorm:"UNIQUE(path_by_repo) INDEX NOT NULL"` Owner *User `xorm:"-"` OwnerID int64 `xorm:"INDEX NOT NULL"` - Path string `xorm:"VARCHAR(3072) UNIQUE(path_by_repo)"` + Path string `xorm:"VARCHAR(2048) UNIQUE(path_by_repo)"` Created time.Time `xorm:"created"` } From d13c73e3cf0c77a8b1766d2877f852a393b61189 Mon Sep 17 00:00:00 2001 From: Antoine GIRARD Date: Tue, 28 Nov 2017 00:40:41 +0100 Subject: [PATCH 27/29] Let place for repoid in key --- models/lfs_lock.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/lfs_lock.go b/models/lfs_lock.go index 1c9b4100602a2..0dd4791474621 100644 --- a/models/lfs_lock.go +++ b/models/lfs_lock.go @@ -20,7 +20,7 @@ type LFSLock struct { RepoID int64 `xorm:"UNIQUE(path_by_repo) INDEX NOT NULL"` Owner *User `xorm:"-"` OwnerID int64 `xorm:"INDEX NOT NULL"` - Path string `xorm:"VARCHAR(2048) UNIQUE(path_by_repo)"` + Path string `xorm:"VARCHAR(1024) UNIQUE(path_by_repo)"` Created time.Time `xorm:"created"` } From 59565f4e75e5a79a596dab2d06361217d5a1e987 Mon Sep 17 00:00:00 2001 From: Antoine GIRARD Date: Tue, 28 Nov 2017 00:44:40 +0100 Subject: [PATCH 28/29] Let place for repoid in key --- models/lfs_lock.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/lfs_lock.go b/models/lfs_lock.go index 0dd4791474621..e1eaf20c4b774 100644 --- a/models/lfs_lock.go +++ b/models/lfs_lock.go @@ -20,7 +20,7 @@ type LFSLock struct { RepoID int64 `xorm:"UNIQUE(path_by_repo) INDEX NOT NULL"` Owner *User `xorm:"-"` OwnerID int64 `xorm:"INDEX NOT NULL"` - Path string `xorm:"VARCHAR(1024) UNIQUE(path_by_repo)"` + Path string `xorm:"VARCHAR(512) UNIQUE(path_by_repo)"` Created time.Time `xorm:"created"` } From 0180efa55329b90b6f0a9022b00d05f4a28ab4a3 Mon Sep 17 00:00:00 2001 From: Antoine GIRARD Date: Tue, 28 Nov 2017 00:49:17 +0100 Subject: [PATCH 29/29] Revert back --- models/lfs_lock.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/models/lfs_lock.go b/models/lfs_lock.go index e1eaf20c4b774..83811bc7bd647 100644 --- a/models/lfs_lock.go +++ b/models/lfs_lock.go @@ -17,10 +17,10 @@ import ( // LFSLock represents a git lfs lock of repository. type LFSLock struct { ID int64 `xorm:"pk autoincr"` - RepoID int64 `xorm:"UNIQUE(path_by_repo) INDEX NOT NULL"` + RepoID int64 `xorm:"INDEX NOT NULL"` Owner *User `xorm:"-"` OwnerID int64 `xorm:"INDEX NOT NULL"` - Path string `xorm:"VARCHAR(512) UNIQUE(path_by_repo)"` + Path string `xorm:"TEXT"` Created time.Time `xorm:"created"` }