Skip to content

Commit b29945f

Browse files
mderkamderka
committed
Second round of comments from @mziccard 
1 parent f652277 commit b29945f

2 files changed

Lines changed: 80 additions & 66 deletions

File tree

‎gcloud-java-dns/src/main/java/com/google/gcloud/dns/DnsRecord.java‎

Lines changed: 39 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import static com.google.common.base.Preconditions.checkArgument;
1919
import static com.google.common.base.Preconditions.checkNotNull;
2020

21+
import com.google.common.base.MoreObjects;
2122
import com.google.common.collect.ImmutableList;
2223

2324
import java.io.Serializable;
@@ -29,7 +30,7 @@
2930
/**
3031
* A class that represents Google Cloud DNS record set.
3132
*
32-
* <p> A unit of data that will be returned by the DNS servers.
33+
* <p>A unit of data that will be returned by the DNS servers.
3334
*
3435
* @see <a href="https://cloud.google.com/dns/api/v1/resourceRecordSets">Google Cloud DNS
3536
* documentation</a>
@@ -44,9 +45,6 @@ public class DnsRecord implements Serializable {
4445
private final String zoneName;
4546
private final Long zoneId;
4647

47-
/**
48-
* A private constructor. Obtain an instance using {@link DnsRecord#Builder}.
49-
*/
5048
private DnsRecord() {
5149
this.name = null;
5250
this.rrdatas = null;
@@ -68,7 +66,7 @@ private DnsRecord() {
6866
/**
6967
* Enum for the DNS record types supported by Cloud DNS.
7068
*
71-
* <p> Google Cloud DNS currently supports records of type A, AAAA, CNAME, MX NAPTR, NS, PTR, SOA,
69+
* <p>Google Cloud DNS currently supports records of type A, AAAA, CNAME, MX NAPTR, NS, PTR, SOA,
7270
* SPF, SRV, TXT.
7371
*
7472
* @see <a href="https://cloud.google.com/dns/what-is-cloud-dns#supported_record_types">Cloud DNS
@@ -85,7 +83,7 @@ public enum DnsRecordType {
8583
SOA,
8684
SPF,
8785
SRV,
88-
TXT;
86+
TXT
8987
}
9088

9189
public static class Builder {
@@ -162,13 +160,23 @@ public DnsRecord build() {
162160

163161
/**
164162
* Sets references to the managed zone that this DNS record belongs to.
163+
*
164+
* todo(mderka): consider if this method is needed; may not be possible when listing records
165165
*/
166-
public Builder managedZone(ManagedZoneInfo parent) {
166+
Builder managedZone(ManagedZoneInfo parent) {
167167
checkNotNull(parent);
168168
this.zoneId = parent.id();
169169
this.zoneName = parent.name();
170170
return this;
171171
}
172+
173+
/**
174+
* Sets name reference to the managed zone that this DNS record belongs to.
175+
*/
176+
Builder managedZone(String managedZoneName) {
177+
this.zoneName = checkNotNull(managedZoneName);
178+
return this;
179+
}
172180
}
173181

174182
/**
@@ -196,12 +204,12 @@ public String name() {
196204
* Returns a list of DNS record stored in this record set.
197205
*/
198206
public List<String> rrdatas() {
199-
return ImmutableList.copyOf(rrdatas);
207+
return rrdatas;
200208
}
201209

202210
/**
203-
* Returns the number of seconds that this ResourceRecordSet can be cached by resolvers. This
204-
* number is provided by the user.
211+
* Returns the number of seconds that this DnsResource can be cached by resolvers. This number is
212+
* provided by the user.
205213
*/
206214
public Integer ttl() {
207215
return ttl;
@@ -224,9 +232,9 @@ public String zoneName() {
224232
}
225233

226234
/**
227-
* Returns name of the managed zone that this record belongs to.
235+
* Returns id of the managed zone that this record belongs to.
228236
*
229-
* <p> The id of the managed zone is determined by the server when the managed zone is created. It
237+
* <p>The id of the managed zone is determined by the server when the managed zone is created. It
230238
* is a read only value. If this DNS record is not associated with a managed zone, or if the id of
231239
* the managed zone was not loaded from the cloud service, this returns null.
232240
*/
@@ -241,30 +249,32 @@ public int hashCode() {
241249

242250
@Override
243251
public boolean equals(Object obj) {
244-
if (obj instanceof DnsRecord) {
245-
DnsRecord other = (DnsRecord) obj;
246-
return zoneId == other.zoneId()
247-
&& zoneName == other.zoneName
248-
&& this.toRRSet().equals(other.toRRSet());
249-
}
250-
return false;
252+
return (obj instanceof DnsRecord) && Objects.equals(this.toPb(), ((DnsRecord) obj).toPb())
253+
&& this.zoneId().equals(((DnsRecord) obj).zoneId())
254+
&& this.zoneName().equals(((DnsRecord) obj).zoneName());
255+
251256
}
252257

253-
com.google.api.services.dns.model.ResourceRecordSet toRRSet() {
254-
com.google.api.services.dns.model.ResourceRecordSet rrset =
258+
com.google.api.services.dns.model.ResourceRecordSet toPb() {
259+
com.google.api.services.dns.model.ResourceRecordSet pb =
255260
new com.google.api.services.dns.model.ResourceRecordSet();
256-
rrset.setName(name);
257-
rrset.setRrdatas(this.rrdatas());
258-
rrset.setTtl(ttl);
259-
rrset.setType(type == null ? null : type.name());
260-
return rrset;
261+
pb.setName(this.name());
262+
pb.setRrdatas(this.rrdatas());
263+
pb.setTtl(this.ttl());
264+
pb.setType(this.type() == null ? null : this.type().name());
265+
return pb;
261266
}
262267

263268
@Override
264269
public String toString() {
265-
return "DnsRecord{" + "name=" + name + ", rrdatas=" + rrdatas
266-
+ ", ttl=" + ttl + ", type=" + type + ", zoneName="
267-
+ zoneName + ", zoneId=" + zoneId + '}';
270+
return MoreObjects.toStringHelper(this)
271+
.add("name", name())
272+
.add("rrdatas", rrdatas())
273+
.add("ttl", ttl())
274+
.add("type", type())
275+
.add("zoneName", zoneName())
276+
.add("zoneId", zoneId())
277+
.toString();
268278
}
269279

270280
}

‎gcloud-java-dns/src/test/java/com/google/gcloud/dns/DnsRecordTest.java‎

Lines changed: 41 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -25,29 +25,30 @@
2525

2626
import org.easymock.EasyMock;
2727

28-
2928
public class DnsRecordTest {
3029

3130
private static final String NAME = "example.com.";
3231
private static final Integer TTL = 3600;
3332
private static final DnsRecord.DnsRecordType TYPE = DnsRecord.DnsRecordType.AAAA;
34-
private static final ManagedZoneInfo MANAGED_ZONE_INFO_MOCK =
35-
EasyMock.createMock(ManagedZoneInfo.class);
3633
private static final Long ZONE_ID = 12L;
3734
private static final String ZONE_NAME = "name";
38-
39-
static {
40-
EasyMock.expect(MANAGED_ZONE_INFO_MOCK.id()).andReturn(ZONE_ID);
41-
EasyMock.expect(MANAGED_ZONE_INFO_MOCK.name()).andReturn(ZONE_NAME);
42-
EasyMock.replay(MANAGED_ZONE_INFO_MOCK);
35+
// the following is initialized in @BeforeClass setUp()
36+
private static DnsRecord record;
37+
private static ManagedZoneInfo managedZoneInfoMock;
38+
39+
@BeforeClass
40+
public static void setUp() {
41+
managedZoneInfoMock = EasyMock.createMock(ManagedZoneInfo.class);
42+
EasyMock.expect(managedZoneInfoMock.id()).andReturn(ZONE_ID);
43+
EasyMock.expect(managedZoneInfoMock.name()).andReturn(ZONE_NAME);
44+
EasyMock.replay(managedZoneInfoMock);
45+
record = DnsRecord.builder()
46+
.name(NAME)
47+
.ttl(TTL)
48+
.managedZone(managedZoneInfoMock)
49+
.build();
4350
}
4451

45-
private static final DnsRecord RECORD = DnsRecord.builder()
46-
.name(NAME)
47-
.ttl(TTL)
48-
.managedZone(MANAGED_ZONE_INFO_MOCK)
49-
.build();
50-
5152
@Test
5253
public void testDefaultDnsRecord() {
5354
DnsRecord record = DnsRecord.builder().build();
@@ -57,20 +58,23 @@ public void testDefaultDnsRecord() {
5758
@Test
5859
public void testBuilder() {
5960

60-
assertEquals(NAME, RECORD.name());
61-
assertEquals(TTL, RECORD.ttl());
61+
assertEquals(NAME, record.name());
62+
assertEquals(TTL, record.ttl());
6263

63-
assertEquals(ZONE_ID, RECORD.zoneId()); // this was never assigned
64-
assertEquals(ZONE_NAME, RECORD.zoneName());
65-
assertEquals(0, RECORD.rrdatas().size());
64+
assertEquals(ZONE_ID, record.zoneId()); // this was never assigned
65+
assertEquals(ZONE_NAME, record.zoneName());
66+
assertEquals(0, record.rrdatas().size());
6667
// verify that one can add records to the record set
6768
String testingRecord = "Testing record";
6869
String anotherTestingRecord = "Another record 123";
69-
DnsRecord anotherRecord = RECORD.toBuilder()
70+
String differentName = ZONE_NAME + "something";
71+
DnsRecord anotherRecord = record.toBuilder()
7072
.add(testingRecord)
7173
.add(anotherTestingRecord)
74+
.managedZone(differentName)
7275
.build();
7376
assertEquals(2, anotherRecord.rrdatas().size());
77+
assertEquals(differentName, anotherRecord.zoneName());
7478
assertTrue(anotherRecord.rrdatas().contains(testingRecord));
7579
assertTrue(anotherRecord.rrdatas().contains(anotherTestingRecord));
7680
}
@@ -89,39 +93,39 @@ public void testValidTtl() {
8993

9094
@Test
9195
public void testEqualsAndNotEquals() {
92-
DnsRecord clone = RECORD.toBuilder().build();
93-
assertEquals(clone, RECORD);
94-
clone = RECORD.toBuilder().add("another record").build();
96+
DnsRecord clone = record.toBuilder().build();
97+
assertEquals(clone, record);
98+
clone = record.toBuilder().add("another record").build();
9599
final String differentName = "totally different name";
96-
clone = RECORD.toBuilder().name(differentName).build();
97-
assertNotEquals(clone, RECORD);
98-
clone = RECORD.toBuilder().ttl(RECORD.ttl() + 1).build();
99-
assertNotEquals(clone, RECORD);
100-
clone = RECORD.toBuilder().type(DnsRecord.DnsRecordType.TXT).build();
101-
assertNotEquals(clone, RECORD);
100+
clone = record.toBuilder().name(differentName).build();
101+
assertNotEquals(clone, record);
102+
clone = record.toBuilder().ttl(record.ttl() + 1).build();
103+
assertNotEquals(clone, record);
104+
clone = record.toBuilder().type(DnsRecord.DnsRecordType.TXT).build();
105+
assertNotEquals(clone, record);
102106
ManagedZoneInfo anotherMock = EasyMock.createMock(ManagedZoneInfo.class);
103107
EasyMock.expect(anotherMock.id()).andReturn(ZONE_ID + 1);
104108
EasyMock.expect(anotherMock.name()).andReturn(ZONE_NAME + "more text");
105109
EasyMock.replay(anotherMock);
106-
clone = RECORD.toBuilder().managedZone(anotherMock).build();
107-
assertNotEquals(clone, RECORD);
110+
clone = record.toBuilder().managedZone(anotherMock).build();
111+
assertNotEquals(clone, record);
108112
}
109113

110114
@Test
111115
public void testSameHashCodeOnEquals() {
112-
int hash = RECORD.hashCode();
113-
DnsRecord clone = RECORD.toBuilder().build();
116+
int hash = record.hashCode();
117+
DnsRecord clone = record.toBuilder().build();
114118
assertEquals(clone.hashCode(), hash);
115119
}
116120

117121
@Test
118122
public void testDifferentHashCodeOnDifferent() {
119-
int hash = RECORD.hashCode();
123+
int hash = record.hashCode();
120124
final String differentName = "totally different name";
121-
DnsRecord clone = RECORD.toBuilder().name(differentName).build();
122-
assertNotEquals(differentName, RECORD.name());
125+
DnsRecord clone = record.toBuilder().name(differentName).build();
126+
assertNotEquals(differentName, record.name());
123127
assertNotEquals(clone.hashCode(), hash);
124-
DnsRecord anotherClone = RECORD.toBuilder().add("another record").build();
128+
DnsRecord anotherClone = record.toBuilder().add("another record").build();
125129
assertNotEquals(anotherClone.hashCode(), hash);
126130
}
127131

0 commit comments

Comments
 (0)