Mirrors/unleash.unleash
1
0
Fork 0
mirror of https://github.com/Unleash/unleash.git synced 2024-12-22 19:07:54 +01:00
Code Issues Projects Releases Wiki Activity

Make feature-toggle-store return 409

Browse Source 
- After seeing frontend behaviour where the user could add the same
tag multiple times, and not get errors or be stopped doing so, we'll
change the backend to return a 409 if you tag a feature with a tag it
already has.

- Previous to this commit, the setup was to do `onConflict().ignore()`
  which caused the frontend to not get any help from the backend as to
  whether or not the operation was allowed

- This fix adds a custom error and adds a branch to the handleError util
  method for handling just that error type with a 409.

- This caused a couple of tests to receive 409, probably due to
  insufficient cleanup between tests. Adding faker as a dev-dependency and randomising
  toggle names and tag values for each test reduces the chance that
  we'll run into duplicate issues in the future for the tests that
  touches this problem

fixes: #711
This commit is contained in:
Christopher Kolstad 2021-02-08 16:22:15 +01:00
parent 0b510e6aa2
commit 063d3f0e4a
No known key found for this signature in database
GPG Key ID: 559ACB0E3DB5538A
6 changed files with 118 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
lib/db/feature-toggle-store.js
View File

@ -3,6 +3,7 @@
const metricsHelper = require('../metrics-helper');
const { DB_TIME } = require('../events');
const NotFoundError = require('../error/notfound-error');
const FeatureHasTagError = require('../error/feature-has-tag-error');
const FEATURE_COLUMNS = [
'name',
@ -240,8 +241,15 @@ class FeatureToggleStore {
const stopTimer = this.timer('tagFeature');
await this.db(FEATURE_TAG_TABLE)
.insert(this.featureAndTagToRow(featureName, tag))
.onConflict(['feature_name', 'tag_type', 'tag_value'])
.ignore();
.catch(err => {
if (err.code === '23505') {
throw new FeatureHasTagError(
`${featureName} already had the tag: [${tag.type}:${tag.value}]`,
);
} else {
throw err;
}
});
stopTimer();
return tag;
}

26
lib/error/feature-has-tag-error.js Normal file
View File

@ -0,0 +1,26 @@
'use strict';
class FeatureHasTagError extends Error {
constructor(message) {
super();
Error.captureStackTrace(this, this.constructor);
this.name = this.constructor.name;
this.message = message;
}
toJSON() {
const obj = {
isJoi: false,
name: this.constructor.name,
details: [
{
message: this.message,
},
],
};
return obj;
}
}
module.exports = FeatureHasTagError;

5
lib/routes/admin-api/util.js
View File

@ -42,6 +42,11 @@ const handleErrors = (res, logger, error) => {
.status(400)
.json(error)
.end();
case 'FeatureHasTagError':
return res
.status(409)
.json(error)
.end();
default:
logger.error('Server failed executing request', error);
return res.status(500).end();

1
package.json
View File

@ -101,6 +101,7 @@
"eslint-config-prettier": "^6.10.1",
"eslint-plugin-import": "^2.20.2",
"eslint-plugin-prettier": "^3.1.3",
"faker": "^5.3.1",
"fetch-mock": "^9.11.0",
"husky": "^4.2.3",
"lint-staged": "^10.0.7",

99
test/e2e/api/admin/feature.e2e.test.js
View File

@ -1,6 +1,7 @@
'use strict';
const test = require('ava');
const faker = require('faker');
const dbInit = require('../../helpers/database-init');
const { setupApp } = require('../../helpers/test-helper');
const getLogger = require('../../../fixtures/no-logger');
@ -338,24 +339,25 @@ test.serial(
test.serial('can untag feature', async t => {
t.plan(1);
const request = await setupApp(stores);
const feature1Name = faker.helpers.slugify(faker.lorem.words(3));
await request.post('/api/admin/features').send({
name: 'test.feature',
name: feature1Name,
type: 'killswitch',
enabled: true,
strategies: [{ name: 'default' }],
});
const tag = { value: 'TeamGreen', type: 'simple' };
const tag = { value: faker.lorem.word(), type: 'simple' };
await request
.post('/api/admin/features/test.feature/tags')
.post(`/api/admin/features/${feature1Name}/tags`)
.send(tag)
.expect(201);
await request
.delete(
`/api/admin/features/test.feature/tags/${tag.type}/${tag.value}`,
`/api/admin/features/${feature1Name}/tags/${tag.type}/${tag.value}`,
)
.expect(200);
return request
.get('/api/admin/features/test.feature/tags')
.get(`/api/admin/features/${feature1Name}/tags`)
.expect('Content-Type', /json/)
.expect(200)
.expect(res => {
@ -366,76 +368,85 @@ test.serial('can untag feature', async t => {
test.serial('Can get features tagged by tag', async t => {
t.plan(2);
const request = await setupApp(stores);
const feature1Name = faker.helpers.slugify(faker.lorem.words(3));
const feature2Name = faker.helpers.slugify(faker.lorem.words(3));
await request.post('/api/admin/features').send({
name: 'test.feature',
name: feature1Name,
type: 'killswitch',
enabled: true,
strategies: [{ name: 'default' }],
});
await request.post('/api/admin/features').send({
name: 'test.feature2',
name: feature2Name,
type: 'killswitch',
enabled: true,
strategies: [{ name: 'default' }],
});
const tag = { value: 'Crazy', type: 'simple' };
const tag = { value: faker.lorem.word(), type: 'simple' };
await request
.post('/api/admin/features/test.feature/tags')
.post(`/api/admin/features/${feature1Name}/tags`)
.send(tag)
.expect(201);
return request
.get('/api/admin/features?tag=simple:Crazy')
.get(`/api/admin/features?tag=${tag.type}:${tag.value}`)
.expect('Content-Type', /json/)
.expect(200)
.expect(res => {
t.is(res.body.features.length, 1);
t.is(res.body.features[0].name, 'test.feature');
t.is(res.body.features[0].name, feature1Name);
});
});
test.serial('Can query for multiple tags using OR', async t => {
t.plan(2);
const request = await setupApp(stores);
const feature1Name = faker.helpers.slugify(faker.lorem.words(3));
const feature2Name = faker.helpers.slugify(faker.lorem.words(3));
await request.post('/api/admin/features').send({
name: 'test.feature',
name: feature1Name,
type: 'killswitch',
enabled: true,
strategies: [{ name: 'default' }],
});
await request.post('/api/admin/features').send({
name: 'test.feature2',
name: feature2Name,
type: 'killswitch',
enabled: true,
strategies: [{ name: 'default' }],
});
const tag = { value: 'Crazy', type: 'simple' };
const tag2 = { value: 'tagb', type: 'simple' };
const tag = { value: faker.name.firstName(), type: 'simple' };
const tag2 = { value: faker.name.firstName(), type: 'simple' };
await request
.post('/api/admin/features/test.feature/tags')
.post(`/api/admin/features/${feature1Name}/tags`)
.send(tag)
.expect(201);
await request
.post('/api/admin/features/test.feature2/tags')
.post(`/api/admin/features/${feature2Name}/tags`)
.send(tag2)
.expect(201);
return request
.get('/api/admin/features?tag[]=simple:Crazy&tag[]=simple:tagb')
.get(
`/api/admin/features?tag[]=${tag.type}:${tag.value}&tag[]=${tag2.type}:${tag2.value}`,
)
.expect('Content-Type', /json/)
.expect(200)
.expect(res => {
t.is(res.body.features.length, 2);
t.is(res.body.features[0].name, 'test.feature');
t.is(res.body.features[0].name, feature1Name);
});
});
test.serial('Querying with multiple filters ANDs the filters', async t => {
const request = await setupApp(stores);
const feature1Name = `test.${faker.helpers.slugify(faker.hacker.phrase())}`;
const feature2Name = faker.helpers.slugify(faker.lorem.words());
await request.post('/api/admin/features').send({
name: 'test.feature',
name: feature1Name,
type: 'killswitch',
enabled: true,
strategies: [{ name: 'default' }],
});
await request.post('/api/admin/features').send({
name: 'test.feature2',
name: feature2Name,
type: 'killswitch',
enabled: true,
strategies: [{ name: 'default' }],
@ -446,14 +457,14 @@ test.serial('Querying with multiple filters ANDs the filters', async t => {
enabled: true,
strategies: [{ name: 'default' }],
});
const tag = { value: 'Crazy', type: 'simple' };
const tag2 = { value: 'tagb', type: 'simple' };
const tag = { value: faker.lorem.word(), type: 'simple' };
const tag2 = { value: faker.name.firstName(), type: 'simple' };
await request
.post('/api/admin/features/test.feature/tags')
.post(`/api/admin/features/${feature1Name}/tags`)
.send(tag)
.expect(201);
await request
.post('/api/admin/features/test.feature2/tags')
.post(`/api/admin/features/${feature2Name}/tags`)
.send(tag2)
.expect(201);
await request
@ -461,16 +472,48 @@ test.serial('Querying with multiple filters ANDs the filters', async t => {
.send(tag)
.expect(201);
await request
.get('/api/admin/features?tag=simple:Crazy')
.get(`/api/admin/features?tag=${tag.type}:${tag.value}`)
.expect('Content-Type', /json/)
.expect(200)
.expect(res => t.is(res.body.features.length, 2));
await request
.get('/api/admin/features?namePrefix=test&tag=simple:Crazy')
.get(`/api/admin/features?namePrefix=test&tag=${tag.type}:${tag.value}`)
.expect('Content-Type', /json/)
.expect(200)
.expect(res => {
t.is(res.body.features.length, 1);
t.is(res.body.features[0].name, 'test.feature');
t.is(res.body.features[0].name, feature1Name);
});
});
test.serial(
'Tagging a feature with a tag it already has should return 409',
async t => {
const request = await setupApp(stores);
const feature1Name = `test.${faker.helpers.slugify(
faker.lorem.words(3),
)}`;
await request.post('/api/admin/features').send({
name: feature1Name,
type: 'killswitch',
enabled: true,
strategies: [{ name: 'default' }],
});
const tag = { value: faker.lorem.word(), type: 'simple' };
await request
.post(`/api/admin/features/${feature1Name}/tags`)
.send(tag)
.expect(201);
return request
.post(`/api/admin/features/${feature1Name}/tags`)
.send(tag)
.expect(409)
.expect(res => {
t.is(
res.body.details[0].message,
`${feature1Name} already had the tag: [${tag.type}:${tag.value}]`,
);
});
},
);

5
yarn.lock
View File

@ -2284,6 +2284,11 @@ eyes@0.1.x:
resolved "https://registry.npmjs.org/eyes/-/eyes-0.1.8.tgz"
integrity sha1-Ys8SAjTGg3hdkCNIqADvPgzCC8A=
faker@^5.3.1:
version "5.3.1"
resolved "https://registry.yarnpkg.com/faker/-/faker-5.3.1.tgz#67f8f5c170b97a76b875389e0e8b9155da7b4853"
integrity sha512-sVdoApX/awJHO9DZHZsHVaJBNFiJW0n3lPs0q/nFxp/Mtya1dr2sCMktST3mdxNMHjkvKTTMAW488E+jH1eSbg==
fast-deep-equal@^3.1.1:
version "3.1.3"
resolved "https://registry.npmjs.org/fast-deep-equal/-/fast-deep-equal-3.1.3.tgz"