Model middleware lifecycle event delete not called on siblings detach and detachAll

[samdze]

Describe the bug

When calling detach or detachAll on a SiblingsProperty, the related pivot model doesn't trigger its delete lifecycle events.
Works fine with create events using attach.

I tried and successfully reproduced the bug on SQLite and PostgreSQL, but it may be present in other drivers too.

To Reproduce

Steps to reproduce the behavior:

1. Create the following models:

final class Todo: Model, Content {
    static let schema = "todo"
    
    @ID(key: .id)
    var id: UUID?

    @Field(key: "title")
    var title: String
    
    @Siblings(through: TodoTagPivot.self, from: \.$todo, to: \.$tag)
    var tags: [Tag]

    init() { }

    init(id: UUID? = nil, title: String) {
        self.id = id
        self.title = title
    }
}

final class Tag: Model, Content {
    static let schema = "tag"
    
    @ID(key: .id)
    var id: UUID?

    @Field(key: "name")
    var name: String
    
    @Siblings(through: TodoTagPivot.self, from: \.$tag, to: \.$todo)
    var todos: [Todo]

    init() { }

    init(id: UUID? = nil, name: String) {
        self.id = id
        self.name = name
    }
}

final class TodoTagPivot: Model, Content {
    static let schema = "todo_tag"
    
    @ID(key: .id)
    var id: UUID?

    @Parent(key: "todo_id")
    var todo: Todo
    
    @Parent(key: "tag_id")
    var tag: Tag

    init() { }

    init(id: UUID? = nil) {
        self.id = id
    }
}

And the following lifecycle middleware:

struct TodoTagLifecycle: AsyncModelMiddleware {
    func softDelete(model: TodoTagPivot, on db: Database, next: AnyAsyncModelResponder) async throws {
        try await model.$todo.load(on: db)
        try await model.$tag.load(on: db)
        print("Soft-deleting the pivot between \(model.todo.title) and \(model.tag.name)")
        try await next.softDelete(model, on: db)
    }
    
    func delete(model: TodoTagPivot, force: Bool, on db: Database, next: AnyAsyncModelResponder) async throws {
        try await model.$todo.load(on: db)
        try await model.$tag.load(on: db)
        print("Deleting the pivot between \(model.todo.title) and \(model.tag.name)")
        try await next.delete(model, force: force, on: db)
    }
    
    func create(model: TodoTagPivot, on db: Database, next: AnyAsyncModelResponder) async throws {
        try await model.$todo.load(on: db)
        try await model.$tag.load(on: db)
        print("Creating the pivot between \(model.todo.title) and \(model.tag.name)")
        try await next.create(model, on: db)
    }
}

2. Add the lifecycle middleware to the database middlewares (SQLite example):

app.databases.middleware.use(TodoTagLifecycle(), on: .sqlite)

3. Send a request to patch a Todo model:

func boot(routes: RoutesBuilder) throws {
    let todos = routes.grouped("todos")
    ...
    todos.group(":todoID") { todo in
        todo.patch(use: update)
        ...
    }
}

extension Todo {
    struct Update: Content {
        let tagIDs: [UUID]
    }
}

func update(req: Request) async throws -> Todo {
    let todoID = try req.parameters.require("todoID", as: UUID.self)
    let dto = try req.content.decode(Todo.Update.self)
    
    guard let todo = try? await Todo.find(todoID, on: req.db) else {
        throw Abort(.notFound)
    }
    let tags = try await Tag.query(on: req.db)
        .filter(\.$id ~~ dto.tagIDs)
        .all()
    try await todo.$tags.detachAll(on: req.db) // TodoTagPivot rows get deleted.
    try await todo.$tags.attach(tags, on: req.db)
    
    try await todo.update(on: req.db)
    return todo
}

4. See how TodoTagPivot create events get emitted, but no delete event is.

Expected behavior

Calling detach or detachAll deletes the corresponding rows in the related pivot table, so lifecycle delete events should be emitted.

Environment

• Vapor Framework version: 4.65.2
• Vapor Toolbox version: 18.5.1
• OS version: macOS 12.6

[kevinzhow]

same problem

[AFutureD]

This problem may be caused by the func SiblingsProperty.detach(_:on:).

    public func detach(_ tos: [To], on database: Database) -> EventLoopFuture<Void> {
        guard let fromID = self.idValue else {...}
        let toIDs = tos.map {...}

        return Through.query(on: database)
            .filter(self.from.appending(path: \.$id) == fromID)
            .filter(self.to.appending(path: \.$id) ~~ toIDs)
            .delete()
    }

The last line which calls .delete() might be the reason why the issue happened.

The .delete() just simply build a sql and runs it, but miss invoking other lifecycle, such as middleware.

To fix this, the simplest solution is changing the .delete() to .delete(on:)

    public func detach_try(_ tos: [To], on database: Database) async throws {
        guard let fromID = self.fromId else {...}
        let toIDs = tos.map {...}
        
        try await Through.query(on: database)
                    .filter(self.from.appending(path: \.$id) == fromID)
                    .filter(self.to.appending(path: \.$id) ~~ toIDs)
                    .all().delete(on: database)
    }