# Phase 3: API Refinement Plan

## Overview

This phase focuses on refining the migrations and ORM APIs to:
1. Establish clear separation of concerns between migrations (schema) and ORM (runtime mapping)
2. Improve the index API with a fluent builder pattern
3. Add database schema introspection for the ORM
4. Ensure consistent naming conventions across builders

## Design Philosophy

### Migrations = Schema, ORM = Runtime Mapping

The key architectural decision is that **migrations own the schema** and **ORM handles runtime mapping only**.

```mermaid
flowchart TB
    subgraph Development Time
        MIG[Migration Files] --> |define|DDL[Schema DDL]
    end
    
    subgraph Runtime
        DDL --> |apply|DB[(Database)]
        DB --> |introspect|ORM[Entity Mapper]
        ORM --> |CRUD operations|DB
    end
```

**Benefits:**
- Single source of truth: the database schema
- No duplication of constraints between layers
- Applications can interface with existing databases without managing migrations
- Clean separation allows each layer to evolve independently

---

## Part 1: ORM ColumnBuilder Simplification

### Current State

The ORM [`ColumnBuilder<T>`](../src/orm/column-builder.vala) has methods that duplicate schema concerns:

```vala
// Current - has schema methods that belong in migrations
public class ColumnBuilder<T> : Object {
    public ColumnBuilder<T> primary_key();
    public ColumnBuilder<T> required();      // Schema concern
    public ColumnBuilder<T> unique();        // Schema concern
    public ColumnBuilder<T> index();         // Schema concern
    public ColumnBuilder<T> auto_increment();
    public ColumnBuilder<T> default_value<TValue>(TValue value);
    public ColumnBuilder<T> default_now();
}
```

### Proposed Changes

Remove ALL schema-related methods from ORM `ColumnBuilder<T>` since schema metadata will be discovered via database introspection:

| Method | Action | Rationale |
|--------|--------|-----------|
| `primary_key()` | **REMOVE** | Discovered via schema introspection |
| `auto_increment()` | **REMOVE** | Discovered via schema introspection |
| `required()` | **REMOVE** | NOT NULL is schema, discovered via introspection |
| `unique()` | **REMOVE** | UNIQUE constraint is schema |
| `index()` | **REMOVE** | Index creation is schema |
| `default_value()` | **REMOVE** | Defaults handled by database |
| `default_now()` | **REMOVE** | Defaults handled by database |

### Simplified API - Remove ColumnBuilder Entirely

After analysis, we decided to **remove `ColumnBuilder<T>` entirely**. The current design actually breaks method chaining because `ColumnBuilder` doesn't have a `column()` method, requiring explicit `.end()` calls.

**Before (current - requires `.end()`):**
```vala
session.register<User>(b => b
    .table("users")
    .column<int64>("id", u => u.id, (u, v) => u.id = v)
        .primary_key()
        .end()  // Required to return to EntityMapperBuilder!
    .column<string>("name", u => u.name, (u, v) => u.name = v));
```

**After (ColumnBuilder removed - natural chaining):**
```vala
session.register_with_schema<User>("users", b => b
    .column<int64>("id", u => u.id, (u, v) => u.id = v)
    .column<string>("name", u => u.name, (u, v) => u.name = v)
    .column<string>("email", u => u.email, (u, v) => u.email = v));
```

The `column<T>()` method will return `EntityMapperBuilder<T>` directly, enabling natural method chaining without intermediate builders.

### Implementation Tasks

**Remove ColumnBuilder entirely:**
- [ ] Delete [`src/orm/column-builder.vala`](../src/orm/column-builder.vala) file
- [ ] Update [`src/orm/entity-mapper-builder.vala`](../src/orm/entity-mapper-builder.vala) - `column<T>()` now returns `EntityMapperBuilder<T>` instead of `ColumnBuilder<T>`
- [ ] Remove `set_primary_key()` method from [`src/orm/entity-mapper-builder.vala`](../src/orm/entity-mapper-builder.vala)
- [ ] Remove `composite_index()` and `composite_unique()` methods from [`src/orm/entity-mapper-builder.vala`](../src/orm/entity-mapper-builder.vala)
- [ ] Update [`src/meson.build`](../src/meson.build) to remove column-builder.vala

**Simplify ColumnDefinition (remove schema properties):**
- [ ] Remove `is_primary_key` property from [`src/orm/column-definition.vala`](../src/orm/column-definition.vala)
- [ ] Remove `auto_increment` property from [`src/orm/column-definition.vala`](../src/orm/column-definition.vala)
- [ ] Remove `is_required` property from [`src/orm/column-definition.vala`](../src/orm/column-definition.vala)
- [ ] Remove `is_unique` property from [`src/orm/column-definition.vala`](../src/orm/column-definition.vala)
- [ ] Remove `has_index` property from [`src/orm/column-definition.vala`](../src/orm/column-definition.vala)
- [ ] Remove `default_value` property from [`src/orm/column-definition.vala`](../src/orm/column-definition.vala)
- [ ] Remove `default_now` property from [`src/orm/column-definition.vala`](../src/orm/column-definition.vala)
- [ ] `ColumnDefinition` should only contain: `name` and `column_type`

**Update EntityMapper and OrmSession:**
- [ ] Update [`src/orm/entity-mapper.vala`](../src/orm/entity-mapper.vala) to get PK from introspected schema
- [ ] Update [`src/orm/orm-session.vala`](../src/orm/orm-session.vala) insert/update/delete to use introspected schema metadata
- [ ] Add `register_with_schema<T>()` method to [`src/orm/orm-session.vala`](../src/orm/orm-session.vala)

**Update tests:**
- [ ] Update tests in [`src/tests/orm-test.vala`](../src/tests/orm-test.vala) to use new API

---

## Part 2: Database Schema Introspection

### Overview

Add schema introspection capabilities so the ORM can discover column metadata (PK, auto-increment) directly from the database.

### New Components

#### SchemaIntrospector Interface

Add to [`src/dialects/sql-dialect.vala`](../src/dialects/sql-dialect.vala):

```vala
public interface SchemaIntrospector : Object {
    /**
     * Introspects table schema from the database.
     * 
     * @param connection The database connection
     * @param table_name The table to introspect
     * @return A TableSchema containing column metadata
     */
    public abstract TableSchema introspect_schema(Connection connection, string table_name) throws SqlError;
}
```

#### TableSchema Class

Add to [`src/orm/`](../src/orm/):

```vala
public class TableSchema : Object {
    public string table_name { get; set; }
    public Vector<ColumnSchema> columns { get; set; }
    public string? primary_key_column { get; set; }
}

public class ColumnSchema : Object {
    public string name { get; set; }
    public ColumnType column_type { get; set; }
    public bool is_primary_key { get; set; }
    public bool auto_increment { get; set; }
    public bool is_required { get; set; }
}
```

#### SQLite Implementation

Add introspection methods to [`src/dialects/sqlite-dialect.vala`](../src/dialects/sqlite-dialect.vala):

```vala
// Query sqlite_master and pragma_table_info for schema metadata
private const string PRAGMA_TABLE_INFO = "PRAGMA table_info(%s)";
private const string PRAGMA_INDEX_LIST = "PRAGMA index_list(%s)";
```

### OrmSession Integration

Add optional schema discovery to [`src/orm/orm-session.vala`](../src/orm/orm-session.vala):

```vala
public class OrmSession : Object {
    /**
     * Registers an entity with automatic schema discovery.
     * 
     * The ORM will query the database to discover primary keys,
     * auto-increment columns, and other metadata.
     * 
     * @param table_name The database table name
     * @param func A builder function that configures only column mappings
     */
    public void register_with_schema<T>(string table_name, owned EntityMappingFunc<T> func) throws SqlError;
}
```

### Usage Example

```vala
// Before (explicit PK/auto-increment)
session.register<User>(b => b
    .table("users")
    .column<int64>("id", u => u.id, (u, v) => u.id = v)
        .primary_key()
        .auto_increment()
    .column<string>("name", u => u.name, (u, v) => u.name = v));

// After (schema introspected from database)
session.register_with_schema<User>("users", b => b
    .column<int64>("id", u => u.id, (u, v) => u.id = v)
    .column<string>("name", u => u.name, (u, v) => u.name = v));
```

### Implementation Tasks

- [ ] Create `TableSchema` and `ColumnSchema` classes in [`src/orm/table-schema.vala`](../src/orm/table-schema.vala)
- [ ] Add `SchemaIntrospector` interface to [`src/dialects/sql-dialect.vala`](../src/dialects/sql-dialect.vala)
- [ ] Implement SQLite introspection in [`src/dialects/sqlite-dialect.vala`](../src/dialects/sqlite-dialect.vala)
- [ ] Add `register_with_schema<T>()` method to [`src/orm/orm-session.vala`](../src/orm/orm-session.vala)
- [ ] Update [`src/meson.build`](../src/meson.build) to include new files
- [ ] Add introspection tests

---

## Part 3: Index API Refactoring

### Current State

Indexes are created via `MigrationBuilder`:

```vala
// Current - index methods on MigrationBuilder
b.create_table("users", t => {
    t.column<string>("email");
});
b.create_index("idx_users_email", "users", {"email"}, false);  // Clunky
b.drop_index("idx_users_email", "users");
```

### Proposed Changes

1. **Remove** `create_index()` and `drop_index()` from `MigrationBuilder`
2. **Add** fluent index builder to `TableBuilder` (noun-based: `index()`)
3. **Add** `create_index()` and `drop_index()` to `AlterTableBuilder` (verb-based)

### New Index Builder

Create [`src/migrations/index-builder.vala`](../src/migrations/index-builder.vala):

```vala
/**
 * Fluent builder for creating indexes.
 * 
 * Used within TableBuilder and AlterTableBuilder.
 */
public class IndexBuilder : Object {
    private string _index_name;
    private Vector<string> _columns;
    private bool _is_unique;
    private TableBuilder? _table_parent;
    private AlterTableBuilder? _alter_parent;
    
    /**
     * Adds a single column to the index.
     */
    public IndexBuilder on_column(string column_name);
    
    /**
     * Adds multiple columns to the index using varargs.
     */
    public IndexBuilder on_columns(string first_column, ...);
    
    /**
     * Marks the index as unique.
     */
    public IndexBuilder unique();
    
    /**
     * Returns to the parent TableBuilder.
     */
    public TableBuilder end();
}
```

### TableBuilder Changes

Update [`src/migrations/table-builder.vala`](../src/migrations/table-builder.vala):

```vala
public class TableBuilder : Object {
    // Existing methods...
    
    /**
     * Creates an index on this table.
     * 
     * Use the returned IndexBuilder to specify columns and uniqueness.
     * 
     * Example:
     * {{{
     * t.index("idx_email").on_column("email")
     * t.index("idx_composite").on_columns("email", "name").unique()
     * }}}
     */
    public IndexBuilder index(string name);
}
```

### AlterTableBuilder Changes

Update [`src/migrations/alter-table-builder.vala`](../src/migrations/alter-table-builder.vala):

```vala
public class AlterTableBuilder : Object {
    // Existing methods...
    
    /**
     * Creates an index on this table.
     * 
     * Use the returned IndexBuilder to specify columns and uniqueness.
     */
    public IndexBuilder create_index(string name);
    
    /**
     * Drops an index from this table.
     */
    public AlterTableBuilder drop_index(string name);
}
```

### MigrationBuilder Changes

Update [`src/migrations/migration-builder.vala`](../src/migrations/migration-builder.vala):

```vala
public class MigrationBuilder : Object {
    // REMOVE: create_index() method
    // REMOVE: drop_index() method
    
    // Keep existing methods:
    // - create_table()
    // - drop_table()
    // - alter_table()
    // - execute_sql()
}
```

### Usage Examples

```vala
// Creating a table with indexes
b.create_table("users", t => {
    t.column<int64>("id").primary_key().auto_increment();
    t.column<string>("email").not_null();
    t.column<string>("name").not_null();
    
    // Single column index
    t.index("idx_email").on_column("email");
    
    // Composite unique index
    t.index("idx_email_name").on_columns("email", "name").unique();
});

// Altering a table - adding and dropping indexes
b.alter_table("users", t => {
    t.add_column<string>("phone");
    t.create_index("idx_phone").on_column("phone");
    t.drop_index("idx_old_email");
});

// Standalone index creation (use alter_table even if no other changes)
b.alter_table("users", t => {
    t.create_index("idx_new").on_column("created_at");
});
```

### SQL Generation Order

When `TableBuilder` generates SQL, indexes are emitted after the CREATE TABLE:

```sql
-- Generated SQL order:
CREATE TABLE users (id INTEGER PRIMARY KEY, email TEXT NOT NULL);
CREATE INDEX idx_email ON users (email);
CREATE UNIQUE INDEX idx_email_name ON users (email, name);
```

### Implementation Tasks

- [ ] Create [`src/migrations/index-builder.vala`](../src/migrations/index-builder.vala)
- [ ] Add `index()` method to [`src/migrations/table-builder.vala`](../src/migrations/table-builder.vala)
- [ ] Add `create_index()` and `drop_index()` methods to [`src/migrations/alter-table-builder.vala`](../src/migrations/alter-table-builder.vala)
- [ ] Remove `create_index()` and `drop_index()` from [`src/migrations/migration-builder.vala`](../src/migrations/migration-builder.vala)
- [ ] Update `CreateTableOperation` to include index operations
- [ ] Update [`src/dialects/sql-dialect.vala`](../src/dialects/sql-dialect.vala) if needed
- [ ] Update [`src/meson.build`](../src/meson.build) to include new files
- [ ] Update tests in [`src/tests/migration-test.vala`](../src/tests/migration-test.vala)

---

## Part 4: API Naming Consistency Review

### Naming Convention

| Builder | Context | Convention | Examples |
|---------|---------|------------|----------|
| `TableBuilder` | Creating | Noun-based, no verbs | `column()`, `index()`, `primary_key()`, `unique()`, `foreign_key()` |
| `AlterTableBuilder` | Modifying | Verb-based, explicit action | `add_column()`, `drop_column()`, `rename_column()`, `create_index()`, `drop_index()` |
| `MigrationBuilder` | Top-level | Verb-based | `create_table()`, `drop_table()`, `alter_table()` |

### Current API Audit

**TableBuilder** (create context) - ✓ Consistent:
- `column<T>()` ✓
- `primary_key()` ✓
- `unique()` ✓
- `foreign_key()` ✓

**AlterTableBuilder** (alter context) - ✓ Consistent:
- `add_column<T>()` ✓
- `drop_column()` ✓
- `rename_column()` ✓

**MigrationBuilder** - ✓ Consistent:
- `create_table()` ✓
- `drop_table()` ✓
- `alter_table()` ✓
- `execute_sql()` ✓

---

## Summary of Changes

### Files to Modify

| File | Changes |
|------|---------|
| [`src/orm/column-definition.vala`](../src/orm/column-definition.vala) | Remove ALL schema properties: `is_primary_key`, `auto_increment`, `is_required`, `is_unique`, `has_index`, `default_value`, `default_now` |
| [`src/orm/entity-mapper-builder.vala`](../src/orm/entity-mapper-builder.vala) | `column<T>()` returns `EntityMapperBuilder<T>` instead of `ColumnBuilder<T>`; remove `set_primary_key()`, `composite_index()`, `composite_unique()` methods |
| [`src/orm/entity-mapper.vala`](../src/orm/entity-mapper.vala) | Update to receive PK from introspected schema |
| [`src/orm/orm-session.vala`](../src/orm/orm-session.vala) | Add `register_with_schema<T>()`, update insert/update/delete to use introspected metadata |
| [`src/migrations/table-builder.vala`](../src/migrations/table-builder.vala) | Add `index()` method |
| [`src/migrations/alter-table-builder.vala`](../src/migrations/alter-table-builder.vala) | Add `create_index()`, `drop_index()` methods |
| [`src/migrations/migration-builder.vala`](../src/migrations/migration-builder.vala) | Remove `create_index()`, `drop_index()` methods |
| [`src/dialects/sql-dialect.vala`](../src/dialects/sql-dialect.vala) | Add `SchemaIntrospector` interface |
| [`src/dialects/sqlite-dialect.vala`](../src/dialects/sqlite-dialect.vala) | Implement schema introspection |
| [`src/meson.build`](../src/meson.build) | Add new files, remove column-builder.vala |

### Files to Create

| File | Purpose |
|------|---------|
| [`src/orm/table-schema.vala`](../src/orm/table-schema.vala) | `TableSchema` and `ColumnSchema` classes |
| [`src/migrations/index-builder.vala`](../src/migrations/index-builder.vala) | Fluent index builder |

### Files to Update (Tests)

| File | Changes |
|------|---------|
| [`src/tests/orm-test.vala`](../src/tests/orm-test.vala) | Update for simplified ColumnBuilder |
| [`src/tests/migration-test.vala`](../src/tests/migration-test.vala) | Update for new index API |

---

## Migration Guide

### For ORM Users

**Before:**
```vala
session.register<User>(b => b
    .table("users")
    .column<int64>("id", u => u.id, (u, v) => u.id = v)
        .primary_key()
        .auto_increment()
    .column<string>("email", u => u.email, (u, v) => u.email = v)
        .required()
        .unique()
        .index()
    .column<string>("name", u => u.name, (u, v) => u.name = v)
        .required()
    .composite_index("idx_name_email", {"name", "email"}));
```

**After:**
```vala
// All schema metadata (PK, auto-increment, etc.) is discovered from the database
session.register_with_schema<User>("users", b => b
    .column<int64>("id", u => u.id, (u, v) => u.id = v)
    .column<string>("email", u => u.email, (u, v) => u.email = v)
    .column<string>("name", u => u.name, (u, v) => u.name = v));
```

**Note:** The `register_with_schema<T>()` method requires the table to exist in the database before registration. This is the recommended approach as it ensures the ORM mapping is always in sync with the actual database schema.

### For Migration Users

**Before:**
```vala
public override void up(MigrationBuilder b) {
    b.create_table("users", t => {
        t.column<int64>("id").primary_key().auto_increment();
        t.column<string>("email").not_null();
    });
    b.create_index("idx_users_email", "users", {"email"}, false);
}

public override void down(MigrationBuilder b) {
    b.drop_index("idx_users_email", "users");
    b.drop_table("users");
}
```

**After:**
```vala
public override void up(MigrationBuilder b) {
    b.create_table("users", t => {
        t.column<int64>("id").primary_key().auto_increment();
        t.column<string>("email").not_null();
        t.index("idx_users_email").on_column("email");
    });
}

public override void down(MigrationBuilder b) {
    b.drop_table("users");  // Indexes dropped automatically with table
}
```

---

## Open Questions

None at this time. All design decisions have been resolved through discussion.