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.
When running syncevo-dbus-server under valgrindcheck.sh,
the following happened occasionally:
- syncevo-dbus-server main thread quits, some threads keep running
=> ps shows the process as <defunct> with ppid = 1 = init
- valgrindcheck.sh notices that the process is done,
reports status and quits
- test-dbus.py fails to wait for the syncevo-dbus-server
process (because it is not the parent) and assumes that
the process is gone
At this point there is a lingering process which occupies the
well-known D-Bus name (= all further tests fail) and which
prevents unmounting the chroot.
It's unknown how the syncevo-dbus-server gets into that state.
Could be valgrind 3.7.0 or the kernel 3.4.28-2.20-xen.
As a workaround, let test-dbus.py collect the pids of all processed that it
couldn't wait for and send them SIGKILLs until that returns with "not
found".
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?!).
When starting the server fails, an exception gets thrown when
trying to determine its pid. This used to abort the whole
test script without recording much information. In particular
the server's output was not included.
Now the exception is caught, recorded as error and testing continues
with the next test.
This did not fix the root cause (a stuck process occupied the
D-Bus name) but at least it helped to identify the problem.
With all the interaction between Client::Sync, Client::Source and
CLIENT_TEST_SERVER it can be hard to figure out which test case file
gets used. Log that explicitly.
Because of Google issue with detached recurrences without parent
(http://code.google.com/p/google-caldav-issues/issues/detail?id=58)
and the SyncEvolution workaround (replacing RECURRENCE-ID with
X-SYNCEVOLUTION-RECURRENCE-ID) only one detached recurrence per UID
can be stored.
Removing the second modified recurrence from the test cases for
Google.
Somehow the second modified recurrence made its way back into the
test data while rewriting the generic eds_event.ics.
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.
See https://bugzilla.gnome.org/show_bug.cgi?id=694730
==18552== 25 bytes in 1 blocks are definitely lost in loss record 2,353 of
6,616
==18552== at 0x4C28BED: malloc (vg_replace_malloc.c:263)
==18552== by 0xADC3D40: g_malloc (gmem.c:159)
==18552== by 0xADDA3BB: g_strdup (gstrfuncs.c:364)
==18552== by 0x1146400B: get_string_cb (e-book-backend-sqlitedb.c:255)
==18552== by 0x854F1CE: sqlite3_exec (in
/usr/lib/x86_64-linux-gnu/libsqlite3.so.0.8.6)
==18552== by 0x114665A2: book_backend_sql_exec_real
(e-book-backend-sqlitedb.c:301)
==18552== by 0x1146666F: book_backend_sql_exec
(e-book-backend-sqlitedb.c:391)
==18552== by 0x11469FC6: e_book_backend_sqlitedb_get_revision
(e-book-backend-sqlitedb.c:3995)
==18552== by 0x1F90480E: e_book_backend_file_open
(e-book-backend-file.c:656)
==18552== by 0x1146C229: book_backend_open (e-book-backend-sync.c:397)
==18552== by 0x11473F26: operation_thread (e-data-book.c:292)
==18552== by 0xADE2181: g_thread_pool_thread_proxy (gthreadpool.c:309)
==18552== by 0xADE1964: g_thread_proxy (gthread.c:797)
==18552== by 0x8E34B4F: start_thread (pthread_create.c:304)
==18552== by 0xB90F70C: clone (clone.S:112)
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.
Adds the possibility to check the servers standard output
similar to its D-Bus log output. Both can now also be set
before invoking runTest() because that method no longer
sets the members.
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.
