Binding _done() to self.start reads the *current* value of self.start
when _done() gets called. We need to bind to an inmuted local instance
instead to achieve the desired effect.
The destructor must be virtual, otherwise destructors in dervived
classes are not getting called when destructing via a downcasted
pointer. Should be relevant, but did not show up in valgrind?!
Klocwork complained about not checking the result. Depending on the
situation we now give up or use some kind of fallback (for example,
when logging).
SyncEvolution itself is not multithreaded, but utility libraries
might be, so better avoid the localtime() function and its global
state. May also be relevant when function A gets a tm * pointer,
calls function B which calls localtime() again, then A tries to
use its original value.
When realloc() returns NULL, the already allocated buffer remains
available. Previously we discarded (and leaked!) it. Now we track
whether that memory already contains parts of the next log message
and use that when realloc fails. The dgram will then get discarded
without reading the rest.
When reading from a stream, realloc will only be called once to
get the initial buffer. When that fails, we give up.
Klocwork warned about using a NULL starttime pointer in the case that
the transport is unknown. Not sure whether this can actually happen at the
moment, fix it anyway.
All of the jobs were allocated dynamically without freeing them
explicitly, instead relying on auto-deletion (enabled by default).
This behavior is surprising for SyncEvolution developers and Klocwork,
which expects explicit delete calls for each new. Better use
traditional RAII.
The advantage also is that each job gets deleted right away, instead
of waiting for another round of event processing, which may or may not
happen soon.
Qt has never been happy with the old code, reporting:
QDBusConnection: session D-Bus connection created before QCoreApplication. Application may misbehave.
This error message became visible again after some recent logging
changes and broke tests, so better avoid it by testing for D-Bus
with a private connection.
Klocwork correctly reported that the underlying implementation in
libsynthesis must not be called with NULL pointers. Therefore always
allow it to set the return values, even if our caller was not
interested.
The old code triggered a Klocwork warning about the missing self check
in the copy operator. This was not a real problem because of the
temporary instance from which m_error got taken, but that was not
obvious even to a human.
Better use standard coding practices:
- skip the operation when assigning to self
- reuse the set() operation
- return reference instead of copy (probably a bug!)
The code tried to transfer ownership of the GDBusMethodInfo and
GDBusSignalInfo arrays to the refcounted GDBusInterfaceInfo instance,
but that does not work because g_dbus_interface_info_unref() does not
free those arrays.
Let's simplify this so that all memory, including the GDBusInterfaceInfo
instance, is owned by DBusObjectHelper and kept around as long as the
object is active.
The stack-allocated GDBusInterfaceVTable happened to work in practice,
but the API description does not say whether
g_dbus_connection_register_object() makes a copy, so better keep that
around longer, too.
Finally, clean up checking for "is activated" to use only
m_connId. m_activated was redundant and the method resp. signal
pointers are now always non-NULL.
Without the explicit TYPE=INTERNET, email addresses sent to a Nokia
e51 were not shown by the phone and even got lost eventually (when
syncing back?).
This commit ensures that the type is set for all emails sent to any
Nokia phone, because there may be other phones which need it and
phones which don't, shouldn't mind. This was spot-checked with a N97
mini, which works fine with and without the INTERNET type.
This behavior can be disabled again for specific Nokia phones by
adding a remote rule which sets the addInternetEmail session variable
to FALSE again.
Non-Nokia phones can enable the feature in a similar way, by setting
the variable to TRUE.
Compilation takes a lot longer when using -O2 and the result
is less debugger friendly, so filter out the flag. This is
relevant when building a release where the flag is set for the
rest of the compilation.
Complaining about "ERROR SUMMARY" when using valgrind is a false
positive. Ignore that text by making it lower case before searching. Other
attempts based on regex matching somehow failed (UTF-8 encoding error?!).
Some peers have problems with meta data (CtCap, old Nokia phones) and
the sync mode extensions required for advertising the restart
capability (Oracle Beehive).
Because the problem occurs when SyncEvolution contacts the peers
before it gets the device information from the peer, dynamic rules
based on the peer identifiers cannot be used. Instead the local config
must already disable these extra features in advance.
The "SyncMLVersion" property gets extended for this. Instead of just
"SyncMLVersion = 1.0" (as before) it now becomes possible to say
"SyncMLVersion = 1.0, noctcap, norestart".
"noctcap" disables sending CtCap. "norestart" disables the sync mode
extensions and thus doing multiple sync cycles in the same session
(used between SyncEvolution instances in some cases to get client and
server into sync in one session).
Both keywords are case-insensitive. There's no error checking for
typos, so beware!
The "SyncMLVersion" property was chosen because it was already in use
for configuring SyncML compatibility aspects and adding a new property
would have been harder.
In the previous attempt (commit a0375e) setting the <syncmodeextensions>
option was only done on the client side, thus breaking multi-cycle
syncing with SyncEvolution as server. On the server side the option
shouldn't be needed (the server never sends the extensions unless
the client did first), but for symmetry and testing reasons it makes
sense to also offer the options there.
Previously, the database field was interpreted as a Collection ID. This adds
logic to allow the database to be interpreted as a folder path. The logic is:
1) If the database is an empty string, pass it through (this is the most
common case as it is interpreted as "use the default folder for the
source type").
2) If the database matches a Collection ID, use the ID (this is the same as
the previous behaviour).
3) If the database matches a folder path name, with an optional leading "/",
use the Collection ID for the matching folder.
4) Otherwise, force a FolderSync to get the latest folder changes from the
server and repeat steps 2 and 3
5) If still no match, throw an error.
Steps 2 and 3 are in the new function lookupFolder. The remaining logic has
been added to the open function. Note that the result is that m_folder (and
hence getFolder()) are always either empty or a Collection ID -- that is as
before so the sync logic itself is unchanged.
The command line swallowed errors thrown by the backend while listing
databases. Instead it just showed "<backend name>: backend failed". The goal
was to not distract users who accidentally access a non-functional backend.
But the result is that operations like --configure or --print-databases could
fail without giving the user any hint about the root cause of the issue.
Now the error explanation in all its gory details is included.
For example, not having activesyncd running leads to:
INFO] eas_contact: backend failed: fetching folder list:
GDBus.Error:org.freedesktop.DBus.Error.ServiceUnknown: The name
org.meego.activesyncd was not provided by any .service files
And running activesyncd without the necessary gconf keys shows up as:
[INFO] eas_contact: backend failed: fetching folder list:
GDBus.Error:org.meego.activesyncd.Error.AccountNotFound: Failed to find
account [syncevolution@lists.intel.com]
Both of these happened when trying to use the "list database" ActiveSync
patches in the nightly test setup...
A new method, findCollections, fetches the folder list from the server and
creates two maps:
m_collections - store all the information about each collection (folder),
indexed by server collection ID
m_folderPaths - map full folder paths to collection IDs
getDatabases uses this data to returns the folder path, collection ID and a
flag indicating if the folder is the default for that type.
Note 1: getDatabases always asks activesyncd to update the folder list from the
server in order to return up to date information.
Note 2: this depends on a new libeasclient routine:
eas_sync_handler_get_folder_list
A quick-and-dirty solution for enabling phone number summaries when
creating contact databases in the PIM Manager: let the EDS backend
recognize the special UIDs used by the PIM Manager and then hard-code
the minimal set of summary fields and indexed fields which allow
executing the E_CONTACT_TEL, E_BOOK_QUERY_EQUALS_NATIONAL_PHONE_NUMBER
query quickly.
A proper solution would use a new EDS function for parsing ESource
defaults from a string and then providing these defaults to the
backend from the PIM Manager.
Also note that configuring the EDS system address book must be covered
elsewhere, because it wouldn't be correct for SyncEvolution as only
one of many clients to change the configuration of that.
To enable the special support, add the following section to
share/evolution-data-server-3.6/rw-sources/system-address-book.source:
[Backend Summary Setup]
SummaryFields=phone
IndexedFields=phone,phone
This patch adds new function calls to code shared by syncecal and syncebook,
so we have to add libebook-contacts to both to avoid link errors.
This reverts commit 2735273ec60b289c5ec2c49f3eacb9d7d04d5ea1.
With this patch, setting up ActiveSync fails with Google as server.
Needs further investigation.
Also note the explicit g_object_unref() and EASFolderUnref - these
should get replace with SE_GOBJECT_TYPE smart pointers.
This reverts commit 7327b23a4dd31abdc9596916743892402bcffe0c.
Depends on 273527 "ActiveSync: added getDatabases support for fetching
folder list" which has to be reverted.
The new ReplaceSearch is more flexible than RefineSearch. It can
handle both tightening the search and relaxing it. The downside of it
is the more expensive implementation (must check all contacts again,
then find minimal set of change signals to update view).
Previously, a search which had no filter set at all at the begining
could not be refined. This limitation of the implementation gets
removed by always using a FilteredView, even if the initial filter is
empty.
When the parent of a FilteredView emits signals, it is not necessarily
always in a consistent state and the FilteredView must not invoke
methods in the parent. Stressing the FilteredView by using it in tests
originally written for the FullView showed that the filling up a view
violated that rule and led to contacts being present multiple
times. Fixed by delaying the reading from the parent into either an
idle callback or the parent's quiescence signal processing, whichever
comes first.
Added priorities and proper tracking of "activated" state. The latter
depends on having the Timeout instance valid when the callback
returns, therefore the Server must delay the deletion of the instance
until the callback is done executing.
Priorities are needed to order different tasks in the PIM manager
correctly.
The "activated" state is something that the PIM manager needs to track
reliably, which had to be done by every callback. If a callback forgot
to do that, in the worst case this might have removed an entirely
unrelated source when the tag got reused in the meantime.
Previously, the database field was interpreted as a Collection ID. This adds
logic to allow the database to be interpreted as a folder path. The logic is:
1) If the database is an empty string, pass it through (this is the most
common case as it is interpreted as "use the default folder for the
source type").
2) If the database matches a Collection ID, use the ID (this is the same as
the previous behaviour).
3) If the database matches a folder path name, with an optional leading "/",
use the Collection ID for the matching folder.
4) Otherwise, force a FolderSync to get the latest folder changes from the
server and repeat steps 2 and 3
5) If still no match, throw an error.
Steps 2 and 3 are in the new function lookupFolder. The remaining logic has
been added to the open function. Note that the result is that m_folder (and
hence getFolder()) are always either empty or a Collection ID -- that is as
before so the sync logic itself is unchanged.
A new method, findCollections, fetches the folder list from the server and
creates two maps:
m_collections - store all the information about each collection (folder),
indexed by server collection ID
m_folderPaths - map full folder paths to collection IDs
getDatabases uses this data to returns the folder path, collection ID and a
flag indicating if the folder is the default for that type.
Note 1: getDatabases always asks activesyncd to update the folder list from the
server in order to return up to date information.
Note 2: this depends on a new libeasclient routine:
eas_sync_handler_get_folder_list
If phone number search is possible, then the direct search in EDS now
uses the more accurate E_BOOK_QUERY_EQUALS_NATIONAL_PHONE_NUMBER
comparison, with the E164 formatted caller ID as value to compare
against. That value includes the current country code.
For testing purposes, setting SYNCEVOLUTION_PIM_EDS_SUBSTRING forces
the usage of the traditional suffix match. This is used to test both
the old and new flavor of testFilterStartupRefine.
FilterStartupRefineSmart is the one which uses phone number matching.
When available, the pre-computed E164 number from EDS will be used
instead of doing one libphonebook parser run for each telephone number
while reading. Benchmarking showed that this parsing was the number
one hotspot, so this is a considerable improvement.
Google became even more strict about checking REV. Tests which
reused a UID after deleting the original item started to fail sometime
since middle of December 2012.
To fix this, tests must differentiate reuse of an item by adding a suffix
("-A", "-B", etc.). If CLIENT_TEST_UNIQUE_UID has a value >= 2, that suffix
will be used when mangling the input item. Otherwise the suffix is ignored
and nothing changes.
For testing Google, CLIENT_TEST_UNIQUE_UID=2 is used.
The backends had SYNCEVOLUTION_LIBS in their _DEPENDENCIES entries,
which is wrong because SYNCEVOLUTION_LIBS must include -lrt (which
can't be a dependency).
Fixed by depending on libsyncevolution.la directly.
The library might be used, for example when the PIM Manager is part of
the server, so better add it. Avoids hard linker errors on some
systems when it is used and not specified, which is better than given
one extraneous library that -as-needed can take care of.
The code which removed a photo from a contact did it by creating a
g_file_icon() with a NULL GFile. This is invalid and caused warnings
from glib. Eventually it must have passed NULL to folks, which is what
the new code does directly.
Don't let libphonenumber write to stdout. Instead redirect into
SyncEvolution logging and manage severity level. For example,
previously parsing errors of phone numbers were logged as [ERROR] by
libphonenumber. Now that appears as "phonenumber error: ...".
The previous macros assumed to be used inside the SyncEvo namespace
and had problems when used in a context where "Logger" was resolved to
something other than SyncEvo::Logger. Making that more explicit solved
that problem.
Use e_book_client_connect_direct_sync(), the official API, when
available. Support for e_book_client_new_direct() is still in the
code; it can be removed onces the 3.6 openismus-work branch adapts the
official API.
The new Bluez 5 API is the third supported API for doing PBAP
transfers. It gets checked first, then the PBAB backend falls back to
new-style obexd (file based, similar to Bluez 5, but not quite the
same) and finally old-style obexd (data transfer via D-Bus).
In contrast to previous APIs, Bluez 5 does not report the reason for a
failed PBAP transfer. SyncEvolution then throws a generic "transfer
failed" error with "reason unknown" as message.
When using more than 10 contacts (TESTPIM_TEST_ACTIVE_NUM), logging
and polling are turned towards causing less work by testpim.py itself.
TESTPIM_TEST_ACTIVE_RESPONSE can be used to give a maximum response
time in seconds, which gets checked by watchdog by calling
GetAllPeers().
This is relevant in two cases, data loaded before folks starts reading
from EDS (TESTPIM_TEST_ACTIVE_LOAD set) and folks getting change
notifications as SyncEvolution imports data into EDS (not set).
Related to FDO #60851. Currently SyncEvolution+Folks do not pass the
performance test.
The server must remain responsive at all times. The new Watchdog
class tests that by calling GetAllPeers() at regular intervals. It
can only be used in tests which do not block the event processing
in the Python script itself.
Once the watchdog runs, it must be removed when a test stops. The new
cleanup test member can be used to add cleanup code at runtime which
will be called when tearing down the test.
The D-Bus server has to respond to the entire request at once, which
makes it unresponsive. Needs to be avoided in the client, because
doing it on the server side is too complicated. The server would have
to start gathering results, but those results may become invalid again
in response to processing events before it can send them out.
Related to BGO #60851.
The \n in the testRead vCard was interpreted by Python instead of
being written into the file. Must mark the string as raw. Did not
affect the test itself, but the libebook vCard parser reported
warnings.
Track which changes need to be stored and how that gets processed. Was
useful for finding that modifying groups failed when the groups hadn't
actually changed.
