OpenSourceArk/Ghost
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Removed sync method from route settings loader

Browse Source 
refs: cf514cdf7

- in commit cf514cdf7 we moved the loadSettings call up to the bridge
- here we can call the async method, so we can remove loadSettingsSync altogether
- all the tests have now been changed to use the async method
This commit is contained in:
Hannah Wolfe 2022-04-10 11:40:55 +01:00 committed by Daniel Lockyer
parent 56af742a0b
commit 9a14c2de4c
No known key found for this signature in database
GPG Key ID: D21186F0B47295AD
4 changed files with 25 additions and 54 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
core/bridge.js
View File

@ -85,7 +85,7 @@ class Bridge {
const siteApp = require('./frontend/web/site');
const routerConfig = {
routeSettings: routeSettings.loadRouteSettingsSync(),
routeSettings: await routeSettings.loadRouteSettings(),
urlService
};

3
core/server/services/route-settings/index.js
View File

@ -28,9 +28,6 @@ module.exports = {
return await defaultSettingsManager.ensureSettingsFileExists();
},
get loadRouteSettingsSync() {
return settingsLoader.loadSettingsSync.bind(settingsLoader);
},
get loadRouteSettings() {
return settingsLoader.loadSettings.bind(settingsLoader);
},

44
core/server/services/route-settings/settings-loader.js
View File

@ -21,10 +21,9 @@ class SettingsLoader {
}
/**
* Functionally same as loadSettingsSync with exception of loading
* settings asynchronously. This method is used at new places to read settings
* to prevent blocking the eventloop
* @returns {Promise<Object>} settingsFile
* Reads the routes.yaml settings file and passes the
* file to the YAML parser which then returns a JSON object.
* @returns {Promise<RouteSettings>} settingsFileContents
*/
async loadSettings() {
try {
@ -49,36 +48,13 @@ class SettingsLoader {
});
}
}
/**
* Reads the routes.yaml settings file and passes the
* file to the YAML parser which then returns a JSON object.
*
* @returns {Object} settingsFile in following format: {routes: {}, collections: {}, resources: {}}
*/
loadSettingsSync() {
try {
const file = fs.readFileSync(this.settingFilePath, 'utf8');
debug('routes settings file found for:', this.settingFilePath);
const object = this.parseYaml(file);
debug('YAML settings file parsed:', this.settingFilePath);
return validate(object);
} catch (err) {
if (errors.utils.isGhostError(err)) {
throw err;
}
throw new errors.InternalServerError({
message: tpl(messages.settingsLoaderError, {
setting: 'routes',
path: this.settingFilePath
}),
err: err
});
}
}
}
module.exports = SettingsLoader;
/**
* @typedef {Object} RouteSettings
* @property {Object} routes
* @property {Object} collections
* @property {Object} taxonomies
*/

30
test/unit/server/services/route-settings/settings-loader.test.js
View File

@ -31,7 +31,7 @@ describe('UNIT > SettingsLoader:', function () {
validateStub = sinon.stub();
});
it('reads a settings object for routes.yaml file', function () {
it('reads a settings object for routes.yaml file', async function () {
const settingsLoader = new SettingsLoader({
parseYaml: yamlParserStub,
settingFilePath: '/content/data/routes.yaml'
@ -49,16 +49,16 @@ describe('UNIT > SettingsLoader:', function () {
author: '/author/{slug}/'
}
};
const fsReadFileStub = sinon.stub(fs, 'readFileSync').returns(settingsStubFile);
const fsReadFileStub = sinon.stub(fs, 'readFile').returns(settingsStubFile);
const result = settingsLoader.loadSettingsSync();
const result = await settingsLoader.loadSettings();
should.exist(result);
result.should.be.an.Object().with.properties('routes', 'collections', 'taxonomies');
fsReadFileStub.calledOnce.should.be.true();
});
it('can find yaml settings file and returns a settings object', function () {
const fsReadFileSpy = sinon.spy(fs, 'readFileSync');
it('can find yaml settings file and returns a settings object', async function () {
const fsReadFileSpy = sinon.spy(fs, 'readFile');
const storageFolderPath = path.join(__dirname, '../../../../utils/fixtures/settings/');
const expectedSettingsFile = path.join(storageFolderPath, 'routes.yaml');
@ -71,7 +71,7 @@ describe('UNIT > SettingsLoader:', function () {
parseYaml: yamlParserStub,
settingFilePath: expectedSettingsFile
});
const setting = settingsLoader.loadSettingsSync();
const setting = await settingsLoader.loadSettings();
should.exist(setting);
setting.should.be.an.Object().with.properties('routes', 'collections', 'taxonomies');
@ -80,7 +80,7 @@ describe('UNIT > SettingsLoader:', function () {
yamlParserStub.callCount.should.be.eql(1);
});
it('can handle errors from YAML parser', function (done) {
it('can handle errors from YAML parser', async function () {
const storageFolderPath = path.join(__dirname, '../../../../utils/fixtures/settings/');
yamlParserStub.throws(new errors.InternalServerError({
message: 'could not parse yaml file',
@ -92,25 +92,24 @@ describe('UNIT > SettingsLoader:', function () {
settingFilePath: path.join(storageFolderPath, 'routes.yaml')
});
try {
settingsLoader.loadSettingsSync();
done(new Error('Loader should fail'));
await settingsLoader.loadSettings();
throw new Error('Should have failed already');
} catch (err) {
should.exist(err);
err.message.should.be.eql('could not parse yaml file');
err.context.should.be.eql('bad indentation of a mapping entry at line 5, column 10');
yamlParserStub.calledOnce.should.be.true();
done();
}
});
it('throws error if file can\'t be accessed', function (done) {
it('throws error if file can\'t be accessed', async function () {
const storageFolderPath = path.join(__dirname, '../../../utils/fixtures/settings/');
const expectedSettingsFile = path.join(storageFolderPath, 'routes.yaml');
const fsError = new Error('no permission');
fsError.code = 'EPERM';
const originalFn = fs.readFileSync;
const fsReadFileStub = sinon.stub(fs, 'readFileSync').callsFake(function (filePath, options) {
const originalFn = fs.readFile;
const fsReadFileStub = sinon.stub(fs, 'readFile').callsFake(function (filePath, options) {
if (filePath.match(/routes\.yaml/)) {
throw fsError;
}
@ -126,13 +125,12 @@ describe('UNIT > SettingsLoader:', function () {
});
try {
settingsLoader.loadSettingsSync();
done(new Error('Loader should fail'));
await settingsLoader.loadSettings();
throw new Error('Should have failed already');
} catch (err) {
err.message.should.match(/Error trying to load YAML setting for routes from/);
fsReadFileStub.calledWith(expectedSettingsFile).should.be.true();
yamlParserStub.calledOnce.should.be.false();
done();
}
});
});