* [pmg-devel] [PATCH pmg-api v2 01/10] RuleCache: reorganize how we gather marks and spaminfo
2024-02-21 12:24 [pmg-devel] [PATCH pmg-api/docs/gui v2] implement and combination and inversion of groups and objects Dominik Csapak
@ 2024-02-21 12:24 ` Dominik Csapak
2024-02-21 12:24 ` [pmg-devel] [PATCH pmg-api v2 02/10] api: refactor rule parameters Dominik Csapak
` (12 subsequent siblings)
13 siblings, 0 replies; 20+ messages in thread
From: Dominik Csapak @ 2024-02-21 12:24 UTC (permalink / raw)
To: pmg-devel
currently, we gather the marks and the spaminfo separately,
with the following properties:
* both are gathered with what matches
* virus and spam matches are global (i.e. not marks but [] to mark the
whole mail)
* only the spam match can differ per user (wl/bl)
* all matches are or'ed (so order and global/per part does not make much
of a difference for matching the rule)
* marks are saved once globally per rule and per target
* spaminfo is saved per target
instead collect spaminfo once and marks per target together. With this,
we can omit the 'global' marks list, since each target has their own
anyway. Also saves the spaminfo only once, since that must be the same
for the mail regardless of target.
Since that shouldn't change the current matching behavior (all marks
are identical for the whole rule), we can simply use the marks from the
first target in `Remove` (the only action where the marks are relevant).
This makes it easier for us when we'll implement and/invert for matches,
as the marks can differ between targets. That is, because the spamlevel
can diverge for them and that can be and-combined with objects that add
marks.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
changes from v1:
* improved commit message
* return marks and spaminfo seperately, this reduces some nesting of objects
* improve/remove outdated comments
src/PMG/RuleCache.pm | 43 +++++++++++++---------------------------
src/PMG/RuleDB/Remove.pm | 18 +++++++++++++----
src/bin/pmg-smtp-filter | 20 +++++++------------
3 files changed, 35 insertions(+), 46 deletions(-)
diff --git a/src/PMG/RuleCache.pm b/src/PMG/RuleCache.pm
index fd22a16..cd56342 100644
--- a/src/PMG/RuleCache.pm
+++ b/src/PMG/RuleCache.pm
@@ -295,53 +295,38 @@ sub what_match {
my $what = $self->{"$ruleid:what"};
- my $res;
-
- # $res->{marks} is used by mark specific actions like remove-attachments
- # $res->{$target}->{marks} is only used in apply_rules() to exclude some
- # targets (spam blacklist and whitelist)
+ my $marks;
+ my $spaminfo;
if (scalar($what->{groups}->@*) == 0) {
# match all targets
foreach my $target (@{$msginfo->{targets}}) {
- $res->{$target}->{marks} = [];
+ $marks->{$target} = [];
}
-
- $res->{marks} = [];
- return $res;
+ return ($marks, $spaminfo);
}
- my $marks;
-
for my $group ($what->{groups}->@*) {
for my $obj ($group->{objects}->@*) {
if (!$obj->can('what_match_targets')) {
if (my $match = $obj->what_match($queue, $element, $msginfo, $dbh)) {
- push @$marks, @$match;
+ for my $target ($msginfo->{targets}->@*) {
+ push $marks->{$target}->@*, $match->@*;
+ }
}
- }
- }
- }
-
- foreach my $target (@{$msginfo->{targets}}) {
- $res->{$target}->{marks} = $marks;
- $res->{marks} = $marks;
- }
-
- for my $group ($what->{groups}->@*) {
- for my $obj ($group->{objects}->@*) {
- if ($obj->can ("what_match_targets")) {
- my $target_info;
- if ($target_info = $obj->what_match_targets($queue, $element, $msginfo, $dbh)) {
- foreach my $k (keys %$target_info) {
- $res->{$k} = $target_info->{$k};
+ } else {
+ if (my $target_info = $obj->what_match_targets($queue, $element, $msginfo, $dbh)) {
+ foreach my $k (keys $target_info->%*) {
+ push $marks->{$k}->@*, $target_info->{$k}->{marks}->@*;
+ # only save spaminfo once
+ $spaminfo = $target_info->{$k}->{spaminfo} if !defined($spaminfo);
}
}
}
}
}
- return $res;
+ return ($marks, $spaminfo);
}
1;
diff --git a/src/PMG/RuleDB/Remove.pm b/src/PMG/RuleDB/Remove.pm
index e7c353c..3acc861 100644
--- a/src/PMG/RuleDB/Remove.pm
+++ b/src/PMG/RuleDB/Remove.pm
@@ -198,9 +198,15 @@ sub execute {
my $rulename = encode('UTF-8', $vars->{RULE} // 'unknown');
- if (!$self->{all} && ($#$marks == -1)) {
- # no marks
- return;
+ if (!$self->{all}) {
+ my $found_mark = 0;
+ for my $target (keys $marks->%*) {
+ if (scalar($marks->{$target}->@*) > 0) {
+ $found_mark = 1;
+ last;
+ }
+ }
+ return if !$found_mark;
}
my $subgroups = $mod_group->subgroups ($targets);
@@ -256,7 +262,11 @@ sub execute {
}
$self->{message_seen} = 0;
- $self->delete_marked_parts($queue, $entity, $html, $rtype, $marks, $rulename);
+
+ # since currently all marks are equal for all target, just use the first one
+ my $match_marks = $marks->{$tg->[0]};
+
+ $self->delete_marked_parts($queue, $entity, $html, $rtype, $match_marks, $rulename);
delete $self->{message_seen};
if ($msginfo->{testmode}) {
diff --git a/src/bin/pmg-smtp-filter b/src/bin/pmg-smtp-filter
index 7da3de8..cc2fcd6 100755
--- a/src/bin/pmg-smtp-filter
+++ b/src/bin/pmg-smtp-filter
@@ -231,6 +231,7 @@ sub apply_rules {
my %rule_targets;
my %rule_actions;
my %rule_marks;
+ my %rule_spaminfo;
my $matching_rules = [];
my $rulecache = $self->{rulecache};
@@ -266,9 +267,12 @@ sub apply_rules {
next;
}
- $rule_marks{$rule->{id}} =
+ my ($marks, $spaminfo) =
$rulecache->what_match ($rule->{id}, $queue, $entity, $msginfo, $dbh);
+ $rule_marks{$rule->{id}} = $marks;
+ $rule_spaminfo{$rule->{id}} = $spaminfo;
+
$rule_actions{$rule->{id}} = $rulecache->get_actions ($rule->{id});
my $fin = $rulecache->final ($rule->{id});
@@ -277,7 +281,6 @@ sub apply_rules {
next if $final->{$target};
next if !defined ($rule_marks{$rule->{id}});
next if !defined ($rule_marks{$rule->{id}}->{$target});
- next if !defined ($rule_marks{$rule->{id}}->{$target}->{marks});
next if !$rulecache->to_match ($rule->{id}, $target, $ldap);
$final->{$target} = $fin;
@@ -320,24 +323,15 @@ sub apply_rules {
my $targets = $rule_targets{$rule->{id}};
next if !$targets;
- my $spaminfo;
- foreach my $t (@$targets) {
- if ($rule_marks{$rule->{id}}->{$t} && $rule_marks{$rule->{id}}->{$t}->{spaminfo}) {
- $spaminfo = $rule_marks{$rule->{id}}->{$t}->{spaminfo};
- # we assume spam info is the same for all matching targets
- last;
- }
- }
-
my $vars = $self->get_prox_vars (
- $queue, $entity, $msginfo, $rule, $rule_targets{$rule->{id}}, $spaminfo);
+ $queue, $entity, $msginfo, $rule, $rule_targets{$rule->{id}}, $rule_spaminfo{$rule->{id}});
my @sorted_actions = sort {$a->priority <=> $b->priority} @{$rule_actions{$rule->{id}}};
foreach my $action (@sorted_actions) {
$action->execute(
$queue, $self->{ruledb}, $mod_group, $rule_targets{$rule->{id}}, $msginfo, $vars,
- $rule_marks{$rule->{id}}->{marks}, $ldap
+ $rule_marks{$rule->{id}}, $ldap
);
last if $action->final;
}
--
2.30.2
^ permalink raw reply [flat|nested] 20+ messages in thread* [pmg-devel] [PATCH pmg-api v2 02/10] api: refactor rule parameters
2024-02-21 12:24 [pmg-devel] [PATCH pmg-api/docs/gui v2] implement and combination and inversion of groups and objects Dominik Csapak
2024-02-21 12:24 ` [pmg-devel] [PATCH pmg-api v2 01/10] RuleCache: reorganize how we gather marks and spaminfo Dominik Csapak
@ 2024-02-21 12:24 ` Dominik Csapak
2024-02-21 12:24 ` [pmg-devel] [PATCH pmg-api v2 03/10] add objectgroup attributes and/invert Dominik Csapak
` (11 subsequent siblings)
13 siblings, 0 replies; 20+ messages in thread
From: Dominik Csapak @ 2024-02-21 12:24 UTC (permalink / raw)
To: pmg-devel
makes it easier to add new ones
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
changes from v1:
* don't give active/direction on initial Rule->new
src/PMG/API2/RuleDB.pm | 23 +++++++----------------
src/PMG/API2/Rules.pm | 41 +++++++++++++++++++++++++++--------------
2 files changed, 34 insertions(+), 30 deletions(-)
diff --git a/src/PMG/API2/RuleDB.pm b/src/PMG/API2/RuleDB.pm
index 1fddb32..6d27b14 100644
--- a/src/PMG/API2/RuleDB.pm
+++ b/src/PMG/API2/RuleDB.pm
@@ -162,7 +162,7 @@ __PACKAGE__->register_method({
permissions => { check => [ 'admin' ] },
parameters => {
additionalProperties => 0,
- properties => {
+ properties => PMG::API2::Rules::get_rule_params({
name => {
description => "Rule name",
type => 'string',
@@ -173,19 +173,7 @@ __PACKAGE__->register_method({
minimum => 0,
maximum => 100,
},
- direction => {
- description => "Rule direction. Value `0` matches incoming mails, value `1` matches outgoing mails, and value `2` matches both directions.",
- type => 'integer',
- minimum => 0,
- maximum => 2,
- optional => 1,
- },
- active => {
- description => "Flag to activate rule.",
- type => 'boolean',
- optional => 1,
- },
- },
+ }),
},
returns => { type => 'integer' },
code => sub {
@@ -193,8 +181,11 @@ __PACKAGE__->register_method({
my $rdb = PMG::RuleDB->new();
- my $rule = PMG::RuleDB::Rule->new (
- $param->{name}, $param->{priority}, $param->{active}, $param->{direction});
+ my $rule = PMG::RuleDB::Rule->new ($param->{name}, $param->{priority});
+
+ for my $key (keys PMG::API2::Rules::get_rule_params()->%*) {
+ $rule->{$key} = $param->{$key} if defined($param->{$key});
+ }
return $rdb->save_rule($rule);
}});
diff --git a/src/PMG/API2/Rules.pm b/src/PMG/API2/Rules.pm
index 4f8c10b..c48370f 100644
--- a/src/PMG/API2/Rules.pm
+++ b/src/PMG/API2/Rules.pm
@@ -136,6 +136,31 @@ __PACKAGE__->register_method ({
return $data;
}});
+my $rule_params = {
+ direction => {
+ description => "Rule direction. Value `0` matches incoming mails, value `1` matches outgoing mails, and value `2` matches both directions.",
+ type => 'integer',
+ minimum => 0,
+ maximum => 2,
+ optional => 1,
+ },
+ active => {
+ description => "Flag to activate rule.",
+ type => 'boolean',
+ optional => 1,
+ },
+};
+
+sub get_rule_params {
+ my ($base) = @_;
+ $base //= {};
+ return {
+ $base->%*,
+ $rule_params->%*
+ };
+}
+
+
__PACKAGE__->register_method ({
name => 'update_config',
path => 'config',
@@ -146,7 +171,7 @@ __PACKAGE__->register_method ({
permissions => { check => [ 'admin' ] },
parameters => {
additionalProperties => 0,
- properties => {
+ properties => get_rule_params({
id => {
description => "Rule ID.",
type => 'integer',
@@ -156,18 +181,6 @@ __PACKAGE__->register_method ({
type => 'string',
optional => 1,
},
- active => {
- description => "Flag to activate rule.",
- type => 'boolean',
- optional => 1,
- },
- direction => {
- description => "Rule direction. Value `0` matches incoming mails, value `1` matches outgoing mails, and value `2` matches both directions.",
- type => 'integer',
- minimum => 0,
- maximum => 2,
- optional => 1,
- },
priority => {
description => "Rule priotity.",
type => 'integer',
@@ -175,7 +188,7 @@ __PACKAGE__->register_method ({
maximum => 100,
optional => 1,
},
- },
+ }),
},
returns => { type => "null" },
code => sub {
--
2.30.2
^ permalink raw reply [flat|nested] 20+ messages in thread* [pmg-devel] [PATCH pmg-api v2 03/10] add objectgroup attributes and/invert
2024-02-21 12:24 [pmg-devel] [PATCH pmg-api/docs/gui v2] implement and combination and inversion of groups and objects Dominik Csapak
2024-02-21 12:24 ` [pmg-devel] [PATCH pmg-api v2 01/10] RuleCache: reorganize how we gather marks and spaminfo Dominik Csapak
2024-02-21 12:24 ` [pmg-devel] [PATCH pmg-api v2 02/10] api: refactor rule parameters Dominik Csapak
@ 2024-02-21 12:24 ` Dominik Csapak
2024-02-21 12:24 ` [pmg-devel] [PATCH pmg-api v2 04/10] add rule attributes and/invert (for each relevant type) Dominik Csapak
` (10 subsequent siblings)
13 siblings, 0 replies; 20+ messages in thread
From: Dominik Csapak @ 2024-02-21 12:24 UTC (permalink / raw)
To: pmg-devel
add a new table Objectgroup_Attributes where we can save additional
attributes for objectgroups (like the Attribut tables for objects).
Adds two new attributes for the groups:
* and
* invert
These will modify the match behaviour for object groups
Add the table to cluster sync, backup and factory reset.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
changes from v1:
* delete attributes on group delete
* add handling to cluster sync, backup and factory reset too
src/PMG/API2/ObjectGroupHelpers.pm | 43 ++++++++-
src/PMG/Backup.pm | 1 +
src/PMG/Cluster.pm | 2 +
src/PMG/DBTools.pm | 16 ++++
src/PMG/RuleDB.pm | 148 ++++++++++++++++++++++-------
5 files changed, 169 insertions(+), 41 deletions(-)
diff --git a/src/PMG/API2/ObjectGroupHelpers.pm b/src/PMG/API2/ObjectGroupHelpers.pm
index 48078fb..a08a6a3 100644
--- a/src/PMG/API2/ObjectGroupHelpers.pm
+++ b/src/PMG/API2/ObjectGroupHelpers.pm
@@ -46,13 +46,29 @@ sub format_object_group {
my $res = [];
foreach my $og (@$ogroups) {
- push @$res, {
- id => $og->{id}, name => $og->{name}, info => $og->{info}
- };
+ my $group = { id => $og->{id}, name => $og->{name}, info => $og->{info} };
+ $group->{and} = $og->{and} if defined($og->{and});
+ $group->{invert} = $og->{invert} if defined($og->{invert});
+ push @$res, $group;
}
return $res;
}
+my $group_attributes = {
+ and => {
+ description => "If set to 1, objects in this group are 'and' combined.",
+ type => 'boolean',
+ default => 0,
+ optional => 1,
+ },
+ invert => {
+ description => "If set to 1, the resulting match is inverted.",
+ type => 'boolean',
+ default => 0,
+ optional => 1,
+ },
+};
+
sub register_group_list_api {
my ($apiclass, $oclass) = @_;
@@ -86,6 +102,11 @@ sub register_group_list_api {
return format_object_group($ogroups);
}});
+ my $additional_parameters = {};
+ if ($oclass =~ /^(?:what|when|who)$/i) {
+ $additional_parameters = { $group_attributes->%* };
+ }
+
$apiclass->register_method({
name => "create_${oclass}_group",
path => $oclass,
@@ -108,6 +129,7 @@ sub register_group_list_api {
maxLength => 255,
optional => 1,
},
+ $additional_parameters->%*,
},
},
returns => { type => 'integer' },
@@ -119,6 +141,10 @@ sub register_group_list_api {
my $og = PMG::RuleDB::Group->new(
$param->{name}, $param->{info} // '', $oclass);
+ for my $prop (qw(and invert)) {
+ $og->{$prop} = $param->{$prop} if defined($param->{$prop});
+ }
+
return $rdb->save_group($og);
}});
}
@@ -199,6 +225,11 @@ sub register_object_group_config_api {
}});
+ my $additional_parameters = {};
+ if ($oclass =~ /^(?:what|when|who)$/i) {
+ $additional_parameters = { $group_attributes->%* };
+ }
+
$apiclass->register_method({
name => 'set_config',
path => $path,
@@ -226,6 +257,7 @@ sub register_object_group_config_api {
maxLength => 255,
optional => 1,
},
+ $additional_parameters->%*,
},
},
returns => { type => "null" },
@@ -243,8 +275,9 @@ sub register_object_group_config_api {
my $og = shift @$list ||
die "$oclass group '$ogroup' not found\n";
- $og->{name} = $param->{name} if defined($param->{name});
- $og->{info} = $param->{info} if defined($param->{info});
+ for my $prop (qw(name info and invert)) {
+ $og->{$prop} = $param->{$prop} if defined($param->{$prop});
+ }
$rdb->save_group($og);
diff --git a/src/PMG/Backup.pm b/src/PMG/Backup.pm
index e41832e..9fc91f8 100644
--- a/src/PMG/Backup.pm
+++ b/src/PMG/Backup.pm
@@ -94,6 +94,7 @@ sub dumpdb {
$dbh->do("SET TRANSACTION ISOLATION LEVEL SERIALIZABLE");
dump_table($dbh, 'attribut', $ofh);
+ dump_table($dbh, 'objectgroup_attributes', $ofh);
dump_table($dbh, 'object', $ofh, 'object_id_seq', 'id');
dump_table($dbh, 'objectgroup', $ofh, 'objectgroup_id_seq', 'id');
dump_table($dbh, 'rule', $ofh, 'rule_id_seq', 'id');
diff --git a/src/PMG/Cluster.pm b/src/PMG/Cluster.pm
index 015e66a..ac50cff 100644
--- a/src/PMG/Cluster.pm
+++ b/src/PMG/Cluster.pm
@@ -532,6 +532,7 @@ sub sync_ruledb_from_master {
$ldb->do("DELETE FROM ObjectGroup");
$ldb->do("DELETE FROM Object");
$ldb->do("DELETE FROM Attribut");
+ $ldb->do("DELETE FROM Objectgroup_Attributes");
eval {
$rdb->begin_work;
@@ -544,6 +545,7 @@ sub sync_ruledb_from_master {
PMG::DBTools::copy_table($ldb, $rdb, "ObjectGroup");
PMG::DBTools::copy_table($ldb, $rdb, "Object", 'value');
PMG::DBTools::copy_table($ldb, $rdb, "Attribut", 'value');
+ PMG::DBTools::copy_table($ldb, $rdb, "Objectgroup_Attributes");
};
$rdb->rollback; # end transaction
diff --git a/src/PMG/DBTools.pm b/src/PMG/DBTools.pm
index 9e133bc..3c8d181 100644
--- a/src/PMG/DBTools.pm
+++ b/src/PMG/DBTools.pm
@@ -295,6 +295,18 @@ my $userprefs_ctablecmd = <<__EOD;
__EOD
+my $object_group_attributes_cmd = <<__EOD;
+ CREATE TABLE Objectgroup_Attributes (
+ Objectgroup_ID INTEGER NOT NULL,
+ Name VARCHAR(20) NOT NULL,
+ Value BYTEA NULL,
+ PRIMARY KEY (Objectgroup_ID, Name)
+ );
+
+ CREATE INDEX Objectgroup_Attributes_Objectgroup_ID_Index ON Objectgroup_Attributes(Objectgroup_ID);
+
+__EOD
+
sub cond_create_dbtable {
my ($dbh, $name, $ctablecmd) = @_;
@@ -439,6 +451,8 @@ sub create_ruledb {
$userprefs_ctablecmd;
$virusinfo_stat_ctablecmd;
+
+ $object_group_attributes_cmd;
EOD
);
@@ -494,6 +508,7 @@ sub upgradedb {
'CStatistic', $cstatistic_ctablecmd,
'ClusterInfo', $clusterinfo_ctablecmd,
'VirusInfo', $virusinfo_stat_ctablecmd,
+ 'Objectgroup_Attributes', $object_group_attributes_cmd,
};
foreach my $table (keys %$tables) {
@@ -605,6 +620,7 @@ sub init_ruledb {
$dbh->do(
"DELETE FROM Rule;"
." DELETE FROM RuleGroup;"
+ ." DELETE FROM Objectgroup_Attributes;"
." DELETE FROM Attribut WHERE Object_ID NOT IN ($glids);"
." DELETE FROM Object WHERE ID NOT IN ($glids);"
." DELETE FROM Objectgroup WHERE class != 'greylist';"
diff --git a/src/PMG/RuleDB.pm b/src/PMG/RuleDB.pm
index a6b0b79..0b112b4 100644
--- a/src/PMG/RuleDB.pm
+++ b/src/PMG/RuleDB.pm
@@ -160,6 +160,30 @@ sub load_groups_by_name {
};
}
+sub update_group_attributes {
+ my ($self, $og) = @_;
+
+ my $attributes = [qw(and invert)];
+
+ for my $attribute ($attributes->@*) {
+ # only save the values if they're set to 1
+ if ($og->{$attribute}) {
+ $self->{dbh}->do(
+ "INSERT INTO Objectgroup_Attributes (Objectgroup_ID, Name, Value) " .
+ "VALUES (?, ?, ?) ".
+ "ON CONFLICT (Objectgroup_ID, Name) DO UPDATE SET Value = ?", undef,
+ $og->{id}, $attribute, $og->{$attribute}, $og->{$attribute},
+ );
+ } else {
+ $self->{dbh}->do(
+ "DELETE FROM Objectgroup_Attributes " .
+ "WHERE Objectgroup_ID = ? AND Name = ?", undef,
+ $og->{id}, $attribute,
+ );
+ }
+ }
+}
+
sub save_group {
my ($self, $og) = @_;
@@ -171,27 +195,51 @@ sub save_group {
die "undefined group attribute - class: ERROR";
if (defined($og->{id})) {
+ $self->{dbh}->begin_work;
+
+ eval {
+ $self->{dbh}->do("UPDATE Objectgroup " .
+ "SET Name = ?, Info = ? " .
+ "WHERE ID = ?", undef,
+ encode('UTF-8', $og->{name}),
+ encode('UTF-8', $og->{info}),
+ $og->{id});
- $self->{dbh}->do("UPDATE Objectgroup " .
- "SET Name = ?, Info = ? " .
- "WHERE ID = ?", undef,
- encode('UTF-8', $og->{name}),
- encode('UTF-8', $og->{info}),
- $og->{id});
+ $self->update_group_attributes($og);
- return $og->{id};
+ $self->{dbh}->commit;
+ };
+ if (my $err = $@) {
+ $self->{dbh}->rollback;
+ syslog('err', $err);
+ return undef;
+ }
} else {
- my $sth = $self->{dbh}->prepare(
- "INSERT INTO Objectgroup (Name, Info, Class) " .
- "VALUES (?, ?, ?);");
+ $self->{dbh}->begin_work;
+
+ eval {
+ my $sth = $self->{dbh}->prepare(
+ "INSERT INTO Objectgroup (Name, Info, Class) " .
+ "VALUES (?, ?, ?);");
- $sth->execute(encode('UTF-8', $og->name), encode('UTF-8', $og->info), $og->class);
+ $sth->execute(encode('UTF-8', $og->name), encode('UTF-8', $og->info), $og->class);
- return $og->{id} = PMG::Utils::lastid($self->{dbh}, 'objectgroup_id_seq');
+ $og->{id} = PMG::Utils::lastid($self->{dbh}, 'objectgroup_id_seq');
+
+ $self->update_group_attributes($og);
+
+ $self->{dbh}->commit;
+ };
+
+ if (my $err = $@) {
+ $self->{dbh}->rollback;
+ syslog('err', $err);
+ return undef;
+ }
}
- return undef;
+ return $og->{id};
}
sub delete_group {
@@ -228,6 +276,9 @@ sub delete_group {
$self->{dbh}->do("DELETE FROM RuleGroup " .
"WHERE Objectgroup_ID = ?", undef, $groupid);
+ $self->{dbh}->do("DELETE FROM Objectgroup_Attributes " .
+ "WHERE Objectgroup_ID = ?", undef, $groupid);
+
$sth = $self->{dbh}->prepare("SELECT * FROM Object " .
"where Objectgroup_ID = ?");
$sth->execute($groupid);
@@ -252,6 +303,18 @@ sub delete_group {
return undef;
}
+sub load_group_attributes {
+ my ($self, $og) = @_;
+
+ my $attribute_sth = $self->{dbh}->prepare("SELECT * FROM Objectgroup_Attributes WHERE Objectgroup_ID = ?");
+ $attribute_sth->execute($og->{id});
+
+ while (my $ref = $attribute_sth->fetchrow_hashref()) {
+ $og->{and} = $ref->{value} if $ref->{name} eq 'and';
+ $og->{invert} = $ref->{value} if $ref->{name} eq 'invert';
+ }
+}
+
sub load_objectgroups {
my ($self, $class, $id) = @_;
@@ -259,34 +322,47 @@ sub load_objectgroups {
defined($class) || die "undefined object class";
- if (!(defined($id))) {
- $sth = $self->{dbh}->prepare(
- "SELECT * FROM Objectgroup where Class = ? ORDER BY name");
- $sth->execute($class);
-
- } else {
- $sth = $self->{dbh}->prepare(
- "SELECT * FROM Objectgroup where Class like ? and id = ? " .
- "order by name");
- $sth->execute($class,$id);
- }
+ $self->{dbh}->begin_work;
my $arr_og = ();
- while (my $ref = $sth->fetchrow_hashref()) {
- my $og = PMG::RuleDB::Group->new($ref->{name}, $ref->{info},
- $ref->{class});
- $og->{id} = $ref->{id};
- if ($class eq 'action') {
- my $objects = $self->load_group_objects($og->{id});
- my $obj = @$objects[0];
- defined($obj) || die "undefined action object: ERROR";
- $og->{action} = $obj;
+ eval {
+ if (!(defined($id))) {
+ $sth = $self->{dbh}->prepare(
+ "SELECT * FROM Objectgroup where Class = ? ORDER BY name");
+ $sth->execute($class);
+
+ } else {
+ $sth = $self->{dbh}->prepare(
+ "SELECT * FROM Objectgroup where Class like ? and id = ? " .
+ "order by name");
+ $sth->execute($class,$id);
}
- push @$arr_og, $og;
- }
- $sth->finish();
+ while (my $ref = $sth->fetchrow_hashref()) {
+ my $og = PMG::RuleDB::Group->new($ref->{name}, $ref->{info},
+ $ref->{class});
+ $og->{id} = $ref->{id};
+
+ if ($class eq 'action') {
+ my $objects = $self->load_group_objects($og->{id});
+ my $obj = @$objects[0];
+ defined($obj) || die "undefined action object: ERROR";
+ $og->{action} = $obj;
+ } else {
+ $self->load_group_attributes($og);
+ }
+ push @$arr_og, $og;
+ }
+
+ $sth->finish();
+ };
+
+ my $err = $@;
+
+ $self->{dbh}->rollback; # finish transaction
+
+ die $err if $err;
return $arr_og;
}
--
2.30.2
^ permalink raw reply [flat|nested] 20+ messages in thread* [pmg-devel] [PATCH pmg-api v2 04/10] add rule attributes and/invert (for each relevant type)
2024-02-21 12:24 [pmg-devel] [PATCH pmg-api/docs/gui v2] implement and combination and inversion of groups and objects Dominik Csapak
` (2 preceding siblings ...)
2024-02-21 12:24 ` [pmg-devel] [PATCH pmg-api v2 03/10] add objectgroup attributes and/invert Dominik Csapak
@ 2024-02-21 12:24 ` Dominik Csapak
2024-02-22 6:46 ` Thomas Lamprecht
2024-02-21 12:24 ` [pmg-devel] [PATCH pmg-api v2 05/10] RuleCache: load rule/objectgroup attributes from database Dominik Csapak
` (9 subsequent siblings)
13 siblings, 1 reply; 20+ messages in thread
From: Dominik Csapak @ 2024-02-21 12:24 UTC (permalink / raw)
To: pmg-devel
like with the objectgroups, add an attributes table for groups, and an
'and'/'invert' attribute for each relevant object type
(what/when/from/to).
This is intended to modify the behaviour for the matching regarding
object groups, so that one has more choice in the logical matching.
Also adds the new table to cluster sync, backup and factory reset.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
changes from v1:
* add comment for rollback
* remove unnecessary transaction for 'load_rules'
(we didn't actually load the attributes there)
* delete attrubutes on rule delete
* add handling to cluster sync, backup and factory reset
* use a regex for checking the props in the db instead of using a
loop label for early exit
src/PMG/API2/ObjectGroupHelpers.pm | 8 ++
src/PMG/API2/Rules.pm | 53 +++++++++++-
src/PMG/Backup.pm | 1 +
src/PMG/Cluster.pm | 2 +
src/PMG/DBTools.pm | 16 ++++
src/PMG/RuleDB.pm | 130 ++++++++++++++++++++++++-----
6 files changed, 186 insertions(+), 24 deletions(-)
diff --git a/src/PMG/API2/ObjectGroupHelpers.pm b/src/PMG/API2/ObjectGroupHelpers.pm
index a08a6a3..3060157 100644
--- a/src/PMG/API2/ObjectGroupHelpers.pm
+++ b/src/PMG/API2/ObjectGroupHelpers.pm
@@ -31,6 +31,14 @@ sub format_rule {
active => $rule->{active},
direction => $rule->{direction},
};
+ my $types = [qw(what when from to)];
+ my $attributes = [qw(and invert)];
+ for my $type ($types->@*) {
+ for my $attribute ($attributes->@*) {
+ my $opt = "${type}-${attribute}";
+ $data->{$opt} = $rule->{$opt} if defined($rule->{$opt});
+ }
+ }
$cond_create_group->($data, 'from', $from);
$cond_create_group->($data, 'to', $to);
diff --git a/src/PMG/API2/Rules.pm b/src/PMG/API2/Rules.pm
index c48370f..1ebadc2 100644
--- a/src/PMG/API2/Rules.pm
+++ b/src/PMG/API2/Rules.pm
@@ -149,6 +149,54 @@ my $rule_params = {
type => 'boolean',
optional => 1,
},
+ 'what-and' => {
+ description => "Flag to 'and' combine WHAT group matches.",
+ type => 'boolean',
+ default => 0,
+ optional => 1,
+ },
+ 'what-invert' => {
+ description => "Flag to invert WHAT group matches.",
+ type => 'boolean',
+ default => 0,
+ optional => 1,
+ },
+ 'when-and' => {
+ description => "Flag to 'and' combine WHEN group matches.",
+ type => 'boolean',
+ default => 0,
+ optional => 1,
+ },
+ 'when-invert' => {
+ description => "Flag to invert WHEN group matches.",
+ type => 'boolean',
+ default => 0,
+ optional => 1,
+ },
+ 'from-and' => {
+ description => "Flag to 'and' combine FROM group matches.",
+ type => 'boolean',
+ default => 0,
+ optional => 1,
+ },
+ 'from-invert' => {
+ description => "Flag to invert FROM group matches.",
+ type => 'boolean',
+ default => 0,
+ optional => 1,
+ },
+ 'to-and' => {
+ description => "Flag to 'and' combine TO group matches.",
+ type => 'boolean',
+ default => 0,
+ optional => 1,
+ },
+ 'to-invert' => {
+ description => "Flag to invert TO group matches.",
+ type => 'boolean',
+ default => 0,
+ optional => 1,
+ },
};
sub get_rule_params {
@@ -203,7 +251,10 @@ __PACKAGE__->register_method ({
my $rule = $rdb->load_rule($id);
- for my $key (qw(name active direction priority)) {
+ my $keys = ["name", "priority"];
+ push $keys->@*, keys get_rule_params()->%*;
+
+ for my $key ($keys->@*) {
$rule->{$key} = $param->{$key} if defined($param->{$key});
}
diff --git a/src/PMG/Backup.pm b/src/PMG/Backup.pm
index 9fc91f8..5891afb 100644
--- a/src/PMG/Backup.pm
+++ b/src/PMG/Backup.pm
@@ -95,6 +95,7 @@ sub dumpdb {
dump_table($dbh, 'attribut', $ofh);
dump_table($dbh, 'objectgroup_attributes', $ofh);
+ dump_table($dbh, 'rule_attributes', $ofh);
dump_table($dbh, 'object', $ofh, 'object_id_seq', 'id');
dump_table($dbh, 'objectgroup', $ofh, 'objectgroup_id_seq', 'id');
dump_table($dbh, 'rule', $ofh, 'rule_id_seq', 'id');
diff --git a/src/PMG/Cluster.pm b/src/PMG/Cluster.pm
index ac50cff..f468618 100644
--- a/src/PMG/Cluster.pm
+++ b/src/PMG/Cluster.pm
@@ -533,6 +533,7 @@ sub sync_ruledb_from_master {
$ldb->do("DELETE FROM Object");
$ldb->do("DELETE FROM Attribut");
$ldb->do("DELETE FROM Objectgroup_Attributes");
+ $ldb->do("DELETE FROM Rule_Attributes");
eval {
$rdb->begin_work;
@@ -545,6 +546,7 @@ sub sync_ruledb_from_master {
PMG::DBTools::copy_table($ldb, $rdb, "ObjectGroup");
PMG::DBTools::copy_table($ldb, $rdb, "Object", 'value');
PMG::DBTools::copy_table($ldb, $rdb, "Attribut", 'value');
+ PMG::DBTools::copy_table($ldb, $rdb, "Rule_Attributes");
PMG::DBTools::copy_table($ldb, $rdb, "Objectgroup_Attributes");
};
diff --git a/src/PMG/DBTools.pm b/src/PMG/DBTools.pm
index 3c8d181..3e814dc 100644
--- a/src/PMG/DBTools.pm
+++ b/src/PMG/DBTools.pm
@@ -295,6 +295,18 @@ my $userprefs_ctablecmd = <<__EOD;
__EOD
+my $rule_attributes_cmd = <<__EOD;
+ CREATE TABLE Rule_Attributes (
+ Rule_ID INTEGER NOT NULL,
+ Name VARCHAR(20) NOT NULL,
+ Value BYTEA NULL,
+ PRIMARY KEY (Rule_ID, Name)
+ );
+
+ CREATE INDEX Rule_Attributes_Rule_ID_Index ON Rule_Attributes(Rule_ID);
+
+__EOD
+
my $object_group_attributes_cmd = <<__EOD;
CREATE TABLE Objectgroup_Attributes (
Objectgroup_ID INTEGER NOT NULL,
@@ -452,6 +464,8 @@ sub create_ruledb {
$virusinfo_stat_ctablecmd;
+ $rule_attributes_cmd;
+
$object_group_attributes_cmd;
EOD
);
@@ -508,6 +522,7 @@ sub upgradedb {
'CStatistic', $cstatistic_ctablecmd,
'ClusterInfo', $clusterinfo_ctablecmd,
'VirusInfo', $virusinfo_stat_ctablecmd,
+ 'Rule_Attributes', $rule_attributes_cmd,
'Objectgroup_Attributes', $object_group_attributes_cmd,
};
@@ -620,6 +635,7 @@ sub init_ruledb {
$dbh->do(
"DELETE FROM Rule;"
." DELETE FROM RuleGroup;"
+ ." DELETE FROM Rule_Attributes;"
." DELETE FROM Objectgroup_Attributes;"
." DELETE FROM Attribut WHERE Object_ID NOT IN ($glids);"
." DELETE FROM Object WHERE ID NOT IN ($glids);"
diff --git a/src/PMG/RuleDB.pm b/src/PMG/RuleDB.pm
index 0b112b4..e5fe56e 100644
--- a/src/PMG/RuleDB.pm
+++ b/src/PMG/RuleDB.pm
@@ -668,6 +668,35 @@ sub delete_object {
return 1;
}
+sub update_rule_attributes {
+ my ($self, $rule) = @_;
+
+ my $types = [qw(what when from to)];
+ my $attributes = [qw(and invert)];
+
+ for my $type ($types->@*) {
+ for my $attribute ($attributes->@*) {
+ my $prop = "$type-$attribute";
+
+ # only save the values if they're set to 1
+ if ($rule->{$prop}) {
+ $self->{dbh}->do(
+ "INSERT INTO Rule_Attributes (Rule_ID, Name, Value) " .
+ "VALUES (?, ?, ?) ".
+ "ON CONFLICT (Rule_ID, Name) DO UPDATE SET Value = ?", undef,
+ $rule->{id}, $prop, $rule->{$prop}, $rule->{$prop},
+ );
+ } else {
+ $self->{dbh}->do(
+ "DELETE FROM Rule_Attributes " .
+ "WHERE Rule_ID = ? AND Name = ?", undef,
+ $rule->{id}, $prop,
+ );
+ }
+ }
+ }
+}
+
sub save_rule {
my ($self, $rule) = @_;
@@ -682,28 +711,53 @@ sub save_rule {
my $rulename = encode('UTF-8', $rule->{name});
if (defined($rule->{id})) {
+ $self->{dbh}->begin_work;
+
+ eval {
+ $self->{dbh}->do(
+ "UPDATE Rule " .
+ "SET Name = ?, Priority = ?, Active = ?, Direction = ? " .
+ "WHERE ID = ?", undef,
+ $rulename, $rule->{priority}, $rule->{active},
+ $rule->{direction}, $rule->{id});
- $self->{dbh}->do(
- "UPDATE Rule " .
- "SET Name = ?, Priority = ?, Active = ?, Direction = ? " .
- "WHERE ID = ?", undef,
- $rulename, $rule->{priority}, $rule->{active},
- $rule->{direction}, $rule->{id});
+ $self->update_rule_attributes($rule);
- return $rule->{id};
+ $self->{dbh}->commit;
+ };
+ if (my $err = $@) {
+ $self->{dbh}->rollback;
+ syslog('err', $err);
+ return undef;
+ }
} else {
- my $sth = $self->{dbh}->prepare(
- "INSERT INTO Rule (Name, Priority, Active, Direction) " .
- "VALUES (?, ?, ?, ?);");
+ $self->{dbh}->begin_work;
+
+ eval {
+ my $sth = $self->{dbh}->prepare(
+ "INSERT INTO Rule (Name, Priority, Active, Direction) " .
+ "VALUES (?, ?, ?, ?);");
+
+ $sth->execute($rulename, $rule->priority, $rule->active,
+ $rule->direction);
+
+
+ $rule->{id} = PMG::Utils::lastid($self->{dbh}, 'rule_id_seq');
- $sth->execute($rulename, $rule->priority, $rule->active,
- $rule->direction);
+ $self->update_rule_attributes($rule);
- return $rule->{id} = PMG::Utils::lastid($self->{dbh}, 'rule_id_seq');
+ $self->{dbh}->commit;
+ };
+
+ if (my $err = $@) {
+ $self->{dbh}->rollback;
+ syslog('err', $err);
+ return undef;
+ }
}
- return undef;
+ return $rule->{id};
}
sub delete_rule {
@@ -718,6 +772,8 @@ sub delete_rule {
"WHERE ID = ?", undef, $ruleid);
$self->{dbh}->do("DELETE FROM RuleGroup " .
"WHERE Rule_ID = ?", undef, $ruleid);
+ $self->{dbh}->do("DELETE FROM Rule_Attributes " .
+ "WHERE Rule_ID = ?", undef, $ruleid);
$self->{dbh}->commit;
};
@@ -829,24 +885,52 @@ sub rule_remove_group {
return 1;
}
+sub load_rule_attributes {
+ my ($self, $rule) = @_;
+
+ my $types = [qw(what when from to)];
+ my $attributes = [qw(and invert)];
+
+ my $attribute_sth = $self->{dbh}->prepare("SELECT * FROM Rule_Attributes WHERE Rule_ID = ?");
+ $attribute_sth->execute($rule->{id});
+
+ while (my $ref = $attribute_sth->fetchrow_hashref()) {
+ if ($ref->{name} =~ m/^((?:what|when|from|to)-(?:and|invert))$/) {
+ my $prop = $1;
+ $rule->{$prop} = $ref->{value};
+ }
+ }
+}
+
sub load_rule {
my ($self, $id) = @_;
defined($id) || die "undefined id: ERROR";
- my $sth = $self->{dbh}->prepare(
- "SELECT * FROM Rule where id = ? ORDER BY Priority DESC");
+ $self->{dbh}->begin_work;
- my $rules = ();
+ my $rule;
+
+ eval {
+ my $sth = $self->{dbh}->prepare(
+ "SELECT * FROM Rule where id = ? ORDER BY Priority DESC");
- $sth->execute($id);
+ $sth->execute($id);
- my $ref = $sth->fetchrow_hashref();
- die "rule '$id' does not exist\n" if !defined($ref);
+ my $ref = $sth->fetchrow_hashref();
+ die "rule '$id' does not exist\n" if !defined($ref);
+
+ $rule = PMG::RuleDB::Rule->new($ref->{name}, $ref->{priority},
+ $ref->{active}, $ref->{direction});
+ $rule->{id} = $ref->{id};
+
+ $self->load_rule_attributes($rule);
+ };
+ my $err = $@;
+
+ $self->{dbh}->rollback; # finish transaction
- my $rule = PMG::RuleDB::Rule->new($ref->{name}, $ref->{priority},
- $ref->{active}, $ref->{direction});
- $rule->{id} = $ref->{id};
+ die $err if $err;
return $rule;
}
--
2.30.2
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [pmg-devel] [PATCH pmg-api v2 04/10] add rule attributes and/invert (for each relevant type)
2024-02-21 12:24 ` [pmg-devel] [PATCH pmg-api v2 04/10] add rule attributes and/invert (for each relevant type) Dominik Csapak
@ 2024-02-22 6:46 ` Thomas Lamprecht
2024-02-22 7:34 ` Dominik Csapak
0 siblings, 1 reply; 20+ messages in thread
From: Thomas Lamprecht @ 2024-02-22 6:46 UTC (permalink / raw)
To: Dominik Csapak, pmg-devel
Am 21/02/2024 um 13:24 schrieb Dominik Csapak:
> +my $rule_attributes_cmd = <<__EOD;
> + CREATE TABLE Rule_Attributes (
> + Rule_ID INTEGER NOT NULL,
> + Name VARCHAR(20) NOT NULL,
> + Value BYTEA NULL,
FWIW, with this being such limited to only support a boolean value, the
name could have been a boolean type for invert/and too..
Else it would make more sense to have this either more generic, or make
it a very explicit table (name) like "Rule_Flags".
As of now the name suggests that its generic, but the schema really isn't.
I.e., this feels a bit like the worst of both worlds.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [pmg-devel] [PATCH pmg-api v2 04/10] add rule attributes and/invert (for each relevant type)
2024-02-22 6:46 ` Thomas Lamprecht
@ 2024-02-22 7:34 ` Dominik Csapak
2024-02-22 7:38 ` Thomas Lamprecht
0 siblings, 1 reply; 20+ messages in thread
From: Dominik Csapak @ 2024-02-22 7:34 UTC (permalink / raw)
To: Thomas Lamprecht, pmg-devel
On 2/22/24 07:46, Thomas Lamprecht wrote:
> Am 21/02/2024 um 13:24 schrieb Dominik Csapak:
>> +my $rule_attributes_cmd = <<__EOD;
>> + CREATE TABLE Rule_Attributes (
>> + Rule_ID INTEGER NOT NULL,
>> + Name VARCHAR(20) NOT NULL,
>> + Value BYTEA NULL,
>
> FWIW, with this being such limited to only support a boolean value, the
> name could have been a boolean type for invert/and too..
>
> Else it would make more sense to have this either more generic, or make
> it a very explicit table (name) like "Rule_Flags".
>
> As of now the name suggests that its generic, but the schema really isn't.
> I.e., this feels a bit like the worst of both worlds.
>
huh? i'm not sure i understand completely what you want to say:
Value is a BYTEA is a 'binary string' [0]
so we can store arbitrary information there, not only booleans
(i reused the data types we already have for the 'Attribut' table, which
stores attributes for the Objects, e.g. for the Notify Action)
0: https://www.postgresql.org/docs/current/datatype-binary.html
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [pmg-devel] [PATCH pmg-api v2 04/10] add rule attributes and/invert (for each relevant type)
2024-02-22 7:34 ` Dominik Csapak
@ 2024-02-22 7:38 ` Thomas Lamprecht
0 siblings, 0 replies; 20+ messages in thread
From: Thomas Lamprecht @ 2024-02-22 7:38 UTC (permalink / raw)
To: Dominik Csapak, pmg-devel
Am 22/02/2024 um 08:34 schrieb Dominik Csapak:
>
>
> On 2/22/24 07:46, Thomas Lamprecht wrote:
>> Am 21/02/2024 um 13:24 schrieb Dominik Csapak:
>>> +my $rule_attributes_cmd = <<__EOD;
>>> + CREATE TABLE Rule_Attributes (
>>> + Rule_ID INTEGER NOT NULL,
>>> + Name VARCHAR(20) NOT NULL,
>>> + Value BYTEA NULL,
>>
>> FWIW, with this being such limited to only support a boolean value, the
>> name could have been a boolean type for invert/and too..
>>
>> Else it would make more sense to have this either more generic, or make
>> it a very explicit table (name) like "Rule_Flags".
>>
>> As of now the name suggests that its generic, but the schema really isn't.
>> I.e., this feels a bit like the worst of both worlds.
>>
>
> huh? i'm not sure i understand completely what you want to say:
>
> Value is a BYTEA is a 'binary string' [0]
> so we can store arbitrary information there, not only booleans
Hmm OK, seems I misread the BYTEA definition yesterday evening and didn't
recheck today, ack then – sorry for the noise.
^ permalink raw reply [flat|nested] 20+ messages in thread
* [pmg-devel] [PATCH pmg-api v2 05/10] RuleCache: load rule/objectgroup attributes from database
2024-02-21 12:24 [pmg-devel] [PATCH pmg-api/docs/gui v2] implement and combination and inversion of groups and objects Dominik Csapak
` (3 preceding siblings ...)
2024-02-21 12:24 ` [pmg-devel] [PATCH pmg-api v2 04/10] add rule attributes and/invert (for each relevant type) Dominik Csapak
@ 2024-02-21 12:24 ` Dominik Csapak
2024-02-21 12:24 ` [pmg-devel] [PATCH pmg-api v2 06/10] RuleCache: implement and/invert for when/from/to Dominik Csapak
` (8 subsequent siblings)
13 siblings, 0 replies; 20+ messages in thread
From: Dominik Csapak @ 2024-02-21 12:24 UTC (permalink / raw)
To: pmg-devel
so that we can use the 'and' and 'invert' flags set.
This also adds the attributes to the digest of the rule cache so the
cluster sync is triggered when the attributes change.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
changes from v1:
* use regex to get the attributes instead of hardcoding each one (for the rules)
* add them to the digest so rule changes get synced
src/PMG/RuleCache.pm | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/src/PMG/RuleCache.pm b/src/PMG/RuleCache.pm
index cd56342..4bde2e7 100644
--- a/src/PMG/RuleCache.pm
+++ b/src/PMG/RuleCache.pm
@@ -67,6 +67,21 @@ sub new {
$self->{"$ruleid:what"} = { groups => [] };
$self->{"$ruleid:action"} = { groups => [] };
+ my $attribute_sth = $dbh->prepare("SELECT * FROM Rule_Attributes WHERE Rule_ID = ? ORDER BY Name");
+ $attribute_sth->execute($ruleid);
+
+ my $rule_attributes = [];
+ while (my $ref = $attribute_sth->fetchrow_hashref()) {
+ if ($ref->{name} =~ m/^(from|to|when|what)-(and|invert)$/) {
+ my $type = $1;
+ my $prop = $2;
+ my $value = $ref->{value};
+ $self->{"${ruleid}:${type}"}->{$prop} = $value;
+
+ $sha1->add("${ruleid}:${type}-${prop}=${value}|");
+ }
+ }
+
my $sth1 = $dbh->prepare(
"SELECT Objectgroup_ID, Grouptype FROM RuleGroup " .
"where RuleGroup.Rule_ID = '$ruleid' " .
@@ -114,6 +129,15 @@ sub new {
objects => $objects,
};
+ my $objectgroup_sth = $dbh->prepare("SELECT * FROM Objectgroup_Attributes WHERE Objectgroup_ID = ?");
+ $objectgroup_sth->execute($groupid);
+
+ while (my $ref = $objectgroup_sth->fetchrow_hashref()) {
+ $group->{and} = $ref->{value} if $ref->{name} eq 'and';
+ $group->{invert} = $ref->{value} if $ref->{name} eq 'invert';
+ }
+ $sha1->add (join(',', $groupid, $group->{and} // 0, $group->{invert} // 0), "|");
+
my $type = $type_map->{$gtype};
push $self->{"$ruleid:$type"}->{groups}->@*, $group;
}
--
2.30.2
^ permalink raw reply [flat|nested] 20+ messages in thread* [pmg-devel] [PATCH pmg-api v2 06/10] RuleCache: implement and/invert for when/from/to
2024-02-21 12:24 [pmg-devel] [PATCH pmg-api/docs/gui v2] implement and combination and inversion of groups and objects Dominik Csapak
` (4 preceding siblings ...)
2024-02-21 12:24 ` [pmg-devel] [PATCH pmg-api v2 05/10] RuleCache: load rule/objectgroup attributes from database Dominik Csapak
@ 2024-02-21 12:24 ` Dominik Csapak
2024-02-21 12:24 ` [pmg-devel] [PATCH pmg-api v2 07/10] MailQueue: return maximum AID Dominik Csapak
` (7 subsequent siblings)
13 siblings, 0 replies; 20+ messages in thread
From: Dominik Csapak @ 2024-02-21 12:24 UTC (permalink / raw)
To: pmg-devel
by introducing 'match_list_with_mode' that gets called for each group
and then on the result of each group match result.
This does not work for 'what' matches since they are not a simple
yes/no match (they include the parts) so this will be done seperately.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
no changes
src/PMG/RuleCache.pm | 65 +++++++++++++++++++++++++++++---------------
1 file changed, 43 insertions(+), 22 deletions(-)
diff --git a/src/PMG/RuleCache.pm b/src/PMG/RuleCache.pm
index 4bde2e7..d0fa1f8 100644
--- a/src/PMG/RuleCache.pm
+++ b/src/PMG/RuleCache.pm
@@ -272,13 +272,14 @@ sub from_match {
$ip = $1;
}
- for my $group ($from->{groups}->@*) {
- for my $obj ($group->{objects}->@*) {
- return 1 if $obj->who_match($addr, $ip, $ldap);
- }
- }
-
- return 0;
+ return match_list_with_mode($from->{groups}, $from->{and}, $from->{invert}, sub {
+ my ($group) = @_;
+ my $list = $group->{objects};
+ return match_list_with_mode($list, $group->{and}, $group->{invert}, sub {
+ my ($obj) = @_;
+ return $obj->who_match($addr, $ip, $ldap);
+ });
+ });
}
sub to_match {
@@ -288,14 +289,14 @@ sub to_match {
return 1 if scalar($to->{groups}->@*) == 0;
- for my $group ($to->{groups}->@*) {
- for my $obj ($group->{objects}->@*) {
- return 1 if $obj->who_match($addr, undef, $ldap);
- }
- }
-
-
- return 0;
+ return match_list_with_mode($to->{groups}, $to->{and}, $to->{invert}, sub {
+ my ($group) = @_;
+ my $list = $group->{objects};
+ return match_list_with_mode($list, $group->{and}, $group->{invert}, sub {
+ my ($obj) = @_;
+ return $obj->who_match($addr, undef, $ldap);
+ });
+ });
}
sub when_match {
@@ -305,13 +306,14 @@ sub when_match {
return 1 if scalar($when->{groups}->@*) == 0;
- for my $group ($when->{groups}->@*) {
- for my $obj ($group->{objects}->@*) {
- return 1 if $obj->when_match($time);
- }
- }
-
- return 0;
+ return match_list_with_mode($when->{groups}, $when->{and}, $when->{invert}, sub {
+ my ($group) = @_;
+ my $list = $group->{objects};
+ return match_list_with_mode($list, $group->{and}, $group->{invert}, sub {
+ my ($obj) = @_;
+ return $obj->when_match($time);
+ });
+ });
}
sub what_match {
@@ -353,4 +355,23 @@ sub what_match {
return ($marks, $spaminfo);
}
+# calls sub with each element of $list, and and/ors/inverts the result
+sub match_list_with_mode($$$$) {
+ my ($list, $and, $invert, $sub) = @_;
+
+ $and //= 0;
+ $invert //= 0;
+
+ for my $el ($list->@*) {
+ my $res = $sub->($el);
+ if (!$and) {
+ return !$invert if $res;
+ } else {
+ return $invert if !$res;
+ }
+ }
+
+ return $and != $invert;
+}
+
1;
--
2.30.2
^ permalink raw reply [flat|nested] 20+ messages in thread* [pmg-devel] [PATCH pmg-api v2 07/10] MailQueue: return maximum AID
2024-02-21 12:24 [pmg-devel] [PATCH pmg-api/docs/gui v2] implement and combination and inversion of groups and objects Dominik Csapak
` (5 preceding siblings ...)
2024-02-21 12:24 ` [pmg-devel] [PATCH pmg-api v2 06/10] RuleCache: implement and/invert for when/from/to Dominik Csapak
@ 2024-02-21 12:24 ` Dominik Csapak
2024-02-21 12:24 ` [pmg-devel] [PATCH pmg-api v2 08/10] ModGroup: add possibility to explode to all targets Dominik Csapak
` (6 subsequent siblings)
13 siblings, 0 replies; 20+ messages in thread
From: Dominik Csapak @ 2024-02-21 12:24 UTC (permalink / raw)
To: pmg-devel
we'll need this in the what_matches to invert mark lists.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
no changes
src/PMG/MailQueue.pm | 4 ++--
src/PMG/Utils.pm | 2 ++
src/bin/pmg-smtp-filter | 3 ++-
3 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/src/PMG/MailQueue.pm b/src/PMG/MailQueue.pm
index 8355c30..4e37cb9 100644
--- a/src/PMG/MailQueue.pm
+++ b/src/PMG/MailQueue.pm
@@ -406,10 +406,10 @@ sub parse_mail {
# we also remove all proxmox-marks from the mail and add an unique
# id to each attachment.
- PMG::Utils::remove_marks ($entity, 1);
+ my $max_aid = PMG::Utils::remove_marks ($entity, 1);
PMG::Utils::add_ct_marks ($entity);
- return $entity;
+ return ($entity, $max_aid);
}
sub decode_entities {
diff --git a/src/PMG/Utils.pm b/src/PMG/Utils.pm
index 12b3ed5..afccaf4 100644
--- a/src/PMG/Utils.pm
+++ b/src/PMG/Utils.pm
@@ -185,6 +185,8 @@ sub remove_marks {
$id++;
});
+
+ return $id - 1; # return max AID
}
sub subst_values {
diff --git a/src/bin/pmg-smtp-filter b/src/bin/pmg-smtp-filter
index cc2fcd6..982e4b9 100755
--- a/src/bin/pmg-smtp-filter
+++ b/src/bin/pmg-smtp-filter
@@ -650,7 +650,8 @@ sub handle_smtp {
my $maxfiles = $pmg_cfg->get('clamav', 'archivemaxfiles');
- my $entity = $queue->parse_mail($maxfiles);
+ my ($entity, $max_aid) = $queue->parse_mail($maxfiles);
+ $msginfo->{max_aid} = $max_aid;
$self->log (3, "$queue->{logid}: new mail message-id=%s", $queue->{msgid});
--
2.30.2
^ permalink raw reply [flat|nested] 20+ messages in thread* [pmg-devel] [PATCH pmg-api v2 08/10] ModGroup: add possibility to explode to all targets
2024-02-21 12:24 [pmg-devel] [PATCH pmg-api/docs/gui v2] implement and combination and inversion of groups and objects Dominik Csapak
` (6 preceding siblings ...)
2024-02-21 12:24 ` [pmg-devel] [PATCH pmg-api v2 07/10] MailQueue: return maximum AID Dominik Csapak
@ 2024-02-21 12:24 ` Dominik Csapak
2024-02-21 12:24 ` [pmg-devel] [PATCH pmg-api v2 09/10] RuleCache: implement and/invert for what matches Dominik Csapak
` (5 subsequent siblings)
13 siblings, 0 replies; 20+ messages in thread
From: Dominik Csapak @ 2024-02-21 12:24 UTC (permalink / raw)
To: pmg-devel
we will need that for the remove action + and/invert
It's probably not the most efficient way to to that, but it's easy to
understand.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
promoted to normal commit instead of WIP, since the only way i'd found
to make it more 'efficient' is to copy/paste the subgroups code
and instead of looping inside the innermost loop about the targets,
do it on the outside, which is basically the same as we do here
(but the innermost loop is always 1 long)
src/PMG/ModGroup.pm | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/src/PMG/ModGroup.pm b/src/PMG/ModGroup.pm
index b0fa9a6..d4e1cd5 100644
--- a/src/PMG/ModGroup.pm
+++ b/src/PMG/ModGroup.pm
@@ -81,6 +81,23 @@ sub subgroups {
return $res;
}
+# explode the groups, so we have one for each target we need
+# only to be used by the rRemove action when there was a spaminfo
+sub explode {
+ my ($self, $targets) = @_;
+
+ my $groups = $self->{groups};
+ my $ea = $self->{ea};
+ my $res;
+
+ # TODO: implment it more direclty with less overhead!
+ for my $target ($targets->@*) {
+ $self->subgroups([$target]);
+ }
+
+ return $self->subgroups($targets);
+}
+
1;
__END__
--
2.30.2
^ permalink raw reply [flat|nested] 20+ messages in thread* [pmg-devel] [PATCH pmg-api v2 09/10] RuleCache: implement and/invert for what matches
2024-02-21 12:24 [pmg-devel] [PATCH pmg-api/docs/gui v2] implement and combination and inversion of groups and objects Dominik Csapak
` (7 preceding siblings ...)
2024-02-21 12:24 ` [pmg-devel] [PATCH pmg-api v2 08/10] ModGroup: add possibility to explode to all targets Dominik Csapak
@ 2024-02-21 12:24 ` Dominik Csapak
2024-02-21 12:24 ` [pmg-devel] [PATCH pmg-api v2 10/10] pmgdb: extend dump output to include add/invert Dominik Csapak
` (4 subsequent siblings)
13 siblings, 0 replies; 20+ messages in thread
From: Dominik Csapak @ 2024-02-21 12:24 UTC (permalink / raw)
To: pmg-devel
Since what matches are not a simple boolean match, but also can contain
"marks" to mark specific parts of the mail, we must implement some
custom logic for and/invert here.
The goal here is to define that groups are on a per part level,
but the rule operates on the whole mail.
To achieve this we have two different and/invert combine functions, one
for the group level and one for the whole what match.
For per group and/inversion we and 'and-combine' and invert the list of
marks, so if it matches part 1,2 of 1,2,3 the inversion would return 3.
For the rule it only matters if the and/inversion part matches at all,
regardless of the marks. If it matches, the marks will be or'ed.
With this, one can represent many different scenarios that were not
possible before.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
src/PMG/RuleCache.pm | 164 +++++++++++++++++++++++++++++++++++++--
src/PMG/RuleDB/Remove.pm | 12 ++-
2 files changed, 167 insertions(+), 9 deletions(-)
diff --git a/src/PMG/RuleCache.pm b/src/PMG/RuleCache.pm
index d0fa1f8..14da88f 100644
--- a/src/PMG/RuleCache.pm
+++ b/src/PMG/RuleCache.pm
@@ -332,29 +332,146 @@ sub what_match {
return ($marks, $spaminfo);
}
+ my $what_matches = {};
+
for my $group ($what->{groups}->@*) {
+ my $group_matches = {};
+ my $and = $group->{and};
+ my $invert = $group->{invert};
for my $obj ($group->{objects}->@*) {
if (!$obj->can('what_match_targets')) {
- if (my $match = $obj->what_match($queue, $element, $msginfo, $dbh)) {
- for my $target ($msginfo->{targets}->@*) {
- push $marks->{$target}->@*, $match->@*;
+ my $match = $obj->what_match($queue, $element, $msginfo, $dbh);
+ for my $target ($msginfo->{targets}->@*) {
+ if (defined($match)) {
+ push $group_matches->{$target}->@*, $match;
+ } else {
+ push $group_matches->{$target}->@*, undef;
}
}
} else {
- if (my $target_info = $obj->what_match_targets($queue, $element, $msginfo, $dbh)) {
- foreach my $k (keys $target_info->%*) {
- push $marks->{$k}->@*, $target_info->{$k}->{marks}->@*;
+ my $target_info = $obj->what_match_targets($queue, $element, $msginfo, $dbh);
+ for my $target ($msginfo->{targets}->@*) {
+ my $match = $target_info->{$target};
+ if (defined($match)) {
+ push $group_matches->{$target}->@*, $match->{marks};
# only save spaminfo once
- $spaminfo = $target_info->{$k}->{spaminfo} if !defined($spaminfo);
+ $spaminfo = $match->{spaminfo} if !defined($spaminfo);
+ } else {
+ push $group_matches->{$target}->@*, undef;
}
}
}
}
+
+ for my $target (keys $group_matches->%*) {
+ my $matches = group_match_and_invert($group_matches->{$target}, $and, $invert, $msginfo);
+ push $what_matches->{$target}->@*, $matches;
+ }
+ }
+
+ for my $target (keys $what_matches->%*) {
+ my $target_marks = what_match_and_invert($what_matches->{$target}, $what->{and}, $what->{invert});
+ $marks->{$target} = $target_marks;
}
return ($marks, $spaminfo);
}
+# combines matches of groups
+# this is only binary, and if it matches, 'or' combines the marks
+# so that all found marks are included
+#
+# this way we can create rules like:
+#
+# ---
+# What is and combined:
+# group1: match filename .*\.pdf
+# group2: spamlevel >= 3
+# ACTION: remove attachments
+# ---
+# which would remove attachments for all *.pdf filenames where
+# the spamlevel is >= 3
+sub what_match_and_invert($$$) {
+ my ($matches, $and, $invert) = @_;
+
+ my $match_result = match_list_with_mode($matches, $and, $invert, sub {
+ my ($match) = @_;
+ return defined($match);
+ });
+
+ if ($match_result) {
+ my $res = [];
+ for my $match ($matches->@*) {
+ push $res->@*, $match->@* if defined($match);
+ }
+ return $res;
+ } else {
+ return undef;
+ }
+}
+
+# combines group matches according to and/invert
+# since we want match groups per mime part, we must
+# look at the marks and possibly invert them
+sub group_match_and_invert($$$$) {
+ my ($group_matches, $and, $invert, $msginfo) = @_;
+
+ my $encountered_parts = 0;
+ if ($and) {
+ my $set = {};
+ my $count = scalar($group_matches->@*);
+ for my $match ($group_matches->@*) {
+ if (!defined($match)) {
+ $set = {};
+ last;
+ }
+
+ if (scalar($match->@*) > 0) {
+ $encountered_parts = 1;
+ $set->{$_}++ for $match->@*;
+ } else {
+ $set->{$_}++ for (1..$msginfo->{max_aid});
+ }
+ }
+
+ $group_matches = undef;
+ for my $key (keys $set->%*) {
+ if ($set->{$key} == $count) {
+ push $group_matches->@*, $key;
+ }
+ }
+ if (defined($group_matches) && scalar($group_matches->@*) == $count && !$encountered_parts) {
+ $group_matches = [];
+ }
+ } else {
+ my $set = {};
+ for my $match ($group_matches->@*) {
+ next if !defined($match);
+ if (scalar($match->@*) == 0) {
+ $set->{$_} = 1 for (1..$msginfo->{max_aid});
+ } else {
+ $encountered_parts = 1;
+ $set->{$_} = 1 for $match->@*;
+ }
+ }
+
+ my $count = scalar(keys $set->%*);
+ if ($count == $msginfo->{max_aid} && !$encountered_parts) {
+ $group_matches = [];
+ } elsif ($count == 0) {
+ $group_matches = undef;
+ } else {
+ $group_matches = [keys $set->%*];
+ }
+ }
+
+ if ($invert) {
+ $group_matches = invert_mark_list($group_matches, $msginfo->{max_aid});
+ }
+
+ return $group_matches;
+}
+
# calls sub with each element of $list, and and/ors/inverts the result
sub match_list_with_mode($$$$) {
my ($list, $and, $invert, $sub) = @_;
@@ -374,4 +491,37 @@ sub match_list_with_mode($$$$) {
return $and != $invert;
}
+# inverts a list of marks with the remaining ones of the mail
+# examples:
+# mail has [1,2,3,4,5]
+#
+# undef => [1,2,3,4,5]
+# [1,2] => [3,4,5]
+# [1,2,3,4,5] => undef
+# [] => undef // [] means the whole mail matched
+sub invert_mark_list($$) {
+ my ($list, $max_aid) = @_;
+
+ if (defined($list)) {
+ my $length = scalar($list->@*);
+ if ($length == 0 || $length == ($max_aid - 1)) {
+ return undef;
+ }
+ }
+
+ $list //= [];
+
+ my $set = {};
+ $set->{$_} = 1 for $list->@*;
+
+ my $new_list = [];
+ for (my $i = 1; $i <= $max_aid; $i++) {
+ if (!$set->{$i}) {
+ push $new_list->@*, $i;
+ }
+ }
+
+ return $new_list;
+}
+
1;
diff --git a/src/PMG/RuleDB/Remove.pm b/src/PMG/RuleDB/Remove.pm
index 3acc861..7cc06b1 100644
--- a/src/PMG/RuleDB/Remove.pm
+++ b/src/PMG/RuleDB/Remove.pm
@@ -209,7 +209,14 @@ sub execute {
return if !$found_mark;
}
- my $subgroups = $mod_group->subgroups ($targets);
+ my $subgroups;
+ if ($marks->{spaminfo}) {
+ # when there was a spam check in the rule, we might have different marks for
+ # different targets, so simply copy the mail for each target that matches
+ $subgroups = $mod_group->explode($targets);
+ } else {
+ $subgroups = $mod_group->subgroups ($targets);
+ }
my $html = PMG::Utils::subst_values($self->{text}, $vars);
@@ -263,7 +270,8 @@ sub execute {
$self->{message_seen} = 0;
- # since currently all marks are equal for all target, just use the first one
+ # if we only had a spam/virus check, the marks are identical
+ # otherwise we get a subgroup per target anyway
my $match_marks = $marks->{$tg->[0]};
$self->delete_marked_parts($queue, $entity, $html, $rtype, $match_marks, $rulename);
--
2.30.2
^ permalink raw reply [flat|nested] 20+ messages in thread* [pmg-devel] [PATCH pmg-api v2 10/10] pmgdb: extend dump output to include add/invert
2024-02-21 12:24 [pmg-devel] [PATCH pmg-api/docs/gui v2] implement and combination and inversion of groups and objects Dominik Csapak
` (8 preceding siblings ...)
2024-02-21 12:24 ` [pmg-devel] [PATCH pmg-api v2 09/10] RuleCache: implement and/invert for what matches Dominik Csapak
@ 2024-02-21 12:24 ` Dominik Csapak
2024-02-21 12:24 ` [pmg-devel] [PATCH pmg-docs v2 1/1] rule system: explain new and mode and invert flag Dominik Csapak
` (3 subsequent siblings)
13 siblings, 0 replies; 20+ messages in thread
From: Dominik Csapak @ 2024-02-21 12:24 UTC (permalink / raw)
To: pmg-devel
if a group type has and/invert set, add a line with that information
and for each object group add its mode too in parenthesis
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
src/PMG/CLI/pmgdb.pm | 38 ++++++++++++++++++++++++++++++++------
1 file changed, 32 insertions(+), 6 deletions(-)
diff --git a/src/PMG/CLI/pmgdb.pm b/src/PMG/CLI/pmgdb.pm
index 8368af8..cd94c23 100644
--- a/src/PMG/CLI/pmgdb.pm
+++ b/src/PMG/CLI/pmgdb.pm
@@ -40,6 +40,8 @@ sub print_objects {
sub print_rule {
my ($ruledb, $rule) = @_;
+ $ruledb->load_rule_attributes($rule);
+
my $direction = {
0 => 'in',
1 => 'out',
@@ -52,26 +54,50 @@ sub print_rule {
print "Found RULE $rule->{id} (prio: $rule->{priority}, $dir, $active): $rulename\n";
my $print_group = sub {
- my ($type, $og) = @_;
+ my ($type, $og, $print_mode) = @_;
my $oname = encode('UTF-8', $og->{name});
- print " FOUND $type GROUP $og->{id}: $oname\n";
+ my $mode = "";
+ if ($print_mode) {
+ my $and = $og->{and} // 0;
+ my $invert = $og->{invert} // 0;
+ $mode = " (and=$and, invert=$invert)";
+ }
+ print " FOUND $type GROUP $og->{id}${mode}: $oname\n";
print_objects($ruledb, $og);
};
+ my $print_type_mode = sub {
+ my ($type) = @_;
+ my $and = $rule->{"$type-and"};
+ my $invert = $rule->{"$type-invert"};
+ if (defined($and) || defined($invert)) {
+ my $print_type = uc($type);
+ print " $print_type mode: and=" . ($and // 0) . " invert=". ($invert // 0) . "\n";
+ }
+ };
+
my ($from, $to, $when, $what, $action) =
$ruledb->load_groups($rule);
+ $print_type_mode->("from") if scalar(@$from);
foreach my $og (@$from) {
- $print_group->("FROM", $og);
+ $ruledb->load_group_attributes($og);
+ $print_group->("FROM", $og, 1);
}
+ $print_type_mode->("to") if scalar(@$to);
foreach my $og (@$to) {
- $print_group->("TO", $og);
+ $ruledb->load_group_attributes($og);
+ $print_group->("TO", $og, 1);
}
+ $print_type_mode->("when") if scalar(@$when);
foreach my $og (@$when) {
- $print_group->("WHEN", $og);
+ $ruledb->load_group_attributes($og);
+ $print_group->("WHEN", $og, 1);
}
+ $print_type_mode->("what") if scalar(@$what);
foreach my $og (@$what) {
- $print_group->("WHAT", $og);
+ $ruledb->load_group_attributes($og);
+ $print_group->("WHAT", $og, 1);
}
foreach my $og (@$action) {
$print_group->("ACTION", $og);
--
2.30.2
^ permalink raw reply [flat|nested] 20+ messages in thread* [pmg-devel] [PATCH pmg-docs v2 1/1] rule system: explain new and mode and invert flag
2024-02-21 12:24 [pmg-devel] [PATCH pmg-api/docs/gui v2] implement and combination and inversion of groups and objects Dominik Csapak
` (9 preceding siblings ...)
2024-02-21 12:24 ` [pmg-devel] [PATCH pmg-api v2 10/10] pmgdb: extend dump output to include add/invert Dominik Csapak
@ 2024-02-21 12:24 ` Dominik Csapak
2024-02-21 12:24 ` [pmg-devel] [PATCH pmg-gui v2 1/2] rules: use tree panel instead of grouping feature of the grid Dominik Csapak
` (2 subsequent siblings)
13 siblings, 0 replies; 20+ messages in thread
From: Dominik Csapak @ 2024-02-21 12:24 UTC (permalink / raw)
To: pmg-devel
and adding that the previous behaviour is now the default.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
changes from v1:
* incorporated stoikos feedback
pmg-mail-filter.adoc | 40 +++++++++++++++++++++++++++++++++++++++-
1 file changed, 39 insertions(+), 1 deletion(-)
diff --git a/pmg-mail-filter.adoc b/pmg-mail-filter.adoc
index 57b8a15..9c0e2c4 100644
--- a/pmg-mail-filter.adoc
+++ b/pmg-mail-filter.adoc
@@ -63,7 +63,7 @@ Application of Rules
When there is more than one object category or multiple objects configured
within a single rule, the following logic is used to determine if the rule
-should be applied:
+should be applied by default:
* Within one category (WHAT/FROM/TO/WHEN), all objects are logical-or linked,
meaning that only one object of any one object group from the same category
@@ -75,6 +75,44 @@ should be applied:
When these conditions are met, all configured actions are executed.
+Alternatively, one can configure the 'mode' to 'any' (the default) or 'all' and
+set 'invert' (default off) per object group and per object category for each
+rule.
+
+When the mode is 'all' for a group, all objects within must match for the
+object group to count as a match. This can be helpful when one wants to match
+multiple conditions at the same time (e.g. file content-type and filename).
+
+When 'all' is set for a category of a rule, all object groups for that
+type must match for the type to match.
+
+When 'invert' is active on a group, the original result of the group will
+simply be inverted, so a match becomes a non-match and vice versa.
+
+The same is true for the object group types for rules.
+
+Special handling is done for WHAT matches that mark mail parts (e.g. filename)
+since that is not a simple yes/no match for the complete mail, but could be a
+match for each part of the e-mail (e.g. attachments, or parts of a multi-part
+e-mail).
+
+So for WHAT match object groups, the 'mode' and 'invert' is applied to
+the single parts of the e-mail, not the message as a whole.
+
+This means one has to be very careful with the 'invert' option, as previously
+not matching parts, will match when using 'invert' (e.g. an inverted filename
+matching will also mark non attachment parts of the mail).
+
+On the rule level, these marks of the parts will always be logical-or linked,
+this way even more scenarios can be represented.
+
+To make it a bit easier to understand, the options are combined to a single
+selection in the web ui:
+
+* Any must match => mode: 'any', invert: 'off'
+* All must match => mode: 'all', invert: 'off'
+* At least one must not match => mode: 'all', invert: 'on'
+* None must match => mode: 'any', invert: 'on'
[[pmg_mailfilter_action]]
'Action' - objects
--
2.30.2
^ permalink raw reply [flat|nested] 20+ messages in thread* [pmg-devel] [PATCH pmg-gui v2 1/2] rules: use tree panel instead of grouping feature of the grid
2024-02-21 12:24 [pmg-devel] [PATCH pmg-api/docs/gui v2] implement and combination and inversion of groups and objects Dominik Csapak
` (10 preceding siblings ...)
2024-02-21 12:24 ` [pmg-devel] [PATCH pmg-docs v2 1/1] rule system: explain new and mode and invert flag Dominik Csapak
@ 2024-02-21 12:24 ` Dominik Csapak
2024-02-21 17:42 ` Thomas Lamprecht
2024-02-21 12:24 ` [pmg-devel] [PATCH pmg-gui v2 2/2] rules/objects: add mode selector dropdown Dominik Csapak
2024-02-21 18:36 ` [pmg-devel] applied-partially: [PATCH pmg-api/docs/gui v2] implement and combination and inversion of groups and objects Stoiko Ivanov
13 siblings, 1 reply; 20+ messages in thread
From: Dominik Csapak @ 2024-02-21 12:24 UTC (permalink / raw)
To: pmg-devel
just in preparation for adding a columnf or the groups
will look similar (though not identical) to before, but this makes
the groups now real entries in the grid, which means we can have
content in additional columns
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
css/ext6-pmg.css | 7 ++++++
js/RuleInfo.js | 59 +++++++++++++++++++++++++++++++-----------------
2 files changed, 45 insertions(+), 21 deletions(-)
diff --git a/css/ext6-pmg.css b/css/ext6-pmg.css
index 2f999f4..1c43b09 100644
--- a/css/ext6-pmg.css
+++ b/css/ext6-pmg.css
@@ -263,3 +263,10 @@ a.download {
color: inherit;
text-decoration: none;
}
+
+.pmx-rule-tree .x-tree-icon,
+.pmx-rule-tree .x-tree-elbow,
+.pmx-rule-tree .x-tree-elbow-end {
+ display: none;
+ width: 0px;
+}
diff --git a/js/RuleInfo.js b/js/RuleInfo.js
index 8f39695..c29c0ca 100644
--- a/js/RuleInfo.js
+++ b/js/RuleInfo.js
@@ -88,7 +88,11 @@ Ext.define('PMG.RuleInfo', {
} else {
viewmodel.set('selectedRule', ruledata);
- var data = [];
+ let data = {
+ leaf: false,
+ 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; }
@@ -111,12 +115,25 @@ Ext.define('PMG.RuleInfo', {
},
});
store.load();
+
+ let group = {
+ name: oc,
+ oclass: oc,
+ type: true,
+ leaf: false,
+ expanded: true,
+ expandable: false,
+ children: [],
+ };
Ext.Array.each(ruledata[oc], function(og) {
- data.push({ oclass: oc, name: og.name, typeid: og.id });
+ group.children.push({ oclass: oc, name: og.name, typeid: og.id, leaf: true });
});
- });
- viewmodel.get('objects').setData(data);
+ if (group.children.length) {
+ data.children.push(group);
+ }
+ });
+ viewmodel.get('objects').setRoot(data);
}
},
@@ -146,7 +163,7 @@ Ext.define('PMG.RuleInfo', {
},
control: {
- 'grid[reference=usedobjects]': {
+ 'treepanel[reference=usedobjects]': {
drop: 'addDrop',
},
'tabpanel[reference=availobjects] > grid': {
@@ -162,6 +179,7 @@ Ext.define('PMG.RuleInfo', {
stores: {
objects: {
+ type: 'tree',
fields: ['oclass', 'name', 'typeid'],
groupField: 'oclass',
sorters: 'name',
@@ -252,21 +270,16 @@ Ext.define('PMG.RuleInfo', {
],
},
{
- xtype: 'grid',
+ xtype: 'treepanel',
reference: 'usedobjects',
hidden: true,
emptyText: gettext('No Objects'),
- features: [{
- id: 'group',
- ftype: 'grouping',
- enableGroupingMenu: false,
- collapsible: false,
- groupHeaderTpl: [
- '{[PMG.Utils.format_oclass(values.name)]}',
- ],
- }],
title: gettext('Used Objects'),
+ rootVisible: false,
+ useArrows: true,
+ rowLines: true,
+ userCls: 'pmx-rule-tree',
viewConfig: {
plugins: {
@@ -285,14 +298,17 @@ Ext.define('PMG.RuleInfo', {
},
columns: [
- {
- header: gettext('Type'),
- dataIndex: 'oclass',
- hidden: true,
- },
{
header: gettext('Name'),
dataIndex: 'name',
+ xtype: 'treecolumn',
+ renderer: PMG.Utils.format_oclass,
+ sorter: function(a, b) {
+ if (a.data.type && b.data.type) {
+ return a.data.oclass.localeCompare(b.data.oclass);
+ }
+ return a.data.text.localeCompare(b.data.text);
+ },
flex: 1,
},
{
@@ -302,8 +318,9 @@ Ext.define('PMG.RuleInfo', {
width: 40,
items: [
{
- iconCls: 'fa fa-fw fa-minus-circle',
tooltip: gettext('Remove'),
+ isActionDisabled: (v, rI, cI, i, rec) => rec.data.type,
+ getClass: (v, mD, { data }) => data.type ? 'pmx-hidden' : 'fa fa-fw fa-minus-circle',
handler: 'removeIconClick',
},
],
--
2.30.2
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [pmg-devel] [PATCH pmg-gui v2 1/2] rules: use tree panel instead of grouping feature of the grid
2024-02-21 12:24 ` [pmg-devel] [PATCH pmg-gui v2 1/2] rules: use tree panel instead of grouping feature of the grid Dominik Csapak
@ 2024-02-21 17:42 ` Thomas Lamprecht
0 siblings, 0 replies; 20+ messages in thread
From: Thomas Lamprecht @ 2024-02-21 17:42 UTC (permalink / raw)
To: Dominik Csapak, pmg-devel
Am 21/02/2024 um 13:24 schrieb Dominik Csapak:
> just in preparation for adding a columnf or the groups
typo (can be amended on applying) s/columnf/column
> will look similar (though not identical) to before, but this makes
> the groups now real entries in the grid, which means we can have
> content in additional columns
looks OK, as nit: while I do not find it bad that the object-group "heading"
has the same text color as the used objects, some more visual separation might
be good.
For starters, one could maybe check if the cell top/bottom padding should
be increased a bit, as that was a bit higher in the old implementation.
but can be fine tuned after wards, doesn't need any v3 for just this.
^ permalink raw reply [flat|nested] 20+ messages in thread
* [pmg-devel] [PATCH pmg-gui v2 2/2] rules/objects: add mode selector dropdown
2024-02-21 12:24 [pmg-devel] [PATCH pmg-api/docs/gui v2] implement and combination and inversion of groups and objects Dominik Csapak
` (11 preceding siblings ...)
2024-02-21 12:24 ` [pmg-devel] [PATCH pmg-gui v2 1/2] rules: use tree panel instead of grouping feature of the grid Dominik Csapak
@ 2024-02-21 12:24 ` Dominik Csapak
2024-02-21 18:31 ` Thomas Lamprecht
2024-02-21 18:36 ` [pmg-devel] applied-partially: [PATCH pmg-api/docs/gui v2] implement and combination and inversion of groups and objects Stoiko Ivanov
13 siblings, 1 reply; 20+ messages in thread
From: Dominik Csapak @ 2024-02-21 12:24 UTC (permalink / raw)
To: pmg-devel
for objects and object types in rules. We add a simple dropdown for the
'and' and 'invert' flags, to be somewhat consistent with the
notification matchers from pve and to make the wording more clear than
simple and/invert.
For What matches add a special warning hint, since that behaves a bit
special because of the mail parts.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
js/Makefile | 1 +
js/ObjectGroup.js | 64 ++++++++++++++++++++++++++++++++--
js/ObjectGroupConfiguration.js | 4 +++
js/RuleInfo.js | 44 +++++++++++++++++++++++
js/form/ModeSelector.js | 11 ++++++
5 files changed, 122 insertions(+), 2 deletions(-)
create mode 100644 js/form/ModeSelector.js
diff --git a/js/Makefile b/js/Makefile
index 78f2b57..4267092 100644
--- a/js/Makefile
+++ b/js/Makefile
@@ -2,6 +2,7 @@ include ../defines.mk
JSSRC= \
Utils.js \
+ form/ModeSelector.js \
FilterProxy.js \
LoginView.js \
RoleSelector.js \
diff --git a/js/ObjectGroup.js b/js/ObjectGroup.js
index 2223ffa..b91bf97 100644
--- a/js/ObjectGroup.js
+++ b/js/ObjectGroup.js
@@ -10,6 +10,7 @@ Ext.define('PMG.ObjectGroup', {
showDirection: false, // only important for SMTP Whitelist
ogdata: undefined,
+ oclass: undefined,
emptyText: gettext('Please select an object.'),
@@ -32,10 +33,14 @@ Ext.define('PMG.ObjectGroup', {
setObjectInfo: function(ogdata) {
let me = this;
+ let mode = ogdata.invert ? 'not' : '';
+ mode += ogdata.and ? 'all' : 'any';
+
me.ogdata = ogdata;
if (me.ogdata === undefined) {
me.down('#oginfo').update(me.emptyText);
+ me.down('#modeBox').setHidden(true);
} else {
let html = '<b>' + Ext.String.htmlEncode(me.ogdata.name) + '</b>';
html += "<br><br>";
@@ -43,6 +48,12 @@ Ext.define('PMG.ObjectGroup', {
me.down('#oginfo').update(html);
me.down('#ogdata').setHidden(false);
+ let modeSelector = me.down('#modeSelector');
+ modeSelector.suspendEvents();
+ me.down('#modeSelector').setValue(mode);
+ modeSelector.resumeEvents();
+ me.down('#modeBox').setHidden(false);
+ me.down('#whatWarning').setHidden(me.oclass !== 'what');
}
},
@@ -205,13 +216,62 @@ Ext.define('PMG.ObjectGroup', {
me.dockedItems.push({
dock: 'top',
border: 1,
- layout: 'anchor',
+ layout: 'hbox',
hidden: !!me.hideGroupInfo,
itemId: 'ogdata',
items: [
+ {
+ xtype: 'container',
+ itemId: 'modeBox',
+ hidden: true,
+ width: 220,
+ padding: 10,
+ layout: {
+ type: 'vbox',
+ align: 'stretch',
+ },
+ items: [
+ {
+ xtype: 'box',
+ html: `<b>${gettext("Match if")}</b>`,
+ },
+ {
+ xtype: 'pmgModeSelector',
+ itemId: 'modeSelector',
+ padding: '10 0 0 0',
+ listeners: {
+ change: function(_field, value) {
+ let invert = value.startsWith('not');
+ let and = value.endsWith('all');
+
+ let url = `${me.baseurl}/config`;
+
+ Proxmox.Utils.API2Request({
+ url,
+ method: 'PUT',
+ params: {
+ and: and ? 1 : 0,
+ invert: invert ? 1 : 0,
+ },
+ success: function() {
+ me.fireEvent('modeUpdate', me);
+ },
+ });
+ },
+ },
+ },
+ {
+ xtype: 'displayfield',
+ itemId: 'whatWarning',
+ hidden: true,
+ value: gettext("Caution: What Objects match per mail part, so be careful with any option besides 'Any matches'."),
+ userCls: 'pmx-hint',
+ },
+ ],
+ },
{
xtype: 'component',
- anchor: '100%',
+ flex: 1,
itemId: 'oginfo',
style: { 'white-space': 'pre' },
padding: 10,
diff --git a/js/ObjectGroupConfiguration.js b/js/ObjectGroupConfiguration.js
index 1d72851..f59f5ed 100644
--- a/js/ObjectGroupConfiguration.js
+++ b/js/ObjectGroupConfiguration.js
@@ -30,6 +30,7 @@ Ext.define('PMG.ObjectGroupConfiguration', {
var right = Ext.create('PMG.ObjectGroup', {
otype_list: me.otype_list,
+ oclass: me.ogclass,
border: false,
region: 'center',
listeners: {
@@ -40,6 +41,9 @@ Ext.define('PMG.ObjectGroupConfiguration', {
left.run_editor();
}
},
+ modeUpdate: function() {
+ left.reload();
+ },
},
});
diff --git a/js/RuleInfo.js b/js/RuleInfo.js
index c29c0ca..d53c1c5 100644
--- a/js/RuleInfo.js
+++ b/js/RuleInfo.js
@@ -120,6 +120,8 @@ Ext.define('PMG.RuleInfo', {
name: oc,
oclass: oc,
type: true,
+ invert: ruledata[`${oc}-invert`],
+ and: ruledata[`${oc}-and`],
leaf: false,
expanded: true,
expandable: false,
@@ -162,6 +164,25 @@ Ext.define('PMG.RuleInfo', {
return true;
},
+ updateMode: function(field, value) {
+ let me = this;
+ let vm = me.getViewModel();
+ let oclass = field.getWidgetRecord().data.oclass;
+
+ let params = {};
+ params[`${oclass}-invert`] = value.startsWith('not') ? 1 : 0;
+ params[`${oclass}-and`] = value.endsWith('all') ? 1 : 0;
+
+ Proxmox.Utils.API2Request({
+ url: `${vm.get('baseurl')}/config`,
+ method: 'PUT',
+ params,
+ success: function() {
+ me.reload();
+ },
+ });
+ },
+
control: {
'treepanel[reference=usedobjects]': {
drop: 'addDrop',
@@ -169,6 +190,9 @@ Ext.define('PMG.RuleInfo', {
'tabpanel[reference=availobjects] > grid': {
drop: 'removeDrop',
},
+ 'pmgModeSelector': {
+ change: 'updateMode',
+ },
},
},
@@ -311,6 +335,26 @@ Ext.define('PMG.RuleInfo', {
},
flex: 1,
},
+ {
+ header: gettext('Match if'),
+ xtype: 'widgetcolumn',
+ width: 200,
+ widget: {
+ xtype: 'pmgModeSelector',
+ },
+ onWidgetAttach: function(col, widget, rec) {
+ if (rec.data.type && rec.data.oclass !== 'action') {
+ let mode = rec.data.invert ? 'not' : '';
+ mode += rec.data.and ? 'all' : 'any';
+ widget.suspendEvents();
+ widget.setValue(mode);
+ widget.resumeEvents();
+ widget.setHidden(false);
+ } else {
+ widget.setHidden(true);
+ }
+ },
+ },
{
text: '',
xtype: 'actioncolumn',
diff --git a/js/form/ModeSelector.js b/js/form/ModeSelector.js
new file mode 100644
index 0000000..9c9275b
--- /dev/null
+++ b/js/form/ModeSelector.js
@@ -0,0 +1,11 @@
+Ext.define('PMG.ModeSelector', {
+ extend: 'Proxmox.form.KVComboBox',
+ alias: 'widget.pmgModeSelector',
+
+ comboItems: [
+ ['all', gettext('All match')],
+ ['any', gettext('Any matches')],
+ ['notall', gettext('At least one does not match')],
+ ['notany', gettext('None matches')],
+ ],
+});
--
2.30.2
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [pmg-devel] [PATCH pmg-gui v2 2/2] rules/objects: add mode selector dropdown
2024-02-21 12:24 ` [pmg-devel] [PATCH pmg-gui v2 2/2] rules/objects: add mode selector dropdown Dominik Csapak
@ 2024-02-21 18:31 ` Thomas Lamprecht
0 siblings, 0 replies; 20+ messages in thread
From: Thomas Lamprecht @ 2024-02-21 18:31 UTC (permalink / raw)
To: Dominik Csapak, pmg-devel
Am 21/02/2024 um 13:24 schrieb Dominik Csapak:
> for objects and object types in rules. We add a simple dropdown for the
> 'and' and 'invert' flags, to be somewhat consistent with the
> notification matchers from pve and to make the wording more clear than
> simple and/invert.
>
> For What matches add a special warning hint, since that behaves a bit
> special because of the mail parts.
>
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> js/Makefile | 1 +
> js/ObjectGroup.js | 64 ++++++++++++++++++++++++++++++++--
> js/ObjectGroupConfiguration.js | 4 +++
> js/RuleInfo.js | 44 +++++++++++++++++++++++
> js/form/ModeSelector.js | 11 ++++++
> 5 files changed, 122 insertions(+), 2 deletions(-)
> create mode 100644 js/form/ModeSelector.js
>
> diff --git a/js/Makefile b/js/Makefile
> index 78f2b57..4267092 100644
> --- a/js/Makefile
> +++ b/js/Makefile
> @@ -2,6 +2,7 @@ include ../defines.mk
>
> JSSRC= \
> Utils.js \
> + form/ModeSelector.js \
Please codify in the file and class name which mode this is for, as of now
the name is rather overly generic.
Something like "MatchModeSelector" would be already better IMO.
> FilterProxy.js \
> LoginView.js \
> RoleSelector.js \
> diff --git a/js/ObjectGroup.js b/js/ObjectGroup.js
> index 2223ffa..b91bf97 100644
> --- a/js/ObjectGroup.js
> +++ b/js/ObjectGroup.js
> @@ -10,6 +10,7 @@ Ext.define('PMG.ObjectGroup', {
> showDirection: false, // only important for SMTP Whitelist
>
> ogdata: undefined,
> + oclass: undefined,
not a fan of those needlessly abbreviated config keys, this, and the existing
one, just makes the code harder to read.
>
> emptyText: gettext('Please select an object.'),
>
> @@ -32,10 +33,14 @@ Ext.define('PMG.ObjectGroup', {
> setObjectInfo: function(ogdata) {
> let me = this;
>
> + let mode = ogdata.invert ? 'not' : '';
> + mode += ogdata.and ? 'all' : 'any';
> +
> me.ogdata = ogdata;
>
> if (me.ogdata === undefined) {
> me.down('#oginfo').update(me.emptyText);
> + me.down('#modeBox').setHidden(true);
> } else {
> let html = '<b>' + Ext.String.htmlEncode(me.ogdata.name) + '</b>';
> html += "<br><br>";
> @@ -43,6 +48,12 @@ Ext.define('PMG.ObjectGroup', {
>
> me.down('#oginfo').update(html);
> me.down('#ogdata').setHidden(false);
> + let modeSelector = me.down('#modeSelector');
> + modeSelector.suspendEvents();
> + me.down('#modeSelector').setValue(mode);
> + modeSelector.resumeEvents();
> + me.down('#modeBox').setHidden(false);
> + me.down('#whatWarning').setHidden(me.oclass !== 'what');
> }
> },
>
> @@ -205,13 +216,62 @@ Ext.define('PMG.ObjectGroup', {
> me.dockedItems.push({
> dock: 'top',
> border: 1,
> - layout: 'anchor',
> + layout: 'hbox',
> hidden: !!me.hideGroupInfo,
> itemId: 'ogdata',
> items: [
> + {
> + xtype: 'container',
> + itemId: 'modeBox',
> + hidden: true,
> + width: 220,
> + padding: 10,
> + layout: {
> + type: 'vbox',
> + align: 'stretch',
> + },
> + items: [
> + {
> + xtype: 'box',
> + html: `<b>${gettext("Match if")}</b>`,
> + },
> + {
> + xtype: 'pmgModeSelector',
> + itemId: 'modeSelector',
> + padding: '10 0 0 0',
> + listeners: {
> + change: function(_field, value) {
> + let invert = value.startsWith('not');
> + let and = value.endsWith('all');
> +
> + let url = `${me.baseurl}/config`;
nit: I'd rather avoid the extra variable here, just an inline:
url: `${me.baseurl}/config`,
> +
> + Proxmox.Utils.API2Request({
> + url,
> + method: 'PUT',
> + params: {
> + and: and ? 1 : 0,
> + invert: invert ? 1 : 0,
nit: while the separate variables have IMO a value here, the ternary
could be used on assignment to and/invert already, then one could just
use `param: { and, invert }` here (maybe not in a single line)
> + },
> + success: function() {
> + me.fireEvent('modeUpdate', me);
> + },
tiny nit: could be an arrow fn:
success: () => me.fireEvent('modeUpdate', me),
> + });
> + },
> + },
> + },
> + {
> + xtype: 'displayfield',
> + itemId: 'whatWarning',
> + hidden: true,
> + value: gettext("Caution: What Objects match per mail part, so be careful with any option besides 'Any matches'."),
Hmm, this is a bit hard to word better, but maybe something slightly adapted
like:
"Caution: 'What Objects' match each mail part separately, so be careful with any option besides 'Any matches'."
Also, we could hide this is 'Any matches' is selected, and making it span over
all columns would make it look better too. Maybe in some bottom docked bar,
as otherwise it can flow over the description.
btw. why does the left store reloads after every match selection change?
Maybe an explicit reload button there (simple icon without text) could be better
for that?
> + userCls: 'pmx-hint',
> + },
> + ],
> + },
> {
> xtype: 'component',
> - anchor: '100%',
> + flex: 1,
> itemId: 'oginfo',
> style: { 'white-space': 'pre' },
> padding: 10,
> diff --git a/js/ObjectGroupConfiguration.js b/js/ObjectGroupConfiguration.js
> index 1d72851..f59f5ed 100644
> --- a/js/ObjectGroupConfiguration.js
> +++ b/js/ObjectGroupConfiguration.js
> @@ -30,6 +30,7 @@ Ext.define('PMG.ObjectGroupConfiguration', {
>
> var right = Ext.create('PMG.ObjectGroup', {
> otype_list: me.otype_list,
> + oclass: me.ogclass,
> border: false,
> region: 'center',
> listeners: {
> @@ -40,6 +41,9 @@ Ext.define('PMG.ObjectGroupConfiguration', {
> left.run_editor();
> }
> },
> + modeUpdate: function() {
> + left.reload();
> + },
nit, could be an arrow fn:
modeUpdate: () => left.reload(),
> },
> });
>
> diff --git a/js/RuleInfo.js b/js/RuleInfo.js
> index c29c0ca..d53c1c5 100644
> --- a/js/RuleInfo.js
> +++ b/js/RuleInfo.js
> @@ -120,6 +120,8 @@ Ext.define('PMG.RuleInfo', {
> name: oc,
> oclass: oc,
> type: true,
> + invert: ruledata[`${oc}-invert`],
> + and: ruledata[`${oc}-and`],
> leaf: false,
> expanded: true,
> expandable: false,
> @@ -162,6 +164,25 @@ Ext.define('PMG.RuleInfo', {
> return true;
> },
>
> + updateMode: function(field, value) {
> + let me = this;
> + let vm = me.getViewModel();
> + let oclass = field.getWidgetRecord().data.oclass;
> +
> + let params = {};
> + params[`${oclass}-invert`] = value.startsWith('not') ? 1 : 0;
> + params[`${oclass}-and`] = value.endsWith('all') ? 1 : 0;
> +
> + Proxmox.Utils.API2Request({
> + url: `${vm.get('baseurl')}/config`,
> + method: 'PUT',
> + params,
> + success: function() {
> + me.reload();
> + },
nit, could be an arrow fn:
success: () => me.reload(),
> + });
> + },
> +
> control: {
> 'treepanel[reference=usedobjects]': {
> drop: 'addDrop',
> @@ -169,6 +190,9 @@ Ext.define('PMG.RuleInfo', {
> 'tabpanel[reference=availobjects] > grid': {
> drop: 'removeDrop',
> },
> + 'pmgModeSelector': {
> + change: 'updateMode',
> + },
> },
> },
>
> @@ -311,6 +335,26 @@ Ext.define('PMG.RuleInfo', {
> },
> flex: 1,
> },
> + {
> + header: gettext('Match if'),
> + xtype: 'widgetcolumn',
> + width: 200,
> + widget: {
> + xtype: 'pmgModeSelector',
> + },
> + onWidgetAttach: function(col, widget, rec) {
> + if (rec.data.type && rec.data.oclass !== 'action') {
> + let mode = rec.data.invert ? 'not' : '';
> + mode += rec.data.and ? 'all' : 'any';
> + widget.suspendEvents();
> + widget.setValue(mode);
> + widget.resumeEvents();
> + widget.setHidden(false);
> + } else {
> + widget.setHidden(true);
> + }
> + },
> + },
> {
> text: '',
> xtype: 'actioncolumn',
^ permalink raw reply [flat|nested] 20+ messages in thread
* [pmg-devel] applied-partially: [PATCH pmg-api/docs/gui v2] implement and combination and inversion of groups and objects
2024-02-21 12:24 [pmg-devel] [PATCH pmg-api/docs/gui v2] implement and combination and inversion of groups and objects Dominik Csapak
` (12 preceding siblings ...)
2024-02-21 12:24 ` [pmg-devel] [PATCH pmg-gui v2 2/2] rules/objects: add mode selector dropdown Dominik Csapak
@ 2024-02-21 18:36 ` Stoiko Ivanov
13 siblings, 0 replies; 20+ messages in thread
From: Stoiko Ivanov @ 2024-02-21 18:36 UTC (permalink / raw)
To: Dominik Csapak; +Cc: pmg-devel
huge thanks for the series and the quick iteration on my nits!
applied the api and docs patches, left the gui patches for now after a
talk with Thomas off-list.
On Wed, 21 Feb 2024 13:24:26 +0100
Dominik Csapak <d.csapak@proxmox.com> wrote:
> This series is the backend part of the and/inversion addition to the
> rule system.
>
> The gui change is rather simple, just one additional drop down for
> groups + one for each type in each rule.
>
> changes from v1:
> * rebase on master
> * include new tables in cluster sync + backup
> * incorporate stoikos feedback
> * improved commit messages
> more details in the relevant patches
>
> changes from rfc:
> * added docs + gui
> * some minor bugfixes
> * fixed the api (forgot to add the info to the GET calls in rfc)
> * changed output for `pmgdb dump` so we get that info in the pmgreport too
>
> pmg-api:
>
> Dominik Csapak (10):
> RuleCache: reorganize how we gather marks and spaminfo
> api: refactor rule parameters
> add objectgroup attributes and/invert
> add rule attributes and/invert (for each relevant type)
> RuleCache: load rule/objectgroup attributes from database
> RuleCache: implement and/invert for when/from/to
> MailQueue: return maximum AID
> ModGroup: add possibility to explode to all targets
> RuleCache: implement and/invert for what matches
> pmgdb: extend dump output to include add/invert
>
> src/PMG/API2/ObjectGroupHelpers.pm | 51 +++++-
> src/PMG/API2/RuleDB.pm | 23 +--
> src/PMG/API2/Rules.pm | 94 ++++++++--
> src/PMG/Backup.pm | 2 +
> src/PMG/CLI/pmgdb.pm | 38 +++-
> src/PMG/Cluster.pm | 4 +
> src/PMG/DBTools.pm | 32 ++++
> src/PMG/MailQueue.pm | 4 +-
> src/PMG/ModGroup.pm | 17 ++
> src/PMG/RuleCache.pm | 274 +++++++++++++++++++++++-----
> src/PMG/RuleDB.pm | 278 +++++++++++++++++++++++------
> src/PMG/RuleDB/Remove.pm | 28 ++-
> src/PMG/Utils.pm | 2 +
> src/bin/pmg-smtp-filter | 23 +--
> 14 files changed, 701 insertions(+), 169 deletions(-)
>
> pmg-docs:
>
> Dominik Csapak (1):
> rule system: explain new and mode and invert flag
>
> pmg-mail-filter.adoc | 40 +++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 39 insertions(+), 1 deletion(-)
>
> pmg-gui:
>
> Dominik Csapak (2):
> rules: use tree panel instead of grouping feature of the grid
> rules/objects: add mode selector dropdown
>
> css/ext6-pmg.css | 7 +++
> js/Makefile | 1 +
> js/ObjectGroup.js | 64 +++++++++++++++++++-
> js/ObjectGroupConfiguration.js | 4 ++
> js/RuleInfo.js | 103 ++++++++++++++++++++++++++-------
> js/form/ModeSelector.js | 11 ++++
> 6 files changed, 167 insertions(+), 23 deletions(-)
> create mode 100644 js/form/ModeSelector.js
>
^ permalink raw reply [flat|nested] 20+ messages in thread