Skip to content

Commit

Permalink
Merge pull request #7 from henrythasler/bugfix/refactor-code-smells
Browse files Browse the repository at this point in the history 
Bugfix/refactor code smells
  • Loading branch information
@henrythasler
henrythasler committed Sep 5, 2019
2 parents 983510d + 06a19f3 commit 9fd323c
Show file tree
Hide file tree
Showing 6 changed files with 69 additions and 64 deletions.
1 change: 1 addition & 0 deletions DEVELOPMENT.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ SELECT ( [${layer1} [|| ${layer2} [|| ...]] ) as data
- Security-Review for Lambda-Code (e.g. SQL-Injection, ...)
- Change all scripts to use Postgres environment variables (PGUSER, ...)
- Omit Postgres credentials altogether and use IAM-role instead
- move lambda-function out of VPC to reduce cold-start-time
- Add raster endpoint with node-mapbox-gl-native to serve pre-rendered raster-images.

## References
Expand Down
1 change: 1 addition & 0 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
"typescript": "^3.5.3"
},
"dependencies": {
"aws-lambda": "^0.1.2",
"aws-sdk": "^2.503.0",
"pg": "^7.11.0"
}
Expand Down
4 changes: 2 additions & 2 deletions src/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ interface Event {

export const handler: Handler = async (event: Event, context: Context): Promise<any> => {
let response;
let vectortile: Vectortile = await tileserver.getVectortile(event.path);
const vectortile: Vectortile = await tileserver.getVectortile(event.path);
if ((vectortile.res >= 0) && (vectortile.data)) {
response = {
statusCode: 200,
Expand All @@ -39,4 +39,4 @@ export const handler: Handler = async (event: Event, context: Context): Promise<
}
}
return Promise.resolve(response)
}
}
38 changes: 19 additions & 19 deletions src/projection.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,39 +40,39 @@ export class Projection {

/** Converts XY point from Pseudo-Mercator (https://epsg.io/3857) to WGS84 (https://epsg.io/4326) */
getWGS84FromMercator(pos: Mercator): Wgs84 {
let lon = (pos.x / this.originShift) * 180.0;
const lon = (pos.x / this.originShift) * 180.0;
let lat = (pos.y / this.originShift) * 180.0;
lat = 180 / Math.PI * (2 * Math.atan(Math.exp(lat * Math.PI / 180.0)) - Math.PI / 2.0)
return ({ lng: lon%360, lat: lat%180 } as Wgs84)
return ({ lng: lon % 360, lat: lat % 180 } as Wgs84)
}

/** Converts pixel coordinates (Origin is top-left) in given zoom level of pyramid to EPSG:900913 */
getMercatorFromPixels(pos: Vector, zoom: number, tileSize: number = 256): Mercator {
getMercatorFromPixels(pos: Vector, zoom: number, tileSize = 256): Mercator {
// zoom = Math.max(0, zoom + 1 - tileSize / 256)
let res = 2 * Math.PI * 6378137 / tileSize / Math.pow(2, zoom);
const res = 2 * Math.PI * 6378137 / tileSize / Math.pow(2, zoom);
return ({ x: pos.x * res - this.originShift, y: this.originShift - pos.y * res } as Mercator)
}

/** Returns bounds of the given tile in Pseudo-Mercator (https://epsg.io/3857) coordinates */
getMercatorTileBounds(tile: Tile, tileSize: number = 256): MercatorBoundingBox {
let leftbottom = this.getMercatorFromPixels(<Vector>{ x: tile.x * tileSize, y: (tile.y + 1) * tileSize }, tile.z, tileSize);
let righttop = this.getMercatorFromPixels(<Vector>{ x: (tile.x + 1) * tileSize, y: tile.y * tileSize }, tile.z, tileSize);
return ({ leftbottom: leftbottom, righttop: righttop } as MercatorBoundingBox)
getMercatorTileBounds(tile: Tile, tileSize = 256): MercatorBoundingBox {
const leftbottom = this.getMercatorFromPixels({ x: tile.x * tileSize, y: (tile.y + 1) * tileSize } as Vector, tile.z, tileSize);
const righttop = this.getMercatorFromPixels({ x: (tile.x + 1) * tileSize, y: tile.y * tileSize } as Vector, tile.z, tileSize);
return ({ leftbottom, righttop } as MercatorBoundingBox)
}

/** Returns bounds of the given tile in WGS84 (https://epsg.io/4326) coordinates */
getWGS84TileBounds(tile: Tile, tileSize: number = 256): WGS84BoundingBox {
let bounds: MercatorBoundingBox = this.getMercatorTileBounds(tile, tileSize);
getWGS84TileBounds(tile: Tile, tileSize = 256): WGS84BoundingBox {
const bounds: MercatorBoundingBox = this.getMercatorTileBounds(tile, tileSize);
return ({
leftbottom: this.getWGS84FromMercator(bounds.leftbottom),
righttop: this.getWGS84FromMercator(bounds.righttop)
} as WGS84BoundingBox)
}

/** Returns center of the given tile in WGS84 (https://epsg.io/4326) coordinates */
getWGS84TileCenter(tile: Tile, tileSize: number = 256): Wgs84 {
let bounds: WGS84BoundingBox = this.getWGS84TileBounds(tile, tileSize);
return ({
getWGS84TileCenter(tile: Tile, tileSize = 256): Wgs84 {
const bounds: WGS84BoundingBox = this.getWGS84TileBounds(tile, tileSize);
return ({
lng: (bounds.righttop.lng + bounds.leftbottom.lng) / 2,
lat: (bounds.righttop.lat + bounds.leftbottom.lat) / 2,
} as Wgs84)
Expand All @@ -83,17 +83,17 @@ export class Projection {
* @param depth How many levels the resulting pyramid will have.
* @return An array of tiles
*/
getTilePyramid(tile: Tile, depth: number = 1): TileList {
let list: TileList = [];
getTilePyramid(tile: Tile, depth = 1): TileList {
const list: TileList = [];
depth = Math.max(0, depth); // do not allow negative values
for (let zoom = 0; zoom <= depth; zoom++) {
for (let y = tile.y * 2 ** zoom; y < (tile.y + 1) * 2 ** zoom; y++) {
for (let x = tile.x * 2 ** zoom; x < (tile.x + 1) * 2 ** zoom; x++) {
list.push(<Tile>{
x: x,
y: y,
list.push({
x,
y,
z: tile.z + zoom
})
} as Tile)
}
}
}
Expand Down
87 changes: 45 additions & 42 deletions src/tileserver.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ export class Tileserver {
* @param config
* @param cacheBucketName
*/
constructor(config: Config, cacheBucketName?: string, logLevel: number = LogLevels.ERROR, gzip: boolean = true) {
constructor(config: Config, cacheBucketName?: string, logLevel: number = LogLevels.ERROR, gzip = true) {
if (cacheBucketName) this.cacheBucketName = cacheBucketName;
this.config = config;
this.gzip = gzip;
Expand All @@ -102,11 +102,11 @@ export class Tileserver {
* @return a tile for subsequent use or null if no valid Tile could be extracted.
*/
extractTile(path: string): Tile | null {
let tile: Tile = { x: 0, y: 0, z: 0 };
let re = new RegExp(/\d+\/\d+\/\d+(?=\.mvt\b)/g);
let tilepath = path.match(re);
const tile: Tile = { x: 0, y: 0, z: 0 };
const re = new RegExp(/\d+\/\d+\/\d+(?=\.mvt\b)/g);
const tilepath = path.match(re);
if (tilepath) {
let numbers = tilepath[0].split("/");
const numbers = tilepath[0].split("/");
tile.y = parseInt(numbers[numbers.length - 1]);
tile.x = parseInt(numbers[numbers.length - 2]);
tile.z = parseInt(numbers[numbers.length - 3]);
Expand All @@ -123,7 +123,7 @@ export class Tileserver {
*/
extractSource(path: string): string | null {
// match the last word between slashes before the actual tile (3-numbers + extension)
let sourceCandidates: RegExpMatchArray | null = path.match(/(?!\/)\w+(?=\/\d+\/\d+\/\d+\.mvt\b)/g)
const sourceCandidates: RegExpMatchArray | null = path.match(/(?!\/)\w+(?=\/\d+\/\d+\/\d+\.mvt\b)/g)
if (sourceCandidates != null && sourceCandidates.length > 0) {
return sourceCandidates[sourceCandidates.length - 1];
}
Expand All @@ -143,17 +143,17 @@ export class Tileserver {

/** check layer zoom if present */
if (
((layer.minzoom != undefined) && (zoom < layer.minzoom)) ||
((layer.maxzoom != undefined) && (zoom >= layer.maxzoom))
((layer.minzoom !== undefined) && (zoom < layer.minzoom)) ||
((layer.maxzoom !== undefined) && (zoom >= layer.maxzoom))
) {
return null;
}

if (layer.variants && layer.variants.length) {
for (let variant of layer.variants) {
for (const variant of layer.variants) {
/** the default zoom-values should cover all use-cases on earth */
let variantMinzoom = (variant.minzoom != undefined) ? variant.minzoom : /* istanbul ignore next: This can't happen due to interface definition */0
let variantMaxzoom = (variant.maxzoom != undefined) ? variant.maxzoom : 32
const variantMinzoom = (variant.minzoom !== undefined) ? variant.minzoom : /* istanbul ignore next: This can't happen due to interface definition */0
const variantMaxzoom = (variant.maxzoom !== undefined) ? variant.maxzoom : 32
if (zoom >= variantMinzoom && zoom < variantMaxzoom) {
/** We have a match: merge the variant with the original layer. */
resolved = { ...layer, ...variant }
Expand Down Expand Up @@ -182,15 +182,15 @@ export class Tileserver {
if (resolved === null) return null;
// FIXME: minzoom and maxzoom must be propagated from source into layer

let layerExtend: number = (resolved.extend != undefined) ? resolved.extend : ((source.extend != undefined) ? source.extend : 4096);
let sql: string = (resolved.sql != undefined) ? resolved.sql : ((source.sql != undefined) ? source.sql : "");
let geom: string = (resolved.geom != undefined) ? resolved.geom : ((source.geom != undefined) ? source.geom : "geometry");
let srid: number = (resolved.srid != undefined) ? resolved.srid : ((source.srid != undefined) ? source.srid : 3857);
let bbox: string = `ST_Transform(ST_MakeEnvelope(${wgs84BoundingBox.leftbottom.lng}, ${wgs84BoundingBox.leftbottom.lat}, ${wgs84BoundingBox.righttop.lng}, ${wgs84BoundingBox.righttop.lat}, 4326), ${srid})`;
let buffer: number = (resolved.buffer != undefined) ? resolved.buffer : ((source.buffer != undefined) ? source.buffer : 256);
let clip_geom: boolean = (resolved.clip_geom != undefined) ? resolved.clip_geom : ((source.clip_geom != undefined) ? source.clip_geom : true);
let prefix: string = (resolved.prefix != undefined) ? resolved.prefix : ((source.prefix != undefined) ? source.prefix : "");
let postfix: string = (resolved.postfix != undefined) ? resolved.postfix : ((source.postfix != undefined) ? source.postfix : "");
const layerExtend: number = (resolved.extend !== undefined) ? resolved.extend : ((source.extend !== undefined) ? source.extend : 4096);
const sql: string = (resolved.sql !== undefined) ? resolved.sql : ((source.sql !== undefined) ? source.sql : "");
const geom: string = (resolved.geom !== undefined) ? resolved.geom : ((source.geom !== undefined) ? source.geom : "geometry");
const srid: number = (resolved.srid !== undefined) ? resolved.srid : ((source.srid !== undefined) ? source.srid : 3857);
const bbox: string = `ST_Transform(ST_MakeEnvelope(${wgs84BoundingBox.leftbottom.lng}, ${wgs84BoundingBox.leftbottom.lat}, ${wgs84BoundingBox.righttop.lng}, ${wgs84BoundingBox.righttop.lat}, 4326), ${srid})`;
const buffer: number = (resolved.buffer !== undefined) ? resolved.buffer : ((source.buffer !== undefined) ? source.buffer : 256);
const clip_geom: boolean = (resolved.clip_geom !== undefined) ? resolved.clip_geom : ((source.clip_geom !== undefined) ? source.clip_geom : true);
const prefix: string = (resolved.prefix !== undefined) ? resolved.prefix : ((source.prefix !== undefined) ? source.prefix : "");
const postfix: string = (resolved.postfix !== undefined) ? resolved.postfix : ((source.postfix !== undefined) ? source.postfix : "");

let keys: string = "";
if (source.keys && source.keys.length) {
Expand Down Expand Up @@ -234,18 +234,18 @@ export class Tileserver {
*/
buildQuery(source: string, wgs84BoundingBox: WGS84BoundingBox, zoom: number): string | null {
let query: string | null = null;
let layerQueries: string[] = [];
let layerNames: string[] = [];
const layerQueries: string[] = [];
const layerNames: string[] = [];

for (let sourceItem of this.config.sources) {
for (const sourceItem of this.config.sources) {
if (sourceItem.name === source) {
for (let layer of sourceItem.layers) {
for (const layer of sourceItem.layers) {
/** Accoring to https://github.com/mapbox/vector-tile-spec/tree/master/2.1#41-layers:
* Prior to appending a layer to an existing Vector Tile, an encoder MUST check the existing name fields in order to prevent duplication.
* implementation solution: ignore subsequent duplicates and log an error*/
if (!layerNames.includes(layer.name)) {
layerNames.push(layer.name);
let layerQuery: string | null = this.buildLayerQuery(sourceItem, layer, wgs84BoundingBox, zoom);
const layerQuery: string | null = this.buildLayerQuery(sourceItem, layer, wgs84BoundingBox, zoom);
if (layerQuery) layerQueries.push(layerQuery);
}
else {
Expand All @@ -257,7 +257,7 @@ export class Tileserver {

/** merge all queries with the string concatenation operator */
if (layerQueries.length) {
let layers = layerQueries.join(" || ");
const layers = layerQueries.join(" || ");
query = `SELECT ( ${layers} ) AS mvt`;
}
else {
Expand All @@ -276,20 +276,23 @@ export class Tileserver {
}

async fetchTileFromDatabase(query: string, clientConfig: ClientConfig): Promise<Buffer> {
let client: Client = new Client(clientConfig);
const client: Client = new Client(clientConfig);
await client.connect();
let res: QueryResult = await client.query(query);
const res: QueryResult = await client.query(query);
this.log.show(res.rows[0], LogLevels.TRACE);
await client.end();
if(res.rows[0].mvt)
if(res.rows[0].mvt) {
return res.rows[0].mvt; // the .d property is taken from the outer AS-alias of the query
else throw new Error("Property 'mvt' does not exist in res.rows[0]")
}
else {
throw new Error("Property 'mvt' does not exist in res.rows[0]")
}
}

getClientConfig(source: string): ClientConfig {
let clientConfig: ClientConfig = {};
const clientConfig: ClientConfig = {};

for (let sourceItem of this.config.sources) {
for (const sourceItem of this.config.sources) {
if (sourceItem.name === source) {
// pick only the connection info from the sourceItem
if ("host" in sourceItem) clientConfig.host = sourceItem.host;
Expand All @@ -307,19 +310,19 @@ export class Tileserver {
* @param path a full path including arbitrary prefix-path, layer, tile and extension
*/
async getVectortile(path: string): Promise<Vectortile> {
let mvt: Vectortile = { res: 0};
const mvt: Vectortile = { res: 0};
const s3 = new S3({ apiVersion: '2006-03-01' });

let tile = this.extractTile(path);
const tile = this.extractTile(path);
if (tile) {
let source = this.extractSource(path);
const source = this.extractSource(path);
if (source) {
let wgs84BoundingBox = this.proj.getWGS84TileBounds(tile);
let query = this.buildQuery(source, wgs84BoundingBox, tile.z)
const wgs84BoundingBox = this.proj.getWGS84TileBounds(tile);
const query = this.buildQuery(source, wgs84BoundingBox, tile.z)
this.log.show(query, LogLevels.DEBUG);
let data: Buffer | null = null;
if (query) {
let pgConfig = this.getClientConfig(source);
const pgConfig = this.getClientConfig(source);
this.log.show(pgConfig, LogLevels.TRACE);
try {
data = await this.fetchTileFromDatabase(query, pgConfig);
Expand All @@ -336,10 +339,10 @@ export class Tileserver {
}
this.log.show(data, LogLevels.TRACE);
if (data) {
let uncompressedBytes = data.byteLength;
if (this.gzip) mvt.data = <Buffer> await asyncgzip(data);
const uncompressedBytes = data.byteLength;
if (this.gzip) mvt.data = await asyncgzip(data) as Buffer;
else mvt.data = data;
let compressedBytes = mvt.data.byteLength;
const compressedBytes = mvt.data.byteLength;
this.log.show(`${path} ${source}/${tile.z}/${tile.x}/${tile.y} ${uncompressedBytes} -> ${compressedBytes}`, LogLevels.INFO);
if (this.cacheBucketName) {
try {
Expand Down Expand Up @@ -377,4 +380,4 @@ export class Tileserver {
}
return mvt;
}
}
}
2 changes: 1 addition & 1 deletion terraform/tileserver-cloudfront.tf
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ resource "aws_cloudfront_distribution" "tiles" {
viewer_protocol_policy = "redirect-to-https"
min_ttl = 0
default_ttl = 0
max_ttl = 86400
max_ttl = 604800
}

restrictions {
Expand Down

0 comments on commit 9fd323c

Please sign in to comment.