m_startTime and m_processName were not used, can be removed. Instead
add tracking of the parent logger and forward messages to that, to
allow combining LoggerSyslog with some other logger (for example,
stdout or redirection).
Together with storing the sort order persistently, this allows
restarting the daemon and have it create the same unified address book
again.
The set is stored as comma, space or tab separated list of words in
the value of the "active" property in the
~/.config/syncevolution/pim-manager.ini file.
Switch to the new EDSRegistryLoader from libsyncevolution. This will
allow sharing the ESourceRegistry with the EDS backend.
We use the asynchronous loading method, to avoid blocking the
server. This requires turning the search() method into an asynchronous
method. The actual code runs once the ESourceRegistry is available,
whether it is needed or not (keeps the logic simpler and minimizes
code changes).
To avoid reindenting the old code, the try/catch block and error
checking for the callback was added in a separate intermediate
function which then calls the old code.
Add a class, EDSRegistryLoader, which owns the creation of
ESourceRegistry and notifies other components synchronously or
asynchronously when the instance is ready.
Using the UID as part of file names gets more problematic when
allowing colons. Remove that character from the API and enforce
the format in the source code.
Bypass folks while it still loads contacts and search for a phone
number directly in EDS. This is necessary to ensure prompt responses
for the caller ID lookup.
Done with a StreamingView which translates EContacts into
FolksIndividuals with the help of folks-eds = edsf.
Combining these intermediate results and switching to the final
results is done by a new MergeView class.
A quiescence signal is emitted after receiving the EDS results and
after folks is done. The first signal will be skipped when folks
finishes first. The second signal will always be send, even if
switching to folks did not change anything.
Together with an artificial delay before folks is considered done,
that approach make testing more reliable.
The PIM code and the EDS backend both need access to common classes
(like the smart pointers) and eventually will share one instance of
the ESourceRegistry.
Log "quiescence" signal. Avoid starting the initial search if "search
== None". Both will be needed for startup search testing.
While at it, use the "quiescence" signal to continue after the initial
search. Much faster than always waiting 5 seconds.
Move IndividualView, FullView and FilteredView into separate source
files. They are large enough to justify that and it'll avoid header
file dependency cycles.
Introduce a new View base class, which contains the "start"
functionality and a new name for debug messages.
The new StreamingView is based on it and will be used for reporting
results in an EDS query.
Avoid passing NULL IndividualCompare pointer to views. Instead make
the CompareFormattedName class available as the default compare
method.
While it is likely that there is a GError when adding a persona fails,
better check for it explicitly. The code would crash with a NULL
persona pointer.
Some finish functions must return more values than just the return
value of the finish function. They do that via return
parameters. Support that by first switching via the arity of the
finish function and then based on types of the parameters + return
value.
Up to five parameters for the finish function are now supported. To
minimize the number of different combinations, only two variantions
are supported once return values are involved:
- non-void return value, GError * as last parameter
- non-void return value, no GError
The special semantic of GError is preserved (owned by callback
wrapper). All other results must be freed by the C++ callback.
To have a sane ordering of results, return value and GError were
swapped, leading to the following ordering:
- return value of the _finish call, if non-void
- all return parameters of the _finish call, in the order
in which they appear there
- a GError is passed as "const GError *", with NULL if the
_finish function did not set an error; it does not have to
be freed.
This affects only one place, the handling of
folks_persona_store_add_persona_from_details in the PIM module.
A 'limit' search term with a number as parameter (formatted as string)
can be added to a 'phone' or 'any-contains' search term to truncate the
search results after a certain number of contacts. Example:
Search([['any-contains', 'Joe'], ['limit', '10']])
=> return the first 10 Joes.
As with any other search, the resulting view will be updated if
contact data changes.
The limit must not be changed in a RefineSearch(). A 'limit' term may
(but doesn't have to) be given. If it is given, its value must match
the value set when creating the search. This limitation simplifies the
implementation and its testing. The limitation could be removed if
there is sufficient demand.
Fix the order of changes to internal data structures and emitting
change signals so that the changes are completed first. That way, if
the change signals are hooked up to classes which call the view back,
those calls can be processed correctly.
The code for removing a contact did this incorrectly: it skipped
updating the indices when the removed contact wasn't included in the
filtered view, although indices in the parent changed.
Due to not mapping the local index in the view to the parent's index,
refining only worked in views where parent and child had the same
index for the contacts in the search view.
Make the tests more complex to trigger this bug and fix it by adding
the required indirection.
Also include the system address book in testing. May matter because
folks considers it the primary address book and thus has special
cases involving it.
This test creates three address books with (by default) 100 contacts
each and then selects different permutations of them. This found a bug
in folks where disabling address books had no effect (see
https://bugzilla.gnome.org/show_bug.cgi?id=689146, patch is pending
review).
Looking up by phone number spends most of its cycles in normalizing of
the phone numbers in the unified address book. Instead of doing that
work over and over again during the search, do it once while loading.
This has implications for several classes:
- IndividualData must be extended to hold the pre-computed values;
what kind of data can be stored there is currently hard-coded
in the LocaleFactory base class, to keep it simple and minimize
memory fragmentation.
- Internal contact changes and access must be based on IndividualData,
not FolksIndividual.
- FullView must know which LocaleFactory will pre-compute the data.
- Changing contacts may have no effect on sorting, but may lead
to different pre-computed data.
Looking up a phone number only once does not gain from this change, it
even gets slower (more memory intensive, less cache locality). Only
searching multiple times becomes faster.
Ultimately it would be best to store the normalized strings together
with the telephone number inside EDS when the contact gets
created.
Caching normalized telephone numbers will lead to two special cases:
- contact changed directly in EDS
- contact changed through folks
Cover both cases with extensions to testFilterLive and
testContactWrite.
The unit tests did not work because creating FolksIndividual
instances with attributes is not supported unless they come from
a specific folks backend.
Removing them in preparation of changing the API of
IndividualAggregator (pass LocaleFactory for pre-computing values).
folks and EDS do not support writing properties in parallel
(https://bugzilla.gnome.org/show_bug.cgi?id=652659). Must serialize
setting of modified properties.
Once that is done, we can also write photos ("is modified" check was
missing, now done with some code refactoring) and notes (which somehow
always end up being treated as "modified").
Set empty values if empty in D-Bus and non-empty in folks. Without
that step, properties that were removed via D-Bus would not get
removed inside folks.
We really must create empty sets. A NULL pointer is not accepted by
folks. Somehow it leads to internal "timeout" errors inside folks.
It can happen that Folks has non-empty roles details, but the
containes roles do not contain values that SyncEvolution exposes via
D-Bus. When checking whether "roles" must be added to the top-level
dictionary, the "non default" check must do a deep dive into the
content of the set, instead of just checking for non-empty (as for
other sets).
The problem is in the C++ D-Bus binding. If the method that gets bound
to D-Bus returns a value, that value is ignored in the signature:
int foo() => no out parameter
It works when the method is declared as having a retval:
void foo (int &result) => integer out parameter
This problem exists for both the libdbus and the GIO D-Bus
bindings. In SyncEvolution it affects methods like GetVersions().
The fix consists of always returning a type for the R return type in
the libdbus bindings. getReturn() returned "" because the type was not
a retval. In the GIO binding, the R type was not handled at all. Also
move some common code into a helper function.
This fixes the hotspot during populating the FullView content: moving
contacts around required copying IndividualData and thus copying
complex C++ structs and strings. Storing pointers and moving those
avoids that, with no lack of convenience thanks to boost::ptr_vector.
Reordering also becomes faster, because the intermediate copy only
needs to be of the pointers instead of the full content.
The new "checkpoints" split up the whole script run into pieces which
are timed separately, with duration printed to stdout. In addition,
tools like "perf" can be started for the duration of one phase.
Benchmarking is different from permanently reading and monitoring
changes. --read-all enables a single read operation once the initial
set of new contact IDs are known.
The signal was not sent reliably. Now it is guaranteed to be sent once
when a search has finished sending its initial results, and not
sooner.
This makes it possible to rely on the "Quiescence" signal when
checking whether the current data contains some contact or not.
When the unified address book (= FullView) was not running yet at the
time when a client wanted to search it, the unified address book was
not started and thus the search never returned results.
Will be tested as part of the upcoming testFilterQuiescence.
Running at full verbosity is much too slow when there many
contacts. Therefore adding:
--verbosity=VERBOSITY
Determine what is printed. 0 = only minimal progress
messages. 1 = also summary of view notifications. 2 =
also a one-line entry per contact. 3 = also a dump of
the contact. 4 = also debug output for the script
itself.
The --debug command line flag enables printing of the debug output
sent by SyncEvolution over D-Bus. To capture it in a timely manner,
the running method calls must be done asynchronously with a running
main loop.
This also solves the timeout problem more nicely and is closer to how
a real application should be written.
Use contact IDs in ReadContacts(). Finding the contacts in the view is
done with an (almost) brute force search: a linear search looks at all
contacts and compares against the requested ID. The only enhancement is
that searching starts at the index of the previous contact, which makes
it a bit more efficient when reading consecutively.
As first step, notifications and contact data include the new string
ID. It is a direct copy of the FolksIndividual ID. The next step is
to adapt the ReadContacts() implementation.
The change primarily affects notification buffering in the
Manager. Not only does it have to remember ranges, but also the
corresponding IDs.
The Python test now stores either a ID or a dictionary at each view
entry. This required changing all comparisons against None, the
previous value for "no data". The assertions in ContactsView now use
the unittest assertions because they produce nicer error output, and
these errors get logged via Python (to make them show up in the D-Bus
log together with the other logging of the notification callbacks).
As before, the ContactsView class is picked up as a test class with no
test members. Request that unittest skips it (will still show up in
testpim.py output, though).
This simplifies writing clients which want to read all new or modified
contacts. Previously, with just a range being used in ReadContacts(),
the client had to be prepared to re-issue reads when changes to the
range happened on the server before the request was processed.
The ViewAgent notifications all take IDs. For added contacts the need
is obvious. For modified contacts it comes from the situations where
one contact replaces another. This was already possible before, so the
comment about "temporarily appear at two different positions" merely
documents existing behavior. Removal notifications include the ID
primarily for consistency with the other notifications. It was also
already useful for debugging.
The actual implementation is the same as for vector, with D-Bus array
on the D-Bus side. Now the common code is in a template base class and
referenced by the traits for vector, list and deque.
No attempt is made to automatically pick that trait for other classes
which also could be treated like an ordered collection of items.
The filtered view did not check whether a parent's removed contact was
really part of the view before sending a removal signal for it. Was
not (and still isn't) found by unit tests because the expected signals
were sent and the tests don't wait for the superfluous additional
removal signals.
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.
--create-database was broken in combination with the final code in EDS
3.6 because it passed NULL for the UID to e_source_new_with_uid(),
which is considered an error by the implementation of that
method. Must use e_source_new() if we don't have a UID.
gee_collection_iterator_get() always transfers ownership. The caller
must g_free() (for strings) or unref (for GObjects) the returned value.
The revised GeeCollCXX does this automatically, with GObject being the default
(relies on the GObject intrusive pointer template classes) and "gchar *" a
special case in the template specialization of an utility traits class.
The usage is almost as before. However, because of the reliance on
SE_GOBJECT_TYPE, that macro must have been used for all types over which
is to be iterated. Requires moving some definitions into header files
where they can be shared.
The main motivation is that it allows other template classes to define a smart
tracking pointer for an arbitrary GObject-derived type. Adding a second flavor
of the class where the constructor with a plain pointer takes over ("steals")
the reference becomes easy.
The caveat is that the template is still unusable without the SE_GOBJECT_TYPE
macro for the type, because without the macro there is no suitable
intrusive_ptr_add_ref/release.
Using a template instead of expanding source code might also be slightly
more efficient regarding compile time. It allows setting breakpoints.
It would be nice to have a default implementation of that which the C++
compiler picks if and only if the pointer is derived from GObject.
Unfortunately that relationship is opaque to the compiler (struct starts
with GObject, but that does not enable any special relationship at the
semantic level). Perhaps play tricks based the common conventions for naming
that first instance ("parent", "parent_instance", "parent_object")?
The change is almost transparent for users of SE_GOBJECT_TYPE,
except for one thing: the defined CXX name is now a typedef instead
of a class. This prevents simple forward declarations, thus folks.h
had to include individual-traits.h.
