Trim user inputs as whitespaces causes discrepencies in caching rules

Change-Id: Ib9d2ab70f435fd996c4a7a4130635d52a0a7c668

Trim whitespaces that is induced by user inputs

Change-Id: Ib9d2ab70f435fd996c4a7a4130635d52a0a7c668

poppy/model/helpers/cachingrule.py

@@ -39,7 +39,7 @@ class CachingRule(common.DictSerializableModel):
 
     @name.setter
     def name(self, value):
-        self._name = value
+        self._name = value.strip()
 
     @property
     def ttl(self):
@@ -81,9 +81,12 @@ class CachingRule(common.DictSerializableModel):
 
         o = cls("unnamed", 3600)
         o.caching = dict_obj.get("caching", "unnamed")
-        o.name = dict_obj.get("name", "unnamed")
+        o.name = dict_obj.get("name", "unnamed").strip()
         o.ttl = dict_obj.get("ttl", 3600)
         rules_dict_list = dict_obj.get("rules", [])
+        for val in rules_dict_list:
+            val['name'] = val['name'].strip()
+            val['request_url'] = val['request_url'].strip()
         o.rules = []
         for rule_dict in rules_dict_list:
             new_rule = rule.Rule(rule_dict['name'])

poppy/model/helpers/origin.py

@@ -37,7 +37,7 @@ class Origin(common.DictSerializableModel):
     @origin.setter
     def origin(self, value):
         """origin setter."""
-        self._origin = value
+        self._origin = value.strip()
 
     @property
     def port(self):
@@ -110,7 +110,7 @@ class Origin(common.DictSerializableModel):
         """
 
         o = cls("unnamed")
-        o.origin = dict_obj.get("origin", "unnamed")
+        o.origin = dict_obj.get("origin", "unnamed").strip()
         o.port = dict_obj.get("port", 80)
         o.ssl = dict_obj.get("ssl", False)
         o.hostheadertype = dict_obj.get("hostheadertype", "domain")
@@ -118,6 +118,9 @@ class Origin(common.DictSerializableModel):
         if o.hostheadertype == 'origin':
             o.hostheadervalue = o.origin
         rules_dict_list = dict_obj.get("rules", [])
+        for val in rules_dict_list:
+            val['name'] = val['name'].strip()
+            val['request_url'] = val['request_url'].strip()
         o.rules = []
         for rule_dict in rules_dict_list:
             new_rule = rule.Rule(rule_dict['name'])

poppy/model/helpers/restriction.py

@@ -42,7 +42,7 @@ class Restriction(common.DictSerializableModel):
 
     @name.setter
     def name(self, value):
-        self._name = value
+        self._name = value.strip()
 
     @property
     def access(self):
@@ -87,8 +87,14 @@ class Restriction(common.DictSerializableModel):
 
         access = dict_obj.get("access", 'whitelist')
         o = cls("unnamed", access)
-        o.name = dict_obj.get("name", "unnamed")
+        o.name = dict_obj.get("name", "unnamed").strip()
         rules_dict_list = dict_obj.get("rules", [])
+        for val in rules_dict_list:
+            val['name'] = val['name'].strip()
+            if 'referrer' in val:
+                val['referrer'] = val['referrer'].strip()
+            elif 'geography' in val:
+                val['geography'] = val['geography'].strip()
         o.rules = []
         for rule_dict in rules_dict_list:
             new_rule = rule.Rule(rule_dict['name'])

poppy/model/helpers/rule.py

@@ -28,13 +28,13 @@ class Rule(common.DictSerializableModel):
         self._request_url = request_url
 
         if referrer:
-            self._referrer = referrer
+            self._referrer = referrer.strip()
         if http_host:
-            self._http_host = http_host
+            self._http_host = http_host.strip()
         if client_ip:
-            self._client_ip = client_ip
+            self._client_ip = client_ip.strip()
         if http_method:
-            self._http_method = http_method
+            self._http_method = http_method.strip()
         if geography:
             # Validate the geography should be in a list of supported
             # countries
@@ -46,7 +46,7 @@ class Rule(common.DictSerializableModel):
                                  'geo zones' % geography))
             self._geography = geography
         if request_url:
-            self._request_url = request_url
+            self._request_url = request_url.strip()
 
     @property
     def name(self):

poppy/model/service.py

@@ -213,7 +213,6 @@ class Service(common.DictSerializableModel):
         o = cls(service_id=uuid.uuid4(), name='unnamed',
                 domains=[], origins=[], flavor_id='unnamed',
                 project_id=project_id)
-
         domains = input_dict.get('domains', [])
         input_dict['domains'] = [domain.Domain.init_from_dict(d)
                                  for d in domains]
@@ -241,7 +240,6 @@ class Service(common.DictSerializableModel):
             log_delivery)
 
         o.from_dict(input_dict)
-
         return o
 
     def to_dict(self):
@@ -273,7 +271,6 @@ class Service(common.DictSerializableModel):
             new_provider_details[provider] = (
                 provider_details[provider].to_dict())
         result['provider_details'] = new_provider_details
-
         result['log_delivery'] = result['log_delivery'].to_dict()
 
         return result

poppy/provider/akamai/services.py

@@ -57,6 +57,23 @@ class ServiceController(base.ServiceBase):
                                'Accept': 'text/plain'}
         self.san_cert_hostname_limit = self.driver.san_cert_hostname_limit
 
+    def reorder_rules(self, post_data):
+        ordered_dict = {}
+        sorted_map = None
+        ordered_list = []
+        if post_data['rules']:
+            for k, v in enumerate(post_data['rules']):
+                if v['matches'][0]['value']:
+                    ordered_dict[k] = \
+                        len(v['matches'][0]['value'].split('/'))
+            sorted_map = sorted(ordered_dict.items(), key=lambda x: x[1],
+                                reverse=True)
+            for val in sorted_map:
+                if val[0] != 0:
+                    ordered_list.append(post_data['rules'][val[0]])
+            ordered_list.append(post_data['rules'][0])
+        return ordered_list
+
     def create(self, service_obj):
         try:
             post_data = {
@@ -79,7 +96,8 @@ class ServiceController(base.ServiceBase):
                 service_obj.caching, post_data['rules'])
             self._process_restriction_rules(
                 service_obj.restrictions, post_data['rules'])
-
+            # Change the order of the caching rules
+            post_data['rules'] = self.reorder_rules(post_data)
             classified_domains = self._classify_domains(service_obj.domains)
 
             # NOTE(tonytan4ever): for akamai it might be possible to have
@@ -254,6 +272,7 @@ class ServiceController(base.ServiceBase):
                 self._process_cache_invalidation_rules(
                     invalidate_url, policy_content['rules'])
             # Update domain if necessary (by adjust digital property)
+            policy_content['rules'] = self.reorder_rules(policy_content)
             classified_domains = self._classify_domains(
                 service_obj.domains)
 

tests/unit/model/test_service.py

@@ -132,16 +132,46 @@ class TestServiceModel(base.TestCase):
                          [origin.to_dict() for origin
                           in myservice.origins])
 
-        # (amitgandhinz) need to add caching and restrictions
+        self.assertEqual([caching.to_dict()['rules'] for caching
-        # self.assertEqual([caching.to_dict() for caching
+                          in cloned_service.caching],
-        #                   in cloned_service.caching],
+                         [caching.to_dict()['rules'] for caching
-        #                  [caching.to_dict() for caching
+                          in myservice.caching])
-        #                   in myservice.caching])
 
-        # self.assertEqual([restrictions.to_dict() for restrictions
+        self.assertEqual([caching.to_dict()['name'] for caching
-        #                   in cloned_service.restrictions],
+                          in cloned_service.caching],
-        #                  [restrictions.to_dict() for restrictions
+                         [caching.to_dict()['name'] for caching
-        #                   in myservice.restrictions])
+                          in myservice.caching])
+
+        self.assertEqual([caching.to_dict()['ttl'] for caching
+                          in cloned_service.caching],
+                         [caching.to_dict()['ttl'] for caching
+                          in myservice.caching])
+
+        self.assertEqual([restrictions.to_dict()['rules'] for restrictions
+                          in cloned_service.restrictions],
+                         [restrictions.to_dict()['rules'] for restrictions
+                          in myservice.restrictions])
+
+        self.assertEqual([restrictions.to_dict()['name'] for restrictions
+                          in cloned_service.restrictions],
+                         [restrictions.to_dict()['name'] for restrictions
+                          in myservice.restrictions])
+
+    def test_init_from_dict_whitespaces(self):
+        caching = [cachingrule.CachingRule('images ', 3600)]
+        restrictions = [restriction.Restriction('client_ip ', 'whitelist')]
+        name = 'tests '
+        myservice = service.Service(
+            self.service_id,
+            name, self.mydomains, self.myorigins, self.flavor_id,
+            caching, restrictions)
+        cloned_service = service.Service.init_from_dict(self.project_id,
+                                                        myservice.to_dict())
+        self.assertNotEqual(cloned_service.name, myservice.name.strip())
+        self.assertNotEqual([x.name for x in cloned_service.caching],
+                            [y.name for y in myservice.caching])
+        self.assertNotEqual([x.name for x in cloned_service.restrictions],
+                            [y.name for y in myservice.restrictions])
 
     @ddt.data(u'', u'apple')
     def test_set_invalid_status(self, status):

tests/unit/provider/akamai/caching_rules.json

@@ -0,0 +1,458 @@
+{"test_reorder_rules":
+{ 
+    "input" :
+            {
+   "rules": [
+      {
+         "matches": [
+            {
+               "name": "url-wildcard",
+               "value": "/*"
+            }
+         ],
+         "behaviors": [
+            {
+               "params": {
+                  "cacheKeyType": "digital_property",
+                  "cacheKeyValue": "-",
+                  "hostHeaderType": "digital_property",
+                  "originDomain": "www.obulpathi.com",
+                  "hostHeaderValue": "-"
+               },
+               "name": "origin",
+               "value": "-"
+            },
+            {
+               "type": "no-store",
+               "name": "caching",
+               "value": "0s"
+            },
+            {
+               "name": "referer-whitelist",
+               "value": "*www.mocksite.com*"
+            },
+            {
+               "type": "country",
+               "name": "geo-whitelist",
+               "value": "US"
+            }
+         ]
+      },
+      {
+         "matches": [
+            {
+               "name": "url-wildcard",
+               "value": "/img"
+            }
+         ],
+         "behaviors": [
+            {
+               "params": {
+                  "cacheKeyType": "digital_property",
+                  "cacheKeyValue": "-",
+                  "hostHeaderType": "digital_property",
+                  "originDomain": "www.gandhi.co.nz",
+                  "hostHeaderValue": "-"
+               },
+               "name": "origin",
+               "value": "-"
+            }
+         ]
+      },
+      {
+         "matches": [
+            {
+               "name": "url-wildcard",
+               "value": "/favicon.ico"
+            }
+         ],
+         "behaviors": [
+            {
+               "type": "fixed",
+               "name": "caching",
+               "value": "86400s"
+            }
+         ]
+      },
+      {
+         "matches": [
+            {
+               "name": "url-wildcard",
+               "value": "/areas/trinseo/scripts/*"
+            }
+         ],
+         "behaviors": [
+            {
+               "type": "fixed",
+               "name": "caching",
+               "value": "86400s"
+            }
+         ]
+      },
+      {
+         "matches": [
+            {
+               "name": "url-wildcard",
+               "value": "/areas/trinseo/image/*"
+            }
+         ],
+         "behaviors": [
+            {
+               "type": "fixed",
+               "name": "caching",
+               "value": "86400s"
+            }
+         ]
+      },
+      {
+         "matches": [
+            {
+               "name": "url-wildcard",
+               "value": "/areas/trinseo/images/*"
+            }
+         ],
+         "behaviors": [
+            {
+               "type": "fixed",
+               "name": "caching",
+               "value": "86400s"
+            }
+         ]
+      },
+      {
+         "matches": [
+            {
+               "name": "url-wildcard",
+               "value": "/areas/trinseo/styles/*"
+            }
+         ],
+         "behaviors": [
+            {
+               "type": "fixed",
+               "name": "caching",
+               "value": "86400s"
+            }
+         ]
+      },
+      {
+         "matches": [
+            {
+               "name": "url-wildcard",
+               "value": "/bundles/wffm/*"
+            }
+         ],
+         "behaviors": [
+            {
+               "type": "fixed",
+               "name": "caching",
+               "value": "86400s"
+            }
+         ]
+      },
+      {
+         "matches": [
+            {
+               "name": "url-wildcard",
+               "value": "/content/wffm/*"
+            }
+         ],
+         "behaviors": [
+            {
+               "type": "fixed",
+               "name": "caching",
+               "value": "86400s"
+            }
+         ]
+      },
+      {
+         "matches": [
+            {
+               "name": "url-wildcard",
+               "value": "/Areas/Trinseo/image/*"
+            }
+         ],
+         "behaviors": [
+            {
+               "type": "fixed",
+               "name": "caching",
+               "value": "86400s"
+            }
+         ]
+      },
+      {
+         "matches": [
+            {
+               "name": "url-wildcard",
+               "value": "/Areas/Trinseo/images/*"
+            }
+         ],
+         "behaviors": [
+            {
+               "type": "fixed",
+               "name": "caching",
+               "value": "86400s"
+            }
+         ]
+      },
+      {
+         "matches": [
+            {
+               "name": "url-wildcard",
+               "value": "/Areas/Trinseo/scripts/*"
+            }
+         ],
+         "behaviors": [
+            {
+               "type": "fixed",
+               "name": "caching",
+               "value": "86400s"
+            }
+         ]
+      },
+      {
+         "matches": [
+            {
+               "name": "url-wildcard",
+               "value": "/Areas/Trinseo/styles/*"
+            }
+         ],
+         "behaviors": [
+            {
+               "type": "fixed",
+               "name": "caching",
+               "value": "86400s"
+            }
+         ]
+      }
+   ]
+},
+
+"output" :
+            {
+   "rules": [
+      {
+         "matches": [
+            {
+               "name": "url-wildcard",
+               "value": "/areas/trinseo/scripts/*"
+            }
+         ],
+         "behaviors": [
+            {
+               "type": "fixed",
+               "name": "caching",
+               "value": "86400s"
+            }
+         ]
+      },
+      {
+         "matches": [
+            {
+               "name": "url-wildcard",
+               "value": "/areas/trinseo/image/*"
+            }
+         ],
+         "behaviors": [
+            {
+               "type": "fixed",
+               "name": "caching",
+               "value": "86400s"
+            }
+         ]
+      },
+      {
+         "matches": [
+            {
+               "name": "url-wildcard",
+               "value": "/areas/trinseo/images/*"
+            }
+         ],
+         "behaviors": [
+            {
+               "type": "fixed",
+               "name": "caching",
+               "value": "86400s"
+            }
+         ]
+      },
+      {
+         "matches": [
+            {
+               "name": "url-wildcard",
+               "value": "/areas/trinseo/styles/*"
+            }
+         ],
+         "behaviors": [
+            {
+               "type": "fixed",
+               "name": "caching",
+               "value": "86400s"
+            }
+         ]
+      },
+      {
+         "matches": [
+            {
+               "name": "url-wildcard",
+               "value": "/Areas/Trinseo/image/*"
+            }
+         ],
+         "behaviors": [
+            {
+               "type": "fixed",
+               "name": "caching",
+               "value": "86400s"
+            }
+         ]
+      },
+      {
+         "matches": [
+            {
+               "name": "url-wildcard",
+               "value": "/Areas/Trinseo/images/*"
+            }
+         ],
+         "behaviors": [
+            {
+               "type": "fixed",
+               "name": "caching",
+               "value": "86400s"
+            }
+         ]
+      },
+      {
+         "matches": [
+            {
+               "name": "url-wildcard",
+               "value": "/Areas/Trinseo/scripts/*"
+            }
+         ],
+         "behaviors": [
+            {
+               "type": "fixed",
+               "name": "caching",
+               "value": "86400s"
+            }
+         ]
+      },
+      {
+         "matches": [
+            {
+               "name": "url-wildcard",
+               "value": "/Areas/Trinseo/styles/*"
+            }
+         ],
+         "behaviors": [
+            {
+               "type": "fixed",
+               "name": "caching",
+               "value": "86400s"
+            }
+         ]
+      },
+      {
+         "matches": [
+            {
+               "name": "url-wildcard",
+               "value": "/bundles/wffm/*"
+            }
+         ],
+         "behaviors": [
+            {
+               "type": "fixed",
+               "name": "caching",
+               "value": "86400s"
+            }
+         ]
+      },
+      {
+         "matches": [
+            {
+               "name": "url-wildcard",
+               "value": "/content/wffm/*"
+            }
+         ],
+         "behaviors": [
+            {
+               "type": "fixed",
+               "name": "caching",
+               "value": "86400s"
+            }
+         ]
+      },
+      {
+         "matches": [
+            {
+               "name": "url-wildcard",
+               "value": "/img"
+            }
+         ],
+         "behaviors": [
+            {
+               "params": {
+                  "cacheKeyType": "digital_property",
+                  "cacheKeyValue": "-",
+                  "hostHeaderType": "digital_property",
+                  "originDomain": "www.gandhi.co.nz",
+                  "hostHeaderValue": "-"
+               },
+               "name": "origin",
+               "value": "-"
+            }
+         ]
+      },
+      {
+         "matches": [
+            {
+               "name": "url-wildcard",
+               "value": "/favicon.ico"
+            }
+         ],
+         "behaviors": [
+            {
+               "type": "fixed",
+               "name": "caching",
+               "value": "86400s"
+            }
+         ]
+      },
+      {
+         "matches": [
+            {
+               "name": "url-wildcard",
+               "value": "/*"
+            }
+         ],
+         "behaviors": [
+            {
+               "params": {
+                  "cacheKeyType": "digital_property",
+                  "cacheKeyValue": "-",
+                  "hostHeaderType": "digital_property",
+                  "originDomain": "www.obulpathi.com",
+                  "hostHeaderValue": "-"
+               },
+               "name": "origin",
+               "value": "-"
+            },
+            {
+               "type": "no-store",
+               "name": "caching",
+               "value": "0s"
+            },
+            {
+               "name": "referer-whitelist",
+               "value": "*www.mocksite.com*"
+            },
+            {
+               "type": "country",
+               "name": "geo-whitelist",
+               "value": "US"
+            }
+         ]
+      }
+   ]
+}
+
+}
+}

tests/unit/provider/akamai/test_services.py

@@ -782,3 +782,8 @@ class TestServices(base.TestCase):
     def test_current_customer(self):
         controller = services.ServiceController(self.driver)
         self.assertIsNone(controller.current_customer)
+
+    @ddt.file_data('caching_rules.json')
+    def test_reorder_rules(self, caching_rules_json):
+        res = self.controller.reorder_rules(caching_rules_json['input'])
+        self.assertEqual(res, caching_rules_json['output']['rules'])