From 3cf14b6a00499b981dc55c2d4e56041d9a5a7ce5 Mon Sep 17 00:00:00 2001
From: Jared O'Connell <joconnel@redhat.com>
Date: Tue, 16 Apr 2024 15:24:23 -0400
Subject: [PATCH] Addressed review comments

---
 workflow/executor.go           |  8 +++----
 workflow/executor_unit_test.go | 38 ++++++++++++++++++++++++++++++----
 2 files changed, 38 insertions(+), 8 deletions(-)

diff --git a/workflow/executor.go b/workflow/executor.go
index 0ee0f55a..d41aee39 100644
--- a/workflow/executor.go
+++ b/workflow/executor.go
@@ -322,10 +322,10 @@ func applyLifecycleScopes(
 		for _, stage := range stepLifecycle.Stages {
 			prefix := "$.steps." + workflowStepID + "." + stage.ID + "."
 			// Apply inputs
-			// Example with stage starting: $.steps.wait_step.starting.inputs.
+			// Example with stage "starting": $.steps.wait_step.starting.inputs.
 			addInputNamespacedScopes(allNamespaces, stage, prefix+"inputs.")
 			// Apply outputs
-			// Example with stage outputs: $.steps.wait_step.outputs.outputs.success
+			// Example with stage "outputs": $.steps.wait_step.outputs.outputs.
 			addOutputNamespacedScopes(allNamespaces, stage, prefix+"outputs.")
 		}
 	}
@@ -387,8 +387,8 @@ func addScopesWithReferences(allNamespaces map[string]schema.Scope, scope schema
 	for propertyID, property := range rootObject.Properties() {
 		if property.Type().TypeID() == schema.TypeIDRef {
 			refProperty := property.Type().(schema.Ref)
-			if refProperty.Namespace() != schema.DEFAULT_NAMESPACE && refProperty.ObjectReady() {
-				// Found a resolved reference with an object that is not included in the scope. Add it to the map.
+			if refProperty.Namespace() != schema.DEFAULT_NAMESPACE {
+				// Found a reference to an object that is not included in the scope. Add it to the map.
 				var referencedObject any = refProperty.GetObject()
 				refObjectSchema := referencedObject.(*schema.ObjectSchema)
 				allNamespaces[prefix+"."+propertyID] = schema.NewScopeSchema(refObjectSchema)
diff --git a/workflow/executor_unit_test.go b/workflow/executor_unit_test.go
index 8ac3965a..cc533c9e 100644
--- a/workflow/executor_unit_test.go
+++ b/workflow/executor_unit_test.go
@@ -85,6 +85,36 @@ func TestAddInputNamespacedScopes(t *testing.T) {
 	}
 }
 
+func TestAddScopesWithMissingCache(t *testing.T) {
+	allNamespaces := make(map[string]schema.Scope)
+	externalRef3 := schema.NewNamespacedRefSchema("scopeTestObjectC", "not-applied-namespace", nil)
+	notAppliedExternalRefProperty := schema.NewPropertySchema(
+		externalRef3,
+		nil,
+		true,
+		nil,
+		nil,
+		nil,
+		nil,
+		nil,
+	)
+	testScope := schema.NewScopeSchema(
+		schema.NewObjectSchema(
+			"scopeTestObjectA",
+			map[string]*schema.PropertySchema{
+				"notAppliedExternalRef": notAppliedExternalRefProperty,
+			},
+		),
+	)
+	assert.PanicsContains(
+		t,
+		func() {
+			addScopesWithReferences(allNamespaces, testScope, "$")
+		},
+		"scope with namespace \"not-applied-namespace\" was not applied successfully",
+	)
+}
+
 func TestAddScopesWithReferences(t *testing.T) {
 	// Test that the scope itself and the resolved references are added.
 	allNamespaces := make(map[string]schema.Scope)
@@ -121,9 +151,9 @@ func TestAddScopesWithReferences(t *testing.T) {
 		nil,
 		nil,
 	)
-	externalRef3 := schema.NewNamespacedRefSchema("scopeTestObjectC", "test-namespace-3", nil)
-	notAppliedExternalRefProperty := schema.NewPropertySchema(
-		externalRef3,
+	// This one shouldn't add a namespace.
+	nonRefProperty := schema.NewPropertySchema(
+		schema.NewStringSchema(nil, nil, nil),
 		nil,
 		true,
 		nil,
@@ -139,7 +169,7 @@ func TestAddScopesWithReferences(t *testing.T) {
 				"internalRef":                  internalRefProperty,
 				"appliedExternalRef":           appliedExternalRefProperty,
 				"rootPrefixAppliedExternalRef": rootPrefixAppliedExternalRefProperty,
-				"notAppliedExternalRef":        notAppliedExternalRefProperty,
+				"nonRefProperty":               nonRefProperty,
 			},
 		),
 	)