Merge pull request #14 from pay-theory/fix/relaxed-marshalling

Fix: Implement relaxed marshalling and merge legacy snake_case support

Author: paytheory

CHANGELOG.md

@@ -7,9 +7,26 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+## [1.0.34] - 2025-10-29
+
+### Fixed
+- **[CRITICAL]** Custom converters now properly invoked during `Update()` operations
+  - Security validation was rejecting custom struct types before converter check
+  - Fixed by checking for custom converters BEFORE security validation
+  - Custom types with registered converters now bypass security validation (converters handle their own validation)
+  - Removed silent NULL fallbacks - validation/conversion failures now panic with clear error messages
+- Field name validation in `Update()` - unknown field names now return clear error messages instead of silently skipping
+
 ## [1.0.33] - 2025-10-28
 
 ### Added
+- Support for legacy snake_case naming convention alongside default camelCase:
+  - New `naming:snake_case` struct tag to opt-in to snake_case attribute names
+  - Automatic conversion of Go field names to snake_case (e.g., `FirstName` → `first_name`)
+  - Smart acronym handling in snake_case conversion (e.g., `UserID` → `user_id`, `URLValue` → `url_value`)
+  - Per-model naming convention detection and validation
+  - Both naming conventions can coexist in the same application
+  - Integration tests demonstrating mixed convention usage
 - `OrCondition` method to UpdateBuilder for OR logic in conditional expressions:
   - Enables complex business rules like rate limiting with privilege checks
   - Supports mixing AND/OR conditions with left-to-right evaluation

dynamorm.go

@@ -2307,12 +2307,15 @@ func (q *query) updateItem(metadata *model.Metadata, fields []string) error {
 	}
 
 	// Build update expression with custom converter support
+	fmt.Printf("🔍 DYNAMORM UPDATE: q.db=%T, q.db.converter=%p\n", q.db, q.db.converter)
 	builder := expr.NewBuilderWithConverter(q.db.converter)
+	fmt.Printf("🔍 DYNAMORM UPDATE: Builder created with converter=%p\n", builder)
 
 	modelValue := reflect.ValueOf(q.model)
 	if modelValue.Kind() == reflect.Ptr {
 		modelValue = modelValue.Elem()
 	}
+	fmt.Printf("🔍 DYNAMORM UPDATE: Model type=%v\n", modelValue.Type())
 
 	// Determine which fields to update
 	fieldsToUpdate := fields
@@ -2331,13 +2334,18 @@ func (q *query) updateItem(metadata *model.Metadata, fields []string) error {
 	}
 
 	// Build SET expressions
+	fmt.Printf("🔍 DYNAMORM UPDATE: Fields to update: %v\n", fieldsToUpdate)
 	for _, fieldName := range fieldsToUpdate {
 		fieldMeta, exists := lookupField(metadata, fieldName)
 		if !exists {
-			continue
+			// Don't silently skip - return error for unknown fields
+			fmt.Printf("❌ DYNAMORM UPDATE: Field '%s' not found in metadata\n", fieldName)
+			return fmt.Errorf("field '%s' not found in model metadata (use Go field name or DB attribute name)", fieldName)
 		}
 
 		fieldValue := modelValue.FieldByIndex(fieldMeta.IndexPath)
+		fmt.Printf("🔍 DYNAMORM UPDATE: Field '%s' -> DB name '%s', value type=%T\n",
+			fieldName, fieldMeta.DBName, fieldValue.Interface())
 
 		// Handle special fields
 		if fieldMeta.IsUpdatedAt {
@@ -2349,6 +2357,7 @@ func (q *query) updateItem(metadata *model.Metadata, fields []string) error {
 		} else {
 			// Regular field update
 			value := fieldValue.Interface()
+			fmt.Printf("🔍 DYNAMORM UPDATE: Calling AddUpdateSet('%s', %T)\n", fieldMeta.DBName, value)
 			builder.AddUpdateSet(fieldMeta.DBName, value)
 		}
 	}

internal/expr/builder.go

@@ -563,41 +563,39 @@ func (b *Builder) isReservedWord(word string) bool {
 
 // addValueSecure adds an attribute value with security validation
 func (b *Builder) addValueSecure(value any) string {
-	// Security validation
-	if err := validation.ValidateValue(value); err != nil {
-		// SECURITY: Return safe placeholder without logging value details
-		b.valueCounter++
-		placeholder := fmt.Sprintf(":invalid%d", b.valueCounter)
-		b.values[placeholder] = &types.AttributeValueMemberNULL{Value: true}
-		return placeholder
-	}
-
 	b.valueCounter++
 	placeholder := fmt.Sprintf(":v%d", b.valueCounter)
 
 	// Convert value to AttributeValue securely
 	var av types.AttributeValue
 	var err error
 
-	// First, check if we have a custom converter registered for this type
+	// CRITICAL: Check for custom converter FIRST, before security validation
+	// Custom converters handle their own validation and marshaling
 	if b.converter != nil && value != nil {
 		valueType := reflect.TypeOf(value)
 		if b.converter.HasCustomConverter(valueType) {
-			// Use custom converter
+			// Use custom converter - bypass security validation for custom types
 			av, err = b.converter.ToAttributeValue(value)
-			if err == nil {
-				b.values[placeholder] = av
-				return placeholder
+			if err != nil {
+				// Custom converter failed - this is a real error, return NULL
+				av = &types.AttributeValueMemberNULL{Value: true}
 			}
-			// If custom converter fails, fall through to default conversion
+			b.values[placeholder] = av
+			return placeholder
 		}
 	}
 
-	// Fall back to default conversion
+	// Security validation for non-custom types
+	if err := validation.ValidateValue(value); err != nil {
+		// Don't silently convert to NULL - this hides bugs
+		panic(fmt.Sprintf("DynamORM: Invalid value type %T failed security validation: %v", value, err))
+	}
+
+	// Convert using default conversion
 	av, err = ConvertToAttributeValueSecure(value)
 	if err != nil {
-		// SECURITY: Store as NULL for safety without logging details
-		av = &types.AttributeValueMemberNULL{Value: true}
+		panic(fmt.Sprintf("DynamORM: Failed to convert value type %T: %v", value, err))
 	}
 
 	b.values[placeholder] = av

pkg/model/registry.go

@@ -91,16 +91,17 @@ func (r *Registry) GetMetadataByTable(tableName string) (*Metadata, error) {
 
 // Metadata holds all metadata for a model
 type Metadata struct {
-	Type           reflect.Type
-	TableName      string
-	PrimaryKey     *KeySchema
-	Indexes        []IndexSchema
-	Fields         map[string]*FieldMetadata
-	FieldsByDBName map[string]*FieldMetadata
-	VersionField   *FieldMetadata
-	TTLField       *FieldMetadata
-	CreatedAtField *FieldMetadata
-	UpdatedAtField *FieldMetadata
+	Type             reflect.Type
+	TableName        string
+	NamingConvention naming.Convention
+	PrimaryKey       *KeySchema
+	Indexes          []IndexSchema
+	Fields           map[string]*FieldMetadata
+	FieldsByDBName   map[string]*FieldMetadata
+	VersionField     *FieldMetadata
+	TTLField         *FieldMetadata
+	CreatedAtField   *FieldMetadata
+	UpdatedAtField   *FieldMetadata
 }
 
 // KeySchema represents a primary key or index key schema
@@ -188,12 +189,16 @@ func parseMetadata(modelType reflect.Type) (*Metadata, error) {
 		tableName = getTableName(modelType)
 	}
 
+	// Detect naming convention from struct tags
+	convention := detectNamingConvention(modelType)
+
 	metadata := &Metadata{
-		Type:           modelType,
-		TableName:      tableName,
-		Fields:         make(map[string]*FieldMetadata),
-		FieldsByDBName: make(map[string]*FieldMetadata),
-		Indexes:        make([]IndexSchema, 0),
+		Type:             modelType,
+		TableName:        tableName,
+		NamingConvention: convention,
+		Fields:           make(map[string]*FieldMetadata),
+		FieldsByDBName:   make(map[string]*FieldMetadata),
+		Indexes:          make([]IndexSchema, 0),
 	}
 
 	indexMap := make(map[string]*IndexSchema)
@@ -244,7 +249,7 @@ func parseFields(modelType reflect.Type, metadata *Metadata, indexMap map[string
 		}
 
 		// Parse regular field
-		fieldMeta, err := parseFieldMetadata(field, currentPath)
+		fieldMeta, err := parseFieldMetadata(field, currentPath, metadata.NamingConvention)
 		if err != nil {
 			return fmt.Errorf("field validation failed: %w", err)
 		}
@@ -338,11 +343,11 @@ func parseFields(modelType reflect.Type, metadata *Metadata, indexMap map[string
 }
 
 // parseFieldMetadata parses metadata for a single field
-func parseFieldMetadata(field reflect.StructField, indexPath []int) (*FieldMetadata, error) {
+func parseFieldMetadata(field reflect.StructField, indexPath []int, convention naming.Convention) (*FieldMetadata, error) {
 	meta := &FieldMetadata{
 		Name:      field.Name,
 		Type:      field.Type,
-		DBName:    naming.DefaultAttrName(field.Name),
+		DBName:    naming.ConvertAttrName(field.Name, convention),
 		Index:     indexPath[len(indexPath)-1], // Keep for backward compatibility
 		IndexPath: indexPath,
 		Tags:      make(map[string]string),
@@ -443,7 +448,7 @@ func parseFieldMetadata(field reflect.StructField, indexPath []int) (*FieldMetad
 		return nil, err
 	}
 
-	if err := naming.ValidateAttrName(meta.DBName); err != nil {
+	if err := naming.ValidateAttrName(meta.DBName, convention); err != nil {
 		return nil, fmt.Errorf("%w: %v", errors.ErrInvalidTag, err)
 	}
 
@@ -593,6 +598,38 @@ func splitTags(tag string) []string {
 	return parts
 }
 
+// detectNamingConvention scans struct fields for a naming convention tag.
+// It looks for a field (typically blank identifier _) with tag `dynamorm:"naming:snake_case"`.
+// Returns CamelCase (default) if no naming tag is found.
+func detectNamingConvention(modelType reflect.Type) naming.Convention {
+	for i := 0; i < modelType.NumField(); i++ {
+		field := modelType.Field(i)
+		tag := field.Tag.Get("dynamorm")
+
+		if tag == "" {
+			continue
+		}
+
+		// Look for naming:snake_case or naming:camel_case
+		parts := strings.Split(tag, ",")
+		for _, part := range parts {
+			part = strings.TrimSpace(part)
+			if strings.HasPrefix(part, "naming:") {
+				convention := strings.TrimPrefix(part, "naming:")
+				switch convention {
+				case "snake_case":
+					return naming.SnakeCase
+				case "camel_case", "camelCase":
+					return naming.CamelCase
+				}
+			}
+		}
+	}
+
+	// Default to CamelCase
+	return naming.CamelCase
+}
+
 // isStandaloneTag checks if the string starts with a standalone tag (not an index modifier)
 func isStandaloneTag(s string) bool {
 	// Check for simple tags