Skip to content

Commit

Permalink
Merge pull request #428 from telefonicaid/hardening/fiqare-perseo-fe-…
Browse files Browse the repository at this point in the history
…improvements-prelanding

FIX software quality improvement based on ISO25010 recommendations
  • Loading branch information
AlvaroVega authored Feb 6, 2020
2 parents 1882afb + 900ffcf commit 2a09897
Show file tree
Hide file tree
Showing 21 changed files with 420 additions and 25 deletions.
2 changes: 2 additions & 0 deletions CHANGES_NEXT_RELEASE
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
- Add: /api-docs endpoint providing swagger-based documentation of the HTTP endpoints exposed by Perseo FE
- Fix: improving logs system, adding more traces and changes to avoid too verbose messages at INFO level
- Hardening: software quality improvement based on ISO25010 recommendations
12 changes: 12 additions & 0 deletions documentation/development.md
Original file line number Diff line number Diff line change
Expand Up @@ -108,3 +108,15 @@ To ensure consistent Markdown formatting run the following:
# Use git-bash on Windows
npm run prettier:text
```

### Swagger

In order to run Swagger, you need to execute the Perseo FE (as explained [here](deployment.md)) and then you can access
to:

```
<server_host>:9090/api-docs
```

The swagger documentation provided at /api-docs covers all the HTTP endpoint exposed by Perseo FE.

14 changes: 8 additions & 6 deletions documentation/plain_rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@

There are two kind of rules:

* Esper-based rules, which include the final EPL statement used by the Esper engine inside perseo-core. In order to work with perseo (front-end) properly, the EPL statement must fulfill several conventions for the rule to be able to operate on the incoming events and trigger adequate actions. Example:
- Esper-based rules, which include the final EPL statement used by the Esper engine inside perseo-core. In order to
work with perseo (front-end) properly, the EPL statement must fulfill several conventions for the rule to be able to
operate on the incoming events and trigger adequate actions. Example:

```json
{
Expand All @@ -40,9 +42,8 @@ There are two kind of rules:
}
```

* No signal rules. They are triggered when a given attribute is not updated in a given interval of time. They
don't use Esper at persero-core (they are checked and triggered by perseo frontend). Example:

- No signal rules. They are triggered when a given attribute is not updated in a given interval of time. They don't
use Esper at persero-core (they are checked and triggered by perseo frontend). Example:

```json
{
Expand Down Expand Up @@ -127,15 +128,16 @@ information on how to scape characters at

## No signal conditions

The no signal condition is specified in the `nosignal` configuration element, which is an object with the following fields:
The no signal condition is specified in the `nosignal` configuration element, which is an object with the following
fields:

- **checkInterval**: _mandatory_, time in minutes for checking the attribute
- **attribute**: _mandatory_, attribute for watch
- **reportInterval**: _mandatory_, time in seconds to see an entity as silent
- **id** or **idRegexp**: _mandatory_ (but not both at the same time), id or regex of the entity to watch
- type: _optional_, type of entities to watch

Is recommended to set checkInterval at least double of reportInterval. Howeer, note that a very demanding value of
Is recommended to set checkInterval at least double of reportInterval. Howeer, note that a very demanding value of
checkInterval could impact on performance.

<a name="actions"></a>
Expand Down
2 changes: 2 additions & 0 deletions lib/alarm.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ var util = require('util'),
function raise(alarm, context, message) {
var state = alarms[alarm];
context = (process.domain && process.domain.context) || {};

if (typeof message === 'object' && message) {
message = util.format('%j', message);
}
Expand All @@ -43,6 +44,7 @@ function raise(alarm, context, message) {
function release(alarm, context, message) {
var state = alarms[alarm];
context = (process.domain && process.domain.context) || {};

if (typeof message === 'object' && message) {
message = util.format('%j', message);
}
Expand Down
4 changes: 4 additions & 0 deletions lib/models/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,9 @@ function validateAction(axn) {
return new errors.IdAsAttribute(JSON.stringify(axnElem.parameters));
} else if (axnElem.parameters.name === 'type') {
return new errors.IdAsAttribute(JSON.stringify(axnElem.parameters));
} else {
//Added default else clause
return null;
}
}
}
Expand Down Expand Up @@ -228,6 +231,7 @@ function DoAction(event, callback) {

actionStore.Find(service, subservice, ruleName, function(err, actions) {
var localError, queue;

if (err) {
return callback(err, null);
}
Expand Down
4 changes: 3 additions & 1 deletion lib/models/actionsStore.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,10 @@ module.exports = {
localError = new errors.NotFoundAction(data);
myutils.logErrorIf(localError);
return callback(localError, null);
} else {
//Added default else clause
return callback(null, data.action);
}
return callback(null, data.action);
});
}
};
Expand Down
3 changes: 1 addition & 2 deletions lib/models/emailAction.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,12 @@
var util = require('util'),
nodemailer = require('nodemailer'),
logger = require('logops'),
smtpTransport = require('nodemailer-smtp-transport'),
config = require('../../config'),
myutils = require('../myutils'),
alarm = require('../alarm'),
metrics = require('./metrics');

var transporter = nodemailer.createTransport(smtpTransport(config.smtp));
var transporter = nodemailer.createTransport(config.smtp);

function buildMailOptions(action, event) {
return {
Expand Down
3 changes: 3 additions & 0 deletions lib/models/entitiesStore.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,9 @@ function findSilentEntities(service, subservice, ruleData, func, callback) {
} catch (e) {
return callback(e, null);
}
} else {
//Added default else clause
logger.debug('findSilentEntities() - Default else clause');
}
if (ruleData.type) {
criterion['_id.type'] = ruleData.type;
Expand Down
4 changes: 4 additions & 0 deletions lib/models/paths.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ function validComponent(component) {
} else if (!componentPattern.test(component)) {
err = new errors.InvalidCharacter(component);
}
else {
//Added default else clause
return err;
}
return err;
}
function validService(service) {
Expand Down
8 changes: 8 additions & 0 deletions lib/models/postAction.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@ function buildPostOptions(action, event) {
} else if (action.template) {
options.text = myutils.expandVar(action.template, event, true);
}
else {
//Added default else clause
return options;
}

return options;
}
Expand All @@ -58,6 +62,10 @@ function doIt(action, event, callback) {
} else if (options.text) {
requestOptions.body = options.text;
}
else {
//Added default else clause
logger.debug('doIt() - Default else clause');
}

metrics.IncMetrics(event.service, event.subservice, metrics.actionHttpPost);

Expand Down
25 changes: 22 additions & 3 deletions lib/models/rules.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ var util = require('util'),
myutils = require('../myutils'),
noSignal = require('./noSignal'),
actions = require('./actions'),
logger = require('logops'),
namePattern = /^[a-zA-Z0-9_-]+$/,
MaxNameLength = 50,
errors = {};
Expand Down Expand Up @@ -58,6 +59,9 @@ function validRule(rule) {
return new errors.InvalidRuleName(rule.name);
} else if (!rule.text && !rule.nosignal) {
return new errors.EmptyRule(JSON.stringify(rule));
} else {
//Added default else clause
logger.debug('validRule() - Default else clause');
}
if (rule.nosignal) {
//Specific checks for no-signal rules
Expand Down Expand Up @@ -218,6 +222,7 @@ module.exports = {
},
Save: function(rule, callback) {
var localError;

localError = validRule(rule);
if (localError !== null) {
myutils.logErrorIf(localError);
Expand All @@ -244,7 +249,12 @@ module.exports = {
rulesStore.Save.bind(null, rule),
function(cb) {
if (rule.nosignal) {
noSignal.AddNSRule(rule.service, rule.subservice, rule.name, rule.nosignal);
noSignal.AddNSRule(
rule.service,
rule.subservice,
rule.name,
rule.nosignal
);
}
cb(null);
}
Expand Down Expand Up @@ -294,14 +304,23 @@ module.exports = {
delR2core.bind(null, rule),
function(cb) {
if (rule.nosignal) {
noSignal.DeleteNSRule(rule.service, rule.subservice, rule.name);
noSignal.DeleteNSRule(
rule.service,
rule.subservice,
rule.name
);
}
cb(null);
},
postR2core.bind(null, rule),
function(cb) {
if (rule.nosignal) {
noSignal.AddNSRule(rule.service, rule.subservice, rule.name, rule.nosignal);
noSignal.AddNSRule(
rule.service,
rule.subservice,
rule.name,
rule.nosignal
);
}
cb(null);
},
Expand Down
2 changes: 2 additions & 0 deletions lib/models/updateAction.js
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,8 @@ function processOptionParams(action, event) {
case 'None':
theValue = null;
break;
default:
//Nothing to do
}
}
var key = myutils.expandVar(attr.name, event);
Expand Down
19 changes: 17 additions & 2 deletions lib/models/visualRules.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ var util = require('util'),
MINOR_OR_EQUAL_THAN: ' <= ',
MATCH: ' regexp '
},
errors = {};
errors = {},
logger = require('logops');

function errorOperator(op) {
if (operatorMap[op] === undefined) {
Expand Down Expand Up @@ -118,7 +119,6 @@ function vr2rule(cardRule) {
false
)
);

break;
case 'XPATH':
errOp = errorOperator(card.conditionList[0].operator);
Expand All @@ -143,18 +143,27 @@ function vr2rule(cardRule) {
}
} else if (card.conditionList[0].parameterName === 'type') {
type = card.conditionList[0].parameterValue;
} else {
//Added default else clause
logger.debug('vr2rule() - Default else clause');
}
break;
case 'OBSERVATION':
errOp = errorOperator(card.conditionList[0].operator);

if (errOp) {
return errOp;
}

if (card.sensorData.measureName === 'id') {
return new errors.IdAsAttribute('');
} else if (card.sensorData.measureName === 'type') {
return new errors.TypeAsAttribute('');
} else {
//Added default else clause
logger.debug('vr2rule() - Default else clause');
}

conditions.push(
makeObservationCondition(
card.sensorData.measureName,
Expand All @@ -163,6 +172,7 @@ function vr2rule(cardRule) {
card.sensorData.dataType === 'Quantity'
)
);

break;
case 'LAST_MEASURE':
checkInterval = parseInt(card.timeData.interval, 10);
Expand Down Expand Up @@ -225,10 +235,15 @@ function vr2rule(cardRule) {
case 'timeElapsed': // Minimal interval for actions
action.interval = card.timeData.interval;
break;
default:
//nothing to do
}
} else {
return new errors.MissingConfigDataInTimeCard(JSON.stringify(card));
}
break;
default:
//nothing to do
}
}

Expand Down
3 changes: 3 additions & 0 deletions lib/myutils.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,9 @@ function expandObject(templateObj, dictionary) {
res[expandVar(key, dictionary)] = expandVar(templateObj[key], dictionary);
} else if (typeof templateObj[key] === 'object') {
res[expandVar(key, dictionary)] = expandObject(templateObj[key], dictionary);
} else {
//Added default else clause
logger.debug('expandObject() - Default else clause');
}
});
}
Expand Down
21 changes: 20 additions & 1 deletion lib/perseo.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,10 @@ var domain = require('domain'),
serviceMiddleware = require('./middleware/service'),
errorMiddleware = require('./middleware/error'),
myutils = require('./myutils'),
d = domain.create();
d = domain.create(),
swaggerJsdoc = require('swagger-jsdoc'),
swaggerUi = require('swagger-ui-express'),
pjson = require('../package.json');

function start(callbackStart) {
var context = { op: 'start', comp: constants.COMPONENT_NAME };
Expand Down Expand Up @@ -98,6 +101,22 @@ function start(callbackStart) {
versionRoutes.AddTo(app);
metricsRoutes.AddTo(app);

const options = {
definition: {
swagger: '2.0', // Specification (optional, defaults to swagger: '2.0')
info: {
title: pjson.name, // Title (required)
version: pjson.version // Version (required)
}
},
// Path to the API docs
apis: ['./lib/routes/*.js']
};

const specs = swaggerJsdoc(options);

app.use('/api-docs', swaggerUi.serve, swaggerUi.setup(specs));

async.series(
[
function(callback) {
Expand Down
Loading

0 comments on commit 2a09897

Please sign in to comment.