Fixes after code review (2024-05-07)

packages/apps/human-app/server/.env.example

@@ -1,8 +1,9 @@
 HOST= # string, example: localhost
 PORT= # number, example: 5010
 REPUTATION_ORACLE_URL= # string
-REDIS_PORT= # string, example: localhost
-REDIS_HOST= # number, example: 6379
+REPUTATION_ORACLE_ADDRESS= # string
+REDIS_HOST= # string, example: localhost
+REDIS_PORT= # number, example: 6379
 CACHE_TTL_ORACLE_DISCOVERY= # number, example: 43200
 CACHE_TTL_ORACLE_STATS= # number, example: 900
 CACHE_TTL_USER_STATS= # number, example: 86400

packages/apps/human-app/server/README.md

@@ -59,7 +59,7 @@ and transformation of data. NestJS provides many built-in pipes, so check the do
 - **Filters**: Classes implementing the `ExceptionFilter` interface, used to handle exceptions.
 - **Interceptors**: Used to bind extra logic before or after method execution or to extend the behavior of the method.
 
-### Model 
+### Interfaces 
 
 Interfaces are used to define the shape and responsibilities of the data:
 

packages/apps/human-app/server/docker-compose.yml

@@ -19,6 +19,7 @@ services:
       REDIS_PORT: ${REDIS_PORT}
       REDIS_HOST: redis
       CACHE_TTL_ORACLE_DISCOVERY: ${CACHE_TTL_ORACLE_DISCOVERY}
+      RPC_URL: ${RPC_URL}
     depends_on:
       - redis
   redis:
@@ -27,6 +28,4 @@ services:
     ports:
       - '${REDIS_PORT}:6379'
     volumes:
-      - redis_data:/data
-volumes:
-  redis_data:
+      - redis_data:/data

packages/apps/human-app/server/package.json

@@ -32,6 +32,8 @@
     "@nestjs/platform-express": "^10.2.6",
     "@nestjs/swagger": "^7.1.13",
     "cache-manager": "^5.4.0",
+    "cache-manager-redis-store": "^3.0.1",
+    "ioredis": "^5.3.2",
     "class-transformer": "^0.5.1",
     "class-validator": "^0.14.1",
     "ethers": "^6.11.0",
@@ -40,7 +42,6 @@
     "rxjs": "^7.2.0"
   },
   "devDependencies": {
-    "@nestjs/cache-manager": "^2.2.1",
     "@nestjs/cli": "^9.4.3",
     "@nestjs/schematics": "^9.2.0",
     "@nestjs/testing": "^9.4.3",
@@ -50,11 +51,9 @@
     "@types/supertest": "^2.0.15",
     "@typescript-eslint/eslint-plugin": "^5.0.0",
     "@typescript-eslint/parser": "^5.0.0",
-    "cache-manager-redis-store": "^3.0.1",
     "eslint": "^8.55.0",
     "eslint-config-prettier": "^9.1.0",
     "eslint-plugin-prettier": "^5.1.3",
-    "ioredis": "^5.3.2",
     "jest": "29.5.0",
     "nock": "^13.5.1",
     "prettier": "^3.1.1",

packages/apps/human-app/server/src/common/config/environment-config.service.ts

@@ -1,6 +1,5 @@
 import { ConfigService } from '@nestjs/config';
 import { Injectable } from '@nestjs/common';
-import { EnvironmentVariableMissingError } from '../interfaces/custom-exceptions.interface';
 const DEFAULT_CACHE_TTL_ORACLE_STATS = 12 * 60 * 60;
 const DEFAULT_CACHE_TTL_USER_STATS = 15 * 60;
 const DEFAULT_CACHE_TTL_ORACLE_DISCOVERY = 24 * 60 * 60;
@@ -10,21 +9,22 @@ const DEFAULT_CORS_ALLOWED_HEADERS = 'Content-Type, Accept';
 export class EnvironmentConfigService {
   constructor(private configService: ConfigService) {}
   get host(): string {
-    return this.conditionallyReturnMandatoryEnvVariable<string>('HOST');
+    return this.configService.getOrThrow<string>('HOST');
   }
   get port(): number {
-    return this.conditionallyReturnMandatoryEnvVariable<number>('PORT');
+    return this.configService.getOrThrow<number>('PORT');
   }
   get reputationOracleUrl(): string {
-    return this.conditionallyReturnMandatoryEnvVariable<string>(
-      'REPUTATION_ORACLE_URL',
-    );
+    return this.configService.getOrThrow<string>('REPUTATION_ORACLE_URL');
+  }
+  get reputationOracleAddress(): string {
+    return this.configService.getOrThrow<string>('REPUTATION_ORACLE_ADDRESS');
   }
   get cachePort(): number {
-    return this.conditionallyReturnMandatoryEnvVariable<number>('REDIS_PORT');
+    return this.configService.getOrThrow<number>('REDIS_PORT');
   }
   get cacheHost(): string {
-    return this.conditionallyReturnMandatoryEnvVariable<string>('REDIS_HOST');
+    return this.configService.getOrThrow<string>('REDIS_HOST');
   }
   get cacheTtlOracleStats(): number {
     return this.configService.get<number>(
@@ -47,7 +47,7 @@ export class EnvironmentConfigService {
     );
   }
   get rpcUrl(): string {
-    return this.conditionallyReturnMandatoryEnvVariable<string>('RPC_URL');
+    return this.configService.getOrThrow<string>('RPC_URL');
   }
   get isCorsEnabled(): boolean {
     return this.configService.get<boolean>('CORS_ENABLED', false);
@@ -61,35 +61,7 @@ export class EnvironmentConfigService {
   get corsAllowedHeaders(): string {
     return this.configService.get<string>(
       'CORS_ALLOWED_HEADERS',
-      DEFAULT_CORS_ALLOWED_HEADERS
+      DEFAULT_CORS_ALLOWED_HEADERS,
     );
   }
-  conditionallyReturnMandatoryEnvVariable<T>(envName: string): T {
-    const value = this.configService.get<T>(envName);
-    if (!value) {
-      throw new EnvironmentVariableMissingError(envName);
-    }
-    return value;
-  }
-  checkMandatoryConfig(): void {
-    const mandatoryVariables = [
-      'HOST',
-      'PORT',
-      'REPUTATION_ORACLE_URL',
-      'REDIS_PORT',
-      'REDIS_HOST',
-      'RPC_URL',
-    ];
-    const missingVariables: string[] = [];
-
-    mandatoryVariables.forEach((variable) => {
-      if (!this.configService.get(variable)) {
-        missingVariables.push(variable);
-      }
-    });
-
-    if (missingVariables.length > 0) {
-      throw new EnvironmentVariableMissingError(missingVariables.join(', '));
-    }
-  }
 }

packages/apps/human-app/server/src/common/interfaces/pageable.interface.ts → packages/apps/human-app/server/src/common/utils/pageable-classes.ts

@@ -1,10 +1,10 @@
 import { ApiPropertyOptional } from '@nestjs/swagger';
 import { Type } from 'class-transformer';
 import { IsEnum, IsNumber, IsOptional, Max, Min } from 'class-validator';
-import { SortOrder } from '../enums/global-common.interface';
+import { SortOrder } from '../enums/global-common';
 import { AutoMap } from '@automapper/classes';
 
-export abstract class PageableDto {
+export abstract class PageableClasses {
   @AutoMap()
   @ApiPropertyOptional({
     minimum: 0,

packages/apps/human-app/server/src/integrations/exchange-oracle/exchange-oracle.gateway.ts

@@ -71,13 +71,15 @@ export class ExchangeOracleGateway {
     const options: AxiosRequestConfig = {
       method: HttpMethod.GET,
       url: `${details.exchangeOracleUrl}/stats`,
+      headers: {
+        Authorization: details.token,
+      },
     };
     return this.callExternalHttpUtilRequest<OracleStatisticsResponse>(options);
   }
   async fetchAssignedJobs(
     details: JobsFetchParamsDetails,
   ): Promise<JobsFetchResponse> {
-    console.log("Details: ", details);
     const jobFetchParamsData = this.mapper.map(
       details.data,
       JobsFetchParams,

packages/apps/human-app/server/src/integrations/exchange-oracle/spec/exchange-oracle.gateway.spec.ts

@@ -3,7 +3,7 @@ import { HttpService } from '@nestjs/axios';
 import { ExchangeOracleGateway } from '../exchange-oracle.gateway';
 import {
   oracleStatsDetailsFixture,
-  statisticsExchangeOracleAddress, statisticsExchangeOracleUrl,
+  statisticsExchangeOracleUrl,
   userStatsDetailsFixture,
 } from '../../../modules/statistics/spec/statistics.fixtures';
 import { AutomapperModule } from '@automapper/nestjs';
@@ -92,7 +92,9 @@ describe('ExchangeOracleApiGateway', () => {
   describe('fetchOracleStatistics', () => {
     it('should successfully call the requested url for oracle statistics', async () => {
       const details = oracleStatsDetailsFixture;
-      nock(statisticsExchangeOracleUrl).get('/stats').reply(200);
+      nock(statisticsExchangeOracleUrl).get('/stats')
+        .matchHeader('Authorization', `Bearer ${details.token}`)
+        .reply(200);
       await gateway.fetchOracleStatistics(details);
       expect(httpService.request).toHaveBeenCalledWith(
         expect.objectContaining({

packages/apps/human-app/server/src/integrations/kv-store/kv-store-gateway.service.ts

@@ -1,11 +1,11 @@
-import { Injectable, OnModuleInit } from '@nestjs/common';
+import { Injectable } from '@nestjs/common';
 import { EnvironmentConfigService } from '../../common/config/environment-config.service';
 import { ethers } from 'ethers';
-import { KVStoreClient } from '@human-protocol/sdk';
+import { KVStoreClient, KVStoreKeys } from '@human-protocol/sdk';
 
 @Injectable()
 export class KvStoreGateway {
-  private URL_KEY = 'url';
+  private URL_KEY = KVStoreKeys.url;
   private kvStoreClient: KVStoreClient;
   constructor(private environmentConfig: EnvironmentConfigService) {}
   async onModuleInit(): Promise<void> {

packages/apps/human-app/server/src/integrations/kv-store/spec/kv-store.gateway.spec.ts

@@ -1,5 +1,5 @@
 import { Test, TestingModule } from '@nestjs/testing';
-import { KVStoreClient } from '@human-protocol/sdk';
+import { KVStoreClient, KVStoreKeys } from '@human-protocol/sdk';
 import { KvStoreGateway } from '../kv-store-gateway.service';
 import { EnvironmentConfigService } from '../../../common/config/environment-config.service';
 import { ethers } from 'ethers';
@@ -12,6 +12,9 @@ jest.mock('@human-protocol/sdk', () => ({
       }),
     ),
   },
+  KVStoreKeys: {
+    url: 'url',
+  },
 }));
 
 describe('KvStoreGateway', () => {
@@ -75,6 +78,11 @@ describe('KvStoreGateway', () => {
         'url',
       );
 
+      expect(service['kvStoreClient'].get).toHaveBeenCalledWith(
+        testAddress,
+        KVStoreKeys.url,
+      );
+
       expect(result).toBe(expectedUrl);
     });
   });

packages/apps/human-app/server/src/main.ts

@@ -12,7 +12,6 @@ async function bootstrap() {
 
   const configService: ConfigService = app.get(ConfigService);
   const envConfigService = new EnvironmentConfigService(configService);
-  envConfigService.checkMandatoryConfig();
   if (envConfigService.isCorsEnabled) {
     app.enableCors({
       origin: envConfigService.corsEnabledOrigin,

packages/apps/human-app/server/src/modules/job-assignment/job-assignment.controller.ts

@@ -2,7 +2,6 @@ import {
   Body,
   Controller,
   Get,
-  Headers,
   Post,
   Query,
   UsePipes,

packages/apps/human-app/server/src/modules/job-assignment/job-assignment.service.ts

@@ -23,7 +23,9 @@ export class JobAssignmentService {
     command: JobAssignmentCommand,
   ): Promise<JobAssignmentResponse> {
     const exchangeOracleUrl =
-      await this.kvStoreGateway.getExchangeOracleUrlByAddress(command.address);
+      await this.kvStoreGateway.getExchangeOracleUrlByAddress(
+        command.data.escrowAddress,
+      );
     const details = this.mapper.map(
       command,
       JobAssignmentCommand,

packages/apps/human-app/server/src/modules/job-assignment/model/job-assignment.model.ts

@@ -3,20 +3,16 @@ import { AutoMap } from '@automapper/classes';
 import { IsEnum, IsNumber, IsOptional, IsString } from 'class-validator';
 import {
   PageableData,
-  PageableDto,
+  PageableClasses,
   PageableParams,
-} from '../../../common/interfaces/pageable.interface';
+} from '../../../common/utils/pageable-classes';
 import {
   AssignmentSortField,
   AssignmentStatus,
-} from '../../../common/enums/global-common.interface';
+} from '../../../common/enums/global-common';
 import { Type } from 'class-transformer';
 
 export class JobAssignmentDto {
-  @AutoMap()
-  @IsString()
-  @ApiProperty()
-  address: string;
   @AutoMap()
   @IsString()
   @ApiProperty()
@@ -39,8 +35,6 @@ export class JobAssignmentCommand {
   data: JobAssignmentParams;
   @AutoMap()
   token: string;
-  @AutoMap()
-  address: string;
 }
 
 export class JobAssignmentDetails {
@@ -71,7 +65,7 @@ export class JobAssignmentResponse {
   expires_at: string;
 }
 
-export class JobsFetchParamsDto extends PageableDto {
+export class JobsFetchParamsDto extends PageableClasses {
   @AutoMap()
   @ApiProperty()
   address: string;

packages/apps/human-app/server/src/modules/job-assignment/spec/job-assignment.fixtures.ts

@@ -17,7 +17,7 @@ import {
   AssignmentSortField,
   AssignmentStatus,
   SortOrder,
-} from '../../../common/enums/global-common.interface';
+} from '../../../common/enums/global-common';
 const EXCHANGE_ORACLE_URL = 'https://www.example.com/api';
 const EXCHANGE_ORACLE_ADDRESS = '0x34df642';
 const ESCROW_ADDRESS = 'test_address';
@@ -40,7 +40,6 @@ const TOKEN = 'test_user_token';
 export const jobAssignmentToken = TOKEN;
 export const jobAssignmentOracleUrl = EXCHANGE_ORACLE_URL;
 export const jobAssignmentDtoFixture: JobAssignmentDto = {
-  address: EXCHANGE_ORACLE_ADDRESS,
   escrow_address: ESCROW_ADDRESS,
   chain_id: CHAIN_ID,
 };
@@ -52,7 +51,6 @@ const jobAssignmentParams: JobAssignmentParams = {
 export const jobAssignmentCommandFixture: JobAssignmentCommand = {
   data: jobAssignmentParams,
   token: TOKEN,
-  address: EXCHANGE_ORACLE_ADDRESS,
 };
 export const jobAssignmentDetailsFixture: JobAssignmentDetails = {
   data: jobAssignmentParams,

packages/apps/human-app/server/src/modules/job-assignment/spec/job-assignment.service.spec.ts

@@ -65,7 +65,7 @@ describe('JobAssignmentService', () => {
 
       expect(
         kvStoreGatewayMock.getExchangeOracleUrlByAddress,
-      ).toHaveBeenCalledWith(command.address);
+      ).toHaveBeenCalledWith(command.data.escrowAddress);
       expect(
         exchangeOracleGatewayMock.postNewJobAssignment,
       ).toHaveBeenCalledWith(details);

packages/apps/human-app/server/src/modules/jobs-discovery/model/jobs-discovery.model.ts

@@ -6,14 +6,14 @@ import {
   JobDiscoveryFieldName,
   JobDiscoverySortField,
   JobStatus,
-} from '../../../common/enums/global-common.interface';
+} from '../../../common/enums/global-common';
 import {
   PageableData,
-  PageableDto,
+  PageableClasses,
   PageableParams,
-} from '../../../common/interfaces/pageable.interface';
+} from '../../../common/utils/pageable-classes';
 
-export class JobsDiscoveryParamsDto extends PageableDto {
+export class JobsDiscoveryParamsDto extends PageableClasses {
   @AutoMap()
   @IsString()
   @ApiProperty()

packages/apps/human-app/server/src/modules/jobs-discovery/spec/jobs-discovery.fixtures.ts

@@ -10,7 +10,7 @@ import {
   JobDiscoveryFieldName,
   JobDiscoverySortField, JobStatus,
   SortOrder,
-} from '../../../common/enums/global-common.interface';
+} from '../../../common/enums/global-common';
 const EXCHANGE_ORACLE_URL = 'https://www.test_url.org';
 const ESCROW_ADDRESS = 'test_address';
 const CHAIN_ID = 1;