Feature/optional rule name #316

cblanco · 2019-01-28T17:40:42Z

Edit: fixes #307

…ule is added

fgalan · 2019-01-29T13:13:58Z

CHANGES_NEXT_RELEASE

@@ -10,3 +10,4 @@
  * sinon-chai
  * grunt and grunt related module
  * closure-linter-wrapper
+- Add Rule auto-naming on rule creation time


I understand issue #307 should be mentioned.

Yes, #307 should be mentioned. Is enough with these comments?

By the way... it could be that the description that I established in the CNR for this functionality is not the most correct. It would seem appropriate to you if I change it to something like - Add 'ruleName' as variable automatically in rule text field on rule creation time?

Yes, I think is a good alternative. Maybe an explicit mention to EPL is clarifying. I mean in sum something like this:

- Add 'ruleName' as variable automatically in rule text field (EPL) on rule creation time (#208)

Fixed in 0bfc9a0

fgalan · 2019-01-29T13:15:14Z

lib/models/rules.js

+
+    var newAs = '"' + rule.name + '" as ruleName';
+    // Add "name as ruleName" if not exist
+    if (rule.text && rule.text.indexOf(' as ruleName') === -1) {


Documentation should be changed to explain that now the name is not mandatory and which happens if the name is ommited (i.e. an example of autogenerated name would be great).

I have changed the documentation but I do not know if I have approached it in the right way, please check if the change seems appropriate. b356343

…clauses in documentation

fgalan · 2019-01-31T12:39:00Z

CHANGES_NEXT_RELEASE

@@ -10,3 +10,4 @@
  * sinon-chai
  * grunt and grunt related module
  * closure-linter-wrapper
+- Add 'ruleName' as variable automatically in rule text field (EPL) on rule creation time (#208)


Argg... issue number is #307, not #208.

My fault sorry :(

Fixed in ef4dda6

fgalan · 2019-01-31T12:45:48Z

documentation/plain_rules.md

 * The *from* pattern must name the event as **ev** and the event stream from which take events must be **iotEvent**
 * A *type=* condition must be concatenated for avoiding mixing different kinds of entities
+* The name of the rule is not mandatory, but for backward compatibility, it can be present as ** ruleName ** alias (`e.g: select *, "blood_rule_update" as ruleName...`) in the select clause. If present, it must be equal to the ‘name’ field of the rule object. If it is not present, you can still use the variable 'ruleName' in the actions, because it will be included automatically.


I'd suggest split this and include part as in a a remarked "backward compatibility note". In particular I mean:

To leave this in the bullet list:

* The variable 'ruleName' in automatically added to the action, even if it is not present in the EPL text.

**Backward compatibility note:** since version 1.8.0 it is not mandatory to specify the name of the rule as part of the EPL text. In fact, it is not recommendable to do that. However, for backward compatibility, it can be present as *ruleName* alias (`e.g: select *, "blood_rule_update" as ruleName...`) in the select clause. If present, it must be equal to the ‘name’ field of the rule object.

Fixed in 55110fb

fgalan · 2019-01-31T12:49:21Z

documentation/architecture.md

@@ -191,7 +191,7 @@ A simplified format in JSON can be used to represent rules. The former visual ru
 ```json
 {
 	"name" : "prueba-test",
-	"text" : "select *,\"prueba-test\" as ruleName  from pattern [every ev=iotEvent((cast(id?, String)  regexp  \"asd\"))]",
+	"text" : "select * from pattern [every ev=iotEvent((cast(id?, String)  regexp  \"asd\"))]",


Doubt: at rule creation time (POST /rules, if I'm remembering correctly) it is clear that ruleName is not include in the EPL and it is automatically added.

However, what about of GET /rules/:id? Is would be the automatically added ruleName added there?

Yes. The ruleName is added during the creation automatically in the 'text' field. If the rule is requested using GET / rules/:id or GET /rules, it will include the ruleName in the 'text' field.

Maybe it's a good idea to mention it in the documentation?

Fixed in 0917cbd

fgalan · 2019-01-31T12:51:09Z

lib/models/rules.js

+    var newAs = '"' + rule.name + '" as ruleName';
+    // Add "name as ruleName" if not exist
+    if (rule.text && rule.text.indexOf(' as ruleName') === -1) {
+        var idx = rule.text.toLowerCase().indexOf('select') + 7;


7 seems to be a magic number, a bit obscure :)

Where it comes from? It seems to be len(select) +1, so maybe it would be more clear (i.e. less magical ;) something like this:

var offset = length('select') + 1; var idx = rule.text.toLowerCase().indexOf('select') + offset;

Fixed in b25bd0f

… when the rule is recovered using GET

fgalan

LGTM. Thanks!

cblanco added 4 commits January 28, 2019 16:31

Add <theRuleName> as ruleName automatically if not present when new r…

0e35221

…ule is added

Comple the auto-naming feature

c5eb1ff

Add modified note in headers

2978155

Add auto-naming feature in CNR

607209a

cblanco mentioned this pull request Jan 28, 2019

Add Roadmap #315

Merged

fgalan reviewed Jan 29, 2019

View reviewed changes

cblanco added 3 commits January 30, 2019 17:28

Fix lint warnings

a9a9cbf

Change ruleName as optional and remove all 'as ruleName' from select …

b356343

…clauses in documentation

Improve ruleName feature description in CNR

0bfc9a0

fgalan reviewed Jan 31, 2019

View reviewed changes

fgalan mentioned this pull request Jan 31, 2019

Automatically set the name of the rule #307

Closed

fgalan reviewed Jan 31, 2019

View reviewed changes

cblanco added 5 commits January 31, 2019 14:16

Merge branch 'master' into feature/optionalRuleName

de35ffd

Fix issue number in CNR

ef4dda6

Improve ruleName documentation

55110fb

Remove magic number

b25bd0f

Add note explainig that ruleName is retrieved as part of the EPL text…

0917cbd

… when the rule is recovered using GET

fgalan approved these changes Feb 1, 2019

View reviewed changes

fgalan merged commit 2e8b2ba into telefonicaid:master Feb 1, 2019

fgalan mentioned this pull request Feb 1, 2019

Add prettier code formatting #314

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/optional rule name #316

Feature/optional rule name #316

cblanco commented Jan 28, 2019 •

edited by fgalan

Loading

fgalan Jan 29, 2019

cblanco Jan 30, 2019

fgalan Jan 31, 2019

cblanco Jan 31, 2019

fgalan Jan 29, 2019

cblanco Jan 30, 2019 •

edited

Loading

fgalan Jan 31, 2019

cblanco Feb 1, 2019

fgalan Jan 31, 2019

cblanco Feb 1, 2019

fgalan Jan 31, 2019

cblanco Feb 1, 2019 •

edited

Loading

fgalan Feb 1, 2019

cblanco Feb 1, 2019

fgalan Jan 31, 2019

cblanco Feb 1, 2019

fgalan left a comment

Feature/optional rule name #316

Feature/optional rule name #316

Conversation

cblanco commented Jan 28, 2019 • edited by fgalan Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cblanco Jan 30, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cblanco Feb 1, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fgalan left a comment

Choose a reason for hiding this comment

cblanco commented Jan 28, 2019 •

edited by fgalan

Loading

cblanco Jan 30, 2019 •

edited

Loading

cblanco Feb 1, 2019 •

edited

Loading