Skip to content

Commit 6f9e1c0

Browse files
mderkamderka
committed
Implements comments from @ajkannan and @aozarov.
1 parent 35cfd61 commit 6f9e1c0

File tree

4 files changed

+62
-91
lines changed

4 files changed

+62
-91
lines changed

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

Lines changed: 55 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,12 @@
1616

1717
package com.google.gcloud.dns;
1818

19+
import static com.google.common.base.Preconditions.checkNotNull;
20+
21+
import com.google.api.services.dns.model.ResourceRecordSet;
22+
import com.google.common.base.Function;
1923
import com.google.common.base.MoreObjects;
24+
import com.google.common.collect.ImmutableList;
2025
import com.google.common.collect.Lists;
2126

2227
import org.joda.time.DateTime;
@@ -27,17 +32,29 @@
2732
import java.util.List;
2833
import java.util.Objects;
2934

30-
import static com.google.common.base.Preconditions.checkNotNull;
31-
3235
/**
33-
* A class representing a change. A change is an atomic update to a collection of {@link DnsRecord}s
34-
* within a {@code ManagedZone}.
36+
* A class representing an atomic update to a collection of {@link DnsRecord}s within a {@code
37+
* ManagedZone}.
3538
*
3639
* @see <a href="https://cloud.google.com/dns/api/v1/changes">Google Cloud DNS documentation</a>
3740
*/
3841
public class ChangeRequest implements Serializable {
3942

40-
private static final long serialVersionUID = 201601251649L;
43+
private static final Function<ResourceRecordSet, DnsRecord> FROM_PB_FUNCTION =
44+
new Function<com.google.api.services.dns.model.ResourceRecordSet, DnsRecord>() {
45+
@Override
46+
public DnsRecord apply(com.google.api.services.dns.model.ResourceRecordSet pb) {
47+
return DnsRecord.fromPb(pb);
48+
}
49+
};
50+
private static final Function<DnsRecord, ResourceRecordSet> TO_PB_FUNCTION =
51+
new Function<DnsRecord, ResourceRecordSet>() {
52+
@Override
53+
public com.google.api.services.dns.model.ResourceRecordSet apply(DnsRecord error) {
54+
return error.toPb();
55+
}
56+
};
57+
private static final long serialVersionUID = -8703939628990291682L;
4158
private final List<DnsRecord> additions;
4259
private final List<DnsRecord> deletions;
4360
private final String id;
@@ -51,24 +68,8 @@ public class ChangeRequest implements Serializable {
5168
* documentation</a>
5269
*/
5370
public enum Status {
54-
PENDING("pending"),
55-
DONE("done");
56-
57-
private final String status;
58-
59-
Status(String status) {
60-
this.status = status;
61-
}
62-
63-
static Status translate(String status) {
64-
if ("pending".equals(status)) {
65-
return PENDING;
66-
} else if ("done".equals(status)) {
67-
return DONE;
68-
} else {
69-
throw new IllegalArgumentException("Such a status is unknown.");
70-
}
71-
}
71+
PENDING,
72+
DONE
7273
}
7374

7475
/**
@@ -101,18 +102,27 @@ public Builder additions(List<DnsRecord> additions) {
101102
this.additions = Lists.newLinkedList(checkNotNull(additions));
102103
return this;
103104
}
105+
106+
/**
107+
* Sets a collection of {@link DnsRecord}s which are to be deleted from the zone upon executing
108+
* this {@code ChangeRequest}.
109+
*/
110+
public Builder deletions(List<DnsRecord> deletions) {
111+
this.deletions = Lists.newLinkedList(checkNotNull(deletions));
112+
return this;
113+
}
104114

105115
/**
106-
* Adds a {@link DnsRecord} which to be <strong>added</strong> to the zone upon executing this
107-
* {@code ChangeRequest}.
116+
* Adds a {@link DnsRecord} to be <strong>added</strong> to the zone upon executing this {@code
117+
* ChangeRequest}.
108118
*/
109119
public Builder add(DnsRecord record) {
110120
this.additions.add(checkNotNull(record));
111121
return this;
112122
}
113123

114124
/**
115-
* Adds a {@link DnsRecord} which to be <strong>deleted</strong> to the zone upon executing this
125+
* Adds a {@link DnsRecord} to be <strong>deleted</strong> to the zone upon executing this
116126
* {@code ChangeRequest}.
117127
*/
118128
public Builder delete(DnsRecord record) {
@@ -139,7 +149,7 @@ public Builder clearDeletions() {
139149
}
140150

141151
/**
142-
* Removes a single {@link DnsRecord} from the collection of of records to be
152+
* Removes a single {@link DnsRecord} from the collection of records to be
143153
* <strong>added</strong> to the zone upon executing this {@code ChangeRequest}.
144154
*/
145155
public Builder removeAddition(DnsRecord record) {
@@ -148,23 +158,14 @@ public Builder removeAddition(DnsRecord record) {
148158
}
149159

150160
/**
151-
* Removes a single {@link DnsRecord} from the collection of of records to be
161+
* Removes a single {@link DnsRecord} from the collection of records to be
152162
* <strong>deleted</strong> from the zone upon executing this {@code ChangeRequest}.
153163
*/
154164
public Builder removeDeletion(DnsRecord record) {
155165
this.deletions.remove(record);
156166
return this;
157167
}
158168

159-
/**
160-
* Sets a collection of {@link DnsRecord}s which are to be deleted from the zone upon executing
161-
* this {@code ChangeRequest}.
162-
*/
163-
public Builder deletions(List<DnsRecord> deletions) {
164-
this.deletions = Lists.newLinkedList(checkNotNull(deletions));
165-
return this;
166-
}
167-
168169
/**
169170
* Associates a server-assigned id to this {@code ChangeRequest}.
170171
*/
@@ -199,8 +200,8 @@ public ChangeRequest build() {
199200
}
200201

201202
private ChangeRequest(Builder builder) {
202-
this.additions = builder.additions;
203-
this.deletions = builder.deletions;
203+
this.additions = ImmutableList.copyOf(builder.additions);
204+
this.deletions = ImmutableList.copyOf(builder.deletions);
204205
this.id = builder.id;
205206
this.startTimeMillis = builder.startTimeMillis;
206207
this.status = builder.status;
@@ -221,15 +222,15 @@ public Builder toBuilder() {
221222
}
222223

223224
/**
224-
* Returns the list of {@link DnsRecord}s to be added to the zone upon executing this {@code
225+
* Returns the list of {@link DnsRecord}s to be added to the zone upon submitting this {@code
225226
* ChangeRequest}.
226227
*/
227228
public List<DnsRecord> additions() {
228229
return additions;
229230
}
230231

231232
/**
232-
* Returns the list of {@link DnsRecord}s to be deleted from the zone upon executing this {@code
233+
* Returns the list of {@link DnsRecord}s to be deleted from the zone upon submitting this {@code
233234
* ChangeRequest}.
234235
*/
235236
public List<DnsRecord> deletions() {
@@ -270,56 +271,39 @@ com.google.api.services.dns.model.Change toPb() {
270271
}
271272
// set status
272273
if (status() != null) {
273-
pb.setStatus(status().status);
274+
pb.setStatus(status().name().toLowerCase());
274275
}
275276
// set a list of additions
276-
if (additions() != null) {
277-
LinkedList<com.google.api.services.dns.model.ResourceRecordSet> additionsPb =
278-
new LinkedList<>();
279-
for (DnsRecord addition : additions()) {
280-
additionsPb.add(addition.toPb());
281-
}
282-
pb.setAdditions(additionsPb);
283-
}
277+
pb.setAdditions(Lists.transform(additions(), TO_PB_FUNCTION));
284278
// set a list of deletions
285-
if (deletions() != null) {
286-
LinkedList<com.google.api.services.dns.model.ResourceRecordSet> deletionsPb =
287-
new LinkedList<>();
288-
for (DnsRecord deletion : deletions()) {
289-
deletionsPb.add(deletion.toPb());
290-
}
291-
pb.setDeletions(deletionsPb);
292-
}
279+
pb.setDeletions(Lists.transform(deletions(), TO_PB_FUNCTION));
293280
return pb;
294281
}
295282

296283
static ChangeRequest fromPb(com.google.api.services.dns.model.Change pb) {
297-
Builder b = builder();
284+
Builder builder = builder();
298285
if (pb.getId() != null) {
299-
b.id(pb.getId());
286+
builder.id(pb.getId());
300287
}
301288
if (pb.getStartTime() != null) {
302-
b.startTimeMillis(DateTime.parse(pb.getStartTime()).getMillis());
289+
builder.startTimeMillis(DateTime.parse(pb.getStartTime()).getMillis());
303290
}
304291
if (pb.getStatus() != null) {
305-
b.status(ChangeRequest.Status.translate(pb.getStatus()));
292+
// we are assuming that status indicated in pb is a lower case version of the enum name
293+
builder.status(ChangeRequest.Status.valueOf(pb.getStatus().toUpperCase()));
306294
}
307295
if (pb.getDeletions() != null) {
308-
for (com.google.api.services.dns.model.ResourceRecordSet deletion : pb.getDeletions()) {
309-
b.delete(DnsRecord.fromPb(deletion));
310-
}
296+
builder.deletions(Lists.transform(pb.getDeletions(), FROM_PB_FUNCTION));
311297
}
312298
if (pb.getAdditions() != null) {
313-
for (com.google.api.services.dns.model.ResourceRecordSet addition : pb.getAdditions()) {
314-
b.add(DnsRecord.fromPb(addition));
315-
}
299+
builder.additions(Lists.transform(pb.getAdditions(), FROM_PB_FUNCTION));
316300
}
317-
return b.build();
301+
return builder.build();
318302
}
319303

320304
@Override
321-
public boolean equals(Object o) {
322-
return (o instanceof ChangeRequest) && this.toPb().equals(((ChangeRequest) o).toPb());
305+
public boolean equals(Object other) {
306+
return (other instanceof ChangeRequest) && toPb().equals(((ChangeRequest) other).toPb());
323307
}
324308

325309
@Override

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -273,14 +273,14 @@ com.google.api.services.dns.model.ResourceRecordSet toPb() {
273273
}
274274

275275
static DnsRecord fromPb(com.google.api.services.dns.model.ResourceRecordSet pb) {
276-
Builder b = builder(pb.getName(), Type.valueOf(pb.getType()));
276+
Builder builder = builder(pb.getName(), Type.valueOf(pb.getType()));
277277
if (pb.getRrdatas() != null) {
278-
b.records(pb.getRrdatas());
278+
builder.records(pb.getRrdatas());
279279
}
280280
if (pb.getTtl() != null) {
281-
b.ttl(pb.getTtl());
281+
builder.ttl(pb.getTtl());
282282
}
283-
return b.build();
283+
return builder.build();
284284
}
285285

286286
@Override

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

Lines changed: 2 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ public void testClearAdditions() {
159159
@Test
160160
public void testAddAddition() {
161161
try {
162-
CHANGE.toBuilder().add(null).build();
162+
CHANGE.toBuilder().add(null);
163163
fail("Should not be able to add null DnsRecord.");
164164
} catch (NullPointerException e) {
165165
// expected
@@ -171,7 +171,7 @@ public void testAddAddition() {
171171
@Test
172172
public void testAddDeletion() {
173173
try {
174-
ChangeRequest clone = CHANGE.toBuilder().delete(null).build();
174+
CHANGE.toBuilder().delete(null);
175175
fail("Should not be able to delete null DnsRecord.");
176176
} catch (NullPointerException e) {
177177
// expected
@@ -203,7 +203,6 @@ public void testRemoveAddition() {
203203
@Test
204204
public void testRemoveDeletion() {
205205
ChangeRequest clone = CHANGE.toBuilder().removeDeletion(RECORD3).build();
206-
assertFalse(clone.deletions().contains(RECORD3));
207206
assertTrue(clone.deletions().isEmpty());
208207
}
209208

@@ -216,16 +215,4 @@ public void testDateParsing() {
216215
assertEquals(change, converted.toPb());
217216
assertEquals(change.getStartTime(), converted.toPb().getStartTime());
218217
}
219-
220-
@Test
221-
public void testStatusTranslation() {
222-
assertEquals(ChangeRequest.Status.DONE, ChangeRequest.Status.translate("done"));
223-
assertEquals(ChangeRequest.Status.PENDING, ChangeRequest.Status.translate("pending"));
224-
try {
225-
ChangeRequest.Status.translate("another");
226-
fail("Such a status does not exist.");
227-
} catch (IllegalArgumentException e) {
228-
// expected
229-
}
230-
}
231218
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,9 @@
1717
package com.google.gcloud.dns;
1818

1919
import static org.junit.Assert.assertEquals;
20+
import static org.junit.Assert.assertNotEquals;
2021
import static org.junit.Assert.assertTrue;
2122
import static org.junit.Assert.fail;
22-
import static org.junit.Assert.assertNotEquals;
2323

2424
import org.junit.Test;
2525

0 commit comments

Comments
 (0)