Skip to Content.
Sympa Menu

devel - Re: [sympa-developpers] [sympa-commits] sympa[11063] trunk/src/lib/Sympa/Message.pm: [dev] pass required values as parameters, instead of accessing configuration directly

Subject: Developers of Sympa

List archive

Chronological Thread  
  • From: David Verdin <address@concealed>
  • To: address@concealed
  • Subject: Re: [sympa-developpers] [sympa-commits] sympa[11063] trunk/src/lib/Sympa/Message.pm: [dev] pass required values as parameters, instead of accessing configuration directly
  • Date: Wed, 25 Jun 2014 12:13:33 +0200

Hi guys,

I agree with Guillaume regarding the test issue. We need to be able to test functions separately and easily.

However, I'd like the Message object to be able to perform some operations standalone. For exemple, decrypting a message. When we create a message object from sympa.pl, we just have a file that we pass to the Message::new method.
The Message object will be able to analyse the message headers, see that it is a crypted message and decrypt is if necessary. For that it needs to access configfuration parameters and we can't anticipate what parameters will be needed.

Additionnaly, these parameters will change from a virtual host to another (the DKIM private key for example). The problem is: only when a Message has been analyzed (using the Message object) can we know what virtual host is to be used.

I don't want the main Sympa binaries to analyze headers. We put all this stuff in modules because they are complex operations that require specialized modules.

So maybe we could improve message handling by first calling Message::new, then perform a set of tests in the main daemons : Message::is_crypted, Message::is_dkim_signed() and so on. But that means performing all these operations anytime we handle a new message, anywhere in the code. That looks like redundant operations.
So maybe we should regroup them in a dedicated sub, but how to factorize such sub if we don't want modules to access configuration?

Another solution would be to do minimal parsing in Message::new(), just to know what list and what virtual host the message is related to, and then give them all the potentially useful parameters in one call (Message::set_contextual_parameters(%params), for example). Taht would mean give a lot of potentially useless parameters but, at least, we would never have to bother passing these parameters to a pre-existing Message object.

No decision in this mail. The mechanisms exposed here are meant to explain why we found usefull to query the configuration from the Message module.

Regards,

David

Le 25/06/14 11:18, Guillaume Rousse a écrit :
address@concealed">Le 25/06/2014 10:42, IKEDA Soji a écrit :
Guillaume,

Can a method like Site->set_parameter() solve the problem you are
encountered?
As stated in my original answer, the problem I see is direct access to the application configuration from a message object. Changing anything on Site class side won't help here.

Simple components (messages, lists, spools, etc...) should not access the configuration themselves, because that makes them dependant of current configuration API. That is usually referred as "strong coupling", and makes changing parts of Sympa code difficult.

It also make those components able to read whatever information they need, without declaring it explicitely (so long for explicit interfaces), and even potentially change this configuration silently (so long for encapsulation). Giving more privileges than needed to a specific component also violates defensive programation principles.

The simple solution to both issue is just to restrict configuration access to the application core, and provide all relevants information to software components either at instanciation time, either when calling specific methods.

--
A bug in Sympa? Quick! To the bug tracker!

 
David Verdin
Études et projets applicatifs
 
Tél : +33 2 23 23 69 71
Fax : +33 2 23 23 71 21
 
www.renater.fr
RENATER
263 Avenue du Gal Leclerc
35042 Rennes Cedex



PNG image

Attachment: smime.p7s
Description: Signature cryptographique S/MIME




Archive powered by MHonArc 2.6.19+.

Top of Page