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

Refactor: Unify the statekey Getting Method #1180

Merged
merged 1 commit into from
Oct 17, 2022
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
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