Merge branch '1051-code-cleanups-for-2.2'

bin/ckan_spam.py

@@ -14,7 +14,7 @@
 
 import os
 
-from sqlobject import *
+import sqlobject
 
 import loadconfig
 path = os.path.abspath(cfg_path)
@@ -36,7 +36,7 @@ def purge_packages_by_name():
     pkg = model.Package.byName(pkg_name)
     # for efficiency reasons best to have revisions in descending order
     sel = model.PackageRevision.select(
-            AND(model.PackageRevision.q.baseID==pkg.id,
+            sqlobject.AND(model.PackageRevision.q.baseID==pkg.id,
                 model.PackageRevision.q.revisionID>=start_at_id),
             orderBy=-model.PackageRevision.q.revisionID,
             )

ckan/lib/navl/__init__.py

@@ -1 +1 @@
-__licence__ = 'MIT'
+__license__ = 'MIT'

ckan/lib/package_saver.py

@@ -1,9 +1,9 @@
-from sqlalchemy import orm
 import ckan.lib.helpers as h
-from ckan.lib.base import *
+import ckan.lib.base as base
+import ckan.model as model
 import ckan.rating
-from pylons import g
-from ckan.lib.dictization import table_dictize
+
+from ckan.common import g, c, _
 
 # Todo: Factor out unused original_name argument.
 
@@ -72,14 +72,14 @@ def _update(cls, fs, log_message, author, client=None):
         fs.validate()
         validates = not (errors or fs.errors)
         if not validates:
-            raise ValidationException(fs)
+            raise base.ValidationException(fs)
         # sync
         try:
             rev = model.repo.new_revision()
             rev.author = author
             rev.message = log_message
             fs.sync()
-        except Exception, inst:
+        except Exception:
             model.Session.rollback()
             raise
         else:

ckan/logic/action/create.py

@@ -325,10 +325,12 @@ def related_create(context, data_dict):
     activity_create_context = {
         'model': model,
         'user': user,
-        'defer_commit':True,
+        'defer_commit': True,
+        'ignore_auth': True,
         'session': session
     }
-    activity_create(activity_create_context, activity_dict, ignore_auth=True)
+    logic.get_action('activity_create')(activity_create_context,
+                                        activity_dict)
     session.commit()
 
     context["related"] = related
@@ -548,11 +550,12 @@ def _group_or_org_create(context, data_dict, is_org=False):
     activity_create_context = {
         'model': model,
         'user': user,
-        'defer_commit':True,
+        'defer_commit': True,
+        'ignore_auth': True,
         'session': session
     }
     logic.get_action('activity_create')(activity_create_context,
-            activity_dict, ignore_auth=True)
+            activity_dict)
 
     if not context.get('defer_commit'):
         model.repo.commit()
@@ -798,6 +801,7 @@ def user_create(context, data_dict):
         'model': model,
         'user': context['user'],
         'defer_commit': True,
+        'ignore_auth': True,
         'session': session
     }
     activity_dict = {
@@ -806,7 +810,7 @@ def user_create(context, data_dict):
             'activity_type': 'new user',
             }
     logic.get_action('activity_create')(activity_create_context,
-            activity_dict, ignore_auth=True)
+            activity_dict)
 
     if not context.get('defer_commit'):
         model.repo.commit()
@@ -895,7 +899,7 @@ def vocabulary_create(context, data_dict):
 
     return model_dictize.vocabulary_dictize(vocabulary, context)
 
-def activity_create(context, activity_dict, ignore_auth=False):
+def activity_create(context, activity_dict, **kw):
     '''Create a new activity stream activity.
 
     You must be a sysadmin to create new activities.
@@ -917,6 +921,14 @@ def activity_create(context, activity_dict, ignore_auth=False):
     :rtype: dictionary
 
     '''
+
+    # this action had a ignore_auth param which has been removed
+    # removed in 2.2
+    if 'ignore_auth' in kw:
+        raise Exception('Activity Stream calling parameters have changed '
+                        'ignore_auth must be passed in the context not as '
+                        'a param')
+
     if not paste.deploy.converters.asbool(
             config.get('ckan.activity_streams_enabled', 'true')):
         return
@@ -930,8 +942,7 @@ def activity_create(context, activity_dict, ignore_auth=False):
     else:
         activity_dict['revision_id'] = None
 
-    if not ignore_auth:
-        _check_access('activity_create', context, activity_dict)
+    _check_access('activity_create', context, activity_dict)
 
     schema = context.get('schema') or ckan.logic.schema.default_create_activity_schema()
     data, errors = _validate(activity_dict, schema, context)
@@ -958,7 +969,7 @@ def package_relationship_create_rest(context, data_dict):
     relationship_dict = _get_action('package_relationship_create')(context, data_dict)
     return relationship_dict
 
-def tag_create(context, tag_dict):
+def tag_create(context, data_dict):
     '''Create a new vocabulary tag.
 
     You must be a sysadmin to create vocabulary tags.
@@ -981,14 +992,14 @@ def tag_create(context, tag_dict):
     '''
     model = context['model']
 
-    _check_access('tag_create', context, tag_dict)
+    _check_access('tag_create', context, data_dict)
 
     schema = context.get('schema') or ckan.logic.schema.default_create_tag_schema()
-    data, errors = _validate(tag_dict, schema, context)
+    data, errors = _validate(data_dict, schema, context)
     if errors:
         raise ValidationError(errors)
 
-    tag = model_save.tag_dict_save(tag_dict, context)
+    tag = model_save.tag_dict_save(data_dict, context)
 
     if not context.get('defer_commit'):
         model.repo.commit()

ckan/logic/action/delete.py

@@ -151,11 +151,12 @@ def related_delete(context, data_dict):
     activity_create_context = {
         'model': model,
         'user': user,
-        'defer_commit':True,
+        'defer_commit': True,
+        'ignore_auth': True,
         'session': session
     }
 
-    _get_action('activity_create')(activity_create_context, activity_dict, ignore_auth=True)
+    _get_action('activity_create')(activity_create_context, activity_dict)
     session.commit()
 
     entity.delete()

ckan/logic/action/update.py

@@ -167,12 +167,12 @@ def related_update(context, data_dict):
     activity_create_context = {
         'model': model,
         'user': context['user'],
-        'defer_commit':True,
+        'defer_commit': True,
+        'ignore_auth': True,
         'session': session
     }
 
-    _get_action('activity_create')(activity_create_context, activity_dict,
-                                   ignore_auth=True)
+    _get_action('activity_create')(activity_create_context, activity_dict)
 
     if not context.get('defer_commit'):
         model.repo.commit()
@@ -515,11 +515,11 @@ def _group_or_org_update(context, data_dict, is_org=False):
         activity_create_context = {
             'model': model,
             'user': user,
-            'defer_commit':True,
+            'defer_commit': True,
+            'ignore_auth': True,
             'session': session
         }
-        _get_action('activity_create')(activity_create_context, activity_dict,
-                ignore_auth=True)
+        _get_action('activity_create')(activity_create_context, activity_dict)
         # TODO: Also create an activity detail recording what exactly changed
         # in the group.
 
@@ -606,10 +606,11 @@ def user_update(context, data_dict):
     activity_create_context = {
         'model': model,
         'user': user,
-        'defer_commit':True,
+        'defer_commit': True,
+        'ignore_auth': True,
         'session': session
     }
-    _get_action('activity_create')(activity_create_context, activity_dict, ignore_auth=True)
+    _get_action('activity_create')(activity_create_context, activity_dict)
     # TODO: Also create an activity detail recording what exactly changed in
     # the user.
 

ckan/tests/functional/api/test_revision_search.py

@@ -29,14 +29,14 @@ def test_12_search_revision_since_rev(self):
         offset = self.offset('/search/revision')
         revs = model.Session.query(model.Revision).all()
         rev_first = revs[-1]
-        params = "?since_id=%s" % str(rev_first.id)
+        params = "?since_id=%s" % rev_first.id
         res = self.app.get(offset+params, status=200)
         res_list = self.data_from_res(res)
         assert rev_first.id not in res_list
         for rev in revs[:-1]:
             assert rev.id in res_list, (rev.id, res_list)
         rev_last = revs[0]
-        params = "?since_id=%s" % str(rev_last.id)
+        params = "?since_id=%s" % rev_last.id
         res = self.app.get(offset+params, status=200)
         res_list = self.data_from_res(res)
         assert res_list == [], res_list

ckan/tests/functional/test_pagination.py

@@ -45,6 +45,7 @@ def setup_class(cls):
         packages = []
         for i in range(cls.num_packages_in_large_group):
             packages.append({
+                # CS: nasty_string ignore
                 'name': u'dataset_%s' % str(i).zfill(2),
                 'groups': u'group_00'
             })
@@ -86,6 +87,7 @@ def setup_class(cls):
         # create enough of each here so that we can test pagination
         cls.num_groups = 22
 
+        # CS: nasty_string ignore
         groups = [u'group_%s' % str(i).zfill(2) for i in range(0, cls.num_groups)]
 
         CreateTestData.create_arbitrary(
@@ -118,6 +120,7 @@ def setup_class(cls):
         # create enough of each here so that we can test pagination
         cls.num_users = 21
 
+        # CS: nasty_string ignore
         users = [u'user_%s' % str(i).zfill(2) for i in range(cls.num_users)]
 
         CreateTestData.create_arbitrary(

ckan/tests/models/test_package_relationships.py

@@ -146,7 +146,7 @@ def teardown_class(self):
 
     def test_rels(self):
         rels = model.Package.by_name(u'homer').relationships
-        assert len(rels) == 5, '%i: %s' % (len(rels), [str(rel) for rel in rels])
+        assert len(rels) == 5, '%i: %s' % (len(rels), [rel for rel in rels])
         def check(rels, subject, type, object):
             for rel in rels:
                 if rel.subject.name == subject and rel.type == type and rel.object.name == object:
@@ -201,7 +201,7 @@ def teardown_class(self):
     def test_01_rels(self):
         "audit the simpsons family relationships"
         rels = model.Package.by_name(u'homer').get_relationships()
-        assert len(rels) == 5, '%i: %s' % (len(rels), [str(rel) for rel in rels])
+        assert len(rels) == 5, '%i: %s' % (len(rels), [rel for rel in rels])
         def check(rels, subject, type, object):
             for rel in rels:
                 if rel.subject.name == subject and rel.type == type and rel.object.name == object:

ckan/tests/test_coding_standards.py

@@ -99,9 +99,7 @@ def cs_filter(f, filter_, ignore_comment_lines=True):
 
 class TestBadSpellings(object):
 
-    BAD_SPELLING_BLACKLIST_FILES = [
-        'ckan/lib/navl/__init__.py',
-    ]
+    BAD_SPELLING_BLACKLIST_FILES = []
 
     # these are the bad spellings with the correct spelling
     # use LOWER case
@@ -173,11 +171,8 @@ class TestNastyString(object):
     # The value is converted to a string anyway so the str() is unneeded in
     # any place.
 
-    NASTY_STR_BLACKLIST_FILES = [
-        'ckan/tests/functional/api/test_revision_search.py',
-        'ckan/tests/functional/test_pagination.py',
-        'ckan/tests/models/test_package_relationships.py',
-    ]
+    NASTY_STR_BLACKLIST_FILES = []
+
     fails = {}
     passes = []
     done = False
@@ -243,7 +238,6 @@ class TestImportFromCkan(object):
         'ckan/lib/authenticator.py',
         'ckan/lib/base.py',
         'ckan/lib/munge.py',
-        'ckan/lib/package_saver.py',
         'ckan/lib/plugins.py',
         'ckan/lib/search/index.py',
         'ckan/lib/search/query.py',
@@ -341,7 +335,6 @@ class TestImportFromCkan(object):
         'ckanext/multilingual/plugin.py',
         'ckanext/reclinepreview/tests/test_preview.py',
         'ckanext/stats/controller.py',
-        'ckanext/stats/stats.py',
         'ckanext/stats/tests/__init__.py',
         'ckanext/stats/tests/test_stats_lib.py',
         'ckanext/stats/tests/test_stats_plugin.py',
@@ -400,9 +393,7 @@ class TestImportStar(object):
     # import * is bad for many reasons and should be avoided.
 
     IMPORT_STAR_BLACKLIST_FILES = [
-        'bin/ckan_spam.py',
         'ckan/lib/helpers.py',
-        'ckan/lib/package_saver.py',
         'ckan/migration/versions/001_add_existing_tables.py',
         'ckan/migration/versions/002_add_author_and_maintainer.py',
         'ckan/migration/versions/003_add_user_object.py',
@@ -495,7 +486,6 @@ class TestImportStar(object):
         'ckan/tests/pylons_controller.py',
         'ckan/tests/test_dumper.py',
         'ckan/tests/test_wsgi_ckanclient.py',
-        'ckanext/stats/stats.py',
         'fabfile.py',
     ]
     fails = {}
@@ -920,7 +910,6 @@ class TestActionAuth(object):
 
     ACTION_FN_SIGNATURES_BLACKLIST = [
         'create: activity_create',
-        'create: tag_create',
     ]
 
     ACTION_NO_AUTH_BLACKLIST = [

ckanext/stats/stats.py

@@ -1,10 +1,10 @@
 import datetime
 
 from pylons import config
-from sqlalchemy import *
+from sqlalchemy import Table, select, func, and_
 
 import ckan.plugins as p
-from ckan import model
+import ckan.model as model
 
 cache_enabled = p.toolkit.asbool(config.get('ckanext.stats.cache_enabled', 'True'))