From 8c2f563b2856165d5fb14bdda1c82ade80c739d7 Mon Sep 17 00:00:00 2001 From: atrovato <1839717+atrovato@users.noreply.github.com> Date: Sun, 20 Dec 2020 11:08:08 +0100 Subject: [PATCH 1/3] Remove NOT_CONFIGURED service status --- front/src/config/demo.json | 6 +++--- front/src/config/i18n/en.json | 4 ---- front/src/config/i18n/fr.json | 4 ---- .../src/routes/settings/settings-service/ServiceItem.jsx | 9 ++------- server/lib/service/service.start.js | 2 +- ...0201220100508-service-remove-not-configured-status.js | 6 ++++++ server/test/lib/service/service.start.test.js | 6 +++--- server/utils/constants.js | 1 - 8 files changed, 15 insertions(+), 23 deletions(-) create mode 100644 server/migrations/20201220100508-service-remove-not-configured-status.js diff --git a/front/src/config/demo.json b/front/src/config/demo.json index dfef8ae0ca..f6732815e3 100644 --- a/front/src/config/demo.json +++ b/front/src/config/demo.json @@ -1124,7 +1124,7 @@ "selector": "telegram", "version": "0.1.0", "has_message_feature": true, - "status": "NOT_CONFIGURED", + "status": "RUNNING", "created_at": "2020-04-11T18:41:40.053Z", "updated_at": "2020-10-30T07:44:07.518Z" }, @@ -1168,7 +1168,7 @@ "selector": "zwave", "version": "0.1.0", "has_message_feature": false, - "status": "NOT_CONFIGURED", + "status": "RUNNING", "created_at": "2020-04-11T18:41:40.056Z", "updated_at": "2020-10-30T07:44:07.594Z" }, @@ -1223,7 +1223,7 @@ "selector": "openweather", "version": "0.1.0", "has_message_feature": false, - "status": "NOT_CONFIGURED", + "status": "RUNNING", "created_at": "2020-08-19T13:04:57.309Z", "updated_at": "2020-10-30T07:44:07.814Z" }, diff --git a/front/src/config/i18n/en.json b/front/src/config/i18n/en.json index 155f5a7b0e..0965c28e5c 100644 --- a/front/src/config/i18n/en.json +++ b/front/src/config/i18n/en.json @@ -990,10 +990,6 @@ "title": "Stopped", "detail": "Service stopped by user" }, - "NOT_CONFIGURED": { - "title": "Not configured", - "detail": "Service is not configured" - }, "ERROR": { "title": "Error", "detail": "Service failed to start." diff --git a/front/src/config/i18n/fr.json b/front/src/config/i18n/fr.json index 84def0da09..e9aeed00c8 100644 --- a/front/src/config/i18n/fr.json +++ b/front/src/config/i18n/fr.json @@ -990,10 +990,6 @@ "title": "Arrêté", "detail": "Service arrêté par un utilisateur" }, - "NOT_CONFIGURED": { - "title": "Non configuré", - "detail": "Service non configuré" - }, "ERROR": { "title": "En erreur", "detail": "Service en erreur lors du démarrage." diff --git a/front/src/routes/settings/settings-service/ServiceItem.jsx b/front/src/routes/settings/settings-service/ServiceItem.jsx index f625626f80..8a983b1045 100644 --- a/front/src/routes/settings/settings-service/ServiceItem.jsx +++ b/front/src/routes/settings/settings-service/ServiceItem.jsx @@ -1,13 +1,12 @@ import { Component } from 'preact'; import { Text, Localizer } from 'preact-i18n'; import { Link } from 'preact-router'; -import cx from 'classnames'; import get from 'get-value'; import { RequestStatus } from '../../../utils/consts'; import { SERVICE_STATUS } from '../../../../../server/utils/constants'; -const STARTED_STATUS = [SERVICE_STATUS.RUNNING, SERVICE_STATUS.NOT_CONFIGURED]; +const STARTED_STATUS = [SERVICE_STATUS.RUNNING]; const HIDDEN_ACTION_STATUS = [SERVICE_STATUS.UNKNOWN, SERVICE_STATUS.DISABLED]; class ServiceItem extends Component { @@ -73,11 +72,7 @@ class ServiceItem extends Component { checked={started} onClick={this.changeState} /> - + )} diff --git a/server/lib/service/service.start.js b/server/lib/service/service.start.js index 6f10032e70..d6a389ccb9 100644 --- a/server/lib/service/service.start.js +++ b/server/lib/service/service.start.js @@ -36,7 +36,7 @@ async function start(name, pod_id = null) { } catch (e) { if (e instanceof ServiceNotConfiguredError) { // If service fails to start due to configuration error, set service status to not configured - status = SERVICE_STATUS.NOT_CONFIGURED; + status = SERVICE_STATUS.RUNNING; logger.info(`Service ${name} is not configured, so it was not started.`); } else { // If service fails to start, set service status to error diff --git a/server/migrations/20201220100508-service-remove-not-configured-status.js b/server/migrations/20201220100508-service-remove-not-configured-status.js new file mode 100644 index 0000000000..e0e0a68532 --- /dev/null +++ b/server/migrations/20201220100508-service-remove-not-configured-status.js @@ -0,0 +1,6 @@ +module.exports = { + up: async (queryInterface) => { + await queryInterface.sequelize.query("UPDATE t_service SET status='RUNNING' WHERE status='NOT_CONFIGURED'"); + }, + down: async () => {}, +}; diff --git a/server/test/lib/service/service.start.test.js b/server/test/lib/service/service.start.test.js index 1ef6d820ab..4de40ba43d 100644 --- a/server/test/lib/service/service.start.test.js +++ b/server/test/lib/service/service.start.test.js @@ -75,7 +75,7 @@ describe('service.start', () => { assert.calledOnce(serviceImpl.start); }); - it('should fail starting a service (dut to config), and set status to NOT_CONFIGURED', async () => { + it('should fail starting a service (dut to config), and set status to RUNNING', async () => { serviceImpl.start = fake.rejects(new ServiceNotConfiguredError('error')); const result = await service.start(serviceName); @@ -86,8 +86,8 @@ describe('service.start', () => { }, }); - expect(serviceInDb.status).eq(SERVICE_STATUS.NOT_CONFIGURED); - expect(result.status).eq(SERVICE_STATUS.NOT_CONFIGURED); + expect(serviceInDb.status).eq(SERVICE_STATUS.RUNNING); + expect(result.status).eq(SERVICE_STATUS.RUNNING); assert.calledOnce(serviceImpl.start); }); diff --git a/server/utils/constants.js b/server/utils/constants.js index 6f6404fba2..ee9a7494e2 100644 --- a/server/utils/constants.js +++ b/server/utils/constants.js @@ -28,7 +28,6 @@ const SERVICE_STATUS = { RUNNING: 'RUNNING', STOPPED: 'STOPPED', ERROR: 'ERROR', - NOT_CONFIGURED: 'NOT_CONFIGURED', }; const SYSTEM_VARIABLE_NAMES = { From 9a11b36f03d58141dd8cd2f603391d01c7f9c460 Mon Sep 17 00:00:00 2001 From: atrovato <1839717+atrovato@users.noreply.github.com> Date: Sun, 20 Dec 2020 11:43:24 +0100 Subject: [PATCH 2/3] Stopped service only on user request --- server/api/controllers/service.controller.js | 2 +- server/lib/service/service.startAll.js | 3 --- server/lib/service/service.stop.js | 19 +++++++++---------- ...08-service-remove-not-configured-status.js | 4 +++- .../test/lib/service/service.startAll.test.js | 4 ++-- server/test/lib/service/service.stop.test.js | 19 ++++++++++--------- 6 files changed, 25 insertions(+), 26 deletions(-) diff --git a/server/api/controllers/service.controller.js b/server/api/controllers/service.controller.js index ee0a619555..4eebd2e31b 100644 --- a/server/api/controllers/service.controller.js +++ b/server/api/controllers/service.controller.js @@ -44,7 +44,7 @@ module.exports = function ServiceController(gladys) { * } */ async function stop(req, res) { - const service = await gladys.service.stop(req.params.service_name); + const service = await gladys.service.stop(req.params.service_name, null, true); if (!service) { res.status(404); } diff --git a/server/lib/service/service.startAll.js b/server/lib/service/service.startAll.js index 3e49428392..9493090be9 100644 --- a/server/lib/service/service.startAll.js +++ b/server/lib/service/service.startAll.js @@ -29,9 +29,6 @@ async function startAll() { case SERVICE_STATUS.STOPPED: logger.info(`Service ${serviceKey} was manually stopped, so it is ignored at startup`); return false; - case SERVICE_STATUS.LOADING: - logger.warn(`Service ${serviceKey} was not correctly loaded at last startup, it will be avoid for now.`); - return false; default: return true; } diff --git a/server/lib/service/service.stop.js b/server/lib/service/service.stop.js index ac3e6df05a..3e71535f89 100644 --- a/server/lib/service/service.stop.js +++ b/server/lib/service/service.stop.js @@ -7,11 +7,12 @@ const db = require('../../models'); * @description Stops one service by name * @param {string} name - The name of the service. * @param {string} pod_id - ID of Gladys instance. + * @param {boolean} userAction - Manually stopped? * @returns {Promise} Requested service. * @example * service.stop('telegram'); */ -async function stop(name, pod_id = null) { +async function stop(name, pod_id = null, userAction = false) { // Load service from DB const serviceInDb = await db.Service.findOne({ where: { @@ -26,16 +27,14 @@ async function stop(name, pod_id = null) { let status; try { await service.stop(); - // Once service started, set service status to stopped - status = SERVICE_STATUS.STOPPED; - } catch (e) { - // If service fails to start, set service status to error - status = SERVICE_STATUS.ERROR; - throw e; } finally { - // Store service status - serviceInDb.set({ status }); - await serviceInDb.save(); + // Once service manually stopped, set service status to stopped + if (userAction) { + status = SERVICE_STATUS.STOPPED; + // Store service status + serviceInDb.set({ status }); + await serviceInDb.save(); + } } } catch (e) { logger.warn(`Unable to stop service ${name}`, e); diff --git a/server/migrations/20201220100508-service-remove-not-configured-status.js b/server/migrations/20201220100508-service-remove-not-configured-status.js index e0e0a68532..980a1d9cd6 100644 --- a/server/migrations/20201220100508-service-remove-not-configured-status.js +++ b/server/migrations/20201220100508-service-remove-not-configured-status.js @@ -1,6 +1,8 @@ module.exports = { up: async (queryInterface) => { - await queryInterface.sequelize.query("UPDATE t_service SET status='RUNNING' WHERE status='NOT_CONFIGURED'"); + await queryInterface.sequelize.query( + "UPDATE t_service SET status='RUNNING' WHERE status='NOT_CONFIGURED' OR status='STOPPED'", + ); }, down: async () => {}, }; diff --git a/server/test/lib/service/service.startAll.test.js b/server/test/lib/service/service.startAll.test.js index 5fc7df20ec..42254854cf 100644 --- a/server/test/lib/service/service.startAll.test.js +++ b/server/test/lib/service/service.startAll.test.js @@ -109,7 +109,7 @@ describe('service.startAll', () => { name: serviceName, }, }); - expect(serviceInDb.status).eq(SERVICE_STATUS.LOADING); - assert.notCalled(serviceImpl.start); + expect(serviceInDb.status).eq(SERVICE_STATUS.RUNNING); + assert.calledOnce(serviceImpl.start); }); }); diff --git a/server/test/lib/service/service.stop.test.js b/server/test/lib/service/service.stop.test.js index 5c97227052..52320e1332 100644 --- a/server/test/lib/service/service.stop.test.js +++ b/server/test/lib/service/service.stop.test.js @@ -21,6 +21,7 @@ describe('service.stop', () => { selector: serviceName, name: serviceName, version: '0.1.0', + status: SERVICE_STATUS.UNKNOWN, }; stateManager = new StateManager(); @@ -42,7 +43,7 @@ describe('service.stop', () => { sinon.reset(); }); - it('should stop a service, and set status to RUNNING', async () => { + it('should stop a service, and not change status', async () => { serviceImpl.stop = fake.resolves(null); const result = await service.stop(serviceName); @@ -53,15 +54,15 @@ describe('service.stop', () => { }, }); - expect(serviceInDb.status).eq(SERVICE_STATUS.STOPPED); - expect(result.status).eq(SERVICE_STATUS.STOPPED); + expect(serviceInDb.status).eq(SERVICE_STATUS.UNKNOWN); + expect(result.status).eq(SERVICE_STATUS.UNKNOWN); assert.calledOnce(serviceImpl.stop); }); - it('should fail stoping a service, and set status to ERROR', async () => { + it('should fail stoping a service, change status to STOPPED', async () => { serviceImpl.stop = fake.rejects(null); - const result = await service.stop(serviceName); + const result = await service.stop(serviceName, null, true); const serviceInDb = await db.Service.findOne({ where: { @@ -69,8 +70,8 @@ describe('service.stop', () => { }, }); - expect(serviceInDb.status).eq(SERVICE_STATUS.ERROR); - expect(result.status).eq(SERVICE_STATUS.ERROR); + expect(serviceInDb.status).eq(SERVICE_STATUS.STOPPED); + expect(result.status).eq(SERVICE_STATUS.STOPPED); assert.calledOnce(serviceImpl.stop); }); @@ -92,8 +93,8 @@ describe('service.stop', () => { }, }); - expect(serviceInDb.status).eq(SERVICE_STATUS.STOPPED); - expect(result.status).eq(SERVICE_STATUS.STOPPED); + expect(serviceInDb.status).eq(SERVICE_STATUS.LOADING); + expect(result.status).eq(SERVICE_STATUS.LOADING); assert.calledOnce(serviceImpl.stop); }); }); From e865c2a635616b071e4875196391e265fa66a891 Mon Sep 17 00:00:00 2001 From: atrovato <1839717+atrovato@users.noreply.github.com> Date: Sun, 20 Dec 2020 13:52:31 +0100 Subject: [PATCH 3/3] Use Sequelize instead of SQL --- ...100508-service-remove-not-configured-status.js | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/server/migrations/20201220100508-service-remove-not-configured-status.js b/server/migrations/20201220100508-service-remove-not-configured-status.js index 980a1d9cd6..f195af78b5 100644 --- a/server/migrations/20201220100508-service-remove-not-configured-status.js +++ b/server/migrations/20201220100508-service-remove-not-configured-status.js @@ -1,7 +1,16 @@ +const db = require('../models'); + module.exports = { - up: async (queryInterface) => { - await queryInterface.sequelize.query( - "UPDATE t_service SET status='RUNNING' WHERE status='NOT_CONFIGURED' OR status='STOPPED'", + up: async () => { + await db.Service.update( + { + status: 'RUNNING', + }, + { + where: { + status: ['NOT_CONFIGURED', 'STOPPED'], + }, + }, ); }, down: async () => {},