Mobrovac has submitted this change and it was merged.

Change subject: Handle SVGO errors
......................................................................


Handle SVGO errors

In general, I wanted to extend the test converage of mathoid
along the lines of
https://github.com/physikerwelt/texvcinfo/commits/master
That's why I put some extra effort to test the compression
feature.

Bug: T130710
Change-Id: Ic0746221ad7343018df95174e74270992028e62a
---
M package.json
M routes/mathoid.js
A test/features/math/svgo.js
3 files changed, 84 insertions(+), 16 deletions(-)

Approvals:
  Mobrovac: Looks good to me, approved
  jenkins-bot: Verified



diff --git a/package.json b/package.json
index bc07651..77b0433 100644
--- a/package.json
+++ b/package.json
@@ -50,21 +50,24 @@
     "svgo": "^0.6.0"
   },
   "devDependencies": {
+    "commander": "^2.9.0",
+    "dom-compare": "^0.2.1",
     "extend": "^3.0.0",
     "istanbul": "^0.4.3",
     "mocha": "^2.4.5",
     "mocha-jshint": "^2.3.1",
     "mocha-lcov-reporter": "^1.2.0",
+    "rewire": "^2.5.1",
     "swagger-router": "^0.4.2",
-    "commander": "^2.9.0",
-    "xmldom": "^0.1.21",
-    "dom-compare":"^0.2.1"
+    "xmldom": "^0.1.21"
   },
   "deploy": {
     "node": "4.3.0",
     "target": "debian",
     "dependencies": {
-      "_all": ["librsvg2-dev"]
+      "_all": [
+        "librsvg2-dev"
+      ]
     }
   }
 }
diff --git a/routes/mathoid.js b/routes/mathoid.js
index a47ceb9..590b3cb 100644
--- a/routes/mathoid.js
+++ b/routes/mathoid.js
@@ -4,12 +4,12 @@
 var sUtil = require('../lib/util');
 var texvcInfo = require('texvcinfo');
 var HTTPError = sUtil.HTTPError;
-var SVGO = require( 'svgo' );
-var svgo = new SVGO( {
+var SVGO = require('svgo');
+var svgo = new SVGO({
     plugins: [
-        { convertTransform: false }
+        {convertTransform: false}
     ]
-} );
+});
 
 
 /**
@@ -24,7 +24,7 @@
 
 
 /* The response headers for different render types */
-var outHeaders = function(data) {
+var outHeaders = function (data) {
     return {
         svg: {
             'content-type': 'image/svg+xml'
@@ -59,7 +59,23 @@
         format + ": true\" to enable " + format + "rendering.");
 }
 
-function handleRequest(res, q, type, outFormat, features) {
+var optimizeSvg = function (data, req, cb) {
+    try {
+        svgo.optimize(data.svg, function (result) {
+            if (!result.error) {
+                data.svg = result.data;
+            } else {
+                req.logger.log('warn/svgo', result.error);
+            }
+            cb();
+        });
+    } catch (e) {
+        req.logger.log('warn/svgo', e);
+        cb();
+    }
+};
+
+function handleRequest(res, q, type, outFormat, features, req) {
     var sanitizedTex, feedback;
     var svg = app.conf.svg && /^svg|json|complete$/.test(outFormat);
     var mml = (type !== "MathML") && /^mml|json|complete$/.test(outFormat);
@@ -102,7 +118,7 @@
         speakText: speech,
         png: png
     };
-    if ( app.conf.dpi ){
+    if (app.conf.dpi) {
         mathJaxOptions.dpi = app.conf.dpi;
     }
     app.mjAPI.typeset(mathJaxOptions, function (data) {
@@ -148,10 +164,7 @@
         }
 
         if (data.svg) {
-            svgo.optimize( data.svg, function ( result ) {
-                data.svg = result.data;
-                outputResponse();
-            } );
+            optimizeSvg(data, req, outputResponse);
         } else {
             outputResponse();
         }
@@ -244,7 +257,7 @@
     } else {
         outFormat = "json";
     }
-    handleRequest(res, q, type, outFormat, {speech: speech});
+    handleRequest(res, q, type, outFormat, {speech: speech}, req);
 
 });
 
diff --git a/test/features/math/svgo.js b/test/features/math/svgo.js
new file mode 100644
index 0000000..e8b39b7
--- /dev/null
+++ b/test/features/math/svgo.js
@@ -0,0 +1,52 @@
+'use strict';
+var rewire = require('rewire');
+var mathoid = rewire('../../../routes/mathoid');
+//To test with invalid files as well, we need to gain direct access the 
optimize SVG method.
+var optimize = mathoid.__get__('optimizeSvg');
+var assert = require('../../utils/assert.js');
+
+
+var testcases = [
+    {in: '<svg>', err: 'Unclosed root tag'},
+    {in: 'invalid', err: 'Error in parsing SVG'},
+    {
+        in: '<?xml version=\"1.0\"?>\r\n    <svg 
xmlns=\"http:\/\/www.w3.org\/2000\/svg\" width=\"200\" height=\"100\">\r\n      
  <text font-size=\"68\" font-weight=\"bold\" font-family=\"DejaVu Sans\"\r\n   
 y=\"52\" x=\"4\" transform=\"scale(.8,1.7)\"><tspan 
fill=\"#248\">W3<\/tspan>C<\/text>\r\n        <path fill=\"none\" 
stroke=\"#490\" stroke-width=\"12\" d=\"m138 66 20 20 30-74\"\/>\r\n        
<\/svg>',
+        title: 'compress from 
https://en.wikipedia.org/wiki/File:W3C_valid.svg',
+        contains: 'W3'
+    }
+];
+describe('Mathoids SVG compression', function () {
+    testcases.forEach(function (t) {
+        it(t.title || ('hanlde ' + t.err), function (done) {
+            var err = false;
+            var msg = false;
+            var fakeReq = {
+                logger: {
+                    log: function (m, e) {
+                        msg = m;
+                        if (e instanceof Error) {
+                            err = e.message;
+                        } else {
+                            err = e;
+                        }
+                    }
+                }
+            };
+            var data = {svg: t.in};
+            optimize(data, fakeReq, function () {
+                if (t.err) {
+                    assert.deepEqual(msg, 'warn/svgo');
+                    assert.ok(err.indexOf(t.err) > 0, err.message + ' should 
contain ' + t.err);
+                } else {
+                    assert.deepEqual(msg, false);
+                    assert.deepEqual(err, false);
+                    assert.ok(data.svg.length <= t.in.length, 'Compression 
increased the file size');
+                    if (t.contains) {
+                        assert.ok(data.svg.indexOf(t.contains) >= 0, 
'compressed svg should contain', t.contains);
+                    }
+                }
+                done();
+            });
+        });
+    });
+});

-- 
To view, visit https://gerrit.wikimedia.org/r/288148
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Ic0746221ad7343018df95174e74270992028e62a
Gerrit-PatchSet: 5
Gerrit-Project: mediawiki/services/mathoid
Gerrit-Branch: master
Gerrit-Owner: Physikerwelt <[email protected]>
Gerrit-Reviewer: Mobrovac <[email protected]>
Gerrit-Reviewer: Physikerwelt <[email protected]>
Gerrit-Reviewer: jenkins-bot <>

_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to