From 19d379ee7259760d92ad1ad8fa6051fe8a5553c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Ronzon?= Date: Sun, 28 Jun 2026 14:21:35 -0600 Subject: [PATCH 1/2] feat(geoloc): expose per-entrance coordinates in networks endpoint - Rewrite NETWORKS_IN_BOUNDS SQL with CTE for stable centroid and entrance list - Network qualification: caves with >1 non-deleted entrances (sensitive count) - Response includes entrances array with id, name, latitude, longitude per entrance - Centroid computed from all non-sensitive, non-deleted entrances (stable across pans) - Add property-based tests for formatNetworks grouping and centroid math - Update integration tests for new response shape - Update Swagger documentation for /geoloc/networks Closes #1697 --- api/services/GeoLocService.js | 83 +++++++--- assets/swaggerV1.yaml | 53 +++++-- .../1_services/GeoLocNameFilter.test.js | 8 +- .../GeoLocNetworks.property.test.js | 145 ++++++++++++++++++ .../1_services/GeoLocService.test.js | 49 +++++- test/integration/4_routes/Geoloc.test.js | 43 +++++- 6 files changed, 343 insertions(+), 38 deletions(-) create mode 100644 test/integration/1_services/GeoLocNetworks.property.test.js diff --git a/api/services/GeoLocService.js b/api/services/GeoLocService.js index dea4de542..7f870e713 100644 --- a/api/services/GeoLocService.js +++ b/api/services/GeoLocService.js @@ -84,17 +84,44 @@ const PUBLIC_ENTRANCES_COORDINATES_IN_BOUNDS_AND_MASSIF = ` `; const NETWORKS_IN_BOUNDS = ` - SELECT c.id as id, COALESCE(nc.name, ne.name) as name, avg(en.longitude) as longitude, avg(en.latitude) as latitude - FROM t_entrance as en - INNER JOIN t_cave c ON c.id = en.id_cave + WITH qualifying_networks AS ( + SELECT c.id AS cave_id + FROM t_cave AS c + INNER JOIN t_entrance AS en ON en.id_cave = c.id + WHERE en.is_deleted = false + AND c.is_deleted = false + GROUP BY c.id + HAVING count(en.id) > 1 + AND bool_or( + en.is_sensitive = false + AND ST_Within(en.point_geom, ST_MakeEnvelope($1, $2, $3, $4, 4326)) + ) + ) + SELECT + c.id AS id, + COALESCE(nc.name, nc_first_ent.name) AS name, + avg(en.longitude) OVER (PARTITION BY c.id) AS longitude, + avg(en.latitude) OVER (PARTITION BY c.id) AS latitude, + en.id AS entrance_id, + ne.name AS entrance_name, + en.longitude AS entrance_longitude, + en.latitude AS entrance_latitude + FROM qualifying_networks AS qn + INNER JOIN t_cave AS c ON c.id = qn.cave_id + INNER JOIN t_entrance AS en ON en.id_cave = c.id LEFT JOIN t_name AS nc ON nc.id_cave = c.id AND nc.is_main = true AND nc.is_deleted = false - LEFT JOIN t_name as ne ON ne.id_entrance = en.id AND ne.is_main = true AND ne.is_deleted = false - WHERE ST_Within(en.point_geom, ST_MakeEnvelope($1, $2, $3, $4, 4326)) - AND en.is_sensitive = false - AND en.is_deleted = false - AND c.is_deleted = false - GROUP BY c.id, COALESCE(nc.name, ne.name) - HAVING count(en.id_cave) > 1 + LEFT JOIN t_name AS ne ON ne.id_entrance = en.id AND ne.is_main = true AND ne.is_deleted = false + LEFT JOIN LATERAL ( + SELECT n2.name + FROM t_entrance AS e2 + JOIN t_name AS n2 ON n2.id_entrance = e2.id AND n2.is_main = true AND n2.is_deleted = false + WHERE e2.id_cave = c.id AND e2.is_deleted = false AND e2.is_sensitive = false + ORDER BY e2.id ASC + LIMIT 1 + ) AS nc_first_ent ON nc.name IS NULL + WHERE en.is_sensitive = false + AND en.is_deleted = false + ORDER BY c.id, en.id; `; const PUBLIC_NETWORKS_COORDINATES_IN_BOUNDS = ` @@ -160,16 +187,34 @@ const NameService = require('./NameService'); const { getQualityData } = require('../utils/computeEntranceDataQuality'); /** - * return a light version of the networks - * @param networks + * Group flat entrance rows into network objects with an entrances array. + * Each row has: id, name, longitude, latitude (network-level centroid), + * entrance_id, entrance_name, entrance_longitude, entrance_latitude. + * @param rows */ -const formatNetworks = (networks) => - networks.map((network) => ({ - id: network.id, - name: network.name, - longitude: Number(network.longitude), - latitude: Number(network.latitude), - })); +const formatNetworks = (rows) => { + const networksMap = new Map(); + + for (const row of rows) { + if (!networksMap.has(row.id)) { + networksMap.set(row.id, { + id: row.id, + name: row.name, + longitude: Number(row.longitude), + latitude: Number(row.latitude), + entrances: [], + }); + } + networksMap.get(row.id).entrances.push({ + id: row.entrance_id, + name: row.entrance_name || null, + latitude: Number(row.entrance_latitude), + longitude: Number(row.entrance_longitude), + }); + } + + return Array.from(networksMap.values()); +}; /** * Format the quality entrances in a lighter version diff --git a/assets/swaggerV1.yaml b/assets/swaggerV1.yaml index 3df143d83..8c10b143f 100644 --- a/assets/swaggerV1.yaml +++ b/assets/swaggerV1.yaml @@ -5086,7 +5086,11 @@ paths: get: tags: - geoloc - description: Count entrances contained inside the given coords + description: >- + Returns networks (caves with multiple entrances) that have at least one + non-sensitive entrance within the given bounding box. Each network includes + a stable centroid (average of all non-sensitive entrance coordinates) and an + entrances array with per-entrance coordinates. parameters: - $ref: '#/components/parameters/sw_lat' - $ref: '#/components/parameters/sw_lng' @@ -5098,16 +5102,43 @@ paths: content: application/json: schema: - type: object - properties: - id: - type: integer - name: - type: string - longitude: - type: number - latitude: - type: number + type: array + items: + type: object + properties: + id: + type: integer + description: Cave ID + name: + type: string + description: Network name (cave name or first entrance name) + longitude: + type: number + description: Centroid longitude (average of all non-sensitive entrance longitudes) + latitude: + type: number + description: Centroid latitude (average of all non-sensitive entrance latitudes) + entrances: + type: array + description: All non-sensitive, non-deleted entrances of the network + items: + type: object + properties: + id: + type: integer + description: Entrance ID + name: + type: string + nullable: true + description: Main entrance name (null if no name defined) + latitude: + type: number + description: Entrance latitude + longitude: + type: number + description: Entrance longitude + '400': + description: Missing or invalid bounding box parameters '/geoloc/networksCoordinates': get: diff --git a/test/integration/1_services/GeoLocNameFilter.test.js b/test/integration/1_services/GeoLocNameFilter.test.js index c7316028a..750400443 100644 --- a/test/integration/1_services/GeoLocNameFilter.test.js +++ b/test/integration/1_services/GeoLocNameFilter.test.js @@ -31,9 +31,6 @@ describe('GeoLocService - name join is_main filter fix', () => { describe('getNetworksMap() no row multiplication', () => { it('should return at most one row per network (no name duplication)', async () => { - // Networks require multiple entrances sharing the same cave. - // Current fixtures have no multi-entrance caves, so this verifies - // the query runs without error and returns no duplicates if any exist. const networks = await GeoLocService.getNetworksMap( southWestBound, northEastBound @@ -46,6 +43,11 @@ describe('GeoLocService - name join is_main filter fix', () => { uniqueIds.length, 'Duplicate network IDs found — name join is causing row multiplication' ); + // Verify new shape: each network has entrances array + for (const network of networks) { + should(network).have.property('entrances'); + should(network.entrances).be.an.Array(); + } } }); }); diff --git a/test/integration/1_services/GeoLocNetworks.property.test.js b/test/integration/1_services/GeoLocNetworks.property.test.js new file mode 100644 index 000000000..a59d6d6fc --- /dev/null +++ b/test/integration/1_services/GeoLocNetworks.property.test.js @@ -0,0 +1,145 @@ +const should = require('should'); +const fc = require('fast-check'); + +/** + * Inline replica of the formatNetworks grouping logic for property testing. + * This allows us to test the pure function independently of the SQL layer. + */ +const formatNetworks = (rows) => { + const networksMap = new Map(); + + for (const row of rows) { + if (!networksMap.has(row.id)) { + networksMap.set(row.id, { + id: row.id, + name: row.name, + longitude: Number(row.longitude), + latitude: Number(row.latitude), + entrances: [], + }); + } + networksMap.get(row.id).entrances.push({ + id: row.entrance_id, + name: row.entrance_name || null, + latitude: Number(row.entrance_latitude), + longitude: Number(row.entrance_longitude), + }); + } + + return Array.from(networksMap.values()); +}; + +/** + * Arbitrary: generate a set of flat rows as would come from the SQL query. + * Each network has 2+ entrances. The centroid (longitude/latitude) is the + * average of entrance coordinates — replicated here to match the window function. + */ +const networkRowsArbitrary = fc + .array( + fc.record({ + caveName: fc.option(fc.string({ minLength: 1, maxLength: 50 }), { + nil: undefined, + }), + entrances: fc.array( + fc.record({ + id: fc.integer({ min: 1, max: 100000 }), + name: fc.option(fc.string({ minLength: 1, maxLength: 50 }), { + nil: null, + }), + latitude: fc.double({ min: -90, max: 90, noNaN: true }), + longitude: fc.double({ min: -180, max: 180, noNaN: true }), + }), + { minLength: 2, maxLength: 10 } + ), + }), + { minLength: 1, maxLength: 5 } + ) + .map((networks) => { + const rows = []; + networks.forEach((net, idx) => { + const caveId = idx + 1; // unique cave IDs + const avgLat = + net.entrances.reduce((s, e) => s + e.latitude, 0) / + net.entrances.length; + const avgLng = + net.entrances.reduce((s, e) => s + e.longitude, 0) / + net.entrances.length; + const name = net.caveName || `Network ${caveId}`; + for (const ent of net.entrances) { + rows.push({ + id: caveId, + name, + longitude: avgLng, + latitude: avgLat, + entrance_id: ent.id, + entrance_name: ent.name, + entrance_longitude: ent.longitude, + entrance_latitude: ent.latitude, + }); + } + }); + return rows; + }); + +describe('GeoLocService formatNetworks - Property Tests', () => { + describe('Property 1: every network has more than one entrance', () => { + it('should produce networks each with at least 2 entrances', function () { + this.timeout(10000); + fc.assert( + fc.property(networkRowsArbitrary, (rows) => { + const networks = formatNetworks(rows); + for (const network of networks) { + should(network.entrances.length).be.greaterThan(1); + } + }), + { numRuns: 100 } + ); + }); + }); + + describe('Property 2: centroid equals arithmetic mean of entrance coordinates', () => { + it('should have centroid matching average of entrance lat/lng', function () { + this.timeout(10000); + fc.assert( + fc.property(networkRowsArbitrary, (rows) => { + const networks = formatNetworks(rows); + for (const network of networks) { + const expectedLat = + network.entrances.reduce((s, e) => s + e.latitude, 0) / + network.entrances.length; + const expectedLng = + network.entrances.reduce((s, e) => s + e.longitude, 0) / + network.entrances.length; + should(network.latitude).be.approximately(expectedLat, 1e-10); + should(network.longitude).be.approximately(expectedLng, 1e-10); + } + }), + { numRuns: 100 } + ); + }); + }); + + describe('Property 6: backwards-compatible output shape', () => { + it('should always include id, name, longitude, latitude, entrances', function () { + this.timeout(10000); + fc.assert( + fc.property(networkRowsArbitrary, (rows) => { + const networks = formatNetworks(rows); + for (const network of networks) { + should(network).have.property('id'); + should(network).have.property('name'); + should(network).have.property('longitude'); + should(network).have.property('latitude'); + should(network).have.property('entrances'); + should(network.entrances).be.an.Array(); + for (const entrance of network.entrances) { + should(entrance).have.property('id'); + should(entrance).have.properties('name', 'latitude', 'longitude'); + } + } + }), + { numRuns: 100 } + ); + }); + }); +}); diff --git a/test/integration/1_services/GeoLocService.test.js b/test/integration/1_services/GeoLocService.test.js index e111fb5cc..1fbb02380 100644 --- a/test/integration/1_services/GeoLocService.test.js +++ b/test/integration/1_services/GeoLocService.test.js @@ -163,14 +163,57 @@ describe('GeoLocService', () => { }); describe('getNetworksMap()', () => { - it('should get networks for map', async () => { - const southWestBound = { lat: 40, lng: 5 }; - const northEastBound = { lat: 50, lng: 10 }; + it('should get networks for map with entrances array', async () => { + // Cave 1 has entrances 1 (lat 62.8, lng 78.5) and 2 (lat 62.9, lng 78.6) + const southWestBound = { lat: 60, lng: 75 }; + const northEastBound = { lat: 65, lng: 80 }; const networks = await GeoLocService.getNetworksMap( southWestBound, northEastBound ); should(networks).be.an.Array(); + should(networks.length).be.greaterThan(0); + + const network = networks.find((n) => n.id === 1); + should(network).not.be.undefined(); + should(network).have.property('id', 1); + should(network).have.property('name'); + should(network).have.property('longitude'); + should(network).have.property('latitude'); + should(network).have.property('entrances'); + should(network.entrances).be.an.Array(); + should(network.entrances.length).be.greaterThan(1); + + for (const entrance of network.entrances) { + should(entrance).have.property('id'); + should(entrance).have.property('name'); + should(entrance).have.property('latitude'); + should(entrance).have.property('longitude'); + should(entrance.id).be.a.Number(); + should(entrance.latitude).be.a.Number(); + should(entrance.longitude).be.a.Number(); + } + }); + + it('should compute centroid as average of entrance coordinates', async () => { + const southWestBound = { lat: 60, lng: 75 }; + const northEastBound = { lat: 65, lng: 80 }; + const networks = await GeoLocService.getNetworksMap( + southWestBound, + northEastBound + ); + const network = networks.find((n) => n.id === 1); + should(network).not.be.undefined(); + + const avgLat = + network.entrances.reduce((sum, e) => sum + e.latitude, 0) / + network.entrances.length; + const avgLng = + network.entrances.reduce((sum, e) => sum + e.longitude, 0) / + network.entrances.length; + + should(network.latitude).be.approximately(avgLat, 0.0001); + should(network.longitude).be.approximately(avgLng, 0.0001); }); it('should return empty array for area with no networks', async () => { diff --git a/test/integration/4_routes/Geoloc.test.js b/test/integration/4_routes/Geoloc.test.js index 40031fc2c..2dd7ce7a7 100644 --- a/test/integration/4_routes/Geoloc.test.js +++ b/test/integration/4_routes/Geoloc.test.js @@ -176,7 +176,40 @@ describe('Geoloc features', () => { .expect(400, done); }); - it('should return code 200 with networks', (done) => { + it('should return code 200 with networks containing entrances', (done) => { + supertest(sails.hooks.http.app) + .get('/api/v1/geoloc/networks') + .set('Content-type', 'application/json') + .set('Accept', 'application/json') + .query({ + sw_lat: 60, + sw_lng: 75, + ne_lat: 65, + ne_lng: 80, + }) + .expect(200) + .end((err, res) => { + if (err) return done(err); + should(res.body).be.an.Array(); + should(res.body.length).be.greaterThan(0); + const network = res.body[0]; + should(network).have.property('id'); + should(network).have.property('name'); + should(network).have.property('longitude'); + should(network).have.property('latitude'); + should(network).have.property('entrances'); + should(network.entrances).be.an.Array(); + should(network.entrances.length).be.greaterThan(0); + const entrance = network.entrances[0]; + should(entrance).have.property('id'); + should(entrance).have.property('name'); + should(entrance).have.property('latitude'); + should(entrance).have.property('longitude'); + return done(); + }); + }); + + it('should return empty array for area with no networks', (done) => { supertest(sails.hooks.http.app) .get('/api/v1/geoloc/networks') .set('Content-type', 'application/json') @@ -187,7 +220,13 @@ describe('Geoloc features', () => { ne_lat: 5, ne_lng: 5, }) - .expect(200, done); + .expect(200) + .end((err, res) => { + if (err) return done(err); + should(res.body).be.an.Array(); + should(res.body.length).equal(0); + return done(); + }); }); }); From a9397323c0ace3edb145b45c9dbf9ef30f996c8e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Ronzon?= Date: Sun, 28 Jun 2026 17:07:27 -0600 Subject: [PATCH 2/2] fix(geoloc): address PR review items for networks endpoint - Replace cave-name LEFT JOIN with LATERAL LIMIT 1 to prevent row duplication - Export formatNetworks from GeoLocService and import in property tests - Add nullable: true to network-level name in swagger spec - Remove trailing semicolon from NETWORKS_IN_BOUNDS for style consistency - Use fc.uniqueArray with selector for entrance ID uniqueness in property tests --- api/services/GeoLocService.js | 11 +++++- assets/swaggerV1.yaml | 1 + .../GeoLocNetworks.property.test.js | 38 +++---------------- 3 files changed, 16 insertions(+), 34 deletions(-) diff --git a/api/services/GeoLocService.js b/api/services/GeoLocService.js index 7f870e713..d4d682897 100644 --- a/api/services/GeoLocService.js +++ b/api/services/GeoLocService.js @@ -109,7 +109,13 @@ const NETWORKS_IN_BOUNDS = ` FROM qualifying_networks AS qn INNER JOIN t_cave AS c ON c.id = qn.cave_id INNER JOIN t_entrance AS en ON en.id_cave = c.id - LEFT JOIN t_name AS nc ON nc.id_cave = c.id AND nc.is_main = true AND nc.is_deleted = false + LEFT JOIN LATERAL ( + SELECT n.name + FROM t_name AS n + WHERE n.id_cave = c.id AND n.is_main = true AND n.is_deleted = false + ORDER BY n.id ASC + LIMIT 1 + ) AS nc ON true LEFT JOIN t_name AS ne ON ne.id_entrance = en.id AND ne.is_main = true AND ne.is_deleted = false LEFT JOIN LATERAL ( SELECT n2.name @@ -121,7 +127,7 @@ const NETWORKS_IN_BOUNDS = ` ) AS nc_first_ent ON nc.name IS NULL WHERE en.is_sensitive = false AND en.is_deleted = false - ORDER BY c.id, en.id; + ORDER BY c.id, en.id `; const PUBLIC_NETWORKS_COORDINATES_IN_BOUNDS = ` @@ -289,6 +295,7 @@ const checkAndGetMassifParam = async (req, res) => { // ==================================== module.exports = { + formatNetworks, checkAndGetMassifParam, checkAndGetCoordinatesParams: (req) => { let errorMessage = ''; diff --git a/assets/swaggerV1.yaml b/assets/swaggerV1.yaml index 8c10b143f..0e4f155ce 100644 --- a/assets/swaggerV1.yaml +++ b/assets/swaggerV1.yaml @@ -5111,6 +5111,7 @@ paths: description: Cave ID name: type: string + nullable: true description: Network name (cave name or first entrance name) longitude: type: number diff --git a/test/integration/1_services/GeoLocNetworks.property.test.js b/test/integration/1_services/GeoLocNetworks.property.test.js index a59d6d6fc..3728da2bb 100644 --- a/test/integration/1_services/GeoLocNetworks.property.test.js +++ b/test/integration/1_services/GeoLocNetworks.property.test.js @@ -1,38 +1,12 @@ const should = require('should'); const fc = require('fast-check'); - -/** - * Inline replica of the formatNetworks grouping logic for property testing. - * This allows us to test the pure function independently of the SQL layer. - */ -const formatNetworks = (rows) => { - const networksMap = new Map(); - - for (const row of rows) { - if (!networksMap.has(row.id)) { - networksMap.set(row.id, { - id: row.id, - name: row.name, - longitude: Number(row.longitude), - latitude: Number(row.latitude), - entrances: [], - }); - } - networksMap.get(row.id).entrances.push({ - id: row.entrance_id, - name: row.entrance_name || null, - latitude: Number(row.entrance_latitude), - longitude: Number(row.entrance_longitude), - }); - } - - return Array.from(networksMap.values()); -}; +const { formatNetworks } = require('../../../api/services/GeoLocService'); /** * Arbitrary: generate a set of flat rows as would come from the SQL query. - * Each network has 2+ entrances. The centroid (longitude/latitude) is the - * average of entrance coordinates — replicated here to match the window function. + * Each network has 2+ entrances with unique IDs (matching SQL primary key + * guarantee). The centroid (longitude/latitude) is the average of entrance + * coordinates — replicated here to match the window function. */ const networkRowsArbitrary = fc .array( @@ -40,7 +14,7 @@ const networkRowsArbitrary = fc caveName: fc.option(fc.string({ minLength: 1, maxLength: 50 }), { nil: undefined, }), - entrances: fc.array( + entrances: fc.uniqueArray( fc.record({ id: fc.integer({ min: 1, max: 100000 }), name: fc.option(fc.string({ minLength: 1, maxLength: 50 }), { @@ -49,7 +23,7 @@ const networkRowsArbitrary = fc latitude: fc.double({ min: -90, max: 90, noNaN: true }), longitude: fc.double({ min: -180, max: 180, noNaN: true }), }), - { minLength: 2, maxLength: 10 } + { minLength: 2, maxLength: 10, selector: (e) => e.id } ), }), { minLength: 1, maxLength: 5 }