PBAP: incremental sync (FDO #59551)

Depending on the SYNCEVOLUTION_PBAP_SYNC env variable, syncing reads
all properties as configured ("all"), excludes photos ("text") or
first text, then all ("incremental").

When excluding photos, only known properties get requested. This
avoids issues with phones which reject the request when enabling
properties via the bit flags. This also helps with
"databaseFormat=^PHOTO".

When excluding photos, the vcard merge script as used by EDS ensures
that existing photo data is preserved. This only works during a slow
sync (merge script not called otherwise, okay for PBAP because it
always syncs in slow sync) and EDS (other backends do not use the
merge script, okay at the moment because PIM Manager is hard-coded to
use EDS).

The PBAP backend must be aware of the PBAP sync mode and request a
second cycle, which again must be a slow sync. This only works because
the sync engine is aware of the special mode and sets a new session
variable "keepPhotoData". It would be better to have the PBAP backend
send CTCap with PHOTO marked as not supported for text-only syncs and
enabled when sending PHOTO data, but that is considerably harder to
implement (CTCap cannot be adjusted at runtime).

beginSync() may only ask for a slow sync when not already called
for one. That's what the command line tool does when accessing
items. It fails when getting the 508 status.

The original goal of overlapping syncing with download has not been
achieved yet. It turned out that all item IDs get requested before
syncing starts, which thus depends on downloading all items in the current
implementation. Can be fixed by making up IDs based on the number of
existing items (see GetSize() in PBAP) and then downloading later when
the data is needed.
This commit is contained in:
Patrick Ohly 2013-07-05 10:39:21 +02:00
parent 309bed01e1
commit ddc1e53b0c
5 changed files with 202 additions and 34 deletions

View file

@ -25,12 +25,7 @@
#include "PbapSyncSource.h"
// SyncEvolution includes a copy of Boost header files.
// They are safe to use without creating additional
// build dependencies. boost::filesystem requires a
// library and therefore is avoided here. Some
// utility functions from SyncEvolution are used
// instead, plus standard C/Posix functions.
#include <boost/assign/list_of.hpp>
#include <boost/algorithm/string/case_conv.hpp>
#include <boost/tokenizer.hpp>
@ -85,6 +80,12 @@ public:
const char *addVCards(int startIndex, const pcrecpp::StringPiece &content);
};
enum PullData
{
PULL_AS_CONFIGURED,
PULL_WITHOUT_PHOTOS
};
class PbapSession : private boost::noncopyable {
public:
static boost::shared_ptr<PbapSession> create(PbapSyncSource &parent);
@ -94,9 +95,10 @@ public:
typedef std::map<std::string, pcrecpp::StringPiece> Content;
typedef std::map<std::string, boost::variant<std::string> > Params;
boost::shared_ptr<PullAll> startPullAll();
boost::shared_ptr<PullAll> startPullAll(PullData pullata);
void checkForError(); // Throws exception if transfer failed.
bool transferComplete() const { return m_transferComplete; }
void resetTransfer() { m_transferComplete = false; m_transferErrorCode = ""; m_transferErrorMsg = ""; }
void shutdown(void);
private:
@ -113,8 +115,11 @@ private:
} m_obexAPI;
/** filter parameters for BLUEZ5 PullAll */
typedef boost::variant< std::string, std::list<std::string> > Bluez5Values;
typedef std::list<std::string> Properties;
typedef boost::variant< std::string, Properties > Bluez5Values;
std::map<std::string, Bluez5Values> m_filter5;
Properties m_filterFields;
Properties supportedProperties() const;
/**
* m_transferComplete will be set to true when observing a
@ -225,6 +230,49 @@ void PbapSession::propertyChangedCb(const GDBusCXX::Path_t &path,
}
}
PbapSession::Properties PbapSession::supportedProperties() const
{
Properties props;
static const std::set<std::string> supported =
boost::assign::list_of("VERSION")
("FN")
("N")
("PHOTO")
("BDAY")
("ADR")
("LABEL")
("TEL")
("EMAIL")
("MAILER")
("TZ")
("GEO")
("TITLE")
("ROLE")
("LOGO")
("AGENT")
("ORG")
("NOTE")
("REV")
("SOUND")
("URL")
("UID")
("KEY")
("NICKNAME")
("CATEGORIES")
("CLASS");
BOOST_FOREACH (const std::string &prop, m_filterFields) {
// Be conservative and only ask for properties that we
// really know how to use. obexd also lists the bit field
// strings ("BIT01") but phones have been seen to reject
// queries when those were enabled.
if (supported.find(prop) != supported.end()) {
props.push_back(prop);
}
}
return props;
}
void PbapSession::initSession(const std::string &address, const std::string &format)
{
if (m_session.get()) {
@ -384,15 +432,14 @@ void PbapSession::initSession(const std::string &address, const std::string &for
SE_LOG_DEBUG(NULL, "PBAP session created: %s", m_session->getPath());
// get filter list so that we can continue validating our format specifier
std::vector<std::string> filterFields =
GDBusCXX::DBusClientCall1< std::vector<std::string> >(*m_session, "ListFilterFields")();
m_filterFields = GDBusCXX::DBusClientCall1< Properties >(*m_session, "ListFilterFields")();
SE_LOG_DEBUG(NULL, "supported PBAP filter fields:\n %s",
boost::join(filterFields, "\n ").c_str());
boost::join(m_filterFields, "\n ").c_str());
std::list<std::string> filter;
Properties filter;
if (negated) {
// negated, start with everything set
filter.insert(filter.begin(), filterFields.begin(), filterFields.end());
filter = supportedProperties();
}
// validate parameters and update filter
@ -401,12 +448,12 @@ void PbapSession::initSession(const std::string &address, const std::string &for
continue;
}
std::vector<std::string>::const_iterator entry =
std::find_if(filterFields.begin(),
filterFields.end(),
Properties::const_iterator entry =
std::find_if(m_filterFields.begin(),
m_filterFields.end(),
boost::bind(&boost::iequals<std::string,std::string>, _1, prop, std::locale()));
if (entry == filterFields.end()) {
if (entry == m_filterFields.end()) {
m_parent.throwError(StringPrintf("invalid property name in PBAP vCard format specification (databaseFormat): %s",
prop.c_str()));
}
@ -419,21 +466,49 @@ void PbapSession::initSession(const std::string &address, const std::string &for
}
GDBusCXX::DBusClientCall0(*m_session, "Select")(std::string("int"), std::string("PB"));
if (m_obexAPI == OBEXD_OLD ||
m_obexAPI == OBEXD_NEW) {
GDBusCXX::DBusClientCall0(*m_session, "SetFilter")(std::vector<std::string>(filter.begin(), filter.end()));
GDBusCXX::DBusClientCall0(*m_session, "SetFormat")(std::string(version == "2.1" ? "vcard21" : "vcard30"));
} else {
m_filter5["Format"] = version == "2.1" ? "vcard21" : "vcard30";
m_filter5["Fields"] = filter;
}
m_filter5["Format"] = version == "2.1" ? "vcard21" : "vcard30";
m_filter5["Fields"] = filter;
SE_LOG_DEBUG(NULL, "PBAP session initialized");
}
boost::shared_ptr<PullAll> PbapSession::startPullAll()
boost::shared_ptr<PullAll> PbapSession::startPullAll(PullData pullData)
{
resetTransfer();
// Update prepared filter to match pullData.
std::map<std::string, Bluez5Values> currentFilter = m_filter5;
std::string &format = boost::get<std::string>(currentFilter["Format"]);
std::list<std::string> &filter = boost::get< std::list<std::string> >(currentFilter["Fields"]);
switch (pullData) {
case PULL_AS_CONFIGURED:
SE_LOG_DEBUG(NULL, "pull all with configured filter: '%s'",
boost::join(filter, " ").c_str());
break;
case PULL_WITHOUT_PHOTOS:
// Remove PHOTO from list or create list with the other properties.
if (filter.empty()) {
filter = supportedProperties();
}
for (Properties::iterator it = filter.begin();
it != filter.end();
++it) {
if (*it == "PHOTO") {
filter.erase(it);
break;
}
}
SE_LOG_DEBUG(NULL, "pull all without photos: '%s'",
boost::join(filter, " ").c_str());
break;
}
if (m_obexAPI == OBEXD_OLD ||
m_obexAPI == OBEXD_NEW) {
GDBusCXX::DBusClientCall0(*m_session, "SetFilter")(filter);
GDBusCXX::DBusClientCall0(*m_session, "SetFormat")(format);
}
boost::shared_ptr<PullAll> state(new PullAll);
state->m_currentContact = 0;
if (m_obexAPI != OBEXD_OLD) {
@ -443,7 +518,7 @@ boost::shared_ptr<PullAll> PbapSession::startPullAll()
std::pair<GDBusCXX::DBusObject_t, Params> tuple =
m_obexAPI == OBEXD_NEW ?
GDBusCXX::DBusClientCall1<std::pair<GDBusCXX::DBusObject_t, Params> >(*m_session, "PullAll")(state->m_tmpFile.filename()) :
GDBusCXX::DBusClientCall2<GDBusCXX::DBusObject_t, Params>(*m_session, "PullAll")(state->m_tmpFile.filename(), m_filter5);
GDBusCXX::DBusClientCall2<GDBusCXX::DBusObject_t, Params>(*m_session, "PullAll")(state->m_tmpFile.filename(), currentFilter);
const GDBusCXX::DBusObject_t &transfer = tuple.first;
const Params &properties = tuple.second;
SE_LOG_DEBUG(NULL, "pullall transfer path %s, %ld properties", transfer.c_str(), (long)properties.size());
@ -570,6 +645,14 @@ PbapSyncSource::PbapSyncSource(const SyncSourceParams &params) :
m_operations.m_readItemAsKey = boost::bind(&PbapSyncSource::readItemAsKey,
this, _1, _2);
m_session = PbapSession::create(*this);
const char *PBAPSyncMode = getenv("SYNCEVOLUTION_PBAP_SYNC");
m_PBAPSyncMode = !PBAPSyncMode ? PBAP_SYNC_NORMAL :
boost::iequals(PBAPSyncMode, "incremental") ? PBAP_SYNC_INCREMENTAL :
boost::iequals(PBAPSyncMode, "text") ? PBAP_SYNC_TEXT :
boost::iequals(PBAPSyncMode, "all") ? PBAP_SYNC_NORMAL :
(throwError(StringPrintf("invalid value for SYNCEVOLUTION_PBAP_SYNC: %s", PBAPSyncMode)), PBAP_SYNC_NORMAL);
m_isFirstCycle = true;
m_hadContacts = false;
}
PbapSyncSource::~PbapSyncSource()
@ -592,14 +675,17 @@ void PbapSyncSource::open()
void PbapSyncSource::beginSync(const std::string &lastToken, const std::string &resumeToken)
{
if (!lastToken.empty()) {
throwError(STATUS_SLOW_SYNC_508, std::string("PBAP cannot do change detection"));
}
}
std::string PbapSyncSource::endSync(bool success)
{
m_pullAll.reset();
m_session->shutdown();
return "";
// Non-empty so that beginSync() can detect non-slow syncs and ask
// for one.
return "1";
}
bool PbapSyncSource::isEmpty()
@ -609,6 +695,7 @@ bool PbapSyncSource::isEmpty()
void PbapSyncSource::close()
{
m_session->shutdown();
}
PbapSyncSource::Databases PbapSyncSource::getDatabases()
@ -652,14 +739,24 @@ void PbapSyncSource::getSynthesisInfo(SynthesisInfo &info,
type = sourceType.m_format;
}
info.m_datatypes = getDataTypeSupport(type, sourceType.m_forceFormat);
/**
* Access to data must be done early so that a slow sync can be
* enforced.
*/
info.m_earlyStartDataRead = true;
}
// TODO: return IDs based on GetSize(), read only when engine needs data.
sysync::TSyError PbapSyncSource::readNextItem(sysync::ItemID aID,
sysync::sInt32 *aStatus,
bool aFirst)
{
if (aFirst) {
m_pullAll = m_session->startPullAll();
m_pullAll = m_session->startPullAll((m_PBAPSyncMode == PBAP_SYNC_TEXT ||
(m_PBAPSyncMode == PBAP_SYNC_INCREMENTAL && m_isFirstCycle)) ? PULL_WITHOUT_PHOTOS :
PULL_AS_CONFIGURED);
}
if (!m_pullAll) {
throwError("logic error: readNextItem without aFirst=true before");
@ -667,10 +764,17 @@ sysync::TSyError PbapSyncSource::readNextItem(sysync::ItemID aID,
std::string id = m_pullAll->getNextID();
if (id.empty()) {
*aStatus = sysync::ReadNextItem_EOF;
if (m_PBAPSyncMode == PBAP_SYNC_INCREMENTAL &&
m_hadContacts &&
m_isFirstCycle) {
requestAnotherSync();
m_isFirstCycle = false;
}
} else {
*aStatus = sysync::ReadNextItem_Unchanged;
aID->item = StrAlloc(id.c_str());
aID->parent = NULL;
m_hadContacts = true;
}
return sysync::LOCERR_OK;
}

View file

@ -66,6 +66,14 @@ class PbapSyncSource : virtual public SyncSource, virtual public SyncSourceSessi
private:
boost::shared_ptr<PbapSession> m_session;
boost::shared_ptr<PullAll> m_pullAll;
enum PBAPSyncMode {
PBAP_SYNC_NORMAL, ///< Read contact data according to filter.
PBAP_SYNC_TEXT, ///< Sync without reading photo data from phone and keeping local photos instead.
PBAP_SYNC_INCREMENTAL ///< Sync first without photo data (as in PBAP_SYNC_TEXT),
/// then add photo data in second cycle.
} m_PBAPSyncMode;
bool m_isFirstCycle;
bool m_hadContacts;
/**
* List items as expected by Synthesis engine.

View file

@ -93,3 +93,29 @@ syncevolution --configure \
# run synchronization
syncevolution pbap
It may or may not be desirable to sync only the text properties of a
contact. This can be considerably faster, because the PHOTO property
is large and increases the download and processing time. SyncEvolution
supports that in three different ways, chosen via the
SYNCEVOLUTION_PBAP_SYNC env variable:
SYNCEVOLUTION_PBAP_SYNC=text syncevolution pbap
Synchronize only the text properties by excluding the PHOTO property,
keep all photos already stored locally.
SYNCEVOLUTION_PBAP_SYNC=all syncevolution pbap
Synchronize all properties according to the databaseFormat in one go,
adding/updating/removing photos locally to match the phone.
SYNCEVOLUTION_PBAP_SYNC=incremental syncevolution pbap
Synchronize first text, then all data. Conceptually this is the same
as two invocations with "text" and then "all". Because the implementation
reuses the same PBAP session and sync session, it is slightly faster.
It is slower than a single "all" sync because the entire set of contacts
must be downloaded and synced twice, once without photo, then with.
SYNCEVOLUTION_PBAP_SYNC has side effect on the entire sync and thus
may only be used in a sync config which synchronizes contacts via
PBAP.

View file

@ -1813,6 +1813,13 @@ void SyncContext::displaySourceProgress(sysync::TProgressEventEnum type,
// (they should only finish the work of the initial
// one)
source.recordRestart();
if (m_serverMode) {
// Done with first cycle, revert to normal photo
// handling if it was disabled.
SharedKey sessionKey = m_engine.OpenSessionKey(m_session);
SharedKey contextKey = m_engine.OpenKeyByPath(sessionKey, "/sessionvars");
m_engine.SetInt32Value(contextKey, "keepPhotoData", false);
}
}
} else {
SE_LOG_INFO(NULL, "%s: restore from backup", source.getDisplayName().c_str());
@ -2441,7 +2448,11 @@ void SyncContext::getConfigXML(string &xml, string &configname)
std::set<std::string> flags = getSyncMLFlags();
bool noctcap = flags.find("noctcap") != flags.end();
bool norestart = flags.find("norestart") != flags.end();
const char *sessioninitscript =
const char *PBAPSyncMode = getenv("SYNCEVOLUTION_PBAP_SYNC");
bool keepPhotoData = PBAPSyncMode &&
(boost::iequals(PBAPSyncMode, "incremental") ||
boost::iequals(PBAPSyncMode, "text"));
std::string sessioninitscript =
" <sessioninitscript><![CDATA[\n"
" // these variables are possibly modified by rule scripts\n"
" TIMESTAMP mindate; // earliest date remote party can handle\n"
@ -2456,6 +2467,14 @@ void SyncContext::getConfigXML(string &xml, string &configname)
" addInternetEmail = FALSE;\n"
" INTEGER stripUID;\n"
" stripUID = FALSE;\n"
" INTEGER keepPhotoData;\n"
" keepPhotoData = "
// Keep local photos in first cycle when using special sync
// mode for PBAP. PBAP source will request second cycle if it
// has contacts whose photo data was not donwloaded. Then we
// will disable this special handling for that cycle and photo
// can be updated and removed normally.
+ std::string(keepPhotoData ? "TRUE" : "FALSE") + ";\n"
" ]]></sessioninitscript>\n";
ostringstream clientorserver;

View file

@ -8,7 +8,18 @@
<macro name="VCARD_MERGESCRIPT"><![CDATA[
integer mode;
mode = MERGEMODE();
if (mode == 1 && WINNING.PHOTO == EMPTY) {
if (SESSIONVAR("keepPhotoData") &&
WINNING.PHOTO == EMPTY &&
LOOSING.PHOTO != EMPTY) {
// The client's item is always the "winning" one in PBAP syncing,
// but it might not have the photo data that is already on the
// server. Therefore keep the server's photo, if there is
// one and we are not in the phase where the client sends
// its photos.
WINNING.PHOTO = LOOSING.PHOTO;
WINNING.PHOTO_TYPE = LOOSING.PHOTO_TYPE;
WINNING.PHOTO_VALUE = LOOSING.PHOTO_VALUE;
} else if (mode == 1 && WINNING.PHOTO == EMPTY) {
// Removing photo from loosing item.
if (LOOSING.PHOTO != EMPTY) {
SETLOOSINGCHANGED(1);