Skip to content

Commit

Permalink
fix: handle objects in top-level context in playground (#6773)
Browse files Browse the repository at this point in the history
Don't include invalid context properties in the contexts that we
evaluate.

This PR removes any non-`properties` fields that have a non-string
value.

This prevents the front end from crashing when trying to render an
object.

Expect follow-up PRs to include more warnings/diagnostics we can show to
the end user to inform them of what fields have been removed and why.
  • Loading branch information
thomasheartman committed Apr 5, 2024
1 parent 770155d commit ac6c05d
Show file tree
Hide file tree
Showing 4 changed files with 132 additions and 1 deletion.
35 changes: 35 additions & 0 deletions src/lib/features/playground/clean-context.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import { cleanContext } from './clean-context';

test('strips invalid context properties from the context', async () => {
const invalidJsonTypes = {
object: {},
array: [],
true: true,
false: false,
number: 123,
null: null,
};

const validValues = {
appName: 'test',
};

const inputContext = {
...invalidJsonTypes,
...validValues,
};

const cleanedContext = cleanContext(inputContext);

expect(cleanedContext).toStrictEqual(validValues);
});

test("doesn't add non-existing properties", async () => {
const input = {
appName: 'test',
};

const output = cleanContext(input);

expect(output).toStrictEqual(input);
});
16 changes: 16 additions & 0 deletions src/lib/features/playground/clean-context.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import type { SdkContextSchema } from '../../openapi';

export const cleanContext = (context: SdkContextSchema): SdkContextSchema => {
const { appName, ...otherContextFields } = context;

const cleanedContextFields = Object.fromEntries(
Object.entries(otherContextFields).filter(
([key, value]) => key === 'properties' || typeof value === 'string',
),
);

return {
...cleanedContextFields,
appName,
};
};
78 changes: 78 additions & 0 deletions src/lib/features/playground/playground-api.e2e.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
import dbInit, { type ITestDb } from '../../../test/e2e/helpers/database-init';
import {
type IUnleashTest,
setupApp,
} from '../../../test/e2e/helpers/test-helper';
import type { IUnleashStores } from '../../types';
import getLogger from '../../../test/fixtures/no-logger';

let stores: IUnleashStores;
let db: ITestDb;
let app: IUnleashTest;

const flag = {
name: 'test-flag',
enabled: true,
strategies: [{ name: 'default' }],
createdByUserId: 9999,
};

beforeAll(async () => {
db = await dbInit('playground_api', getLogger);
stores = db.stores;

await stores.featureToggleStore.create('default', flag);

app = await setupApp(stores);
});

afterAll(async () => {
await db.destroy();
});

test('strips invalid context properties from input before using it', async () => {
const validData = {
appName: 'test',
};

const inputContext = {
invalid: {},
...validData,
};

const { body } = await app.request
.post('/api/admin/playground/advanced')
.send({
context: inputContext,
environments: ['production'],
projects: '*',
})
.expect(200);

const evaluatedContext =
body.features[0].environments.production[0].context;

expect(evaluatedContext).toStrictEqual(validData);
});

test('returns the input context exactly as it came in, even if invalid values have been removed for the evaluation', async () => {
const invalidData = {
invalid: {},
};

const inputContext = {
...invalidData,
appName: 'test',
};

const { body } = await app.request
.post('/api/admin/playground/advanced')
.send({
context: inputContext,
environments: ['production'],
projects: '*',
})
.expect(200);

expect(body.input.context).toMatchObject(inputContext);
});
4 changes: 3 additions & 1 deletion src/lib/features/playground/playground-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import type { AdvancedPlaygroundEnvironmentFeatureSchema } from '../../openapi/s
import { validateQueryComplexity } from './validateQueryComplexity';
import type { IPrivateProjectChecker } from '../private-project/privateProjectCheckerType';
import { getDefaultVariant } from './feature-evaluator/variant';
import { cleanContext } from './clean-context';

type EvaluationInput = {
features: FeatureConfigurationClient[];
Expand Down Expand Up @@ -124,7 +125,8 @@ export class PlaygroundService {
this.resolveFeatures(filteredProjects, env),
),
);
const contexts = generateObjectCombinations(context);

const contexts = generateObjectCombinations(cleanContext(context));

validateQueryComplexity(
environments.length,
Expand Down

0 comments on commit ac6c05d

Please sign in to comment.