cybercyst/drone
1
0
Fork 0
mirror of https://github.com/harness/drone.git synced 2025-05-05 15:32:56 +00:00
Code Issues Packages Projects Releases Wiki Activity

feat: [CODE-3530]: Return new file SHAs as part of commit API response (#3753)

Browse Source 
* Apply suggestions from code review
* fixed formatting
* fixed fileRefs initialization
* Merge branch 'CODE-3530' of https://git0.harness.io/l7B_kbSEQD2wjrM7PShm5w/PROD/Harness_Commons/gitness into CODE-3530
* addressed review comments
* Apply suggestion from code review
* addressed review comments
* Apply suggestions from code review
* feat: [CODE-3530]: Return new file SHAs as part of commit API response
This commit is contained in:
Karan Saraswat 2025-05-05 13:40:55 +00:00 committed by Harness
parent 0658c11bc7
commit 0d67a5358f
4 changed files with 100 additions and 61 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
app/api/controller/repo/commit.go
View File

@ -73,6 +73,17 @@ func (in *CommitFilesOptions) Sanitize() error {
return nil
}
func mapChangedFiles(files []git.FileReference) []types.FileReference {
changedFiles := make([]types.FileReference, len(files))
for i, file := range files {
changedFiles[i] = types.FileReference{
Path: file.Path,
SHA: file.SHA.String(),
}
}
return changedFiles
}
func (c *Controller) CommitFiles(ctx context.Context,
session *auth.Session,
repoRef string,
@ -214,5 +225,6 @@ func (c *Controller) CommitFiles(ctx context.Context,
DryRunRulesOutput: types.DryRunRulesOutput{
RuleViolations: violations,
},
ChangedFiles: mapChangedFiles(commit.ChangedFiles),
}, nil, nil
}

91
git/operations.go
View File

@ -81,8 +81,14 @@ func (p *CommitFilesParams) Validate() error {
return p.WriteParams.Validate()
}
type FileReference struct {
Path string
SHA sha.SHA
}
type CommitFilesResponse struct {
CommitID sha.SHA
CommitID sha.SHA
ChangedFiles []FileReference
}
//nolint:gocognit,nestif
@ -134,6 +140,7 @@ func (s *Service) CommitFiles(ctx context.Context, params *CommitFilesParams) (C
// ref updater
var refOldSHA sha.SHA
var refNewSHA sha.SHA
var changedFiles []FileReference
branchRef := api.GetReferenceFromBranchName(params.Branch)
if params.Branch != params.NewBranch {
@ -150,14 +157,13 @@ func (s *Service) CommitFiles(ctx context.Context, params *CommitFilesParams) (C
}
// run the actions in a shared repo
err = sharedrepo.Run(ctx, refUpdater, s.sharedRepoRoot, repoPath, func(r *sharedrepo.SharedRepo) error {
var parentCommits []sha.SHA
var oldTreeSHA sha.SHA
if isEmpty {
oldTreeSHA = sha.EmptyTree
err = s.prepareTreeEmptyRepo(ctx, r, params.Actions)
changedFiles, err = s.prepareTreeEmptyRepo(ctx, r, params.Actions)
if err != nil {
return fmt.Errorf("failed to prepare empty tree: %w", err)
}
@ -176,7 +182,7 @@ func (s *Service) CommitFiles(ctx context.Context, params *CommitFilesParams) (C
return fmt.Errorf("failed to set index in shared repository: %w", err)
}
err = s.prepareTree(ctx, r, commit.SHA, params.Actions)
changedFiles, err = s.prepareTree(ctx, r, commit.SHA, params.Actions)
if err != nil {
return fmt.Errorf("failed to prepare tree: %w", err)
}
@ -238,16 +244,9 @@ func (s *Service) CommitFiles(ctx context.Context, params *CommitFilesParams) (C
return CommitFilesResponse{}, fmt.Errorf("CommitFiles: failed to create commit in shared repository: %w", err)
}
// get commit
commit, err = s.git.GetCommit(ctx, repoPath, refNewSHA.String())
if err != nil {
return CommitFilesResponse{}, fmt.Errorf("failed to get commit for SHA %s: %w",
refNewSHA.String(), err)
}
return CommitFilesResponse{
CommitID: commit.SHA,
CommitID: refNewSHA,
ChangedFiles: changedFiles,
}, nil
}
@ -256,13 +255,15 @@ func (s *Service) prepareTree(
r *sharedrepo.SharedRepo,
treeishSHA sha.SHA,
actions []CommitFileAction,
) error {
) ([]FileReference, error) {
// patch file actions are executed in batch for a single file
patchMap := map[string][]*CommitFileAction{}
// keep track of what paths have been written to detect conflicting actions
modifiedPaths := map[string]bool{}
fileRefs := make([]FileReference, 0, len(actions))
for i := range actions {
act := &actions[i]
@ -272,14 +273,19 @@ func (s *Service) prepareTree(
continue
}
// anything else is executed as is
modifiedPath, err := s.processAction(ctx, r, treeishSHA, act)
modifiedPath, objectSHA, err := s.processAction(ctx, r, treeishSHA, act)
if err != nil {
return fmt.Errorf("failed to process action %s on %q: %w", act.Action, act.Path, err)
return nil, fmt.Errorf("failed to process action %s on %q: %w", act.Action, act.Path, err)
}
if modifiedPaths[modifiedPath] {
return errors.InvalidArgument("More than one conflicting actions are modifying file %q.", modifiedPath)
return nil, errors.InvalidArgument("More than one conflicting actions are modifying file %q.", modifiedPath)
}
fileRefs = append(fileRefs, FileReference{
Path: modifiedPath,
SHA: objectSHA,
})
modifiedPaths[modifiedPath] = true
}
@ -296,7 +302,7 @@ func (s *Service) prepareTree(
// there can only be one file sha for a given path and commit.
if !act.SHA.IsEmpty() && !fileSHA.Equal(act.SHA) {
return errors.InvalidArgument(
return nil, errors.InvalidArgument(
"patch text actions for %q contain different SHAs %q and %q",
filePath,
act.SHA,
@ -305,40 +311,54 @@ func (s *Service) prepareTree(
}
}
if err := r.PatchTextFile(ctx, treeishSHA, filePath, fileSHA, payloads); err != nil {
return fmt.Errorf("failed to process action %s on %q: %w", PatchTextAction, filePath, err)
objectSHA, err := r.PatchTextFile(ctx, treeishSHA, filePath, fileSHA, payloads)
if err != nil {
return nil, fmt.Errorf("failed to process action %s on %q: %w", PatchTextAction, filePath, err)
}
if modifiedPaths[filePath] {
return errors.InvalidArgument("More than one conflicting action are modifying file %q.", filePath)
return nil, errors.InvalidArgument("More than one conflicting action are modifying file %q.", filePath)
}
fileRefs = append(fileRefs, FileReference{
Path: filePath,
SHA: objectSHA,
})
modifiedPaths[filePath] = true
}
return nil
return fileRefs, nil
}
func (s *Service) prepareTreeEmptyRepo(
ctx context.Context,
r *sharedrepo.SharedRepo,
actions []CommitFileAction,
) error {
) ([]FileReference, error) {
fileRefs := make([]FileReference, 0, len(actions))
for _, action := range actions {
if action.Action != CreateAction {
return errors.PreconditionFailed("action not allowed on empty repository")
return nil, errors.PreconditionFailed("action not allowed on empty repository")
}
filePath := api.CleanUploadFileName(action.Path)
if filePath == "" {
return errors.InvalidArgument("invalid path")
return nil, errors.InvalidArgument("invalid path")
}
if err := r.CreateFile(ctx, sha.None, filePath, filePermissionDefault, action.Payload); err != nil {
return errors.Internal(err, "failed to create file '%s'", action.Path)
objectSHA, err := r.CreateFile(ctx, sha.None, filePath, filePermissionDefault, action.Payload)
if err != nil {
return nil, errors.Internal(err, "failed to create file '%s'", action.Path)
}
fileRefs = append(fileRefs, FileReference{
Path: filePath,
SHA: objectSHA,
})
}
return nil
return fileRefs, nil
}
func (s *Service) validateAndPrepareCommitFilesHeader(
@ -393,26 +413,27 @@ func (s *Service) processAction(
r *sharedrepo.SharedRepo,
treeishSHA sha.SHA,
action *CommitFileAction,
) (modifiedPath string, err error) {
) (modifiedPath string, objectSHA sha.SHA, err error) {
filePath := api.CleanUploadFileName(action.Path)
if filePath == "" {
return "", errors.InvalidArgument("path cannot be empty")
return "", sha.None, errors.InvalidArgument("path cannot be empty")
}
modifiedPath = filePath
switch action.Action {
case CreateAction:
err = r.CreateFile(ctx, treeishSHA, filePath, filePermissionDefault, action.Payload)
objectSHA, err = r.CreateFile(ctx, treeishSHA, filePath, filePermissionDefault, action.Payload)
case UpdateAction:
err = r.UpdateFile(ctx, treeishSHA, filePath, action.SHA, filePermissionDefault, action.Payload)
objectSHA, err = r.UpdateFile(ctx, treeishSHA, filePath, action.SHA, filePermissionDefault, action.Payload)
case MoveAction:
modifiedPath, err = r.MoveFile(ctx, treeishSHA, filePath, action.SHA, filePermissionDefault, action.Payload)
modifiedPath, objectSHA, err =
r.MoveFile(ctx, treeishSHA, filePath, action.SHA, filePermissionDefault, action.Payload)
case DeleteAction:
err = r.DeleteFile(ctx, filePath)
case PatchTextAction:
return "", fmt.Errorf("action %s not supported by this method", action.Action)
return "", sha.None, fmt.Errorf("action %s not supported by this method", action.Action)
default:
err = fmt.Errorf("unknown file action %q", action.Action)
}
return modifiedPath, err
return modifiedPath, objectSHA, err
}

52
git/sharedrepo/sharedrepo.go
View File

@ -502,25 +502,25 @@ func (r *SharedRepo) CreateFile(
treeishSHA sha.SHA,
filePath, mode string,
payload []byte,
) error {
) (sha.SHA, error) {
// only check path availability if a source commit is available (empty repo won't have such a commit)
if !treeishSHA.IsEmpty() {
if err := r.checkPathAvailability(ctx, treeishSHA, filePath, true); err != nil {
return err
return sha.None, err
}
}
objectSHA, err := r.WriteGitObject(ctx, bytes.NewReader(payload))
if err != nil {
return fmt.Errorf("createFile: error hashing object: %w", err)
return sha.None, fmt.Errorf("createFile: error hashing object: %w", err)
}
// Add the object to the index
if err = r.AddObjectToIndex(ctx, mode, objectSHA, filePath); err != nil {
return fmt.Errorf("createFile: error creating object: %w", err)
return sha.None, fmt.Errorf("createFile: error creating object: %w", err)
}
return nil
return objectSHA, nil
}
func (r *SharedRepo) UpdateFile(
@ -530,11 +530,11 @@ func (r *SharedRepo) UpdateFile(
objectSHA sha.SHA,
mode string,
payload []byte,
) error {
) (sha.SHA, error) {
// get file mode from existing file (default unless executable)
entry, err := r.getFileEntry(ctx, treeishSHA, objectSHA, filePath)
if err != nil {
return err
return sha.None, err
}
if entry.IsExecutable() {
@ -543,14 +543,14 @@ func (r *SharedRepo) UpdateFile(
objectSHA, err = r.WriteGitObject(ctx, bytes.NewReader(payload))
if err != nil {
return fmt.Errorf("updateFile: error hashing object: %w", err)
return sha.None, fmt.Errorf("updateFile: error hashing object: %w", err)
}
if err = r.AddObjectToIndex(ctx, mode, objectSHA, filePath); err != nil {
return fmt.Errorf("updateFile: error updating object: %w", err)
return sha.None, fmt.Errorf("updateFile: error updating object: %w", err)
}
return nil
return objectSHA, nil
}
func (r *SharedRepo) MoveFile(
@ -560,21 +560,21 @@ func (r *SharedRepo) MoveFile(
objectSHA sha.SHA,
mode string,
payload []byte,
) (string, error) {
) (string, sha.SHA, error) {
newPath, newContent, err := parseMovePayload(payload)
if err != nil {
return "", err
return "", sha.None, err
}
// ensure file exists and matches SHA
entry, err := r.getFileEntry(ctx, treeishSHA, objectSHA, filePath)
if err != nil {
return "", err
return "", sha.None, err
}
// ensure new path is available
if err = r.checkPathAvailability(ctx, treeishSHA, newPath, false); err != nil {
return "", err
return "", sha.None, err
}
var fileHash sha.SHA
@ -582,7 +582,7 @@ func (r *SharedRepo) MoveFile(
if newContent != nil {
hash, err := r.WriteGitObject(ctx, bytes.NewReader(newContent))
if err != nil {
return "", fmt.Errorf("moveFile: error hashing object: %w", err)
return "", sha.None, fmt.Errorf("moveFile: error hashing object: %w", err)
}
fileHash = hash
@ -597,14 +597,14 @@ func (r *SharedRepo) MoveFile(
}
if err = r.AddObjectToIndex(ctx, fileMode, fileHash, newPath); err != nil {
return "", fmt.Errorf("moveFile: add object error: %w", err)
return "", sha.None, fmt.Errorf("moveFile: add object error: %w", err)
}
if err = r.RemoveFilesFromIndex(ctx, filePath); err != nil {
return "", fmt.Errorf("moveFile: remove object error: %w", err)
return "", sha.None, fmt.Errorf("moveFile: remove object error: %w", err)
}
return newPath, nil
return newPath, fileHash, nil
}
func (r *SharedRepo) DeleteFile(ctx context.Context, filePath string) error {
@ -628,25 +628,25 @@ func (r *SharedRepo) PatchTextFile(
filePath string,
objectSHA sha.SHA,
payloadsRaw [][]byte,
) error {
) (sha.SHA, error) {
payloads, err := parsePatchTextFilePayloads(payloadsRaw)
if err != nil {
return err
return sha.None, err
}
entry, err := r.getFileEntry(ctx, treeishSHA, objectSHA, filePath)
if err != nil {
return err
return sha.None, err
}
blob, err := api.GetBlob(ctx, r.repoPath, nil, entry.SHA, 0)
if err != nil {
return fmt.Errorf("error reading blob: %w", err)
return sha.None, fmt.Errorf("error reading blob: %w", err)
}
scanner, lineEnding, err := parser.ReadTextFile(blob.Content, nil)
if err != nil {
return fmt.Errorf("error reading blob as text file: %w", err)
return sha.None, fmt.Errorf("error reading blob as text file: %w", err)
}
pipeReader, pipeWriter := io.Pipe()
@ -660,14 +660,14 @@ func (r *SharedRepo) PatchTextFile(
objectSHA, err = r.WriteGitObject(ctx, pipeReader)
if err != nil {
return fmt.Errorf("error writing patched file to git store: %w", err)
return sha.None, fmt.Errorf("error writing patched file to git store: %w", err)
}
if err = r.AddObjectToIndex(ctx, entry.Mode.String(), objectSHA, filePath); err != nil {
return fmt.Errorf("error updating object: %w", err)
return sha.None, fmt.Errorf("error updating object: %w", err)
}
return nil
return objectSHA, nil
}
// nolint: gocognit, gocyclo, cyclop

6
types/commit.go
View File

@ -18,4 +18,10 @@ package types
type CommitFilesResponse struct {
CommitID string `json:"commit_id"`
DryRunRulesOutput
ChangedFiles []FileReference `json:"changed_files"`
}
type FileReference struct {
Path string `json:"path"`
SHA string `json:"blob_sha"`
}