Remove RecordId

`NameService` shouldn't enforce the use of record IDs as it implementations may
not always operate on IDs. Creating synthetic IDs in such cases is more trouble
than it's worth.

6 files changed, 97 insertions, 110 deletions

diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/dns/MemoryNameService.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/dns/MemoryNameService.java
index 46a6d1c35ad..e8b0c8a66ef 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/dns/MemoryNameService.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/dns/MemoryNameService.java
@@ -2,15 +2,11 @@
 package com.yahoo.vespa.hosted.controller.api.integration.dns;
 
 
-import java.util.Collection;
 import java.util.Collections;
 import java.util.Comparator;
-import java.util.HashMap;
-import java.util.LinkedHashSet;
 import java.util.List;
-import java.util.Map;
 import java.util.Set;
-import java.util.UUID;
+import java.util.TreeSet;
 import java.util.stream.Collectors;
 
 /**
@@ -20,67 +16,64 @@ import java.util.stream.Collectors;
  */
 public class MemoryNameService implements NameService {
 
-    private final Map<RecordId, Set<Record>> records = new HashMap<>();
+    private final Set<Record> records = new TreeSet<>();
 
-    public Map<RecordId, Set<Record>> records() {
-        return Collections.unmodifiableMap(records);
+    public Set<Record> records() {
+        return Collections.unmodifiableSet(records);
     }
 
     @Override
-    public RecordId createCname(RecordName name, RecordData canonicalName) {
-        RecordId id = new RecordId(UUID.randomUUID().toString());
-        records.put(id, Set.of(new Record(id, Record.Type.CNAME, name, canonicalName)));
-        return id;
+    public Record createCname(RecordName name, RecordData canonicalName) {
+        Record record = new Record(Record.Type.CNAME, name, canonicalName);
+        records.add(record);
+        return record;
     }
 
     @Override
-    public RecordId createAlias(RecordName name, Set<AliasTarget> targets) {
-        RecordId id = new RecordId(UUID.randomUUID().toString());
-        Set<Record> records = targets.stream()
+    public List<Record> createAlias(RecordName name, Set<AliasTarget> targets) {
+        List<Record> records = targets.stream()
                                      .sorted((a, b) -> Comparator.comparing(AliasTarget::name).compare(a, b))
-                                     .map(target -> new Record(id, Record.Type.ALIAS, name,
+                                     .map(target -> new Record(Record.Type.ALIAS, name,
                                                                RecordData.fqdn(target.name().value())))
-                                     .collect(Collectors.toCollection(LinkedHashSet::new));
+                                     .collect(Collectors.toList());
         // Satisfy idempotency contract of interface
-        findRecords(Record.Type.ALIAS, name).stream().map(Record::id).forEach(this::removeRecord);
-        this.records.put(id, records);
-        return id;
+        removeRecords(findRecords(Record.Type.ALIAS, name));
+        this.records.addAll(records);
+        return records;
     }
 
     @Override
     public List<Record> findRecords(Record.Type type, RecordName name) {
-        return records.values().stream()
-                      .flatMap(Collection::stream)
+        return records.stream()
                       .filter(record -> record.type() == type && record.name().equals(name))
                       .collect(Collectors.toUnmodifiableList());
     }
 
     @Override
     public List<Record> findRecords(Record.Type type, RecordData data) {
-        return records.values().stream()
-                      .flatMap(Collection::stream)
+        return records.stream()
                       .filter(record -> record.type() == type && record.data().equals(data))
                       .collect(Collectors.toUnmodifiableList());
     }
 
     @Override
-    public void updateRecord(RecordId id, RecordData newData) {
-        records.computeIfPresent(id, (k, records) -> {
-            if (records.isEmpty()) {
-                throw new IllegalArgumentException("No record with data '" + newData.asString() + "' exists");
-            }
-            if (records.size() > 1) {
-                throw new IllegalArgumentException("Cannot update multi-value record '" + id.asString() + "' with '" +
-                                                   newData.asString() + "'");
-            }
-            Record existing = records.iterator().next();
-            return Set.of(new Record(id, existing.type(), existing.name(), newData));
-        });
+    public void updateRecord(Record record, RecordData newData) {
+        List<Record> records = findRecords(record.type(), record.name());
+        if (records.isEmpty()) {
+            throw new IllegalArgumentException("No record with data '" + newData.asString() + "' exists");
+        }
+        if (records.size() > 1) {
+            throw new IllegalArgumentException("Cannot update multi-value record '" + record.name().asString() +
+                                               "' with '" + newData.asString() + "'");
+        }
+        Record existing = records.get(0);
+        this.records.remove(existing);
+        this.records.add(new Record(existing.type(), existing.name(), newData));
     }
 
     @Override
-    public void removeRecord(RecordId id) {
-        records.remove(id);
+    public void removeRecords(List<Record> records) {
+        this.records.removeAll(records);
     }
 
 }
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/dns/NameService.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/dns/NameService.java
index 537460c8b1e..444c8dda8d3 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/dns/NameService.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/dns/NameService.java
@@ -14,24 +14,31 @@ public interface NameService {
     /**
      * Create a new CNAME record
      *
-     * @param alias The alias to create
-     * @param canonicalName The canonical name which the alias should point to. This must be a FQDN.
+     * @param name          The alias to create (lhs of the record)
+     * @param canonicalName The canonical name which the alias should point to (rhs of the record). This must be a FQDN.
+     * @return The created record
      */
-    RecordId createCname(RecordName alias, RecordData canonicalName);
+    Record createCname(RecordName name, RecordData canonicalName);
 
-    /** Create a non-standard ALIAS record pointing to given targets. Implementations of this are expected to be idempotent */
-    RecordId createAlias(RecordName name, Set<AliasTarget> targets);
+    /**
+     * Create a non-standard ALIAS record pointing to given targets. Implementations of this can be expected to be
+     * idempotent
+     *
+     * @param targets Targets that should be resolved by this alias. pointing to given targets.
+     * @return The created records. One for each target.
+     */
+    List<Record> createAlias(RecordName name, Set<AliasTarget> targets);
 
-    /** Find records matching type and name */
+    /** Find all records matching given type and name */
     List<Record> findRecords(Record.Type type, RecordName name);
 
-    /** Find records matching type and data */
+    /** Find all records matching given type and data */
     List<Record> findRecords(Record.Type type, RecordData data);
 
     /** Update existing record */
-    void updateRecord(RecordId id, RecordData newData);
+    void updateRecord(Record record, RecordData newData);
 
-    /** Remove record by ID */
-    void removeRecord(RecordId id);
+    /** Remove given record(s) */
+    void removeRecords(List<Record> record);
 
 }
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/dns/Record.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/dns/Record.java
index 218fc9f5266..f47f2061000 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/dns/Record.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/dns/Record.java
@@ -1,43 +1,41 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.hosted.controller.api.integration.dns;
 
+import java.util.Comparator;
 import java.util.Objects;
 
 /**
- * A basic representation of a DNS resource record, containing the record id, type, name and value.
+ * A basic representation of a DNS resource record, containing the record type, name and data.
  *
  * @author mpolden
  */
-public class Record {
+public class Record implements Comparable<Record> {
+
+    private static final Comparator<Record> comparator = Comparator.comparing(Record::type)
+                                                                   .thenComparing(Record::name)
+                                                                   .thenComparing(Record::data);
 
-    private final RecordId id;
     private final Type type;
     private final RecordName name;
     private final RecordData data;
 
-    public Record(RecordId id, Type type, RecordName name, RecordData data) {
-        this.id = Objects.requireNonNull(id, "id cannot be null");
+    public Record(Type type, RecordName name, RecordData data) {
         this.type = Objects.requireNonNull(type, "type cannot be null");
         this.name = Objects.requireNonNull(name, "name cannot be null");
         this.data = Objects.requireNonNull(data, "data cannot be null");
     }
 
-    /** Unique identifier for this */
-    public RecordId id() {
-        return id;
-    }
-
     /** DNS type of this */
     public Type type() {
         return type;
     }
 
-    /** Data in this, e.g. IP address for "A" record */
+    /** Data in this, e.g. IP address for records of type A */
     public RecordData data() {
         return data;
     }
 
-    /** Name of this, e.g. a FQDN for "A" record */
+    /** Name of this, e.g. a FQDN for records of type A */
     public RecordName name() {
         return name;
     }
@@ -57,7 +55,7 @@ public class Record {
 
     @Override
     public String toString() {
-        return String.format("%s: %s %s -> %s", id, type, name, data);
+        return String.format("%s %s -> %s", type, name, data);
     }
 
     @Override
@@ -65,14 +63,19 @@ public class Record {
         if (this == o) return true;
         if (o == null || getClass() != o.getClass()) return false;
         Record record = (Record) o;
-        return Objects.equals(id, record.id) &&
-               type == record.type &&
-               Objects.equals(name, record.name) &&
-               Objects.equals(data, record.data);
+        return type == record.type &&
+               name.equals(record.name) &&
+               data.equals(record.data);
     }
 
     @Override
     public int hashCode() {
-        return Objects.hash(id, type, name, data);
+        return Objects.hash(type, name, data);
     }
+
+    @Override
+    public int compareTo(Record that) {
+        return comparator.compare(this, that);
+    }
+
 }
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/dns/RecordData.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/dns/RecordData.java
index 6c765efd35a..fddcd85e8af 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/dns/RecordData.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/dns/RecordData.java
@@ -10,7 +10,7 @@ import java.util.Objects;
  *
  * @author mpolden
  */
-public class RecordData {
+public class RecordData implements Comparable<RecordData> {
 
     private final String data;
 
@@ -40,7 +40,7 @@ public class RecordData {
         return data;
     }
 
-    /** Create a new record containing the given data */
+    /** Create data containing the given data */
     public static RecordData from(String data) {
         return new RecordData(data);
     }
@@ -50,4 +50,9 @@ public class RecordData {
         return from(data.endsWith(".") ? data : data + ".");
     }
 
+    @Override
+    public int compareTo(RecordData that) {
+        return this.data.compareTo(that.data);
+    }
+
 }
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/dns/RecordId.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/dns/RecordId.java
deleted file mode 100644
index ed628f0c827..00000000000
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/dns/RecordId.java
+++ /dev/null
@@ -1,40 +0,0 @@
-// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-package com.yahoo.vespa.hosted.controller.api.integration.dns;
-
-import java.util.Objects;
-
-/**
- * Unique identifier for a resource record.
- *
- * @author mpolden
- */
-public class RecordId {
-
-    private final String id;
-
-    public RecordId(String id) {
-        this.id = id;
-    }
-
-    public String asString() {
-        return id;
-    }
-
-    @Override
-    public String toString() {
-        return id;
-    }
-
-    @Override
-    public boolean equals(Object o) {
-        if (this == o) return true;
-        if (o == null || getClass() != o.getClass()) return false;
-        RecordId recordId = (RecordId) o;
-        return id.equals(recordId.id);
-    }
-
-    @Override
-    public int hashCode() {
-        return Objects.hash(id);
-    }
-}
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/dns/RecordName.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/dns/RecordName.java
index d3abad9fb62..f092209c1d8 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/dns/RecordName.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/dns/RecordName.java
@@ -4,11 +4,11 @@ package com.yahoo.vespa.hosted.controller.api.integration.dns;
 import java.util.Objects;
 
 /**
- * Represents the name field of a DNS record (NAME). This is typically a FQDN.
+ * Represents the name field of a DNS record (NAME).
  *
  * @author mpolden
  */
-public class RecordName {
+public class RecordName implements Comparable<RecordName> {
 
     private final String name;
 
@@ -20,6 +20,16 @@ public class RecordName {
         return name;
     }
 
+    /** Returns whether this is a fully qualified domain name (ends in trailing dot) */
+    public boolean isFqdn() {
+        return name.endsWith(".");
+    }
+
+    /** Returns this as a fully qualified domain name (ends in trailing dot) */
+    public RecordName asFqdn() {
+        return isFqdn() ? this : new RecordName(name + ".");
+    }
+
     @Override
     public boolean equals(Object o) {
         if (this == o) return true;
@@ -42,4 +52,13 @@ public class RecordName {
         return new RecordName(name);
     }
 
+    public static RecordName fqdn(String name) {
+        return from(name).asFqdn();
+    }
+
+    @Override
+    public int compareTo(RecordName that) {
+        return this.name.compareTo(that.name);
+    }
+
 }