Skip to content

Commit

Permalink
refactoring: Use addrs.Map for maps with addresses as keys
Browse files Browse the repository at this point in the history
We introduced the addrs.UniqueKey and addrs.UniqueKeyer mechanics as part
of implementing the ValidateMoves and ApplyMoves functions, as a way to
better encapsulate the solution to the problem that lots of our address
types aren't comparable and so cannot be used directly as map keys.

However, exposing addrs.UniqueKey handling directly in the logic adds
various noise to the algorithms and, in particular, obscures the fact that
MoveResults.Changes and MoveResult.Blocked both have different map key
types.

Here then we'll use the new addrs.Map helper type, which encapsulates the
idea of a map from an addrs.UniqueKeyer type to an arbitrary value type,
using the unique keys as the map keys internally. This does unfortunately
mean that we lose the conventional Go map access syntax and have to use
a method-based API instead, but I (subjectively) think that's an okay
compromise in return for avoiding the need to keep track inline of which
addrs.UniqueKey values correspond with which real addresses.

This is intended as an entirely-mechanical change, with equivalent
behavior to what it replaced. If anything here is doing something
materially different than what it replaced then that's a mistake.
  • Loading branch information
apparentlymart committed Jun 13, 2022
1 parent 2ea5505 commit 03edffd
Show file tree
Hide file tree
Showing 4 changed files with 138 additions and 131 deletions.
35 changes: 18 additions & 17 deletions internal/refactoring/move_execute.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,7 @@ import (
// ApplyMoves expects exclusive access to the given state while it's running.
// Don't read or write any part of the state structure until ApplyMoves returns.
func ApplyMoves(stmts []MoveStatement, state *states.State) MoveResults {
ret := MoveResults{
Changes: make(map[addrs.UniqueKey]MoveSuccess),
Blocked: make(map[addrs.UniqueKey]MoveBlocked),
}
ret := makeMoveResults()

if len(stmts) == 0 {
return ret
Expand Down Expand Up @@ -71,25 +68,23 @@ func ApplyMoves(stmts []MoveStatement, state *states.State) MoveResults {
log.Printf("[TRACE] refactoring.ApplyMoves: Processing 'moved' statements in the configuration\n%s", logging.Indent(g.String()))

recordOldAddr := func(oldAddr, newAddr addrs.AbsResourceInstance) {
oldAddrKey := oldAddr.UniqueKey()
newAddrKey := newAddr.UniqueKey()
if prevMove, exists := ret.Changes[oldAddrKey]; exists {
if prevMove, exists := ret.Changes.GetOk(oldAddr); exists {
// If the old address was _already_ the result of a move then
// we'll replace that entry so that our results summarize a chain
// of moves into a single entry.
delete(ret.Changes, oldAddrKey)
ret.Changes.Remove(oldAddr)
oldAddr = prevMove.From
}
ret.Changes[newAddrKey] = MoveSuccess{
ret.Changes.Put(newAddr, MoveSuccess{
From: oldAddr,
To: newAddr,
}
})
}
recordBlockage := func(newAddr, wantedAddr addrs.AbsMoveable) {
ret.Blocked[newAddr.UniqueKey()] = MoveBlocked{
ret.Blocked.Put(newAddr, MoveBlocked{
Wanted: wantedAddr,
Actual: newAddr,
}
})
}

g.ReverseDepthFirstWalk(startNodes, func(v dag.Vertex, depth int) error {
Expand Down Expand Up @@ -292,7 +287,7 @@ type MoveResults struct {
//
// In the return value from ApplyMoves, all of the keys are guaranteed to
// be unique keys derived from addrs.AbsResourceInstance values.
Changes map[addrs.UniqueKey]MoveSuccess
Changes addrs.Map[addrs.AbsResourceInstance, MoveSuccess]

// Blocked is a map from the unique keys of the final new
// resource instances addresses to information about where they "wanted"
Expand All @@ -308,7 +303,14 @@ type MoveResults struct {
//
// In the return value from ApplyMoves, all of the keys are guaranteed to
// be unique keys derived from values of addrs.AbsMoveable types.
Blocked map[addrs.UniqueKey]MoveBlocked
Blocked addrs.Map[addrs.AbsMoveable, MoveBlocked]
}

func makeMoveResults() MoveResults {
return MoveResults{
Changes: addrs.MakeMap[addrs.AbsResourceInstance, MoveSuccess](),
Blocked: addrs.MakeMap[addrs.AbsMoveable, MoveBlocked](),
}
}

type MoveSuccess struct {
Expand All @@ -327,15 +329,14 @@ type MoveBlocked struct {
// If AddrMoved returns true, you can pass the same address to method OldAddr
// to find its original address prior to moving.
func (rs MoveResults) AddrMoved(newAddr addrs.AbsResourceInstance) bool {
_, ok := rs.Changes[newAddr.UniqueKey()]
return ok
return rs.Changes.Has(newAddr)
}

// OldAddr returns the old address of the given resource instance address, or
// just returns back the same address if the given instance wasn't affected by
// any move statements.
func (rs MoveResults) OldAddr(newAddr addrs.AbsResourceInstance) addrs.AbsResourceInstance {
change, ok := rs.Changes[newAddr.UniqueKey()]
change, ok := rs.Changes.GetOk(newAddr)
if !ok {
return newAddr
}
Expand Down
Loading

0 comments on commit 03edffd

Please sign in to comment.