Reverts Zone.Identifier

`Zone.Identifier` doesn't pull its weight. The only thing it adds beyond
`Provider.supportsDuplicateZones`, is that it can indicate a zone id is
the same as its name. This isn't enough to warrant the added complexity.

see #350

CHANGELOG.md

@@ -3,10 +3,8 @@
 * Adds email and ttl to CLI zone list output
 * Adds `ZoneApi.iterateByName()` to support lookups
 * Adds `-n` parameter to CLI zone list
-* Refines zone identifiers and handling of zones with the same name
-  * Adds `Zone.qualifier()` in support duplicate zones
-  * Deprecates `supportsDuplicateZoneNames()` in favor of `Provider.zoneIdentification()` in `NAME`, `OPAQUE` and `QUALIFIED`
-  * Deprecates `Zone.idOrName()` as `Zone.id()` cannot be null
+* `supportsDuplicateZoneNames()` means `Zone.qualifier()` is present
+* Deprecates `Zone.idOrName()` as `Zone.id()` cannot be null
 * Documents third-party provider process
 * Adds example server
 * Enforces source compatibility with animal-sniffer

cli/README.md

@@ -85,15 +85,14 @@ Different providers connect to different urls and need different credentials.  F
 
 ```bash
 $ denominator providers
-provider   url                                                 zoneIds   credentialType credentialArgs
-clouddns   https://identity.api.rackspacecloud.com/v2.0        opaque    password       username password
-clouddns   https://identity.api.rackspacecloud.com/v2.0        opaque    apiKey         username apiKey
-designate  http://localhost:5000/v2.0                          opaque    password       tenantId username password
-dynect     https://api2.dynect.net/REST                        name      password       customer username password
-mock       mem:mock                                            name           
-route53    https://route53.amazonaws.com                       qualified accessKey      accessKey secretKey
-route53    https://route53.amazonaws.com                       qualified session        accessKey secretKey sessionToken
-ultradns   https://ultra-api.ultradns.com:8443/UltraDNS_WS/v01 name      password       username password
+provider   url                                                 duplicateZones credentialType credentialArgs
+mock       mem:mock                                            false
+clouddns   https://identity.api.rackspacecloud.com/v2.0/       true           apiKey         username apiKey
+designate  http://localhost:5000/v2.0                          true           password       tenantId username password
+dynect     https://api2.dynect.net/REST                        false          password       customer username password
+route53    https://route53.amazonaws.com                       true           accessKey      accessKey secretKey
+route53    https://route53.amazonaws.com                       true           session        accessKey secretKey sessionToken
+ultradns   https://ultra-api.ultradns.com:8443/UltraDNS_WS/v01 false          password       username password
 ```
 
 If the credentialType column is blank it needs no credentials.  Otherwise, credentialArgs describes what you need to pass.  Say for example, you were using `ultradns`.  
@@ -110,8 +109,7 @@ If you need to connect to an alternate url, pass the `-u` parameter:
 ./denominator -p ultradns -u https://ultra-api.ultradns.com:8443/UltraDNS_WS/v01-BETA -c myusername -c mypassword zone list
 ```
 
-The `zoneIds` column indicates how the provider addresses zones. For example, `name` means you simply
-pass the zone name (like `denominator.io.`) to commands that need it. This topic is further described in the `Zone Identifiers` section to follow.
+The `duplicateZones` column indicates that zone list output will include a qualifier, which differentiates zones with the same name.
 
 ### Zone
 `zone list` returns the zones in your account.  Ex.
@@ -131,7 +129,7 @@ email.netflix.com.                                 A     3600   69.53.237.168
 --snip--
 ```
 
-### Zone ID
+### Zone Id
 The first column in the zone list is the `id`. This can be used to disambiguate zones with the same
 name, or to improve performance by eliminating a network call. If you wish to use a zone's
 identifier, simply pass it instead of the zone name in record commands.

cli/src/main/java/denominator/cli/Denominator.java

@@ -12,7 +12,6 @@
 import com.google.common.collect.Iterators;
 import com.google.common.collect.Ordering;
 import com.google.common.io.Files;
-import com.google.common.net.InternetDomainName;
 import com.google.gson.Gson;
 import com.google.gson.GsonBuilder;
 import com.google.gson.InstanceCreator;
@@ -61,8 +60,9 @@
 import denominator.cli.ResourceRecordSetCommands.ResourceRecordSetList;
 import denominator.cli.ResourceRecordSetCommands.ResourceRecordSetRemove;
 import denominator.cli.ResourceRecordSetCommands.ResourceRecordSetReplace;
+import denominator.dynect.DynECTProvider;
 import denominator.model.Zone;
-import denominator.model.Zone.Identification;
+import denominator.ultradns.UltraDNSProvider;
 import feign.Logger;
 import feign.Logger.Level;
 import io.airlift.airline.Cli;
@@ -173,14 +173,17 @@ static Object logModule(boolean quiet, boolean verbose) {
   }
 
   static String id(DNSApiManager mgr, String zoneIdOrName) {
-    if (mgr.provider().zoneIdentification() == Identification.NAME) {
+    if (zoneIdOrName.indexOf('.') == -1) { // Assume that ids don't have dots in them!
       return zoneIdOrName;
-    } else if (zoneIdOrName.indexOf('.') != -1 && InternetDomainName.isValid(zoneIdOrName)) {
-      Iterator<Zone> result = mgr.api().zones().iterateByName(zoneIdOrName);
-      checkArgument(result.hasNext(), "zone %s not found", zoneIdOrName);
-      return result.next().id();
     }
-    return zoneIdOrName;
+    // Special-case providers known to use zone names as ids, as this usually saves 1-200ms of
+    // lookups. We can later introduce a flag or other means to help third-party providers.
+    if (mgr.provider() instanceof UltraDNSProvider || mgr.provider() instanceof DynECTProvider) {
+      return zoneIdOrName;
+    }
+    Iterator<Zone> result = mgr.api().zones().iterateByName(zoneIdOrName);
+    checkArgument(result.hasNext(), "zone %s not found", zoneIdOrName);
+    return result.next().id();
   }
 
   @Command(name = "version", description = "output the version of denominator and java runtime in use")
@@ -195,25 +198,22 @@ public void run() {
   @Command(name = "providers", description = "List the providers and their metadata ")
   public static class ListProviders implements Runnable {
 
-    final static String table = "%-10s %-51s %-9s %-14s %s%n";
+    final static String table = "%-10s %-51s %-14s %-14s %s%n";
 
     public static String providerAndCredentialsTable() {
       StringBuilder builder = new StringBuilder();
 
-      builder.append(
-          format(table, "provider", "url", "zoneIds", "credentialType", "credentialArgs"));
-      for (Provider provider : ImmutableSortedSet
-          .copyOf(Ordering.usingToString(), Providers.list())) {
-        if (provider.credentialTypeToParameterNames().isEmpty()) {
-          builder.append(format("%-10s %-51s %-14s %n", provider.name(), provider.url(),
-                                provider.zoneIdentification().toString().toLowerCase()));
+      builder.append(format(
+          table, "provider", "url", "duplicateZones", "credentialType", "credentialArgs"));
+      for (Provider p : ImmutableSortedSet.copyOf(Ordering.usingToString(), Providers.list())) {
+        if (p.credentialTypeToParameterNames().isEmpty()) {
+          builder.append(
+              format("%-10s %-51s %-14s %n", p.name(), p.url(), p.supportsDuplicateZoneNames()));
         }
-        for (Entry<String, Collection<String>> entry : provider.credentialTypeToParameterNames()
-            .entrySet()) {
-          String parameters = Joiner.on(' ').join(entry.getValue());
-          builder.append(format(table, provider.name(), provider.url(),
-                                provider.zoneIdentification().toString().toLowerCase(),
-                                entry.getKey(), parameters));
+        for (Entry<String, Collection<String>> e : p.credentialTypeToParameterNames().entrySet()) {
+          String params = Joiner.on(' ').join(e.getValue());
+          builder.append(format(
+              table, p.name(), p.url(), p.supportsDuplicateZoneNames(), e.getKey(), params));
         }
       }
       return builder.toString();

cli/src/main/java/denominator/cli/GeoResourceRecordSetCommands.java

@@ -58,7 +58,7 @@ static void validateRegions(Map<String, Collection<String>> regionsToAdd,
     }
   }
 
-  static enum GeoResourceRecordSetToString implements Function<ResourceRecordSet<?>, String> {
+  enum GeoResourceRecordSetToString implements Function<ResourceRecordSet<?>, String> {
     INSTANCE;
 
     @Override

cli/src/main/java/denominator/cli/ResourceRecordSetCommands.java

@@ -39,7 +39,7 @@
 
 class ResourceRecordSetCommands {
 
-  static enum ResourceRecordSetToString implements Function<ResourceRecordSet<?>, String> {
+  enum ResourceRecordSetToString implements Function<ResourceRecordSet<?>, String> {
     INSTANCE;
 
     @Override
@@ -433,8 +433,7 @@ public boolean hasNext() {
 
         @Override
         public String next() {
-          mgr.api().basicRecordSetsInZone(id(mgr, zoneIdOrName))
-              .deleteByNameAndType(name, type);
+          mgr.api().basicRecordSetsInZone(id(mgr, zoneIdOrName)).deleteByNameAndType(name, type);
           done = true;
           return ";; ok";
         }

cli/src/test/java/denominator/cli/DenominatorTest.java

@@ -60,15 +60,15 @@ public class DenominatorTest {
   public void listsAllProvidersWithCredentials() {
     assertThat(ListProviders.providerAndCredentialsTable())
         .isEqualTo(Util.join('\n',
-                             "provider   url                                                 zoneIds   credentialType credentialArgs",
-                             "clouddns   https://identity.api.rackspacecloud.com/v2.0        opaque    password       username password",
-                             "clouddns   https://identity.api.rackspacecloud.com/v2.0        opaque    apiKey         username apiKey",
-                             "designate  http://localhost:5000/v2.0                          opaque    password       tenantId username password",
-                             "dynect     https://api2.dynect.net/REST                        name      password       customer username password",
-                             "mock       mem:mock                                            name           ",
-                             "route53    https://route53.amazonaws.com                       qualified accessKey      accessKey secretKey",
-                             "route53    https://route53.amazonaws.com                       qualified session        accessKey secretKey sessionToken",
-                             "ultradns   https://ultra-api.ultradns.com:8443/UltraDNS_WS/v01 name      password       username password",
+                             "provider   url                                                 duplicateZones credentialType credentialArgs",
+                             "clouddns   https://identity.api.rackspacecloud.com/v2.0        false          password       username password",
+                             "clouddns   https://identity.api.rackspacecloud.com/v2.0        false          apiKey         username apiKey",
+                             "designate  http://localhost:5000/v2.0                          false          password       tenantId username password",
+                             "dynect     https://api2.dynect.net/REST                        false          password       customer username password",
+                             "mock       mem:mock                                            false          ",
+                             "route53    https://route53.amazonaws.com                       true           accessKey      accessKey secretKey",
+                             "route53    https://route53.amazonaws.com                       true           session        accessKey secretKey sessionToken",
+                             "ultradns   https://ultra-api.ultradns.com:8443/UltraDNS_WS/v01 false          password       username password",
                              ""));
   }
 

clouddns/src/main/java/denominator/clouddns/CloudDNSProvider.java

@@ -24,8 +24,6 @@
 import denominator.config.NothingToClose;
 import denominator.config.OnlyBasicResourceRecordSets;
 import denominator.config.WeightedUnsupported;
-import denominator.model.Zone;
-import denominator.model.Zone.Identification;
 import feign.Feign;
 import feign.Logger;
 import feign.Target.EmptyTarget;
@@ -52,11 +50,6 @@ public String url() {
     return url;
   }
 
-  @Override
-  public Identification zoneIdentification() {
-    return Identification.OPAQUE;
-  }
-
   // http://docs.rackspace.com/cdns/api/v1.0/cdns-devguide/content/supported_record_types.htm
   // PTR records can only be created with a live Cloud Server
   // SRV records fail to be found by name and type, a bug has been filed and SRV records may be supported later

clouddns/src/test/java/denominator/clouddns/CloudDNSProviderTest.java

@@ -12,7 +12,6 @@
 import denominator.Credentials.MapCredentials;
 import denominator.DNSApiManager;
 import denominator.Provider;
-import denominator.model.Zone.Identification;
 
 import static denominator.CredentialsConfiguration.credentials;
 import static denominator.Denominator.create;
@@ -30,7 +29,7 @@ public class CloudDNSProviderTest {
   @Test
   public void testCloudDNSMetadata() {
     assertThat(PROVIDER.name()).isEqualTo("clouddns");
-    assertThat(PROVIDER.zoneIdentification()).isEqualTo(Identification.OPAQUE);
+    assertThat(PROVIDER.supportsDuplicateZoneNames()).isFalse();
     assertThat(PROVIDER.credentialTypeToParameterNames())
         .containsEntry("password", Arrays.asList("username", "password"))
         .containsEntry("apiKey", Arrays.asList("username", "apiKey"));

core/src/main/java/denominator/BasicProvider.java

@@ -8,8 +8,6 @@
 import java.util.Set;
 import java.util.regex.Pattern;
 
-import denominator.model.Zone.Identification;
-
 import static denominator.common.Preconditions.checkArgument;
 import static java.util.Arrays.asList;
 
@@ -51,11 +49,6 @@ public String url() {
     return "mem:" + name();
   }
 
-  @Override
-  public Identification zoneIdentification() {
-    return Identification.NAME;
-  }
-
   @Override
   public Set<String> basicRecordTypes() {
     Set<String> result = new LinkedHashSet<String>();
@@ -74,7 +67,7 @@ public Map<String, Collection<String>> profileToRecordTypes() {
 
   @Override
   public boolean supportsDuplicateZoneNames() {
-    return zoneIdentification() == Identification.QUALIFIED;
+    return false;
   }
 
   @Override

core/src/main/java/denominator/Provider.java

@@ -6,7 +6,6 @@
 
 import denominator.model.ResourceRecordSet;
 import denominator.model.Zone;
-import denominator.model.Zone.Identification;
 
 /**
  * Metadata about a provider of DNS services.
@@ -45,13 +44,6 @@ public interface Provider {
    */
   String url();
 
-  /**
-   * Metadata about zone identifiers and uniqueness.
-   *
-   * @since 4.5
-   */
-  Identification zoneIdentification();
-
   /**
    * The set of basic {@link ResourceRecordSet#type() record types} that are supported by {@link
    * ResourceRecordSetApi} commands. <br> For example:
@@ -78,10 +70,8 @@ public interface Provider {
   Map<String, Collection<String>> profileToRecordTypes();
 
   /**
-   * @deprecated Use {@link #zoneIdentification()}. {@link denominator.model.Zone.Identification#QUALIFIED
-   * Qualified} indicates duplicate zones are supported. This will be removed in version 5.
+   * Zones have {@link Zone#qualifier()} set, as duplicate zones can exist with the same name.
    */
-  @Deprecated
   boolean supportsDuplicateZoneNames();
 
   /**

core/src/main/java/denominator/ZoneApi.java

@@ -15,8 +15,7 @@ public interface ZoneApi extends Iterable<Zone> {
 
   /**
    * Returns a potentially empty iterator of zones with the supplied {@link Zone#name()}. This can
-   * only have multiple results when {@link Provider#zoneIdentification() zone identification} is
-   * {@link denominator.model.Zone.Identification#QUALIFIED qualified}.
+   * only have multiple results when {@link Provider#supportsDuplicateZoneNames()}.
    *
    * @since 4.5
    */

core/src/test/java/denominator/ReadOnlyLiveTest.java

@@ -7,7 +7,6 @@
 import java.util.ArrayList;
 import java.util.Iterator;
 import java.util.List;
-import java.util.UUID;
 
 import denominator.model.ResourceRecordSet;
 import denominator.model.Zone;
@@ -26,25 +25,13 @@ public void zoneIdentification() {
     Iterator<Zone> zones = manager.api().zones().iterator();
     assumeTrue("No zones to test", zones.hasNext());
     Zone zone = zones.next();
-    switch (manager.provider().zoneIdentification()) {
-      case NAME:
-        assertThat(zone).hasNoQualifier()
-            .hasId(zone.name());
-        break;
-      case OPAQUE:
-        assertThat(zone).hasNoQualifier();
-        assertThat(zone.id())
-            .isNotNull()
-            .isNotEqualTo(zone.name());
-        break;
-      case QUALIFIED:
-        assertThat(zone.qualifier()).isNotNull();
-        assertThat(zone.id())
-            .isNotNull()
-            .isNotEqualTo(zone.name());
-        break;
-      default:
-        throw new AssertionError("unknown zone identification");
+    if (manager.provider().supportsDuplicateZoneNames()) {
+      assertThat(zone.qualifier()).isNotNull();
+      assertThat(zone.id())
+          .isNotNull()
+          .isNotEqualTo(zone.name());
+    } else {
+      assertThat(zone).hasNoQualifier();
     }
   }
 

core/src/test/java/denominator/mock/MockProviderTest.java

@@ -3,7 +3,6 @@
 import org.junit.Test;
 
 import denominator.Provider;
-import denominator.model.Zone.Identification;
 
 import static denominator.Denominator.create;
 import static denominator.Providers.list;
@@ -16,7 +15,7 @@ public class MockProviderTest {
   @Test
   public void testMockMetadata() {
     assertThat(PROVIDER.name()).isEqualTo("mock");
-    assertThat(PROVIDER.zoneIdentification()).isEqualTo(Identification.NAME);
+    assertThat(PROVIDER.supportsDuplicateZoneNames()).isFalse();
     assertThat(PROVIDER.credentialTypeToParameterNames()).isEmpty();
   }
 

designate/src/main/java/denominator/designate/DesignateProvider.java

@@ -25,7 +25,6 @@
 import denominator.designate.DesignateAdapters.DomainListAdapter;
 import denominator.designate.DesignateAdapters.RecordAdapter;
 import denominator.designate.DesignateAdapters.RecordListAdapter;
-import denominator.model.Zone.Identification;
 import feign.Feign;
 import feign.Logger;
 import feign.Target.EmptyTarget;
@@ -52,11 +51,6 @@ public String url() {
     return url;
   }
 
-  @Override
-  public Identification zoneIdentification() {
-    return Identification.OPAQUE;
-  }
-
   // http://docs.hpcloud.com/api/dns/#create_record-jumplink-span
   @Override
   public Set<String> basicRecordTypes() {

designate/src/test/java/denominator/designate/DesignateProviderTest.java

@@ -12,7 +12,6 @@
 import denominator.Credentials.MapCredentials;
 import denominator.DNSApiManager;
 import denominator.Provider;
-import denominator.model.Zone.Identification;
 
 import static denominator.CredentialsConfiguration.credentials;
 import static denominator.Denominator.create;
@@ -30,7 +29,7 @@ public class DesignateProviderTest {
   @Test
   public void testDesignateMetadata() {
     assertThat(PROVIDER.name()).isEqualTo("designate");
-    assertThat(PROVIDER.zoneIdentification()).isEqualTo(Identification.OPAQUE);
+    assertThat(PROVIDER.supportsDuplicateZoneNames()).isFalse();
     assertThat(PROVIDER.credentialTypeToParameterNames())
         .containsEntry("password", Arrays.asList("tenantId", "username", "password"));
   }