diff options
author | jonmv <venstad@gmail.com> | 2023-01-25 17:37:28 +0100 |
---|---|---|
committer | jonmv <venstad@gmail.com> | 2023-01-25 17:37:28 +0100 |
commit | b536ab147d2610dcd3664a2b4742fb4e7a6569fd (patch) | |
tree | ba17e739481aedbc69c914d7dd913c484991482b | |
parent | a3812e336b4984d29f1540fa030de952724aaf93 (diff) |
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"), |