diff --git a/workspaces/marketplace/.changeset/popular-kiwis-wink.md b/workspaces/marketplace/.changeset/popular-kiwis-wink.md new file mode 100644 index 0000000000..b742a8809d --- /dev/null +++ b/workspaces/marketplace/.changeset/popular-kiwis-wink.md @@ -0,0 +1,5 @@ +--- +'@red-hat-developer-hub/backstage-plugin-marketplace-backend': minor +--- + +**BREAKING** Replace POST with PATCH `/package/:namespace/:name/configuration/disable` endpoint to update packages disabled status diff --git a/workspaces/marketplace/plugins/marketplace-backend/__fixtures__/mockData.ts b/workspaces/marketplace/plugins/marketplace-backend/__fixtures__/mockData.ts index 16ccd59441..616018d2d5 100644 --- a/workspaces/marketplace/plugins/marketplace-backend/__fixtures__/mockData.ts +++ b/workspaces/marketplace/plugins/marketplace-backend/__fixtures__/mockData.ts @@ -161,7 +161,7 @@ export const mockFileInstallationStorage = { }), updatePackage: jest.fn(), updatePackages: jest.fn(), - addPackageDisabled: jest.fn(), + setPackageDisabled: jest.fn(), setPackagesDisabled: jest.fn(), } as unknown as jest.Mocked; @@ -171,7 +171,7 @@ export const mockInstallationDataService = { getInitializationError: jest.fn().mockReturnValue(undefined), updatePackageConfig: jest.fn(), updatePluginConfig: jest.fn(), - addPackageDisabled: jest.fn(), + setPackageDisabled: jest.fn(), setPluginDisabled: jest.fn(), } as unknown as jest.Mocked; diff --git a/workspaces/marketplace/plugins/marketplace-backend/package.json b/workspaces/marketplace/plugins/marketplace-backend/package.json index 861a212421..48371511a8 100644 --- a/workspaces/marketplace/plugins/marketplace-backend/package.json +++ b/workspaces/marketplace/plugins/marketplace-backend/package.json @@ -45,6 +45,7 @@ "@red-hat-developer-hub/backstage-plugin-marketplace-common": "workspace:^", "express": "^4.17.1", "express-promise-router": "^4.1.0", + "yaml": "^2.7.1", "zod": "^3.22.4" }, "devDependencies": { @@ -56,8 +57,7 @@ "@types/express": "*", "@types/supertest": "^2.0.12", "msw": "^1.0.0", - "supertest": "^6.2.4", - "yaml": "^2.7.1" + "supertest": "^6.2.4" }, "files": [ "dist" diff --git a/workspaces/marketplace/plugins/marketplace-backend/src/installation/FileInstallationStorage.test.ts b/workspaces/marketplace/plugins/marketplace-backend/src/installation/FileInstallationStorage.test.ts index d5609c27a6..18acedc075 100644 --- a/workspaces/marketplace/plugins/marketplace-backend/src/installation/FileInstallationStorage.test.ts +++ b/workspaces/marketplace/plugins/marketplace-backend/src/installation/FileInstallationStorage.test.ts @@ -24,7 +24,6 @@ import { mockPackages, } from '../../__fixtures__/mockData'; import { FileInstallationStorage } from './FileInstallationStorage'; -import { ConflictError } from '@backstage/errors'; describe('FileInstallationStorage', () => { const newPackageName = './dynamic-plugins/dist/package3-backend-dynamic'; @@ -353,7 +352,7 @@ describe('FileInstallationStorage', () => { }); }); - describe('addPackageDisabled', () => { + describe('setPackageDisabled', () => { afterEach(() => { fs.writeFileSync( resolve(__dirname, '../../__fixtures__/data/validPluginsConfig.yaml'), @@ -367,7 +366,7 @@ describe('FileInstallationStorage', () => { ); }); - it('should add package with disabled', () => { + it('should add new package with disabled status', () => { const configFileName = resolve( __dirname, '../../__fixtures__/data/validPluginsConfig.yaml', @@ -377,7 +376,7 @@ describe('FileInstallationStorage', () => { ); fileInstallationStorage.initialize(); - fileInstallationStorage.addPackageDisabled(newPackageName, false); + fileInstallationStorage.setPackageDisabled(newPackageName, false); const updatedCatalogInfoYaml = fs.readFileSync(configFileName, 'utf8'); const configYaml = parse(updatedCatalogInfoYaml); @@ -389,7 +388,7 @@ describe('FileInstallationStorage', () => { ]); }); - it('should raise ConflictError for package already in the config', () => { + it('should update existing package with disabled status', () => { const configFileName = resolve( __dirname, '../../__fixtures__/data/validPluginsConfig.yaml', @@ -399,16 +398,18 @@ describe('FileInstallationStorage', () => { ); fileInstallationStorage.initialize(); - expect(() => { - fileInstallationStorage.addPackageDisabled( - mockDynamicPackage11.package, - false, - ); - }).toThrow( - new ConflictError( - `Package '${mockDynamicPackage11.package}' already exists in the configuration`, - ), + fileInstallationStorage.setPackageDisabled( + mockDynamicPackage12.package, + false, ); + + const updatedCatalogInfoYaml = fs.readFileSync(configFileName, 'utf8'); + const configYaml = parse(updatedCatalogInfoYaml); + expect(configYaml.plugins).toEqual([ + mockDynamicPackage11, + { ...mockDynamicPackage12, disabled: false }, + mockDynamicPackage21, + ]); }); }); diff --git a/workspaces/marketplace/plugins/marketplace-backend/src/installation/FileInstallationStorage.ts b/workspaces/marketplace/plugins/marketplace-backend/src/installation/FileInstallationStorage.ts index 455439d782..aa363cfd80 100644 --- a/workspaces/marketplace/plugins/marketplace-backend/src/installation/FileInstallationStorage.ts +++ b/workspaces/marketplace/plugins/marketplace-backend/src/installation/FileInstallationStorage.ts @@ -27,7 +27,6 @@ import { InstallationInitErrorReason, } from '../errors/InstallationInitError'; import type { JsonValue } from '@backstage/types'; -import { ConflictError } from '@backstage/errors'; export interface InstallationStorage { initialize?(): void; @@ -35,7 +34,7 @@ export interface InstallationStorage { updatePackage(packageName: string, newConfig: string): void; getPackages(packageNames: Set): string | undefined; updatePackages(packageNames: Set, newConfig: string): void; - addPackageDisabled(packageName: string, disabled: boolean): void; + setPackageDisabled(packageName: string, disabled: boolean): void; setPackagesDisabled(packageNames: Set, disabled: boolean): void; } @@ -134,17 +133,14 @@ export class FileInstallationStorage implements InstallationStorage { this.save(); } - addPackageDisabled(packageName: string, disabled: boolean) { - const existingPackage = this.getPackageYamlMap(packageName); - if (existingPackage) { - throw new ConflictError( - `Package '${packageName}' already exists in the configuration`, - ); + setPackageDisabled(packageName: string, disabled: boolean) { + let pkg = this.getPackageYamlMap(packageName); + if (!pkg) { + pkg = new YAMLMap(); + pkg.set('package', packageName); + this.packages.add(pkg); } - const newPackage = new YAMLMap(); - newPackage.set('package', packageName); - newPackage.set('disabled', disabled); - this.packages.add(newPackage); + pkg.set('disabled', disabled); this.save(); } diff --git a/workspaces/marketplace/plugins/marketplace-backend/src/installation/InstallationDataService.test.ts b/workspaces/marketplace/plugins/marketplace-backend/src/installation/InstallationDataService.test.ts index 53d691129b..7eadd6cf66 100644 --- a/workspaces/marketplace/plugins/marketplace-backend/src/installation/InstallationDataService.test.ts +++ b/workspaces/marketplace/plugins/marketplace-backend/src/installation/InstallationDataService.test.ts @@ -237,7 +237,7 @@ describe('InstallationDataService', () => { }); }); - describe('addPackageDisabled', () => { + describe('setPackageDisabled', () => { beforeEach(() => { installationDataService = InstallationDataService.fromConfig({ config: validConfig, @@ -246,14 +246,14 @@ describe('InstallationDataService', () => { }); }); - it('should add package with disabled', async () => { - installationDataService.addPackageDisabled( + it('should set package with disabled', async () => { + installationDataService.setPackageDisabled( mockDynamicPackage11.package, false, ); expect( - mockFileInstallationStorage.addPackageDisabled, + mockFileInstallationStorage.setPackageDisabled, ).toHaveBeenCalledWith(mockDynamicPackage11.package, false); }); }); diff --git a/workspaces/marketplace/plugins/marketplace-backend/src/installation/InstallationDataService.ts b/workspaces/marketplace/plugins/marketplace-backend/src/installation/InstallationDataService.ts index ac6d29ae35..982a47e66c 100644 --- a/workspaces/marketplace/plugins/marketplace-backend/src/installation/InstallationDataService.ts +++ b/workspaces/marketplace/plugins/marketplace-backend/src/installation/InstallationDataService.ts @@ -169,8 +169,8 @@ export class InstallationDataService { this.installationStorage.updatePackages(dynamicArtifacts, newConfig); } - addPackageDisabled(packageDynamicArtifact: string, disabled: boolean) { - this.installationStorage.addPackageDisabled( + setPackageDisabled(packageDynamicArtifact: string, disabled: boolean) { + this.installationStorage.setPackageDisabled( packageDynamicArtifact, disabled, ); diff --git a/workspaces/marketplace/plugins/marketplace-backend/src/router.test.ts b/workspaces/marketplace/plugins/marketplace-backend/src/router.test.ts index 160fde45cd..98d4546f8d 100644 --- a/workspaces/marketplace/plugins/marketplace-backend/src/router.test.ts +++ b/workspaces/marketplace/plugins/marketplace-backend/src/router.test.ts @@ -149,9 +149,9 @@ const configurationEndpointsTestCases = [ body: { configYaml: stringify(mockDynamicPackage11) }, }, { - description: 'POST /package/:namespace/:name/configuration/disable', + description: 'PATCH /package/:namespace/:name/configuration/disable', reqBuilder: (req: request.SuperTest) => - req.post( + req.patch( '/api/extensions/package/default/package11/configuration/disable', ), body: { disabled: true }, @@ -773,11 +773,11 @@ describe('createRouter', () => { }); }); - describe('POST /package/:namespace/:name/configuration/disable', () => { + describe('PATCH /package/:namespace/:name/configuration/disable', () => { it('should fail when disabled missing with InputError 400', async () => { const { backendServer } = await setupTestWithMockCatalog(PACKAGE_SETUP); - const response = await request(backendServer).post( + const response = await request(backendServer).patch( '/api/extensions/package/default/package11/configuration/disable', ); expectInputError(response, "'disabled' must be present boolean"); @@ -787,7 +787,9 @@ describe('createRouter', () => { const { backendServer } = await setupTestWithMockCatalog(PACKAGE_SETUP); const response = await request(backendServer) - .post('/api/extensions/package/default/package11/configuration/disable') + .patch( + '/api/extensions/package/default/package11/configuration/disable', + ) .send({ disabled: 'invalid' }); expectInputError(response, "'disabled' must be present boolean"); }); @@ -801,7 +803,9 @@ describe('createRouter', () => { }); const response = await request(backendServer) - .post('/api/extensions/package/default/not-found/configuration/disable') + .patch( + '/api/extensions/package/default/not-found/configuration/disable', + ) .send({ disabled: true }); expectNotFoundError(response, MarketplaceKind.Package); }); @@ -813,10 +817,12 @@ describe('createRouter', () => { const { backendServer } = await setupTestWithMockCatalog(PACKAGE_SETUP); const response = await request(backendServer) - .post('/api/extensions/package/default/package11/configuration/disable') + .patch( + '/api/extensions/package/default/package11/configuration/disable', + ) .send({ disabled }); expect( - mockInstallationDataService.addPackageDisabled, + mockInstallationDataService.setPackageDisabled, ).toHaveBeenCalledWith(mockDynamicPackage11.package, disabled); expect(response.status).toEqual(200); expect(response.body).toEqual({ status: 'OK' }); diff --git a/workspaces/marketplace/plugins/marketplace-backend/src/router.ts b/workspaces/marketplace/plugins/marketplace-backend/src/router.ts index 4aefb0ffc4..0766fbca06 100644 --- a/workspaces/marketplace/plugins/marketplace-backend/src/router.ts +++ b/workspaces/marketplace/plugins/marketplace-backend/src/router.ts @@ -300,7 +300,7 @@ export async function createRouter( }); }); - router.post( + router.patch( '/package/:namespace/:name/configuration/disable', requireInitializedInstallationDataService, async (req, res) => { @@ -319,7 +319,7 @@ export async function createRouter( if (typeof disabled !== 'boolean') { throw new InputError("'disabled' must be present boolean"); } - installationDataService.addPackageDisabled( + installationDataService.setPackageDisabled( marketplacePackage.spec.dynamicArtifact, disabled, );