diff --git a/cli/azd/pkg/ioc/container.go b/cli/azd/pkg/ioc/container.go index 3a911b2618a..cb4c16c5842 100644 --- a/cli/azd/pkg/ioc/container.go +++ b/cli/azd/pkg/ioc/container.go @@ -37,9 +37,11 @@ func NewNestedContainer(parent *NestedContainer) *NestedContainer { current := container.New() if parent != nil { - // Copy the bindings to the new container - // The bindings hold the concrete instance of singleton registrations - maps.Copy(current, parent.inner) + for registeredType, bindings := range parent.inner { + // clone bindings as well, otherwise changes in child scopes will + // actually modify the map for the parent! + current[registeredType] = maps.Clone(bindings) + } } instance := &NestedContainer{ diff --git a/cli/azd/pkg/ioc/container_test.go b/cli/azd/pkg/ioc/container_test.go index 4896fda2907..48cc3e2d7e1 100644 --- a/cli/azd/pkg/ioc/container_test.go +++ b/cli/azd/pkg/ioc/container_test.go @@ -165,18 +165,62 @@ func Test_Container_Singleton_Instance_Register_Resolve(t *testing.T) { err = scope1.Resolve(&scope1Instance1) require.NoError(t, err) require.NotNil(t, scope1Instance1) + require.Same(t, scope1Instance, scope1Instance1) var scope2Instance1 *singletonService err = scope2.Resolve(&scope2Instance1) require.NoError(t, err) require.NotNil(t, scope2Instance1) - - // Instance 1 & 2 are singletons but overriden in each child scope so they should be different - require.NotSame(t, rootInstance, rootInstanceResolved) - require.NotSame(t, scope1Instance, scope2Instance) + require.Same(t, scope2Instance, scope2Instance1) }) } +func Test_Container_LayerEnvironmentManagerOverridesAreIsolated(t *testing.T) { + // This is a case I ran into when I was trying to make it so we could inject a new + // environment manager instance for each infra provider. Prior to the fix for this + // we'd accidentally end up mutating the _root_ environment manager instead of just the + // lower level manager. + + rootContainer := NewNestedContainer(nil) + rootEnvManager := &fakeEnvironmentManager{name: "root"} + RegisterInstance[environmentManager](rootContainer, rootEnvManager) + + // now we're going to register an env manager, but each one is in a new + // scope, so the root should be unaffected (there was a bug where this was + // NOT the case) + layer1Scope, err := rootContainer.NewScope() + require.NoError(t, err) + layer1EnvManager := &fakeEnvironmentManager{name: "layer-1"} + RegisterInstance[environmentManager](layer1Scope, layer1EnvManager) // override with our own env manager + + layer2Scope, err := rootContainer.NewScope() + require.NoError(t, err) + layer2EnvManager := &fakeEnvironmentManager{name: "layer-2"} + RegisterInstance[environmentManager](layer2Scope, layer2EnvManager) // override with our own env manager + + // okay, at this point we've created this structure in our IoC, each independent from + // each other: + // + // root container + // |-- root env manager + // |-- layer 1 scope + // | |-- layer 1 env manager + // |-- layer 2 scope + // |-- layer 2 env manager + + var resolvedRoot environmentManager + require.NoError(t, rootContainer.Resolve(&resolvedRoot)) + require.Same(t, rootEnvManager, resolvedRoot) + + var resolvedLayer1 environmentManager + require.NoError(t, layer1Scope.Resolve(&resolvedLayer1)) + require.Same(t, layer1EnvManager, resolvedLayer1) + + var resolvedLayer2 environmentManager + require.NoError(t, layer2Scope.Resolve(&resolvedLayer2)) + require.Same(t, layer2EnvManager, resolvedLayer2) +} + type singletonService struct { timestamp time.Time } @@ -209,6 +253,18 @@ func newTransientService() *transientService { // ---------- helper types for tests ---------- +type environmentManager interface { + Name() string +} + +type fakeEnvironmentManager struct { + name string +} + +func (m *fakeEnvironmentManager) Name() string { + return m.name +} + type greeter interface { Greet() string } @@ -849,22 +905,39 @@ func Test_DependencyChain(t *testing.T) { func Test_NewNestedContainer_InheritsParent(t *testing.T) { t.Parallel() - parent := NewNestedContainer(nil) - parent.MustRegisterSingleton(func() *counterService { - return &counterService{calls: 99} + t.Run("ParentAccessesFirst", func(t *testing.T) { + parent := NewNestedContainer(nil) + parent.MustRegisterSingleton(func() *counterService { + return &counterService{calls: 99} + }) + child := NewNestedContainer(parent) + + var parentInst *counterService + err := parent.Resolve(&parentInst) + require.NoError(t, err) + + var childInst *counterService + err = child.Resolve(&childInst) + require.NoError(t, err) + require.Same(t, parentInst, childInst) }) - // Resolve in parent first to cache the singleton - var parentInst *counterService - err := parent.Resolve(&parentInst) - require.NoError(t, err) + t.Run("ChildAccessesFirst", func(t *testing.T) { + parent := NewNestedContainer(nil) + parent.MustRegisterSingleton(func() *counterService { + return &counterService{calls: 99} + }) + child := NewNestedContainer(parent) - child := NewNestedContainer(parent) - var childInst *counterService - err = child.Resolve(&childInst) - require.NoError(t, err) - // Child inherits parent's cached singleton - require.Same(t, parentInst, childInst) + var childInst *counterService + err := child.Resolve(&childInst) + require.NoError(t, err) + + var parentInst *counterService + err = parent.Resolve(&parentInst) + require.NoError(t, err) + require.Same(t, parentInst, childInst) + }) } // ---------- inspectResolveError ----------