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

command line: relaxed checking of config creation (BMC #14805) 

SyncEvolution used to insist on having a template when creating a
config from scratch. This was meant to address typos like "--configure
sheduleworld".  But when the command line contains enough information,
no template is needed.

The same logic was applied to configuring a source: if a source was not
among those defined by the template, configuring it was rejected as a
typo.

With this patch, there are two ways around these checks:
- --template none and/or
- specifying required properties on the command line

This patch also removes the special cases for "your SyncML server
account name/password". Username/password aren't always needed
(previous patch), nor are they always for a SyncML server anymore.
This commit is contained in:
Patrick Ohly 2011-03-23 12:14:31 +01:00
parent 377112b99d
commit 21dd62cd71
4 changed files with 308 additions and 43 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
README.rst
View File

@ -300,9 +300,15 @@ a list of valid values.
--configure|-c
Modify the configuration files for the selected peer and/or sources.
If no such configuration exists, then a new one is created using one
of the template configurations (see --template option). When
creating a new configuration and listing sources explicitly on the
of the template configurations (see --template option). Choosing a
template sets most of the relevant properties for the peer and the
default set of sources (see above for a list of those). Anything
specific to the user (like username/password) still has to be set
manually.
When creating a new configuration and listing sources explicitly on the
command line, only those sources will be set to active in the new
configuration, i.e. `syncevolution -c memotoo addressbook`
followed by `syncevolution memotoo` will only synchronize the
@ -310,6 +316,15 @@ a list of valid values.
When modifying an existing configuration and sources are specified,
then the source properties of only those sources are modified.
By default, creating a config requires a template. Source names on the
command line must match those in the template. This allows catching
typos in the peer and source names. But it also prevents some advanced
use cases. Therefore it is possible to disable these checks in two ways::
- use `--template none` or
- specify all required sync and source properties that are normally
in the templates on the command line (syncURL, backend, ...)
--run|-r
To prevent accidental sync runs when a configuration change was
intended, but the `--configure` option was not used, `--run` must be

314
src/syncevo/Cmdline.cpp
View File

@ -779,8 +779,10 @@ bool Cmdline::run() {
// Both config changes and migration are implemented as copying from
// another config (template resp. old one). Migration also moves
// the old config.
// the old config. The target configuration is determined by m_server,
// but the exact semantic of it depends on the operation.
boost::shared_ptr<SyncConfig> from;
boost::shared_ptr<SyncContext> to;
string origPeer;
if (m_migrate) {
if (!m_sources.empty()) {
@ -836,8 +838,14 @@ bool Cmdline::run() {
// rename on disk and point "from" to it
makeObsolete(from);
// modify the config referenced by the (possibly modified) m_server
to.reset(createSyncClient());
} else {
from.reset(new SyncConfig(m_server));
// m_server known, modify it
to.reset(createSyncClient());
if (!from->exists()) {
// creating from scratch, look for template
fromScratch = true;
@ -865,34 +873,32 @@ bool Cmdline::run() {
SyncConfig::splitConfigString(SyncConfig::normalizeConfigString(m_server), tmp, context);
}
from = SyncConfig::createPeerTemplate(peer);
list<string> missing;
if (!from.get()) {
m_err << "ERROR: no configuration template for '" << configTemplate << "' available." << endl;
// check if all obligatory sync properties are specified, if so, allow user
// to proceed with "none" template
ConfigProps syncProps = m_props.createSyncFilter(to->getContextName());
bool complete = true;
BOOST_FOREACH(const ConfigProperty *prop, SyncConfig::getRegistry()) {
if (prop->isObligatory() &&
syncProps.find(prop->getMainName()) == syncProps.end()) {
missing.push_back(prop->getMainName());
complete = false;
}
}
if (complete) {
from = SyncConfig::createPeerTemplate("none");
}
}
if (!from.get()) {
m_err << "ERROR: no configuration template for '" << configTemplate << "' available."
" Use '--template none' and/or specify relevant properties on the command line to create a configuration without a template." <<
" Need values for: " << boost::join(missing, ", ") <<
endl;
dumpConfigTemplates("Available configuration templates:",
SyncConfig::getPeerTemplates(SyncConfig::DeviceList()));
return false;
}
if (!from->getPeerIsClient()) {
// Templates no longer contain these strings, because
// GUIs would have to localize them. For configs created
// via the command line, the extra hint that these
// properties need to be set is useful, so set these
// strings here. They'll get copied into the new
// config only if no other value was given on the
// command line.
if (!from->getUsername()[0]) {
from->setUsername("your SyncML server account name");
}
if (!from->getPassword()[0]) {
from->setPassword("your SyncML server password");
}
} else {
// uncomment SyncURL, so that it can be shown by
// sync-ui
if (from->getSyncURL().size() == 0) {
from->setSyncURL ("input your peer address here");
}
}
}
}
@ -917,6 +923,21 @@ bool Cmdline::run() {
sources = m_sources;
}
// Also copy (aka create) sources listed on the command line if
// creating from scratch and
// - "--template none" enables the "do what I want" mode or
// - source properties apply to it.
// Creating from scratch with other sources is a possible typo
// and will trigger an error below.
if (fromScratch) {
BOOST_FOREACH(const string &source, m_sources) {
if (m_template == "none" ||
!m_props.createSourceFilter(to->getContextName(), source).empty()) {
sources.insert(source);
}
}
}
// Special case for migration away from "type": older
// SyncEvolution could cope with "type" only set correctly for
// peers. Real-world case: Memotoo config, context had "type =
@ -933,7 +954,6 @@ bool Cmdline::run() {
from->getConfigVersion(CONFIG_LEVEL_CONTEXT, CONFIG_CUR_VERSION) == 0) {
list<string> peers = from->getPeers();
peers.sort(); // make code below deterministic
boost::shared_ptr<SyncContext> to(createSyncClient());
BOOST_FOREACH(const std::string source, from->getSyncSources()) {
BOOST_FOREACH(const string &peer, peers) {
@ -979,7 +999,6 @@ bool Cmdline::run() {
// creates a SyncContext for m_server, with propert
// implementation of the password handling methods in derived
// classes (D-Bus server, real command line)
boost::shared_ptr<SyncContext> to(createSyncClient());
copyConfig(from, to, sources);
// Sources are active now according to the server default.
@ -1954,18 +1973,45 @@ static string filterConfig(const string &buffer)
return res.str();
}
// remove comment lines from scanFiles() output
static string filterFiles(const string &buffer)
{
ostringstream res;
typedef boost::split_iterator<string::const_iterator> string_split_iterator;
for (string_split_iterator it =
boost::make_split_iterator(buffer, boost::first_finder("\n", boost::is_iequal()));
it != string_split_iterator();
++it) {
string line = boost::copy_range<string>(*it);
if (line.find(":#") == line.npos) {
res << line;
// do not add extra newline after last newline
if (!line.empty() || it->end() < buffer.end()) {
res << endl;
}
}
}
return res.str();
}
static string injectValues(const string &buffer)
{
string res = buffer;
// username/password not set in templates, only in configs created via
// the command line
#if 0
// username/password not set in templates, only in configs created
// via the command line - not anymore, but if it ever comes back,
// here's the place for it
boost::replace_first(res,
"# username = ",
"username = your SyncML server account name");
boost::replace_first(res,
"# password = ",
"password = your SyncML server password");
#endif
return res;
}
@ -2120,6 +2166,7 @@ class CmdlineTest : public CppUnit::TestFixture {
CPPUNIT_TEST(testAddSource);
CPPUNIT_TEST(testSync);
CPPUNIT_TEST(testConfigure);
CPPUNIT_TEST(testConfigureTemplates);
CPPUNIT_TEST(testConfigureSources);
CPPUNIT_TEST(testOldConfigure);
CPPUNIT_TEST(testListSources);
@ -3161,6 +3208,197 @@ protected:
}
}
/**
* Test semantic of config creation (instead of updating) with and without
* templates. See BMC #14805.
*/
void testConfigureTemplates() {
ScopedEnvChange templates("SYNCEVOLUTION_TEMPLATE_DIR", "/dev/null");
ScopedEnvChange xdg("XDG_CONFIG_HOME", m_testDir);
ScopedEnvChange home("HOME", m_testDir);
rm_r(m_testDir);
{
// catch possible typos like "sheduleworld"
TestCmdline failure("--configure", "foo", NULL);
CPPUNIT_ASSERT(failure.m_cmdline->parse());
CPPUNIT_ASSERT(!failure.m_cmdline->run());
CPPUNIT_ASSERT(boost::starts_with(failure.m_out.str(), "Available configuration templates:\n"));
CPPUNIT_ASSERT_EQUAL(string("ERROR: no configuration template for 'foo@default' available. Use '--template none' and/or specify relevant properties on the command line to create a configuration without a template. Need values for: syncURL\n"),
lastLine(failure.m_err.str()));
}
string fooconfig =
StringPrintf("syncevolution/.internal.ini:rootMinVersion = %d\n"
"syncevolution/.internal.ini:rootCurVersion = %d\n"
"syncevolution/default/.internal.ini:contextMinVersion = %d\n"
"syncevolution/default/.internal.ini:contextCurVersion = %d\n"
"syncevolution/default/config.ini:deviceId = fixed-devid\n"
"syncevolution/default/peers/foo/.internal.ini:peerMinVersion = %d\n"
"syncevolution/default/peers/foo/.internal.ini:peerCurVersion = %d\n",
CONFIG_ROOT_MIN_VERSION, CONFIG_ROOT_CUR_VERSION,
CONFIG_CONTEXT_MIN_VERSION, CONFIG_CONTEXT_CUR_VERSION,
CONFIG_PEER_MIN_VERSION, CONFIG_PEER_CUR_VERSION);
string syncurl =
"syncevolution/default/peers/foo/config.ini:syncURL = local://@bar\n";
string configsource =
"syncevolution/default/peers/foo/sources/ical20/config.ini:sync = two-way\n"
"syncevolution/default/sources/ical20/config.ini:backend = calendar\n";
rm_r(m_testDir);
{
// allow user to proceed if they wish: should result in no sources configured
TestCmdline failure("--configure", "--template", "none", "foo", NULL);
CPPUNIT_ASSERT(failure.m_cmdline->parse());
CPPUNIT_ASSERT(failure.m_cmdline->run());
CPPUNIT_ASSERT_EQUAL_DIFF("", failure.m_out.str());
CPPUNIT_ASSERT_EQUAL_DIFF("", failure.m_err.str());
string res = scanFiles(m_testDir);
removeRandomUUID(res);
CPPUNIT_ASSERT_EQUAL_DIFF(fooconfig, filterFiles(res));
}
rm_r(m_testDir);
{
// allow user to proceed if they wish: should result in no sources configured,
// even if general source properties are specified
TestCmdline failure("--configure", "--template", "none", "backend=calendar", "foo", NULL);
CPPUNIT_ASSERT(failure.m_cmdline->parse());
CPPUNIT_ASSERT(failure.m_cmdline->run());
CPPUNIT_ASSERT_EQUAL_DIFF("", failure.m_out.str());
CPPUNIT_ASSERT_EQUAL_DIFF("", failure.m_err.str());
string res = scanFiles(m_testDir);
removeRandomUUID(res);
CPPUNIT_ASSERT_EQUAL_DIFF(fooconfig, filterFiles(res));
}
rm_r(m_testDir);
{
// allow user to proceed if they wish: should result in no sources configured,
// even if specific source properties are specified
TestCmdline failure("--configure", "--template", "none", "ical20/backend=calendar", "foo", NULL);
CPPUNIT_ASSERT(failure.m_cmdline->parse());
CPPUNIT_ASSERT(failure.m_cmdline->run());
CPPUNIT_ASSERT_EQUAL_DIFF("", failure.m_out.str());
CPPUNIT_ASSERT_EQUAL_DIFF("", failure.m_err.str());
string res = scanFiles(m_testDir);
removeRandomUUID(res);
CPPUNIT_ASSERT_EQUAL_DIFF(fooconfig, filterFiles(res));
}
rm_r(m_testDir);
{
// allow user to proceed if they wish and possible: here ical20 is not usable
TestCmdline failure("--configure", "--template", "none", "foo", "ical20", NULL);
CPPUNIT_ASSERT(failure.m_cmdline->parse());
bool caught = false;
try {
CPPUNIT_ASSERT(failure.m_cmdline->run());
} catch (const StatusException &ex) {
if (!strcmp(ex.what(), "ical20: no backend available")) {
caught = true;
} else {
throw;
}
}
CPPUNIT_ASSERT(caught);
}
rm_r(m_testDir);
{
// allow user to proceed if they wish and possible: here ical20 is not configurable
TestCmdline failure("--configure", "syncURL=local://@bar", "foo", "ical20", NULL);
CPPUNIT_ASSERT(failure.m_cmdline->parse());
bool caught = false;
try {
CPPUNIT_ASSERT(failure.m_cmdline->run());
} catch (const StatusException &ex) {
if (!strcmp(ex.what(), "no such source(s): ical20")) {
caught = true;
} else {
throw;
}
}
CPPUNIT_ASSERT(caught);
}
rm_r(m_testDir);
{
// allow user to proceed if they wish and possible: here ical20 is not configurable (wrong context)
TestCmdline failure("--configure", "syncURL=local://@bar", "ical20/backend@xyz=calendar", "foo", "ical20", NULL);
CPPUNIT_ASSERT(failure.m_cmdline->parse());
bool caught = false;
try {
CPPUNIT_ASSERT(failure.m_cmdline->run());
} catch (const StatusException &ex) {
if (!strcmp(ex.what(), "no such source(s): ical20")) {
caught = true;
} else {
throw;
}
}
CPPUNIT_ASSERT(caught);
}
rm_r(m_testDir);
{
// allow user to proceed if they wish: configure exactly the specified sources
TestCmdline failure("--configure", "--template", "none", "backend=calendar", "foo", "ical20", NULL);
CPPUNIT_ASSERT(failure.m_cmdline->parse());
CPPUNIT_ASSERT(failure.m_cmdline->run());
CPPUNIT_ASSERT_EQUAL_DIFF("", failure.m_out.str());
CPPUNIT_ASSERT_EQUAL_DIFF("", failure.m_err.str());
string res = scanFiles(m_testDir);
removeRandomUUID(res);
CPPUNIT_ASSERT_EQUAL_DIFF(fooconfig + configsource, filterFiles(res));
}
rm_r(m_testDir);
{
// allow user to proceed if they provide enough information: should result in no sources configured
TestCmdline failure("--configure", "syncURL=local://@bar", "foo", NULL);
CPPUNIT_ASSERT(failure.m_cmdline->parse());
CPPUNIT_ASSERT(failure.m_cmdline->run());
CPPUNIT_ASSERT_EQUAL_DIFF("", failure.m_out.str());
CPPUNIT_ASSERT_EQUAL_DIFF("", failure.m_err.str());
string res = scanFiles(m_testDir);
removeRandomUUID(res);
CPPUNIT_ASSERT_EQUAL_DIFF(fooconfig + syncurl, filterFiles(res));
}
rm_r(m_testDir);
{
// allow user to proceed if they provide enough information;
// source created because listed and usable
TestCmdline failure("--configure", "syncURL=local://@bar", "backend=calendar", "foo", "ical20", NULL);
CPPUNIT_ASSERT(failure.m_cmdline->parse());
CPPUNIT_ASSERT(failure.m_cmdline->run());
CPPUNIT_ASSERT_EQUAL_DIFF("", failure.m_out.str());
CPPUNIT_ASSERT_EQUAL_DIFF("", failure.m_err.str());
string res = scanFiles(m_testDir);
removeRandomUUID(res);
CPPUNIT_ASSERT_EQUAL_DIFF(fooconfig + syncurl + configsource, filterFiles(res));
}
rm_r(m_testDir);
{
// allow user to proceed if they provide enough information;
// source created because listed and usable
TestCmdline failure("--configure", "syncURL=local://@bar", "ical20/backend@default=calendar", "foo", "ical20", NULL);
CPPUNIT_ASSERT(failure.m_cmdline->parse());
CPPUNIT_ASSERT(failure.m_cmdline->run());
CPPUNIT_ASSERT_EQUAL_DIFF("", failure.m_out.str());
CPPUNIT_ASSERT_EQUAL_DIFF("", failure.m_err.str());
string res = scanFiles(m_testDir);
removeRandomUUID(res);
CPPUNIT_ASSERT_EQUAL_DIFF(fooconfig + syncurl + configsource, filterFiles(res));
}
}
void testConfigureSources() {
ScopedEnvChange templates("SYNCEVOLUTION_TEMPLATE_DIR", "/dev/null");
ScopedEnvChange xdg("XDG_CONFIG_HOME", m_testDir);
@ -3655,8 +3893,8 @@ protected:
string oldConfig =
"config.ini:logDir = none\n"
"peers/scheduleworld/config.ini:syncURL = http://sync.scheduleworld.com/funambol/ds\n"
"peers/scheduleworld/config.ini:username = your SyncML server account name\n"
"peers/scheduleworld/config.ini:password = your SyncML server password\n"
"peers/scheduleworld/config.ini:# username = \n"
"peers/scheduleworld/config.ini:# password = \n"
"peers/scheduleworld/sources/addressbook/config.ini:sync = two-way\n"
"peers/scheduleworld/sources/addressbook/config.ini:uri = card3\n"
@ -3664,8 +3902,8 @@ protected:
"sources/addressbook/config.ini:type = calendar\n" // wrong!
"peers/funambol/config.ini:syncURL = http://sync.funambol.com/funambol/ds\n"
"peers/funambol/config.ini:username = your SyncML server account name\n"
"peers/funambol/config.ini:password = your SyncML server password\n"
"peers/funambol/config.ini:# username = \n"
"peers/funambol/config.ini:# password = \n"
"peers/funambol/sources/calendar/config.ini:sync = refresh-from-server\n"
"peers/funambol/sources/calendar/config.ini:uri = cal\n"
@ -3675,8 +3913,8 @@ protected:
"sources/calendar/config.ini:type = memos\n" // wrong!
"peers/memotoo/config.ini:syncURL = http://sync.memotoo.com/memotoo/ds\n"
"peers/memotoo/config.ini:username = your SyncML server account name\n"
"peers/memotoo/config.ini:password = your SyncML server password\n"
"peers/memotoo/config.ini:# username = \n"
"peers/memotoo/config.ini:# password = \n"
"peers/memotoo/sources/memo/config.ini:sync = refresh-from-client\n"
"peers/memotoo/sources/memo/config.ini:uri = cal\n"
@ -3791,8 +4029,8 @@ private:
"peers/scheduleworld/.internal.ini:# lastNonce = \n"
"peers/scheduleworld/.internal.ini:# deviceData = \n"
"peers/scheduleworld/config.ini:syncURL = http://sync.scheduleworld.com/funambol/ds\n"
"peers/scheduleworld/config.ini:username = your SyncML server account name\n"
"peers/scheduleworld/config.ini:password = your SyncML server password\n"
"peers/scheduleworld/config.ini:# username = \n"
"peers/scheduleworld/config.ini:# password = \n"
".internal.ini:contextMinVersion = %d\n"
".internal.ini:contextCurVersion = %d\n"
"config.ini:# logdir = \n"
@ -3907,8 +4145,8 @@ private:
// old style paths
string oldConfig =
"spds/syncml/config.txt:syncURL = http://sync.scheduleworld.com/funambol/ds\n"
"spds/syncml/config.txt:username = your SyncML server account name\n"
"spds/syncml/config.txt:password = your SyncML server password\n"
"spds/syncml/config.txt:# username = \n"
"spds/syncml/config.txt:# password = \n"
"spds/syncml/config.txt:# logdir = \n"
"spds/syncml/config.txt:# loglevel = 0\n"
"spds/syncml/config.txt:# printChanges = 1\n"

17
src/syncevo/SyncConfig.cpp
View File

@ -816,6 +816,11 @@ boost::shared_ptr<SyncConfig> SyncConfig::createPeerTemplate(const string &serve
config->setDefaults(false);
config->setDevID(string("syncevolution-") + UUID());
// leave the rest empty for special "none" template
if (server == "none") {
return config;
}
// create sync source configs and set non-default values
config->setSourceDefaults("addressbook", false);
config->setSourceDefaults("calendar", false);
@ -1770,9 +1775,15 @@ ConfigPropertyRegistry &SyncConfig::getRegistry()
#endif
// obligatory sync properties
syncPropUsername.setObligatory(true);
syncPropPassword.setObligatory(true);
syncPropDevID.setObligatory(true);
//
// username/password used to be
// considered obligatory, but are not anymore because there are
// cases where they are not needed (local sync, Bluetooth)
// syncPropUsername.setObligatory(true);
// syncPropPassword.setObligatory(true);
//
// created if not given:
// syncPropDevID.setObligatory(true);
syncPropSyncURL.setObligatory(true);
// hidden sync properties

1
src/syncevo/SyncConfig.h
View File

@ -1132,6 +1132,7 @@ class SyncConfig {
*
* @param peer a configuration name, *without* a context (scheduleworld, not scheduleworld@default),
* or a configuration path in the system directory which can avoid another fuzzy match process.
* "none" returns an empty template (default sync properties and dev ID set).
* @return NULL if no such template
*/
static boost::shared_ptr<SyncConfig> createPeerTemplate(const string &peer);