test(util/component): add unit tests for component deduplication

util/cmd/cmd.go

@@ -120,12 +120,16 @@ func tryRun(requests []func(*component.DepRequests)) error {
 // and does not block until shutdown.
 // Returns as soon as startup completes to allow the caller to orchestrate integration tests.
 func MockStartup(ctx context.Context, requests []func(*component.DepRequests)) component.ApiMap {
+	return MockStartupWithCliArgs(ctx, requests, []string{})
+}
+
+func MockStartupWithCliArgs(ctx context.Context, requests []func(*component.DepRequests), cliArgs []string) component.ApiMap {
 	components := component.ResolveList(requests)
 
 	fs := new(pflag.FlagSet)
 	setupFlags(components, fs)
 
-	if err := fs.Parse([]string{}); err != nil {
+	if err := fs.Parse(cliArgs); err != nil {
 		panic(err)
 	}
 

util/cmd/cmd_test.go

@@ -0,0 +1,173 @@
+// Copyright 2024 The Podseidon Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package cmd_test
+
+import (
+	"context"
+	"flag"
+	"fmt"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"k8s.io/utils/ptr"
+
+	"github.com/kubewharf/podseidon/util/cmd"
+	"github.com/kubewharf/podseidon/util/component"
+	"github.com/kubewharf/podseidon/util/util"
+)
+
+var TestingT = component.Declare(
+	func(_ *testing.T) string { return "testing" },
+	func(_ *testing.T, _ *flag.FlagSet) util.Empty { return util.Empty{} },
+	func(_ *testing.T, _ *component.DepRequests) util.Empty { return util.Empty{} },
+	func(_ context.Context, _ *testing.T, _ util.Empty, _ util.Empty) (*util.Empty, error) {
+		return &util.Empty{}, nil
+	},
+	component.Lifecycle[*testing.T, util.Empty, util.Empty, util.Empty]{
+		Start:        nil,
+		Join:         nil,
+		HealthChecks: nil,
+	},
+	func(d *component.Data[*testing.T, util.Empty, util.Empty, util.Empty]) *testing.T { return d.Args },
+)
+
+var Counter = component.Declare[CounterArgs, CounterOptions, CounterDeps, CounterState, CounterApi](
+	func(args CounterArgs) string { return args.Name },
+	func(args CounterArgs, fs *flag.FlagSet) CounterOptions {
+		return CounterOptions{
+			Multiplier:         fs.Int("multiplier", 10, ""),
+			ExpectMutableField: fs.String("expect", "", ""),
+		}
+	},
+	func(_ CounterArgs, reqs *component.DepRequests) CounterDeps {
+		return CounterDeps{Testing: component.DepPtr(reqs, TestingT(nil))}
+	},
+	func(_ context.Context, args CounterArgs, options CounterOptions, deps CounterDeps) (*CounterState, error) {
+		return &CounterState{}, nil
+	},
+	component.Lifecycle[CounterArgs, CounterOptions, CounterDeps, CounterState]{
+		Start: func(_ context.Context, args *CounterArgs, options *CounterOptions, deps *CounterDeps, state *CounterState) error {
+			assert.Equalf(
+				deps.Testing.Get(),
+				*options.ExpectMutableField,
+				strings.Join(state.mutableField, ","),
+				"start is called strictly after all init",
+			)
+			return nil
+		},
+		Join:         nil,
+		HealthChecks: nil,
+	},
+	func(d *component.Data[CounterArgs, CounterOptions, CounterDeps, CounterState]) CounterApi {
+		return CounterApi{multiplier: *d.Options.Multiplier, state: d.State}
+	},
+)
+
+type CounterArgs struct {
+	Name string
+}
+
+type CounterOptions struct {
+	Multiplier         *int
+	ExpectMutableField *string
+}
+
+type CounterDeps struct {
+	Testing component.Dep[*testing.T]
+}
+
+type CounterState struct {
+	mutableField []string
+}
+
+type CounterApi struct {
+	multiplier int
+	state      *CounterState
+}
+
+func (api CounterApi) Update(value int) {
+	api.state.mutableField = append(api.state.mutableField, fmt.Sprint(api.multiplier*value))
+}
+
+// the pointer is intentionally added to trigger panic when it is unused despite expected to be used.
+func Mutator(mutatorArgs MutatorArgs, counterDepName *string, mutatorDepNames *[]string) component.Declared[util.Empty] {
+	return component.Declare(
+		func(args MutatorArgs) string { return args.Name },
+		func(_ MutatorArgs, fs *flag.FlagSet) MutatorOptions {
+			return MutatorOptions{Value: fs.Int("value", 0, "")}
+		},
+		func(_ MutatorArgs, reqs *component.DepRequests) MutatorDeps {
+			counterDep := component.DepPtr(reqs, Counter(CounterArgs{Name: *counterDepName}))
+			for _, mutatorDepName := range *mutatorDepNames {
+				component.DepPtr(reqs, Mutator(MutatorArgs{Name: mutatorDepName}, nil, nil))
+			}
+
+			return MutatorDeps{
+				Counter: counterDep,
+			}
+		},
+		func(_ context.Context, _ MutatorArgs, options MutatorOptions, deps MutatorDeps) (*util.Empty, error) {
+			deps.Counter.Get().Update(*options.Value)
+			return &util.Empty{}, nil
+		},
+		component.Lifecycle[MutatorArgs, MutatorOptions, MutatorDeps, util.Empty]{
+			Start:        nil,
+			Join:         nil,
+			HealthChecks: nil,
+		},
+		func(d *component.Data[MutatorArgs, MutatorOptions, MutatorDeps, util.Empty]) util.Empty {
+			return util.Empty{}
+		},
+	)(mutatorArgs)
+}
+
+type MutatorArgs struct {
+	Name string
+}
+
+type MutatorOptions struct {
+	Value *int
+}
+
+type MutatorDeps struct {
+	Counter component.Dep[CounterApi]
+}
+
+func TestDeduplication(t *testing.T) {
+	cmd.MockStartupWithCliArgs(
+		context.Background(),
+		[]func(*component.DepRequests){
+			component.RequireDep(TestingT(t).SkipFutureMerges()),
+			component.RequireDep(Mutator(MutatorArgs{
+				Name: "mutator1",
+			}, ptr.To("counter1"), ptr.To[[]string](nil))),
+			component.RequireDep(Mutator(MutatorArgs{
+				Name: "mutator2",
+			}, ptr.To("counter1"), ptr.To([]string{"mutator1"}))),
+			component.RequireDep(Mutator(MutatorArgs{
+				Name: "mutator3",
+			}, ptr.To("counter2"), ptr.To([]string{"mutator1"}))),
+		},
+		[]string{
+			"--counter1-multiplier=100",
+			"--counter1-expect=100,200",
+			"--counter2-expect=30",
+			"--mutator1-value=1",
+			"--mutator2-value=2",
+			"--mutator3-value=3",
+		},
+	)
+}

util/component/component.go

@@ -83,8 +83,10 @@ var ErrRecursiveDependencies = errors.TagErrorf(
 //
 // This interface is only useful for lifecycle orchestration and should not be implemented by other packages.
 type Component interface {
+	getName() string
+
 	// Describes this component for dependency resolution.
-	manifest() manifest
+	depRequests() []*depRequest
 
 	canBeMergedInto() bool
 	// Updates the other component upon name duplication.
@@ -106,11 +108,6 @@ type Component interface {
 	RegisterHealthChecks(handler *healthz.Handler, onFail func(name string, err error))
 }
 
-type manifest struct {
-	Name         string
-	Dependencies []*depRequest
-}
-
 // A registry of dependencies requested by components.
 type DepRequests struct {
 	requests []*depRequest
@@ -164,7 +161,7 @@ func DepPtr[Api any](requests *DepRequests, base Declared[Api]) Dep[Api] {
 			if !ok {
 				panic(fmt.Sprintf(
 					"Components of types %T and %T declared the same name %q with incompatible APIs %T and %v",
-					comp, base, comp.manifest().Name, util.Type[Api]().Out(0), reflect.TypeOf(api).Out(0),
+					comp, base, comp.getName(), util.Type[Api]().Out(0), reflect.TypeOf(api).Out(0),
 				))
 			}
 
@@ -258,10 +255,10 @@ func resolveRequest(
 	request *depRequest,
 ) (string, Component, any) {
 	requestComp, requestApi := request.getNew()
-	manifest := requestComp.manifest()
+	name := requestComp.getName()
 
 	// already exists, return previous value
-	if prev, hasPrev := componentMap[manifest.Name]; hasPrev {
+	if prev, hasPrev := componentMap[name]; hasPrev {
 		deps := requestComp.mergeInto(prev.comp)
 		// resolve incremental dependencies
 
@@ -271,25 +268,25 @@ func resolveRequest(
 			prev.deps.Insert(depName)
 		}
 
-		return manifest.Name, prev.comp, prev.apiGetter
+		return name, prev.comp, prev.apiGetter
 	}
 
 	requestDeps := sets.New[string]()
 
 	// new component; resolve dependencies, init and return the instance we got
-	for _, dep := range manifest.Dependencies {
+	for _, dep := range requestComp.depRequests() {
 		depName, depComp, depApi := resolveRequest(componentMap, dep)
 		dep.set(depComp, depApi)
 		requestDeps.Insert(depName)
 	}
 
-	componentMap[manifest.Name] = &componentMapEntry{
+	componentMap[name] = &componentMapEntry{
 		comp:      requestComp,
 		apiGetter: requestApi,
 		deps:      requestDeps,
 	}
 
-	return manifest.Name, requestComp, requestApi
+	return name, requestComp, requestApi
 }
 
 // Accessor to interact with components by name.
@@ -391,18 +388,20 @@ func strictMergeIntoFn[Args any, Options any, Deps any, State any](
 			return []*depRequest{}
 		}
 
-		if other, isValidType := other.(*componentImpl[Args, Options, Deps, State]); isValidType {
-			reqs := DepRequests{requests: nil}
-			onMerge(&other.Args, &other.Deps, &reqs)
-			return reqs.requests
-		} else {
+		otherTyped, isValidType := other.(*componentImpl[Args, Options, Deps, State])
+		if !isValidType {
 			panic(fmt.Sprintf(
 				"cannot merge %q [%v, %v, %v, %v] into incompatible Component type %T",
 				compName,
 				util.Type[Args](), util.Type[Options](), util.Type[Deps](), util.Type[State](),
-				other,
+				otherTyped,
 			))
 		}
+
+		reqs := DepRequests{requests: nil}
+		onMerge(&otherTyped.Args, &otherTyped.Deps, &reqs)
+
+		return reqs.requests
 	}
 }
 
@@ -437,13 +436,14 @@ func Declare[Args any, Options any, Deps any, State any, Api any](
 				Deps:    util.Zero[Deps](),
 				State:   nil,
 			},
-			name:        name,
-			optionsFn:   newOptions,
-			depsFn:      newDeps,
-			init:        init,
-			lifecycle:   lifecycle,
-			mergeIntoFn: strictMergeIntoFn[Args, Options, Deps, State](name, func(*Args, *Deps, *DepRequests) {}),
-			phase:       nil,
+			name:             name,
+			optionsFn:        newOptions,
+			depsFn:           newDeps,
+			init:             init,
+			lifecycle:        lifecycle,
+			mergeIntoFn:      strictMergeIntoFn[Args, Options, Deps, State](name, func(*Args, *Deps, *DepRequests) {}),
+			skipFutureMerges: false,
+			phase:            nil,
 		}
 
 		if start := impl.lifecycle.Start; start != nil {
@@ -484,6 +484,7 @@ func (ctor DeclaredCtor[Args, Deps, Api]) SkipFutureMerges() DeclaredCtor[Args,
 	return func(args Args) Declared[Api] {
 		decl := ctor(args)
 		decl.SkipFutureMerges()
+
 		return decl
 	}
 }
@@ -588,14 +589,15 @@ type Lifecycle[Args any, Options any, Deps any, State any] struct {
 // and returns a non-nil error for unready status.
 type HealthChecks map[string]func() error
 
-func (impl *componentImpl[Args, Options, Deps, State]) manifest() manifest {
+func (impl *componentImpl[Args, Options, Deps, State]) getName() string {
+	return impl.name
+}
+
+func (impl *componentImpl[Args, Options, Deps, State]) depRequests() []*depRequest {
 	deps := DepRequests{requests: []*depRequest{}}
 	impl.Deps = impl.depsFn(impl.Args, &deps)
 
-	return manifest{
-		Name:         impl.name,
-		Dependencies: deps.requests,
-	}
+	return deps.requests
 }
 
 func (impl *componentImpl[Args, Options, Deps, State]) canBeMergedInto() bool {
@@ -673,14 +675,15 @@ type emptyComponent struct {
 	name string
 }
 
-func (comp emptyComponent) manifest() manifest {
-	return manifest{
-		Name:         comp.name,
-		Dependencies: []*depRequest{},
-	}
+func (comp emptyComponent) getName() string {
+	return comp.name
+}
+
+func (emptyComponent) depRequests() []*depRequest {
+	return nil
 }
 
-func (comp emptyComponent) canBeMergedInto() bool {
+func (emptyComponent) canBeMergedInto() bool {
 	// do not merge into empty components since the implementation is exclusively determined by the test case
 	return false
 }