On Linux and Windows the file watcher can't distinguish between changes
that were caused by the process itself, like during a sync operation,
and external changes. To work around that the client keeps a list of
files it has touched and blocks notifications on these files for a bit.
The duration of this block was originally and arbitrarily set at 15
seconds. During manual tests I regularly thought there was a bug when
syncs didn't trigger, when the only problem was that my changes happened
too close to a previous sync operation.
This change reduces the duration to three seconds. I imagine that this
is still enough.
Also use std::chrono while at it.
Also, calling deleteLater() on jobs is unnecessary (they autodelete
after finished()) and deleting the attached QSettings is also
unnecessary because the settings object is parented to the job.
Seeing "Currently available online only" for a currently hydrated file
was odd. It makes sense since current hydration status and pin state are
independent.
The new text will say something like "Currently available, but marked
online only" to better indicate that the file might be dehydrated later
since it wasn't pinned.
supportsSelectiveSync(): clearer than !supportsVirtualFiles() and allows
extra logic
isVfsOnOffSwitchPending(): Somewhat awkward way of dealing with the
phase between a user requesting vfs state to be switched and it
actually happening
slotDiscoveryJobFinished -> slotDiscoveryFinished
slotFinished -> slotPropagationFinished
This should be clearer. Particular the
slotFinished -> finalize -> emit finished()
chain was confusing before.
This allows enabling and disabling vfs.
To distinguish this operation from setting the root pin state, the
availability setting is adjusted as well to be similar to the
menu that shows in the shell extensions.
It has a destructor and these operations make sense. Particularly the
move is important for code like:
Result<x, y> foo() { Result<x, y> v; return v; }
because the move-ctor will not autogenerate if x or y are not trivially
destructible.
The idea is to allow folders (and later maybe files?) to be
- pinned to be available locally
- pinned to be online only
- inherit their pin from the parent
Where this pinning only controls the default for new files.
Subfolders may have a different pin state, and contained files
may be hydrated or dehydrated based on user actions.
This value is stored in a new 'flags' table. The idea is to store
data there that doesn't necessarily exist for each metadata entry.
The selective sync state could be migrated to this table.
This is just a port to QtTest, I did not change the layout of the test.
I did search and replace to replace the assert with QCOMPARE/QVERIFY
I still call setup and setup_init like before (only explicitly, now)
Also ported the preformence tests to QBENCHMAK because windows don't have
gettimeofday.
Relates #6358
This is to avoid issues on OSX, where the ._ prefix has special meaning.
Originally (before 2.3.2) ._ was necessary to guarantee exclusion. But
since then the .sync_ prefix is excluded as well.
This does not affect existing database files.
Previously there'd likely be a mess if a 2.6 winvfs folder was attempted
to be used with a 2.5 client. Now the older clients will ignore these
folders.
This helps support 2.5 settings where there are virtual files in the
tree but new files aren't created virtual.
It's also a prelude for #6815
There's currently no way of
- upgrading vfs plugins (a silent suffix->winvfs upgrade is attempted
once only, when moving to master)
- disabling vfs capabilities outright
Inh most case we already have a record from before, so avoid doing a useless
lookup in the database.
In owncloudpropagator.cpp, directories do not have a checksum so no need
to call a function that preserves it
When enabling TOKEN_AUTH_ONLY, the code path using QPainter is disabled.
So we also don't need the includes.
This header is not available for Remarkable.
The E2EE code path would get the engine to go wrong in case of db error.
It's just better to have a failing upload or failing mkdir later in those
cases.
Emitting signals from a ctor is a bad idea anyway
Signed-off-by: Kevin Ottens <kevin.ottens@nextcloud.com>
Migration from 2.4: fallback to move file by file if directory move failed
This can happen if the directory already exist because, say, it was
created by the ownCloud outlook plugin which save its file in the same directory
OAuth2 access token typically only has a token valid for 1 hour.
Before this patch, when the token was timing out during the sync, the
sync was aborted, and the ConnectionValidator was then requesting a new
token, so the sync can be started over.
If the discovery takes longer than the oauth2 validity, this means that
the sync can never proceed, as it would be always restarted from scratch.
With this patch, we try to transparently renew the OAuth2 token and restart
the jobs that failed because the access token was invalid.
Note that some changes were required in the GETFile job because it handled
the error itself and so it was erroring the jobs before its too late.
Issue #6814
- adjust virtual file path handing
- helpers for vfs suffix adding/removal
- helpers for isDirectory/isVirtual on SyncJournalRecords
- be clear about what PathTuple _local/_server mean
So we can quickly query the items in a parent directory
This uses a custom slite3 function, which means that when downgrading the client,
or using another tool to add entries in the database, any insertion in the metadata
table will produce an error: "unknown function: parent_hash()"
(This will crash the client 2.5)
If the server does not set the mtime, it is not a big problem for the
synchronisation.
The test was used before so we could do a PROPPATCH for server that did not
support this header. But now that all server supports that we don't need to
to the check. (We do not do the PROPPATCH since we got rid of the neon
dependency)
Apparently, it may happen that some backend don't support setting mtime
and this can lead to this error.
https://github.com/owncloud/client/issues/6797
We need to use the user id to check if we are connected to the right account.
These might be different from the HTTP Basic Auth login. (LDAP setups)
When the account was configured as an oauth2 account form the wisard, the
http_user was already set correctly to the user id. But when the server is
upgrading from basic auth to oauth2, we need to pick the right login.
Note that Account::davUser() already defaults to the HTTP user when none
is set, so this means the upgrade will be fine if this is not set in the
config.
Issues:
https://github.com/owncloud/oauth2/issues/109https://github.com/owncloud/enterprise/issues/2781
And test the Remove/Remove case.
This means we need to always query the database for all the entries.
This showed another small bug in the test in which sync item for virtual
files at the root could have a slash in front of them.
When resolving a conflict because the file was just updated on the server,
we write all the metadata on the database immediatly, so INSTRUCITON_NONE
is enough and UPDATE_METADATA is not needed
The previous code considered the also HTTP 207 code without the
application/xml header to have this message.
httpCode 0 does not make much sense anyway.
This change the behavior to consider any 2xx without the xml header
to show this error message
Since we do not recurse within some directories, many files are not seen.
The stale entry will cleanup by themself as the sync engine try to remove
the files that are already removed.
Should we need to actually do this cleanup, it should be dotected in the
discovery.
Move the finialization in its own function.
This allow to save a bit of code duplication.
Also change the order of the parameter in the constructor for consistency
Unfortunately to do this, the local update phase must write to the
database, creating a new side-effect and order dependency (local update
must run before remote update).
Fixes two bugs that appeared since the introduction of the struct:
- when reading permissions from the journal, null ("") was read as
empty-not-null
- when reading permissinos from the server, empty ("") was read as null
Addresses #4608
The check was added for #6317 in commit
13eb64584f.
We did see missing mtimes in replies in tests with live servers though.
Possibly those were old incomplete responses cached in the stat cache?
Installing to lib/${APPLICATION_EXECUTABLE} has caused a bunch of
irritations in the past and subtle annoying to fix bugs. To avoid name
clashes with branded clients ${APPLICATION_EXECUTABLE} becomes now
part of the filename instead of the subfolder.
The concrete motivation to change this now is that on Windows there
is no RPATH and it's not possible to run owncloud directly from the
Craft Root folder, which is nice when you're developing on Windows.
It would have been possible to change this just for Windows but as
written earlier this has caused lots of issues and thus I think it's
a good idea to just stay consistent accross platforms when touching it.
Issue #6574
When there is an error in the advanced page, OwncloudAdvancedSetupPage::updateStatus
(and others) call completeChanged(), which is connected to
QWizardPrivate::_q_updateButtonStates which will re-enable the back button from the
last page.
When the user click "back" and re-open the browser, the account's credentials
already have a oauth token set. So the call to the API to get a new token fails
because we use the previous token instead of using the client's secret_id.
Fix this with the HttpCredentials::DontAddCredentialsAttribute.
Now, this is still not working because the session cookies are confusing the
server. So we'll clear the cookies when re-opening the browser
In FolderStatusModel::slotLscolFinishedWithError, the call to parentInfo->resetSubs
deleted the 'job' and the reply 'r' which we accessed later to get the error code.
Fix this problem twice by
1) Get the error code before caling resetSubs
2) in FolderStatusModel::SubFolderInfo::resetSubs, call deleteLater instead of delete
Regression introduced in commit d69936e0
Issue #6562
The problem here is that we were sometimes allocating the error_string with
qstrdup, which need to be released with delete[] and not free().
Simplify the code by using QString instead.
```
==7230== Mismatched free() / delete / delete []
==7230== at 0x4C2E10B: free (vg_replace_malloc.c:530)
==7230== by 0x57C2321: csync_s::reinitialize() (csync.cpp:247)
==7230== by 0x548130F: OCC::SyncEngine::finalize(bool) (syncengine.cpp:1212)
==7230== by 0x5481223: OCC::SyncEngine::handleSyncError(csync_s*, char const*) (syncengine.cpp:746)
==7230== by 0x5483E78: OCC::SyncEngine::slotDiscoveryJobFinished(int) (syncengine.cpp:965)
==7230== by 0x5495E34: QtPrivate::FunctorCall<QtPrivate::IndexesList<0>, QtPrivate::List<int>, void, void (OCC::SyncEngine::*)(int)>::call(void (OCC::SyncEngine::*)(int), OCC::SyncEngine*, void**) (qobjectdefs_impl.h:134)
==7230== by 0x5495D92: void QtPrivate::FunctionPointer<void (OCC::SyncEngine::*)(int)>::call<QtPrivate::List<int>, void>(void (OCC::SyncEngine::*)(int), OCC::SyncEngine*, void**) (qobjectdefs_impl.h:167)
==7230== by 0x5495CB5: QtPrivate::QSlotObject<void (OCC::SyncEngine::*)(int), QtPrivate::List<int>, void>::impl(int, QtPrivate::QSlotObjectBase*, QObject*, void**, bool*) (qobjectdefs_impl.h:396)
==7230== by 0xA9BF2E1: QObject::event(QEvent*) (in /usr/lib/libQt5Core.so.5.11.0)
==7230== by 0x64BE983: QApplicationPrivate::notify_helper(QObject*, QEvent*) (in /usr/lib/libQt5Widgets.so.5.11.0)
==7230== by 0x64C625A: QApplication::notify(QObject*, QEvent*) (in /usr/lib/libQt5Widgets.so.5.11.0)
==7230== by 0xA994BC8: QCoreApplication::notifyInternal2(QObject*, QEvent*) (in /usr/lib/libQt5Core.so.5.11.0)
==7230== Address 0x225b2640 is 0 bytes inside a block of size 50 alloc'd
==7230== at 0x4C2DC6F: operator new[](unsigned long) (vg_replace_malloc.c:423)
==7230== by 0xA7E8FC8: qstrdup(char const*) (in /usr/lib/libQt5Core.so.5.11.0)
==7230== by 0x53F5750: OCC::DiscoveryJob::remote_vio_opendir_hook(char const*, void*) (discoveryphase.cpp:666)
==7230== by 0x57E1278: csync_vio_opendir(csync_s*, char const*) (csync_vio.cpp:39)
==7230== by 0x57D718F: csync_ftw(csync_s*, char const*, int (*)(csync_s*, std::unique_ptr<csync_file_stat_s, std::default_delete<csync_file_stat_s> >), unsigned int) (csync_update.cpp:674)
==7230== by 0x57C1B05: csync_update(csync_s*) (csync.cpp:109)
==7230== by 0x53F5BCC: OCC::DiscoveryJob::start() (discoveryphase.cpp:718)
==7230== by 0x54B8F74: OCC::DiscoveryJob::qt_static_metacall(QObject*, QMetaObject::Call, int, void**) (moc_discoveryphase.cpp:494)
==7230== by 0xA9BF2E1: QObject::event(QEvent*) (in /usr/lib/libQt5Core.so.5.11.0)
==7230== by 0x64BE983: QApplicationPrivate::notify_helper(QObject*, QEvent*) (in /usr/lib/libQt5Widgets.so.5.11.0)
==7230== by 0x64C625A: QApplication::notify(QObject*, QEvent*) (in /usr/lib/libQt5Widgets.so.5.11.0)
==7230== by 0xA994BC8: QCoreApplication::notifyInternal2(QObject*, QEvent*) (in /usr/lib/libQt5Core.so.5.11.0)
==7230==
```
We want to keep the body so we can get the message from it
(Issue #6459)
TestDownload::testErrorMessage did not fail because the FakeErrorReply
did not emit readyRead and did not implement bytesAvailable.
Signed-off-by: Kevin Ottens <kevin.ottens@nextcloud.com>
Otherwise a interrupted or unsuccessful download would mean that the
download-intend was forgotten. The next sync would reestablish the
virtual file instead.
Also, if there is too-new configuration, backup the file, show a
warning message asking the user whether it's ok to discard the
configuration from the future.
See #6504
Issue #6420
Store the X-Request-ID in the SyncFileItem and also in the blacklist.
Note that for consistency reason, the X-Request-ID is also in the
SyncFileItem if the request succeeds.
Currently there is no UI to access it, but it can be queried with sql
commands
Similar to the use for the checksum.
I know that zlib is required in principle, but i don't have it
in one of my test building environment, and it is easier to just
disable it.
From issue #7015, the code is wrong because the path is the file system path and
not the path on the DB.
But since this is a conflict, this means the reconcile will still want to download
the file from the server next sync, so we need not to worry about this case
To do this, we add the placeholder extension to the user exclude file
automatically. However, newer clients shouldn't use that exclude
pattern: so we also add version directives that allow making exclude
patterns dependent on the client version.
Now the db entries for placeholders will have the full placeholder
paths. That way older clients will, on remote discovery, delete the
placeholders and download the real files.
- Controled by an option.
- New remote files start out as ItemTypePlaceholder, are created with a
.owncloud extension.
- When their db entry is set to ItemTypePlaceholderDownload the next
sync run will download them.
- Files that aren't in the placeholder state sync as usual.
- See test cases in testsyncplaceholders.
Missing:
- User ui for triggering placeholder file download
- Maybe: Going back from file to placeholder?
You'd expect that after a conflict resolution the file watcher would
pick up the change and trigger a sync. For some reason it doesn't seem
to happen on at least some Ubuntu systems. In such cases the user would
then still have a stale conflict entry in the activity list and wouldn't
be able to do anything with it.
Signed-off-by: Kevin Ottens <kevin.ottens@nextcloud.com>
The model was just checking for the user list being empty or not which
is overly optimistic. Indeed there might be cases where the id is
actually outside the boundaries so properly check for this.
Signed-off-by: Kevin Ottens <kevin.ottens@nextcloud.com>
UserModel can't be connected to AccountSettings if the settings dialog
doesn't exist. This is the case now since we delay the creation of that
dialog and free it after use.
Instead it should be properly channeled through the Systray object all
the way up to OwncloudGui which knows how to handle this properly.
Signed-off-by: Kevin Ottens <kevin.ottens@nextcloud.com>
Split widgets and slot to handle the refreshing of the view:
- refreshSelectiveSyncStatus is connected to signal dirtyChanged
and will handle big folder warning.
- slotSelectiveSyncChanged which is connected to dataChanged signal
and will handle the selective sync warning. It fixes#1029 because
it looks for the checkbox state before showing the warning.
Signed-off-by: Camila <hello@camila.codes>
The previous check didn't take into the account that .sync-exclude.lst
might be empty which would crash at Q_ASSERT(_allExcludes.contains(basePath))
in the prepare function. It also takes into account that
_allExcludes[basePath] was creating new items in the list.
Signed-off-by: Camila <hello@camila.codes>