From 14e1a6093590aa1b4fdb3c53c0df577111ee3e7f Mon Sep 17 00:00:00 2001 From: Hanbyul Jo Date: Mon, 2 Dec 2024 08:57:14 +0900 Subject: [PATCH] fix: Polygon simplification on E&A page (#1268) ### Description of Changes Increase the limit of the number of each aoi polygon. Separate out functions that handles the aoi validation for easy test and better readability. Fix error handling. Add unit test for aoi validation. --- .../map/controls/hooks/use-custom-aoi.test.ts | 195 ++++++++++++++++++ .../map/controls/hooks/use-custom-aoi.tsx | 179 +++++++++------- 2 files changed, 302 insertions(+), 72 deletions(-) create mode 100644 app/scripts/components/common/map/controls/hooks/use-custom-aoi.test.ts diff --git a/app/scripts/components/common/map/controls/hooks/use-custom-aoi.test.ts b/app/scripts/components/common/map/controls/hooks/use-custom-aoi.test.ts new file mode 100644 index 000000000..c51222616 --- /dev/null +++ b/app/scripts/components/common/map/controls/hooks/use-custom-aoi.test.ts @@ -0,0 +1,195 @@ +import { Feature, MultiPolygon, Polygon } from '@turf/helpers'; +import { + getNumPoints, + validateGeometryType, + extractPolygonsFromGeojson, + validateFeatureCount, + removePolygonHoles, + simplifyFeatures, + getAoiAppropriateFeatures, + PolygonGeojson, + eachFeatureMaxPointNum, + maxPolygonNum, + INVALID_GEOMETRY_ERROR, + TOO_MANY_POLYGONS_ERROR +} from './use-custom-aoi'; + +// Mock data +const mockPolygon: Feature = { + type: 'Feature', + geometry: { + type: 'Polygon', + coordinates: [ + [ + [0, 0], + [1, 1], + [2, 2], + [0, 0] + ] + ] + }, + properties: {} +}; + +const mockMultiPolygon: Feature = { + type: 'Feature', + geometry: { + type: 'MultiPolygon', + coordinates: [ + [ + [ + [0, 0], + [1, 1], + [2, 2], + [0, 0] + ] + ] + ] + }, + properties: {} +}; + +const mockGeojson = { + type: 'FeatureCollection', + features: [mockPolygon, mockMultiPolygon] +} as PolygonGeojson; + +describe('getNumPoints', () => { + it('should return the number of points in a polygon', () => { + const numPoints = getNumPoints(mockPolygon); + expect(numPoints).toBe(4); + }); +}); + +describe('validateGeometryType', () => { + it('should validate that all features are polygons or multipolygons', () => { + const isValid = validateGeometryType(mockGeojson); + expect(isValid).toBe(true); + }); + + it('should return false for invalid geometry types', () => { + const invalidGeojson = { + ...mockGeojson, + features: [ + { + type: 'Feature', + geometry: { type: 'Point', coordinates: [0, 0] }, + properties: {} + } + ] + }; + // @ts-expect-error: testing invalid case + const isValid = validateGeometryType(invalidGeojson); + expect(isValid).toBe(false); + }); +}); + +describe('extractPolygonsFromGeojson', () => { + it('should extract all polygons from GeoJSON, including from MultiPolygons', () => { + const polygons = extractPolygonsFromGeojson(mockGeojson); + expect(polygons).toHaveLength(2); + expect(polygons[0].geometry.type).toBe('Polygon'); + }); +}); + +describe('validateFeatureCount', () => { + it('should validate that the number of features is within the allowed limit', () => { + const isValid = validateFeatureCount([mockPolygon]); + expect(isValid).toBe(true); + }); + + it('should return false if feature count exceeds the limit', () => { + const tooManyFeatures = Array(maxPolygonNum + 1).fill(mockPolygon); + const isValid = validateFeatureCount(tooManyFeatures); + expect(isValid).toBe(false); + }); +}); + +describe('removePolygonHoles', () => { + it('should remove holes from polygons and return warnings if holes are removed', () => { + const polygonWithHole = { + ...mockPolygon, + geometry: { + ...mockPolygon.geometry, + coordinates: [ + [ + [0, 0], + [1, 1], + [2, 2], + [0, 0] + ], + [ + [0.1, 0.1], + [0.2, 0.2], + [0.3, 0.3], + [0.1, 0.1] + ] + ] + } + }; + const { simplifiedFeatures, warnings } = removePolygonHoles([ + polygonWithHole + ]); + expect(simplifiedFeatures[0].geometry.coordinates).toHaveLength(1); + expect(warnings).toContain( + 'Polygons with rings are not supported and were simplified to remove them' + ); + }); +}); + +describe('simplifyFeatures', () => { + it('should simplify features to reduce the number of points if necessary', () => { + const largePolygon = { + ...mockPolygon, + geometry: { + ...mockPolygon.geometry, + coordinates: [ + Array.from({ length: 600 }, (_, i) => [ + Math.cos(i * ((2 * Math.PI) / 600)), + Math.sin(i * ((2 * Math.PI) / 600)) + ]).concat([[1, 0]]) // Ensure the ring is closed + ] + } + }; + const { simplifiedFeatures, warnings } = simplifyFeatures([largePolygon]); + expect(getNumPoints(simplifiedFeatures[0])).toBeLessThanOrEqual( + eachFeatureMaxPointNum + ); + expect(warnings).toContainEqual(expect.stringContaining('simplified')); + }); +}); + +describe('getAoiAppropriateFeatures', () => { + it('should process GeoJSON and return simplified features with warnings', () => { + const result = getAoiAppropriateFeatures(mockGeojson); + expect(result.simplifiedFeatures).toHaveLength(2); + expect(result.warnings).toEqual(expect.any(Array)); + }); + + it('should throw an error if geometry type validation fails', () => { + const invalidGeojson = { + ...mockGeojson, + features: [ + { + type: 'Feature', + geometry: { type: 'Point', coordinates: [0, 0] }, + properties: {} + } + ] + }; + // @ts-expect-error: testing invalid case + expect(() => getAoiAppropriateFeatures(invalidGeojson)).toThrow( + INVALID_GEOMETRY_ERROR + ); + }); + + it('should throw an error if feature count exceeds the allowed limit', () => { + const tooManyFeatures = { + ...mockGeojson, + features: Array(maxPolygonNum + 1).fill(mockPolygon) + }; + expect(() => getAoiAppropriateFeatures(tooManyFeatures)).toThrow( + TOO_MANY_POLYGONS_ERROR + ); + }); +}); diff --git a/app/scripts/components/common/map/controls/hooks/use-custom-aoi.tsx b/app/scripts/components/common/map/controls/hooks/use-custom-aoi.tsx index 85fc68a13..82cc2dfce 100644 --- a/app/scripts/components/common/map/controls/hooks/use-custom-aoi.tsx +++ b/app/scripts/components/common/map/controls/hooks/use-custom-aoi.tsx @@ -7,42 +7,48 @@ import { multiPolygonToPolygons } from '../../utils'; import { round } from '$utils/format'; const extensions = ['geojson', 'json', 'zip']; -const eachFeatureMaxPointNum = 100; +const maxTolerance = 5; + +export const eachFeatureMaxPointNum = 500; +export const maxPolygonNum = 200; export const acceptExtensions = extensions.map((ext) => `.${ext}`).join(', '); +export const INVALID_GEOMETRY_ERROR = + 'Wrong geometry type. Only polygons or multi polygons are accepted.'; +export const TOO_MANY_POLYGONS_ERROR = + 'Only files with up to 200 polygons are accepted.'; +const RING_POLYGON_WARNING = + 'Polygons with rings are not supported and were simplified to remove them'; +const TOLERANCE_ACCELERATOR = 1.5; + export interface FileInfo { name: string; extension: string; type: 'Shapefile' | 'GeoJSON'; } -interface PolygonGeojson { - "type": "FeatureCollection", - "features": Feature[]; +export interface PolygonGeojson { + type: 'FeatureCollection'; + features: Feature[]; } -function getNumPoints(feature: Feature): number { +export function getNumPoints(feature: Feature): number { return feature.geometry.coordinates.reduce((acc, current) => { return acc + current.length; }, 0); } -export function getAoiAppropriateFeatures(geojson: PolygonGeojson) { - - let warnings: string[] = []; - - if ( - geojson.features.some( - (feature) => - !['MultiPolygon', 'Polygon'].includes(feature.geometry.type) - ) - ) { - throw new Error( - 'Wrong geometry type. Only polygons or multi polygons are accepted.' - ); - } +export function validateGeometryType(geojson: PolygonGeojson): boolean { + const hasInvalidGeometry = geojson.features.some( + (feature) => !['MultiPolygon', 'Polygon'].includes(feature.geometry.type) + ); + return !hasInvalidGeometry; +} - const features: Feature[] = geojson.features.reduce( +export function extractPolygonsFromGeojson( + geojson: PolygonGeojson +): Feature[] { + return geojson.features.reduce( (acc: Feature[], feature: Feature) => { if (feature.geometry.type === 'MultiPolygon') { return acc.concat( @@ -53,22 +59,18 @@ export function getAoiAppropriateFeatures(geojson: PolygonGeojson) { }, [] ); +} - if (features.length > 200) { - throw new Error('Only files with up to 200 polygons are accepted.'); - } - - // Simplify features; - const originalTotalFeaturePoints = features.reduce( - (acc, f) => acc + getNumPoints(f), - 0 - ); - let numPoints = originalTotalFeaturePoints; - let tolerance = 0.001; +export function validateFeatureCount(features: Feature[]): boolean { + return features.length <= maxPolygonNum; +} - // Remove holes from polygons as they're not supported. +export function removePolygonHoles(features: Feature[]): { + simplifiedFeatures: Feature[]; + warnings: string[]; +} { let polygonHasRings = false; - let simplifiedFeatures = features.map>((feature) => { + const simplifiedFeatures = features.map>((feature) => { if (feature.geometry.coordinates.length > 1) { polygonHasRings = true; return { @@ -79,69 +81,103 @@ export function getAoiAppropriateFeatures(geojson: PolygonGeojson) { } }; } - return feature; }); - if (polygonHasRings) { - warnings = [ - ...warnings, - 'Polygons with rings are not supported and were simplified to remove them' - ]; - } + // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition + const warnings = polygonHasRings ? [RING_POLYGON_WARNING] : []; + return { simplifiedFeatures, warnings }; +} - // Simplify each feature if needed to reduce point count to less than 50 points per feature - simplifiedFeatures = features.map((feature) => { +export function simplifyFeatures(features: Feature[]): { + simplifiedFeatures: Feature[]; + warnings: string[]; +} { + const warnings: string[] = []; + let tolerance = 0.001; + // 1. Simplify each feature if needed to reduce point count to ${eachFeatureMaximumPointsNum} + const simplifiedFeatures = features.map((feature) => { const numPoints = getNumPoints(feature); - if (numPoints > 30) { - let tolerance = 0.001; + if (numPoints > eachFeatureMaxPointNum) { let simplifiedFeature = feature; - // Continuously simplify the feature until it has less than or equal to 30 points - while (getNumPoints(simplifiedFeature) > eachFeatureMaxPointNum && tolerance < 5) { + while ( + getNumPoints(simplifiedFeature) > eachFeatureMaxPointNum && + tolerance < maxTolerance + ) { simplifiedFeature = simplify(simplifiedFeature, { tolerance }); - tolerance *= 2; // Increase tolerance to simplify more aggressively if needed + tolerance *= TOLERANCE_ACCELERATOR; } return simplifiedFeature; } return feature; }); - // Add a warning if any feature has been simplified to less than 30 points - const numberOfSimplifedFeatures = simplifiedFeatures.filter((feature, index) => { - return getNumPoints(feature) < getNumPoints(features[index]); - }).length; + const simplifiedCount = simplifiedFeatures.filter( + (feature, index) => getNumPoints(feature) < getNumPoints(features[index]) + ).length; - if (numberOfSimplifedFeatures > 0) { - const featureWPrefix = numberOfSimplifedFeatures === 1? 'feature was': 'features were'; + if (simplifiedCount > 0) { + const featureText = simplifiedCount === 1 ? 'feature was' : 'features were'; // eslint-disable-next-line fp/no-mutating-methods - warnings= [...warnings, `${numberOfSimplifedFeatures} ${featureWPrefix} simplified to have less than ${eachFeatureMaxPointNum} points.`]; + warnings.push( + `${simplifiedCount} ${featureText} simplified to have less than ${eachFeatureMaxPointNum} points.` + ); } + const originalTotalPoints = features.reduce( + (acc, f) => acc + getNumPoints(f), + 0 + ); + let totalPoints = simplifiedFeatures.reduce( + (acc, f) => acc + getNumPoints(f), + 0 + ); + // Further Simplify features in case there are a lot of features - // so the sum of the points doesn't exceed 1000 - while (numPoints > 1000 && tolerance < 5) { - simplifiedFeatures = simplifiedFeatures.map((feature) => - simplify(feature, { tolerance }) - ); - numPoints = simplifiedFeatures.reduce( + // to control the number of the total points (10000 for now) + while (totalPoints > 10000 && tolerance < maxTolerance) { + simplifiedFeatures.forEach((feature, i) => { + simplifiedFeatures[i] = simplify(feature, { tolerance }); + }); + totalPoints = simplifiedFeatures.reduce( (acc, f) => acc + getNumPoints(f), 0 ); - tolerance = Math.min(tolerance * 1.8, 5); + tolerance = Math.min(tolerance * TOLERANCE_ACCELERATOR, 5); } - if (originalTotalFeaturePoints !== numPoints) { - warnings = [ - ...warnings, + if (originalTotalPoints !== totalPoints) { + // eslint-disable-next-line fp/no-mutating-methods + warnings.push( `The geometry has been simplified (${round( - (1 - numPoints / originalTotalFeaturePoints) * 100 + (1 - totalPoints / originalTotalPoints) * 100 )} % less).` - ]; + ); } + return { simplifiedFeatures, warnings }; +} + +export function getAoiAppropriateFeatures(geojson: PolygonGeojson) { + const geometryValidated = validateGeometryType(geojson); + if (!geometryValidated) { + throw new Error(INVALID_GEOMETRY_ERROR); + } + + const features = extractPolygonsFromGeojson(geojson); + const featureCountValidated = validateFeatureCount(features); + if (!featureCountValidated) { + throw new Error(TOO_MANY_POLYGONS_ERROR); + } + const { simplifiedFeatures: noHolesFeatures, warnings: holeWarnings } = + removePolygonHoles(features); + + const { simplifiedFeatures, warnings: simplificationWarnings } = + simplifyFeatures(noHolesFeatures); + return { simplifiedFeatures, - warnings + warnings: [...holeWarnings, ...simplificationWarnings] }; } @@ -163,7 +199,6 @@ function useCustomAoI() { const onLoad = async () => { if (!reader.current) return; - let geojson; if (typeof reader.current.result === 'string') { const rawGeoJSON = reader.current.result; @@ -185,13 +220,13 @@ function useCustomAoI() { return; } } - if (!geojson?.features?.length) { - setError('Error uploading file: Invalid GeoJSON'); + setError('Error uploading file: Invalid data'); return; } try { - const { simplifiedFeatures, warnings } = getAoiAppropriateFeatures(geojson); + const { simplifiedFeatures, warnings } = + getAoiAppropriateFeatures(geojson); setUploadFileWarnings(warnings); setUploadFileError(null); setFeatures( @@ -201,7 +236,7 @@ function useCustomAoI() { })) ); } catch (e) { - setError(e); + setError(e.message); return; } };