feat: improve powersync_core attachments API

[itsezlife]

1. The updateHook is required for saveFile and deleteFile in AttachmentQueue. Why is it required? In my personal use-case, I’ve never used updateHook.
2. The updateHook can confuse people. When I tried to use updateHook in saveFile I needed to UPDATE ${AttachmentsQueueTable.defaultTableName} SET message_id = ? WHERE id = , and it didn’t work, because updateHook is called before it inserted into attachments queue table, therefore I couldn’t execute update statement. It can be solved by introducing preUpdateHook and postUpdateHook, or something like that. Or we could have a single updateHook, but call it after the *await* attachmentContext.upsertAttachment(attachment, tx);, in the saveFile specifically.
3. When calling deleteFile , first it checks if attachment exists in the queue. If it doesn’t, attachment not found exception is thrown. It makes sense, but in my specific use-case I can call deleteFile mostly to delete attachment from the remote storage, sometimes it may not be inside the attachments queue, therefore it won’t be deleted. I have to call both deleteFile from AttachmentsQueue and RemoteStorage(which deletes from Supabase storage) in order to ensure that even when sometimes attachment may not exist in the queue, the attachment is 100% deleted from the storage. I’m just sharing about my experience and not sure whether anything needs to be touched here.

/// Resolves the filename for new attachment items.
  /// Concatenates the attachment ID and extension by default.
  Future<String> resolveNewAttachmentFilename(
    String attachmentId,
    String? fileExtension,
  ) async {
    return '$attachmentId.${fileExtension ?? 'dat'}';
  }

Why you always append extension in the resolveNewAttachmentFilename? Why just not append any extension if user has specifically not passed any extension(when fileExtension is null). I found it really frustrating. In old powersync_attachments_helper I could override those methods, but now AttachmentQueue is not extendable.

[stevensJourney]

The updateHook is required for saveFile and deleteFile in AttachmentQueue. Why is it required? In my personal use-case, I’ve never used updateHook.

The attachment helper logic will archive any attachment record which is not referenced by the watchAttachments stream. We expose the updateHook in order for consumers to assign the relationship in the same transaction that the attachment record is created. This is a extra failsafe to avoid race conditions where the attachment record might exist without a relationship being reported by watchAttachments. We do have extra logic to restore an archived pending upload attachment - when it is eventually resolved by watchAttachments, but generally: it's quicker and more deterministic to assign the relationship when creating the attachment.

The updateHook can confuse people. When I tried to use updateHook in saveFile I needed to UPDATE ${AttachmentsQueueTable.defaultTableName} SET message_id = ? WHERE id = , and it didn’t work, because updateHook is called before it inserted into attachments queue table, therefore I couldn’t execute update statement. It can be solved by introducing preUpdateHook and postUpdateHook, or something like that. Or we could have a single updateHook, but call it after the await attachmentContext.upsertAttachment(attachment, tx);, in the saveFile specifically.

I'm assuming you're using Raw Tables with foreign key constraints? Our standard Views don't support such constraints, so this might have been overlooked. Moving the updateHook to after the creation of the attachment record should be fine and should avoid this issue. I'm not against exposing a preUpdateHook and postUpdateHook - although that might make things more confusing to users.

Why you always append extension in the resolveNewAttachmentFilename? Why just not append any extension if user has specifically not passed any extension(when fileExtension is null). I found it really frustrating. In old powersync_attachments_helper I could override those methods, but now AttachmentQueue is not extendable.

That's fair feedback. In other SDKs this method is open for extension. We should fix the optional behaviour and make it extensible.