mirror of
https://github.com/Unleash/unleash.git
synced 2025-02-23 00:22:19 +01:00
fix: return 404 when gettings tags for a non existing feature (#3560)
From the discussion here https://github.com/Unleash/unleash/pull/3496#discussion_r1163745860 This PR checks the existence of the feature before trying to get tags for the feature. Doing so by stealing the exists method from the feature store, since that's what we need to know exists, and avoiding having the feature store as a dependency to the featureTagStore seemed reasonable.
This commit is contained in:
parent
0e80484068
commit
4f7fd46623
@ -9,6 +9,7 @@ import {
|
|||||||
IFeatureTagStore,
|
IFeatureTagStore,
|
||||||
} from '../types/stores/feature-tag-store';
|
} from '../types/stores/feature-tag-store';
|
||||||
import { Db } from './db';
|
import { Db } from './db';
|
||||||
|
import NotFoundError from '../error/notfound-error';
|
||||||
|
|
||||||
const COLUMNS = ['feature_name', 'tag_type', 'tag_value'];
|
const COLUMNS = ['feature_name', 'tag_type', 'tag_value'];
|
||||||
const TABLE = 'feature_tag';
|
const TABLE = 'feature_tag';
|
||||||
@ -95,12 +96,27 @@ class FeatureTagStore implements IFeatureTagStore {
|
|||||||
|
|
||||||
async getAllTagsForFeature(featureName: string): Promise<ITag[]> {
|
async getAllTagsForFeature(featureName: string): Promise<ITag[]> {
|
||||||
const stopTimer = this.timer('getAllForFeature');
|
const stopTimer = this.timer('getAllForFeature');
|
||||||
|
if (await this.featureExists(featureName)) {
|
||||||
const rows = await this.db
|
const rows = await this.db
|
||||||
.select(COLUMNS)
|
.select(COLUMNS)
|
||||||
.from<FeatureTagTable>(TABLE)
|
.from<FeatureTagTable>(TABLE)
|
||||||
.where({ feature_name: featureName });
|
.where({ feature_name: featureName });
|
||||||
stopTimer();
|
stopTimer();
|
||||||
return rows.map(this.featureTagRowToTag);
|
return rows.map(this.featureTagRowToTag);
|
||||||
|
} else {
|
||||||
|
throw new NotFoundError(
|
||||||
|
`Could not find feature with name ${featureName}`,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
async featureExists(featureName: string): Promise<boolean> {
|
||||||
|
const result = await this.db.raw(
|
||||||
|
'SELECT EXISTS (SELECT 1 FROM features WHERE name = ?) AS present',
|
||||||
|
[featureName],
|
||||||
|
);
|
||||||
|
const { present } = result.rows[0];
|
||||||
|
return present;
|
||||||
}
|
}
|
||||||
|
|
||||||
async getAllByFeatures(features: string[]): Promise<IFeatureTag[]> {
|
async getAllByFeatures(features: string[]): Promise<IFeatureTag[]> {
|
||||||
@ -187,14 +203,11 @@ class FeatureTagStore implements IFeatureTagStore {
|
|||||||
}
|
}
|
||||||
|
|
||||||
featureTagRowToTag(row: FeatureTagTable): ITag {
|
featureTagRowToTag(row: FeatureTagTable): ITag {
|
||||||
if (row) {
|
|
||||||
return {
|
return {
|
||||||
value: row.tag_value,
|
value: row.tag_value,
|
||||||
type: row.tag_type,
|
type: row.tag_type,
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
return null;
|
|
||||||
}
|
|
||||||
|
|
||||||
rowToFeatureAndTag(row: FeatureTagTable): IFeatureAndTag {
|
rowToFeatureAndTag(row: FeatureTagTable): IFeatureAndTag {
|
||||||
return {
|
return {
|
||||||
|
@ -120,7 +120,7 @@ class FeatureController extends Controller {
|
|||||||
operationId: 'listTags',
|
operationId: 'listTags',
|
||||||
responses: {
|
responses: {
|
||||||
200: createResponseSchema('tagsSchema'),
|
200: createResponseSchema('tagsSchema'),
|
||||||
...getStandardResponses(401),
|
...getStandardResponses(401, 403, 404),
|
||||||
},
|
},
|
||||||
}),
|
}),
|
||||||
],
|
],
|
||||||
|
@ -6182,6 +6182,12 @@ If the provided project does not exist, the list of events will be empty.",
|
|||||||
"401": {
|
"401": {
|
||||||
"description": "Authorization information is missing or invalid. Provide a valid API token as the \`authorization\` header, e.g. \`authorization:*.*.my-admin-token\`.",
|
"description": "Authorization information is missing or invalid. Provide a valid API token as the \`authorization\` header, e.g. \`authorization:*.*.my-admin-token\`.",
|
||||||
},
|
},
|
||||||
|
"403": {
|
||||||
|
"description": "User credentials are valid but does not have enough privileges to execute this operation",
|
||||||
|
},
|
||||||
|
"404": {
|
||||||
|
"description": "The requested resource was not found.",
|
||||||
|
},
|
||||||
},
|
},
|
||||||
"summary": "Get all tags for a feature.",
|
"summary": "Get all tags for a feature.",
|
||||||
"tags": [
|
"tags": [
|
||||||
|
@ -2,6 +2,7 @@ import { IFeatureTagStore } from 'lib/types/stores/feature-tag-store';
|
|||||||
import { IFeatureToggleStore } from 'lib/types/stores/feature-toggle-store';
|
import { IFeatureToggleStore } from 'lib/types/stores/feature-toggle-store';
|
||||||
import dbInit from '../helpers/database-init';
|
import dbInit from '../helpers/database-init';
|
||||||
import getLogger from '../../fixtures/no-logger';
|
import getLogger from '../../fixtures/no-logger';
|
||||||
|
import NotFoundError from '../../../lib/error/notfound-error';
|
||||||
|
|
||||||
let stores;
|
let stores;
|
||||||
let db;
|
let db;
|
||||||
@ -43,7 +44,7 @@ test('should tag feature', async () => {
|
|||||||
expect(featureTag.tagValue).toBe(tag.value);
|
expect(featureTag.tagValue).toBe(tag.value);
|
||||||
});
|
});
|
||||||
|
|
||||||
test('feature tag exits', async () => {
|
test('feature tag exists', async () => {
|
||||||
await featureTagStore.tagFeature(featureName, tag);
|
await featureTagStore.tagFeature(featureName, tag);
|
||||||
const exists = await featureTagStore.exists({
|
const exists = await featureTagStore.exists({
|
||||||
featureName,
|
featureName,
|
||||||
@ -97,3 +98,20 @@ test('should import feature tags', async () => {
|
|||||||
const all = await featureTagStore.getAll();
|
const all = await featureTagStore.getAll();
|
||||||
expect(all).toHaveLength(2);
|
expect(all).toHaveLength(2);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test('should throw not found error if feature does not exist', async () => {
|
||||||
|
await expect(async () =>
|
||||||
|
featureTagStore.getAllTagsForFeature('non.existing.toggle'),
|
||||||
|
).rejects.toThrow(
|
||||||
|
new NotFoundError(
|
||||||
|
`Could not find feature with name non.existing.toggle`,
|
||||||
|
),
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('Returns empty tag list for existing feature with no tags', async () => {
|
||||||
|
const name = 'feature.with.no.tags';
|
||||||
|
await featureToggleStore.create('default', { name });
|
||||||
|
let tags = await featureTagStore.getAllTagsForFeature(name);
|
||||||
|
expect(tags).toHaveLength(0);
|
||||||
|
});
|
||||||
|
Loading…
Reference in New Issue
Block a user