feat: make import/export work with project patterns (#4652)

This PR adds feature name pattern validation to the import validation
step. When errors occur, they are rendered with all the offending
features, the pattern to match, plus the pattern's description and
example if available.


![image](https://github.com/Unleash/unleash/assets/17786332/69956090-afc6-41c8-8f6e-fb45dfaf0a9d)

To achieve this I've added an extra method to the feature toggle service
that checks feature names without throwing errors (because catching `n`
async errors in a loop became tricky and hard to grasp). This method is
also reused in the existing feature name validation method and handles
the feature enabled chcek.

In doing so, I've also added tests to check that the pattern is applied.

src/lib/features/export-import-toggles/export-import-service.ts

@@ -45,6 +45,7 @@ import { IImportTogglesStore } from './import-toggles-store-type';
 import { ImportPermissionsService, Mode } from './import-permissions-service';
 import { ImportValidationMessages } from './import-validation-messages';
 import { findDuplicates } from '../../util/findDuplicates';
+import { FeatureNameCheckResult } from 'lib/services/feature-toggle-service';
 
 export default class ExportImportService {
     private logger: Logger;
@@ -156,6 +157,7 @@ export default class ExportImportService {
             existingProjectFeatures,
             missingPermissions,
             duplicateFeatures,
+            featureNameCheckResult,
         ] = await Promise.all([
             this.getUnsupportedStrategies(dto),
             this.getUsedCustomStrategies(dto),
@@ -169,6 +171,7 @@ export default class ExportImportService {
                 mode,
             ),
             this.getDuplicateFeatures(dto),
+            this.getInvalidFeatureNames(dto),
         ]);
 
         const errors = ImportValidationMessages.compileErrors({
@@ -177,6 +180,7 @@ export default class ExportImportService {
             contextFields: unsupportedContextFields || [],
             otherProjectFeatures,
             duplicateFeatures,
+            featureNameCheckResult,
         });
         const warnings = ImportValidationMessages.compileWarnings({
             archivedFeatures,
@@ -500,6 +504,16 @@ export default class ExportImportService {
         }
     }
 
+    private async getInvalidFeatureNames({
+        project,
+        data,
+    }: ImportTogglesSchema): Promise<FeatureNameCheckResult> {
+        return this.featureToggleService.checkFeatureFlagNamesAgainstProjectPattern(
+            project,
+            data.features.map((f) => f.name),
+        );
+    }
+
     private async getUnsupportedStrategies(
         dto: ImportTogglesSchema,
     ): Promise<FeatureStrategySchema[]> {

src/lib/features/export-import-toggles/export-import.e2e.test.ts

@@ -144,6 +144,7 @@ beforeAll(async () => {
             experimental: {
                 flags: {
                     featuresExportImport: true,
+                    featureNamingPattern: true,
                 },
             },
         },
@@ -833,6 +834,7 @@ test('reject import with duplicate features', async () => {
 
 test('validate import data', async () => {
     await createProjects();
+
     const contextField: IContextFieldDto = {
         name: 'validate_context_field',
         legalValues: [{ value: 'Value1' }],
@@ -864,6 +866,16 @@ test('validate import data', async () => {
         },
     };
 
+    // note: this must be done after creating the feature on the earlier lines,
+    // to prevent the pattern from blocking the creation.
+    await projectStore.update({
+        id: DEFAULT_PROJECT,
+        name: 'default',
+        description: '',
+        mode: 'open',
+        featureNaming: { pattern: 'testpattern.+' },
+    });
+
     const { body } = await validateImport(importPayloadWithContextFields, 200);
 
     expect(body).toMatchObject({
@@ -883,6 +895,11 @@ test('validate import data', async () => {
                     'We detected the following features are duplicate in your import data:',
                 affectedItems: [defaultFeatureName],
             },
+
+            {
+                message: expect.stringMatching(/\btestpattern.+\b/),
+                affectedItems: [defaultFeatureName],
+            },
         ],
         warnings: [
             {
@@ -941,3 +958,44 @@ test('should not import archived features tags', async () => {
         tags: resultTags,
     });
 });
+
+test(`should give errors with flag names if the flags don't match the project pattern`, async () => {
+    await db.stores.environmentStore.create({
+        name: DEFAULT_ENV,
+        type: 'production',
+    });
+
+    const pattern = 'testpattern.+';
+    for (const project of [DEFAULT_PROJECT]) {
+        await db.stores.projectStore.create({
+            name: project,
+            description: '',
+            id: project,
+            mode: 'open' as const,
+            featureNaming: { pattern },
+        });
+        await app.linkProjectToEnvironment(project, DEFAULT_ENV);
+    }
+
+    const flagName = 'unusedfeaturenamethatdoesntmatchpattern';
+
+    const { body } = await app.importToggles(
+        {
+            ...defaultImportPayload,
+            data: {
+                ...defaultImportPayload.data,
+                features: [
+                    {
+                        project: 'old_project',
+                        name: flagName,
+                        type: 'release',
+                    },
+                ],
+            },
+        },
+        400,
+    );
+
+    expect(body.message).toContain(pattern);
+    expect(body.message).toContain(flagName);
+});

src/lib/features/export-import-toggles/import-validation-messages.ts

@@ -1,3 +1,4 @@
+import { FeatureNameCheckResult } from 'lib/services/feature-toggle-service';
 import {
     FeatureStrategySchema,
     ImportTogglesValidateItemSchema,
@@ -10,6 +11,7 @@ export interface IErrorsParams {
     contextFields: IContextFieldDto[];
     otherProjectFeatures: string[];
     duplicateFeatures: string[];
+    featureNameCheckResult: FeatureNameCheckResult;
 }
 
 export interface IWarningParams {
@@ -40,6 +42,7 @@ export class ImportValidationMessages {
         contextFields,
         otherProjectFeatures,
         duplicateFeatures,
+        featureNameCheckResult,
     }: IErrorsParams): ImportTogglesValidateItemSchema[] {
         const errors: ImportTogglesValidateItemSchema[] = [];
 
@@ -72,6 +75,23 @@ export class ImportValidationMessages {
                 affectedItems: duplicateFeatures,
             });
         }
+        if (featureNameCheckResult.state === 'invalid') {
+            const baseError = `Features imported into this project must match the project's feature naming pattern: "${featureNameCheckResult.featureNaming.pattern}".`;
+
+            const exampleInfo = featureNameCheckResult.featureNaming.example
+                ? ` For example: "${featureNameCheckResult.featureNaming.example}".`
+                : '';
+
+            const descriptionInfo = featureNameCheckResult.featureNaming
+                .description
+                ? ` The pattern is described as follows: "${featureNameCheckResult.featureNaming.description}"`
+                : '';
+
+            errors.push({
+                message: `${baseError}${exampleInfo}${descriptionInfo} The following features do not match the pattern:`,
+                affectedItems: [...featureNameCheckResult.invalidNames].sort(),
+            });
+        }
 
         return errors;
     }

src/lib/routes/admin-api/feature.ts

@@ -310,7 +310,10 @@ class FeatureController extends Controller {
         const { name, projectId } = req.body;
 
         await this.service.validateName(name);
-        await this.service.validateFeatureFlagPattern(name, projectId);
+        await this.service.validateFeatureFlagNameAgainstPattern(
+            name,
+            projectId ?? undefined,
+        );
         res.status(200).end();
     }
 

src/lib/services/feature-toggle-service.ts

@@ -43,6 +43,7 @@ import {
     StrategiesOrderChangedEvent,
     PotentiallyStaleOnEvent,
     IStrategyStore,
+    IFeatureNaming,
 } from '../types';
 import { Logger } from '../logger';
 import { PatternError } from '../error';
@@ -111,6 +112,14 @@ export interface IGetFeatureParams {
     userId?: number;
 }
 
+export type FeatureNameCheckResult =
+    | { state: 'valid' }
+    | {
+          state: 'invalid';
+          invalidNames: Set<string>;
+          featureNaming: IFeatureNaming;
+      };
+
 const oneOf = (values: string[], match: string) => {
     return values.some((value) => value === match);
 };
@@ -1035,7 +1044,7 @@ class FeatureToggleService {
     ): Promise<FeatureToggle> {
         this.logger.info(`${createdBy} creates feature toggle ${value.name}`);
         await this.validateName(value.name);
-        await this.validateFeatureFlagPattern(value.name, projectId);
+        await this.validateFeatureFlagNameAgainstPattern(value.name, projectId);
 
         const exists = await this.projectStore.hasProject(projectId);
 
@@ -1091,20 +1100,49 @@ class FeatureToggleService {
         throw new NotFoundError(`Project with id ${projectId} does not exist`);
     }
 
-    async validateFeatureFlagPattern(
+    async checkFeatureFlagNamesAgainstProjectPattern(
+        projectId: string,
+        featureNames: string[],
+    ): Promise<FeatureNameCheckResult> {
+        if (this.flagResolver.isEnabled('featureNamingPattern')) {
+            const project = await this.projectStore.get(projectId);
+            const patternData = project.featureNaming;
+            const namingPattern = patternData?.pattern;
+
+            if (namingPattern) {
+                const regex = new RegExp(namingPattern);
+                const mismatchedNames = featureNames.filter(
+                    (name) => !regex.test(name),
+                );
+
+                if (mismatchedNames.length > 0) {
+                    return {
+                        state: 'invalid',
+                        invalidNames: new Set(mismatchedNames),
+                        featureNaming: patternData,
+                    };
+                }
+            }
+        }
+        return { state: 'valid' };
+    }
+
+    async validateFeatureFlagNameAgainstPattern(
         featureName: string,
         projectId?: string,
     ): Promise<void> {
-        if (this.flagResolver.isEnabled('featureNamingPattern') && projectId) {
-            const project = await this.projectStore.get(projectId);
-            const namingPattern = project.featureNaming?.pattern;
-            const namingExample = project.featureNaming?.example;
-            const namingDescription = project.featureNaming?.description;
+        if (projectId) {
+            const result =
+                await this.checkFeatureFlagNamesAgainstProjectPattern(
+                    projectId,
+                    [featureName],
+                );
+
+            if (result.state === 'invalid') {
+                const namingPattern = result.featureNaming.pattern;
+                const namingExample = result.featureNaming.example;
+                const namingDescription = result.featureNaming.description;
 
-            if (
-                namingPattern &&
-                !featureName.match(new RegExp(namingPattern))
-            ) {
                 const error = `The feature flag name "${featureName}" does not match the project's naming pattern: "${namingPattern}".`;
                 const example = namingExample
                     ? ` Here's an example of a name that does match the pattern: "${namingExample}"."`

src/lib/types/model.ts

@@ -191,8 +191,8 @@ export type ProjectMode = 'open' | 'protected';
 
 export interface IFeatureNaming {
     pattern: string | null;
-    example: string | null;
-    description: string | null;
+    example?: string | null;
+    description?: string | null;
 }
 
 export interface IProjectOverview {

src/test/e2e/services/feature-toggle-service-v2.e2e.test.ts

@@ -33,7 +33,9 @@ const mockConstraints = (): IConstraint[] => {
 const irrelevantDate = new Date();
 
 beforeAll(async () => {
-    const config = createTestConfig();
+    const config = createTestConfig({
+        experimental: { flags: { featureNamingPattern: true } },
+    });
     db = await dbInit(
         'feature_toggle_service_v2_service_serial',
         config.getLogger,
@@ -638,3 +640,72 @@ test('getPlaygroundFeatures should return ids and titles (if they exist) on clie
         expect(strategy.id).not.toBeUndefined();
     }
 });
+
+describe('flag name validation', () => {
+    test('should validate names if project has flag name pattern', async () => {
+        const projectId = 'pattern-validation';
+        const featureNaming = {
+            pattern: 'testpattern.+',
+            example: 'testpattern-one!',
+            description: 'naming description',
+        };
+        const project = {
+            id: projectId,
+            name: projectId,
+            mode: 'open' as const,
+            defaultStickiness: 'default',
+            featureNaming,
+        };
+
+        await stores.projectStore.create(project);
+
+        const validFeatures = ['testpattern-feature', 'testpattern-feature2'];
+        const invalidFeatures = ['a', 'b', 'c'];
+        const result = await service.checkFeatureFlagNamesAgainstProjectPattern(
+            projectId,
+            [...validFeatures, ...invalidFeatures],
+        );
+
+        expect(result).toMatchObject({
+            state: 'invalid',
+            invalidNames: new Set(invalidFeatures),
+            featureNaming: featureNaming,
+        });
+
+        const validResult =
+            await service.checkFeatureFlagNamesAgainstProjectPattern(
+                projectId,
+                validFeatures,
+            );
+
+        expect(validResult).toMatchObject({ state: 'valid' });
+    });
+
+    test.each([null, undefined, ''])(
+        'should not validate names if the pattern is %s',
+        async (pattern) => {
+            const projectId = `empty-pattern-validation-${pattern}`;
+            const featureNaming = {
+                pattern,
+            };
+            const project = {
+                id: projectId,
+                name: projectId,
+                mode: 'open' as const,
+                defaultStickiness: 'default',
+                featureNaming,
+            };
+
+            await stores.projectStore.create(project);
+
+            const features = ['a', 'b'];
+            const result =
+                await service.checkFeatureFlagNamesAgainstProjectPattern(
+                    projectId,
+                    features,
+                );
+
+            expect(result).toMatchObject({ state: 'valid' });
+        },
+    );
+});