Why two IsRTAddress() and CullRTAddresses()?

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@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@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

Check out 3.6.3RC2, there is some changes in these functions, but
still we should get rid of all duplicates.On 12/19/06, Petter Reinholdtsen pere@hungry.com wrote:

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@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@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


List info: The rt-devel Archives

Best regards, Ruslan.