From 1924dad51b27c8eea576468cd644a24366f6e985 Mon Sep 17 00:00:00 2001 From: Clement Date: Fri, 20 Sep 2019 13:11:05 -0700 Subject: [PATCH] CR changes --- libraries/networking/src/AccountManager.cpp | 24 ++++--- libraries/networking/src/AccountManager.h | 4 +- libraries/networking/src/AccountSettings.cpp | 72 ++++++-------------- libraries/networking/src/AccountSettings.h | 42 ++++-------- 4 files changed, 56 insertions(+), 86 deletions(-) diff --git a/libraries/networking/src/AccountManager.cpp b/libraries/networking/src/AccountManager.cpp index 4e6fd3afb7..a31d117f59 100644 --- a/libraries/networking/src/AccountManager.cpp +++ b/libraries/networking/src/AccountManager.cpp @@ -47,6 +47,9 @@ Q_DECLARE_METATYPE(JSONCallbackParameters) const QString ACCOUNTS_GROUP = "accounts"; +const int POST_SETTINGS_INTERVAL = 10 * MSECS_PER_SECOND; +const int PULL_SETTINGS_RETRY_INTERVAL = 1 * MSECS_PER_SECOND; + JSONCallbackParameters::JSONCallbackParameters(QObject* callbackReceiver, const QString& jsonCallbackMethod, const QString& errorCallbackMethod) : @@ -89,7 +92,6 @@ AccountManager::AccountManager(UserAgentGetter userAgentGetter) : connect(this, &AccountManager::loginComplete, this, &AccountManager::uploadPublicKey); connect(this, &AccountManager::loginComplete, this, &AccountManager::requestAccountSettings); - static int POST_SETTINGS_INTERVAL = 10 * MSECS_PER_SECOND; _postSettingsTimer = new QTimer(this); _postSettingsTimer->setInterval(POST_SETTINGS_INTERVAL); connect(this, SIGNAL(loginComplete(QUrl)), _postSettingsTimer, SLOT(start())); @@ -98,7 +100,6 @@ AccountManager::AccountManager(UserAgentGetter userAgentGetter) : connect(qApp, &QCoreApplication::aboutToQuit, this, &AccountManager::postAccountSettings); } -const QString DOUBLE_SLASH_SUBSTITUTE = "slashslash"; const QString ACCOUNT_MANAGER_REQUESTED_SCOPE = "owner"; void AccountManager::logout() { @@ -114,6 +115,8 @@ void AccountManager::logout() { emit logoutComplete(); // the username has changed to blank emit usernameChanged(QString()); + + _settings.loggedOut(); } QString accountFileDir() { @@ -806,6 +809,8 @@ void AccountManager::requestAccountSettings() { QNetworkReply* lockerReply = networkAccessManager.get(lockerRequest); connect(lockerReply, &QNetworkReply::finished, this, &AccountManager::requestAccountSettingsFinished); connect(lockerReply, SIGNAL(error(QNetworkReply::NetworkError)), this, SLOT(requestAccountSettingsError(QNetworkReply::NetworkError))); + + _settings.startedLoading(); } void AccountManager::requestAccountSettingsFinished() { @@ -821,25 +826,27 @@ void AccountManager::requestAccountSettingsFinished() { emit accountSettingsLoaded(); } else { qCDebug(networking) << "Error in response for account settings: no data object"; + QTimer::singleShot(PULL_SETTINGS_RETRY_INTERVAL, this, &AccountManager::requestAccountSettings); } } else { - // TODO: error handling qCDebug(networking) << "Error in response for account settings" << lockerReply->errorString(); + QTimer::singleShot(PULL_SETTINGS_RETRY_INTERVAL, this, &AccountManager::requestAccountSettings); } } void AccountManager::requestAccountSettingsError(QNetworkReply::NetworkError error) { - // TODO: error handling qCWarning(networking) << "Account settings request encountered an error" << error; + QTimer::singleShot(PULL_SETTINGS_RETRY_INTERVAL, this, &AccountManager::requestAccountSettings); } void AccountManager::postAccountSettings() { - if (!_settings.somethingChanged()) { + if (_settings.lastChangeTimestamp() <= _lastSuccessfulSyncTimestamp && _lastSuccessfulSyncTimestamp != 0) { // Nothing changed, skipping settings post return; } if (!isLoggedIn()) { qCWarning(networking) << "Can't post account settings: Not logged in"; + return; } QNetworkAccessManager& networkAccessManager = NetworkAccessManager::getInstance(); @@ -853,6 +860,7 @@ void AccountManager::postAccountSettings() { lockerRequest.setHeader(QNetworkRequest::ContentTypeHeader, "application/json"); lockerRequest.setRawHeader(ACCESS_TOKEN_AUTHORIZATION_HEADER, _accountInfo.getAccessToken().authorizationHeaderValue()); + _currentSyncTimestamp = _settings.lastChangeTimestamp(); QJsonObject dataObj; dataObj.insert("locker", _settings.pack()); @@ -869,14 +877,14 @@ void AccountManager::postAccountSettingsFinished() { QJsonDocument jsonResponse = QJsonDocument::fromJson(lockerReply->readAll()); const QJsonObject& rootObject = jsonResponse.object(); - if (!rootObject.contains("status") || rootObject["status"].toString() != "success") { - // TODO: error handling + if (rootObject.contains("status") && rootObject["status"].toString() == "success") { + _lastSuccessfulSyncTimestamp = _currentSyncTimestamp; + } else { qCDebug(networking) << "Error in response for account settings post" << lockerReply->errorString(); } } void AccountManager::postAccountSettingsError(QNetworkReply::NetworkError error) { - // TODO: error handling qCWarning(networking) << "Post encountered an error" << error; } diff --git a/libraries/networking/src/AccountManager.h b/libraries/networking/src/AccountManager.h index a5ff2ee100..f29221d671 100644 --- a/libraries/networking/src/AccountManager.h +++ b/libraries/networking/src/AccountManager.h @@ -183,7 +183,9 @@ private: QString _configFileURL; AccountSettings _settings; - QTimer* _postSettingsTimer; + quint64 _currentSyncTimestamp { 0 }; + quint64 _lastSuccessfulSyncTimestamp { 0 }; + QTimer* _postSettingsTimer { nullptr }; }; #endif // hifi_AccountManager_h diff --git a/libraries/networking/src/AccountSettings.cpp b/libraries/networking/src/AccountSettings.cpp index ac6f54b40a..6562826dd7 100644 --- a/libraries/networking/src/AccountSettings.cpp +++ b/libraries/networking/src/AccountSettings.cpp @@ -15,25 +15,15 @@ #include #include "NetworkLogging.h" +#include "SharedUtil.h" -// Examples: -//static QString SOME_STRING_KEY { "some_string" }; -//static QString SOME_INT_KEY { "some_int" }; -//static QString SOME_BOOL_KEY { "some_bool" }; - -// Examples: -//QString AccountSettings::DEFAULT_SOME_STRING { "" }; -//int AccountSettings::DEFAULT_SOME_INT { 17 }; -//bool AccountSettings::DEFAULT_SOME_BOOL { true }; +static QString HOME_LOCATION_KEY { "home_location" }; QJsonObject AccountSettings::pack() { QJsonObject data; QReadLocker lock(&_settingsLock); -// Examples: -// data.insert(SOME_STRING_KEY, _someString); -// data.insert(SOME_INT_KEY, _someInt); -// data.insert(SOME_BOOL_KEY, _someBool); + data.insert(HOME_LOCATION_KEY, _homeLocation); return data; } @@ -41,43 +31,25 @@ QJsonObject AccountSettings::pack() { void AccountSettings::unpack(QJsonObject data) { QWriteLocker lock(&_settingsLock); -// Examples: -// auto it = data.find(SOME_STRING_KEY); -// _hasSomeString = it != data.end() && it->isString(); -// _someString = _hasSomeString ? it->toString() : DEFAULT_SOME_STRING; -// -// it = data.find(SOME_INT_KEY); -// _hasSomeInt = it != data.end() && it->isDouble(); -// _someInt = _hasSomeInt ? it->toInt() : DEFAULT_SOME_INT; -// -// it = data.find(SOME_BOOL_KEY); -// _hasSomeBool = it != data.end() && it->isBool(); -// _someBool = _hasSomeBool ? it->toBool() : DEFAULT_SOME_BOOL; + _lastChangeTimestamp = usecTimestampNow(); - _somethingChanged = false; + auto it = data.find(HOME_LOCATION_KEY); + _homeLocationState = it != data.end() && it->isString() ? Loaded : NotPresent; + _homeLocation = _homeLocationState == Loaded ? it->toString() : ""; } -// Examples: -//void AccountSettings::setSomeString(QString someString) { -// QWriteLocker lock(&_settingsLock); -// if (someString != _someString) { -// _somethingChanged = true; -// } -// _someString = someString; -//} -// -//void AccountSettings::setSomeInt(int someInt) { -// QWriteLocker lock(&_settingsLock); -// if (someInt != _someInt) { -// _somethingChanged = true; -// } -// _someInt = someInt; -//} -// -//void AccountSettings::setSomeBool(bool someBool) { -// QWriteLocker lock(&_settingsLock); -// if (someBool != _someBool) { -// _somethingChanged = true; -// } -// _someBool = someBool; -//} +void AccountSettings::setHomeLocation(QString homeLocation) { + QWriteLocker lock(&_settingsLock); + if (homeLocation != _homeLocation) { + _lastChangeTimestamp = usecTimestampNow(); + } + _homeLocation = homeLocation; +} + +void AccountSettings::startedLoading() { + _homeLocationState = Loading; +} + +void AccountSettings::loggedOut() { + _homeLocationState = LoggedOut; +} diff --git a/libraries/networking/src/AccountSettings.h b/libraries/networking/src/AccountSettings.h index 4a5205bd30..7bec5ce767 100644 --- a/libraries/networking/src/AccountSettings.h +++ b/libraries/networking/src/AccountSettings.h @@ -18,42 +18,30 @@ class AccountSettings { public: -// Examples: -// static QString DEFAULT_SOME_STRING; -// static int DEFAULT_SOME_INT; -// static bool DEFAULT_SOME_BOOL; + enum State { + LoggedOut, + Loading, + Loaded, + NotPresent + }; - bool somethingChanged() const { return _somethingChanged; } + void loggedOut(); + void startedLoading(); + quint64 lastChangeTimestamp() const { return _lastChangeTimestamp; } QJsonObject pack(); void unpack(QJsonObject data); -// Examples: -// bool hasSomeString() const { QReadLocker lock(&_settingsLock); return _hasSomeString; } -// QString getSomeString() const { QReadLocker lock(&_settingsLock); return _someString; } -// void setSomeString(QString someString); -// -// bool hasSomeInt() const { QReadLocker lock(&_settingsLock); return _hasSomeInt; } -// int getSomeInt() const { QReadLocker lock(&_settingsLock); return _someInt; } -// void setSomeInt(int someInt); -// -// bool hasSomeBool() const { QReadLocker lock(&_settingsLock); return _hasSomeBool; } -// bool getSomeBool() const { QReadLocker lock(&_settingsLock); return _someBool; } -// void setSomeBool(bool someBool); + State homeLocationState() const { QReadLocker lock(&_settingsLock); return _homeLocationState; } + QString getHomeLocation() const { QReadLocker lock(&_settingsLock); return _homeLocation; } + void setHomeLocation(QString homeLocation); private: mutable QReadWriteLock _settingsLock; - bool _somethingChanged { false }; + quint64 _lastChangeTimestamp { 0 }; -// Examples: -// bool _hasSomeString { false }; -// bool _hasSomeInt { false }; -// bool _hasSomeBool { false }; - -// Examples: -// QString _someString { DEFAULT_SOME_STRING }; -// int _someInt { DEFAULT_SOME_INT }; -// bool _someBool { DEFAULT_SOME_BOOL }; + State _homeLocationState { LoggedOut }; + QString _homeLocation; }; #endif /* hifi_AccountSettings_h */