chore: Add tests (#17)

* feat: return encoded source ID from writer.addSource() This will be used in tests, and is generally useful. * feat: store upstream source ids on style metadata * chore!: adjust Reader API * chore: improve download types * chore: stricter (readonly) BBox type * [WIP] add tests (+1 squashed commit) Squashed commits: [3b28975] [WIP] add tests * fix up Writer & improve type safety * fix Reader.getUrl for empty baseUrl string * fix style metadata * types cleanup for reader * make params optional for tileIterator * fix: writer should remove layers without source in SMP * chore: DRY template function * types: stricter style spec for SMP * chore: add a bunch of tests * external geojson test * test for writing & reading sprites * cleanup sprite test * test multiple sprites * raster source tests * remove unused method from styleDownloader * fixup types * update readme * run tests on CI
digidem · Sep 26, 2024 · 010f68b · 010f68b
1 parent d5a8faa
commit 010f68b
Show file tree

Hide file tree

Showing 43 changed files with 7,388 additions and 169 deletions.
diff --git a/.github/workflows/node.yml b/.github/workflows/node.yml
@@ -0,0 +1,30 @@
+name: Node.js Tests
+
+on:
+  push:
+    branches: [main]
+  pull_request:
+    branches: [main]
+
+jobs:
+  test:
+    runs-on: ubuntu-latest
+
+    strategy:
+      matrix:
+        node-version: [18.x, 20.x, 22.x]
+
+    steps:
+      - name: Checkout repository
+        uses: actions/checkout@v4
+
+      - name: Set up Node.js ${{ matrix.node-version }}
+        uses: actions/setup-node@v4
+        with:
+          node-version: ${{ matrix.node-version }}
+
+      - name: Install dependencies
+        run: npm ci
+
+      - name: Run tests
+        run: npm test
diff --git a/README.md b/README.md
@@ -2,6 +2,14 @@
 
 A Styled Map Package (`.smp`) file is a Zip archive containing all the resources needed to serve a Maplibre vector styled map offline. This includes the style JSON, vector and raster tiles, glyphs (fonts), the sprite image, and the sprite metadata.
 
+## Installation
+
+Install globally to use the `smp` command.
+
+```sh
+npm install --global styled-map-package
+```
+
 ## Usage
 
 Download an online map to a styled map package file, specifying the bounding box (west, south, east, north) and max zoom level.

diff --git a/lib/download.js b/lib/download.js
@@ -25,7 +25,7 @@ import StyleDownloader from './style-downloader.js'
  * @param {string} opts.styleUrl URL of the style to download
  * @param { (progress: DownloadProgress) => void } [opts.onprogress] Optional callback for reporting progress
  * @param {string} [opts.accessToken]
- * @returns {import('stream').Readable} Readable stream of the output styled map file
+ * @returns {import('./types.js').DownloadStream} Readable stream of the output styled map file
  */
 export default function download({
   bbox,
@@ -75,15 +75,11 @@ export default function download({
   ;(async () => {
     const style = await downloader.getStyle()
     const writer = new Writer(style)
+    handleProgress({ style: { done: true } })
     writer.outputStream.pipe(sizeCounter)
     writer.on('error', (err) => sizeCounter.destroy(err))
 
     try {
-      for await (const [sourceId, source] of downloader.getSources()) {
-        writer.addSource(sourceId, source)
-      }
-      handleProgress({ style: { done: true } })
-
       for await (const spriteInfo of downloader.getSprites()) {
         await writer.addSprite(spriteInfo)
         handleProgress({

diff --git a/lib/reader.js b/lib/reader.js
@@ -58,15 +58,15 @@ export default class Reader {
    * will be transformed to use the provided base URL.
    *
    * @param {string | null} [baseUrl] Base URL where you plan to serve the resources in this styled map package, e.g. `http://localhost:3000/maps/styleA`
-   * @returns {Promise<Resource>}
+   * @returns {Promise<import('./types.js').SMPStyle>}
    */
   async getStyle(baseUrl = null) {
     const styleEntry = (await this.#entriesPromise).get(STYLE_FILE)
     if (!styleEntry) throw new Error(`File not found: ${STYLE_FILE}`)
     const stream = await styleEntry.openReadStream()
     const style = await json(stream)
     if (!validateStyle(style)) {
-      throw new Error('Invalid style')
+      throw new AggregateError(validateStyle.errors, 'Invalid style')
     }
     if (typeof style.glyphs === 'string') {
       style.glyphs = getUrl(style.glyphs, baseUrl)
@@ -83,13 +83,9 @@ export default class Reader {
         source.tiles = source.tiles.map((tile) => getUrl(tile, baseUrl))
       }
     }
-    const transformedStyleJSON = JSON.stringify(style)
-    return {
-      contentType: 'application/json; charset=utf-8',
-      contentLength: Buffer.byteLength(transformedStyleJSON, 'utf8'),
-      resourceType: 'style',
-      stream: intoStream(transformedStyleJSON),
-    }
+    // Hard to get this type-safe without a validation function. Instead we
+    // trust the Writer and the tests for now.
+    return /** @type {import('./types.js').SMPStyle} */ (style)
   }
 
   /**
@@ -101,7 +97,15 @@ export default class Reader {
    */
   async getResource(path) {
     if (path[0] === '/') path = path.slice(1)
-    if (path === STYLE_FILE) return this.getStyle()
+    if (path === STYLE_FILE) {
+      const styleJSON = JSON.stringify(await this.getStyle())
+      return {
+        contentType: 'application/json; charset=utf-8',
+        contentLength: Buffer.byteLength(styleJSON, 'utf8'),
+        resourceType: 'style',
+        stream: intoStream(styleJSON),
+      }
+    }
     const entry = (await this.#entriesPromise).get(path)
     if (!entry) throw new Error(`File not found: ${path}`)
     const resourceType = getResourceType(path)
@@ -141,6 +145,6 @@ function getUrl(smpUri, baseUrl) {
   if (!smpUri.startsWith(URI_BASE)) {
     throw new Error(`Invalid SMP URI: ${smpUri}`)
   }
-  if (!baseUrl) return smpUri
+  if (typeof baseUrl !== 'string') return smpUri
   return smpUri.replace(URI_BASE, baseUrl + '/')
 }
diff --git a/lib/server.js b/lib/server.js
@@ -36,11 +36,11 @@ export default function (fastify, { filepath, lazy = false }, done) {
     reader = new Reader(filepath)
   }
 
-  fastify.get('/style.json', async (_request, reply) => {
+  fastify.get('/style.json', async () => {
     if (!reader) {
       reader = new Reader(filepath)
     }
-    return sendResource(reply, await reader.getStyle(fastify.listeningOrigin))
+    return reader.getStyle(fastify.listeningOrigin)
   })
 
   fastify.get('*', async (request, reply) => {

diff --git a/lib/style-downloader.js b/lib/style-downloader.js
@@ -1,3 +1,4 @@
+import { check as checkGeoJson } from '@placemarkio/check-geojson'
 import { includeKeys } from 'filter-obj'
 import ky from 'ky'
 import Queue from 'yocto-queue'
@@ -12,15 +13,19 @@ import {
   normalizeStyleURL,
 } from './utils/mapbox.js'
 import { clone, noop } from './utils/misc.js'
-import { assertTileJSON, mapFontStacks } from './utils/style.js'
+import {
+  assertTileJSON,
+  isInlinedSource,
+  mapFontStacks,
+  validateStyle,
+} from './utils/style.js'
 
-/** @import { StyleSpecification } from '@maplibre/maplibre-gl-style-spec' */
-/** @import { TileSource } from './types.js' */
-/** @import { TileInfo, GlyphInfo, GlyphRange, TileFormat } from './writer.js' */
+/** @import { SourceSpecification, StyleSpecification } from '@maplibre/maplibre-gl-style-spec' */
+/** @import { TileInfo, GlyphInfo, GlyphRange } from './writer.js' */
 /** @import { TileDownloadStats } from './tile-downloader.js' */
+/** @import { StyleInlinedSources, InlinedSource } from './types.js'*/
 
 /** @typedef { import('ky').ResponsePromise & { body: ReadableStream<Uint8Array> } } ResponsePromise */
-/** @typedef { import('type-fest').SetRequired<TileSource, 'tiles'> } TileSourceWithTiles */
 /** @import { DownloadResponse } from './utils/fetch.js' */
 
 /**
@@ -38,7 +43,7 @@ export default class StyleDownloader {
   /** @type {null | string} */
   #styleURL = null
   /** @type {null | StyleSpecification} */
-  #style = null
+  #inputStyle = null
   /** @type {FetchQueue} */
   #fetchQueue
   #mapboxAccessToken
@@ -55,8 +60,10 @@ export default class StyleDownloader {
       this.#mapboxAccessToken =
         searchParams.get('access_token') || mapboxAccessToken
       this.#styleURL = normalizeStyleURL(style, this.#mapboxAccessToken)
+    } else if (validateStyle(style)) {
+      this.#inputStyle = clone(style)
     } else {
-      this.#style = clone(style)
+      throw new AggregateError(validateStyle.errors, 'Invalid style')
     }
     this.#fetchQueue = new FetchQueue(concurrency)
   }
@@ -69,61 +76,82 @@ export default class StyleDownloader {
   }
 
   /**
-   * Download the style JSON for this style.
+   * Download the style JSON for this style and inline the sources
    *
-   * @returns {Promise<StyleSpecification>}
+   * @returns {Promise<StyleInlinedSources>}
    */
   async getStyle() {
-    if (!this.#style && this.#styleURL) {
-      this.#style = /** @type {StyleSpecification} */ (
-        await ky(this.#styleURL).json()
-      )
-    } else if (!this.#style) {
+    if (!this.#inputStyle && this.#styleURL) {
+      const downloadedStyle = await ky(this.#styleURL).json()
+      if (!validateStyle(downloadedStyle)) {
+        throw new AggregateError(
+          validateStyle.errors,
+          'Invalid style: ' + this.#styleURL,
+        )
+      }
+      this.#inputStyle = downloadedStyle
+    } else if (!this.#inputStyle) {
       throw new Error('Unexpected state: no style or style URL provided')
     }
-    return this.#style
+    /** @type {{ [_:string]: InlinedSource }} */
+    const inlinedSources = {}
+    for (const [sourceId, source] of Object.entries(this.#inputStyle.sources)) {
+      inlinedSources[sourceId] = await this.#getInlinedSource(source)
+    }
+    return {
+      ...this.#inputStyle,
+      sources: inlinedSources,
+    }
   }
 
   /**
-   * Download info about the sources referenced by this style. Will ignore/skip
-   * sources which are not raster or vector tiles.
-   *
-   * @returns {AsyncGenerator<[string, TileSourceWithTiles]>}
+   * @param {SourceSpecification} source
+   * @returns {Promise<InlinedSource>}
    */
-  async *getSources() {
-    const style = await this.getStyle()
-    for (const sourceId in style.sources) {
-      let source = style.sources[sourceId]
-      if (source.type !== 'raster' && source.type !== 'vector') {
-        continue
+  async #getInlinedSource(source) {
+    if (isInlinedSource(source)) {
+      return source
+    }
+    if (
+      source.type === 'raster' ||
+      source.type === 'vector' ||
+      source.type === 'raster-dem'
+    ) {
+      if (!source.url) {
+        throw new Error('Source is missing both url and tiles properties')
       }
-      if (!source.tiles) {
-        if (!source.url) continue
-        const sourceUrl = normalizeSourceURL(
-          source.url,
-          this.#mapboxAccessToken,
-        )
-        const tilejson = await ky(sourceUrl).json()
-        assertTileJSON(tilejson)
-        Object.assign(
-          source,
-          includeKeys(tilejson, [
-            'bounds',
-            'maxzoom',
-            'minzoom',
-            'tiles',
-            'description',
-            'attribution',
-            'vector_layers',
-          ]),
-        )
+      const sourceUrl = normalizeSourceURL(source.url, this.#mapboxAccessToken)
+      const tilejson = await ky(sourceUrl).json()
+      assertTileJSON(tilejson)
+      return {
+        ...source,
+        ...includeKeys(tilejson, [
+          'bounds',
+          'maxzoom',
+          'minzoom',
+          'tiles',
+          'description',
+          'attribution',
+          'vector_layers',
+        ]),
+      }
+    } else if (source.type === 'geojson') {
+      if (typeof source.data !== 'string') {
+        // Shouldn't get here because of the `isInlineSource()` check above, but
+        // Typescript can't fiture that out.
+        throw new Error('Unexpected data property for GeoJson source')
+      }
+      const geojsonUrl = normalizeSourceURL(
+        source.data,
+        this.#mapboxAccessToken,
+      )
+      const data = checkGeoJson(await ky(geojsonUrl).text())
+      return {
+        ...source,
+        data,
       }
-      yield [
-        sourceId,
-        // @ts-expect-error - we mutate this to add tiles prop
-        source,
-      ]
     }
+    return source
   }
 
   /**
@@ -265,7 +293,13 @@ export default class StyleDownloader {
 
     /** @type {ReturnType<StyleDownloader['getTiles']>} */
     const tiles = (async function* () {
-      for await (const [sourceId, source] of _this.getSources()) {
+      const inlinedStyle = await _this.getStyle()
+      for await (const [sourceId, source] of Object.entries(
+        inlinedStyle.sources,
+      )) {
+        if (source.type !== 'raster' && source.type !== 'vector') {
+          continue
+        }
         // Baseline stats for this source, used in the `onprogress` closure
         // below. Sorry for the hard-to-follow code! `onprogress` can be called
         // after we are already reading the next source, hence the need for a
@@ -275,7 +309,7 @@ export default class StyleDownloader {
           tileUrls: source.tiles,
           bounds,
           maxzoom: Math.min(maxzoom, source.maxzoom || maxzoom),
-          minzoom: source.minzoom || 0,
+          minzoom: source.minzoom,
           sourceBounds: source.bounds,
           boundsBuffer: true,
           scheme: source.scheme,

diff --git a/lib/tile-downloader.js b/lib/tile-downloader.js
@@ -154,24 +154,25 @@ export function downloadTiles({
 /**
  *
  * @param {object} opts
- * @param {import('./utils/geo.js').BBox} opts.bounds
- * @param {import('./utils/geo.js').BBox} opts.sourceBounds
- * @param {boolean} opts.boundsBuffer
- * @param {number} opts.minzoom
+ * @param {import('./utils/geo.js').BBox} [opts.bounds]
+ * @param {import('./utils/geo.js').BBox} [opts.sourceBounds]
+ * @param {boolean} [opts.boundsBuffer]
+ * @param {number} [opts.minzoom]
  * @param {number} opts.maxzoom
  */
-function* tileIterator({
-  bounds,
-  minzoom,
+export function* tileIterator({
+  bounds = [...MAX_BOUNDS],
+  minzoom = 0,
   maxzoom,
   sourceBounds,
-  boundsBuffer,
+  boundsBuffer = false,
 }) {
   const sm = new SphericalMercator({ size: 256 })
   for (let z = minzoom; z <= maxzoom; z++) {
-    let { minX, minY, maxX, maxY } = sm.xyz(bounds, z)
+    // Cloning bounds passed to sm.xyz because no guarantee it won't mutate the array
+    let { minX, minY, maxX, maxY } = sm.xyz([...bounds], z)
     let sourceXYBounds = sourceBounds
-      ? sm.xyz(sourceBounds, z)
+      ? sm.xyz([...sourceBounds], z)
       : { minX, minY, maxX, maxY }
     const buffer = boundsBuffer ? 1 : 0
     minX = Math.max(0, minX - buffer, sourceXYBounds.minX)