* [pmg-devel] [PATCH pmg-api v2 1/5] api: statistics: remove unneeded RESTEnvironment
2021-01-21 15:51 [pmg-devel] [PATCH pmg-api v2 0/5] allow / in local part of pmg-email-address Stoiko Ivanov
@ 2021-01-21 15:51 ` Stoiko Ivanov
2021-01-27 8:34 ` [pmg-devel] applied: " Thomas Lamprecht
2021-01-21 15:51 ` [pmg-devel] [PATCH pmg-api v2 2/5] api: statistics: refactor return for detail calls Stoiko Ivanov
` (4 subsequent siblings)
5 siblings, 1 reply; 11+ messages in thread
From: Stoiko Ivanov @ 2021-01-21 15:51 UTC (permalink / raw)
To: pmg-devel
none of the API calls in PMG::API2::Statistics use the
RESTEnvironment - so remove the unused code.
Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
src/PMG/API2/Statistics.pm | 34 ----------------------------------
1 file changed, 34 deletions(-)
diff --git a/src/PMG/API2/Statistics.pm b/src/PMG/API2/Statistics.pm
index 7eb8d18..adc7650 100644
--- a/src/PMG/API2/Statistics.pm
+++ b/src/PMG/API2/Statistics.pm
@@ -238,9 +238,6 @@ __PACKAGE__->register_method ({
code => sub {
my ($param) = @_;
- my $restenv = PMG::RESTEnvironment->get();
- my $cinfo = $restenv->{cinfo};
-
my ($start, $end) = $extract_start_end->($param);
my $cfg = PMG::Config->new();
@@ -383,9 +380,6 @@ __PACKAGE__->register_method ({
code => sub {
my ($param) = @_;
- my $restenv = PMG::RESTEnvironment->get();
- my $cinfo = $restenv->{cinfo};
-
my ($start, $end) = $extract_start_end->($param);
my $stat = PMG::Statistic->new($start, $end);
@@ -530,9 +524,6 @@ __PACKAGE__->register_method ({
code => sub {
my ($param) = @_;
- my $restenv = PMG::RESTEnvironment->get();
- my $cinfo = $restenv->{cinfo};
-
my ($start, $end) = $extract_start_end->($param);
my $cfg = PMG::Config->new();
@@ -684,9 +675,6 @@ __PACKAGE__->register_method ({
code => sub {
my ($param) = @_;
- my $restenv = PMG::RESTEnvironment->get();
- my $cinfo = $restenv->{cinfo};
-
my ($start, $end) = $extract_start_end->($param);
my $stat = PMG::Statistic->new($start, $end);
@@ -791,9 +779,6 @@ __PACKAGE__->register_method ({
code => sub {
my ($param) = @_;
- my $restenv = PMG::RESTEnvironment->get();
- my $cinfo = $restenv->{cinfo};
-
my ($start, $end) = $extract_start_end->($param);
my $stat = PMG::Statistic->new($start, $end);
@@ -910,8 +895,6 @@ __PACKAGE__->register_method ({
code => sub {
my ($param) = @_;
- my $restenv = PMG::RESTEnvironment->get();
-
my $hours = $param->{hours} // 12;
my $span = $param->{timespan} // 1800;
@@ -972,8 +955,6 @@ __PACKAGE__->register_method ({
code => sub {
my ($param) = @_;
- my $restenv = PMG::RESTEnvironment->get();
-
my $hours = $param->{hours} // 12;
my $limit = $param->{limit} // 5;
@@ -1071,9 +1052,6 @@ __PACKAGE__->register_method ({
code => sub {
my ($param) = @_;
- my $restenv = PMG::RESTEnvironment->get();
- my $cinfo = $restenv->{cinfo};
-
my ($start, $end) = $extract_start_end->($param);
my $span = $param->{timespan} // 3600;
@@ -1138,9 +1116,6 @@ __PACKAGE__->register_method ({
code => sub {
my ($param) = @_;
- my $restenv = PMG::RESTEnvironment->get();
- my $cinfo = $restenv->{cinfo};
-
my ($start, $end) = $extract_start_end->($param);
my $stat = PMG::Statistic->new($start, $end);
@@ -1185,9 +1160,6 @@ __PACKAGE__->register_method ({
code => sub {
my ($param) = @_;
- my $restenv = PMG::RESTEnvironment->get();
- my $cinfo = $restenv->{cinfo};
-
my ($start, $end) = $extract_start_end->($param);
my $stat = PMG::Statistic->new($start, $end);
@@ -1284,9 +1256,6 @@ __PACKAGE__->register_method ({
code => sub {
my ($param) = @_;
- my $restenv = PMG::RESTEnvironment->get();
- my $cinfo = $restenv->{cinfo};
-
my ($start, $end) = $extract_start_end->($param);
my $stat = PMG::Statistic->new($start, $end);
@@ -1345,9 +1314,6 @@ __PACKAGE__->register_method ({
code => sub {
my ($param) = @_;
- my $restenv = PMG::RESTEnvironment->get();
- my $cinfo = $restenv->{cinfo};
-
my ($start, $end) = $extract_start_end->($param);
my $span = $param->{timespan} // 3600;
--
2.20.1
^ permalink raw reply [flat|nested] 11+ messages in thread* [pmg-devel] [PATCH pmg-api v2 2/5] api: statistics: refactor return for detail calls
2021-01-21 15:51 [pmg-devel] [PATCH pmg-api v2 0/5] allow / in local part of pmg-email-address Stoiko Ivanov
2021-01-21 15:51 ` [pmg-devel] [PATCH pmg-api v2 1/5] api: statistics: remove unneeded RESTEnvironment Stoiko Ivanov
@ 2021-01-21 15:51 ` Stoiko Ivanov
2021-01-27 8:34 ` [pmg-devel] applied: " Thomas Lamprecht
2021-01-21 15:51 ` [pmg-devel] [PATCH pmg-api v2 3/5] api: statistics: refactor " Stoiko Ivanov
` (3 subsequent siblings)
5 siblings, 1 reply; 11+ messages in thread
From: Stoiko Ivanov @ 2021-01-21 15:51 UTC (permalink / raw)
To: pmg-devel
all api methods returning information for a particular sender,
receiver or contact have similar returns.
This commit pulls the common ones out into a sub like the common method
parameters in $default_properties.
Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
v1->v2:
* keep the common properties in a sub instead of a hash (for consistency
with $default_properties)
src/PMG/API2/Statistics.pm | 109 ++++++++++++++-----------------------
1 file changed, 40 insertions(+), 69 deletions(-)
diff --git a/src/PMG/API2/Statistics.pm b/src/PMG/API2/Statistics.pm
index adc7650..064865d 100644
--- a/src/PMG/API2/Statistics.pm
+++ b/src/PMG/API2/Statistics.pm
@@ -257,6 +257,40 @@ __PACKAGE__->register_method ({
return $res;
}});
+my $detail_return_properties = sub {
+ my ($prop) = @_;
+
+ $prop //= {};
+
+ $prop->{time} = {
+ description => "Receive time stamp",
+ type => 'integer',
+ };
+
+ $prop->{bytes} = {
+ description => "Mail traffic (Bytes).",
+ type => 'number',
+ };
+
+ $prop->{blocked} = {
+ description => "Mail was blocked.",
+ type => 'boolean',
+ };
+
+ $prop->{spamlevel} = {
+ description => "Spam score.",
+ type => 'number',
+ };
+
+ $prop->{virusinfo} = {
+ description => "Virus name.",
+ type => 'string',
+ optional => 1,
+ };
+
+ return $prop;
+};
+
__PACKAGE__->register_method ({
name => 'contactdetails',
path => 'contact/{contact}',
@@ -282,33 +316,12 @@ __PACKAGE__->register_method ({
type => 'array',
items => {
type => "object",
- properties => {
- time => {
- description => "Receive time stamp",
- type => 'integer',
- },
+ properties => $detail_return_properties->({
sender => {
description => "Sender email.",
type => 'string',
},
- bytes => {
- description => "Mail traffic (Bytes).",
- type => 'number',
- },
- blocked => {
- description => "Mail was blocked.",
- type => 'boolean',
- },
- spamlevel => {
- description => "Spam score.",
- type => 'number',
- },
- virusinfo => {
- description => "Virus name.",
- type => 'string',
- optional => 1,
- },
- },
+ }),
},
},
code => sub {
@@ -421,33 +434,12 @@ __PACKAGE__->register_method ({
type => 'array',
items => {
type => "object",
- properties => {
- time => {
- description => "Receive time stamp",
- type => 'integer',
- },
+ properties => $detail_return_properties->({
receiver => {
description => "Receiver email.",
type => 'string',
},
- bytes => {
- description => "Mail traffic (Bytes).",
- type => 'number',
- },
- blocked => {
- description => "Mail was blocked.",
- type => 'boolean',
- },
- spamlevel => {
- description => "Spam score.",
- type => 'number',
- },
- virusinfo => {
- description => "Virus name.",
- type => 'string',
- optional => 1,
- },
- },
+ }),
},
},
code => sub {
@@ -568,33 +560,12 @@ __PACKAGE__->register_method ({
type => 'array',
items => {
type => "object",
- properties => {
- time => {
- description => "Receive time stamp",
- type => 'integer',
- },
+ properties => $detail_return_properties->({
sender => {
description => "Sender email.",
type => 'string',
},
- bytes => {
- description => "Mail traffic (Bytes).",
- type => 'number',
- },
- blocked => {
- description => "Mail was blocked.",
- type => 'boolean',
- },
- spamlevel => {
- description => "Spam score.",
- type => 'number',
- },
- virusinfo => {
- description => "Virus name.",
- type => 'string',
- optional => 1,
- },
- },
+ }),
},
},
code => sub {
--
2.20.1
^ permalink raw reply [flat|nested] 11+ messages in thread* [pmg-devel] applied: [PATCH pmg-api v2 2/5] api: statistics: refactor return for detail calls
2021-01-21 15:51 ` [pmg-devel] [PATCH pmg-api v2 2/5] api: statistics: refactor return for detail calls Stoiko Ivanov
@ 2021-01-27 8:34 ` Thomas Lamprecht
0 siblings, 0 replies; 11+ messages in thread
From: Thomas Lamprecht @ 2021-01-27 8:34 UTC (permalink / raw)
To: Stoiko Ivanov, pmg-devel
On 21.01.21 16:51, Stoiko Ivanov wrote:
> all api methods returning information for a particular sender,
> receiver or contact have similar returns.
>
> This commit pulls the common ones out into a sub like the common method
> parameters in $default_properties.
>
> Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
> ---
> v1->v2:
> * keep the common properties in a sub instead of a hash (for consistency
> with $default_properties)
> src/PMG/API2/Statistics.pm | 109 ++++++++++++++-----------------------
> 1 file changed, 40 insertions(+), 69 deletions(-)
>
>
applied, thanks!
^ permalink raw reply [flat|nested] 11+ messages in thread
* [pmg-devel] [PATCH pmg-api v2 3/5] api: statistics: refactor detail calls
2021-01-21 15:51 [pmg-devel] [PATCH pmg-api v2 0/5] allow / in local part of pmg-email-address Stoiko Ivanov
2021-01-21 15:51 ` [pmg-devel] [PATCH pmg-api v2 1/5] api: statistics: remove unneeded RESTEnvironment Stoiko Ivanov
2021-01-21 15:51 ` [pmg-devel] [PATCH pmg-api v2 2/5] api: statistics: refactor return for detail calls Stoiko Ivanov
@ 2021-01-21 15:51 ` Stoiko Ivanov
2021-01-27 8:34 ` [pmg-devel] applied: " Thomas Lamprecht
2021-01-21 15:51 ` [pmg-devel] [PATCH pmg-api v2] api: statistics: make email a parameter Stoiko Ivanov
` (2 subsequent siblings)
5 siblings, 1 reply; 11+ messages in thread
From: Stoiko Ivanov @ 2021-01-21 15:51 UTC (permalink / raw)
To: pmg-devel
the API calls returning the detailed statistics for a particular
email use much common code.
This patch introduces a sub to be used in all detail api calls.
Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
src/PMG/API2/Statistics.pm | 82 ++++++++++++++++----------------------
1 file changed, 34 insertions(+), 48 deletions(-)
diff --git a/src/PMG/API2/Statistics.pm b/src/PMG/API2/Statistics.pm
index 064865d..72bfeb5 100644
--- a/src/PMG/API2/Statistics.pm
+++ b/src/PMG/API2/Statistics.pm
@@ -291,6 +291,37 @@ my $detail_return_properties = sub {
return $prop;
};
+sub get_detail_statistics {
+ my ($type, $param) =@_;
+
+ my ($start, $end) = $extract_start_end->($param);
+
+ my $stat = PMG::Statistic->new($start, $end);
+ my $rdb = PMG::RuleDB->new();
+
+ my $sorters = [];
+ if ($param->{orderby}) {
+ my $props = ['time', 'sender', 'bytes', 'blocked', 'spamlevel', 'virusinfo'];
+ $props->[1] = 'receiver' if $type eq 'sender';
+ $sorters = $decode_orderby->($param->{orderby}, $props);
+ }
+
+ my $res = [];
+ if ($type eq 'contact') {
+ $res = $stat->user_stat_contact_details(
+ $rdb, $param->{contact}, $userstat_limit, $sorters, $param->{filter});
+ } elsif ($type eq 'sender') {
+ $res = $stat->user_stat_sender_details(
+ $rdb, $param->{sender}, $userstat_limit, $sorters, $param->{filter});
+ } elsif ($type eq 'receiver') {
+ $res = $stat->user_stat_receiver_details(
+ $rdb, $param->{receiver}, $userstat_limit, $sorters, $param->{filter});
+ } else {
+ die "invalid type provided (not 'contact', 'sender', 'receiver')\n";
+ }
+ return $res;
+}
+
__PACKAGE__->register_method ({
name => 'contactdetails',
path => 'contact/{contact}',
@@ -327,22 +358,7 @@ __PACKAGE__->register_method ({
code => sub {
my ($param) = @_;
- my $restenv = PMG::RESTEnvironment->get();
- my $cinfo = $restenv->{cinfo};
-
- my ($start, $end) = $extract_start_end->($param);
-
- my $stat = PMG::Statistic->new($start, $end);
- my $rdb = PMG::RuleDB->new();
-
- my $sorters = [];
- if ($param->{orderby}) {
- my $props = ['time', 'sender', 'bytes', 'blocked', 'spamlevel', 'virusinfo'];
- $sorters = $decode_orderby->($param->{orderby}, $props);
- }
-
- return $stat->user_stat_contact_details(
- $rdb, $param->{contact}, $userstat_limit, $sorters, $param->{filter});
+ return get_detail_statistics('contact', $param);
}});
__PACKAGE__->register_method ({
@@ -445,22 +461,7 @@ __PACKAGE__->register_method ({
code => sub {
my ($param) = @_;
- my $restenv = PMG::RESTEnvironment->get();
- my $cinfo = $restenv->{cinfo};
-
- my ($start, $end) = $extract_start_end->($param);
-
- my $stat = PMG::Statistic->new($start, $end);
- my $rdb = PMG::RuleDB->new();
-
- my $sorters = [];
- if ($param->{orderby}) {
- my $props = ['time', 'receiver', 'bytes', 'blocked', 'spamlevel', 'virusinfo'];
- $sorters = $decode_orderby->($param->{orderby}, $props);
- }
-
- return $stat->user_stat_sender_details(
- $rdb, $param->{sender}, $userstat_limit, $sorters, $param->{filter});
+ return get_detail_statistics('sender', $param);
}});
__PACKAGE__->register_method ({
@@ -571,22 +572,7 @@ __PACKAGE__->register_method ({
code => sub {
my ($param) = @_;
- my $restenv = PMG::RESTEnvironment->get();
- my $cinfo = $restenv->{cinfo};
-
- my ($start, $end) = $extract_start_end->($param);
-
- my $stat = PMG::Statistic->new($start, $end);
- my $rdb = PMG::RuleDB->new();
-
- my $sorters = [];
- if ($param->{orderby}) {
- my $props = ['time', 'sender', 'bytes', 'blocked', 'spamlevel', 'virusinfo'];
- $sorters = $decode_orderby->($param->{orderby}, $props);
- }
-
- return $stat->user_stat_receiver_details(
- $rdb, $param->{receiver}, $userstat_limit, $sorters, $param->{filter});
+ return get_detail_statistics('receiver', $param);
}});
__PACKAGE__->register_method ({
--
2.20.1
^ permalink raw reply [flat|nested] 11+ messages in thread* [pmg-devel] [PATCH pmg-api v2] api: statistics: make email a parameter
2021-01-21 15:51 [pmg-devel] [PATCH pmg-api v2 0/5] allow / in local part of pmg-email-address Stoiko Ivanov
` (2 preceding siblings ...)
2021-01-21 15:51 ` [pmg-devel] [PATCH pmg-api v2 3/5] api: statistics: refactor " Stoiko Ivanov
@ 2021-01-21 15:51 ` Stoiko Ivanov
2021-01-27 8:49 ` Thomas Lamprecht
2021-01-21 15:51 ` [pmg-devel] [PATCH pmg-api v2 5/5] utils: allow '/' inside email address localpart Stoiko Ivanov
2021-01-21 15:51 ` [pmg-devel] [PATCH pmg-gui v2 1/1] statistics: use new api structure for detailed stats Stoiko Ivanov
5 siblings, 1 reply; 11+ messages in thread
From: Stoiko Ivanov @ 2021-01-21 15:51 UTC (permalink / raw)
To: pmg-devel
This patch changes the semantics for the detailed contact, sender and
receiver statistics call:
additionally to the current way of providing the e-mail address as
path component, it is now also possible to provide 'detail' as path
and the address in the additional parameter 'detailaddress'.
This allows the statistics to be displayed for addresses containing
'/' in their localpart, once this is permitted in our api schema.
the idea follows a similar change for the user blocklists in
e8d909c11faeb5a4f84f39ef50e0eaf8ea65046d
The verification of the paramter now happens inside the common sub
for detail statistics instead of the API decomposer.
Backward compatibility is ensured, since the only additional value
'detail' was not allowed as value for 'pmg-email-address'.
Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
v1->v2:
* instead of adding the new paths contacts, senders, receivers in addition
to their singular counterparts, handle the detail calls in the old path
with the special value 'detail' (instead of an e-mail address)
src/PMG/API2/Statistics.pm | 59 ++++++++++++++++++++++++++++++++------
1 file changed, 50 insertions(+), 9 deletions(-)
src/PMG/API2/Statistics.pm | 58 ++++++++++++++++++++++++++++++++------
1 file changed, 49 insertions(+), 9 deletions(-)
diff --git a/src/PMG/API2/Statistics.pm b/src/PMG/API2/Statistics.pm
index 72bfeb5..732a3b4 100644
--- a/src/PMG/API2/Statistics.pm
+++ b/src/PMG/API2/Statistics.pm
@@ -294,6 +294,19 @@ my $detail_return_properties = sub {
sub get_detail_statistics {
my ($type, $param) =@_;
+ my $detail_address;
+ if ($param->{$type} eq 'detail') {
+ if (!defined($param->{detailaddress})) {
+ raise_param_exc({ $type => 'need detailaddress'});
+ }
+ $detail_address = $param->{detailaddress};
+ } else {
+ $detail_address = $param->{$type};
+ }
+
+ PVE::JSONSchema::validate($detail_address, get_standard_option('pmg-email-address'),
+ 'Address parameter verification failed');
+
my ($start, $end) = $extract_start_end->($param);
my $stat = PMG::Statistic->new($start, $end);
@@ -309,19 +322,22 @@ sub get_detail_statistics {
my $res = [];
if ($type eq 'contact') {
$res = $stat->user_stat_contact_details(
- $rdb, $param->{contact}, $userstat_limit, $sorters, $param->{filter});
+ $rdb, $detail_address, $userstat_limit, $sorters, $param->{filter});
} elsif ($type eq 'sender') {
$res = $stat->user_stat_sender_details(
- $rdb, $param->{sender}, $userstat_limit, $sorters, $param->{filter});
+ $rdb, $detail_address, $userstat_limit, $sorters, $param->{filter});
} elsif ($type eq 'receiver') {
$res = $stat->user_stat_receiver_details(
- $rdb, $param->{receiver}, $userstat_limit, $sorters, $param->{filter});
+ $rdb, $detail_address, $userstat_limit, $sorters, $param->{filter});
} else {
die "invalid type provided (not 'contact', 'sender', 'receiver')\n";
}
return $res;
}
+# FIXME: change for PMG 7.0 - remove support for providing addresses as path
+# addresses can contain '/' which breaks API path resolution - hardcode
+# 'detail' for the path pattern.
__PACKAGE__->register_method ({
name => 'contactdetails',
path => 'contact/{contact}',
@@ -331,8 +347,14 @@ __PACKAGE__->register_method ({
parameters => {
additionalProperties => 0,
properties => $default_properties->({
- contact => get_standard_option('pmg-email-address', {
- description => "Contact email address.",
+ contact => {
+ type => 'string',
+ description => "'detail', with the address provided as 'detailaddress' or"
+ . "the address - the latter is deprecated.",
+ },
+ detailaddress => get_standard_option('pmg-email-address', {
+ description => "The e-mail address if contact is 'detail'.",
+ optional => 1,
}),
filter => {
description => "Sender address filter.",
@@ -425,6 +447,9 @@ __PACKAGE__->register_method ({
return $res;
}});
+# FIXME: change for PMG 7.0 - remove support for providing addresses as path
+# addresses can contain '/' which breaks API path resolution - hardcode
+# 'detail' for the path pattern.
__PACKAGE__->register_method ({
name => 'senderdetails',
path => 'sender/{sender}',
@@ -434,8 +459,14 @@ __PACKAGE__->register_method ({
parameters => {
additionalProperties => 0,
properties => $default_properties->({
- sender => get_standard_option('pmg-email-address', {
- description => "Sender email address.",
+ sender => {
+ type => 'string',
+ description => "'detail', with the address provided as 'detailaddress' or"
+ . "the address - the latter is deprecated.",
+ },
+ detailaddress => get_standard_option('pmg-email-address', {
+ description => "The e-mail address if sender is 'detail'.",
+ optional => 1,
}),
filter => {
description => "Receiver address filter.",
@@ -536,6 +567,9 @@ __PACKAGE__->register_method ({
return $res;
}});
+# FIXME: change for PMG 7.0 - remove support for providing addresses as path
+# addresses can contain '/' which breaks API path resolution - hardcode
+# 'detail' for the path pattern.
__PACKAGE__->register_method ({
name => 'receiverdetails',
path => 'receiver/{receiver}',
@@ -545,8 +579,14 @@ __PACKAGE__->register_method ({
parameters => {
additionalProperties => 0,
properties => $default_properties->({
- receiver => get_standard_option('pmg-email-address', {
- description => "Receiver email address.",
+ receiver => {
+ type => 'string',
+ description => "'detail', with the address provided as 'detailaddress' or"
+ . "the address - the latter is deprecated.",
+ },
+ detailaddress => get_standard_option('pmg-email-address', {
+ description => "The e-mail address if contact is 'detail'.",
+ optional => 1,
}),
filter => {
description => "Sender address filter.",
--
2.20.1
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [pmg-devel] [PATCH pmg-api v2] api: statistics: make email a parameter
2021-01-21 15:51 ` [pmg-devel] [PATCH pmg-api v2] api: statistics: make email a parameter Stoiko Ivanov
@ 2021-01-27 8:49 ` Thomas Lamprecht
0 siblings, 0 replies; 11+ messages in thread
From: Thomas Lamprecht @ 2021-01-27 8:49 UTC (permalink / raw)
To: Stoiko Ivanov, pmg-devel
On 21.01.21 16:51, Stoiko Ivanov wrote:
> This patch changes the semantics for the detailed contact, sender and
> receiver statistics call:
> additionally to the current way of providing the e-mail address as
> path component, it is now also possible to provide 'detail' as path
> and the address in the additional parameter 'detailaddress'.
>
> This allows the statistics to be displayed for addresses containing
> '/' in their localpart, once this is permitted in our api schema.
hmm, but this won't really work, or at least play nicely, when trying
to deprecate the old API path {template} and mount /detail there as
the index of the 'receiver', 'sender' and 'contact' API endpoints still
needs to return unfiltered, undetailed statistics for all senders.
Could be hacked in, would only work with keeping a dynamic {templated}
API path as else those indexes would need to return a pseudo entry for
linking to the /detail sub-endpoint.
Alternative to the old two proposals:
(1) Add new API endpoint "detailed" API endpoint with the types as
child-endpoint, or maybe even better as common parameter, as then only
one new API point would be required.
(2) move now also the unfiltered info a level down, i.e.:
/sender
/detailed
/all
/receiver
/detailed
/all
/contact
/detailed
/all
This way we can move the parent sender, receiver, contact endpoints
to return statically the /all and /detailed children hrefs and the
children be leaf-nodes and just return data.
Both are about the same work and in general fine to me. The decision is
IMO only over what fits the rest of the API better and deemed to be
Better/Nicer/Cleaner Way™.
> the idea follows a similar change for the user blocklists in
> e8d909c11faeb5a4f84f39ef50e0eaf8ea65046d
>
> The verification of the paramter now happens inside the common sub
> for detail statistics instead of the API decomposer.
>
> Backward compatibility is ensured, since the only additional value
> 'detail' was not allowed as value for 'pmg-email-address'.
>
> Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
> ---
> v1->v2:
> * instead of adding the new paths contacts, senders, receivers in addition
> to their singular counterparts, handle the detail calls in the old path
> with the special value 'detail' (instead of an e-mail address)
>
> src/PMG/API2/Statistics.pm | 59 ++++++++++++++++++++++++++++++++------
> 1 file changed, 50 insertions(+), 9 deletions(-)
> src/PMG/API2/Statistics.pm | 58 ++++++++++++++++++++++++++++++++------
> 1 file changed, 49 insertions(+), 9 deletions(-)
>
> diff --git a/src/PMG/API2/Statistics.pm b/src/PMG/API2/Statistics.pm
> index 72bfeb5..732a3b4 100644
> --- a/src/PMG/API2/Statistics.pm
> +++ b/src/PMG/API2/Statistics.pm
> @@ -294,6 +294,19 @@ my $detail_return_properties = sub {
> sub get_detail_statistics {
> my ($type, $param) =@_;
>
> + my $detail_address;
> + if ($param->{$type} eq 'detail') {
> + if (!defined($param->{detailaddress})) {
> + raise_param_exc({ $type => 'need detailaddress'});
> + }
> + $detail_address = $param->{detailaddress};
> + } else {
> + $detail_address = $param->{$type};
> + }
> +
> + PVE::JSONSchema::validate($detail_address, get_standard_option('pmg-email-address'),
> + 'Address parameter verification failed');
> +
> my ($start, $end) = $extract_start_end->($param);
>
> my $stat = PMG::Statistic->new($start, $end);
> @@ -309,19 +322,22 @@ sub get_detail_statistics {
> my $res = [];
> if ($type eq 'contact') {
> $res = $stat->user_stat_contact_details(
> - $rdb, $param->{contact}, $userstat_limit, $sorters, $param->{filter});
> + $rdb, $detail_address, $userstat_limit, $sorters, $param->{filter});
> } elsif ($type eq 'sender') {
> $res = $stat->user_stat_sender_details(
> - $rdb, $param->{sender}, $userstat_limit, $sorters, $param->{filter});
> + $rdb, $detail_address, $userstat_limit, $sorters, $param->{filter});
> } elsif ($type eq 'receiver') {
> $res = $stat->user_stat_receiver_details(
> - $rdb, $param->{receiver}, $userstat_limit, $sorters, $param->{filter});
> + $rdb, $detail_address, $userstat_limit, $sorters, $param->{filter});
> } else {
> die "invalid type provided (not 'contact', 'sender', 'receiver')\n";
> }
> return $res;
> }
>
> +# FIXME: change for PMG 7.0 - remove support for providing addresses as path
> +# addresses can contain '/' which breaks API path resolution - hardcode
> +# 'detail' for the path pattern.
> __PACKAGE__->register_method ({
> name => 'contactdetails',
> path => 'contact/{contact}',
> @@ -331,8 +347,14 @@ __PACKAGE__->register_method ({
> parameters => {
> additionalProperties => 0,
> properties => $default_properties->({
> - contact => get_standard_option('pmg-email-address', {
> - description => "Contact email address.",
> + contact => {
> + type => 'string',
> + description => "'detail', with the address provided as 'detailaddress' or"
> + . "the address - the latter is deprecated.",
> + },
> + detailaddress => get_standard_option('pmg-email-address', {
> + description => "The e-mail address if contact is 'detail'.",
> + optional => 1,
> }),
> filter => {
> description => "Sender address filter.",
> @@ -425,6 +447,9 @@ __PACKAGE__->register_method ({
> return $res;
> }});
>
> +# FIXME: change for PMG 7.0 - remove support for providing addresses as path
> +# addresses can contain '/' which breaks API path resolution - hardcode
> +# 'detail' for the path pattern.
> __PACKAGE__->register_method ({
> name => 'senderdetails',
> path => 'sender/{sender}',
> @@ -434,8 +459,14 @@ __PACKAGE__->register_method ({
> parameters => {
> additionalProperties => 0,
> properties => $default_properties->({
> - sender => get_standard_option('pmg-email-address', {
> - description => "Sender email address.",
> + sender => {
> + type => 'string',
> + description => "'detail', with the address provided as 'detailaddress' or"
> + . "the address - the latter is deprecated.",
> + },
> + detailaddress => get_standard_option('pmg-email-address', {
> + description => "The e-mail address if sender is 'detail'.",
> + optional => 1,
> }),
> filter => {
> description => "Receiver address filter.",
> @@ -536,6 +567,9 @@ __PACKAGE__->register_method ({
> return $res;
> }});
>
> +# FIXME: change for PMG 7.0 - remove support for providing addresses as path
> +# addresses can contain '/' which breaks API path resolution - hardcode
> +# 'detail' for the path pattern.
> __PACKAGE__->register_method ({
> name => 'receiverdetails',
> path => 'receiver/{receiver}',
> @@ -545,8 +579,14 @@ __PACKAGE__->register_method ({
> parameters => {
> additionalProperties => 0,
> properties => $default_properties->({
> - receiver => get_standard_option('pmg-email-address', {
> - description => "Receiver email address.",
> + receiver => {
> + type => 'string',
> + description => "'detail', with the address provided as 'detailaddress' or"
> + . "the address - the latter is deprecated.",
> + },
> + detailaddress => get_standard_option('pmg-email-address', {
> + description => "The e-mail address if contact is 'detail'.",
> + optional => 1,
> }),
> filter => {
> description => "Sender address filter.",
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [pmg-devel] [PATCH pmg-api v2 5/5] utils: allow '/' inside email address localpart
2021-01-21 15:51 [pmg-devel] [PATCH pmg-api v2 0/5] allow / in local part of pmg-email-address Stoiko Ivanov
` (3 preceding siblings ...)
2021-01-21 15:51 ` [pmg-devel] [PATCH pmg-api v2] api: statistics: make email a parameter Stoiko Ivanov
@ 2021-01-21 15:51 ` Stoiko Ivanov
2021-01-21 15:51 ` [pmg-devel] [PATCH pmg-gui v2 1/1] statistics: use new api structure for detailed stats Stoiko Ivanov
5 siblings, 0 replies; 11+ messages in thread
From: Stoiko Ivanov @ 2021-01-21 15:51 UTC (permalink / raw)
To: pmg-devel
The change is motivated by a report in our community forum [0], where
a mail addressed to an address containing '/' in its local-part ended
up in the quarantine.
This is permitted by RFC5322 ([1]), and, probably more relevant,
happily accepted and processed by postfix.
Once inside the quarantine (or the statistic database) the records cannot
be displayed (due to the parameter verification failure).
This leaves the user unable to delete the quarantined mail.
Apart from the quarantine and statistics the 'pmg-email-address'
format is only used in the PBSConfig and the fetchmail configuration
(both of which are available only to the admin and can be still be
edited irrespective of the set localpart).
[0]
https://forum.proxmox.com/threads/pmg-error-parameter-verification-failed-400.82353/
[1] https://tools.ietf.org/html/rfc5322
Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
src/PMG/Utils.pm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/PMG/Utils.pm b/src/PMG/Utils.pm
index 149bcdc..4341dbd 100644
--- a/src/PMG/Utils.pm
+++ b/src/PMG/Utils.pm
@@ -117,7 +117,7 @@ PVE::JSONSchema::register_standard_option('username', {
PVE::JSONSchema::register_standard_option('pmg-email-address', {
description => "Email Address (allow most characters).",
type => 'string',
- pattern => '(?:[^\s\/\\\@]+\@[^\s\/\\\@]+)',
+ pattern => '(?:[^\s\\\@]+\@[^\s\/\\\@]+)',
maxLength => 512,
minLength => 3,
});
--
2.20.1
^ permalink raw reply [flat|nested] 11+ messages in thread* [pmg-devel] [PATCH pmg-gui v2 1/1] statistics: use new api structure for detailed stats
2021-01-21 15:51 [pmg-devel] [PATCH pmg-api v2 0/5] allow / in local part of pmg-email-address Stoiko Ivanov
` (4 preceding siblings ...)
2021-01-21 15:51 ` [pmg-devel] [PATCH pmg-api v2 5/5] utils: allow '/' inside email address localpart Stoiko Ivanov
@ 2021-01-21 15:51 ` Stoiko Ivanov
5 siblings, 0 replies; 11+ messages in thread
From: Stoiko Ivanov @ 2021-01-21 15:51 UTC (permalink / raw)
To: pmg-devel
the detail api calls take the address for which to display the statistics
as explicit parameter instead of component of the path if the path
component is set to 'detail'.
This makes it possible to accept '/' as part of an e-mail address
which is allowed (in the local-part by RFC5322 [0], and accepted by
postfix.
[0] https://tools.ietf.org/html/rfc5322
Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
v1->v2:
* adapt to new api-structure
* still pass url as parameter - passing undefined is handled specially
(huge thanks to Dominik for noticing this after a 2 second glance :)
js/ContactStatistics.js | 9 ++++-----
js/ReceiverStatistics.js | 9 ++++-----
js/SenderStatistics.js | 9 ++++-----
js/StatStore.js | 11 +++++++++--
4 files changed, 21 insertions(+), 17 deletions(-)
diff --git a/js/ContactStatistics.js b/js/ContactStatistics.js
index 6c9d2d4..8fc58b8 100644
--- a/js/ContactStatistics.js
+++ b/js/ContactStatistics.js
@@ -16,10 +16,10 @@ Ext.define('PMG.ContactDetails', {
plugins: 'gridfilters',
- setUrl: function(url, title) {
+ setUrl: function(url, detailaddress, title) {
var me = this;
- me.store.setUrl(url);
+ me.store.setUrl(url, { detailaddress: detailaddress });
me.store.setRemoteFilter(url !== undefined);
Proxmox.Utils.setErrorMask(me, false);
me.store.reload();
@@ -201,9 +201,8 @@ Ext.define('PMG.ContactStatistics', {
var details = this.lookupReference('details');
if (selected.length > 0) {
var contact = selected[0].data.contact;
- var url = "/api2/json/statistics/contact/" +
- encodeURIComponent(contact);
- details.setUrl(url, '<b>' + gettext('Contact') + ':</b> ' + Ext.htmlEncode(contact));
+ var url = "/api2/json/statistics/contact/detail";
+ details.setUrl(url, contact, '<b>' + gettext('Contact') + ':</b> ' + Ext.htmlEncode(contact));
} else {
details.setUrl();
}
diff --git a/js/ReceiverStatistics.js b/js/ReceiverStatistics.js
index 6378e06..069c2bf 100644
--- a/js/ReceiverStatistics.js
+++ b/js/ReceiverStatistics.js
@@ -16,10 +16,10 @@ Ext.define('PMG.ReceiverDetails', {
plugins: 'gridfilters',
- setUrl: function(url, title) {
+ setUrl: function(url, detailaddress, title) {
var me = this;
- me.store.setUrl(url);
+ me.store.setUrl(url, { detailaddress: detailaddress });
me.store.setRemoteFilter(url !== undefined);
Proxmox.Utils.setErrorMask(me, false);
me.store.reload();
@@ -212,10 +212,9 @@ Ext.define('PMG.ReceiverStatistics', {
selectionChange: function(grid, selected, eOpts) {
var details = this.lookupReference('details');
if (selected.length > 0) {
+ var url = "/api2/json/statistics/receiver/detail";
var receiver = selected[0].data.receiver;
- var url = "/api2/json/statistics/receiver/" +
- encodeURIComponent(receiver);
- details.setUrl(url, '<b>' + gettext('Receiver') + ':</b> ' + Ext.htmlEncode(receiver));
+ details.setUrl(url, receiver, '<b>' + gettext('Receiver') + ':</b> ' + Ext.htmlEncode(receiver));
} else {
details.setUrl();
}
diff --git a/js/SenderStatistics.js b/js/SenderStatistics.js
index 43c5438..9b9f48d 100644
--- a/js/SenderStatistics.js
+++ b/js/SenderStatistics.js
@@ -16,10 +16,10 @@ Ext.define('PMG.SenderDetails', {
plugins: 'gridfilters',
- setUrl: function(url, title) {
+ setUrl: function(url, detailaddress, title) {
var me = this;
- me.store.setUrl(url);
+ me.store.setUrl(url, { detailaddress: detailaddress });
me.store.setRemoteFilter(url !== undefined);
Proxmox.Utils.setErrorMask(me, false);
me.store.reload();
@@ -201,9 +201,8 @@ Ext.define('PMG.SenderStatistics', {
var details = this.lookupReference('details');
if (selected.length > 0) {
var sender = selected[0].data.sender;
- var url = "/api2/json/statistics/sender/" +
- encodeURIComponent(sender);
- details.setUrl(url, '<b>' + gettext('Sender') + ':</b> ' + Ext.htmlEncode(sender));
+ var url = "/api2/json/statistics/sender/detail";
+ details.setUrl(url, sender, '<b>' + gettext('Sender') + ':</b> ' + Ext.htmlEncode(sender));
} else {
details.setUrl();
}
diff --git a/js/StatStore.js b/js/StatStore.js
index ec11777..0fc1d31 100644
--- a/js/StatStore.js
+++ b/js/StatStore.js
@@ -8,13 +8,17 @@ Ext.define('PMG.data.StatStore', {
includeTimeSpan: false,
- setUrl: function(url) {
+ setUrl: function(url, extraparam) {
var me = this;
me.proxy.abort(); // abort pending requests
me.staturl = url;
me.proxy.extraParams = {};
+ if (extraparam !== undefined) {
+ me.proxy.extraParams = extraparam;
+ }
+
me.setData([]);
},
@@ -38,7 +42,10 @@ Ext.define('PMG.data.StatStore', {
}
me.proxy.url = me.staturl;
- me.proxy.extraParams = { starttime: ts.starttime, endtime: ts.endtime };
+ Ext.apply(me.proxy.extraParams, {
+ starttime: ts.starttime,
+ endtime: ts.endtime,
+ });
var timespan = 3600;
if (me.includeTimeSpan) {
--
2.20.1
^ permalink raw reply [flat|nested] 11+ messages in thread