maryjane/syncevolution
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

PIM: introduce CreateConfig()

Browse source 
That SetPeer() allows modifying and creating a config leads to race
conditions when multiple clients want to create a config. The new
CreateConfig() avoids that by atomically checking that a config does
not exist yet and creating it.

SetPeer() is still available for backwards compatibility. It continues
to be used for modifying an existing config in TestContacts.testSync
to check the effect of the logging settings.
This commit is contained in:
Patrick Ohly 2013-02-12 08:42:04 +01:00
parent 3c0fee9536
commit b29ffb2da7
4 changed files with 82 additions and 12 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

47
src/dbus/server/pim/manager.cpp
View file

@ -45,6 +45,8 @@ static const char * const MANAGER_PATH = "/org/01/pim/contacts";
static const char * const MANAGER_IFACE = "org._01.pim.contacts.Manager";
static const char * const MANAGER_ERROR_ABORTED = "org._01.pim.contacts.Manager.Aborted";
static const char * const MANAGER_ERROR_BAD_STATUS = "org._01.pim.contacts.Manager.BadStatus";
static const char * const MANAGER_ERROR_ALREADY_EXISTS = "org._01.pim.contacts.Manager.AlreadyExists";
static const char * const MANAGER_ERROR_NOT_FOUND = "org._01.pim.contacts.Manager.NotFound";
static const char * const AGENT_IFACE = "org._01.pim.contacts.ViewAgent";
static const char * const CONTROL_IFACE = "org._01.pim.contacts.ViewControl";
@ -132,7 +134,8 @@ void Manager::init()
add(this, &Manager::search, "Search");
add(this, &Manager::getActiveAddressBooks, "GetActiveAddressBooks");
add(this, &Manager::setActiveAddressBooks, "SetActiveAddressBooks");
add(this, &Manager::setPeer, "SetPeer");
add(this, &Manager::modifyPeer, "SetPeer"); // The original method name, keep it for backwards compatibility.
add(this, &Manager::createPeer, "CreatePeer"); // Strict version: uid must be new.
add(this, &Manager::removePeer, "RemovePeer");
add(this, &Manager::syncPeer, "SyncPeer");
add(this, &Manager::stopSync, "StopSync");
@ -980,8 +983,21 @@ static void checkPeerUID(const std::string &uid)
}
}
void Manager::createPeer(const boost::shared_ptr<GDBusCXX::Result0> &result,
const std::string &uid, const StringMap &properties)
{
setPeer(result, uid, properties, CREATE_PEER);
}
void Manager::modifyPeer(const boost::shared_ptr<GDBusCXX::Result0> &result,
const std::string &uid, const StringMap &properties)
{
setPeer(result, uid, properties, SET_PEER);
}
void Manager::setPeer(const boost::shared_ptr<GDBusCXX::Result0> &result,
const std::string &uid, const StringMap &properties)
const std::string &uid, const StringMap &properties,
ConfigureMode mode)
{
checkPeerUID(uid);
runInSession(StringPrintf("@%s%s",
@ -989,7 +1005,7 @@ void Manager::setPeer(const boost::shared_ptr<GDBusCXX::Result0> &result,
uid.c_str()),
Server::SESSION_FLAG_NO_SYNC,
result,
boost::bind(&Manager::doSetPeer, this, _1, result, uid, properties));
boost::bind(&Manager::doSetPeer, this, _1, result, uid, properties, mode));
}
static const char * const PEER_KEY_PROTOCOL = "protocol";
@ -1020,7 +1036,8 @@ static std::string GetEssential(const StringMap &properties, const char *key,
void Manager::doSetPeer(const boost::shared_ptr<Session> &session,
const boost::shared_ptr<GDBusCXX::Result0> &result,
const std::string &uid, const StringMap &properties)
const std::string &uid, const StringMap &properties,
ConfigureMode mode)
{
// The session is active now, we have exclusive control over the
// databases and the config. Create or update config.
@ -1063,8 +1080,28 @@ void Manager::doSetPeer(const boost::shared_ptr<Session> &session,
if (protocol == PEER_PBAP_PROTOCOL ||
protocol == PEER_FILES_PROTOCOL) {
// Create or set local config.
// Create, modify or set local config.
boost::shared_ptr<SyncConfig> config(new SyncConfig(MANAGER_LOCAL_CONFIG + context));
switch (mode) {
case CREATE_PEER:
if (config->exists()) {
// When creating a config, the uid must not be in use already.
result->failed(GDBusCXX::dbus_error(MANAGER_ERROR_ALREADY_EXISTS, StringPrintf("uid %s is already in use", uid.c_str())));
return;
}
break;
case MODIFY_PEER:
if (!config->exists()) {
// Modifying expects the config to exist already.
result->failed(GDBusCXX::dbus_error(MANAGER_ERROR_NOT_FOUND, StringPrintf("uid %s is not in use", uid.c_str())));
return;
}
break;
case SET_PEER:
// May or may not exist, doesn't matter.
break;
}
config->setDefaults();
config->prepareConfigForWrite();
config->setPreventSlowSync(false);

21
src/dbus/server/pim/manager.h
View file

@ -103,13 +103,26 @@ class Manager : public GDBusCXX::DBusObjectHelper
/** Manager.SetActiveAddressBooks() */
void setActiveAddressBooks(const std::vector<std::string> &dbIDs);
/** Manager.SetPeer() */
void setPeer(const boost::shared_ptr<GDBusCXX::Result0> &result,
const std::string &uid, const StringMap &properties);
/** Manager.CreatePeer() */
void createPeer(const boost::shared_ptr<GDBusCXX::Result0> &result,
const std::string &uid, const StringMap &properties);
/** Manager.ModifyPeer() */
void modifyPeer(const boost::shared_ptr<GDBusCXX::Result0> &result,
const std::string &uid, const StringMap &properties);
private:
enum ConfigureMode {
SET_PEER,
CREATE_PEER,
MODIFY_PEER
};
void setPeer(const boost::shared_ptr<GDBusCXX::Result0> &result,
const std::string &uid, const StringMap &properties,
ConfigureMode mode);
void doSetPeer(const boost::shared_ptr<Session> &session,
const boost::shared_ptr<GDBusCXX::Result0> &result,
const std::string &uid, const StringMap &properties);
const std::string &uid, const StringMap &properties,
ConfigureMode mode);
public:
/** Manager.RemovePeer() */

17
src/dbus/server/pim/pim-manager-api.txt
View file

@ -216,10 +216,21 @@ Methods:
Sets the address books which contribute to the unified
address book.
void SetPeer(string uid, dict properties)
void CreatePeer(string uid, dict properties)
Adds or modifies a peer. Modifying a peer does *not* affect
any contact data which might be cached for it.
Creates a peer. Will fail with a
org._01.pim.contacts.Manager.AlreadyExists error if the uid
is already in use. To change the configuration of a peer,
remove and recreate it. This ensures that its data gets
removed, too.
[A ModifyPeer() might get added if there is demand for it
and the desired behavior (remove cached data or keep it?)
is better understood.]
As a backwards compatibility measure this method is
also available as SetPeer() with the semantic that the
config is created or modified automatically as needed.
void RemovePeer(string uid)

9
src/dbus/server/pim/testpim.py
View file

@ -414,6 +414,15 @@ END:VCARD(\r|\n)*''',
self.assertEqual(peers, self.manager.GetAllPeers(timeout=self.timeout))
self.assertEqual(expected, self.currentSources())
# PIM Manager must not allow overwriting an existing config.
# Uses the new name for SetPeer().
with self.assertRaisesRegexp(dbus.DBusException,
'org._01.pim.contacts.Manager.AlreadyExists: uid ' + uid + ' is already in use') as cm:
self.manager.CreatePeer(uid,
peers[uid],
timeout=self.timeout)
self.assertEqual('org._01.pim.contacts.Manager.AlreadyExists', cm.exception.get_dbus_name())
# TODO: work around EDS bug: e_source_remove_sync() quickly after
# e_source_registry_create_sources_sync() leads to an empty .source file:
# [Data Source]