KEMBAR78
Prevent ModelObserver update backlogs by BinaryMuse · Pull Request #541 · atom/github · GitHub
Skip to content
This repository was archived by the owner on Dec 15, 2022. It is now read-only.

Prevent ModelObserver update backlogs #541

Merged
merged 9 commits into from
Feb 22, 2017
Merged

Prevent ModelObserver update backlogs #541

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
41b3590
Update ModelObserver to prevent update backlogs
BinaryMuse Feb 19, 2017
24f8969
Re-fetch immediately when active model changes
Feb 20, 2017
a10100f
Update ModelObserver tests
Feb 20, 2017
139d965
:fire: unused method call counts
Feb 21, 2017
2d98659
Improve some tests
Feb 21, 2017
10ef38b
:arrow_up: test-until to allow latch functions to use `await`
BinaryMuse Feb 22, 2017
ad806a1
Don't await didUpdate result in ModelObserver
BinaryMuse Feb 22, 2017
c981d0b
Merge remote-tracking branch 'origin/master' into mkt-ku-prevent-mode…
BinaryMuse Feb 22, 2017
69a24b6
Flakes!
BinaryMuse Feb 22, 2017
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
2 changes: 1 addition & 1 deletion assert-async-plugin.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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;
Expand Down
34 changes: 27 additions & 7 deletions lib/models/model-observer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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));
Expand All @@ -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();
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down
24 changes: 19 additions & 5 deletions test/controllers/git-panel-controller.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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() {
Expand All @@ -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);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -319,6 +320,7 @@ describe('GitPanelController', function() {
didChangeAmending,
});
await controller.getLastModelDataRefreshPromise();
await etch.getScheduler().getNextUpdatePromise();

extractReferences();
});
Expand Down Expand Up @@ -387,6 +389,7 @@ describe('GitPanelController', function() {
resolutionProgress, refreshResolutionProgress,
});
await controller.getLastModelDataRefreshPromise();
await etch.getScheduler().getNextUpdatePromise();

extractReferences();
});
Expand Down Expand Up @@ -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;

Expand All @@ -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);

Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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;
Expand All @@ -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
Expand Down
35 changes: 29 additions & 6 deletions test/controllers/status-bar-tile-controller.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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');
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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);
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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');

Expand All @@ -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;
Expand All @@ -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() {
Expand All @@ -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;
Expand All @@ -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];
Expand Down Expand Up @@ -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;

Expand All @@ -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');

Expand Down
Loading