The signal name was not stored, making the actual filtering more
relaxed than it should have been. Worse, the path was redundantly
checked for a complete match even when SIGNAL_FILTER_PATH_PREFIX was
set, thus preventing signal deliver in that case.
Working with obexd 0.47's Transfer object depends on path prefix
filtering of signals to be efficient. This patch adds that support in
the GDBus C++ bindings, together with some documentation on how to
use it.
It also allows relaxing the signal matching by interpreting empty
strings as "match any".
Finally it adds support for passing of sender, path, interface and
member (= signal name) information to the user of the D-Bus
binding. This is necessary because a handler with wildcards for
matching cannot know the actual values unless they are passed to the
callback explicitly. This was not possible at all before for signals
(because extracting that information relied on a message pointer,
which wasn't set while decoding signal messages) and limited to just
the sender in method implementations.
Besides in functionality, GDBus for GIO and for libdbus now also
differ slightly in their API. In order to pass meta data about a
signal into the demarshalling get() methods, they have to be stored in
ExtractArgs first and passed to get() via that struct, which is an
interface change.
Derived code which extends marshalling/demarshalling needs to be
adapted accordingly. Allowing some ifdefs elsewhere seems simpler than
adapting GDBus for libdbus. Those ifdefs can be removed once libdbus
support is no longer needed.
The patch relaxes access control to DBusObject and DBusRemoteObject
because that simplifies the code which works better with std::string.
Traditionally the sender of a signal was not checked. This is probably
related to unsolved questions like "should watching org.foo.bar follow
owner changes", but not doing any checking is just not right
either. Not fixed in this patch.
The PBAP backend must link against pcrecpp explicitly. The header
files are part of the global search path, but the library is not.
Having to specify the library explicitly is right (avoids
adding it to modules which don't need it); perhaps the header
file flags also should have to be added explicitly.
It worked when linked into an executable which also uses pcrecpp, but
can fail to build depending on how strict the toolchain is. Found
by dist checks in the nightly testing.
When using GIO D-Bus, sometimes messages created shortly before
shutting down were not sent. Must flush a connection explicitly.
A NOP when using libdbus.
First, clarified that a source has to set all revision strings or
none. This was implied, but not spelled out explicitly.
Second, use that to detect when a sync must be done in slow mode.
Third, avoid calculating changes when that isn't needed. This last
point was left as TODO while in release preparations and can
be committed now.
This changes is needed for the PBAP backend which does not currently
set revision strings and thus cannot be used in incremental syncing.
This patch introduces support for true one-way syncing ("caching"):
the local datastore is meant to be an exact copy of the data on the
remote side. The assumption is that no modifications are ever made
locally outside of syncing. This is different from one-way sync modes,
which allows local changes and only temporarily disables sending them
to the remote side.
Another goal of the new mode is to avoid data writes as much as
possible.
This new mode only works on the server side of a sync, where the
engine has enough control over the data flow.
Most of the changes are in libsynthesis. SyncEvolution only needs to
enable the new mode, which is done via an extension of the "sync"
property:
- "local-cache-incremental" will do an incremental sync (if possible)
or a slow sync (otherwise). This is usually the right mode to use,
and thus has "local-cache" as alias.
- "local-cache-slow" will always do a slow sync. Useful for
debugging or after (accidentally) making changes on the server side.
An incremental sync will ignore such changes because they are not
meant to happen and thus leave client and sync out-of-sync!
Both modes are recorded in the sync report of the local side. The
target side is the client and records the normal "two-way" or "slow"
sync modes.
With the current SyncEvolution contact field list, first, middle and
last name are used to find matches during any kind of slow sync. The
organization field is ignored for matching during the initial slow
sync and used in all following ones. That's okay, the difference won't
matter in practice because the initial slow sync in PBAP caching will
be done with no local data. The test achieve the same result in both
cases by keeping the organization set in the reduced data set.
It's also okay to include the property in the comparison, because it
might help to distinguish between "John Doe" in different companies.
It might be worthwhile to add more fields as match criteria, for
example the birthday. Currently they are excluded, probably because
they are not trusted to be supported by SyncML peers. In caching mode
the situation is different, because all our data came from the peer.
The downside is that in cases where matching has to be done all the
time because change detection is not supported (PBAP), including the
birthday as criteria will cause unnecessary contact removed/added
events (and thus disk IO) when a contact was originally created
without birthday locally and then a birthday gets added on the phone.
Testing is done as part of the D-Bus testing framework, because usually
this functionality will be used as part of the D-Bus server and writing
tests in Python is easier.
A new test class "TestLocalCache" contains the new tests. They include
tests for removing extra items during a slow sync (testItemRemoval),
adding new client items under various conditions (testItemAdd*) and
updating/removing an item during incremental syncing
(testItemUpdate/Delete*). Doing these changes during a slow sync could
also be tested (not currently covered).
The tests for removing properties (testPropertyRemoval*) cover
removing almost all contact properties during an initial slow sync, a
second slow sync (which is treated differently in libsynthesis, see
merge=always and merge=slowsync), and an incremental sync.
For example in the new TestLocalCache.testItemDelete100, the
percentage value in the ProgressChanged signal become larger
than 100 and then revert to 100 at the end of the sync.
Seems the underlying calculation is faulty or simply inaccurate.
This patch does not fix that. Instead it just clips the final
result to the valid range. It also cleans up ownership of the
actual int32_t progress value.
Instad of having three of the helper classes as members,
only store pointers to them in Server. The advantage is
that including Server.h becomes simpler for .cpp files
which do not need to use these classes and that they
can be constructed/destructed more explicitly. This is
particularly important because they get access to the
Server instance which is still in the process of
initializing itself.
Change tracking in the file backend used to be based on the
modification time in seconds. When running many syncs quickly (as in
testing), that can lead to changes not being detected when they happen
within a second.
Now the file backend also includes the sub-second part of the
modification time stamp, if available. The revision string
intentionally uses no leading zeros, because that would make it
unnecessarily larger.
This change is relevant when upgrading SyncEvolution: most of the
items will be considered "updated" once during the first sync after
the upgrade (or a downgrade) because the revision strings get
calculated differently.
With this patch, the databaseFormat can be used as follows:
[(2.1|3.0)][:][^]propname,propname,...
3.0 = download in vCard 3.0 instead of the default 2.1
3.0:^PHOTO = download in vCard 3.0 format, excluding PHOTO
PHOTO = download in vCard 2.1 format, only the PHOTO
Valid property names are pulled from obexd with ListFilterFields().
Binding a temporary std::string (the result of getDatabase() in this
case) to a const reference is broken, because the temporary instance
will get deleted before the reference.
Local IDs across sessions are only useful when we also have useful
revision strings. For debugging it is easier to just enumerate the
contacts. Would be nice to use the same number as in the PBAP session,
but that information is not currently available via obexd (see "PBAP +
two-step download" on Bluez mailing list).
As it stands now, the PBAP backend can only be used in slow syncs
where the engine does the matching. Perhaps that's the right way to do
it, instead of adding redundant code to the backends.
When using libdbus instead of GIO D-Bus (as done by syncevolution.org
binaries and SyncEvolution on Maemo), local sync may have aborted
after 25 seconds when syncing many items with a D-Bus timeout error:
[ERROR] sending message to child failed: org.freedesktop.DBus.Error.NoReply: Did not receive a reply. Possible causes include: the remote application did not send a reply, the message bus security policy blocked the reply, the reply timeout expired, or the network connection was broken.
Reported by Toke Høiland-Jørgensen for Harmattan. Somehow not encountered
elsewhere.
Root cause is the use of the default D-Bus timeout of 25 seconds
instead of disabling the timeout altogether. Fixed by using
DBUS_TIMEOUT_INFINITE and INT_MAX as fallback.
When using libdbus instead of GIO D-Bus (as done by syncevolution.org
binaries and SyncEvolution on Maemo), local sync may have aborted
after 25 seconds when syncing many items with a D-Bus timeout error:
[ERROR] sending message to child failed: org.freedesktop.DBus.Error.NoReply: Did not receive a reply. Possible causes include: the remote application did not send a reply, the message bus security policy blocked the reply, the reply timeout expired, or the network connection was broken.
Reported by Toke Høiland-Jørgensen for Harmattan. Somehow not encountered
elsewhere.
Root cause is the use of the default D-Bus timeout of 25 seconds
instead of disabling the timeout altogether. Fixed by using
DBUS_TIMEOUT_INFINITE and INT_MAX as fallback.
Some unnamed version of KDE crashes in KApplication when invoked
without a D-Bus session. The reporter ran into this when compiling
from source, because the SyncEvolution binary is invoked as part of
the build process, which ran outside of a D-Bus session.
Avoid the crash by checking for a D-Bus session bus with
QDBusConnection::sessionBus().isConnected() before instantiating
KApplication. The QDBusConnection API does not say explicitly when it
connects to the daemon, but testing shows that in practice this
detects missing env variables and an unreachable daemon right away as
expected, while passing when the daemon can be contacted.
Instantiating KApplication was added for KWallet support. Without
D-Bus, KWallet does not work either, therefore throw an explicit error
when the lack of D-Bus was detected.
A combination of Funambol Android and Funambol server recently led to
the Funambol server sending PHOTO data with TYPE=image/jpeg. This is
invalid and caused EDS to reject the photo (Vladimir Elisseev,
"[SyncEvolution] issues with syncing photos").
Work around the problem by only keeping the part of the type after the
last slash, if there is any. For image/jpeg and similar types that
leads to the desired value and does not affect valid values, because
those do not contain a slash
(http://www.iana.org/assignments/media-types/image/index.html).
When syncevo-dbus-server was started on demand by the D-Bus daemon,
then it registered itself with the daemon before it was ready to
serve requests. Only happened in combination with GIO D-Bus and
thus was not a problem before 1.2.99.x.
One user-visible effect was that the GTK UI did select the default
service when it was started for the first time, because it could not
retrieve that information from syncevo-dbus-server.
The fix consists of delaying the name acquisition. That gives the
caller a chance to register D-Bus objects first, before completing the
connection setup. The semantic change is that
dbus_bus_connection_undelay() must be called on new connections which
have a name (a NOP with libdbus).
This patch tries to minimize code changes. The downside of not
changing the GDBusCXX API more radically is that the bus name must be
attached to DBusConnectionPtr, where it will be copied into each
reference to the connection. Hopefully std::string is smart enough to
share the (small) data in this case. Should be solved cleaner once
libdbus support can be deprecated.
A test for auto-activation and double start of syncevo-dbus-server
is also added.
One method was still virtual, from the time when the class had a base
class. Remove the "virtual" keyword because a) it causes a compiler
warning in recent g++ about a class with non-virtual destructor with a
virtual method and b) it makes the generated code unnecessarily
complex.
The old explanation made it sound like nothing would get deleted by
default ("If set, ..."). That's not correct, by default only 10
sessions are kept.
Also explain the behavior of deleting intermediate sessions first.
Auto syncing was not getting triggered when using an autoSyncDelay > 0;
by default it is 5 minutes. Thanks to Vladimir Elisseev for reporting
this problem.
The root cause was a "greater than" instead of a "less than"
comparison. This was not found by the tests because they set
autoSyncDelay to zero to speed up testing.
Changing one test (TestSessionAPIsDummy.testAutoSyncNetworkFailure) so
that it uses non-zero autoSyncDelay triggered the problem. Also added
a variation of that test that simulates the "no Connman and no
NetworkManager" setup used by Vladimir.
Using "template" as parameter name causes problems for Qt, because the
name gets used in the generated C++ stubs. The earlier solution for
the problem (spelling it as "tmplate") was accidentally removed in
e4bd5bd while preparing 1.2.99.1.
Let's use "getTemplate" instead of "templateName", because the
parameter itself does not actually contain the name; it's just
a boolean.
This reverts commit 2d3153bc48.
Reverting to the Qt 4 compatible annotations and fixing
the accidental removal of the "template" parameter in
this method.
When compiled against EDS 3.5.x or later, SyncEvolution now uses
the backend code originally written for the EClient API introduced
in EDS 3.2. That code was changed so that it works with the new
include file rules and ESourceRegistry in EDS 3.5.x. Support
for using the EClient API with EDS 3.4 was removed because maintaining
three different flavors of the EDS backend code would be too much
work and not gain much (just the possibility to test the EDSClient
code with 3.4).
At the moment, this is a compile time choice made automatically
by configure. syncevolution.org binaries are compiled against
an older EDS and thus do not work with EDS 3.5.x or later.
EDS 3.5.x handles authentication itself, using a standard system
prompt if necessary. SyncEvolution can no longer provide the password,
and thus the "databaseUser/Password" options have no effect when using
EDS 3.5.x.
The patch leaves code for older EDS almost completely unchanged and
therefore is considered safe for the stable release series leading to
1.3. Using EClient is an all-or-nothing choice now, because the common
EvolutionSyncSource needs to be compiled differently. Thanks to the
reorganized API, a lot more common code for ECal and EBook sources
could be moved into EvolutionSyncSource.
Instantiating an ActiveSyncSource for testing must work with
empty database names now. Testing no longer forces the
database to be set.
While at it rewrote the code to avoid the explicit pointer.
synccompare on the target side of a local sync was invoked with its
output being redirected via an unreliable socket to the local sync
parent. When the output was large, some of it might have been lost.
Fixed by reading the output locally in the syncevo-local-sync process
with reliable output redirection and sending it to the parent via
D-Bus. Execute() does that when both stdout and stderr are redirected
via LogRedirect, which makes sense also for other stdout output.
When processing stdout from syncevo-local-child in
syncevo-dbus-helper, the LogRedirect class was invoked recursively and
tried to print the same stdout data repeatedly until the
syncevo-dbus-helper crashed due to the infinite recurssion.
The output shouldn't have been sent via stdout in the first place
(will be fixed separately), but nor should LogRedirect have crashed.
Fixed by swapping the data into a temporary buffer and thus not using
it again.
A backend cannot know whether it'll be used together
with a client-test which supports integration testing.
Therefore the backend should always register its tests.
Updating failed when using Google because the Synthesis engine
tried to read the existing item in order to merge it with
the update. This failed because Google does not implement the
Fetch command.
Pretending to update the item intelligently avoids that. It
also helps to improve performance of updates with Exchange.
The downside is that syncing with local storages which do
not support all ActiveSync fields will cause data loss.
Need to check whether Exchange-only attributes get lost
also when the local storage supports everything, for
example because activesynd unintentionally removes data.
The error message for "Invalid synchronization key" changed in
activesyncd, now it contains the D-Bus error type as prefix. Fixed by
doing a substring search.
Also, not freeing the GError before trying again is a bug. Apparently
that was ignored earlier, now it triggers an assert.
As mentioned by Tino Keitel on the mailing list, some libs and
executables were only implicitly linked against libraries that they
called directly. This happened to work by chance because these libraries
ended up in the running executable anyway, due to indirect loading.
To catch such problems, the "make installcheck" was extended:
dpkg-shlibdeps is run, if available, and the error output is scanned
for the messages which indicate that a symbol is used without linking
to the right library (example output below).
Had to fix quite a few _LIBADD lines to pass the test.
Some exceptions are allowed:
- libsmltk depends on the caller providing SySync logging support.
- libneon is intentionally not linked explicitly for syncevolution.org
binaries, to make resulting binaries work with GNUTLS and OpenSSL.
dpkg-shlibdeps: warning: debian/syncevolution-libs/usr/lib/syncevolution/backends/syncdav.so contains an unresolvable reference to symbol icalparameter_new_from_value_string: it's probably a plugin.
dpkg-shlibdeps: warning: 51 other similar warnings have been skipped (use -v to see them all).
...
dpkg-shlibdeps: warning: symbol dlsym used by debian/libsyncevolution0/usr/lib/libsyncevolution.so.0.0.0 found in none of the libraries.
dpkg-shlibdeps: warning: symbol dlerror used by debian/libsyncevolution0/usr/lib/libsyncevolution.so.0.0.0 found in none of the libraries.
dpkg-shlibdeps: warning: symbol dlopen used by debian/libsyncevolution0/usr/lib/libsyncevolution.so.0.0.0 found in none of the libraries.
Testing with libdbus 1.6.0 on Debian Testing failed because the
lib changed some behavior: instead of looking up the owner of
a certain bus name immediately, it now does that when invoking
a method. Therefore the check for "have connection" in NetworkManager
and ConnMan client code was too simplistic and missed the fact that
both were not usable, causing the server to assume that HTTP was down
while in reality it should have assumed it to be up.
