diff --git a/NEWS.md b/NEWS.md index e70b8df2..db11b9b5 100644 --- a/NEWS.md +++ b/NEWS.md @@ -3,7 +3,8 @@ ## 4.3.1 Released 2017-mm-dd -Announcements: +Bug fix: + - Fixed bug introduced in version 4.0.1 that brokes the static map generation using JPG as format ## 4.3.0 Released 2017-12-11 diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index 43dd9c3a..23c0f6cd 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -357,7 +357,9 @@ LayergroupController.prototype.center = function(req, res, next) { LayergroupController.prototype.staticMap = function(req, res, width, height, zoom /* bounds */, center, next) { var format = req.params.format === 'jpg' ? 'jpeg' : 'png'; - req.params.format = req.params.format || 'png'; + // We force always the tile to be generated using PNG because + // is the only format we support by now + res.locals.format = 'png'; res.locals.layer = res.locals.layer || 'all'; var self = this; @@ -507,4 +509,4 @@ function validateLayerRouteMiddleware(req, res, next) { } next(); -} \ No newline at end of file +} diff --git a/lib/cartodb/controllers/named_maps.js b/lib/cartodb/controllers/named_maps.js index a88c8592..cd2d120f 100644 --- a/lib/cartodb/controllers/named_maps.js +++ b/lib/cartodb/controllers/named_maps.js @@ -28,7 +28,7 @@ NamedMapsController.prototype.register = function(app) { userMiddleware, this.prepareContext, this.tile.bind(this), - vectorError() + vectorError() ); app.get( @@ -121,7 +121,9 @@ NamedMapsController.prototype.staticMap = function(req, res, next) { var cdbUser = res.locals.user; var format = req.params.format === 'jpg' ? 'jpeg' : 'png'; - res.locals.format = req.params.format || 'png'; + // We force always the tile to be generated using PNG because + // is the only format we support by now + res.locals.format = 'png'; res.locals.layer = res.locals.layer || 'all'; var namedMapProvider; diff --git a/test/acceptance/named_maps_static_view.js b/test/acceptance/named_maps_static_view.js index b7af4197..9844c667 100644 --- a/test/acceptance/named_maps_static_view.js +++ b/test/acceptance/named_maps_static_view.js @@ -19,7 +19,8 @@ describe('named maps static view', function() { var username = 'localhost'; var templateName = 'template_with_view'; - var IMAGE_TOLERANCE = 20; + var PNG_IMAGE_TOLERANCE = 20; + var JPG_IMAGE_TOLERANCE = 100; function createTemplate(view, layers) { return { @@ -92,8 +93,8 @@ describe('named maps static view', function() { }); } - function previewFixture(version) { - return './test/fixtures/previews/populated_places_simple_reduced-' + version + '.png'; + function previewFixture(version, format='png') { + return './test/fixtures/previews/populated_places_simple_reduced-' + version + '.' + format; } it('should return an image estimating its bounds based on dataset', function (done) { @@ -103,7 +104,7 @@ describe('named maps static view', function() { } getStaticMap(function(err, img) { assert.ok(!err); - assert.imageIsSimilarToFile(img, previewFixture('estimated'), IMAGE_TOLERANCE, done); + assert.imageIsSimilarToFile(img, previewFixture('estimated'), PNG_IMAGE_TOLERANCE, done); }); }); }); @@ -122,7 +123,7 @@ describe('named maps static view', function() { } getStaticMap(function(err, img) { assert.ok(!err); - assert.imageIsSimilarToFile(img, previewFixture('zoom-center'), IMAGE_TOLERANCE, done); + assert.imageIsSimilarToFile(img, previewFixture('zoom-center'), PNG_IMAGE_TOLERANCE, done); }); }); }); @@ -142,7 +143,7 @@ describe('named maps static view', function() { } getStaticMap(function(err, img) { assert.ok(!err); - assert.imageIsSimilarToFile(img, previewFixture('bounds'), IMAGE_TOLERANCE, done); + assert.imageIsSimilarToFile(img, previewFixture('bounds'), PNG_IMAGE_TOLERANCE, done); }); }); }); @@ -167,7 +168,7 @@ describe('named maps static view', function() { } getStaticMap(function(err, img) { assert.ok(!err); - assert.imageIsSimilarToFile(img, previewFixture('zoom-center'), IMAGE_TOLERANCE, done); + assert.imageIsSimilarToFile(img, previewFixture('zoom-center'), PNG_IMAGE_TOLERANCE, done); }); }); }); @@ -192,7 +193,7 @@ describe('named maps static view', function() { } getStaticMap({ zoom: 3 }, function(err, img) { assert.ok(!err); - assert.imageIsSimilarToFile(img, previewFixture('override-zoom'), IMAGE_TOLERANCE, done); + assert.imageIsSimilarToFile(img, previewFixture('override-zoom'), PNG_IMAGE_TOLERANCE, done); }); }); }); @@ -217,7 +218,7 @@ describe('named maps static view', function() { } getStaticMap({ bbox: '0,45,90,45' }, function(err, img) { assert.ok(!err); - assert.imageIsSimilarToFile(img, previewFixture('override-bbox'), IMAGE_TOLERANCE, done); + assert.imageIsSimilarToFile(img, previewFixture('override-bbox'), PNG_IMAGE_TOLERANCE, done); }); }); }); @@ -256,7 +257,27 @@ describe('named maps static view', function() { } getStaticMap({ layer: 0 }, function(err, img) { assert.ok(!err); - assert.imageIsSimilarToFile(img, previewFixture('bounds'), IMAGE_TOLERANCE, done); + assert.imageIsSimilarToFile(img, previewFixture('bounds'), PNG_IMAGE_TOLERANCE, done); + }); + }); + }); + + it('should return jpg static map', function (done) { + var view = { + zoom: 4, + center: { + lng: 40, + lat: 20 + } + }; + templateMaps.addTemplate(username, createTemplate(view), function (err) { + if (err) { + return done(err); + } + getStaticMap({ format: 'jpeg' }, function(err, img) { + assert.ok(!err); + assert.imageIsSimilarToFile(img, previewFixture('zoom-center', 'jpeg'), + JPG_IMAGE_TOLERANCE, done, 'jpeg'); }); }); }); diff --git a/test/fixtures/previews/populated_places_simple_reduced-zoom-center.jpeg b/test/fixtures/previews/populated_places_simple_reduced-zoom-center.jpeg new file mode 100644 index 00000000..e3f62673 Binary files /dev/null and b/test/fixtures/previews/populated_places_simple_reduced-zoom-center.jpeg differ diff --git a/test/results/jpeg/.gitignore b/test/results/jpeg/.gitignore new file mode 100644 index 00000000..c23ac972 --- /dev/null +++ b/test/results/jpeg/.gitignore @@ -0,0 +1,3 @@ +*.jpeg +*.jpg +*.js diff --git a/test/support/assert.js b/test/support/assert.js index bcb186aa..6815a63f 100644 --- a/test/support/assert.js +++ b/test/support/assert.js @@ -37,7 +37,7 @@ assert.imageBuffersAreSimilar = function(bufferA, bufferB, tolerance, callback) imagesAreSimilar(testImage, referenceImage, tolerance, callback); }; -assert.imageIsSimilarToFile = function(testImage, referenceImageRelativeFilePath, tolerance, callback) { +assert.imageIsSimilarToFile = function(testImage, referenceImageRelativeFilePath, tolerance, callback, format='png') { callback = callback || function(err) { assert.ifError(err); }; var referenceImageFilePath = path.resolve(referenceImageRelativeFilePath); @@ -46,19 +46,23 @@ assert.imageIsSimilarToFile = function(testImage, referenceImageRelativeFilePath imagesAreSimilar(testImage, referenceImage, tolerance, function(err) { if (err) { - var testImageFilePath = randomImagePath(); - testImage.save(testImageFilePath); + var testImageFilePath = randomImagePath(format); + testImage.save(testImageFilePath, format); } callback(err); - }); + }, format); }; -function imagesAreSimilar(testImage, referenceImage, tolerance, callback) { +function imagesAreSimilar(testImage, referenceImage, tolerance, callback, format='png') { if (testImage.width() !== referenceImage.width() || testImage.height() !== referenceImage.height()) { return callback(new Error('Images are not the same size')); } - var pixelsDifference = referenceImage.compare(testImage); + var options = {}; + if (format === 'jpeg') { + options.alpha = false; + } + var pixelsDifference = referenceImage.compare(testImage, options); var similarity = pixelsDifference / (referenceImage.width() * referenceImage.height()); var tolerancePerMil = (tolerance / 1000); @@ -73,8 +77,8 @@ function imagesAreSimilar(testImage, referenceImage, tolerance, callback) { } } -function randomImagePath() { - return path.resolve('test/results/png/image-test-' + Date.now() + '.png'); +function randomImagePath(format='png') { + return path.resolve('test/results/' + format + '/image-test-' + Date.now() + '.' + format); } assert.response = function(server, req, res, callback) {