XNSIO
  About   Slides   Home  

 
Managed Chaos
Naresh Jain's Random Thoughts on Software Development and Adventure Sports
     
`
 
RSS Feed
Recent Thoughts
Tags
Recent Comments

Refactoring Teaser II: Solution: Take 1

Following is the first take at refactoring the II Teaser.

Started off by refactoring the tests:

@Test
public void mineSenderFromEdgeServerRecordInMailHeaders() {
    header(
            mail_from("67.205.47.130", "agileindia.org").receivedBy("75.119.213.4", "mails.agileindia.org"),
            mail_from(null, "mail-vw0-f172.google.com").receivedBy("67.205.47.130", "agileindia.org"));
    senderIP = extractSenderIPFrom(headers).forRecipientDomain(GMAIL);
    assertThat(senderIP).is("209.85.212.172");
    assertDistanceOfMatchingHeaderFromTopIs(2);
}
@Test
public void useSenderIPForInvalidSenderEdgeServerDomainName() {
    header(
            mail_from("67.205.47.130", "agileindia.org").receivedBy("75.119.213.4", "mails.agileindia.org"),
            mail_from("209.85.212.172", "cannot-exist.agilefaqs.com").receivedBy("67.205.47.130", "agileindia.org"));
    senderIP = extractSenderIPFrom(headers).forRecipientDomain(GMAIL);
    assertThat(senderIP).is("209.85.212.172");
    assertDistanceOfMatchingHeaderFromTopIs(2);
}
@Test
public void senderNameCanBeIPAddress() {
    header(mail_from(null, "209.85.212.172").receivedBy("67.205.47.130", "agileindia.org"));
    senderIP = extractSenderIPFrom(headers).forRecipientDomain(GMAIL);
    assertThat(senderIP).is("209.85.212.172");
    assertDistanceOfMatchingHeaderFromTopIs(1);
}
@Test
public void matchMXRecordIPWithReciever() {
    header(mail_from("", "mail-vw0-f172.google.com").receivedBy("67.205.47.130", "apache2-echo.robin.dreamhost.com"));
    senderIP = extractSenderIPFrom(headers).forRecipientDomain(GMAIL);
    assertThat(senderIP).is("209.85.212.172");
    assertDistanceOfMatchingHeaderFromTopIs(1);
}
@Test
public void skipHeaderRecordsThatDontCrossEdgeServers() {
    header(
            mail_from("192.168.1.47", "smtp.gmail.com").receivedBy("75.119.213.4", "mail.gmail.com"),
            mail_from("192.168.1.3", "192.168.6.242").receivedBy("192.168.1.47", "smtp.gmail.com"));
    senderIP = extractSenderIPFrom(headers).forRecipientDomain(GMAIL);
    assertFalse(senderIP.isValid());
    assertDistanceOfMatchingHeaderFromTopIs(0);
}

Following is the crux of the Sender Edge Server IP Extraction Algo:

public IPAddressExtractor(final List receivedHeaders, final Domain recepientDomain) {
    Domain recepientMXRecord = recepientDomain.retrieveFirstMXRecord();
    for (MatchingCriterion critierion : asList(MATCHING_DOMAIN, MATCHING_IP, MATCHING_SECOND_LEVEL_DOMAIN)) {
        Match result = foreach(receivedHeaders).and(recepientMXRecord).match(critierion);
        if (result.success()) {
            storeSenderIPWithDistance(result);
            break;
        }
    }
}

To do the whole Fluent interfaces on line number 21, I had to create a private method:

private CriterionMatcher foreach(final List mailHeaders) {
    return new CriterionMatcher(mailHeaders);
}

and a package protected CriterionMatcher class

class CriterionMatcher {
    private final List mailHeaders;
    private int counter;
    private Domain mxRecord;
 
    CriterionMatcher(final List mailHeaders) {
        this.mailHeaders = mailHeaders;
    }
 
    CriterionMatcher and(final Domain mxRecord) {
        this.mxRecord = mxRecord;
        return this;
    }
 
    Match match(final MatchingCriterion criterion) {
        for (EmailHeader header : mailHeaders) {
            counter++;
 
            if (criterion.isSatisfiedBy(mxRecord, header)) {
                return new Match(header.fromDomain, header.fromIp, counter);
            }
        }
        return Match.NULL;
    }
}

Other than the switch statement smell and conditional complexity, the original code was obsessed with Primitive Obsession code smell. To fix this issue, the first thing I had to do was great first class citizens (Objects). So I ended up creating

public class IPAddress {
    private static final String IP_ADDRESS_REGEX = "\\b(?:\\d{1,3}\\.){3}\\d{1,3}\\b";
    private static final Pattern VALID_IP = Pattern.compile(IP_ADDRESS_REGEX);
    private static final String LOCAL_HOST = "127.0.0.1";
    public static final IPAddress NULL = new IPAddress("");
 
    private final String ip;
 
    private IPAddress(final String address) {
        ip = address;
    }
 
    public static IPAddress parse(final String address) {
        if (address == null) {
            return NULL;
        }
        Matcher matcher = VALID_IP.matcher(address);
 
        if (!matcher.find()) {
            return NULL;
        }
        return new IPAddress(matcher.group(0));
    }
 
    @Override
    public String toString() {
        return ip;
    }
 
    public boolean isLocalhost() {
        return LOCAL_HOST.equals(ip);
    }
 
    public boolean isValid() {
        return this != NULL;
    }
}

and

public class Domain {
    private static final Pattern SLD = Pattern.compile("(.*\\.)?(.*\\..*)");
    public static final Domain NULL = new Domain("", Network.NULL);
 
    private final String name;
    private final Network network;
 
    protected Domain(final String domainName, final Network network) {
        name = domainName.toLowerCase();
        this.network = network;
    }
 
    public IPAddress resolveIP() {
        try {
            String ipAddress = network.ipAddressOf(name);
            return IPAddress.parse(ipAddress);
        } catch (UnknownHostException e) {
            return IPAddress.NULL;
        }
    }
 
    public Domain secondLevelDomain() {
        Matcher mxRecordMatch = SLD.matcher(name);
        if (mxRecordMatch.find()) {
            return new Domain(mxRecordMatch.group(2), network);
        }
        return this;
    }
 
    public Domain retrieveFirstMXRecord() {
        List mxRecords = network.firstMXRecordFor(name);
        if (mxRecords.size() > 0) {
            return new Domain(mxRecords.get(0), network);
        }
        return NULL;
    }
 
    public boolean isValid() {
        return this != NULL;
    }
}

To create a Domain, we use a static Factory called DomainFactory

public final class DomainFactory {
    private static final String DOMAIN_NAME_REGEX = "[\\w.-]+\\.[A-Za-z]{2,6}";
    private static final int MAX_DOMAIN_NAME_LENGTH = 255;
 
    public static Domain build(final String domainName, final Network network) {
        if (isValidDomain(domainName)) {
            return new Domain(domainName, network);
        }
        IPAddress ip = IPAddress.parse(domainName);
        if (ip.isValid()) {
            return retrieveDomainName(ip, network);
        }
        return Domain.NULL;
    }
 
    private static Domain retrieveDomainName(final IPAddress ip, final Network network) {
        try {
            String hostName = network.hostNameFor(ip.toString());
            if (ip.toString().equals(hostName)) {
                return Domain.NULL;
            }
            return new Domain(hostName, network);
        } catch (UnknownHostException e) {
            return Domain.NULL;
        }
    }
}

Finally we’ve the 3 Criterion for checking if a header contains the edge server

public abstract class MatchingCriterion {
    public static final MatchingCriterion MATCHING_DOMAIN = new MatchingDomainCriterion();
    public static final MatchingCriterion MATCHING_IP = new MatchingIPCriterion();
    public static final MatchingCriterion MATCHING_SECOND_LEVEL_DOMAIN = new MatchingSecondLevelDomainCriterion();
 
    public boolean isSatisfiedBy(final Domain mxRecord, final EmailHeader header) {
        return header.fromDomain.isValid() && satisfies(mxRecord, header);
    }
 
    protected abstract boolean satisfies(Domain mxRecord, EmailHeader header);
}
private static class MatchingDomainCriterion extends MatchingCriterion {
    @Override
    protected boolean satisfies(final Domain mxRecord, final EmailHeader header) {
        return !(header.byIp.equals(header.fromIp) || header.fromIp.isLocalhost() || !header.byDomain.equals(mxRecord));
    }
}
private static class MatchingIPCriterion extends MatchingCriterion {
    @Override
    protected boolean satisfies(final Domain mxRecord, final EmailHeader header) {
        return header.byIp.equals(mxRecord.resolveIP());
    }
}
private static class MatchingSecondLevelDomainCriterion extends MatchingCriterion {
    @Override
    protected boolean satisfies(final Domain mxRecord, final EmailHeader header) {
        Domain secondLevelDomain = mxRecord.secondLevelDomain();
        return secondLevelDomain.equals(header.byDomain.secondLevelDomain());
    }
}

Also notice that for testing purpose we don’t want to hit the network, so I created a FakeNetwork class which stubs out all Network calls. Network is injected into all Domain classes through the DomainFactory. (I’m not very happy with this design, it feels like a bit of a hack to inject Network this way.)

public class FakeNetwork extends Network {
    private static final Map domain2ip = new HashMap() {
        {
            put("mail-vw0-f172.google.com", "209.85.212.172");
            put("209.85.212.172", "mail-vw0-f172.google.com");
            put("mails.agileindia.org", "72.14.203.121");
            put("agileindia.org", "67.205.47.130");
        }
    };
 
    @Override
    public String ipAddressOf(final String domainName) throws UnknownHostException {
        return lookup(domainName);
    }
 
    @Override
    public String hostNameFor(final String ipAddress) throws UnknownHostException {
        return lookup(ipAddress);
    }
 
    @Override
    public List firstMXRecordFor(final String name) {
        return asList("agileindia.org");
    }
 
    private String lookup(final String value) throws UnknownHostException {
        String data = domain2ip.get(value);
        if (data == null) {
            throw new UnknownHostException();
        }
        return data;
    }
}

Feel free to download the whole project.


    Licensed under
Creative Commons License