[BUGFIX] Le InMemoryTemporaryStorage ne suit pas la TemporaryStorage …

…API et est utilisé à tort dans les tests d'intégration (PIX-12551) #8954
1024pix · May 22, 2024 · 26e8f32 · 26e8f32
2 parents 9a97cea + b2f47d6
commit 26e8f32
Show file tree

Hide file tree

Showing 9 changed files with 48 additions and 47 deletions.
diff --git a/api/lib/infrastructure/temporary-storage/InMemoryTemporaryStorage.js b/api/lib/infrastructure/temporary-storage/InMemoryTemporaryStorage.js
@@ -11,55 +11,55 @@ class InMemoryTemporaryStorage extends TemporaryStorage {
     this._client = new NodeCache();
   }
 
-  save({ key, value, expirationDelaySeconds }) {
+  async save({ key, value, expirationDelaySeconds }) {
     const storageKey = trim(key) || InMemoryTemporaryStorage.generateKey();
     this._client.set(storageKey, value, expirationDelaySeconds);
     return storageKey;
   }
 
-  update(key, value) {
+  async update(key, value) {
     const storageKey = trim(key);
     const timeoutMs = this._client.getTtl(storageKey);
     const expirationDelaySeconds = timeoutMs === 0 ? 0 : (timeoutMs - Date.now()) / 1000;
     this._client.set(storageKey, value, expirationDelaySeconds);
   }
 
-  get(key) {
+  async get(key) {
     return this._client.get(key);
   }
 
-  delete(key) {
+  async delete(key) {
     return this._client.del(key);
   }
 
   quit() {
     noop;
   }
 
-  expire({ key, expirationDelaySeconds }) {
+  async expire({ key, expirationDelaySeconds }) {
     return this._client.ttl(key, expirationDelaySeconds);
   }
 
-  ttl(key) {
+  async ttl(key) {
     return this._client.getTtl(key);
   }
 
-  lpush(key, value) {
+  async lpush(key, value) {
     let list = this._client.get(key) || [];
     list = [value, ...list];
     this._client.set(key, list);
     return list.length;
   }
 
-  lrem(key, value) {
+  async lrem(key, value) {
     const list = this._client.get(key) || [];
     const filtered = list.filter((item) => item !== value);
     const removed = list.filter((item) => item === value);
     this._client.set(key, filtered);
     return removed.length;
   }
 
-  lrange(key) {
+  async lrange(key) {
     return this._client.get(key) || [];
   }
 }

diff --git a/api/lib/infrastructure/temporary-storage/index.js b/api/lib/infrastructure/temporary-storage/index.js
@@ -1,12 +1,13 @@
 import { config } from '../../config.js';
-const REDIS_URL = config.temporaryStorage.redisUrl;
+
+const redisUrl = config.temporaryStorage.redisUrl;
 
 import { InMemoryTemporaryStorage } from './InMemoryTemporaryStorage.js';
 import { RedisTemporaryStorage } from './RedisTemporaryStorage.js';
 
 function _createTemporaryStorage() {
-  if (REDIS_URL) {
-    return new RedisTemporaryStorage(REDIS_URL);
+  if (redisUrl) {
+    return new RedisTemporaryStorage(redisUrl);
   } else {
     return new InMemoryTemporaryStorage();
   }

diff --git a/api/package.json b/api/package.json
@@ -159,7 +159,7 @@
     "test:api": "for testType in 'unit' 'integration' 'acceptance'; do npm run test:api:$testType || status=1 ; done ; exit $status",
     "test:api:path": "NODE_ENV=test mocha --exit --recursive --reporter=${MOCHA_REPORTER:-dot}",
     "test:api:scripts": "npm run test:api:path -- tests/integration/scripts",
-    "test:api:unit": "TEST_DATABASE_URL=postgres://should.not.reach.db.in.unit.tests REDIS_URL= npm run test:api:path -- 'tests/**/unit/**/*test.js'",
+    "test:api:unit": "TEST_DATABASE_URL=postgres://should.not.reach.db.in.unit.tests TEST_REDIS_URL= npm run test:api:path -- 'tests/**/unit/**/*test.js'",
     "test:api:integration": "npm run test:api:path -- 'tests/**/integration/**/*test.js'",
     "test:api:acceptance": "npm run test:api:path -- 'tests/**/acceptance/**/*test.js'",
     "test:api:debug": "NODE_ENV=test mocha --inspect-brk=9229 --recursive --exit --reporter dot tests",

diff --git a/api/src/identity-access-management/domain/services/fwb-oidc-authentication-service.js b/api/src/identity-access-management/domain/services/fwb-oidc-authentication-service.js
@@ -19,7 +19,7 @@ export class FwbOidcAuthenticationService extends OidcAuthenticationService {
   async getRedirectLogoutUrl({ userId, logoutUrlUUID }) {
     const redirectTarget = new URL(this.logoutUrl);
     const key = `${userId}:${logoutUrlUUID}`;
-    const idToken = this.sessionTemporaryStorage.get(key);
+    const idToken = await this.sessionTemporaryStorage.get(key);
 
     const params = [{ key: 'id_token_hint', value: idToken }];
 

diff --git a/...src/identity-access-management/domain/services/pole-emploi-oidc-authentication-service.js b/...src/identity-access-management/domain/services/pole-emploi-oidc-authentication-service.js
@@ -51,7 +51,7 @@ export class PoleEmploiOidcAuthenticationService extends OidcAuthenticationServi
   async getRedirectLogoutUrl({ userId, logoutUrlUUID }) {
     const redirectTarget = new URL(this.logoutUrl);
     const key = `${userId}:${logoutUrlUUID}`;
-    const idToken = this.sessionTemporaryStorage.get(key);
+    const idToken = await this.sessionTemporaryStorage.get(key);
 
     const params = [
       { key: 'id_token_hint', value: idToken },

diff --git a/api/src/shared/config.js b/api/src/shared/config.js
@@ -393,7 +393,7 @@ const configuration = (function () {
 
     config.temporaryKey.secret = 'the-password-must-be-at-least-32-characters-long';
 
-    config.temporaryStorage.redisUrl = null;
+    config.temporaryStorage.redisUrl = process.env.TEST_REDIS_URL;
 
     config.saml.accessTokenLifespanMs = 1000;
 
@@ -448,6 +448,7 @@ const configuration = (function () {
     config.logging.enableLogStartingEventDispatch = false;
     config.logging.enableLogEndingEventDispatch = false;
 
+    // TODO: Rather use config.caching.redisUrl = process.env.TEST_REDIS_URL;
     config.caching.redisUrl = null;
     config.caching.redisCacheKeyLockTTL = 100;
     config.caching.redisCacheLockedWaitBeforeRetry = 1;
@@ -456,7 +457,6 @@ const configuration = (function () {
 
     config.redis = {
       url: process.env.TEST_REDIS_URL,
-      database: 1,
     };
 
     config.import = {

diff --git a/...ity-access-management/integration/domain/services/fwb-oidc-authentication-service_test.js b/...ity-access-management/integration/domain/services/fwb-oidc-authentication-service_test.js
@@ -32,7 +32,7 @@ describe('Integration | Identity Access Management | Domain | Service | fwb-oidc
 
       // then
       const expectedResult = await defaultSessionTemporaryStorage.get(key);
-      expect(expectedResult).to.be.undefined;
+      expect(expectedResult).to.be.null;
 
       expect(redirectTarget).to.equal(
         'https://logout-url.org/?id_token_hint=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c',

diff --git a/...ss-management/integration/domain/services/pole-emploi-oidc-authentication-service_test.js b/...ss-management/integration/domain/services/pole-emploi-oidc-authentication-service_test.js
@@ -87,7 +87,7 @@ describe('Integration | Identity Access Management | Domain | Services | pole-em
 
       // then
       const expectedResult = await defaultSessionTemporaryStorage.get(key);
-      expect(expectedResult).to.be.undefined;
+      expect(expectedResult).to.be.null;
 
       expect(redirectTarget).to.equal(
         'https://logout-url.fr/?id_token_hint=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c&redirect_uri=https%3A%2F%2Fafter-logout.fr',

diff --git a/api/tests/unit/infrastructure/temporary-storage/InMemoryTemporaryStorage_test.js b/api/tests/unit/infrastructure/temporary-storage/InMemoryTemporaryStorage_test.js
@@ -19,20 +19,20 @@ describe('Unit | Infrastructure | temporary-storage | InMemoryTemporaryStorage',
       clock.restore();
     });
 
-    it('should resolve with the generated key', function () {
+    it('should resolve with the generated key', async function () {
       // when
-      const key = inMemoryTemporaryStorage.save({ value: {}, expirationDelaySeconds: 1000 });
+      const key = await inMemoryTemporaryStorage.save({ value: {}, expirationDelaySeconds: 1000 });
 
       // then
-      expect(key).to.exist;
+      expect(key).to.be.a.string;
     });
 
-    it('should return a key from passed key parameter if valid', function () {
+    it('should return a key from passed key parameter if valid', async function () {
       // given
       const keyParameter = 'KEY-PARAMETER';
 
       // when
-      const returnedKey = inMemoryTemporaryStorage.save({
+      const returnedKey = await inMemoryTemporaryStorage.save({
         key: keyParameter,
         value: {},
         expirationDelaySeconds: 1000,
@@ -42,12 +42,12 @@ describe('Unit | Infrastructure | temporary-storage | InMemoryTemporaryStorage',
       expect(returnedKey).to.be.equal(keyParameter);
     });
 
-    it('should return a generated key if key parameter is not valid', function () {
+    it('should return a generated key if key parameter is not valid', async function () {
       // given
       const keyParameter = '  ';
 
       // when
-      const returnedKey = inMemoryTemporaryStorage.save({
+      const returnedKey = await inMemoryTemporaryStorage.save({
         key: keyParameter,
         value: {},
         expirationDelaySeconds: 1000,
@@ -57,87 +57,87 @@ describe('Unit | Infrastructure | temporary-storage | InMemoryTemporaryStorage',
       expect(returnedKey).not.be.equal(keyParameter);
     });
 
-    it('should save key value with a defined ttl in seconds', function () {
+    it('should save key value with a defined ttl in seconds', async function () {
       // given
       const TWO_MINUTES_IN_SECONDS = 2 * 60;
       const TWO_MINUTES_IN_MILLISECONDS = 2 * 60 * 1000;
 
       // when
-      const key = inMemoryTemporaryStorage.save({
+      const key = await inMemoryTemporaryStorage.save({
         value: { name: 'name' },
         expirationDelaySeconds: TWO_MINUTES_IN_SECONDS,
       });
 
       // then
-      const expirationKeyInTimestamp = inMemoryTemporaryStorage._client.getTtl(key);
+      const expirationKeyInTimestamp = await inMemoryTemporaryStorage._client.getTtl(key);
       expect(expirationKeyInTimestamp).to.equal(TWO_MINUTES_IN_MILLISECONDS);
     });
   });
 
   describe('#get', function () {
-    it('should retrieve the value if it exists', function () {
+    it('should retrieve the value if it exists', async function () {
       // given
       const value = { name: 'name' };
       const expirationDelaySeconds = 1000;
 
-      const key = inMemoryTemporaryStorage.save({ value, expirationDelaySeconds });
+      const key = await inMemoryTemporaryStorage.save({ value, expirationDelaySeconds });
 
       // when
-      const result = inMemoryTemporaryStorage.get(key);
+      const result = await inMemoryTemporaryStorage.get(key);
 
       // then
       expect(result).to.deep.equal(value);
     });
   });
 
   describe('#update', function () {
-    it('should set a new value', function () {
+    it('should set a new value', async function () {
       // given
-      const key = inMemoryTemporaryStorage.save({
+      const key = await inMemoryTemporaryStorage.save({
         value: { name: 'name' },
       });
 
       // when
-      inMemoryTemporaryStorage.update(key, { url: 'url' });
+      await inMemoryTemporaryStorage.update(key, { url: 'url' });
 
       // then
-      const result = inMemoryTemporaryStorage.get(key);
+      const result = await inMemoryTemporaryStorage.get(key);
       expect(result).to.deep.equal({ url: 'url' });
     });
 
     it('should not change the time to live', async function () {
       // given
-      const keyWithTtl = inMemoryTemporaryStorage.save({
+      const keyWithTtl = await inMemoryTemporaryStorage.save({
         value: {},
         expirationDelaySeconds: 1,
       });
-      const keyWithoutTtl = inMemoryTemporaryStorage.save({ value: {} });
+      const keyWithoutTtl = await inMemoryTemporaryStorage.save({ value: {} });
 
       // when
       await new Promise((resolve) => setTimeout(resolve, 500));
-      inMemoryTemporaryStorage.update(keyWithTtl, {});
-      inMemoryTemporaryStorage.update(keyWithoutTtl, {});
+      await inMemoryTemporaryStorage.update(keyWithTtl, {});
+      await inMemoryTemporaryStorage.update(keyWithoutTtl, {});
       await new Promise((resolve) => setTimeout(resolve, 600));
 
       // then
-      expect(inMemoryTemporaryStorage.get(keyWithTtl)).to.be.undefined;
-      expect(inMemoryTemporaryStorage.get(keyWithoutTtl)).not.to.be.undefined;
+      expect(await inMemoryTemporaryStorage.get(keyWithTtl)).to.be.undefined;
+      expect(await inMemoryTemporaryStorage.get(keyWithoutTtl)).not.to.be.undefined;
     });
   });
 
   describe('#delete', function () {
-    it('should delete the value if it exists', function () {
+    it('should delete the value if it exists', async function () {
       // given
       const value = { name: 'name' };
       const expirationDelaySeconds = 1000;
 
-      const key = inMemoryTemporaryStorage.save({ value, expirationDelaySeconds });
+      const key = await inMemoryTemporaryStorage.save({ value, expirationDelaySeconds });
 
       // when
-      inMemoryTemporaryStorage.delete(key);
+      await inMemoryTemporaryStorage.delete(key);
 
       // then
-      const savedKey = inMemoryTemporaryStorage.get(key);
+      const savedKey = await inMemoryTemporaryStorage.get(key);
       expect(savedKey).to.be.undefined;
     });
   });
@@ -168,7 +168,7 @@ describe('Unit | Infrastructure | temporary-storage | InMemoryTemporaryStorage',
       const key = 'key:lpush';
       await inMemoryTemporaryStorage.lpush(key, 'value');
       await inMemoryTemporaryStorage.expire({ key, expirationDelaySeconds: 120 });
-      const remainingExpirationSeconds = inMemoryTemporaryStorage.ttl(key);
+      const remainingExpirationSeconds = await inMemoryTemporaryStorage.ttl(key);
 
       // then
       expect(remainingExpirationSeconds).to.be.above(Date.now());