Skip to content

Fixes #7292 - API File Contents bug #7301

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 24 commits into from
Jun 29, 2019
Merged

Fixes #7292 - API File Contents bug #7301

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
3dfafaf
Fixes #7292 - API File Contents bug
richmahn Jun 26, 2019
c66f026
Fix to linting
richmahn Jun 26, 2019
74564a2
Fixes lint issues
richmahn Jun 26, 2019
989f4fb
Fixes integreation test
richmahn Jun 26, 2019
f676db5
API contents endpoint full functionality for directories, symlinks an…
richmahn Jun 27, 2019
35dd33d
API contents endpoint full functionality for directories, symlinks an…
richmahn Jun 27, 2019
ac16348
Lint fixes
richmahn Jun 27, 2019
6dc209b
Fixes to content and swagger
richmahn Jun 27, 2019
459efd4
Fixes for swagger
richmahn Jun 27, 2019
c941709
Fixes to content and swagger
richmahn Jun 27, 2019
ae0b30b
Swagger update
richmahn Jun 27, 2019
4078622
Swagger fix
richmahn Jun 27, 2019
b436486
Fix to test
richmahn Jun 28, 2019
5931946
Merge remote-tracking branch 'upstream/master' into fix-7292-api-file…
richmahn Jun 28, 2019
e0d98b0
ContentsResponse struct update: better name and some fields can be nil
richmahn Jun 28, 2019
cc4212b
ContentsResponse struct update: better name and some fields can be nil
richmahn Jun 28, 2019
ff2b1c4
Update comments
richmahn Jun 28, 2019
c3e496d
Fixes to integration tests
richmahn Jun 28, 2019
4c8b11d
Swagger update
richmahn Jun 28, 2019
19ac334
Merge remote-tracking branch 'upstream/master' into fix-7292-api-file…
richmahn Jun 28, 2019
0d65643
Fixes to integration tests
richmahn Jun 28, 2019
3d2c6ad
Adds comment
richmahn Jun 28, 2019
dd963a1
Adds comment
richmahn Jun 28, 2019
23498be
Merge remote-tracking branch 'upstream/master' into fix-7292-api-file…
richmahn Jun 29, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 59 additions & 24 deletions integrations/api_repo_file_content_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,29 +12,32 @@ import (

"code.gitea.io/gitea/models"
"code.gitea.io/gitea/modules/context"
"code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/setting"
api "code.gitea.io/gitea/modules/structs"

"github.com/stretchr/testify/assert"
)

func getExpectedFileContentResponseForFileContents(branch string) *api.FileContentResponse {
func getExpectedFileContentResponseForFileContents(ref, refType string) *api.FileContentResponse {
treePath := "README.md"
sha := "4b4851ad51df6a7d9f25c979345979eaeb5b349f"
return &api.FileContentResponse{
Name: filepath.Base(treePath),
Path: treePath,
SHA: sha,
Size: 30,
URL: setting.AppURL + "api/v1/repos/user2/repo1/contents/" + treePath,
HTMLURL: setting.AppURL + "user2/repo1/blob/" + branch + "/" + treePath,
URL: setting.AppURL + "api/v1/repos/user2/repo1/contents/" + treePath + "?ref=" + ref,
HTMLURL: setting.AppURL + "user2/repo1/src/" + refType + "/" + ref + "/" + treePath,
GitURL: setting.AppURL + "api/v1/repos/user2/repo1/git/blobs/" + sha,
DownloadURL: setting.AppURL + "user2/repo1/raw/branch/" + branch + "/" + treePath,
DownloadURL: setting.AppURL + "user2/repo1/raw/" + refType + "/" + ref + "/" + treePath,
Type: "blob",
Encoding: "base64",
Content: "IyByZXBvMQoKRGVzY3JpcHRpb24gZm9yIHJlcG8x",
Links: &api.FileLinksResponse{
Self: setting.AppURL + "api/v1/repos/user2/repo1/contents/" + treePath,
Self: setting.AppURL + "api/v1/repos/user2/repo1/contents/" + treePath + "?ref=" + ref,
GitURL: setting.AppURL + "api/v1/repos/user2/repo1/git/blobs/" + sha,
HTMLURL: setting.AppURL + "user2/repo1/blob/" + branch + "/" + treePath,
HTMLURL: setting.AppURL + "user2/repo1/src/" + refType + "/" + ref + "/" + treePath,
},
}
}
Expand All @@ -44,6 +47,7 @@ func TestAPIGetFileContents(t *testing.T) {
}

func testAPIGetFileContents(t *testing.T, u *url.URL) {
/*** SETUP ***/
user2 := models.AssertExistsAndLoadBean(t, &models.User{ID: 2}).(*models.User) // owner of the repo1 & repo16
user3 := models.AssertExistsAndLoadBean(t, &models.User{ID: 3}).(*models.User) // owner of the repo3, is an org
user4 := models.AssertExistsAndLoadBean(t, &models.User{ID: 4}).(*models.User) // owner of neither repos
Expand All @@ -61,53 +65,84 @@ func testAPIGetFileContents(t *testing.T, u *url.URL) {
token4 := getTokenForLoggedInUser(t, session)
session = emptyTestSession(t)

// Make a second master branch in repo1
repo1.CreateNewBranch(user2, repo1.DefaultBranch, "master2")

// ref is default branch
branch := repo1.DefaultBranch
req := NewRequestf(t, "GET", "/api/v1/repos/%s/%s/contents/%s?ref=%s", user2.Name, repo1.Name, treePath, branch)
// Make a new branch in repo1
newBranch := "test_branch"
repo1.CreateNewBranch(user2, repo1.DefaultBranch, newBranch)
// Get the commit ID of the default branch
gitRepo, _ := git.OpenRepository(repo1.RepoPath())
commitID, _ := gitRepo.GetBranchCommitID(repo1.DefaultBranch)
// Make a new tag in repo1
newTag := "test_tag"
gitRepo.CreateTag(newTag, commitID)
/*** END SETUP ***/

// ref is default ref
ref := repo1.DefaultBranch
refType := "branch"
req := NewRequestf(t, "GET", "/api/v1/repos/%s/%s/contents/%s?ref=%s", user2.Name, repo1.Name, treePath, ref)
resp := session.MakeRequest(t, req, http.StatusOK)
var fileContentResponse api.FileContentResponse
DecodeJSON(t, resp, &fileContentResponse)
assert.NotNil(t, fileContentResponse)
expectedFileContentResponse := getExpectedFileContentResponseForFileContents(branch)
expectedFileContentResponse := getExpectedFileContentResponseForFileContents(ref, refType)
assert.EqualValues(t, *expectedFileContentResponse, fileContentResponse)

// No ref
refType = "branch"
req = NewRequestf(t, "GET", "/api/v1/repos/%s/%s/contents/%s", user2.Name, repo1.Name, treePath)
resp = session.MakeRequest(t, req, http.StatusOK)
DecodeJSON(t, resp, &fileContentResponse)
assert.NotNil(t, fileContentResponse)
expectedFileContentResponse = getExpectedFileContentResponseForFileContents(repo1.DefaultBranch)
expectedFileContentResponse = getExpectedFileContentResponseForFileContents(repo1.DefaultBranch, refType)
assert.EqualValues(t, *expectedFileContentResponse, fileContentResponse)

// ref is the branch we created above in setup
ref = newBranch
refType = "branch"
req = NewRequestf(t, "GET", "/api/v1/repos/%s/%s/contents/%s?ref=%s", user2.Name, repo1.Name, treePath, ref)
resp = session.MakeRequest(t, req, http.StatusOK)
DecodeJSON(t, resp, &fileContentResponse)
assert.NotNil(t, fileContentResponse)
expectedFileContentResponse = getExpectedFileContentResponseForFileContents(ref, refType)
assert.EqualValues(t, *expectedFileContentResponse, fileContentResponse)

// ref is the new tag we created above in setup
ref = newTag
refType = "tag"
req = NewRequestf(t, "GET", "/api/v1/repos/%s/%s/contents/%s?ref=%s", user2.Name, repo1.Name, treePath, ref)
resp = session.MakeRequest(t, req, http.StatusOK)
DecodeJSON(t, resp, &fileContentResponse)
assert.NotNil(t, fileContentResponse)
expectedFileContentResponse = getExpectedFileContentResponseForFileContents(ref, refType)
assert.EqualValues(t, *expectedFileContentResponse, fileContentResponse)

// ref is master2
branch = "master2"
req = NewRequestf(t, "GET", "/api/v1/repos/%s/%s/contents/%s?ref=%s", user2.Name, repo1.Name, treePath, branch)
// ref is a commit
ref = commitID
refType = "commit"
req = NewRequestf(t, "GET", "/api/v1/repos/%s/%s/contents/%s?ref=%s", user2.Name, repo1.Name, treePath, ref)
resp = session.MakeRequest(t, req, http.StatusOK)
DecodeJSON(t, resp, &fileContentResponse)
assert.NotNil(t, fileContentResponse)
expectedFileContentResponse = getExpectedFileContentResponseForFileContents("master2")
expectedFileContentResponse = getExpectedFileContentResponseForFileContents(ref, refType)
assert.EqualValues(t, *expectedFileContentResponse, fileContentResponse)

// Test file contents a file with the wrong branch
branch = "badbranch"
req = NewRequestf(t, "GET", "/api/v1/repos/%s/%s/contents/%s?ref=%s", user2.Name, repo1.Name, treePath, branch)
// Test file contents a file with a bad ref
ref = "badref"
req = NewRequestf(t, "GET", "/api/v1/repos/%s/%s/contents/%s?ref=%s", user2.Name, repo1.Name, treePath, ref)
resp = session.MakeRequest(t, req, http.StatusInternalServerError)
expectedAPIError := context.APIError{
Message: "object does not exist [id: " + branch + ", rel_path: ]",
Message: "object does not exist [id: " + ref + ", rel_path: ]",
URL: setting.API.SwaggerURL,
}
var apiError context.APIError
DecodeJSON(t, resp, &apiError)
assert.Equal(t, expectedAPIError, apiError)

// Test accessing private branch with user token that does not have access - should fail
// Test accessing private ref with user token that does not have access - should fail
req = NewRequestf(t, "GET", "/api/v1/repos/%s/%s/contents/%s?token=%s", user2.Name, repo16.Name, treePath, token4)
session.MakeRequest(t, req, http.StatusNotFound)

// Test access private branch of owner of token
// Test access private ref of owner of token
req = NewRequestf(t, "GET", "/api/v1/repos/%s/%s/contents/readme.md?token=%s", user2.Name, repo16.Name, token2)
session.MakeRequest(t, req, http.StatusOK)

Expand Down
12 changes: 7 additions & 5 deletions integrations/api_repo_file_create_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,15 +50,17 @@ func getExpectedFileResponseForCreate(commitID, treePath string) *api.FileRespon
Path: treePath,
SHA: sha,
Size: 16,
URL: setting.AppURL + "api/v1/repos/user2/repo1/contents/" + treePath,
HTMLURL: setting.AppURL + "user2/repo1/blob/master/" + treePath,
URL: setting.AppURL + "api/v1/repos/user2/repo1/contents/" + treePath + "?ref=master",
HTMLURL: setting.AppURL + "user2/repo1/src/branch/master/" + treePath,
GitURL: setting.AppURL + "api/v1/repos/user2/repo1/git/blobs/" + sha,
DownloadURL: setting.AppURL + "user2/repo1/raw/branch/master/" + treePath,
Type: "blob",
Encoding: "base64",
Content: "VGhpcyBpcyBuZXcgdGV4dA==",
Links: &api.FileLinksResponse{
Self: setting.AppURL + "api/v1/repos/user2/repo1/contents/" + treePath,
Self: setting.AppURL + "api/v1/repos/user2/repo1/contents/" + treePath + "?ref=master",
GitURL: setting.AppURL + "api/v1/repos/user2/repo1/git/blobs/" + sha,
HTMLURL: setting.AppURL + "user2/repo1/blob/master/" + treePath,
HTMLURL: setting.AppURL + "user2/repo1/src/branch/master/" + treePath,
},
},
Commit: &api.FileCommitResponse{
Expand Down Expand Up @@ -145,7 +147,7 @@ func TestAPICreateFile(t *testing.T) {
var fileResponse api.FileResponse
DecodeJSON(t, resp, &fileResponse)
expectedSHA := "a635aa942442ddfdba07468cf9661c08fbdf0ebf"
expectedHTMLURL := fmt.Sprintf(setting.AppURL+"user2/repo1/blob/new_branch/new/file%d.txt", fileID)
expectedHTMLURL := fmt.Sprintf(setting.AppURL+"user2/repo1/src/branch/new_branch/new/file%d.txt", fileID)
expectedDownloadURL := fmt.Sprintf(setting.AppURL+"user2/repo1/raw/branch/new_branch/new/file%d.txt", fileID)
assert.EqualValues(t, expectedSHA, fileResponse.Content.SHA)
assert.EqualValues(t, expectedHTMLURL, fileResponse.Content.HTMLURL)
Expand Down
14 changes: 8 additions & 6 deletions integrations/api_repo_file_update_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,15 +38,17 @@ func getExpectedFileResponseForUpdate(commitID, treePath string) *api.FileRespon
Path: treePath,
SHA: sha,
Size: 20,
URL: setting.AppURL + "api/v1/repos/user2/repo1/contents/" + treePath,
HTMLURL: setting.AppURL + "user2/repo1/blob/master/" + treePath,
URL: setting.AppURL + "api/v1/repos/user2/repo1/contents/" + treePath + "?ref=master",
HTMLURL: setting.AppURL + "user2/repo1/src/branch/master/" + treePath,
GitURL: setting.AppURL + "api/v1/repos/user2/repo1/git/blobs/" + sha,
DownloadURL: setting.AppURL + "user2/repo1/raw/branch/master/" + treePath,
Type: "blob",
Encoding: "base64",
Content: "VGhpcyBpcyB1cGRhdGVkIHRleHQ=",
Links: &api.FileLinksResponse{
Self: setting.AppURL + "api/v1/repos/user2/repo1/contents/" + treePath,
Self: setting.AppURL + "api/v1/repos/user2/repo1/contents/" + treePath + "?ref=master",
GitURL: setting.AppURL + "api/v1/repos/user2/repo1/git/blobs/" + sha,
HTMLURL: setting.AppURL + "user2/repo1/blob/master/" + treePath,
HTMLURL: setting.AppURL + "user2/repo1/src/branch/master/" + treePath,
},
},
Commit: &api.FileCommitResponse{
Expand Down Expand Up @@ -135,7 +137,7 @@ func TestAPIUpdateFile(t *testing.T) {
var fileResponse api.FileResponse
DecodeJSON(t, resp, &fileResponse)
expectedSHA := "08bd14b2e2852529157324de9c226b3364e76136"
expectedHTMLURL := fmt.Sprintf(setting.AppURL+"user2/repo1/blob/new_branch/update/file%d.txt", fileID)
expectedHTMLURL := fmt.Sprintf(setting.AppURL+"user2/repo1/src/branch/new_branch/update/file%d.txt", fileID)
expectedDownloadURL := fmt.Sprintf(setting.AppURL+"user2/repo1/raw/branch/new_branch/update/file%d.txt", fileID)
assert.EqualValues(t, expectedSHA, fileResponse.Content.SHA)
assert.EqualValues(t, expectedHTMLURL, fileResponse.Content.HTMLURL)
Expand All @@ -154,7 +156,7 @@ func TestAPIUpdateFile(t *testing.T) {
resp = session.MakeRequest(t, req, http.StatusOK)
DecodeJSON(t, resp, &fileResponse)
expectedSHA = "08bd14b2e2852529157324de9c226b3364e76136"
expectedHTMLURL = fmt.Sprintf(setting.AppURL+"user2/repo1/blob/master/rename/update/file%d.txt", fileID)
expectedHTMLURL = fmt.Sprintf(setting.AppURL+"user2/repo1/src/branch/master/rename/update/file%d.txt", fileID)
expectedDownloadURL = fmt.Sprintf(setting.AppURL+"user2/repo1/raw/branch/master/rename/update/file%d.txt", fileID)
assert.EqualValues(t, expectedSHA, fileResponse.Content.SHA)
assert.EqualValues(t, expectedHTMLURL, fileResponse.Content.HTMLURL)
Expand Down
10 changes: 6 additions & 4 deletions integrations/repofiles_update_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,15 +53,17 @@ func getExpectedFileResponseForRepofilesCreate(commitID string) *api.FileRespons
Path: "new/file.txt",
SHA: "103ff9234cefeee5ec5361d22b49fbb04d385885",
Size: 18,
URL: setting.AppURL + "api/v1/repos/user2/repo1/contents/new/file.txt",
HTMLURL: setting.AppURL + "user2/repo1/blob/master/new/file.txt",
URL: setting.AppURL + "api/v1/repos/user2/repo1/contents/new/file.txt?ref=master",
HTMLURL: setting.AppURL + "user2/repo1/src/branch/master/new/file.txt",
GitURL: setting.AppURL + "api/v1/repos/user2/repo1/git/blobs/103ff9234cefeee5ec5361d22b49fbb04d385885",
DownloadURL: setting.AppURL + "user2/repo1/raw/branch/master/new/file.txt",
Type: "blob",
Encoding: "base64",
Content: "VGhpcyBpcyBhIE5FVyBmaWxl",
Links: &api.FileLinksResponse{
Self: setting.AppURL + "api/v1/repos/user2/repo1/contents/new/file.txt",
Self: setting.AppURL + "api/v1/repos/user2/repo1/contents/new/file.txt?ref=master",
GitURL: setting.AppURL + "api/v1/repos/user2/repo1/git/blobs/103ff9234cefeee5ec5361d22b49fbb04d385885",
HTMLURL: setting.AppURL + "user2/repo1/blob/master/new/file.txt",
HTMLURL: setting.AppURL + "user2/repo1/src/branch/master/new/file.txt",
},
},
Commit: &api.FileCommitResponse{
Expand Down
48 changes: 48 additions & 0 deletions modules/git/repo.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,54 @@ func (repo *Repository) parsePrettyFormatLogToList(logs []byte) (*list.List, err
return l, nil
}

// RepoRefType type of repo reference
type RepoRefType uint8

const (
// RepoRefTypeInvalid invalid ref
RepoRefTypeInvalid RepoRefType = iota
// RepoRefTypeBranch branch
RepoRefTypeBranch
// RepoRefTypeTag tag
RepoRefTypeTag
// RepoRefTypeCommit commit
RepoRefTypeCommit
// RepoRefTypeBlob blob
RepoRefTypeBlob
)

// String converts RepoRefType to a the string representation
func (t RepoRefType) String() string {
switch t {
case RepoRefTypeBlob:
return "blob"
case RepoRefTypeBranch:
return "branch"
case RepoRefTypeCommit:
return "commit"
case RepoRefTypeTag:
return "tag"
case RepoRefTypeInvalid:
return ""
default:
return ""
}
}

// GetRefType gets the type of the ref based on the string
func (repo *Repository) GetRefType(ref string) RepoRefType {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we return ObjectType on https://github.com/go-gitea/gitea/blob/master/modules/git/repo_object.go#L14 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking into it. I actually realized I did not have this at all responding like GitHub, especially when it comes to being 4 different types of objects (dir, file, symlink, submodule) that could be returned, as well well as a list of entries. So this is being redone.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added ObjectBranch and now use those as a way to get the string of a ref. Also moved my GetRefType() into this file.

if repo.IsTagExist(ref) {
return RepoRefTypeTag
} else if repo.IsBranchExist(ref) {
return RepoRefTypeBranch
} else if repo.IsCommitExist(ref) {
return RepoRefTypeCommit
} else if _, err := repo.GetBlob(ref); err == nil {
return RepoRefTypeBlob
}
return RepoRefTypeInvalid
}

// IsRepoURLAccessible checks if given repository URL is accessible.
func IsRepoURLAccessible(url string) bool {
_, err := NewCommand("ls-remote", "-q", "-h", url, "HEAD").Run()
Expand Down
30 changes: 22 additions & 8 deletions modules/repofiles/content.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,21 @@
package repofiles

import (
"fmt"
"net/url"
"strings"

"code.gitea.io/gitea/models"
"code.gitea.io/gitea/modules/git"
api "code.gitea.io/gitea/modules/structs"
)

// GetFileContents gets the meta data on a file's contents
// GetFileContents gets the meta data on a file's contents. Ref can be a branch, commit or tag
func GetFileContents(repo *models.Repository, treePath, ref string) (*api.FileContentResponse, error) {
if ref == "" {
ref = repo.DefaultBranch
}
origRef := ref

// Check that the path given in opts.treePath is valid (not a git path)
treePath = CleanUploadFileName(treePath)
Expand All @@ -36,21 +39,30 @@ func GetFileContents(repo *models.Repository, treePath, ref string) (*api.FileCo
if err != nil {
return nil, err
}
commitID := commit.ID.String()
if len(ref) >= 4 && strings.HasPrefix(commitID, ref) {
ref = commit.ID.String()
}

entry, err := commit.GetTreeEntryByPath(treePath)
if err != nil {
return nil, err
}

urlRef := ref
if _, err := gitRepo.GetBranchCommit(ref); err == nil {
urlRef = "branch/" + ref
blobResponse, err := GetBlobBySHA(repo, entry.ID.String())
if err != nil {
return nil, err
}

refType := gitRepo.GetRefType(ref)
if refType == git.RepoRefTypeInvalid {
return nil, fmt.Errorf("no commit found for the ref [ref: %s]", ref)
}

selfURL, _ := url.Parse(repo.APIURL() + "/contents/" + treePath)
gitURL, _ := url.Parse(repo.APIURL() + "/git/blobs/" + entry.ID.String())
downloadURL, _ := url.Parse(repo.HTMLURL() + "/raw/" + urlRef + "/" + treePath)
htmlURL, _ := url.Parse(repo.HTMLURL() + "/blob/" + ref + "/" + treePath)
selfURL, _ := url.Parse(fmt.Sprintf("%s/contents/%s?ref=%s", repo.APIURL(), treePath, origRef))
gitURL, _ := url.Parse(fmt.Sprintf("%s/git/blobs/%s", repo.APIURL(), entry.ID.String()))
downloadURL, _ := url.Parse(fmt.Sprintf("%s/raw/%s/%s/%s", repo.HTMLURL(), refType.String(), ref, treePath))
htmlURL, _ := url.Parse(fmt.Sprintf("%s/src/%s/%s/%s", repo.HTMLURL(), refType.String(), ref, treePath))

fileContent := &api.FileContentResponse{
Name: entry.Name(),
Expand All @@ -62,6 +74,8 @@ func GetFileContents(repo *models.Repository, treePath, ref string) (*api.FileCo
GitURL: gitURL.String(),
DownloadURL: downloadURL.String(),
Type: entry.Type(),
Encoding: blobResponse.Encoding,
Content: blobResponse.Content,
Links: &api.FileLinksResponse{
Self: selfURL.String(),
GitURL: gitURL.String(),
Expand Down
Loading