folks and/or SyncEvolution leak some small amounts of memory,
as reported by valgrind during testing. Ignore these analyzed
problems for now, fix them later.
g++ 4.5 on Debian Wheezy fails to recognize that the boolean variable
gets initialized before use. Keep that compiler happy by explicitly
initializing the variables it complains about.
Renaming "listing" to "acessing" broke testPrintDatabases (because
it needs to suppress that error for KDE, which is not running
during D-Bus testing) and introduced a typo.
These operations manipulate the system address book. To avoid
unexpected data loss when a developer runs the script in his
normal environment, check that testpim.py gets run like this:
XDG_CONFIG_HOME=`pwd`/temp-testpim/config \
XDG_DATA_HOME=`pwd`/temp-testpim/local/cache \
PATH=<SyncEvolution install path>/bin:<SyncEvolution install path>/libexec:$PATH
<path to SyncEvolution source>/test/dbus-session.sh \
<path to SyncEvolution source>/src/dbus/server/pim/testpim.py
This will use temp-testpim in addition to temp-testdbus in the current
directory for temporary files.
Adding depends on conversion from D-Bus into a GHashSet of persona
details.
Modifying uses the same GHashSet as intermediate representation, but
then has to look up the Persona and modify its properties. Currently
each property write triggers a write to EDS, which is inefficient and
worse, deadlocks EDS when many properties are set (see
https://bugzilla.gnome.org/show_bug.cgi?id=652659). As a partial
solution, SyncEvolution checks that a property is really different
before setting it. This is good enough to get a few properties changed
without running into the deadlock. But ultimately this needs to be
improved in folks and fixed in EDS.
The nickname cannot be set or modified. Depends on a folks change, see
https://bugzilla.gnome.org/show_bug.cgi?id=686695
Removing a contact shares the "lookup persona" code with modifying.
All three operations can only be done once the EDS backend is loaded
and the system address book is ready. The actual execution therefore
has to be delayed, which requires making these operations
asynchronous in IndividualAggregator.
Grant access to stored Result callbacks, needed because PIM manager
needs to report failures and can do that more efficiently when keeping
a copy of just the error callback, instead of adding yet another
boost::bind on top of Result.
Also add factory methods which wrap the D-Bus result object in a
Result instance of the right type. Also needed for PIM manager.
Only tested as part of fixed database content. The only difference
(and potential bug) in live views is that the refined filter must be
remembered by the FilterView (which it does, see the source code).
Instead of always stopping, first check that there are no views in use
by a client. This can be determined based on the use count of the full
view.
For testing this, an internal, undocumented Manager.IsRunning() D-Bus
method gets added.
While writing the corresponding test, it became obvious that setting the sort
order had the undesired side effect of starting up folks. Aggregation still
didn't run (that gets started by ViewResource), but even just having folks
running is too much for a syncevo-dbus-server that is just used for syncing.
The "sort" property in the ini-style ~/.config/config/pim-manager.ini
file stores the string persistently. XDG_CONFIG_ROOT and HOME are
checked to find the file.
Changing the sort order only has an effect if the new order is valid.
Most of the tests are executed with "sort =" (empty), which picks the
simpler builtin sorting from FullView, as before. Other tests check
that the initial content of the .ini file is used and that it gets
modified by SetSortOrder().
Lookup and search are different: the former is based on a valid
number, the later on user input.
A lookup can compare normalized numbers including the country code, to
ensure that the lookup is exact and does not mismatch numbers from
different countries. Heuristics like suffix matching do not do this
correctly in all cases.
A search is based on user input, which might contain just some digits
in the middle of the phone number. The search ignores formatting in
both input and address book.
In both cases, alpha characters are treated as aliases for their
corresponding digit on a keypad.
The implementation uses Google's libphonebook to implement phone
number normalization and boost::locale to get the ISO-3199 country
code of the current locale.
Here some examples how the implementation works (from the tests),
based on de_DE.UTF-8 locale = DE = +49:
'any-contains 1234', 'any-contains 23', 'any-contains 12/34' find 1234.
'any-contains 5678' finds 56/78.
'any-contains 366227', 'any-contains +1-800-foobar' find +1-800-FOOBAR.
'phone +1800366227' matches +1-800-FOOBAR.
'phone +49897888', 'phone 0897888' all match 089/7888-99 and not
+189788899.
'phone +49897888000' does not match 089/7888-99.
The testFilterExisting covers the semantic of "any-contains",
in particular case-insensitiveness when applied to non-ASCII
characters and matching different properties of the individuals.
The testFilterLive covers adding, updating and removing contacts while
a view is active.
Covers the "any-contains" search, using boost::locale to get a
case-independent representation when asked for case-insensitive
searching. boost::icontains doesn't work as well because it can only
ignore the case of ASCII characters when dealing with UTF-8.
Telephone search still needs to be implemented.
At least in one case, folks emitted a removed + changed signal
when a contact was modified in EDS. Therefore implement the
merging of these two different change signals into on at the
D-Bus level.
Using a list of lists allows composing more complex searches.
In SyncEvolution, some basic searches are going to be implemented:
- caller ID lookup ("phone")
- text search ("any-contains")
Avoid printing the message in runUntil() every 0.1 second.
When running syncevo-dbus-server in a debugger, print to stdout so
that the developer can follow the progress of the Python test.
This is based on the API description of the different sort modes. It
does not make assumptions about how the view changes in reaction to
reordering, except that the final result must have the right indices
marked as changed.
The test data uses accents, which must be ignored when using the
de_DE.UTF-8 locale that is set in the environment before starting the
D-Bus server.
Clarify language, change fallback for missing full name to
concatenation of name components starting with first name and ending
with suffix. This is how Google Contacts sorts.
Change the --enable-dbus-service-pim parameter into one which takes a
parameter that specifies how locale-aware sorting and searching is to
be implemented. The default implementation uses boost::locale. It is
expected to get replaced or augemented by OEMs which want to implement
more complex sorting or searching (like ignoring Tussenvoegsel in the
Netherlands).
The LocaleFactory instance takes the current locale from the
environment. Making it and its users aware of locale changes at
runtime might be needed at some point but is not part of the API at
the moment.
The Manager class uses the factory to handle sorting and searching
requests coming in via D-Bus. Right now, that is not functional yet
because the boost::locale implementation is just a stub. It only
compiles and links.
FullView::setSortOrder is now functional.
Clean up view code a bit:
- All views delay populating their content until the caller asks for
it. For the FullView this will only happen once, so the caller must be
able to handle an already populated view, which was missing
in ViewResource. Still need a test for this.
- Use init(<smart pointer) consistently.
The set of active EDS databases is set after backends are loaded and
before the aggregator is asked to start. This ensures that we never
load data from inactive databases.
The patch depends on a functional implementation of the
folks_backend_set_persona_stores() method. The
folks_individual_aggregator_new_with_backend_store() is used although
it is not necessary at the moment, because there is only a single
instance of the FolksBackendStore class.
Tests cover running with no active address book and merging data from
two address books. The testMerge test case has several properties.
The one which causes folks to link it is X-JABBER and EMAIL (with
the patch for https://bugzilla.gnome.org/show_bug.cgi?id=685401).
Allow checks to change their result without changes inside the event
loop. Previously, the checks were only invoked if there were
events inside the event loop.
Necessary for checking events for a certain period of time.
Imports one vCard into EDS, then checks that reading it via folks and
PIM Manager yields the expected result. Uses pretty much all fields
supported by EDS. Not all of these need to be represented in the PIM
Manager API, see spec.
unittest.assertEqual() works for standard Python to dbus type
comparisons, but the output for mismatches is very unreadable. The
custom version which is now used everywhere reduces the dbus types to
their Python counterparts, which makes the error output much more
useful (diff with Python 2.7).
Also added an optional sortLists parameter which, if true, normalizes
the lists in the values. This is necessary for D-Bus APIs which return
lists without guaranteeing a specific order of the elements.
Previously, strings had to be passed into GDBus as std::string.
Now "const char *" also works. A NULL pointer is allowed and
is turned into an empty string.
Generalization of the code which defined GMainLoopCXX: assumes that
there are _ref/_unref() methods and defines an intrusive boost pointer
for the type.
Add two tests, one with just a single contact which gets added,
modified and removed and one which works with three different
contacts. The second test covers code paths like merging change events
before sending via D-Bus and re-ordering contacts when they get
renamed.
Changing the sort order and searching still need to be implemented and
tested.
Currently the tests only pass when none of the other EDS address books
contain data, because the aggregated view is not limited to the test
address books.
Avoid race condition after starting gdb by simply checking for the
presence of syncevo-dbus-server on the session bus. Previously the
output was checked, which failed due to race conditions and/or
unexpected output when setting breakpoints (not investigated further).
Once the test failed, print the failure or error immediately instead
of in the summary after gdb quit. This is information which might be
relevant for debugging the running syncevo-dbus-server.
After gdb quit, don't attempt to read the syncevo-dbus-server output
file (doesn't exit) and don't dump the D-Bus output to the screen
(usually too long to read without less, which is not possible when
doing interactive debugging). Instead point to the log file.
When TEST_DBUS_GDB=1 is set, syncevo-dbus-server gets run under gdb.
That gdb is passed the modified environment with a (potentially)
replaced HOME. In that case gdb needs an explicit -x parameter,
otherwise it won't find the user's .gdbinit.
The Timeout class now can also invoke the callback when the process is
idle. Pass a negative timeout to trigger that behavior. Most of the
code is similar to waiting for a certain amount of time, which is why
adding that functionality here makes sense despite the slightly
misleading "Timeout" class name.
It was observed that raising an exception in the signal handler did
not propagate to the calling Python code when it was currently in
loop.get_context().iteration().
This commit adds debug logging for timeouts and some utility methods
which avoid the problem by adding logging to the iteration()
call. Python then re-raises the exception in the D-Bus logging code.
This looks like a deficiency (bug?!) of the Python bindings for
iteration() which is merely worked around.
However, the utility methods may also be useful by their own right,
for example to avoid the awkward loop.run() + loop.quit() pairs.
Adding, updating (via GObject "notify") and removing contacts works.
FullView needs access to the FolksIndividualAggregator for
this.
The order of instantiating was changed so that Manager creates the
FullView as part of starting, instead of having an idle FullView as
envisioned earlier. Seems easier that way.
The CompareFormattedName class (previously only used for unit testing)
is used for sorting in ascending "last, first" by default. It is
convenient for testing, because the sort criteria are plain strings
which can be dumped in a debugger (in contrast to boost::locale
transformed strings). Later this default will have to be replaced with
a true locale aware sorting.
Local changes are delayed and merged as much as reasonably possible in
the Manager before sending them out via D-Bus, to avoid excessive
traffic.
This is done until the underlying view is stable
("quiesent"). Detecting that when reading from libfolks is difficult
(see GNOME feature request #684766). The current approach is to flush
pending changes when the process goes idle. In practice this prevents
merging multiple "add" changes, because the process goes idle between
imports from EDS - probably need further work, either in libfolks or
SyncEvolution (use a timeout after all, despite the extra latency?).
Interaction between delaying change signals and reading contacts must
be handled carefully. The client must be up-to-date when it receives
the results of the read request, and "modified" signals must be
enabled again for the data that was sent.
Normally invalidating the same contacts multiple times is prevented,
because folks sends "modified" signals pretty often (GNOME bug
modified FolksIndividual instances until the process is idle and all
major changes to the instance are (hopefully) done.
There is a slight race condition (client reads contact that was
already modified, SyncEvolution processes "modified" signal later) but
because the result is only that the client is asked to re-read the
contact, that is not a problem.
DBusObject_t is just an alias for std::string, so functional there's
no change. Adding getObject() which returns a const reference to it
allows the caller to use the value in D-Bus traits where the
difference between a plain string and an object path matters.
The tests were originally added to test compilation of code written
against the proposed APIs. After implementing them it turned out that
creating FolksIndividual instances and setting properties is not
supported by libfolks
(https://bugzilla.gnome.org/show_bug.cgi?id=684557), and thus testing
like this is impossible.
Keeping the tests anyway, without running them, because perhaps such
testing becomes possible at some point (mock FolksIndividual?!) and
the tests still cover compilation against the APIs.
The test covers aborting a running sync as well as a pending one; both
goes through different code paths.
The asserts only work in unittest from Python >= 2.7. For testing the
PIM Manager that is okay, because it needs a fairly recent software
elsewhere, too. Only test-dbus.py itself must still work with older
Python.
org._01.pim.contacts.Manager.Aborted may be useful if the caller of
SyncPeer() wants to distinguish between sync failures (BadStatus) and
intentionally aborted syncs.
