WIP: feat: support notify_file_id push notifications
#8502
Conversation
| static constexpr int MAX_ALLOWED_FAILED_AUTHENTICATION_ATTEMPTS = 3; | ||
| static constexpr int PING_INTERVAL = 30 * 1000; | ||
|
|
||
| static constexpr QLatin1String NOTIFY_FILE_ID_PREFIX = QLatin1String{"notify_file_id "}; |
There was a problem hiding this comment.
is this space at the end intended?
There was a problem hiding this comment.
yes
the push notification received looks like:
notify_file_id [1,2,3,156,6456,269243]
I think parsing that array would even work with the trailing space
| SqlQuery query( | ||
| QLatin1String{"SELECT 1 FROM metadata WHERE ROUND(fileid) IN (%1) LIMIT 1;"} | ||
| .arg(fileIdStrings.join(QLatin1String{", "})).toLocal8Bit(), | ||
| _db | ||
| ); |
There was a problem hiding this comment.
please use a prepared SQL request with SQL placeholders ?
for example https://github.com/nextcloud/desktop/blob/master/src/common/syncjournaldb.cpp#L1111-L1121
There was a problem hiding this comment.
I'm not sure how useful the prepared SQL query would be in this case as there is simply no way to bind an array to a single placeholder, and the passed fileIds array does not have a consistent size. I would also rather avoid to have query each possible fileid one after the other until at least one entry was found...
| [[nodiscard]] bool updateLocalMetadata(const QString &filename, | ||
| qint64 modtime, qint64 size, quint64 inode, const SyncJournalFileLockInfo &lockInfo); | ||
|
|
||
| [[nodiscard]] bool hasFileIds(const QList<qint64> &fileIds); |
src/gui/folderman.cpp
Outdated
| continue; | ||
| } | ||
|
|
||
| if (!folder->journalDb()->hasFileIds(fileIds)) { |
There was a problem hiding this comment.
is there any reason to use this apart from when using selective synchronization ?
did you checked that the request is fast enough ?
nilsding
force-pushed
the
feature/notify-file-ids
branch
from
a44cead to
a2a4ae9
Compare
For this to work we also now need to store the fileId from the sync root folder, otherwise we would never know about new files inside the configured sync root Signed-off-by: Jyrki Gadinger <[email protected]>
nilsding
force-pushed
the
feature/notify-file-ids
branch
from
a2a4ae9 to
5eb8f40
Compare
|
Artifact containing the AppImage: nextcloud-appimage-pr-8502.zip Digest: To test this change/fix you can download the above artifact file, unzip it, and run it. Please make sure to quit your existing Nextcloud app and backup your data. |
|
only basic support for now: schedule a folder to sync if any of the received
fileids is known.the current WIP implementation has an issue though: if a file/folder was created in the sync folder root, that folder will not be synced even though that root folder'sfileidis part of the list received through the notification