Fix Git LFS object/repo link storage in database and small refactoring (#2803)

diff --git a/models/lfs.go b/models/lfs.go
index 842e48e409b4c2eac5cf5df78f523a0a7e811b51..d6cdc688927da09cca8769450ce04a79d0193535 100644 (file)
--- 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 44ab082867d531d1dfb3a26cdec8246577974f4b..1a15aba0014b6e8559d5339a6d2975b6d854bbff 100644 (file)
--- 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
        }