code modularization

Discussion re sg development. You don't have to be a developer.

code modularization

Postby josh » Wed Feb 04, 2004 7:44 pm

This is a thread to discuss [further] modularization of sg code. Currently, the web code and the mail code are wholly distinct, but they're starting to share more functions, as we add the "send the first message" feature. Here's the state of things:


spameater doesn't use modules of it's own, and it has the spamgourmet.config file for configuration info.

the web code does have modules:

1) WebConfig.pm - provides the database connection and configuration data, which is hard coded in the module (no config file)
2) Session.pm - provides web session management functionality, and wraps CGI.pm and Dialogs.pm
3) Dialogs.pm - provides the translated versions of dialogs
4) Page.pm - provides template building functionality

Here's what we could do:
a) rename WebConfig.pm to be SpamgourmetConfig.pm, or something, and cause it to read configuration data from a config file. Both the web code and spameater code can then use it for database and configuration params.
b) add a new module SpamgourrmetUtils.pm that contains the code that is shared between spameater and the web code

Then, we'd deploy the same module set for both web and mail:
spameater would load and use SpamgourmetConfig.pm and SpamgourmetUtils.pm, and leave the others alone, and the web code could start using SpamgourmetUtils.pm to access the shared code.

spameater could perhaps use Dialogs.pm for translations of the Subject tagline (currently, it's English-only). I believe the module already supports loading different sets of dialogs, so that spameater wouldn't have to load up the big set of web dialogs. We would have the restriction that subject text be in ascii -- the full utf-8 set isn't supported by SMTP, AFAIK -- that could be hairy.

Does that sound good?
josh
 
Posts: 1371
Joined: Fri Aug 29, 2003 2:28 pm

Postby SysKoll » Wed Feb 04, 2004 7:49 pm

Josh,

That's a Good Thing you're proposing here.
-- SysKoll
SysKoll
 
Posts: 893
Joined: Thu Aug 28, 2003 9:24 pm

Postby josh » Thu Feb 05, 2004 10:25 pm

Does anyone have a favorite (*fast*) way of loading values from a properties/config file?

spameater currently just includes the config file, which is, syntacticly, perl. I'm telling myself there's no security problem here, since the config file is well protected in a good deployment.

If no one says anything, I'm going to do the same thing with SpamgourmetConfig.pm, then in the constructor, set the $self->{'variablename'} equivalents to the class globals that are defined in the config file.
josh
 
Posts: 1371
Joined: Fri Aug 29, 2003 2:28 pm

Postby josh » Thu Feb 05, 2004 11:51 pm

I decided to proceed with reworking the config file for the modules. I thought I'd try it out first on index2.pl, but I messed up and switched the modules over for the whole site :oops:

Anyway, after a few 500's, the site's up again with the new package names and SpamgourmetConfig.pm reads its values from a config file.
josh
 
Posts: 1371
Joined: Fri Aug 29, 2003 2:28 pm

Postby josh » Fri Feb 06, 2004 8:10 pm

spameater is now up with the set of modules, but the Utils module is still empty. I'll tackle that next.

I realize I'm not being a good team player by fast-tracking all this. When I'm done, I'll want to be doing a big check-in into cvs of the modules and the updated spameater, then we start acting like a project again.

Any thoughts on a good way to structure cvs to accomodate
1) the production version of the spameater script
2) the "next-gen" version(s) of the spameater script (with Mail::Audit)
3) the web specific code
4) the modules

?
josh
 
Posts: 1371
Joined: Fri Aug 29, 2003 2:28 pm

Postby SysKoll » Fri Feb 06, 2004 10:32 pm

Josh,

I believe the simplest way to structure this would be separate directories. You can have a next-gen directory in CVS. Same for the web stuff.
-- SysKoll
SysKoll
 
Posts: 893
Joined: Thu Aug 28, 2003 9:24 pm

Postby josh » Sat Feb 07, 2004 4:00 am

I added the files to CVS (I think -- they're not showing up in the web cvs browse thing). I moved what was there to a subdirectory called 'mailhandler', and I added a directory called Mail (the modules are in there), I also added a directory called 'production' and put the current spameater there. I put the current spamgourmet.config (which now gets loaded into SpamgourmetConfig.pm) in the root.

I haven't yet added the next-gen directory.
josh
 
Posts: 1371
Joined: Fri Aug 29, 2003 2:28 pm

Postby Guest » Sat Feb 07, 2004 4:00 pm

I don't agree with creating a next-gen directory. The reason being that it will create two copies of the files in CVS. This is better handled with the CVS branching mechanism. i.e. create a branch before a release, make production code run off the branch version of the files. This way development can continue on the main branch while hot-fixes can be applied to the production branch if needed.
Guest
 

Postby maratheamit » Sat Feb 07, 2004 4:10 pm

Previous message was posted by me. Amit.
maratheamit
 
Posts: 82
Joined: Fri Aug 29, 2003 2:35 pm

Postby josh » Sat Feb 07, 2004 8:49 pm

I've continued to refactor and keep CVS updated with changes to the modules and spameater. The web code is still so hideous that I haven't checked it in yet.

I don't know how to make a branch :oops: but that sounds like a great idea -- doesn't it?

Further along those lines, should we introduce two more modules?
a) one that deteremines all the relevant information from the message -- MessageParser ? (my code doesn't parse, I know)
b) one that sends mail -- Mailer ?

Each would wrap either the existing code or the Mail::Audit code, and that way we could just branch those two?
josh
 
Posts: 1371
Joined: Fri Aug 29, 2003 2:28 pm

Postby josh » Sun Feb 08, 2004 5:20 am

btw - as I was picking through spameater, I added another feature that lets you omit the number altogether -- that is, you can use an address like:
word.user@spamgourmet.com

I don't know how desirable this feature really is, but it was pretty easy to add (just keep matching to see if you get the xxx.xxx pattern), so I went ahead. If it sounds like a good thing, we can figure out how to document it.

the getNumberFromString() method returns a default of 3 if it can't figure out what to do, and that's the case here, so the address is created with 3 messages. Later, we can add a column to the User table to store a user default count.



I also externalized the delimiters to the config file:

Code: Select all
$delimiters = '\.|~';


not really pretty, because you have to escape any that have regex significance, and you have to use the pipe. I didn't want to make it so that spameater has to parse it, though, so that was that...

We're pretty much stuck with the two (. and ~) because we've got many usernames and words with other conceivable characters, but I thought it would be nice to put it in the config file anyway.


there's still a long march until the code I wrote gets as pretty as what you all write :), but it's moving
josh
 
Posts: 1371
Joined: Fri Aug 29, 2003 2:28 pm

Postby SysKoll » Sun Feb 08, 2004 8:50 am

Josh,

Cool new features.

The "no number = 3" thing can be added to the advanced mode tab, I think.
-- SysKoll
SysKoll
 
Posts: 893
Joined: Thu Aug 28, 2003 9:24 pm

Postby josh » Sun Feb 08, 2004 2:06 pm

back to the branching topic -- maybe we can use a simple factory pattern and avoid branching altogether? eg, in the configfile, we could have:
Code: Select all
$parsermodule = 1; # 1 standard, 2 legacy, (3 milter?)
$mailermodule = 1; # 1 standard, 2 legacy, etc.

(please let's come up with a better word than parser)

Then, for instance, the Mailer module could take in an instance of SpamgourmetConfig
Code: Select all
my $mailer = Mailer->getInstance(config=>$config);


then it could look at the config setting
Code: Select all
sub getInstance() {
  my $config = shift;
  my $type = $config->getMailerModuleSetting();
  if ($type == 1) {
    return StandardMailer->new(config=>$config);
  } elsif ($type == 2) {
    return LegacyMailer->new(config=>$config);
  } elsif ($type == 3) {
    return MilterMailer->new(config=>$config);
  } else {
    die 'unsupported mailer type';
  }
}


(not that we have a miltermailer, of course) -- we'd probably want to implement some caching capability in the MailerFactory so it wouldn't be creating new objects for each call if it were running as a daemon.

The various implementation modules would use a common "interface" that we decide on.

Anyway, then, no branch. It would also let us play with mixing the Mail::Audit stuff with the legacy parser in production -- possibly a lot less heavy, because we'd only require the Mail::Audit modules in the Mailer implementation, which, as we know, would only get called 1 out of 10 times.
josh
 
Posts: 1371
Joined: Fri Aug 29, 2003 2:28 pm

Better word for the message parser

Postby SysKoll » Mon Feb 09, 2004 1:04 am

(please let's come up with a better word than parser)

Josh,

If you are looking for a better word to designate the piece of code that analyzes the message header and looks for its various fields (from, to, subject, etc.), you can call it the Message Decoder (or Messagealyzer if you feel inspired by police equipment :-)).
-- SysKoll
SysKoll
 
Posts: 893
Joined: Thu Aug 28, 2003 9:24 pm

Postby josh » Tue Feb 10, 2004 6:50 am

OK - I decided to dispense with the factory class and just put the two line factory method as Config->getMailer()
Code: Select all
sub getMailer {
  my $self = shift;
  return $self->getMailerClass()->new(config=>$self);
}

sub getMailerClass {
  my $self = shift;
  return $self->{'mailerclass'}; # classname set in config file
}


and in spamgourmet.config
Code: Select all
## this setting determines which class will be used to send email
  $mailerclass = 'Mail::Spamgourmet::CommandLineMailer';


And I went ahead and moved the existing mailer methods (web and spameater) into an "implementation" - Mail::Spamgourmet::CommandLineMailer

This is all in cvs and live on the mail server, "beta" on the webserver (for the next few minutes).

How about Mail::Spamgourmet::MessageAttributeHandler as the base name for the other set of classes -- too long, but getting there?
josh
 
Posts: 1371
Joined: Fri Aug 29, 2003 2:28 pm


Return to Developers

Who is online

Users browsing this forum: No registered users and 18 guests

cron