shared layout: fix for showing and setting "type" property (MB #9939)
The "type" property is special because it must be set in the per-peer config node (backend and data format) and in the shared config node (only backend relevant). The MultiplexConfigNode picks the right node, but that was based on the (invalid) assumption that the peer nodes would be NULL when not in use. What really happens is that dummy instances are used, to allow writing into these nodes. Because of this false assumption, the "type" for the context was always printed as the default "select backend". Fixed by telling the MultiplexConfigNode that its peer nodes are dummies. In addition, setting the "type" property for a context didn't work because copyProperties() treated "type" as a per-peer property which had to be skipped. Fixed by teaching it about the special case. The D-Bus and Cmdline tests were extended to cover these cases. They failed (as expected) before fixing the implementation.
This commit is contained in:
parent
96dd220f60
commit
169adb3d91
|
@ -211,7 +211,7 @@ bool Cmdline::run() {
|
|||
boost::shared_ptr<FilterConfigNode> hiddenNode(new VolatileConfigNode());
|
||||
boost::shared_ptr<FilterConfigNode> trackingNode(new VolatileConfigNode());
|
||||
boost::shared_ptr<FilterConfigNode> serverNode(new VolatileConfigNode());
|
||||
SyncSourceNodes nodes(sharedNode, configNode, hiddenNode, trackingNode, serverNode);
|
||||
SyncSourceNodes nodes(true, sharedNode, configNode, hiddenNode, trackingNode, serverNode);
|
||||
SyncSourceParams params("list", nodes);
|
||||
|
||||
BOOST_FOREACH(const RegisterSyncSource *source, registry) {
|
||||
|
@ -1979,7 +1979,40 @@ protected:
|
|||
|
||||
rm_r(m_testDir);
|
||||
testSetupScheduleWorld();
|
||||
doConfigure(ScheduleWorldConfig(), "sources/addressbook/config.ini:");
|
||||
string expected = doConfigure(ScheduleWorldConfig(), "sources/addressbook/config.ini:");
|
||||
|
||||
{
|
||||
// updating type for peer must also update type for context
|
||||
TestCmdline cmdline("--configure",
|
||||
"--source-property", "type=file:text/vcard:3.0",
|
||||
"scheduleworld", "addressbook",
|
||||
NULL);
|
||||
cmdline.doit();
|
||||
CPPUNIT_ASSERT_EQUAL_DIFF("", cmdline.m_err.str());
|
||||
CPPUNIT_ASSERT_EQUAL_DIFF("", cmdline.m_out.str());
|
||||
boost::replace_all(expected,
|
||||
"type = addressbook:text/vcard",
|
||||
"type = file:text/vcard:3.0");
|
||||
CPPUNIT_ASSERT_EQUAL_DIFF(expected,
|
||||
filterConfig(printConfig("scheduleworld")));
|
||||
string shared = filterConfig(printConfig("@default"));
|
||||
CPPUNIT_ASSERT(shared.find("type = file:text/vcard:3.0") != shared.npos);
|
||||
}
|
||||
|
||||
{
|
||||
// updating type for context must not affect peer
|
||||
TestCmdline cmdline("--configure",
|
||||
"--source-property", "type=file:text/vcard:2.1",
|
||||
"@default", "addressbook",
|
||||
NULL);
|
||||
cmdline.doit();
|
||||
CPPUNIT_ASSERT_EQUAL_DIFF("", cmdline.m_err.str());
|
||||
CPPUNIT_ASSERT_EQUAL_DIFF("", cmdline.m_out.str());
|
||||
CPPUNIT_ASSERT_EQUAL_DIFF(expected,
|
||||
filterConfig(printConfig("scheduleworld")));
|
||||
string shared = filterConfig(printConfig("@default"));
|
||||
CPPUNIT_ASSERT(shared.find("type = file:text/vcard:2.1") != shared.npos);
|
||||
}
|
||||
|
||||
string syncProperties("syncURL:\n"
|
||||
"\n"
|
||||
|
@ -2124,7 +2157,7 @@ protected:
|
|||
doConfigure(oldConfig, "spds/sources/addressbook/config.txt:");
|
||||
}
|
||||
|
||||
void doConfigure(const string &SWConfig, const string &addressbookPrefix) {
|
||||
string doConfigure(const string &SWConfig, const string &addressbookPrefix) {
|
||||
string expected;
|
||||
|
||||
{
|
||||
|
@ -2193,6 +2226,8 @@ protected:
|
|||
CPPUNIT_ASSERT_EQUAL_DIFF(expected,
|
||||
filterConfig(printConfig("scheduleworld")));
|
||||
}
|
||||
|
||||
return expected;
|
||||
}
|
||||
|
||||
void testListSources() {
|
||||
|
|
|
@ -36,9 +36,10 @@ MultiplexConfigNode::getNode(const string &property,
|
|||
m_nodes[prop->isHidden()][prop->getSharing()].get();
|
||||
|
||||
// special case: fall back to shared node if no unshared
|
||||
// node, and property isprimarily stored unshared, but
|
||||
// also in the shared node
|
||||
if (!node &&
|
||||
// node or only dummy one, and property is primarily
|
||||
// stored unshared, but also in the shared node
|
||||
if ((!node || !m_havePeerNodes) &&
|
||||
prop->getSharing() == ConfigProperty::NO_SHARING &&
|
||||
(prop->getFlags() & ConfigProperty::SHARED_AND_UNSHARED)) {
|
||||
node = m_nodes[prop->isHidden()][ConfigProperty::SOURCE_SET_SHARING].get();
|
||||
}
|
||||
|
|
|
@ -36,6 +36,7 @@ class MultiplexConfigNode : public FilterConfigNode
|
|||
const std::string m_name;
|
||||
boost::shared_ptr<FilterConfigNode> m_nodes[2][3];
|
||||
const ConfigPropertyRegistry &m_registry;
|
||||
bool m_havePeerNodes;
|
||||
int m_hiddenLower, m_hiddenUpper;
|
||||
|
||||
FilterConfigNode *getNode(const string &property,
|
||||
|
@ -48,6 +49,7 @@ class MultiplexConfigNode : public FilterConfigNode
|
|||
FilterConfigNode(boost::shared_ptr<ConfigNode>()),
|
||||
m_name(name),
|
||||
m_registry(registry),
|
||||
m_havePeerNodes(true),
|
||||
m_hiddenLower(0), m_hiddenUpper(1) {}
|
||||
|
||||
/** only join hidden or user-visible properties */
|
||||
|
@ -57,8 +59,13 @@ class MultiplexConfigNode : public FilterConfigNode
|
|||
FilterConfigNode(boost::shared_ptr<ConfigNode>()),
|
||||
m_name(name),
|
||||
m_registry(registry),
|
||||
m_havePeerNodes(true),
|
||||
m_hiddenLower(hidden), m_hiddenUpper(hidden) {}
|
||||
|
||||
/** true when peer nodes are used (default), false when they are dummy nodes */
|
||||
bool getHavePeerNodes() const { return m_havePeerNodes; }
|
||||
void setHavePeerNodes(bool havePeerNodes) { m_havePeerNodes = havePeerNodes; }
|
||||
|
||||
/** configure the nodes to use */
|
||||
void setNode(bool hidden, ConfigProperty::Sharing sharing,
|
||||
const boost::shared_ptr<FilterConfigNode> &node) {
|
||||
|
|
|
@ -284,6 +284,7 @@ SyncConfig::SyncConfig(const string &peer,
|
|||
mnode.reset(new MultiplexConfigNode(m_peerNode->getName(),
|
||||
getRegistry(),
|
||||
false));
|
||||
mnode->setHavePeerNodes(!m_peerPath.empty());
|
||||
m_props[false] = mnode;
|
||||
mnode->setNode(false, ConfigProperty::GLOBAL_SHARING,
|
||||
m_globalNode);
|
||||
|
@ -295,6 +296,7 @@ SyncConfig::SyncConfig(const string &peer,
|
|||
mnode.reset(new MultiplexConfigNode(m_hiddenPeerNode->getName(),
|
||||
getRegistry(),
|
||||
true));
|
||||
mnode->setHavePeerNodes(!m_peerPath.empty());
|
||||
m_props[true] = mnode;
|
||||
mnode->setNode(true, ConfigProperty::SOURCE_SET_SHARING,
|
||||
m_contextHiddenNode);
|
||||
|
@ -869,7 +871,7 @@ SyncSourceNodes SyncConfig::getSyncSourceNodes(const string &name,
|
|||
}
|
||||
}
|
||||
|
||||
return SyncSourceNodes(sharedNode, peerNode, hiddenPeerNode, trackingNode, serverNode);
|
||||
return SyncSourceNodes(!peerPath.empty(), sharedNode, peerNode, hiddenPeerNode, trackingNode, serverNode);
|
||||
}
|
||||
|
||||
ConstSyncSourceNodes SyncConfig::getSyncSourceNodes(const string &name,
|
||||
|
@ -1679,7 +1681,9 @@ static void copyProperties(const ConfigNode &fromProps,
|
|||
{
|
||||
BOOST_FOREACH(const ConfigProperty *prop, allProps) {
|
||||
if (prop->isHidden() == hidden &&
|
||||
(unshared || prop->getSharing() != ConfigProperty::NO_SHARING)) {
|
||||
(unshared ||
|
||||
prop->getSharing() != ConfigProperty::NO_SHARING ||
|
||||
(prop->getFlags() & ConfigProperty::SHARED_AND_UNSHARED))) {
|
||||
string name = prop->getName();
|
||||
bool isDefault;
|
||||
string value = prop->getProperty(fromProps, &isDefault);
|
||||
|
@ -1986,7 +1990,8 @@ ConfigPropertyRegistry &SyncSourceConfig::getRegistry()
|
|||
return registry;
|
||||
}
|
||||
|
||||
SyncSourceNodes::SyncSourceNodes(const boost::shared_ptr<FilterConfigNode> &sharedNode,
|
||||
SyncSourceNodes::SyncSourceNodes(bool havePeerNode,
|
||||
const boost::shared_ptr<FilterConfigNode> &sharedNode,
|
||||
const boost::shared_ptr<FilterConfigNode> &peerNode,
|
||||
const boost::shared_ptr<ConfigNode> &hiddenPeerNode,
|
||||
const boost::shared_ptr<ConfigNode> &trackingNode,
|
||||
|
@ -2001,6 +2006,7 @@ SyncSourceNodes::SyncSourceNodes(const boost::shared_ptr<FilterConfigNode> &shar
|
|||
mnode.reset(new MultiplexConfigNode(m_peerNode->getName(),
|
||||
SyncSourceConfig::getRegistry(),
|
||||
false));
|
||||
mnode->setHavePeerNodes(havePeerNode);
|
||||
m_props[false] = mnode;
|
||||
mnode->setNode(false, ConfigProperty::SOURCE_SET_SHARING,
|
||||
m_sharedNode);
|
||||
|
|
|
@ -1426,6 +1426,9 @@ class SyncSourceNodes {
|
|||
SyncSourceNodes() {}
|
||||
|
||||
/**
|
||||
* @param havePeerNode false when peerNode is a dummy instance which has to
|
||||
* be ignored for properties which may exist there as
|
||||
* well as in the shared node (for example, "type")
|
||||
* @param sharedNode node for user-visible properties, shared between peers
|
||||
* @param peerNode node for user-visible, per-peer properties (the same
|
||||
* as sharedNode in SYNC4J_LAYOUT and HTTP_SERVER_LAYOUT)
|
||||
|
@ -1436,7 +1439,8 @@ class SyncSourceNodes {
|
|||
* @param serverNode node for tracking items in a server (always different
|
||||
* than the other nodes)
|
||||
*/
|
||||
SyncSourceNodes(const boost::shared_ptr<FilterConfigNode> &sharedNode,
|
||||
SyncSourceNodes(bool havePeerNode,
|
||||
const boost::shared_ptr<FilterConfigNode> &sharedNode,
|
||||
const boost::shared_ptr<FilterConfigNode> &peerNode,
|
||||
const boost::shared_ptr<ConfigNode> &hiddenPeerNode,
|
||||
const boost::shared_ptr<ConfigNode> &trackingNode,
|
||||
|
|
|
@ -2036,6 +2036,35 @@ class TestMultipleConfigs(unittest.TestCase, DBusUtil):
|
|||
self.failUnlessEqual(config[""]["deviceId"], "shared-device-identifier")
|
||||
self.failUnlessEqual(config["source/addressbook"]["evolutionsource"], "Work")
|
||||
|
||||
def testSharedType(self):
|
||||
"""'type' must be set per-peer and shared"""
|
||||
self.setupConfigs()
|
||||
|
||||
# writing for peer modifies "type" in "foo" and context
|
||||
self.setUpSession("Foo@deFAULT")
|
||||
config = self.session.GetConfig(False, utf8_strings=True)
|
||||
config["source/addressbook"]["type"] = "file:text/vcard:3.0"
|
||||
self.session.SetConfig(True, False,
|
||||
config,
|
||||
utf8_strings=True)
|
||||
config = self.server.GetConfig("Foo", False, utf8_strings=True)
|
||||
self.failUnlessEqual(config["source/addressbook"]["type"], "file:text/vcard:3.0")
|
||||
config = self.server.GetConfig("@default", False, utf8_strings=True)
|
||||
self.failUnlessEqual(config["source/addressbook"]["type"], "file:text/vcard:3.0")
|
||||
self.session.Detach()
|
||||
|
||||
# writing in context only changes the context
|
||||
self.setUpSession("@deFAULT")
|
||||
config = self.session.GetConfig(False, utf8_strings=True)
|
||||
config["source/addressbook"]["type"] = "file:text/x-vcard:2.1"
|
||||
self.session.SetConfig(True, False,
|
||||
config,
|
||||
utf8_strings=True)
|
||||
config = self.server.GetConfig("Foo", False, utf8_strings=True)
|
||||
self.failUnlessEqual(config["source/addressbook"]["type"], "file:text/vcard:3.0")
|
||||
config = self.server.GetConfig("@default", False, utf8_strings=True)
|
||||
self.failUnlessEqual(config["source/addressbook"]["type"], "file:text/x-vcard:2.1")
|
||||
|
||||
def testOtherContext(self):
|
||||
"""write into independent context"""
|
||||
self.setupConfigs()
|
||||
|
|
Loading…
Reference in New Issue