If you are bored and looking for someone to beat some sense into, read on. Note: this post is somewhat of a followup to my question yesterday.
First, a little background to put this in context. I started learning Perl in the late 90s. I'm a very on again/off again programmer. I don't code for a living and I write some hellacious spaghetti code. But every year or two I get the programming bug but usually end up biting off more than I can chew and/or get sidetracked with other stuff. But the last couple of weeks I've decided to pour my heart into getting as good as I can get at programming with perl so I can take on some larger projects I'd like to work on. First, to sharpen my chops, I decided to work on a smaller project, a family of modules and roles to manipulate my webserver from my local machine. The primary purpose of this project is not to write the best possible mechanism for issuing commands to a remote server. While I want this program to be useful, its primary purpose is to help me cut my teeth more with Moose, seeing what it can do, and getting more adept with it and other tools (like testing, vim, etc.).
So anyway, what I'm looking for is a critique of what I have to see if I'm way out to lunch. I'm particularly concerned with how I'm using roles, which seems very convoluted. I'll explain in more detail as I show the code below. I have more to code but I'm far enough along to have enough shape to it. What I have written so far works and has been tested. Feel free to bash me. I can take it.
So first I have my
WebServerRemote class. It is intended to be the kind of glue that holds my family of my modules together and does odd tasks and delegates other tasks out to other modules and subclasses:
package WebServerRemote 0.000001;
use Carp;
use Moose;
use Data::Dumper;
use Modern::Perl;
use Params::Validate;
use MyWebsite;
use namespace::autoclean;
with 'MyOpenSSH';
with 'Apache2Info';
sub get_file {
validate_pos(@_, 1, 1);
my $self = shift;
my $file_path = shift;
return $self->capture("cat $file_path");
}
# get website objects based on domain name
sub get_websites {
validate_pos(@_, 1, 1);
my ($self, $domain) = @_;
my @websites = (); # website o
+bjects
my $config_files = $self->lookup_config_files($domain); # list of c
+onfig files
croak 'No config files found with ' . $domain if !@$config_files;
foreach my $file (@$config_files) {
my $config = $self->get_file($file);
#my @cmds = qw( servername, suexecusergroup, customlog, serveralia
+s );
foreach my $docroot (@{$self->get_docroots_from_string($config)})
+{
my $vh = $self->get_vh_context($config, 'documentroot'
+, $docroot);
my @aliases = ();
my $aliases = '';
while (my $alias = $vh->cmd_config('serveralias')) {
push @aliases, $alias;
}
$aliases = join ', ', @aliases;
my $suexec = $vh->cmd_config('suexecusergroup') || '';
my $servername = $vh->cmd_config('servername') || '';
my $errorlog = $vh->cmd_config('errorlog') || '';
push @websites,
MyWebsite->new (
docroot => $docroot,
apache_config_path => $file,
domain => $servername,
suexecgroup => $suexec,
aliases => $aliases,
error_log => $errorlog,
ssh => $self->ssh->get_user . '@' . $self
+->ssh->get_host,
);
}
}
return \@websites;
}
sub check_dir_for_files {
validate_pos(@_, 1, 1, 1);
my $self = shift;
my $dir = shift;
my $files = shift;
my $listing = $self->capture('ls -1 ' . $dir);
my %files = map { $_ => 1 } split /\n/, $listing;
my @fail = ();
# $files can be a scalar or an array
if (ref $files) {
push @fail, grep { !exists $files{$_} } @$files;
return !@fail;
} else {
return $files{$files};
}
}
##########################################
The module above consumes two roles. First, there is MyOpenSSH which is just a wrapper for Net::OpenSSH:
package MyOpenSSH 0.000001;
use Carp;
use Data::Dumper;
use Moose::Role;
use Modern::Perl;
use Net::OpenSSH;
use Params::Validate;
has 'ssh' => (is => 'rw', isa => 'Net::OpenSSH', required => 1, lazy =
+> 0, handles => qr/[^(capture)]/, );
around BUILDARGS => sub {
my $orig = shift;
my $class = shift;
my %args = ref $_[0] ? %{$_[0]} : @_;
croak 'a host must be supplied for ssh: ssh => (\'<user>@<host>\', %
+opts)' if !%args;
my ($host, %opts) = $args{ssh};
return $class->$orig( %args) if ref $host eq 'Net::OpenSSH';
delete $args{ssh};
my $ssh = Net::OpenSSH->new($host, %opts);
$ssh->error and croak "could not connect to host: $ssh->error";
return $class->$orig( ssh => $ssh, %args );
};
# wrapper for system method
sub exec {
validate_pos(@_, 1, 1);
my $self = shift;
my $cmd = shift;
$self->ssh->system($cmd) || carp 'Command failed: ' . $self->ssh->er
+ror;
}
# wrapper for capture method
sub capture {
validate_pos(@_, 1, 1);
my $self = shift;
my $cmd = shift;
$self->ssh->capture($cmd) || carp 'Command failed: ' . $self->ssh->e
+rror;
}
###########################################
Now, I'm sure the first question will be, "Why is he using Net::OpenSSH and not just doing it directly on the machine?" Well, mostly because I wanted to get familiar with it and also because I want to be able to develop everything on my local machine to see if it can be done. I'm sure the other question will be "Why a wrapper for Net::OpenSSH?" The answer to that is twofold: I wanted to see how it might be done and two, I don't want to have to remember how to construct a Net::OpenSSH object. I can now create a WebServerRemote object with something as simple as $wsr = WebServerRemote(ssh => me@host). Yeah, I'm that lazy.
I found some nice side benefits to wrapping Net::OpenSSH. For example, I can automatically check for errors every time I run a command on the remote server.
Now, the BUILDARGS is the most interesting (convoluted?) feature of the Net::OpenSSH role. It was hacked together with trial and error until I got it to work. I will get back to this later. It's a doozy.
So, the other role I have is called Apache2Info. Its job is to do boring things related to retrieving information from Apache config files. So far, it mostly has methods I will use for reporting. I've left out a lot of the code of this role because it's not very interesting or pertinent to this post:
package Apache2Info 0.000001;
use Carp;
use Try::Tiny;
use File::Spec;
use File::Util;
use Moose::Role;
use Data::Dumper;
use Modern::Perl;
use File::Listing;
use File::Basename;
use Params::Validate;
use Apache::ConfigFile;
requires 'ssh';
# get document roots of a config file
sub get_docroots_from_string {
--snip--
}
# searches a config file string for a command and returns virtual host
+ config
# if a match is found
sub get_vh_context {
--snip--
}
# Apache::ConfigParser requires a path to a file as an argument
# so we save contents to a file first and then read it
sub _read {
--snip--
}
# get list of absolute, non-canonical paths to all apache configuratio
+n file
sub get_enabled_apache_config_filenames {
--snip--
}
# get a listing of all directories where config files reside
sub get_config_file_dirs {
--snip--
}
# get all docroots
sub get_all_docroots {
--snip--
}
# find the config files for a given domain name
sub lookup_config_files {
--snip--
###############################################
The only really interesting thing here is the requires 'ssh' bit because this role needs a Net::OpenSSH functionality to get stuff from the server. I satisfy that in my consumers by having an ssh attribute. You'll notice the ssh attribute is supplied by the MyOpenSSH role. This is also where stuff gets kind of convoluted.
The last piece of the puzzle is the MyWebsite class which extends WebServerRemote. I'm not sure if this is a good idea or not but in the interest of experimenting I decided to try it and see what happens. My thinking on this was that since I want my website objects to use the MyOpenSSH role and Apache2Info, that I could just extend WebServerRemote and have those features automatically included. Also, there are methods in WebServerRemote that will be used by my MyWebsite. So, anyway, here is the code:
package MyWebsite 0.000001;
use Carp;
use Moose;
use Modern::Perl;
use Drupal;
use WordPress;
extends 'WebServerRemote';
with 'MyOpenSSH';
use namespace::autoclean;
has 'db' => (is => 'rw', isa => 'Str', required => 0,
+lazy => 1, default => '', );
has 'ver' => (is => 'rw', isa => 'Str', required => 0,
+lazy => 1, default => '', );
has 'type' => (is => 'ro', isa => 'Str', required => 0,
+lazy => 1, builder => '_set_type');
has 'domain' => (is => 'rw', isa => 'Str', required => 0,
+lazy => 1, default => '', );
has 'aliases' => (is => 'rw', isa => 'Str', required => 0,
+lazy => 1, default => '', );
has 'docroot' => (is => 'rw', isa => 'Str', required => 1,
+lazy => 0 );
has 'db_user' => (is => 'ro', isa => 'Str', required => 0,
+lazy => 1, default => '', writer => '_set_dbuser', );
has 'db_pass' => (is => 'rw', isa => 'Str', required => 0,
+lazy => 1, default => '', );
has 'root_dir' => (is => 'rw', isa => 'Str', required => 0,
+lazy => 1, default => '', );
has 'error_log' => (is => 'rw', isa => 'Str', required => 0,
+lazy => 1, default => '', );
has 'site_config' => (is => 'rw', isa => 'Str', required => 0,
+lazy => 1, default => '', );
has 'suexecgroup' => (is => 'rw', isa => 'Str', required => 0,
+lazy => 1, default => '', );
has 'apache_config' => (is => 'rw', isa => 'Str', required => 0,
+lazy => 1, default => '', );
has 'site_config_path' => (is => 'rw', isa => 'Str', required => 0,
+lazy => 1, default => '', );
has 'apache_config_path' => (is => 'rw', isa => 'Str', required => 0,
+lazy => 1, default => '', );
sub _set_type {
my $self = shift;
# check for drupal multi site
if ($self->check_dir_for_files($self->docroot, ['sites', 'includes',
+ 'modules'])) {
$self = Drupal->meta->rebless_instance($self);
return 'drupal';
}
if ($self->check_dir_for_files($self->docroot, 'wp-config.php')) {
$self = WordPress->meta->rebless_instance($self);
return 'wordpress';
}
if ($self->check_dir_for_files($self->docroot, 'settings.php')) {
$self = Drupal->meta->rebless_instance($self);
return 'drupal';
}
}
##############################################
So, I have to have the with 'MyOpenSSH'; bit in there. I thought by extending MyWebsite I wouldn't need that. However, when I remove that line, things break when the _set_type method gets run. I get an error saying there is no capture method which is found in MyOpenSSH. But putting this line wreaked all other kinds of havoc. I think the ssh attribute was trying to get set twice. I'm not sure. Anyway, after fiddling with the BUILDARGS method of MyOpenSSH and playing with the order of the with statements, I was able to get it to work somehow. I feel in my bones this is a horrible hack but I don't know how to fix it properly.
The other thing I do is apparently what's called an "object factory" where the MyWebsite object detects what kind of website it is and then subclasses itself when the _set_type method is called. Perhaps this is a bad idea. I'm not sure if there's a real good reason to do it except to see if it can be done. But I'm thinking it may come in handy because different kinds of websites will have different methods.
Alright, that's it. Feel free to beat on me if this is a ridiculous mess. :)
