New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent ModelObserver update backlogs #541

Merged
merged 9 commits into from Feb 22, 2017
View
@@ -1,6 +1,6 @@
module.exports = function({types: t, template}) {
const generator = template(`
UNTIL_FUNC(fail => {
UNTIL_FUNC(async fail => {
try {
ASSERT_CALL;
return true;
@@ -5,6 +5,8 @@ export default class ModelObserver {
this.activeModel = null;
this.activeModelData = null;
this.activeModelUpdateSubscription = null;
this.inProgress = false;
this.pending = false;
}
setActiveModel(model) {
@@ -15,6 +17,8 @@ export default class ModelObserver {
}
this.activeModel = model;
this.activeModelData = null;
this.inProgress = false;
this.pending = false;
this.didUpdate(model);
if (model) {
this.activeModelUpdateSubscription = model.onDidUpdate(() => this.refreshModelData(model));
@@ -25,18 +29,34 @@ export default class ModelObserver {
}
refreshModelData(model = this.activeModel) {
if (this.inProgress) {
this.pending = true;
return null;
}
this.lastModelDataRefreshPromise = this._refreshModelData(model);
return this.lastModelDataRefreshPromise;
}
async _refreshModelData(model) {
const fetchDataPromise = this.fetchData(model);
this.lastFetchDataPromise = fetchDataPromise;
const modelData = await fetchDataPromise;
if (fetchDataPromise === this.lastFetchDataPromise) {
this.activeModel = model;
this.activeModelData = modelData;
await this.didUpdate(model);
try {
this.inProgress = true;
const fetchDataPromise = this.fetchData(model);
this.lastFetchDataPromise = fetchDataPromise;
const modelData = await fetchDataPromise;
// Since we re-fetch immediately when the model changes,
// we need to ensure a fetch for an old active model
// does not trample the newer fetch for the newer active model.
if (fetchDataPromise === this.lastFetchDataPromise) {
this.activeModel = model;
this.activeModelData = modelData;
this.didUpdate(model);
}
} finally {
this.inProgress = false;
if (this.pending) {
this.pending = false;
this.refreshModelData();
}
}
}
View
@@ -90,7 +90,7 @@
"simulant": "^0.2.2",
"sinon": "2.0.0-pre.4",
"temp": "^0.8.3",
"test-until": "^1.0.0"
"test-until": "^1.1.1"
},
"consumedServices": {
"status-bar": {
@@ -49,9 +49,9 @@ describe('GitPanelController', function() {
await controller.getLastModelDataRefreshPromise();
assert.equal(controller.getActiveRepository(), repository);
assert.isUndefined(controller.refs.gitPanel.refs.repoLoadingMessage);
assert.isDefined(controller.refs.gitPanel.refs.stagingView);
assert.isDefined(controller.refs.gitPanel.refs.commitViewController);
await assert.async.isUndefined(controller.refs.gitPanel.refs.repoLoadingMessage);
await assert.async.isDefined(controller.refs.gitPanel.refs.stagingView);
await assert.async.isDefined(controller.refs.gitPanel.refs.commitViewController);
});
it('keeps the state of the GitPanelView in sync with the assigned repository', async function() {
@@ -86,7 +86,7 @@ describe('GitPanelController', function() {
fs.unlinkSync(path.join(workdirPath2, 'b.txt'));
repository2.refresh();
await controller.getLastModelDataRefreshPromise();
assert.deepEqual(controller.refs.gitPanel.props.unstagedChanges, await repository2.getUnstagedChanges());
await assert.async.deepEqual(controller.refs.gitPanel.props.unstagedChanges, await repository2.getUnstagedChanges());
});
it('displays the staged changes since the parent commit when amending', async function() {
@@ -100,7 +100,7 @@ describe('GitPanelController', function() {
isAmending: false,
});
await controller.getLastModelDataRefreshPromise();
assert.deepEqual(controller.refs.gitPanel.props.stagedChanges, []);
await assert.async.deepEqual(controller.refs.gitPanel.props.stagedChanges, []);
assert.equal(didChangeAmending.callCount, 0);
await controller.setAmending(true);
@@ -257,6 +257,7 @@ describe('GitPanelController', function() {
resolutionProgress, refreshResolutionProgress,
});
await controller.getLastModelDataRefreshPromise();
await etch.getScheduler().getNextUpdatePromise();
const gitPanel = controller.refs.gitPanel;
const stagingView = gitPanel.refs.stagingView;
@@ -319,6 +320,7 @@ describe('GitPanelController', function() {
didChangeAmending,
});
await controller.getLastModelDataRefreshPromise();
await etch.getScheduler().getNextUpdatePromise();
extractReferences();
});
@@ -387,6 +389,7 @@ describe('GitPanelController', function() {
resolutionProgress, refreshResolutionProgress,
});
await controller.getLastModelDataRefreshPromise();
await etch.getScheduler().getNextUpdatePromise();
extractReferences();
});
@@ -426,6 +429,7 @@ describe('GitPanelController', function() {
resolutionProgress, refreshResolutionProgress,
});
await controller.getLastModelDataRefreshPromise();
await etch.getScheduler().getNextUpdatePromise();
const stagingView = controller.refs.gitPanel.refs.stagingView;
const commitView = controller.refs.gitPanel.refs.commitViewController.refs.commitView;
@@ -434,14 +438,17 @@ describe('GitPanelController', function() {
await stagingView.dblclickOnItem({}, stagingView.props.unstagedChanges[0]).stageOperationPromise;
repository.refresh();
await controller.getLastModelDataRefreshPromise();
await etch.getScheduler().getNextUpdatePromise();
await stagingView.dblclickOnItem({}, stagingView.props.unstagedChanges[0]).stageOperationPromise;
repository.refresh();
await controller.getLastModelDataRefreshPromise();
await etch.getScheduler().getNextUpdatePromise();
assert.equal(stagingView.props.unstagedChanges.length, 0);
assert.equal(stagingView.props.stagedChanges.length, 2);
await stagingView.dblclickOnItem({}, stagingView.props.stagedChanges[1]).stageOperationPromise;
repository.refresh();
await controller.getLastModelDataRefreshPromise();
await etch.getScheduler().getNextUpdatePromise();
assert.equal(stagingView.props.unstagedChanges.length, 1);
assert.equal(stagingView.props.stagedChanges.length, 1);
@@ -467,6 +474,7 @@ describe('GitPanelController', function() {
resolutionProgress, refreshResolutionProgress,
});
await controller.getLastModelDataRefreshPromise();
await etch.getScheduler().getNextUpdatePromise();
const stagingView = controller.refs.gitPanel.refs.stagingView;
assert.equal(stagingView.props.mergeConflicts.length, 5);
@@ -487,6 +495,7 @@ describe('GitPanelController', function() {
await stagingView.dblclickOnItem({}, conflict1).stageOperationPromise;
repository.refresh();
await controller.getLastModelDataRefreshPromise();
await etch.getScheduler().getNextUpdatePromise();
assert.equal(atom.confirm.calledOnce, true);
assert.equal(stagingView.props.mergeConflicts.length, 5);
assert.equal(stagingView.props.stagedChanges.length, 0);
@@ -497,6 +506,7 @@ describe('GitPanelController', function() {
await stagingView.dblclickOnItem({}, conflict1).stageOperationPromise;
repository.refresh();
await controller.getLastModelDataRefreshPromise();
await etch.getScheduler().getNextUpdatePromise();
assert.equal(atom.confirm.calledOnce, true);
assert.equal(stagingView.props.mergeConflicts.length, 4);
assert.equal(stagingView.props.stagedChanges.length, 1);
@@ -508,6 +518,7 @@ describe('GitPanelController', function() {
await stagingView.dblclickOnItem({}, conflict2).stageOperationPromise;
repository.refresh();
await controller.getLastModelDataRefreshPromise();
await etch.getScheduler().getNextUpdatePromise();
assert.equal(atom.confirm.called, false);
assert.equal(stagingView.props.mergeConflicts.length, 3);
assert.equal(stagingView.props.stagedChanges.length, 2);
@@ -522,6 +533,7 @@ describe('GitPanelController', function() {
workspace, commandRegistry, repository, resolutionProgress, refreshResolutionProgress,
});
await controller.getLastModelDataRefreshPromise();
await etch.getScheduler().getNextUpdatePromise();
const stagingView = controller.refs.gitPanel.refs.stagingView;
assert.equal(stagingView.props.unstagedChanges.length, 2);
@@ -552,6 +564,7 @@ describe('GitPanelController', function() {
workspace, commandRegistry, repository, resolutionProgress, refreshResolutionProgress,
});
await controller.getLastModelDataRefreshPromise();
await etch.getScheduler().getNextUpdatePromise();
const stagingView = controller.refs.gitPanel.refs.stagingView;
const [addedFilePatch] = stagingView.props.unstagedChanges;
@@ -570,6 +583,7 @@ describe('GitPanelController', function() {
await repository.git.applyPatch(patchString, {index: true});
repository.refresh();
await controller.getLastModelDataRefreshPromise();
await etch.getScheduler().getNextUpdatePromise();
// since unstaged changes are calculated relative to the index,
// which now has new-file.txt on it, the working directory version of
@@ -2,6 +2,7 @@ import fs from 'fs';
import path from 'path';
import etch from 'etch';
import until from 'test-until';
import {cloneRepository, buildRepository, setUpLocalAndRemoteRepositories} from '../helpers';
import StatusBarTileController from '../../lib/controllers/status-bar-tile-controller';
@@ -29,6 +30,7 @@ describe('StatusBarTileController', function() {
const controller = new StatusBarTileController({workspace, repository, commandRegistry});
await controller.getLastModelDataRefreshPromise();
await etch.getScheduler().getNextUpdatePromise();
const branchView = controller.refs.branchView;
assert.equal(branchView.element.textContent, 'master');
@@ -54,6 +56,7 @@ describe('StatusBarTileController', function() {
const controller = new StatusBarTileController({workspace, repository, commandRegistry});
await controller.getLastModelDataRefreshPromise();
await etch.getScheduler().getNextUpdatePromise();
const branchMenuView = controller.branchMenuView;
const {list} = branchMenuView.refs;
@@ -88,6 +91,7 @@ describe('StatusBarTileController', function() {
const controller = new StatusBarTileController({workspace, repository, commandRegistry, notificationManager});
await controller.getLastModelDataRefreshPromise();
await etch.getScheduler().getNextUpdatePromise();
const branchMenuView = controller.branchMenuView;
const {list} = branchMenuView.refs;
@@ -117,6 +121,7 @@ describe('StatusBarTileController', function() {
const controller = new StatusBarTileController({workspace, repository, commandRegistry});
await controller.getLastModelDataRefreshPromise();
await etch.getScheduler().getNextUpdatePromise();
const branchMenuView = controller.branchMenuView;
const {list, newBranchButton} = branchMenuView.refs;
@@ -137,6 +142,7 @@ describe('StatusBarTileController', function() {
await newBranchButton.onclick();
repository.refresh();
await controller.getLastModelDataRefreshPromise();
await etch.getScheduler().getNextUpdatePromise();
assert.isUndefined(branchMenuView.refs.editor);
assert.isDefined(branchMenuView.refs.list);
@@ -153,6 +159,7 @@ describe('StatusBarTileController', function() {
const controller = new StatusBarTileController({workspace, repository, commandRegistry, notificationManager});
await controller.getLastModelDataRefreshPromise();
await etch.getScheduler().getNextUpdatePromise();
const branchMenuView = controller.branchMenuView;
const {list, newBranchButton} = branchMenuView.refs;
@@ -187,6 +194,7 @@ describe('StatusBarTileController', function() {
const controller = new StatusBarTileController({workspace, repository, commandRegistry});
await controller.getLastModelDataRefreshPromise();
await etch.getScheduler().getNextUpdatePromise();
const pushPullView = controller.refs.pushPullView;
const {aheadCount, behindCount} = pushPullView.refs;
@@ -196,12 +204,14 @@ describe('StatusBarTileController', function() {
await repository.git.exec(['reset', '--hard', 'head~2']);
repository.refresh();
await controller.getLastModelDataRefreshPromise();
await etch.getScheduler().getNextUpdatePromise();
assert.equal(aheadCount.textContent, '');
assert.equal(behindCount.textContent, '2');
await repository.git.commit('new local commit', {allowEmpty: true});
repository.refresh();
await controller.getLastModelDataRefreshPromise();
await etch.getScheduler().getNextUpdatePromise();
assert.equal(aheadCount.textContent, '1');
assert.equal(behindCount.textContent, '2');
@@ -223,6 +233,7 @@ describe('StatusBarTileController', function() {
const controller = new StatusBarTileController({workspace, repository, commandRegistry});
await controller.getLastModelDataRefreshPromise();
await etch.getScheduler().getNextUpdatePromise();
const pushPullMenuView = controller.pushPullMenuView;
const {pushButton, pullButton, fetchButton, message} = pushPullMenuView.refs;
@@ -232,12 +243,20 @@ describe('StatusBarTileController', function() {
assert.match(message.innerHTML, /No remote detected.*Pushing will set up a remote tracking branch/);
pushButton.dispatchEvent(new MouseEvent('click'));
repository.refresh();
await controller.getLastModelDataRefreshPromise();
assert.isFalse(pullButton.disabled);
assert.isFalse(fetchButton.disabled);
assert.equal(message.textContent, '');
await until(async fail => {
try {
repository.refresh();
await controller.getLastModelDataRefreshPromise();
await etch.getScheduler().getNextUpdatePromise();
assert.isFalse(pullButton.disabled);
assert.isFalse(fetchButton.disabled);
assert.equal(message.textContent, '');
return true;
} catch (err) {
return fail(err);
}
});
});
it('displays an error message if push fails and allows force pushing if meta key is pressed', async function() {
@@ -248,6 +267,7 @@ describe('StatusBarTileController', function() {
const controller = new StatusBarTileController({workspace, repository, commandRegistry, notificationManager});
await controller.getLastModelDataRefreshPromise();
await etch.getScheduler().getNextUpdatePromise();
const pushPullMenuView = controller.pushPullMenuView;
const {pushButton, pullButton} = pushPullMenuView.refs;
@@ -259,6 +279,7 @@ describe('StatusBarTileController', function() {
pushButton.dispatchEvent(new MouseEvent('click'));
await controller.getLastModelDataRefreshPromise();
await etch.getScheduler().getNextUpdatePromise();
await assert.async.isTrue(notificationManager.addError.called);
const notificationArgs = notificationManager.addError.args[0];
@@ -341,6 +362,7 @@ describe('StatusBarTileController', function() {
const toggleGitPanel = sinon.spy();
const controller = new StatusBarTileController({workspace, repository, toggleGitPanel, commandRegistry});
await controller.getLastModelDataRefreshPromise();
await etch.getScheduler().getNextUpdatePromise();
const changedFilesCountView = controller.refs.changedFilesCountView;
@@ -352,6 +374,7 @@ describe('StatusBarTileController', function() {
await repository.stageFiles(['a.txt']);
repository.refresh();
await controller.getLastModelDataRefreshPromise();
await etch.getScheduler().getNextUpdatePromise();
assert.equal(changedFilesCountView.element.textContent, '2 files');
Oops, something went wrong.