in reply to Re: My first module. I can't find any problems with it!
in thread Code review request for my first module - WWW::PostiniAdmin

I like the idea of reading the URI data from a config file, but I would hate to confuse people by including one. Also, the sections for parsing the input are pretty specific to this site as well, so I am not so sure that pointing it at different URI's would be a Good Thing. I expect to possibly send this in to postini for them to distribute, but I don't see it ending up on CPAN so I think that the specific nature works

I would probably have figured this part out the part about 'our @ISA' and the rest, but I am not using them, and I got my stubs from h2xs and that is what it generated, so I assumed they were correct. Silly me :-)

Interesting, I haven't used the 3-arg version before. Kewlness. I will change it to open DOMAINS, '>', 'Domains.txt' or die and use that form in the future!

Here is the full code of the module:

package WWW::PostiniAdmin; use 5.006; use strict; use warnings; require Exporter; require LWP::UserAgent; our $VERSION = '0.05'; =pod =head1 NAME WWW::PostiniAdmin - Perl extension for accessing Postini's administati +on website =head1 SYNOPSIS =head2 EXAMPLE 1 use WWW::PostiniAdmin; # Create a new instance my $postini = new WWW::PostiniAdmin; # and log in $postini->Login($User, $Password) || die $postini->Last_Error; # Try to retrieve the list of users. my $users = $postini->Get_Users_For_Domain('domain.name') || die $postini->Last_Error; # Show the date the list says it was downloaded print "Downloaded list: $users->{'Downloaded'}\n"; # do some simple output of the message centers to a CSV foreach my $user ($users->{'Column_Names'}, @{ $users->{'Users'} } ) +{ print join ",", @$user; print "\n"; } =head2 EXAMPLE 2 use WWW::PostiniAdmin; # Create a new instance my $postini = new WWW::PostiniAdmin(); # and log in $postini->Login($User, $Password) || die $postini->Last_Error; # Get the list of domains my $domains = $postini->Get_Domains() || die $postini->Last_Error; open DOMAINS, ">Domains.txt" or die "Couldn't open file Domains.txt: +$!"; print DOMAINS "Domain, Organization, Org_ID, Email_Config, Email_Conf +ig_Org_ID, Account, Account_Org_ID\n"; foreach my $domain (keys %$domains) { print $domain, "\n"; print DOMAINS $domain . ", "; print DOMAINS $$domains{$domain}{Organization} . ", "; print DOMAINS $$domains{$domain}{Org_ID} . ", "; print DOMAINS $$domains{$domain}{Email_Config} . ", "; print DOMAINS $$domains{$domain}{Email_Config_Org_ID} . ", "; print DOMAINS $$domains{$domain}{Account} . ", "; print DOMAINS $$domains{$domain}{Account_Org_ID}; print DOMAINS "\n"; # Try to retrieve the list of users. my $users = $postini->Get_Users_For_Domain($domain); open USERS, ">$domain.txt" || die "Couldn't open file $domain.txt +: $!"; if ($users) { # Show the date the list says it was downloaded print USERS "Downloaded list: $users->{'Downloaded'}\n"; # do some simple output of the message centers to a CSV foreach my $user ($users->{'Column_Names'}, @{ $users->{'User +s'} } ) { print USERS join ",", @$user; print USERS "\n"; } } else { print STDERR $postini->Last_Error, "\n"; print USERS "Couldn't retrieve User List!\n"; } close USERS; } close DOMAINS; =head1 DESCRIPTION Warning! This is my first perl module, so it is probably not up to the + best coding practices, but it appears to work, at least well enough +to me. This module will allow you to login to Postini's administration websit +e (www.postini.com) and access information on domains and message cen +ters associated with those domains. It makes use of LWP::UserAgent to get the pages. =cut # I don't currently export anything, but I don't know if I will want t +o so I am just commenting out this structure. #our @ISA = qw(Exporter); # Items to export into callers namespace by default. Note: do not expo +rt # names by default without a very good reason. Use EXPORT_OK instead. # Do not simply export all your public functions/methods/constants. # This allows declaration use WWW::PostiniAdmin ':all'; # If you do not need this, moving things directly into @EXPORT or @EXP +ORT_OK # will save memory. #our %EXPORT_TAGS = ( 'all' => [ qw( #) ] ); #our @EXPORT_OK = ( @{ $EXPORT_TAGS{'all'} } ); #our @EXPORT = qw( #); # These are current for Postini as of 2003.04.30 use constant LOGIN_URL => 'https://login.postini.com/exec/login'; use constant BASE_URL => 'https://user-1.postini.com/exec'; use constant USERS_URL => BASE_URL . '/Admin?action=load_userDownl +oad&nav=4.6&targetGroupId='; # tag the Org_ID onto this and you are s +et use constant DOMAIN_URL => BASE_URL . '/Admin?action=change_orgRedir&r +edir=View%20hierarchy&showDomains=True'; # just using the heirarchy v +iew for now, wish they would have made me a tab-delimited thing tho. =pod =head2 FUNCTIONS All functions should return undef if they fail. They will also set WWW +::PostiniAdmin->{'Last_Error'} to the error message. The better way t +o get the last error is to call the WWW::PostiniAdmin->Last_Error() f +unction that just returns it. =head3 Most popular functions =over 4 =item WWW::PostiniAdmin->new() This returns an object, that you can change variables in. =cut # # Create the new object, pretty much initialize some variables. # sub new { my ($class) = @_; # Create the UserAgent that will login to the account. my $ua = LWP::UserAgent->new( env_proxy => 1, keep_alive => 1, timeout => 30, cookie_jar => { file => "$ENV{TEMP}/.postinicookie +s.txt" }, ); # Allow POST to be redirected since they do that. push @{ $ua->requests_redirectable }, 'POST'; my $self = { "Login_URL" => LOGIN_URL, "Users_URL" => USERS_URL, "Domain_URL" => DOMAIN_URL, "Last_Error" => undef, "UserAgent" => $ua, }; bless($self, $class); return $self; } =pod =item WWW::PostiniAdmin->Login() Call this as $postini->Login($Username, $Password) and it will return +1 if it succeeds. =cut # # Do the login, take the username and password and create the user age +nt, submit the form, get the cookie, check the result and if it works +, return a success. # sub Login { my ($self, $username, $password) = @_; # Start with making sure they passed a username and password unless (defined $username && defined $password) { $self->{'Last_Error'} = "You need to pass Username, Password t +o call new"; return undef; } # Attempt to login to postini my $response = $self->{'UserAgent'}->post($self->{'Login_URL'}, { 'email' => $username, 'pword' => $password, 'action' => 'login', } ); # fail if it the POST failed. if ($response->is_error) { $self->{'Last_Error'} = "Error while getting " . $response->re +quest->uri . " -- " . $response->status_line; return undef; } # Check to make sure the login was successful, if not it will be a +sking us to try it again. if ($response->content =~ m#<title>Log in</title>#) { $self->{'Last_Error'} = "Couldn't log in. Check your username + and password!\n"; return undef; } # Tell the module that we are logged in, and put the UserAgent obj +ect somewhere it can be accessed later $self->{"Logged_In"} = 1; # Return successfull return 1; } =pod =item WWW::PostiniAdmin->Last_Error() Returns the value of the last error that was seen as a string. =cut # # Just returns $self->{'Last_Error'} # sub Last_Error { my $self = shift; return $self->{'Last_Error'}; } =pod =item WWW::PostiniAdmin->Domains() This returns WWW::PostiniAdmin->{'Domains'}. If it isn't defined, it +will call Get_Domains() to try and get the list of domains. The forma +t of the return value in is documented under Get_Domains() . =cut # # Just returns $self->{'Domains'} without downloading the list unless +it hasn't been downloaded before. This allows you do retrieve the lis +t, but use the one we have already if it has been downloaded. # sub Domains { my $self = shift; unless ( $self->{'Domains'} ) { $self->Get_Domains or return undef; } return $self->{'Domains'}; } =pod =item WWW::PostiniAdmin->Get_Users_For_Domain() This is called as $postini->Get_Users_for_Domain(domain.name). It mai +nly just converts the domain to org id with Domain_To_Org_ID() . It t +hen calls Get_Users_For_Org_ID() to actually get the users. The retur +n value is documented under Get_Users_For_Org_ID() . =cut # # This converts a domain name to an Organization org id ( with Domain_ +To_Org_ID() ) and then calls Get_Users_For_Org_ID() with the org id a +nd returns the result. # sub Get_Users_For_Domain { my $self = shift; my $domain = shift; # Check to make sure that there was a value passed unless ($domain) { $self->{'Last_Error'} = "No org_id passed!"; return undef; } # Convert the domain name into an org_id my $org_id = $self->Domain_To_Org_ID($domain); unless ($org_id) { $self->{'Last_Error'} = "Couldn't find org_id for $domain!"; return undef; } # Get the user list from another function and return it return $self->Get_Users_For_Org_ID($org_id); } =pod =back =head3 Other functions =over 4 =item WWW::PostiniAdmin->Domain_To_Org_ID() This takes a domain name passed to it, and returns the org id of the o +rganization that the domain is under. If $WWW::PostiniAdmin->{'Domains'} doesn't have any contents, it will +call WWW::PostiniAdmin->Get_Domains() to fill it in. It then looks up the org id in the hash in $WWW::PostiniAdmin->{'Domai +ns'}. It returns it if it is found. =cut # # Takes a domain name and converts it to an Org ID by looking it up in + the Domains table. If there is no $self->{'Domains'} set, it will at +tempt to download one. # sub Domain_To_Org_ID { my $self = shift; # Check to make sure we are logged in before trying to download th +e URL unless ($self->{'Logged_In'}) { $self->{'Last_Error'} = "Not Logged in! Please retry!"; return undef; } my $domain = shift; # If the domain list doesn't exist yet, try to download it. unless ( $self->{'Domains'} ) { $self->Get_Domains or return undef; } # Look to see if the domain exists and has an Org_ID and returns i +t if so, otherwise errors if (defined $self->{'Domains'}->{$domain}->{Org_ID}) { return $self->{'Domains'}->{$domain}->{Org_ID}; } else { $self->{'Last_Error'} = "Couldn't find ID for domain: $domain" +; return undef; } } =pod =item WWW::PostiniAdmin->Get_Domains() This downloads the current domain list from the web and sets WWW::Post +iniAdmin->{'Domains'} and returns the same value that is set there as + a reference to a hash of hashes in this format: \%domain = { 'domain.name' => { 'Organization' => 'org', 'Org_ID' => '10002', 'Email_Config' => '00', 'Email_Config_Org_ID' => '10001', 'Account' => 'Account Name', 'Account_Org_ID' => '10000', }, } I wanted to make this and the Users domain have similar data structure +s, but I couldn't find a compatible way of doing it. =cut # # This actually does the downloading of the domain list, if it works, +it sets $self->{'Domains'} and returns the result. # sub Get_Domains { my $self = shift; # Make sure we are logged in unless ($self->{'Logged_In'}) { $self->{'Last_Error'} = "Not Logged in! Please retry!"; return undef; } # retrieve domain list my $response = $self->{'UserAgent'}->get($self->{"Domain_URL"}); # fail unless the GET worked. unless ($response->is_success) { $self->{'Last_Error'} = "Couln't retrieve page " . $self->{"Do +main_URL"}; return undef; } # Make the page we got wasn't an error. if ($response->content =~ /A request could not be completed becaus +e of a system error/) { $self->{'Last_Error'} = "A request could not be completed beca +use of a system error."; return undef; } # Make sure it doesn't think we need to log in again for some reas +on if ($response->content =~ m#<title>Log in</title>#) { $self->{'Last_Error'} = "Not logged in. Try logging in again +!\n"; return undef; } #print $response->content; # Parse the response into a usable format my $domains = $self->_parse_Domain_List( $response->content ); # Then stuff the data into somewhere we can get it later $self->{'Domains'} = $domains; # and return the result return $domains; } =pod =item WWW::PostiniAdmin->Get_Users_For_Org_ID() It takes the org id, sets WWW::PostiniAdmin->{'Current_Users'} and ret +urns a reference to hash of users in this format: \%Users = { 'Downloaded' => '1970/01/01 00:00:00', # The date the file was downloaded 'Title' => 'List Users', # Seems to always contain 'List Users' but it +is # filled in from the downloaded page 'Organization' => 'organization', # The name of the organization as entered # into postini 'Num Records' => '123', # the number of message centers 'Columns' => 'Address,Creation Date, . . .', # the comma seperated list of column names 'Column_Names' => [ 'Address', 'Creation Date', '. . . ' ], # An array reference to the column names 'Users' => [ # An array reference containing . . . # references to arrays for each user [ 'user1@domain', '01-Jan-70 00:00:00', '. . .' ], [ 'user2@domain', '-', '. . .' ], ], }; =cut # # Here we download the list of users from the web site, sets the $self +->{'Current_Users'} and returns the information. # sub Get_Users_For_Org_ID { my $self = shift; # Make sure we are still logged in unless ($self->{'Logged_In'}) { $self->{'Last_Error'} = "Not Logged in! Please retry!"; return undef; } # Here we get the users for the org_id that was selected # and parse it out, using _parse_User_List(). my $org_id = shift; # Remove the old user list just in case something fails, we don't +want to have to wrong data hanging around anywhere. $self->{'Current_Users'} = undef; # Attempt to retrieve list from postini my $response = $self->{'UserAgent'}->get($self->{"Users_URL"} . $o +rg_id); #print $response->content; # fail unless the GET worked. unless ($response->is_success) { $self->{'Last_Error'} = "Couln't retrieve page " . $self->{'Us +ers_URL'} . $org_id; return undef; } # If we got an error page instead, fail if ($response->content =~ /A request could not be completed becaus +e of a system error/) { $self->{'Last_Error'} = "A request could not be completed beca +use of a system error."; return undef; } # Fail if we get the login page, we must have gotten logged out so +mehow if ($response->content =~ m#<title>Log in</title>#) { $self->{'Last_Error'} = "Not logged in. Try logging in again +!\n"; return undef; } #print $response->content; # Change the list from a web page into a data structure we can use my $Current_Users = $self->_parse_User_List($response->content); # then stuff it into a variable we can get at later $self->{'Current_Users'} = $Current_Users; # and return it so we can use it now return $Current_Users } =pod =back =head3 Internal only functions =over 4 =item WWW::PostiniAdmin->_parse_User_List() Converts from the page that was downloaded to the hash of hashes. =cut # # Here we take the $response from the get of the user list and convert + it to the data format that we return # sub _parse_User_List { my $self = shift; # This is the unformatted page that was downloaded my $list = shift; # Fail if nothing was passed to the domain unless ($list) { $self->{'Last_Error'} = "Nothing was passed to _parse_User_Lis +t!"; return undef; } # The page needs to be turned into a list so we can look at each l +ine my @list = split /\n/, $list; # Declare some variables to stuff the data into my %Header; my @Users; # Iterate over the list of users foreach (@list) { # if it is one of the header lines if (/^#/) { # Split the line into name and value my ($key, $value) = $_ =~ /^#([^:]+):(.*)$/; #print "$key, $value\n"; # Strip beginning and ending whitespace on both variables $key =~ s/^\s+//; $key =~ s/\s+$//; $value =~ s/^\s+//; $value =~ s/\s+$//; # Store the header $Header{$key} = $value; # If it is the column name list, split it into an array we + can use if ($key eq 'Columns') { my @columns = split /,/, $value; $Header{Column_Names} = \@columns; } # Go on to the next line since we are done with this one next; } # Now we are dealing with a user line since it doesn't start w +ith a # # Split the line into an array, should corruspond with the Col +umn_Names array that we got above my @user = split /,/, $_; # Put this user into the list of users push @Users, \@user; } # Get the total number of users that we read in my $total_users_read = scalar @Users; #print $Header{'Num Records'} . " : " . $total_users_read . "\n"; # Check the number of records that we read against the number that + we wee supposed to get and fail unless they match. unless ($Header{'Num Records'} == $total_users_read) { $self->{'Last_Error'} = "Number of records don't match! There +should be $Header{'Num Records'} but there are $total_users_read."; return undef; } # Put the list of users into an entry in the hash so it is all in +one place $Header{'Users'} = \@Users; # Return the fixed up information return \%Header; } =pod =item WWW::PostiniAdmin->_parse_Domain_List() Converts from the page that was downloaded to the hash of hashes. =cut # # Takes the response from getting the Heirarchy (with domains) and tur +ns it into a hash that gets returned. # sub _parse_Domain_List { my $self = shift; # This should be the my $page = shift; # Fail if there was nothing passed unless ($page) { $self->{'Last_Error'} = "Nothing was passed to _parse_Domain_L +ist!"; return undef; } # The page need to be in an array for us to iterate over it my @page = split /\n/, $page; # Initialize some variables to be used my $Account_Name = 'UNKNOWN'; my $Account_Org_ID = '00'; my $Email_Config_Name = 'UNKNOWN'; my $Email_Config_Org_ID = '00'; my $Organization_Name = 'UNKNOWN'; my $Organization_Org_ID = '00'; my $Domain = 'UNKNOWN'; my %Domains; # Here are some variables we use to keep track of where we are in +the file. # $In_Content tells when we have gotten down to the content sectio +n of the page my $In_Content = 0; # $Found_End_of_Content is so we can check to make sure we got to +the end of the content and we didn't have a truncated file or somethi +ng my $Found_End_of_Content = 0; # Iterate over each line on the page foreach my $line (@page) { # check for the beginning of the content if ($line =~ /<!-- ##### CONTENT: BEGIN ##### -->/) { # Woo hoo, we are into the content, time to start working $In_Content = 1; next; } # Skip the rest until we are in the content section next unless $In_Content; # check for the end of the content if ($line =~ /<!-- ##### CONTENT: END ##### -->/) { # Woo hoo! this means we got everything we need $Found_End_of_Content = 1; # Stop when we get to the end of the content; last; } # We start looking at the content now # First we check for the account line and grab the information + we need from that if ($line =~/^&nbsp; &nbsp; &nbsp; &nbsp; <a href/) { ($Account_Name) = $line =~ />([^<]+)<\/a/; ($Account_Org_ID) = $line =~ /targetorgid=(\d+)/; #print "Account: $Account_Name\tOrg ID: $Account_Org_ID\n" +; # Then the Email Config line } elsif ($line =~ /Email Config/) { ($Email_Config_Name) = $line =~ /Email Config (\d+)/; ($Email_Config_Org_ID) = $line =~ /targetorgid=(\d+)/; #print "\n\tEmail: $Email_Config_Name\tOrg ID: $Email_Conf +ig_Org_ID\n"; # Then the line that has the organization information } elsif ($line =~ /^&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; +&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; <a href/) { ($Organization_Name) = $line =~ />([^<]+)<\/a/; ($Organization_Org_ID) = $line =~ /targetorgid=(\d+)/; #print "\t\tOrganization: $Organization_Name\tOrg ID: $Org +anization_Org_ID\n"; # Now check if this is a line that has a domain on it. } elsif ($line =~ /<font color=/) { ($Domain) = $line =~ />([^<]+)<\/font/; #print "\t\t\tDomain: $Domain\n"; # Now that we have a domain, we can fill in all the inform +ation $Domains{$Domain} = { 'Organization' => $Organization_Name, 'Org_ID' => $Organization_Org_ID, 'Email_Config' => $Email_Config_Name, 'Email_Config_Org_ID' => $Email_Config_Org_ID, 'Account' => $Account_Name, 'Account_Org_ID' => $Account_Org_ID, }; } # print $line; } # Make sure we got the complete page, at least what we need. If no +t, fail if ($Found_End_of_Content) { return \%Domains; } else { $self->{'Last_Error'} = "Got to end of file before finding the + end of the content!"; return undef; } } 1; __END__ =back =head2 VARIABLES -- Mostly used internally, but if you need them, go f +or it These can be assigned too, but other than the URL ones I don't see any + real reason to do it. This module will probably have to be largely r +ewritten (ok, probably just the _parse bits) if Postini changes stuff + anyway. =head3 Variables with data in them =over 4 =item * WWW::PostiniAdmin->{"Current_Users"} Contains the list of current users in the format described in Get_User +s_For_Org_ID() . =item * WWW::PostiniAdmin->{"Domains"} Contains the list of domains formatted as shown in Get_Domains() . =back =head3 Initialized by the call to new =over 4 =item * WWW::PostiniAdmin->{"Login_URL"} Contains the URL that is used for login =item * WWW::PostiniAdmin->{"Users_URL"} Contains the URL of the page where to download the list of users. It +gets the Org_ID tacked onto the end, so it should probably end with & +targetGroupId= to make that work =item * WWW::PostiniAdmin->{"Domain_URL"} Contains the URL of the page to download the list of domains. The page + used is currently the Show Heirarchy page with the Show Domains box +checked. That is the page that the parser expects. =item * WWW::PostiniAdmin->{"Last_Error"} Contains undef until there has been an error, then contains the text o +f the error. The best way to check for an error is not to check if t +his is set, but to check for the return value of a call being undef. +If it is, this variable should say why, but you can just call the fun +ction, Last_Error and it will say. =item * WWW::PostiniAdmin->{"UserAgent"} Contains a reference to a LWP::UserAgent object that is currently bein +g used to get pages. You can call it directly to get other pages, bu +t currently it is only meant to be used internally. =back =head3 Initialized by the call to Login =over 4 =item * WWW::PostiniAdmin->{"Logged_In"} This is a boolean value, once we are logged in successfully, it is set + to 1, if not set, we are not logged in. =back =head2 EXPORT None by default. =head1 AUTHOR andrew fresh, E<lt>andrew@mad-techies.orgE<gt> =cut
andrew