vapor / fluent

Vapor ORM (queries, models, and relations) for NoSQL and SQL databases
https://docs.vapor.codes/4.0/fluent/overview/
MIT License
1.3k stars 171 forks source link

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

Open samdze opened 1 year ago

samdze commented 1 year ago

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:
```swift
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)
    }
}
  1. Add the lifecycle middleware to the database middlewares (SQLite example):
    app.databases.middleware.use(TodoTagLifecycle(), on: .sqlite)
  2. 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

<!-- We must know your exact environment or it is very difficult to help. -->
<!-- Hint: use `vapor --version` in the root directory of your Vapor project. -->

* Vapor Framework version: 4.65.2
* Vapor Toolbox version: 18.5.1
* OS version: macOS 12.6
kevinzhow commented 1 year ago

same problem

AFutureD commented 1 year ago

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)
    }