phase-11-projection-mapper-setter-fix.md 12 KB

Phase 11: Fix ProjectionMapper to Use Setter Lambdas

Problem Statement

The ProjectionMapper.set_property_on_object() method incorrectly uses the friendly_name to set properties via GObject reflection. This is wrong because:

  1. friendly_name is for query expressions - It's used in WHERE/ORDER BY clauses, not for property mapping
  2. Setter lambdas are already provided - Each selection type has a setter delegate that should be used instead

Current Incorrect Implementation

In projection-mapper.vala:

private void map_scalar_selection(
    Object instance,
    SelectionDefinition selection,
    Invercargill.Properties row
) throws ProjectionError {
    string column_name = selection.friendly_name;
    
    var element = row.get(column_name);
    if (element == null) {
        return;
    }
    
    Value? val = extract_value_from_element(element, selection.value_type);
    if (val == null) {
        return;
    }
    
    // INCORRECT: Uses friendly_name to find GObject property via reflection
    set_property_on_object(instance, selection.friendly_name, val);
}

Analysis: Can We Be Fully Generic Like EntityMapper?

How EntityMapper Works

EntityMapper<T> 
  → PropertyMapper<T> 
    → List<PropertyMapping<T, TProp>> 
      → PropertySetter<T, TProp>(T instance, TProp value)

Key insight: T is constant throughout. Only TProp varies. The PropertyMapper<T> is fully generic and iterates over mappings with full type knowledge.

Why Projections Are Different

A single projection has selections with multiple different value types:

.select<int64>("user_id", expr("u.id"), (x, v) => x.user_id = v)      // TValue = int64
.select<string>("user_name", expr("u.name"), (x, v) => x.user_name = v) // TValue = string
.select<double>("total", expr("SUM(o.total)"), (x, v) => x.total = v)  // TValue = double

This means:

  • ScalarSelection<TProjection, int64>
  • ScalarSelection<TProjection, string>
  • ScalarSelection<TProjection, double>

These are different types that cannot be stored in a single typed collection in Vala.

The Fundamental Limitation

Vala doesn't support:

  • Vector<ScalarSelection<TProjection, ?>> (wildcard types)
  • Vector<IScalarSelection<TProjection>> where the interface has generic methods
  • Storing delegates with different type parameters in a single collection

Alternative Designs Considered

Option A: Make ProjectionDefinition Generic

public class ProjectionDefinition<TProjection> : Object {
    public Vector<ScalarSelection<TProjection, ?>> selections; // Doesn't compile
}

Problem: Vala doesn't support wildcard types.

Option B: Visitor/Callback Pattern

// SelectionDefinition
public abstract void apply<T>(
    T instance, 
    Element element,
    owned ApplyDelegate<T> callback
);

public delegate void ApplyDelegate<T>(T instance, TValue value);

Problem: Can't have generic methods in an abstract class that stores implementations with different TValue types.

Option C: Expression-Based Setters (Like PropertyMapper)

Store setters as expressions that are compiled/evaluated:

public class SelectionDefinition {
    public owned SetterExpression setter_expression { get; set; }
}

Problem: Loses type safety, adds complexity, still needs runtime casting.

Option D: Current Design with Runtime Type Check

public abstract class SelectionDefinition : Object {
    public Type projection_type { get; protected set; }
    
    public abstract bool apply_element_value(Object instance, Element element);
}

// In subclass
public override bool apply_element_value(Object instance, Element element) {
    assert(instance.get_type().is_a(projection_type)); // Debug validation
    var typed = (TProjection)instance; // Safe by construction
    // ...
}

This is the recommended approach.

Why Option D Is Acceptable

  1. Type safety by construction: ProjectionBuilder<TProjection> creates selections with matching TProjection
  2. Runtime validation: assert() catches any misuse in debug builds
  3. Same pattern as PropertyMapper: PropertyMapper also does internal casting when iterating mappings
  4. Practical: No language can express "heterogeneous generic collection with type-safe access"

Implementation Details

1. Add Abstract Method to SelectionDefinition

In projection-definition.vala:

public abstract class SelectionDefinition : Object {
    public string friendly_name { get; private set; }
    public Type value_type { get; private set; }
    public virtual Type? nested_projection_type { get { return null; } }
    
    /**
     * The projection type this selection was created for.
     * Used for runtime type validation during materialization.
     */
    public Type projection_type { get; protected set; }
    
    protected SelectionDefinition(string friendly_name, Type value_type) {
        this.friendly_name = friendly_name;
        this.value_type = value_type;
    }
    
    /**
     * Applies a value from an Element to the projection instance using this selection's setter.
     * 
     * @param instance The projection instance. Must be of type projection_type.
     * @param element The Element containing the value from the database
     * @return true if a value was applied, false if the element was null/undefined
     */
    public abstract bool apply_element_value(Object instance, Invercargill.Element element);
}

2. Update ScalarSelection

In selection-types.vala:

public class ScalarSelection<TProjection, TValue> : SelectionDefinition {
    public Expression expression { get; private set; }
    public PropertySetter<TProjection, TValue> setter { get; private set; }
    
    private PropertySetter<TProjection, TValue> _owned_setter;
    
    public ScalarSelection(
        string friendly_name,
        Expression expression,
        owned PropertySetter<TProjection, TValue> setter
    ) {
        base(friendly_name, typeof(TValue));
        this.projection_type = typeof(TProjection);
        this.expression = expression;
        _owned_setter = (owned)setter;
        this.setter = _owned_setter;
    }
    
    public override bool apply_element_value(Object instance, Invercargill.Element element) {
        if (element.is_null() || element.is_undefined()) {
            return false;
        }
        
        // Runtime type validation for debug builds
        assert(instance.get_type().is_a(projection_type));
        
        // Cast is safe - TProjection is guaranteed to match by construction
        var typed_instance = (TProjection)instance;
        
        TValue? val = null;
        if (element.try_get_as<TValue>(out val)) {
            setter(typed_instance, val);
            return true;
        }
        
        return false;
    }
}

3. Update NestedProjectionSelection

public class NestedProjectionSelection<TProjection, TNested> : SelectionDefinition {
    public Expression entry_point_expression { get; private set; }
    public PropertySetter<TProjection, TNested> setter { get; private set; }
    
    public override Type? nested_projection_type { 
        get { return typeof(TNested); } 
    }
    
    private PropertySetter<TProjection, TNested> _owned_setter;
    
    public NestedProjectionSelection(
        string friendly_name,
        Expression entry_point_expression,
        owned PropertySetter<TProjection, TNested> setter
    ) {
        base(friendly_name, typeof(TNested));
        this.projection_type = typeof(TProjection);
        this.entry_point_expression = entry_point_expression;
        _owned_setter = (owned)setter;
        this.setter = _owned_setter;
    }
    
    public override bool apply_element_value(Object instance, Invercargill.Element element) {
        if (element.is_null() || element.is_undefined()) {
            return false;
        }
        
        assert(instance.get_type().is_a(projection_type));
        
        var typed_instance = (TProjection)instance;
        TNested? val = null;
        
        if (element.try_get_as<TNested>(out val)) {
            setter(typed_instance, val);
            return true;
        }
        
        return false;
    }
}

4. Update CollectionProjectionSelection

public class CollectionProjectionSelection<TProjection, TItem> : SelectionDefinition {
    public Expression entry_point_expression { get; private set; }
    public PropertySetter<TProjection, Invercargill.Enumerable<TItem>> setter { get; private set; }
    
    public override Type? nested_projection_type { 
        get { return typeof(TItem); } 
    }
    
    private PropertySetter<TProjection, Invercargill.Enumerable<TItem>> _owned_setter;
    
    public CollectionProjectionSelection(
        string friendly_name,
        Expression entry_point_expression,
        owned PropertySetter<TProjection, Invercargill.Enumerable<TItem>> setter
    ) {
        base(friendly_name, typeof(Invercargill.Enumerable<TItem>));
        this.projection_type = typeof(TProjection);
        this.entry_point_expression = entry_point_expression;
        _owned_setter = (owned)setter;
        this.setter = _owned_setter;
    }
    
    public override bool apply_element_value(Object instance, Invercargill.Element element) {
        if (element.is_null() || element.is_undefined()) {
            return false;
        }
        
        assert(instance.get_type().is_a(projection_type));
        
        var typed_instance = (TProjection)instance;
        Invercargill.Enumerable<TItem>? val = null;
        
        if (element.try_get_as<Invercargill.Enumerable<TItem>>(out val)) {
            setter(typed_instance, val);
            return true;
        }
        
        return false;
    }
}

5. Update ProjectionMapper

In projection-mapper.vala:

private void map_scalar_selection(
    Object instance,
    SelectionDefinition selection,
    Invercargill.Properties row
) throws ProjectionError {
    
    string column_name = selection.friendly_name;
    
    var element = row.get(column_name);
    if (element == null) {
        return;
    }
    
    // Delegates to the selection which uses its type-safe setter
    selection.apply_element_value(instance, element);
}

6. Remove Unused Methods

Remove from ProjectionMapper (~200 lines):

  • extract_value_from_element()
  • convert_value()
  • set_property_on_object()
  • friendly_name_to_property_name_for_type()
  • snake_to_camel()

Implementation Checklist

  • Add projection_type property and apply_element_value() to SelectionDefinition
  • Update ScalarSelection constructor and implement apply_element_value()
  • Update NestedProjectionSelection similarly
  • Update CollectionProjectionSelection similarly
  • Update ProjectionMapper.map_scalar_selection() to use apply_element_value()
  • Remove 5 unused methods from ProjectionMapper
  • Run tests to verify no regression
  • Add test verifying setter receives correct values

Files to Modify

File Changes
src/orm/projections/projection-definition.vala Add projection_type and apply_element_value()
src/orm/projections/selection-types.vala Implement in all 3 selection classes
src/orm/projections/projection-mapper.vala Update map_scalar_selection(), remove ~200 lines
src/tests/projection-test.vala Add value verification test

Summary

The Object cast is unavoidable due to Vala's type system limitations with heterogeneous generic collections. However:

  1. Type safety is guaranteed by construction - selections are created with matching types
  2. Runtime validation - assert() catches misuse in debug builds
  3. Same pattern as PropertyMapper - internal casting is also used there
  4. Significant improvement - replaces ~200 lines of reflection code with direct setter calls

This is the most type-safe design possible within Vala's constraints.