Merge pull request #89 from entireio/fix/buildplans-managed-map-mutation

4b1f4c1→main·

Soph·4w ago·2 files·+47 added/-1 removed

Don't mutate the caller's managed map in BuildPlans

Changes

2

• internal/planner

• Mplanner.go+6/-1

• Mplanner_test.go+41

133 unmodified lines

134
135
136
137
138
139
140
141
142
60 unmodified lines

203
204
205
203
206
207
208
209
210
211
212

133 unmodified lines

) ([]BranchPlan, error) {
    cfg = normalizeAllRefs(cfg)
    if cfg.Prune {
        // addPruneCandidates mutates the map, so copy first — the caller's
        // managed map must not be modified (matches BuildReplicationPlans).
        managed = copyManagedTargets(managed)
        addPruneCandidates(managed, targetRefs, cfg)
    }

60 unmodified lines

cfg PlanConfig,
) ([]BranchPlan, error) {
    cfg = normalizeAllRefs(cfg)
    managed = copyManagedTargets(managed)
    if cfg.Prune {
        // Copy before the only mutation so the caller's managed map is left
        // untouched (matches BuildPlans).
        managed = copyManagedTargets(managed)
        addPruneCandidates(managed, targetRefs, cfg)
    }

Minternal/planner/planner.go+6/-1

75 unmodified lines

76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122

75 unmodified lines

}
}

// BuildPlans must not mutate the caller's managed map: under --prune it adds
// prune candidates, and those additions must land in a copy.
func TestBuildPlansDoesNotMutateManagedMap(t *testing.T) {
    mainRef := plumbing.NewBranchReferenceName("main")
    staleRef := plumbing.NewBranchReferenceName("stale")
hash := plumbing.NewHash("1111111111111111111111111111111111111111")

managed := map[plumbing.ReferenceName]ManagedTarget{
        mainRef: {Kind: RefKindBranch, Label: "main"},
    }
    desired := map[plumbing.ReferenceName]DesiredRef{
        mainRef: {Kind: RefKindBranch, Label: "main", SourceRef: mainRef, TargetRef: mainRef, SourceHash: hash},
    }
    targetRefs := map[plumbing.ReferenceName]plumbing.Hash{
        mainRef:  hash, // same hash -> skip, no ancestry walk needed
        staleRef: plumbing.NewHash("2222222222222222222222222222222222222222"),
    }

plans, err := BuildPlans(memory.NewStorage(), desired, targetRefs, managed, PlanConfig{Prune: true})
    if err != nil {
        t.Fatalf("BuildPlans: %v", err)
    }
    // Sanity: the stale ref was planned for deletion (so prune actually ran).
    var sawDelete bool
    for _, p := range plans {
        if p.TargetRef == staleRef && p.Action == ActionDelete {
            sawDelete = true
        }
    }
    if !sawDelete {
        t.Fatalf("expected a delete plan for the stale ref; plans = %+v", plans)
    }

if len(managed) != 1 {
        t.Fatalf("BuildPlans mutated caller's managed map: len = %d, want 1", len(managed))
    }
    if _, leaked := managed[staleRef]; leaked {
        t.Fatalf("prune candidate %s leaked into the caller's managed map", staleRef)
    }
}

func TestPlanReplicationRefOverwritesDivergence(t *testing.T) {
    target := plumbing.NewHash("1111111111111111111111111111111111111111")
    source := plumbing.NewHash("2222222222222222222222222222222222222222")

Minternal/planner/planner_test.go+41