sort Route53 resource record sets (#3864)

* sort Route53 resource record sets

* address comments

moto/route53/models.py

@@ -1,5 +1,6 @@
 from __future__ import unicode_literals
 
+import itertools
 from collections import defaultdict
 
 import string
@@ -264,20 +265,21 @@ class FakeZone(CloudFormationModel):
         ]
 
     def get_record_sets(self, start_type, start_name):
-        record_sets = list(self.rrsets)  # Copy the list
+        def predicate(rrset):
+            rrset_name_reversed = reverse_domain_name(rrset.name)
+            start_name_reversed = reverse_domain_name(start_name)
+            return rrset_name_reversed < start_name_reversed or (
+                rrset_name_reversed == start_name_reversed and rrset.type_ < start_type
+            )
+
+        record_sets = sorted(
+            self.rrsets,
+            key=lambda rrset: (reverse_domain_name(rrset.name), rrset.type_),
+        )
+
         if start_name:
-            record_sets = [
+            start_type = start_type or ""
-                record_set
+            record_sets = itertools.dropwhile(predicate, record_sets)
-                for record_set in record_sets
-                if reverse_domain_name(record_set.name)
-                >= reverse_domain_name(start_name)
-            ]
-        if start_type:
-            record_sets = [
-                record_set
-                for record_set in record_sets
-                if record_set.type_ >= start_type
-            ]
 
         return record_sets
 

moto/route53/responses.py

@@ -166,6 +166,10 @@ class Route53(BaseResponse):
             template = Template(LIST_RRSET_RESPONSE)
             start_type = querystring.get("type", [None])[0]
             start_name = querystring.get("name", [None])[0]
+
+            if start_type and not start_name:
+                return 400, headers, "The input is not valid"
+
             record_sets = the_zone.get_record_sets(start_type, start_name)
             return 200, headers, template.render(record_sets=record_sets)
 

tests/test_route53/test_route53.py

@@ -43,23 +43,27 @@ def test_hosted_zone():
 def test_rrset():
     conn = boto.connect_route53("the_key", "the_secret")
 
-    conn.get_all_rrsets.when.called_with("abcd", type="A").should.throw(
+    conn.get_all_rrsets.when.called_with("abcd").should.throw(
         boto.route53.exception.DNSServerError, "404 Not Found"
     )
 
     zone = conn.create_hosted_zone("testdns.aws.com")
     zoneid = zone["CreateHostedZoneResponse"]["HostedZone"]["Id"].split("/")[-1]
 
+    conn.get_all_rrsets.when.called_with(zoneid, type="A").should.throw(
+        boto.route53.exception.DNSServerError, "400 Bad Request"
+    )
+
     changes = ResourceRecordSets(conn, zoneid)
     change = changes.add_change("CREATE", "foo.bar.testdns.aws.com", "A")
     change.add_value("1.2.3.4")
     changes.commit()
 
-    rrsets = conn.get_all_rrsets(zoneid, type="A")
+    rrsets = conn.get_all_rrsets(zoneid)
     rrsets.should.have.length_of(1)
     rrsets[0].resource_records[0].should.equal("1.2.3.4")
 
-    rrsets = conn.get_all_rrsets(zoneid, type="CNAME")
+    rrsets = conn.get_all_rrsets(zoneid, name="foo.bar.testdns.aws.com.", type="CNAME")
     rrsets.should.have.length_of(0)
 
     changes = ResourceRecordSets(conn, zoneid)
@@ -68,7 +72,7 @@ def test_rrset():
     change.add_value("5.6.7.8")
     changes.commit()
 
-    rrsets = conn.get_all_rrsets(zoneid, type="A")
+    rrsets = conn.get_all_rrsets(zoneid)
     rrsets.should.have.length_of(1)
     rrsets[0].resource_records[0].should.equal("5.6.7.8")
 
@@ -84,7 +88,7 @@ def test_rrset():
     change.add_value("1.2.3.4")
     changes.commit()
 
-    rrsets = conn.get_all_rrsets(zoneid, type="A")
+    rrsets = conn.get_all_rrsets(zoneid)
     rrsets.should.have.length_of(1)
     rrsets[0].resource_records[0].should.equal("1.2.3.4")
 
@@ -93,7 +97,7 @@ def test_rrset():
     change.add_value("5.6.7.8")
     changes.commit()
 
-    rrsets = conn.get_all_rrsets(zoneid, type="A")
+    rrsets = conn.get_all_rrsets(zoneid)
     rrsets.should.have.length_of(1)
     rrsets[0].resource_records[0].should.equal("5.6.7.8")
 
@@ -113,24 +117,36 @@ def test_rrset():
     changes.commit()
 
     changes = ResourceRecordSets(conn, zoneid)
+    change = changes.add_change("CREATE", "bar.foo.testdns.aws.com", "TXT")
+    change.add_value("bar")
     change = changes.add_change("CREATE", "foo.bar.testdns.aws.com", "A")
     change.add_value("1.2.3.4")
     change = changes.add_change("CREATE", "bar.foo.testdns.aws.com", "A")
     change.add_value("5.6.7.8")
     changes.commit()
 
-    rrsets = conn.get_all_rrsets(zoneid, type="A")
+    rrsets = conn.get_all_rrsets(zoneid)
-    rrsets.should.have.length_of(2)
+    rrsets.should.have.length_of(3)
-
+    rrsets[0].resource_records[0].should.equal("1.2.3.4")
-    rrsets = conn.get_all_rrsets(zoneid, name="bar.foo.testdns.aws.com", type="A")
+    rrsets[1].resource_records[0].should.equal("5.6.7.8")
-    rrsets.should.have.length_of(1)
+    rrsets[2].resource_records[0].should.equal("bar")
-    rrsets[0].resource_records[0].should.equal("5.6.7.8")
 
     rrsets = conn.get_all_rrsets(zoneid, name="foo.bar.testdns.aws.com", type="A")
-    rrsets.should.have.length_of(2)
+    rrsets.should.have.length_of(3)
     resource_records = [rr for rr_set in rrsets for rr in rr_set.resource_records]
     resource_records.should.contain("1.2.3.4")
     resource_records.should.contain("5.6.7.8")
+    resource_records.should.contain("bar")
+
+    rrsets = conn.get_all_rrsets(zoneid, name="foo.testdns.aws.com", type="A")
+    rrsets.should.have.length_of(2)
+    rrsets[0].resource_records[0].should.equal("5.6.7.8")
+    rrsets[1].resource_records[0].should.equal("bar")
+
+    rrsets = conn.get_all_rrsets(zoneid, name="foo.bar.testdns.aws.com", type="TXT")
+    rrsets.should.have.length_of(2)
+    rrsets[0].resource_records[0].should.equal("5.6.7.8")
+    rrsets[1].resource_records[0].should.equal("bar")
 
     rrsets = conn.get_all_rrsets(zoneid, name="foo.foo.testdns.aws.com", type="A")
     rrsets.should.have.length_of(0)
@@ -148,7 +164,7 @@ def test_rrset_with_multiple_values():
     change.add_value("5.6.7.8")
     changes.commit()
 
-    rrsets = conn.get_all_rrsets(zoneid, type="A")
+    rrsets = conn.get_all_rrsets(zoneid)
     rrsets.should.have.length_of(1)
     set(rrsets[0].resource_records).should.equal(set(["1.2.3.4", "5.6.7.8"]))
 
@@ -176,17 +192,17 @@ def test_alias_rrset():
     )
     changes.commit()
 
-    rrsets = conn.get_all_rrsets(zoneid, type="A")
+    rrsets = conn.get_all_rrsets(zoneid, name="alias.testdns.aws.com")
     alias_targets = [rr_set.alias_dns_name for rr_set in rrsets]
     alias_targets.should.have.length_of(2)
     alias_targets.should.contain("foo.testdns.aws.com")
     alias_targets.should.contain("bar.testdns.aws.com")
-    rrsets[0].alias_dns_name.should.equal("foo.testdns.aws.com")
-    rrsets[0].resource_records.should.have.length_of(0)
-    rrsets = conn.get_all_rrsets(zoneid, type="CNAME")
-    rrsets.should.have.length_of(1)
     rrsets[0].alias_dns_name.should.equal("bar.testdns.aws.com")
     rrsets[0].resource_records.should.have.length_of(0)
+    rrsets = conn.get_all_rrsets(zoneid, name="foo.alias.testdns.aws.com", type="A")
+    rrsets.should.have.length_of(1)
+    rrsets[0].alias_dns_name.should.equal("foo.testdns.aws.com")
+    rrsets[0].resource_records.should.have.length_of(0)
 
 
 @mock_route53_deprecated