From 6cd154aac69e25e9dfe50c64b03052d2aced774a Mon Sep 17 00:00:00 2001 From: Marie <51697796+ijreilly@users.noreply.github.com> Date: Thu, 4 Jul 2024 15:32:42 +0200 Subject: [PATCH] Forbid names above 63 characters to comply with pg identifier limit (#6095) Fixes #6032. Pg has a char limit on identifiers (= table, columns, enum names) of 63 bytes. Let's limit the metadata names that will be converted to identifiers (objects names, fields names, relation names, enum values) to 63 chars. For the sake of simplicity in the FE we will limit the input length of labels. --------- Co-authored-by: Charles Bochet --- .../DatabaseIdentifierMaximumLength.ts | 1 + .../constants/FieldNameMaximumLength.ts | 3 ++ .../constants/ObjectNameMaximumLength.ts | 3 ++ .../constants/OptionValueMaximumLength.ts | 3 ++ .../SettingsDataModelFieldAboutForm.tsx | 3 ++ .../SettingsDataModelFieldRelationForm.tsx | 2 + ...tingsDataModelFieldSelectFormOptionRow.tsx | 2 + .../SettingsDataModelObjectAboutForm.tsx | 2 + .../ui/input/components/TextInputV2.tsx | 11 +++++- .../data-model/SettingsObjectFieldEdit.tsx | 2 + .../SettingsObjectNewFieldStep2.tsx | 5 ++- .../field-metadata/field-metadata.service.ts | 22 ++++++++++- .../validate-object-metadata-input.util.ts | 19 +++++++++- .../relation-metadata.service.ts | 6 +-- .../__tests__/validate-metadata-name.spec.ts | 37 +++++++++++++++---- .../utils/metadata.constants.ts | 1 + ...lidate-database-identifier-length.utils.ts | 5 +++ .../utils/validate-metadata-name.utils.ts | 19 ++++++++-- 18 files changed, 127 insertions(+), 19 deletions(-) create mode 100644 packages/twenty-front/src/modules/settings/data-model/constants/DatabaseIdentifierMaximumLength.ts create mode 100644 packages/twenty-front/src/modules/settings/data-model/constants/FieldNameMaximumLength.ts create mode 100644 packages/twenty-front/src/modules/settings/data-model/constants/ObjectNameMaximumLength.ts create mode 100644 packages/twenty-front/src/modules/settings/data-model/constants/OptionValueMaximumLength.ts create mode 100644 packages/twenty-server/src/engine/metadata-modules/utils/metadata.constants.ts create mode 100644 packages/twenty-server/src/engine/metadata-modules/utils/validate-database-identifier-length.utils.ts diff --git a/packages/twenty-front/src/modules/settings/data-model/constants/DatabaseIdentifierMaximumLength.ts b/packages/twenty-front/src/modules/settings/data-model/constants/DatabaseIdentifierMaximumLength.ts new file mode 100644 index 0000000000..d7ac7dd9a2 --- /dev/null +++ b/packages/twenty-front/src/modules/settings/data-model/constants/DatabaseIdentifierMaximumLength.ts @@ -0,0 +1 @@ +export const DATABASE_IDENTIFIER_MAXIMUM_LENGTH = 63; diff --git a/packages/twenty-front/src/modules/settings/data-model/constants/FieldNameMaximumLength.ts b/packages/twenty-front/src/modules/settings/data-model/constants/FieldNameMaximumLength.ts new file mode 100644 index 0000000000..bd647b2cbe --- /dev/null +++ b/packages/twenty-front/src/modules/settings/data-model/constants/FieldNameMaximumLength.ts @@ -0,0 +1,3 @@ +import { DATABASE_IDENTIFIER_MAXIMUM_LENGTH } from '@/settings/data-model/constants/DatabaseIdentifierMaximumLength'; + +export const FIELD_NAME_MAXIMUM_LENGTH = DATABASE_IDENTIFIER_MAXIMUM_LENGTH; diff --git a/packages/twenty-front/src/modules/settings/data-model/constants/ObjectNameMaximumLength.ts b/packages/twenty-front/src/modules/settings/data-model/constants/ObjectNameMaximumLength.ts new file mode 100644 index 0000000000..6ba3692374 --- /dev/null +++ b/packages/twenty-front/src/modules/settings/data-model/constants/ObjectNameMaximumLength.ts @@ -0,0 +1,3 @@ +import { DATABASE_IDENTIFIER_MAXIMUM_LENGTH } from '@/settings/data-model/constants/DatabaseIdentifierMaximumLength'; + +export const OBJECT_NAME_MAXIMUM_LENGTH = DATABASE_IDENTIFIER_MAXIMUM_LENGTH; diff --git a/packages/twenty-front/src/modules/settings/data-model/constants/OptionValueMaximumLength.ts b/packages/twenty-front/src/modules/settings/data-model/constants/OptionValueMaximumLength.ts new file mode 100644 index 0000000000..97f5cc7726 --- /dev/null +++ b/packages/twenty-front/src/modules/settings/data-model/constants/OptionValueMaximumLength.ts @@ -0,0 +1,3 @@ +import { DATABASE_IDENTIFIER_MAXIMUM_LENGTH } from '@/settings/data-model/constants/DatabaseIdentifierMaximumLength'; + +export const OPTION_VALUE_MAXIMUM_LENGTH = DATABASE_IDENTIFIER_MAXIMUM_LENGTH; diff --git a/packages/twenty-front/src/modules/settings/data-model/fields/forms/components/SettingsDataModelFieldAboutForm.tsx b/packages/twenty-front/src/modules/settings/data-model/fields/forms/components/SettingsDataModelFieldAboutForm.tsx index e4c5e2141d..01411114ce 100644 --- a/packages/twenty-front/src/modules/settings/data-model/fields/forms/components/SettingsDataModelFieldAboutForm.tsx +++ b/packages/twenty-front/src/modules/settings/data-model/fields/forms/components/SettingsDataModelFieldAboutForm.tsx @@ -22,6 +22,7 @@ type SettingsDataModelFieldAboutFormValues = z.infer< type SettingsDataModelFieldAboutFormProps = { disabled?: boolean; fieldMetadataItem?: FieldMetadataItem; + maxLength?: number; }; const StyledInputsContainer = styled.div` @@ -34,6 +35,7 @@ const StyledInputsContainer = styled.div` export const SettingsDataModelFieldAboutForm = ({ disabled, fieldMetadataItem, + maxLength, }: SettingsDataModelFieldAboutFormProps) => { const { control } = useFormContext(); @@ -63,6 +65,7 @@ export const SettingsDataModelFieldAboutForm = ({ value={value} onChange={onChange} disabled={disabled} + maxLength={maxLength} fullWidth /> )} diff --git a/packages/twenty-front/src/modules/settings/data-model/fields/forms/relation/components/SettingsDataModelFieldRelationForm.tsx b/packages/twenty-front/src/modules/settings/data-model/fields/forms/relation/components/SettingsDataModelFieldRelationForm.tsx index f41b48bbf5..4415bc64aa 100644 --- a/packages/twenty-front/src/modules/settings/data-model/fields/forms/relation/components/SettingsDataModelFieldRelationForm.tsx +++ b/packages/twenty-front/src/modules/settings/data-model/fields/forms/relation/components/SettingsDataModelFieldRelationForm.tsx @@ -7,6 +7,7 @@ import { useFilteredObjectMetadataItems } from '@/object-metadata/hooks/useFilte import { FieldMetadataItem } from '@/object-metadata/types/FieldMetadataItem'; import { isObjectMetadataAvailableForRelation } from '@/object-metadata/utils/isObjectMetadataAvailableForRelation'; import { fieldMetadataItemSchema } from '@/object-metadata/validation-schemas/fieldMetadataItemSchema'; +import { FIELD_NAME_MAXIMUM_LENGTH } from '@/settings/data-model/constants/FieldNameMaximumLength'; import { RELATION_TYPES } from '@/settings/data-model/constants/RelationTypes'; import { useRelationSettingsFormInitialValues } from '@/settings/data-model/fields/forms/relation/hooks/useRelationSettingsFormInitialValues'; import { RelationType } from '@/settings/data-model/types/RelationType'; @@ -163,6 +164,7 @@ export const SettingsDataModelFieldRelationForm = ({ value={value} onChange={onChange} fullWidth + maxLength={FIELD_NAME_MAXIMUM_LENGTH} /> )} /> diff --git a/packages/twenty-front/src/modules/settings/data-model/fields/forms/select/components/SettingsDataModelFieldSelectFormOptionRow.tsx b/packages/twenty-front/src/modules/settings/data-model/fields/forms/select/components/SettingsDataModelFieldSelectFormOptionRow.tsx index c8f4174fbd..0cb96fc764 100644 --- a/packages/twenty-front/src/modules/settings/data-model/fields/forms/select/components/SettingsDataModelFieldSelectFormOptionRow.tsx +++ b/packages/twenty-front/src/modules/settings/data-model/fields/forms/select/components/SettingsDataModelFieldSelectFormOptionRow.tsx @@ -13,6 +13,7 @@ import { import { v4 } from 'uuid'; import { FieldMetadataItemOption } from '@/object-metadata/types/FieldMetadataItem'; +import { OPTION_VALUE_MAXIMUM_LENGTH } from '@/settings/data-model/constants/OptionValueMaximumLength'; import { getOptionValueFromLabel } from '@/settings/data-model/fields/forms/select/utils/getOptionValueFromLabel'; import { LightIconButton } from '@/ui/input/button/components/LightIconButton'; import { TextInput } from '@/ui/input/components/TextInput'; @@ -133,6 +134,7 @@ export const SettingsDataModelFieldSelectFormOptionRow = ({ }) } RightIcon={isDefault ? IconCheck : undefined} + maxLength={OPTION_VALUE_MAXIMUM_LENGTH} /> )} /> diff --git a/packages/twenty-front/src/modules/ui/input/components/TextInputV2.tsx b/packages/twenty-front/src/modules/ui/input/components/TextInputV2.tsx index 19a0b92ada..530f27a7c1 100644 --- a/packages/twenty-front/src/modules/ui/input/components/TextInputV2.tsx +++ b/packages/twenty-front/src/modules/ui/input/components/TextInputV2.tsx @@ -145,6 +145,7 @@ const TextInputV2Component = ( RightIcon, LeftIcon, autoComplete, + maxLength, }: TextInputV2ComponentProps, // eslint-disable-next-line @nx/workspace-component-props-naming ref: ForwardedRef, @@ -182,7 +183,15 @@ const TextInputV2Component = ( onChange?.(event.target.value); }} onKeyDown={onKeyDown} - {...{ autoFocus, disabled, placeholder, required, value, LeftIcon }} + {...{ + autoFocus, + disabled, + placeholder, + required, + value, + LeftIcon, + maxLength, + }} /> {error && ( diff --git a/packages/twenty-front/src/pages/settings/data-model/SettingsObjectFieldEdit.tsx b/packages/twenty-front/src/pages/settings/data-model/SettingsObjectFieldEdit.tsx index bb2ac5e637..acc0bf1d1a 100644 --- a/packages/twenty-front/src/pages/settings/data-model/SettingsObjectFieldEdit.tsx +++ b/packages/twenty-front/src/pages/settings/data-model/SettingsObjectFieldEdit.tsx @@ -22,6 +22,7 @@ import { RecordFieldValueSelectorContextProvider } from '@/object-record/record- import { SaveAndCancelButtons } from '@/settings/components/SaveAndCancelButtons/SaveAndCancelButtons'; import { SettingsHeaderContainer } from '@/settings/components/SettingsHeaderContainer'; import { SettingsPageContainer } from '@/settings/components/SettingsPageContainer'; +import { FIELD_NAME_MAXIMUM_LENGTH } from '@/settings/data-model/constants/FieldNameMaximumLength'; import { SettingsDataModelFieldAboutForm } from '@/settings/data-model/fields/forms/components/SettingsDataModelFieldAboutForm'; import { SettingsDataModelFieldSettingsFormCard } from '@/settings/data-model/fields/forms/components/SettingsDataModelFieldSettingsFormCard'; import { SettingsDataModelFieldTypeSelect } from '@/settings/data-model/fields/forms/components/SettingsDataModelFieldTypeSelect'; @@ -202,6 +203,7 @@ export const SettingsObjectFieldEdit = () => {
diff --git a/packages/twenty-front/src/pages/settings/data-model/SettingsObjectNewField/SettingsObjectNewFieldStep2.tsx b/packages/twenty-front/src/pages/settings/data-model/SettingsObjectNewField/SettingsObjectNewFieldStep2.tsx index da7fa13d25..2ab3322796 100644 --- a/packages/twenty-front/src/pages/settings/data-model/SettingsObjectNewField/SettingsObjectNewFieldStep2.tsx +++ b/packages/twenty-front/src/pages/settings/data-model/SettingsObjectNewField/SettingsObjectNewFieldStep2.tsx @@ -17,6 +17,7 @@ import { RecordFieldValueSelectorContextProvider } from '@/object-record/record- import { SaveAndCancelButtons } from '@/settings/components/SaveAndCancelButtons/SaveAndCancelButtons'; import { SettingsHeaderContainer } from '@/settings/components/SettingsHeaderContainer'; import { SettingsPageContainer } from '@/settings/components/SettingsPageContainer'; +import { FIELD_NAME_MAXIMUM_LENGTH } from '@/settings/data-model/constants/FieldNameMaximumLength'; import { SettingsDataModelFieldAboutForm } from '@/settings/data-model/fields/forms/components/SettingsDataModelFieldAboutForm'; import { SettingsDataModelFieldSettingsFormCard } from '@/settings/data-model/fields/forms/components/SettingsDataModelFieldSettingsFormCard'; import { SettingsDataModelFieldTypeSelect } from '@/settings/data-model/fields/forms/components/SettingsDataModelFieldTypeSelect'; @@ -202,7 +203,9 @@ export const SettingsObjectNewFieldStep2 = () => { title="Name and description" description="The name and description of this field" /> - +
(fieldMetadataInput: T): T { if (fieldMetadataInput.name) { try { - validateMetadataName(fieldMetadataInput.name); + validateMetadataNameOrThrow(fieldMetadataInput.name); } catch (error) { if (error instanceof InvalidStringException) { throw new FieldMetadataException( `Characters used in name "${fieldMetadataInput.name}" are not supported`, FieldMetadataExceptionCode.INVALID_FIELD_INPUT, ); + } else if (error instanceof NameTooLongException) { + throw new FieldMetadataException( + `Name "${fieldMetadataInput.name}" exceeds 63 characters`, + FieldMetadataExceptionCode.INVALID_FIELD_INPUT, + ); } else { throw error; } } } + if (fieldMetadataInput.options) { + for (const option of fieldMetadataInput.options) { + if (exceedsDatabaseIdentifierMaximumLength(option.value)) { + throw new FieldMetadataException( + `Option value "${option.value}" exceeds 63 characters`, + FieldMetadataExceptionCode.INVALID_FIELD_INPUT, + ); + } + } + } + return fieldMetadataInput; } } diff --git a/packages/twenty-server/src/engine/metadata-modules/object-metadata/utils/validate-object-metadata-input.util.ts b/packages/twenty-server/src/engine/metadata-modules/object-metadata/utils/validate-object-metadata-input.util.ts index 23087298be..a8a07ccc33 100644 --- a/packages/twenty-server/src/engine/metadata-modules/object-metadata/utils/validate-object-metadata-input.util.ts +++ b/packages/twenty-server/src/engine/metadata-modules/object-metadata/utils/validate-object-metadata-input.util.ts @@ -4,8 +4,9 @@ import { ObjectMetadataException, ObjectMetadataExceptionCode, } from 'src/engine/metadata-modules/object-metadata/object-metadata.exception'; +import { exceedsDatabaseIdentifierMaximumLength } from 'src/engine/metadata-modules/utils/validate-database-identifier-length.utils'; import { - validateMetadataName, + validateMetadataNameOrThrow, InvalidStringException, } from 'src/engine/metadata-modules/utils/validate-metadata-name.utils'; import { camelCase } from 'src/utils/camel-case'; @@ -54,6 +55,9 @@ export const validateObjectMetadataInputOrThrow = < validateNameIsNotReservedKeywordOrThrow(objectMetadataInput.nameSingular); validateNameIsNotReservedKeywordOrThrow(objectMetadataInput.namePlural); + + validateNameIsNotTooLongThrow(objectMetadataInput.nameSingular); + validateNameIsNotTooLongThrow(objectMetadataInput.namePlural); }; const validateNameIsNotReservedKeywordOrThrow = (name?: string) => { @@ -78,10 +82,21 @@ const validateNameCamelCasedOrThrow = (name?: string) => { } }; +const validateNameIsNotTooLongThrow = (name?: string) => { + if (name) { + if (exceedsDatabaseIdentifierMaximumLength(name)) { + throw new ObjectMetadataException( + `Name exceeds 63 characters: ${name}`, + ObjectMetadataExceptionCode.INVALID_OBJECT_INPUT, + ); + } + } +}; + const validateNameCharactersOrThrow = (name?: string) => { try { if (name) { - validateMetadataName(name); + validateMetadataNameOrThrow(name); } } catch (error) { if (error instanceof InvalidStringException) { diff --git a/packages/twenty-server/src/engine/metadata-modules/relation-metadata/relation-metadata.service.ts b/packages/twenty-server/src/engine/metadata-modules/relation-metadata/relation-metadata.service.ts index 8ba348dcd7..b287f9e6fa 100644 --- a/packages/twenty-server/src/engine/metadata-modules/relation-metadata/relation-metadata.service.ts +++ b/packages/twenty-server/src/engine/metadata-modules/relation-metadata/relation-metadata.service.ts @@ -24,7 +24,7 @@ import { ObjectMetadataEntity } from 'src/engine/metadata-modules/object-metadat import { computeObjectTargetTable } from 'src/engine/utils/compute-object-target-table.util'; import { generateMigrationName } from 'src/engine/metadata-modules/workspace-migration/utils/generate-migration-name.util'; import { - validateMetadataName, + validateMetadataNameOrThrow, InvalidStringException, } from 'src/engine/metadata-modules/utils/validate-metadata-name.utils'; import { WorkspaceCacheVersionService } from 'src/engine/metadata-modules/workspace-cache-version/workspace-cache-version.service'; @@ -63,8 +63,8 @@ export class RelationMetadataService extends TypeOrmQueryService { +describe('validateMetadataNameOrThrow', () => { it('does not throw if string is valid', () => { const input = 'testName'; - expect(validateMetadataName(input)).not.toThrow; + expect(validateMetadataNameOrThrow(input)).not.toThrow; }); it('throws error if string has spaces', () => { const input = 'name with spaces'; - expect(() => validateMetadataName(input)).toThrow(InvalidStringException); + expect(() => validateMetadataNameOrThrow(input)).toThrow( + InvalidStringException, + ); }); it('throws error if string starts with capital letter', () => { const input = 'StringStartingWithCapitalLetter'; - expect(() => validateMetadataName(input)).toThrow(InvalidStringException); + expect(() => validateMetadataNameOrThrow(input)).toThrow( + InvalidStringException, + ); }); it('throws error if string has non latin characters', () => { const input = 'בְרִבְרִ'; - expect(() => validateMetadataName(input)).toThrow(InvalidStringException); + expect(() => validateMetadataNameOrThrow(input)).toThrow( + InvalidStringException, + ); }); it('throws error if starts with digits', () => { const input = '123string'; - expect(() => validateMetadataName(input)).toThrow(InvalidStringException); + expect(() => validateMetadataNameOrThrow(input)).toThrow( + InvalidStringException, + ); + }); + it('does not throw if string is less than 63 characters', () => { + const inputWith63Characters = + 'thisIsAstringWithSixtyThreeCharacters11111111111111111111111111'; + + expect(validateMetadataNameOrThrow(inputWith63Characters)).not.toThrow; + }); + it('throws error if string is above 63 characters', () => { + const inputWith64Characters = + 'thisIsAstringWithSixtyFourCharacters1111111111111111111111111111'; + + expect(() => validateMetadataNameOrThrow(inputWith64Characters)).toThrow( + NameTooLongException, + ); }); }); diff --git a/packages/twenty-server/src/engine/metadata-modules/utils/metadata.constants.ts b/packages/twenty-server/src/engine/metadata-modules/utils/metadata.constants.ts new file mode 100644 index 0000000000..ce2dabd729 --- /dev/null +++ b/packages/twenty-server/src/engine/metadata-modules/utils/metadata.constants.ts @@ -0,0 +1 @@ +export const IDENTIFIER_MAX_CHAR_LENGTH = 63; diff --git a/packages/twenty-server/src/engine/metadata-modules/utils/validate-database-identifier-length.utils.ts b/packages/twenty-server/src/engine/metadata-modules/utils/validate-database-identifier-length.utils.ts new file mode 100644 index 0000000000..dff01b4f33 --- /dev/null +++ b/packages/twenty-server/src/engine/metadata-modules/utils/validate-database-identifier-length.utils.ts @@ -0,0 +1,5 @@ +import { IDENTIFIER_MAX_CHAR_LENGTH } from 'src/engine/metadata-modules/utils/metadata.constants'; + +export const exceedsDatabaseIdentifierMaximumLength = (string: string) => { + return string.length > IDENTIFIER_MAX_CHAR_LENGTH; +}; diff --git a/packages/twenty-server/src/engine/metadata-modules/utils/validate-metadata-name.utils.ts b/packages/twenty-server/src/engine/metadata-modules/utils/validate-metadata-name.utils.ts index 0711863cdb..72835eead4 100644 --- a/packages/twenty-server/src/engine/metadata-modules/utils/validate-metadata-name.utils.ts +++ b/packages/twenty-server/src/engine/metadata-modules/utils/validate-metadata-name.utils.ts @@ -1,8 +1,13 @@ +import { exceedsDatabaseIdentifierMaximumLength } from 'src/engine/metadata-modules/utils/validate-database-identifier-length.utils'; + const VALID_STRING_PATTERN = /^[a-z][a-zA-Z0-9]*$/; -export const validateMetadataName = (string: string) => { - if (!string.match(VALID_STRING_PATTERN)) { - throw new InvalidStringException(string); +export const validateMetadataNameOrThrow = (name: string) => { + if (!name.match(VALID_STRING_PATTERN)) { + throw new InvalidStringException(name); + } + if (exceedsDatabaseIdentifierMaximumLength(name)) { + throw new NameTooLongException(name); } }; @@ -13,3 +18,11 @@ export class InvalidStringException extends Error { super(message); } } + +export class NameTooLongException extends Error { + constructor(string: string) { + const message = `String "${string}" exceeds 63 characters limit`; + + super(message); + } +}