diff --git a/packages/core-app-api/src/apis/implementations/StorageApi/UserSettingsStorage.test.ts b/packages/core-app-api/src/apis/implementations/StorageApi/UserSettingsStorage.test.ts index cc48fc925c..d8ea87b38d 100644 --- a/packages/core-app-api/src/apis/implementations/StorageApi/UserSettingsStorage.test.ts +++ b/packages/core-app-api/src/apis/implementations/StorageApi/UserSettingsStorage.test.ts @@ -49,14 +49,6 @@ describe('Persistent Storage API', () => { }); }; - beforeEach(() => { - server.use( - rest.get(`${mockBaseUrl}/buckets/`, async (_req, res, ctx) => { - return res(ctx.json([])); - }), - ); - }); - afterEach(() => { jest.resetAllMocks(); }); @@ -90,7 +82,8 @@ describe('Persistent Storage API', () => { `${mockBaseUrl}/buckets/:bucket/keys/:key`, async (req, res, ctx) => { const body = await req.json(); - const data = { value: JSON.stringify(dummyValue) }; + const data = { value: dummyValue }; + expect(body).toEqual(data); return res(ctx.json(data)); @@ -113,7 +106,7 @@ describe('Persistent Storage API', () => { `${mockBaseUrl}/buckets/:bucket/keys/:key`, async (req, res, ctx) => { const body = await req.json(); - const data = { value: JSON.stringify(dummyValue) }; + const data = { value: dummyValue }; expect(body).toEqual(data); return res(ctx.json(data)); @@ -136,7 +129,7 @@ describe('Persistent Storage API', () => { `${mockBaseUrl}/buckets/:bucket/keys/:key`, async (req, res, ctx) => { const body = await req.json(); - const data = { value: JSON.stringify(mockData) }; + const data = { value: mockData }; expect(body).toEqual(data); return res(ctx.json(data)); @@ -315,7 +308,7 @@ describe('Persistent Storage API', () => { rest.get( `${mockBaseUrl}/buckets/:bucket/keys/:key`, async (_req, res, ctx) => { - return res(ctx.text(JSON.stringify({ value: JSON.stringify(data) }))); + return res(ctx.json({ value: data })); }, ), ); diff --git a/packages/core-app-api/src/apis/implementations/StorageApi/UserSettingsStorage.ts b/packages/core-app-api/src/apis/implementations/StorageApi/UserSettingsStorage.ts index 0763ad8187..d01d5021ee 100644 --- a/packages/core-app-api/src/apis/implementations/StorageApi/UserSettingsStorage.ts +++ b/packages/core-app-api/src/apis/implementations/StorageApi/UserSettingsStorage.ts @@ -106,7 +106,7 @@ export class UserSettingsStorage implements StorageApi { async set(key: string, data: T): Promise { const fetchUrl = await this.getFetchUrl(key); - const body = JSON.stringify({ value: JSON.stringify(data) }); + const body = JSON.stringify({ value: data }); const response = await this.fetchApi.fetch(fetchUrl, { method: 'PUT', @@ -121,7 +121,7 @@ export class UserSettingsStorage implements StorageApi { this.notifyChanges({ key, - value: JSON.parse(value), + value, presence: 'present', }); } @@ -165,7 +165,7 @@ export class UserSettingsStorage implements StorageApi { try { const { value: rawValue } = await response.json(); - const value = JSON.parse(rawValue, (_key, val) => { + const value = JSON.parse(JSON.stringify(rawValue), (_key, val) => { if (typeof val === 'object' && val !== null) { Object.freeze(val); } diff --git a/plugins/user-settings-backend/package.json b/plugins/user-settings-backend/package.json index 41178d49b8..690d1d13c9 100644 --- a/plugins/user-settings-backend/package.json +++ b/plugins/user-settings-backend/package.json @@ -32,6 +32,7 @@ "@backstage/catalog-model": "^1.1.0", "@backstage/errors": "^1.1.0", "@backstage/plugin-auth-node": "^0.2.5-next.1", + "@backstage/types": "^1.0.0", "@types/express": "^4.17.6", "express": "^4.17.1", "express-promise-router": "^4.1.0", diff --git a/plugins/user-settings-backend/src/database/DatabaseUserSettingsStore.test.ts b/plugins/user-settings-backend/src/database/DatabaseUserSettingsStore.test.ts index e98ee0010e..eb50e26179 100644 --- a/plugins/user-settings-backend/src/database/DatabaseUserSettingsStore.test.ts +++ b/plugins/user-settings-backend/src/database/DatabaseUserSettingsStore.test.ts @@ -84,13 +84,13 @@ describe.each(databases.eachSupportedId())( user_entity_ref: 'user-1', bucket: 'bucket-a', key: 'key-a', - value: 'value-a', + value: JSON.stringify('value-a'), }, { user_entity_ref: 'user-2', bucket: 'bucket-c', key: 'key-c', - value: 'value-c', + value: JSON.stringify('value-c'), }, ]); @@ -122,7 +122,7 @@ describe.each(databases.eachSupportedId())( user_entity_ref: 'user-1', bucket: 'bucket-a', key: 'key-a', - value: 'value-a', + value: JSON.stringify('value-a'), }, ]); }); @@ -147,7 +147,7 @@ describe.each(databases.eachSupportedId())( user_entity_ref: 'user-1', bucket: 'bucket-a', key: 'key-a', - value: 'value-b', + value: JSON.stringify('value-b'), }, ]); }); @@ -170,13 +170,13 @@ describe.each(databases.eachSupportedId())( user_entity_ref: 'user-1', bucket: 'bucket-a', key: 'key-a', - value: 'value-a', + value: JSON.stringify('value-a'), }, { user_entity_ref: 'user-2', bucket: 'bucket-c', key: 'key-c', - value: 'value-c', + value: JSON.stringify('value-c'), }, ]); @@ -190,7 +190,7 @@ describe.each(databases.eachSupportedId())( user_entity_ref: 'user-2', bucket: 'bucket-c', key: 'key-c', - value: 'value-c', + value: JSON.stringify('value-c'), }, ]); }); diff --git a/plugins/user-settings-backend/src/database/DatabaseUserSettingsStore.ts b/plugins/user-settings-backend/src/database/DatabaseUserSettingsStore.ts index 4b46b6383f..b0fe8fdecc 100644 --- a/plugins/user-settings-backend/src/database/DatabaseUserSettingsStore.ts +++ b/plugins/user-settings-backend/src/database/DatabaseUserSettingsStore.ts @@ -19,8 +19,9 @@ import { resolvePackagePath, } from '@backstage/backend-common'; import { NotFoundError } from '@backstage/errors'; +import { JsonValue } from '@backstage/types'; import { Knex } from 'knex'; -import { type UserSetting, UserSettingsStore } from './UserSettingsStore'; +import { UserSettingsStore, type UserSetting } from './UserSettingsStore'; const migrationsDir = resolvePackagePath( '@backstage/plugin-user-settings-backend', @@ -79,21 +80,25 @@ export class DatabaseUserSettingsStore implements UserSettingsStore { ); } - return rows[0]; + return { + bucket: rows[0].bucket, + key: rows[0].key, + value: JSON.parse(rows[0].value), + }; } async set(options: { userEntityRef: string; bucket: string; key: string; - value: string; + value: JsonValue; }): Promise { await this.db('user_settings') .insert({ user_entity_ref: options.userEntityRef, bucket: options.bucket, key: options.key, - value: options.value, + value: JSON.stringify(options.value), }) .onConflict(['user_entity_ref', 'bucket', 'key']) .merge(['value']); diff --git a/plugins/user-settings-backend/src/database/UserSettingsStore.ts b/plugins/user-settings-backend/src/database/UserSettingsStore.ts index d14af091ef..289c37523e 100644 --- a/plugins/user-settings-backend/src/database/UserSettingsStore.ts +++ b/plugins/user-settings-backend/src/database/UserSettingsStore.ts @@ -14,13 +14,15 @@ * limitations under the License. */ +import { JsonValue } from '@backstage/types'; + /** * A single setting in a bucket */ export type UserSetting = { bucket: string; key: string; - value: string; + value: JsonValue; }; /** @@ -37,7 +39,7 @@ export interface UserSettingsStore { userEntityRef: string; bucket: string; key: string; - value: string; + value: JsonValue; }): Promise; delete(options: { diff --git a/plugins/user-settings-backend/src/service/router.test.ts b/plugins/user-settings-backend/src/service/router.test.ts index 7640714477..c8df77a845 100644 --- a/plugins/user-settings-backend/src/service/router.test.ts +++ b/plugins/user-settings-backend/src/service/router.test.ts @@ -148,7 +148,7 @@ describe('createRouter', () => { }); }); - it('returns an error if the value is not a string', async () => { + it('returns an error if the value is not given', async () => { authenticateMock.mockResolvedValue({ identity: { userEntityRef: 'user-1' }, }); @@ -156,7 +156,7 @@ describe('createRouter', () => { const responses = await request(app) .put('/buckets/my-bucket/keys/my-key') .set('Authorization', 'Bearer foo') - .send({ value: { invalid: 'because not a string' } }); + .send({}); expect(responses.status).toEqual(400); diff --git a/plugins/user-settings-backend/src/service/router.ts b/plugins/user-settings-backend/src/service/router.ts index d677ee4bfb..487a6b4fee 100644 --- a/plugins/user-settings-backend/src/service/router.ts +++ b/plugins/user-settings-backend/src/service/router.ts @@ -96,8 +96,8 @@ export async function createRouterInternal(options: { const { bucket, key } = req.params; const { value } = req.body; - if (typeof value !== 'string') { - throw new InputError('Value must be a string'); + if (value === undefined) { + throw new InputError('Missing required field "value"'); } await options.userSettingsStore.set({ diff --git a/yarn.lock b/yarn.lock index 9df599d106..4d821c0e64 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7321,6 +7321,7 @@ __metadata: "@backstage/cli": ^0.19.0-next.1 "@backstage/errors": ^1.1.0 "@backstage/plugin-auth-node": ^0.2.5-next.1 + "@backstage/types": ^1.0.0 "@types/express": ^4.17.6 "@types/supertest": ^2.0.8 express: ^4.17.1