Re: [Mailman-Developers] Migration of header_filter_rules
On Sep 11, 2015, at 04:25 PM, Aurelien Bompard wrote:
If you want to look at it : https://gitlab.com/abompard/mailman/commits/import-header-matches
I took a brief look, and I think you're heading in the right direction. I'll do a more details review once the mp is up[*].
In the code and in the tests, I'm importing the new model object, HeaderMatch. Is it OK? Should I go through the corresponding interface instead? It looks like models are almost never imported from throughout the code (except in the model module), and I don't know if that's a best practice that should be followed here. If it is and you have details on the reasoning, I'm interested :-)
You're right that model classes should never be imported outside mailman/model although there are a few violations of this rule. It's less hard-and-fast for tests since sometimes that's just the most convenient way to handle it.
In cases like this though, the typical pattern is to create a "set" class in the model, and use it to create, delete, and lookup model objects in the set. For list-centric collections, you then also create an ZCA adapter which returns an object in the context of the mailing list.
It's always easier to understand examples, so here's a classic.
Mailing lists have an associated set of acceptable aliases which are kept in the acceptableasliases table. Individual alias entries are defined by the IAcceptableAlias interface and implemented by the AcceptableAlias class. There's also an IAcceptableAliasSet and AcceptableAliasSet pair (the latter which does *not* derive from Model), and you'll find .add(), .clear(), and .remove() methods, and a convenience .aliases attribute.
So, you never add AcceptableAlias objects directly, you always do it through the IAcceptableAliasSet interface. How do you get an instance of that without importing from the model subpackage? You *adapt* an existing mailing list object to that interface:
from mailman.interfaces.mailinglist import IAcceptableAliasSet
...
alias_set = IAcceptableAliasSet(my_mlist)
The thread that ties it all together is this claus in src/mailman/config/configure.zcml (the configuration file for the Zope Component Architecture -- ZCA):
<adapter for="mailman.interfaces.mailinglist.IMailingList" provides="mailman.interfaces.mailinglist.IAcceptableAliasSet" factory="mailman.model.mailinglist.AcceptableAliasSet" />
Thus when you call the interface, passing in the IMailingList instance, the ZCA looks up a factory providing that interface, and calls the object, passing in the argument. You can see that AcceptableAliasSet takes a single argument, the mailing list object which provides the context for that object's operations.
I think header checks will probably work almost exactly the same way.
Cheers, -Barry
[*] Side note: NFL season starts here in the USA this weekend, and there's no better way to salvage a Sunday afternoon normally wasted watching your crappy ass home team disappoint for another year than doing some fun Mailman hacking as you scream at the TV. Just resist the urge to throw your laptop through the TV!
You're right that model classes should never be imported outside mailman/model although there are a few violations of this rule. It's less hard-and-fast for tests since sometimes that's just the most convenient way to handle it. [...]
Great explanation, thanks. I'll send a merge request right now because I won't be able to do the interface dance this weekend, and you may want to check out the rest of the code. I'll create the required sets and interfaces on Monday.
Have a fun NFL season start! May all teams win! ;-)
Aurélien
participants (2)
-
Aurelien Bompard
-
Barry Warsaw