fix(scene): validation scene json on deserialize

awslabs · Apr 12, 2024 · 3f2ad73 · 3f2ad73
1 parent 760aa8c
commit 3f2ad73
Show file tree

Hide file tree

Showing 12 changed files with 710 additions and 50 deletions.
diff --git a/packages/scene-composer/public/invalidInputTest.scene.json b/packages/scene-composer/public/invalidInputTest.scene.json
diff --git a/...rc/components/panels/scene-components/tag-style/ColorSelectorCombo/ColorSelectorCombo.tsx b/...rc/components/panels/scene-components/tag-style/ColorSelectorCombo/ColorSelectorCombo.tsx
@@ -7,6 +7,7 @@ import '../IconPicker/IconPickerUtils/IconPicker-aws-overrides.scss';
 import { ColorRepresentation } from 'three';
 
 import { colors } from '../../../../../utils/styleUtils';
+import { isValidHexCode } from '../../../../../utils/colorUtils';
 import { IColorPickerProps } from '../interface';
 import { ColorPicker } from '../../../ColorPicker/ColorPicker';
 import { hexString } from '../../../ColorPicker/ColorPickerHelpers';
@@ -40,19 +41,6 @@ export const ColorSelectorCombo = ({
   const [randomDomId] = useState<string>(generateRandomString);
   const intl = useIntl();
 
-  /**
-   * This method uses a regular expression (`hexRegex`) to validate a hex color code.
-   * The regex checks if the hex code starts with a "#" symbol, followed by either a
-   * 6-digit or 3-digit combination of characters from A-F, a-f, and 0-9.
-   * The `test` method is then used to validate the `hexCode` against the regex pattern.
-   * @param hexCode
-   * @returns
-   */
-  const isValidHexCode = (hexCode: string) => {
-    const hexRegex = /^#([A-Fa-f0-9]{6})$/;
-    return hexRegex.test(hexCode);
-  };
-
   const handleOutsideClick = useCallback((event: MouseEvent) => {
     const target = event.target as HTMLElement;
     const buttonsvg = document.getElementById('button-svg' + `${randomDomId}`);

diff --git a/packages/scene-composer/src/components/panels/scene-settings/FogSettingsEditor.spec.tsx b/packages/scene-composer/src/components/panels/scene-settings/FogSettingsEditor.spec.tsx
@@ -24,7 +24,14 @@ describe('FogSettingsEditor', () => {
   });
 
   it('should save fogsettings when enabled', async () => {
-    getScenePropertyMock.mockReturnValue(undefined);
+    getScenePropertyMock.mockImplementation((property: string) => {
+      if (property === KnownSceneProperty.FogSettings) {
+        return undefined;
+      } else if (property === KnownSceneProperty.FogCustomColors) {
+        const customColors: string[] = [];
+        return customColors;
+      }
+    });
     useStore('default').setState(baseState);
     const { container } = render(<FogSettingsEditor />);
     const polarisWrapper = wrapper(container);
@@ -45,10 +52,17 @@ describe('FogSettingsEditor', () => {
   });
 
   it('should clear fogsettings when untoggled', async () => {
-    getScenePropertyMock.mockReturnValue({
-      color: '#cccccc',
-      near: 1,
-      far: 1000,
+    getScenePropertyMock.mockImplementation((property: string) => {
+      if (property === KnownSceneProperty.FogSettings) {
+        return {
+          color: '#cccccc',
+          near: 1,
+          far: 1000,
+        };
+      } else if (property === KnownSceneProperty.FogCustomColors) {
+        const customColors: string[] = [];
+        return customColors;
+      }
     });
     useStore('default').setState(baseState);
     const { container } = render(<FogSettingsEditor />);
@@ -66,10 +80,17 @@ describe('FogSettingsEditor', () => {
   });
 
   it('should update fog when near changes', async () => {
-    getScenePropertyMock.mockReturnValue({
-      color: '#cccccc',
-      near: 1,
-      far: 1000,
+    getScenePropertyMock.mockImplementation((property: string) => {
+      if (property === KnownSceneProperty.FogSettings) {
+        return {
+          color: '#cccccc',
+          near: 1,
+          far: 1000,
+        };
+      } else if (property === KnownSceneProperty.FogCustomColors) {
+        const customColors: string[] = [];
+        return customColors;
+      }
     });
     useStore('default').setState(baseState);
     const { container } = render(<FogSettingsEditor />);
@@ -91,10 +112,17 @@ describe('FogSettingsEditor', () => {
   });
 
   it('should update fog when far changes', async () => {
-    getScenePropertyMock.mockReturnValue({
-      color: '#cccccc',
-      near: 1,
-      far: 1000,
+    getScenePropertyMock.mockImplementation((property: string) => {
+      if (property === KnownSceneProperty.FogSettings) {
+        return {
+          color: '#cccccc',
+          near: 1,
+          far: 1000,
+        };
+      } else if (property === KnownSceneProperty.FogCustomColors) {
+        const customColors: string[] = [];
+        return customColors;
+      }
     });
     useStore('default').setState(baseState);
     const { container } = render(<FogSettingsEditor />);

diff --git a/packages/scene-composer/src/components/panels/scene-settings/FogSettingsEditor.tsx b/packages/scene-composer/src/components/panels/scene-settings/FogSettingsEditor.tsx
@@ -3,7 +3,13 @@ import { useIntl } from 'react-intl';
 import { FormField, Input, SpaceBetween, Toggle } from '@cloudscape-design/components';
 
 import { sceneComposerIdContext } from '../../../common/sceneComposerIdContext';
-import { IFogSettings, KnownSceneProperty } from '../../../interfaces';
+import {
+  DEFAULT_FOG_COLOR,
+  DEFAULT_FOG_FAR,
+  DEFAULT_FOG_NEAR,
+  IFogSettings,
+  KnownSceneProperty,
+} from '../../../interfaces';
 import useLifecycleLogging from '../../../logger/react-logger/hooks/useLifecycleLogging';
 import { useStore } from '../../../store';
 import { ColorSelectorCombo } from '../scene-components/tag-style/ColorSelectorCombo/ColorSelectorCombo';
@@ -19,9 +25,9 @@ export const FogSettingsEditor: React.FC = () => {
   );
 
   const [fogEnabled, setFogEnabled] = useState(!!fogSettings);
-  const [internalColor, setInternalColor] = useState(fogSettings?.color || '#cccccc');
-  const [internalNearDistance, setInternalNearDistance] = useState(fogSettings?.near || 1);
-  const [internalFarDistance, setInternalFarDistance] = useState(fogSettings?.far || 1000);
+  const [internalColor, setInternalColor] = useState(fogSettings?.color || DEFAULT_FOG_COLOR);
+  const [internalNearDistance, setInternalNearDistance] = useState(fogSettings?.near || DEFAULT_FOG_NEAR);
+  const [internalFarDistance, setInternalFarDistance] = useState(fogSettings?.far || DEFAULT_FOG_FAR);
 
   const fogColors = useStore(sceneComposerId)((state) =>
     state.getSceneProperty<string[]>(KnownSceneProperty.FogCustomColors, []),

diff --git a/...es/scene-composer/src/components/panels/scene-settings/GroundPlaneSettingsEditor.spec.tsx b/...es/scene-composer/src/components/panels/scene-settings/GroundPlaneSettingsEditor.spec.tsx
@@ -66,9 +66,16 @@ describe('GroundPlaneSettingsEditor', () => {
   });
 
   it('should open asset browser when select texture clicked and set texture uri', () => {
-    getScenePropertyMock.mockReturnValue({
-      color: '#cccccc',
-      opacity: 0,
+    getScenePropertyMock.mockImplementation((property: string) => {
+      if (property === KnownSceneProperty.GroundPlaneSettings) {
+        return {
+          color: '#cccccc',
+          opacity: 0,
+        };
+      } else if (property === KnownSceneProperty.GroundCustomColors) {
+        const customColors: string[] = [];
+        return customColors;
+      }
     });
     const globalSettingsMock = getGlobalSettings as jest.Mock;
     globalSettingsMock.mockReturnValue({ featureConfig: mockFeatureConfigOn });
@@ -94,9 +101,16 @@ describe('GroundPlaneSettingsEditor', () => {
   });
 
   it('should remove texture uri', () => {
-    getScenePropertyMock.mockReturnValue({
-      textureUri: 'filepath',
-      opacity: 1,
+    getScenePropertyMock.mockImplementation((property: string) => {
+      if (property === KnownSceneProperty.GroundPlaneSettings) {
+        return {
+          textureUri: 'filepath',
+          opacity: 1,
+        };
+      } else if (property === KnownSceneProperty.GroundCustomColors) {
+        const customColors: string[] = [];
+        return customColors;
+      }
     });
     const globalSettingsMock = getGlobalSettings as jest.Mock;
     globalSettingsMock.mockReturnValue({ featureConfig: mockFeatureConfigOn });
@@ -118,9 +132,16 @@ describe('GroundPlaneSettingsEditor', () => {
   });
 
   it('should update opacity', () => {
-    getScenePropertyMock.mockReturnValue({
-      textureUri: 'filepath',
-      opacity: 0,
+    getScenePropertyMock.mockImplementation((property: string) => {
+      if (property === KnownSceneProperty.GroundPlaneSettings) {
+        return {
+          textureUri: 'filepath',
+          opacity: 0,
+        };
+      } else if (property === KnownSceneProperty.GroundCustomColors) {
+        const customColors: string[] = [];
+        return customColors;
+      }
     });
     const globalSettingsMock = getGlobalSettings as jest.Mock;
     globalSettingsMock.mockReturnValue({ featureConfig: mockFeatureConfigOn });

diff --git a/packages/scene-composer/src/components/panels/scene-settings/GroundPlaneSettingsEditor.tsx b/packages/scene-composer/src/components/panels/scene-settings/GroundPlaneSettingsEditor.tsx
@@ -4,7 +4,13 @@ import { Button, FormField, Input, SpaceBetween } from '@cloudscape-design/compo
 
 import { getGlobalSettings } from '../../../common/GlobalSettings';
 import { sceneComposerIdContext } from '../../../common/sceneComposerIdContext';
-import { IGroundPlaneSettings, KnownSceneProperty, COMPOSER_FEATURES, TextureFileTypeList } from '../../../interfaces';
+import {
+  DEFAULT_GROUND_PLANE_COLOR,
+  IGroundPlaneSettings,
+  KnownSceneProperty,
+  COMPOSER_FEATURES,
+  TextureFileTypeList,
+} from '../../../interfaces';
 import useLifecycleLogging from '../../../logger/react-logger/hooks/useLifecycleLogging';
 import { useStore } from '../../../store';
 import { ColorSelectorCombo } from '../scene-components/tag-style/ColorSelectorCombo/ColorSelectorCombo';
@@ -23,7 +29,7 @@ export const GroundPlaneSettingsEditor: React.FC = () => {
     state.getSceneProperty<IGroundPlaneSettings>(KnownSceneProperty.GroundPlaneSettings),
   );
 
-  const [internalColor, setInternalColor] = useState(groundSettings?.color || '#00FF00');
+  const [internalColor, setInternalColor] = useState(groundSettings?.color || DEFAULT_GROUND_PLANE_COLOR);
   const [internalUri, setInternalUri] = useState(groundSettings?.textureUri || '');
   const [internalOpacity, setInternalOpacity] = useState(groundSettings?.opacity || 0);
 

diff --git a/...cene-composer/src/components/panels/scene-settings/SceneBackgroundSettingsEditor.spec.tsx b/...cene-composer/src/components/panels/scene-settings/SceneBackgroundSettingsEditor.spec.tsx
@@ -34,7 +34,14 @@ describe('SceneBackgroundSettingsEditor', () => {
   });
 
   it('should save background on clean scene', () => {
-    getScenePropertyMock.mockReturnValue(undefined);
+    getScenePropertyMock.mockImplementation((property: string) => {
+      if (property === KnownSceneProperty.SceneBackgroundSettings) {
+        return undefined;
+      } else if (property === KnownSceneProperty.BackgroundCustomColors) {
+        const customColors: string[] = [];
+        return customColors;
+      }
+    });
     const globalSettingsMock = getGlobalSettings as jest.Mock;
     globalSettingsMock.mockReturnValue({ featureConfig: mockFeatureConfigOn });
     useStore('default').setState(baseState);
@@ -78,8 +85,15 @@ describe('SceneBackgroundSettingsEditor', () => {
   });
 
   it('should open asset browser when select texture clicked and set texture uri', () => {
-    getScenePropertyMock.mockReturnValue({
-      color: '#cccccc',
+    getScenePropertyMock.mockImplementation((property: string) => {
+      if (property === KnownSceneProperty.SceneBackgroundSettings) {
+        return {
+          color: '#cccccc',
+        };
+      } else if (property === KnownSceneProperty.BackgroundCustomColors) {
+        const customColors: string[] = [];
+        return customColors;
+      }
     });
     const globalSettingsMock = getGlobalSettings as jest.Mock;
     globalSettingsMock.mockReturnValue({ featureConfig: mockFeatureConfigOn });
@@ -104,8 +118,15 @@ describe('SceneBackgroundSettingsEditor', () => {
   });
 
   it('should remove texture uri', () => {
-    getScenePropertyMock.mockReturnValue({
-      textureUri: 'filepath',
+    getScenePropertyMock.mockImplementation((property: string) => {
+      if (property === KnownSceneProperty.SceneBackgroundSettings) {
+        return {
+          textureUri: 'filepath',
+        };
+      } else if (property === KnownSceneProperty.BackgroundCustomColors) {
+        const customColors: string[] = [];
+        return customColors;
+      }
     });
     const globalSettingsMock = getGlobalSettings as jest.Mock;
     globalSettingsMock.mockReturnValue({ featureConfig: mockFeatureConfigOn });

diff --git a/...ges/scene-composer/src/components/panels/scene-settings/SceneBackgroundSettingsEditor.tsx b/...ges/scene-composer/src/components/panels/scene-settings/SceneBackgroundSettingsEditor.tsx
@@ -6,6 +6,7 @@ import { getGlobalSettings } from '../../../common/GlobalSettings';
 import { sceneComposerIdContext } from '../../../common/sceneComposerIdContext';
 import {
   COMPOSER_FEATURES,
+  DEFAULT_SCENE_BACKGROUND_COLOR,
   ISceneBackgroundSetting,
   KnownSceneProperty,
   TextureFileTypeList,
@@ -31,19 +32,26 @@ export const SceneBackgroundSettingsEditor: React.FC = () => {
     (state) => state.getEditorConfig().showAssetBrowserCallback,
   );
 
-  const [internalColor, setInternalColor] = useState(backgroundSettings?.color || '#2a2e33');
+  const [internalColor, setInternalColor] = useState(backgroundSettings?.color || DEFAULT_SCENE_BACKGROUND_COLOR);
   const [internalUri, setInternalUri] = useState(backgroundSettings?.textureUri || '');
   const backgroundColors = useStore(sceneComposerId)((state) =>
     state.getSceneProperty<string[]>(KnownSceneProperty.BackgroundCustomColors, []),
   );
   const setBackgroundColorsSceneProperty = useStore(sceneComposerId)((state) => state.setSceneProperty<string[]>);
 
-  //set default
+  //set default and restore editor when background failed validiation elsewhere
   useEffect(() => {
     if (!backgroundSettings?.color && !backgroundSettings?.textureUri) {
       setSceneProperty(KnownSceneProperty.SceneBackgroundSettings, {
         color: internalColor,
       });
+    } else {
+      if (backgroundSettings.color && backgroundSettings.color !== internalColor) {
+        setInternalColor(backgroundSettings?.color);
+      }
+      if (backgroundSettings.textureUri && backgroundSettings.textureUri !== internalUri) {
+        setInternalUri(backgroundSettings.textureUri);
+      }
     }
   }, [backgroundSettings]);
 

diff --git a/packages/scene-composer/src/interfaces/interfaces.tsx b/packages/scene-composer/src/interfaces/interfaces.tsx
@@ -79,7 +79,7 @@ export enum KnownSceneProperty {
   BackgroundCustomColors = 'backgroundCustomColors',
   LayerDefaultRefreshInterval = 'layerDefaultRefreshInterval',
   GeometryCustomColors = 'geometryCustomColors',
-  GroundCustomColors = 'groundCustomColors,',
+  GroundCustomColors = 'groundCustomColors',
 }
 
 /************************************************
@@ -155,13 +155,19 @@ export interface IFogSettings {
   far: number;
 }
 
+export const DEFAULT_FOG_COLOR = '#cccccc';
+export const DEFAULT_FOG_NEAR = 1;
+export const DEFAULT_FOG_FAR = 1000;
+
 export interface ISceneBackgroundSetting {
   color?: string;
   textureUri?: string;
 }
+export const DEFAULT_SCENE_BACKGROUND_COLOR = '#2a2e33';
 
 export interface IGroundPlaneSettings {
   color?: string;
   textureUri?: string;
   opacity: number;
 }
+export const DEFAULT_GROUND_PLANE_COLOR = '#00FF00';