diff --git a/.changeset/unlucky-ghosts-yell.md b/.changeset/unlucky-ghosts-yell.md new file mode 100644 index 0000000..76ccd30 --- /dev/null +++ b/.changeset/unlucky-ghosts-yell.md @@ -0,0 +1,5 @@ +--- +"@saleor/app-sdk": minor +--- + +Added new method to SettingsManager - "delete". It can delete metadata by key in Saleor. Implemented in MetadataManager and EncryptedMetadataManager diff --git a/docs/settings-manager.md b/docs/settings-manager.md index eb7af2a..5d97201 100644 --- a/docs/settings-manager.md +++ b/docs/settings-manager.md @@ -12,12 +12,28 @@ Entries in the manager are stored using structure: domain?: string; ``` +## `DeleteMetadataArg` interface + +Argument that can be used to remove metadata via `manager.delete()` method + +It contains key and domain: + +```typescript +type DeleteMetadataArg = { + key: string; + domain: string; +}; +``` + For values which should not be migrated during environment cloning (as private keys to payment provider), developer should use domain field to bind it to particular store instance. ## Available methods - `get: (key: string, domain?: string) => Promise` - `set: (settings: SettingsValue[] | SettingsValue) => Promise` +- `delete: (args: string | string[] | DeleteMetadataArg | DeleteMetadataArg[]) => Promise` + +Warning: delete method can throw, if instance of SettingsManager was not configured with proper mutation in constructor. # MetadataManager @@ -31,6 +47,7 @@ import { FetchAppDetailsDocument, FetchAppDetailsQuery, UpdateAppMetadataDocument, + DeleteMetadataDocument, } from "../generated/graphql"; export async function fetchAllMetadata(client: Client): Promise { @@ -56,6 +73,10 @@ export async function mutateMetadata(client: Client, metadata: MetadataEntry[]) })) || [] ); } + +export async function deleteMetadata(client: Client, keys: string[]) { + return client.mutation(DeleteMetadataDocument, { keys }).toPromise(); +} ``` And create MetadataManager instance: @@ -64,6 +85,7 @@ And create MetadataManager instance: const settings = new MetadataManager({ fetchMetadata: () => fetchAllMetadata(client), mutateMetadata: (md) => mutateMetadata(client, md), + deleteMetadata: (keys) => deleteMetadata(client, keys), }); ``` @@ -77,6 +99,7 @@ new EncryptedMetadataManager({ encryptionKey: process.env.SECRET_KEY, // secrets should be saved in the environment variables, never in the source code fetchMetadata: () => fetchAllMetadata(client), mutateMetadata: (metadata) => mutateMetadata(client, metadata), + deleteMetadata: (keys) => deleteMetadata(client, keys), }); ``` diff --git a/src/settings-manager/encrypted-metadata-manager.test.ts b/src/settings-manager/encrypted-metadata-manager.test.ts index 1694463..aa60788 100644 --- a/src/settings-manager/encrypted-metadata-manager.test.ts +++ b/src/settings-manager/encrypted-metadata-manager.test.ts @@ -62,6 +62,8 @@ describe("settings-manager", () => { }); describe("Manager operations", () => { + // eslint-disable-next-line @typescript-eslint/no-empty-function + const deleteMetadataMock = vi.fn(async () => {}); const fetchMock = vi.fn(async () => metadata); const mutateMock = vi.fn(async (md: MetadataEntry[]) => [...metadata, ...md]); const manager = new EncryptedMetadataManager({ @@ -122,6 +124,48 @@ describe("settings-manager", () => { // Set method should populate cache with updated values, so fetch is never called expect(fetchMock).toBeCalledTimes(0); }); + + /** + * Smoke test operations for "delete" method. + * Details tests are in MetadataManager that is under the hood of EncryptedMetadataManager + */ + it("Calls delete metadata mutation when key deleted - all formats", async () => { + const managerWithDelete = new EncryptedMetadataManager({ + fetchMetadata: fetchMock, + mutateMetadata: mutateMock, + deleteMetadata: deleteMetadataMock, + encryptionKey: "key", + }); + + await managerWithDelete.delete("single-arg-1"); + await managerWithDelete.delete(["multiple-arg-1", "multiple-arg-2"]); + await managerWithDelete.delete({ + key: "single-arg-1-domain", + domain: "https://example.com/graphql/", + }); + await managerWithDelete.delete([ + { + key: "multiple-arg-1-domain", + domain: "https://example.com/graphql/", + }, + { + key: "multiple-arg-2-domain", + domain: "https://example.com/graphql/", + }, + ]); + + expect(deleteMetadataMock).toBeCalledTimes(4); + + expect(deleteMetadataMock).toHaveBeenNthCalledWith(1, ["single-arg-1"]); + expect(deleteMetadataMock).toHaveBeenNthCalledWith(2, ["multiple-arg-1", "multiple-arg-2"]); + expect(deleteMetadataMock).toHaveBeenNthCalledWith(3, [ + "single-arg-1-domain__https://example.com/graphql/", + ]); + expect(deleteMetadataMock).toHaveBeenNthCalledWith(4, [ + "multiple-arg-1-domain__https://example.com/graphql/", + "multiple-arg-2-domain__https://example.com/graphql/", + ]); + }); }); }); }); diff --git a/src/settings-manager/encrypted-metadata-manager.ts b/src/settings-manager/encrypted-metadata-manager.ts index e914627..e9d11e5 100644 --- a/src/settings-manager/encrypted-metadata-manager.ts +++ b/src/settings-manager/encrypted-metadata-manager.ts @@ -1,7 +1,7 @@ import crypto from "crypto"; import { MetadataManager, MetadataManagerConfig } from "./metadata-manager"; -import { SettingsManager, SettingsValue } from "./settings-manager"; +import { DeleteSettingsValue, SettingsManager, SettingsValue } from "./settings-manager"; export type EncryptCallback = (value: string, secret: string) => string; @@ -66,10 +66,12 @@ export class EncryptedMetadataManager implements SettingsManager { encryptionKey, encryptionMethod, decryptionMethod, + deleteMetadata, }: EncryptedMetadataManagerConfig) { this.metadataManager = new MetadataManager({ fetchMetadata, mutateMetadata, + deleteMetadata, }); if (encryptionKey) { this.encryptionKey = encryptionKey; @@ -110,4 +112,8 @@ export class EncryptedMetadataManager implements SettingsManager { })); return this.metadataManager.set(encryptedSettings); } + + async delete(args: DeleteSettingsValue | DeleteSettingsValue[] | string | string[]) { + await this.metadataManager.delete(args); + } } diff --git a/src/settings-manager/metadata-manager.test.ts b/src/settings-manager/metadata-manager.test.ts index 9713e98..06c9620 100644 --- a/src/settings-manager/metadata-manager.test.ts +++ b/src/settings-manager/metadata-manager.test.ts @@ -13,6 +13,8 @@ describe("settings-manager", () => { describe("metadata-manager", () => { const fetchMock = vi.fn(async () => metadata); const mutateMock = vi.fn(async (md: MetadataEntry[]) => [...metadata, ...md]); + // eslint-disable-next-line @typescript-eslint/no-empty-function + const deleteMetadataMock = vi.fn(async () => {}); beforeEach(() => { vi.restoreAllMocks(); @@ -53,5 +55,71 @@ describe("settings-manager", () => { // Set method should populate cache with updated values, so fetch is never called expect(fetchMock).toBeCalledTimes(0); }); + + describe("Delete metadata method", () => { + /** + * Ensure no breaking changes introduced + */ + it("Constructs if deleteMetadata param is not passed", () => { + const manager = new MetadataManager({ + fetchMetadata: fetchMock, + mutateMetadata: mutateMock, + }); + + expect(manager).toBeDefined(); + }); + + it("Throws if \"delete\" method is called, but deleteMetadata was not passed to constructor", async () => { + const manager = new MetadataManager({ + fetchMetadata: fetchMock, + mutateMetadata: mutateMock, + }); + + await expect(manager.delete("test")).rejects.toThrowErrorMatchingInlineSnapshot( + "\"Delete not implemented. Ensure MetadataManager is configured with deleteMetadata param in constructor\"" + ); + }); + + it("Calls deleteMetadata constructor param when \"delete\" method called", async () => { + const manager = new MetadataManager({ + fetchMetadata: fetchMock, + mutateMetadata: mutateMock, + deleteMetadata: deleteMetadataMock, + }); + + await manager.delete("single-key"); + await manager.delete(["multiple-key-1", "multiple-key-2"]); + + expect(deleteMetadataMock).toBeCalledTimes(2); + /** + * Ensure callback is called with array type, even if single string was passed + */ + expect(deleteMetadataMock).toHaveBeenNthCalledWith(1, ["single-key"]); + expect(deleteMetadataMock).toHaveBeenNthCalledWith(2, ["multiple-key-1", "multiple-key-2"]); + }); + + it("Accepts records with key and domain and constructs scoped suffixes", async () => { + const manager = new MetadataManager({ + fetchMetadata: fetchMock, + mutateMetadata: mutateMock, + deleteMetadata: deleteMetadataMock, + }); + + await manager.delete({ key: "test", domain: "https://example.com/graphql/" }); + await manager.delete([ + { key: "test1", domain: "https://example.com/graphql/" }, + { key: "test2", domain: "https://example.com/graphql/" }, + ]); + + expect(deleteMetadataMock).toBeCalledTimes(2); + expect(deleteMetadataMock).toHaveBeenNthCalledWith(1, [ + "test__https://example.com/graphql/", + ]); + expect(deleteMetadataMock).toHaveBeenNthCalledWith(2, [ + "test1__https://example.com/graphql/", + "test2__https://example.com/graphql/", + ]); + }); + }); }); }); diff --git a/src/settings-manager/metadata-manager.ts b/src/settings-manager/metadata-manager.ts index 0339f11..d2f74f3 100644 --- a/src/settings-manager/metadata-manager.ts +++ b/src/settings-manager/metadata-manager.ts @@ -1,13 +1,16 @@ -import { SettingsManager, SettingsValue } from "./settings-manager"; +import { DeleteSettingsValue, SettingsManager, SettingsValue } from "./settings-manager"; export type MetadataEntry = { key: string; value: string; }; +/** + * TODO Rename "Callback" suffixes, these are not callbacks + */ export type FetchMetadataCallback = () => Promise; - export type MutateMetadataCallback = (metadata: MetadataEntry[]) => Promise; +export type DeleteMetadataCallback = (keys: string[]) => Promise; const deserializeMetadata = ({ key, value }: MetadataEntry): SettingsValue => { // domain specific metadata use convention key__domain, e.g. `secret_key__example.com` @@ -20,6 +23,8 @@ const deserializeMetadata = ({ key, value }: MetadataEntry): SettingsValue => { }; }; +const constructDomainSpecificKey = (key: string, domain: string) => [key, domain].join("__"); + const serializeSettingsToMetadata = ({ key, value, domain }: SettingsValue): MetadataEntry => { // domain specific metadata use convention key__domain, e.g. `secret_key__example.com` if (!domain) { @@ -27,7 +32,7 @@ const serializeSettingsToMetadata = ({ key, value, domain }: SettingsValue): Met } return { - key: [key, domain].join("__"), + key: constructDomainSpecificKey(key, domain), value, }; }; @@ -35,6 +40,11 @@ const serializeSettingsToMetadata = ({ key, value, domain }: SettingsValue): Met export interface MetadataManagerConfig { fetchMetadata: FetchMetadataCallback; mutateMetadata: MutateMetadataCallback; + /** + * Keep it optional, to avoid breaking changes. If not provided, delete will throw. + * TODO: Make it required in next major version + */ + deleteMetadata?: DeleteMetadataCallback; } /** @@ -51,9 +61,12 @@ export class MetadataManager implements SettingsManager { private mutateMetadata: MutateMetadataCallback; - constructor({ fetchMetadata, mutateMetadata }: MetadataManagerConfig) { + private deleteMetadata?: DeleteMetadataCallback; + + constructor({ fetchMetadata, mutateMetadata, deleteMetadata }: MetadataManagerConfig) { this.fetchMetadata = fetchMetadata; this.mutateMetadata = mutateMetadata; + this.deleteMetadata = deleteMetadata; } async get(key: string, domain?: string) { @@ -77,4 +90,28 @@ export class MetadataManager implements SettingsManager { const metadata = await this.mutateMetadata(serializedMetadata); this.settings = metadata.map(deserializeMetadata); } + + /** + * Typescript doesn't properly infer arguments, so they have to be rewritten explicitly + */ + async delete(args: DeleteSettingsValue | DeleteSettingsValue[] | string | string[]) { + if (!this.deleteMetadata) { + throw new Error( + "Delete not implemented. Ensure MetadataManager is configured with deleteMetadata param in constructor" + ); + } + + const argsArray = Array.isArray(args) ? args : [args]; + const keysToDelete = argsArray.map((keyOrDomainPair) => { + if (typeof keyOrDomainPair === "string") { + return keyOrDomainPair; + } + const { key, domain } = keyOrDomainPair; + return constructDomainSpecificKey(key, domain); + }); + + await this.deleteMetadata(keysToDelete); + + this.settings = this.settings.filter((setting) => !argsArray.includes(setting.key)); + } } diff --git a/src/settings-manager/settings-manager.ts b/src/settings-manager/settings-manager.ts index c2a2644..31ef55f 100644 --- a/src/settings-manager/settings-manager.ts +++ b/src/settings-manager/settings-manager.ts @@ -4,7 +4,18 @@ export type SettingsValue = { domain?: string; }; +export type DeleteSettingsValue = { + key: string; + domain: string; +}; + +type DeleteFnSimple = (keys: string | string[]) => Promise; +type DeleteFnWithDomain = ( + keysWithDomain: DeleteSettingsValue | DeleteSettingsValue[] +) => Promise; + export interface SettingsManager { get: (key: string, domain?: string) => Promise; set: (settings: SettingsValue[] | SettingsValue) => Promise; + delete: DeleteFnSimple | DeleteFnWithDomain; }