Skip to content

Commit 873c416

Browse files
committed
Use snapshots and locks for all config reads/writes
1 parent 6e2b28d commit 873c416

File tree

3 files changed

+42
-16
lines changed

3 files changed

+42
-16
lines changed

spec/tests/Base-spec.js

+17-3
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,26 @@ const { Base } = NodeGit.Flow.__TEST__;
77
describe('Base', function() {
88
beforeEach(function() {
99
const repoConfig = {
10-
getString(key) {
11-
const defaultConfig = NodeGit.Flow.getConfigDefault();
12-
return defaultConfig[key] || true;
10+
lock() {
11+
return Promise.resolve({
12+
commit() {
13+
return Promise.resolve();
14+
}
15+
});
16+
},
17+
getString() {
18+
throw new Error('Do not call config.getString without taking a snapshot first.');
1319
},
1420
setString() {
1521
return Promise.resolve();
22+
},
23+
snapshot() {
24+
return Promise.resolve({
25+
getString(key) {
26+
const defaultConfig = NodeGit.Flow.getConfigDefault();
27+
return defaultConfig[key] || true;
28+
}
29+
});
1630
}
1731
};
1832
this.repo = {

src/Base.js

+20-10
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,8 @@ module.exports = (NodeGit, { constants }, { Config, Feature, Hotfix, Release })
4242
}
4343

4444
return repo.config()
45-
.then((config) => config.getString('gitflow.branch.develop'))
45+
.then((config) => config.snapshot())
46+
.then((snapshot) => snapshot.getString('gitflow.branch.develop'))
4647
.then((developBranchName) => NodeGit.Branch.lookup(repo, developBranchName, NodeGit.Branch.BRANCH.LOCAL))
4748
.then(() => true)
4849
.catch(() => false);
@@ -96,12 +97,19 @@ module.exports = (NodeGit, { constants }, { Config, Feature, Hotfix, Release })
9697
.then(() => repo.config())
9798
.then((config) => {
9899
// Set the config values. We chain them so we don't have concurrent setString calls to the same config file
99-
return configKeys.reduce((last, next) => {
100-
return last
101-
.then(() => {
102-
return config.setString(next, configToUse[next]);
103-
});
104-
}, Promise.resolve());
100+
return config.lock().then((transaction) => {
101+
const setPromise = configKeys.reduce((last, next) => {
102+
return last
103+
.then(() => {
104+
return config.setString(next, configToUse[next]);
105+
});
106+
}, Promise.resolve());
107+
108+
// NOTE The only way to unlock is to call `commit` or to free the transaction.
109+
// NodeGit doesn't expose explicitly freeing the transaction, so if setPromise rejects,
110+
// we simply wait for it to self-free.
111+
return setPromise.then(() => transaction.commit());
112+
});
105113
})
106114
.then(() => createFlowInstance(repo));
107115
}
@@ -118,9 +126,10 @@ module.exports = (NodeGit, { constants }, { Config, Feature, Hotfix, Release })
118126
}
119127

120128
return repo.config()
121-
.then((config) => {
129+
.then((config) => config.snapshot())
130+
.then((snapshot) => {
122131
const promises = Config.getConfigRequiredKeys().map((key) => {
123-
return config.getString(key);
132+
return snapshot.getString(key);
124133
});
125134

126135
return Promise.all(promises)
@@ -147,7 +156,8 @@ module.exports = (NodeGit, { constants }, { Config, Feature, Hotfix, Release })
147156
}
148157

149158
return repo.config()
150-
.then((config) => config.getString('gitflow.branch.master'))
159+
.then((config) => config.snapshot())
160+
.then((snapshot) => snapshot.getString('gitflow.branch.master'))
151161
.then((masterBranchName) => NodeGit.Branch.lookup(repo, masterBranchName, NodeGit.Branch.BRANCH.LOCAL))
152162
.then(() => true)
153163
.catch(() => false);

src/Config.js

+5-3
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,8 @@ module.exports = (NodeGit, { constants }) => {
1313
}
1414

1515
return repo.config()
16-
.then((config) => config.getString(configKey))
16+
.then((config) => config.snapshot())
17+
.then((snapshot) => snapshot.getString(configKey))
1718
.catch(() => Promise.reject(new Error(`Failed to read config value ${configKey}`)));
1819
}
1920

@@ -89,9 +90,10 @@ module.exports = (NodeGit, { constants }) => {
8990
const configKeys = _getConfigKeys();
9091

9192
return repo.config()
92-
.then((config) => {
93+
.then((config) => config.snapshot())
94+
.then((snapshot) => {
9395
const promises = configKeys.map((key) => {
94-
return config.getString(key);
96+
return snapshot.getString(key);
9597
});
9698

9799
return Promise.all(promises);

0 commit comments

Comments
 (0)