From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 62D1B6B8C1 for ; Wed, 27 Jan 2021 09:50:23 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 4D84520268 for ; Wed, 27 Jan 2021 09:49:53 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [212.186.127.180]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id 782D92025E for ; Wed, 27 Jan 2021 09:49:52 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 3942041808 for ; Wed, 27 Jan 2021 09:49:52 +0100 (CET) To: Stoiko Ivanov , pmg-devel@lists.proxmox.com References: <20210121155107.1971-1-s.ivanov@proxmox.com> <20210121155107.1971-5-s.ivanov@proxmox.com> From: Thomas Lamprecht Message-ID: <476a26ea-8433-98e2-b9d0-c161de4cfd78@proxmox.com> Date: Wed, 27 Jan 2021 09:49:50 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:85.0) Gecko/20100101 Thunderbird/85.0 MIME-Version: 1.0 In-Reply-To: <20210121155107.1971-5-s.ivanov@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Language: en-US Content-Transfer-Encoding: quoted-printable X-SPAM-LEVEL: Spam detection results: 0 AWL -0.069 Adjusted score from AWL reputation of From: address KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment NICE_REPLY_A -0.001 Looks like a legit reply (A) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record T_FILL_THIS_FORM_SHORT 0.01 Fill in a short form with personal information URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [statistics.pm] Subject: Re: [pmg-devel] [PATCH pmg-api v2] api: statistics: make email a parameter X-BeenThere: pmg-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Mail Gateway development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 27 Jan 2021 08:50:23 -0000 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'. >=20 > 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=E2=84=A2. > the idea follows a similar change for the user blocklists in > e8d909c11faeb5a4f84f39ef50e0eaf8ea65046d >=20 > The verification of the paramter now happens inside the common sub > for detail statistics instead of the API decomposer. >=20 > Backward compatibility is ensured, since the only additional value > 'detail' was not allowed as value for 'pmg-email-address'. >=20 > Signed-off-by: Stoiko Ivanov > --- > v1->v2: > * instead of adding the new paths contacts, senders, receivers in addit= ion > to their singular counterparts, handle the detail calls in the old pa= th > with the special value 'detail' (instead of an e-mail address) >=20 > 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(-) >=20 > 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 =3D sub { > sub get_detail_statistics { > my ($type, $param) =3D@_; > =20 > + my $detail_address; > + if ($param->{$type} eq 'detail') { > + if (!defined($param->{detailaddress})) { > + raise_param_exc({ $type =3D> 'need detailaddress'}); > + } > + $detail_address =3D $param->{detailaddress}; > + } else { > + $detail_address =3D $param->{$type}; > + } > + > + PVE::JSONSchema::validate($detail_address, get_standard_option('pm= g-email-address'), > + 'Address parameter verification failed'); > + > my ($start, $end) =3D $extract_start_end->($param); > =20 > my $stat =3D PMG::Statistic->new($start, $end); > @@ -309,19 +322,22 @@ sub get_detail_statistics { > my $res =3D []; > if ($type eq 'contact') { > $res =3D $stat->user_stat_contact_details( > - $rdb, $param->{contact}, $userstat_limit, $sorters, $param->{filt= er}); > + $rdb, $detail_address, $userstat_limit, $sorters, $param->{filter= }); > } elsif ($type eq 'sender') { > $res =3D $stat->user_stat_sender_details( > - $rdb, $param->{sender}, $userstat_limit, $sorters, $param->{filte= r}); > + $rdb, $detail_address, $userstat_limit, $sorters, $param->{filter= }); > } elsif ($type eq 'receiver') { > $res =3D $stat->user_stat_receiver_details( > - $rdb, $param->{receiver}, $userstat_limit, $sorters, $param->{fil= ter}); > + $rdb, $detail_address, $userstat_limit, $sorters, $param->{filter= }); > } else { > die "invalid type provided (not 'contact', 'sender', 'receiver')\n"; > } > return $res; > } > =20 > +# FIXME: change for PMG 7.0 - remove support for providing addresses a= s path > +# addresses can contain '/' which breaks API path resolution - hardcod= e > +# 'detail' for the path pattern. > __PACKAGE__->register_method ({ > name =3D> 'contactdetails', > path =3D> 'contact/{contact}', > @@ -331,8 +347,14 @@ __PACKAGE__->register_method ({ > parameters =3D> { > additionalProperties =3D> 0, > properties =3D> $default_properties->({ > - contact =3D> get_standard_option('pmg-email-address', { > - description =3D> "Contact email address.", > + contact =3D> { > + type =3D> 'string', > + description =3D> "'detail', with the address provided as 'detailaddr= ess' or" > + . "the address - the latter is deprecated.", > + }, > + detailaddress =3D> get_standard_option('pmg-email-address', { > + description =3D> "The e-mail address if contact is 'detail'.", > + optional =3D> 1, > }), > filter =3D> { > description =3D> "Sender address filter.", > @@ -425,6 +447,9 @@ __PACKAGE__->register_method ({ > return $res; > }}); > =20 > +# FIXME: change for PMG 7.0 - remove support for providing addresses a= s path > +# addresses can contain '/' which breaks API path resolution - hardcod= e > +# 'detail' for the path pattern. > __PACKAGE__->register_method ({ > name =3D> 'senderdetails', > path =3D> 'sender/{sender}', > @@ -434,8 +459,14 @@ __PACKAGE__->register_method ({ > parameters =3D> { > additionalProperties =3D> 0, > properties =3D> $default_properties->({ > - sender =3D> get_standard_option('pmg-email-address', { > - description =3D> "Sender email address.", > + sender =3D> { > + type =3D> 'string', > + description =3D> "'detail', with the address provided as 'detailaddr= ess' or" > + . "the address - the latter is deprecated.", > + }, > + detailaddress =3D> get_standard_option('pmg-email-address', { > + description =3D> "The e-mail address if sender is 'detail'.", > + optional =3D> 1, > }), > filter =3D> { > description =3D> "Receiver address filter.", > @@ -536,6 +567,9 @@ __PACKAGE__->register_method ({ > return $res; > }}); > =20 > +# FIXME: change for PMG 7.0 - remove support for providing addresses a= s path > +# addresses can contain '/' which breaks API path resolution - hardcod= e > +# 'detail' for the path pattern. > __PACKAGE__->register_method ({ > name =3D> 'receiverdetails', > path =3D> 'receiver/{receiver}', > @@ -545,8 +579,14 @@ __PACKAGE__->register_method ({ > parameters =3D> { > additionalProperties =3D> 0, > properties =3D> $default_properties->({ > - receiver =3D> get_standard_option('pmg-email-address', { > - description =3D> "Receiver email address.", > + receiver =3D> { > + type =3D> 'string', > + description =3D> "'detail', with the address provided as 'detailaddr= ess' or" > + . "the address - the latter is deprecated.", > + }, > + detailaddress =3D> get_standard_option('pmg-email-address', { > + description =3D> "The e-mail address if contact is 'detail'.", > + optional =3D> 1, > }), > filter =3D> { > description =3D> "Sender address filter.", >=20