Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DROOLS-7639] ansible-rulebook : support event path for collecting el… #124

Merged
merged 5 commits into from
Oct 29, 2024

Conversation

tkobayas
Copy link
Collaborator

…ements in array in array

  • WIP: test case only

…ements in array in array

- WIP: test case only
{
"SelectAttrExpression":{
"lhs":{
"Event":"incident.alerts.tags"
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At the moment, this rule doesn't match because "Event":"incident.alerts.tags" doesn't treat alerts as an array. If we change it to "Event":"incident.alerts[1].tags", the rule would match.

private Object collectPathsForAllElementsInArray(IdentifierNode n) {
// if accessing a list without an index, evaluate all elements and collect matching children
List<?> theList = (List<?>) cur;
List<Object> nextNodeList = new PathWrapperList();
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When a visitor faces array access without brackets (e.g. alerts), I introduce PathWrapperList to broaden the search paths. I haven't yet tested a case where PathWrapperList is nested (probably I need to strip it).

import org.drools.ansible.rulebook.integration.protoextractor.ast.SquaredAccessorNode;
import org.kie.api.prototype.Prototype;

public class ValueCollectVisitor extends DefaultedVisitor<Object> {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I introduced ValueCollectVisitor and simply replaced with ValueExtractionVisitor. But probably we will need to switch them depending on operator used in a rule condition.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand correctly the ValueCollectVisitor is a features superset of the ValueExtractionVisitor and as such probably we don't need the second anymore. Also I believe it will be hard if not impossible to discover when the second is enough or it is necessary to use the first. If all I wrote is correct maybe we can totally drop the ValueExtractionVisitor, but let's keep it for now.

Copy link
Collaborator Author

@tkobayas tkobayas Oct 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Yes, I thought that ValueCollectVisitor could diverge from ValueExtractionVisitor, but it seems to be a superset now. We would be able to drop ValueExtractionVisitor in the near future, but let's keep it for now because Madhu's request may not be finalized yet.

@tkobayas tkobayas marked this pull request as ready for review October 28, 2024 10:31
@tkobayas tkobayas merged commit 4aaf597 into kiegroup:main Oct 29, 2024
1 check passed
tkobayas added a commit to tkobayas/drools-ansible-rulebook-integration that referenced this pull request Nov 11, 2024
kiegroup#124)

* [DROOLS-7639] ansible-rulebook : support event path for collecting elements in array in array
- WIP: test case only

* WIP implementation

* - improved nested list management

* Added more tests

* convert Set too
mariofusco pushed a commit that referenced this pull request Nov 11, 2024
#124) (#128)

* [DROOLS-7639] ansible-rulebook : support event path for collecting elements in array in array
- WIP: test case only

* WIP implementation

* - improved nested list management

* Added more tests

* convert Set too
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants