This addresses two different warnings from Fedora Rawhide:
/srv/runtests/work/sources/syncevolution/src/syncevo/SyncContext.cpp: In member function 'std::string SyncEvo::XMLFiles::get(SyncEvo::XMLFiles::Category)':
/srv/runtests/work/sources/syncevolution/src/syncevo/SyncContext.cpp:2390:28: error: loop variable 'entry' of type 'const StringPair&' {aka 'const std::pair<std::__cxx11::basic_string<char>, std::__cxx11::basic_string<char> >&'} binds to a temporary constructed from type 'std::pair<const std::__cxx11::basic_string<char>, std::__cxx11::basic_string<char> >' [-Werror=range-loop-construct]
2390 | for (const StringPair &entry: m_files[category]) {
| ^~~~~
/srv/runtests/work/sources/syncevolution/src/syncevo/SyncContext.cpp:2390:28: note: use non-reference type 'const StringPair' {aka 'const std::pair<std::__cxx11::basic_string<char>, std::__cxx11::basic_string<char> >'} to make the copy explicit or 'const std::pair<const std::__cxx11::basic_string<char>, std::__cxx11::basic_string<char> >&' to prevent copying
This fails because StringPair has non-const members. By using "auto",
we get rid of the need to define and pick exactly the right type.
/srv/runtests/work/sources/syncevolution/src/syncevo/SyncConfig.cpp: In member function 'void SyncEvo::SyncConfig::removeSyncSource(const string&)':
/srv/runtests/work/sources/syncevolution/src/syncevo/SyncConfig.cpp:2552:36: error: loop variable 'peer' creates a copy from type 'const string' {aka 'const std::__cxx11::basic_string<char>'} [-Werror=range-loop-construct]
2552 | for (const std::string peer: m_tree->getChildren(m_contextPath + "/peers")) {
| ^~~~
/srv/runtests/work/sources/syncevolution/src/syncevo/SyncConfig.cpp:2552:36: note: use reference type to prevent copying
2552 | for (const std::string peer: m_tree->getChildren(m_contextPath + "/peers")) {
| ^~~~
| &
We could have used "auto" also instead of "std::string", but here it
doesn't save that much typing and is more readable. We just have to
use a reference.
Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
Having to specify the type of an iterator is annoying and does not
really add clarity to the code. With C++11 we can use "auto"
instead and in some cases remove helper typedefs.
Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
We can use std::shared_ptr and std::function instead now.
Lambdas are usually a better alternative to boost/std::bind. The
downside is the need to explicitly specify parameters completely. When
inlining callbacks entirely with lambdas, duplication of that
parameter list can be avoided.
Whenever possible, use std::make_shared to construct objects that are
tracked by std::shared_ptr.
Some objects need a std::weak_ptr during object destruction. For that
we have to use our own implementation of std::enable_shared_from_this,
with a matching creator function. The additional benefit is that we
can get rid of explicit static "create" methods by making that create
function a friend.
Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
boost/foreach.hpp is no longer needed, range-based loops work
the same. With some helpers, even reverse iteration and
boost::make_split_iterator() can be handled the same way.
"auto" makes it possible to avoid explicitly spelling out the
expected type.
Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
The previous approach made FileConfigTree more complex than necessary.
Having an abstract ConfigTree::getRootPath() with undefined behavior
is bad design.
The code was had undesiredable side effects: inside a peer config,
another "peers" directory was created because FileConfigTree didn't
know whether creating that dir was required or not.
Now all of the complexity is in SyncConfig, which arguably knows
better what the tree stands for and how to use
it.
Commit 006bcf26 introduced a more versatile replacement for
FileConfigNode, but didn't switch over old code to the replacement
because of a code freeze. Switching now.
Also use the hash variant in VolatileConfigNode, because it is more
efficient by dropping the (in that case irrelevant) order of
properties.
Removed all the "using namespace std;" statements from the header
files and made corrosponding changes. Where needed added "using
namespace std;" statement to the implementation files.
Reloading properties of a node or a complete tree is necessary
when some other operation modified the background storage. One
use cases is rewriting .ini files during a migration while the
a SyncConfig has an active config tree for that config.
This is not a general solution to the problem of detecting
changes to the background store. Reloading must be triggered by
the code which made the change there.
This is necessary for local sync, which has to add per-peer file
config nodes in a location which is outside of the non-peer context
accessed by the ConfigTree.
Adding these instances is necessary for two reasons:
- ConfigTree::flush() must also flush these additional nodes.
- Temporary settings made for sources are only found if the
corresponding nodes can be found via the ConfigTree.
Running a sync with SyncEvolution > 1.0.99.6 and a configuration created with
SyncEvolution <1.0 works once, then fails with "No configuration for ... found"
("[SyncEvolution] Problem after upgrade - old-style config not found in 1.1").
The root cause is this commit:
commit 1b62240708
Author: Patrick Ohly <patrick.ohly@intel.com>
Date: Mon Aug 2 14:27:48 2010 +0200
command line: allow creating contexts without peers
That patch incorrectly creates the "peers" directory also for configs
without a context. The bug was introduced because FileConfigTree only had
an ambiguously named boolean. Fixed by distinguishing behavior based on
the SyncContext enum.
When configuring a context for local sync, no peer is needed
inside that context. Therefore "--configure" without "--template"
is valid, because the templates only differ in peer-specific
properties.
This patch uses the "SyncEvolution" template as automatic fallback
for context configs. A change to FileConfigTree ensures that the
"peers" directory is created, as expected by the code which checks
for new-style configs.
This patch introduces additional semantic for SetConfig(update=False, {})
(= removing configuration) and SetConfig(update=False, {<some sources removed>}
(= removing sources): when the config view is for a specific peer, then
only properties specific to that peer are removed.
When the view is peer-independent, it removes the complete
configuration (including all peers) resp. removes the shared and
per-peer source settings in all peers.
Global properties are never removed. With this patch, they can be read
by selecting the "@default" (= "") config view. Previously, reading
this view was rejected unless it contained real config nodes.
ConfigTree::removeSubtree() was merged into ConfigTree::remove(). The
functions were clearly related. The tree remains usable in the revised
remove() call.
SyncConfig::remove() did not work since the introduction of the "nodes
not in use" check in FileConfigTree::reset(). SyncConfig has to drop
all references to config nodes before FileConfigTree::remove() can do
its job. This is done via a new SyncConfig::makeVolatile().
test-dbus.py was updated to cover the new remove/clear
semantic. Because clearing sources with the "dummy-test-for-config"
view only removes some source properties, the remaining ones are still
reported in testClearConfigSources, breaking it. The test is
superseeded by the new tests and thus was removed.
I believe the functions would fail in this situation:
- entity A holds a reference to a node
- entity B calls clearNodes() or reset()
Because entity A still holds a reference, the node continues
to exist and might get flushed again to disk, recreating it
with old or inconsistent content.
The situation does not occur right now the way how we use
the file config, but it might happen once we allow concurrent
sessions.
This patch adds a check that a node really will be deleted
before removing it from the cache.
This patch makes the configuration layout with per-source and per-peer
properties the default for new configurations. Migrating old
configurations is not implemented. The command line has not
been updated at all (MB #8048). The D-Bus API is fairly complete,
only listing sessions independently of a peer is missing (MB #8049).
The key concept of this patch is that a pseudo-node implemented by
MultiplexConfigNode provides a view on all user-visible or hidden
properties. Based on the property name, it looks up the property
definition, picks one of the underlying nodes based on the property
visibility and sharing attributes, then reads and writes the property
via that node. Clearing properties is not needed and not implemented,
because of the uncertain semantic (really remove shared properties?!).
The "sync" property must be available both in the per-source config
(to pick a backend independently of a specific peer) and in the
per-peer configuration (to select a specific data format). This is
solved by making the property special (SHARED_AND_UNSHARED flag) and
then writing it into two nodes. Reading is done from the more specific
per-peer node, with the other node acting as fallback.
The MultiplexConfigNode has to implement the FilterConfigNode API
because it is used as one by the code which sets passwords in the
filter. For this to work, the base FilterConfigNode implementation must
use virtual method calls.
The TestDBusSessionConfig.testUpdateConfigError checks that the error
generated for an incorrect "sync" property contains the path of the
config.ini file. The meaning of the error message in this case is that
the wrong value is *for* that file, not that the property is already
wrong *in* the file, but that's okay.
The MultiplexConfigNode::getName() can only return a fixed name. To
satisfy the test and because it is the right choice at the moment for
all properties which might trigger such an error, it now is configured
so that it returns the most specific path of the non-shared
properties.
"syncevolution --print-config" shows errors that are in files. Wrong
command line parameters are rejected with a message that refers to the
command line parameter ("--source-property sync=foo").
A future enhancement would be to make the name depend on the
property (MB#8037).
Because an empty string is now a valid configuration name (referencing
the source properties without the per-peer properties) several checks
for such empty strings were removed. The corresponding tests were
updated resp. removed. Instead of talking about "server not found",
the more neutral name "configuration" is used. The new
TestMultipleConfigs.testSharing() covers the semantic of sharing
properties between multiple configs.
Access to non-existant nodes is routed into the new
DevNullConfigNode. It always returns an empty string when reading and
throws an error when trying to write into it. Unintentionally writing
into a config.ini file therefore became harder, compared with the
previous instantiation of SyncContext() with empty config name.
The parsing of incoming messages uses a SyncContext which is bound to
a VolatileConfigNode. This allows reading and writing of properties
without any risk of touching files on disk.
The patch which introduced the new config nodes was not complete yet
with regards to the new layout. Removing nodes and trees used the
wrong root path: getRootPath() refers to the most specific peer
config, m_root to the part without the peer path. SyncConfig must
distinguish between a view with peer-specific properties and one
without, which is done by setting the m_peerPath only if a peer was
selected. Copying properties must know whether writing per-specific
properties ("unshared") is wanted, because trying to do it for a view
without those properties would trigger the DevNullConfigNode
exception.
SyncConfig::removeSyncSource() removes source properties both in the
shared part of the config and in *all* peers. This is used by
Session.SetConfig() for the case that the caller is a) setting instead
of updating the config and b) not providing any properties for the
source. This is clearly a risky operation which should not be done
when there are other peers which still use the source. We might have a
problem in our D-Bus API definition for "removing a peer
configuration" (MB #8059) because it always has an effect on other
peers.
The property registries were initialized implicitly before. With the
recent changes it happened that SyncContext was initialized to analyze
a SyncML message without initializing the registry, which caused
getRemoteDevID() to use a property where the hidden flag had not been
set yet.
Moving all of these additional flags into the property constructors is
awkward (which is why they are in the getRegistry() methods), so this
was fixed by initializing the properties in the SyncConfig
constructors by asking for the registries. Because there is no way to
access them except via the registry and SyncConfig instances (*), this
should ensure that the properties are valid when used.
(*) Exception are some properties which are declared publicly to have access
to their name. Nobody's perfect...
This patch introduces code changes for the new layout without actually
using it yet. Therefore all existing tests for the older layout
still pass. The new meaning of the former "server name" is introduced:
- A plain string now refers to a peer configuration (can be client
or server).
- The @ sign allows selecting a specific context. Different contexts
have independent sets of local sources and peer definitions.
- An empty peer name selects a view on the configuration which contains
no peer-specific properties. Not fully implemented yet.
The FileConfigTree is instantiated with a root which is one level high
up compare to before this patch (for example,
"~/.config/syncevolution" instead of
"./config/syncevolution/scheduleworld") and then config files include
that dropped level in their relative path name
("scheduleworld/config.ini" instead of "config.ini"). This allows
accessing the global properties in
"~/.config/syncevolution/config.ini" and will be used to move peers
further down the hierarchy
("~/.config/syncevolution/peers/scheduleworld/config.ini").
To keep the output of "--print-servers" consistent, the FileConfigTree
gets another parameter which identifies the subset of the larger tree
that is referenced by this FileConfigTree instance.
One side effect of this change is that FileConfigTree instances are no
longer completely separate. Something to keep in mind when
instantiating SyncContext multiple times (MB#8006).
Code may no longer make assumptions in which config node a property is
stored. This is determined by the new getNode() calls based on the
property attributes (hidden, sharing). The new layout is represented as
a set of config nodes. Older layouts use the same set of nodes with
identical instances assigned to them, if they don't really have separate
files for each of them.
SyncSourceNodes no longer grants direct access to the nodes, to catch
code which incorrectly access a specific node directly. For the same
reason the name of the nodes changed.
Code which needs access to all hidden or all visible properties now
does this via a config node returned by getProperties(). Currently
this is identical to the underlying nodes. Once the new layout is
active, this node will act as a multiplexer which gathers properties
from all underlying nodes when reading and picks the right one when
writing.
The "change ID" parameter for sources has been obsolete for a while
and was removed.
Reorganized the property registration so that it is a bit easier
to see which properties are hidden and which use non-default sharing.
The default sharing is "no sharing".
Some other code was improved while touching it:
- removed useless visibility[] array in favor of a i != 0 check in
SyncConfig::copy()
- added default parameters to save/checkPassword() methods
- some constness definition changes
- Property::getProperty() is a virtual call which could only be
overloaded in one case because the constness was wrong; now
getProperty() always returns the string value and getPropertyValue()
some other kind of representation of it, depending on the
class.
- ConstSyncSourceNodes is based on SyncSourceNodes instead of duplicating
it, which simplifies the implementation.
The simplified SyncSourceAdmin API changed slightly: instead of passing
a pointer to the source's SyncSourceNodes, the default nodes are now
found via the SyncSource pointer. For callers this is a bit less
work and it is more general.
Previous impl removes necessary meta information, such
as global synthesis and source meta.
This patch is only to remove un-set sources and don't remove
meta information for global synthesis and sources which aren't be
removed
Previously, sync failed because the datastore configuration specified
no means of storing the admin data (single chunk of text and local ID/remote ID
mapping). This could have been added by configuring the SDK_textdb as
<plugin_admin_module>.
Instead this patch implements the functionality inside SyncEvolution,
using a new ".server.ini" config node for the mapping and an
internal "adminData" property for the admin data text chunk. This is
more natural because it keeps the data under our control.
Authentication is configured via <simpleauthuser/pw>. In contrast to
using the SDK_textdb, this allows choosing the username and password.
Ultimately this should also be done inside SyncEvolution, to avoid
writing the username/password into the XML (unsolved encoding issue)
and into log files (privacy issue).
The intention is to get rid of the historic and inconsistent
naming of some classes and their corresponding files:
* EvolutionSyncClient = class derived from Funambol's SyncClient,
* SyncEvolutionConfig = SyncEvolution's config
With the strict 'namespace SyncEvo' and the syncevo/ path prefix for
most header files it is no longer necessary to have "SyncEvolution" or
"Evolution" in the names. This patch thus renames as follows:
EvolutionSyncClient => SyncContext
EvolutionSmartPtr => SmartPtr
SyncEvolutionCmdline => Cmdline
SyncEvolutionConfig => SyncConfig
SyncEvolutionUtil => util
The former EvolutionSyncClient always had a role that went beyond just
running a sync, for example it also provided config access. With the
upcoming server support it also won't be just a client. Thus the new
name "SyncContext".
The 'syncevo/' prefix is used throughout the code now.
removed whenever the prefix made it clear that the file belongs
to SyncEvolution. This helps finding incorrect include paths.
Quotes should be used exclusively for SyncEvolution files which don't
have a specific prefix yet (test.h, config.h) to help identifying
them.
Added syncevo/declarations.h, which has
This is now used for all SyncEvolution source files, except
for the GTK UI, which is written in plain C. In the library
it helps to avoid name clashes.
The reason for using defines instead of spelling out "namespace SyncEvo"
is twofold:
1. if that should ever become necessary, it is easier to
rename the namespace via configure options by changing
the define
2. editors don't indent the whole file content
