From 3734aeab628fd187f12d0d397d8e3e9fca24c0bd Mon Sep 17 00:00:00 2001
From: Patrik Oldsberg <poldsberg@gmail.com>
Date: Fri, 24 Apr 2020 16:28:48 +0200
Subject: [PATCH] plugins/graphiql: more tests for StorageBucket and fix
 removeItem

---
 .../src/lib/storage/StorageBucket.test.ts     | 51 +++++++++++++++++++
 .../graphiql/src/lib/storage/StorageBucket.ts |  4 +-
 2 files changed, 53 insertions(+), 2 deletions(-)

diff --git a/plugins/graphiql/src/lib/storage/StorageBucket.test.ts b/plugins/graphiql/src/lib/storage/StorageBucket.test.ts
index 21fe10165c..d18d038cb5 100644
--- a/plugins/graphiql/src/lib/storage/StorageBucket.test.ts
+++ b/plugins/graphiql/src/lib/storage/StorageBucket.test.ts
@@ -28,6 +28,13 @@ describe('StorageBucket', () => {
     }).toThrow('Direct property access is not allowed for StorageBuckets');
   });
 
+  it('should not implement all methods', () => {
+    const bucket = StorageBucket.forLocalStorage('hello');
+
+    expect(() => bucket.length).toThrow('Method not implemented.');
+    expect(() => bucket.key()).toThrow('Method not implemented.');
+  });
+
   describe('with mocked underlying storage', () => {
     const mockStorage = {
       getItem: jest.fn(),
@@ -88,5 +95,49 @@ describe('StorageBucket', () => {
       expect(mockStorage.setItem).toHaveBeenCalledTimes(0);
       expect(mockStorage.removeItem).toHaveBeenCalledTimes(0);
     });
+
+    it('should remove an item', () => {
+      mockStorage.getItem.mockReturnValueOnce(
+        JSON.stringify({ x: 'X', y: 'Y' }),
+      );
+      bucket.removeItem('x');
+
+      expect(mockStorage.getItem).toHaveBeenCalledTimes(1);
+      expect(mockStorage.getItem).toHaveBeenLastCalledWith('my-bucket');
+      expect(mockStorage.setItem).toHaveBeenCalledTimes(1);
+      expect(mockStorage.setItem).toHaveBeenLastCalledWith(
+        'my-bucket',
+        JSON.stringify({ y: 'Y' }),
+      );
+      expect(mockStorage.removeItem).toHaveBeenCalledTimes(0);
+    });
+
+    it('should not bother to write when deleting a missing key', () => {
+      mockStorage.getItem.mockReturnValueOnce(JSON.stringify({ y: 'Y' }));
+      bucket.removeItem('x');
+
+      expect(mockStorage.getItem).toHaveBeenCalledTimes(1);
+      expect(mockStorage.getItem).toHaveBeenLastCalledWith('my-bucket');
+      expect(mockStorage.setItem).toHaveBeenCalledTimes(0);
+      expect(mockStorage.removeItem).toHaveBeenCalledTimes(0);
+    });
+
+    it('should ignore bad data', () => {
+      mockStorage.getItem.mockReturnValue('derp');
+
+      expect(bucket.getItem('x')).toBe(null);
+
+      expect(mockStorage.getItem).toHaveBeenCalledTimes(1);
+      expect(mockStorage.getItem).toHaveBeenLastCalledWith('my-bucket');
+      expect(mockStorage.setItem).toHaveBeenCalledTimes(0);
+      expect(mockStorage.removeItem).toHaveBeenCalledTimes(0);
+
+      bucket.removeItem('x');
+
+      expect(mockStorage.getItem).toHaveBeenCalledTimes(2);
+      expect(mockStorage.getItem).toHaveBeenLastCalledWith('my-bucket');
+      expect(mockStorage.setItem).toHaveBeenCalledTimes(0);
+      expect(mockStorage.removeItem).toHaveBeenCalledTimes(0);
+    });
   });
 });
diff --git a/plugins/graphiql/src/lib/storage/StorageBucket.ts b/plugins/graphiql/src/lib/storage/StorageBucket.ts
index 06263583c3..9d275e5f0d 100644
--- a/plugins/graphiql/src/lib/storage/StorageBucket.ts
+++ b/plugins/graphiql/src/lib/storage/StorageBucket.ts
@@ -71,8 +71,8 @@ export class StorageBucket implements Storage {
       return;
     }
 
-    const deleted = delete data[key];
-    if (deleted) {
+    if (key in data) {
+      delete data[key];
       this.write(data);
     }
   }