From ca32634d237ef2ada15513a33b672028d3e07542 Mon Sep 17 00:00:00 2001 From: Klaas Hoekema Date: Mon, 28 Jan 2019 16:51:53 -0500 Subject: [PATCH] Remove layer filter Removes the querystring-based layer filter functionality, since it's not being used and isn't compatible with the intended caching strategy. --- src/tilegarden/src/api.js | 18 +- src/tilegarden/src/tiler.js | 4 +- src/tilegarden/src/util/layer-filter.js | 119 ------- src/tilegarden/tests/api.test.js | 38 +-- src/tilegarden/tests/layer-filter.test.js | 359 ---------------------- 5 files changed, 3 insertions(+), 535 deletions(-) delete mode 100644 src/tilegarden/src/util/layer-filter.js delete mode 100644 src/tilegarden/tests/layer-filter.test.js diff --git a/src/tilegarden/src/api.js b/src/tilegarden/src/api.js index fb94d897..282c33da 100644 --- a/src/tilegarden/src/api.js +++ b/src/tilegarden/src/api.js @@ -38,23 +38,8 @@ const getPositionalFilters = (req) => { return remainder } -// Returns a properly formatted list of layers -// or an empty list if there are none -const processLayers = (req) => { - if (req.queryStringParameters.layers) return JSON.parse(req.queryStringParameters.layers) - else if (req.queryStringParameters.layer || - req.queryStringParameters.filter || - req.queryStringParameters.filters) { - /* eslint-disable-next-line quotes */ - throw HTTPError("Invalid argument, did you mean '&layers='?", 400) - } - - return [] -} - // Parses out the configuration specifications const processConfig = req => ({ - s3bucket: req.queryStringParameters.s3bucket, config: req.pathParameters.config, }) @@ -114,10 +99,9 @@ api.get( try { const { z, x, y } = processCoords(req) const filters = getPositionalFilters(req) - const layers = processLayers(req) const configOptions = processConfig(req) - return imageTile(createMap(z, x, y, filters, layers, configOptions)) + return imageTile(createMap(z, x, y, filters, configOptions)) .then(tile => writeToS3(tile, req)) .then(img => new APIBuilder.ApiResponse(img, IMAGE_HEADERS, 200)) .catch(handleError) diff --git a/src/tilegarden/src/tiler.js b/src/tilegarden/src/tiler.js index d03eba2c..43bcce11 100644 --- a/src/tilegarden/src/tiler.js +++ b/src/tilegarden/src/tiler.js @@ -14,7 +14,6 @@ const readFile = promisify(require('fs').readFile) const addParamFilters = require('./util/param-filter') const bbox = require('./util/bounding-box') -const filterVisibleLayers = require('./util/layer-filter') const HTTPError = require('./util/error-builder') const { parseXml, buildXml } = require('./util/xml-tools') @@ -78,7 +77,7 @@ const fetchMapFile = (options) => { * @param y * @returns {Promise} */ -module.exports.createMap = (z, x, y, filters, layers, configOptions) => { +module.exports.createMap = (z, x, y, filters, configOptions) => { // Create a webmercator map with specified bounds const map = new mapnik.Map(TILE_WIDTH, TILE_HEIGHT) map.bufferSize = 64 @@ -86,7 +85,6 @@ module.exports.createMap = (z, x, y, filters, layers, configOptions) => { // Load map specification from xml string return fetchMapFile(configOptions) .then(parseXml) - .then(xmlJsObj => filterVisibleLayers(xmlJsObj, layers)) .then(xmlJsObj => addParamFilters(xmlJsObj, filters)) .then(buildXml) .then(xml => new Promise((resolve, reject) => { diff --git a/src/tilegarden/src/util/layer-filter.js b/src/tilegarden/src/util/layer-filter.js deleted file mode 100644 index e416a883..00000000 --- a/src/tilegarden/src/util/layer-filter.js +++ /dev/null @@ -1,119 +0,0 @@ -/** - * Defines a function that accepts a config XML converted to an object and returns the same - * object with the "status" property set to false for all layers not in the list of enabled - * layers, excluding them from rendering. - */ - -const sqlString = require('sql-escape-string') - -const HTTPError = require('./error-builder') - -// Determine comparison mode (if more than one parameter) -const processMode = (mode, i) => { - const MODES = ['AND', 'OR'] - - if (i > 0) { - if (mode) { - if (MODES.includes(mode.toUpperCase())) { - return mode.toUpperCase() - } - throw HTTPError( - `Filter comparison mode '${mode}' is unsupported.`, - 405, - ) - } - return 'AND' - } - return '' -} - -// Determine comparison operator -const processOp = (op) => { - const OP_WHITELIST = ['=', '<', '>', '>=', '<=', '<>', '!=', - 'LIKE', 'IS', 'IS NOT', 'IS DISTINCT FROM', 'IS NOT DISTINCT FROM'] - - if (op) { - if (OP_WHITELIST.includes(op.toUpperCase())) { - return op.toUpperCase() - } - throw HTTPError(`Operator '${op}' is unsupported.`, 405) - } - return '=' -} - -// Escape col but replace outer '-s with "-s to make a delimited identifier -const processCol = col => `"${sqlString(col).slice(1, -1)}"` - -/** - * Structures sql based on existing query and json options - */ -const structureQuery = (existingQuery, currentLayer) => { - // Base of the query, wrapping pre-existing query - let query = `SELECT * FROM ${existingQuery} WHERE` - - // Add each filter to the query string - currentLayer.filters.forEach((filterObj, i) => { - const mode = processMode(currentLayer.mode, i) - const op = processOp(filterObj.op) - const col = processCol(filterObj.col) - const val = sqlString(filterObj.val) - - query += `${mode ? ` ${mode}` : ''} ` + - `${col} ${op} ${val}` - }) - - // Wrap in variable and set in xml - return `(${query}) as m` -} - -/** - * Set the status of each layer NOT in the list to false to disable - */ -const filter = (xmlJsObj, enabledLayers) => { - let givenLayers = enabledLayers - - xmlJsObj.Map.Layer.forEach((layer) => { - // Filter list of enabled layers by the name of the current layer - const current = enabledLayers.filter(e => e === layer.$.name || e.name === layer.$.name)[0] - - // If there is no matching enabledLayers entry, disable the current layer - if (!current) { - /* eslint-disable-next-line no-param-reassign */ - layer.$.status = 'false' - } else { - // Remove layer from the list to mark it as "seen" - givenLayers = givenLayers.filter(l => l !== current) - - // If there IS a matching entry, wrap SQL accordingly - if (current.filters) { - // This locates the query field in the data structure, trust me - // also I'm sorry for how janked up this is - const queryObj = layer.Datasource[0].Parameter.filter(p => p.$.name === 'table')[0] - - if (queryObj) { - queryObj._ = structureQuery(queryObj._, current) - } - } - } - }) - - // If there are any layers left in enabledLayers, something's wrong - if (givenLayers.length > 0) { - throw HTTPError( - `No layer(s) found with the name(s) '${JSON.stringify(givenLayers.join(', '))}'.`, - 400, - ) - } - - return xmlJsObj -} - -module.exports = (xmlJsObj, enabledLayers) => { - // skip entire process if no layers are to be parsed - if (!enabledLayers || enabledLayers.length < 1) { - return xmlJsObj - } - - return filter(xmlJsObj, enabledLayers) -} - diff --git a/src/tilegarden/tests/api.test.js b/src/tilegarden/tests/api.test.js index 14348cf8..220b12a8 100644 --- a/src/tilegarden/tests/api.test.js +++ b/src/tilegarden/tests/api.test.js @@ -1,7 +1,6 @@ const rewire = require('rewire') const api = rewire('../src/api'), - processCoords = api.__get__('processCoords'), - processLayers = api.__get__('processLayers') + processCoords = api.__get__('processCoords') describe('processCoords', () => { test('properly parses properly formatted coords', () => { @@ -73,38 +72,3 @@ describe('processCoords', () => { }) }) -describe('processLayers', () => { - test('properly parses list of layer', () => { - const req = { - queryStringParameters: { - layers: '["layer1","layer2","layer3","layer4"]' - } - } - expect(processLayers(req)).toEqual(['layer1','layer2','layer3','layer4']) - }) - - test('properly parses just one layer', () => { - const req = { - queryStringParameters: { - layers: '["layer"]' - } - } - expect(processLayers(req)).toEqual(['layer']) - }) - - test('properly parses no fields', () => { - const req = { - queryStringParameters: {} - } - expect(processLayers(req)).toEqual([]) - }) - - test('properly parses a blank field', () => { - const req = { - queryStringParameters: { - layers: '' - } - } - expect(processLayers(req)).toEqual([]) - }) -}) diff --git a/src/tilegarden/tests/layer-filter.test.js b/src/tilegarden/tests/layer-filter.test.js deleted file mode 100644 index 6daaff03..00000000 --- a/src/tilegarden/tests/layer-filter.test.js +++ /dev/null @@ -1,359 +0,0 @@ -const rewire = require('rewire') -const filterLayers = require('../src/util/layer-filter') -const { parseXml, buildXml } = require('../src/util/xml-tools') -const filterer = rewire('../src/util/layer-filter'), - structureQuery = filterer.__get__('structureQuery') - -describe('structureQuery', () => { - test('One value', () => { - const layer = { - name: 'layer', - filters: [ - { - col: 'col', - val: 'val' - } - ] - } - const existingQuery = 'table' - const expectedQuery = "(SELECT * FROM table WHERE \"col\" = 'val') as m" - - expect(structureQuery(existingQuery, layer)).toBe(expectedQuery) - }) - - test('Multiple values, no AND', () => { - const layer = { - name: 'layer', - filters: [ - { - col: 'col', - val: 'val' - }, - { - col: 'col', - val: 'val' - } - ] - } - const existingQuery = 'table' - const expectedQuery = "(SELECT * FROM table WHERE \"col\" = 'val' AND \"col\" = 'val') as m" - - expect(structureQuery(existingQuery, layer)).toBe(expectedQuery) - }) - - test('Test AND', () => { - const layer = { - name: 'layer', - mode: 'AND', - filters: [ - { - col: 'col', - val: 'val' - }, - { - col: 'col', - val: 'val' - } - ] - } - const existingQuery = 'table' - const expectedQuery = "(SELECT * FROM table WHERE \"col\" = 'val' AND \"col\" = 'val') as m" - - expect(structureQuery(existingQuery, layer)).toBe(expectedQuery) - }) - - test('Test OR', () => { - const layer = { - name: 'layer', - mode: 'OR', - filters: [ - { - col: 'col', - val: 'val' - }, - { - col: 'col', - val: 'val' - } - ] - } - const existingQuery = 'table' - const expectedQuery = "(SELECT * FROM table WHERE \"col\" = 'val' OR \"col\" = 'val') as m" - - expect(structureQuery(existingQuery, layer)).toBe(expectedQuery) - }) - - test('Test non-allowed mode', () => { - const layer = { - name: 'layer', - mode: 'XOR', - filters: [ - { - col: 'col', - val: 'val' - }, - { - col: 'col', - val: 'val' - } - ] - } - const existingQuery = 'table' - const expectedQuery = "(SELECT * FROM table WHERE \"col\" = 'val' AND \"col\" = 'val') as m" - - expect(() => { structureQuery(existingQuery, layer) }).toThrow() - }) - - test('Test mathematical operator', () => { - const layer = { - name: 'layer', - filters: [ - { - col: 'col', - val: 'val', - op: '>' - }, - ] - } - const existingQuery = 'table' - const expectedQuery = "(SELECT * FROM table WHERE \"col\" > 'val') as m" - - expect(structureQuery(existingQuery, layer)).toBe(expectedQuery) - }) - - test('Test other operator', () => { - const layer = { - name: 'layer', - filters: [ - { - col: 'col', - val: 'val', - op: 'IS DISTINCT FROM' - }, - ] - } - const existingQuery = 'table' - const expectedQuery = "(SELECT * FROM table WHERE \"col\" IS DISTINCT FROM 'val') as m" - - expect(structureQuery(existingQuery, layer)).toBe(expectedQuery) - }) - - test('Test non-allowed operator', () => { - const layer = { - name: 'layer', - filters: [ - { - col: 'col', - val: 'val', - op: 'IS ABOVE' - }, - ] - } - const existingQuery = 'table' - - expect(() => { structureQuery(existingQuery, layer) }).toThrow() - }) -}) - -async function filterLayersWithParsing(xml, layers) { - return parseXml(xml) - .then(xmlJsObj => filterLayers(xmlJsObj, layers)) - .then(buildXml) -} - -describe('filterLayers', () => { - test('Just a string name', () => { - const layers = ['PWD'] - expect.assertions(1) - - const xml = -` - - - - -` - - const expected = -` - - - - -` - - return expect(filterLayersWithParsing(xml, layers)).resolves.toBe(expected) - }) - - test('Several string names', () => { - const layers = ['PWD', 'STATE'] - expect.assertions(1) - - const xml = - ` - - - - -` - - const expected = - ` - - - - -` - - return expect(filterLayersWithParsing(xml, layers)).resolves.toBe(expected) - }) - - test('Just an object name', () => { - const layers = [{name:'PWD'}] - expect.assertions(1) - - const xml = - ` - - - - -` - - const expected = - ` - - - - -` - - return expect(filterLayersWithParsing(xml, layers)).resolves.toBe(expected) - }) - - test('Several object names', () => { - const layers = [{name:'PWD'}, {name:'STATE'}] - expect.assertions(1) - - const xml = - ` - - - - -` - - const expected = - ` - - - - -` - - return expect(filterLayersWithParsing(xml, layers)).resolves.toBe(expected) - }) - - test('Object name + 1 filter', () => { - const layers = [{name:'PWD', filters: [{col:'owner', val:'PWD'}]}] - expect.assertions(1) - - const xml = - ` - - - - inlets - - - - -` - - const expected = - ` - - - - (SELECT * FROM inlets WHERE "owner" = 'PWD') as m - - - - -` - - return expect(filterLayersWithParsing(xml, layers)).resolves.toBe(expected) - }) - - test('Object name + 2 filters', () => { - const layers = [{name:'PWD', filters: [{col:'owner', val:'PWD'}, {col:'operator', val:'PWD'}]}] - expect.assertions(1) - - const xml = - ` - - - - inlets - - - - -` - - const expected = - ` - - - - (SELECT * FROM inlets WHERE "owner" = 'PWD' AND "operator" = 'PWD') as m - - - - -` - - return expect(filterLayersWithParsing(xml, layers)).resolves.toBe(expected) - }) - - test('dont filter if layer list is empty', () => { - const layers = [] - const xml = -` - - - - -` - expect.assertions(1) - return expect(filterLayersWithParsing(xml, layers)).resolves.toBe(xml) - }) - - test('dont filter if there is no layer list', () => { - const xml = -` - - - - -` - expect.assertions(1) - return expect(filterLayersWithParsing(xml)).resolves.toBe(xml) - }) - - test('Throw error if a layer doesn\'t exist', () => { - const layers = ['dog'] - expect.assertions(1) - - const xml = - ` - - - - -` - - return expect(filterLayersWithParsing(xml, layers)).rejects.toBeInstanceOf(Error) - }) -}) -