Merge branch 'defect-191-modification-date'

ckan/lib/dictization/model_dictize.py

@@ -263,10 +263,12 @@ def package_to_api1(pkg, context):
     site_url = config.get('ckan.site_url', None)
     if site_url:
         dictized['ckan_url'] = '%s/dataset/%s' % (site_url, pkg.name)
-    dictized['metadata_modified'] = pkg.metadata_modified.isoformat() \
-        if pkg.metadata_modified else None
-    dictized['metadata_created'] = pkg.metadata_created.isoformat() \
-        if pkg.metadata_created else None
+    metadata_modified = pkg.metadata_modified
+    dictized['metadata_modified'] = metadata_modified.isoformat() \
+        if metadata_modified else None
+    metadata_created = pkg.metadata_created
+    dictized['metadata_created'] = metadata_created.isoformat() \
+        if metadata_created else None
 
     subjects = dictized.pop("relationships_as_subject") 
     objects = dictized.pop("relationships_as_object") 

ckan/model/group.py

@@ -12,7 +12,7 @@
 
 __all__ = ['group_table', 'Group', 'package_revision_table',
            'PackageGroup', 'GroupRevision', 'PackageGroupRevision',
-           'package_group_revision_table']
+           'package_group_table', 'package_group_revision_table']
 
 package_group_table = Table('package_group', metadata,
     Column('id', UnicodeText, primary_key=True, default=make_uuid),

ckan/model/package.py

@@ -457,17 +457,15 @@ def diff(self, to_revision=None, from_revision=None):
                         results[key] = value_diff
         return results
 
-    @staticmethod
-    def last_modified(*av):
+    @property
+    def metadata_modified(self):
         """
-        Return most recent timestamp for a package revision, with optionally
-        extra where clause.
+        Return most recent timestamp for revisions related to this package.
+        NB Excludes changes to the package's groups
         """
-        from ckan import model
-        where = []
-        for arg in av:
-            if isinstance(arg, expression.ClauseElement) or isinstance(arg, basestring):
-                where.append(arg)
+        import ckan.model as model
+
+        where = [model.package_table.c.id == self.id]
         where_clauses = [
             and_(model.package_table.c.revision_id == model.revision_table.c.id, *where),
             and_(model.package_extra_table.c.package_id == model.package_table.c.id,
@@ -484,31 +482,30 @@ def last_modified(*av):
             and_(model.package_tag_table.c.package_id == model.package_table.c.id,
                  model.package_tag_table.c.revision_id == model.revision_table.c.id, *where)
             ]
+
         query = union(*[select([model.revision_table.c.timestamp], x) for x in where_clauses]
-                      ).order_by("timestamp DESC").limit(1)
-        conn = model.meta.engine.connect()
+                      ).order_by('timestamp DESC').limit(1)
+        # Use current connection because we might be in a 'before_commit' of
+        # a SessionExtension - only by using the current connection can we get
+        # at the newly created revision etc. objects.
+        conn = model.Session.connection() 
         result = conn.execute(query).fetchone()
+
         if result:
             result_datetime = iso_date_to_datetime_for_sqlite(result[0])
-            timestamp = result_datetime.utctimetuple()
+            timestamp_without_usecs = result_datetime.utctimetuple()
             usecs = float(result_datetime.microsecond) / 1e6
-        else:
-            timestamp, usecs = gmtime(), 0
-        # use timegm instead of mktime, because we don't want it localised
-        return timegm(timestamp) + usecs
+            # use timegm instead of mktime, because we don't want it localised
+            timestamp_float = timegm(timestamp_without_usecs) + usecs
+            return datetime.datetime.utcfromtimestamp(timestamp_float)
 
-    @property
-    def metadata_modified(self):
-        import ckan.model as model
-        epochtime = self.last_modified(model.package_table.c.id==self.id)
-        return datetime.datetime.utcfromtimestamp(epochtime)
-
     @property
     def metadata_created(self):
         import ckan.model as model
         q = model.Session.query(model.PackageRevision)\
             .filter(model.PackageRevision.id == self.id)\
-            .order_by(model.PackageRevision.revision_timestamp.asc())
+            .order_by(model.PackageRevision.revision_timestamp.asc())\
+            .limit(1)
         ts = q.first()
         if ts is not None:
             return ts.revision_timestamp

ckan/tests/functional/api/test_action.py

@@ -1135,3 +1135,49 @@ def test_3_bad_param(self):
                             status=400)
         assert '"message": "Search Query is invalid:' in res.body, res.body
         assert '"Invalid search parameters: [u\'weird_param\']' in res.body, res.body
+
+    def test_4_sort_by_metadata_modified(self):
+        search_params = '%s=1' % json.dumps({
+            'q': '*:*',
+            'fl': 'name, metadata_modified',
+            'sort': u'metadata_modified desc'
+        })
+
+        # modify warandpeace, check that it is the first search result
+        rev = model.repo.new_revision()
+        pkg = model.Package.get('warandpeace')
+        pkg.title = "War and Peace [UPDATED]"
+        model.repo.commit_and_remove()
+
+        res = self.app.post('/api/action/package_search', params=search_params)
+        result = json.loads(res.body)['result']
+        result_names = [r['name'] for r in result['results']]
+        assert result_names == ['warandpeace', 'annakarenina'], result_names
+
+        # modify annakarenina, check that it is the first search result
+        rev = model.repo.new_revision()
+        pkg = model.Package.get('annakarenina')
+        pkg.title = "A Novel By Tolstoy [UPDATED]"
+        model.repo.commit_and_remove()
+
+        res = self.app.post('/api/action/package_search', params=search_params)
+        result = json.loads(res.body)['result']
+        result_names = [r['name'] for r in result['results']]
+        assert result_names == ['annakarenina', 'warandpeace'], result_names
+
+        # add a tag to warandpeace, check that it is the first result
+        pkg = model.Package.get('warandpeace')
+        pkg_params = '%s=1' % json.dumps({'id': pkg.id})
+        res = self.app.post('/api/action/package_show', params=pkg_params)
+        pkg_dict = json.loads(res.body)['result']
+        pkg_dict['tags'].append({'name': 'new-tag'})
+        pkg_params = '%s=1' % json.dumps(pkg_dict)
+        res = self.app.post('/api/action/package_update', params=pkg_params,
+                            extra_environ={'Authorization': 'tester'})
+
+        res = self.app.post('/api/action/package_search', params=search_params)
+        result = json.loads(res.body)['result']
+        result_names = [r['name'] for r in result['results']]
+        assert result_names == ['warandpeace', 'annakarenina'], result_names
+
+

ckan/tests/models/test_package.py

@@ -128,7 +128,7 @@ def test_metadata_created_and_modified(self):
         assert package.metadata_modified == created_timestamp,\
             (package.metadata_modified, created_timestamp)
 
-        # update the package it
+        # update the package
         rev = model.repo.new_revision()
         package = model.Package.by_name(name)
         package.title = "test_metadata_new_title"
@@ -138,6 +138,59 @@ def test_metadata_created_and_modified(self):
         package = model.Package.by_name(name)
         assert package.metadata_created == created_timestamp
         assert package.metadata_modified == modified_timestamp
+        last_modified_timestamp = modified_timestamp
+
+        # update a package's tag
+        rev = model.repo.new_revision()
+        package = model.Package.by_name(name)
+        package.add_tag_by_name('new-tag')
+        modified_timestamp = model.Session().revision.timestamp
+        assert modified_timestamp != last_modified_timestamp
+        model.repo.commit_and_remove()
+
+        package = model.Package.by_name(name)
+        assert package.metadata_created == created_timestamp
+        assert package.metadata_modified == modified_timestamp
+        last_modified_timestamp = modified_timestamp
+
+        # update a package's extra
+        rev = model.repo.new_revision()
+        package = model.Package.by_name(name)
+        package.extras['new-key'] = 'value'
+        modified_timestamp = model.Session().revision.timestamp
+        assert modified_timestamp != last_modified_timestamp
+        model.repo.commit_and_remove()
+
+        package = model.Package.by_name(name)
+        assert package.metadata_created == created_timestamp
+        assert package.metadata_modified == modified_timestamp
+        last_modified_timestamp = modified_timestamp
+
+        # update a package's relationship
+        rev = model.repo.new_revision()
+        package = model.Package.by_name(name)
+        anna = model.Package.by_name(u'annakarenina')
+        package.add_relationship(u'child_of', anna)
+        modified_timestamp = model.Session().revision.timestamp
+        assert modified_timestamp != last_modified_timestamp
+        model.repo.commit_and_remove()
+
+        package = model.Package.by_name(name)
+        assert package.metadata_created == created_timestamp
+        assert package.metadata_modified == modified_timestamp
+        last_modified_timestamp = modified_timestamp
+
+        # update a package's group - NB no change this time
+        rev = model.repo.new_revision()
+        group = model.Group.by_name('roger')
+        group.add_package_by_name(name)
+        modified_timestamp = model.Session().revision.timestamp
+        assert modified_timestamp != last_modified_timestamp
+        model.repo.commit_and_remove()
+
+        package = model.Package.by_name(name)
+        assert package.metadata_created == created_timestamp
+        assert package.metadata_modified == last_modified_timestamp # no change
 
 
 class TestPackageWithTags:

doc/apiv3.rst

@@ -365,7 +365,7 @@ These parameters are all the standard SOLR syntax (in contrast to the syntax use
 |                       |               || fl=*                            |                                  |
 +-----------------------+---------------+----------------------------------+----------------------------------+
 | sort                  | field name,   || sort=name asc                   | Changes the sort order according |
-|                       | asc / dec     |                                  | to the field and direction given.|
+|                       | asc / dec     || sort=metadata_modified asc      | to the field and direction given.|
 |                       |               |                                  | default: score desc, name asc    |
 +-----------------------+---------------+----------------------------------+----------------------------------+
 | start, rows           | result-int    | start=40&rows=20             | Pagination options. Start is the |
@@ -426,6 +426,7 @@ These parameters are all the standard SOLR syntax (in contrast to the syntax use
 |                       |               |                                                     | included in the results.         |
 +-----------------------+---------------+-----------------------------------------------------+----------------------------------+
 
+
 Status Codes
 ~~~~~~~~~~~~