Include Storage Location In Fake Cache Key

[justindbaur]

Type of change

- [x] Bug fix
- [ ] New feature development
- [ ] Tech debt (refactoring, code cleanup, dependency upgrades, etc)
- [ ] Build/deploy pipeline (DevOps)
- [ ] Other

Objective

Include storage location in cache key for fake state. states is publicly exposed but wasn't used in any tests.

The storage location is already used in the cache key for the real state providers.

Code changes

• Use storage location in cache key.

Screenshots

Before you submit

• Please add unit tests where it makes sense to do so (encouraged but not required)
• If this change requires a documentation update - notify the documentation team
• If this change has particular deployment requirements - notify the DevOps team
• Ensure that all UI additions follow WCAG AA requirements

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 25.05%. Comparing base (4ba2717) to head (ccbd811).
Report is 2 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #8203      +/-   ##
==========================================
- Coverage   25.06%   25.05%   -0.01%     
==========================================
  Files        2248     2246       -2     
  Lines       65776    65773       -3     
  Branches    12410    12404       -6     
==========================================
- Hits        16484    16481       -3     
  Misses      47952    47952              
  Partials     1340     1340              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[bitwarden-bot]

Checkmarx One – Scan Summary & Details – 1ed38afd-8478-4624-b242-1548ef29e5c5

No New Or Fixed Issues Found

[JaredSnider-Bitwarden]

I tested this change locally, and this solved both of my issues that I encountered:

(1) When setting up test data, you couldn't set data for different KeyDefinitions without overriding the previous ones.

// set real value in memory  
 singleUserStateProvider
          .getFake(userIdFromAccessToken, ACCESS_TOKEN_MEMORY)
          .stateSubject.next([userIdFromAccessToken, accessTokenJwt]);

// set undefined value in disk
  singleUserStateProvider
          .getFake(userIdFromAccessToken, ACCESS_TOKEN_DISK)
          .stateSubject.next([userIdFromAccessToken, undefined]);  // this sets ACCESS_TOKEN_MEMORY to undefined.

(2): When asserting data was stored in a specific location, the tests would pass even if data wasn't actually stored in the asserted KeyDefinition

 expect(
            singleUserStateProvider.getFake(userIdFromAccessToken, REFRESH_TOKEN_DISK).nextMock,
          ).toHaveBeenCalledWith(refreshToken); // passes even if data was stored on REFRESH_TOKEN_MEMORY and not on REFRESH_TOKEN_DISK

[MGibson1]

Thanks for shoring this up!