review feedback

internal/runtime/internal/controller/loader.go

@@ -268,6 +268,8 @@ func (l *Loader) Apply(options ApplyOptions) diag.Diagnostics {
 			Severity: diag.SeverityLevelError,
 			Message:  fmt.Sprintf("Failed to update the internal cache with the new list of components: %s", err),
 		})
+		// return it now because there is no point to process further
+		return diags
 	}
 	l.blocks = options.ComponentBlocks
 	if l.globals.OnExportsChange != nil && l.cache.ExportChangeIndex() != l.moduleExportIndex {
@@ -623,7 +625,7 @@ func (l *Loader) wireGraphEdges(g *dag.Graph) diag.Diagnostics {
 
 		// Finally, wire component references.
 		l.cache.mut.RLock()
-		refs, nodeDiags := ComponentReferences(n, g, l.log, l.cache.GetScope(), l.globals.MinStability)
+		refs, nodeDiags := ComponentReferences(n, g, l.log, l.cache.GetContext(), l.globals.MinStability)
 		l.cache.mut.RUnlock()
 		for _, ref := range refs {
 			g.AddEdge(dag.Edge{From: n, To: ref.Target})
@@ -653,7 +655,7 @@ func (l *Loader) wireCustomComponentNode(g *dag.Graph, cc *CustomComponentNode)
 // Variables returns the Variables the Loader exposes for other components to
 // reference.
 func (l *Loader) Variables() map[string]interface{} {
-	return l.cache.GetScope().Variables
+	return l.cache.GetContext().Variables
 }
 
 // Components returns the current set of loaded components.
@@ -791,7 +793,7 @@ func (l *Loader) concurrentEvalFn(n dag.Node, spanCtx context.Context, tracer tr
 	var err error
 	switch n := n.(type) {
 	case BlockNode:
-		ectx := l.cache.GetScope()
+		ectx := l.cache.GetContext()
 
 		// RLock before evaluate to prevent Evaluating while the config is being reloaded
 		l.mut.RLock()
@@ -830,13 +832,15 @@ func (l *Loader) concurrentEvalFn(n dag.Node, spanCtx context.Context, tracer tr
 // evaluate constructs the final context for the BlockNode and
 // evaluates it. mut must be held when calling evaluate.
 func (l *Loader) evaluate(logger log.Logger, bn BlockNode) error {
-	ectx := l.cache.GetScope()
+	ectx := l.cache.GetContext()
 	err := bn.Evaluate(ectx)
 	return l.postEvaluate(logger, bn, err)
 }
 
 // postEvaluate is called after a node has been evaluated. It updates the caches and logs any errors.
 // mut must be held when calling postEvaluate.
+// TODO: Refactor this function to avoid using an error as input solely for shadowing purposes.
+// This design choice limits extensibility and makes it harder to modify or add functionality.
 func (l *Loader) postEvaluate(logger log.Logger, bn BlockNode, err error) error {
 	switch c := bn.(type) {
 	case ComponentNode:

internal/runtime/internal/controller/value_cache.go

@@ -12,22 +12,25 @@ import (
 // This special keyword is used to expose the argument values to the custom components.
 const argumentLabel = "argument"
 
-// valueCache caches exports to expose as variables for Alloy expressions.
+// valueCache caches exports and module arguments to expose as variables for Alloy expressions.
+// It also caches module exports to expose them to the parent loader.
 // The exports are stored directly in the scope which is used to evaluate Alloy expressions.
 type valueCache struct {
 	mut                sync.RWMutex
-	componentIds       map[string]ComponentID
-	moduleExports      map[string]any // name -> value for the value of module exports
-	moduleChangedIndex int            // Everytime a change occurs this is incremented
-	scope              *vm.Scope      // scope provides additional context for the nodes in the module
+	componentIds       map[string]ComponentID // NodeID -> ComponentID
+	moduleExports      map[string]any         // Export label -> Export value
+	moduleArguments    map[string]any         // Argument label -> Map with the key "value" that points to the Argument value
+	moduleChangedIndex int                    // Everytime a change occurs this is incremented
+	scope              *vm.Scope              // scope provides additional context for the nodes in the module
 }
 
 // newValueCache creates a new ValueCache.
 func newValueCache() *valueCache {
 	return &valueCache{
-		componentIds:  make(map[string]ComponentID, 0),
-		moduleExports: make(map[string]any),
-		scope:         vm.NewScope(make(map[string]any)),
+		componentIds:    make(map[string]ComponentID, 0),
+		moduleExports:   make(map[string]any),
+		moduleArguments: make(map[string]any),
+		scope:           vm.NewScope(make(map[string]any)),
 	}
 }
 
@@ -70,10 +73,7 @@ func (vc *valueCache) CacheExports(id ComponentID, exports component.Exports) er
 func (vc *valueCache) GetModuleArgument(key string) (any, bool) {
 	vc.mut.RLock()
 	defer vc.mut.RUnlock()
-	if _, ok := vc.scope.Variables[argumentLabel]; !ok {
-		return nil, false
-	}
-	v, exist := vc.scope.Variables[argumentLabel].(map[string]any)[key]
+	v, exist := vc.moduleArguments[key]
 	return v, exist
 }
 
@@ -82,12 +82,9 @@ func (vc *valueCache) CacheModuleArgument(key string, value any) {
 	vc.mut.Lock()
 	defer vc.mut.Unlock()
 
-	if _, ok := vc.scope.Variables[argumentLabel]; !ok {
-		vc.scope.Variables[argumentLabel] = make(map[string]any)
-	}
 	keyMap := make(map[string]any)
 	keyMap["value"] = value
-	vc.scope.Variables[argumentLabel].(map[string]any)[key] = keyMap
+	vc.moduleArguments[key] = keyMap
 }
 
 // CacheModuleExportValue saves the value to the map
@@ -151,6 +148,7 @@ func (vc *valueCache) SyncIDs(ids map[string]ComponentID) error {
 
 	// Remove the component exports from the scope.
 	for _, id := range cleanupIds {
+		// Start with the index "0". It refers to the first part of the componentID and it's used for recursion.
 		err := cleanup(vc.scope.Variables, id, 0)
 		if err != nil {
 			return fmt.Errorf("failed to sync component %s: %w", id.String(), err)
@@ -196,26 +194,25 @@ func (vc *valueCache) SyncModuleArgs(args map[string]any) {
 	vc.mut.Lock()
 	defer vc.mut.Unlock()
 
-	if _, ok := vc.scope.Variables[argumentLabel]; !ok {
-		return
-	}
-
-	argsMap := vc.scope.Variables[argumentLabel].(map[string]any)
-	for arg := range argsMap {
+	for arg := range vc.moduleArguments {
 		if _, ok := args[arg]; !ok {
-			delete(argsMap, arg)
+			delete(vc.moduleArguments, arg)
 		}
 	}
-	if len(argsMap) == 0 {
-		delete(vc.scope.Variables, argumentLabel)
-	}
 }
 
-// GetScope returns the current scope.
-func (vc *valueCache) GetScope() *vm.Scope {
+// GetContext returns a scope that can be used for evaluation.
+func (vc *valueCache) GetContext() *vm.Scope {
 	vc.mut.RLock()
 	defer vc.mut.RUnlock()
-	return vm.NewScope(deepCopyMap(vc.scope.Variables))
+	vars := deepCopyMap(vc.scope.Variables)
+
+	// Add module arguments if there are any.
+	if len(vc.moduleArguments) > 0 {
+		vars[argumentLabel] = deepCopyMap(vc.moduleArguments)
+	}
+
+	return vm.NewScope(vars)
 }
 
 func deepCopyMap(original map[string]any) map[string]any {

internal/runtime/internal/controller/value_cache_test.go

@@ -56,7 +56,7 @@ func TestValueCache(t *testing.T) {
 
 	require.NoError(t, vc.CacheExports(ComponentID{"foo"}, fooExports{SomethingElse: true}))
 
-	res := vc.GetScope()
+	res := vc.GetContext()
 
 	var (
 		expectKeys = []string{"foo"}
@@ -149,20 +149,20 @@ func TestModuleArgumentCache(t *testing.T) {
 			vc.CacheModuleArgument("arg", tc.argValue)
 
 			// Build the scope and validate it
-			res := vc.GetScope()
+			res := vc.GetContext()
 			expected := map[string]any{"arg": map[string]any{"value": tc.argValue}}
 			require.Equal(t, expected, res.Variables["argument"])
 
 			// Sync arguments where the arg shouldn't change
 			syncArgs := map[string]any{"arg": tc.argValue}
 			vc.SyncModuleArgs(syncArgs)
-			res = vc.GetScope()
+			res = vc.GetContext()
 			require.Equal(t, expected, res.Variables["argument"])
 
 			// Sync arguments where the arg should clear out
 			syncArgs = map[string]any{}
 			vc.SyncModuleArgs(syncArgs)
-			res = vc.GetScope()
+			res = vc.GetContext()
 			require.Equal(t, map[string]any{}, res.Variables)
 		})
 	}
@@ -178,7 +178,7 @@ func TestScope(t *testing.T) {
 		},
 	)
 	require.NoError(t, vc.CacheExports(ComponentID{"foo", "bar"}, barArgs{Number: 12}))
-	res := vc.GetScope()
+	res := vc.GetContext()
 
 	expected := map[string]any{
 		"test": map[string]any{
@@ -205,7 +205,7 @@ func TestScopeSameNamespace(t *testing.T) {
 		},
 	)
 	require.NoError(t, vc.CacheExports(ComponentID{"test", "bar"}, barArgs{Number: 12}))
-	res := vc.GetScope()
+	res := vc.GetContext()
 
 	expected := map[string]any{
 		"test": map[string]any{
@@ -230,7 +230,7 @@ func TestScopeOverride(t *testing.T) {
 		},
 	)
 	require.NoError(t, vc.CacheExports(ComponentID{"test", "scope"}, barArgs{Number: 12}))
-	res := vc.GetScope()
+	res := vc.GetContext()
 
 	expected := map[string]any{
 		"test": map[string]any{
@@ -269,7 +269,7 @@ func TestScopeComplex(t *testing.T) {
 	)
 	require.NoError(t, vc.CacheExports(ComponentID{"test", "cp1", "foo"}, barArgs{Number: 12}))
 	require.NoError(t, vc.CacheExports(ComponentID{"test", "cp1", "bar", "fizz"}, barArgs{Number: 2}))
-	res := vc.GetScope()
+	res := vc.GetContext()
 
 	expected := map[string]any{
 		"test": map[string]any{
@@ -335,7 +335,7 @@ func TestSyncIds(t *testing.T) {
 			},
 		},
 	}
-	res := vc.GetScope()
+	res := vc.GetContext()
 	require.Equal(t, expected, res.Variables)
 }
 
@@ -354,3 +354,23 @@ func TestSyncIdsError(t *testing.T) {
 		"failed to sync component test.cp1.bar.fizz.foo: expected a map but found a value for \"fizz\"",
 	)
 }
+
+func TestCapsule(t *testing.T) {
+	vc := newValueCache()
+	bar := barArgs{Number: 13}
+	vc.scope = vm.NewScope(
+		map[string]any{
+			"test": map[string]any{
+				"scope": &bar,
+			},
+		},
+	)
+	res := vc.GetContext()
+
+	expected := map[string]any{
+		"test": map[string]any{
+			"scope": &bar,
+		},
+	}
+	require.Equal(t, expected, res.Variables)
+}