fix(geo): update validations and error messages (#9670)

packages/geo/__tests__/Geo.test.ts

@@ -36,7 +36,6 @@ import {
 	validGeofence1,
 	singleGeofenceCamelcaseResults,
 	batchGeofencesCamelcaseResults,
-	geofencesWithInvalidId,
 } from './testData';
 
 import {
@@ -570,21 +569,6 @@ describe('Geo', () => {
 			);
 		});
 
-		test('should error if there is a bad geofence in the input', async () => {
-			jest.spyOn(Credentials, 'get').mockImplementationOnce(() => {
-				return Promise.resolve(credentials);
-			});
-
-			const geo = new GeoClass();
-			geo.configure(awsConfig);
-
-			await expect(
-				geo.saveGeofences(geofencesWithInvalidId)
-			).rejects.toThrowError(
-				`Invalid geofenceId: t|-|!$ !$ N()T V@|_!D Ids can only contain alphanumeric characters, hyphens, underscores and periods.`
-			);
-		});
-
 		test('should fail if there is no provider', async () => {
 			jest.spyOn(Credentials, 'get').mockImplementationOnce(() => {
 				return Promise.resolve(credentials);
@@ -633,24 +617,6 @@ describe('Geo', () => {
 			};
 			expect(input).toEqual(output);
 		});
-
-		test('getGeofence errors when a bad geofenceId is given', async () => {
-			jest.spyOn(Credentials, 'get').mockImplementationOnce(() => {
-				return Promise.resolve(credentials);
-			});
-
-			LocationClient.prototype.send = jest
-				.fn()
-				.mockImplementationOnce(mockGetGeofenceCommand);
-
-			const geo = new GeoClass();
-			geo.configure(awsConfig);
-
-			const badGeofenceId = 't|-|!$ !$ N()T V@|_!D';
-			await expect(geo.getGeofence(badGeofenceId)).rejects.toThrow(
-				`Invalid geofenceId: ${badGeofenceId} Ids can only contain alphanumeric characters, hyphens, underscores and periods.`
-			);
-		});
 	});
 
 	describe('listGeofences', () => {

packages/geo/__tests__/Providers/AmazonLocationServiceProvider.test.ts

@@ -27,6 +27,7 @@ import {
 	validGeofences,
 	batchGeofencesCamelcaseResults,
 	validGeometry,
+	clockwiseGeofence,
 } from '../testData';
 import {
 	createGeofenceInputArray,
@@ -670,6 +671,25 @@ describe('AmazonLocationServiceProvider', () => {
 			expect(results).toEqual(expected);
 		});
 
+		test('should error if a geofence is wound clockwise', async () => {
+			jest.spyOn(Credentials, 'get').mockImplementation(() => {
+				return Promise.resolve(credentials);
+			});
+
+			LocationClient.prototype.send = jest
+				.fn()
+				.mockImplementation(mockBatchPutGeofenceCommand);
+
+			const locationProvider = new AmazonLocationServiceProvider();
+			locationProvider.configure(awsConfig.geo.amazon_location_service);
+
+			await expect(
+				locationProvider.saveGeofences([clockwiseGeofence])
+			).rejects.toThrow(
+				'geofenceWithClockwiseGeofence: LinearRing coordinates must be wound counterclockwise'
+			);
+		});
+
 		test('should error if there are no geofenceCollections in config', async () => {
 			jest.spyOn(Credentials, 'get').mockImplementationOnce(() => {
 				return Promise.resolve(credentials);
@@ -713,6 +733,24 @@ describe('AmazonLocationServiceProvider', () => {
 			await expect(results).toEqual(expected);
 		});
 
+		test('getGeofence errors when a bad geofenceId is given', async () => {
+			jest.spyOn(Credentials, 'get').mockImplementationOnce(() => {
+				return Promise.resolve(credentials);
+			});
+
+			LocationClient.prototype.send = jest
+				.fn()
+				.mockImplementationOnce(mockGetGeofenceCommand);
+
+			const locationProvider = new AmazonLocationServiceProvider();
+			locationProvider.configure(awsConfig.geo.amazon_location_service);
+
+			const badGeofenceId = 't|-|!$ !$ N()T V@|_!D';
+			await expect(locationProvider.getGeofence(badGeofenceId)).rejects.toThrow(
+				`Invalid geofenceId: '${badGeofenceId}' - IDs can only contain alphanumeric characters, hyphens, underscores and periods.`
+			);
+		});
+
 		test('should error if there are no geofenceCollections in config', async () => {
 			jest.spyOn(Credentials, 'get').mockImplementationOnce(() => {
 				return Promise.resolve(credentials);
@@ -895,6 +933,23 @@ describe('AmazonLocationServiceProvider', () => {
 			expect(results).toEqual(expected);
 		});
 
+		test('should error if there is a bad geofence in the input', async () => {
+			jest.spyOn(Credentials, 'get').mockImplementationOnce(() => {
+				return Promise.resolve(credentials);
+			});
+			const locationProvider = new AmazonLocationServiceProvider();
+			locationProvider.configure(awsConfig.geo.amazon_location_service);
+			await expect(
+				locationProvider.deleteGeofences([
+					'thisIsAGoodId',
+					't|-|!$ !$ N()T V@|_!D',
+					'#2 t|-|!$ !$ N()T V@|_!D',
+				])
+			).rejects.toThrow(
+				`Invalid geofence ids: t|-|!$ !$ N()T V@|_!D, #2 t|-|!$ !$ N()T V@|_!D`
+			);
+		});
+
 		test('should error if there are no geofenceCollections in config', async () => {
 			jest.spyOn(Credentials, 'get').mockImplementationOnce(() => {
 				return Promise.resolve(credentials);

packages/geo/__tests__/testData.ts

@@ -126,6 +126,14 @@ export const validLinearRing: LinearRing = [
 	validCoordinates1,
 ];
 
+export const clockwiseLinearRing: LinearRing = [
+	validCoordinates1,
+	validCoordinates4,
+	validCoordinates3,
+	validCoordinates2,
+	validCoordinates1,
+];
+
 export const linearRingIncomplete: LinearRing = [
 	validCoordinates1,
 	validCoordinates2,
@@ -147,6 +155,11 @@ export const linearRingBadCoordinates: LinearRing = [
 
 // Polygons
 export const validPolygon: GeofencePolygon = [validLinearRing];
+
+export const polygonClockwiseLinearRing: GeofencePolygon = [
+	clockwiseLinearRing,
+];
+
 export const polygonTooBig: GeofencePolygon = [
 	validLinearRing,
 	validLinearRing,
@@ -170,11 +183,17 @@ export const validGeofence3: GeofenceInput = {
 	geofenceId: 'validGeofenceId3',
 	geometry: validGeometry,
 };
+
 export const geofenceWithInvalidId: GeofenceInput = {
 	geofenceId: 't|-|!$ !$ N()T V@|_!D',
 	geometry: validGeometry,
 };
 
+export const clockwiseGeofence: GeofenceInput = {
+	geofenceId: 'geofenceWithClockwiseGeofence',
+	geometry: { polygon: polygonClockwiseLinearRing },
+};
+
 export const validGeofences = [];
 for (let i = 0; i < 132; i++) {
 	validGeofences.push({
@@ -194,6 +213,7 @@ export const geofencesWithInvalidId = [
 	validGeofence2,
 	validGeofence3,
 	geofenceWithInvalidId,
+	validGeofence1,
 ];
 
 export const singleGeofenceCamelcaseResults = {

packages/geo/__tests__/util.test.ts

@@ -15,14 +15,15 @@ import {
 	validateCoordinates,
 	validateLinearRing,
 	validatePolygon,
-	validateGeofences,
+	validateGeofencesInput,
 } from '../src/util';
 
 import {
 	invalidLngCoordinates,
 	invalidLatCoordinates,
 	infiniteCoordinates,
 	validLinearRing,
+	clockwiseLinearRing,
 	linearRingIncomplete,
 	linearRingTooSmall,
 	linearRingBadCoordinates,
@@ -72,18 +73,31 @@ describe('Geo utility functions', () => {
 			expect(() => result).not.toThrowError();
 		});
 		test('should error if first and last coordinates do not match', () => {
-			expect(() => validateLinearRing(linearRingIncomplete)).toThrowError(
-				`LinearRing's first and last coordinates are not the same`
+			expect(() =>
+				validateLinearRing(linearRingIncomplete, 'linearRingIncomplete')
+			).toThrowError(
+				`linearRingIncomplete: LinearRing's first and last coordinates are not the same`
 			);
 		});
 		test('should error if LinearRing has less than 4 elements', () => {
-			expect(() => validateLinearRing(linearRingTooSmall)).toThrowError(
-				'LinearRing must contain 4 or more coordinates.'
+			expect(() =>
+				validateLinearRing(linearRingTooSmall, 'linearRingTooSmall')
+			).toThrowError(
+				'linearRingTooSmall: LinearRing must contain 4 or more coordinates.'
 			);
 		});
 		test('should error if any coordinates are not valid', () => {
-			expect(() => validateLinearRing(linearRingBadCoordinates)).toThrowError(
-				'One or more of the coordinates are not valid: [{"coordinates":[181,0],"error":"Longitude must be between -180 and 180 degrees inclusive."},{"coordinates":[0,-91],"error":"Latitude must be between -90 and 90 degrees inclusive."}]'
+			expect(() =>
+				validateLinearRing(linearRingBadCoordinates, 'linearRingBadCoordinates')
+			).toThrowError(
+				'linearRingBadCoordinates: One or more of the coordinates in the Polygon LinearRing are not valid: [{"coordinates":[181,0],"error":"Longitude must be between -180 and 180 degrees inclusive."},{"coordinates":[0,-91],"error":"Latitude must be between -90 and 90 degrees inclusive."}]'
+			);
+		});
+		test('should error if the coordinates are not in counterclockwise order', () => {
+			expect(() =>
+				validateLinearRing(clockwiseLinearRing, 'clockwiseLinearRing')
+			).toThrowError(
+				'clockwiseLinearRing: LinearRing coordinates must be wound counterclockwise'
 			);
 		});
 	});
@@ -93,32 +107,30 @@ describe('Geo utility functions', () => {
 			expect(() => validatePolygon(validPolygon)).not.toThrowError();
 		});
 		test('should error if polygon is not a length of 1', () => {
-			expect(() => validatePolygon(polygonTooBig)).toThrowError(
-				`Polygon ${JSON.stringify(
-					polygonTooBig
-				)} geometry.polygon must have a single LinearRing array`
+			expect(() =>
+				validatePolygon(polygonTooBig, 'polygonTooBig')
+			).toThrowError(
+				`polygonTooBig: Polygon must have a single LinearRing array. Note: We do not currently support polygons with holes, multipolygons, polygons that are wound clockwise, or that cross the antimeridian.`
 			);
-			expect(() => validatePolygon([])).toThrowError(
-				`Polygon ${JSON.stringify(
-					[]
-				)} geometry.polygon must have a single LinearRing array`
+			expect(() => validatePolygon([], 'emptyPolygon')).toThrowError(
+				`emptyPolygon: Polygon must have a single LinearRing array.`
 			);
 		});
 	});
 
-	describe('validateGeofences', () => {
+	describe('validateGeofencesInput', () => {
 		test('should not throw an error for valid geofences', () => {
-			const result = validateGeofences(validGeofences);
+			const result = validateGeofencesInput(validGeofences);
 			expect(() => result).not.toThrowError();
 		});
 		test('should error if a geofenceId is not unique', () => {
-			expect(() => validateGeofences(geofencesWithDuplicate)).toThrowError(
+			expect(() => validateGeofencesInput(geofencesWithDuplicate)).toThrowError(
 				`Duplicate geofenceId: validGeofenceId1`
 			);
 		});
 		test('should error if a geofenceId is not valid', () => {
-			expect(() => validateGeofences(geofencesWithInvalidId)).toThrowError(
-				`Invalid geofenceId: t|-|!$ !$ N()T V@|_!D Ids can only contain alphanumeric characters, hyphens, underscores and periods.`
+			expect(() => validateGeofencesInput(geofencesWithInvalidId)).toThrowError(
+				`Invalid geofenceId: 't|-|!$ !$ N()T V@|_!D' - IDs can only contain alphanumeric characters, hyphens, underscores and periods.`
 			);
 		});
 	});

packages/geo/package.json

@@ -43,7 +43,8 @@
 	"dependencies": {
 		"@aws-amplify/core": "4.4.2",
 		"@aws-sdk/client-location": "3.48.0",
-		"camelcase-keys": "6.2.2"
+		"camelcase-keys": "6.2.2",
+		"@turf/boolean-clockwise": "6.5.0"
 	},
 	"jest": {
 		"globals": {

packages/geo/src/Geo.ts

@@ -17,18 +17,13 @@ import {
 } from '@aws-amplify/core';
 import { AmazonLocationServiceProvider } from './Providers/AmazonLocationServiceProvider';
 
-import {
-	validateCoordinates,
-	validateGeofences,
-	validateGeofenceId,
-} from './util';
+import { validateCoordinates } from './util';
 
 import {
 	Place,
 	GeoConfig,
 	Coordinates,
 	SearchByTextOptions,
-	SearchForSuggestionsResults,
 	SearchByCoordinatesOptions,
 	GeoProvider,
 	MapStyle,
@@ -240,8 +235,6 @@ export class GeoClass {
 		}
 
 		try {
-			// Validate all geofences are unique and valid before calling Provider
-			validateGeofences(geofenceInputArray);
 			return await prov.saveGeofences(geofenceInputArray, options);
 		} catch (error) {
 			logger.debug(error);
@@ -263,8 +256,6 @@ export class GeoClass {
 		const prov = this.getPluggable(providerName);
 
 		try {
-			// Validate geofenceId is valid before calling Provider
-			validateGeofenceId(geofenceId);
 			return await prov.getGeofence(geofenceId, options);
 		} catch (error) {
 			logger.debug(error);
@@ -316,20 +307,6 @@ export class GeoClass {
 			geofenceIdsInputArray = geofenceIds;
 		}
 
-		// Validate all geofenceIds are valid
-		const badGeofenceIds = geofenceIdsInputArray.filter(geofenceId => {
-			try {
-				validateGeofenceId(geofenceId);
-			} catch (error) {
-				return false;
-			}
-		});
-		if (badGeofenceIds.length > 0) {
-			const errorString = `Invalid geofence ids: ${badGeofenceIds}`;
-			logger.debug(errorString);
-			throw new Error(errorString);
-		}
-
 		//  Delete geofences
 		try {
 			return await prov.deleteGeofences(geofenceIdsInputArray, options);