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.
This entry was posted
on Tuesday, August 4th, 2009 at 4:13 PM and is filed under Agile, Design, Programming, Testing.
You can follow any responses to this entry through the RSS 2.0 feed.
You can skip to the end and leave a response. Pinging is currently not allowed.
