SyncEngine: Reads the data-fingerprint property #5056
Conversation
|
Smashbox test: owncloud/smashbox#149 |
src/libsync/discoveryphase.cpp
Outdated
There was a problem hiding this comment.
If this is anyway server global according to @PVince81 we could always read it maybe?
There was a problem hiding this comment.
Aren't we wasting bandwidth by requesting it all the time so it is returned with each file/directory in each PROPFIND in the discovery?
There was a problem hiding this comment.
oops, leftover from debugging. well spot.
But we don't request it all the time, only for the first folder, see previous hunk (_isRootPath)
There was a problem hiding this comment.
I do not like the isRootPath parameter. It's cluttering of the interface.
I think it would be nicer if you would either set all props from the class that creates the job using setProperties(), or even better add a method addProperty( a_property) that is called in case it is a root object.
There was a problem hiding this comment.
DiscoverySingleDirectoryJob is an internal class to the discovery job, it's not really a public class with an exported interface. Its role is to abstract the protocol part of the discovery. In that sens, the name of the properties are part of the protocol and should be IMHO kept inside this class.
src/libsync/ownsql.cpp
Outdated
There was a problem hiding this comment.
Is this correct? Should it use sqlite3_bind_blob(64)?
There was a problem hiding this comment.
I actually don't know what's the difference here. I guess it's maybe related to \0 in the string? Then maybe indeed blob could be better (more generic) but currently, in owncloud, we don't have any field of type blob, only text.
There was a problem hiding this comment.
And this is just an optimisation, maybe i could leave it out and then it would use the text16 fallback as toString which is just a bit of a waste.
There was a problem hiding this comment.
(I meant because of encoding issues..QByteArray.. but since the data-fingerprint is from WebDAV it is probably anyway safe ascii or so)
| auto databaseFingerprint = _journal->dataFingerprint(); | ||
| // If databaseFingerprint is null, this means that there was no information in the database | ||
| // (for example, upgrading from a previous version, or first sync) | ||
| // Note that an empty ("") fingerprint is valid and means it was empty on the server before. |
There was a problem hiding this comment.
i still don't understand why there is a difference between empty and null. Why can't they be treated the same?
There was a problem hiding this comment.
Null means there was no entry before.
Empty means there was one, and it was empty.
By default, on the server, it is empty.
There was a problem hiding this comment.
Ah. Weeeeird. But I guess this is because it was added later to the server
FYI @rullzer
For owncloud/client#5056 Users can configure arbitrary subfolders for syncing, therefore we should always return it when asked for. The sync client makes sure to not always ask for it to save bandwidth.
When it changes, assume a backup was recovered, and keep conflict files. Issues: #2325 and https://github.com/owncloud/enterprise/issues/966
ogoffart
force-pushed
the
restore_backup
branch
from
bb8d0f4 to
b7b5bfd
Compare
|
OK 👍 |
|
There is a smashbox test: owncloud/smashbox#149 |
For owncloud/client#5056 Users can configure arbitrary subfolders for syncing, therefore we should always return it when asked for. The sync client makes sure to not always ask for it to save bandwidth.
For owncloud/client#5056 Users can configure arbitrary subfolders for syncing, therefore we should always return it when asked for. The sync client makes sure to not always ask for it to save bandwidth.
For owncloud/client#5056 Users can configure arbitrary subfolders for syncing, therefore we should always return it when asked for. The sync client makes sure to not always ask for it to save bandwidth.
|
@davitol this is already tested, isn't it? |
|
@rperezb No, it isn't. The scenarios tested were related to Fix detection of backup or full resync, but not to the data-fingerprint property. Right now i'm catching up how all this stuff of data-fingerprint works in oC. ASAP it is tested, i'll write it here. |
For owncloud/client#5056 Users can configure arbitrary subfolders for syncing, therefore we should always return it when asked for. The sync client makes sure to not always ask for it to save bandwidth.
For owncloud/client#5056 Users can configure arbitrary subfolders for syncing, therefore we should always return it when asked for. The sync client makes sure to not always ask for it to save bandwidth.
When it changes, assume a backup was recovered, and keep conflict files.
Issues: #2325 and https://github.com/owncloud/enterprise/issues/966