Re: [pmg-devel] [PATCH pmg-api v2] api: statistics: make email a parameter

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

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.",
>