yapstudios / YapDatabase

YapDB is a collection/key/value store with a plugin architecture. It's built atop sqlite, for Swift & objective-c developers.
Other
3.35k stars 365 forks source link

Relationship deletion rule ".notifyIfDestinationDeleted" rule not working (yapDatabaseRelationshipEdgeDeleted is not called upon deletion) #533

Open SwiftedMind opened 4 years ago

SwiftedMind commented 4 years ago

Hey,

I have a problem with the relationships extension right now and it's driving me crazy :D

I have two example models Story and Timeline which look like this:

import Foundation
import YapDatabase

public struct Story: Identifiable, Codable, Hashable, YapDatabaseRelationshipNode { ... }

public struct Timeline: Identifiable, Codable, Hashable, YapDatabaseRelationshipNode {

    enum CodingKeys: String, CodingKey {
        case id = "id"
        case storyId = "storyId"
    }

    static let storyEdgeName = "storyEdge"

    public var id: UUID
    var storyId: UUID?

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

    public func hash(into hasher: inout Hasher) {
        hasher.combine(id)
    }

    public func yapDatabaseRelationshipEdges() -> [YapDatabaseRelationshipEdge]? {
        var edges: [YapDatabaseRelationshipEdge] = []

        if let storyId = storyId {
            let storyDeleteRules: YDB_NodeDeleteRules = [
                .notifyIfDestinationDeleted
            ]

            let storyEdge = YapDatabaseRelationshipEdge(name: Timeline.storyEdgeName, destinationKey: storyId.uuidString, collection: "stories", nodeDeleteRules: storyDeleteRules)

            edges.append(storyEdge)
        }

        return edges
    }

    public func yapDatabaseRelationshipEdgeDeleted(_ edge: YapDatabaseRelationshipEdge, with reason: YDB_NotifyReason) -> Any? {
        print("called")
        return nil
    }
}

I'm pretty much following the tutorial from the wiki. I have a Timeline that belongs to a Story and I want to create an explicit relationship (in this case, an implicit relationship would suffice, I've just chosen the simplest way to demonstrate the problem). And I've confirmed that this edge is actually created correctly.

However, here comes the problem: When I delete a Story instance (that is in a relationship with a timeline object), the yapDatabaseRelationshipEdgeDeleted(_:reason:) method is not called. But it should be called. The destination (the story) is deleted, so the source (the timeline) should be notified as per the delete rules.

I've tried debugging this and I've found the point in the framework where this behavior occurs.

Inside YapDatabaseRelationshipTransaction.m, there are these lines (beginning at line 3799): Link to the line

SEL selector = @selector(yapDatabaseRelationshipEdgeDeleted:withReason:);
if ([srcNode respondsToSelector:selector])
{
    ... // In here, the yapDatabaseRelationshipEdgeDeleted method would have been called
}

The problem is that [srcNode respondsToSelector:selector] returns false (or the obj-c equivalent of that, I don't know :D), so the actual call to notify the source is skipped. I can't figure out why that is. I've printed srcNode and it correctly says it's an instance of Timeline, so it knows the source, it just can't find the method from the selector.

Does anybody know what's going on here? Please let me know if you need more information: I've tried to include everything that's necessary while leaving out unimportant things to make it simpler.

Thanks so much for any kind of help with this!

chrisballinger commented 4 years ago

Swift value types cannot respond to Obj-C selectors. I think there was an attempt by @robbiehanson to fix this bridging issue, but it looks like there are still some rough edges. In the meantime you might want to use an @objc class instead of a struct.

SwiftedMind commented 4 years ago

Thanks for the answer!

Sadly, that would be disastrous for my project. :/ I'm using YapDatabase mainly because I can use structs everywhere, which makes everything so much easier and better. I can't really switch back to classes at this point (also I really wouldn't want to. structs are too awesome).

If this is really the case (although the documentation explicitly says that structs are supported for the YapDatabaseRelationshipNode protocol) I will have to look for alternatives to YapDatabase. That would be a shame because I really really love this framework :/

chrisballinger commented 4 years ago

Yeah I agree, structs are awesome when it comes to modeling data. Although I still really like YapDatabase, I've been reaching for GRDB lately which has some very similar qualities.

You might be able to fix the root of your issue by patching YapDatabase and submitting a pull request, see this commit for some guidance: https://github.com/yapstudios/YapDatabase/commit/dda1ecea8d3292facb9db7d989418875ebc07c70#diff-79c4eeb31d7ad1574e0faf3bb00a691f19d3c343c447417d5bfe18031d6b1c5c

SwiftedMind commented 4 years ago

Thank you very much! I will look into both, though I'm not sure that I'm experienced enough to fix the issue myself :D. I'm definitely going to try, though, might be fun just to experiment a bit with this.

mickeyl commented 4 years ago

Yeah I agree, structs are awesome when it comes to modeling data. Although I still really like YapDatabase, I've been reaching for GRDB lately which has some very similar qualities.

Sorry for being off topic here, but that raises my interest. The one thing I really like with YapDatabase are the Views, ViewMappings, and the differential change notifications which allow us to derive animated table- and collection views so easy. I haven't seen any other database framework that manages this so excellent (apart from CoreData perhaps, but CoreData has lots of other problems). In fact, so many frameworks fail at providing this that Apple felt the need to create their differential data sources (which have not been written with databases in mind… they're killing performance if you have a sufficient amount of entries). How does GRDB handle this?

groue commented 4 years ago

@mickeyl, this is the author of GRDB speaking. I'll try to answer your question. GRDB currently provides absolutely no diffing algorithm, because the current trend is to split data production from data processing. GRDB exposes many ways to fetch database values, and observe changes in database values, but leaves processing such as diffing to external tools, such as UITableViewDiffableDataSource, SwiftUI lists, or whatever diffing algorithm you prefer.

It is not that I disagree with your comment about performance. Indeed I think that performing efficient diffing at the database level could make marvels. Yet the request for such a feature was not expressed yet. And being able to feed all sorts of diffing algorithms is a versatile approach that, if not the most efficient, allows most users to achieve their wide variety of goals. I doubt that one diffing algorithm is able to fulfill all needs.

mickeyl commented 4 years ago

@groue Thanks for your comment. I know it's always tough providing both high abstractions but also allow to use lower-level constructs, if necessary. For now, I'm still satisfied with YapDatabase, although it shows some rough issues with Swift and its value types. Still, it's good to know that there is an alternative. That said, following the stream of Swift team reinventing core frameworks, I expect Apple to come up with a first class database abstraction made for Swift within the next couple of years. Until then we have to use what we have.

groue commented 4 years ago

Exactly! Long life to the large landscape or database apis for Obj-C and Swift developers :-)