[Rt-devel] Why two IsRTAddress() and CullRTAddresses()?

[Petter Reinholdtsen]

In RT 3.6.1 there seem to be two functions using the
$RT::RTAddressRegexp regular expression.  Both
RT::EmailParser::IsRTAddress() and RT::Interface::Email::IsRTAddress()
exists.  Why?  The latter do not seem to be used.  Should it be
deleted?

I'm looking into rewriting it to get it to look up addresses in LDAP
to check if they are forwarded to the RT server, and found it strange
that I need to override two functions to do this.  CullRTAddresses()
is duplicated as well.  The only functional difference seem to be that
the ones in EmailParser.pm take $self as the first argument and the
one in Email.pm accept undef.

lib/RT/EmailParser.pm:

    sub IsRTAddress {
        my $self = shift;
        my $address = shift;
    
        # Example: the following rule would tell RT not to Cc
        #   "tickets at noc.example.com"
        if ( defined($RT::RTAddressRegexp) &&
                           $address =~ /$RT::RTAddressRegexp/i ) {
            return(1);
        } else {
            return (undef);
        }
    }
    sub CullRTAddresses {
        my $self = shift;
        my @addresses= (@_);
        my @addrlist;
    
        foreach my $addr( @addresses ) {
                                     # We use the class instead of the instance
                                     # because sloppy code calls this method
                                     # without a $self
          push (@addrlist, $addr)    unless RT::EmailParser->IsRTAddress($addr);
        }
        return (@addrlist);
    }


lib/RT/Interface/Email.pm:

    sub IsRTAddress {
        my $address = shift || '';
    
        # Example: the following rule would tell RT not to Cc
        #   "tickets at noc.example.com"
        if ( defined($RT::RTAddressRegexp)
            && $address =~ /$RT::RTAddressRegexp/i )
        {
            return (1);
        } else {
            return (undef);
        }
    }

    sub CullRTAddresses {
        return ( grep { IsRTAddress($_) } @_ );
    }

The CullRTAddresses() version in lib/RT/Interface/Email.pm is more
elegant than the one in lib/RT/EmailParser.pm.  I suggest to rewrite
them like this:

lib/RT/EmailParser.pm:

    sub IsRTAddress {
    [no change]
    }
    sub CullRTAddresses {
        my $self = shift;

        # We use the class instead of the instance because sloppy code
        # calls this method without a $self
        return ( grep { RT::EmailParser->IsRTAddress($_) } @_ );
    }

lib/RT/Interface/Email.pm (or just remove them because they are
unused):

    sub IsRTAddress {
        my $address = shift || '';
        return RT:EmailParser->IsRTAddress($address);
    }

    sub CullRTAddresses {
        return RT:EmailParser->CullRTAddresses(@_);
    }

Is this a good idea?

Friendly,
-- 
Petter Reinholdtsen