remove mutable default values for kwargs (#2668)

This subtle misunderstanding of Python tends to cause confusing bugs.

Suppose you have a definition statement like 'def function(keys=[]): ...'
This def statement is evaluated once, at import time, to create a function
object. It is at this time that '[]' is evaluated, and it is that ONE
list object which is retained as the default value for 'keys'.
Python doesn't store the expression and re-evaluate it every time the
default is needed, it evaluates it once at import time and the resulting
value is passed for every call where that arg is not given.

To explain this with code, the effect is something like this:

    default_keys = []
    def function(keys=default_keys):
        ...

When function() is called with a value for keys, no problem will be seen.
Even when it is called with no value for keys, and we default to that list,
you might not see a problem. The problem that tends to happen unpredictably
is that the mutable default value is mutated. Those changes are persistent
and globally visible to any call of the function which uses the default.

Then the default value is non-empty. Or information leaks across calls
that shouldn't be leaking. Or the same dict keeps growing forever.

To avoid this, we can use a sentinel value such as None, and then check
for that value in the body of the function. It is a cheap safeguard against
situations that can be very annoying.

geonode/api/api.py

@@ -99,7 +99,9 @@ class TypeFilteredResource(ModelResource):
 
     count = fields.IntegerField()
 
-    def build_filters(self, filters={}):
+    def build_filters(self, filters=None):
+        if filters is None:
+            filters = {}
         self.type_filter = None
         self.title_filter = None
 
@@ -114,7 +116,9 @@ def build_filters(self, filters={}):
 
         return orm_filters
 
-    def serialize(self, request, data, format, options={}):
+    def serialize(self, request, data, format, options=None):
+        if options is None:
+            options = {}
         options['title_filter'] = getattr(self, 'title_filter', None)
         options['type_filter'] = getattr(self, 'type_filter', None)
         options['user'] = request.user
@@ -125,7 +129,9 @@ def serialize(self, request, data, format, options={}):
 class TagResource(TypeFilteredResource):
     """Tags api"""
 
-    def serialize(self, request, data, format, options={}):
+    def serialize(self, request, data, format, options=None):
+        if options is None:
+            options = {}
         options['count_type'] = 'keywords'
 
         return super(TagResource, self).serialize(request, data, format, options)
@@ -143,7 +149,9 @@ class Meta:
 class RegionResource(TypeFilteredResource):
     """Regions api"""
 
-    def serialize(self, request, data, format, options={}):
+    def serialize(self, request, data, format, options=None):
+        if options is None:
+            options = {}
         options['count_type'] = 'regions'
 
         return super(RegionResource, self).serialize(request, data, format, options)
@@ -163,7 +171,9 @@ class Meta:
 class TopicCategoryResource(TypeFilteredResource):
     """Category api"""
 
-    def serialize(self, request, data, format, options={}):
+    def serialize(self, request, data, format, options=None):
+        if options is None:
+            options = {}
         options['count_type'] = 'category'
 
         return super(TopicCategoryResource, self).serialize(request, data, format, options)
@@ -216,8 +226,10 @@ class ProfileResource(TypeFilteredResource):
     current_user = fields.BooleanField(default=False)
     activity_stream_url = fields.CharField(null=True)
 
-    def build_filters(self, filters={}):
+    def build_filters(self, filters=None):
         """adds filtering by group functionality"""
+        if filters is None:
+            filters = {}
 
         orm_filters = super(ProfileResource, self).build_filters(filters)
 
@@ -292,7 +304,9 @@ def prepend_urls(self):
         else:
             return []
 
-    def serialize(self, request, data, format, options={}):
+    def serialize(self, request, data, format, options=None):
+        if options is None:
+            options = {}
         options['count_type'] = 'owner'
 
         return super(ProfileResource, self).serialize(request, data, format, options)
@@ -314,7 +328,9 @@ class Meta:
 class OwnersResource(TypeFilteredResource):
     """Owners api, lighter and faster version of the profiles api"""
 
-    def serialize(self, request, data, format, options={}):
+    def serialize(self, request, data, format, options=None):
+        if options is None:
+            options = {}
         options['count_type'] = 'owner'
 
         return super(OwnersResource, self).serialize(request, data, format, options)

geonode/api/resourcebase_api.py

@@ -84,7 +84,9 @@ class CommonModelApi(ModelResource):
         full=True)
     owner = fields.ToOneField(ProfileResource, 'owner', full=True)
 
-    def build_filters(self, filters={}):
+    def build_filters(self, filters=None):
+        if filters is None:
+            filters = {}
         orm_filters = super(CommonModelApi, self).build_filters(filters)
         if 'type__in' in filters and filters[
                 'type__in'] in FILTER_TYPES.keys():

geonode/layers/utils.py

@@ -370,12 +370,17 @@ def extract_tarfile(upload_file, extension='.shp', tempdir=None):
 
 
 def file_upload(filename, name=None, user=None, title=None, abstract=None,
-                keywords=[], category=None, regions=[], date=None,
+                keywords=None, category=None, regions=None, date=None,
                 skip=True, overwrite=False, charset='UTF-8',
                 metadata_uploaded_preserve=False):
     """Saves a layer in GeoNode asking as little information as possible.
        Only filename is required, user and title are optional.
     """
+    if keywords is None:
+        keywords = []
+    if regions is None:
+        regions = []
+
     # Get a valid user
     theuser = get_valid_user(user)
 

geonode/services/views.py

@@ -947,10 +947,12 @@ def _process_arcgis_service(arcserver, name, owner=None, parent=None):
     return return_dict
 
 
-def _process_arcgis_folder(folder, name, services=[], owner=None, parent=None):
+def _process_arcgis_folder(folder, name, services=None, owner=None, parent=None):
     """
     Iterate through folders and services in an ArcGIS REST service folder
     """
+    if services is None:
+        services = []
     for service in folder.services:
         return_dict = {}
         if not isinstance(service, ArcMapService):

geonode/upload/views.py

@@ -120,9 +120,11 @@ class JSONResponse(HttpResponse):
 
     def __init__(self,
                  obj='',
-                 json_opts={},
+                 json_opts=None,
                  content_type="application/json", *args, **kwargs):
 
+        if json_opts is None:
+            json_opts = {}
         content = json.dumps(obj, **json_opts)
         super(JSONResponse, self).__init__(content, content_type, *args, **kwargs)