diff --git a/.circleci/config.yml b/.circleci/config.yml index 8aeea9df..3ed292fe 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -3,10 +3,10 @@ jobs: build: working_directory: /app docker: - - image: docker:17.05.0-ce-git + - image: docker:18.06.1-ce-git steps: - checkout - setup_remote_docker - run: name: Build and push image to Docker Hub - command: sh .circleci/docker.sh + command: apk --no-cache add curl && curl "https://raw.githubusercontent.com/pelias/ci-tools/master/build-docker-images.sh" | sh - diff --git a/.circleci/docker.sh b/.circleci/docker.sh deleted file mode 100644 index 0dfa433c..00000000 --- a/.circleci/docker.sh +++ /dev/null @@ -1,32 +0,0 @@ -#!/bin/bash -set -u - -# collect params from ENV vars -DATE=`date +%Y-%m-%d` -DOCKER_REPOSITORY="pelias" -DOCKER_PROJECT="${DOCKER_REPOSITORY}/${CIRCLE_PROJECT_REPONAME}" - -BRANCH="$(echo $CIRCLE_BRANCH | tr '/' '-')" #slashes are not valid in docker tags. replace with dashes - -# the name of the image that represents the "branch", that is an image that will be updated over time with the git branch -# the production branch is changed to "latest", otherwise the git branch becomes the name of the version -if [[ "${BRANCH}" == "production" ]]; then - DOCKER_BRANCH_IMAGE_VERSION="latest" -else - DOCKER_BRANCH_IMAGE_VERSION="$BRANCH" -fi -DOCKER_BRANCH_IMAGE_NAME="${DOCKER_PROJECT}:${DOCKER_BRANCH_IMAGE_VERSION}" - -# the name of the image that represents the "tag", that is an image that is named with the date and git commit and will never be changed -DOCKER_TAG_IMAGE_VERSION="${BRANCH}-${DATE}-${CIRCLE_SHA1}" -DOCKER_TAG_IMAGE_NAME="${DOCKER_PROJECT}:${DOCKER_TAG_IMAGE_VERSION}" - -# build image and login to docker hub -docker build -t $DOCKER_PROJECT . -docker login -u="$DOCKER_USERNAME" -p="$DOCKER_PASSWORD" - -# copy the image to each of the two tags, and push -docker tag $DOCKER_PROJECT $DOCKER_BRANCH_IMAGE_NAME -docker tag $DOCKER_PROJECT $DOCKER_TAG_IMAGE_NAME -docker push $DOCKER_BRANCH_IMAGE_NAME -docker push $DOCKER_TAG_IMAGE_NAME diff --git a/.travis.yml b/.travis.yml index 5aec4536..3dd2c224 100644 --- a/.travis.yml +++ b/.travis.yml @@ -11,8 +11,12 @@ matrix: script: npm run travis before_install: - npm i -g npm -after_success: - - npx semantic-release branches: except: - /^v\d+\.\d+\.\d+$/ +jobs: + include: + - stage: release + node_js: 10 + script: curl "https://raw.githubusercontent.com/pelias/ci-tools/master/semantic-release.sh" | bash - + if: branch = production diff --git a/Dockerfile b/Dockerfile index dc7836e0..d2a4ba13 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,11 +1,7 @@ # base image FROM pelias/baseimage -RUN useradd -ms /bin/bash pelias USER pelias -# maintainer information -LABEL maintainer="pelias.team@gmail.com" - # Where the app is built and run inside the docker fs ENV WORK=/home/pelias WORKDIR ${WORK} diff --git a/README.md b/README.md index d90f834c..5f6c7864 100644 --- a/README.md +++ b/README.md @@ -66,7 +66,9 @@ A good starting configuration file includes this section (fill in the service an "url": "http://libpostal:8080" }, "pip": { - "url": "http://pip-service:4200" + "url": "http://pip-service:4200", + "timeout": 1000, + "retries": 2 }, "interpolation": { "url": "http://interpolation:4300" @@ -79,6 +81,8 @@ A good starting configuration file includes this section (fill in the service an } ``` +The `timeout` and `retry` values, as showin in the `pip` service section, are optional but configurable for all services (see [pelias/microservice-wrapper](https://github.com/pelias/microservice-wrapper) for more details). + ## Contributing diff --git a/bin/units b/bin/units index b011e2ee..fa0ea072 100755 --- a/bin/units +++ b/bin/units @@ -1,5 +1,7 @@ #!/bin/bash +# run tests with pipefail to avoid false passes +# see https://github.com/pelias/pelias/issues/744 set -euo pipefail -node test/unit/run.js | ./node_modules/.bin/tap-dot +node test/unit/run.js | npx tap-dot diff --git a/controller/libpostal.js b/controller/libpostal.js index 7ecbe3b6..70a2cc1e 100644 --- a/controller/libpostal.js +++ b/controller/libpostal.js @@ -1,5 +1,5 @@ const _ = require('lodash'); -const iso3166 = require('iso3166-1'); +const iso3166 = require('../helper/iso3166'); const Debug = require('../helper/debug'); const debugLog = new Debug('controller:libpostal'); const logger = require('pelias-logger').get('api'); @@ -70,6 +70,7 @@ function setup(libpostalService, should_execute) { const initialTime = debugLog.beginTimer(req); libpostalService(req, (err, response) => { + if (err) { // push err.message or err onto req.errors req.errors.push( _.get(err, 'message', err) ); @@ -82,6 +83,10 @@ function setup(libpostalService, should_execute) { return next(); } else { + + // apply fixes for known bugs in libpostal + response = patchBuggyResponses(response); + req.clean.parser = 'libpostal'; req.clean.parsed_text = response.reduce(function(o, f) { if (field_mapping.hasOwnProperty(f.label)) { @@ -91,8 +96,8 @@ function setup(libpostalService, should_execute) { return o; }, {}); - if (_.has(req.clean.parsed_text, 'country') && iso3166.is2(_.toUpper(req.clean.parsed_text.country))) { - req.clean.parsed_text.country = iso3166.to3(_.toUpper(req.clean.parsed_text.country)); + if (_.has(req.clean.parsed_text, 'country') && iso3166.isISO2Code(req.clean.parsed_text.country)) { + req.clean.parsed_text.country = iso3166.convertISO2ToISO3(req.clean.parsed_text.country); } debugLog.push(req, {parsed_text: req.clean.parsed_text}); @@ -109,4 +114,29 @@ function setup(libpostalService, should_execute) { return controller; } +const DIAGONAL_DIRECTIONALS = ['ne','nw','se','sw']; + +// apply fixes for known bugs in libpostal +function patchBuggyResponses(response){ + if( !Array.isArray(response) || !response.length ){ return response; } + + // known bug where the street name is only a directional, in this case we will merge it + // with the subsequent element. + // note: the bug only affects diagonals, not N,S,E,W + // https://github.com/OpenTransitTools/trimet-mod-pelias/issues/20#issuecomment-417732128 + for( var i=0; i 'THA' @@ -8,8 +8,8 @@ function _sanitize( raw, clean ){ // error & warning messages const messages = { errors: [], warnings: [] }; - if (clean.hasOwnProperty('parsed_text') && iso3166.is2(_.toUpper(clean.parsed_text.country))) { - clean.parsed_text.country = iso3166.to3(_.toUpper(clean.parsed_text.country)); + if (clean.hasOwnProperty('parsed_text') && iso3166.isISO2Code(clean.parsed_text.country)) { + clean.parsed_text.country = iso3166.convertISO2ToISO3(clean.parsed_text.country); } return messages; diff --git a/service/configurations/Language.js b/service/configurations/Language.js index c20fa861..a4d96c96 100644 --- a/service/configurations/Language.js +++ b/service/configurations/Language.js @@ -15,12 +15,18 @@ class Language extends ServiceConfiguration { Array.prototype.push.apply(acc, _.values(_.pickBy(doc.parent, (v, k) => _.endsWith(k, '_id') ) ) ); return acc; }, []); - - return { + const lang = _.get(req, 'clean.lang.iso6393'); + const parameters = { // arrays will be nested, so flatten first, then uniqify, and finally join elements with comma ids: _.uniq(_.flattenDeep(ids)).join(',') }; + if (lang) { + parameters.lang = lang; + } + + return parameters; + } getUrl(req) { diff --git a/test/unit/controller/libpostal.js b/test/unit/controller/libpostal.js index 56b47521..b66e4908 100644 --- a/test/unit/controller/libpostal.js +++ b/test/unit/controller/libpostal.js @@ -303,6 +303,62 @@ module.exports.tests.success_conditions = (test, common) => { }; +module.exports.tests.bug_fixes = (test, common) => { + test('bug fix: incorrect parsing of diagonal directionals', t => { + const service = (req, callback) => { + const response =[ + { + 'label': 'house_number', + 'value': '4004' + }, + { + 'label': 'road', + 'value': 'nw' + }, + { + 'label': 'suburb', + 'value': 'beaverton-hillsdale' + }, + { + 'label': 'city', + 'value': 'portland' + } + ]; + + callback(null, response); + }; + + const controller = libpostal(service, () => true); + + const req = { + clean: { + text: 'original query' + }, + errors: [] + }; + + controller(req, undefined, () => { + t.deepEquals(req, { + clean: { + text: 'original query', + parser: 'libpostal', + parsed_text: { + number: '4004', + street: 'nw beaverton-hillsdale', + city: 'portland' + } + }, + errors: [] + }, 'req should not have been modified'); + + t.end(); + + }); + + }); + +}; + module.exports.all = (tape, common) => { function test(name, testFunction) { diff --git a/test/unit/controller/placeholder.js b/test/unit/controller/placeholder.js index 82ace9bd..6d9df491 100644 --- a/test/unit/controller/placeholder.js +++ b/test/unit/controller/placeholder.js @@ -291,7 +291,7 @@ module.exports.tests.success = (test, common) => { }; t.deepEquals(res, expected_res); - t.ok(logger.isInfoMessage('[controller:placeholder] [result_count:2]')); + t.ok(logger.isDebugMessage('[controller:placeholder] [result_count:2]')); t.end(); }); @@ -355,7 +355,7 @@ module.exports.tests.success = (test, common) => { }; t.deepEquals(res, expected_res); - t.ok(logger.isInfoMessage('[controller:placeholder] [result_count:1]')); + t.ok(logger.isDebugMessage('[controller:placeholder] [result_count:1]')); t.end(); }); @@ -415,7 +415,7 @@ module.exports.tests.success = (test, common) => { }; t.deepEquals(res, expected_res); - t.ok(logger.isInfoMessage('[controller:placeholder] [result_count:1]')); + t.ok(logger.isDebugMessage('[controller:placeholder] [result_count:1]')); t.end(); }); @@ -473,7 +473,7 @@ module.exports.tests.success = (test, common) => { }; t.deepEquals(res, expected_res); - t.ok(logger.isInfoMessage('[controller:placeholder] [result_count:1]')); + t.ok(logger.isDebugMessage('[controller:placeholder] [result_count:1]')); t.end(); }); @@ -538,7 +538,7 @@ module.exports.tests.success = (test, common) => { }; t.deepEquals(res, expected_res); - t.ok(logger.isInfoMessage('[controller:placeholder] [result_count:1]')); + t.ok(logger.isDebugMessage('[controller:placeholder] [result_count:1]')); }); }); @@ -607,7 +607,7 @@ module.exports.tests.success = (test, common) => { }; t.deepEquals(res, expected_res); - t.ok(logger.isInfoMessage('[controller:placeholder] [result_count:1]')); + t.ok(logger.isDebugMessage('[controller:placeholder] [result_count:1]')); t.end(); }); @@ -1403,7 +1403,7 @@ module.exports.tests.result_filtering = (test, common) => { }; t.deepEquals(res, expected_res); - t.ok(logger.isInfoMessage('[controller:placeholder] [result_count:3]')); + t.ok(logger.isDebugMessage('[controller:placeholder] [result_count:3]')); t.end(); }); @@ -1538,7 +1538,7 @@ module.exports.tests.result_filtering = (test, common) => { }; t.deepEquals(res, expected_res); - t.ok(logger.isInfoMessage('[controller:placeholder] [result_count:3]')); + t.ok(logger.isDebugMessage('[controller:placeholder] [result_count:3]')); t.end(); }); @@ -1680,7 +1680,7 @@ module.exports.tests.result_filtering = (test, common) => { }; t.deepEquals(res, expected_res); - t.ok(logger.isInfoMessage('[controller:placeholder] [result_count:2]')); + t.ok(logger.isDebugMessage('[controller:placeholder] [result_count:2]')); t.end(); }); @@ -1852,7 +1852,7 @@ module.exports.tests.result_filtering = (test, common) => { }; t.deepEquals(res, expected_res); - t.ok(logger.isInfoMessage('[controller:placeholder] [result_count:3]')); + t.ok(logger.isDebugMessage('[controller:placeholder] [result_count:3]')); t.end(); }); @@ -2356,7 +2356,7 @@ module.exports.tests.error_conditions = (test, common) => { controller(req, res, () => { t.deepEquals(res, {}, 'res should not have been modified'); t.deepEquals(req.errors, ['placeholder service error']); - t.notOk(logger.isInfoMessage(/\\[controller:placeholder\\] \\[result_count:\\d+\\]/)); + t.notOk(logger.isDebugMessage(/\\[controller:placeholder\\] \\[result_count:\\d+\\]/)); t.end(); }); @@ -2385,7 +2385,7 @@ module.exports.tests.error_conditions = (test, common) => { controller(req, res, () => { t.deepEquals(res, {}, 'res should not have been modified'); t.deepEquals(req.errors, ['placeholder service error']); - t.notOk(logger.isInfoMessage(/\\[controller:placeholder\\] \\[result_count:\\d+\\]/)); + t.notOk(logger.isDebugMessage(/\\[controller:placeholder\\] \\[result_count:\\d+\\]/)); t.end(); }); @@ -2410,7 +2410,7 @@ module.exports.tests.error_conditions = (test, common) => { controller(req, res, () => { t.deepEquals(res, {}, 'res should not have been modified'); t.deepEquals(req.errors, [{ error_key: 'error_value' }]); - t.notOk(logger.isInfoMessage(/\\[controller:placeholder\\] \\[result_count:\\d+\\]/)); + t.notOk(logger.isDebugMessage(/\\[controller:placeholder\\] \\[result_count:\\d+\\]/)); t.end(); }); diff --git a/test/unit/helper/iso3166.js b/test/unit/helper/iso3166.js new file mode 100644 index 00000000..0d78715b --- /dev/null +++ b/test/unit/helper/iso3166.js @@ -0,0 +1,67 @@ +var iso3166 = require('../../../helper/iso3166'); + +module.exports.tests = {}; + +module.exports.tests.recognizingISOCodes = function(test, common) { + test('Recognizes iso2 codes', function(t) { + t.true(iso3166.isISO2Code('US')); + t.true(iso3166.isISO2Code('Us')); + t.false(iso3166.isISO2Code('xx')); + t.end(); + }); + + test('Recognizes iso3 codes', function(t) { + t.true(iso3166.isISO3Code('USA')); + t.true(iso3166.isISO3Code('UsA')); + t.false(iso3166.isISO3Code('xxx')); + t.end(); + }); +}; + +module.exports.tests.convertingISOCodes = function(test, common) { + test('converts iso2 to iso3', function(t) { + t.equal('USA', iso3166.convertISO2ToISO3('uS')); + t.equal('FRA', iso3166.convertISO2ToISO3('FR')); + t.equal('FRA', iso3166.convertISO2ToISO3('Fr')); + t.equal(undefined, iso3166.convertISO2ToISO3('uSa')); + t.equal(undefined, iso3166.convertISO2ToISO3('xx')); + t.end(); + }); + + test('converts iso3 to iso2', function(t) { + t.equal('US', iso3166.convertISO3ToISO2('uSa')); + t.equal('FR', iso3166.convertISO3ToISO2('fra')); + t.equal('FR', iso3166.convertISO3ToISO2('frA')); + t.equal(undefined, iso3166.convertISO3ToISO2('xxx')); + t.equal(undefined, iso3166.convertISO3ToISO2('fr')); + t.end(); + }); +}; + +module.exports.tests.getISO3Code = function(test, common) { + test('Gets iso 3 code for iso 2 code', function(t) { + t.equal('USA', iso3166.iso3Code('uS')); + t.equal('FRA', iso3166.iso3Code('fr')); + t.equal(undefined, iso3166.iso3Code('xxx')); + t.end(); + }); + + test('Recognizes and returns existing ISO 3 code', function(t) { + t.equal('USA', iso3166.iso3Code('USA')); + t.equal('FRA', iso3166.iso3Code('FRA')); + t.end(); + }); + + test('Upcases given ISO3 code if needed', function(t) { + t.equal('USA', iso3166.iso3Code('UsA')); + t.equal('USA', iso3166.iso3Code('usa')); + t.equal('FRA', iso3166.iso3Code('FRa')); + t.end(); + }); +}; + +module.exports.all = function (test, common) { + for( var testCase in module.exports.tests ){ + module.exports.tests[testCase](test, common); + } +}; diff --git a/test/unit/run.js b/test/unit/run.js index f196dc53..c8eee73a 100644 --- a/test/unit/run.js +++ b/test/unit/run.js @@ -37,6 +37,7 @@ var tests = [ require('./helper/fieldValue'), require('./helper/geojsonify_place_details'), require('./helper/geojsonify'), + require('./helper/iso3166'), require('./helper/logging'), require('./helper/TypeMapping'), require('./helper/type_mapping'), diff --git a/test/unit/service/configurations/Language.js b/test/unit/service/configurations/Language.js index 72cfb7d3..682ddc0a 100644 --- a/test/unit/service/configurations/Language.js +++ b/test/unit/service/configurations/Language.js @@ -117,6 +117,24 @@ module.exports.tests.all = (test, common) => { }); + + test('getParameters should return lang when req.clean.lang.iso6393 is defined', (t) => { + const configBlob = { + url: 'http://localhost:1234', + timeout: 17, + retries: 19 + }; + + const req = {clean: {lang: {iso6393: 'eng' }}}; + const res = { }; + + const language = new Language(configBlob); + + t.deepEquals(language.getParameters(req, res), { ids: '', lang: 'eng' }); + t.end(); + + }); + test('getHeaders should return empty object', (t) => { const configBlob = { url: 'base url',