Refactor with abstract parent, and make name() non-optional

8 files changed, 56 insertions, 58 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/AbstractNameServiceRequest.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/AbstractNameServiceRequest.java
new file mode 100644
index 00000000000..9d21f5b26bd
--- /dev/null
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/AbstractNameServiceRequest.java
@@ -0,0 +1,33 @@
+package com.yahoo.vespa.hosted.controller.dns;
+
+import com.yahoo.vespa.hosted.controller.api.integration.dns.RecordName;
+import com.yahoo.vespa.hosted.controller.application.TenantAndApplicationId;
+
+import java.util.Optional;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * @author jonmv
+ */
+public abstract class AbstractNameServiceRequest implements NameServiceRequest {
+
+    private final Optional<TenantAndApplicationId> owner;
+    private final RecordName name;
+
+    AbstractNameServiceRequest(Optional<TenantAndApplicationId> owner, RecordName name) {
+        this.owner = requireNonNull(owner);
+        this.name = requireNonNull(name);
+    }
+
+    @Override
+    public RecordName name() {
+        return name;
+    }
+
+    @Override
+    public Optional<TenantAndApplicationId> owner() {
+        return owner;
+    }
+
+}
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/CreateRecord.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/CreateRecord.java
index f1e4ca3b82b..6f4ee3dfc06 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/CreateRecord.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/CreateRecord.java
@@ -15,14 +15,13 @@ import java.util.Optional;
  *
  * @author mpolden
  */
-public class CreateRecord implements NameServiceRequest {
+public class CreateRecord extends AbstractNameServiceRequest {
 
-    private final Optional<TenantAndApplicationId> owner;
     private final Record record;
 
     /** DO NOT USE. Public for serialization purposes */
     public CreateRecord(Optional<TenantAndApplicationId> owner, Record record) {
-        this.owner = Objects.requireNonNull(owner, "owner must be non-null");
+        super(owner, record.name());
         this.record = Objects.requireNonNull(record, "record must be non-null");
         if (record.type() != Record.Type.CNAME && record.type() != Record.Type.A) {
             throw new IllegalArgumentException("Record of type " + record.type() + " is not supported: " + record);
@@ -34,16 +33,6 @@ public class CreateRecord implements NameServiceRequest {
     }
 
     @Override
-    public Optional<RecordName> name() {
-        return Optional.of(record.name());
-    }
-
-    @Override
-    public Optional<TenantAndApplicationId> owner() {
-        return owner;
-    }
-
-    @Override
     public void dispatchTo(NameService nameService) {
         List<Record> records = nameService.findRecords(record.type(), record.name());
         records.forEach(r -> {
@@ -67,12 +56,12 @@ public class CreateRecord implements NameServiceRequest {
         if (this == o) return true;
         if (o == null || getClass() != o.getClass()) return false;
         CreateRecord that = (CreateRecord) o;
-        return owner.equals(that.owner) && record.equals(that.record);
+        return owner().equals(that.owner()) && record.equals(that.record);
     }
 
     @Override
     public int hashCode() {
-        return Objects.hash(owner, record);
+        return Objects.hash(owner(), record);
     }
 
 }
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/CreateRecords.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/CreateRecords.java
index a668c408794..ef7b74a4d4b 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/CreateRecords.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/CreateRecords.java
@@ -20,17 +20,14 @@ import java.util.stream.Collectors;
  *
  * @author mpolden
  */
-public class CreateRecords implements NameServiceRequest {
+public class CreateRecords extends AbstractNameServiceRequest {
 
-    private final Optional<TenantAndApplicationId> owner;
-    private final RecordName name;
     private final Record.Type type;
     private final List<Record> records;
 
     /** DO NOT USE. Public for serialization purposes */
     public CreateRecords(Optional<TenantAndApplicationId> owner, List<Record> records) {
-        this.owner = Objects.requireNonNull(owner, "owner must be non-null");
-        this.name = requireOneOf(Record::name, records);
+        super(owner, requireOneOf(Record::name, records));
         this.type = requireOneOf(Record::type, records);
         this.records = List.copyOf(Objects.requireNonNull(records, "records must be non-null"));
         if (type != Record.Type.ALIAS && type != Record.Type.TXT && type != Record.Type.DIRECT) {
@@ -43,29 +40,19 @@ public class CreateRecords implements NameServiceRequest {
     }
 
     @Override
-    public Optional<RecordName> name() {
-        return Optional.of(name);
-    }
-
-    @Override
-    public Optional<TenantAndApplicationId> owner() {
-        return owner;
-    }
-
-    @Override
     public void dispatchTo(NameService nameService) {
         switch (type) {
             case ALIAS -> {
                 var targets = records.stream().map(Record::data).map(AliasTarget::unpack).collect(Collectors.toSet());
-                nameService.createAlias(name, targets);
+                nameService.createAlias(name(), targets);
             }
             case DIRECT -> {
                 var targets = records.stream().map(Record::data).map(DirectTarget::unpack).collect(Collectors.toSet());
-                nameService.createDirect(name, targets);
+                nameService.createDirect(name(), targets);
             }
             case TXT -> {
                 var dataFields = records.stream().map(Record::data).toList();
-                nameService.createTxtRecords(name, dataFields);
+                nameService.createTxtRecords(name(), dataFields);
             }
         }
     }
@@ -80,12 +67,12 @@ public class CreateRecords implements NameServiceRequest {
         if (this == o) return true;
         if (o == null || getClass() != o.getClass()) return false;
         CreateRecords that = (CreateRecords) o;
-        return owner.equals(that.owner) && records.equals(that.records);
+        return owner().equals(that.owner()) && records.equals(that.records);
     }
 
     @Override
     public int hashCode() {
-        return Objects.hash(owner, records);
+        return Objects.hash(owner(), records);
     }
 
     /** Find exactly one distinct value of field in given list */
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/NameServiceRequest.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/NameServiceRequest.java
index c43130ce4e9..dd3cca9a4fa 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/NameServiceRequest.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/NameServiceRequest.java
@@ -14,7 +14,8 @@ import java.util.Optional;
  */
 public interface NameServiceRequest {
 
-    Optional<RecordName> name();
+    /** The record name this request pertains to. */
+    RecordName name();
 
     /** The application owning this request */
     Optional<TenantAndApplicationId> owner();
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/RemoveRecords.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/RemoveRecords.java
index 2ff2edf11f4..273136ba0a6 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/RemoveRecords.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/RemoveRecords.java
@@ -21,11 +21,9 @@ import java.util.Optional;
  *
  * @author mpolden
  */
-public class RemoveRecords implements NameServiceRequest {
+public class RemoveRecords extends AbstractNameServiceRequest {
 
-    private final Optional<TenantAndApplicationId> owner;
     private final Record.Type type;
-    private final RecordName name;
     private final Optional<RecordData> data;
 
     public RemoveRecords(Optional<TenantAndApplicationId> owner, Record.Type type, RecordName name) {
@@ -38,9 +36,8 @@ public class RemoveRecords implements NameServiceRequest {
 
     /** DO NOT USE. Public for serialization purposes */
     public RemoveRecords(Optional<TenantAndApplicationId> owner, Record.Type type, RecordName name, Optional<RecordData> data) {
-        this.owner = Objects.requireNonNull(owner, "owner must be non-null");
+        super(owner, name);
         this.type = Objects.requireNonNull(type, "type must be non-null");
-        this.name = Objects.requireNonNull(name, "name must be non-null");
         this.data = Objects.requireNonNull(data, "data must be non-null");
     }
 
@@ -48,16 +45,6 @@ public class RemoveRecords implements NameServiceRequest {
         return type;
     }
 
-    @Override
-    public Optional<RecordName> name() {
-        return Optional.of(name);
-    }
-
-    @Override
-    public Optional<TenantAndApplicationId> owner() {
-        return owner;
-    }
-
     public Optional<RecordData> data() {
         return data;
     }
@@ -66,7 +53,7 @@ public class RemoveRecords implements NameServiceRequest {
     public void dispatchTo(NameService nameService) {
         // Deletions require all records fields to match exactly, data may be incomplete even if present. To ensure
         // completeness we search for the record(s) first
-        List<Record> completeRecords = nameService.findRecords(type, name).stream()
+        List<Record> completeRecords = nameService.findRecords(type, name()).stream()
                                                   .filter(record -> data.isEmpty() || matchingFqdnIn(data.get(), record))
                                                   .toList();
         nameService.removeRecords(completeRecords);
@@ -74,7 +61,7 @@ public class RemoveRecords implements NameServiceRequest {
 
     @Override
     public String toString() {
-        return "remove records of type " + type + ", by name " + name +
+        return "remove records of type " + type + ", by name " + name() +
                data.map(d -> " data " + d).orElse("");
     }
 
@@ -83,12 +70,12 @@ public class RemoveRecords implements NameServiceRequest {
         if (this == o) return true;
         if (o == null || getClass() != o.getClass()) return false;
         RemoveRecords that = (RemoveRecords) o;
-        return owner.equals(that.owner) && type == that.type && name.equals(that.name) && data.equals(that.data);
+        return owner().equals(that.owner()) && type == that.type && name().equals(that.name()) && data.equals(that.data);
     }
 
     @Override
     public int hashCode() {
-        return Objects.hash(owner, type, name, data);
+        return Objects.hash(owner(), type, name(), data);
     }
 
     private static boolean matchingFqdnIn(RecordData data, Record record) {
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/NameServiceQueueSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/NameServiceQueueSerializer.java
index 89230969164..d02d27b5293 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/NameServiceQueueSerializer.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/NameServiceQueueSerializer.java
@@ -92,7 +92,7 @@ public class NameServiceQueueSerializer {
     private void toSlime(Cursor object, RemoveRecords removeRecords) {
         object.setString(requestType, Request.removeRecords.name());
         object.setString(typeField, removeRecords.type().name());
-        removeRecords.name().ifPresent(name -> object.setString(nameField, name.asString()));
+        object.setString(nameField, removeRecords.name().asString());
         removeRecords.data().ifPresent(data -> object.setString(dataField, data.asString()));
     }
 
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicies.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicies.java
index c737f9b58ef..8be0b78acbd 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicies.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicies.java
@@ -399,7 +399,7 @@ public class RoutingPolicies {
                           while (controller.clock().instant().isBefore(doom)) {
                               try (Mutex lock = controller.curator().lockNameServiceQueue()) {
                                   if (controller.curator().readNameServiceQueue().requests().stream()
-                                                .noneMatch(request -> request.name().equals(Optional.of(challenge.name())))) {
+                                                .noneMatch(request -> request.name().equals(challenge.name()))) {
                                       try { challenge.trigger().run(); }
                                       finally { nameServiceForwarderIn(deploymentId.zoneId()).removeRecords(Type.TXT, challenge.name(), Priority.normal, ownerOf(deploymentId)); }
                                       return;
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/NameServiceQueueSerializerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/NameServiceQueueSerializerTest.java
index 582514163e1..ad9ebd2a968 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/NameServiceQueueSerializerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/NameServiceQueueSerializerTest.java
@@ -12,6 +12,7 @@ import com.yahoo.vespa.hosted.controller.application.TenantAndApplicationId;
 import com.yahoo.vespa.hosted.controller.dns.CreateRecord;
 import com.yahoo.vespa.hosted.controller.dns.CreateRecords;
 import com.yahoo.vespa.hosted.controller.dns.NameServiceQueue;
+import com.yahoo.vespa.hosted.controller.dns.NameServiceRequest;
 import com.yahoo.vespa.hosted.controller.dns.RemoveRecords;
 import org.junit.jupiter.api.Test;
 
@@ -32,7 +33,7 @@ public class NameServiceQueueSerializerTest {
         Optional<TenantAndApplicationId> owner = Optional.of(TenantAndApplicationId.from("t", "a"));
         var record1 = new Record(Record.Type.CNAME, RecordName.from("cname.example.com"), RecordData.from("example.com"));
         var record2 = new Record(Record.Type.TXT, RecordName.from("txt.example.com"), RecordData.from("text"));
-        var requests = List.of(
+        var requests = List.<NameServiceRequest>of(
                 new CreateRecord(owner, record1),
                 new CreateRecords(owner, List.of(record2)),
                 new CreateRecords(owner, List.of(new Record(Record.Type.ALIAS, RecordName.from("alias.example.com"),