Can't Delete AND Notify

[afriedmanGlacier]

I can't seem to delete an edge (destination or source) and also receive notification in order to do further processing. I ran into this problem while testing ChatSecure (see ChatSecure/ChatSecure-iOS#865)

The wiki page, (https://github.com/yapstudios/YapDatabase/wiki/Relationships#notify), shows that you can have multiple delete rules that allow deleting and notifying

...nodeDeleteRules:YDB_DeleteDestinationIfSourceDeleted | YDB_NotifyIfDestinationDeleted];

Maybe I am misunderstanding the wiki example? Here's the situation using ChatSecure as the example:

OTRBaseMessage in ChatSecure currently has "YDB_DeleteDestinationIfSourceDeleted | YDB_NotifyIfSourceDeleted" for nodeDeleteRules if there is a media item attached to the message. It deletes the media item (in YapDatabaseRelationshipTransaction) without sending notification.

I also tried changing YDB_NotifyIfSourceDeleted to YDB_NotifyIfDestinationDeleted in case this method was called again as a result of the destination being deleted due to the YDB_DeleteDestinationIfSourceDeleted rule, but that didn't work either.

If we change it to just YDB_NotifyIfSourceDeleted, it will send the notification and call yapDatabaseRelationshipEdgeDeleted in OTRMediaItem, but it doesn't actually delete the destination edge item.

So, I'm thinking the best option seems to me like YapDatabaseRelationshipTransaction should check for the YDB_NotifyIfSourceDeleted inside the condition that looks for YDB_DeleteDestinationIfSourceDeleted, which it currently doesnt, and notify before deleting if needed (same with source).

Is that right? Or am I missing something? The flush method is already huge with all the conditions, but being able to both Notify and Delete should be possible and seems like the intent in this case.

[afriedmanGlacier]

I started working towards a pull request that seems logical and would work for ChatSecure purposes, but then I realized it might cause other issues that do not fit with the intentions for the YapDatabase code, and that would be difficult for me to test.

In the flush method of YapDatabaseRelationshipTransaction, inside the YDB_DeleteDestinationIfSourceDeleted condition (and YDB_DeleteSourceIfDestinationDeleted), I was planning to check for the YDB_NotifyIfSourceDeleted condition and notify before deleting in order to allow preprocessing (removing the media item file at its path on disk). However, the NotifyIf conditions allow the listener to modify the object and send it back. That wouldn't do any good here because after the notification, we are deleting the object. If the client code modified it, it would then just get deleted. I'm probably not explaining that well.

I'm still not sure why delete and notify doesn't seem to work the way it says at https://github.com/yapstudios/YapDatabase/wiki/Relationships#notify. But again, maybe I don't understand the intent correctly. Can someone explain this?

[robbiehanson]

The problem is that I didn't foresee this use case. I imagined only the following:

• developer wants src/dst to be deleted
• developer does NOT want src/dst to be deleted, but they do want to update src/dst accordingly

So I would definitely classify this as a bug. The only question is, what's the best way to solve it ?

Can you give me some more context on what you're trying to achieve? I gather that an OTRBaseMessage is being deleted, along with a corresponding media item? And this action (the deletion of the message or media item) needs to, in turn, trigger something else ?

[robbiehanson]

Can you try out those changes I pushed (to the "issue426" branch) and make sure that fixes the problem you're referencing ?

[afriedmanGlacier]

I will try it out, thanks! As for the context (which may be irrelevant since you already looked at it), when ChatSecure creates a mediaItem object, it saves the actual media file to a path on the OS. When the conversation thread is deleted, the mediaItem objects in the YapDatabase are deleted, but the actual image/media files on the file system are not. I created one means to fix this, but had to go through every message prior to deleting the messages in the thread in order to see which ones had mediaItems, and delete those files from disk. @chrisballinger suggested a secondary index or the deletion notify in order to be more efficient. (Feel free to correct my wording and understanding of this Chris). I like the idea of the notification in order to also delete the file at that time.

Might not have the chance to look at it or test this until Monday, but I will let you know as soon as I do.

[chrisballinger]

Ya that's basically it. The complication is that the files are stored within a virtual file system (sqlfs) instead of regular system paths which is why we can't use the built-in support for file deletion.

[robbiehanson]

The complication is that the files are stored within a virtual file system (sqlfs) instead of regular system paths which is why we can't use the built-in support for file deletion.

Ah! That's what I didn't foresee. Thank you (both) for explaining.

[afriedmanGlacier]

Just tested this, and it looks good! Not sure if I am supposed to close this first or wait till it is merged into master, but I think it can be closed.

[robbiehanson]

Merged into master.