Implements various feedback from PR review (WIP)

pkg/construct/properties.go

@@ -27,7 +27,7 @@ func (r *Resource) SetProperty(pathStr string, value any) error {
 }
 
 // GetProperty is a wrapper around [PropertyPath.Get] for convenience.
-// It returns PropertyDoesNotExist if the property does not exist.
+// It returns ErrPropertyDoesNotExist if the property does not exist.
 func (r *Resource) GetProperty(pathStr string) (any, error) {
 	path, err := r.PropertyPath(pathStr)
 	if err != nil {
@@ -40,8 +40,8 @@ func (r *Resource) GetProperty(pathStr string) (any, error) {
 	return nil, nil
 }
 
-// PropertyDoesNotExist is returned when a property does not exist.
-var PropertyDoesNotExist = errors.New("property does not exist")
+// ErrPropertyDoesNotExist is returned when a property does not exist.
+var ErrPropertyDoesNotExist = errors.New("property does not exist")
 
 // AppendProperty is a wrapper around [PropertyPath.Append] for convenience.
 func (r *Resource) AppendProperty(pathStr string, value any) error {
@@ -126,7 +126,7 @@ func (p *Properties) GetProperty(pathStr string) (any, error) {
 	if value, ok := path.Get(); ok {
 		return value, nil
 	}
-	return nil, PropertyDoesNotExist
+	return nil, ErrPropertyDoesNotExist
 }
 
 func (p Properties) AppendProperty(pathStr string, value any) error {

pkg/k2/constructs/construct_evaluator.go

@@ -758,7 +758,7 @@ func (ce *ConstructEvaluator) initializeInputs(c HasInputs, i construct.Properti
 				inputErrors = errors.Join(inputErrors, err)
 				return nil
 			}
-		} else if errors.Is(err, construct.PropertyDoesNotExist) {
+		} else if errors.Is(err, construct.ErrPropertyDoesNotExist) {
 			if dv, err := input.GetDefaultValue(DynamicValueContext{}, nil); err == nil {
 				if dv == nil {
 					dv = input.ZeroValue()

pkg/k2/constructs/import_resources.go

@@ -4,6 +4,8 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"strings"
+
 	"github.com/dominikbraun/graph"
 	"github.com/klothoplatform/klotho/pkg/construct"
 	"github.com/klothoplatform/klotho/pkg/engine/solution"
@@ -12,7 +14,6 @@ import (
 	"github.com/klothoplatform/klotho/pkg/k2/constructs/template/property"
 	"github.com/klothoplatform/klotho/pkg/k2/model"
 	"github.com/klothoplatform/klotho/pkg/logging"
-	"strings"
 )
 
 func (ce *ConstructEvaluator) importFrom(ctx context.Context, o InfraOwner, ic *Construct) error {
@@ -153,12 +154,11 @@ func filterImportProperties(resources map[construct.ResourceId]*construct.Resour
 	return errors.Join(errs...)
 }
 
+// importResourcesFromInputs imports resources from the construct-type inputs of the provided [InfraOwner], o.
+// It returns an error if the input value is does not represent a valid construct
+// or if importing the resources fails.
 func (ce *ConstructEvaluator) importResourcesFromInputs(o InfraOwner, ctx context.Context) error {
 	return o.ForEachInput(func(i property.Property) error {
-		// get the construct from the evaluator if it exists and is the correct type or return an error
-		// then go through the resources of the construct and add them to the imported resources of the current construct
-		// if the resource is not found, return an error
-
 		// if the input is a construct, import the resources from it
 		if !strings.HasPrefix(i.Type(), "construct") {
 			return nil

pkg/k2/constructs/template/construct_template_test.go

@@ -1,12 +1,12 @@
 package template
 
 import (
+	"testing"
+
 	"github.com/klothoplatform/klotho/pkg/k2/constructs/template/properties"
 	"github.com/klothoplatform/klotho/pkg/k2/constructs/template/property"
-	"testing"
 
 	"github.com/klothoplatform/klotho/pkg/construct"
-	"github.com/klothoplatform/klotho/pkg/k2/model"
 	"github.com/stretchr/testify/assert"
 	"gopkg.in/yaml.v3"
 )
@@ -45,7 +45,7 @@ func TestUnmarshalConstructTemplateId(t *testing.T) {
 	}
 }
 
-func TestParseConstructTemplateId(t *testing.T) {
+func TestParseConstructType(t *testing.T) {
 	tests := []struct {
 		name     string
 		input    string
@@ -84,48 +84,6 @@ func TestConstructType_String(t *testing.T) {
 	assert.Equal(t, expected, ctId.String())
 }
 
-func TestConstructType_FromURN(t *testing.T) {
-	tests := []struct {
-		name     string
-		input    string
-		expected property.ConstructType
-		wantErr  bool
-	}{
-		{
-			name:     "Valid URN",
-			input:    "urn:accountid:project:dev::construct/package.name",
-			expected: property.ConstructType{Package: "package", Name: "name"},
-			wantErr:  false,
-		},
-		{
-			name:    "Invalid URN type",
-			input:   "urn:accountid:project:dev::other/package.name",
-			wantErr: true,
-		},
-		{
-			name:    "Invalid URN format",
-			input:   "urn:accountid:project:dev::construct/invalid",
-			wantErr: true,
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			var ctId property.ConstructType
-			urn, err := model.ParseURN(tt.input)
-			if assert.NoError(t, err) {
-				err = ctId.FromURN(*urn)
-				if tt.wantErr {
-					assert.Error(t, err)
-				} else {
-					assert.NoError(t, err)
-					assert.Equal(t, tt.expected, ctId)
-				}
-			}
-		})
-	}
-}
-
 func TestEdgeTemplate_UnmarshalYAML(t *testing.T) {
 	tests := []struct {
 		name     string

pkg/k2/constructs/template/inputs/properties_template.go

@@ -2,13 +2,14 @@ package inputs
 
 import (
 	"fmt"
+	"reflect"
+	"regexp"
+	"strings"
+
 	"github.com/google/uuid"
 	"github.com/klothoplatform/klotho/pkg/k2/constructs/template/properties"
 	"github.com/klothoplatform/klotho/pkg/k2/constructs/template/property"
 	"gopkg.in/yaml.v3"
-	"reflect"
-	"regexp"
-	"strings"
 )
 
 type (
@@ -55,7 +56,8 @@ type (
 		Path string `json:"-" yaml:"-"`
 	}
 
-	PropertyType string
+	PropertyType       string
+	FieldConverterFunc func(val reflect.Value, p *InputTemplate, kp property.Property) error
 )
 
 var (
@@ -291,7 +293,7 @@ func (p *InputTemplate) Clone() *InputTemplate {
 }
 
 // fieldConversion is a map providing functionality on how to convert inputs into our internal types if they are not inherently the same structure
-var fieldConversion = map[string]func(val reflect.Value, p *InputTemplate, kp property.Property) error{
+var fieldConversion = map[string]FieldConverterFunc{
 	"SanitizeTmpl": func(val reflect.Value, p *InputTemplate, kp property.Property) error {
 		sanitizeTmpl, ok := val.Interface().(string)
 		if !ok {

pkg/k2/constructs/template/properties/bool_property.go

@@ -30,7 +30,7 @@ func (b *BoolProperty) AppendProperty(properties construct.Properties, value any
 
 func (b *BoolProperty) RemoveProperty(properties construct.Properties, value any) error {
 	propVal, err := properties.GetProperty(b.Path)
-	if errors.Is(err, construct.PropertyDoesNotExist) {
+	if errors.Is(err, construct.ErrPropertyDoesNotExist) {
 		return nil
 	}
 	if err != nil {

pkg/k2/constructs/template/properties/construct_property.go

@@ -47,7 +47,7 @@ func (r *ConstructProperty) AppendProperty(properties construct.Properties, valu
 
 func (r *ConstructProperty) RemoveProperty(properties construct.Properties, value any) error {
 	propVal, err := properties.GetProperty(r.Path)
-	if errors.Is(err, construct.PropertyDoesNotExist) {
+	if errors.Is(err, construct.ErrPropertyDoesNotExist) {
 		return nil
 	}
 	if err != nil {

pkg/k2/constructs/template/properties/float_property.go

@@ -38,7 +38,7 @@ func (f *FloatProperty) AppendProperty(properties construct.Properties, value an
 
 func (f *FloatProperty) RemoveProperty(properties construct.Properties, value any) error {
 	propVal, err := properties.GetProperty(f.Path)
-	if errors.Is(err, construct.PropertyDoesNotExist) {
+	if errors.Is(err, construct.ErrPropertyDoesNotExist) {
 		return nil
 	}
 	if err != nil {

pkg/k2/constructs/template/properties/int_property.go

@@ -33,7 +33,7 @@ func (i *IntProperty) AppendProperty(properties construct.Properties, value any)
 
 func (i *IntProperty) RemoveProperty(properties construct.Properties, value any) error {
 	propVal, err := properties.GetProperty(i.Path)
-	if errors.Is(err, construct.PropertyDoesNotExist) {
+	if errors.Is(err, construct.ErrPropertyDoesNotExist) {
 		return nil
 	}
 	if err != nil {

pkg/k2/constructs/template/properties/key_value_list.go

@@ -39,7 +39,7 @@ func (kvl *KeyValueListProperty) AppendProperty(properties construct.Properties,
 		return err
 	}
 	propVal, err := properties.GetProperty(kvl.Path)
-	if err != nil && !errors.Is(err, construct.PropertyDoesNotExist) {
+	if err != nil && !errors.Is(err, construct.ErrPropertyDoesNotExist) {
 		return err
 	}
 	if propVal == nil {
@@ -54,7 +54,7 @@ func (kvl *KeyValueListProperty) AppendProperty(properties construct.Properties,
 
 func (kvl *KeyValueListProperty) RemoveProperty(properties construct.Properties, value any) error {
 	propVal, err := properties.GetProperty(kvl.Path)
-	if errors.Is(err, construct.PropertyDoesNotExist) {
+	if errors.Is(err, construct.ErrPropertyDoesNotExist) {
 		return nil
 	}
 	if err != nil {

pkg/k2/constructs/template/properties/list_property.go

@@ -32,7 +32,7 @@ func (l *ListProperty) SetProperty(properties construct.Properties, value any) e
 
 func (l *ListProperty) AppendProperty(properties construct.Properties, value any) error {
 	propVal, err := properties.GetProperty(l.Path)
-	if err != nil && !errors.Is(err, construct.PropertyDoesNotExist) {
+	if err != nil && !errors.Is(err, construct.ErrPropertyDoesNotExist) {
 		return err
 	}
 	if propVal == nil {
@@ -58,7 +58,7 @@ func (l *ListProperty) AppendProperty(properties construct.Properties, value any
 
 func (l *ListProperty) RemoveProperty(properties construct.Properties, value any) error {
 	propVal, err := properties.GetProperty(l.Path)
-	if errors.Is(err, construct.PropertyDoesNotExist) {
+	if errors.Is(err, construct.ErrPropertyDoesNotExist) {
 		return nil
 	}
 	if err != nil {

pkg/k2/constructs/template/properties/map_property.go

@@ -34,7 +34,7 @@ func (m *MapProperty) AppendProperty(properties construct.Properties, value any)
 
 func (m *MapProperty) RemoveProperty(properties construct.Properties, value any) error {
 	propVal, err := properties.GetProperty(m.Path)
-	if errors.Is(err, construct.PropertyDoesNotExist) {
+	if errors.Is(err, construct.ErrPropertyDoesNotExist) {
 		return nil
 	}
 	if err != nil {

pkg/k2/constructs/template/properties/path_property.go

@@ -45,7 +45,7 @@ func (p *PathProperty) AppendProperty(properties construct.Properties, value any
 
 func (p *PathProperty) RemoveProperty(properties construct.Properties, value any) error {
 	propVal, err := properties.GetProperty(p.Path)
-	if errors.Is(err, construct.PropertyDoesNotExist) {
+	if errors.Is(err, construct.ErrPropertyDoesNotExist) {
 		return nil
 	}
 	if err != nil {

pkg/k2/constructs/template/properties/set_property.go

@@ -36,7 +36,7 @@ func (s *SetProperty) SetProperty(properties construct.Properties, value any) er
 
 func (s *SetProperty) AppendProperty(properties construct.Properties, value any) error {
 	propVal, err := properties.GetProperty(s.Path)
-	if err != nil && !errors.Is(err, construct.PropertyDoesNotExist) {
+	if err != nil && !errors.Is(err, construct.ErrPropertyDoesNotExist) {
 		return err
 	}
 	if propVal == nil {
@@ -49,7 +49,7 @@ func (s *SetProperty) AppendProperty(properties construct.Properties, value any)
 
 func (s *SetProperty) RemoveProperty(properties construct.Properties, value any) error {
 	propVal, err := properties.GetProperty(s.Path)
-	if errors.Is(err, construct.PropertyDoesNotExist) {
+	if errors.Is(err, construct.ErrPropertyDoesNotExist) {
 		return nil
 	}
 	if err != nil {

pkg/k2/constructs/template/properties/string_property.go

@@ -35,7 +35,7 @@ func (str *StringProperty) AppendProperty(properties construct.Properties, value
 
 func (str *StringProperty) RemoveProperty(properties construct.Properties, value any) error {
 	propVal, err := properties.GetProperty(str.Path)
-	if errors.Is(err, construct.PropertyDoesNotExist) {
+	if errors.Is(err, construct.ErrPropertyDoesNotExist) {
 		return nil
 	}
 	if err != nil {

pkg/k2/constructs/template/property/construct_type.go

@@ -2,9 +2,11 @@ package property
 
 import (
 	"fmt"
+	"regexp"
+	"strings"
+
 	"github.com/klothoplatform/klotho/pkg/k2/model"
 	"gopkg.in/yaml.v3"
-	"strings"
 )
 
 type ConstructReference struct {
@@ -17,20 +19,22 @@ type ConstructType struct {
 	Name    string `yaml:"name"`
 }
 
-func (c *ConstructType) UnmarshalYAML(value *yaml.Node) error {
-	// Split the value into parts
-	parts := strings.Split(value.Value, ".")
+var constructTypeRegexp = regexp.MustCompile(`^(?:([\w-]+)\.)+([\w-]+)$`)
 
-	// Check if there are at least two parts: package and name
-	if len(parts) < 2 {
-		return fmt.Errorf("invalid construct template id: %s", value.Value)
+func (c *ConstructType) UnmarshalYAML(value *yaml.Node) error {
+	var typeString string
+	err := value.Decode(&typeString)
+	if err != nil {
+		return fmt.Errorf("failed to decode construct type: %w", err)
 	}
 
-	// The name is the last part
-	c.Name = parts[len(parts)-1]
+	if !constructTypeRegexp.MatchString(typeString) {
+		return fmt.Errorf("invalid construct type: %s", typeString)
+	}
 
-	// The package is all the parts except the last one, joined by a dot
-	c.Package = strings.Join(parts[:len(parts)-1], ".")
+	lastDot := strings.LastIndex(typeString, ".")
+	c.Name = typeString[lastDot+1:]
+	c.Package = typeString[:lastDot]
 
 	return nil
 }