fix(extensions): update package disabled status endpoint

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

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<FileInstallationStorage>;
 
@@ -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<InstallationDataService>;
 

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"

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,
+      ]);
     });
   });
 

workspaces/marketplace/plugins/marketplace-backend/src/installation/FileInstallationStorage.ts

@@ -27,15 +27,14 @@ import {
   InstallationInitErrorReason,
 } from '../errors/InstallationInitError';
 import type { JsonValue } from '@backstage/types';
-import { ConflictError } from '@backstage/errors';
 
 export interface InstallationStorage {
   initialize?(): void;
   getPackage(packageName: string): string | undefined;
   updatePackage(packageName: string, newConfig: string): void;
   getPackages(packageNames: Set<string>): string | undefined;
   updatePackages(packageNames: Set<string>, newConfig: string): void;
-  addPackageDisabled(packageName: string, disabled: boolean): void;
+  setPackageDisabled(packageName: string, disabled: boolean): void;
   setPackagesDisabled(packageNames: Set<string>, 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<string, JsonValue>();
+      pkg.set('package', packageName);
+      this.packages.add(pkg);
     }
-    const newPackage = new YAMLMap<string, JsonValue>();
-    newPackage.set('package', packageName);
-    newPackage.set('disabled', disabled);
-    this.packages.add(newPackage);
+    pkg.set('disabled', disabled);
     this.save();
   }
 

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);
     });
   });

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,
     );

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<request.Test>) =>
-      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' });

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,
       );