vapor / fluent-kit

Swift ORM (queries, models, and relations) for NoSQL and SQL databases
MIT License
217 stars 116 forks source link

Migration Crash #617

Closed mredig closed 2 weeks ago

mredig commented 1 month ago

Describe the issue

A migration configuration is causing a crash

Vapor version

hummingbird-main branch 56336d7

Operating system and version

macOS 14.5 (23F79)

Swift version

swift-driver version: 1.115 Apple Swift version 6.0 (swiftlang-6.0.0.9.10 clang-1600.0.26.2)

Steps to reproduce

I'm not 100% sure this isn't just me holding it wrong, or that this is even the right project (tho I'm more certain of the latter).

To get the error/crash, I have the following migration:

struct UpdateUserWithLocaleDBModel_5: AsyncMigration {
    func prepare(on database: any Database) async throws {
        try await database.schema(User.schema)
            .field(User.Key.locale, .string)
            .field(User.Key.timezone, .string)
            .update()
    }

    func revert(on database: any Database) async throws {
        try await database.schema(User.schema)
            .deleteField(User.Key.locale)
            .deleteField(User.Key.timezone)
            .update()
    }
}

with the following model:


final class User: Model, PasswordAuthenticatable, @unchecked Sendable {
    static let schema = "user"

    enum Key {
        static let firstName: FieldKey = "firstName"
        static let lastName: FieldKey = "lastName"
        static let email: FieldKey = "email"
        static let password: FieldKey = "password"
        static let isAdmin: FieldKey = "isAdmin"

        static let locale: FieldKey = "locale"
        static let timezone: FieldKey = "timezone"
    }

    @ID(key: .id)
    var id: UUID?

    @Field(key: Key.email)
    var email: String

    @Field(key: Key.firstName)
    var firstName: String

    @Field(key: Key.lastName)
    var lastName: String

    @Field(key: Key.password)
    var passwordHash: String?

    @Field(key: Key.isAdmin)
    var isAdmin: Bool

    @Children(for: \.$owner)
    var photos: [Photo]

    @Children(for: \.$owner)
    var albums: [Album]

    var primaryAlbum: Album? {
        $albums.value?.first
    }

    @OptionalField(key: Key.locale)
    private var _locale: String?

    var locale: Locale {
        get { _locale.map { Locale(identifier: $0) } ?? .us }
        set { _locale = newValue.identifier }
    }

    @OptionalField(key: Key.timezone)
    private var _timezone: String?

    var timezone: TimeZone {
        get { _timezone.flatMap { TimeZone(identifier: $0) } ?? .chicago }
        set { _timezone = newValue.identifier }
    }

    init() {}

    init(
        id: UUID? = nil,
        email: String,
        firstName: String,
        lastName: String,
        passwordHash: String,
        isAdmin: Bool
    ) {
        self.id = id
        self.email = email
        self.firstName = firstName
        self.lastName = lastName
        self.passwordHash = passwordHash
        self.isAdmin = isAdmin
    }
}

and, for context, here's the initial user creation migration (and the only other user migration):

struct CreateUserDBModel_1: AsyncMigration {
    func prepare(on database: any Database) async throws {
        try await database.schema(User.schema)
            .id()
            .field(User.Key.firstName, .string, .required)
            .field(User.Key.lastName, .string, .required)
            .field(User.Key.email, .string, .required)
            .field(User.Key.password, .string)
            .field(User.Key.isAdmin, .datetime)
            .unique(on: User.Key.email)
            .create()
    }

    func revert(on database: any Database) async throws {
        try await database.schema(User.schema).delete()
    }
}

So, with this setup, I just build and run and the crash happens on startup when running migrations.

Outcome

I get this error and a crash:

2024-10-01T03:19:34-0500 error photoshare : database-id=sqlite error=error: near ",": syntax error migration=photoshare.UpdateUserWithLocaleDBModel_5 [FluentKit] [Migrator] Failed prepare - I'm thinking the bug is in FluentKit because it's explicitly logging an error and saying as such.

Additional notes

M1 - just running debug build in Xcode.

mredig commented 1 month ago

I tested combining the migrations into a single, creation migration and it worked.

mredig commented 1 month ago

This also worked:

struct CreateUserDBModel_1: AsyncMigration {
    func prepare(on database: any Database) async throws {
        try await database.schema(User.schema)
            .id()
            .field(User.Key.firstName, .string, .required)
            .field(User.Key.lastName, .string, .required)
            .field(User.Key.email, .string, .required)
            .field(User.Key.password, .string)
            .field(User.Key.isAdmin, .datetime)
            .unique(on: User.Key.email)
            .create()
    }

    func revert(on database: any Database) async throws {
        try await database.schema(User.schema).delete()
    }
}

struct UpdateUserWithLocaleDBModel_5: AsyncMigration {
    func prepare(on database: any Database) async throws {
        try await database.schema(User.schema)
            .field(User.Key.locale, .string)
            .update()
        try await database.schema(User.schema)
            .field(User.Key.timezone, .string)
            .update()
    }

    func revert(on database: any Database) async throws {
        try await database.schema(User.schema)
            .deleteField(User.Key.locale)
            .deleteField(User.Key.timezone)
            .update()
    }
}

So it looks like it's specifically borked when combining two fields into a single schema builder call. If it wasn't declared earlier, this is with a SQLite db, in case that matters.

0xTim commented 1 month ago

Can you enabled SQL Logging and see what it's generating?

gwynne commented 2 weeks ago

This is not a Fluent issue; it's because SQLite's ALTER TABLE statement is extremely limited and can't change more than one column at a time. Fluent has no way of reporting this, so you just end up with a syntax error when it constructs the syntax that would work with other databases. Your solution here is the correct one for SQLite.