While there are sessions pending or active, the server should not shut down.
It did that while executing a long-running PIM Manager SyncPeer() operations,
by default after 10 minutes.
This was not a problem elsewhere because other operations are associated with
a client, whose presence also prevents shutdowns. Perhaps PIM Manager should
also track the caller and treat it like a client.
Like everything else that waits for a certain event on the main loop,
SYNCEVO_GLIB_CALL_SYNC() should also better use GRunWhile(). This is
necessary to be usable in threads.
obexd 0.48 is almost the same as obexd 0.47, except that it dropped
the SetFilter and SetFormat methods in favor of passing a Bluex 5-style
filter parameter to PullAll.
SyncEvolution now supports 4, in words, four different obexd
APIs. Sigh.
The code works with gSSO (https://01.org/gsso). With some tweaks to
the configure check and some ifdefs it probably could be made to work
Ubuntu Online Accounts.
The code depends on an account accessible via libaccounts-glib which
has a provider and and (optionally) services enabled for that
provider. It is not necessary that the account already has a signon
identity ID, the backend will create that for the provider (and thus
shared between all services) if necessary.
Therefore it is possible to use the ag-tool to create and enable the
account and services. Provider and service templates are in the next
commit.
If given an AuthProvider which can handle OAuth2, then OAuth2 is
used instead of plain username/password authentication.
Obtaining the OAuth2 token must be done at a point where we can still
abort the request. If obtaining the token fails, then this should be
considered a fatal error which aborts scanning for resources. Other
errors cause the current URL to be skipped while scanning continues.
This commit moves the "execute request" functionality back into the
Neon::Session class, because that is where most of the logic (retry
request?) and state is (access tokens which persist across requests).
The real username is only relevant when running a sync. When looking
at a config with a D-Bus client like the GTK UI, the username should
always be "id:<config>", to avoid accidentally removing the
indirection, while the password should be the real one, to allow the
user to edit like he normally would with passwords stored in a
keyring.
To achive this, overriding the username must be suppressed when
resolving as part of the D-Bus config API. While at it, move the
entire "iterate over properties" into a common utility function in
PasswordConfigProperty.
A SHARED_LAYOUT config tree caches config nodes. Allow a second config
to use those same nodes as an already existing config. This will be
useful in combination with indirect password lookup, because then the
credentials can be stored as temporary property values and be reused
when used multiple times in a process (for example, by CardDAV and by
CalDAV).
In practice, the methods are always called for a specific SyncConfig.
Passing that allows removing several other parameters and, more
importantly, also grants access to the config and through that other
configs. This will be needed for the indirect credential lookup.
"username", "proxyUsername" and "databaseUser" used to be simply a
string containing the name of the respective user or (in the case of
the ActiveSync backend) the account ID in gconf.
Now it is also possible to provide credentials (username + password)
indirectly: when any of these properties is set to "id:<config name>",
then the "username/password" properties in that config are used
instead. This is useful in particular with WebDAV, where credentials
had to be repeated several times (target config, in each database when
used as part of SyncML) or when using a service which requires several
configs (Google via SyncML and CalDAV).
For user names which contain colons, the new "user:<user name>" format
must be used. Strings without colons are assumed to be normal user
names.
This commit changes the SyncConfig APIs for this extension. More work
is needed to make the indirect lookup via "id" functional.
Passwords are cached after the initial check as temporary property
values. The explicit string members are obsolete and can be removed
together with the code using them.
Like everything else that waits for a certain event on the main loop,
SYNCEVO_GLIB_CALL_SYNC() should also better use GRunWhile(). This is
necessary to be usable in threads.
When clients like the GTK sync-ui stored a password, it was always
stored as plain text in the config.ini file by the
syncevo-dbus-server. The necessary code for redirecting the password
storage in a keyring (GNOME or KWallet) simply wasn't called in that
case.
The command line tool, even when using the D-Bus server to run the
operation, had the necessary code active and thus was not affected.
uint16 happened to work when compiling with a recent GNOME stack, but
without that uint16 is not defined. The right approach is to use
stdint.h and uint16_t.
We used to kill it when it showed the first sign of life via D-Bus log output
- any output! Depending on timing, it may or may not have been able to send
the "target side ready" INFO message. If it did, our strict output check
failed.
Fix that by waiting for that message, which should be the only INFO message
and thus the only one which will appear in the output text, before killing the
process.
Both maintaining the map items inside the Synthesis engine and storing
them in .ini hash config nodes inside SyncEvolution are fairly heavy
operations which are not needed at all during an ephemeral sync (= no
meta data stored, done by the PIM Manager when triggering a pure PBAP
sync).
Using the new Synthesis CA_ResumeSupported DB capability it is
possible to suppress these operations without having to fiddle with
the Synthesis DB API that SyncEvolution provides. To make it possible
at the DB layer to detect that the meta data is not needed, the
ConfigNode passed to it must be marked as volatile.
This change sped up a sync with 10000 unmodified, known items from 38s
to 23s.
The synthesisID value is required for each Synthesis source progress
event, which can be fairly frequent (more than one per item). Instead
of going down to the underlying .ini config node each time, cache the
value in the SyncSourceConfig layer.
Syncing was slowed down by fowarding all log messages from the local
sync helper to its parent and from the D-Bus helper to
syncevo-dbus-server. Quite often, the log messages then were simply
discarded by the recipient. To speed up syncing, better filter at the
source.
The syncevo-dbus-helper is told which messages are relevant by
forwarding the syncevo-dbus-server "D-Bus log level" command line
setting to the helper process as part of its argv parameters.
The synevo-local-sync helper applies its own log level now also to the
messages sent to the parent. This ensures that messages stored in the
client log also show up in the parent log (unless the parent has more
restrictive settints, which is uncommon) and that INFO/SHOW messages
still reach the user.
For each incoming change, one INFO line with "received x[/out of y]"
was printed, immediately followed by another line with total counts
"added x, updated y, removed z". For each outgoing change, a "sent
x[/out of y]" was printed.
In addition, these changes were forwarded to the D-Bus server where a
"percent complete" was calculated and broadcasted to clients. All of
that caused a very high overhead for every single change, even if the
actual logging was off. The syncevo-dbus-server was constantly
consuming CPU time during a sync when it should have been mostly idle.
To avoid this overhead, the updated received/sent numbers that come
from the Synthesis engine are now cached and only processed when done
with a SyncML message or some other event happens (whatever happens
first).
To keep the implementation simple, the "added x, updated y, removed z"
information is ignored completely and no longer appears in the output.
As a result, syncevo-dbus-server is now almost completely idle during
a running sync with no log output. Such a sync involving 10000 contacts
was sped up from 37s to 26s total runtime.
When asked to run quietly via a non-empty TEST_DBUS_QUIET,
test-dbus.py still enabled INFO messages to determine that the server
is ready. Waiting for bus name (as already done when starting with a
debugger) avoids that and allows us to disable all LogOutput signals.
If a client gave up waiting for the server's response and resent its message
while the server was still processing the message, syncing failed with
"protocol error: already processing a message" raised by the
syncevo-dbus-server because it wasn't prepared to handle that situation.
The right place to handle this is inside the syncevo-http-server, because it
depends on the protocol (HTTP in this case) whether resending is valid or
not. It handles that now by tracking the message that is currently in
processing and matching it against each new message. If it matches, the new
request replaces the obsolete one without sending the message again to
syncevo-dbus-server. When syncevo-dbus-server replies to the old message, the
reply is used to finish the newer request.
This situation is covered by
Client::Sync::eds_event_eds_contact::testManyDeletes with 100 items and
client and server running under valgrind. The test failed earlier and works
now.
Set SYNCEVOLUTION_DBUS_HELPER_VGDB=1, add --vgdb-error=1 --vgdb=yes
to VALGRIND_ARGS, run test, wait for vgdb message in valgrind*.out files,
attach as instructed.
With --vgdb-error=0, all processes block during startup, waiting for
the debugger to attach.
The previous attempt with concurrent reading while listing IDs did not
work, that listing must complete before the SyncML client contacts the
server. What works is transfering and parsing after the engine starts
to ask for the actual data.
For that we need to list IDs in advance. We use GetSize() for that.
If contacts get deleted while we read, getting the data for the
contacts at the end of the range will fail with 404, which is
understood by the Synthesis engine and leads to ignoring the ID, as
intended.
If contacts get added while we read, we will ignore them even if they
happen to be in the result of PullAll. The next sync will include
them.
When doing a PBAP sync, PIM manager asks the D-Bus sync helper to set
its SYNCEVOLUTION_PBAP_SYNC to "incremental". If the env variable
is already set, it does not get overwritten, which allows overriding
this default.
Depending on the SYNCEVOLUTION_PBAP_SYNC env variable, syncing reads
all properties as configured ("all"), excludes photos ("text") or
first text, then all ("incremental").
When excluding photos, only known properties get requested. This
avoids issues with phones which reject the request when enabling
properties via the bit flags. This also helps with
"databaseFormat=^PHOTO".
When excluding photos, the vcard merge script as used by EDS ensures
that existing photo data is preserved. This only works during a slow
sync (merge script not called otherwise, okay for PBAP because it
always syncs in slow sync) and EDS (other backends do not use the
merge script, okay at the moment because PIM Manager is hard-coded to
use EDS).
The PBAP backend must be aware of the PBAP sync mode and request a
second cycle, which again must be a slow sync. This only works because
the sync engine is aware of the special mode and sets a new session
variable "keepPhotoData". It would be better to have the PBAP backend
send CTCap with PHOTO marked as not supported for text-only syncs and
enabled when sending PHOTO data, but that is considerably harder to
implement (CTCap cannot be adjusted at runtime).
beginSync() may only ask for a slow sync when not already called
for one. That's what the command line tool does when accessing
items. It fails when getting the 508 status.
The original goal of overlapping syncing with download has not been
achieved yet. It turned out that all item IDs get requested before
syncing starts, which thus depends on downloading all items in the current
implementation. Can be fixed by making up IDs based on the number of
existing items (see GetSize() in PBAP) and then downloading later when
the data is needed.
If PHOTO and/or GEO were the only modified properties during a slow
sync, the updated item was not written into local storage because
they were marked as compare="never" = "not relevant".
For PHOTO this was intentional in the sample config, with the
rationale that local storages often don't store the data exactly as
requested. When that happens, comparing the data would lead to
unnecessary writes. But EDS and probably all other local SyncEvolution
storages (KDE, file) store the photo exactly as requested, so not
considering changes had the undesirable effect of not always writing
new photo data.
For GEO, ignoring it was accidental.
A special merge script handles EDS file:// photo URIs. When the
loosing item has the data in a file and the winning item has binary
data, the data in the file may still be up-to-date, so before allowing
MERGEFIELDS() to overwrite the file reference with binary data and
thus forcing EDS to write a new file, check the content. If it
matches, use the file reference in both items.
Performance is improved by requesting multiple contacts at once and
overlapping reading with processing. On a fast system (SSD, CPU fast
enough to not be the limiting factor), testpim.py's testSync takes 8
seconds for a "match" sync where 1000 contacts get loaded and compared
against the same set of contacts. Read-ahead with only 1 contact per
query speeds that up to 6.7s due to overlapping IO and
processing. Read-ahead with the default 50 contacts per query takes
5.5s. It does not get much faster with larger queries.
While returning items from one cache populated with a single
e_book_client_get_contacts() call, another query is started to overlap
processing and loading.
To achieve efficient read-ahead, the backend relies on the hint given
to it via setReadAheadOrder(). As soon as it detects that a contact is
not the next one according to that order, it switches back to reading
one contact at a time. This happens during the write phase of a sync
where the Synthesis engine needs to read, update, and write back
changes based on updates sent by the peer.
Cache statistics show that this works well for --print-items, --export
and slow syncs.
Writing into the database must invalidate the corresponding cached
contact. Otherwise the backup operation after a sync may end up
reading stale data.
Trying to predict in the SyncSource which items will be needed is
hard. It depends what the item is retrieved for (sync or
backup/printing) and on the sync mode (during a sync).
Instead of guessing, better have the user of the source tell the
source in advance what it will read. In most cases this is cheap
because it does not involve copying of items luids ("read all items",
"read new or modified items"). The source then can use its own internal
mechanisms to figure out what that means.
Only extracting specific items specified on the command line and the
backup operation must explicitly pass an ordered list of luids. For
the sake of simplicity, do that in a std::vector even though it
involves copying.
The
while (<something>) g_main_context_iterate(NULL, true);
pattern only works in the main thread. As soon as multiple
threads are allowed to process events, race conditions occur,
as described before.
But the pattern is useful, so support it in all threads by
shifting the check into the main thread, which will then notify
other threads via a condition variable when something changes.
