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

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

Open
SwiftedMind opened this issue Oct 30, 2020 · 8 comments

Comments

@SwiftedMind
Copy link

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
Copy link
Contributor

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
Copy link
Author

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
Copy link
Contributor

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: dda1ece#diff-79c4eeb31d7ad1574e0faf3bb00a691f19d3c343c447417d5bfe18031d6b1c5c

@SwiftedMind
Copy link
Author

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
Copy link

mickeyl commented Oct 31, 2020

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
Copy link

groue commented Oct 31, 2020

@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
Copy link

mickeyl commented Nov 2, 2020

@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
Copy link

groue commented Nov 2, 2020

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants