simplify addDomainMetadata trimming logic

MetaMask · May 20, 2020 · bb8e211 · bb8e211
1 parent bec3dab
commit bb8e211
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 17 deletions.
diff --git a/app/scripts/controllers/permissions/index.js b/app/scripts/controllers/permissions/index.js
@@ -535,20 +535,13 @@ export class PermissionsController {
 
     // attempt to pop metadata of a single domain without permissions if the
     // store is too large overall
-    if (this._newMetadataOrigins.size >= METADATA_CACHE_MAX_SIZE) {
+    if (this._pendingSiteMetadata.size >= METADATA_CACHE_MAX_SIZE) {
       const permissionsDomains = this.permissions.getDomains()
 
-      for (const origin of this._newMetadataOrigins.values()) {
-
-        // We can stop keeping track of all domains we iterate over, because
-        // we will either delete 'origin' now if it does not have permissions,
-        // or we won't delete 'origin' until the next time MetaMask boots.
-        this._newMetadataOrigins.delete(origin)
-
-        if (!permissionsDomains[origin]) {
-          delete newMetadataState[origin]
-          break
-        }
+      const oldOrigin = this._pendingSiteMetadata.values().next().value
+      this._pendingSiteMetadata.delete(oldOrigin)
+      if (!permissionsDomains[oldOrigin]) {
+        delete newMetadataState[oldOrigin]
       }
     }
 
@@ -558,7 +551,7 @@ export class PermissionsController {
       ...metadata,
       lastUpdated: Date.now(),
     }
-    this._newMetadataOrigins.add(origin)
+    this._pendingSiteMetadata.add(origin)
     this._setDomainMetadata(newMetadataState)
   }
 
@@ -573,7 +566,7 @@ export class PermissionsController {
     const metadataState = restoredState[METADATA_STORE_KEY] || {}
     const newMetadataState = this._trimDomainMetadata(metadataState)
 
-    this._newMetadataOrigins = new Set()
+    this._pendingSiteMetadata = new Set()
     this._setDomainMetadata(newMetadataState)
   }
 

diff --git a/test/unit/app/controllers/permissions/permissions-controller-test.js b/test/unit/app/controllers/permissions/permissions-controller-test.js
@@ -1419,7 +1419,9 @@ describe('permissions controller', function () {
       )
     })
 
-    it('pops metadata when the metadata store is too large', function () {
+    it('pops metadata on add when too many origins are pending', function () {
+
+      sinon.spy(permController._pendingSiteMetadata, 'delete')
 
       const mockMetadata = getMockMetadata(METADATA_CACHE_MAX_SIZE)
       const expectedDeletedOrigin = Object.keys(mockMetadata)[0]
@@ -1428,10 +1430,10 @@ describe('permissions controller', function () {
         [METADATA_STORE_KEY]: { ...mockMetadata },
       })
 
-      // populate permController.newMetadataOrigins, as though these origins
+      // populate permController._pendingSiteMetadata, as though these origins
       // were actually added
       Object.keys(mockMetadata).forEach((origin) => {
-        permController._newMetadataOrigins.add(origin)
+        permController._pendingSiteMetadata.add(origin)
       })
 
       permController.addDomainMetadata(ORIGINS.a, { foo: 'bar' })
@@ -1450,6 +1452,10 @@ describe('permissions controller', function () {
       }
       delete expectedMetadata[expectedDeletedOrigin]
 
+      assert.ok(
+        permController._pendingSiteMetadata.delete.calledOnceWithExactly(expectedDeletedOrigin),
+        'should have called _pendingSiteMetadata.delete once'
+      )
       assert.equal(
         permController._setDomainMetadata.getCalls().length, 1,
         'should have called _setDomainMetadata once'