Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes after code review (2024-05-07) #75

Merged
merged 31 commits into from
May 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
9562495
File `global-common.interface.ts` has been renamed to `global-common.…
MWBlocky May 7, 2024
e3c3880
Renamed and moved EnvironmentVariableMissingError class.
MWBlocky May 7, 2024
37bfb3c
Renamed and moved Pageable classes to utils directory.
MWBlocky May 7, 2024
bb234c6
Remove unused import in exchange-oracle.gateway.spec.ts
MWBlocky May 7, 2024
e9e3f44
Remove console.log from fetchAssignedJobs method.
MWBlocky May 7, 2024
4fd1d68
Update dependencies in package.json.
MWBlocky May 7, 2024
e59e945
Update terminology in README.md
MWBlocky May 7, 2024
709539d
Remove unused Headers import from job-assignment.controller.ts
MWBlocky May 7, 2024
320959a
Removed OnModuleInit import from kv-store-gateway.service.ts
MWBlocky May 7, 2024
a1ba473
Corrected the data types for REDIS_PORT and REDIS_HOST in .env.exampl…
MWBlocky May 7, 2024
18ce38a
Remove redundant volume configuration in Docker-compose
MWBlocky May 7, 2024
8dce76f
Add RPC_URL environment variable to docker-compose
MWBlocky May 7, 2024
e92e428
Replace hardcoded URL_KEY in KvStoreGateway with imported constant.
MWBlocky May 7, 2024
3dfe042
fix: Update KVStoreClient import and test in KvStoreGateway
MWBlocky May 7, 2024
cd1a23a
Add Reputation Oracle address to .env and refactor Oracle Discovery.
MWBlocky May 7, 2024
56ffd41
Add token authentication for Oracle statistics.
MWBlocky May 7, 2024
dc826c0
Remove manual environment variable check.
MWBlocky May 8, 2024
5220e10
Refactor address to use escrowAddress in job assignment service.
MWBlocky May 8, 2024
8cbdb5d
Ensure environment variables validation takes place before module loa…
MWBlocky May 9, 2024
1ed59b1
Revert "Add token authentication for Oracle statistics."
MWBlocky May 9, 2024
fa99045
Update terminology from 'Interfaces' to 'Model' in README
MWBlocky May 9, 2024
dd675f8
Refactor PageableClasses to PageableDto and the file renamed from pag…
MWBlocky May 9, 2024
c851c98
Update KV store gateway test to use actual SDK.
MWBlocky May 9, 2024
a2c1f15
Added REPUTATION_ORACLE_ADDRESS in app configuration and docker compo…
MWBlocky May 9, 2024
1406153
Integrate and use EscrowUtils for job assignment processing.
MWBlocky May 9, 2024
8d73ca3
Added chainId parameter to `getExchangeOracleAddressByEscrowAddress` …
MWBlocky May 9, 2024
2732020
Remove redundant code KV-store gateway test
MWBlocky May 9, 2024
1274ff3
Update EscrowUtilsGateway tests.
MWBlocky May 9, 2024
a2134ad
Add mock ConfigService to oracle-discovery service test.
MWBlocky May 9, 2024
283abf9
Revert "Add mock ConfigService to oracle-discovery service test."
MWBlocky May 9, 2024
8ec5fe1
Update oracle-discovery.service.spec.ts tests.
MWBlocky May 9, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions packages/apps/human-app/server/.env.example
Original file line number Diff line number Diff line change
@@ -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
Expand Down
2 changes: 1 addition & 1 deletion packages/apps/human-app/server/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ and transformation of data. NestJS provides many built-in pipes, so check the do

### Model
MWBlocky marked this conversation as resolved.
Show resolved Hide resolved

Interfaces are used to define the shape and responsibilities of the data:
Models are used to define the shape and responsibilities of the data:

- **Dto (Data Transfer Object)**: Data sent from/to the frontend.
- **Command**: Datatype used for data manipulation in business logic.
Expand Down
6 changes: 3 additions & 3 deletions packages/apps/human-app/server/docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,11 @@ services:
HOST: ${HOST}
PORT: ${PORT}
REPUTATION_ORACLE_URL: ${REPUTATION_ORACLE_URL}
REPUTATION_ORACLE_ADDRESS: ${REPUTATION_ORACLE_ADDRESS}
REDIS_PORT: ${REDIS_PORT}
MWBlocky marked this conversation as resolved.
Show resolved Hide resolved
REDIS_HOST: redis
CACHE_TTL_ORACLE_DISCOVERY: ${CACHE_TTL_ORACLE_DISCOVERY}
RPC_URL: ${RPC_URL}
depends_on:
- redis
redis:
Expand All @@ -27,6 +29,4 @@ services:
ports:
- '${REDIS_PORT}:6379'
volumes:
- redis_data:/data
volumes:
redis_data:
- redis_data:/data
5 changes: 2 additions & 3 deletions packages/apps/human-app/server/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand Down
12 changes: 12 additions & 0 deletions packages/apps/human-app/server/src/app.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,23 @@ import { StatisticsModule } from './modules/statistics/statistics.module';
import { StatisticsController } from './modules/statistics/statistics.controller';
import { ExchangeOracleModule } from './integrations/exchange-oracle/exchange-oracle.module';
import { KvStoreModule } from './integrations/kv-store/kv-store.module';
import { EscrowUtilsModule } from './integrations/escrow/escrow-utils.module';
import Joi from 'joi';

@Module({
imports: [
ConfigModule.forRoot({
envFilePath: '.env',
isGlobal: true,
validationSchema: Joi.object({
HOST: Joi.string().required(),
PORT: Joi.number().required(),
REPUTATION_ORACLE_URL: Joi.string().required(),
MWBlocky marked this conversation as resolved.
Show resolved Hide resolved
REPUTATION_ORACLE_ADDRESS: Joi.string().required(),
REDIS_PORT: Joi.number().required(),
REDIS_HOST: Joi.string().required(),
RPC_URL: Joi.string().required(),
}),
}),
AutomapperModule.forRoot({
strategyInitializer: classes(),
Expand All @@ -44,6 +55,7 @@ import { KvStoreModule } from './integrations/kv-store/kv-store.module';
OracleDiscoveryModule,
StatisticsModule,
KvStoreModule,
EscrowUtilsModule,
],
controllers: [
AppController,
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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>(
Expand All @@ -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);
Expand All @@ -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 {
MWBlocky marked this conversation as resolved.
Show resolved Hide resolved
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(', '));
}
}
}

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
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 {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import { Injectable, NotFoundException } from '@nestjs/common';
import { ChainId, EscrowUtils } from '@human-protocol/sdk';

@Injectable()
export class EscrowUtilsGateway {
async getExchangeOracleAddressByEscrowAddress(
chainId: ChainId,
address: string,
): Promise<string> {
const escrowsData = await EscrowUtils.getEscrow(chainId, address);
if (!escrowsData.exchangeOracle) {
throw new NotFoundException('Exchange Oracle not found');
}
return escrowsData.exchangeOracle;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { Module } from '@nestjs/common';
import { EscrowUtilsGateway } from './escrow-utils-gateway.service';

@Module({
providers: [EscrowUtilsGateway],
exports: [EscrowUtilsGateway],
})
export class EscrowUtilsModule {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
import { Test, TestingModule } from '@nestjs/testing';
import { EscrowUtilsGateway } from '../escrow-utils-gateway.service';
import { EscrowUtils, ChainId } from '@human-protocol/sdk';
import { NotFoundException } from '@nestjs/common';

jest.mock('@human-protocol/sdk', () => {
return {
EscrowUtils: {
getEscrow: jest.fn().mockResolvedValue({
exchangeOracle: '0x',
}),
},
ChainId: {
POLYGON_AMOY: 1,
},
};
});

describe('EscrowUtilsGateway', () => {
let service: EscrowUtilsGateway;

beforeEach(async () => {
const module: TestingModule = await Test.createTestingModule({
providers: [EscrowUtilsGateway],
}).compile();

service = module.get<EscrowUtilsGateway>(EscrowUtilsGateway);
});

afterEach(async () => {
jest.restoreAllMocks();
});

it('should be defined', () => {
expect(service).toBeDefined();
});

describe('getExchangeOracleAddressByEscrowAddress', () => {
it('should fetch data from EscrowUtils', async () => {
const escrowAddress = 'escrowAddress';
const expectedUrl = '0x';

const result = await service.getExchangeOracleAddressByEscrowAddress(
ChainId.POLYGON_AMOY,
escrowAddress,
);
expect(EscrowUtils.getEscrow).toHaveBeenCalledWith(
ChainId.POLYGON_AMOY,
escrowAddress,
);
expect(result).toBe(expectedUrl);
});

it('should throw error if Exchange Oracle not found', async () => {
const escrowAddress = 'escrowAddress';

(EscrowUtils.getEscrow as jest.Mock).mockResolvedValue({});

await expect(
service.getExchangeOracleAddressByEscrowAddress(
ChainId.POLYGON_AMOY,
escrowAddress,
),
).rejects.toThrow(new NotFoundException('Exchange Oracle not found'));
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,6 @@ export class ExchangeOracleGateway {
async fetchAssignedJobs(
details: JobsFetchParamsDetails,
): Promise<JobsFetchResponse> {
console.log("Details: ", details);
const jobFetchParamsData = this.mapper.map(
details.data,
JobsFetchParams,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
Original file line number Diff line number Diff line change
@@ -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> {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,18 +1,22 @@
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';

jest.mock('@human-protocol/sdk', () => ({
KVStoreClient: {
build: jest.fn().mockImplementation(() =>
Promise.resolve({
get: jest.fn().mockResolvedValue('https://example.com'),
}),
),
},
}));
jest.mock('@human-protocol/sdk', () => {
const actualSdk = jest.requireActual('@human-protocol/sdk');
return {
...actualSdk,
KVStoreClient: {
build: jest.fn().mockImplementation(() =>
Promise.resolve({
get: jest.fn().mockResolvedValue('https://example.com'),
}),
),
},
};
});

describe('KvStoreGateway', () => {
let service: KvStoreGateway;
Expand Down Expand Up @@ -72,7 +76,7 @@ describe('KvStoreGateway', () => {

expect(service['kvStoreClient'].get).toHaveBeenCalledWith(
testAddress,
'url',
KVStoreKeys.url,
);

expect(result).toBe(expectedUrl);
Expand Down
1 change: 0 additions & 1 deletion packages/apps/human-app/server/src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import {
Body,
Controller,
Get,
Headers,
Post,
Query,
UsePipes,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,10 @@ import { JobAssignmentProfile } from './job-assignment.mapper';
import { Module } from '@nestjs/common';
import { ExchangeOracleModule } from '../../integrations/exchange-oracle/exchange-oracle.module';
import { KvStoreModule } from '../../integrations/kv-store/kv-store.module';
import { EscrowUtilsModule } from '../../integrations/escrow/escrow-utils.module';

@Module({
imports: [ExchangeOracleModule, KvStoreModule],
imports: [ExchangeOracleModule, KvStoreModule, EscrowUtilsModule],
providers: [JobAssignmentService, JobAssignmentProfile],
exports: [JobAssignmentService],
})
Expand Down
Loading
Loading