fix(usecase): drop the unused repository read before the merge lock
merge reads the repository again under the lock; the first read only checks access.
1 file · +2 -3
M internal/usecase/mergerequests.go +2 -3
| @@ -1002,8 +1002,7 @@ func (m *MergeRequests) checkMode(ctx context.Context, mr domain.MergeRequest, m | |||
| 1002 | 1002 | } | |
| 1003 | 1003 | ||
| 1004 | 1004 | func (m *MergeRequests) merge(ctx context.Context, p domain.Principal, name domain.RepoName, number int64, params ports.MergeParams) (domain.MergeRequest, error) { | |
| 1005 | - | repo, err := m.writable(ctx, p, name, domain.ScopeMRMerge) | |
| 1006 | - | if err != nil { | |
| 1005 | + | if _, err := m.writable(ctx, p, name, domain.ScopeMRMerge); err != nil { | |
| 1007 | 1006 | return domain.MergeRequest{}, err | |
| 1008 | 1007 | } | |
| 1009 | 1008 | mode := params.Mode | |
| @@ -1048,7 +1047,7 @@ func (m *MergeRequests) merge(ctx context.Context, p domain.Principal, name doma | |||
| 1048 | 1047 | return domain.MergeRequest{}, fmt.Errorf("%w: !%d is a draft: mark it ready for review first", domain.ErrPreconditionFailed, number) | |
| 1049 | 1048 | } | |
| 1050 | 1049 | // The settings are read again under the lock: a change applies at once. | |
| 1051 | - | repo, err = m.meta.Get(ctx, name) | |
| 1050 | + | repo, err := m.meta.Get(ctx, name) | |
| 1052 | 1051 | if err != nil { | |
| 1053 | 1052 | if errors.Is(err, domain.ErrNotFound) { | |
| 1054 | 1053 | return domain.MergeRequest{}, err // deleted meanwhile | |
| @@ -1002,8 +1002,7 @@ func (m *MergeRequests) checkMode(ctx context.Context, mr domain.MergeRequest, m | |||||
| 1002 | } | 1002 | } | ||
| 1003 | 1003 | ||||
| 1004 | func (m *MergeRequests) merge(ctx context.Context, p domain.Principal, name domain.RepoName, number int64, params ports.MergeParams) (domain.MergeRequest, error) { | 1004 | func (m *MergeRequests) merge(ctx context.Context, p domain.Principal, name domain.RepoName, number int64, params ports.MergeParams) (domain.MergeRequest, error) { | ||
| 1005 | - | repo, err := m.writable(ctx, p, name, domain.ScopeMRMerge) | 1005 | + | if _, err := m.writable(ctx, p, name, domain.ScopeMRMerge); err != nil { |
| 1006 | - | if err != nil { | |||
| 1007 | return domain.MergeRequest{}, err | 1006 | return domain.MergeRequest{}, err | ||
| 1008 | } | 1007 | } | ||
| 1009 | mode := params.Mode | 1008 | mode := params.Mode | ||
| @@ -1048,7 +1047,7 @@ func (m *MergeRequests) merge(ctx context.Context, p domain.Principal, name doma | |||||
| 1048 | return domain.MergeRequest{}, fmt.Errorf("%w: !%d is a draft: mark it ready for review first", domain.ErrPreconditionFailed, number) | 1047 | return domain.MergeRequest{}, fmt.Errorf("%w: !%d is a draft: mark it ready for review first", domain.ErrPreconditionFailed, number) | ||
| 1049 | } | 1048 | } | ||
| 1050 | // The settings are read again under the lock: a change applies at once. | 1049 | // The settings are read again under the lock: a change applies at once. | ||
| 1051 | - | repo, err = m.meta.Get(ctx, name) | 1050 | + | repo, err := m.meta.Get(ctx, name) |
| 1052 | if err != nil { | 1051 | if err != nil { | ||
| 1053 | if errors.Is(err, domain.ErrNotFound) { | 1052 | if errors.Is(err, domain.ErrNotFound) { | ||
| 1054 | return domain.MergeRequest{}, err // deleted meanwhile | 1053 | return domain.MergeRequest{}, err // deleted meanwhile | ||