Merge branch 'master' into 2939-orgs

Conflicts:
	ckan/logic/auth/get.py

dashboard authorizations FIXMEs amended otherwise non-issue

ckan/lib/cli.py

@@ -130,6 +130,15 @@ def command(self):
             if self.verbose:
                 print 'Initialising DB: SUCCESS'
         elif cmd == 'clean' or cmd == 'drop':
+
+            # remove any *.pyc version files to prevent conflicts
+            v_path = os.path.join(os.path.dirname(__file__),
+                               '..', 'migration', 'versions', '*.pyc')
+            import glob
+            filelist = glob.glob(v_path)
+            for f in filelist:
+                os.remove(f)
+
             model.repo.clean_db()
             search.clear()
             if self.verbose:

ckan/logic/action/get.py

@@ -1917,8 +1917,11 @@ def group_activity_list(context, data_dict):
 
     # Get the group's activities.
     query = model.Session.query(model.Activity)
-    query = query.filter(_or_(model.Activity.object_id == group_id,
-        model.Activity.object_id.in_(dataset_ids)))
+    if dataset_ids:
+        query = query.filter(_or_(model.Activity.object_id == group_id,
+            model.Activity.object_id.in_(dataset_ids)))
+    else:
+        query = query.filter(model.Activity.object_id == group_id)
     query = query.order_by(_desc(model.Activity.timestamp))
     query = query.limit(15)
     activity_objects = query.all()

ckan/logic/auth/get.py

@@ -202,18 +202,26 @@ def member_roles_list(context, data_dict):
 
 
 def dashboard_activity_list(context, data_dict):
-    if 'user' in context:
+    # FIXME: context['user'] could be an IP address but that case is not
+    # handled here. Maybe add an auth helper function like is_logged_in().
+    if context.get('user'):
         return {'success': True}
     else:
         return {'success': False,
                 'msg': _("You must be logged in to access your dashboard.")}
 
 
 def dashboard_new_activities_count(context, data_dict):
+    # FIXME: This should go through check_access() not call is_authorized()
+    # directly, but wait until 2939-orgs is merged before fixing this.
+    # This is so a better not authourized message can be sent.
     return new_authz.is_authorized('dashboard_activity_list',
             context, data_dict)
 
 
 def dashboard_mark_all_new_activities_as_old(context, data_dict):
+    # FIXME: This should go through check_access() not call is_authorized()
+    # directly, but wait until 2939-orgs is merged before fixing this.
+    # This is so a better not authourized message can be sent.
     return new_authz.is_authorized('dashboard_activity_list',
             context, data_dict)

ckan/tests/functional/api/test_dashboard.py

@@ -46,27 +46,32 @@ def setup_class(cls):
     def teardown_class(cls):
         ckan.model.repo.rebuild_db()
 
+    def post(self, action, params=None, apikey=None, status=200):
+        '''Post to the CKAN API and return the result.'''
+        if params is None:
+            params = {}
+        params = json.dumps(params)
+        response = self.app.post('/api/action/{0}'.format(action),
+                params=params,
+                extra_environ={'Authorization': str(apikey)},
+                status=status)
+        if status in (200,):
+            assert response.json['success'] is True
+            return response.json['result']
+        else:
+            assert response.json['success'] is False
+            return response.json['error']
+
     def dashboard_new_activities_count(self, user):
         '''Return the given user's new activities count from the CKAN API.'''
-        params = json.dumps({})
-        response = self.app.post('/api/action/dashboard_new_activities_count',
-                params=params,
-                extra_environ={'Authorization': str(user['apikey'])})
-        assert response.json['success'] is True
-        new_activities_count = response.json['result']
-        return new_activities_count
+        return self.post('dashboard_new_activities_count',
+                apikey=user['apikey'])
 
     def dashboard_activity_list(self, user):
         '''Return the given user's dashboard activity list from the CKAN API.
 
         '''
-        params = json.dumps({})
-        response = self.app.post('/api/action/dashboard_activity_list',
-                params=params,
-                extra_environ={'Authorization': str(user['apikey'])})
-        assert response.json['success'] is True
-        activity_list = response.json['result']
-        return activity_list
+        return self.post('dashboard_activity_list', apikey=user['apikey'])
 
     def dashboard_new_activities(self, user):
         '''Return the activities from the user's dashboard activity stream
@@ -75,12 +80,17 @@ def dashboard_new_activities(self, user):
         return [activity for activity in activity_list if activity['is_new']]
 
     def dashboard_mark_all_new_activities_as_old(self, user):
-        params = json.dumps({})
-        response = self.app.post(
-                '/api/action/dashboard_mark_all_new_activities_as_old',
-                params=params,
-                extra_environ={'Authorization': str(user['apikey'])})
-        assert response.json['success'] is True
+        self.post('dashboard_mark_all_new_activities_as_old',
+                apikey=user['apikey'])
+
+    def test_00_dashboard_activity_list_not_logged_in(self):
+        self.post('dashboard_activity_list', status=403)
+
+    def test_00_dashboard_new_activities_count_not_logged_in(self):
+        self.post('dashboard_new_activities_count', status=403)
+
+    def test_00_dashboard_mark_new_activities_not_logged_in(self):
+        self.post('dashboard_mark_all_new_activities_as_old', status=403)
 
     def test_01_new_activities_count_for_new_user(self):
         '''Test that a newly registered user's new activities count is 0.'''
@@ -196,10 +206,11 @@ def test_06_mark_new_activities_as_read(self):
     def test_07_maximum_number_of_new_activities(self):
         '''Test that the new activities count does not go higher than 15, even
         if there are more than 15 new activities from the user's followers.'''
-        for n in range(0,20):
+        for n in range(0, 20):
             notes = "Updated {n} times".format(n=n)
             params = json.dumps({'name': 'warandpeace', 'notes': notes})
-            response = self.app.post('/api/action/package_update', params=params,
+            response = self.app.post('/api/action/package_update',
+                params=params,
                 extra_environ={'Authorization': str(self.joeadmin['apikey'])})
             assert response.json['success'] is True
         assert self.dashboard_new_activities_count(self.new_user) == 15

ckan_deb/usr/lib/ckan/common.sh

@@ -155,7 +155,7 @@ ckan_ensure_db_exists () {
         COMMAND_OUTPUT=`sudo -u postgres psql -c "select datname from pg_database where datname='$INSTANCE'"`
         if ! [[ "$COMMAND_OUTPUT" =~ ${INSTANCE} ]] ; then
             echo "Creating the database ..."
-            sudo -u postgres createdb -O ${INSTANCE} ${INSTANCE}
+            sudo -u postgres createdb -O ${INSTANCE} ${INSTANCE} -E utf-8
             paster --plugin=ckan db init --config=/etc/ckan/${INSTANCE}/${INSTANCE}.ini
         fi
     fi

doc/architecture.rst

@@ -0,0 +1,206 @@
+======================
+CKAN Code Architecture
+======================
+
+This section tries to give some guidelines for writing code that is consistent
+with the intended, overall design and architecture of CKAN.
+
+
+Encapsulate SQLAlchemy in ``ckan.model``
+````````````````````````````````````````
+
+Ideally SQLAlchemy should only be used within ``ckan.model`` and not from other
+packages such as ``ckan.logic``.  For example instead of using an SQLAlchemy
+query from the logic package to retrieve a particular user from the database,
+we add a ``get()`` method to ``ckan.model.user.User``::
+
+    @classmethod
+    def get(cls, user_id):
+        query = ...
+        .
+        .
+        .
+        return query.first()
+
+Now we can call this method from the logic package.
+
+Database Migrations
+```````````````````
+
+When changes are made to the model classes in ``ckan.model`` that alter CKAN's
+database schema, a migration script has to be added to migrate old CKAN
+databases to the new database schema when they upgrade their copies of CKAN.
+See :doc:`migration`.
+
+Always go through the Action Functions
+``````````````````````````````````````
+
+Whenever some code, for example in ``ckan.lib`` or ``ckan.controllers``, wants
+to get, create, update or delete an object from CKAN's model it should do so by
+calling a function from the ``ckan.logic.action`` package, and *not* by
+accessing ``ckan.model`` directly.
+
+
+Action Functions are Exposed in the API
+```````````````````````````````````````
+
+The functions in ``ckan.logic.action`` are exposed to the world as the
+:doc:`apiv3`.  The API URL for an action function is automatically generated
+from the function name, for example
+``ckan.logic.action.create.package_create()`` is exposed at
+``/api/action/package_create``. See `Steve Yegge's Google platforms rant
+<https://plus.google.com/112678702228711889851/posts/eVeouesvaVX>`_ for some
+interesting discussion about APIs.
+
+**All** publicly visible functions in the
+``ckan.logic.action.{create,delete,get,update}`` namespaces will be exposed
+through the :doc:`apiv3`. **This includes functions imported** by those
+modules, **as well as any helper functions** defined within those modules.  To
+prevent inadvertent exposure of non-action functions through the action api,
+care should be taken to:
+
+1. Import modules correctly (see `Imports`_).  For example: ::
+
+     import ckan.lib.search as search
+
+     search.query_for(...)
+
+2. Hide any locally defined helper functions: ::
+
+     def _a_useful_helper_function(x, y, z):
+        '''This function is not exposed because it is marked as private```
+        return x+y+z
+
+3. Bring imported convenience functions into the module namespace as private
+   members: ::
+
+     _get_or_bust = logic.get_or_bust
+
+
+Use ``get_action()``
+````````````````
+
+Don't call ``logic.action`` functions directly, instead use ``get_action()``.
+This allows plugins to override action functions using the ``IActions`` plugin
+interface. For example::
+
+    ckan.logic.get_action('group_activity_list_html')(...)
+
+Instead of ::
+
+    ckan.logic.action.get.group_activity_list_html(...)
+
+
+Auth Functions and ``check_access()``
+``````````````
+
+Each action function defined in ``ckan.logic.action`` should use its own
+corresponding auth function defined in ``ckan.logic.auth``. Instead of calling
+its auth function directly, an action function should go through
+``ckan.logic.check_access`` (which is aliased ``_check_access`` in the action
+modules) because this allows plugins to override auth functions using the
+``IAuthFunctions`` plugin interface. For example::
+
+    def package_show(context, data_dict):
+        _check_access('package_show', context, data_dict)
+
+``check_access`` will raise an exception if the user is not authorized, which
+the action function should not catch. When this happens the user will be shown
+an authorization error in their browser (or will receive one in their response
+from the API).
+
+
+``logic.get_or_bust()``
+`````````````
+
+The ``data_dict`` parameter of logic action functions may be user provided, so
+required files may be invalid or absent. Naive Code like::
+
+  id = data_dict['id']
+
+may raise a ``KeyError`` and cause CKAN to crash with a 500 Server Error
+and no message to explain what went wrong. Instead do::
+
+  id = _get_or_bust(data_dict, "id")
+
+which will raise ``ValidationError`` if ``"id"`` is not in ``data_dict``. The
+``ValidationError`` will be caught and the user will get a 400 Bad Request
+response and an error message explaining the problem.
+
+
+Validation and ``ckan.logic.schema``
+````````````````````````````````````
+
+Logic action functions can use schema defined in ``ckan.logic.schema`` to
+validate the contents of the ``data_dict`` parameters that users pass to them.
+
+An action function should first check for a custom schema provided in the
+context, and failing that should retrieve its default schema directly, and
+then call ``_validate()`` to validate and convert the data. For example, here
+is the validation code from the ``user_create()`` action function::
+
+ schema = context.get('schema') or ckan.logic.schema.default_user_schema()
+ session = context['session']
+ validated_data_dict, errors = _validate(data_dict, schema, context)
+ if errors:
+     session.rollback()
+     raise ValidationError(errors)
+
+
+Controller & Template Helper Functions
+--------------------------------------
+
+``ckan.lib.helpers`` contains helper functions that can be used from
+``ckan.controllers`` or from templates. When developing for ckan core, only use
+the helper functions found in ``ckan.lib.helpers.__allowed_functions__``.
+
+
+.. _Testing:
+
+Testing
+-------
+
+- Functional tests which test the behaviour of the web user interface, and the
+  APIs should be placed within ``ckan/tests/functional``.  These tests can be a
+  lot slower to run that unit tests which don't access the database or solr.  So
+  try to bear that in mind, and attempt to cover just what is neccessary, leaving
+  what can be tested via unit-testing in unit-tests.
+
+- ``nose.tools.assert_in`` and ``nose.tools.assert_not_in`` are only available
+  in Python>=2.7.  So import them from ``ckan.tests``, which will provide
+  alternatives if they're not available.
+
+- the `mock`_ library can be used to create and interrogate mock objects.
+
+See :doc:`test` for further information on testing in CKAN.
+
+.. _mock: http://pypi.python.org/pypi/mock
+
+Writing Extensions
+------------------
+
+Please see :doc:`writing-extensions` for information about writing ckan
+extensions, including details on the API available to extensions.
+
+Deprecation
+-----------
+
+- Anything that may be used by extensions (see :doc:`writing-extensions`) needs
+  to maintain backward compatibility at call-site.  ie - template helper
+  functions and functions defined in the plugins toolkit.
+
+- The length of time of deprecation is evaluated on a function-by-function
+  basis.  At minimum, a function should be marked as deprecated during a point
+  release.
+
+- To mark a helper function, use the ``deprecated`` decorator found in
+  ``ckan.lib.maintain`` eg: ::
+
+    
+    @deprecated()
+    def facet_items(*args, **kwargs):
+        """
+        DEPRECATED: Use the new facet data structure, and `unselected_facet_items()`
+        """
+        # rest of function definition.
+