Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

AutoUpdate Framework #14177

Merged
merged 16 commits into from
Apr 18, 2018
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/brackets.config.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
"extension_registry" : "https://s3.amazonaws.com/extend.brackets/registry.json",
"extension_url" : "https://s3.amazonaws.com/extend.brackets/{0}/{0}-{1}.zip",
"linting.enabled_by_default" : true,
"build_timestamp" : ""
"build_timestamp" : "",
"update_download_url" : "https://github.com/adobe/brackets/releases/download/"
}
}
19 changes: 18 additions & 1 deletion src/document/DocumentCommandHandlers.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@ define(function (require, exports, module) {
StatusBar = require("widgets/StatusBar"),
WorkspaceManager = require("view/WorkspaceManager"),
LanguageManager = require("language/LanguageManager"),
_ = require("thirdparty/lodash");
_ = require("thirdparty/lodash"),
UpdateNotification = require("utils/UpdateNotification");

/**
* Handlers for commands related to document handling (opening, saving, etc.)
Expand Down Expand Up @@ -848,6 +849,19 @@ define(function (require, exports, module) {
return result.promise();
}

/**
* Checks and handles if auto update is currently in progress
*/
function handleIfAutoUpdateInProgress() {
brackets.app.isAutoUpdateInProgress(function (err, isAutoUpdateInProgress) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add if (brackets.app.isAutoUpdateInProgress) { to check if this function is registered or not.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isAutoUpdateInProgress() function has been added to the core. It is not a registered function. Hence, we need not have this check.

Copy link
Contributor

@nethip nethip Apr 9, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mbhavi It is a registered in appshell_extensions.js. Consider this scenario where a user who is running the older shell but with newer brackets code, will never find this function, so it is a good idea to bailout. Please add a check here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I strongly believe suppressing the error would turn out to be harmful in this case, not showing the error to the user would be misleading to him/her believing that the feature can work with just the brackets changes.
Moreover, I don't see that with any of the other functions' invocations. I believe doing that will be unnecessary bloating of the code, as this is most likely to happen only in the dev machines, which I think should be okay.

if(err) {
UpdateNotification.trigger(UpdateNotification.AUTOUPDATE_ERROR);
} else if(isAutoUpdateInProgress){
UpdateNotification.trigger(UpdateNotification.DIRTY_FILESAVE_CANCELLED);
}
});
}

/**
* Opens the native OS save as dialog and saves document.
* The original document is reverted in case it was dirty.
Expand Down Expand Up @@ -996,6 +1010,7 @@ define(function (require, exports, module) {
if (selectedPath) {
_doSaveAfterSaveDialog(selectedPath);
} else {
handleIfAutoUpdateInProgress();
result.reject(USER_CANCELED);
}
} else {
Expand Down Expand Up @@ -1217,6 +1232,7 @@ define(function (require, exports, module) {
)
.done(function (id) {
if (id === Dialogs.DIALOG_BTN_CANCEL) {
handleIfAutoUpdateInProgress();
result.reject();
} else if (id === Dialogs.DIALOG_BTN_OK) {
// "Save" case: wait until we confirm save has succeeded before closing
Expand Down Expand Up @@ -1322,6 +1338,7 @@ define(function (require, exports, module) {
)
.done(function (id) {
if (id === Dialogs.DIALOG_BTN_CANCEL) {
handleIfAutoUpdateInProgress();
result.reject();
} else if (id === Dialogs.DIALOG_BTN_OK) {
// Save all unsaved files, then if that succeeds, close all
Expand Down
39 changes: 39 additions & 0 deletions src/extensions/default/AutoUpdate/MessageIds.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/*
* Copyright (c) 2018 - present Adobe Systems Incorporated. All rights reserved.
*
* Permission is hereby granted, free of charge, to any person obtaining a
* copy of this software and associated documentation files (the "Software"),
* to deal in the Software without restriction, including without limitation
* the rights to use, copy, modify, merge, publish, distribute, sublicense,
* and/or sell copies of the Software, and to permit persons to whom the
* Software is furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
* FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
* DEALINGS IN THE SOFTWARE.
*
*/

define(function (require, exports, module) {
"use strict";

exports.DOWNLOAD_INSTALLER = "node.downloadInstaller";
exports.PERFORM_CLEANUP = "node.performCleanup";
exports.VALIDATE_INSTALLER = "node.validateInstaller";
exports.INITIALIZE_STATE = "node.initializeState";
exports.SHOW_STATUS_INFO = "brackets.showStatusInfo";
exports.NOTIFY_DOWNLOAD_SUCCESS = "brackets.notifyDownloadSuccess";
exports.SHOW_ERROR_MESSAGE = "brackets.showErrorMessage";
exports.NOTIFY_DOWNLOAD_FAILURE = "brackets.notifyDownloadFailure";
exports.NOTIFY_SAFE_TO_DOWNLOAD = "brackets.notifySafeToDownload";
exports.NOTIFY_INITIALIZATION_COMPLETE = "brackets.notifyinitializationComplete";
exports.NOTIFY_VALIDATION_STATUS = "brackets.notifyvalidationStatus";
exports.REGISTER_BRACKETS_FUNCTIONS = "brackets.registerBracketsFunctions";
});
208 changes: 208 additions & 0 deletions src/extensions/default/AutoUpdate/StateHandler.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,208 @@
/*
* Copyright (c) 2018 - present Adobe Systems Incorporated. All rights reserved.
*
* Permission is hereby granted, free of charge, to any person obtaining a
* copy of this software and associated documentation files (the "Software"),
* to deal in the Software without restriction, including without limitation
* the rights to use, copy, modify, merge, publish, distribute, sublicense,
* and/or sell copies of the Software, and to permit persons to whom the
* Software is furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
* FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
* DEALINGS IN THE SOFTWARE.
*
*/

define(function (require, exports, module) {
"use strict";

var FileSystem = brackets.getModule("filesystem/FileSystem"),
FileUtils = brackets.getModule("file/FileUtils");

var FILE_NOT_FOUND = 0,
FILE_NOT_READ = 1,
FILE_PARSE_EXCEPTION = 2,
FILE_READ_FAIL = 3;

/**
* @constructor
* Creates a StateHandler object for a JSON file. It maintains the following :
* path - path to the JSON file,
* state - parsed content of the file
* @param {string} path - path to the JSON file
*/
var StateHandler = function (path) {
this.path = path;
this.state = null;
};

/**
* Checks if the file exists
* @returns {$.Deferred} - a jquery deferred promise,
* that is resolved with existence or non-existence
* of json file.
*/
StateHandler.prototype.exists = function () {
var result = $.Deferred(),
_file = FileSystem.getFileForPath(this.path);

_file.exists(function (err, exists) {
if(err) {
result.reject();
} else if(exists) {
result.resolve();
} else {
result.reject();
}
});

return result.promise();
};

/**
* Parses the JSON file, and maintains a state for the parsed data
* @returns {$.Deferred} - a jquery deferred promise,
* that is resolved with a parsing success or failure
*/
StateHandler.prototype.parse = function () {
var result = $.Deferred(),
_file = FileSystem.getFileForPath(this.path);
var self = this;

this.exists()
.done(function () {
FileUtils.readAsText(_file)
.done(function (text) {
try {
if (text) {
self.state = JSON.parse(text);
result.resolve();
} else {
result.reject(FILE_READ_FAIL);
}
} catch (error) {
result.reject(FILE_PARSE_EXCEPTION);
}
})
.fail(function () {
result.reject(FILE_NOT_READ);
});

})
.fail(function () {
result.reject(FILE_NOT_FOUND);
});

return result.promise();
};

/**
* Sets the value of a key in a json file.
* @param {string} key - key for which the value is to be set
* @param {string} value - the value to be set for the given key
* @returns {$.Deferred} - a jquery deferred promise, that is resolved with a write success or failure
*/
StateHandler.prototype.set = function (key, value) {
this.state = this.state || {};
this.state[key] = value;

return this.write(true);
};

/**
* Gets the value for a given key, from the in-memory state maintained for a json file.
* @param {string} key - key for which value is to be retrieved
* @returns {string} value for the given key
*/
StateHandler.prototype.get = function (key) {
var retval = null;

if (this.state && this.state[key]) {
retval = this.state[key];
}

return retval;
};


/**
* Performs the write of JSON object to a file.
* @param {string} filepath - path to JSON file
* @param {object} json - JSON object to write
* @returns {$.Deferred} - a jquery deferred promise,
* that is resolved with the write success or failure
*/
function _write(filePath, json) {
var result = $.Deferred(),
_file = FileSystem.getFileForPath(filePath);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check for validity of _file

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_write is an internal function, which I have wrapped inside the function writePromise(), which in turn, is wrapped inside the write() function, which checks for the validity of the file. Hence, checking here is redundant.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there is not harm in checking for validity of _file given the fact that you are going to stringify the JSON irrespective of whether _file is valid or not.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nethip , the requirement here is to do a force write. If the file exists, the content is overwritten. If it doesn't exist, the file is created and written. I am passing the boolean to be "true" in writeText, which ensures this.
Hence, the validity is not needed here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mbhavi Let us not have discussion of having a null check or not. It is a simple null check. If you still feel that is not required, post merging this PR, one of us will add it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nethip , addressed in 4fb6582


var content = JSON.stringify(json);
FileUtils.writeText(_file, content, true)
.done(function () {
result.resolve();
})
.fail(function (err) {
result.reject();
});

return result.promise();
}
/**
* Writes content into a json file
* @param {boolean} overwrite - true if file is to be overwritten, false otherwise
* @param {object} [content=this.state] - content to be written into the json file.
* @returns {$.Deferred} - a jquery deferred promise, that is resolved with a write success or failure
*/
StateHandler.prototype.write = function (overwrite) {
var result = $.Deferred(),
self = this;

function writePromise(path, contentToWrite) {
_write(path, contentToWrite)
.done(function () {
result.resolve();
})
.fail(function (err) {
result.reject();
});
}

var content = self.state;
if (overwrite) {
writePromise(self.path, content);
} else {
//check for existence
self.exists()
.fail(function () {
writePromise(self.path, content);
}).done(function (){
result.reject();
});
}

return result.promise();
};

/**
* Resets the content of the in-memory state
*/
StateHandler.prototype.reset = function () {
this.state = null;
};

exports.StateHandler = StateHandler;
exports.MessageKeys = {
FILE_NOT_FOUND: FILE_NOT_FOUND,
FILE_NOT_READ: FILE_NOT_READ,
FILE_PARSE_EXCEPTION: FILE_PARSE_EXCEPTION,
FILE_READ_FAIL: FILE_READ_FAIL
};
});
Loading