Skip to content

Commit

Permalink
refactor: unify the statekey getting method
Browse files Browse the repository at this point in the history
Signed-off-by: Daniel Hu <tao.hu@merico.dev>
  • Loading branch information
daniel-hutao committed Oct 17, 2022
1 parent 83b7e2b commit 403a369
Show file tree
Hide file tree
Showing 12 changed files with 31 additions and 36 deletions.
2 changes: 1 addition & 1 deletion internal/pkg/configmanager/configmanager.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func (c *Config) ValidateDependency() []error {
toolMap := make(map[string]bool)
// creating the set
for _, tool := range c.Tools {
toolMap[tool.Key()] = true
toolMap[tool.KeyWithNameAndInstanceID()] = true
}

for _, tool := range c.Tools {
Expand Down
2 changes: 1 addition & 1 deletion internal/pkg/configmanager/toolconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ func (t *Tool) DeepCopy() *Tool {
return &retTool
}

func (t *Tool) Key() string {
func (t *Tool) KeyWithNameAndInstanceID() string {
return fmt.Sprintf("%s.%s", t.Name, t.InstanceID)
}

Expand Down
4 changes: 2 additions & 2 deletions internal/pkg/configmanager/toolconfig_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,12 @@ var _ = Describe("Tool Config", func() {
})
})

Context("Key method", func() {
Context("KeyWithNameAndInstanceID method", func() {
It("should return joined key", func() {
toolName := "test_tool"
toolInstance := "0"
tool := configmanager.Tool{Name: toolName, InstanceID: toolInstance}
key := tool.Key()
key := tool.KeyWithNameAndInstanceID()
Expect(key).Should(Equal(fmt.Sprintf("%s.%s", toolName, toolInstance)))
})
})
Expand Down
14 changes: 7 additions & 7 deletions internal/pkg/plugin/jiragithub/templates.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package jiragithub

var jiraIssuesBuilder = `name: jira-github-integ Builder
var jiraIssuesBuilder = `name: jira-github-integ Builder
on:
issues:
types: [opened, reopened, edited, closed]
Expand Down Expand Up @@ -28,7 +28,7 @@ jobs:
issuetype: Task
summary: ${{ github.event.issue.title }}
description: ${{ github.event.issue.body }}
- name: Rename github issue
if: ${{ github.event.action == 'opened' }}
uses: actions-cool/issues-helper@v3
Expand All @@ -40,14 +40,14 @@ jobs:
title: "[${{ steps.create.outputs.issue }}] ${{ github.event.issue.title }}"
update-mode: 'replace'
emoji: '+1'
- name: Find Jira Issue Key
id: find
if: ${{ github.event.action != 'opened' }}
uses: atlassian/gajira-find-issue-key@master
with:
string: "${{ github.event.issue.title }}"
- name: Transition issue to In Progress
id: transition_inprogress
if: ${{ github.event.action == 'edited' }}
Expand All @@ -71,7 +71,7 @@ jobs:
with:
issue: ${{ steps.find.outputs.issue }}
transition: "To Do"
issue_comment_integration:
if: ${{ github.event_name == 'issue_comment' }}
runs-on: ubuntu-latest
Expand All @@ -87,7 +87,7 @@ jobs:
id: find
uses: atlassian/gajira-find-issue-key@master
with:
string: "${{ github.event.issue.title }}"
string: "${{ github.event.issue.title }}"
- name: Create issue
id: create_issue
if: ${{ github.event.action == 'created' }}
Expand All @@ -103,7 +103,7 @@ jobs:
with:
issue: ${{ steps.find.outputs.issue }}
comment: "updated: ${{ github.event.comment.body }} from: ${{ github.event.changes.body.from }}"
- name: Delete issue
id: del_issue
if: ${{ github.event.action == 'deleted' }}
Expand Down
4 changes: 2 additions & 2 deletions internal/pkg/pluginengine/change.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ func handleResult(smgr statemanager.Manager, change *Change) error {
}

if change.ActionName == statemanager.ActionDelete {
key := statemanager.StateKeyGenerateFunc(change.Tool)
key := statemanager.GenerateStateKeyByToolNameAndInstanceID(change.Tool.Name, change.Tool.InstanceID)
log.Infof("Prepare to delete '%s' from States.", key)
err := smgr.DeleteState(key)
if err != nil {
Expand All @@ -154,7 +154,7 @@ func handleResult(smgr statemanager.Manager, change *Change) error {
return nil
}

key := statemanager.StateKeyGenerateFunc(change.Tool)
key := statemanager.GenerateStateKeyByToolNameAndInstanceID(change.Tool.Name, change.Tool.InstanceID)
state := statemanager.State{
Name: change.Tool.Name,
InstanceID: change.Tool.InstanceID,
Expand Down
16 changes: 8 additions & 8 deletions internal/pkg/pluginengine/change_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ func GetChangesForApply(smgr statemanager.Manager, cfg *configmanager.Config) (c
// 3. generate changes for each tool
for _, batch := range batchesOfTools {
for _, tool := range batch {
state := smgr.GetState(statemanager.StateKeyGenerateFunc(&tool))
state := smgr.GetState(statemanager.GenerateStateKeyByToolNameAndInstanceID(tool.Name, tool.InstanceID))

if state == nil {
// tool not in the state, create, no need to Read resource before Create
Expand Down Expand Up @@ -142,7 +142,7 @@ func GetChangesForApply(smgr statemanager.Manager, cfg *configmanager.Config) (c
}

// delete the tool from the temporary state map since it's already been processed above
tmpStatesMap.Delete(statemanager.StateKeyGenerateFunc(&tool))
tmpStatesMap.Delete(statemanager.GenerateStateKeyByToolNameAndInstanceID(tool.Name, tool.InstanceID))
}
}

Expand Down Expand Up @@ -177,7 +177,7 @@ func GetChangesForDelete(smgr statemanager.Manager, cfg *configmanager.Config, i
batch := batchesOfTools[i]
for _, tool := range batch {
if !isForceDelete {
state := smgr.GetState(statemanager.StateKeyGenerateFunc(&tool))
state := smgr.GetState(statemanager.GenerateStateKeyByToolNameAndInstanceID(tool.Name, tool.InstanceID))
if state == nil {
continue
}
Expand Down Expand Up @@ -220,7 +220,7 @@ func GetChangesForDestroy(smgr statemanager.Manager, isForceDestroy bool) (chang
batch := batchesOfTools[i]
for _, tool := range batch {
if !isForceDestroy {
state := smgr.GetState(statemanager.StateKeyGenerateFunc(&tool))
state := smgr.GetState(statemanager.GenerateStateKeyByToolNameAndInstanceID(tool.Name, tool.InstanceID))
if state == nil {
continue
}
Expand Down Expand Up @@ -264,7 +264,7 @@ func topologicalSortChangesInBatch(changes []*Change) ([][]*Change, error) {
// for each tool in the batch, find the change that matches it
for _, tool := range batch {
// only add the change that has the tool match with it
if change, ok := changesKeyMap[tool.Key()]; ok {
if change, ok := changesKeyMap[tool.KeyWithNameAndInstanceID()]; ok {
changesOneBatch = append(changesOneBatch, change)
}
}
Expand All @@ -285,9 +285,9 @@ func getToolsFromChanges(changes []*Change) []configmanager.Tool {

// get tools from changes avoiding duplicated tools
for _, change := range changes {
if _, ok := toolsKeyMap[change.Tool.Key()]; !ok {
if _, ok := toolsKeyMap[change.Tool.KeyWithNameAndInstanceID()]; !ok {
tools = append(tools, *change.Tool)
toolsKeyMap[change.Tool.Key()] = struct{}{}
toolsKeyMap[change.Tool.KeyWithNameAndInstanceID()] = struct{}{}
}
}

Expand All @@ -297,7 +297,7 @@ func getToolsFromChanges(changes []*Change) []configmanager.Tool {
func getChangesKeyMap(changes []*Change) map[string]*Change {
changesKeyMap := make(map[string]*Change)
for _, change := range changes {
changesKeyMap[change.Tool.Key()] = change
changesKeyMap[change.Tool.KeyWithNameAndInstanceID()] = change
}
return changesKeyMap
}
2 changes: 1 addition & 1 deletion internal/pkg/pluginengine/outputs.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ func HandleOutputsReferences(smgr statemanager.Manager, options map[string]inter
continue
}

outputs, err := smgr.GetOutputs(statemanager.GenerateStateKeyByToolNameAndPluginKind(toolName, instanceID))
outputs, err := smgr.GetOutputs(statemanager.GenerateStateKeyByToolNameAndInstanceID(toolName, instanceID))
if err != nil {
errorsList = append(errorsList, err)
continue
Expand Down
6 changes: 3 additions & 3 deletions internal/pkg/pluginengine/topological_sort.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func topologicalSort(tools []configmanager.Tool) ([][]configmanager.Tool, error)
// a "graph", which contains "nodes" that haven't been processed yet
unprocessedNodeSet := make(map[string]bool)
for _, tool := range tools {
unprocessedNodeSet[tool.Key()] = true
unprocessedNodeSet[tool.KeyWithNameAndInstanceID()] = true
}

// while there is still a node in the graph left to be processed:
Expand All @@ -42,7 +42,7 @@ func topologicalSort(tools []configmanager.Tool) ([][]configmanager.Tool, error)

for _, tool := range tools {
// if the tool has already been processed (not in the unprocessedNodeSet anymore), pass
if _, ok := unprocessedNodeSet[tool.Key()]; !ok {
if _, ok := unprocessedNodeSet[tool.KeyWithNameAndInstanceID()]; !ok {
continue
}

Expand All @@ -68,7 +68,7 @@ func topologicalSort(tools []configmanager.Tool) ([][]configmanager.Tool, error)

// remove tools from the unprocessedNodeSet because they have been added to the batch
for _, tool := range batch {
delete(unprocessedNodeSet, tool.Key())
delete(unprocessedNodeSet, tool.KeyWithNameAndInstanceID())
}

// add the batch to the final result
Expand Down
2 changes: 1 addition & 1 deletion internal/pkg/show/status/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ func showAll(smgr statemanager.Manager) error {
func showOne(smgr statemanager.Manager, id, plugin string) error {
// get state from statemanager
state := smgr.GetState(
statemanager.GenerateStateKeyByToolNameAndPluginKind(plugin, id),
statemanager.GenerateStateKeyByToolNameAndInstanceID(plugin, id),
)
if state == nil {
return fmt.Errorf("state with (id: %s, plugin: %s) not found", id, plugin)
Expand Down
9 changes: 2 additions & 7 deletions internal/pkg/statemanager/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (

"gopkg.in/yaml.v3"

"github.com/devstream-io/devstream/internal/pkg/configmanager"
"github.com/devstream-io/devstream/pkg/util/log"
"github.com/devstream-io/devstream/pkg/util/mapz/concurrentmap"
)
Expand Down Expand Up @@ -85,13 +84,9 @@ func (s StatesMap) Format() []byte {
return buf.Bytes()
}

// Note: Please use the StateKeyGenerateFunc function to generate StateKey instance.
// Note: Please use the GenerateStateKeyByToolNameAndInstanceID function to generate StateKey instance.
type StateKey string

func StateKeyGenerateFunc(t *configmanager.Tool) StateKey {
return StateKey(fmt.Sprintf("%s_%s", t.Name, t.InstanceID))
}

func GenerateStateKeyByToolNameAndPluginKind(toolName string, instanceID string) StateKey {
func GenerateStateKeyByToolNameAndInstanceID(toolName string, instanceID string) StateKey {
return StateKey(fmt.Sprintf("%s_%s", toolName, instanceID))
}
4 changes: 2 additions & 2 deletions internal/pkg/statemanager/state_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
)

var _ = Describe("Statemanager.state", func() {
Describe("GenerateStateKeyByToolNameAndPluginKind func", func() {
Describe("GenerateStateKeyByToolNameAndInstanceID func", func() {
It("should ouput state key base on toolName and plugin kind", func() {
var testCases = []struct {
toolName string
Expand All @@ -22,7 +22,7 @@ var _ = Describe("Statemanager.state", func() {
{"123", "1", "123_1"},
}
for _, t := range testCases {
funcResult := GenerateStateKeyByToolNameAndPluginKind(t.toolName, t.plugKind)
funcResult := GenerateStateKeyByToolNameAndInstanceID(t.toolName, t.plugKind)
Expect(funcResult).Should(Equal(t.expectStateKey))
}
})
Expand Down
2 changes: 1 addition & 1 deletion test/state/drifted_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func resourceFromFile() map[string]interface{} {

tool := cfg.Tools[0]

state := smgr.GetState(statemanager.StateKeyGenerateFunc(&tool))
state := smgr.GetState(statemanager.GenerateStateKeyByToolNameAndInstanceID(tool.Name, tool.InstanceID))

return state.Resource
}
Expand Down

0 comments on commit 403a369

Please sign in to comment.