From 82fc50f4ee6c078d669a8d51310c48e450a3886d Mon Sep 17 00:00:00 2001 From: Lauris BH Date: Mon, 30 Oct 2017 14:11:56 +0200 Subject: [PATCH] Fix Git LFS object/repo link storage in database and small refactoring (#2803) --- models/lfs.go | 9 ++--- modules/lfs/server.go | 88 +++++++++++++++++-------------------------- 2 files changed, 38 insertions(+), 59 deletions(-) diff --git a/models/lfs.go b/models/lfs.go index 842e48e40..d6cdc6889 100644 --- a/models/lfs.go +++ b/models/lfs.go @@ -70,12 +70,12 @@ func NewLFSMetaObject(m *LFSMetaObject) (*LFSMetaObject, error) { // GetLFSMetaObjectByOid selects a LFSMetaObject entry from database by its OID. // It may return ErrLFSObjectNotExist or a database error. If the error is nil, // the returned pointer is a valid LFSMetaObject. -func GetLFSMetaObjectByOid(oid string) (*LFSMetaObject, error) { +func (repo *Repository) GetLFSMetaObjectByOid(oid string) (*LFSMetaObject, error) { if len(oid) == 0 { return nil, ErrLFSObjectNotExist } - m := &LFSMetaObject{Oid: oid} + m := &LFSMetaObject{Oid: oid, RepositoryID: repo.ID} has, err := x.Get(m) if err != nil { return nil, err @@ -87,7 +87,7 @@ func GetLFSMetaObjectByOid(oid string) (*LFSMetaObject, error) { // RemoveLFSMetaObjectByOid removes a LFSMetaObject entry from database by its OID. // It may return ErrLFSObjectNotExist or a database error. -func RemoveLFSMetaObjectByOid(oid string) error { +func (repo *Repository) RemoveLFSMetaObjectByOid(oid string) error { if len(oid) == 0 { return ErrLFSObjectNotExist } @@ -98,8 +98,7 @@ func RemoveLFSMetaObjectByOid(oid string) error { return err } - m := &LFSMetaObject{Oid: oid} - + m := &LFSMetaObject{Oid: oid, RepositoryID: repo.ID} if _, err := sess.Delete(m); err != nil { return err } diff --git a/modules/lfs/server.go b/modules/lfs/server.go index 44ab08286..1a15aba00 100644 --- a/modules/lfs/server.go +++ b/modules/lfs/server.go @@ -86,11 +86,11 @@ func ObjectOidHandler(ctx *context.Context) { if ctx.Req.Method == "GET" || ctx.Req.Method == "HEAD" { if MetaMatcher(ctx.Req) { - GetMetaHandler(ctx) + getMetaHandler(ctx) return } if ContentMatcher(ctx.Req) || len(ctx.Params("filename")) > 0 { - GetContentHandler(ctx) + getContentHandler(ctx) return } } else if ctx.Req.Method == "PUT" && ContentMatcher(ctx.Req) { @@ -100,26 +100,35 @@ func ObjectOidHandler(ctx *context.Context) { } -// GetContentHandler gets the content from the content store -func GetContentHandler(ctx *context.Context) { - - rv := unpack(ctx) - - meta, err := models.GetLFSMetaObjectByOid(rv.Oid) +func getAuthenticatedRepoAndMeta(ctx *context.Context, rv *RequestVars, requireWrite bool) (*models.LFSMetaObject, *models.Repository) { + repositoryString := rv.User + "/" + rv.Repo + repository, err := models.GetRepositoryByRef(repositoryString) if err != nil { + log.Debug("Could not find repository: %s - %s", repositoryString, err) writeStatus(ctx, 404) - return + return nil, nil } - repository, err := models.GetRepositoryByID(meta.RepositoryID) + if !authenticate(ctx, repository, rv.Authorization, requireWrite) { + requireAuth(ctx) + return nil, nil + } + meta, err := repository.GetLFSMetaObjectByOid(rv.Oid) if err != nil { writeStatus(ctx, 404) - return + return nil, nil } - if !authenticate(ctx, repository, rv.Authorization, false) { - requireAuth(ctx) + return meta, repository +} + +// getContentHandler gets the content from the content store +func getContentHandler(ctx *context.Context) { + rv := unpack(ctx) + + meta, _ := getAuthenticatedRepoAndMeta(ctx, rv, false) + if meta == nil { return } @@ -160,26 +169,12 @@ func GetContentHandler(ctx *context.Context) { logRequest(ctx.Req, statusCode) } -// GetMetaHandler retrieves metadata about the object -func GetMetaHandler(ctx *context.Context) { - +// getMetaHandler retrieves metadata about the object +func getMetaHandler(ctx *context.Context) { rv := unpack(ctx) - meta, err := models.GetLFSMetaObjectByOid(rv.Oid) - if err != nil { - writeStatus(ctx, 404) - return - } - - repository, err := models.GetRepositoryByID(meta.RepositoryID) - - if err != nil { - writeStatus(ctx, 404) - return - } - - if !authenticate(ctx, repository, rv.Authorization, false) { - requireAuth(ctx) + meta, _ := getAuthenticatedRepoAndMeta(ctx, rv, false) + if meta == nil { return } @@ -210,7 +205,6 @@ func PostHandler(ctx *context.Context) { repositoryString := rv.User + "/" + rv.Repo repository, err := models.GetRepositoryByRef(repositoryString) - if err != nil { log.Debug("Could not find repository: %s - %s", repositoryString, err) writeStatus(ctx, 404) @@ -222,7 +216,6 @@ func PostHandler(ctx *context.Context) { } meta, err := models.NewLFSMetaObject(&models.LFSMetaObject{Oid: rv.Oid, Size: rv.Size, RepositoryID: repository.ID}) - if err != nil { writeStatus(ctx, 404) return @@ -281,9 +274,9 @@ func BatchHandler(ctx *context.Context) { return } - meta, err := models.GetLFSMetaObjectByOid(object.Oid) - contentStore := &ContentStore{BasePath: setting.LFS.ContentPath} + + meta, err := repository.GetLFSMetaObjectByOid(object.Oid) if err == nil && contentStore.Exists(meta) { // Object is found and exists responseObjects = append(responseObjects, Represent(object, meta, true, false)) continue @@ -291,9 +284,8 @@ func BatchHandler(ctx *context.Context) { // Object is not found meta, err = models.NewLFSMetaObject(&models.LFSMetaObject{Oid: object.Oid, Size: object.Size, RepositoryID: repository.ID}) - if err == nil { - responseObjects = append(responseObjects, Represent(object, meta, meta.Existing, true)) + responseObjects = append(responseObjects, Represent(object, meta, meta.Existing, !contentStore.Exists(meta))) } } @@ -310,30 +302,18 @@ func BatchHandler(ctx *context.Context) { func PutHandler(ctx *context.Context) { rv := unpack(ctx) - meta, err := models.GetLFSMetaObjectByOid(rv.Oid) - - if err != nil { - writeStatus(ctx, 404) - return - } - - repository, err := models.GetRepositoryByID(meta.RepositoryID) - - if err != nil { - writeStatus(ctx, 404) - return - } - - if !authenticate(ctx, repository, rv.Authorization, true) { - requireAuth(ctx) + meta, repository := getAuthenticatedRepoAndMeta(ctx, rv, true) + if meta == nil { return } contentStore := &ContentStore{BasePath: setting.LFS.ContentPath} if err := contentStore.Put(meta, ctx.Req.Body().ReadCloser()); err != nil { - models.RemoveLFSMetaObjectByOid(rv.Oid) ctx.Resp.WriteHeader(500) fmt.Fprintf(ctx.Resp, `{"message":"%s"}`, err) + if err = repository.RemoveLFSMetaObjectByOid(rv.Oid); err != nil { + log.Error(4, "RemoveLFSMetaObjectByOid: %v", err) + } return }