fixes for comments from code reviews

domain-server/resources/web/content/js/content.js

@@ -36,9 +36,7 @@ $(document).ready(function(){
 
   // when the selected file is changed, enable the button if there's a selected file
   $('body').on('change', '#' + RESTORE_SETTINGS_FILE_ID, function() {
-    if ($(this).val()) {
+    $('#' + RESTORE_SETTINGS_UPLOAD_ID).attr('disabled', $(this).val().length == 0);
-        $('#' + RESTORE_SETTINGS_UPLOAD_ID).attr('disabled', false);
-    }
   });
 
   // when the upload button is clicked, send the file to the DS
@@ -48,7 +46,7 @@ $(document).ready(function(){
     e.preventDefault();
 
     swalAreYouSure(
-      "Your domain content will be replaced by the uploaded Content Archive or entity file",
+      "Your domain content will be replaced by the uploaded content archive or entity file",
       "Restore content",
       function() {
         var files = $('#' + RESTORE_SETTINGS_FILE_ID).prop('files');
@@ -100,7 +98,7 @@ $(document).ready(function(){
     // construct the HTML needed for the content archives panel
     var html = "<div id='" + CONTENT_ARCHIVES_NORMAL_ID + "'><div class='form-group'>";
     html += "<label class='control-label'>Automatic Content Archives</label>";
-    html += "<span class='help-block'>Your domain server makes regular archives of the content in your domain. In the list below, you can see and download all of your domain content and settings backups. "
+    html += "<span class='help-block'>Your domain server makes regular archives of the content in your domain. In the list below, you can see and download all of your backups of domain content and content settings."
     html += "<a href='/settings/#automatic_content_archives' id='" + AUTO_ARCHIVES_SETTINGS_LINK_ID + "'>Click here to manage automatic content archive intervals.</a></span>";
     html += "</div>";
     html += "<table class='table sortable' id='" + AUTOMATIC_ARCHIVES_TABLE_ID + "'>";
@@ -149,6 +147,7 @@ $(document).ready(function(){
       if (isRestoring && !data.status.isRecovering) {
         // we were recovering and we finished - the DS is going to restart so show the restart modal
         showRestartModal();
+        return;
       }
 
       isRestoring = data.status.isRecovering;
@@ -171,7 +170,7 @@ $(document).ready(function(){
 
       function updateProgressBars($progressBar, value) {
         $progressBar.attr('aria-valuenow', value).attr('style', 'width: ' + value + '%');
-        $progressBar.find('.sr-only').html(data.status.recoveryProgress + "% Complete");
+        $progressBar.find('.sr-only').html(value + "% Complete");
       }
 
       // before we add any new rows and update existing ones
@@ -218,12 +217,9 @@ $(document).ready(function(){
         $backupRow.addClass(ACTIVE_BACKUP_ROW_CLASS);
       }
 
-      var automaticRows = "";
-
       if (automaticBackups.length > 0) {
         for (var backupIndex in automaticBackups) {
           updateOrAddTableRow(automaticBackups[backupIndex], AUTOMATIC_ARCHIVES_TBODY_ID);
-
         }
       }
 
@@ -382,7 +378,7 @@ $(document).ready(function(){
 
     // show a sweet alert to ask the user to provide a name for their content archive
     swal({
-      title: "Generate a Content Archive",
+      title: "Generate a content archive",
       type: "input",
       text: "This will capture the state of all the content in your domain right now, which you can save as a backup and restore from later.",
       confirmButtonText: "Generate Archive",

domain-server/src/ContentSettingsBackupHandler.cpp

@@ -23,20 +23,31 @@ static const QString CONTENT_SETTINGS_BACKUP_FILENAME = "content-settings.json";
 
 void ContentSettingsBackupHandler::createBackup(const QString& backupName, QuaZip& zip) {
 
-    // grab the content settings as JSON,excluding default values and values hidden from backup
+    // grab the content settings as JSON, excluding default values and values hidden from backup
-    QJsonObject contentSettingsJSON = _settingsManager.settingsResponseObjectForType("", true, false, true, false, true);
+    QJsonObject contentSettingsJSON = _settingsManager.settingsResponseObjectForType(
+        "", // include all settings types
+        DomainServerSettingsManager::Authenticated, DomainServerSettingsManager::NoDomainSettings,
+        DomainServerSettingsManager::IncludeContentSettings, DomainServerSettingsManager::NoDefaultSettings,
+        DomainServerSettingsManager::ForBackup
+    );
 
-    // make a QJSonDocument using the object
+    // make a QJsonDocument using the object
     QJsonDocument contentSettingsDocument { contentSettingsJSON };
 
     QuaZipFile zipFile { &zip };
 
-    zipFile.open(QIODevice::WriteOnly, QuaZipNewInfo(CONTENT_SETTINGS_BACKUP_FILENAME));
+    if (zipFile.open(QIODevice::WriteOnly, QuaZipNewInfo(CONTENT_SETTINGS_BACKUP_FILENAME))) {
-    zipFile.write(contentSettingsDocument.toJson());
+        if (zipFile.write(contentSettingsDocument.toJson()) == -1) {
-    zipFile.close();
+            qCritical().nospace() << "Failed to write to " << CONTENT_SETTINGS_BACKUP_FILENAME << ": " << zipFile.getZipError();
+        }
 
-    if (zipFile.getZipError() != UNZ_OK) {
+        zipFile.close();
-        qCritical().nospace() << "Failed to zip " << CONTENT_SETTINGS_BACKUP_FILENAME << ": " << zipFile.getZipError();
+
+        if (zipFile.getZipError() != UNZ_OK) {
+            qCritical().nospace() << "Failed to zip " << CONTENT_SETTINGS_BACKUP_FILENAME << ": " << zipFile.getZipError();
+        }
+    } else {
+        qCritical().nospace() << "Failed to open " << CONTENT_SETTINGS_BACKUP_FILENAME << ": " << zipFile.getZipError();
     }
 }
 
@@ -59,7 +70,5 @@ void ContentSettingsBackupHandler::recoverBackup(const QString& backupName, QuaZ
 
     if (!_settingsManager.restoreSettingsFromObject(jsonDocument.object(), ContentSettings)) {
         qCritical() << "Failed to restore settings from" << CONTENT_SETTINGS_BACKUP_FILENAME << "in content archive";
-        return;
     }
-
 }

domain-server/src/DomainGatekeeper.cpp

@@ -613,7 +613,7 @@ bool DomainGatekeeper::isWithinMaxCapacity() {
     // find out what our maximum capacity is
     QVariant maximumUserCapacityVariant =
         _server->_settingsManager.valueForKeyPath(MAXIMUM_USER_CAPACITY);
-    unsigned int maximumUserCapacity = !maximumUserCapacityVariant.isValid() ? maximumUserCapacityVariant.toUInt() : 0;
+    unsigned int maximumUserCapacity = maximumUserCapacityVariant.isValid() ? maximumUserCapacityVariant.toUInt() : 0;
 
     if (maximumUserCapacity > 0) {
         unsigned int connectedUsers = _server->countConnectedUsers();

domain-server/src/DomainServer.cpp

@@ -751,7 +751,7 @@ bool DomainServer::resetAccountManagerAccessToken() {
         if (accessToken.isEmpty()) {
             QVariant accessTokenVariant = _settingsManager.valueForKeyPath(ACCESS_TOKEN_KEY_PATH);
 
-            if (accessTokenVariant.isValid() && accessTokenVariant.canConvert(QMetaType::QString)) {
+            if (accessTokenVariant.canConvert(QMetaType::QString)) {
                 accessToken = accessTokenVariant.toString();
             } else {
                 qWarning() << "No access token is present. Some operations that use the metaverse API will fail.";
@@ -1637,7 +1637,6 @@ void DomainServer::sendHeartbeatToIceServer() {
             qWarning() << "Waiting for keypair generation to complete before sending ICE heartbeat.";
 
             if (!limitedNodeList->getSessionUUID().isNull()) {
-                qDebug() << "generating keypair";
                 accountManager->generateNewDomainKeypair(limitedNodeList->getSessionUUID());
             } else {
                 qWarning() << "Attempting to send ICE server heartbeat with no domain ID. This is not supported";

domain-server/src/DomainServerSettingsManager.cpp

@@ -33,7 +33,6 @@
 #include <SettingHelpers.h>
 #include <AvatarData.h> //for KillAvatarReason
 #include <FingerprintUtils.h>
-#include <shared/QtHelpers.h>
 
 #include "DomainServerNodeData.h"
 
@@ -686,12 +685,12 @@ void DomainServerSettingsManager::unpackPermissions() {
 
 #ifdef WANT_DEBUG
     qDebug() << "--------------- permissions ---------------------";
-    std::list<NodePermissionsMap*> permissionsSets {
+    std::array<NodePermissionsMap*, 7> permissionsSets {{
         &_standardAgentPermissions, &_agentPermissions,
         &_groupPermissions, &_groupForbiddens,
         &_ipPermissions, &_macPermissions,
         &_machineFingerprintPermissions
-    };
+    }};
 
     foreach (auto permissionSet, permissionsSets) {
         auto& permissionKeyMap = permissionSet->get();
@@ -1169,17 +1168,20 @@ bool DomainServerSettingsManager::handleAuthenticatedHTTPRequest(HTTPConnection
 
             QJsonObject rootObject;
 
-            bool forDomainSettings = (url.path() == SETTINGS_PATH_JSON);
+            DomainSettingsInclusion domainSettingsInclusion = (url.path() == SETTINGS_PATH_JSON)
-            bool forContentSettings = (url.path() == CONTENT_SETTINGS_PATH_JSON);;
+                ? IncludeDomainSettings : NoDomainSettings;
+            ContentSettingsInclusion contentSettingsInclusion = (url.path() == CONTENT_SETTINGS_PATH_JSON)
+                ? IncludeContentSettings : NoContentSettings;
 
-            rootObject[SETTINGS_RESPONSE_DESCRIPTION_KEY] = forDomainSettings
+            rootObject[SETTINGS_RESPONSE_DESCRIPTION_KEY] = (url.path() == SETTINGS_PATH_JSON)
                 ? _domainSettingsDescription : _contentSettingsDescription;
 
             // grab a domain settings object for all types, filtered for the right class of settings
             // and exclude default values
-            rootObject[SETTINGS_RESPONSE_VALUE_KEY] = settingsResponseObjectForType("", true,
+            rootObject[SETTINGS_RESPONSE_VALUE_KEY] = settingsResponseObjectForType("", Authenticated,
-                                                                                    forDomainSettings, forContentSettings,
+                                                                                    domainSettingsInclusion,
-                                                                                    true);
+                                                                                    contentSettingsInclusion,
+                                                                                    IncludeDefaultSettings);
 
             connection->respond(HTTPConnection::StatusCode200, QJsonDocument(rootObject).toJson(), "application/json");
 
@@ -1191,7 +1193,8 @@ bool DomainServerSettingsManager::handleAuthenticatedHTTPRequest(HTTPConnection
         } else if (url.path() == SETTINGS_BACKUP_PATH) {
             // grab the settings backup as an authenticated user
             // for the domain settings type only, excluding hidden and default values
-            auto currentDomainSettingsJSON = settingsResponseObjectForType("", true, true, false, false, true);
+            auto currentDomainSettingsJSON = settingsResponseObjectForType("", Authenticated, IncludeDomainSettings,
+                                                                           NoContentSettings, NoDefaultSettings, ForBackup);
 
             // setup headers that tell the client to download the file wth a special name
             Headers downloadHeaders;
@@ -1343,13 +1346,15 @@ bool DomainServerSettingsManager::restoreSettingsFromObject(QJsonObject settings
     }
 }
 
-QJsonObject DomainServerSettingsManager::settingsResponseObjectForType(const QString& typeValue, bool isAuthenticated,
+QJsonObject DomainServerSettingsManager::settingsResponseObjectForType(const QString& typeValue,
-                                                                       bool includeDomainSettings,
+                                                                       SettingsRequestAuthentication authentication,
-                                                                       bool includeContentSettings,
+                                                                       DomainSettingsInclusion domainSettingsInclusion,
-                                                                       bool includeDefaults, bool isForBackup) {
+                                                                       ContentSettingsInclusion contentSettingsInclusion,
+                                                                       DefaultSettingsInclusion defaultSettingsInclusion,
+                                                                       SettingsBackupFlag settingsBackupFlag) {
     QJsonObject responseObject;
 
-    if (!typeValue.isEmpty() || isAuthenticated) {
+    if (!typeValue.isEmpty() || authentication == Authenticated) {
         // convert the string type value to a QJsonValue
         QJsonValue queryType = typeValue.isEmpty() ? QJsonValue() : QJsonValue(typeValue.toInt());
 
@@ -1358,9 +1363,9 @@ QJsonObject DomainServerSettingsManager::settingsResponseObjectForType(const QSt
         // only enumerate the requested settings type (domain setting or content setting)
         QJsonArray* filteredDescriptionArray = &_descriptionArray;
 
-        if (includeDomainSettings && !includeContentSettings) {
+        if (domainSettingsInclusion == IncludeDomainSettings && contentSettingsInclusion != IncludeContentSettings) {
             filteredDescriptionArray = &_domainSettingsDescription;
-        } else if (includeContentSettings && !includeDomainSettings) {
+        } else if (contentSettingsInclusion == IncludeContentSettings && domainSettingsInclusion != IncludeDomainSettings) {
             filteredDescriptionArray = &_contentSettingsDescription;
         }
 
@@ -1383,14 +1388,14 @@ QJsonObject DomainServerSettingsManager::settingsResponseObjectForType(const QSt
                 bool includedInBackups = !settingObject.contains(DESCRIPTION_BACKUP_FLAG_KEY)
                     || settingObject[DESCRIPTION_BACKUP_FLAG_KEY].toBool();
 
-                if (!settingObject[VALUE_HIDDEN_FLAG_KEY].toBool() && (!isForBackup || includedInBackups)) {
+                if (!settingObject[VALUE_HIDDEN_FLAG_KEY].toBool() && (settingsBackupFlag != ForBackup || includedInBackups)) {
                     QJsonArray affectedTypesArray = settingObject[AFFECTED_TYPES_JSON_KEY].toArray();
                     if (affectedTypesArray.isEmpty()) {
                         affectedTypesArray = groupObject[AFFECTED_TYPES_JSON_KEY].toArray();
                     }
 
                     if (affectedTypesArray.contains(queryType) ||
-                        (queryType.isNull() && isAuthenticated)) {
+                        (queryType.isNull() && authentication == Authenticated)) {
                         QString settingName = settingObject[DESCRIPTION_NAME_KEY].toString();
 
                         // we need to check if the settings map has a value for this setting
@@ -1408,7 +1413,7 @@ QJsonObject DomainServerSettingsManager::settingsResponseObjectForType(const QSt
 
                         // final check for inclusion
                         // either we include default values or we don't but this isn't a default value
-                        if (includeDefaults || variantValue.isValid()) {
+                        if ((defaultSettingsInclusion == IncludeDefaultSettings) || variantValue.isValid()) {
                             QJsonValue result;
 
                             if (!variantValue.isValid()) {

domain-server/src/DomainServerSettingsManager.h

@@ -110,10 +110,19 @@ public:
 
     void debugDumpGroupsState();
 
+    enum SettingsRequestAuthentication { NotAuthenticated, Authenticated };
+    enum DomainSettingsInclusion { NoDomainSettings, IncludeDomainSettings };
+    enum ContentSettingsInclusion { NoContentSettings, IncludeContentSettings };
+    enum DefaultSettingsInclusion { NoDefaultSettings, IncludeDefaultSettings };
+    enum SettingsBackupFlag { NotForBackup, ForBackup };
+
     /// thread safe method to retrieve a JSON representation of settings
-    Q_INVOKABLE QJsonObject settingsResponseObjectForType(const QString& typeValue, bool isAuthenticated = false,
+    QJsonObject settingsResponseObjectForType(const QString& typeValue,
-                                              bool includeDomainSettings = true, bool includeContentSettings = true,
+                                              SettingsRequestAuthentication authentication = NotAuthenticated,
-                                              bool includeDefaults = true, bool isForBackup = false);
+                                              DomainSettingsInclusion domainSettingsInclusion = IncludeDomainSettings,
+                                              ContentSettingsInclusion contentSettingsInclusion = IncludeContentSettings,
+                                              DefaultSettingsInclusion defaultSettingsInclusion = IncludeDefaultSettings,
+                                              SettingsBackupFlag settingsBackupFlag = NotForBackup);
     /// thread safe method to restore settings from a JSON object
     Q_INVOKABLE bool restoreSettingsFromObject(QJsonObject settingsToRestore, SettingsType settingsType);
 
@@ -156,7 +165,7 @@ private:
     QJsonArray _contentSettingsDescription;
     QJsonObject _settingsMenuGroups;
 
-    // any method that calls _valueForKeyPath on this _configMap must get a write lock it keeps until it
+    // any method that calls valueForKeyPath on this _configMap must get a write lock it keeps until it
     // is done with the returned QVariant*
     HifiConfigVariantMap _configMap;