* [pmg-devel] [PATCH pmg-api/gui/docs, proxmox-widget-toolkit] Extend rule system
@ 2023-04-07 13:42 Leo Nunner
2023-04-07 13:42 ` [pmg-devel] [PATCH pmg-api 1/8] feature: negation: add field to database Leo Nunner
` (12 more replies)
0 siblings, 13 replies; 17+ messages in thread
From: Leo Nunner @ 2023-04-07 13:42 UTC (permalink / raw)
To: pmg-devel
This series introduces two new features for the PMG rule system: object
negation, and match groups. Negation allows the user to negate objects
inside a rule, meaning that they will evaluate to true if their
condition is NOT fulfilled.
The second feature, match groups, allows objects to be chained together.
A match group functions as a container for multiple other objects; it
will only evaluate to true if all its members evaluate to true. Match groups
are connected via logical 'or' to all other objects inside the rule;
take the following rule system:
- Rule
- Match Group
- Object 1
- Object 2
- Object 3
It will match if either (Object 1 && Object 2), or if Object 3
evaluate to true.
To properly achieve match groups inside the GUI, the rule overview was
reworked as a tree, instead of a simple list. It can now be
collapsed/expanded, and each object type (except actions) has a single
subfolder called 'Match Group'.
In combination, these features should cover quite a few use cases, and
make it possible to model more complex rule systems.
pmg-api:
Leo Nunner (8):
feature: negation: add field to database
feature: negation: parse negation value into objects
feature: negation: expand/implement API endpoints
feature: negation: implement matching logic
feature: match groups: add field to database
feature: match groups: parse field into objects
feature: match groups: update API endpoints
feature: match groups: implement matching logic
src/PMG/API2/ObjectGroupHelpers.pm | 6 +-
src/PMG/API2/Rules.pm | 119 +++++++++++++++++++++++++
src/PMG/DBTools.pm | 28 ++++++
src/PMG/RuleCache.pm | 61 +++++++++++--
src/PMG/RuleDB.pm | 65 +++++++++++++-
src/PMG/RuleDB/ArchiveFilter.pm | 6 +-
src/PMG/RuleDB/ContentTypeFilter.pm | 6 +-
src/PMG/RuleDB/MatchArchiveFilename.pm | 4 +-
src/PMG/RuleDB/MatchField.pm | 2 +-
src/PMG/RuleDB/MatchFilename.pm | 2 +-
10 files changed, 280 insertions(+), 19 deletions(-)
pmg-gui:
Leo Nunner (2):
feature: negate objects inside rules
feature: introduce logical 'and' for rules
css/ext6-pmg.css | 10 ++
js/RuleInfo.js | 321 +++++++++++++++++++++++++++++++++++++----------
js/Utils.js | 14 +--
3 files changed, 271 insertions(+), 74 deletions(-)
pmg-docs:
Leo Nunner (1):
docs: document negation and match groups
pmg-mail-filter.adoc | 29 ++++++++++++++++++++++++++++-
1 file changed, 28 insertions(+), 1 deletion(-)
proxmox-widget-toolkit:
Leo Nunner (1):
dark-mode: fix colour of default tree icons
src/proxmox-dark/scss/other/_icons.scss | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--
2.30.2
^ permalink raw reply [flat|nested] 17+ messages in thread
* [pmg-devel] [PATCH pmg-api 1/8] feature: negation: add field to database
2023-04-07 13:42 [pmg-devel] [PATCH pmg-api/gui/docs, proxmox-widget-toolkit] Extend rule system Leo Nunner
@ 2023-04-07 13:42 ` Leo Nunner
2023-04-07 13:42 ` [pmg-devel] [PATCH pmg-api 2/8] feature: negation: parse negation value into objects Leo Nunner
` (11 subsequent siblings)
12 siblings, 0 replies; 17+ messages in thread
From: Leo Nunner @ 2023-04-07 13:42 UTC (permalink / raw)
To: pmg-devel
Add a 'Negation field to the RuleGroup table.
Signed-off-by: Leo Nunner <l.nunner@proxmox.com>
---
src/PMG/DBTools.pm | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/src/PMG/DBTools.pm b/src/PMG/DBTools.pm
index 0b37361..07d39b1 100644
--- a/src/PMG/DBTools.pm
+++ b/src/PMG/DBTools.pm
@@ -407,6 +407,7 @@ sub create_ruledb {
Objectgroup_ID INTEGER NOT NULL,
Rule_ID INTEGER NOT NULL,
Grouptype INTEGER NOT NULL,
+ Negate INTEGER NOT NULL DEFAULT 0,
PRIMARY KEY (Objectgroup_ID, Rule_ID, Grouptype)
);
@@ -569,6 +570,19 @@ sub upgradedb {
}
}
+ # Allow negating rule objects
+ if (!database_column_exists($dbh, 'RuleGroup', 'Negate')) {
+ eval {
+ $dbh->begin_work;
+ $dbh->do("ALTER TABLE RuleGroup ADD COLUMN Negate INTEGER NOT NULL DEFAULT 0");
+ $dbh->commit;
+ };
+ if (my $err = $@) {
+ $dbh->rollback;
+ die $err;
+ }
+ }
+
foreach my $table (keys %$tables) {
eval { $dbh->do("ANALYZE $table"); };
warn $@ if $@;
--
2.30.2
^ permalink raw reply [flat|nested] 17+ messages in thread
* [pmg-devel] [PATCH pmg-api 2/8] feature: negation: parse negation value into objects
2023-04-07 13:42 [pmg-devel] [PATCH pmg-api/gui/docs, proxmox-widget-toolkit] Extend rule system Leo Nunner
2023-04-07 13:42 ` [pmg-devel] [PATCH pmg-api 1/8] feature: negation: add field to database Leo Nunner
@ 2023-04-07 13:42 ` Leo Nunner
2023-04-07 13:42 ` [pmg-devel] [PATCH pmg-api 3/8] feature: negation: expand/implement API endpoints Leo Nunner
` (10 subsequent siblings)
12 siblings, 0 replies; 17+ messages in thread
From: Leo Nunner @ 2023-04-07 13:42 UTC (permalink / raw)
To: pmg-devel
Expand the parsing code so that the 'negate' field is written into
objects for further handling. Two new functions are introduced: one for
reading a specific object-rule mapping (and its values), the other to
set the 'negate' value for such an object.
Signed-off-by: Leo Nunner <l.nunner@proxmox.com>
---
src/PMG/API2/ObjectGroupHelpers.pm | 5 +++-
src/PMG/RuleCache.pm | 7 ++++-
src/PMG/RuleDB.pm | 46 +++++++++++++++++++++++++++++-
3 files changed, 55 insertions(+), 3 deletions(-)
diff --git a/src/PMG/API2/ObjectGroupHelpers.pm b/src/PMG/API2/ObjectGroupHelpers.pm
index 48078fb..c3e6448 100644
--- a/src/PMG/API2/ObjectGroupHelpers.pm
+++ b/src/PMG/API2/ObjectGroupHelpers.pm
@@ -47,7 +47,10 @@ sub format_object_group {
my $res = [];
foreach my $og (@$ogroups) {
push @$res, {
- id => $og->{id}, name => $og->{name}, info => $og->{info}
+ id => $og->{id},
+ name => $og->{name},
+ info => $og->{info},
+ negate => $og->{negate},
};
}
return $res;
diff --git a/src/PMG/RuleCache.pm b/src/PMG/RuleCache.pm
index b8690ea..c5a57f6 100644
--- a/src/PMG/RuleCache.pm
+++ b/src/PMG/RuleCache.pm
@@ -56,7 +56,7 @@ sub new {
my ($from, $to, $when, $what, $action);
my $sth1 = $dbh->prepare(
- "SELECT Objectgroup_ID, Grouptype FROM RuleGroup " .
+ "SELECT Objectgroup_ID, Grouptype, Negate FROM RuleGroup " .
"where RuleGroup.Rule_ID = '$ruleid' " .
"ORDER BY Grouptype, Objectgroup_ID");
@@ -64,6 +64,7 @@ sub new {
while (my $ref1 = $sth1->fetchrow_hashref()) {
my $gtype = $ref1->{grouptype};
my $groupid = $ref1->{objectgroup_id};
+ my $negate = $ref1->{negate};
# emtyp groups differ from non-existent groups!
@@ -90,6 +91,10 @@ sub new {
$sha1->add (join (',', $objid, $gtype, $groupid) . "|");
$sha1->add ($obj->{digest}, "|");
+ if ($gtype != 4) { # it doesn't make any sense to negate actions
+ $obj->{negate} = $negate;
+ }
+
if ($gtype == 0) { #from
push @$from, $obj;
} elsif ($gtype == 1) { # to
diff --git a/src/PMG/RuleDB.pm b/src/PMG/RuleDB.pm
index a6b0b79..98beda3 100644
--- a/src/PMG/RuleDB.pm
+++ b/src/PMG/RuleDB.pm
@@ -107,7 +107,7 @@ sub load_groups {
my $sth = $self->{dbh}->prepare(
"SELECT RuleGroup.Grouptype, Objectgroup.ID, " .
- "Objectgroup.Name, Objectgroup.Info " .
+ "Objectgroup.Name, Objectgroup.Info, Negate " .
"FROM Rulegroup, Objectgroup " .
"WHERE Rulegroup.Rule_ID = ? and " .
"Rulegroup.Objectgroup_ID = Objectgroup.ID " .
@@ -123,6 +123,10 @@ sub load_groups {
my $og = PMG::RuleDB::Group->new($ref->{name}, $ref->{info});
$og->{id} = $ref->{id};
+ if ($ref->{'grouptype'} != 4) { # this doesn't make any sense for actions
+ $og->{negate} = $ref->{negate};
+ }
+
if ($ref->{'grouptype'} == 0) { #from
push @$from, $og;
} elsif ($ref->{'grouptype'} == 1) { # to
@@ -753,6 +757,46 @@ sub rule_remove_group {
return 1;
}
+
+sub rule_get_group_settings {
+ my ($self, $ruleid, $groupid, $gtype_str) = @_;
+
+ my $gtype = $grouptype_hash->{$gtype_str} //
+ die "unknown group type '$gtype_str'\n";
+
+ defined($ruleid) || die "undefined rule id: ERROR";
+ defined($groupid) || die "undefined group id: ERROR";
+ defined($gtype) || die "undefined group type: ERROR";
+
+ my $sth = $self->{dbh}->prepare("SELECT * FROM RuleGroup WHERE " .
+ "Objectgroup_ID = ? and Rule_ID = ? and Grouptype = ?");
+
+ $sth->execute($groupid, $ruleid, $gtype);
+
+ my $ref = $sth->fetchrow_hashref();
+ die "rule does not exist\n" if !defined($ref);
+
+ return $ref;
+}
+
+sub rule_set_group_setting_negate {
+ my ($self, $value, $ruleid, $groupid, $gtype_str) = @_;
+
+ my $gtype = $grouptype_hash->{$gtype_str} //
+ die "unknown group type '$gtype_str'\n";
+
+ defined($ruleid) || die "undefined rule id: ERROR";
+ defined($groupid) || die "undefined group id: ERROR";
+ defined($gtype) || die "undefined group type: ERROR";
+
+ my $sth = $self->{dbh}->prepare("UPDATE RuleGroup SET Negate = ? " .
+ "WHERE Objectgroup_ID = ? and Rule_ID = ? and Grouptype = ?");
+
+ $sth->execute($value, $groupid, $ruleid, $gtype);
+
+ return 1;
+}
+
sub load_rule {
my ($self, $id) = @_;
--
2.30.2
^ permalink raw reply [flat|nested] 17+ messages in thread
* [pmg-devel] [PATCH pmg-api 3/8] feature: negation: expand/implement API endpoints
2023-04-07 13:42 [pmg-devel] [PATCH pmg-api/gui/docs, proxmox-widget-toolkit] Extend rule system Leo Nunner
2023-04-07 13:42 ` [pmg-devel] [PATCH pmg-api 1/8] feature: negation: add field to database Leo Nunner
2023-04-07 13:42 ` [pmg-devel] [PATCH pmg-api 2/8] feature: negation: parse negation value into objects Leo Nunner
@ 2023-04-07 13:42 ` Leo Nunner
2023-04-07 13:42 ` [pmg-devel] [PATCH pmg-api 4/8] feature: negation: implement matching logic Leo Nunner
` (9 subsequent siblings)
12 siblings, 0 replies; 17+ messages in thread
From: Leo Nunner @ 2023-04-07 13:42 UTC (permalink / raw)
To: pmg-devel
The existing 'add' enpoint was expanded with a 'negate' parameter, so
that new objects can be added in an already negated state. New API
endpoints were added to get and update the specific group-rule relation
settings (which is only 'negate' for now). These endpoints get added to
all ogroups except 'actions', since negating actions doesn't make much
sense.
Signed-off-by: Leo Nunner <l.nunner@proxmox.com>
---
src/PMG/API2/Rules.pm | 96 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 96 insertions(+)
diff --git a/src/PMG/API2/Rules.pm b/src/PMG/API2/Rules.pm
index 4f8c10b..ad3f6c7 100644
--- a/src/PMG/API2/Rules.pm
+++ b/src/PMG/API2/Rules.pm
@@ -261,6 +261,11 @@ my $register_rule_group_api = sub {
description => "Groups ID.",
type => 'integer',
},
+ negate => {
+ description => "Negate group.",
+ type => 'boolean',
+ optional => 1,
+ },
},
},
returns => { type => 'null' },
@@ -273,6 +278,10 @@ my $register_rule_group_api = sub {
$rdb->rule_add_group($param->{id}, $param->{ogroup}, $name);
+ if (defined($param->{negate})) {
+ $rdb->rule_set_group_setting_negate($param->{negate}, $param->{id}, $param->{ogroup}, $name);
+ }
+
PMG::DBTools::reload_ruledb();
return undef;
@@ -314,6 +323,93 @@ my $register_rule_group_api = sub {
return undef;
}});
+ if($name ne 'action') {
+ __PACKAGE__->register_method ({
+ name => "get_${name}_group_settings",
+ path => "$name/{ogroup}",
+ method => 'GET',
+ description => "Get '$name' group settings for rule.",
+ proxyto => 'master',
+ protected => 1,
+ permissions => { check => [ 'admin' ] },
+ parameters => {
+ additionalProperties => 0,
+ properties => {
+ id => {
+ description => "Rule ID.",
+ type => 'integer',
+ },
+ ogroup => {
+ description => "Groups ID.",
+ type => 'integer',
+ },
+ },
+ },
+ returns => {
+ type => 'array',
+ items => {
+ type => "object",
+ properties => {
+ negate => {
+ description=> "Negate group.",
+ type => 'boolean',
+ },
+ }
+ }
+ },
+ code => sub {
+ my ($param) = @_;
+
+ my $rdb = PMG::RuleDB->new();
+
+ my $settings = $rdb->rule_get_group_settings($param->{id}, $param->{ogroup}, $name);
+
+ my $ret = {
+ negate => $settings->{negate},
+ };
+
+ return $ret;
+ }});
+
+ __PACKAGE__->register_method ({
+ name => "set_${name}_group_settings",
+ path => "$name/{ogroup}",
+ method => 'PUT',
+ description => "Update '$name' group settings for rule.",
+ proxyto => 'master',
+ protected => 1,
+ permissions => { check => [ 'admin' ] },
+ parameters => {
+ additionalProperties => 0,
+ properties => {
+ id => {
+ description => "Rule ID.",
+ type => 'integer',
+ },
+ ogroup => {
+ description => "Groups ID.",
+ type => 'integer',
+ },
+ negate => {
+ description => "Negate group.",
+ type => 'boolean',
+ optional => 1,
+ },
+ },
+ },
+ returns => { type => 'null' },
+ code => sub {
+ my ($param) = @_;
+
+ my $rdb = PMG::RuleDB->new();
+
+ if(defined($param->{negate})) {
+ $rdb->rule_set_group_setting_negate($param->{negate}, $param->{id}, $param->{ogroup}, $name);
+ }
+
+ return;
+ }});
+ }
};
$register_rule_group_api->('from');
--
2.30.2
^ permalink raw reply [flat|nested] 17+ messages in thread
* [pmg-devel] [PATCH pmg-api 4/8] feature: negation: implement matching logic
2023-04-07 13:42 [pmg-devel] [PATCH pmg-api/gui/docs, proxmox-widget-toolkit] Extend rule system Leo Nunner
` (2 preceding siblings ...)
2023-04-07 13:42 ` [pmg-devel] [PATCH pmg-api 3/8] feature: negation: expand/implement API endpoints Leo Nunner
@ 2023-04-07 13:42 ` Leo Nunner
2023-04-11 7:35 ` Leo Nunner
2023-04-07 13:42 ` [pmg-devel] [PATCH pmg-api 5/8] feature: match groups: add field to database Leo Nunner
` (8 subsequent siblings)
12 siblings, 1 reply; 17+ messages in thread
From: Leo Nunner @ 2023-04-07 13:42 UTC (permalink / raw)
To: pmg-devel
Straightforward for most objects, where the matching result gets XOR'd
with the specific negation setting. For 'what' objects, more changes
were necessary: the different types of what matches all needed to be
expanded inside their respective parse_entity functions. For some of
these, the result might be _strange_ (which is more due to the concept
of inverting what objects itself), but seems to work as intended.
Signed-off-by: Leo Nunner <l.nunner@proxmox.com>
---
src/PMG/RuleCache.pm | 8 ++++----
src/PMG/RuleDB/ArchiveFilter.pm | 6 ++++--
src/PMG/RuleDB/ContentTypeFilter.pm | 6 ++++--
src/PMG/RuleDB/MatchArchiveFilename.pm | 4 ++--
src/PMG/RuleDB/MatchField.pm | 2 +-
src/PMG/RuleDB/MatchFilename.pm | 2 +-
6 files changed, 16 insertions(+), 12 deletions(-)
diff --git a/src/PMG/RuleCache.pm b/src/PMG/RuleCache.pm
index c5a57f6..04e35da 100644
--- a/src/PMG/RuleCache.pm
+++ b/src/PMG/RuleCache.pm
@@ -252,7 +252,7 @@ sub from_match {
}
foreach my $obj (@$from) {
- return 1 if $obj->who_match($addr, $ip, $ldap);
+ return 1 if ($obj->who_match($addr, $ip, $ldap) xor $obj->{negate});
}
return 0;
@@ -266,7 +266,7 @@ sub to_match {
return 1 if !defined ($to);
foreach my $obj (@$to) {
- return 1 if $obj->who_match($addr, undef, $ldap);
+ return 1 if ($obj->who_match($addr, undef, $ldap) xor $obj->{negate});
}
return 0;
@@ -280,7 +280,7 @@ sub when_match {
return 1 if !defined ($when);
foreach my $obj (@$when) {
- return 1 if $obj->when_match($time);
+ return 1 if ($obj->when_match($time) xor $obj->{negate});
}
return 0;
@@ -325,7 +325,7 @@ sub what_match {
foreach my $obj (@$what) {
if ($obj->can ("what_match_targets")) {
my $target_info;
- if ($target_info = $obj->what_match_targets($queue, $element, $msginfo, $dbh)) {
+ if (($target_info = $obj->what_match_targets($queue, $element, $msginfo, $dbh)) xor $obj->{negate}) {
foreach my $k (keys %$target_info) {
my $cmarks = $target_info->{$k}->{marks}; # make a copy
$res->{$k} = $target_info->{$k};
diff --git a/src/PMG/RuleDB/ArchiveFilter.pm b/src/PMG/RuleDB/ArchiveFilter.pm
index 6d91556..61f6c50 100755
--- a/src/PMG/RuleDB/ArchiveFilter.pm
+++ b/src/PMG/RuleDB/ArchiveFilter.pm
@@ -60,10 +60,12 @@ sub parse_entity {
my $glob_ct = $entity->{PMX_glob_ct};
if ($header_ct && $header_ct =~ m|$self->{field_value}|) {
- push @$res, $id;
+ push @$res, $id if !$self->{negate};
} elsif ($magic_ct && $magic_ct =~ m|$self->{field_value}|) {
- push @$res, $id;
+ push @$res, $id if !$self->{negate};
} elsif ($glob_ct && $glob_ct =~ m|$self->{field_value}|) {
+ push @$res, $id if !$self->{negate};
+ } elsif ($self->{negate}) {
push @$res, $id;
} else {
# match inside archives
diff --git a/src/PMG/RuleDB/ContentTypeFilter.pm b/src/PMG/RuleDB/ContentTypeFilter.pm
index 76fc1ce..eb35292 100755
--- a/src/PMG/RuleDB/ContentTypeFilter.pm
+++ b/src/PMG/RuleDB/ContentTypeFilter.pm
@@ -72,10 +72,12 @@ sub parse_entity {
my $glob_ct = $entity->{PMX_glob_ct};
if ($header_ct && $header_ct =~ m|$self->{field_value}|) {
- push @$res, $id;
+ push @$res, $id if !$self->{negate};
} elsif ($magic_ct && $magic_ct =~ m|$self->{field_value}|) {
- push @$res, $id;
+ push @$res, $id if !$self->{negate};
} elsif ($glob_ct && $glob_ct =~ m|$self->{field_value}|) {
+ push @$res, $id if !$self->{negate};
+ } elsif ($self->{negate}) {
push @$res, $id;
}
}
diff --git a/src/PMG/RuleDB/MatchArchiveFilename.pm b/src/PMG/RuleDB/MatchArchiveFilename.pm
index 2ef3543..8abd592 100644
--- a/src/PMG/RuleDB/MatchArchiveFilename.pm
+++ b/src/PMG/RuleDB/MatchArchiveFilename.pm
@@ -29,12 +29,12 @@ sub parse_entity {
chomp $id;
my $fn = PMG::Utils::extract_filename($entity->head);
- if (defined($fn) && $fn =~ m|^$self->{fname}$|i) {
+ if (defined($fn) && ($fn =~ m|^$self->{fname}$|i xor $self->{negate})) {
push @$res, $id;
} elsif (my $filenames = $entity->{PMX_filenames}) {
# Match inside archives
for my $fn (keys %$filenames) {
- if ($fn =~ m|^$self->{fname}$|i) {
+ if ($fn =~ m|^$self->{fname}$|i xor $self->{negate}) {
push @$res, $id;
last;
}
diff --git a/src/PMG/RuleDB/MatchField.pm b/src/PMG/RuleDB/MatchField.pm
index 2b56058..8f89297 100644
--- a/src/PMG/RuleDB/MatchField.pm
+++ b/src/PMG/RuleDB/MatchField.pm
@@ -111,7 +111,7 @@ sub parse_entity {
my $decvalue = PMG::Utils::decode_rfc1522($value);
$decvalue = PMG::Utils::try_decode_utf8($decvalue);
- if ($decvalue =~ m|$self->{field_value}|i) {
+ if (($decvalue =~ m|$self->{field_value}|i) xor $self->{negate}) {
push @$res, $id;
}
}
diff --git a/src/PMG/RuleDB/MatchFilename.pm b/src/PMG/RuleDB/MatchFilename.pm
index c9cdbe0..0665efc 100644
--- a/src/PMG/RuleDB/MatchFilename.pm
+++ b/src/PMG/RuleDB/MatchFilename.pm
@@ -91,7 +91,7 @@ sub parse_entity {
chomp $id;
if (my $value = PMG::Utils::extract_filename($entity->head)) {
- if ($value =~ m|^$self->{fname}$|i) {
+ if (($value =~ m|^$self->{fname}$|i) xor $self->{negate}) {
push @$res, $id;
}
}
--
2.30.2
^ permalink raw reply [flat|nested] 17+ messages in thread
* [pmg-devel] [PATCH pmg-api 5/8] feature: match groups: add field to database
2023-04-07 13:42 [pmg-devel] [PATCH pmg-api/gui/docs, proxmox-widget-toolkit] Extend rule system Leo Nunner
` (3 preceding siblings ...)
2023-04-07 13:42 ` [pmg-devel] [PATCH pmg-api 4/8] feature: negation: implement matching logic Leo Nunner
@ 2023-04-07 13:42 ` Leo Nunner
2023-04-07 13:42 ` [pmg-devel] [PATCH pmg-api 6/8] feature: match groups: parse field into objects Leo Nunner
` (7 subsequent siblings)
12 siblings, 0 replies; 17+ messages in thread
From: Leo Nunner @ 2023-04-07 13:42 UTC (permalink / raw)
To: pmg-devel
Adds a 'MatchGroup' field to the 'RuleGroup' table.
Signed-off-by: Leo Nunner <l.nunner@proxmox.com>
---
src/PMG/DBTools.pm | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/src/PMG/DBTools.pm b/src/PMG/DBTools.pm
index 07d39b1..e038365 100644
--- a/src/PMG/DBTools.pm
+++ b/src/PMG/DBTools.pm
@@ -408,6 +408,7 @@ sub create_ruledb {
Rule_ID INTEGER NOT NULL,
Grouptype INTEGER NOT NULL,
Negate INTEGER NOT NULL DEFAULT 0,
+ MatchGroup INTEGER NOT NULL DEFAULT 0,
PRIMARY KEY (Objectgroup_ID, Rule_ID, Grouptype)
);
@@ -583,6 +584,19 @@ sub upgradedb {
}
}
+ # Allow logical AND for rule objects
+ if (!database_column_exists($dbh, 'RuleGroup', 'MatchGroup')) {
+ eval {
+ $dbh->begin_work;
+ $dbh->do("ALTER TABLE RuleGroup ADD COLUMN MatchGroup INTEGER NOT NULL DEFAULT 0");
+ $dbh->commit;
+ };
+ if (my $err = $@) {
+ $dbh->rollback;
+ die $err;
+ }
+ }
+
foreach my $table (keys %$tables) {
eval { $dbh->do("ANALYZE $table"); };
warn $@ if $@;
--
2.30.2
^ permalink raw reply [flat|nested] 17+ messages in thread
* [pmg-devel] [PATCH pmg-api 6/8] feature: match groups: parse field into objects
2023-04-07 13:42 [pmg-devel] [PATCH pmg-api/gui/docs, proxmox-widget-toolkit] Extend rule system Leo Nunner
` (4 preceding siblings ...)
2023-04-07 13:42 ` [pmg-devel] [PATCH pmg-api 5/8] feature: match groups: add field to database Leo Nunner
@ 2023-04-07 13:42 ` Leo Nunner
2023-04-07 13:42 ` [pmg-devel] [PATCH pmg-api 7/8] feature: match groups: update API endpoints Leo Nunner
` (6 subsequent siblings)
12 siblings, 0 replies; 17+ messages in thread
From: Leo Nunner @ 2023-04-07 13:42 UTC (permalink / raw)
To: pmg-devel
Parse the MatchGroup value from the databsae into the 'and' property of
our abstraction objects. We are skipping this step for 'action' objects.
A function was added to update the MatchGroup value for a specific
object-rule relation.
Signed-off-by: Leo Nunner <l.nunner@proxmox.com>
---
src/PMG/API2/ObjectGroupHelpers.pm | 1 +
src/PMG/RuleCache.pm | 4 +++-
src/PMG/RuleDB.pm | 21 ++++++++++++++++++++-
3 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/src/PMG/API2/ObjectGroupHelpers.pm b/src/PMG/API2/ObjectGroupHelpers.pm
index c3e6448..9bedce2 100644
--- a/src/PMG/API2/ObjectGroupHelpers.pm
+++ b/src/PMG/API2/ObjectGroupHelpers.pm
@@ -51,6 +51,7 @@ sub format_object_group {
name => $og->{name},
info => $og->{info},
negate => $og->{negate},
+ 'and' => $og->{and},
};
}
return $res;
diff --git a/src/PMG/RuleCache.pm b/src/PMG/RuleCache.pm
index 04e35da..9a531ef 100644
--- a/src/PMG/RuleCache.pm
+++ b/src/PMG/RuleCache.pm
@@ -56,7 +56,7 @@ sub new {
my ($from, $to, $when, $what, $action);
my $sth1 = $dbh->prepare(
- "SELECT Objectgroup_ID, Grouptype, Negate FROM RuleGroup " .
+ "SELECT Objectgroup_ID, Grouptype, Negate, MatchGroup FROM RuleGroup " .
"where RuleGroup.Rule_ID = '$ruleid' " .
"ORDER BY Grouptype, Objectgroup_ID");
@@ -65,6 +65,7 @@ sub new {
my $gtype = $ref1->{grouptype};
my $groupid = $ref1->{objectgroup_id};
my $negate = $ref1->{negate};
+ my $and = $ref1->{matchgroup};
# emtyp groups differ from non-existent groups!
@@ -93,6 +94,7 @@ sub new {
if ($gtype != 4) { # it doesn't make any sense to negate actions
$obj->{negate} = $negate;
+ $obj->{and} = $and;
}
if ($gtype == 0) { #from
diff --git a/src/PMG/RuleDB.pm b/src/PMG/RuleDB.pm
index 98beda3..dc2be20 100644
--- a/src/PMG/RuleDB.pm
+++ b/src/PMG/RuleDB.pm
@@ -107,7 +107,7 @@ sub load_groups {
my $sth = $self->{dbh}->prepare(
"SELECT RuleGroup.Grouptype, Objectgroup.ID, " .
- "Objectgroup.Name, Objectgroup.Info, Negate " .
+ "Objectgroup.Name, Objectgroup.Info, Negate, MatchGroup " .
"FROM Rulegroup, Objectgroup " .
"WHERE Rulegroup.Rule_ID = ? and " .
"Rulegroup.Objectgroup_ID = Objectgroup.ID " .
@@ -125,6 +125,7 @@ sub load_groups {
if ($ref->{'grouptype'} != 4) { # this doesn't make any sense for actions
$og->{negate} = $ref->{negate};
+ $og->{and} = $ref->{matchgroup};
}
if ($ref->{'grouptype'} == 0) { #from
@@ -797,6 +798,24 @@ sub rule_set_group_setting_negate {
return 1;
}
+sub rule_set_group_setting_matchgroup {
+ my ($self, $value, $ruleid, $groupid, $gtype_str) = @_;
+
+ my $gtype = $grouptype_hash->{$gtype_str} //
+ die "unknown group type '$gtype_str'\n";
+
+ defined($ruleid) || die "undefined rule id: ERROR";
+ defined($groupid) || die "undefined group id: ERROR";
+ defined($gtype) || die "undefined group type: ERROR";
+
+ my $sth = $self->{dbh}->prepare("UPDATE RuleGroup SET MatchGroup = ? " .
+ "WHERE Objectgroup_ID = ? and Rule_ID = ? and Grouptype = ?");
+
+ $sth->execute($value, $groupid, $ruleid, $gtype);
+
+ return 1;
+}
+
sub load_rule {
my ($self, $id) = @_;
--
2.30.2
^ permalink raw reply [flat|nested] 17+ messages in thread
* [pmg-devel] [PATCH pmg-api 7/8] feature: match groups: update API endpoints
2023-04-07 13:42 [pmg-devel] [PATCH pmg-api/gui/docs, proxmox-widget-toolkit] Extend rule system Leo Nunner
` (5 preceding siblings ...)
2023-04-07 13:42 ` [pmg-devel] [PATCH pmg-api 6/8] feature: match groups: parse field into objects Leo Nunner
@ 2023-04-07 13:42 ` Leo Nunner
2023-04-07 13:42 ` [pmg-devel] [PATCH pmg-api 8/8] feature: match groups: implement matching logic Leo Nunner
` (5 subsequent siblings)
12 siblings, 0 replies; 17+ messages in thread
From: Leo Nunner @ 2023-04-07 13:42 UTC (permalink / raw)
To: pmg-devel
Adds the 'and' parameter to all rule API endpoints. The conditional
endpoints from the 'negate' setting are still being used again here,
since we don't need this property for actions.
Signed-off-by: Leo Nunner <l.nunner@proxmox.com>
---
src/PMG/API2/Rules.pm | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/src/PMG/API2/Rules.pm b/src/PMG/API2/Rules.pm
index ad3f6c7..4882c5a 100644
--- a/src/PMG/API2/Rules.pm
+++ b/src/PMG/API2/Rules.pm
@@ -266,6 +266,11 @@ my $register_rule_group_api = sub {
type => 'boolean',
optional => 1,
},
+ 'and' => {
+ description => "Add to logical AND list.",
+ type => 'boolean',
+ optional => 1,
+ },
},
},
returns => { type => 'null' },
@@ -282,6 +287,10 @@ my $register_rule_group_api = sub {
$rdb->rule_set_group_setting_negate($param->{negate}, $param->{id}, $param->{ogroup}, $name);
}
+ if (defined($param->{and})) {
+ $rdb->rule_set_group_setting_matchgroup($param->{and}, $param->{id}, $param->{ogroup}, $name);
+ }
+
PMG::DBTools::reload_ruledb();
return undef;
@@ -354,6 +363,10 @@ my $register_rule_group_api = sub {
description=> "Negate group.",
type => 'boolean',
},
+ 'and' => {
+ description=> "In match group.",
+ type => 'boolean',
+ }
}
}
},
@@ -366,6 +379,7 @@ my $register_rule_group_api = sub {
my $ret = {
negate => $settings->{negate},
+ 'and' => $settings->{matchgroup},
};
return $ret;
@@ -395,6 +409,11 @@ my $register_rule_group_api = sub {
type => 'boolean',
optional => 1,
},
+ 'and' => {
+ description => "Add to logical AND list.",
+ type => 'boolean',
+ optional => 1,
+ },
},
},
returns => { type => 'null' },
@@ -407,6 +426,10 @@ my $register_rule_group_api = sub {
$rdb->rule_set_group_setting_negate($param->{negate}, $param->{id}, $param->{ogroup}, $name);
}
+ if(defined($param->{and})) {
+ $rdb->rule_set_group_setting_matchgroup($param->{and}, $param->{id}, $param->{ogroup}, $name);
+ }
+
return;
}});
}
--
2.30.2
^ permalink raw reply [flat|nested] 17+ messages in thread
* [pmg-devel] [PATCH pmg-api 8/8] feature: match groups: implement matching logic
2023-04-07 13:42 [pmg-devel] [PATCH pmg-api/gui/docs, proxmox-widget-toolkit] Extend rule system Leo Nunner
` (6 preceding siblings ...)
2023-04-07 13:42 ` [pmg-devel] [PATCH pmg-api 7/8] feature: match groups: update API endpoints Leo Nunner
@ 2023-04-07 13:42 ` Leo Nunner
2023-04-07 13:42 ` [pmg-devel] [PATCH pmg-gui 1/2] feature: negate objects inside rules Leo Nunner
` (4 subsequent siblings)
12 siblings, 0 replies; 17+ messages in thread
From: Leo Nunner @ 2023-04-07 13:42 UTC (permalink / raw)
To: pmg-devel
Implements the logic behind match groups; A match group holds
several objects inside it, and only evalutes to true if ALL the
contained objects evaluate to true. Match groups are connected via
logical 'or' to all other objects inside the rule:
- Rule
- Match Group
- Object 1
- Object 2
- Object 3
This rule will match if either (Object 1 && Object 2), or if Object 3
evaluate to true.
Signed-off-by: Leo Nunner <l.nunner@proxmox.com>
---
RFC: This might be a bit weird for What objects: currently, it checks if
there are matches for each of the objects inside the match groups; if
one exists, *all* marks get pushed to the list. I'm not sure if this is
exactly what we want, but I can't really think of a better way here.
src/PMG/RuleCache.pm | 50 +++++++++++++++++++++++++++++++++++++-------
1 file changed, 43 insertions(+), 7 deletions(-)
diff --git a/src/PMG/RuleCache.pm b/src/PMG/RuleCache.pm
index 9a531ef..eceffaf 100644
--- a/src/PMG/RuleCache.pm
+++ b/src/PMG/RuleCache.pm
@@ -253,11 +253,19 @@ sub from_match {
$ip = $1;
}
+ # don't return true if there are no elements marked with and!
+ my $and = grep { $_->{and} } @$from;
foreach my $obj (@$from) {
- return 1 if ($obj->who_match($addr, $ip, $ldap) xor $obj->{negate});
+ my $match = ($obj->who_match($addr, $ip, $ldap) xor $obj->{negate});
+
+ if ($obj->{and}) {
+ $and &&= $match;
+ } else {
+ return 1 if $match;
+ }
}
- return 0;
+ return $and;
}
sub to_match {
@@ -267,11 +275,18 @@ sub to_match {
return 1 if !defined ($to);
+ my $and = grep { $_->{and} } @$to;
foreach my $obj (@$to) {
- return 1 if ($obj->who_match($addr, undef, $ldap) xor $obj->{negate});
+ my $match = ($obj->who_match($addr, undef, $ldap) xor $obj->{negate});
+
+ if ($obj->{and}) {
+ $and &&= $match;
+ } else {
+ return 1 if $match;
+ }
}
- return 0;
+ return $and;
}
sub when_match {
@@ -281,11 +296,18 @@ sub when_match {
return 1 if !defined ($when);
+ my $and = grep { $_->{and} } @$when;
foreach my $obj (@$when) {
- return 1 if ($obj->when_match($time) xor $obj->{negate});
+ my $match = ($obj->when_match($time) xor $obj->{negate});
+
+ if ($obj->{and}) {
+ $and &&= $match;
+ } else {
+ return 1 if $match;
+ }
}
- return 0;
+ return $and;
}
sub what_match {
@@ -311,14 +333,28 @@ sub what_match {
my $marks;
+ # First, we loop through all objects which are not 'Spam Filter'
+ my $group_valid = 1;
+ my @group_matches = ();
foreach my $obj (@$what) {
if (!$obj->can('what_match_targets')) {
if (my $match = $obj->what_match($queue, $element, $msginfo, $dbh)) {
- push @$marks, @$match;
+ if ($obj->{and}) {
+ push @group_matches, $match;
+ } else {
+ push @$marks, @$match;
+ }
+ } elsif ($obj->{and}) {
+ $group_valid = 0;
}
}
}
+ # If the 'Match All' group matches, push everything into the list
+ if ($group_valid) {
+ push(@$marks, @group_matches);
+ }
+
foreach my $target (@{$msginfo->{targets}}) {
$res->{$target}->{marks} = $marks;
$res->{marks} = $marks;
--
2.30.2
^ permalink raw reply [flat|nested] 17+ messages in thread
* [pmg-devel] [PATCH pmg-gui 1/2] feature: negate objects inside rules
2023-04-07 13:42 [pmg-devel] [PATCH pmg-api/gui/docs, proxmox-widget-toolkit] Extend rule system Leo Nunner
` (7 preceding siblings ...)
2023-04-07 13:42 ` [pmg-devel] [PATCH pmg-api 8/8] feature: match groups: implement matching logic Leo Nunner
@ 2023-04-07 13:42 ` Leo Nunner
2023-04-07 13:42 ` [pmg-devel] [PATCH pmg-gui 2/2] feature: introduce logical 'and' for rules Leo Nunner
` (3 subsequent siblings)
12 siblings, 0 replies; 17+ messages in thread
From: Leo Nunner @ 2023-04-07 13:42 UTC (permalink / raw)
To: pmg-devel
This patch exposes the new 'negate' parameter through the GUI. All
objects (except for actions) now have a small icon next to them in the
rule overview, and clicking it will toggle the respective negation
setting. Negated objects are marked by a small 'NOT' before the object
name.
Signed-off-by: Leo Nunner <l.nunner@proxmox.com>
---
js/RuleInfo.js | 45 +++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 43 insertions(+), 2 deletions(-)
diff --git a/js/RuleInfo.js b/js/RuleInfo.js
index b7802fa..d14e229 100644
--- a/js/RuleInfo.js
+++ b/js/RuleInfo.js
@@ -58,6 +58,22 @@ Ext.define('PMG.RuleInfo', {
);
},
+ updateNegateObjectGroup: function(rec) {
+ var me = this;
+ Proxmox.Utils.API2Request({
+ url: me.getViewModel().get('baseurl') + '/' + rec.data.oclass + '/'+ rec.data.typeid,
+ method: 'PUT',
+ params: { negate: rec.data.negate ? 0 : 1 },
+ waitMsgTarget: me.getView(),
+ callback: function() {
+ me.reload();
+ },
+ failure: function(response, opts) {
+ Ext.Msg.alert(gettext('Error'), response.htmlStatus);
+ },
+ });
+ },
+
addObjectGroup: function(type, record) {
var me = this;
var baseurl = me.getViewModel().get('baseurl');
@@ -113,7 +129,7 @@ Ext.define('PMG.RuleInfo', {
});
store.load();
Ext.Array.each(ruledata[oc], function(og) {
- data.push({ oclass: oc, name: og.name, typeid: og.id });
+ data.push({ oclass: oc, name: og.name, typeid: og.id, negate: og.negate });
});
});
@@ -126,6 +142,11 @@ Ext.define('PMG.RuleInfo', {
me.removeObjectGroup(record);
},
+ negateIconClick: function(gridView, rowindex, colindex, button, event, record) {
+ var me = this;
+ me.updateNegateObjectGroup(record);
+ },
+
removeDrop: function(gridView, data, overModel) {
var me = this;
var record = data.records[0]; // only one
@@ -163,7 +184,7 @@ Ext.define('PMG.RuleInfo', {
stores: {
objects: {
- fields: ['oclass', 'name', 'typeid'],
+ fields: ['oclass', 'name', 'typeid', 'negate'],
groupField: 'oclass',
sorters: 'name',
},
@@ -294,8 +315,28 @@ Ext.define('PMG.RuleInfo', {
{
header: gettext('Name'),
dataIndex: 'name',
+ renderer: function(value, data, record) {
+ return record.data.negate ? '<span style="color:gray">' + gettext("NOT") + ' </span>' + value : value;
+ },
flex: 1,
},
+ {
+ text: '',
+ xtype: 'actioncolumn',
+ align: 'center',
+ width: 40,
+ items: [
+ {
+ getClass: function(v, m, { data }) {
+ if (data.oclass === 'action') return '';
+ return 'fa fa-fw fa-refresh';
+ },
+ isActionDisabled: (v, r, c, i, { data }) => data.oclass === 'action',
+ tooltip: gettext('Negate'),
+ handler: 'negateIconClick',
+ },
+ ],
+ },
{
text: '',
xtype: 'actioncolumn',
--
2.30.2
^ permalink raw reply [flat|nested] 17+ messages in thread
* [pmg-devel] [PATCH pmg-gui 2/2] feature: introduce logical 'and' for rules
2023-04-07 13:42 [pmg-devel] [PATCH pmg-api/gui/docs, proxmox-widget-toolkit] Extend rule system Leo Nunner
` (8 preceding siblings ...)
2023-04-07 13:42 ` [pmg-devel] [PATCH pmg-gui 1/2] feature: negate objects inside rules Leo Nunner
@ 2023-04-07 13:42 ` Leo Nunner
2023-04-07 13:42 ` [pmg-devel] [PATCH pmg-docs] docs: document negation and match groups Leo Nunner
` (2 subsequent siblings)
12 siblings, 0 replies; 17+ messages in thread
From: Leo Nunner @ 2023-04-07 13:42 UTC (permalink / raw)
To: pmg-devel
Rework the rule info view, which is now displayed as a tree structure.
The first level of objects gets connected via logical 'OR'. The
subfolder 'Match group' is for objects that get connected via logical
'AND', meaning that all objects inside the 'Match group' folder need to
match. 'Match group' is also connected via logical 'OR' with all other
objects on the first level.
The drag and drop behaviour is the same as before, with the addition of
more specific actions: objects can be dragged into/out of the 'Match
group' folder, and can also be dragged directly from the 'Available
Objects' table into their respective match group.
Signed-off-by: Leo Nunner <l.nunner@proxmox.com>
---
css/ext6-pmg.css | 10 ++
js/RuleInfo.js | 306 ++++++++++++++++++++++++++++++++++-------------
js/Utils.js | 14 +--
3 files changed, 243 insertions(+), 87 deletions(-)
diff --git a/css/ext6-pmg.css b/css/ext6-pmg.css
index 2f999f4..9a8397d 100644
--- a/css/ext6-pmg.css
+++ b/css/ext6-pmg.css
@@ -96,6 +96,12 @@
content: "\f115";
}
+.x-tree-icon-custom {
+ line-height: 1.6em;
+ color: #555;
+ margin-right: 5px;
+}
+
/* loading in task list */
.x-grid-row-loading {
background: no-repeat center center;
@@ -134,6 +140,10 @@ div.inline-block {
cursor: pointer;
}
+.move {
+ cursor: move;
+}
+
.x-grid-filters-filtered-column{
font-style: italic;
font-weight: bold;
diff --git a/js/RuleInfo.js b/js/RuleInfo.js
index d14e229..a36ecda 100644
--- a/js/RuleInfo.js
+++ b/js/RuleInfo.js
@@ -33,6 +33,143 @@ Ext.define('PMG.RuleInfo', {
});
},
+ renderUsedObjects: function(v) {
+ let me = this;
+
+ me.usedDragZone = new Ext.dd.DragZone(v.getEl(), {
+ getDragData: function(e) {
+ var sourceEl = e.getTarget(".x-grid-item", 10);
+
+ if (sourceEl) {
+ let record = v.getView().getRecord(sourceEl);
+
+ let ddel = document.createElement('div');
+ ddel.innerHTML = record.data.name;
+ ddel.id = Ext.id();
+
+ return {
+ ddel: ddel,
+ repairXY: Ext.fly(sourceEl).getXY(),
+ record: record,
+ };
+ }
+
+ return null;
+ },
+
+ getRepairXY: function() {
+ return this.dragData.repairXY;
+ },
+
+ onBeforeDrag: function(source, e) {
+ return source.record.data.leaf;
+ },
+ });
+
+ me.usedDropZone = new Ext.dd.DropZone(v.getEl(), {
+ getTargetFromEvent: function(e) {
+ var sourceEl = e.getTarget(".x-grid-item", 10);
+ return sourceEl ? v.getView().getRecord(sourceEl) : null;
+ },
+
+ onNodeOver: function(target, dd, e, source) {
+ if (source.add) return Ext.dd.DropZone.prototype.dropAllowed;
+ if (source.record.data.oclass === 'action') return Ext.dd.DropZone.prototype.dropNotAllowed;
+
+ if (source.record.data.oclass === target.data.oclass &&
+ target.data.leaf === false) {
+ return Ext.dd.DropZone.prototype.dropAllowed;
+ } else {
+ return Ext.dd.DropZone.prototype.dropNotAllowed;
+ }
+ },
+
+ onNodeDrop: function(target, dd, e, source) {
+ if (source.record.data.oclass === 'action') return false;
+
+ // First, do we want to add a new object?
+ if (source.add) {
+ let and = target.data.name !== PMG.Utils.oclass_text[source.type];
+ me.addObjectGroup(source.type, source.record, and);
+ return true;
+ }
+
+ // Otherwise, update an existing one
+ if (source.record.data.oclass !== target.data.oclass ||
+ target.data.leaf) {
+ return false;
+ }
+
+ let record = source.record;
+
+ if (target.data.name !== PMG.Utils.oclass_text[record.data.oclass]) {
+ if (record.data.and) return false;
+
+ me.updateRecordMatchAll(record, 1);
+ } else {
+ me.updateRecordMatchAll(record, 0);
+ }
+
+ return true;
+ },
+ });
+ },
+
+ renderAvailObjects: function(v) {
+ let me = this;
+
+ me.availDragZone = new Ext.dd.DragZone(v.getEl(), {
+ getDragData: function(e) {
+ var sourceEl = e.getTarget(".x-grid-item", 10);
+
+ if (sourceEl) {
+ let tab = v.getActiveTab();
+ let record = tab.getView().getRecord(sourceEl);
+
+ let ddel = document.createElement('div');
+ ddel.innerHTML = record.data.name;
+ ddel.id = Ext.id();
+
+ return {
+ ddel: ddel,
+ repairXY: Ext.fly(sourceEl).getXY(),
+ record: record,
+ add: true,
+ type: tab.type,
+ };
+ }
+
+ return null;
+ },
+
+ getRepairXY: function() {
+ return this.dragData.repairXY;
+ },
+ });
+
+ me.availDropZone = new Ext.dd.DropZone(v.getEl(), {
+ getTargetFromEvent: function(e) {
+ var sourceEl = e.getTarget(".x-grid-item", 10);
+ let tab = v.getActiveTab();
+ return sourceEl ? tab.getView().getRecord(sourceEl) : null;
+ },
+
+ onNodeOver: function(target, dd, e, source) {
+ if (source.add) {
+ return Ext.dd.DropZone.prototype.dropNotAllowed;
+ } else {
+ return Ext.dd.DropZone.prototype.dropAllowed;
+ }
+ },
+
+ onNodeDrop: function(target, dd, e, source) {
+ if (source.add) return false;
+ me.removeObjectGroup(source.record);
+ return true;
+ },
+ });
+ },
+
removeObjectGroup: function(rec) {
var me = this;
Ext.Msg.confirm(
@@ -74,14 +211,34 @@ Ext.define('PMG.RuleInfo', {
});
},
- addObjectGroup: function(type, record) {
+ updateRecordMatchAll: function(rec, val) {
+ var me = this;
+ Proxmox.Utils.API2Request({
+ url: me.getViewModel().get('baseurl') + '/' + rec.data.oclass + '/'+ rec.data.typeid,
+ method: 'PUT',
+ params: { and: val },
+ waitMsgTarget: me.getView(),
+ callback: function() {
+ me.reload();
+ },
+ failure: function(response, opts) {
+ Ext.Msg.alert(gettext('Error'), response.htmlStatus);
+ },
+ });
+ },
+
+ addObjectGroup: function(type, record, and, negate) {
var me = this;
var baseurl = me.getViewModel().get('baseurl');
var url = baseurl + '/' + type;
var id = type === 'action'?record.data.ogroup:record.data.id;
Proxmox.Utils.API2Request({
url: url,
- params: { ogroup: id },
+ params: {
+ ogroup: id,
+ and: and ? 1 : 0,
+ negate: negate ? 1: 0,
+ },
method: 'POST',
waitMsgTarget: me.getView(),
callback: function() {
@@ -105,7 +262,7 @@ Ext.define('PMG.RuleInfo', {
ruledata.name = Ext.String.htmlEncode(ruledata.name);
viewmodel.set('selectedRule', ruledata);
- var data = [];
+ var root = { expanded: true, children: [] };
Ext.Array.each(['from', 'to', 'when', 'what', 'action'], function(oc) {
var store = viewmodel.get(oc + 'objects');
if (ruledata[oc] === undefined || store === undefined) { return; }
@@ -128,12 +285,45 @@ Ext.define('PMG.RuleInfo', {
},
});
store.load();
+
+ var child = {
+ name: PMG.Utils.oclass_text[oc],
+ oclass: oc,
+ iconCls: PMG.Utils.oclass_icon[oc],
+ children: [],
+ leaf: false,
+ expanded: true,
+ };
+
+ var and_list = { name: "Match Group", oclass: oc, children: [] };
+
Ext.Array.each(ruledata[oc], function(og) {
- data.push({ oclass: oc, name: og.name, typeid: og.id, negate: og.negate });
+ var entry = {
+ oclass: oc,
+ name: og.name,
+ typeid: og.id,
+ negate: og.negate,
+ leaf: true,
+ cls: 'move',
+ iconCls: PMG.Utils.oclass_icon[oc],
+ };
+
+ if (og.and) {
+ and_list.children.push(entry);
+ } else {
+ child.children.push(entry);
+ }
});
+
+ child.expanded = child.children.length || and_list.children.length;
+ and_list.expanded = and_list.children.length !== 0;
+
+ if (oc !== "action") child.children.push(and_list);
+
+ root.children.push(child);
});
- viewmodel.get('objects').setData(data);
+ viewmodel.get('objects').setRoot(root);
}
},
@@ -147,34 +337,17 @@ Ext.define('PMG.RuleInfo', {
me.updateNegateObjectGroup(record);
},
- removeDrop: function(gridView, data, overModel) {
- var me = this;
- var record = data.records[0]; // only one
- me.removeObjectGroup(record);
- return true;
- },
-
addIconClick: function(gridView, rowindex, colindex, button, event, record) {
var me = this;
- me.addObjectGroup(gridView.panel.type, record);
+ me.addObjectGroup(gridView.panel.type, record, false, false);
return true;
},
- addDrop: function(gridView, data, overModel) {
+ addAndIconClick: function(gridView, rowindex, colindex, button, event, record) {
var me = this;
- var record = data.records[0]; // only one
- me.addObjectGroup(data.view.panel.type, record);
+ me.addObjectGroup(gridView.panel.type, record, true, false);
return true;
},
-
- control: {
- 'grid[reference=usedobjects]': {
- drop: 'addDrop',
- },
- 'tabpanel[reference=availobjects] > grid': {
- drop: 'removeDrop',
- },
- },
},
viewModel: {
@@ -184,9 +357,10 @@ Ext.define('PMG.RuleInfo', {
stores: {
objects: {
- fields: ['oclass', 'name', 'typeid', 'negate'],
- groupField: 'oclass',
+ fields: ['oclass', 'name', 'typeid', 'negate', 'and'],
sorters: 'name',
+ folderSort: true,
+ type: 'tree',
},
actionobjects: {
@@ -274,38 +448,14 @@ Ext.define('PMG.RuleInfo', {
],
},
{
- xtype: 'grid',
+ xtype: 'treepanel',
reference: 'usedobjects',
hidden: true,
+ rootVisible: false,
emptyText: gettext('No Objects'),
- features: [{
- id: 'group',
- ftype: 'grouping',
- enableGroupingMenu: false,
- collapsible: false,
- groupHeaderTpl: [
- '{[PMG.Utils.format_oclass(values.name)]}',
- ],
- }],
title: gettext('Used Objects'),
- viewConfig: {
- plugins: {
- ptype: 'gridviewdragdrop',
- copy: true,
- dragGroup: 'usedobjects',
- dropGroup: 'unusedobjects',
-
- // do not show default grid dragdrop behaviour
- dropZone: {
- indicatorHtml: '',
- indicatorCls: '',
- handleNodeDrop: Ext.emptyFn,
- },
- },
- },
-
columns: [
{
header: gettext('Type'),
@@ -315,6 +465,7 @@ Ext.define('PMG.RuleInfo', {
{
header: gettext('Name'),
dataIndex: 'name',
+ xtype: 'treecolumn',
renderer: function(value, data, record) {
return record.data.negate ? '<span style="color:gray">' + gettext("NOT") + ' </span>' + value : value;
},
@@ -324,27 +475,20 @@ Ext.define('PMG.RuleInfo', {
text: '',
xtype: 'actioncolumn',
align: 'center',
- width: 40,
+ width: 80,
items: [
{
getClass: function(v, m, { data }) {
- if (data.oclass === 'action') return '';
+ if (!data.leaf || data.oclass === 'action') return '';
return 'fa fa-fw fa-refresh';
},
- isActionDisabled: (v, r, c, i, { data }) => data.oclass === 'action',
+ isActionDisabled: (v, r, c, i, { data }) => !data.leaf || data.oclass === 'action',
tooltip: gettext('Negate'),
handler: 'negateIconClick',
},
- ],
- },
- {
- text: '',
- xtype: 'actioncolumn',
- align: 'center',
- width: 40,
- items: [
{
- iconCls: 'fa fa-fw fa-minus-circle',
+ getClass: (v, m, { data }) => data.leaf ? 'fa fa-fw fa-minus-circle' : '',
+ isActionDisabled: (v, r, c, i, { data }) => !data.leaf,
tooltip: gettext('Remove'),
handler: 'removeIconClick',
},
@@ -356,6 +500,10 @@ Ext.define('PMG.RuleInfo', {
store: '{objects}',
hidden: '{!selectedRule}',
},
+
+ listeners: {
+ render: "renderUsedObjects",
+ },
},
{
xtype: 'tabpanel',
@@ -368,23 +516,10 @@ Ext.define('PMG.RuleInfo', {
defaults: {
xtype: 'grid',
emptyText: gettext('No Objects'),
- viewConfig: {
- plugins: {
- ptype: 'gridviewdragdrop',
- dragGroup: 'unusedobjects',
- dropGroup: 'usedobjects',
-
- // do not show default grid dragdrop behaviour
- dropZone: {
- indicatorHtml: '',
- indicatorCls: '',
- handleNodeDrop: Ext.emptyFn,
- },
- },
- },
columns: [
{
header: gettext('Name'),
+ innerCls: 'move',
dataIndex: 'name',
flex: 1,
},
@@ -392,8 +527,16 @@ Ext.define('PMG.RuleInfo', {
text: '',
xtype: 'actioncolumn',
align: 'center',
- width: 40,
+ width: 80,
items: [
+ {
+ iconCls: 'fa fa-fw fa-crosshairs',
+ isActionDisabled: function(v, r, c, i, { data }) {
+ return v.up("tabpanel").getActiveTab().type === 'action';
+ },
+ tooltip: gettext('Add to AND list'),
+ handler: 'addAndIconClick',
+ },
{
iconCls: 'fa fa-fw fa-plus-circle',
tooltip: gettext('Add'),
@@ -445,6 +588,9 @@ Ext.define('PMG.RuleInfo', {
},
},
],
+ listeners: {
+ render: "renderAvailObjects",
+ },
},
],
});
diff --git a/js/Utils.js b/js/Utils.js
index 7fa154e..2495f70 100644
--- a/js/Utils.js
+++ b/js/Utils.js
@@ -61,12 +61,12 @@ Ext.define('PMG.Utils', {
},
oclass_icon: {
- who: '<span class="fa fa-fw fa-user-circle"></span> ',
- what: '<span class="fa fa-fw fa-cube"></span> ',
- when: '<span class="fa fa-fw fa-clock-o"></span> ',
- action: '<span class="fa fa-fw fa-flag"></span> ',
- from: '<span class="fa fa-fw fa-user-circle"></span> ',
- to: '<span class="fa fa-fw fa-user-circle"></span> ',
+ who: 'fa fa-fw fa-user-circle',
+ what: 'fa fa-fw fa-cube',
+ when: 'fa fa-fw fa-clock-o',
+ action: 'fa fa-fw fa-flag',
+ from: 'fa fa-fw fa-user-circle',
+ to: 'fa fa-fw fa-user-circle',
},
mail_status_map: {
@@ -105,7 +105,7 @@ Ext.define('PMG.Utils', {
},
format_oclass: function(oclass) {
- var icon = PMG.Utils.oclass_icon[oclass] || '';
+ var icon = '<span class="' + PMG.Utils.oclass_icon[oclass] + '"></span>' || '';
var text = PMG.Utils.oclass_text[oclass] || oclass;
return icon + text;
},
--
2.30.2
^ permalink raw reply [flat|nested] 17+ messages in thread
* [pmg-devel] [PATCH pmg-docs] docs: document negation and match groups
2023-04-07 13:42 [pmg-devel] [PATCH pmg-api/gui/docs, proxmox-widget-toolkit] Extend rule system Leo Nunner
` (9 preceding siblings ...)
2023-04-07 13:42 ` [pmg-devel] [PATCH pmg-gui 2/2] feature: introduce logical 'and' for rules Leo Nunner
@ 2023-04-07 13:42 ` Leo Nunner
2023-04-07 13:42 ` [pmg-devel] [PATCH widget-toolkit] dark-mode: fix colour of default tree icons Leo Nunner
2023-04-11 9:52 ` [pmg-devel] [PATCH pmg-api/gui/docs, proxmox-widget-toolkit] Extend rule system Thomas Lamprecht
12 siblings, 0 replies; 17+ messages in thread
From: Leo Nunner @ 2023-04-07 13:42 UTC (permalink / raw)
To: pmg-devel
Signed-off-by: Leo Nunner <l.nunner@proxmox.com>
---
pmg-mail-filter.adoc | 29 ++++++++++++++++++++++++++++-
1 file changed, 28 insertions(+), 1 deletion(-)
diff --git a/pmg-mail-filter.adoc b/pmg-mail-filter.adoc
index 3aafe4c..60bb8c0 100644
--- a/pmg-mail-filter.adoc
+++ b/pmg-mail-filter.adoc
@@ -58,9 +58,36 @@ You can also disable a rule completely, which is mostly useful for
testing and debugging. The 'Factory Defaults' button allows you to
reset the filter rules.
+[[pmg_mailfilter_structure]]
+Rule structure
+--------------
+
+Rules have a predefined structure. A rule contains objects of all
+categories (Action, Who, What, When), which are grouped by their
+respective types.
+
+Matches
+~~~~~~~
+
+The logic behind matches is as follows: all categories need to
+match separately; if a category is empty, it matches automatically.
+As for the objects inside a category, it's enough if only
+one of them matches; if one object matches, the whole category
+matches. Finally, if all categories match, the whole rule counts as
+a match and the defined actions are executed.
+
+This behaviour can be changed by using match groups. Each category
+contains a child item titled 'Match groups'. All objects that are
+added to this group will be evaluated together: the match group only
+evaluates as a match if all objects inside it do.
+
+Objects can also be inverted, so that they only match if their
+criteria are *not* fulfilled. This is indicated via a prefixed
+"NOT" in the object overview.
+
[[pmg_mailfilter_action]]
-'Action' - objects
+'Action' objects
------------------
[thumbnail="pmg-gui-mail-filter-actions.png", big=1]
--
2.30.2
^ permalink raw reply [flat|nested] 17+ messages in thread
* [pmg-devel] [PATCH widget-toolkit] dark-mode: fix colour of default tree icons
2023-04-07 13:42 [pmg-devel] [PATCH pmg-api/gui/docs, proxmox-widget-toolkit] Extend rule system Leo Nunner
` (10 preceding siblings ...)
2023-04-07 13:42 ` [pmg-devel] [PATCH pmg-docs] docs: document negation and match groups Leo Nunner
@ 2023-04-07 13:42 ` Leo Nunner
2023-04-11 9:52 ` [pmg-devel] [PATCH pmg-api/gui/docs, proxmox-widget-toolkit] Extend rule system Thomas Lamprecht
12 siblings, 0 replies; 17+ messages in thread
From: Leo Nunner @ 2023-04-07 13:42 UTC (permalink / raw)
To: pmg-devel
Signed-off-by: Leo Nunner <l.nunner@proxmox.com>
---
src/proxmox-dark/scss/other/_icons.scss | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/proxmox-dark/scss/other/_icons.scss b/src/proxmox-dark/scss/other/_icons.scss
index 691ea75..e0cce8f 100644
--- a/src/proxmox-dark/scss/other/_icons.scss
+++ b/src/proxmox-dark/scss/other/_icons.scss
@@ -56,8 +56,8 @@
.pbs-icon-tape-drive,
.x-tree-icon-leaf:not(.x-tree-icon-custom),
// default tree panel icons (api viewer, pbs backup panel)
-.x-tree-icon-parent:not(.x-tree-icon-custom),
-.x-tree-icon-parent-expanded:not(.x-tree-icon-custom) {
+.x-tree-icon-parent:not(.x-tree-icon):not(.x-tree-icon-custom),
+.x-tree-icon-parent-expanded:not(.x-tree-icon):not(.x-tree-icon-custom) {
filter: invert($icon-brightness);
}
--
2.30.2
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [pmg-devel] [PATCH pmg-api 4/8] feature: negation: implement matching logic
2023-04-07 13:42 ` [pmg-devel] [PATCH pmg-api 4/8] feature: negation: implement matching logic Leo Nunner
@ 2023-04-11 7:35 ` Leo Nunner
0 siblings, 0 replies; 17+ messages in thread
From: Leo Nunner @ 2023-04-11 7:35 UTC (permalink / raw)
To: pmg-devel
Just noticed something: the matching logic here won't work for most
objects as intended, since, if the rule is negated, it will already
return 'true' if the first object in the list (e.g., the first email
address) doesn't match… I think it should rather be if the match isn't
present in the *whole* list, instead of "there are objects that don't
match in this list". I'll fix this in a v2, but I'll wait for further
feedback before I send it.
On 2023-04-07 15:42, Leo Nunner wrote:
> Straightforward for most objects, where the matching result gets XOR'd
> with the specific negation setting. For 'what' objects, more changes
> were necessary: the different types of what matches all needed to be
> expanded inside their respective parse_entity functions. For some of
> these, the result might be _strange_ (which is more due to the concept
> of inverting what objects itself), but seems to work as intended.
>
> Signed-off-by: Leo Nunner <l.nunner@proxmox.com>
> ---
> src/PMG/RuleCache.pm | 8 ++++----
> src/PMG/RuleDB/ArchiveFilter.pm | 6 ++++--
> src/PMG/RuleDB/ContentTypeFilter.pm | 6 ++++--
> src/PMG/RuleDB/MatchArchiveFilename.pm | 4 ++--
> src/PMG/RuleDB/MatchField.pm | 2 +-
> src/PMG/RuleDB/MatchFilename.pm | 2 +-
> 6 files changed, 16 insertions(+), 12 deletions(-)
>
> diff --git a/src/PMG/RuleCache.pm b/src/PMG/RuleCache.pm
> index c5a57f6..04e35da 100644
> --- a/src/PMG/RuleCache.pm
> +++ b/src/PMG/RuleCache.pm
> @@ -252,7 +252,7 @@ sub from_match {
> }
>
> foreach my $obj (@$from) {
> - return 1 if $obj->who_match($addr, $ip, $ldap);
> + return 1 if ($obj->who_match($addr, $ip, $ldap) xor $obj->{negate});
> }
>
> return 0;
> @@ -266,7 +266,7 @@ sub to_match {
> return 1 if !defined ($to);
>
> foreach my $obj (@$to) {
> - return 1 if $obj->who_match($addr, undef, $ldap);
> + return 1 if ($obj->who_match($addr, undef, $ldap) xor $obj->{negate});
> }
>
> return 0;
> @@ -280,7 +280,7 @@ sub when_match {
> return 1 if !defined ($when);
>
> foreach my $obj (@$when) {
> - return 1 if $obj->when_match($time);
> + return 1 if ($obj->when_match($time) xor $obj->{negate});
> }
>
> return 0;
> @@ -325,7 +325,7 @@ sub what_match {
> foreach my $obj (@$what) {
> if ($obj->can ("what_match_targets")) {
> my $target_info;
> - if ($target_info = $obj->what_match_targets($queue, $element, $msginfo, $dbh)) {
> + if (($target_info = $obj->what_match_targets($queue, $element, $msginfo, $dbh)) xor $obj->{negate}) {
> foreach my $k (keys %$target_info) {
> my $cmarks = $target_info->{$k}->{marks}; # make a copy
> $res->{$k} = $target_info->{$k};
> diff --git a/src/PMG/RuleDB/ArchiveFilter.pm b/src/PMG/RuleDB/ArchiveFilter.pm
> index 6d91556..61f6c50 100755
> --- a/src/PMG/RuleDB/ArchiveFilter.pm
> +++ b/src/PMG/RuleDB/ArchiveFilter.pm
> @@ -60,10 +60,12 @@ sub parse_entity {
> my $glob_ct = $entity->{PMX_glob_ct};
>
> if ($header_ct && $header_ct =~ m|$self->{field_value}|) {
> - push @$res, $id;
> + push @$res, $id if !$self->{negate};
> } elsif ($magic_ct && $magic_ct =~ m|$self->{field_value}|) {
> - push @$res, $id;
> + push @$res, $id if !$self->{negate};
> } elsif ($glob_ct && $glob_ct =~ m|$self->{field_value}|) {
> + push @$res, $id if !$self->{negate};
> + } elsif ($self->{negate}) {
> push @$res, $id;
> } else {
> # match inside archives
> diff --git a/src/PMG/RuleDB/ContentTypeFilter.pm b/src/PMG/RuleDB/ContentTypeFilter.pm
> index 76fc1ce..eb35292 100755
> --- a/src/PMG/RuleDB/ContentTypeFilter.pm
> +++ b/src/PMG/RuleDB/ContentTypeFilter.pm
> @@ -72,10 +72,12 @@ sub parse_entity {
> my $glob_ct = $entity->{PMX_glob_ct};
>
> if ($header_ct && $header_ct =~ m|$self->{field_value}|) {
> - push @$res, $id;
> + push @$res, $id if !$self->{negate};
> } elsif ($magic_ct && $magic_ct =~ m|$self->{field_value}|) {
> - push @$res, $id;
> + push @$res, $id if !$self->{negate};
> } elsif ($glob_ct && $glob_ct =~ m|$self->{field_value}|) {
> + push @$res, $id if !$self->{negate};
> + } elsif ($self->{negate}) {
> push @$res, $id;
> }
> }
> diff --git a/src/PMG/RuleDB/MatchArchiveFilename.pm b/src/PMG/RuleDB/MatchArchiveFilename.pm
> index 2ef3543..8abd592 100644
> --- a/src/PMG/RuleDB/MatchArchiveFilename.pm
> +++ b/src/PMG/RuleDB/MatchArchiveFilename.pm
> @@ -29,12 +29,12 @@ sub parse_entity {
> chomp $id;
>
> my $fn = PMG::Utils::extract_filename($entity->head);
> - if (defined($fn) && $fn =~ m|^$self->{fname}$|i) {
> + if (defined($fn) && ($fn =~ m|^$self->{fname}$|i xor $self->{negate})) {
> push @$res, $id;
> } elsif (my $filenames = $entity->{PMX_filenames}) {
> # Match inside archives
> for my $fn (keys %$filenames) {
> - if ($fn =~ m|^$self->{fname}$|i) {
> + if ($fn =~ m|^$self->{fname}$|i xor $self->{negate}) {
> push @$res, $id;
> last;
> }
> diff --git a/src/PMG/RuleDB/MatchField.pm b/src/PMG/RuleDB/MatchField.pm
> index 2b56058..8f89297 100644
> --- a/src/PMG/RuleDB/MatchField.pm
> +++ b/src/PMG/RuleDB/MatchField.pm
> @@ -111,7 +111,7 @@ sub parse_entity {
> my $decvalue = PMG::Utils::decode_rfc1522($value);
> $decvalue = PMG::Utils::try_decode_utf8($decvalue);
>
> - if ($decvalue =~ m|$self->{field_value}|i) {
> + if (($decvalue =~ m|$self->{field_value}|i) xor $self->{negate}) {
> push @$res, $id;
> }
> }
> diff --git a/src/PMG/RuleDB/MatchFilename.pm b/src/PMG/RuleDB/MatchFilename.pm
> index c9cdbe0..0665efc 100644
> --- a/src/PMG/RuleDB/MatchFilename.pm
> +++ b/src/PMG/RuleDB/MatchFilename.pm
> @@ -91,7 +91,7 @@ sub parse_entity {
> chomp $id;
>
> if (my $value = PMG::Utils::extract_filename($entity->head)) {
> - if ($value =~ m|^$self->{fname}$|i) {
> + if (($value =~ m|^$self->{fname}$|i) xor $self->{negate}) {
> push @$res, $id;
> }
> }
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [pmg-devel] [PATCH pmg-api/gui/docs, proxmox-widget-toolkit] Extend rule system
2023-04-07 13:42 [pmg-devel] [PATCH pmg-api/gui/docs, proxmox-widget-toolkit] Extend rule system Leo Nunner
` (11 preceding siblings ...)
2023-04-07 13:42 ` [pmg-devel] [PATCH widget-toolkit] dark-mode: fix colour of default tree icons Leo Nunner
@ 2023-04-11 9:52 ` Thomas Lamprecht
2023-04-11 11:04 ` Leo Nunner
12 siblings, 1 reply; 17+ messages in thread
From: Thomas Lamprecht @ 2023-04-11 9:52 UTC (permalink / raw)
To: Leo Nunner, pmg-devel
For now some higher level review inline from my side, not sure if I get to
a more in-depth for each patch soonish.
Am 07/04/2023 um 15:42 schrieb Leo Nunner:
> This series introduces two new features for the PMG rule system: object
> negation, and match groups. Negation allows the user to negate objects
> inside a rule, meaning that they will evaluate to true if their
> condition is NOT fulfilled.
Do we really don't have any test system for the rule system, if that'd be some
technical debt to address probably rather sooner than later.
>
> The second feature, match groups, allows objects to be chained together.
> A match group functions as a container for multiple other objects; it
> will only evaluate to true if all its members evaluate to true. Match groups
> are connected via logical 'or' to all other objects inside the rule;
> take the following rule system:
>
> - Rule
> - Match Group
> - Object 1
> - Object 2
> - Object 3
>
> It will match if either (Object 1 && Object 2), or if Object 3
> evaluate to true.
Why not allow or matches? Most mail filter tools (like e.g., the x-sieve
fronted of open-xchange) allow to configure if all or any of a group's matching
rules must trigger for the whole group to be true.
I mean, OR matching can be workarounded by duplicated rules but if we add
groups with AND combination, then OR is something that I'd think is also
expected to be there.
>
> To properly achieve match groups inside the GUI, the rule overview was
> reworked as a tree, instead of a simple list. It can now be
> collapsed/expanded, and each object type (except actions) has a single
> subfolder called 'Match Group'.
>
> In combination, these features should cover quite a few use cases, and
> make it possible to model more complex rule systems.
>
> pmg-api:
>
> Leo Nunner (8):
can we please drop the weird feature prefix, doesn't really add anything and is
relatively hard to parse as the subsystem is missing – PMG is somewhat small
enough in scope, so one can guess that; but I'd rather have that explicit.
Also, order and separation is a bit odd as some things that are implementing a
single semantic work are split, and others like the api expand+add are merged,
also making the whole thing work only after exposing it via the API just feels
plain wrong and is actually risky if someone overlooks this and applies the
first few patches already.
So, order/separation could be IMO better if:
commit 2 ("parse negation value into objects"), and 4 ("implement matching
logic") are squashed into a "rule system: implement match negation" as they
implement the internal/backend functionallity, and then exposing that via the
api could be done in the next commit. The one adding the field to the rule db
schema could be also squashed into the commit 2 & 4, but less hard feelings
there, albeit I'd note in the commit message that it's a preparation for the
next commits.
> feature: negation: add field to database
if not squashed:
rule db: add negation field
> feature: negation: parse negation value into objects
would always squash
> feature: negation: expand/implement API endpoints
That one could be actually split, the part that extends the existing API could
be squashed and the addition of the new "get/set object group" is rather
unrelated and something different, so should probably be it's own commit, i.e.
with a subject like:
api: rules: make object group settings editable
or:
api: rule group: make existing rules updateable
> feature: negation: implement matching logic
needs to order before exposing it via the API and should be suqashed
> feature: match groups: add field to database
> feature: match groups: parse field into objects
> feature: match groups: update API endpoints
> feature: match groups: implement matching logic
same for above w.r.t. s/feature/<actual subsys>/ for the commit subject and
order/sepration.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [pmg-devel] [PATCH pmg-api/gui/docs, proxmox-widget-toolkit] Extend rule system
2023-04-11 9:52 ` [pmg-devel] [PATCH pmg-api/gui/docs, proxmox-widget-toolkit] Extend rule system Thomas Lamprecht
@ 2023-04-11 11:04 ` Leo Nunner
2023-04-11 11:19 ` Thomas Lamprecht
0 siblings, 1 reply; 17+ messages in thread
From: Leo Nunner @ 2023-04-11 11:04 UTC (permalink / raw)
To: Thomas Lamprecht, pmg-devel
On 2023-04-11 11:52, Thomas Lamprecht wrote:
> For now some higher level review inline from my side, not sure if I get to
> a more in-depth for each patch soonish.
>
> Am 07/04/2023 um 15:42 schrieb Leo Nunner:
>> This series introduces two new features for the PMG rule system: object
>> negation, and match groups. Negation allows the user to negate objects
>> inside a rule, meaning that they will evaluate to true if their
>> condition is NOT fulfilled.
> Do we really don't have any test system for the rule system, if that'd be some
> technical debt to address probably rather sooner than later.
We have regression tests for PMG, which should already cover this AFAIK.
>> The second feature, match groups, allows objects to be chained together.
>> A match group functions as a container for multiple other objects; it
>> will only evaluate to true if all its members evaluate to true. Match groups
>> are connected via logical 'or' to all other objects inside the rule;
>> take the following rule system:
>>
>> - Rule
>> - Match Group
>> - Object 1
>> - Object 2
>> - Object 3
>>
>> It will match if either (Object 1 && Object 2), or if Object 3
>> evaluate to true.
> Why not allow or matches? Most mail filter tools (like e.g., the x-sieve
> fronted of open-xchange) allow to configure if all or any of a group's matching
> rules must trigger for the whole group to be true.
>
> I mean, OR matching can be workarounded by duplicated rules but if we add
> groups with AND combination, then OR is something that I'd think is also
> expected to be there.
Not sure if I follow…
- Rule
- Who Objects
- Object 1
- When Objects
- Object 2
You would want something like this to match if either Who *or* When
produce a match? IIRC those are connected via AND right now.
>> To properly achieve match groups inside the GUI, the rule overview was
>> reworked as a tree, instead of a simple list. It can now be
>> collapsed/expanded, and each object type (except actions) has a single
>> subfolder called 'Match Group'.
>>
>> In combination, these features should cover quite a few use cases, and
>> make it possible to model more complex rule systems.
>>
>> pmg-api:
>>
>> Leo Nunner (8):
> can we please drop the weird feature prefix, doesn't really add anything and is
> relatively hard to parse as the subsystem is missing – PMG is somewhat small
> enough in scope, so one can guess that; but I'd rather have that explicit.
>
> Also, order and separation is a bit odd as some things that are implementing a
> single semantic work are split, and others like the api expand+add are merged,
> also making the whole thing work only after exposing it via the API just feels
> plain wrong and is actually risky if someone overlooks this and applies the
> first few patches already.
>
> So, order/separation could be IMO better if:
>
> commit 2 ("parse negation value into objects"), and 4 ("implement matching
> logic") are squashed into a "rule system: implement match negation" as they
> implement the internal/backend functionallity, and then exposing that via the
> api could be done in the next commit. The one adding the field to the rule db
> schema could be also squashed into the commit 2 & 4, but less hard feelings
> there, albeit I'd note in the commit message that it's a preparation for the
> next commits.
>
> […]
I'll change the commits around accordingly (and keep the formatting in
mind for future patches), thanks!
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [pmg-devel] [PATCH pmg-api/gui/docs, proxmox-widget-toolkit] Extend rule system
2023-04-11 11:04 ` Leo Nunner
@ 2023-04-11 11:19 ` Thomas Lamprecht
0 siblings, 0 replies; 17+ messages in thread
From: Thomas Lamprecht @ 2023-04-11 11:19 UTC (permalink / raw)
To: Leo Nunner, pmg-devel
Am 11/04/2023 um 13:04 schrieb Leo Nunner:
> On 2023-04-11 11:52, Thomas Lamprecht wrote:
>> For now some higher level review inline from my side, not sure if I get to
>> a more in-depth for each patch soonish.
>>
>> Am 07/04/2023 um 15:42 schrieb Leo Nunner:
>>> This series introduces two new features for the PMG rule system: object
>>> negation, and match groups. Negation allows the user to negate objects
>>> inside a rule, meaning that they will evaluate to true if their
>>> condition is NOT fulfilled.
>> Do we really don't have any test system for the rule system, if that'd be some
>> technical debt to address probably rather sooner than later.
> We have regression tests for PMG, which should already cover this AFAIK.
Yeah I know of those regression tests, but a lot of the rule system would
not require a (postgres) DB to be spun up, but could often run on a higher
level, e.g., by mocking the higher level methods; iow. more like integrated
test that, while covering a smaller/more specific surface area, can be hosted
in the pmg-api repo directly and run on every package build, vs. the more
complex integration regression test suite that also contains some possibly
sensible material. I mean, the existing regression tests are half-way between
some suite that tests explicitly the external API/mail-transports endpoints only
and some internal testing done.
Also, you nobody mention that you have added, or will add, explicit tests for
the new features ;-)
>>> The second feature, match groups, allows objects to be chained together.
>>> A match group functions as a container for multiple other objects; it
>>> will only evaluate to true if all its members evaluate to true. Match groups
>>> are connected via logical 'or' to all other objects inside the rule;
>>> take the following rule system:
>>>
>>> - Rule
>>> - Match Group
>>> - Object 1
>>> - Object 2
>>> - Object 3
>>>
>>> It will match if either (Object 1 && Object 2), or if Object 3
>>> evaluate to true.
>> Why not allow or matches? Most mail filter tools (like e.g., the x-sieve
>> fronted of open-xchange) allow to configure if all or any of a group's matching
>> rules must trigger for the whole group to be true.
>>
>> I mean, OR matching can be workarounded by duplicated rules but if we add
>> groups with AND combination, then OR is something that I'd think is also
>> expected to be there.
> Not sure if I follow…
>
> - Rule
> - Who Objects
> - Object 1
> - When Objects
> - Object 2
>
> You would want something like this to match if either Who *or* When
> produce a match? IIRC those are connected via AND right now.
I hab more a switch for the "Match Group" in mind, i.e., to configure if it
should act as and, or as or. But, tbf, I did not worked with the PMG rule
system that closely in recent times – maybe I'm just missing something here.
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2023-04-11 11:20 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-07 13:42 [pmg-devel] [PATCH pmg-api/gui/docs, proxmox-widget-toolkit] Extend rule system Leo Nunner
2023-04-07 13:42 ` [pmg-devel] [PATCH pmg-api 1/8] feature: negation: add field to database Leo Nunner
2023-04-07 13:42 ` [pmg-devel] [PATCH pmg-api 2/8] feature: negation: parse negation value into objects Leo Nunner
2023-04-07 13:42 ` [pmg-devel] [PATCH pmg-api 3/8] feature: negation: expand/implement API endpoints Leo Nunner
2023-04-07 13:42 ` [pmg-devel] [PATCH pmg-api 4/8] feature: negation: implement matching logic Leo Nunner
2023-04-11 7:35 ` Leo Nunner
2023-04-07 13:42 ` [pmg-devel] [PATCH pmg-api 5/8] feature: match groups: add field to database Leo Nunner
2023-04-07 13:42 ` [pmg-devel] [PATCH pmg-api 6/8] feature: match groups: parse field into objects Leo Nunner
2023-04-07 13:42 ` [pmg-devel] [PATCH pmg-api 7/8] feature: match groups: update API endpoints Leo Nunner
2023-04-07 13:42 ` [pmg-devel] [PATCH pmg-api 8/8] feature: match groups: implement matching logic Leo Nunner
2023-04-07 13:42 ` [pmg-devel] [PATCH pmg-gui 1/2] feature: negate objects inside rules Leo Nunner
2023-04-07 13:42 ` [pmg-devel] [PATCH pmg-gui 2/2] feature: introduce logical 'and' for rules Leo Nunner
2023-04-07 13:42 ` [pmg-devel] [PATCH pmg-docs] docs: document negation and match groups Leo Nunner
2023-04-07 13:42 ` [pmg-devel] [PATCH widget-toolkit] dark-mode: fix colour of default tree icons Leo Nunner
2023-04-11 9:52 ` [pmg-devel] [PATCH pmg-api/gui/docs, proxmox-widget-toolkit] Extend rule system Thomas Lamprecht
2023-04-11 11:04 ` Leo Nunner
2023-04-11 11:19 ` Thomas Lamprecht
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox