Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
samdze opened this issue Sep 22, 2022 · 1 comment
Labels
bug Something isn't working

Comments

@samdze
Copy link

samdze commented Sep 22, 2022

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

Note: same issue as vapor/fluent#747
May be better to close it.

@samdze samdze added the bug Something isn't working label Sep 22, 2022
@kevinzhow
Copy link

kevinzhow commented Sep 28, 2022

Same problem
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.
vapor/fluent#747 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants