Skip to content

Commit

Permalink
refactoring: incorporate renaming and reuse feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
nam committed Nov 12, 2019
1 parent ff32067 commit 0eecc75
Show file tree
Hide file tree
Showing 13 changed files with 139 additions and 222 deletions.
30 changes: 14 additions & 16 deletions api/modelgeneration/modelgeneration.go
Expand Up @@ -69,9 +69,9 @@ func (c *Client) CommitBranch(branchName string) (int, error) {
return result.Result, nil
}

// ListCommits lists the committed branches commits,
// ListCommits returns the details of all committed model branches.
func (c *Client) ListCommits() (model.GenerationCommits, error) {
var result params.GenerationCommitResults
var result params.BranchResults
err := c.facade.FacadeCall("ListCommits", nil, &result)
if err != nil {
return nil, errors.Trace(err)
Expand All @@ -82,11 +82,9 @@ func (c *Client) ListCommits() (model.GenerationCommits, error) {
return generationCommitsFromResults(result), nil
}

// CommitBranch commits the branch with the input name to the model,
// effectively completing it and applying all branch changes across the model.
// The new generation ID of the model is returned.
// ShowCommit details of the branch with the input generation ID.
func (c *Client) ShowCommit(generationId int) (model.GenerationCommit, error) {
var result params.GenerationCommitResult
var result params.GenerationResult
arg := params.GenerationId{GenerationId: generationId}
err := c.facade.FacadeCall("ShowCommit", arg, &result)
if err != nil {
Expand Down Expand Up @@ -158,7 +156,7 @@ func (c *Client) BranchInfo(
arg.BranchNames = []string{branchName}
}

var result params.GenerationResults
var result params.BranchResults
err := c.facade.FacadeCall("BranchInfo", arg, &result)
if err != nil {
return nil, errors.Trace(err)
Expand All @@ -176,7 +174,7 @@ func argForBranch(branchName string) params.BranchArg {
}

func generationInfoFromResult(
results params.GenerationResults, detailed bool, formatTime func(time.Time) string,
results params.BranchResults, detailed bool, formatTime func(time.Time) string,
) model.GenerationSummaries {
summaries := make(model.GenerationSummaries)
for _, res := range results.Generations {
Expand Down Expand Up @@ -204,9 +202,9 @@ func generationInfoFromResult(
return summaries
}

func generationCommitsFromResults(results params.GenerationCommitResults) model.GenerationCommits {
commits := make(model.GenerationCommits, len(results.GenerationCommits))
for i, gen := range results.GenerationCommits {
func generationCommitsFromResults(results params.BranchResults) model.GenerationCommits {
commits := make(model.GenerationCommits, len(results.Generations))
for i, gen := range results.Generations {
commits[i] = model.GenerationCommit{
GenerationId: gen.GenerationId,
Completed: time.Unix(gen.Completed, 0),
Expand All @@ -217,14 +215,14 @@ func generationCommitsFromResults(results params.GenerationCommitResults) model.
return commits
}

func generationCommitFromResult(result params.GenerationCommitResult) model.GenerationCommit {
genCommit := result.GenerationCommit
appChanges := make([]model.GenerationCommitApplication, len(genCommit.Applications))
func generationCommitFromResult(result params.GenerationResult) model.GenerationCommit {
genCommit := result.Generation
appChanges := make([]model.GenerationApplication, len(genCommit.Applications))
for i, a := range genCommit.Applications {
app := model.GenerationCommitApplication{
app := model.GenerationApplication{
ApplicationName: a.ApplicationName,
ConfigChanges: a.ConfigChanges,
UnitsTracking: a.UnitsTracking,
UnitDetail: &model.GenerationUnits{UnitsTracking: a.UnitsTracking},
}
appChanges[i] = app
}
Expand Down
2 changes: 1 addition & 1 deletion api/modelgeneration/modelgeneration_test.go
Expand Up @@ -127,7 +127,7 @@ func (s *modelGenerationSuite) TestHasActiveBranch(c *gc.C) {
func (s *modelGenerationSuite) TestBranchInfo(c *gc.C) {
defer s.setUpMocks(c).Finish()

resultSource := params.GenerationResults{Generations: []params.Generation{{
resultSource := params.BranchResults{Generations: []params.Generation{{
BranchName: "new-branch",
Created: time.Time{}.Unix(),
CreatedBy: "test-user",
Expand Down
4 changes: 2 additions & 2 deletions apiserver/facades/client/modelgeneration/interface.go
Expand Up @@ -26,8 +26,8 @@ type Model interface {
AddBranch(string, string) error
Branch(string) (Generation, error)
Branches() ([]Generation, error)
CommittedBranch(int) (Generation, error)
CommittedBranches() ([]Generation, error)
Generation(int) (Generation, error)
Generations() ([]Generation, error)
}

// ModelCache describes a cached model used by the model generation API.
Expand Down
20 changes: 10 additions & 10 deletions apiserver/facades/client/modelgeneration/mocks/package_mock.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

54 changes: 25 additions & 29 deletions apiserver/facades/client/modelgeneration/modelgeneration.go
Expand Up @@ -251,8 +251,8 @@ func (api *API) AbortBranch(arg params.BranchArg) (params.ErrorResult, error) {
// master generation.
// An error is returned if no in-flight branch matching in input is found.
func (api *API) BranchInfo(
args params.BranchInfoArgs) (params.GenerationResults, error) {
result := params.GenerationResults{}
args params.BranchInfoArgs) (params.BranchResults, error) {
result := params.BranchResults{}

isModelAdmin, err := api.hasAdminAccess()
if err != nil {
Expand All @@ -270,19 +270,19 @@ func (api *API) BranchInfo(
branches = make([]Generation, len(args.BranchNames))
for i, name := range args.BranchNames {
if branches[i], err = api.model.Branch(name); err != nil {
return generationResultsError(err)
return branchResultsError(err)
}
}
} else {
if branches, err = api.model.Branches(); err != nil {
return generationResultsError(err)
return branchResultsError(err)
}
}

results := make([]params.Generation, len(branches))
for i, b := range branches {
if results[i], err = api.oneBranchInfo(b, args.Detailed); err != nil {
return generationResultsError(err)
return branchResultsError(err)
}
}
result.Generations = results
Expand All @@ -292,8 +292,8 @@ func (api *API) BranchInfo(
// ShowCommit will return details a commit given by its generationId
// An error is returned if either no branch can be found corresponding to the generation id.
// Or the generation id given is below 1.
func (api *API) ShowCommit(arg params.GenerationId) (params.GenerationCommitResult, error) {
result := params.GenerationCommitResult{}
func (api *API) ShowCommit(arg params.GenerationId) (params.GenerationResult, error) {
result := params.GenerationResult{}

isModelAdmin, err := api.hasAdminAccess()
if err != nil {
Expand All @@ -304,28 +304,28 @@ func (api *API) ShowCommit(arg params.GenerationId) (params.GenerationCommitResu
}
if arg.GenerationId < 1 {
err := errors.Errorf("supplied generation id has to be higher than 0")
return generationCommitResultError(err)
return generationResultError(err)
}

branch, err := api.model.CommittedBranch(arg.GenerationId)
branch, err := api.model.Generation(arg.GenerationId)
if err != nil {
result.Error = common.ServerError(err)
return result, nil
}

generationCommit, err := api.getGenerationCommit(branch)
if err != nil {
return generationCommitResultError(err)
return generationResultError(err)
}

result.GenerationCommit = generationCommit
result.Generation = generationCommit

return result, nil
}

// ListCommits will return the commits, hence only branches with generation_id higher than 0
func (api *API) ListCommits() (params.GenerationCommitResults, error) {
var result params.GenerationCommitResults
func (api *API) ListCommits() (params.BranchResults, error) {
var result params.BranchResults

isModelAdmin, err := api.hasAdminAccess()
if err != nil {
Expand All @@ -336,13 +336,13 @@ func (api *API) ListCommits() (params.GenerationCommitResults, error) {
}

var branches []Generation
if branches, err = api.model.CommittedBranches(); err != nil {
return generationCommitResultsError(err)
if branches, err = api.model.Generations(); err != nil {
return branchResultsError(err)
}

results := make([]params.GenerationCommit, len(branches))
results := make([]params.Generation, len(branches))
for i, b := range branches {
gen := params.GenerationCommit{
gen := params.Generation{
BranchName: b.BranchName(),
Completed: b.Completed(),
CompletedBy: b.CompletedBy(),
Expand All @@ -351,7 +351,7 @@ func (api *API) ListCommits() (params.GenerationCommitResults, error) {
results[i] = gen
}

result.GenerationCommits = results
result.Generations = results
return result, nil
}

Expand Down Expand Up @@ -403,12 +403,12 @@ func (api *API) oneBranchInfo(branch Generation, detailed bool) (params.Generati
}, nil
}

func (api *API) getGenerationCommit(branch Generation) (params.GenerationCommit, error) {
func (api *API) getGenerationCommit(branch Generation) (params.Generation, error) {
generation, err := api.oneBranchInfo(branch, true)
if err != nil {
return params.GenerationCommit{}, errors.Trace(err)
return params.Generation{}, errors.Trace(err)
}
return params.GenerationCommit{
return params.Generation{
BranchName: branch.BranchName(),
Completed: branch.Completed(),
CompletedBy: branch.CompletedBy(),
Expand Down Expand Up @@ -443,16 +443,12 @@ func (api *API) HasActiveBranch(arg params.BranchArg) (params.BoolResult, error)
return result, nil
}

func generationResultsError(err error) (params.GenerationResults, error) {
return params.GenerationResults{Error: common.ServerError(err)}, nil
}

func generationCommitResultsError(err error) (params.GenerationCommitResults, error) {
return params.GenerationCommitResults{Error: common.ServerError(err)}, nil
func branchResultsError(err error) (params.BranchResults, error) {
return params.BranchResults{Error: common.ServerError(err)}, nil
}

func generationCommitResultError(err error) (params.GenerationCommitResult, error) {
return params.GenerationCommitResult{Error: common.ServerError(err)}, nil
func generationResultError(err error) (params.GenerationResult, error) {
return params.GenerationResult{Error: common.ServerError(err)}, nil
}

func intResultsError(err error) (params.IntResult, error) {
Expand Down
12 changes: 6 additions & 6 deletions apiserver/facades/client/modelgeneration/shim.go
Expand Up @@ -37,17 +37,17 @@ func (g *modelShim) Branches() ([]Generation, error) {
return res, nil
}

// CommittedBranch wraps the state model CommittedBranch method,
// Generation wraps the state model Generation method,
// returning the locally defined Generation interface.
func (g *modelShim) CommittedBranch(id int) (Generation, error) {
m, err := g.Model.CommittedBranch(id)
func (g *modelShim) Generation(id int) (Generation, error) {
m, err := g.Model.Generation(id)
return m, errors.Trace(err)
}

// Branches wraps the state model CommittedBranches method,
// Branches wraps the state model Generations method,
// returning a collection of the Generation interface.
func (g *modelShim) CommittedBranches() ([]Generation, error) {
branches, err := g.Model.CommittedBranches()
func (g *modelShim) Generations() ([]Generation, error) {
branches, err := g.Model.Generations()
if err != nil {
return nil, errors.Trace(err)
}
Expand Down

0 comments on commit 0eecc75

Please sign in to comment.