From c1ab8f74d7929cc2105ac9732c8b3df32d11e5fb Mon Sep 17 00:00:00 2001 From: James Milner Date: Sun, 15 Dec 2024 22:44:31 +0000 Subject: [PATCH] feat: no longer throw error on validation failures in addFeatures and return validation status --- development/src/index.ts | 19 ++- e2e/src/index.ts | 12 +- guides/2.STORE.md | 17 ++- guides/4.MODES.md | 7 +- src/common.ts | 5 +- .../angled-rectangle.mode.spec.ts | 21 +-- .../angled-rectangle/angled-rectangle.mode.ts | 33 +++-- src/modes/base.mode.ts | 52 +++++++- src/modes/circle/circle.mode.spec.ts | 17 ++- src/modes/circle/circle.mode.ts | 32 +++-- src/modes/freehand/freehand.mode.spec.ts | 17 ++- src/modes/freehand/freehand.mode.ts | 26 ++-- src/modes/linestring/linestring.mode.spec.ts | 23 ++-- src/modes/linestring/linestring.mode.ts | 27 ++-- src/modes/point/point.mode.spec.ts | 19 ++- src/modes/point/point.mode.ts | 21 ++- src/modes/polygon/polygon.mode.spec.ts | 33 +++-- src/modes/polygon/polygon.mode.ts | 25 ++-- src/modes/rectangle/rectangle.mode.spec.ts | 22 +++- src/modes/rectangle/rectangle.mode.ts | 28 ++-- src/modes/render/render.mode.spec.ts | 10 +- src/modes/render/render.mode.ts | 27 +++- src/modes/sector/sector.mode.spec.ts | 24 +++- src/modes/sector/sector.mode.ts | 33 +++-- .../drag-coordinate-resize.behavior.spec.ts | 2 +- .../drag-coordinate-resize.behavior.ts | 2 +- .../drag-coordinate.behavior.spec.ts | 6 +- .../behaviors/drag-coordinate.behavior.ts | 2 +- .../behaviors/drag-feature.behavior.spec.ts | 14 +- .../select/behaviors/drag-feature.behavior.ts | 2 +- src/modes/select/select.mode.ts | 6 +- src/modes/sensor/sensor.mode.spec.ts | 19 ++- src/modes/sensor/sensor.mode.ts | 39 +++--- src/store/store-feature-validation.ts | 26 +++- src/store/store-validation.spec.ts | 121 ++++++++++-------- src/store/store.spec.ts | 101 +++++++++------ src/store/store.ts | 86 +++++++++---- src/terra-draw.spec.ts | 92 +++++++++++-- src/terra-draw.ts | 61 +++++---- 39 files changed, 761 insertions(+), 368 deletions(-) diff --git a/development/src/index.ts b/development/src/index.ts index 160dc3b0..fafa727e 100644 --- a/development/src/index.ts +++ b/development/src/index.ts @@ -150,10 +150,17 @@ const getModes = () => { feature: { validation: (feature) => { if (feature.geometry.type !== "Polygon") { - return false; + return { + valid: false, + reason: "Feature is not a valid Polygon feature", + }; } - return ValidateMinAreaSquareMeters(feature, 1000); + const result = ValidateMinAreaSquareMeters(feature, 1000); + return { + valid: result, + reason: result ? undefined : "Area too small", + }; }, draggable: true, coordinates: { @@ -182,9 +189,13 @@ const getModes = () => { snapping: true, validation: (feature, { updateType }) => { if (updateType === "finish" || updateType === "commit") { - return ValidateNotSelfIntersecting(feature); + const valid = ValidateNotSelfIntersecting(feature); + return { + valid, + reason: valid ? undefined : "Polygon must not self-intersect", + }; } - return true; + return { valid: true }; }, }), new TerraDrawRectangleMode(), diff --git a/e2e/src/index.ts b/e2e/src/index.ts index 93aaff85..7e45cb4c 100644 --- a/e2e/src/index.ts +++ b/e2e/src/index.ts @@ -71,12 +71,16 @@ const example = { this.config?.includes("validationSuccess") || this.config?.includes("validationFailure") ? (feature) => { - return ValidateMaxAreaSquareMeters( + const result = ValidateMaxAreaSquareMeters( feature, this.config?.includes("validationFailure") ? 1000000 : 2000000, ); + return { + valid: result, + reason: result ? undefined : "Area too large", + }; } : undefined, draggable: true, @@ -151,12 +155,16 @@ const example = { this.config?.includes("validationSuccess") || this.config?.includes("validationFailure") ? (feature) => { - return ValidateMaxAreaSquareMeters( + const result = ValidateMaxAreaSquareMeters( feature, this.config?.includes("validationFailure") ? 1000000 : 2000000, ); + return { + valid: result, + reason: "Area too large", + }; } : undefined, }), diff --git a/guides/2.STORE.md b/guides/2.STORE.md index fe438782..7c0930b8 100644 --- a/guides/2.STORE.md +++ b/guides/2.STORE.md @@ -63,7 +63,7 @@ Features can also be added to the Store programmatically using the `addFeatures` ```javascript // Add a Point to the Store -draw.addFeatures([ +const result = draw.addFeatures([ { type: "Feature", geometry: { @@ -75,6 +75,21 @@ draw.addFeatures([ }, }, ]); + +console.log(result) + +// Will log: +// [{ +// id: 'aa5c8826-cbf1-4489-9b31-987c871866af', +// valid: true, +// }] +``` + +`addFeatures` returns an array of objects providing information about the added features such as the id and the feature was valid. Only valid features are added to the store. This allows developers to handle failure cases how they see fit: + +```typescript +const invalidFeatures = result.filter(({ valid }) => !valid); +// Do something with the information ``` If you want to get an ID for a feature and create that outside Terra Draw to do something with it later, you can use the `getFeatureId` method on the Terra Draw instance, like so: diff --git a/guides/4.MODES.md b/guides/4.MODES.md index d2c887da..1726de2a 100644 --- a/guides/4.MODES.md +++ b/guides/4.MODES.md @@ -370,7 +370,7 @@ It is possible to select and deselect a feature via the draw instance, which use draw.start(); // Add feature programmatically - draw.addFeatures([ + const result = draw.addFeatures([ { id: "f8e5a38d-ecfa-4294-8461-d9cff0e0d7f8", type: "Feature", @@ -384,6 +384,11 @@ It is possible to select and deselect a feature via the draw instance, which use }, ]); + // Log out if the feature passed validation or not + if (!result[0].valid) { + throw new Error(result.reason ? result.reason : 'Feature f8e5a38d-ecfa-4294-8461-d9cff0e0d7f8 was invalid') + } + // Select a given feature draw.selectFeature("f8e5a38d-ecfa-4294-8461-d9cff0e0d7f8"); diff --git a/src/common.ts b/src/common.ts index 40b19aae..dca598e3 100644 --- a/src/common.ts +++ b/src/common.ts @@ -103,7 +103,10 @@ type ValidationContext = Pick< export type Validation = ( feature: GeoJSONStoreFeatures, context: ValidationContext, -) => boolean; +) => { + valid: boolean; + reason?: string; +}; export type TerraDrawModeState = | "unregistered" diff --git a/src/modes/angled-rectangle/angled-rectangle.mode.spec.ts b/src/modes/angled-rectangle/angled-rectangle.mode.spec.ts index 0132d2ae..81184b59 100644 --- a/src/modes/angled-rectangle/angled-rectangle.mode.spec.ts +++ b/src/modes/angled-rectangle/angled-rectangle.mode.spec.ts @@ -119,7 +119,7 @@ describe("TerraDrawAngledRectangleMode", () => { store = new GeoJSONStore(); angledRectangleMode = new TerraDrawAngledRectangleMode({ validation: () => { - return true; + return { valid: true }; }, }); const mockConfig = MockModeConfig(angledRectangleMode.mode); @@ -190,7 +190,7 @@ describe("TerraDrawAngledRectangleMode", () => { describe("with successful validation", () => { beforeEach(() => { angledRectangleMode = new TerraDrawAngledRectangleMode({ - validation: () => true, + validation: () => ({ valid: true }), }); const mockConfig = MockModeConfig(angledRectangleMode.mode); @@ -224,7 +224,7 @@ describe("TerraDrawAngledRectangleMode", () => { describe("with unsuccessful validation", () => { beforeEach(() => { angledRectangleMode = new TerraDrawAngledRectangleMode({ - validation: () => false, + validation: () => ({ valid: false, reason: "Test" }), }); const mockConfig = MockModeConfig(angledRectangleMode.mode); @@ -380,7 +380,7 @@ describe("TerraDrawAngledRectangleMode", () => { describe("validateFeature", () => { it("returns true for valid rectangle feature with validation that returns true", () => { const rectangleMode = new TerraDrawAngledRectangleMode({ - validation: () => true, + validation: () => ({ valid: true }), }); rectangleMode.register(MockModeConfig("angled-rectangle")); @@ -405,14 +405,14 @@ describe("TerraDrawAngledRectangleMode", () => { updatedAt: 1685655518118, }, }), - ).toBe(true); + ).toEqual({ + valid: true, + }); }); it("returns false for valid rectangle feature but with validation that returns false", () => { const rectangleMode = new TerraDrawAngledRectangleMode({ - validation: () => { - return false; - }, + validation: () => ({ valid: false, reason: "Test" }), }); rectangleMode.register(MockModeConfig("angled-rectangle")); @@ -437,7 +437,10 @@ describe("TerraDrawAngledRectangleMode", () => { updatedAt: 1685655518118, }, }), - ).toBe(false); + ).toEqual({ + reason: "Test", + valid: false, + }); }); }); diff --git a/src/modes/angled-rectangle/angled-rectangle.mode.ts b/src/modes/angled-rectangle/angled-rectangle.mode.ts index b9f54020..26346332 100644 --- a/src/modes/angled-rectangle/angled-rectangle.mode.ts +++ b/src/modes/angled-rectangle/angled-rectangle.mode.ts @@ -12,11 +12,19 @@ import { TerraDrawBaseDrawMode, BaseModeOptions, CustomStyling, + ModeMismatchValidationFailure, } from "../base.mode"; import { coordinatesIdentical } from "../../geometry/coordinates-identical"; import { getDefaultStyling } from "../../util/styling"; -import { FeatureId, GeoJSONStoreFeatures } from "../../store/store"; -import { ValidatePolygonFeature } from "../../validations/polygon.validation"; +import { + FeatureId, + GeoJSONStoreFeatures, + StoreValidation, +} from "../../store/store"; +import { + ValidateNonIntersectingPolygonFeature, + ValidatePolygonFeature, +} from "../../validations/polygon.validation"; import { webMercatorDestination } from "../../geometry/measure/destination"; import { webMercatorBearing } from "../../geometry/measure/bearing"; import { midpointCoordinate } from "../../geometry/midpoint-coordinate"; @@ -258,7 +266,7 @@ export class TerraDrawAngledRectangleMode extends TerraDrawBaseDrawMode + ValidateNonIntersectingPolygonFeature( + baseValidatedFeature, + this.coordinatePrecision, + ), + "Feature is not a valid simple Polygon feature", + ); } } diff --git a/src/modes/base.mode.ts b/src/modes/base.mode.ts index d6f7e224..472ab15c 100644 --- a/src/modes/base.mode.ts +++ b/src/modes/base.mode.ts @@ -34,6 +34,8 @@ export enum ModeTypes { Render = "render", } +type BaseValidationResult = { valid: boolean; reason?: string }; + export type BaseModeOptions = { styles?: Partial; pointerDistance?: number; @@ -41,6 +43,11 @@ export type BaseModeOptions = { projection?: Projection; }; +export const ModeMismatchValidationFailure = { + valid: false, + reason: "Feature mode property does not match the mode being added to", +}; + export abstract class TerraDrawBaseDrawMode { protected _state: TerraDrawModeState; get state() { @@ -150,7 +157,11 @@ export abstract class TerraDrawBaseDrawMode { } } - validateFeature(feature: unknown): feature is GeoJSONStoreFeatures { + validateFeature(feature: unknown): BaseValidationResult { + return this.performFeatureValidation(feature); + } + + private performFeatureValidation(feature: unknown): BaseValidationResult { if (this._state === "unregistered") { throw new Error("Mode must be registered"); } @@ -162,15 +173,50 @@ export abstract class TerraDrawBaseDrawMode { // We also want tp validate based on any specific valdiations passed in if (this.validate) { - return this.validate(feature as GeoJSONStoreFeatures, { + const validation = this.validate(feature as GeoJSONStoreFeatures, { project: this.project, unproject: this.unproject, coordinatePrecision: this.coordinatePrecision, updateType: UpdateTypes.Provisional, }); + + return { + // validatedFeature: feature as GeoJSONStoreFeatures, + valid: validStoreFeature.valid && validation.valid, + reason: validation.reason, + }; + } + + return { + // validatedFeature: feature as GeoJSONStoreFeatures, + valid: validStoreFeature.valid, + reason: validStoreFeature.reason, + }; + } + + protected validateModeFeature( + feature: unknown, + modeValidationFn: (feature: GeoJSONStoreFeatures) => boolean, + defaultError: string, + ): BaseValidationResult { + const validation = this.performFeatureValidation(feature); + if (validation.valid) { + const validatedFeature = feature as GeoJSONStoreFeatures; + const matches = validatedFeature.properties.mode === this.mode; + if (!matches) { + return ModeMismatchValidationFailure; + } + const modeValidation = modeValidationFn(validatedFeature); + return { + valid: modeValidation, + reason: modeValidation ? undefined : defaultError, + }; } - return validStoreFeature; + return { + valid: false, + reason: validation.reason, + }; } abstract start(): void; diff --git a/src/modes/circle/circle.mode.spec.ts b/src/modes/circle/circle.mode.spec.ts index 7c600421..74f1a833 100644 --- a/src/modes/circle/circle.mode.spec.ts +++ b/src/modes/circle/circle.mode.spec.ts @@ -219,7 +219,7 @@ describe("TerraDrawCircleMode", () => { beforeEach(() => { circleMode = new TerraDrawCircleMode({ - validation: () => valid, + validation: () => ({ valid }), }); const mockConfig = MockModeConfig(circleMode.mode); @@ -570,7 +570,10 @@ describe("TerraDrawCircleMode", () => { updatedAt: 1685568435434, }, }), - ).toBe(false); + ).toEqual({ + reason: "Feature is not a valid simple Polygon feature", + valid: false, + }); }); it("returns true for valid circle feature", () => { @@ -666,13 +669,15 @@ describe("TerraDrawCircleMode", () => { updatedAt: 1685568435434, }, }), - ).toBe(true); + ).toEqual({ + valid: true, + }); }); it("returns false for valid circle feature but with validation that returns false", () => { const circleMode = new TerraDrawCircleMode({ validation: () => { - return false; + return { valid: false }; }, styles: { fillColor: "#ffffff", @@ -765,7 +770,9 @@ describe("TerraDrawCircleMode", () => { updatedAt: 1685568435434, }, }), - ).toBe(false); + ).toEqual({ + valid: false, + }); }); }); }); diff --git a/src/modes/circle/circle.mode.ts b/src/modes/circle/circle.mode.ts index 7cbfd4e2..c17d3e6b 100644 --- a/src/modes/circle/circle.mode.ts +++ b/src/modes/circle/circle.mode.ts @@ -11,11 +11,16 @@ import { } from "../../common"; import { haversineDistanceKilometers } from "../../geometry/measure/haversine-distance"; import { circle, circleWebMercator } from "../../geometry/shape/create-circle"; -import { FeatureId, GeoJSONStoreFeatures } from "../../store/store"; +import { + FeatureId, + GeoJSONStoreFeatures, + StoreValidation, +} from "../../store/store"; import { getDefaultStyling } from "../../util/styling"; import { BaseModeOptions, CustomStyling, + ModeMismatchValidationFailure, TerraDrawBaseDrawMode, } from "../base.mode"; import { ValidateNonIntersectingPolygonFeature } from "../../validations/polygon.validation"; @@ -104,7 +109,7 @@ export class TerraDrawCircleMode extends TerraDrawBaseDrawMode(finishedId); - const valid = this.validate( + const validationResult = this.validate( { type: "Feature", id: finishedId, @@ -119,7 +124,7 @@ export class TerraDrawCircleMode extends TerraDrawBaseDrawMode + ValidateNonIntersectingPolygonFeature( + baseValidatedFeature, + this.coordinatePrecision, + ), + "Feature is not a valid simple Polygon feature", + ); } private updateCircle(event: TerraDrawMouseEvent) { @@ -335,7 +341,7 @@ export class TerraDrawCircleMode extends TerraDrawBaseDrawMode { beforeEach(() => { freehandMode = new TerraDrawFreehandMode({ validation: () => { - return valid; + return { valid }; }, }); store = new GeoJSONStore(); @@ -738,7 +738,10 @@ describe("TerraDrawFreehandMode", () => { updatedAt: 1685568435434, }, }), - ).toBe(false); + ).toEqual({ + valid: false, + reason: "Feature mode property does not match the mode being added to", + }); }); it("returns true for valid freehand feature", () => { @@ -781,13 +784,15 @@ describe("TerraDrawFreehandMode", () => { updatedAt: 1685569593386, }, }), - ).toBe(true); + ).toEqual({ + valid: true, + }); }); it("returns false for valid freehand feature but the validate function returns false", () => { const freehandMode = new TerraDrawFreehandMode({ validation: () => { - return false; + return { valid: false }; }, styles: { closingPointColor: "#ffffff", @@ -827,7 +832,9 @@ describe("TerraDrawFreehandMode", () => { updatedAt: 1685569593386, }, }), - ).toBe(false); + ).toEqual({ + valid: false, + }); }); }); }); diff --git a/src/modes/freehand/freehand.mode.ts b/src/modes/freehand/freehand.mode.ts index 52c0c087..2ff3a892 100644 --- a/src/modes/freehand/freehand.mode.ts +++ b/src/modes/freehand/freehand.mode.ts @@ -16,7 +16,11 @@ import { TerraDrawBaseDrawMode, } from "../base.mode"; import { getDefaultStyling } from "../../util/styling"; -import { FeatureId, GeoJSONStoreFeatures } from "../../store/store"; +import { + FeatureId, + GeoJSONStoreFeatures, + StoreValidation, +} from "../../store/store"; import { cartesianDistance } from "../../geometry/measure/pixel-distance"; import { ValidatePolygonFeature } from "../../validations/polygon.validation"; @@ -133,7 +137,7 @@ export class TerraDrawFreehandMode extends TerraDrawBaseDrawMode + ValidatePolygonFeature(baseValidatedFeature, this.coordinatePrecision), + "Feature is not a valid Polygon feature", + ); } } diff --git a/src/modes/linestring/linestring.mode.spec.ts b/src/modes/linestring/linestring.mode.spec.ts index f1dddee5..f9b4bb32 100644 --- a/src/modes/linestring/linestring.mode.spec.ts +++ b/src/modes/linestring/linestring.mode.spec.ts @@ -311,9 +311,9 @@ describe("TerraDrawLineStringMode", () => { lineStringMode = new TerraDrawLineStringMode({ validation: (feature, { updateType }) => { if (updateType === "finish" || updateType === "commit") { - return ValidateNotSelfIntersecting(feature); + return { valid: ValidateNotSelfIntersecting(feature) }; } - return true; + return { valid: true }; }, }); @@ -385,9 +385,9 @@ describe("TerraDrawLineStringMode", () => { lineStringMode = new TerraDrawLineStringMode({ validation: (feature, { updateType }) => { if (updateType === "finish" || updateType === "commit") { - return ValidateNotSelfIntersecting(feature); + return { valid: ValidateNotSelfIntersecting(feature) }; } - return true; + return { valid: true }; }, }); @@ -827,7 +827,10 @@ describe("TerraDrawLineStringMode", () => { updatedAt: 1685654950609, }, }), - ).toBe(false); + ).toEqual({ + reason: "Feature is not a valid LineString feature", + valid: false, + }); }); it("returns true for valid linestring feature", () => { @@ -855,13 +858,15 @@ describe("TerraDrawLineStringMode", () => { updatedAt: 1685654950609, }, }), - ).toBe(true); + ).toEqual({ + valid: true, + }); }); it("returns false for valid linestring feature with validate function that returns false", () => { const lineStringMode = new TerraDrawLineStringMode({ validation: () => { - return false; + return { valid: false }; }, styles: { lineStringColor: "#ffffff", @@ -886,7 +891,9 @@ describe("TerraDrawLineStringMode", () => { updatedAt: 1685654950609, }, }), - ).toBe(false); + ).toEqual({ + valid: false, + }); }); }); }); diff --git a/src/modes/linestring/linestring.mode.ts b/src/modes/linestring/linestring.mode.ts index e9a64fba..269f6e96 100644 --- a/src/modes/linestring/linestring.mode.ts +++ b/src/modes/linestring/linestring.mode.ts @@ -11,6 +11,7 @@ import { LineString, Point, Position } from "geojson"; import { BaseModeOptions, CustomStyling, + ModeMismatchValidationFailure, TerraDrawBaseDrawMode, } from "../base.mode"; import { cartesianDistance } from "../../geometry/measure/pixel-distance"; @@ -23,10 +24,12 @@ import { FeatureId, GeoJSONStoreFeatures, GeoJSONStoreGeometries, + StoreValidation, } from "../../store/store"; import { InsertCoordinatesBehavior } from "../insert-coordinates.behavior"; import { haversineDistanceKilometers } from "../../geometry/measure/haversine-distance"; import { coordinatesIdentical } from "../../geometry/coordinates-identical"; +import { ValidateLineStringFeature } from "../../validations/linestring.validation"; type TerraDrawLineStringModeKeyEvents = { cancel: KeyboardEvent["key"] | null; @@ -162,7 +165,7 @@ export class TerraDrawLineStringMode extends TerraDrawBaseDrawMode= 2 - ); - } else { - return false; - } + validateFeature(feature: unknown): StoreValidation { + return this.validateModeFeature( + feature, + (baseValidatedFeature) => + ValidateLineStringFeature( + baseValidatedFeature, + this.coordinatePrecision, + ), + "Feature is not a valid LineString feature", + ); } } diff --git a/src/modes/point/point.mode.spec.ts b/src/modes/point/point.mode.spec.ts index 091e7598..6ec7f936 100644 --- a/src/modes/point/point.mode.spec.ts +++ b/src/modes/point/point.mode.spec.ts @@ -120,7 +120,7 @@ describe("TerraDrawPointMode", () => { it("does not create the point if validation returns false", () => { const pointMode = new TerraDrawPointMode({ validation: (feature) => { - return (feature.geometry as Point).coordinates[0] > 45; + return { valid: (feature.geometry as Point).coordinates[0] > 45 }; }, }); @@ -140,7 +140,7 @@ describe("TerraDrawPointMode", () => { it("does create the point if validation returns true", () => { const pointMode = new TerraDrawPointMode({ validation: (feature) => { - return (feature.geometry as Point).coordinates[0] > 45; + return { valid: (feature.geometry as Point).coordinates[0] > 45 }; }, }); @@ -325,7 +325,10 @@ describe("TerraDrawPointMode", () => { updatedAt: 1685654950609, }, }), - ).toBe(false); + ).toEqual({ + reason: "Feature mode property does not match the mode being added to", + valid: false, + }); }); it("returns true for valid point feature", () => { @@ -351,12 +354,14 @@ describe("TerraDrawPointMode", () => { updatedAt: 1685654950609, }, }), - ).toBe(true); + ).toEqual({ + valid: true, + }); }); it("returns false for valid point feature but validate function returns false", () => { const pointMode = new TerraDrawPointMode({ - validation: () => false, + validation: () => ({ valid: false }), styles: { pointColor: "#ffffff", }, @@ -378,7 +383,9 @@ describe("TerraDrawPointMode", () => { updatedAt: 1685654950609, }, }), - ).toBe(false); + ).toEqual({ + valid: false, + }); }); }); }); diff --git a/src/modes/point/point.mode.ts b/src/modes/point/point.mode.ts index 8d470eb2..95809b61 100644 --- a/src/modes/point/point.mode.ts +++ b/src/modes/point/point.mode.ts @@ -6,11 +6,12 @@ import { Cursor, UpdateTypes, } from "../../common"; -import { GeoJSONStoreFeatures } from "../../store/store"; +import { GeoJSONStoreFeatures, StoreValidation } from "../../store/store"; import { getDefaultStyling } from "../../util/styling"; import { BaseModeOptions, CustomStyling, + ModeMismatchValidationFailure, TerraDrawBaseDrawMode, } from "../base.mode"; import { ValidatePointFeature } from "../../validations/point.validation"; @@ -91,7 +92,7 @@ export class TerraDrawPointMode extends TerraDrawBaseDrawMode }, ); - if (!valid) { + if (!valid.valid) { return; } } @@ -162,14 +163,12 @@ export class TerraDrawPointMode extends TerraDrawBaseDrawMode return styles; } - validateFeature(feature: unknown): feature is GeoJSONStoreFeatures { - if (super.validateFeature(feature)) { - return ( - feature.properties.mode === this.mode && - ValidatePointFeature(feature, this.coordinatePrecision) - ); - } else { - return false; - } + validateFeature(feature: unknown): StoreValidation { + return this.validateModeFeature( + feature, + (baseValidatedFeature) => + ValidatePointFeature(baseValidatedFeature, this.coordinatePrecision), + "Feature is not a valid Point feature", + ); } } diff --git a/src/modes/polygon/polygon.mode.spec.ts b/src/modes/polygon/polygon.mode.spec.ts index d7605326..e7ceecfa 100644 --- a/src/modes/polygon/polygon.mode.spec.ts +++ b/src/modes/polygon/polygon.mode.spec.ts @@ -229,9 +229,13 @@ describe("TerraDrawPolygonMode", () => { const validation: Validation = (feature, { updateType }) => { if (updateType === "finish" || updateType === "commit") { - return ValidateNotSelfIntersecting(feature); + const validation = ValidateNotSelfIntersecting(feature); + return { + valid: validation, + reason: validation ? undefined : "Self intersecting", + }; } - return true; + return { valid: true }; }; beforeEach(() => { @@ -540,9 +544,13 @@ describe("TerraDrawPolygonMode", () => { polygonMode = new TerraDrawPolygonMode({ validation: (feature, { updateType }) => { if (updateType === "finish" || updateType === "commit") { - return ValidateNotSelfIntersecting(feature); + const validation = ValidateNotSelfIntersecting(feature); + return { + valid: validation, + reason: validation ? undefined : "Self intersecting", + }; } - return true; + return { valid: true }; }, }); const mockConfig = MockModeConfig(polygonMode.mode); @@ -914,7 +922,10 @@ describe("validateFeature", () => { updatedAt: 1685568435434, }, }), - ).toBe(false); + ).toEqual({ + valid: false, + reason: "Feature mode property does not match the mode being added to", + }); }); it("returns true for valid polygon feature", () => { @@ -950,12 +961,15 @@ describe("validateFeature", () => { updatedAt: 1685655518118, }, }), - ).toBe(true); + ).toEqual({ + reason: undefined, + valid: true, + }); }); it("returns false for valid polygon feature but validate function returns false", () => { const polygonMode = new TerraDrawPolygonMode({ - validation: () => false, + validation: () => ({ valid: false }), }); polygonMode.register(MockModeConfig("polygon")); @@ -981,6 +995,9 @@ describe("validateFeature", () => { updatedAt: 1685655518118, }, }), - ).toBe(false); + ).toEqual({ + reason: undefined, + valid: false, + }); }); }); diff --git a/src/modes/polygon/polygon.mode.ts b/src/modes/polygon/polygon.mode.ts index 92a5aa48..a1165850 100644 --- a/src/modes/polygon/polygon.mode.ts +++ b/src/modes/polygon/polygon.mode.ts @@ -12,6 +12,7 @@ import { TerraDrawBaseDrawMode, BaseModeOptions, CustomStyling, + ModeMismatchValidationFailure, } from "../base.mode"; import { PixelDistanceBehavior } from "../pixel-distance.behavior"; import { ClickBoundingBoxBehavior } from "../click-bounding-box.behavior"; @@ -21,7 +22,11 @@ import { SnappingBehavior } from "../snapping.behavior"; import { coordinatesIdentical } from "../../geometry/coordinates-identical"; import { ClosingPointsBehavior } from "./behaviors/closing-points.behavior"; import { getDefaultStyling } from "../../util/styling"; -import { FeatureId, GeoJSONStoreFeatures } from "../../store/store"; +import { + FeatureId, + GeoJSONStoreFeatures, + StoreValidation, +} from "../../store/store"; import { ValidatePolygonFeature } from "../../validations/polygon.validation"; type TerraDrawPolygonModeKeyEvents = { @@ -255,7 +260,7 @@ export class TerraDrawPolygonMode extends TerraDrawBaseDrawMode }, ); - if (!valid) { + if (!valid.valid) { return false; } } @@ -561,14 +566,12 @@ export class TerraDrawPolygonMode extends TerraDrawBaseDrawMode return styles; } - validateFeature(feature: unknown): feature is GeoJSONStoreFeatures { - if (super.validateFeature(feature)) { - return ( - feature.properties.mode === this.mode && - ValidatePolygonFeature(feature, this.coordinatePrecision) - ); - } else { - return false; - } + validateFeature(feature: unknown): StoreValidation { + return this.validateModeFeature( + feature, + (baseValidatedFeature) => + ValidatePolygonFeature(baseValidatedFeature, this.coordinatePrecision), + "Feature is not a valid Polygon feature", + ); } } diff --git a/src/modes/rectangle/rectangle.mode.spec.ts b/src/modes/rectangle/rectangle.mode.spec.ts index 442acffe..6d389fab 100644 --- a/src/modes/rectangle/rectangle.mode.spec.ts +++ b/src/modes/rectangle/rectangle.mode.spec.ts @@ -447,7 +447,10 @@ describe("TerraDrawRectangleMode", () => { updatedAt: 1685568435434, }, }), - ).toBe(false); + ).toEqual({ + reason: "Feature mode property does not match the mode being added to", + valid: false, + }); }); it("returns false for self intersecting polygon feature", () => { @@ -482,13 +485,16 @@ describe("TerraDrawRectangleMode", () => { updatedAt: 1685655518118, }, }), - ).toBe(false); + ).toEqual({ + valid: false, + reason: "Feature is not a valid simple Polygon feature", + }); }); it("returns true for valid rectangle feature with validation that returns true", () => { const rectangleMode = new TerraDrawRectangleMode({ validation: () => { - return true; + return { valid: true }; }, }); rectangleMode.register(MockModeConfig("rectangle")); @@ -514,13 +520,15 @@ describe("TerraDrawRectangleMode", () => { updatedAt: 1685655518118, }, }), - ).toBe(true); + ).toEqual({ + valid: true, + }); }); it("returns false for valid rectangle feature but with validation that returns false", () => { const rectangleMode = new TerraDrawRectangleMode({ validation: () => { - return false; + return { valid: false }; }, }); rectangleMode.register(MockModeConfig("rectangle")); @@ -546,7 +554,9 @@ describe("TerraDrawRectangleMode", () => { updatedAt: 1685655518118, }, }), - ).toBe(false); + ).toEqual({ + valid: false, + }); }); }); }); diff --git a/src/modes/rectangle/rectangle.mode.ts b/src/modes/rectangle/rectangle.mode.ts index 5c45129e..ff1f1317 100644 --- a/src/modes/rectangle/rectangle.mode.ts +++ b/src/modes/rectangle/rectangle.mode.ts @@ -8,11 +8,16 @@ import { Cursor, UpdateTypes, } from "../../common"; -import { FeatureId, GeoJSONStoreFeatures } from "../../store/store"; +import { + FeatureId, + GeoJSONStoreFeatures, + StoreValidation, +} from "../../store/store"; import { getDefaultStyling } from "../../util/styling"; import { BaseModeOptions, CustomStyling, + ModeMismatchValidationFailure, TerraDrawBaseDrawMode, } from "../base.mode"; import { ValidateNonIntersectingPolygonFeature } from "../../validations/polygon.validation"; @@ -108,7 +113,7 @@ export class TerraDrawRectangleMode extends TerraDrawBaseDrawMode + ValidateNonIntersectingPolygonFeature( + baseValidatedFeature, + this.coordinatePrecision, + ), + "Feature is not a valid simple Polygon feature", + ); } } diff --git a/src/modes/render/render.mode.spec.ts b/src/modes/render/render.mode.spec.ts index 276335be..7144675f 100644 --- a/src/modes/render/render.mode.spec.ts +++ b/src/modes/render/render.mode.spec.ts @@ -240,21 +240,25 @@ describe("TerraDrawRenderMode", () => { const renderMode = new TerraDrawRenderMode(options); renderMode.register(MockModeConfig("arbitary")); - expect(renderMode.validateFeature(MockPoint())).toBe(true); + expect(renderMode.validateFeature(MockPoint())).toEqual({ valid: true }); }); it("validates linestrings", () => { const renderMode = new TerraDrawRenderMode(options); renderMode.register(MockModeConfig("arbitary")); - expect(renderMode.validateFeature(MockLineString())).toBe(true); + expect(renderMode.validateFeature(MockLineString())).toEqual({ + valid: true, + }); }); it("validates polygons", () => { const renderMode = new TerraDrawRenderMode(options); renderMode.register(MockModeConfig("arbitary")); - expect(renderMode.validateFeature(MockPolygonSquare())).toBe(true); + expect(renderMode.validateFeature(MockPolygonSquare())).toEqual({ + valid: true, + }); }); }); }); diff --git a/src/modes/render/render.mode.ts b/src/modes/render/render.mode.ts index c2254c79..bfcc8462 100644 --- a/src/modes/render/render.mode.ts +++ b/src/modes/render/render.mode.ts @@ -15,6 +15,7 @@ import { GeoJSONStoreFeatures } from "../../terra-draw"; import { ValidatePointFeature } from "../../validations/point.validation"; import { ValidatePolygonFeature } from "../../validations/polygon.validation"; import { ValidateLineStringFeature } from "../../validations/linestring.validation"; +import { StoreValidation } from "../../store/store"; type RenderModeStyling = { pointColor: HexColorStyling; @@ -152,12 +153,24 @@ export class TerraDrawRenderMode extends TerraDrawBaseDrawMode { store = new GeoJSONStore(); sectorMode = new TerraDrawSectorMode({ validation: () => { - return true; + return { valid: true }; }, }); const mockConfig = MockModeConfig(sectorMode.mode); @@ -205,7 +205,9 @@ describe("TerraDrawSectorMode", () => { describe("with successful validation", () => { beforeEach(() => { sectorMode = new TerraDrawSectorMode({ - validation: () => true, + validation: () => { + return { valid: true }; + }, }); const mockConfig = MockModeConfig(sectorMode.mode); @@ -239,7 +241,9 @@ describe("TerraDrawSectorMode", () => { describe("with unsuccessful validation", () => { beforeEach(() => { sectorMode = new TerraDrawSectorMode({ - validation: () => false, + validation: () => { + return { valid: false }; + }, }); const mockConfig = MockModeConfig(sectorMode.mode); @@ -379,7 +383,9 @@ describe("TerraDrawSectorMode", () => { describe("validateFeature", () => { it("returns true for valid sector feature with validation that returns true", () => { const sectorMode = new TerraDrawSectorMode({ - validation: () => true, + validation: () => { + return { valid: true }; + }, }); sectorMode.register(MockModeConfig("sector")); @@ -419,13 +425,15 @@ describe("TerraDrawSectorMode", () => { updatedAt: 1685655518118, }, }), - ).toBe(true); + ).toEqual({ + valid: true, + }); }); it("returns false for valid sector feature but with validation that returns false", () => { const sectorMode = new TerraDrawSectorMode({ validation: () => { - return false; + return { valid: true }; }, }); sectorMode.register(MockModeConfig("sector")); @@ -466,7 +474,9 @@ describe("TerraDrawSectorMode", () => { updatedAt: 1685655518118, }, }), - ).toBe(false); + ).toEqual({ + valid: true, + }); }); }); diff --git a/src/modes/sector/sector.mode.ts b/src/modes/sector/sector.mode.ts index c8447792..c308e803 100644 --- a/src/modes/sector/sector.mode.ts +++ b/src/modes/sector/sector.mode.ts @@ -12,11 +12,19 @@ import { TerraDrawBaseDrawMode, BaseModeOptions, CustomStyling, + ModeMismatchValidationFailure, } from "../base.mode"; import { coordinatesIdentical } from "../../geometry/coordinates-identical"; import { getDefaultStyling } from "../../util/styling"; -import { FeatureId, GeoJSONStoreFeatures } from "../../store/store"; -import { ValidatePolygonFeature } from "../../validations/polygon.validation"; +import { + FeatureId, + GeoJSONStoreFeatures, + StoreValidation, +} from "../../store/store"; +import { + ValidateNonIntersectingPolygonFeature, + ValidatePolygonFeature, +} from "../../validations/polygon.validation"; import { webMercatorDestination } from "../../geometry/measure/destination"; import { normalizeBearing, @@ -290,7 +298,7 @@ export class TerraDrawSectorMode extends TerraDrawBaseDrawMode + ValidateNonIntersectingPolygonFeature( + baseValidatedFeature, + this.coordinatePrecision, + ), + "Feature is not a valid simple Polygon feature", + ); } } diff --git a/src/modes/select/behaviors/drag-coordinate-resize.behavior.spec.ts b/src/modes/select/behaviors/drag-coordinate-resize.behavior.spec.ts index 5c5411da..666dbb37 100644 --- a/src/modes/select/behaviors/drag-coordinate-resize.behavior.spec.ts +++ b/src/modes/select/behaviors/drag-coordinate-resize.behavior.spec.ts @@ -150,7 +150,7 @@ describe("DragCoordinateResizeBehavior", () => { MockCursorEvent({ lng: 0, lat: 0 }), "center", () => { - return false; + return { valid: false }; }, ); diff --git a/src/modes/select/behaviors/drag-coordinate-resize.behavior.ts b/src/modes/select/behaviors/drag-coordinate-resize.behavior.ts index 8f221c6c..97d2ba09 100644 --- a/src/modes/select/behaviors/drag-coordinate-resize.behavior.ts +++ b/src/modes/select/behaviors/drag-coordinate-resize.behavior.ts @@ -732,7 +732,7 @@ export class DragCoordinateResizeBehavior extends TerraDrawModeBehavior { updateType: UpdateTypes.Provisional, }, ); - if (!valid) { + if (!valid.valid) { return false; } } diff --git a/src/modes/select/behaviors/drag-coordinate.behavior.spec.ts b/src/modes/select/behaviors/drag-coordinate.behavior.spec.ts index a42fbd4e..5c3e173f 100644 --- a/src/modes/select/behaviors/drag-coordinate.behavior.spec.ts +++ b/src/modes/select/behaviors/drag-coordinate.behavior.spec.ts @@ -144,13 +144,13 @@ describe("DragCoordinateBehavior", () => { dragCoordinateBehavior.drag( MockCursorEvent({ lng: 0, lat: 0 }), true, - () => false, + () => ({ valid: false }), ); expect(config.store.updateGeometry).toHaveBeenCalledTimes(0); }); - it("validation returning false means updates are not called", () => { + it("validation returning true means updates are called", () => { const id = createStorePolygon(config); dragCoordinateBehavior.startDragging(id, 0); @@ -160,7 +160,7 @@ describe("DragCoordinateBehavior", () => { dragCoordinateBehavior.drag( MockCursorEvent({ lng: 0, lat: 0 }), true, - () => true, + () => ({ valid: true }), ); expect(config.store.updateGeometry).toHaveBeenCalledTimes(1); diff --git a/src/modes/select/behaviors/drag-coordinate.behavior.ts b/src/modes/select/behaviors/drag-coordinate.behavior.ts index 72c909e3..5b9015e6 100644 --- a/src/modes/select/behaviors/drag-coordinate.behavior.ts +++ b/src/modes/select/behaviors/drag-coordinate.behavior.ts @@ -170,7 +170,7 @@ export class DragCoordinateBehavior extends TerraDrawModeBehavior { }, ); - if (!valid) { + if (!valid.valid) { return false; } } diff --git a/src/modes/select/behaviors/drag-feature.behavior.spec.ts b/src/modes/select/behaviors/drag-feature.behavior.spec.ts index 8a0b8b6f..0824f756 100644 --- a/src/modes/select/behaviors/drag-feature.behavior.spec.ts +++ b/src/modes/select/behaviors/drag-feature.behavior.spec.ts @@ -140,10 +140,9 @@ describe("DragFeatureBehavior", () => { jest.spyOn(config.store, "updateGeometry"); jest.spyOn(config.store, "getGeometryCopy"); - dragFeatureBehavior.drag( - MockCursorEvent({ lng: 0, lat: 0 }), - () => false, - ); + dragFeatureBehavior.drag(MockCursorEvent({ lng: 0, lat: 0 }), () => ({ + valid: false, + })); expect(config.store.getGeometryCopy).toHaveBeenCalledTimes(1); expect(config.store.updateGeometry).toHaveBeenCalledTimes(0); @@ -158,10 +157,9 @@ describe("DragFeatureBehavior", () => { jest.spyOn(config.store, "updateGeometry"); jest.spyOn(config.store, "getGeometryCopy"); - dragFeatureBehavior.drag( - MockCursorEvent({ lng: 0, lat: 0 }), - () => true, - ); + dragFeatureBehavior.drag(MockCursorEvent({ lng: 0, lat: 0 }), () => ({ + valid: true, + })); expect(config.store.getGeometryCopy).toHaveBeenCalledTimes(1); expect(config.store.updateGeometry).toHaveBeenCalledTimes(1); diff --git a/src/modes/select/behaviors/drag-feature.behavior.ts b/src/modes/select/behaviors/drag-feature.behavior.ts index bd0b8290..81a4e3aa 100644 --- a/src/modes/select/behaviors/drag-feature.behavior.ts +++ b/src/modes/select/behaviors/drag-feature.behavior.ts @@ -174,7 +174,7 @@ export class DragFeatureBehavior extends TerraDrawModeBehavior { }, ); - if (!valid) { + if (!valid.valid) { return false; } } diff --git a/src/modes/select/select.mode.ts b/src/modes/select/select.mode.ts index 135d79f6..1c2d4939 100644 --- a/src/modes/select/select.mode.ts +++ b/src/modes/select/select.mode.ts @@ -180,9 +180,7 @@ export class TerraDrawSelectMode extends TerraDrawBaseSelectMode boolean; + this.validations[mode] = feature.validation; } } } @@ -383,7 +381,7 @@ export class TerraDrawSelectMode extends TerraDrawBaseSelectMode { store = new GeoJSONStore(); sensorMode = new TerraDrawSensorMode({ validation: () => { - return true; + return { valid: true }; }, }); const mockConfig = MockModeConfig(sensorMode.mode); @@ -181,7 +181,7 @@ describe("TerraDrawSensorMode", () => { describe("with successful validation", () => { beforeEach(() => { sensorMode = new TerraDrawSensorMode({ - validation: () => true, + validation: () => ({ valid: true }), }); const mockConfig = MockModeConfig(sensorMode.mode); @@ -257,7 +257,7 @@ describe("TerraDrawSensorMode", () => { describe("with non successful validation", () => { beforeEach(() => { sensorMode = new TerraDrawSensorMode({ - validation: () => false, + validation: () => ({ valid: false }), }); const mockConfig = MockModeConfig(sensorMode.mode); @@ -398,7 +398,7 @@ describe("TerraDrawSensorMode", () => { describe("validateFeature", () => { it("returns true for valid rectangle feature with validation that returns true", () => { const sensorMode = new TerraDrawSensorMode({ - validation: () => true, + validation: () => ({ valid: true }), }); sensorMode.register(MockModeConfig("sensor")); @@ -438,13 +438,15 @@ describe("TerraDrawSensorMode", () => { updatedAt: 1685655518118, }, }), - ).toBe(true); + ).toEqual({ + valid: true, + }); }); it("returns false for valid rectangle feature but with validation that returns false", () => { const sensorMode = new TerraDrawSensorMode({ validation: () => { - return false; + return { valid: false, reason: "Test" }; }, }); sensorMode.register(MockModeConfig("sensor")); @@ -485,7 +487,10 @@ describe("TerraDrawSensorMode", () => { updatedAt: 1685655518118, }, }), - ).toBe(false); + ).toEqual({ + valid: false, + reason: "Test", + }); }); }); diff --git a/src/modes/sensor/sensor.mode.ts b/src/modes/sensor/sensor.mode.ts index 123750dc..c6f01ea1 100644 --- a/src/modes/sensor/sensor.mode.ts +++ b/src/modes/sensor/sensor.mode.ts @@ -12,10 +12,18 @@ import { TerraDrawBaseDrawMode, BaseModeOptions, CustomStyling, + ModeMismatchValidationFailure, } from "../base.mode"; import { getDefaultStyling } from "../../util/styling"; -import { FeatureId, GeoJSONStoreFeatures } from "../../store/store"; -import { ValidatePolygonFeature } from "../../validations/polygon.validation"; +import { + FeatureId, + GeoJSONStoreFeatures, + StoreValidation, +} from "../../store/store"; +import { + ValidateNonIntersectingPolygonFeature, + ValidatePolygonFeature, +} from "../../validations/polygon.validation"; import { webMercatorDestination } from "../../geometry/measure/destination"; import { normalizeBearing, @@ -424,7 +432,7 @@ export class TerraDrawSensorMode extends TerraDrawBaseDrawMode + ValidateNonIntersectingPolygonFeature( + baseValidatedFeature, + this.coordinatePrecision, + ), + "Feature is not a valid simple Polygon feature", + ); } private getDeltaBearing( diff --git a/src/store/store-feature-validation.ts b/src/store/store-feature-validation.ts index ec5cef28..1255fe57 100644 --- a/src/store/store-feature-validation.ts +++ b/src/store/store-feature-validation.ts @@ -25,6 +25,19 @@ function isObject( ); } +export function hasModeProperty( + feature: unknown, +): feature is { properties: { mode: string } } { + return Boolean( + feature && + typeof feature === "object" && + "properties" in feature && + typeof feature.properties === "object" && + feature.properties !== null && + "mode" in feature.properties, + ); +} + function dateIsValid(timestamp: unknown): boolean { return ( typeof timestamp === "number" && @@ -34,7 +47,7 @@ function dateIsValid(timestamp: unknown): boolean { export function isValidTimestamp(timestamp: unknown): boolean { if (!dateIsValid(timestamp)) { - throw new Error(StoreValidationErrors.InvalidTrackedProperties); + return false; } return true; @@ -43,7 +56,10 @@ export function isValidTimestamp(timestamp: unknown): boolean { export function isValidStoreFeature( feature: unknown, isValidId: IdStrategy["isValidId"], -): feature is GeoJSONStoreFeatures { +): { + valid: boolean; + reason?: string; +} { let error; if (!isObject(feature)) { error = StoreValidationErrors.FeatureIsNotObject; @@ -68,12 +84,12 @@ export function isValidStoreFeature( !feature.properties.mode || typeof feature.properties.mode !== "string" ) { - throw new Error(StoreValidationErrors.InvalidModeProperty); + return { valid: false, reason: StoreValidationErrors.InvalidModeProperty }; } if (error) { - throw new Error(error); + return { valid: false, reason: error }; } - return true; + return { valid: true }; } diff --git a/src/store/store-validation.spec.ts b/src/store/store-validation.spec.ts index cc77be31..5a4002da 100644 --- a/src/store/store-validation.spec.ts +++ b/src/store/store-validation.spec.ts @@ -8,59 +8,71 @@ import { describe("isValidStoreFeature", () => { const isValidId = defaultIdStrategy.isValidId; - it("throws on data with non object feature", () => { - expect(() => isValidStoreFeature(undefined, isValidId)).toThrow( - StoreValidationErrors.FeatureIsNotObject, - ); - expect(() => isValidStoreFeature(null, isValidId)).toThrow( - StoreValidationErrors.FeatureIsNotObject, - ); + it("returns valid false with reason on data with non object feature", () => { + expect(isValidStoreFeature(undefined, isValidId)).toEqual({ + reason: StoreValidationErrors.FeatureIsNotObject, + valid: false, + }); + expect(isValidStoreFeature(null, isValidId)).toEqual({ + reason: StoreValidationErrors.FeatureIsNotObject, + valid: false, + }); }); - it("throws on data with no id", () => { - expect(() => isValidStoreFeature({ id: undefined }, isValidId)).toThrow( - StoreValidationErrors.FeatureHasNoId, - ); - expect(() => isValidStoreFeature({ id: null }, isValidId)).toThrow( - StoreValidationErrors.FeatureHasNoId, - ); + it("returns valid false with reason on data with no id", () => { + expect(isValidStoreFeature({ id: undefined }, isValidId)).toEqual({ + reason: StoreValidationErrors.FeatureHasNoId, + valid: false, + }); + expect(isValidStoreFeature({ id: null }, isValidId)).toEqual({ + reason: StoreValidationErrors.FeatureHasNoId, + valid: false, + }); }); - it("throws on data with non string id", () => { - expect(() => isValidStoreFeature({ id: 1 }, isValidId)).toThrow( - StoreValidationErrors.FeatureIdIsNotValid, - ); + it("returns valid false with reason on data with non string id", () => { + expect(isValidStoreFeature({ id: 1 }, isValidId)).toEqual({ + reason: StoreValidationErrors.FeatureIdIsNotValid, + valid: false, + }); }); - it("throws on data with non uuid4 id", () => { - expect(() => isValidStoreFeature({ id: "1" }, isValidId)).toThrow( - StoreValidationErrors.FeatureIdIsNotValid, - ); + it("returns valid false with reason on data with non uuid4 id", () => { + expect(isValidStoreFeature({ id: "1" }, isValidId)).toEqual({ + reason: StoreValidationErrors.FeatureIdIsNotValid, + valid: false, + }); }); - it("throws on data with no geometry", () => { - expect(() => + it("returns valid false with reason on data with no geometry", () => { + expect( isValidStoreFeature( { id: "e3ccd3b9-afb1-4f0b-91d8-22a768d5f284" }, isValidId, ), - ).toThrow(StoreValidationErrors.FeatureHasNoGeometry); + ).toEqual({ + reason: StoreValidationErrors.FeatureHasNoGeometry, + valid: false, + }); }); - it("throws on data with no properties", () => { - expect(() => { + it("returns valid false with reason on data with no properties", () => { + expect( isValidStoreFeature( { id: "e3ccd3b9-afb1-4f0b-91d8-22a768d5f284", geometry: {}, - } as any, + } as unknown, isValidId, - ); - }).toThrow(StoreValidationErrors.FeatureHasNoProperties); + ), + ).toEqual({ + reason: StoreValidationErrors.FeatureHasNoProperties, + valid: false, + }); }); - it("throws on data with non Point, LineString, Polygon geometry type", () => { - expect(() => { + it("returns valid false with reason on data with non Point, LineString, Polygon geometry type", () => { + expect( isValidStoreFeature( { id: "e3ccd3b9-afb1-4f0b-91d8-22a768d5f284", @@ -68,14 +80,17 @@ describe("isValidStoreFeature", () => { type: "MultiLineString", }, properties: {}, - } as any, + } as unknown, isValidId, - ); - }).toThrow(StoreValidationErrors.FeatureGeometryNotSupported); + ), + ).toEqual({ + reason: StoreValidationErrors.FeatureGeometryNotSupported, + valid: false, + }); }); - it("throws on data with supported geometry with non array coordinate property", () => { - expect(() => { + it("returns valid false with reason on data with supported geometry with non array coordinate property", () => { + expect( isValidStoreFeature( { id: "e3ccd3b9-afb1-4f0b-91d8-22a768d5f284", @@ -84,14 +99,17 @@ describe("isValidStoreFeature", () => { coordinates: "[]", }, properties: {}, - }, + } as unknown, isValidId, - ); - }).toThrow(StoreValidationErrors.FeatureCoordinatesNotAnArray); + ), + ).toEqual({ + reason: StoreValidationErrors.FeatureCoordinatesNotAnArray, + valid: false, + }); }); - it("throws if mode is not provided as a string", () => { - expect(() => + it("returns valid false with reason if mode is not provided as a string", () => { + expect( isValidStoreFeature( { id: "e3ccd3b9-afb1-4f0b-91d8-22a768d5f284", @@ -103,11 +121,14 @@ describe("isValidStoreFeature", () => { }, isValidId, ), - ).toThrow(StoreValidationErrors.InvalidModeProperty); + ).toEqual({ + reason: StoreValidationErrors.InvalidModeProperty, + valid: false, + }); }); it("does not throw if mode is provide as a string", () => { - expect(() => + expect( isValidStoreFeature( { id: "e3ccd3b9-afb1-4f0b-91d8-22a768d5f284", @@ -119,16 +140,16 @@ describe("isValidStoreFeature", () => { }, isValidId, ), - ).not.toThrow(); + ).toEqual({ + valid: true, + }); }); - it("throws if tracked is explicitly true and tracked properties are not provided", () => { - expect(() => isValidTimestamp(undefined)).toThrow( - StoreValidationErrors.InvalidTrackedProperties, - ); + it("returns valid false with reason if tracked is explicitly true and tracked properties are not provided", () => { + expect(isValidTimestamp(undefined)).toEqual(false); }); it("does not throw if tracked is true and tracked properties are provided", () => { - expect(() => isValidTimestamp(+new Date())).not.toThrow(); + expect(isValidTimestamp(+new Date())).toEqual(true); }); }); diff --git a/src/store/store.spec.ts b/src/store/store.spec.ts index cdd344ff..e52c6093 100644 --- a/src/store/store.spec.ts +++ b/src/store/store.spec.ts @@ -383,60 +383,79 @@ describe("GeoJSONStore", () => { it("errors if feature does not pass validation", () => { const store = new GeoJSONStore({ tracked: false }); - expect(() => { - store.load( - [ - { - type: "Feature", - properties: {}, - geometry: { type: "Point", coordinates: [0, 0] }, - }, - ], - (feature) => { - return Boolean( - feature && - typeof feature === "object" && - "type" in feature && - feature.type === "Polygon", - ); + const result = store.load( + [ + { + type: "Feature", + properties: {}, + geometry: { type: "Point", coordinates: [0, 0] }, }, - ); - }).toThrow(); + ], + (feature) => ({ + valid: Boolean( + feature && + typeof feature === "object" && + "type" in feature && + feature.type === "Polygon", + ), + reason: "Test", + }), + ); + + expect(result).toStrictEqual([ + { + id: expect.any(String), + reason: expect.any(String), + valid: false, + }, + ]); }); it("errors if feature createdAt is not valid numeric timestamps", () => { const store = new GeoJSONStore({ tracked: true }); - expect(() => { - store.load([ - { - type: "Feature", - properties: { - mode: "point", - createdAt: new Date().toISOString(), - }, - geometry: { type: "Point", coordinates: [0, 0] }, + const result = store.load([ + { + type: "Feature", + properties: { + mode: "point", + createdAt: new Date().toISOString(), }, - ]); - }).toThrow(StoreValidationErrors.InvalidTrackedProperties); + geometry: { type: "Point", coordinates: [0, 0] }, + }, + ]); + + return expect(result).toStrictEqual([ + { + id: expect.any(String), + reason: "createdAt is not a valid numeric timestamp", + valid: false, + }, + ]); }); it("errors if feature createdAt is not valid numeric timestamps", () => { const store = new GeoJSONStore({ tracked: true }); - expect(() => { - store.load([ - { - type: "Feature", - properties: { - mode: "point", - createdAt: +new Date(), - updatedAt: new Date().toISOString(), - }, - geometry: { type: "Point", coordinates: [0, 0] }, + const result = store.load([ + { + type: "Feature", + properties: { + mode: "point", + createdAt: +new Date(), + updatedAt: new Date().toISOString(), }, - ]); - }).toThrow(StoreValidationErrors.InvalidTrackedProperties); + geometry: { type: "Point", coordinates: [0, 0] }, + }, + ]); + + expect(result).toStrictEqual([ + { + id: expect.any(String), + reason: "updatedAt is not a valid numeric timestamp", + valid: false, + }, + ]); }); }); diff --git a/src/store/store.ts b/src/store/store.ts index cce49d63..4a205fbd 100644 --- a/src/store/store.ts +++ b/src/store/store.ts @@ -21,6 +21,12 @@ export type GeoJSONStoreFeatures = Feature< DefinedProperties >; +export type StoreValidation = { + id?: FeatureId; + valid: boolean; + reason?: string; +}; + type StoreChangeEvents = "delete" | "create" | "update" | "styling"; export type StoreChangeHandler = ( @@ -84,62 +90,90 @@ export class GeoJSONStore { load( data: GeoJSONStoreFeatures[], - featureValidation?: (feature: unknown, tracked?: boolean) => boolean, - ) { + featureValidation?: ( + feature: unknown, + tracked?: boolean, + ) => StoreValidation, + ): StoreValidation[] { if (data.length === 0) { - return; + return []; } // We don't want to update the original data - const clonedData = this.clone(data); + let clonedData = this.clone(data); - // We try to be a bit forgiving here as many users - // may not set a feature id as UUID or createdAt/updatedAt - clonedData.forEach((feature) => { + const changes: FeatureId[] = []; // The list of changes that we will trigger to onChange + const validations: StoreValidation[] = []; // The list of validations that we will return + + // We filter out the features that are not valid and do not add them to the store + clonedData = clonedData.filter((feature) => { if (feature.id === undefined || feature.id === null) { feature.id = this.idStrategy.getId(); } + const id = feature.id as FeatureId; + if (featureValidation) { + const validation = featureValidation(feature); + + // Generic error handling if the featureValidation function + // does not throw something more specific itself + if (!validation.valid) { + validations.push({ id, valid: false, reason: validation.reason }); + return false; + } + } + if (this.tracked) { if (!feature.properties.createdAt) { feature.properties.createdAt = +new Date(); } else { - isValidTimestamp(feature.properties.createdAt); + const valid = isValidTimestamp(feature.properties.createdAt); + if (!valid) { + validations.push({ + id: feature.id as FeatureId, + valid: false, + reason: "createdAt is not a valid numeric timestamp", + }); + return false; + } } if (!feature.properties.updatedAt) { feature.properties.updatedAt = +new Date(); } else { - isValidTimestamp(feature.properties.updatedAt); - } - } - }); - - const changes: FeatureId[] = []; - clonedData.forEach((feature) => { - const id = feature.id as FeatureId; - if (featureValidation) { - const isValid = featureValidation(feature); - - // Generic error handling if the featureValidation function - // does not throw something more specific itself - if (!isValid) { - throw new Error( - `Feature ${id} is not valid: ${JSON.stringify(feature)}`, - ); + const valid = isValidTimestamp(feature.properties.updatedAt); + if (!valid) { + validations.push({ + id: feature.id as FeatureId, + valid: false, + reason: "updatedAt is not a valid numeric timestamp", + }); + return false; + } } } // We have to be sure that the feature does not already exist with this ID if (this.has(id)) { - throw new Error(`Feature already exists with this id: ${id}`); + validations.push({ + id, + valid: false, + reason: `Feature already exists with this id: ${id}`, + }); + return false; } this.store[id] = feature; changes.push(id); + + validations.push({ id, valid: true }); + + return true; }); this.spatialIndex.load(clonedData); this._onChange(changes, "create"); + + return validations; } search( diff --git a/src/terra-draw.spec.ts b/src/terra-draw.spec.ts index 61ffcb2f..2e4283d1 100644 --- a/src/terra-draw.spec.ts +++ b/src/terra-draw.spec.ts @@ -85,7 +85,7 @@ describe("Terra Draw", () => { draw.start(); - draw.addFeatures([ + const result = draw.addFeatures([ { type: "Feature", geometry: { @@ -98,6 +98,8 @@ describe("Terra Draw", () => { }, ]); + expect(result[0].valid).toBe(true); + expect(typeof result[0].id).toBe("string"); const snapshot = draw.getSnapshot(); expect(typeof snapshot[0].id).toBe("string"); expect(snapshot[0].id).toHaveLength(36); @@ -117,7 +119,7 @@ describe("Terra Draw", () => { draw.start(); - draw.addFeatures([ + const result = draw.addFeatures([ { type: "Feature", geometry: { @@ -130,12 +132,15 @@ describe("Terra Draw", () => { }, ]); + expect(result[0].valid).toBe(true); + expect(result[0].id).toBe(1); + const snapshot = draw.getSnapshot(); expect(typeof snapshot[0].id).toBe("number"); expect(snapshot[0].id).toBe(1); }); - it("does not allow features with same id to be added twice ", () => { + it("returns invalid feature when a duplicate feature id is added", () => { const draw = new TerraDraw({ adapter: adapter, modes: [new TerraDrawPointMode()], @@ -143,7 +148,7 @@ describe("Terra Draw", () => { draw.start(); - expect(() => { + expect( draw.addFeatures([ { id: "e90e54ea-0a63-407e-b433-08717009d9f6", @@ -167,11 +172,22 @@ describe("Terra Draw", () => { mode: "point", }, }, - ]); - }).toThrow(); + ]), + ).toEqual([ + { + id: "e90e54ea-0a63-407e-b433-08717009d9f6", + valid: true, + }, + { + id: "e90e54ea-0a63-407e-b433-08717009d9f6", + reason: + "Feature already exists with this id: e90e54ea-0a63-407e-b433-08717009d9f6", + valid: false, + }, + ]); }); - it("does not allow features with incorrect id strategy to be added", () => { + it("returns invalid feature when an incorrect id strategy is used", () => { const draw = new TerraDraw({ adapter: adapter, modes: [new TerraDrawPointMode()], @@ -179,7 +195,7 @@ describe("Terra Draw", () => { draw.start(); - expect(() => { + expect( draw.addFeatures([ { id: 1, @@ -203,8 +219,64 @@ describe("Terra Draw", () => { mode: "point", }, }, - ]); - }).toThrow(); + ]), + ).toEqual([ + { + id: 1, + reason: "Feature must match the id strategy (default is UUID4)", + valid: false, + }, + { + id: 2, + reason: "Feature must match the id strategy (default is UUID4)", + valid: false, + }, + ]); + }); + + it("returns invalid feature when the modes do not match any instantiated modes", () => { + const draw = new TerraDraw({ + adapter: adapter, + modes: [new TerraDrawPolygonMode()], + }); + + draw.start(); + + expect( + draw.addFeatures([ + { + type: "Feature", + geometry: { + type: "Point", + coordinates: [-25.431289673, 34.355907891], + }, + properties: { + mode: "point", + }, + }, + { + type: "Feature", + geometry: { + type: "Point", + coordinates: [-26.431289673, 34.355907891], + }, + properties: { + mode: "rectangle", + }, + }, + ]), + ).toEqual([ + { + id: expect.any(String), + reason: "point mode is not in the list of instantiated modes", + valid: false, + }, + { + id: expect.any(String), + reason: "rectangle mode is not in the list of instantiated modes", + valid: false, + }, + ]); }); }); diff --git a/src/terra-draw.ts b/src/terra-draw.ts index 25e841f0..d6aefa26 100644 --- a/src/terra-draw.ts +++ b/src/terra-draw.ts @@ -40,6 +40,7 @@ import { GeoJSONStoreFeatures, IdStrategy, StoreChangeHandler, + StoreValidation, } from "./store/store"; import { BehaviorConfig } from "./modes/base.behavior"; import { cartesianDistance } from "./geometry/measure/pixel-distance"; @@ -54,6 +55,7 @@ import { TerraDrawAngledRectangleMode } from "./modes/angled-rectangle/angled-re import { TerraDrawSectorMode } from "./modes/sector/sector.mode"; import { TerraDrawSensorMode } from "./modes/sensor/sensor.mode"; import * as TerraDrawExtend from "./extend"; +import { hasModeProperty } from "./store/store-feature-validation"; type FinishListener = (id: FeatureId, context: OnFinishContext) => void; type ChangeListener = (ids: FeatureId[], type: string) => void; @@ -581,49 +583,58 @@ class TerraDraw { } /** - * A method for adding features to the store. This method will validate the features. - * Features must match one of the modes enabled in the instance. - * @param mode - * @param features - * @returns + * A method for adding features to the store. This method will validate the features + * returning an array of validation results. Features must match one of the modes enabled + * in the instance. + * @param features - an array of GeoJSON features + * @returns an array of validation results * * @beta */ - addFeatures(features: GeoJSONStoreFeatures[]) { + addFeatures(features: GeoJSONStoreFeatures[]): StoreValidation[] { this.checkEnabled(); if (features.length === 0) { - return; + return []; } - this._store.load(features, (feature) => { - const hasModeProperty = Boolean( - feature && - typeof feature === "object" && - "properties" in feature && - typeof feature.properties === "object" && - feature.properties !== null && - "mode" in feature.properties, - ); - - if (hasModeProperty) { - const modeToAddTo = - this._modes[ - (feature as { properties: { mode: string } }).properties.mode - ]; + return this._store.load(features, (feature) => { + // If the feature has a mode property, we use that to validate the feature + if (hasModeProperty(feature)) { + const featureMode = feature.properties.mode; + const modeToAddTo = this._modes[featureMode]; // if the mode does not exist, we return false if (!modeToAddTo) { - return false; + return { + id: (feature as { id?: FeatureId }).id, + valid: false, + reason: `${featureMode} mode is not in the list of instantiated modes`, + }; } // use the inbuilt validation of the mode const validation = modeToAddTo.validateFeature.bind(modeToAddTo); - return validation(feature); + const validationResult = validation(feature); + const valid = validationResult.valid; + const reason = validationResult.reason + ? validationResult.reason + : !validationResult.valid + ? "Feature is invalid" + : undefined; + return { + id: (feature as { id?: FeatureId }).id, + valid, + reason, + }; } // If the feature does not have a mode property, we return false - return false; + return { + id: (feature as { id?: FeatureId }).id, + valid: false, + reason: "Mode property does not exist", + }; }); }