fix: only shutdown providers that are not attached to any client (#444)

Signed-off-by: Lukas Reining <lukas.reining@codecentric.de> Co-authored-by: Michael Beemer <beeme1mr@users.noreply.github.com>
open-feature · Jun 2, 2023 · 7e469c4 · 7e469c4
1 parent 5d55ea1
commit 7e469c4
Show file tree

Hide file tree

Showing 3 changed files with 49 additions and 2 deletions.
diff --git a/packages/client/test/open-feature.spec.ts b/packages/client/test/open-feature.spec.ts
@@ -80,6 +80,29 @@ describe('OpenFeature', () => {
 
       expect(namedClient.metadata.providerMetadata.name).toEqual(fakeProvider.metadata.name);
     });
+
+    it('should close a provider if it is replaced and no other client uses it', async () => {
+      const provider1 = { ...MOCK_PROVIDER, onClose: jest.fn() };
+      const provider2 = { ...MOCK_PROVIDER, onClose: jest.fn() };
+
+      OpenFeature.setProvider('client1', provider1);
+      expect(provider1.onClose).not.toHaveBeenCalled();
+      OpenFeature.setProvider('client1', provider2);
+      expect(provider1.onClose).toHaveBeenCalledTimes(1);
+    });
+
+    it('should not close provider if it is used by another client', async () => {
+      const provider1 = { ...MOCK_PROVIDER, onClose: jest.fn() };
+
+      OpenFeature.setProvider('client1', provider1);
+      OpenFeature.setProvider('client2', provider1);
+
+      OpenFeature.setProvider('client1', { ...provider1 });
+      expect(provider1.onClose).not.toHaveBeenCalled();
+
+      OpenFeature.setProvider('client2', { ...provider1 });
+      expect(provider1.onClose).toHaveBeenCalledTimes(1);
+    });
   });
 
   describe('Requirement 1.1.4', () => {

diff --git a/packages/server/test/open-feature.spec.ts b/packages/server/test/open-feature.spec.ts
@@ -82,6 +82,29 @@ describe('OpenFeature', () => {
 
       expect(namedClient.metadata.providerMetadata.name).toEqual(fakeProvider.metadata.name);
     });
+
+    it('should close a provider if it is replaced and no other client uses it', async () => {
+      const provider1 = { ...MOCK_PROVIDER, onClose: jest.fn() };
+      const provider2 = { ...MOCK_PROVIDER, onClose: jest.fn() };
+
+      OpenFeature.setProvider('client1', provider1);
+      expect(provider1.onClose).not.toHaveBeenCalled();
+      OpenFeature.setProvider('client1', provider2);
+      expect(provider1.onClose).toHaveBeenCalledTimes(1);
+    });
+
+    it('should not close provider if it is used by another client', async () => {
+      const provider1 = { ...MOCK_PROVIDER, onClose: jest.fn() };
+
+      OpenFeature.setProvider('client1', provider1);
+      OpenFeature.setProvider('client2', provider1);
+
+      OpenFeature.setProvider('client1', { ...provider1 });
+      expect(provider1.onClose).not.toHaveBeenCalled();
+
+      OpenFeature.setProvider('client2', { ...provider1 });
+      expect(provider1.onClose).toHaveBeenCalledTimes(1);
+    });
   });
 
   describe('Requirement 1.1.4', () => {

diff --git a/packages/shared/src/open-feature.ts b/packages/shared/src/open-feature.ts
@@ -129,10 +129,11 @@ export abstract class OpenFeatureCommonAPI<P extends CommonProvider = CommonProv
 
     this.transferListeners(oldProvider, provider, clientName, clientEmitter);
 
-    // Do not close the default provider if a named client used the default provider
-    if (!clientName || (clientName && oldProvider !== this._defaultProvider)) {
+    // Do not close a provider that is bound to any client
+    if (![...this._clientProviders.values(), this._defaultProvider].includes(oldProvider)) {
       oldProvider?.onClose?.();
     }
+
     return this;
   }