From 37de3b485f2aa4e608eab20a1e631ccb6e12fdfe Mon Sep 17 00:00:00 2001 From: Justin Walgran Date: Tue, 10 Oct 2017 16:52:21 -0700 Subject: [PATCH 1/3] Add instance configuration middleware to support unit conversion Both mobile apps and the web app make tile requests. The most efficient way to implement unit conversion support for all of these platforms is to add code to the tiler. All tile requests are associated with an instance, so we will always be able to fetch the config object from the instance row. I am implementing this lookup as middleware to ensure that the config is retireved occurs before windshaft handles the request. I had initially tried adding the lookup to `req2params` but this generated exceptions. Just inserting a `setTimeout` into `req2params` produced the same exceptions, so it looks like windshaft is expecting this method to be synchronous. It is inefficient to look up the configuration for every tile request but: - In my testing the query timings printed to the console in "debug mode" maxed out at 250ms and went as low as 5ms. - This O(1) query will has the same cost regardless of the number of trees in an instance or, because of the index on `id`, the number of instances in the DB. - Avoiding the need to invalidate a cache keeps the implementation as simple as possible. --- http/middleware.js | 30 ++++++++++++++++++++++++++++++ http/windshaftServer.js | 13 ++++++++++++- 2 files changed, 42 insertions(+), 1 deletion(-) create mode 100644 http/middleware.js diff --git a/http/middleware.js b/http/middleware.js new file mode 100644 index 0000000..1b9bf41 --- /dev/null +++ b/http/middleware.js @@ -0,0 +1,30 @@ +module.exports = { + instanceConfig: function(opts) { + return function (req, res, next) { + var start = Date.now(); + req.instanceConfig = {}; + opts.dbPool.connect(function(err, client, done) { + if (!err) { + var instanceId = parseInt(req.query.instance_id, 10); + client.query('SELECT config FROM treemap_instance WHERE id = $1', + [instanceId], function(err, result) { + if (!err && result && result.rows && result.rows.length > 0) { + req.instanceConfig = JSON.parse(result.rows[0].config); + } + done(); + if (opts.debug) { + console.log('[instanceConfig] query time ' + (Date.now() - start)); + } + next(err); + }); + } else { + done(); + if (opts.debug) { + console.log('[instanceConfig] query time ' + (Date.now() - start)); + } + next(err); + } + }); + }; + } +}; diff --git a/http/windshaftServer.js b/http/windshaftServer.js index 509c9a7..2d6a4f9 100644 --- a/http/windshaftServer.js +++ b/http/windshaftServer.js @@ -6,13 +6,22 @@ var debug = require('debug')('windshaft:server'); var express = require('express'); var RedisPool = require('redis-mpool'); +var Pg = require('pg'); var _ = require('underscore'); var mapnik = require('mapnik'); var windshaft = require('windshaft'); var MapController = require('./mapController.js'); - +var middleware = require('./middleware.js'); + +var dbPool = new Pg.Pool({ + user: process.env.OTM_DB_USER || 'otm', + password: process.env.OTM_DB_PASSWORD || 'otm', + host: process.env.OTM_DB_HOST || 'localhost', + port: process.env.OTM_DB_PORT || 5432, + database: process.env.OTM_DB_NAME || 'otm' +}); // // @param opts server options object. Example value: // { @@ -74,6 +83,8 @@ module.exports = function(opts) { var app = bootstrap(opts); addFilters(app, opts); + app.use(middleware.instanceConfig({dbPool: dbPool})); + var redisPool = makeRedisPool(opts.redis); var map_store = new windshaft.storage.MapStore({ From 4a2718af6967a4f9d80374dd705b94f4bc6e0e86 Mon Sep 17 00:00:00 2001 From: Justin Walgran Date: Wed, 11 Oct 2017 10:29:20 -0700 Subject: [PATCH 2/3] Add filter value unit conversion This code is based on `units.py` in otm-core. https://github.com/OpenTreeMap/otm-core/blob/83e32dad3c420db3a94fade7d4fd4e017283023d/opentreemap/treemap/units.py Both mobile apps and the web app make tile requests. The most efficient way to implement unit conversion support for all of these platforms is to add code to the tiler. --- makeSql.js | 7 ++-- server.js | 3 +- test/testUnits.js | 70 ++++++++++++++++++++++++++++++++++ units.js | 95 +++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 171 insertions(+), 4 deletions(-) create mode 100644 test/testUnits.js create mode 100644 units.js diff --git a/makeSql.js b/makeSql.js index 6bab25d..e73a341 100644 --- a/makeSql.js +++ b/makeSql.js @@ -32,6 +32,7 @@ var displayFiltersToWhere = require('./displayFiltersToWhere'); var filtersToTables = require('./filtersToTables'); var addDefaultsToFilter = require('./addDefaultsToFilter'); var config = require('./config'); +var units = require('./units'); var utils = require('./filterObjectUtils'); @@ -39,15 +40,15 @@ var utils = require('./filterObjectUtils'); // Assumes that instanceid is an integer, ready to be plugged // directly into SQL function makeSqlForMapFeatures(filterString, displayString, restrictFeatureString, instanceid, - zoom, isUtfGridRequest, isPolygonRequest) { + zoom, isUtfGridRequest, isPolygonRequest, instanceConfig) { var geom_spec = config.sqlForMapFeatures.fields.geom, geom_field = isPolygonRequest ? geom_spec.polygon : geom_spec.point, parsedFilterObject = filterString ? JSON.parse(filterString) : {}, displayFilters = displayString ? JSON.parse(displayString) : undefined, restrictFeatureFilters = restrictFeatureString ? JSON.parse(restrictFeatureString) : undefined, - filterObject = addDefaultsToFilter(parsedFilterObject, zoom, isPolygonRequest), - + filterObjectWithDefaults = addDefaultsToFilter(parsedFilterObject, zoom, isPolygonRequest), + filterObject = units.convertFilterUnits(filterObjectWithDefaults, instanceConfig), tables = filtersToTables(filterObject, displayFilters, isPolygonRequest, isUtfGridRequest), where = '', diff --git a/server.js b/server.js index 52cc263..c7e5d78 100644 --- a/server.js +++ b/server.js @@ -107,7 +107,8 @@ var windshaftConfig = { instanceid, zoom, isUtfGridRequest, - isPolygonRequest); + isPolygonRequest, + req.instanceConfig); if (isPolygonRequest) { req.params.style = styles.polygonalMapFeature; } else if (isUtfGridRequest) { diff --git a/test/testUnits.js b/test/testUnits.js new file mode 100644 index 0000000..8bfb879 --- /dev/null +++ b/test/testUnits.js @@ -0,0 +1,70 @@ +"use strict"; + +var assert = require("assert"); +var _ = require("underscore"); +var units = require("../units"); +var config = require("../config"); + +describe('convertFilterUnits', function() { + var round = function(number, precision) { + var factor = Math.pow(10, precision); + var tempNumber = number * factor; + var roundedTempNumber = Math.round(tempNumber); + return roundedTempNumber / factor; + }; + + it('exists', function() { + assert.ok(_.isFunction(units.convertFilterUnits), + 'convertFilterUnits function should exist'); + }); + + it('ignores non-convertible fields', function(){ + var filter = {'tree.something': {'IS': 'some value'}}; + var config = {value_display: {tree: {diameter: {'units': 'in'}}}}; + var expectedFilter = _.clone(filter); + units.convertFilterUnits(filter, config); + assert.deepEqual(filter, expectedFilter, 'Filter should not change'); + }); + + it('does not change a value with default unit', function(){ + var filter = {'tree.diameter': {'MIN': 1, 'MAX': 2}}; + var config = {value_display: {tree: {diameter: {'units': 'in'}}}}; + var expectedFilter = _.clone(filter); + units.convertFilterUnits(filter, config); + assert.deepEqual(filter, expectedFilter, 'Filter should not change'); + }); + + it('converts min and max diameter filter', function(){ + var filter = {'tree.diameter': {'MIN': 1, 'MAX': 2}}; + var config = {value_display: {tree: {diameter: {'units': 'cm'}}}}; + var expectedFilter = {'tree.diameter': {'MIN': 0.393701, 'MAX': 0.787402}}; + units.convertFilterUnits(filter, config); + // Round before asserting because the actual conversion to compare floats with a known level of precision + filter['tree.diameter'].MIN = round(filter['tree.diameter'].MIN, 6); + filter['tree.diameter'].MAX = round(filter['tree.diameter'].MAX, 6); + assert.deepEqual(filter, expectedFilter, 'Filter should show cm->in conversion'); + }); + + it('converts min and max filter with alternate syntax', function(){ + var filter = {'tree.diameter': {'MIN': {'VALUE': 1}, 'MAX': {'VALUE': 2}}}; + var config = {value_display: {tree: {diameter: {'units': 'cm'}}}}; + var expectedFilter = {'tree.diameter': {'MIN': {'VALUE': 0.393701}, 'MAX': {'VALUE': 0.787402}}}; + units.convertFilterUnits(filter, config); + // Round before asserting because the actual conversion to compare floats with a known level of precision + filter['tree.diameter'].MIN.VALUE = round(filter['tree.diameter'].MIN.VALUE, 6); + filter['tree.diameter'].MAX.VALUE = round(filter['tree.diameter'].MAX.VALUE, 6); + assert.deepEqual(filter, expectedFilter, 'Filter should show cm->in conversion'); + }); + + it('converts min and max bioswale filter', function(){ + var filter = {'bioswale.drainage_area': {'MIN': 1, 'MAX': 2}}; + var config = {value_display: {bioswale: {drainage_area: {'units': 'sq_m'}}}}; + var expectedFilter = {'bioswale.drainage_area': {'MIN': 10.7643, 'MAX': 21.5285}}; + units.convertFilterUnits(filter, config); + // Round before asserting because the actual conversion to compare floats with a known level of precision + filter['bioswale.drainage_area'].MIN = round(filter['bioswale.drainage_area'].MIN, 4); + filter['bioswale.drainage_area'].MAX = round(filter['bioswale.drainage_area'].MAX, 4); + assert.deepEqual(filter, expectedFilter, 'Filter should show sq_ft->sq_m conversion'); + }); + +}); diff --git a/units.js b/units.js new file mode 100644 index 0000000..7ebdae6 --- /dev/null +++ b/units.js @@ -0,0 +1,95 @@ +"use strict"; + +var _ = require("underscore"); + +var convertableFields = ['tree.diameter', 'tree.height', 'tree.canopy_height', + 'plot.width', 'plot.length', 'bioswale.drainage_area', + 'rainBarrel.capacity', 'rainGarden.drainage_area']; + +var unitDefaults = { + plot: { + width: 'in', + length: 'in' + }, + tree: { + diameter: 'in', + height: 'ft', + canopy_height: 'ft' + }, + bioswale: { + drainage_area: 'sq_ft' + }, + rainBarrel: { + capacity: 'gal' + }, + rainGarden: { + drainage_area: 'sq_ft' + } +}; + +var unitConversions = { + 'in': {'in': 1, 'ft': 1 / 12, 'cm': 2.54, 'm': 0.0254}, + 'ft': {'in': 12, 'ft': 1, 'cm': 30.48, 'm': 0.3048}, + 'lbs/year': {'lbs/year': 1, 'kg/year': 0.453592}, + 'lbs': {'lbs': 1, 'kg': 0.453592}, + 'gal': {'gal': 1, 'L': 3.785}, + 'gal/year': {'gal/year': 1, 'L/year': 3.785}, + 'kwh/year': {'kwh/year': 1}, + 'sq_m': {'sq_m': 1, 'sq_ft': 10.7639}, + 'sq_ft': {'sq_m': 0.0929, 'sq_ft': 1} +}; + +function getFilterFactor(instanceConfig, model, field) { + var unit, defaultUnit, + factor = 1; + if (instanceConfig.value_display[model]) { + if (instanceConfig.value_display[model][field]) { + unit = instanceConfig.value_display[model][field].units; + defaultUnit = unitDefaults[model][field]; + factor = 1 / unitConversions[defaultUnit][unit]; + } + } + return factor; +} + +function convertFilterValue(value, factor) { + if (_.isObject(value)) { + _.each(['MIN', 'MAX', 'IS'], function(k) { + var floatValue; + if (value[k]) { + if (_.isObject(value[k])) { + floatValue = parseFloat(value[k].VALUE); + if (_.isNumber(floatValue)) { + value[k].VALUE = floatValue * factor; + } + } else { + floatValue = parseFloat(value[k]); + if (_.isNumber(floatValue)) { + value[k] = floatValue * factor; + } + } + } + }); + } + return value; +} + +function convertFilterUnits(filterObject, instanceConfig) { + // if there is no unit configuration, there is no need to convert + if (instanceConfig && instanceConfig.value_display) { + _.each(_.keys(filterObject), function(fieldName) { + var value = filterObject[fieldName]; + if (_.contains(convertableFields, fieldName)) { + var model = fieldName.split('.')[0], + field = fieldName.substring(fieldName.indexOf('.') + 1), + factor = getFilterFactor(instanceConfig, model, field); + convertFilterValue(value, factor); + } + }); + } + return filterObject; +} + +exports = module.exports = { + convertFilterUnits: convertFilterUnits +}; From 82b31d038dfea88bd5e9b283e3b8e7d70d1a5767 Mon Sep 17 00:00:00 2001 From: Justin Walgran Date: Mon, 16 Oct 2017 12:30:41 -0700 Subject: [PATCH 3/3] Exit instance middleware early if query param is missing Prior to this commit exceptions were being raised by the `instanceConfig` middleware when it ran before health check requests that do not pass an `instance_id` query parameter. --- http/middleware.js | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/http/middleware.js b/http/middleware.js index 1b9bf41..09767a5 100644 --- a/http/middleware.js +++ b/http/middleware.js @@ -1,7 +1,14 @@ module.exports = { instanceConfig: function(opts) { return function (req, res, next) { - var start = Date.now(); + var start = Date.now(), + hasInstance = !!req.query.instance_id; + if (!hasInstance) { + // If we don't have an instance_id (e.g. a health-check request) + // skip this middleware. + next(); + return; + } req.instanceConfig = {}; opts.dbPool.connect(function(err, client, done) { if (!err) {