Developers of Sympa

[Etienne MELEARD <address@concealed>]

Hi all,

Le 30/09/13 17:51, Guillaume Rousse a écrit :
> Le 24/09/2013 10:50, David Verdin a écrit :
> > Hi Guillaume,
> >
> > Le 19/09/13 19:18, Guillaume Rousse a écrit :
> > > Le 19/09/2013 17:35, David Verdin a écrit :
> > > > Well, taht's it. I think I went through most of our recent discussion
> > > > and proposed a consensus. Here's to you now!
> > > I'd prefer to sort issues, before we rush again into an endless
> > > discussion about minor API details.
> > No, it's not about having a discussion. I spent a lot of time digging
> > through the discussions we had since several months and looked for the
> > consensus points;
> > I would like to get rid of all these minor points, so that they are all
> > out of our system. This way, they won't come back when we keep on
> > working on the code.
> > So if you could please just validate all these proposals, this would
> > really simplify our work.
> Some of these points are easy to conclude (style issues, notably), 
> others will be more difficult as long as we don't have a workable code 
> base to compare actual code. But OK, let's try to review them.
>
> [..]
> > > However:
> > > a) I don't think we need an 'is a sort of' relationship (inheritance)
> > > between the two concepts: AFAIK, an apache virtual host is not a
> > > specialized kind of apache host. It's just a way to simulate a
> > > distinct web server from the outside, by overriding some specific
> > > configuration parameters (not all), depending of the context. They are
> > > different, apparented concepts, but without specialisation 
> > > relationship.
> > We need the mechanism to express the fact that we can define
> > configurations and software behaviour at different level : Site ->
> > virtual host -> list.
> > The things you can define at each level :
> > - scenari
> > - configuration parameters
> > - templates (web and mail)
> > - edit_list.conf
> > - etc.
> >
> > So the inheritance mechanism is a perfectly legitimate way to factorize
> > reusable code at each of these levels.
> I'm afraid you're confusing inheritance semantics ("is a sort of") 
> with one of its side-effect (code factorization). A mailing list is 
> not a specific kind of virtual host, even if they share some traits 
> (not *all*). In particular, you can not pass a mailing list object to 
> a code expecting a virtual host object.
>
> Here is an alternative concept hierarchy:
> - ConfigurableObject
> |- Site
> |- VirtualHost
> |- List
>
> This way, a site, a virtual host and a list are all specific kind of 
> configurable objects, but none is a subtype of any other. You got both 
> code factorization, and proper object semantics.

I can't agree more, in my mind inheriting inner workings and features is 
not the same as cascade configuration parameter seeking, what you can 
inherit is some way to get the config parameter value though ...

In my mind I picture it like this :

/!\ WARNING /!\
May not be accurate perl, for example I used croak as an exception 
throwing but I may be mistaken about it :p

I'll may add another inherited class of ConfigurableObject "above" Site 
that is Default, or just use the configuration parameters map of 
Site/Host defaults, but I think that putting it in a separate class does 
not add much overhead and adds the benefit to be easier to manage 
(defaults are all in a small package, Host package is lighter in terms 
of code), don't really know ...

Using the terminology Guillaume proposed, the classes inheritance model 
would be :

- ConfigurableObject
|- Default (singleton) (opt)
|- Host (singleton)
|- VirtualHost
|- List


The configuration inheritance model would be implemented by "linking" 
objects with their "configuration parent" :

Default's singleton's parameters are populated at startup (this one is 
easy to implement, only a hash, opt).
Host's singleton's is created, populated from parsed config (and linked 
to Default's singleton) at startup.
VirtualHost objects are created, populated from parsed config, linked to 
Host's singleton and cached when first used.
List objects are created, populated from parsed config, linked to 
"their" VirtualHost instance and cached when first used.


Seeking a value for a configuration parameter can thus be a method of 
ConfigurableObject like :

sub get_parameter {
    my ($me, $key) = @_;

    my %parameter_definition = %{$me->{'configuration_map'}{$key}}; # 
Maybe configuration_map should be a class variable ?
    croak "$me has no $key parameter"
        unless defined %parameter_definition;

    return $me->{'parameters'}{$key}
        if defined $me->{'parameters'}{$key};

    croak "$me has no defined value for parameter $key and cannot 
inherit it"
        unless defined $parameter_definition{'is_inherited'};

    croak "$me should inherit parameter $key but has no configuration 
parent"
        unless defined $me->{'configuration_parent'};

    return $me->{'configuration_parent'}->get_parameter($key);
}

No default handling here, should be added ...

Regards,

Etienne

> > > b) we should find better names for those concepts. For instance:
> > > - Host and VirtualHost
> > > - Configuration and Overlay
> > > - etc...
> > Site is a very good name, because it represents what is defined for the
> > whole Sympa instance. The "site" concept is therefore pertinent : the
> > site is the place where you regroup all your virtual hosts;
> > Robot is badly named -the fault lying here in Sympa historic naming
> > patterns - we should name it virtualhost, or vhost.
> The problem with this terminology (Site/VirtualHost) is that you're 
> opposing two term pairs:
> - Site vs Host
> - '' vs Virtual
>
> Basically, there is nothing in those names linking those concepts 
> together. Hence my proposal of 'Host/VirtualHost'.
>
> > > c) I'm strongly opposed to storing anything variable in class
> > > variables. Even if can only have one unique Site, we should use an
> > > instance for this (singleton pattern, for design pattern addicts)
> > Works for me.
> > > Now; for the Site_r concept, I'm puzzled: what is a Site_r object
> > > supposed to be ? And how is a mailing-list a specific kind of Site_r ?
> > >
> > > If Site_r is just a place holder for generic code, it should better
> > > get renamed as 'Object', 'ConfigurableObject' or some other name
> > > following 'qualifier-noun' pattern.
> > Site_default ?
> A list is still not a kind of 'Site default' :)