You are viewing a plain text version of this content. The canonical link for it is here.
Posted to server-dev@james.apache.org by "Noel J. Bergman" <no...@devtech.com> on 2003/05/14 06:03:01 UTC
Marco's AbstractQuotaMatcher and and Vincenzo's RecipientIsOverFixedQuota
Marco and Vincenzo,
Attached are your AbstractQuotaMatcher and RecipientIsOverFixedQuota
matchers. I have tested them with all current repository types: file, db
and dbfile.
I made a few changes to the code you sent.
The first change that I made was to use MailAddress instead of String as the
parameter type. That preserves some information that would otherwise be
lost, and could be useful (see next paragraph).
Another change was to make getStorageUsed public, allowing a specialized
class to reimplement it, and to rename the methods getUsed() and getQuota().
The reason for these changes was to support other types of quota-based
matchers, such as message count, monthly transfer, etc.
Next, I moved Vincenzo's quota string parser into the base class so that
others can make use of it.
Finally, I tweaked the exception handling. The exception handling changes
are necessary because the current file-based repository has a really nasty
habit of tossing RuntimeExceptions when it can't find a file, e.g., because
the user is currently deleting mail from the inbox.
OK, guys ... what do you think? And thanks for all of your hard work! :-)
By the way, the overhead of using this matcher with our current JDBC-based
repositories is pretty brutal (although probably not even noticable for most
users), but it works quite well with the file-based repository.
--- Noel
Re: Marco's AbstractQuotaMatcher and and Vincenzo's RecipientIsOverFixedQuota
Posted by Marco Tedone <mt...@jemos.org>.
With respect to this logic, with now is clever, I would suggest another
approach which, at the end, would obtain the same result, 'short
circuiting', but could be more dynamically implemented:
Suppose I created a method like the following:
//Tells whether to check for quota
private boolean toBeChecked(MailAddress recipient){
//TODO TO insert the code to avoid check in case the sender is the
postmaster
/*
* If the MailRecipient hasn't got a local account or it doesn't exists,
* then the quota hasn't to be checked
*/
if (!(mailetContext.isLocalServer(recipient.getHost()) &&
mailetContext.isLocalUser(recipient.getUser())))
return false;
return true;
}
Then, the matchRecipient method could look like the following:
public boolean matchRecipient(MailAddress recipient) {
//Perform the Quota check only for valid cases
if (toBeChecked(recipient)){
try {
//If used storage met the limit, then pass the mail to the
Mailet
boolean over = (getUsed(recipient) >= getQuota(recipient));
if (over) log(recipient + " is over quota.");
return over;
} catch (Throwable e) {
log("Exception checking storage used for user: " + recipient,
e);
return false;
}
}
//Return default value: Skip The Mailet
return false;
}
It allows space in the toBeChecked() for the postmaster case to be checked
and eventually for adding other controls, leaving the code clean and tidy.
What do you think?
Marco
----- Original Message -----
From: "Noel J. Bergman" <no...@devtech.com>
To: "James Developers List" <ja...@jakarta.apache.org>
Sent: Wednesday, May 14, 2003 8:59 PM
Subject: RE: Marco's AbstractQuotaMatcher and and Vincenzo's
RecipientIsOverFixedQuota
> > sorry but I have some difficulties to understand the condition you
> > tested in the matchRecipient method.
>
> The key to understanding the logic is remembering that && and || short
> circuit. If the left hand side of && is false, the righthand side is not
> executed. If the lefthand side of || is true, then the righthand side is
> not executed.
>
> The original logic was fairly dense. It checked to see that the host were
> local and the user were local. If BOTH were TRUE, then expression would
be
> TRUE. Therefore the ! would make it FALSE, and the quotas would be
checked.
> If either the host were not local or the user were not local, then the
> expression would be FALSE, the ! would make it TRUE, and the quotas would
> NOT be checked. Finally, the second ! inverted the result for our return
> value.
>
> I simplied that logic in the most recent post.
>
> > Should [isLocalServer && isLocalUser] return true, does it mean that
> > the mail should be considered for quota calculation, and reverserly
> > should it return false should the mail be treated as an over
> > quota?
>
> Not quite ... if (isLocalServer && isLocaUser) then the quota should be
> checked, otherwise there IS NO QUOTA to check. That is why getUsed
returns
> 0 (no storage is used), and getQuota returns Long.MAX_VALUE (not limit for
> non-local users). That is all reflected in the most recent post.
>
> --- Noel
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: james-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: james-dev-help@jakarta.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: james-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: james-dev-help@jakarta.apache.org
RE: Marco's AbstractQuotaMatcher and and Vincenzo's RecipientIsOverFixedQuota
Posted by "Noel J. Bergman" <no...@devtech.com>.
> sorry but I have some difficulties to understand the condition you
> tested in the matchRecipient method.
The key to understanding the logic is remembering that && and || short
circuit. If the left hand side of && is false, the righthand side is not
executed. If the lefthand side of || is true, then the righthand side is
not executed.
The original logic was fairly dense. It checked to see that the host were
local and the user were local. If BOTH were TRUE, then expression would be
TRUE. Therefore the ! would make it FALSE, and the quotas would be checked.
If either the host were not local or the user were not local, then the
expression would be FALSE, the ! would make it TRUE, and the quotas would
NOT be checked. Finally, the second ! inverted the result for our return
value.
I simplied that logic in the most recent post.
> Should [isLocalServer && isLocalUser] return true, does it mean that
> the mail should be considered for quota calculation, and reverserly
> should it return false should the mail be treated as an over
> quota?
Not quite ... if (isLocalServer && isLocaUser) then the quota should be
checked, otherwise there IS NO QUOTA to check. That is why getUsed returns
0 (no storage is used), and getQuota returns Long.MAX_VALUE (not limit for
non-local users). That is all reflected in the most recent post.
--- Noel
---------------------------------------------------------------------
To unsubscribe, e-mail: james-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: james-dev-help@jakarta.apache.org
Re: Marco's AbstractQuotaMatcher and and Vincenzo's RecipientIsOverFixedQuota
Posted by Marco Tedone <mt...@jemos.org>.
Noel, sorry but I have some difficulties to understand the condition you
tested in the matchRecipient method. Should the following test return true,
does it mean that the mail should be considered for quota calculation, and
reverserly should it return false should the mail be treated as an over
quota?:
(mailetContext.isLocalServer(recipient.getHost()) &&
mailetContext.isLocalUser(user)))
Thanks,
Marco
----- Original Message -----
From: "Noel J. Bergman" <no...@devtech.com>
To: "James Developers List" <ja...@jakarta.apache.org>
Sent: Wednesday, May 14, 2003 7:39 AM
Subject: RE: Marco's AbstractQuotaMatcher and and Vincenzo's
RecipientIsOverFixedQuota
> > could you please explain the intent of testing for a LocalServer
> > or a LocalUser?
>
> 1. If the user does not have a local mail box, you don't want to try
> looking for one. The current system will actually create an empty mailbox
> if you query one that doesn't exist.
>
> 2. The Matcher is called for all recipients on the message, and if the
> recipient is not a local recipient, you would not have a storage quota.
>
> > As I said yesterday, I can't get the point to include those conditions
in
> a
> > Size Matcher
>
> Sorry, I hadn't notice the question.
>
> > I think there is a problem with the AbstractQuotaMatcher code
> > you sent; in particular I think that the
> > getUsed(MailAddress recipient) method should interact with the
> > parseQuota(String amount) method, otherwise we would risk
> > asynchronous data between the storage used and the storage
> > allowed.
>
> The parseQuota method is simply a text to numeric converter. There is no
> need to call it other than during initialization. And getUsed already
gets
> a numeric value, so there is no string to convert.
>
> As for what I perceive to be your actual point, James is inherently
> asynchronous, at least in the current implementation. There will always
be
> a certain amount of "slop" in the quota system. For example, you could
> approve messages when you shouldn't, because the mailbox is going to be
over
> quota from messages still in the queue between the quota check and being
> stored in the mailbox. Conversely, you could disallow due to an over
quota
> condition while the mailbox is in the process of deleting messages that
> would free up space. Administrators will simply have to take that into
> account, and adjust their quotas accordingly. In most cases, these
> theoretical race conditions won't occur.
>
> --- Noel
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: james-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: james-dev-help@jakarta.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: james-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: james-dev-help@jakarta.apache.org
RE: Marco's AbstractQuotaMatcher and and Vincenzo's RecipientIsOverFixedQuota
Posted by "Noel J. Bergman" <no...@devtech.com>.
> could you please explain the intent of testing for a LocalServer
> or a LocalUser?
1. If the user does not have a local mail box, you don't want to try
looking for one. The current system will actually create an empty mailbox
if you query one that doesn't exist.
2. The Matcher is called for all recipients on the message, and if the
recipient is not a local recipient, you would not have a storage quota.
> As I said yesterday, I can't get the point to include those conditions in
a
> Size Matcher
Sorry, I hadn't notice the question.
> I think there is a problem with the AbstractQuotaMatcher code
> you sent; in particular I think that the
> getUsed(MailAddress recipient) method should interact with the
> parseQuota(String amount) method, otherwise we would risk
> asynchronous data between the storage used and the storage
> allowed.
The parseQuota method is simply a text to numeric converter. There is no
need to call it other than during initialization. And getUsed already gets
a numeric value, so there is no string to convert.
As for what I perceive to be your actual point, James is inherently
asynchronous, at least in the current implementation. There will always be
a certain amount of "slop" in the quota system. For example, you could
approve messages when you shouldn't, because the mailbox is going to be over
quota from messages still in the queue between the quota check and being
stored in the mailbox. Conversely, you could disallow due to an over quota
condition while the mailbox is in the process of deleting messages that
would free up space. Administrators will simply have to take that into
account, and adjust their quotas accordingly. In most cases, these
theoretical race conditions won't occur.
--- Noel
---------------------------------------------------------------------
To unsubscribe, e-mail: james-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: james-dev-help@jakarta.apache.org
Re: Marco's AbstractQuotaMatcher and and Vincenzo's RecipientIsOverFixedQuota
Posted by Marco Tedone <mt...@jemos.org>.
Noel, Vincenzo, thank you for you job, it emproved the code in a good way:
in particular the parserQuota method allow the AbstractQuotaXML class to
work with the <uom> elements as well, and it saved to me further time to
spend on this issue.
Noel, could you please explain the intent of testing for a LocalServer or a
LocalUser?
As I said yesterday, I can't get the point to include those conditions in a
Size Matcher, but it is due to the fact that I'm not aware of the Local
Delivery architecture.
Marco
----- Original Message -----
From: "Noel J. Bergman" <no...@devtech.com>
To: "James Developers List" <ja...@jakarta.apache.org>
Sent: Wednesday, May 14, 2003 5:03 AM
Subject: Marco's AbstractQuotaMatcher and and Vincenzo's
RecipientIsOverFixedQuota
> Marco and Vincenzo,
>
> Attached are your AbstractQuotaMatcher and RecipientIsOverFixedQuota
> matchers. I have tested them with all current repository types: file, db
> and dbfile.
>
> I made a few changes to the code you sent.
>
> The first change that I made was to use MailAddress instead of String as
the
> parameter type. That preserves some information that would otherwise be
> lost, and could be useful (see next paragraph).
>
> Another change was to make getStorageUsed public, allowing a specialized
> class to reimplement it, and to rename the methods getUsed() and
getQuota().
> The reason for these changes was to support other types of quota-based
> matchers, such as message count, monthly transfer, etc.
>
> Next, I moved Vincenzo's quota string parser into the base class so that
> others can make use of it.
>
> Finally, I tweaked the exception handling. The exception handling changes
> are necessary because the current file-based repository has a really nasty
> habit of tossing RuntimeExceptions when it can't find a file, e.g.,
because
> the user is currently deleting mail from the inbox.
>
> OK, guys ... what do you think? And thanks for all of your hard work!
:-)
>
> By the way, the overhead of using this matcher with our current JDBC-based
> repositories is pretty brutal (although probably not even noticable for
most
> users), but it works quite well with the file-based repository.
>
> --- Noel
>
----------------------------------------------------------------------------
----
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: james-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: james-dev-help@jakarta.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: james-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: james-dev-help@jakarta.apache.org
Re: Marco's AbstractQuotaMatcher and and Vincenzo's RecipientIsOverFixedQuota
Posted by Marco Tedone <mt...@jemos.org>.
Noel, Vincenzo, I think there is a problem with the AbstractQuotaMatcher
code you sent; in particular I think that the getUsed(MailAddress recipient)
method should interact with the parseQuota(String amount) method, otherwise
we would risk asynchronous data between the storage used and the storage
allowed.
One solution would be in the getUsed() method to return a sort of:
return parseQuota(size + "?"), where instead of "?" we should think to a way
to receive a parameter from the Matcher definition. At this point, I'm
wondering if we shouldn't implement a method like the one used in the
"RecipientIs" class to allow more than one parameter.
What do you think?
Marco
----- Original Message -----
From: "Noel J. Bergman" <no...@devtech.com>
To: "James Developers List" <ja...@jakarta.apache.org>
Sent: Wednesday, May 14, 2003 5:03 AM
Subject: Marco's AbstractQuotaMatcher and and Vincenzo's
RecipientIsOverFixedQuota
> Marco and Vincenzo,
>
> Attached are your AbstractQuotaMatcher and RecipientIsOverFixedQuota
> matchers. I have tested them with all current repository types: file, db
> and dbfile.
>
> I made a few changes to the code you sent.
>
> The first change that I made was to use MailAddress instead of String as
the
> parameter type. That preserves some information that would otherwise be
> lost, and could be useful (see next paragraph).
>
> Another change was to make getStorageUsed public, allowing a specialized
> class to reimplement it, and to rename the methods getUsed() and
getQuota().
> The reason for these changes was to support other types of quota-based
> matchers, such as message count, monthly transfer, etc.
>
> Next, I moved Vincenzo's quota string parser into the base class so that
> others can make use of it.
>
> Finally, I tweaked the exception handling. The exception handling changes
> are necessary because the current file-based repository has a really nasty
> habit of tossing RuntimeExceptions when it can't find a file, e.g.,
because
> the user is currently deleting mail from the inbox.
>
> OK, guys ... what do you think? And thanks for all of your hard work!
:-)
>
> By the way, the overhead of using this matcher with our current JDBC-based
> repositories is pretty brutal (although probably not even noticable for
most
> users), but it works quite well with the file-based repository.
>
> --- Noel
>
----------------------------------------------------------------------------
----
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: james-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: james-dev-help@jakarta.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: james-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: james-dev-help@jakarta.apache.org
RE: Marco's AbstractQuotaMatcher and and Vincenzo's RecipientIsOverFixedQuota
Posted by "Noel J. Bergman" <no...@devtech.com>.
> The reason for these changes was to support other types of quota-based
> matchers, such as message count, monthly transfer, etc.
Perhaps we change the specification for getQuota() so that it returns
Long.MAX_VALUE if there is no quota, e.g., for a non-local recipient, and
remove the check from the base class? The getUsed() method would likewise
have to do its own check. This would help with those other types of
quota-based matchers that might not have the same local versus non-local
semantics.
A revision doing that is attached. And I'm calling it a night.
--- Noel