Skip to content
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.

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

State revert uses the build planner #2858

Merged
merged 7 commits into from
Nov 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
111 changes: 66 additions & 45 deletions internal/runners/revert/revert.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (
"github.com/ActiveState/cli/internal/runbits/commitmediator"
"github.com/ActiveState/cli/pkg/cmdlets/commit"
gqlmodel "github.com/ActiveState/cli/pkg/platform/api/graphql/model"
"github.com/ActiveState/cli/pkg/platform/api/mono/mono_models"
"github.com/ActiveState/cli/pkg/platform/authentication"
"github.com/ActiveState/cli/pkg/platform/model"
"github.com/ActiveState/cli/pkg/platform/runtime/target"
Expand Down Expand Up @@ -65,39 +64,33 @@ func (r *Revert) Run(params *Params) error {
if err != nil {
return errs.Wrap(err, "Unable to get local commit")
}

if params.CommitID == latestCommit.String() && params.To {
return locale.NewInputError("err_revert_to_current_commit", "The commit to revert to cannot be the latest commit")
}
r.out.Notice(locale.Tl("operating_message", "", r.project.NamespaceString(), r.project.Dir()))
commitID := strfmt.UUID(params.CommitID)

var targetCommit *mono_models.Commit // the commit to revert the contents of, or the commit to revert to
var fromCommit, toCommit strfmt.UUID
if !params.To {
priorCommits, err := model.CommitHistoryPaged(commitID, 0, 2)
if err != nil {
return errs.AddTips(
locale.WrapError(err, "err_revert_get_commit", "", params.CommitID),
locale.T("tip_private_project_auth"),
)
}
if priorCommits.TotalCommits < 2 {
return locale.NewInputError("err_revert_no_history", "Cannot revert commit {{.V0}}: no prior history", params.CommitID)
}
targetCommit = priorCommits.Commits[0]
fromCommit = commitID
toCommit = priorCommits.Commits[1].CommitID // parent commit
} else {
var err error
targetCommit, err = model.GetCommitWithinCommitHistory(latestCommit, commitID)
if err != nil {
return errs.AddTips(
locale.WrapError(err, "err_revert_get_commit", "", params.CommitID),
locale.T("tip_private_project_auth"),
)
}
fromCommit = latestCommit
toCommit = targetCommit.CommitID
bp := model.NewBuildPlannerModel(r.auth)
targetCommitID := params.CommitID // the commit to revert the contents of, or the commit to revert to
revertParams := revertParams{
organization: r.project.Owner(),
project: r.project.Name(),
parentCommitID: latestCommit.String(),
revertCommitID: params.CommitID,
}
revertFunc := r.revertCommit
preposition := ""
if params.To {
revertFunc = r.revertToCommit
preposition = " to" // need leading whitespace
}

targetCommit, err := model.GetCommitWithinCommitHistory(latestCommit, strfmt.UUID(targetCommitID))
if err != nil {
return errs.AddTips(
locale.WrapError(err, "err_revert_get_commit", "", params.CommitID),
locale.T("tip_private_project_auth"),
)
}

var orgs []gqlmodel.Organization
Expand All @@ -108,10 +101,7 @@ func (r *Revert) Run(params *Params) error {
return locale.WrapError(err, "err_revert_get_organizations", "Could not get organizations for current user")
}
}
preposition := ""
if params.To {
preposition = " to" // need leading whitespace
}

if !r.out.Type().IsStructured() {
r.out.Print(locale.Tl("revert_info", "You are about to revert{{.V0}} the following commit:", preposition))
commit.PrintCommit(r.out, targetCommit, orgs)
Expand All @@ -126,41 +116,72 @@ func (r *Revert) Run(params *Params) error {
return locale.NewInputError("err_revert_aborted", "Revert aborted by user")
}

revertCommit, err := model.RevertCommitWithinHistory(fromCommit, toCommit, latestCommit)
revertCommit, err := revertFunc(revertParams, bp)
if err != nil {
return errs.AddTips(
locale.WrapError(err, "err_revert_commit", "", preposition, params.CommitID),
locale.Tl("tip_revert_sync", "Please ensure that the local project is synchronized with the platform and that the given commit ID belongs to the current project"),
locale.T("tip_private_project_auth"))
}

err = runbits.RefreshRuntime(r.auth, r.out, r.analytics, r.project, revertCommit.CommitID, true, target.TriggerRevert, r.svcModel)
err = commitmediator.Set(r.project, revertCommit.String())
if err != nil {
return locale.WrapError(err, "err_refresh_runtime")
return errs.Wrap(err, "Unable to set local commit")
}

err = commitmediator.Set(r.project, revertCommit.CommitID.String())
err = runbits.RefreshRuntime(r.auth, r.out, r.analytics, r.project, revertCommit, true, target.TriggerRevert, r.svcModel)
if err != nil {
return errs.Wrap(err, "Unable to set local commit")
return locale.WrapError(err, "err_refresh_runtime")
Comment on lines +127 to +134
Copy link
Contributor

Choose a reason for hiding this comment

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

Good call to swap these two.

}

r.out.Print(output.Prepare(
locale.Tl("revert_success", "Successfully reverted{{.V0}} commit: {{.V1}}", preposition, params.CommitID),
&struct {
CurrentCommitID string `json:"current_commit_id"`
}{
revertCommit.CommitID.String(),
revertCommit.String(),
},
))
r.out.Notice(locale.T("operation_success_local"))
return nil
}

func containsCommitID(history []*mono_models.Commit, commitID strfmt.UUID) bool {
for _, c := range history {
if c.CommitID == commitID {
return true
}
type revertFunc func(params revertParams, bp *model.BuildPlanner) (strfmt.UUID, error)

type revertParams struct {
organization string
project string
parentCommitID string
revertCommitID string
}

func (r *Revert) revertCommit(params revertParams, bp *model.BuildPlanner) (strfmt.UUID, error) {
newCommitID, err := bp.RevertCommit(params.organization, params.project, params.parentCommitID, params.revertCommitID)
if err != nil {
return "", errs.Wrap(err, "Could not revert commit")
}

return newCommitID, nil
}

func (r *Revert) revertToCommit(params revertParams, bp *model.BuildPlanner) (strfmt.UUID, error) {
buildExpression, err := bp.GetBuildExpression(params.organization, params.project, params.revertCommitID)
if err != nil {
return "", errs.Wrap(err, "Could not get build expression")
}

stageCommitParams := model.StageCommitParams{
Owner: params.organization,
Project: params.project,
ParentCommit: params.parentCommitID,
Description: locale.Tl("revert_commit_description", "Revert to commit {{.V0}}", params.revertCommitID),
Expression: buildExpression,
}
return false

newCommitID, err := bp.StageCommit(stageCommitParams)
if err != nil {
return "", errs.Wrap(err, "Could not stage commit")
}

return newCommitID, nil
}
19 changes: 19 additions & 0 deletions pkg/platform/api/buildplanner/model/buildplan.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,22 +46,29 @@ const (
BuildLogRecipeID = "RECIPE_ID"
BuildRequestID = "BUILD_REQUEST_ID"

// Version Comparators
ComparatorEQ string = "eq"
ComparatorGT = "gt"
ComparatorGTE = "gte"
ComparatorLT = "lt"
ComparatorLTE = "lte"
ComparatorNE = "ne"

// Version Requirement keys
VersionRequirementComparatorKey = "comparator"
VersionRequirementVersionKey = "version"

// MIME types
XArtifactMimeType = "application/x.artifact"
XActiveStateArtifactMimeType = "application/x-activestate-artifacts"
XCamelInstallerMimeType = "application/x-camel-installer"
XGozipInstallerMimeType = "application/x-gozip-installer"
XActiveStateBuilderMimeType = "application/x-activestate-builder"

// RevertCommit strategies
RevertCommitStrategyForce = "Force"
RevertCommitStrategyDefault = "Default"

// MergeCommit strategies
MergeCommitStrategyRecursive MergeStrategy = "Recursive"
MergeCommitStrategyRecursiveOverwriteOnConflict MergeStrategy = "RecursiveOverwriteOnConflict"
Expand Down Expand Up @@ -408,6 +415,18 @@ type CreateProjectResult struct {
ProjectCreated *projectCreated `json:"createProject"`
}

type revertedCommit struct {
Type string `json:"__typename"`
Commit *Commit `json:"commit"`
CommonAncestor strfmt.UUID `json:"commonAncestorID"`
ConflictPaths []string `json:"conflictPaths"`
*Error
}

type RevertCommitResult struct {
RevertedCommit *revertedCommit `json:"revertCommit"`
}

type mergedCommit struct {
Type string `json:"__typename"`
Commit *Commit `json:"commit"`
Expand Down
79 changes: 79 additions & 0 deletions pkg/platform/api/buildplanner/request/revertcommit.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
package request
Copy link
Contributor

Choose a reason for hiding this comment

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

Hey, I understand this now!


import "github.com/ActiveState/cli/pkg/platform/api/buildplanner/model"

func RevertCommit(organization, project, targetVcsRef, commitID string) *revertCommit {
return &revertCommit{map[string]interface{}{
"organization": organization,
"project": project,
"targetVcsRef": targetVcsRef,
"commitId": commitID,
// Currently, we use the force strategy for all revert commits.
// This is because we don't have a way to show the user the conflicts
// and let them resolve them yet.
// https://activestatef.atlassian.net/browse/AR-80?focusedCommentId=46998
"strategy": model.RevertCommitStrategyForce,
}}
}

type revertCommit struct {
vars map[string]interface{}
}

func (r *revertCommit) Query() string {
return `
mutation ($organization: String!, $project: String!, $commitId: String!, $targetVcsRef: String!, $strategy: RevertStrategy) {
revertCommit(
input: {organization: $organization, project: $project, commitId: $commitId, targetVcsRef: $targetVcsRef, strategy: $strategy}
) {
... on RevertedCommit {
__typename
commit {
__typename
commitId
}
}
... on RevertConflict {
__typename
message
commitId
targetCommitId
conflictPaths
}
... on CommitHasNoParent {
__typename
message
}
... on NotFound {
__typename
message
mayNeedAuthentication
}
... on ParseError {
__typename
message
path
}
... on ValidationError {
__typename
message
}
... on Forbidden {
__typename
message
}
... on HeadOnBranchMoved {
__typename
message
}
... on NoChangeSinceLastCommit {
message
commitId
}
}
}`
}

func (r *revertCommit) Vars() map[string]interface{} {
return r.vars
}
23 changes: 23 additions & 0 deletions pkg/platform/model/buildplanner.go
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,29 @@ func (bp *BuildPlanner) CreateProject(params *CreateProjectParams) (strfmt.UUID,
return resp.ProjectCreated.Commit.CommitID, nil
}

func (bp *BuildPlanner) RevertCommit(organization, project, parentCommitID, commitID string) (strfmt.UUID, error) {
logging.Debug("RevertCommit, organization: %s, project: %s, commitID: %s", organization, project, commitID)
resp := &bpModel.RevertCommitResult{}
err := bp.client.Run(request.RevertCommit(organization, project, parentCommitID, commitID), resp)
if err != nil {
return "", processBuildPlannerError(err, "Failed to revert commit")
}

if resp.RevertedCommit == nil {
return "", errs.New("Commit is nil")
}

if bpModel.IsErrorResponse(resp.RevertedCommit.Type) {
return "", bpModel.ProcessCommitError(resp.RevertedCommit.Commit, "Could not revert commit")
}

if resp.RevertedCommit.Commit.CommitID == "" {
return "", errs.New("Commit does not contain commitID")
}

return resp.RevertedCommit.Commit.CommitID, nil
}

type MergeCommitParams struct {
Owner string
Project string
Expand Down
6 changes: 3 additions & 3 deletions test/integration/revert_int_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func (suite *RevertIntegrationTestSuite) TestRevert() {
e2e.OptArgs("history"),
e2e.OptWD(wd),
)
cp.Expect("Revert commit " + commitID)
cp.Expect("Reverted commit for commit " + commitID)
cp.Expect("- urllib3")
cp.Expect("+ argparse") // parent commit
cp.Expect("+ urllib3") // commit whose changes were just reverted
Expand Down Expand Up @@ -83,7 +83,7 @@ func (suite *RevertIntegrationTestSuite) TestRevert_failsOnCommitNotInHistory()
cp.Expect(fmt.Sprintf("Operating on project %s", namespace))
cp.SendLine("Y")
cp.Expect(commitID)
cp.Expect("The commit being reverted is not within the current commit's history")
cp.Expect("The target commit is not within the current commit's history")
cp.ExpectNotExitCode(0)
}

Expand Down Expand Up @@ -115,7 +115,7 @@ func (suite *RevertIntegrationTestSuite) TestRevertTo() {
e2e.OptArgs("history"),
e2e.OptWD(wd),
)
cp.Expect("Reverting to commit " + commitID)
Copy link
Contributor

Choose a reason for hiding this comment

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

What is printed to the user in this case? Should we be asserting it?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch, this is actually the commit message that is now different with the buildplanner

cp.Expect("Revert to commit " + commitID)
cp.Expect("- argparse") // effectively reverting previous commit
cp.Expect("+ argparse") // commit being effectively reverted
cp.Expect("+ urllib3") // commit reverted to
Expand Down
Loading