Refactor FeatureServer/Output-GeoServices Integration (#999)

* Refactor: use restInfo handler directly in output-geoservice (#967)

* Refactor server info-handler (#974)

* refactor layers-info handler (#975)

* refactor feature server layers-info handler (#976)

* refactor: query handler (#977)

* refactor: generate-renderer and query-related-requests (#997)

* refactor: remove route module (#998)

.changeset/rare-lizards-exercise.md

@@ -0,0 +1,5 @@
+---
+"@koopjs/featureserver": major
+---
+
+- Remove route module and refactor signatures of handlers; consumers can no longer use FeatureServer.route.  If using this directly with Express, you need to define each route and bind to the correct FeatureServer handler.

.github/workflows/pr-tests.yml

@@ -5,11 +5,13 @@ on:
     branches:
       - master
       - beta
+      - next
     paths:
       - "./.github/**.yml"
       - "**/packages/**.js"
       - "**/packages/**/package.json"
       - "test/**/*.js"
+      - "ci/**/*.js"
 
 jobs:
   pr-tests:

.husky/pre-push

@@ -0,0 +1,3 @@
+#!/usr/bin/env sh
+echo "\nRunning code linting...\n"
+npm run lint

README.md

@@ -57,12 +57,12 @@ The Koop dependency graph is shown below.
 ![Screen Shot 2022-11-30 at 1 03 46 PM](https://user-images.githubusercontent.com/4369192/204908289-82659cfe-fcf3-404a-aa70-79baf540f1b8.png)
 
 ### Test Coverage
-Test coverages for each package are shown below. Coverage for some packages includes integration tests as opposed to true unit tests. Difference in coverage is shown below. Our goal is to have complete unit test coverage, and breakout integration/e2e tests separately.
+Test coverages for each package are shown below. Coverage for winnow package includes integration tests as opposed to true unit tests. Our goal is to have complete unit test coverage, and breakout integration/e2e tests separately.
 
 | package | integration + unit | unit |
 |---|---|---|
 |cache-memory|N/A|![coverage](./packages/cache-memory/coverage.svg)|
-|featureserver|![coverage](./packages/featureserver/coverage.svg)|![coverage](./packages/featureserver/coverage-unit.svg)|
+|featureserver|![coverage](./packages/featureserver/coverage.svg)|
 |koop-core|N/A|![coverage](./packages/core/coverage.svg)|
 |logger|N/A|![coverage](./packages/logger/coverage.svg)|
 |output-geoservices|N/A|![coverage](./packages/output-geoservices/coverage.svg)|

ci/format-branch-coverage-changes.js

@@ -1,7 +1,6 @@
 const { writeFileSync, existsSync } = require('fs');
 const json2md = require('json2md');
 const coverageSummary = require('../.coverage_json/coverage-summary.json');
-
 const markdownFilePath = '.branch-coverage-changes.md';
 
 if (!existsSync('.coverage_changes_json/coverage-summary.json')) {
@@ -16,29 +15,43 @@ if (!existsSync('.coverage_changes_json/coverage-summary.json')) {
 }
 
 const coverageChangesSummary = require('../.coverage_changes_json/coverage-summary.json');
+
 const rows = Object.entries(coverageChangesSummary)
   .filter(([filePath]) => {
     return filePath !== 'total';
   })
   .map(([filePath, changesCoverage]) => {
     const packageFilePath = `packages${filePath.split('packages')[1]}`;
     const masterCoverage = coverageSummary[packageFilePath];
+
     return [
       packageFilePath,
-      formatCovComparison(changesCoverage.statements.pct, masterCoverage?.statements?.pct || 0),
-      formatCovComparison(changesCoverage.branches.pct, masterCoverage?.branches?.pct || 0),
-      formatCovComparison(changesCoverage.functions.pct, masterCoverage?.functions?.pct || 0),
-      formatCovComparison(changesCoverage.lines.pct, masterCoverage?.lines?.pct || 0),
+      formatCovComparison(
+        changesCoverage.statements.pct,
+        masterCoverage?.statements?.pct || null,
+      ),
+      formatCovComparison(
+        changesCoverage.branches.pct,
+        masterCoverage?.branches?.pct || null,
+      ),
+      formatCovComparison(
+        changesCoverage.functions.pct,
+        masterCoverage?.functions?.pct || null,
+      ),
+      formatCovComparison(
+        changesCoverage.lines.pct,
+        masterCoverage?.lines?.pct || null,
+      ),
     ];
   });
 
-const headers = ['File Path', 'Statements', 'Branches', 'Functions', 'Lines'];
+const headers = ['File Path', 'Statements', 'Branches', 'Functions', ' Lines '];
 
 const table = json2md([{ h2: 'Coverage Report (change vs master)' }, { table: { headers, rows } }]);
 
 const alignedTable = table.replace(
-  '| --------- | ---------- | -------- | --------- | ----- |',
-  '| :--------- | ----------: | --------: | ---------: | -----: |',
+  '| --------- | --------- | -------- | --------- | --------- |',
+  '| :--------- | ---------: | --------: | ---------: | ---------: |',
 );
 
 const markdown = `[g-img]: https://github.com/koopjs/koop/assets/4369192/fd82d4b7-7f6e-448c-a56c-82ac6781a629
@@ -52,25 +65,29 @@ ${alignedTable}`;
 writeFileSync(markdownFilePath, markdown, 'utf8');
 
 function formatCovComparison(changePct, mainPct) {
-  return `${formatCovPct(changePct)} vs ${formatCovPct(mainPct)}`;
+  return `${formatCovPct(changePct)}<br>vs<br>${formatCovPct(mainPct)}`;
 }
 
 function formatCovPct(pct) {
+  if (!pct) {
+    return '(NA)';
+  }
+
   if (pct === 100) {
-    return `${pct} ![green][g-img]`;
+    return `${pct.toFixed(1)} ![green][g-img]`;
   }
 
   if (pct > 90) {
-    return `${pct} ![yellowGreen][yg-img]`;
+    return `${pct.toFixed(1)} ![yellowGreen][yg-img]`;
   }
 
   if (pct > 80) {
-    return `${pct} ![yellow][y-img]`;
+    return `${pct.toFixed(1)} ![yellow][y-img]`;
   }
 
   if (pct > 70) {
-    return `${pct} ![orange][o-img]`;
+    return `${pct.toFixed(1)} ![orange][o-img]`;
   }
 
-  return `${pct} ![red][r-img]`;
+  return `${pct.toFixed(1)} ![red][r-img]`;
 }

ci/run-coverage-on-branch-changes.js

@@ -62,7 +62,7 @@ function getCovCmd(package, srcFiles) {
 }
 
 function getTestCmd(package) {
-  if (package === 'featureserver' || package === 'winnow') {
+  if (package === 'winnow') {
     return 'npm run test:unit > /dev/null';
   }
 

ci/run-test-coverage-analysis.js

@@ -8,35 +8,35 @@ const context = argv.context || 'all';
 
 workspaces.forEach((workspace) => {
   process.chdir(workspace);
-  const package = workspace.split(path.sep).pop();
-  console.log(`Package "${package}":`);
+  const pkg = workspace.split(path.sep).pop();
+  console.log(`Package "${pkg}":`);
   process.stdout.write(`  - running ${context} test coverage...`);
-  shell.exec(getCovCmd(package, context));
+  shell.exec(getCovCmd(pkg, context));
   process.stdout.write('completed.\n');
   process.stdout.write(`  - generating ${context} test coverage badge...`);
-  shell.exec(getBadgeCmd(package, context));
+  shell.exec(getBadgeCmd(pkg, context));
   process.stdout.write('completed.\n\n');
   process.chdir('../..');
 });
 
-function getCovCmd(package, context) {
-  if (package === 'output-geoservices') {
+function getCovCmd(pkg, context) {
+  if (pkg === 'output-geoservices') {
     return 'npm test -- --coverage --coverageDirectory=.coverage/all --reporters --silent > /dev/null';
   }
 
-  return `npx cross-env SUPPRESS_NO_CONFIG_WARNING=true nyc -r=json-summary -r=json --report-dir=.coverage/all --temp-dir=.coverage/all/analysis ${getTestCmd(package, context)}`;
+  return `npx cross-env SUPPRESS_NO_CONFIG_WARNING=true nyc -r=json-summary -r=json --report-dir=.coverage/all --temp-dir=.coverage/all/analysis ${getTestCmd(pkg, context)}`;
 }
 
-function getBadgeCmd(package, context) {
-  if (context === 'unit' && (package === 'featureserver' || package === 'winnow')) {
+function getBadgeCmd(pkg, context) {
+  if (context === 'unit' && pkg === 'winnow') {
     return 'npx coverage-badges-cli --source .coverage/all/coverage-summary.json --output ./coverage-unit.svg > /dev/null';
   }
 
   return 'npx coverage-badges-cli --source .coverage/all/coverage-summary.json --output ./coverage.svg > /dev/null';
 }
 
-function getTestCmd(package, context) {
-  if (context === 'unit' && (package === 'featureserver' || package === 'winnow')) {
+function getTestCmd(pkg, context) {
+  if (context === 'unit' && pkg === 'winnow') {
     return 'npm run test:unit > /dev/null';
   }
 

packages/featureserver/README.md

@@ -14,26 +14,20 @@ const app = express() // set up a basic express server
 const FeatureServer = require('@koopjs/featureserver')
 const cache = require('cache')
 
-// We only need one handler because FeatureServer.route is going to do all the work
+
 const handler = (req, res) => {
   cache.get(/* some geojson */, (err, data) => {
     if (err) res.status(500).json({error: err.message})
-    else FeatureServer.route(req, res, data)
+    else FeatureServer.query(req, res, data)
   })
 }
 
-// Sets up all of the handled routes to support `GET` and `POST`
-const routes = ['/FeatureServer', '/FeatureServer/layers', '/FeatureServer/:layer', '/FeatureServer/:layer/:method']
-
-routes.forEach(route => {
-  app.route(route)
+app.route('/FeatureServer/:layer/query')
   .get(handler)
   .post(handler)
-})
 ```
 
 ## API
-* [FeatureServer.route](#featureserver.route)
 * [FeatureServer.query](#featureserver.query)
 * [FeatureServer.restInfo](#featureserver.serverInfo)
 * [FeatureServer.serverInfo](#featureserver.serverInfo)
@@ -43,83 +37,6 @@ routes.forEach(route => {
 * [FeatureServer.queryRelatedRecords](#featureserver.queryRelatedRecords)
 * [FeatureServer.setDefaults](#featureserver.setDefaults)
 
-### FeatureServer.route
-Pass in an `incoming request object`, an `outgoing response object`, a `geojson` object, and `options` and this function will route and return a geoservices compliant response
-
-- Supports: '/FeatureServer', '/FeatureServer/layers', '/FeatureServer/:layer', '/FeatureServer/:layer/:method'
-	- _Note_: only `query`, `info`, and `generateRenderer` are supported methods at this time.
-
-```js
-FeatureServer.route(req, res, data, options)
-```
-
-- **data** is either a geojson object extended with some additional properties or an object with a layers property which an array of extended geojson objects. These properties are optional and can be used to provide more specific metadata or to shortcut the built in filtering mechanism.
-
-e.g.
-
-```js
-{
-  type: 'FeatureCollection' // Static
-  features: Array, // GeoJSON features
-  statistics: Object, // pass statistics to an outStatistics request to or else they will be calculated from geojson features passed in
-  metadata: {
-    id: number, // The unique layer id.  If supplied for one layer, you should supply for all layers to avoid multiple layers having the same id.
-    name: String, // The name of the layer
-    description: String, // The description of the layer
-    copyrightText: String, // The copyright text (layer attribution text)
-    extent: Array, // valid extent array e.g. [[180,90],[-180,-90]]
-    displayField: String, // The display field to be used by a client
-    geometryType: String // REQUIRED if no features are returned with this object Point || MultiPoint || LineString || MultiLineString || Polygon || MultiPolygon
-    idField: String, // unique identifier field,
-    maxRecordCount: Number, // the maximum number of features a provider can return at once
-    limitExceeded: Boolean, // whether or not the server has limited the features returned
-    timeInfo: Object, // describes the time extent and capabilities of the layer,
-    transform: Object, // describes a quantization transformation
-    renderer: Object, // provider can over-ride default symbology of FeatureServer output with a renderer object. See https://developers.arcgis.com/web-map-specification/objects/simpleRenderer, for object specification.
-    defaultVisibility: boolean, // The default visibility of this layer
-    minScale: number, // The minScale value for this layer
-    maxScale: number, // The maxScale value for this layer
-    fields: [
-     { // Subkeys are optional
-       name: String,
-       type: String, // 'Date' || 'Double' || 'Integer' || 'String'
-       alias: String, // how should clients display this field name,
-     }
-    ],
-    supportedQueryFormats: String | Array // 'JSON,geojson' || ['JSON', 'geojson']
-  },
-  capabilities: {
-    quantization: Boolean // True if the provider supports quantization
-  },
-  filtersApplied: {
-    all: Boolean // true if all post processing should be skipped
-    geometry: Boolean, // true if a geometric filter has already been applied to the data
-    where: Boolean, // true if a sql-like where filter has already been applied to the data
-    offset: Boolean // true if the result offset has already been applied to the data,
-    limit: Boolean // true if the result count has already been limited,
-    projection: Boolean // true if the result data has already been projected
-  }
-  count: Number // pass count if the number of features in a query has been pre-calculated
-}
-```
-
-or
-
-```js
-{
-  layers: [
-    {
-      type: 'FeatureCollection'
-      ...
-    },
-    {
-      type: 'FeatureCollection'
-      ...
-    }
-]
-```
-- **options** is an object that dictates method actions. See `FeatureServer.query` and `FeatureServer.generateRenderer` for more details.
-
 ### FeatureServer.query
 Pass in `geojson` and `options` (a valid [geoservices query object](https://geoservices.github.io/query.html)), and the function will perform the query and return a valid geoservices query object. The in addition to input `statistics: {}`, following is an example of _all_ query `options` that can be passed into the query route: '/FeatureServer/:layer/query'
 

packages/featureserver/package.json

@@ -7,8 +7,7 @@
     "test": "test"
   },
   "scripts": {
-    "test": "mocha '**/*.spec.js' --reporter even-more-min --recursive -t 5000",
-    "test:unit": "mocha 'src/**/*.spec.js' --reporter even-more-min --recursive -t 5000"
+    "test": "mocha '**/*.spec.js' --reporter even-more-min --recursive -t 5000"
   },
   "contributors": [
     {