Merge pull request #169 from wikimedia/phab/T88652-2

Revisions

config.example.yaml

@@ -106,9 +106,11 @@ spec:
   # Some more general RESTBase info
   paths:
     /{domain:en.wikipedia.org}: *wp/default/1.0.0
-#    /{domain:de.wikipedia.org}: *wp/default/1.0.0
-#    /{domain:es.wikipedia.org}: *wp/default/1.0.0
-#    /{domain:nl.wikipedia.org}: *wp/default/1.0.0
+#   /{domain:de.wikipedia.org}: *wp/default/1.0.0
+#   /{domain:es.wikipedia.org}: *wp/default/1.0.0
+#   /{domain:nl.wikipedia.org}: *wp/default/1.0.0
+#
+    # test domain
     /{domain:en.wikipedia.test.local}: *wp/default/1.0.0
 
 

mods/page_revisions.js

@@ -43,8 +43,10 @@ PRS.prototype.getTableSchema = function () {
             rev: 'int',             // MediaWiki oldid
             latest_rev: 'int',      // Latest MediaWiki revision
             tid: 'timeuuid',
+            // revision deletion or suppression, can be:
+            // - sha1hidden, commenthidden, texthidden
+            restrictions: 'set<string>',
             // Revision tags. Examples:
-            // - revision deletion or suppression
             // - minor revision
             tags: 'set<string>',
             // Page renames. null, to:destination or from:source
@@ -68,7 +70,7 @@ PRS.prototype.getTableSchema = function () {
                 { attribute: 'rev', type: 'hash' },
                 { attribute: 'tid', type: 'range', order: 'desc' },
                 { attribute: 'title', type: 'range', order: 'asc' },
-                { attribute: 'tags', type: 'proj' }
+                { attribute: 'restrictions', type: 'proj' }
             ]
         }
     };
@@ -108,7 +110,8 @@ PRS.prototype.fetchAndStoreMWRevision = function (restbase, req) {
             format: 'json',
             action: 'query',
             prop: 'revisions',
-            rvprop: 'ids|timestamp|user|userid|size|sha1|contentmodel|comment'
+            continue: '',
+            rvprop: 'ids|timestamp|user|userid|size|sha1|contentmodel|comment|tags'
         }
     };
     if (/^[0-9]+$/.test(rp.revision)) {
@@ -129,34 +132,59 @@ PRS.prototype.fetchAndStoreMWRevision = function (restbase, req) {
                 }
             });
         }
-        var apiRev = apiRes.body.items[0].revisions[0];
+        // the response item
+        var dataResp = apiRes.body.items[0];
+        // the revision info
+        var apiRev = dataResp.revisions[0];
+        // are there any restrictions set?
+        var restrictions = Object.keys(apiRev).filter(function(key) { return /hidden$/.test(key) });
+        // the tid to store this info under
         var tid = rbUtil.tidFromDate(apiRev.timestamp);
         return restbase.put({ // Save / update the revision entry
             uri: self.tableURI(rp.domain),
             body: {
                 table: self.tableName,
                 attributes: {
-                    title: rp.title,
+                    // FIXME: if a title has been given, check it
+                    // matches the one returned by the MW API
+                    // cf. https://phabricator.wikimedia.org/T87393
+                    title: dataResp.title,
                     rev: parseInt(apiRev.revid),
                     tid: tid,
                     user_id: apiRev.userid,
                     user_text: apiRev.user,
-                    comment: apiRev.comment
+                    comment: apiRev.comment,
+                    tags: apiRev.tags,
+                    restrictions: restrictions
                 }
             }
         })
         .then(function() {
             rp.revision = apiRev.revid + '';
+            rp.title = dataResp.title;
             return self.getTitleRevision(restbase, req);
         });
+    }).catch(function(e) {
+        // if a bad revision is supplied, the action module
+        // returns a 500 with the 'Missing query pages' message
+        // so catch that and turn it into a 404 in our case
+        if(e.status === 500 && /^Missing query pages/.test(e.description)) {
+            throw new rbUtil.HTTPError({
+                status: 404,
+                body: {
+                    type: 'not_found#page_revisions',
+                    description: 'Page or revision not found.',
+                    apiRequest: apiReq
+                }
+            });
+        }
+        throw e;
     });
 };
 
 PRS.prototype.getTitleRevision = function(restbase, req) {
     var self = this;
     var rp = req.params;
-
-
     var revisionRequest;
     if (/^[0-9]+$/.test(rp.revision)) {
         // Check the local db
@@ -214,6 +242,68 @@ PRS.prototype.listTitleRevisions = function(restbase, req) {
     });
 };
 
+// /rev/
+PRS.prototype.listRevisions = function(restbase, req) {
+    var rp = req.params;
+    var listReq = {
+        uri: this.tableURI(rp.domain),
+        body: {
+            table: this.tableName,
+            index: 'by_rev',
+            proj: ['rev'],
+            distinct: true
+        }
+    };
+    return restbase.get(listReq)
+    .then(function(res) {
+        if (res.status === 200) {
+            res.body.items = res.body.items.map(function(row) {
+                return row.rev;
+            });
+        }
+        return res;
+    });
+};
+
+PRS.prototype.getRevision = function(restbase, req) {
+    var rp = req.params;
+    var self = this;
+    // sanity check
+    if (!/^[0-9]+$/.test(rp.revision)) {
+        throw new rbUtil.HTTPError({
+            status: 400,
+            body: {
+                type: 'invalidRevision',
+                description: 'Invalid revision specified.'
+            }
+        });
+    }
+    if (req.headers && /no-cache/.test(req.headers['cache-control'])) {
+        // ask the MW API directly and
+        // store and return its result
+        return this.fetchAndStoreMWRevision(restbase, req);
+    }
+    // check the storage, and, if no match is found
+    // ask the MW API about the revision
+    return restbase.get({
+        uri: this.tableURI(rp.domain),
+        body: {
+            table: this.tableName,
+            index: 'by_rev',
+            attributes: {
+                rev: parseInt(rp.revision)
+            },
+            limit: 1
+        }
+    })
+    .catch(function(e) {
+        if (e.status !== 404) {
+            throw e;
+        }
+        return self.fetchAndStoreMWRevision(restbase, req);
+    });
+};
+
 module.exports = function(options) {
     var prs = new PRS(options);
     // XXX: add docs
@@ -224,6 +314,8 @@ module.exports = function(options) {
             listTitleRevisions: prs.listTitleRevisions.bind(prs),
             getTitleRevision: prs.getTitleRevision.bind(prs),
             //getTitleRevisionId: prs.getTitleRevisionId.bind(prs)
+            listRevisions: prs.listRevisions.bind(prs),
+            getRevision: prs.getRevision.bind(prs)
         },
         resources: [
             {

mods/page_revisions.yaml

@@ -27,12 +27,12 @@ paths:
 #      summary: Get the tid range for a given revision
 #      operationId: getTitleRevisionTidRange
 #  
-#  /rev/:
-#    get:
-#      summary: List all revisions, ever
-#      
-#  /rev/{revision}:
-#    get:
-#      summary: Get the revision metadata (incl. title) for a specific
-#        revision
-#      operationId: getRevision
+  /rev/:
+    get:
+      summary: List all revisions, ever
+      operationId: listRevisions
+
+  /rev/{revision}:
+    get:
+      summary: Get the revision metadata for a specific revision
+      operationId: getRevision

package.json

@@ -42,7 +42,7 @@
     "node-uuid": "git+https://github.com/gwicke/node-uuid#master",
     "preq": "~0.3.6",
     "request": "^2.44.0",
-    "restbase-mod-table-cassandra": "~0.4.1",
+    "restbase-mod-table-cassandra": "~0.4.3",
     "swagger-router": "^0.0.3",
     "swagger-ui": "^2.1.1-M1",
     "url-template": "2.0.4",

specs/mediawiki/v1/content.yaml

@@ -311,6 +311,15 @@ paths:
       description: List available revisions
       produces:
         - application/json
+      parameters:
+        - name: domain
+          in: path
+          description: The domain under which the data resides
+          type: string
+          required: true
+          default: en.wikipedia.org
+      x-backend-request:
+        uri: /{domain}/sys/page_revisions/rev/
 
   /{module:page}/revision/{revision}:
     get:
@@ -335,6 +344,10 @@ paths:
       responses:
         '200':
           description: The revision's properties
+        '400':
+          description: Invalid revision
+          schema:
+            $ref: '#/definitions/invalidRevision'
         '404':
           description: Unknown revision id or domain
           schema:
@@ -343,10 +356,8 @@ paths:
           description: Unexpected error
           schema:
             $ref: '#/definitions/defaultError'
-      # x-backend-request:
-      #   uri: /{domain}/sys/key_rev_value/{revision}
-      #   headers:
-      #     cache-control: $req.headers.cache-control
+      x-backend-request:
+        uri: /{domain}/sys/page_revisions/rev/{revision}
 
   /{module:transform}/html/to/wikitext{/title}{/revision}:
     post:

test/features/pagecontent/pagecontent.js

@@ -140,24 +140,17 @@ describe('item requests', function() {
 
 });
 
-describe('pagecontent bucket', function() {
+describe('page content hierarchy', function() {
     this.timeout(20000);
-    // TODO: figure out what we'd like to return for /page
-    //it('should provide bucket info', function() {
-    //    this.timeout(20000);
-    //    return preq.get({
-    //        uri: server.config.bucketURL,
-    //    })
-    //    .then(function(res) {
-    //        assert.deepEqual(res.status, 200);
-    //    });
-    //});
-    it('should list its contents', function() {
+    it('should list available properties', function() {
         return preq.get({
             uri: server.config.bucketURL + '/',
         })
         .then(function(res) {
             assert.deepEqual(res.status, 200);
+            if (!res.body.items || res.body.items.indexOf('html') === -1) {
+                throw new Error('Expected property listing that includes "html"');
+            }
         });
     });
 });

test/features/pagecontent/revisions.js

@@ -0,0 +1,68 @@
+'use strict';
+
+// mocha defines to avoid JSHint breakage
+/* global describe, it, before, beforeEach, after, afterEach */
+
+var assert = require('../../utils/assert.js');
+var preq   = require('preq');
+var server = require('../../utils/server.js');
+
+
+describe('revision requests', function() {
+
+    this.timeout(20000);
+
+    before(function () { return server.start(); });
+
+    it('should return valid revision info', function() {
+        return preq.get({ uri: server.config.bucketURL + '/revision/642497713' })
+        .then(function(res) {
+            assert.deepEqual(res.status, 200);
+            assert.deepEqual(res.body.items.length, 1);
+            assert.deepEqual(res.body.items[0].rev, 642497713);
+            assert.deepEqual(res.body.items[0].title, 'Foobar');
+        });
+    });
+
+    it('should query the MW API for revision info', function() {
+        var slice = server.config.logStream.slice();
+        return preq.get({
+            uri: server.config.bucketURL + '/revision/642497713',
+            headers: { 'cache-control': 'no-cache' }
+        })
+        .then(function(res) {
+            slice.halt();
+            assert.deepEqual(res.status, 200);
+            assert.deepEqual(res.body.items.length, 1);
+            assert.deepEqual(res.body.items[0].rev, 642497713);
+            assert.deepEqual(res.body.items[0].title, 'Foobar');
+            assert.remoteRequests(slice, true);
+        });
+    });
+
+    it('should fail for an invalid revision', function() {
+        return preq.get({ uri: server.config.bucketURL + '/revision/faultyrevid' })
+        .then(function(res) {
+            throw new Error('Expected status 400 for an invalid revision, got ' + res.status);
+        },
+        function(res) {
+            assert.deepEqual(res.status, 400);
+        });
+    });
+
+    it('should query the MW API for a non-existent revision and return a 404', function() {
+        var slice = server.config.logStream.slice();
+        return preq.get({ uri: server.config.bucketURL + '/revision/0' })
+        .then(function(res) {
+            slice.halt();
+            throw new Error('Expected status 404 for an invalid revision, got ' + res.status);
+        },
+        function(res) {
+            slice.halt();
+            assert.deepEqual(res.status, 404);
+            assert.remoteRequests(slice, true);
+        });
+    });
+
+});
+