Skip to content

Commit

Permalink
Reverts Zone.qualifier
Browse files Browse the repository at this point in the history
Zone.qualifier was only introduced to support creation of Route53 hosted
zones. We were using this for the caller reference. Turns out that
caller references can never be reused. This means that if you delete a
zone, you can never create a new one with the same caller reference. As
such, we shouldn't expose this to users.

see #350
  • Loading branch information
Adrian Cole committed Mar 30, 2015
1 parent ace082a commit 48d5e42
Show file tree
Hide file tree
Showing 15 changed files with 32 additions and 110 deletions.
1 change: 0 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
* Adds email and ttl to CLI zone list output
* Adds `ZoneApi.iterateByName()` to support lookups
* Adds `-n` parameter to CLI zone list
* `supportsDuplicateZoneNames()` means `Zone.qualifier()` is present
* Deprecates `Zone.idOrName()` as `Zone.id()` cannot be null
* Documents third-party provider process
* Adds example server
Expand Down
3 changes: 1 addition & 2 deletions cli/src/main/java/denominator/cli/Denominator.java
Original file line number Diff line number Diff line change
Expand Up @@ -385,8 +385,7 @@ public Iterator<String> doRun(final DNSApiManager mgr) {
return Iterators.transform(zones, new Function<Zone, String>() {
@Override
public String apply(Zone input) {
return format("%-24s %-36s %-19s %-36s %d", input.id(), input.name(),
input.qualifier() != null ? input.qualifier() : "", input.email(),
return format("%-24s %-36s %-36s %d", input.id(), input.name(), input.email(),
input.ttl());
}
});
Expand Down
2 changes: 1 addition & 1 deletion cli/src/test/java/denominator/cli/DenominatorTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ public void listsAllProvidersWithCredentials() {
@Test // denominator -p mock zone list
public void testZoneList() {
assertThat(new ZoneList().doRun(mgr)).containsExactly(
"denominator.io. denominator.io. admin.denominator.io. 86400"
"denominator.io. denominator.io. admin.denominator.io. 86400"
);
}

Expand Down
3 changes: 1 addition & 2 deletions core/src/main/java/denominator/Provider.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
import java.util.Set;

import denominator.model.ResourceRecordSet;
import denominator.model.Zone;

/**
* Metadata about a provider of DNS services.
Expand Down Expand Up @@ -70,7 +69,7 @@ public interface Provider {
Map<String, Collection<String>> profileToRecordTypes();

/**
* Zones have {@link Zone#qualifier()} set, as duplicate zones can exist with the same name.
* Duplicate zones can exist with the same name.
*/
boolean supportsDuplicateZoneNames();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import org.junit.Test;
import org.junit.rules.ExpectedException;

import java.util.Arrays;
import java.util.Collection;
import java.util.Iterator;
import java.util.LinkedHashMap;
Expand All @@ -27,6 +26,7 @@
import static denominator.CredentialsConfiguration.checkValidForProvider;
import static denominator.CredentialsConfiguration.credentials;
import static denominator.Denominator.create;
import static java.util.Arrays.asList;
import static org.assertj.core.api.Assertions.assertThat;

public class DynamicCredentialsProviderExampleTest {
Expand Down Expand Up @@ -68,9 +68,9 @@ public void testImplicitDynamicCredentialsUpdate() {
DNSApiManager mgr = create(new DynamicCredentialsProvider());
ZoneApi zones = mgr.api().zones();
assertThat(zones.iterator())
.containsExactly(Zone.builder().name("acme").qualifier("wily").email("coyote").build());
.containsExactly(Zone.builder().name("acme").email("coyote").build());
assertThat(zones.iterator())
.containsExactly(Zone.builder().name("acme").qualifier("road").email("runner").build());
.containsExactly(Zone.builder().name("acme").email("runner").build());

// now, if the supplier doesn't supply a set of credentials, we should
// get a correct message
Expand Down Expand Up @@ -118,9 +118,7 @@ public Iterator<Zone> iterator() {
CustomerUsernamePassword cup = creds.get();
// normally, the credentials object would be used to invoke a remote
// command. in this case, we don't and say we did :)
return Arrays.asList(
Zone.builder().name(cup.customer).qualifier(cup.username).email(cup.password).build())
.iterator();
return asList(Zone.builder().name(cup.customer).email(cup.password).build()).iterator();
}

@Override
Expand All @@ -145,7 +143,7 @@ public String name() {
@Override
public Map<String, Collection<String>> credentialTypeToParameterNames() {
Map<String, Collection<String>> options = new LinkedHashMap<String, Collection<String>>();
options.put("username", Arrays.asList("customer", "username", "password"));
options.put("username", asList("customer", "username", "password"));
return options;
}

Expand All @@ -158,9 +156,9 @@ public Map<String, Collection<String>> credentialTypeToParameterNames() {
static class Module {

final AtomicInteger credentialIndex = new AtomicInteger();
final List<List<String>> credentials = Arrays.asList(
Arrays.asList("acme", "wily", "coyote"),
Arrays.asList("acme", "road", "runner")
final List<List<String>> credentials = asList(
asList("acme", "wily", "coyote"),
asList("acme", "road", "runner")
);

/**
Expand Down
3 changes: 0 additions & 3 deletions core/src/test/java/denominator/ReadOnlyLiveTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,9 @@ public void zoneIdentification() {
assumeTrue("No zones to test", zones.hasNext());
Zone zone = zones.next();
if (manager.provider().supportsDuplicateZoneNames()) {
assertThat(zone.qualifier()).isNotNull();
assertThat(zone.id())
.isNotNull()
.isNotEqualTo(zone.name());
} else {
assertThat(zone).hasNoQualifier();
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public void iteratorWhenPresent() throws Exception {
ZoneApi api = server.connect().api().zones();

assertThat(api.iterator()).containsExactly(
Zone.builder().name("denominator.io").id("denominator.io.").email("fake@denominator.io.")
Zone.builder().name("denominator.io").id("denominator.io").email("fake@denominator.io.")
.ttl(1800).build()
);

Expand Down
43 changes: 7 additions & 36 deletions model/src/main/java/denominator/model/Zone.java
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,12 @@
public class Zone {

private final String name;
private final String qualifier;
private final String id;
private final String email;
private final int ttl;

Zone(String name, String qualifier, String id, String email, int ttl) {
Zone(String name, String id, String email, int ttl) {
this.name = checkNotNull(name, "name");
this.qualifier = qualifier;
this.id = id;
this.email = checkNotNull(email, "email of %s", name);
checkArgument(ttl >= 0, "Invalid ttl value: %s, must be 0-%s", ttl, Integer.MAX_VALUE);
Expand All @@ -37,24 +35,10 @@ public String name() {
return name;
}

/**
* A user-defined unique string that differentiates zones with the same name. Only supported when
* the {@code denominator.Provider#supportsDuplicateZoneNames()}.
*
* @return qualifier or null if the provider doesn't support multiple zones with the same name.
* @since 4.5
*/
public String qualifier() {
return qualifier;
}

/**
* The potentially transient and opaque string that uniquely identifies the zone. This may be null
* when used as an input object.
*
* <p/>Note that this is not used in {@link #hashCode()} or {@link #equals(Object)}, as it may
* change over time.
*
* @since 4.5
*/
public String id() {
Expand Down Expand Up @@ -96,7 +80,7 @@ public boolean equals(Object obj) {
if (obj instanceof Zone) {
Zone other = (Zone) obj;
return name().equals(other.name())
&& equal(qualifier(), other.qualifier())
&& equal(id(), other.id())
&& email().equals(other.email())
&& ttl() == other.ttl();
}
Expand All @@ -107,7 +91,7 @@ && email().equals(other.email())
public int hashCode() {
int result = 17;
result = 31 * result + name().hashCode();
result = 31 * result + (qualifier() != null ? qualifier().hashCode() : 0);
result = 31 * result + (id() != null ? id().hashCode() : 0);
result = 31 * result + email().hashCode();
result = 31 * result + ttl();
return result;
Expand All @@ -118,9 +102,6 @@ public String toString() {
StringBuilder builder = new StringBuilder();
builder.append("Zone [");
builder.append("name=").append(name());
if (qualifier() != null) {
builder.append(", ").append("qualifier=").append(qualifier());
}
if (!name().equals(id())) {
builder.append(", ").append("id=").append(id());
}
Expand All @@ -131,8 +112,7 @@ public String toString() {
}

/**
* Represent a zone without a {@link #qualifier() qualifier} when its {@link #id() id} is its
* name.
* Represent a zone when its {@link #id() id} is its name.
*
* @param name corresponds to {@link #name()} and {@link #id()}
* @deprecated Use {@link #builder()}. This will be removed in version 5.
Expand All @@ -143,15 +123,15 @@ public static Zone create(String name) {
}

/**
* Represent a zone without a {@link #qualifier() qualifier}.
* Represent a zone with a fake email and defaul ttl of 86400.
*
* @param name corresponds to {@link #name()}
* @param id corresponds to {@link #id()}
* @deprecated Use {@link #builder()}. This will be removed in version 5.
*/
@Deprecated
public static Zone create(String name, String id) {
return new Zone(name, null, id, "fake@" + name, 86400);
return new Zone(name, id, "fake@" + name, 86400);
}

public static Builder builder() {
Expand All @@ -164,7 +144,6 @@ public static Builder builder() {
public static final class Builder {

private String name;
private String qualifier;
private String id;
private int ttl = 86400;
private String email;
Expand All @@ -177,14 +156,6 @@ public Builder name(String name) {
return this;
}

/**
* @see Zone#qualifier()
*/
public Builder qualifier(String qualifier) {
this.qualifier = qualifier;
return this;
}

/**
* Can be null for input objects.
*
Expand Down Expand Up @@ -214,7 +185,7 @@ public Builder ttl(Integer ttl) {
}

public Zone build() {
return new Zone(name, qualifier, id, email, ttl);
return new Zone(name, id, email, ttl);
}
}
}
12 changes: 0 additions & 12 deletions model/src/test/java/denominator/assertj/ZoneAssert.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,6 @@ public ZoneAssert hasName(String expected) {
return this;
}

public ZoneAssert hasQualifier(String expected) {
isNotNull();
objects.assertEqual(info, actual.qualifier(), expected);
return this;
}

public ZoneAssert hasId(String expected) {
isNotNull();
objects.assertEqual(info, actual.id(), expected);
Expand All @@ -42,10 +36,4 @@ public ZoneAssert hasTtl(Integer expected) {
objects.assertEqual(info, actual.ttl(), expected);
return this;
}

public ZoneAssert hasNoQualifier() {
isNotNull();
objects.assertNull(info, actual.qualifier());
return this;
}
}
40 changes: 8 additions & 32 deletions model/src/test/java/denominator/model/ZoneTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,53 +15,29 @@ public class ZoneTest {
public void factoryMethodsWork() {
Zone name = Zone.create("denominator.io.");
Zone id = Zone.create("denominator.io.", "ABCD");
Zone qualifier = Zone.builder()
.name("denominator.io.")
.qualifier("Test-Zone")
.id("ABCD")
.email("fake@denominator.io.").build();

assertThat(name)
.hasName("denominator.io.")
.hasNoQualifier()
.hasId("denominator.io.")
.isEqualTo(name)
.isEqualTo(id)
.isNotEqualTo(qualifier);
.isNotEqualTo(id);

assertThat(name.hashCode())
.isEqualTo(id.hashCode())
.isNotEqualTo(qualifier.hashCode());
assertThat(name.toString()).isEqualTo("Zone [name=denominator.io., email=fake@denominator.io., ttl=86400]");
.isNotEqualTo(id.hashCode());
assertThat(name.toString())
.isEqualTo("Zone [name=denominator.io., email=fake@denominator.io., ttl=86400]");

assertThat(id)
.hasName("denominator.io.")
.hasNoQualifier()
.hasId("ABCD")
.isEqualTo(id)
.isEqualTo(name)
.isNotEqualTo(qualifier);
.isNotEqualTo(name);

assertThat(id.hashCode())
.isEqualTo(name.hashCode())
.isNotEqualTo(qualifier.hashCode());

assertThat(id.toString()).isEqualTo("Zone [name=denominator.io., id=ABCD, email=fake@denominator.io., ttl=86400]");

assertThat(qualifier)
.hasName("denominator.io.")
.hasQualifier("Test-Zone")
.hasId("ABCD")
.isEqualTo(qualifier)
.isNotEqualTo(name)
.isNotEqualTo(id);

assertThat(qualifier.hashCode())
.isNotEqualTo(name.hashCode())
.isNotEqualTo(id.hashCode());
.isNotEqualTo(name.hashCode());

assertThat(qualifier.toString())
.isEqualTo("Zone [name=denominator.io., qualifier=Test-Zone, id=ABCD, email=fake@denominator.io., ttl=86400]");
assertThat(id.toString()).isEqualTo(
"Zone [name=denominator.io., id=ABCD, email=fake@denominator.io., ttl=86400]");
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,6 @@ public void endElement(String uri, String name, String qName) {
zone.name = currentText.toString().trim();
} else if (qName.equals("Id")) {
zone.id = currentText.toString().trim().replace("/hostedzone/", "");
} else if (qName.equals("CallerReference")) {
zone.callerReference = currentText.toString().trim();
} else if (qName.equals("HostedZone")) {
zones.add(zone);
zone = new HostedZone();
Expand Down
1 change: 0 additions & 1 deletion route53/src/main/java/denominator/route53/Route53.java
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ class HostedZone {

String id;
String name;
String callerReference;
}

class HostedZoneList extends ArrayList<HostedZone> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ private Zone zipWithSOA(HostedZone next) {
SOAData soaData = (SOAData) soa.get(0).records().get(0);
return Zone.builder()
.name(next.name)
.qualifier(next.callerReference)
.id(next.id)
.ttl(soaData.minimum())
.email(soaData.rname()).build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,9 @@ public void decodeZoneListWithNext() throws Exception {
+ " <MaxItems>10</MaxItems>\n"
+ "</ListHostedZonesResponse>"), HostedZoneList.class);

assertThat(result).extracting("name", "callerReference", "id").containsExactly(
tuple("example.com.", "a_unique_reference", "Z21DW1QVGID6NG"),
tuple("example2.com.", "a_unique_reference2", "Z2682N5HXP0BZ4")
assertThat(result).extracting("name", "id").containsExactly(
tuple("example.com.", "Z21DW1QVGID6NG"),
tuple("example2.com.", "Z2682N5HXP0BZ4")
);

assertThat(result.next).isEqualTo("Z333333YYYYYYY");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ public void iteratorWhenPresent() throws Exception {

assertThat(domains.next())
.hasName("denominator.io.")
.hasQualifier("denomination")
.hasId("Z1PA6795UKMFR9")
.hasEmail("awsdns-hostmaster.amazon.com.")
.hasTtl(86400);
Expand Down Expand Up @@ -153,9 +152,9 @@ public void iterateByNameWhenPresent() throws Exception {

ZoneApi api = server.connect().api().zones();
assertThat(api.iterateByName("denominator.io.")).containsExactly(
Zone.builder().name("denominator.io.").qualifier("Foo").id("Z2ZEEJCUZCVG56")
Zone.builder().name("denominator.io.").id("Z2ZEEJCUZCVG56")
.email("awsdns-hostmaster.amazon.com.").ttl(86400).build(),
Zone.builder().name("denominator.io.").qualifier("Bar").id("Z3OQLQGABCU3T")
Zone.builder().name("denominator.io.").id("Z3OQLQGABCU3T")
.email("awsdns-hostmaster.amazon.com.").ttl(86400).build()
);

Expand Down

0 comments on commit 48d5e42

Please sign in to comment.