From d9ad2b7417f1ca5e975ef9e84a411036ec2af61d Mon Sep 17 00:00:00 2001 From: Rich Gwozdz Date: Tue, 26 Mar 2024 17:42:38 -0700 Subject: [PATCH] fix: add exceededTransferLimit to PBF encoding (#958) * fix: add exceededTransferLimit to PBF encoding * chore: add e2e tests * chore: fix ci on dep-bot PRs --- .changeset/tall-icons-dream.md | 5 ++ .github/workflows/pr-tests.yml | 10 +-- .../send-pbf/transform-features-for-pbf.js | 3 +- .../transform-features-for-pbf.spec.js | 1 + packages/winnow/src/query/standard-query.js | 6 +- test/geoservice-pbf.spec.js | 49 ++------------- test/geoservice-query.spec.js | 62 +++++++++++++++++++ ...w-metadata-exceeded-transfer-limit.geojson | 48 ++++++++++++++ 8 files changed, 128 insertions(+), 56 deletions(-) create mode 100644 .changeset/tall-icons-dream.md create mode 100644 test/provider-data/points-w-metadata-exceeded-transfer-limit.geojson diff --git a/.changeset/tall-icons-dream.md b/.changeset/tall-icons-dream.md new file mode 100644 index 000000000..ca4cbcadc --- /dev/null +++ b/.changeset/tall-icons-dream.md @@ -0,0 +1,5 @@ +--- +"@koopjs/featureserver": patch +--- + +- add exceededTransferLimit to PBF encoding diff --git a/.github/workflows/pr-tests.yml b/.github/workflows/pr-tests.yml index 47e733836..9053bc03d 100644 --- a/.github/workflows/pr-tests.yml +++ b/.github/workflows/pr-tests.yml @@ -50,11 +50,11 @@ jobs: run: npm run test:e2e - name: Run Coverage - if: ${{ matrix.os == 'ubuntu-latest' }} + if: ${{ matrix.os == 'ubuntu-latest' && github.triggering_actor != 'dependabot[bot]' }} run: npm run cov:changes:json && npm run cov:changes:2:md - - name: Find previous coverage comment - if: ${{ matrix.os == 'ubuntu-latest' }} + - name: Find previous coverage report comment + if: ${{ matrix.os == 'ubuntu-latest' && github.triggering_actor != 'dependabot[bot]' }} uses: peter-evans/find-comment@v1 id: findcomment with: @@ -62,8 +62,8 @@ jobs: comment-author: 'github-actions[bot]' body-includes: Coverage Report - - name: Create or update comment - if: ${{ matrix.os == 'ubuntu-latest' }} + - name: Create or update previous coverage report comment + if: ${{ matrix.os == 'ubuntu-latest' && github.triggering_actor != 'dependabot[bot]' }} uses: peter-evans/create-or-update-comment@v4 with: comment-id: ${{ steps.findcomment.outputs.comment-id }} diff --git a/packages/featureserver/src/response-handlers/helpers/send-pbf/transform-features-for-pbf.js b/packages/featureserver/src/response-handlers/helpers/send-pbf/transform-features-for-pbf.js index 50f756c21..82fba08b0 100644 --- a/packages/featureserver/src/response-handlers/helpers/send-pbf/transform-features-for-pbf.js +++ b/packages/featureserver/src/response-handlers/helpers/send-pbf/transform-features-for-pbf.js @@ -4,7 +4,7 @@ const { transformToPbfAttributes } = require('./transform-to-pbf-attributes'); const { transformToPbfGeometry } = require('./transform-to-pbf-geometry'); function transformFeaturesForPbf(json, quantizationParameters) { - const { objectIdFieldName, uniqueIdField, geometryType, spatialReference } = json; + const { objectIdFieldName, uniqueIdField, geometryType, spatialReference, exceededTransferLimit } = json; const fields = _.orderBy(json.fields, ['name'], ['asc']); const geometryTransform = getGeometryTransform(spatialReference, quantizationParameters); @@ -19,6 +19,7 @@ function transformFeaturesForPbf(json, quantizationParameters) { spatialReference, fields: fields.map(({name, alias, type}) => ({ name, alias, fieldType: type})), features, + exceededTransferLimit, transform: geometryTransform, geometryType: geometryType ? geometryType.replace('esriGeometry', 'esriGeometryType') : 'esriGeometryTypeNone' }; diff --git a/packages/featureserver/src/response-handlers/helpers/send-pbf/transform-features-for-pbf.spec.js b/packages/featureserver/src/response-handlers/helpers/send-pbf/transform-features-for-pbf.spec.js index a5552a760..61f8f9bff 100644 --- a/packages/featureserver/src/response-handlers/helpers/send-pbf/transform-features-for-pbf.spec.js +++ b/packages/featureserver/src/response-handlers/helpers/send-pbf/transform-features-for-pbf.spec.js @@ -90,6 +90,7 @@ const transformedFixture = { }, }, ], + exceededTransferLimit: true }; describe('transform features for PBF', () => { diff --git a/packages/winnow/src/query/standard-query.js b/packages/winnow/src/query/standard-query.js index 9e2fc45fb..2585d575d 100644 --- a/packages/winnow/src/query/standard-query.js +++ b/packages/winnow/src/query/standard-query.js @@ -10,11 +10,7 @@ function standardQuery(features, sqlString, options = {}) { // 1) For GeoService API queries there is always a limit // 2) options.limit is incremented by one in normalizeOptions.js; if filtered.length === options.limit, original limit option has been exceeded - if ( - options.skipLimitHandling || - !options.limit || - filtered.length !== limit - ) { + if (options.skipLimitHandling || !limit || filtered.length !== limit) { return packageFeatures(filtered, options); } diff --git a/test/geoservice-pbf.spec.js b/test/geoservice-pbf.spec.js index 1ad8ffac9..b2cce1a64 100644 --- a/test/geoservice-pbf.spec.js +++ b/test/geoservice-pbf.spec.js @@ -21,7 +21,7 @@ describe('koop', () => { try { const response = await request(koop.server) .get( - '/file-geojson/rest/services/points-w-objectid/FeatureServer/0/query?f=pbf', + '/file-geojson/rest/services/points-w-objectid/FeatureServer/0/query?f=pbf&resultRecordCount=1', ) .responseType('blob') .buffer(); @@ -34,10 +34,11 @@ describe('koop', () => { expect(response.headers['content-type']).toBe( 'application/x-protobuf', ); - expect(response.headers['content-length']).toBe('337'); + expect(response.headers['content-length']).toBe('229'); expect(pbfJson).toEqual({ queryResult: { featureResult: { + exceededTransferLimit: true, objectIdFieldName: 'OBJECTID', uniqueIdField: { name: 'OBJECTID', isSystemMaintained: true }, geometryType: 0, @@ -73,49 +74,7 @@ describe('koop', () => { { low: 769803776, high: -6, unsigned: false }, ], }, - }, - { - attributes: [ - { uintValue: 2 }, - { stringValue: 'pinto' }, - { stringValue: 'Fireman' }, - { - sint64Value: { - low: 1865197776, - high: 369, - unsigned: false, - }, - }, - ], - geometry: { - lengths: [2], - coords: [ - { low: 259084288, high: -28, unsigned: false }, - { low: -2050327040, high: -11, unsigned: false }, - ], - }, - }, - { - attributes: [ - { uintValue: 3 }, - { stringValue: 'draft' }, - { stringValue: 'Workhorse' }, - { - sint64Value: { - low: -1455179568, - high: 332, - unsigned: false, - }, - }, - ], - geometry: { - lengths: [2], - coords: [ - { low: -1215752192, high: -24, unsigned: false }, - { low: -1345294336, high: -10, unsigned: false }, - ], - }, - }, + } ], }, }, diff --git a/test/geoservice-query.spec.js b/test/geoservice-query.spec.js index 59f633121..f5e495a7b 100644 --- a/test/geoservice-query.spec.js +++ b/test/geoservice-query.spec.js @@ -186,6 +186,68 @@ describe('koop', () => { } }); }); + + describe('exceededTransferLimit', () => { + test('should be calculated by Koop and equal true', async () => { + try { + const response = await request(koop.server).get( + '/file-geojson/rest/services/points-w-objectid/FeatureServer/0/query?resultRecordCount=2', + ); + expect(response.status).toBe(200); + const { features, exceededTransferLimit } = response.body; + expect(features.length).toBe(2); + expect(exceededTransferLimit).toBe(true); + } catch (error) { + console.error(error); + throw error; + } + }); + + test('should be calculated by Koop and equal false', async () => { + try { + const response = await request(koop.server).get( + '/file-geojson/rest/services/points-w-objectid/FeatureServer/0/query', + ); + expect(response.status).toBe(200); + const { features, exceededTransferLimit } = response.body; + expect(features.length).toBe(3); + expect(exceededTransferLimit).toBe(false); + } catch (error) { + console.error(error); + throw error; + } + }); + + test('should be acquired from provider metadata', async () => { + try { + const response = await request(koop.server).get( + '/file-geojson/rest/services/points-w-metadata-exceeded-transfer-limit/FeatureServer/0/query?resultRecordCount=2', + ); + expect(response.status).toBe(200); + const { features, exceededTransferLimit } = response.body; + expect(features.length).toBe(2); + expect(exceededTransferLimit).toBe(true); + } catch (error) { + console.error(error); + throw error; + } + }); + + test('should be acquired from provider metadata', async () => { + try { + const response = await request(koop.server).get( + '/file-geojson/rest/services/points-w-metadata-exceeded-transfer-limit/FeatureServer/0/query', + ); + expect(response.status).toBe(200); + const { features, exceededTransferLimit } = response.body; + expect(features.length).toBe(3); + expect(exceededTransferLimit).toBe(true); + } catch (error) { + console.error(error); + throw error; + } + }); + }); }); }); }); diff --git a/test/provider-data/points-w-metadata-exceeded-transfer-limit.geojson b/test/provider-data/points-w-metadata-exceeded-transfer-limit.geojson new file mode 100644 index 000000000..3532cf218 --- /dev/null +++ b/test/provider-data/points-w-metadata-exceeded-transfer-limit.geojson @@ -0,0 +1,48 @@ +{ + "type": "FeatureCollection", + "metadata": { + "idField": "id", + "exceededTransferLimit": true + }, + "features": [ + { + "type": "Feature", + "properties": { + "id": "aaa", + "timestamp": "2023-04-10T16:15:30.000Z", + "label": "White Leg", + "category": "pinto" + }, + "geometry": { + "type": "Point", + "coordinates": [-80, 25] + } + }, + { + "type": "Feature", + "properties": { + "id": "bbb", + "timestamp": "2020-04-12T16:15:30.000Z", + "label": "Fireman", + "category": "pinto" + }, + "geometry": { + "type": "Point", + "coordinates": [-120, 45] + } + }, + { + "type": "Feature", + "properties": { + "id": "ccc", + "timestamp": "2015-04-11T16:15:30.000Z", + "label": "Workhorse", + "category": "draft" + }, + "geometry": { + "type": "Point", + "coordinates": [-100, 40] + } + } + ] +}